Skip to content

Give the gate more than one message shape to judge (#223) - #252

Merged
kurok merged 1 commit into
masterfrom
feat/223-gate-coverage
Sep 17, 2026
Merged

kurok merged 1 commit into
masterfrom
feat/223-gate-coverage

Conversation

@kurok

@kurok kurok commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

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_message moved 2%. The thing it changed was nearly invisible to the thing that judges changes.

What

benchmark covers
parse_small the per-call floor on ~0.8 KB — FFI, header map, address and date parse
parse_many_small_serial the same cost ×2000, serial; the form to prefer if a single 1.7 µs call proves 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

Local 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 .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.

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 as parse_many_small_serial — same batch, same threads=1 call — 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_message and parse_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 with ab_median.py that all three new names register as treatment (no informational tag) and with check_benchmark.py that the gate pair still resolves by exact name. The gate pair bodies, the three *___full_read table rows and bench_table.py ROWS are untouched.

@kurok
kurok force-pushed the feat/223-gate-coverage branch from 3b75d4e to 0462f2b Compare September 17, 2026 12:07
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
kurok force-pushed the feat/223-gate-coverage branch from 0462f2b to 22724cc Compare September 17, 2026 13:03
@kurok
kurok merged commit 47965f8 into master Sep 17, 2026
15 checks passed
@kurok
kurok deleted the feat/223-gate-coverage branch September 17, 2026 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Benchmark gate only sees one 767 KiB base64 message: add gated small, quoted-printable and RFC 2047-heavy benchmarks

1 participant