Give the gate more than one message shape to judge (#223) - #252
Merged
Merged
Conversation
kurok
force-pushed
the
feat/223-gate-coverage
branch
from
September 17, 2026 12:07
3b75d4e to
0462f2b
Compare
Every gated benchmark measured the same message: large_message.eml, 767 KiB and 99% base64 attachment. So the gate judged the decode path and nothing else, and a change that doubled the per-call floor or halved header handling would have passed it without comment. That is not hypothetical. #238's header work moved the small serial batch 23% while the gate's own parse_message moved 2% -- the thing it changed was nearly invisible to the thing that judges changes. Three gated benchmarks for the shapes it could not see: parse_small the per-call floor on ~0.8 KB: FFI, header map, address and date parse parse_many_small_serial the same cost x2000, serial, in milliseconds -- the form to prefer if the single call is too noisy on the runner parse_rfc2047_headers ~30 KB of headers with encoded words throughout; the tokenizer runs on ~250 headers and decodes on ~30 of them Each asserts correctness once outside the timed call -- subject, header count, and warnings == [] -- so none of them can be quietly timing a repair instead of a parse. The RFC 2047 input is built in the module rather than committed under tests/data/. Every .eml there is auto-enrolled in eight correctness suites: the RFC corpus wants a CASES entry, packaging invariants want a BUILDERS entry, and the parity, lazy, metadata, tree, parse_many and warning suites all glob the directories. A benchmark input has no business being an oracle in eight places. _small_message() is the precedent. Also removed test__threaded___parse_many_small_threads1, added by #238: it is byte-for-byte the same measurement as parse_many_small_serial above, and the gated form supersedes it -- same numbers, judged rather than merely printed. Keeping both would run 3.5 ms twice per side on every round for nothing. Its metadata twin stays, because metadata mode has no gated benchmark on this batch to be read against. Quoted-printable coverage, items 3 and 4 of the issue, arrived with #229 as parse_qp_message and parse_qp_message_metadata; not duplicated here. Test-only: the extension is byte-identical. Signed-off-by: kurok <22548029+kurok@users.noreply.github.com>
kurok
force-pushed
the
feat/223-gate-coverage
branch
from
September 17, 2026 13:03
0462f2b to
22724cc
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #223.
Branched from
master(5027ca5). Test-only — the extension is byte-identical.Why
Every gated benchmark measured the same message:
large_message.eml, 767 KiB and 99% base64 attachment. So the gate judged the decode path and nothing else.That gap is measurable, not theoretical. #238's header work moved the small serial batch 23% while the gate's own
parse_messagemoved 2%. The thing it changed was nearly invisible to the thing that judges changes.What
parse_smallparse_many_small_serialparse_rfc2047_headersLocal values (M4): 1.667 µs, 3.550 ms, 60.9 µs.
Each asserts correctness once outside the timed call — subject, header count, and
warnings == []— so none of them can be quietly timing a repair instead of a parse.The RFC 2047 input is built in the module, not committed to
tests/data/. Every.emlthere is auto-enrolled in eight correctness suites: the RFC corpus wants aCASESentry, packaging invariants want aBUILDERSentry, and the parity, lazy, metadata, tree,parse_manyand warning suites all glob the directories. A benchmark input has no business being an oracle in eight places._small_message()is the precedent.One removal worth calling out
I deleted
test__threaded___parse_many_small_threads1, which I added myself in #238. It is byte-for-byte the same measurement asparse_many_small_serial— same batch, samethreads=1call — and the gated form supersedes it: same numbers, judged rather than merely printed. Keeping both would run 3.5 ms twice per side on every round for nothing.Its metadata twin stays, since metadata mode has no gated benchmark on this batch to be read against; I added a note there pointing at the gated sibling.
Scope note
Items 3 and 4 of the issue (quoted-printable, full and metadata) already landed with #229 as
parse_qp_messageandparse_qp_message_metadata. Not duplicated.A tension worth stating
This PR widens the judged set at a moment when the gate is demonstrably unreliable: rebasing two PRs with zero code changes recently flipped one from fail to pass and moved the other's failing benchmark by 25 points, against a 0.5% control floor (#240 / #249). Three more gated benchmarks are three more places a placement artifact can trip a PR that changed nothing relevant.
I still think this is right, and the two are complementary rather than opposed: the gate's problem is that its verdicts have a scatter nobody has measured, not that it judges too much. Narrow coverage does not make the scatter smaller — it just means the scatter lands on one benchmark. #249 measures the scatter; this makes the coverage honest. But if you would rather land #249 and its dispatches first and take this after, that is a reasonable order and I would not argue with it.
Checks run locally
pytest tests --ignore=tests/benchmark(827 passed, 3 skipped), the full benchmark suite (31 passed),ruff check .. Verified withab_median.pythat all three new names register as treatment (noinformationaltag) and withcheck_benchmark.pythat the gate pair still resolves by exact name. The gate pair bodies, the three*___full_readtable rows andbench_table.pyROWSare untouched.