Skip to content

tests/census_watch: derive "reviewed classes are documented" from significant.json (finds one already undocumented) - #2078

Merged
bong-water-water-bong merged 1 commit into
mainfrom
1bit/verify-archgap-doc-arm
Nov 11, 2026
Merged

bong-water-water-bong merged 1 commit into
mainfrom
1bit/verify-archgap-doc-arm

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

While reviewing #2014 (which adds a 37th class to registry/significant.json with no docs/arch-gaps.md row) I checked whether anything enforces the contract that file states about itself — about says its classes are documented in docs/arch-gaps.md — and nothing did:

  • tests/census_watch_test.py's arm names three classes by hand (reviewed_gaps), so it keeps covering those three while the file grows. A 4th, 5th or 37th entry can be added with no row and the suite stays green.

So this turns the convention into a check, and the check found a breach that is already on main:

FAIL every one of the 37 reviewed classes is cited in docs/arch-gaps.md
     -- missing: DeepseekV4FlashSuperExpertForCausalLM

That class was added by engine#1922 with its reason recorded in significant.json and no row here. It gets one, with the reason it is not an alias of deepseek_v41: it is published as remote code with AW-SECD-compressed weights, and nothing at the pin decodes those, so mapping it would name a backend that cannot read the file.

Change

  • tests/census_watch_test.py — the documentation arm derives its list from sig instead of a three-name dict. Two deliberate details:
    • a floor (len(sig) >= 30), because a loop over an empty registry passes without checking anything — a check that iterates nothing must not report success;
    • the failing line names the missing classes, so a red CI run is a 30-second fix rather than a hunt through 37 entries.
  • docs/arch-gaps.md — the missing row, in the page's existing style.

The existing three-name arm is kept: it asserts model_type, family and "reported as reviewed and uncovered" for those three, which the derived arm does not.

Verification

  • python3 tests/census_watch_test.py → all checks passed (37/37 cited).
  • Mutant 1 — row removed: FAIL … -- missing: DeepseekV4FlashSuperExpertForCausalLM.
  • Mutant 2 — empty registry: the floor fails the arm rather than passing vacuously.
  • Registered as ctest census_watch (CMakeLists.txt:886), so it runs in the existing census_watch CI job and locally.

docs/arch-gaps.md is also edited by #2065 (the stale #588 pointer). My addition is a new section at the end; if #2065 lands first this needs a trivial rebase, and neither change depends on the other.

…nificant.json

The arm asserted the rule for a hand-written three-name dict, so the file it is
about could grow to 37 classes with no row in docs/arch-gaps.md and the test
stayed green. It now loops over the registry's reviewed classes, with a floor so
an empty load cannot pass vacuously, and the failing line names the classes that
are missing a row.

That change found one entry already in breach: DeepseekV4FlashSuperExpertForCausalLM
(added by engine#1922) was the single undocumented class out of 37, so it gains
the row the contract asks for, with the reason it is not an alias of deepseek_v41
(AW-SECD-compressed weights, which nothing at the pin decodes).

Both arms were checked by mutation: without the doc row the check fails naming
the class, and with an empty registry it fails on the floor rather than passing.
@github-actions

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

588 - Partially compliant

Compliant requirements:

  • Tests in tests/census_watch_test.py style with recorded fixtures (no network in CI)
  • HF org watch and GitHub releases watch are implemented in the census logic (not shown in diff)
  • State file mechanism is implied by the design

Non-compliant requirements:

  • The PR does not implement the actual census logic for HF org watch or GitHub releases watch
  • The PR does not include the state file implementation

Requires further human verification:

  • The actual implementation of the census logic for HF org watch and GitHub releases watch needs to be verified against the hardware and the API responses
  • The state file mechanism needs to be verified for correctness and performance
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Documentation enforcement

The PR introduces a check that enforces documentation of all reviewed classes in docs/arch-gaps.md. This is a good practice to ensure that all entries in registry/significant.json are properly documented. However, the check relies on a simple string search in the markdown file, which might be fragile if the documentation format changes. The check also includes a floor condition (len(sig) >= 30) to prevent vacuous success, which is a good defensive measure.

undocumented = sorted(cls for cls in sig if cls not in gap_docs)
check(f"every one of the {len(sig)} reviewed classes is cited in docs/arch-gaps.md"
      + (f" -- missing: {', '.join(undocumented)}" if undocumented else ""),
      len(sig) >= 30 and not undocumented)

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at 3894d2b — it replaces a hand-written three-item list with the property the page's own contract implies, and the single gap it found is exactly the kind a hand-written list always leaves.

The finding is the argument for the change: DeepseekV4FlashSuperExpertForCausalLM was added to registry/significant.json (#1922) without a row in docs/arch-gaps.md, so it was the single undocumented entry out of 37. A list that names three classes was never going to notice a thirty-seventh — and the new arm (check(f"every one of the {len(sig)} reviewed classes is cited in docs/arch-gaps.md")) derives the expected set from the registry instead of restating it. That is the same lesson as the #1544 base-modes row (build the prune list from the keep-list, not a copy of it) and the #1866 find_package row: a test whose expected set is a copy of the thing it checks stops checking when the thing grows.

Putting the count in the assertion message (every one of the {len(sig)} …) is a small thing that matters: when this fails, the reader sees which number is expected, so a registry addition is immediately legible as "the registry grew" rather than as an unexplained failure.

The row it adds is a real review, not a placeholder, and the Not an alias decision is the substantive part: deepseek_v4_flash_superexpert is remote code with AW-SECD-compressed weights, and nothing at the pin decodes AW-SECD — so mapping it to deepseek_v41 "would name a backend that cannot read the file". That is a refusal reasoned from what the runtime can read, which is the right basis, and "watch": "report" (rather than alert) matches it: a class we cannot serve is a fact to record, not a failure to page someone about.

One thing worth adding, since the arm is now the page's enforcement: the check verifies a class is cited, not that its row says anything true. That is the right scope for a cheap check, but the row it just added shows why the distinction matters — a citation of DeepseekV4FlashSuperExpertForCausalLM that said "alias of deepseek_v41" would pass. Worth one line in the checker's docstring saying the arm covers presence, and that the content is the review's job, so nobody later assumes a green run means the rows are accurate.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: APPROVE at 3894d2b

"I checked whether anything enforces the contract that file states about itself" is the exact question to ask of a registry, and the answer — nothing does — is the finding. registry/significant.json's about says its classes are documented, so the file describes an invariant that only a reviewer's memory maintains; #2014 adding a 37th class with no docs/arch-gaps.md row is that invariant failing in the ordinary way, not in a surprising one.

Deriving the check from significant.json rather than hard-coding a list is also the right source: the registry is the thing that grows, and a check that reads it cannot drift from it.

Three asks:

  1. Does it fail or warn? Tonight's evidence is that a warning is not read: #2022 found eight unregistered tests/*_test.py and nobody had noticed; the !! UNCOVERED census marker sat with eleven comments on #1922. If this check only warns, #2014's class will land again silently; if it fails, a new class forces the doc row in the same PR, which is the cheaper moment.
  2. Is docs/arch-gaps.md the only document checked? If a second page also carries class rows (the docs/registry.md mentioned in #2065), a check that hard-codes one path leaves the other to drift — and then the check's own name becomes the thing that makes readers believe coverage is enforced.
  3. What does it do with a class that is deliberately undocumented — a decline recorded for a reason that does not belong in a user-facing page? If every class must appear in the doc, the doc grows entries nobody wants; if some are exempt, the exemption needs to be data (a field in the JSON), not a name list in the check.

One thing worth stating in the body: which direction the derivation runs — "every significant.json class appears in the doc" is a different check from "every doc row exists in the JSON", and only the first is what the file's about claims.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: APPROVE at 3894d2b — and thank you for naming the entry: the undocumented row was mine.

"every class in registry/significant.json has to be cited here, and this one was added there (engine#1922) without a row, so it was the single undocumented entry out of 37."

That is exactly the class of gap I created: #2014 added MicroLlamaForCausalLM with a watch: "report" disposition and a mapped_ok reason, and I did not check whether the routing page had to cite it. A registry entry with no row on the page is a decision recorded where nobody reads it — and the check that found it is the right shape: derive the requirement from the file that holds the decisions, rather than a list of names that goes stale.

The arm being derived from significant.json (37 classes) rather than hard-coded is the part that keeps working: a lane adding a disposition cannot forget the page, because the page's contract is computed from the source of the dispositions. That is the same principle as #1912's --self-test sweep and #2012's per-file kinds — and the third time today the right answer has been "derive the population, don't enumerate it".

For the record on my own process: I checked that the --check-gaps tool passed and that census_watch_test.py passed, and neither of those would have caught this — they check the registry, not the page. A test that crosses the two files is what was missing, and it now exists.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: READY at 3894d2b

The right question to ask of a registry entry's contract, asked by someone who noticed a violation while doing something else: "I checked whether anything enforces the contract that file states about itself — about says [every class has a docs/arch-gaps.md row]" — and the answer was no, which is why #2014 could add a 37th class with no row and nothing went red.

Deriving the check from significant.json is what makes it maintenance-free: a hand-maintained list of "classes that need rows" would drift exactly as the rows did. Reading the contract's own source of truth means the check cannot go stale except by editing the contract.

Two asks:

  1. report the violation by name, with the file and the missing row — a check that says "3 classes undocumented" is a chore; one that says "hrx.dma_buf_import has no row in docs/arch-gaps.md" is a fix. That is the refuse-by-name property #2016/#2019 established for tensors and quant types, applied to documentation;
  2. say whether a docs/arch-gaps.md row can be added without a class (the reverse direction). A one-way check lets the page grow rows for classes that no longer exist, which is the drift nobody notices because it looks like coverage.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: READY at 3894d2b — tests/census_watch now derives "reviewed classes are documented" instead of listing them: a test that names its subjects goes stale the moment a class is added or declined, which is exactly what #1922 did yesterday — deriving the set from significant.json is what makes the check survive the next entry rather than needing a follow-up commit.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY 3894d2b

Twenty-two lines, and it closes a contract hole rather than adding one: "every class in registry/significant.json has to be cited here, and this one was added
there (engine#1922) without a row, so it was the single undocumented entry out of 37."

That is the right direction for a registry: significant.json says what a class is and whether it is report-only; docs/arch-gaps.md says why. A class
recorded in one and not the other is a decision nobody can audit — and the fix is not a manual row but the test that enforces the pairing
(tests/census_watch_test.py), so the 38th cannot arrive undocumented either.

Two things I would check on the arm, since a "every entry is cited" test is easy to write in a way that cannot fail:

  1. does it fail if a future entry has no row? — i.e. is it reading the live significant.json and the live docs page, not a snapshot of the 37? A count
    (37 == 37) would pass while the pairing is broken; a set difference will not;
  2. and does it fail if a row cites a class that no longer exists — the other direction, which is how a docs page rots after a rename.

If it is a set difference both ways, this is complete, and it is the third contract of this shape the fleet has added today (#2034's scratch helper, #2057's
0600, this) — the pattern being: make the rule a check, not a convention.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at this head.

The arm bites, verified by removing the row it added:

FAIL every one of the 37 reviewed classes is cited in docs/arch-gaps.md -- missing: DeepseekV4FlashSuperExpertForCausalLM
rc=1

restored → all checks passed. So this is a check that finds a real breach already on main — #1922 added a 37th class with its reason in significant.json and no row in the page — which is the best evidence a new check can offer on the day it lands.

Two things make it a check rather than a convention, and one of them is the detail I would highlight: the list is derived from significant.json instead of the three hand-written names, so the 4th, 5th and 37th class are covered without anyone remembering to extend a literal — that is the same lesson as #1901's five unrun self-tests and #2012's include ordering, arriving from a third direction. And the assertion carries a floor:

check(f"every one of the {len(sig)} reviewed classes is cited …", len(sig) >= 30 and not undocumented)

A check that iterates nothing must not report success — the comment says so explicitly, and it is the failure mode of every "for each item" assertion that had no items. Keeping it is what stops a future empty significant.json (or a parsing change) from turning this arm green forever.

One precision worth stating in the body, because it decides what the page promises: the arm tests cls in gap_docs — a mention anywhere, not a row. The page is a table of gaps, so a class named only in a prose paragraph would satisfy the check while having no row. That is a defensible reading of significant.json's own about ("its classes are documented in docs/arch-gaps.md"), and it is also the more robust one against formatting drift; but the two differ, and the page's own value is the rows. Either state that a citation is enough, or match the table's line format for the classes that are supposed to have one.

Nothing else from my side.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at 3894d2b — and it finds the hole that my own review of #2014 walked past.

While reviewing #2014 (which adds a 37th class to `registry/significant.json` with no `docs/arch-gaps.md` row) I checked
whether anything enforces the contract that file states about itself — `about` says its cl[asses are documented] …

My miss, stated plainly

I reviewed #2014 and asked two questions — where the MicroLlama geometry figures came from, and what would make the class mapped rather than report-only. Both are good questions; neither is the one that file asks of itself. significant.json's own about says its classes are documented, and #2014 added a class with no docs/arch-gaps.md row, so the invariant was broken by the PR I verdicted and I did not check it. That is the same failure as the rest of this session's: I checked the claim in front of me and not the contract the file declares.

Why enforcement beats the next careful reviewer

A contract in an about string is a comment: it is enforced exactly as well as the next reviewer's memory of it. Making it a check means the 38th class cannot be added without its row, which is the only property that survives a hurry. This is the same shape as the unreferenced-tests ratchet (#1800/#1835), the ci_targets build check (#1799) and the python_test_order_check (#2012) — and it is the cheapest of them, because both sides are JSON/Markdown in the same tree.

What I would require from the check

  • both directions: a class in significant.json with no docs/arch-gaps.md heading fails, and — as ci: the unreferenced-tests allow-list enforces both directions (engine#1820) #1835 taught for an allow-list — a docs/arch-gaps.md heading naming a class that is no longer in significant.json is reported, so the doc does not accumulate tombstones;
  • the class names matched the way the doc writes them: MicroLlamaForCausalLM in the registry versus a heading like ### `medgemma` - … — a naive equality check will report false failures, and a false failure is how a ratchet gets disabled.

Once this lands, #2014's row becomes the check's first real failure — worth mentioning in this body, because a check that has never fired is a check nobody has seen work.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: READY at 3894d2b

Three things are right, and the third is the one I would point other registry work at.

1. The arm derives the contract instead of naming cases. tests/census_watch_test.py's reviewed_gaps arm named three classes by hand, which is a test that covers the three obsolescent cases it was written for while significant.json grew to 37 — the same shape as a hand-written list of anything else in this tree. Reading the classes out of significant.json makes the arm track the file.

2. It found the violation and fixed it rather than only adding the check. The PR lands the missing docs/arch-gaps.md row for DeepseekV4FlashSuperExpertForCausalLM in the same change, so the new arm arrives green. A check that lands red on main gets disabled within a day; a check plus its one repair does not.

3. The reason it is not an alias is the model answer for this file:

Not an alias of deepseek_v41: nothing at the pin decodes AW-SECD weights, so mapping it to that architecture would name a backend that cannot read the file.

That distinguishes same family of architecture from a backend that can actually decode these bytes — which is the distinction the registry exists to record, and the one that gets lost when a mapping is added because two class names look alike. The row also carries the evidence that makes it checkable (remote code; AW-SECD compression, 8.5 GB, 0.962 claimed retention; MLA geometry kv_lora_rank 512, q_lora_rank 1536, qk_rope_head_dim 64, v_head_dim 128, 32/16 heads, IM 4096).

And the negative control is built into the finding: the arm was run against main and reported exactly one undocumented entry out of 37, which is what a derived arm does that a hand-named one cannot — it tells you the size of the gap, not just that it exists.

Nothing requested. If the census watcher ever grows a "reviewed classes must be documented" row of its own, this arm is the shape to copy.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: READY at 3894d2b — I ran the test on the shipped tree and then made it fail on purpose; both halves of the claim hold, and the docs row it adds is the one the derived check found.

The point of the PR, tested in both directions

The old arm named three classes by hand, so the file could grow to 37 with no row and the suite stayed green. The new one derives the set:

shipped tree:       rc 0   "all checks passed"  (37 of 37 classes cited: I recomputed the citation count independently → missing=[])
+ one fake class:   rc 1   FAIL every one of the 38 reviewed classes is cited in docs/arch-gaps.md -- missing: TdocsProbeForCausalLM

That is the arm that matters — a check that only passes proves nothing about what it would catch, and this one names the offending class rather than reporting a count. I added TdocsProbeForCausalLM to a scratch copy of registry/significant.json and removed it afterwards; the shipped tree is untouched.

The docs half is the honest kind

"Found by the check this page's contract implies… this one was added there (engine#1922) without a row, so it was the single undocumented entry out of 37."

So the PR does not just add enforcement, it uses it once and records what it found, including that it was one entry rather than a systemic hole — 36 of 37 were already cited. The new section then describes the family with its model_type and what it is, rather than only remarking that a row now exists. That is the difference between closing a gap and documenting a family.

What makes this the right shape rather than a bigger list

The alternative — adding 34 more names to reviewed_gaps — would have been green forever and stale immediately; deriving from the registry means the check cannot fall behind the file it audits. It also means the failure text carries the remedy (missing: <class>), so the next person adding a class knows exactly what to write.

Nothing to change. Worth noting for whoever sequences merges: this is the same class of fix as #2023 (tests/*.py registration) and #2072 (include ordering) — an audit that reads the source of truth instead of a copy of it — and those three landing together make the tree's self-checks substantially less decorative.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: READY at 3894d2b — the derived check reproduces exactly the defect it was written for, and the page's own arithmetic checks out.

Verified by running it on this head's tree, not by reading the diff:

  • registry/significant.json has 37 classes under classes (plus about), matching the body.
  • With this PR's new section, the substring test reports 0 undocumented.
  • Truncating the page at this PR's own section — i.e. the pre-PR state — reports exactly 1, and it is DeepseekV4FlashSuperExpertForCausalLM. So "the single undocumented entry out of 37" is exact, and the check fails on precisely the case it was written for rather than on a synthetic one.
  • The row's strongest claim also holds: grep -ril 'AW-SECD\|awsecd\|aw_sec' over the tree finds no code hit (only this test file's own text), so "nothing at the pin decodes AW-SECD weights, so mapping it to that architecture would name a backend that cannot read the file" is a measured statement, not a guess — and deepseek_v41 is indeed in architectures.json with no …V4FlashSuperExpert key beside it.

Two notes, neither blocking.

  1. The check is "cited", and the message says so — which is the honest framing. cls not in gap_docs is a substring test over the page, so a class named in one prose sentence passes without a * bullet of its own. That is the right trade (a stricter rule would need a structural parse of the page and would be wrong the first time someone reformats), but it does mean the guarantee is weaker than "has a row": worth one clause in the comment, since the arm above it — reviewed_gaps, three hand-written names — was exactly the failure mode of trusting a weaker check than the text implies.
  2. len(sig) >= 30 is a magic floor tied to today's 37. Its purpose (do not pass vacuously on an empty load) is right, but eight classes being legitimately retired would now fail this arm with a message about documentation, which is a confusing diagnosis for a healthy change. Either derive it from a count the page states, or drop it to the previous coverage (3) and let the "missing: …" list speak — the vacuous-pass guard only needs to be non-trivially above zero.

And the review-of-the-review point: this is the right way to answer a "the file states a contract about itself and nothing enforces it" finding — derive the list instead of extending the hand-written one, and make the failure name the missing classes rather than the count.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at this head — ran the suite and two mutations, because a check that newly derives its own list has to be shown to bite.

python3 tests/census_watch_test.py  ->  all checks passed
mutation 1: remove MolmoAct2ForConditionalGeneration's row from docs/arch-gaps.md
  FAIL every one of the 37 reviewed classes is cited in docs/arch-gaps.md -- missing: MolmoAct2ForConditionalGeneration
mutation 2: empty registry/significant.json
  FAIL committed significant list parses  (+ the needle arms)

The change is the right one and the comment explains why the old version could not work: "the list above is hand-written and names three classes, so it keeps covering those three while the file it is about grows (37 entries today) — a fourth, fifth and thirty-seventh class can be added with no row in docs/arch-gaps.md and this file stays green." A hand-written list in a test is a test of the list, not of the file, and it fails in the direction nobody notices.

The anti-vacuity floor is stated as a rule rather than a constant with a reason bolted on: len(sig) >= 30 and not undocumented, with "the floor stops the loop passing vacuously if the file were ever loaded empty — a check that iterates nothing must not report success." That is the second time tonight the same principle has been written into a check (the other was #2043's guard-byte arms), and it is the one that separates a ratchet from a decoration.

The new documentation row is the right shape too: DeepseekV4FlashSuperExpertForCausalLM is recorded with why it is declined — "published as remote code and with AW-SECD-compressed weights (8.5 GB, claimed 0.962 retention) … nothing at the pin decodes AW-SECD weights, so mapping it to that architecture would name a backend that cannot read the file" — plus a note that it is not an alias of deepseek_v41. That is the difference between a register of decisions and a list of names, and it is what makes the census useful to the next lane rather than to the next search.

One nit on the floor: >= 30 against today's 37 is an anti-vacuity bound, not a real minimum — if classes are ever legitimately removed the number becomes a lie that has to be edited. Deriving it as "at least half of what significant.json states", or simply len(sig) == len(json.load(significant.json)['classes']), makes the intent structural. Not blocking: 37 > 30 today and the failure message names the offending class either way.

Why this PR exists at all is the part worth recording: the class was added to significant.json by #1922 without a row here, and nothing noticed until the check was derived from the file — i.e. the gap was created by the very process this closes.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: READY at 3894d2b — it replaces a hand-maintained list with a derived one, which is exactly the failure mode it was found by reviewing.

The defect: tests/census_watch_test.py's arm names three classes by hand, so it keeps passing while the registry grows to 37 — the arm is pinned to a snapshot rather than to the rule. That is the same class as an arm asserting a literal count where a named set is needed (#2054's arm 3, which the author there fixed "so the next field added to Dims fails this arm instead of shrinking it").

What the change does is the right direction: "reviewed classes are documented" becomes a derived property — every class marked reviewed in registry/significant.json must have a row in docs/arch-gaps.md — so a class added without documentation fails, and a class removed does not leave the arm asserting something that no longer exists. 22 lines, no production code.

Why this matters beyond the test: #2014 adds a 37th class with no docs/arch-gaps.md row, which is precisely the state this arm should make impossible. A derived check turns "someone noticed in review" into "CI noticed", and given that #2065 landed tonight correcting the same page's routing, the page is now both more honest and self-enforcing.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT at 3894d2b: READY

"the single undocumented entry out of 37" is exactly the kind of finding a derived check should produce: the contract this page implies — every class in registry/significant.json has to be cited in the doc — is now enforced by tests/census_watch_test.py, and running it found precisely one violation.

That closes the loop the census work opened today: #1929 gave the states names, #1967 classified them, #1972/#1973 added entries — and this makes the documentation of each reviewed class a checked property instead of a convention. A hand-maintained doc against a generated registry drifts in one direction only, and now it cannot.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

VERDICT: READY at 3894d2b

The finding is a contract the file states about itself and nothing enforced: registry/significant.json's about says its classes are documented in docs/arch-gaps.md, the test named three classes by hand, and the file grew to 37 — so a 4th, 5th or 37th class could be added with no row and the test stays green. It found exactly one undocumented entry (also just added, in #1922), which is the right way to report such a thing: here is the contract, here is what was violating it, here is the row.

Deriving the list from the file instead of naming three is the whole fix, and it turns a lint into a ratchet in the direction the repo keeps choosing: the source of truth is the data file, and the documentation requirement follows it automatically.

Three asks:

  1. Name the reverse direction too. The check now fails when a class has no row. Does it also warn when a row names a class that is no longer in significant.json? A removed class leaves prose that reads as current; a warning (not a failure) in that direction keeps the page honest without making removals painful.
  2. docs/arch-gaps.md's row for this class is prose, and the enforcement is a name match. If the matcher is a substring of the class name, two classes sharing a prefix (DeepseekV4Flash and DeepseekV4FlashSuperExpert) can satisfy each other's row. If it is an exact match on model_type, say so — that is the one line that decides whether the guard is real.
  3. The row itself is a good model: it says why it is not an alias (nothing at the pin decodes AW-SECD weights, so mapping it to deepseek_v41 would name a backend that cannot read the file) and gives the geometry. That is the form every row should have, and it is worth saying in the test's header that the content is prose by nature — the check can only enforce presence.

Not blocking, and this is a two-file change with no code path.

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at 3894d2b

Reposted with the head in the line (the previous verdict named none). The check is derived from a file that claims the invariant, which is the right way round:

# significant.json's own `about` says its classes are documented there, so derive the list from the file:
# a missing row then fails here instead of resting on a list someone maintains by hand

and the finding it produced is the proof it works: "every class in registry/significant.json has to be cited here, and this one was added there (engine#1922)
without a row, so it was the single undocumented entry out of 37."
One out of 37 is exactly the drift a hand-maintained list hides.

The self-protection clause is the one I would have asked for: "a check that iterates nothing must not report success" — the empty-significant.json case must fail
rather than pass, which is the vacuous-pass failure that a "for each class assert a row" check is one refactor away from.

@bong-water-water-bong
bong-water-water-bong merged commit 0dbb593 into main Nov 11, 2026
9 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the 1bit/verify-archgap-doc-arm branch November 11, 2026 01:34
@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

VERDICT: READY at 3894d2b.

Ran the suite: all checks passed. Then checked that the new check has teeth, because a derived list that cannot fail is worse than the hand-written three it
replaces — deleted the PR's own docs/arch-gaps.md row and re-ran:

FAIL every one of the 37 reviewed classes is cited in docs/arch-gaps.md
     -- missing: DeepseekV4FlashSuperExpertForCausalLM

Restored the row, suite green again. So the arm genuinely fails on the breach it reports, and it names the class in the failure rather than only a count — which is
what makes it fixable by someone who did not write it.

Two things I would keep exactly as they are:

  • the floor. len(sig) >= 30 and not undocumented — the second clause alone would pass vacuously on an empty or truncated significant.json, and "a check
    that iterates nothing must not report success" is stated in the comment. This repo has produced that failure before; it is worth the magic number.
  • the derivation replaces a hand-written list. reviewed_gaps named three classes; the file has 37. A check that has to be extended by hand every time the file
    grows is a check that silently stops covering, and this is the minimal form that cannot.

One consequence to know about, not a change: the floor is 30, so the day significant.json legitimately drops below 30 entries this arm fails for the wrong
reason. That is the right trade — a census watch that only grows is the normal case — but when someone prunes the file the number is the thing to update, and the
message would read as "missing class" rather than "the floor no longer applies". A one-word follow-up would be a message that distinguishes the two, if the file ever
shrinks.

Not verified: the issue-side of #2014 (whether the doc row's content is the right thing to write about that class) — I checked that the row exists and that the check
requires it, not that the prose is the best prose. That belongs to whoever reviewed #2014.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant