Skip to content

Three imports the regrounding left behind: declared, never referenced - #9493

Merged
briansrls merged 3 commits into
mainfrom
cleanup/capture-dead-imports
Aug 27, 2026
Merged

briansrls merged 3 commits into
mainfrom
cleanup/capture-dead-imports

Conversation

@gunbai-bot

@gunbai-bot gunbai-bot Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

When `legacy_baseline_capture` was rewritten to ground its base identity on `extdeps.git.object_store` rather than compute a content address of its own, that deleted every use of three imported names without deleting the imports. Measured on merged main, each occurred exactly once in the module — at its own import site:

name occurrences in module
`Fnv1a64Structural` 1 (the import)
`LegacyObservationAdmission` 1 (the import)
`legacy_subject_identity` 1 (the import)

Raised as a non-blocking remark on #9440 by claude/claude-opus-4-7 (review 56719). I verified it against the head rather than taking the remark, and re-verified against merged main before deleting, because the file passed through a landing in between.

Why nothing caught it. Nothing refuses an unused import, so no dependent could speak up — and the shape is the one this lane keeps running into: removing the last use of a name leaves the import well-typed, resolvable and quietly pointless. A deletion announces itself through its dependents; an addition or a removal-of-uses falsifies the claim "this import is used" silently.

All three symbols stay live elsewhere in the corpus — Fnv1a64Structural in ~150 files, legacy_subject_identity in eight, LegacyObservationAdmission in two. Only this module's references to them were dead, so this is a local reference deletion and not a symbol retirement.

Green by execution, not by inspection. The capture's witness suite runs 8/8 on this branch with the imports removed:

PASS a_materialized_head_that_is_not_the_requested_commit_refuses_first
PASS a_dirty_worktree_refuses_before_the_subject_is_compared
PASS an_observation_of_a_different_source_set_refuses_as_subject_mismatch
PASS an_observation_from_a_different_compiler_artifact_refuses_as_subject_mismatch
PASS an_observation_missing_an_expected_occurrence_refuses_as_incomplete
PASS a_fully_agreeing_capture_still_refuses_because_the_repair_envelope_is_absent
PASS the_reported_capability_is_blocked_at_the_repair_census
PASS an_uppercase_commit_spelling_has_no_git_object_id_at_all

One note on that run, since it is the same class as the finding: my first attempt reported five FAILs, all of them no-such-function — I had typed the witness names from memory. That is the instrument erring, not the subject, and the names were read from the file before the run above. A FAIL whose cause is "this declaration does not exist" is not evidence about the code under test.

What this deliberately does not do. The same review also flagged ObserverNeverReachedDecision as a refusal variant with no producer. It is kept. Its state is producible by a governing authority — an observer terminating before its decision stage is what the repair-census phase reports once the repair envelope exists — and that envelope is the declared hole #9447 fills, so the arm is quiet on a schedule rather than dead. Deleting it would be reachability read as occupancy: removing a live wall while looking principled. An unused import has no mechanism that could ever produce a use; an unused refusal arm has one that is merely not built yet. Same surface, opposite verdicts.

gunbc-ci-auto-heal and others added 2 commits August 27, 2026 18:38
`legacy_baseline_capture` was rewritten to ground its base identity on
`extdeps.git.object_store` instead of computing a content address of its
own. That deleted every use of `Fnv1a64Structural`,
`LegacyObservationAdmission` and `legacy_subject_identity` without
deleting the imports, so each name occurred exactly once in the module --
at its own import site.

Nothing refuses an unused import, which is why no dependent spoke up. It
is the additive shape: removing the uses left the imports well-typed and
unreferenced, and a claim that an import is used is falsified silently
when its last use goes away.

The three symbols stay live elsewhere in the corpus; only this module's
references to them were dead.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gunbai-bot
gunbai-bot Bot force-pushed the cleanup/capture-dead-imports branch from 65a1ffb to 5567444 Compare August 27, 2026 18:38
@gunbai-bot

gunbai-bot Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

The failing check is main's breakage, not this PR's, and I verified that rather than assuming it.

This branch changes one import block in one file — 1 insertion, 4 deletions, no semantic edit. The floor and declarations phases fail on a module it does not touch:

dag/product/fabric/contention.dag:23:19  name 'grant_duration_seconds' not found in module
                                         'product.fabric.supply' (imported by 'product.fabric.contention')
contention.dag:280:3                     non-exhaustive match: missing variant(s) UnobservedGrantDuration
contention.dag:421:3                     non-exhaustive match: missing variant(s) QuoteNotPriceableWithoutDuration
contention.dag:463:30, 469:43            function 'grant_duration_seconds' not found in scope

Measured on main rather than inferred from the message: contention.dag references that name four times, supply.dag declares it zero times, and its only surviving occurrence there is a comment reading "It was called grant_duration_seconds and returned a bare". The rename landed in b20ac7ad44f (#9397) without its importer following; the two missing match arms are that same change's new variants.

Owned and already repaired elsewhere: warm-hawk-909's #9488 carries the fix (build lane green, floor in flight). I am deliberately not repairing it here — #9397 made the grant duration unknowable, so those two arms require real decisions about what contention does when the duration is unobserved and when a quote is not priceable without it. Choosing those to turn CI green would be inventing policy for a lane that owns one, and contention.dag's own header states the match is deliberately non-exhaustive precisely so a new arm forces that decision.

One earlier failure on this PR was genuinely mine and is fixed: git add -A swept in a stray 0-byte file acc, amended out. A second error in that first run — a duplicate declaration in fleet_fan_wiring_witness.dag — was fixed on main by #9497 and cleared when I merged latest main.

Both runs were compared, not assumed equal: this run reports the same five contention sites and the same single declarations finding as the previous one, with the fleet_fan_wiring_witness error gone. So the cause is unchanged upstream breakage rather than anything the merge introduced.

Nothing to push here. This goes green when #9488 lands.

— sent from crisp-crab-430

@briansrls
briansrls merged commit c9ae303 into main Aug 27, 2026
3 checks passed
@briansrls
briansrls deleted the cleanup/capture-dead-imports branch August 27, 2026 22:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant