Repository navigation
Import-strip measurement cleanups: classifier raw-population repair and harness removal - #8079
Conversation
…ssed it THE DEFECT. `classify_rows` built `candidates` as a SET of modules and passed it into `convention_holds`, whose whole job is to decide whether a convention name is declared at most once per module. The set had already made that true by construction, so two declarations of a convention name in one module would have been silently excused as a convention instead of landing in `duplicate_unclassified`. Raw population and display population are two different facts; the repair separates them rather than swapping one for the other, so the ledger's candidate columns stay distinct-module. THE CONTROL THAT MISSED IT IS THE MORE IMPORTANT HALF. #8067 shipped a control named "convention name violating one-per-module is not excused" and it PASSED against the defective code, because it called `disposition()` directly with a duplicated list — bypassing the deduplication the production path performs before `disposition()` is ever reached. That is the same class the same PR had just repaired one layer down: a test of the predicate is not a test of the behaviour. So the loop is extracted from `main()` into `classify_rows()` — not for tidiness, but because the production seam was unreachable, which is why no control could have caught this. Four new controls go through it. The gap is demonstrated rather than asserted: with the repair reverted, the direct-call control still reports ok while the production-path control fails. NOT A CORRECTION TO THE MERGED MEASUREMENT, and this is checked rather than argued: regenerating the ledger from the merged receipts with the repaired classifier reproduces `docs/plans/import-strip-residual-ledger.tsv` BYTE-IDENTICALLY (3,160 = 10 + 3,150). At the measured tree the two named convention populations really are one-per-module — 497 and 94 declarations across 497 and 94 distinct modules — so the collapsed check happened to agree. The 152→0 close stands unchanged. Commit 1 of the import-deletion lane, per operator direction: this classifier is scheduled to delete when production closure lands, so it does not get its own PR.
Checkpoint 2 — scoping finding, before buildingI surveyed the existing carriers rather than assuming the plan's shape, and the gap is narrower and differently placed than "no production producer". Recording it because it changes what checkpoint 2 has to build. What already exists in production
That substrate exists because of a constraint worth restating, since it rules out the obvious implementation: What is actually missing is therefore only the production driver at the file boundary:
Evidence for "no production producer" as a starting quantity: every construction of Two named debts adjacent to this that I am not silently absorbing: — sent from calm-ram-435 |
…uling) The three scripts under docs/plans/import-strip-measurement/ are deleted, and their .gitignore negation entries are removed at the gunbc.gitignore_authority single authority with .gitignore regenerated from it — not hand-edited. WHAT THIS COSTS, STATED RATHER THAN LEFT STANDING. #8067's README claimed the measurement regenerates byte-identically from a clean checkout, and shipped the scripts to make that claim true. With them untracked the claim is FALSE, so it is withdrawn: the receipts cannot be regenerated from a fresh clone, the recipe is retitled as a record of what was executed rather than as runnable instructions, and the one citation of the controls in the cascade diagnosis now says the harness is untracked instead of pointing at a path that no longer resolves. A stale pointer to a deleted file is the positional-citation rot DESIGN §3 names; leaving the reproducibility sentence would have been worse, because it is a claim about evidence rather than a pointer. What survives is what a reader can still check: the committed receipts, and the corpus-hash anchor that decides whether they describe the tree in front of them. This does not change any measured figure. The hygiene close (152 -> 0) and the merged reconciliation (3,160 = 10 + 3,150) were established by execution before the harness was deleted; deleting the instrument does not retract the reading. The classifier's dissolution trigger has not moved either — it was always due to delete when B2 lands OrdinaryLoadedCompilationClosure production. This just pays that cost now, and loses re-runnability in exchange for not carrying a parallel text-scanning classifier in the tree.
gunbai-bot
Bot
changed the title
|
Rescoped per the roadmap owner's ruling: retitled, body rewritten to the two cleanups, deletion vertical explicitly excluded and parked pending post-stabilization operator approval. Diff is 9 files confined to Staying draft deliberately, for a reason that will clear on its own. This branch has merged main, so it inherits main's red: It flips to ready once #8082 lands and this branch picks up the healed main. Nothing else is pending on it. — sent from calm-ram-435 |
…nt> is declared (#8082) Main is RED at 837cc99 (#8071). dag_compile_clean_gate_passes fails with: roadmap_repo_atlas_sandbox.dag:205:3: error: if branches resolve to incompatible types: Container(List,Primitive(Fragment)) vs Container(List,Primitive(Element)) `Element` is a VARIANT of `Fragment`, so a bare `Element { .. }` literal coerces fine at return position — which is why the sibling `atlas_structure_plane` compiles — but inside a list literal it types as `List<Element>`, while the `then` arm's `[]` takes `List<Fragment>` from the declared return type. Repaired by using `el_class`, which the module already imports and which returns `Fragment`. EMISSION IS UNCHANGED, and that is checked rather than assumed: the module's local `sattr` is a pass-through to `std.markup` `attr` (`fn sattr(name, value) -> Attribute { attr(name: name, value: value) }`), and `el_class` builds exactly `attr(name: "class", value: cls)`, so the produced attribute is identical. MEASURED, not inferred from the diff: compiling unmodified `origin/main` produces 53 hard diagnostics; this tree produces 52, and the single difference is this error. Every other diagnostic (32 `undefined variable 'Empty'` in complexity_accumulator_copy/analyze.dag, 16 annotation-grain in host_phase_status_witness_test.dag, 4 in fleet_converge_cli.dag) is present on main unchanged and is not touched here. Split out of #8079 deliberately: that PR is under an operator HOLD pending classification, and a red main blocks every lane including the stabilization work the hold exists to protect. A heal should not wait behind a held PR. Co-authored-by: gunbc-ci-auto-heal <gunbc-ci-auto-heal@users.noreply.github.com>
Correction to my checkpoint-2 scoping comment aboveThat comment is wrong in a way that would cause real damage if a future worker followed it, so I am correcting it rather than letting it stand as the last word on this thread. What I wrote: "I found no Why it is wrong: I grepped for a shape (
So: do not add a filesystem enumeration or reader operation. Doing so would mint a third source-population authority beside the compiler ingest and the module-graph census — a §3 fork, and exactly the kind this lane exists to remove. A second correction, in the other direction. I also wrote that "only the file-boundary driver is missing". That was too optimistic. Reading files is not the hard part; choosing a provider population without already relying on imports is. It is still valuable — as the exact current-path control, the source-content carrier, and the old-vs-new differential oracle — but it cannot be the terminal mechanism. And the edge producer must be replaced, not joined. What stands from the original comment is the substrate inventory: per-file parse to None of this changes what is in this PR — no vertical work is starting under the hold. It corrects the record so the parked plan is right when it is unparked. — sent from calm-ram-435 |
|
Picked up healed main (#8082, The inherited atlas red this branch was failing on is gone at the base — the Now opted into auto-reviews and CI on — sent from calm-ram-435 |
…thdrawal
review 50839 (REQUEST_CHANGES) found the document contradicting itself: I added a
withdrawal section saying the receipts cannot be regenerated from a clean
checkout, and left three earlier passages asserting the opposite — the title
("reproducer"), the head paragraph ("Everything below regenerates from a named
tree"), the scaffold block describing the scripts in the present tense as though
still tracked, and the closing line "everything the measurement asserts is
byte-reproducible". A reader met the regenerability claim BEFORE the withdrawal.
That is the same defect class this PR exists to fix, one level up: an appended
correction is not a correction if the original claim survives above it. It is
also the rung-honesty failure DESIGN §5 names — the document reported a stronger
standing than the tree supports.
Reconciled rather than annotated:
- title is "retained execution record", not "reproducer";
- the head now states the frozen-record fact FIRST, before the recipe, so the
ordering the review objected to is inverted;
- the scaffold block is past tense and says the scripts were removed EARLY, with
the declared dissolution trigger (B2 landing OrdinaryLoadedCompilationClosure
production) explicitly recorded as NOT yet fired — an honest early removal, not
a trigger silently treated as satisfied;
- the byte-reproducibility passage is scoped to what it established at the time
and followed by what a reader can check now without the harness: the
corpus-hash anchor.
Swept the whole file for the class rather than fixing the three cited lines: no
present-tense regenerability claim remains.
|
Fixed on Reconciled rather than annotated:
I swept the whole file for the class rather than patching the three cited lines. No present-tense regenerability claim remains; the only surviving — sent from calm-ram-435 |
…laim review 50840 (REQUEST_CHANGES): §15.1 still called the measurement a "full reproducer" with scripts "committed" and "the deterministic command sequence that regenerates every number below", so the two authorities disagreed about whether the measurement is regenerable — the README said no, the diagnosis said yes. This is the SECOND time in this PR I fixed the cited location and left the class alive elsewhere: c8710bf swept the README and patched exactly one line of the diagnosis. Sweeping one authority is not sweeping the claim, and a §15 reader got the old standing from the canonical design-linked document. Recorded plainly because the pattern, not the line, is the defect. §15.1 now states that the directory is a retained execution record, that the numbers cannot be regenerated from a clean checkout, and — the part the review did not ask for but honesty does — that the harness was removed EARLY, before its declared dissolution trigger fired. The trigger (B2 landing OrdinaryLoadedCompilationClosure production) is named as NOT fired, so nobody can read the deletion as evidence that B2 landed. What remains checkable without the harness is named: the corpus-hash anchor. Swept the whole tree this time rather than the cited file: the only two documents referencing the measurement are this one and the measurement README, and they now agree.
|
Fixed on §15.1 now states that One addition beyond the finding, because the omission would mislead: it records that the harness was removed early, before its declared dissolution trigger fired, and names that trigger (B2 landing This time I swept the tree rather than the cited file. The only two documents referencing the measurement are the diagnosis and the measurement README, and they now agree; no — sent from calm-ram-435 |
1 participant
Remove the disallowed measurement harness, and truthfully downgrade its historical receipt from a reproducible experiment to a retained execution record.
That is the durable change. Everything else here is history.
What lands
The three Python scripts under
docs/plans/import-strip-measurement/are deleted (operator ruling: they may not live in the repository), and their.gitignorenegation rows are removed at thegunbc.gitignore_authoritysingle authority with.gitignoreregenerated from it — not hand-edited.The claim they backed is withdrawn rather than left standing. #8067's README said the measurement regenerates byte-identically from a clean checkout, and shipped the scripts to make that true. Untracked, that is false. So the recipe is retitled as a record of what was executed rather than runnable instructions, and the one citation of the controls in the cascade diagnosis now says the harness is untracked instead of pointing at a path that no longer resolves. A stale pointer to a deleted file is the positional-citation rot DESIGN §3 names; leaving the reproducibility sentence would have been worse, because it is a claim about evidence.
What a reader can still check: the committed receipts, and the corpus-hash anchor that decides whether they describe the tree in front of them.
The classifier repair is history, not capability
Commit 1 repairs a real fail-open in the classifier's one-per-module check, and the finding is worth keeping — the control that was supposed to catch it passed against the defective code, because it called the predicate directly and bypassed the deduplication the production path does first. A test of the predicate is not a test of the behaviour.
But the file it repairs is deleted two commits later in this same tree. It is investigation history, not a shipped capability, and this PR does not claim otherwise. It is retained because the #8067 verification verdict asked for it as commit 1 of this PR rather than as its own lane, and because the finding outlives the file.
It corrects no measured figure: regenerating from #8067's receipts with the repaired classifier reproduced the ledger byte-identically (3,160 = 10 + 3,150). At the measured tree the two convention populations genuinely are one-per-module, so the collapsed check happened to agree.
The receipts are now FROZEN
docs/plans/import-strip-measurement/receipts/andimport-strip-residual-ledger.tsvare retained as a historical execution record of a specific tree, and are not to be rebased or refreshed as main advances. I regenerated them twice during #8067 to keep the corpus-hash anchor matching HEAD; that was the right call while the harness existed and the numbers were the deliverable, and it is the wrong call now. Chasing main with a deleted instrument would be maintenance with no oracle behind it.The anchor stays self-verifying, so a future reader gets a truthful answer — these receipts describe a different tree than yours — rather than a silently stale number. Production binding observations replace this measurement system; they do not get a refreshed TSV.
Deliberately excluded: the import-deletion vertical
Per operator directive F and the roadmap owner's rescope ruling, no resolver, loader, emitter or grammar work is in this PR. The deletion vertical is parked and starts only on explicit operator approval post-stabilization.
The standing is unchanged and stays stated: the permanent closing authority reports all six A–F admissions
Unavailable, no import is deleted here, and the 3,150-row strip-attributable residual stands as measured.Diff is nine files, confined to
docs/plans/and the.gitignoreauthority.Notes