test(docs): pin the validation-page numbers to their source artifacts - #436
test(docs): pin the validation-page numbers to their source artifacts#436bioedca wants to merge 7 commits into
Conversation
docs/validation.md is a transcription page: it restates the four $.tolerance bounds, the four $.tolerance_by_method.ebhmm bounds, both pooled_worst blocks, the kinSoftChallenge rate matrices and roughly a dozen module constants from src/. Nothing pinned any of them, so a re-freeze or a constant edit left the published validation page silently wrong while mkdocs build --strict, which is a link-and-nav check rather than a value check, stayed green. That is the failure mode #158 was written to eliminate. tests/test_docs_validation_numbers.py closes it. Each pin locates a region structurally before it reads a number - a table selected by a header cell then a row by its label and a column by its header, or the one non-table paragraph of a section that names an anchor token - and every step asserts a unique match, so a page edit that duplicates or renames a table, row or column fails loudly instead of silently widening what counts. Inside a paragraph a literal belongs to the nearest name before it, which is how the page writes every one of them, so moving a number next to a neighbouring key fails rather than passing because the digits are still present. Artifacts are read with json and module constants with ast, so the guard imports no tether and runs unmarked on the 3-OS matrix. Figures with no committed source - upstream commit dates and change counts, historical Actions runs, and ADR-0022's local-run numbers - are enumerated in EXCLUSIONS with a reason each, because an unexplained omission on a validation page is indistinguishable from an oversight. Closes #214.
📝 WalkthroughWalkthroughAdds a dependency-free test suite that structurally validates numeric transcriptions and quotations in ChangesDocumentation validation pinning
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
The kbright / kdark pins restated 7 and 0.007 as literals in the registry, so they asserted the test module's own copy of two numbers rather than the committed artifact's. A pin that reads nothing is prose, which is the defect this whole module exists to remove. Both are the one pair kinsoft_reference.json keeps inside free text - "blinking kbright=7 s^-1, kdark=0.007 s^-1" in the level-3 ground-truth note - rather than as fields of rates_s_inv, so _blinking_rate reads them with a regex bounded to the key name and asserts rather than guesses when the note stops recording them. Types are preserved: int 7, float 0.007. Adds the matching one-value mutation case, so the new pins are shown to fail rather than only to pass.
All four let a real docs/source mismatch through, and each falsified something
this module claims about itself.
Pin the measurement counts. $.method.n_runs_per_fixture and both n_comparisons
are machine-readable, but were neither pinned nor listed in EXCLUSIONS, so a
re-measure with a different run count left "20 self-reseeded fits per fixture;
39 recorded comparisons" and "19 cross-seed comparisons" green. The 39 is now
summed from spread_by_fixture rather than restated. Reaching that paragraph
needed a second anchor form - it carries no pointer, only the bold lead-in
**Measured result.** - and the fallback is consulted only when the backticked
form matches nothing, so no existing anchor changes meaning.
Compare quoted claims with the page. test_the_page_quotes_the_artifacts_verbatim
read only the artifacts, never PAGE_TEXT, so rewording "never gates main" to its
opposite kept it green. A quotation has two sides; both are now checked, and the
separate "records only" claim is asserted separately from the string itself.
Bind each DOI to its label. The citation pins rendered a bare {}, so the paper
doi and the data doi could swap places and both still be somewhere in the
paragraph. Each template now carries the words identifying its field, and Quote
compares whitespace-normalised text so the page's hard wrap between "Paper doi"
and the DOI cannot break the binding.
Reject duplicate sections. _sections assigned into a dict, so a stale duplicate
of a validation section inserted ABOVE the canonical one was read by nothing and
published anyway. A repeated ## heading is now an error, and the heading regex no
longer parses a future ### sub-heading as a section named "# ...".
Each fix carries the negative test that would have caught it.
Two more false passes from the second Codex read, both of them the same shape as what the module already claims to prevent. A Prose pin accepted the key if ANY literal attributed to it matched, so a paragraph stating one constant twice kept a stale contradictory restatement published as long as one occurrence stayed right - and DEFAULT_SHIP_BAR_PTS is stated twice. Every comparable literal must now agree. Comparable is scoped by KIND rather than exact type: any two numbers are candidates for the same claim, so 11 contradicts 10.0 instead of being waved through, while the nearest-name rule is still allowed to sweep up a trailing count of another kind - "measured_utc "2026-07-08", 19 cross-seed comparisons" attributes both, and only the date is a restatement of measured_utc. Equality itself stays type-exact. The level-3 ground truth lists six non-zero transitions; "all other off-diagonal rates zero" is what makes it a 4x4 matrix. Pinning the six and not the clause let the page say the unlisted transitions were 0.01 with every guard green. Both sides are now checked - the artifact writes "are 0" and the page writes "zero", so the clause is matched by shape and the captured word is then required to MEAN zero - and the enumerated rate set is required to equal the artifact's, without which "all other" no longer covers what it claims to. Each carries the negative test that would have caught it.
…prose
The previous round taught the Prose branch that keeping the right number is not
enough if a wrong one is published beside it. Cell and Quote were left on the old
first-match rule, so the identical stale restatement passed or failed depending
only on whether the page happened to print it in a table. Codex found all three.
_states now requires every comparable number in the selected cell to agree, so
"0.95 (was 0.93)" fails. Quote compiles its template into a pattern and checks
EVERY occurrence rather than testing for one substring, so a paragraph that keeps
"(5 % average)" and adds "(6 % average)" fails. The level-3 zero clause is read
with finditer on both sides, so a second contradictory clause after a correct one
is no longer invisible.
The licence template gains one character of context. Rendering it as a bare
"({})" made it a pattern that also fires on the citation's "(2022)" two clauses
earlier, which under all-occurrence checking is a false failure rather than a
false pass - but it is still the template being wrong about where its claim lives.
Also narrows an over-broad claim rather than chasing it. The docstring said
EXCLUSIONS enumerates everything deliberately unpinned; the page restates some
pinned values a second time in running prose (RMS <= 0.5 px in (a), the rounded
pooled-worst figures in (b)) and those sit outside every selected region. A
page-wide numerical manifest is real scope that #214's approved criteria do not
carry, so the claim now says what the module actually does and points at #435.
… bite Fifth Codex read. Five findings, all in the class the previous rounds were closing, so they are finished here rather than left half-done. The string branch of _states was still first-match while the numeric branch required agreement, so a date or interpreter-version cell could keep the right token and publish a stale one beside it. A cell now has to state exactly one value. That completes the rule the docstring already claims: Cell, Prose and Quote all reject a contradiction, not only a wrong value. _blinking_rate read the note with [0-9.]+, which truncates kbright=7e-1 to 7 and would let the page keep saying 7 while the artifact had come to mean 0.7. The literal is matched whole - sign, decimals, exponent, trailing boundary - and every occurrence must agree, so a note that contradicts itself has no single value to pin against and says so. The completeness test filtered constants by splitting a code span on whitespace, so a conventional `NEW_LIMIT=5` produced the token "NEW_LIMIT=5" while the literal scanner produced "NEW_LIMIT". The two never met and a new page-printed constant could arrive unpinned. It now takes the leading identifier. Three mutation tests re-implemented the check they were meant to protect, so they would have kept passing after the real guard regressed - a test that tests itself. The zero-clause scan and the two-sided quotation check are now functions the guard and the mutation tests both call.
Three structural-selection holes, each one a place the module already claimed to assert a unique match and did not. A duplicate target column bound every pin to the first one, because `in` followed by `.index()` never counts. A stale second `$.tolerance` column could therefore publish a contradicting value with the guard green. The count is now asserted. A bare row label matched by prefix with no boundary, so `level2` still answered for a row renamed or mistyped `level20 (Fig. 3)` and validated that row's numbers under a label the page no longer uses. Uniqueness could not catch it on its own: only one row still started with `level2`. A prefix match now has to end at a non-identifier character. The level-3 rate-set assertion read the artifact twice and the page never, so its own failure message described a comparison it was not making. The page could add `k13 0.01` beside the six correct rates and still claim every other transition was zero - a contradiction with itself rather than with the JSON. The transitions the page enumerates are now extracted and compared.
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (7)
tests/test_docs_validation_numbers.py (7)
325-332: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
:grenders at 6 significant digits and can hide a truncated page value.
value_textformats every numeric value with:g. That format keeps 6 significant digits. Every value aQuotepins today is short, so the rendering is exact. If a laterQuotepins a longer float,:gtruncates the live value, and the guard then compares a truncated artifact value with a truncated page value. The pin would accept a page that rounds, which is the exact defecttest_a_deliberate_one_value_mismatch_failsrejects for$.pooled_worst.Add a round-trip guard so the rendering stays exact.
♻️ Proposed fix
def value_text(self) -> str: """The live value as the page spells it.""" if isinstance(self.value, str): return self.value - return f"{self.value * self.scale:g}" + scaled = self.value * self.scale + text = f"{scaled:g}" + # `:g` keeps 6 significant digits. A pin whose value needs more would compare a + # truncated artifact value against a truncated page value and accept a rounding. + assert float(text) == scaled, ( + f"{self.source} needs more than 6 significant digits; render it exactly " + f"rather than with ':g' ({text!r} != {scaled!r})" + ) + return text🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_docs_validation_numbers.py` around lines 325 - 332, Update Quote.value_text so numeric values render with round-trip-safe precision instead of the 6-significant-digit :g format. Preserve string handling and scaling, ensuring rendered() compares the exact numeric value represented by the page rather than a truncated approximation.
1219-1241: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider routing every mutation through
_mutate.
_mutateassertsbody.count(old) == 1, which proves the mutation applied exactly once. Lines 1231, 1319, 1360 and 1405 use a barestr.replaceinstead, so a page reword turns the mutation into a no-op.Each of these sites is fail-closed today: a no-op makes the following assertion fail rather than pass. So there is no false-green risk. The failure message would report the wrong cause, though. Routing them through
_mutate, or adding acount(old) == 1assertion, would name the real problem.Also applies to: 1299-1323, 1358-1369, 1403-1410
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_docs_validation_numbers.py` around lines 1219 - 1241, Route the direct string mutations in test_a_number_moved_to_a_neighbouring_key_fails and the additional mutation sites around the affected tests through _mutate, or add an equivalent count(old) == 1 assertion before replacement. Ensure each mutation verifies exactly one source occurrence so wording changes fail with a mutation-specific error instead of a downstream assertion.
696-704: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe
DEFAULT_SHIP_BAR_PTSanchor hardcodes the live value.The pin reads the constant live at line 702. The anchor at line 699 is
"DEFAULT_SHIP_BAR_PTS = 10.0", which repeats the value the pin is meant to verify.If
src/tether/ml/prequential.pychanges the constant and the page is updated correctly, the anchor matches nothing andparagraph()reports a selection failure on a correct page. Anchor on the bare identifierDEFAULT_SHIP_BAR_PTSinstead, if it selects exactly one paragraph in that section.This shares a root cause with the
$.band.rate_rel_deviation_maxanchor at lines 816-835.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_docs_validation_numbers.py` around lines 696 - 704, The Prose anchor for DEFAULT_SHIP_BAR_PTS hardcodes its current value instead of identifying the constant independently. Update the anchor argument in the DEFAULT_SHIP_BAR_PTS pin to use the bare identifier, preserving the live module_constant value used for validation; apply the same anchor-only approach to the analogous $.band.rate_rel_deviation_max pin.
932-936: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
PAGE.is_file()can never fail here.Line 73 reads
PAGEat import time. If the page is missing, collection raisesFileNotFoundErrorbefore this test runs. The assertion at line 933 is therefore unreachable in its failing state, and the messagef"{PAGE} is missing"never appears.The import-time read is the right choice for this guard: a missing artifact should break collection, not silently skip. Consider dropping the redundant assertion so the test states only what it can prove.
♻️ Proposed simplification
def test_page_exists_and_still_has_its_sections() -> None: - assert PAGE.is_file(), f"{PAGE} is missing" sections = _sections(PAGE_TEXT) for name in ("(a) Extraction vs Deep-LASI", _PARITY_SECTION, _KINSOFT_SECTION): assert name in sections, f"docs/validation.md lost its '{name}' section"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_docs_validation_numbers.py` around lines 932 - 936, Remove the redundant PAGE.is_file() assertion from test_page_exists_and_still_has_its_sections; PAGE_TEXT is already read at import time, so a missing page fails during collection. Keep the section-presence checks and their existing failure messages unchanged.
99-108: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
module_constantreturns the first top-level binding, not the effective one.The loop returns on the first matching
ast.Assign/ast.AnnAssign. Python semantics give the last top-level binding. If a module rebinds a constant later at module scope, this pin asserts the page against a value the runtime never uses, and the guard reports green.Track the last match instead of returning early.
♻️ Proposed fix
path = SRC / Path(*dotted.split(".")).with_suffix(".py") + found: object = None + seen = False for stmt in ast.parse(path.read_text(encoding="utf-8"), filename=str(path)).body: if isinstance(stmt, ast.AnnAssign): if isinstance(stmt.target, ast.Name) and stmt.target.id == name: assert stmt.value is not None - return ast.literal_eval(stmt.value) + found, seen = ast.literal_eval(stmt.value), True elif isinstance(stmt, ast.Assign): for target in stmt.targets: if isinstance(target, ast.Name) and target.id == name: - return ast.literal_eval(stmt.value) + found, seen = ast.literal_eval(stmt.value), True + if seen: + return found raise AssertionError(f"{dotted} has no module-level constant {name!r}")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_docs_validation_numbers.py` around lines 99 - 108, Update module_constant to scan all top-level statements and retain the value from the latest matching ast.Assign or ast.AnnAssign binding, returning it only after the loop completes. Preserve the existing literal evaluation and missing-constant assertion behavior.
811-841: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTwo anchors embed the value the pin reads live. Each pin reads its value from the artifact or from
src/, which is correct. The anchor strings that select the paragraph repeat that same value. When the source value changes and the page is updated correctly, no paragraph carries the old code span, soparagraph()fails with a selection error and advises re-anchoring. The maintainer then sees a selection failure on a page that is correct. Anchor on the stable pointer or identifier so selection survives a value change while the pin still detects a stale page.
tests/test_docs_validation_numbers.py#L811-L841: replace the anchor"$.band.rate_rel_deviation_max = 0.12"with the bare pointer"$.band.rate_rel_deviation_max"in theProsepin at line 816 and in bothQuotepins at lines 825 and 835.tests/test_docs_validation_numbers.py#L696-L704: replace the anchor"DEFAULT_SHIP_BAR_PTS = 10.0"at line 699 with the bare identifier"DEFAULT_SHIP_BAR_PTS".Confirm each replacement anchor still selects exactly one non-table paragraph in its section before you land the change.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_docs_validation_numbers.py` around lines 811 - 841, The anchors in tests/test_docs_validation_numbers.py#L811-L841 should use the stable pointer "$.band.rate_rel_deviation_max" instead of embedding "= 0.12" in the Prose and both Quote pins; tests/test_docs_validation_numbers.py#L696-L704 likewise should use the bare identifier "DEFAULT_SHIP_BAR_PTS" instead of "DEFAULT_SHIP_BAR_PTS = 10.0". Confirm each replacement selects exactly one non-table paragraph in its section.
151-203: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
_sectionsand_tablesdo not track fenced code blocks.Both parsers scan raw lines. A line inside a
```fence that starts with##splits a section, and a line that starts with|opens a phantom table. The page may add a fenced JSON or shell example later.The failure mode is loud, not silent: a phantom table lacks the pin's header cell, and a phantom heading trips the duplicate or missing section assertion. So this is a robustness note, not a correctness defect today. If the page gains fenced examples, add a fence toggle to
_sectionsso both parsers see only prose and tables.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_docs_validation_numbers.py` around lines 151 - 203, Update _sections and _tables to track Markdown fenced code blocks and ignore headings, table separators, and table rows encountered while a fence is open. Toggle fence state on fenced-delimiter lines, ensure fenced content cannot split sections or create phantom tables, and preserve existing parsing behavior for prose and actual tables.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/test_docs_validation_numbers.py`:
- Around line 325-332: Update Quote.value_text so numeric values render with
round-trip-safe precision instead of the 6-significant-digit :g format. Preserve
string handling and scaling, ensuring rendered() compares the exact numeric
value represented by the page rather than a truncated approximation.
- Around line 1219-1241: Route the direct string mutations in
test_a_number_moved_to_a_neighbouring_key_fails and the additional mutation
sites around the affected tests through _mutate, or add an equivalent count(old)
== 1 assertion before replacement. Ensure each mutation verifies exactly one
source occurrence so wording changes fail with a mutation-specific error instead
of a downstream assertion.
- Around line 696-704: The Prose anchor for DEFAULT_SHIP_BAR_PTS hardcodes its
current value instead of identifying the constant independently. Update the
anchor argument in the DEFAULT_SHIP_BAR_PTS pin to use the bare identifier,
preserving the live module_constant value used for validation; apply the same
anchor-only approach to the analogous $.band.rate_rel_deviation_max pin.
- Around line 932-936: Remove the redundant PAGE.is_file() assertion from
test_page_exists_and_still_has_its_sections; PAGE_TEXT is already read at import
time, so a missing page fails during collection. Keep the section-presence
checks and their existing failure messages unchanged.
- Around line 99-108: Update module_constant to scan all top-level statements
and retain the value from the latest matching ast.Assign or ast.AnnAssign
binding, returning it only after the loop completes. Preserve the existing
literal evaluation and missing-constant assertion behavior.
- Around line 811-841: The anchors in
tests/test_docs_validation_numbers.py#L811-L841 should use the stable pointer
"$.band.rate_rel_deviation_max" instead of embedding "= 0.12" in the Prose and
both Quote pins; tests/test_docs_validation_numbers.py#L696-L704 likewise should
use the bare identifier "DEFAULT_SHIP_BAR_PTS" instead of "DEFAULT_SHIP_BAR_PTS
= 10.0". Confirm each replacement selects exactly one non-table paragraph in its
section.
- Around line 151-203: Update _sections and _tables to track Markdown fenced
code blocks and ignore headings, table separators, and table rows encountered
while a fence is open. Toggle fence state on fenced-delimiter lines, ensure
fenced content cannot split sections or create phantom tables, and preserve
existing parsing behavior for prose and actual tables.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e57e20d0-7d33-4dea-a161-37a93b7157a2
📒 Files selected for processing (1)
tests/test_docs_validation_numbers.py
|
Deferred: CodeRabbit''s seven nitpicks. Tracked in #440. Recorded here rather than as thread replies because the review posted zero inline comments — there is no thread to answer or resolve. Its body opens straight onto None is a false pass. The strongest of them — four pin anchors that embed the value the pin reads live — bites in the safe direction: it raises a selection error on a page that has been correctly updated, rather than passing a stale one. Codex''s one deferred finding (a page-wide manifest accounting for every numerical restatement as pinned-or-excluded) is tracked in #435 instead, since that issue already owns the figures this PR left unpinned. #435 and #440 both touch |
1 participant
Summary
docs/validation.mdis a transcription page. It restates the four$.tolerancebounds, the four$.tolerance_by_method.ebhmmbounds, bothpooled_worstblocks, the kinSoftChallenge rate matricesand roughly a dozen module constants from
src/— and nothing pinned any of them.mkdocs build --strictis a link-and-nav check, not a value check, so a re-freeze or a constant edit left thepublished validation page silently wrong while every gate stayed green. That is the exact failure
mode #158 was written to eliminate.
This adds
tests/test_docs_validation_numbers.py— one new file, 1410 lines, nothing elsetouched. No
src/, schema, lock, CI ordocs/change.Structural selection, not literal grepping. A pin never searches the page for its own number; it
locates a region first and then asks that region what it states:
Cell— the table in a named##section carrying a given header cell, then the row by its label,then the column by its header text. Every step asserts a unique match, so a page edit that
duplicates or renames a table, row or column fails loudly rather than silently widening what counts.
Prose— the one non-table paragraph of a section naming an anchor token, then each literalattributed to the nearest preceding name. Ownership, not presence: swapping two ebFRET
pooled-worst values leaves both literals on the page and both distinct, and
test_a_number_moved_to_a_neighbouring_key_failsproves the guard still rejects it.Quote— a live artifact value rendered into a fixed phrase carrying the words that say whichfield it is, whitespace-normalised so a Markdown re-wrap cannot break it. Carries the measurement
counts, the DOIs and licence, and the two figures the page prints as percentages (
5 %,≥ 3 %).All three reject a contradiction, not just a wrong value: keeping the right number while
publishing a stale one beside it fails in a cell, in a sentence and in a quoted phrase alike.
Section, table, row, column and paragraph selection each assert a unique match, and a repeated
##heading is an error rather than a last-one-wins overwrite.Artifacts are read with
json, module constants withast. Notetherimport, no SciPy/h5py — themodule is unmarked and runs on the plain 3-OS matrix in ~0.2 s.
Grooming recheck. The 2026-08-02 grooming pass required every enumerated tolerance to be
re-derived against the current artifacts before landing (the measured-literal trap named on #300),
not inherited from the issue text. Done: all 8 bounds, both
pooled_worstblocks, both artifacts'schema_version/frozen_at_milestone/measured_utc, the build table's two Python versions, thekinSoft band, level-1 ground truth/Tether/deviations and the level-3 matrix were each re-read from
schema/parity_tolerance.json,schema/kinsoft_reference.jsonandsrc/. Every one alreadymatched, so this PR changes no prose — the page was accurate, just unguarded.
Coverage guards. Four tests assert the registry cannot lag its sources: a bound added to either
tolerance block, a metric added to either
pooled_worst, a rate added to either ground-truth matrix,or a
SCREAMING_CASEconstant the page newly prints a value for all fail until pinned. That last onefilters on code spans, because
PRD §11.2,PEP 610andRMS ≤ 0.5 pxare shaped like constants.Exclusions are enumerated in
EXCLUSIONSwith a reason each — upstream commit dates andchange counts, two historical Actions runs, ADR-0022's local-run numbers, and the paper figures the
artifact holds only inside free-text prose. One is worth the maintainer's eye: #214 puts the tMAVEN
commit hashes out of scope on the ground that they are external, but the artifact does record both
under
$.method.tmaven_commitand$.measured_by_method.ebhmm.method.tmaven_commit. I honoured theapproved criterion rather than quietly widening it, and filed the gap separately — see below.
Linked tracking
src/, schema, lock, CI or docs change. Worst case is afalse failure that forces a look at the page, which is the safe direction for a validation guard.
completed reads (
codex review --base main, CLI 0.147.0), each on the then-current head, thelast on
ed892b6. Unmetered, so uncapped. Nineteen findings across the five reads — eighteen fixed,one deferred; see Findings below. Its verdict on the fourth read, which is the one that drove
the largest batch of fixes: "The current values pass, but the new regression guard has demonstrated
false-pass paths for duplicate columns, prefix-colliding row labels, and extra matrix transitions.
These undermine its core structural pinning behavior." Every one of the nineteen was P2 —
below Codex's own top two severity bands, so nothing blocking by
AGENTS.md's severity test. Theywere fixed anyway under the stricter clause (a finding that falsifies a claim this PR introduces),
which is what each of them was.
.agents/bin/greptile_usage.py), but this is a low-risk, test-only diff with nosrc/, schema,lock or CI change, and Codex has already made five passes over it. Recording the reason rather
than spending the maintainer's credit.
status read
successrather thanpending, so no run in flight was destroyed.commit_id:ed892b6f073875970293f10b5295206035f29cad— the final head.submitted_at:2026-08-10T23:26:26Z· state:COMMENTED🧹 Nitpick comments (7). There is noActionable comments posted:line, which is how zero is written. Seven nitpicks, every onelabelled
🔵 Trivial— its lowest band — and zero inline review comments, so there are nothreads to resolve. Deferred to chore(docs): harden the validation-page guard (deferred #436 nitpicks) #440; see below.
The 'gpt-5.6-sol' model requires a newer version of Codex. That run reviewed nothing and is notcounted as a pass; the CLI was upgraded to 0.147.0 and the read redone. Codex is unmetered, so the
retry cost nothing.
18serious (fixed) |8below the floor —1deferred to chore(docs): pin the validation-page figures #214 left out of scope #435 (Codex's page-widecoverage manifest) and
7to chore(docs): harden the validation-page guard (deferred #436 nitpicks) #440 (CodeRabbit's nitpicks, all🔵 Trivial). No thread to resolveon either: Codex reviews locally and CodeRabbit posted zero inline comments, so both deferrals are
recorded here and in a PR comment rather than as thread replies.
committed in
schema/kinsoft_reference.jsonand already printed on the page; pinning them assertsthe transcription, it does not introduce a citation.
Type of change
docs/chore/ci/build/refactor/test/perfSelf-review checklist (PRD §12.4)
schema/*.json.docs-build(
mkdocs build --strict, pinnedrequirements-docs.txt) is green on this draft.AGENTS.md§Review) — the diff went green on the draft before anythingwas asked to read it; Codex then read it five times on the green diff, before any metered
provider, and its findings are answered above; Greptile is recorded as skipped with its
reason; and CodeRabbit returned no actionable comments at the final head
ed892b6, quotedabove with its permalink, full 40-hex
commit_id,submitted_at,COMMENTEDstate and theopening of its body. Serious findings fixed, the rest deferred to chore(docs): pin the validation-page figures #214 left out of scope #435 and chore(docs): harden the validation-page guard (deferred #436 nitpicks) #440.
GPL-3.0-or-laterheader present(
reuse lintgreen — 415/415 files).Testing
Local gates, native-PowerShell lane,
QT_QPA_PLATFORM=offscreen:pytest tests/test_docs_validation_numbers.py— 105 passed in 0.22 spytest -m "not large and not sidecar and not deep"— 2922 passed, 19 skipped, 34 deselectedruff check+ruff format— clean (pre-commit'sruff-check/ruff-formathooks)reuse lint— compliant, 415/415 filespre-commithooks: LF endings, no trailing whitespace,terminal newline, 36 KB (well under
--maxkb=512), no YAML/TOML touched.Every required check is green on this draft, which is where
mkdocs build --strictand the fullpre-commithook set were actually confirmed — a localmkdocsrun was started but was stillresolving its pinned toolchain, so CI is the evidence rather than a local claim:
lint·pre-commit·commitlint·secret-scan·conda-lock-verify·docs-build·schema-guard·sidecar / parity·test (ubuntu-latest)·test (macos-latest)·test (windows-latest)·CodeQL/Analyze (actions)/Analyze (python)The guard was verified to fail in both directions, not merely to pass:
test_a_deliberate_one_value_mismatch_failsmutates exactly one number in eightplaces (a tightened ebFRET floor, a rounded
pooled_worst, a truncated ebFRET value, a moduleconstant, a kinSoft ground-truth rate, one entry of the level-3 matrix, a blinking rate, and the
second of two statements of one constant) and asserts the owning pin rejects the page. One
mutation had to be widened during development: a last-place edit inside one ULP parses back to the
same double, and the same double is the same number.
test_a_contradiction_beside_a_correct_value_failskeeps the right number and publishes a stale one next to it, in a table cell and in a
Quoteparagraph, and requires both to fail.
$.levels.level1.ground_truth.rates_s_inv.k12_low_highwas temporarily edited0.15 → 0.16inschema/kinsoft_reference.json; the run went to1 failed, 86 passednaming thatpointer, and the artifact was restored with
git checkout.test_an_ambiguous_anchor_fails_loudly_rather_than_picking_oneduplicates aparagraph anchor and requires the selector to raise rather than pick one.
Findings
Codex read the diff five times and raised nineteen findings, all P2.
kbright/kdarkrestated7and0.007as literals in the registrynote, the one place the artifact keeps them as free text (self-caught, before Codex)n_runs_per_fixtureand bothn_comparisonspinned; the total summed fromspread_by_fixture, not restatedtest_..._quotes_the_artifacts_verbatimnever read the page##sections silently shadowed###Proseaccepted the key on any matching literalDEFAULT_SHIP_BAR_PTSis stated twiceCellstill accepted a contradiction0.95 (was 0.93)failsQuotestill accepted a contradictionfinditeron both sides[0-9.]+readkbright=7e-1as7re.searchon the blinking notespan.split()[0]producedNEW_LIMIT=5where the scanner producedNEW_LIMITstartswithwith no boundary —level2answered forlevel20Each fix ships with the negative test that would have caught it. The module went 87 → 105 tests
across the review; the growth is almost entirely those.
Why these were fixed rather than deferred. Every one is the same defect — a pin or a test that
looks like it binds and does not — and each contradicted something the diff itself asserts. #3 and
#15 are the clearest: a test named
..._quotes_the_artifacts_verbatimthat never read the page, andthree mutation tests that re-implemented the check they existed to protect, so all four would have
kept passing after the real guard regressed. Reviews 4 and 5 found progressively narrower
hypotheticals, which is where the marginal return ran out and the Codex leg was closed.
Deferred — 1, to #435. Codex asks for a page-wide manifest accounting for every numerical
restatement as pinned-or-excluded. It is right that the gap exists: the page restates some pinned
values a second time in running prose (
RMS ≤ 0.5 pxin (a), the rounded$.pooled_worstfigures in(b)'s blockquote), outside the region any pin selects. But that is scope #214's approved title/body
snapshot does not carry, and
AGENTS.mdis explicit that fixing a non-serious finding here is scopebreach. What was fixed is the part that was genuinely this PR's: the docstring claimed
EXCLUSIONSenumerates everything deliberately unpinned, which was false. It now states what the module actually
does, names the two known restatements, and points at #435.
Follow-ups filed
#435 —
chore(docs): pin the validation-page figures #214 left out of scope. It covers the twotmaven_commitvalues (which are in the committed artifact, unlike the upstream dates andchanged-line counts printed beside them) and three prose passages that restate a pinned value outside
the table row or anchored paragraph this guard selects. Filed separately rather than absorbed here,
because #214's title/body snapshot was approved as written and its criteria name those figures as
excluded. It also carries the page-wide coverage manifest Codex asked for.
#440 —
chore(docs): harden the validation-page guard (deferred #436 nitpicks). CodeRabbit's seven🔵 Trivialitems on this module's internals: four anchors that embed the value the pin reads live(a false failure on a correct page rather than a false pass),
Quote.value_text's:grenderingkeeping only 6 significant digits,
module_constanttaking the first top-level binding, unfencedcode-block parsing, a dead
is_file()assertion, and mutations that bypass the_mutateuniquenesshelper. Deferred rather than fixed because
AGENTS.mdis explicit that fixing a non-serious findingin this PR is scope breach; the two files overlap, so #435 and #440 should not be worked
concurrently.