test(tools): gate the logprob gap check on the engine preamble - #1357
Conversation
|
Authored by Claude Fable 5.1 in Claude Code, analysis in partnership with @monotophic Merged current @JustVugg similar steering request to #1102, #1353, #1355, and #1356, this PR is part of the set to make the full checkpoint-quality GLM 5.2/5.3 containers work and to establish the OpenAI API instrumentation needed for container quality studies. Let me know if this PR is off target or otherwise not likely to merge. Thanks again! |
|
Reviewed, and I am asking for a split rather than a change. The
That is 62% of this PR. It is the same pattern you identified yourself in #1353 and offered to drop there, and I would like to be consistent about it: send the witness alongside the engine change that emits the record it needs, and the logprob-gap hardening as its own small PR now. One question about the half that stays: neither transcript tool is referenced by |
Add tools/engine_evidence.py, a shared parser for the two fixed-format lines the C engine prints at startup: the banner line (backend, cache size, compute/idot precision) and the loaded line (load time, resident memory, layer/expert counts, MTP state). parse_engine_preamble splits a captured stream into the two and hands each to its own exact-text parser, raising PreambleError on anything that does not match the documented grammar rather than guessing at a partial parse. Every numeric field is range-checked on both sides of its bound, and the two parsers only ever accept the exact banner/loaded text the engine actually emits -- no fuzzy matching, no optional fields treated as present. Downstream tools that need to bind evidence to a specific engine run share this module instead of re-deriving the grammar. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…mble and widen the numeric-tail check
check_data_logprob_gaps.py's original speculative-decoding gap was
already closed on today's engine (an accepted draft token is emitted
through the same mux_data() call as any other generated token), so
retarget the module as regression coverage for that invariant and
harden the rest of the check well past the original narrow gap hunt.
capture_mode() now names and enforces the transcript's shape explicitly
(full-process / ready-suffix / request-only): a transcript whose
leading line is not a recognized global preamble record or a
legitimate request-frame kind is rejected as invalid before any other
check runs, so nothing reaches later checks unclassified. Where a
BANNER/LOADED preamble is present, it is parsed with the shared
engine_evidence grammar (tools/engine_evidence.py) rather than a
local, looser re-implementation.
The numeric-tail check now recognizes either spelling the engine has
shipped -- the fixed six-decimal form ("fixed6") and the older %.17g
form ("c17g") -- classifying each token independently against both
grammars rather than trying one first and hiding the other; a token
matching both is reported "ambiguous" (informational only, never
affects the verdict), and a nan/inf/-inf spelling is accepted as a
value but always fails the finite/non-positive check with a named
reason. Every reported problem now cites the byte offset of the
offending frame's own header line.
Coverage: test_check_data_logprob_gaps.py exercises capture_mode's
classification, both numeric grammars (including the ambiguous and
special-token cases), the preamble gate's coupling to engine_evidence,
and the byte-offset reporting, against literal transcript fixtures
built by hand rather than a real engine capture.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
engine_evidence.py is shared verbatim across branches, not immutable -- a root fix here costs nothing while the sibling branches are unbuilt. F3 (root fix): parse_engine_banner/parse_engine_loaded now catch ValueError from their own int()/float() conversions and re-raise as PreambleError, so a numeric field beyond Python's 4300-digit int-string conversion limit refuses with this module's own named error instead of escaping as a bare ValueError from all three entry points. eval_glm.py's downstream catches of (PreambleError, ValueError) stay as defense in depth. F2: corrected premise -- check_data_logprob_gaps.py in this tree has no engine_evidence coupling, but JustVugg#1357's own (much larger) copy does, and its _global_header treats any problem entry, including a spurious malformed-preamble complaint, as a whole-transcript hard failure. Confirmed at primary source (grep across every "loaded"-prefixed printf in the C sources) that no engine anywhere emits "loaded index" or anything else that collides with the "loaded in" prefix test today, so per that finding, the fix is prose: parse_engine_preamble's docstring now states plainly that "starts like one of them" is a literal prefix test, not a semantic one, and names the collision. test_engine_evidence.py (also shared, also not frozen) gains: - IdotKernelRosterTest: every IDOT_KERNELS entry parsed from the real banner format, sourced from c/quant.h:582-594 with file:line citations ("neon" confirmed RAN via this host's own default-build compiler flags), plus a roster-size backstop -- closes the roster gap the previous round wrongly reported as already closed (that claim rested on eval_glm.py's own test, not on this module's own 45-test suite, which a scratch reduction to ("avx2",) alone left fully green). - test_trailing_newline_rejected on both parse_engine_banner and parse_engine_loaded: fullmatch requires consuming a trailing "\n"; $ alone does not, so match()/search() would accept a newline- terminated banner/load record that fullmatch correctly refuses -- the shape a real `for line in f:` caller actually hands over. - test_oversized_field_raises_preamble_error_not_bare_value_error on both functions, pinning the F3 root fix directly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
e047474 to
da0c938
Compare
monotophic
changed the title
|
Set-level explanation and the full traceability matrix: #1355 (comment) You were right about the redundancy, and the PR is now the small one you asked for: 11 files and +7,464 → 4 files and +2,319. The witness is parked
They are preserved at What is left is the "What is meant to invoke them?"A person (or agent). These are manually-invoked argparse tools, like We think that is reasonable simplicity rather than a gap, but if you would rather they were wired into something, say Your asks
Five further files left this PR — Details — origin accountingmaintainer-requested: the witness park; the split. The wire-format literals in this tool are hand transcriptions of the C source. Module drift is pinned by tests;
|
3 participants
Authored by Opus 5 in Claude Code, analysis in partnership with @monotophic
Revised 2026-09-17 in response to review. The native-MTP witness is parked and this PR is now the logprob-gap
hardening alone — 11 files and +7,464 down to 4 files and +2,319. Title updated to match; the previous one
still advertised the witness.
A command-line check that reads an engine transcript and says whether it is internally consistent.
tests/check_data_logprob_gaps.pyalready existed and had two weaknesses. It read a transcript without firstestablishing that the transcript came from an engine it understands, so an unrecognised header was treated as no
header; and it reported problems without saying where in the byte stream they were, which makes a disagreement on a long capture tedious to localise. It now parses the engine's preamble through a shared helper and fails loudly
when that preamble is unparsable, the protocol grammar is exact, the numeric tail and the top-k cap are enforced,
and every reported problem cites the byte offset of the frame it concerns.
The numeric grammar accepts both the fixed six-decimal form the engine ships today and an earlier
%.17gform(plus
nan/inf/-inf); a token can satisfy both spellings exactly, so form classification stays informational —a tally in the summary line — rather than a pass/fail rule.
What was parked, and why.
tests/check_native_mtp_witness.pyand its test — 2,613 lines exactly — are outof this PR. The review was right on every number:
MTPEMITappears in zero files ondev, so the tool could onlyever exit 3
UNSUPPORTEDon any build that exists, and carrying a reader ahead of its writer is the pattern weidentified ourselves in #1353. The work is preserved at
monotophic/colibricodex/parked-mtp-witness@e0474748and is proposed again alongside the engine change that emits the record it needs — not before.What invokes this tool. A person. It is a human-invoked
argparsetool, liketools/check_ablate_evidence.py,which is why nothing in
c/Makefileor any workflow references it and why an import walk cannot find a caller. Itsunit tests run in CI; the tool itself is run by hand against a captured transcript when diagnosing an engine. If you
would rather it were wired into something, say which and we will do that as a separate change.
Context. This PR is one of five independent contributions derived from a single locally-verified working tree
(the fp8 container line in #1102, the OpenAI-compatible server hardening in #1353, the evaluation harness in #1355,
the engine's ablation scoring mode in #1356, and this transcript check). This one carries the logprob-gap check and
its suite. It imports
tools/engine_evidence.pyfor the preamble grammar and carries that file itself, so it builds and tests standalone; once #1355 lands, that file drops out of this diff on the nextdevmerge. #1356 does not carryit — that PR ended up importing nothing from the module, so it was dropped there. The others
are proposed separately, each with its own evidence. The set-level explanation and a traceability matrix covering every request across the three PRs are in the #1355 comment: #1355 (comment).
Behavioral contract
Capstone matrix — one decisive artifact per claim.
test_unparsed_banner_is_a_named_failure_quoting_the_line,test_unparsed_loaded_is_a_named_failure_quoting_the_line, andtest_full_capture_with_unparsed_preamble_fails_the_whole_check— the last drives a complete, otherwise-valid capture and requires the whole check to failtest_preamble_returning_none_is_defensively_named_not_silently_passed— the case where the parser returns nothing at all is asserted to be a named failure, which is the shape a "no news is good news" gate would passtest_dropping_<record>_breaks_its_own_ordinal_checkcases (BANNER,LOADED,READY,STAT,HWINFO,TIERS,EMAP) plustest_required_order_matches_the_frozen_anchor. Before this revision every one of the seven could be dropped individually with the suite green — the list was declared, not enforced. Each now fails on its owntest_fixed6_form_is_accepted_and_reported,test_c17g_form_is_accepted_and_reported, andtest_a_token_matching_both_grammars_is_ambiguous_and_still_accepted; the fixtures use the engine's own printf formatstest_done_domains_and_fixed_grammar_bite_independently— each rule fails on its own input rather than being masked by a neighbourtest_checker_dispatch_prefixes_stay_recognized_by_the_module— the check's own dispatch prefixes are asserted to still be recognised byengine_evidence, in both directions. An earlier one-directional version of this test passed while the module's dispatch was deleted outrightFuller matrix, what changed since the reviewed head, and origin accounting
What changed since
e0474748, the head reviewed on 2026-09-17check_native_mtp_witness.py+ its test parked (2,613 lines)eval_glm.py,test_eval_glm.py,check_ablate_evidence.py,test_check_ablate_evidence.py,test_pack_python.py-infs a vocabulary logit, so the only path to one is the total-vocabulary numerical collapse thatsample.h's own comments call a blow-up upstream. Behaviour is unchanged, and this is added explanation — not the correction of a false claim: measured across the two heads, the mentions of grammar/masking went up, not downdevplus this PR's filespython3 -m pytest tests/test_check_data_logprob_gaps.pySuite cost. Parking the witness takes the two transcript-tool suites from 107 tests / 0.64 s to 61 tests /
0.11 s, measured at
e0474748and at this head. No test here exceeds 0.02 s (slowest three measured at 0.02 s); there is nofixture that waits on a timeout.
A limitation we would rather state than have you find. The gap this check hunts is not producible by the
engine's current multiplexing loop, so on today's builds it is regression coverage for an invariant rather than a
live detector, and the preamble check binds no binary or container digest by itself. Both facts are in the tool's
own docstring. Separately: the wire-format literals here are hand transcriptions of formats printed by
c/colibri.candc/telemetry.h. Drift in the Python module is pinned by the tests above; drift in the C sourceis not — no test in this branch reads a C file. That predates this revision, and fixing it would reopen the
shared file across all three PRs, so it is noted as a follow-up rather than folded in here.
Scope note: no file outside the four listed is touched, and no surrounding code was reformatted.
Durable vs current state: the grammar, the preamble gate and the frame-order contract are durable. The case
count (61) is current state, measured 2026-09-17 against base
cc756a63on macOS (Apple silicon).