Skip to content

Sweep the ArtifactIdentity rename into its remaining consumer - #8187

Merged
briansrls merged 1 commit into
mainfrom
session/gentle-dove-143-artifactidentity-sweep
Aug 12, 2026
Merged

briansrls merged 1 commit into
mainfrom
session/gentle-dove-143-artifactidentity-sweep

Conversation

@briansrls

@briansrls briansrls commented Aug 12, 2026 •

Copy link
Copy Markdown
Contributor

main is red; this is the sweep

#8177 renamed v2.compiler.self_host.generation's ArtifactIdentity to GeneratedArtifactIdentity and swept the consumers that existed when it was written. #8176 had added another one two minutes earlier, so the rename left it importing a name that no longer exists:

dag/gunbc/self_host_artifact_materialization.dag:32:3:
  error: name 'ArtifactIdentity' not found in module 'v2.compiler.self_host.generation'
         (imported by 'gunbc.self_host_artifact_materialization')

Three lines in one file: the import, artifact_identity_of's return type, and one prose mention citing the retired symbol (a stale citation is the DESIGN §3 class — the name no longer resolves, so leaving it rots silently).

Evidence — both directions

verdict
pre-sweep (main today) exit 1 — name 'ArtifactIdentity' not found in module …generation
post-sweep exit 0 — PASS a_failed_build_does_not_materialize_even_with_a_good_digest

The two witness files need no change, and that is deliberate rather than an omission: they call artifact_identity_of qualified and match on ArtifactMaterialized / ArtifactNotMaterialized, which are variant names a type rename does not touch.

Corpus scan: no other reference to the renamed symbol remains. The surviving ArtifactIdentity uses are std.cache_interface's unrelated ArtifactIdentity<T> and spark_serving_release's own RuntimeArtifactIdentity / ModelArtifactIdentity — distinct concepts sharing a word, which is what allowed the original collision.

Attribution, stated because it is not a fault in either PR

#8176 and #8177 were each correct against their own base and each passed their own CI. The break exists only in their composition, so no affected set either author could compute would have shown it. This is the fourth instance today of one mechanism — a change validated against the module it edits, breaking modules it does not touch (#8129, #8148, #8153, #8177). The operator is root-causing the mechanism separately; this PR only unbreaks main.

Credit: the diagnosis is the press-0 session's, from ec5a259283fa. Their fix lives inside #8132; this lands the same sweep minimally against main so main's red does not wait on that stack.

main is red. #8177 renamed `v2.compiler.self_host.generation`'s
`ArtifactIdentity` to `GeneratedArtifactIdentity` and swept the consumers
that existed when it was written. #8176 had added another one two minutes
earlier, so the rename left it importing a name that no longer exists:

  dag/gunbc/self_host_artifact_materialization.dag:32:3:
    error: name 'ArtifactIdentity' not found in module
           'v2.compiler.self_host.generation'

Three lines in one file: the import, `artifact_identity_of`'s return type,
and one prose mention that cited the retired symbol (a stale citation is
the class DESIGN §3 names -- the name no longer resolves, so leaving it
would rot silently).

The two witness files need no change: they call `artifact_identity_of`
qualified and match on `ArtifactMaterialized` / `ArtifactNotMaterialized`,
which are VARIANT names a type rename does not touch.

Discriminating control, both directions:

  pre-sweep  -> exit 1, name 'ArtifactIdentity' not found in module
  post-sweep -> exit 0, PASS a_failed_build_does_not_materialize_even_with_a_good_digest

Corpus scan: no other reference to the renamed symbol remains. The
surviving `ArtifactIdentity` uses are `std.cache_interface`'s unrelated
`ArtifactIdentity<T>` and `spark_serving_release`'s own
`RuntimeArtifactIdentity` / `ModelArtifactIdentity` -- distinct concepts
that share a word, which is what allowed the original collision.

Not fixed here: #8176 and #8177 were each correct against their own base
and each passed their own CI. The break exists only in their composition,
so no affected set either author could compute would have shown it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XxLzhuMisV3GAPpP2mBtSG
@gunbai-bot gunbai-bot Bot changed the title main is red Sweep the ArtifactIdentity rename into its remaining consumer Aug 12, 2026
@gunbai-bot
gunbai-bot Bot marked this pull request as ready for review August 12, 2026 11:06
@briansrls
briansrls merged commit b33a9c0 into main Aug 12, 2026
1 of 6 checks passed
@briansrls
briansrls deleted the session/gentle-dove-143-artifactidentity-sweep branch August 12, 2026 11:13
gunbai-bot Bot pushed a commit that referenced this pull request Aug 12, 2026
Brings in the ArtifactIdentity collision repair (#8177 + #8187) that main's
whole-tree compile-clean was red on, plus the intervening merges. This PR's red was
that inherited defect, not its own content: the identical five `unresolved type
'ArtifactIdentity'` errors appeared on a sibling PR containing no `.dag` change at
all, and then on main itself at #8146.

DESIGN.md was the only conflict, and it is resolved to main's copy deliberately:
it is a PROJECTION of `dag/gunbc/design_document.dag`, which merged cleanly. Hand
-merging a generated artifact would author bytes no authority produced; the
`heal_generated_artifacts` job re-projects it from the merged authority, as it
already did on this branch in f7ddaef.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018ZrxLJrt8ASGLegiK72fAi
gunbai-bot Bot pushed a commit that referenced this pull request Aug 12, 2026
…elpers

`cargo check -p v1-compiler --all-targets` does not compile on main:

    error[E0425]: cannot find function `with_env_test_lock` in this scope
    error[E0433]: unresolved module `floor_discovery_snapshot`
    error[E0433]: use of undeclared type `EnvGuard`

`coordinated_consumer_refuses_corpus_without_verified_snapshot` sits in
`discovery_summary_merge_tests`, which closes before the helpers it calls:
`with_env_test_lock` and `EnvGuard` are defined in the SIBLING module
`witness_layer_roots_compile_clean_tests`, and nothing imports them across. The
test is moved to that module, where every symbol it uses is already in scope, and
`ExecutionMode` is qualified rather than widening the destination's import list.
No assertion, name, or behavior changes.

WHY IT LANDED, and why this is worth more than the three-line fix. Nothing in CI
compiles `#[cfg(test)]` code any more: clippy came out 2026-07-08, nextest
2026-07-11, and gunbc#8146 retired the v1 Rust test suite today. `build` compiles
the binaries, not the test targets. So a unit-test module can be broken on main and
every gate stays green — this one is, and was inherited by this branch through an
ordinary main merge rather than authored here.

This is the same shape as the ArtifactIdentity collision repaired hours ago in
gunbc#8177/#8187: a defect invisible to the checks that run, surfacing only when
someone happens to execute the wider thing. Reported rather than silently absorbed;
the gap itself — no CI consumer for test-target compilation — is not closed by this
commit and is left named.

Green by execution: `cargo check -p v1-compiler --all-targets` finishes clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018ZrxLJrt8ASGLegiK72fAi
gunbai-bot Bot pushed a commit that referenced this pull request Aug 12, 2026
This branch's ci was red only because main carried a broken import between
#8176 and #8187; the witness corpus here ran fully green.
briansrls pushed a commit that referenced this pull request Aug 12, 2026
* Delete the two module-graph facts fields the discovery snapshot carried with no reader

Cut 4 begins with the field census the ruling requires: for each field of
`ModuleGraphFactsSnapshot`, the exact live reader, not a plausible one. Seven of
nine have a production reader (`nodes`, `adjacency`, `selection_adjacency`,
`reference_unaccounted`, `declared_paths`, `path_to_module`, `read_refusals`);
the table is recorded in the `floor_discovery_snapshot` module doc beside the
struct it describes. Two have none, and this change deletes them.

`edges` is consumed only as a local inside `build_module_graph_facts_live_uncached`,
where it builds `adjacency` and `selection_adjacency` and is then dead. Its sole
reader on the struct was `import_closure_bfs_vs_fixpoint_perf_receipt`, an
`#[ignore]`d manual receipt timing `import_closure_from_facts_fixpoint_legacy`, a
legacy fixpoint that exists only inside that test. At the corpus's ~17.4k import
lines it was the payload's largest term — one `{path, import_module,
target_declared}` row per edge, serialized by the producer, hashed into
`module_graph_facts_digest` and the payload digest, then decoded and reconstructed
by every scoped child. The receipt and the legacy helper are deleted with it.

`observed_paths` is the ~3.5k-path source inventory. The fail-closed job its
comment claimed — a vanished module must not be indistinguishable from an absent
one — is done by `read_refusals`, which is what `refuse_on_module_graph_read_refusals`
actually reads. The inventory itself is produced and asserted on at
`ImportResolutionObservation`, one call above, and that is unchanged; the copy
retained on the facts struct was read by nothing but tests.

DECLARED SCOPE LOSS: no consumer can ask the facts value for the raw edge list or
the source inventory any more. Both remain available at the observation.

NOT CLAIMED: that the seven surviving fields are consumed by the coordinated
scoped child. A production reader anywhere is a different fact from a read on that
path, and the difference is exactly what Cut 4 is for; the module doc says so
rather than letting the table imply it. No cost claim is made here — the acceptance
bar for that is `build_module_graph_facts_live_uncached` reaching zero calls at the
deleted discovery phase, which is a runtime measurement and is not in this diff.

Green by execution: `cargo check -p v1-compiler --all-targets`;
`cargo test -p v1-compiler --lib floor_discovery` 7 passed;
`cargo test -p v1-compiler --lib module_graph` 3 passed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018ZrxLJrt8ASGLegiK72fAi

* Delete the dead path_to_module wrapper and the per-entry map rebuild beside it

Two follow-ons found by continuing the census past the two dead fields.

`reference_only_direct_import_modules` has zero callers. It was the reader the
field census cited for `path_to_module`, so that row cited dead code; the field is
genuinely live, but through a different reader — the discovery witness run loop,
where an entry with no module identity refuses rather than fabricating a
`DeclarationRef`. Wrapper deleted, struct comment corrected to name the real
readers. `reference_only_direct_import_paths` (2 live callers) is untouched.

`declared_source_refs_axis_for_entry` rebuilt `path_to_module` from `facts.nodes`
on every call, and it is called per ENTRY on the selection path — an O(corpus) map
allocation per question against the identical map the facts build already
precomputes once and hands it in `facts`. It now reads the retained field, and
`path_to_module_from_declaration_facts` is deleted with its last caller. DESIGN §6
bare-minimum cost: a proven cost-shape defect is fixed regardless of realized n.

Green by execution: `cargo check -p v1-compiler --all-targets`, `cargo fmt --all --check`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018ZrxLJrt8ASGLegiK72fAi

* Point the census table at path_to_module's surviving readers

Review 51294: the table row still named `reference_only_direct_import_modules`,
which the previous commit deletes as dead code. A citation naming a symbol the same
diff removes is the §3 stale-citation class landing inside the census whose whole
purpose is to name exact readers — caught by review rather than by the corpus,
because a doc-comment name is not reachable from the `Node` tree any lens walks.

The surviving readers are the discovery witness run loop, where an entry with no
module identity refuses rather than fabricating a `DeclarationRef`, and
`declared_source_refs_axis_for_entry`. The struct comment in `cli_run.rs` was
already corrected in that commit; this is the copy in the module doc that was not.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018ZrxLJrt8ASGLegiK72fAi

* Move the coordinated-consumer test into the module that defines its helpers

`cargo check -p v1-compiler --all-targets` does not compile on main:

    error[E0425]: cannot find function `with_env_test_lock` in this scope
    error[E0433]: unresolved module `floor_discovery_snapshot`
    error[E0433]: use of undeclared type `EnvGuard`

`coordinated_consumer_refuses_corpus_without_verified_snapshot` sits in
`discovery_summary_merge_tests`, which closes before the helpers it calls:
`with_env_test_lock` and `EnvGuard` are defined in the SIBLING module
`witness_layer_roots_compile_clean_tests`, and nothing imports them across. The
test is moved to that module, where every symbol it uses is already in scope, and
`ExecutionMode` is qualified rather than widening the destination's import list.
No assertion, name, or behavior changes.

WHY IT LANDED, and why this is worth more than the three-line fix. Nothing in CI
compiles `#[cfg(test)]` code any more: clippy came out 2026-07-08, nextest
2026-07-11, and gunbc#8146 retired the v1 Rust test suite today. `build` compiles
the binaries, not the test targets. So a unit-test module can be broken on main and
every gate stays green — this one is, and was inherited by this branch through an
ordinary main merge rather than authored here.

This is the same shape as the ArtifactIdentity collision repaired hours ago in
gunbc#8177/#8187: a defect invisible to the checks that run, surfacing only when
someone happens to execute the wider thing. Reported rather than silently absorbed;
the gap itself — no CI consumer for test-target compilation — is not closed by this
commit and is left named.

Green by execution: `cargo check -p v1-compiler --all-targets` finishes clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018ZrxLJrt8ASGLegiK72fAi

---------

Co-authored-by: gunbc-ci-auto-heal <gunbc-ci-auto-heal@users.noreply.github.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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