:sparkles: feat(fixtures): specify the drop-reason taxonomy and its boundary by jasonwc · Pull Request #296 · papercomputeco/tapes

jasonwc · 2026-08-06T23:32:50Z

Stacked on #295 — review that first; this branch contains its commits.

One empty-body rule for every content-coding

#295 documented, rather than hid, an inconsistency: an empty gzip body errored while an
empty zstd body returned success with zero bytes. Identical input, opposite outcomes,
decided only by which decoder the header named. An empty body under a claimed coding means
something went wrong — a body lost in flight, a truncated read — and one of the two codings
was swallowing it.

refuseEmpty now runs in both the gzip and zstd branches. It sits inside each supported
branch rather than once above the switch, deliberately: an unsupported coding must stay
unsupported whatever the bytes are, since nothing is read and the body cannot change the
answer. A guard above the switch would report an unknown coding plus an empty body as
undecodable, collapsing two failure classes that are kept apart on purpose.

One downstream consequence worth stating: a POST to a turn path carrying a content-encoding
header and no body now drops as request_decode rather than dispatching an empty request.
That is the outcome gzip already produced for the same input. Health probes are unaffected —
the non-turn-request check runs first.

Specify the drop-reason taxonomy

The ext_proc adapter carries a vocabulary of drop reasons that existed in Go only and was
written down nowhere. A second implementation had no way to know which of them it was
supposed to reproduce.

They split 7 / 7, by a sharper test than intuition: could a conformant implementation with
different plumbing produce this reason at all?

Contract — upstream_status, non_turn_request, request_decode, empty_response,
unknown_provider, response_decode, reducer_error.
Transport — client_disconnect, upstream_no_response, missing_status, sem_full,
ingest_reject, ingest_timeout, marshal_error.

The close calls are argued in their case files. Two worth surfacing:

empty_response and upstream_no_response both mean "no response bytes" and they
split. A response phase that completed carrying nothing is a property of the exchange; a
stream torn down before its response phase is a property of the connection, and only a
streaming interception path can tell them apart.
missing_status reads like contract and is classified transport. It is reachable
only because ext_proc can deliver a body before its headers; a path that reads a status
off a response object always has one, so it can neither conform nor diverge.

The contract half now lives in pkg/capture, and the adapter's constants are conversions
of it, so this build cannot change a wire-visible label on its own. Conformance is gated in
both directions: a reason in code but not the corpus fails, and so does the reverse.

Two things recorded rather than changed

DefaultLargeTurnThreshold is not contract. It labels one metric; two builds that
disagree about it capture, dispatch and store byte-identical turns and differ only in a
sizing dashboard. A threshold that dropped or truncated a turn would be contract.

The client currently has no upstream-status gate and no turn-path gate, so a failed
exchange, or a GET carrying a JSON body, becomes a stored turn there and nothing here.
Both are recorded as known divergences. Neither would have been found by capturing
sessions and comparing — they produce extra rows, not missing ones.

related to PCC-1126

linear-code · 2026-08-06T23:32:53Z

PCC-1126

greptile-apps · 2026-08-06T23:36:02Z

Greptile Summary

This PR standardizes empty-body handling across supported content codings and introduces a shared drop-reason taxonomy distinguishing capture policy from adapter-specific transport failures.

Rejects empty gzip and zstd bodies consistently while preserving unsupported-coding precedence.
Defines shared capture-policy drop reasons in pkg/capture and maps extproc labels to that vocabulary.
Adds sealed fixture corpora and conformance tests for content-encoding behavior and drop-reason classification.
Corrects the decoder documentation to describe the standalone and extproc caller behavior accurately.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
pkg/capture/contentencoding.go	Adds a uniform empty-body rejection for supported non-identity codings and accurately documents extproc’s caller and gate ordering.
extproc/dispatcher.go	Sources capture-policy drop labels from the shared package while retaining adapter-specific transport reasons locally.
extproc/processor.go	Extracts the existing method-and-path capture gate into a corpus-testable predicate without changing its behavior.
extproc/dropreason_corpus_test.go	Adds bidirectional vocabulary, classification, digest, and executable-example conformance gates.
pkg/capture/dropreason.go	Establishes the shared capture-policy half of the drop-reason vocabulary.
fixtures/drop-reason/README.md	Documents the policy-versus-transport boundary, precedence, schema, and known cross-path divergences.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Request reaches extproc EOS] --> B[Decode claimed content coding]
  B --> C{Capturable POST turn path?}
  C -- No --> D[Drop: non_turn_request]
  C -- Yes --> E{Request decode failed?}
  E -- Yes --> F[Drop: request_decode]
  E -- No --> G[Continue response and reducer gates]

_{Reviews (2): Last reviewed commit: "🧹 chore(capture): say what the gateway ..." | Re-trigger Greptile}

cloudflare-workers-and-pages · 2026-08-07T00:09:51Z

Deploying docs-tapes-dev with Cloudflare Pages

Latest commit:	`e8ea566`
Status:	✅ Deploy successful!
Preview URL:	https://5f60088a.tapes-dev.pages.dev
Branch Preview URL:	https://jason-capture-policy-taxonom.tapes-dev.pages.dev

View logs

jasonwc · 2026-08-07T00:10:49Z

@greptileai review

The caller-precondition comment is corrected. Verified your reading is right: the gateway adapter calls the decode unconditionally at request end-of-stream with no check that bytes arrived, so the claim that both paths return early for bodiless requests was false.

The comment now says which path returns early and which does not, notes that ordinary bodiless traffic is filtered by the turn-request gate that runs before the decode error is consulted, and states plainly what is left over and intended: a POST to a turn path claiming a coding with no body terminates as a request_decode drop. Documentation only — no behavior change.

Five lint findings this branch introduced are also fixed (a goconst from the corpus test spelling two strings once per example, a gofumpt wrap, and three hand-rolled searches replaced with slices calls).

…corpus The envelope and thread corpora make *header* contracts executable. This does the same one layer in, for a *policy*: which content-codings a captured body may use, how stacked layers compose, how much output is allowed, and what a corrupt or half-arrived stream is worth. That policy is implemented independently in Go (pkg/capture/contentencoding.go, the reference) and in Rust (tapesctl's client decoder). Their agreement was a point-in-time claim by whoever last read both files, and it had already decayed once: the client dropped every zstd request body — all of Codex/pi's traffic — while the gateway route decoded the same bytes fine, with nothing red. Bodies are recipes (a plaintext, the layers to apply, an optional truncation) rather than committed bytes. Compressed output is not stable across implementations, so pinning it would assert compressor identity instead of the policy, and would make review a diff of base64 blobs. Byte-exact base64 is kept only where the bytes are the assertion: two hand-built zstd frames differing solely in their window descriptor, two deliberately corrupt frames, and the empty body. Three cases record a decision rather than a settled rule, in a `contested` block that travels with the case into every vendored copy: - the zstd bound is asserted as the window only. Go also passes a max-memory bound and the Rust binding cannot; with the window already at the output cap, no constructible frame distinguishes them, so a case for max-memory would be asserting a language binding's API rather than the policy. - a bodiless request whose headers still claim an encoding errors at the decoder, and must never reach it. Not-calling-the-decoder is recorded as a caller precondition, which a proxy that returns early satisfies. - the same empty body under zstd currently succeeds with zero bytes. That is recorded as observed and flagged as a suspected bug rather than promoted to a rule — encoding the intended answer would make the case fail on the very implementation it describes. The failure taxonomy is three classes because both implementations already distinguish exactly three; corrupt-body and window-refused stay merged under `undecodable`, with the difference in unasserted prose, rather than inventing a distinction neither side carries. This is not extproc's DropResponseDecode metrics label, which still collapses all three and is untouched. related to PCC-1126

… produced output The case that pins the second half of the salvage rule — a stream that ended early AND produced nothing is refused, not salvaged — truncated its zstd body at half its encoded length. Whether a complete block lands inside that half is a property of the compressor, not of the decoder: its block splitting and its level decide it. A conformant decoder handed another compressor's stream could legitimately produce output there, correctly report a salvage, and fail a corpus whose whole premise is that it asserts policy rather than compressor identity. The cut is now an absolute prefix derived from the container format instead: 9 bytes, being the 4-byte magic plus the smallest frame header the format allows plus a block header. Byte 10 is the earliest position at which any encoder's first byte of block content can appear, and a block that has delivered no content can have produced no output whatever its type — so "nothing was producible" is true for every encoder rather than for the one that built the fixture. A larger frame header only pushes that byte further out. The error is unchanged, including its unasserted detail, because an unfinished frame reports the same early end wherever it was cut. That needs a truncation form the schema did not have. drop_tail_bytes fixes what is missing and keep_head_ratio fixes a fraction; neither can express a cut point that comes from the format rather than from one encoder's output length, so keep_head_bytes is added beside them and the three are documented by what each holds fixed across compressors. The coverage gate is rewritten in the same pass, for a failure of the same shape. It asserted properties over the whole corpus — "some case decodes gzip" — which any neighbour sharing the shape satisfies: the alias case, a cap case, a stacked case. Deleting the dedicated case therefore left the gate green while the rule it pinned was pinned nowhere, and the seal does not catch that either, since a digest recomputed from the surviving cases makes delete-and-reseal a legal two-line diff. Each rule now names the case that pins it, and the predicate runs against that case, so a deletion fails and so does gutting the case in place. The table is closed in the other direction too: a case no rule names fails, which keeps it from falling behind the corpus it describes. related to PCC-1126

…st member A gzip stream is a series of members (RFC 1952 2.2) and zstd frames may likewise be concatenated. A streaming compressor that flushes and restarts mid-body emits exactly that shape, so it is ordinary request traffic rather than an edge case. It is worth a case because of how it fails. A decoder that stops at the first member returns *success*, reports no truncation, and hands back a prefix, so the capture is silently short and the loss reappears far downstream as a parse error with nothing pointing at the decoder. Every other way these decoders can lose bytes is loud; this one is not. The two implementations of this policy did not agree on it. Go's compress/gzip reader continues into later members unless Multistream(false) is set; the Rust client's flate2::read::GzDecoder stops at the first, and only MultiGzDecoder continues. Both sides already read every zstd frame, which is pinned here too rather than left implicit: the two codings go through unrelated libraries, so agreement on one is not evidence about the other, and an unpinned rule is one a binding swap can quietly drop. Expressing this needed a `members` count in the build recipe, alongside `truncate`. It splits the *plaintext* rather than the encoded stream, so the member boundary sits at the same logical offset whichever compressor built the fixture and the case can still assert equality with the whole plaintext -- an assertion a split of the encoded bytes could not make, since it would be cutting at an offset only one encoder's output has. related to PCC-1126

DecodeContentEncoding returned opposite outcomes for identical input, decided only by which decoder the header named: an empty body under gzip errored, and the same empty body under zstd succeeded with zero bytes. Neither answer was chosen. Go's gzip reader consumes its header eagerly and klauspost's zstd reader does not, and the difference between two dependencies became the policy. The asymmetry matters because an empty body under a claimed coding is an anomaly — a body lost in flight, a truncated proxy read, a client that set the header and sent nothing. Under gzip that was loud. Under zstd it was indistinguishable from a legitimately empty payload, which is the failure this decoder exists to make loud. The rule is now stated above both decoder libraries: a body with no bytes under any non-identity coding is an error. It is applied inside each supported branch rather than once above the switch, so an unsupported coding stays unsupported whatever the bytes are — nothing is read for a coding with no decoder, so the body cannot change the answer, and the two failure classes the corpus keeps apart do not collapse into one by argument order. This is not a choice between two implementations. The Rust client already errored under both codings, and already gave the answer the corpus's own contested block argued for, so this closes a Go-internal inconsistency rather than moving the line between the two. The corpus case that recorded the old behaviour now asserts the new rule and is renamed away from "divergence", because it no longer is one. Its argument travels with it: what was observed, what the second implementation answered, and why the repair belonged in the reference implementation rather than in either test. The caller precondition it pairs with is untouched and still load-bearing — a bodiless request must never reach the decoder at all, which is what keeps this rule unreachable in ordinary traffic. One consumer changes shape: extproc decodes the request body on every request EOS, so a POST to a turn path carrying a content-encoding header and no body now drops as request_decode instead of dispatching an empty request. That is the same outcome gzip already produced for the same input. related to PCC-1134

The gateway adapter carries a closed enum of fourteen answers to "why was this turn not captured". The standalone client carries none — it logs a sentence per site. Neither was specified anywhere, so nothing had decided the question that actually matters: which of those answers are rules any implementation of tapes capture must share, and which are artefacts of how one deployment moves bytes. Without that line drawn, the two obvious moves are both wrong. Unifying the enum with the client's three decode error classes merges two different axes — those classes are *why a body was unreadable*, not *why a turn was not captured*. Leaving it alone keeps the client free to invent a second vocabulary for the same decisions, which is the drift the fixture corpora exist to kill, one layer up from bytes. So the line is drawn and written down. Seven reasons are capture policy: they say what makes a turn capturable at all — its method and path, its status, its bytes, whether a reducer can read them — and two paths that disagree about any of them record different sessions from identical traffic. Seven are transport and runtime: a full dispatch queue, a disconnected client, an ingest POST that timed out, an envelope that failed to marshal. Each of those names a component another implementation need not have, and promoting them would make one deployment's plumbing everyone's contract. Both halves are specified, which is the part worth arguing for. A corpus of only the shared reasons would leave the other seven exactly as they are today — unclassified, and therefore something the next implementation guesses about. Recording "this one is deliberately not shared" is what makes the boundary reviewable instead of implicit. Three were close calls and the cases carry the argument rather than the verdict. empty_response and upstream_no_response both mean "no response bytes", and only the first is contract: a response phase that completed carrying nothing is a property of the exchange, while a stream torn down before it is a property of the connection. missing_status reads like policy and is not — it exists only because ext_proc can deliver a body before its headers, so no path that reads a status off a response can reach it. marshal_error is a bug rather than a rule. The adapter now reads the policy half instead of declaring it: those constants are conversions of the specified vocabulary, so this build cannot change a policy reason's spelling — a wire-visible metric label — on its own. The transport constants stay exactly where they are, which is where they belong. The gate asserts conformance in both directions, so a reason added to either side alone fails rather than being discovered later. Only non_turn_request is a pure function of request data, so it is the only reason with executable examples; the predicate behind it is now a named function so the cases run against the code the processor runs rather than against a copy. Every other case declares why it is not yet expressible. Expressing more of them means giving each implementation a pure classifier first — a change to the implementations, then to the corpus, never the reverse. Specifying the vocabulary immediately surfaced two live divergences, recorded rather than repaired here: the client captures a turn whatever the upstream status, and has no turn-path or method gate at all. Both are exactly what a specification is for. No observable behaviour changes: same strings, same labels, same outcomes. related to PCC-1126

…less request The empty-body rule's comment claimed that callers on both capture paths keep a bodiless request away from the decoder by returning early. Only one of them does. The gateway adapter decodes the request body on every request end-of-stream with no check that any bytes arrived, so an empty body frame does reach the decoder and does error — and the comment's tidier claim hid the one consequence of this rule a reader most needs: a POST to a turn path that claims a coding and carries no body now terminates as a request_decode drop rather than dispatching an empty request. That outcome is intended. It is the answer gzip already gave for the same input, now given for every coding, and ordinary bodiless traffic never gets that far because the turn-request gate filters it before the decode error is consulted. None of which was written down, so the comment now says all of it. Behaviour is unchanged; this is the documentation catching up to the code. Three lint findings the drop-reason work introduced are cleared in the same pass. The corpus test spelled its two expectation values out once per example, which pushed the package's count of one of them past the repeated-string threshold and reported it against unrelated code in the metrics file; they are named constants now, which also means a typo in one fails rather than quietly weakening an assertion. Two membership loops there and one in the reason vocabulary become slices lookups, and one slice literal is re-wrapped. related to PCC-1126

jasonwc · 2026-08-07T01:42:47Z

The tapes:check-lint red is not this branch.

I ran that check locally through the same Dagger engine version the module pins (v0.21.7 — a checksum-verified release binary, since the engine on this machine was older than the module requires and refuses to load it). It passes:

✔ .checkLint: String!   1m12s
0 issues.

That is the actual check, not an approximation of it. Alongside it, golangci-lint v2.8.0 against the repo's own config reports 0 issues. directly.

Every failing status here reports error rather than failure — what a runner that did not complete looks like, as opposed to a check that ran and found something. With the same check passing locally, the remaining difference is the hosted runner.

# Conflicts: # fixtures/README.md # fixtures/content-encoding/DIGEST # fixtures/content-encoding/README.md # fixtures/content-encoding/cases/contested-empty-body-under-gzip.json # fixtures/manifest.json # pkg/capture/contentencoding_corpus_test.go

greptile-apps Bot reviewed Aug 6, 2026

View reviewed changes

Comment thread pkg/capture/contentencoding.go Outdated

jasonwc force-pushed the jason/capture-policy-taxonomy branch from 493f40c to 10c867e Compare August 7, 2026 00:09

jasonwc force-pushed the jason/capture-policy-taxonomy branch from 10c867e to ba0c84f Compare August 7, 2026 00:33

jasonwc added 6 commits August 6, 2026 18:37

jasonwc force-pushed the jason/capture-policy-fixture-corpus branch from 2c4607c to 35c79be Compare August 7, 2026 01:39

jasonwc force-pushed the jason/capture-policy-taxonomy branch from ba0c84f to f1eda48 Compare August 7, 2026 01:39

jasonwc mentioned this pull request Aug 7, 2026

✨ feat(fixtures): make turn eligibility an executable agreement #298

Merged

jasonwc changed the base branch from jason/capture-policy-fixture-corpus to main August 7, 2026 17:51

jasonwc merged commit b5ff59a into main Aug 7, 2026
25 of 26 checks passed

jasonwc deleted the jason/capture-policy-taxonomy branch August 7, 2026 18:19

Navigation Menu

Uh oh!

✨ feat(fixtures): specify the drop-reason taxonomy and its boundary - #296

✨ feat(fixtures): specify the drop-reason taxonomy and its boundary#296
jasonwc merged 7 commits into
mainfrom
jason/capture-policy-taxonomy

jasonwc commented Aug 6, 2026 •

edited

Loading

Uh oh!

linear-code Bot commented Aug 6, 2026

Uh oh!

greptile-apps Bot commented Aug 6, 2026 •

edited

Loading

Greptile Summary

Important Files Changed

Flowchart

Uh oh!

Uh oh!

cloudflare-workers-and-pages Bot commented Aug 7, 2026 •

edited

Loading

Uh oh!

jasonwc commented Aug 7, 2026

Uh oh!

jasonwc commented Aug 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

jasonwc commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

One empty-body rule for every content-coding

Specify the drop-reason taxonomy

Two things recorded rather than changed

Uh oh!

linear-code Bot commented Aug 6, 2026

Uh oh!

greptile-apps Bot commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Uh oh!

Uh oh!

cloudflare-workers-and-pages Bot commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Deploying docs-tapes-dev with Cloudflare Pages

Uh oh!

jasonwc commented Aug 7, 2026

Uh oh!

jasonwc commented Aug 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

jasonwc commented Aug 6, 2026 •

edited

Loading

greptile-apps Bot commented Aug 6, 2026 •

edited

Loading

cloudflare-workers-and-pages Bot commented Aug 7, 2026 •

edited

Loading