Skip to content

Refuse a module-filename collision instead of silently overwriting the emitted file - #9486

Merged
briansrls merged 5 commits into
mainfrom
session/deep-bear-239
Aug 27, 2026
Merged

briansrls merged 5 commits into
mainfrom
session/deep-bear-239

Conversation

@gunbai-bot

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

Copy link
Copy Markdown
Contributor

v1.compiler.emit_core_support module_to_filename maps '.' to '_', which is
MANY-TO-ONE: a.b_c and a_b.c both render a_b_c. Every target emitter names
one emitted file per module by that mapping, so two colliding modules wrote the
same path and whichever was emitted second silently replaced the first — a wrong
artifact with no diagnostic anywhere.

MEASUREMENT FIRST. Across dag, src/v1 and src/v2 there are 4187 distinct
authored module names and 4187 distinct filenames under the mapping: zero
collisions today. That is luck rather than structure — 3537 of the 4187 names
already contain an underscore, so the surface the mapping is many-to-one over is
most of the corpus. The wall therefore lands green over the live tree and is not
a ratchet against existing debt.

THE REFUSAL. The mapping cannot be made injective without changing every emitted
path and every cross-module reference derived from one, so the residue is refused
rather than absorbed (DESIGN.md §5 — a failure arm refuses, never widens).
module_filename_collision_diagnostics folds the whole module population once
and returns one located ModuleFilenameCollision per colliding pair, naming the
filename and both modules. It is called by emit_rust, emit_python and
emit_go before any file is produced, because the collision is a fact about the
module set and not about any one target's rendering.

ModuleFilenameCollision is a new CompilerDiagnostic variant. Both total
matches over that coproduct in 00_core.dag gain an arm, and its two
hand-maintained seed projections in cli_run.rs (the histogram key and the
subject-name extractor) gain the mechanical arm each, per the receipt in
compiler_diagnostic_seed_projection_note. It is blocking by default — no
advisory-allowlist entry is owed.

EVIDENCE, BY EXECUTION. dag/test/claim/module_filename_collision_witness_test.dag
uses tools.multi_module_compile_fixture, because the subject IS a relationship
between two modules and no single-module census can express it. The
discriminating RED compiles mmfc.a_b + mmfc_a.b and asserts a BLOCKING row of
class ModuleFilenameCollision — naming the class, not merely "something
refused". The positive control renames one segment so the filenames differ and
asserts zero diagnostics of ANY severity. Both were executed on this branch's
compiler and returned true.

The five regenerated stage0 mirrors are the emitter's own output:
claim_executor --required-regen reports first_generation_equal=true.

Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com
🤖 Generated with Claude Code

Brian Searls and others added 4 commits August 27, 2026 17:22
…e emitted file

`v1.compiler.emit_core_support` `module_to_filename` maps '.' to '_', which is
MANY-TO-ONE: `a.b_c` and `a_b.c` both render `a_b_c`. Every target emitter names
one emitted file per module by that mapping, so two colliding modules wrote the
same path and whichever was emitted second silently replaced the first — a wrong
artifact with no diagnostic anywhere.

MEASUREMENT FIRST. Across `dag`, `src/v1` and `src/v2` there are 4187 distinct
authored module names and 4187 distinct filenames under the mapping: zero
collisions today. That is luck rather than structure — 3537 of the 4187 names
already contain an underscore, so the surface the mapping is many-to-one over is
most of the corpus. The wall therefore lands green over the live tree and is not
a ratchet against existing debt.

THE REFUSAL. The mapping cannot be made injective without changing every emitted
path and every cross-module reference derived from one, so the residue is refused
rather than absorbed (DESIGN.md §5 — a failure arm refuses, never widens).
`module_filename_collision_diagnostics` folds the whole module population once
and returns one located `ModuleFilenameCollision` per colliding pair, naming the
filename and both modules. It is called by `emit_rust`, `emit_python` and
`emit_go` before any file is produced, because the collision is a fact about the
module set and not about any one target's rendering.

`ModuleFilenameCollision` is a new `CompilerDiagnostic` variant. Both total
matches over that coproduct in `00_core.dag` gain an arm, and its two
hand-maintained seed projections in `cli_run.rs` (the histogram key and the
subject-name extractor) gain the mechanical arm each, per the receipt in
`compiler_diagnostic_seed_projection_note`. It is blocking by default — no
advisory-allowlist entry is owed.

EVIDENCE, BY EXECUTION. `dag/test/claim/module_filename_collision_witness_test.dag`
uses `tools.multi_module_compile_fixture`, because the subject IS a relationship
between two modules and no single-module census can express it. The
discriminating RED compiles `mmfc.a_b` + `mmfc_a.b` and asserts a BLOCKING row of
class `ModuleFilenameCollision` — naming the class, not merely "something
refused". The positive control renames one segment so the filenames differ and
asserts zero diagnostics of ANY severity. Both were executed on this branch's
compiler and returned true.

The five regenerated stage0 mirrors are the emitter's own output:
`claim_executor --required-regen` reports `first_generation_equal=true`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three conflicts, resolved by kind rather than by side.

`src/v1/05_emit_rust.dag` — main refactored emit_rust's preamble into
`build_emit_rust_context`, and this branch had added the filename-collision guard
at the top of the old emit_rust. Took main's refactor and re-placed the guard
BESIDE ITS PEER DIAGNOSTIC GUARDS, after the context is built, rather than ahead
of it. That is deliberate and is annotated in place: main's new preamble carries
a MEASURED order-dependence in exactly that region — independent-looking bindings
move emitted bytes — and reading every module's authored name before the preamble
runs is the kind of perturbation that finding warns about. Refusing a few
microseconds later is not worth reopening it. The annotation sits above `fn
emit_rust` rather than inside its body, because §4c admits only module-item
grain and the first placement was refused by the compiler.

`v1_std_core.rs` and `v1_compiler_emit_rust.rs` — both sides changed these
generated projections since the merge base, so neither side's bytes are the
projection of the MERGED authorities and the driver left them unmerged rather
than picking one. They are NOT hand-resolved. All five mirrors this branch
touches were regenerated from the merged `.dag`: reset to main's bytes to obtain
a seed that builds, `--required-regen` to emit the candidate against the merged
authority, install, rebuild, and verify again. The second pass reports
`first_generation_equal=true` over 138 planned/executed, so the committed mirrors
are the merged authority's own output and not a merge artifact.

Both witness arms re-executed on the merged compiler and returned true — the
discriminating RED (`mmfc.a_b` + `mmfc_a.b` refusing with a blocking
`ModuleFilenameCollision` row) and the positive control (distinct filenames,
zero diagnostics of any severity).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… (codex review 56887)

The two new cli_run.rs arms projecting ModuleFilenameCollision are hand-maintained
Rust. compiler_diagnostic_seed_projection_note requires the next lane adding a
CompilerDiagnostic variant to re-measure and append; this lane had not. Appended
the FOURTH LANE clause with the three commands re-run against origin/main
9538523: fn census 896 at main and 896 on branch (flat), 2 added / 0 removed
lines, 0 added fn declarations. Dissolution inherits the ROADMAP row
v1-zero-hand-maintained-rust; no fresh trigger minted.

Regenerated v1_std_core.rs from the note (first_generation_equal=true, 138/138).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gunbai-bot

gunbai-bot Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the REQUEST_CHANGES from review 56887 in 113269e.

The finding was correct: compiler_diagnostic_seed_projection_note ends with the DuplicateDeclaration lane and explicitly requires the next lane to re-measure, and this PR expanded the seed without doing so. The note now carries a FOURTH LANE clause with the three commands the receipt itself names, re-run against origin/main 9538523:

  • grep -cE "^(pub )?fn " src/v1/stage0/src/cli_run.rs — 896 at origin/main, 896 on this branch. The carrier census is FLAT: no new hand-Rust function.
  • git diff --numstat origin/main -- src/v1/stage0/src/cli_run.rs — 2 added, 0 removed.
  • git diff origin/main -- src/v1/stage0/src/cli_run.rs | grep -cE "^\+.*\bfn " — 0 added fn declarations.

Two things the clause states rather than leaves to inference. The prior lane recorded 8 changed lines against a base of 890 and this one records 2 against 896; the difference is rustfmt wrapping of a wider variant pattern, not added capability, and the prior lane figures are left untouched rather than amended. And ModuleFilenameCollision is a blocking diagnostic, so the advisory-allowlist obligation the note carries for advisory variants does not fire here.

Dissolution: the clause inherits the existing ROADMAP row v1-zero-hand-maintained-rust (referenced from dag/test/claim/roadmap_authority_test.dag) rather than minting a fresh trigger — these two arms are projection rows in the same carrier that row already governs, and a per-lane trigger beside it would be a second authority for one fact.

v1_std_core.rs regenerated from the amended note; claim_executor --required-regen reports first_generation_equal=true over 138/138.

Also merged origin/main to pick up #9497. The required floor is currently red on two findings in files this PR does not touch — dag/product/fabric/contention.dag imports grant_duration_seconds, which #9397 renamed on main and did not update the caller for. That break is owned by #9488 and is not mine to repair.

— sent from deep-bear-239

@gunbai-bot

gunbai-bot Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Thanks — on the grouping nit from review 56913, declining, with the reasoning stated so it can be overridden.

For three modules a, b, c that render one filename, the fold reports [a, b] and [a, c]: two blocking rows rather than one [a, b, c]. Both rows are true, both name the shared filename, and both name a real pair that cannot coexist. The line stops either way.

Grouping is representable — modules is already a List<String> — but it costs a second pass over the owner map to collect classes before emitting, and it buys a presentational improvement in a case the live corpus does not contain: measured over the current tree, 4187 distinct module names render 4187 distinct filenames, so the collision population is zero and the three-way sub-population is a subset of that. Against a zero population I would rather not add a pass whose only effect is grouping, and the seed regeneration it would require is ~25 minutes of build and verify for that.

The one thing I would change my mind on is if the pairwise form were misleading rather than merely ungrouped — if a reader could repair [a, b], re-run, and be surprised by [a, c]. That is real, but it is the correct behaviour for this wall: renaming b genuinely does not resolve c, and a reader who saw one grouped row would still have to make two renames. The second diagnostic is the second obligation, not a repeat of the first.

Recorded here rather than in the carrier because it is a diagnostic-quality preference and not an invariant.

— sent from deep-bear-239

@briansrls
briansrls merged commit 42eed29 into main Aug 27, 2026
3 checks passed
@briansrls
briansrls deleted the session/deep-bear-239 branch August 27, 2026 22:45
briansrls pushed a commit that referenced this pull request Aug 27, 2026
…eneration

All three conflicts were GENERATED mirrors; zero .dag conflicts, so the
authorities merged clean and only their projections collided. The
generated-artifact driver refuses rather than writing markers, leaving the
ours side unmerged with a regeneration recipe -- hand-resolving would
produce bytes that satisfy git and then diverge from the emitter, which
required-regen compares byte-for-byte.

Neither side could be taken wholesale: main added CallTargetIdentity
(#9486) so main's hand-Rust needs main's mirror, while this branch's
cli_run.rs arms need this branch's. Both pure resolutions fail to compile
before regen can run.

So stage0 was reduced to EXACTLY main's -- main's mirrors, these two
cli_run.rs arms temporarily stripped -- which builds, and the merged .dag
emitted the union. The mirrors here are what the compiler produced, not
what anyone assembled.

Six files drift, not the three that conflicted: adding variants to
CompilerDiagnostic changes emitted match arms in every emitter that
matches on it, so emit_core_support, emit_go and emit_python move too. The
set of files a merge CONFLICTS on and the set a regen CHANGES are
different sets, and only the second is the truth.
gunbai-bot Bot pushed a commit that referenced this pull request Aug 28, 2026
Recomputed at 5a62da7 (RUNNER_HEAD confirmed on the runner), replacing the
be89e23-based bytes: a mirror computed at a stale base is stale whatever CI
later reports, so the base window was closed rather than waited out.

Two-generation procedure, pristine tree at that head:
  gen-0  first_generation_equal=false, drift in exactly these four files
  install candidate, REBUILD claim_executor
  gen-1  first_generation_equal=true

The rebuild between passes is the point: a single pass verifies an emission
against a binary that predates it.

The four files are byte-identical to the be89e23-based output, which
answers a question we had declined to spend a corpus emit on: #9535 added seven
test fn declarations under dag/test/claim, enlarging the module INDEX while
leaving the regen POPULATION (the import-only union under src/v1) untouched,
and the emitted qualification did not move. #9543 independently regenerated the
same four files at the same head and got the same bytes. Two independent
negatives on index-sensitivity at this grain.

Attribution, stated because an earlier draft of this body had it wrong: the
drift is NOT #9436. It is a composition of #9486, which introduced
module_filename_collision_diagnostics, and #9461, which changed import-candidate
selection so calls to it need qualifying -- merged three minutes apart from an
identical base aea5e0d. Neither alone produces the stale bytes and there was
no textual conflict for any gate to see, which is why delete-first's census
could not surface it.

Discriminator for the next red, with its left endpoint at the base these bytes
were computed at:
  git log --oneline 5a62da7..origin/main -- 'src/v1/**/*.dag'
Empty means the population has not moved and a regen red is not base staleness.
briansrls pushed a commit that referenced this pull request Aug 28, 2026
…qualification is closure-dependent, not hand-carried wrong (#9551)

`required-regen` has refused every main push for six hours:

  required-regen: first_generation_equal=false planned=138 executed=138 declared_divergent=1 [main.rs]
  required-ci: regen FAIL generated surface drift: v1_compiler_emit_core_support.rs,
    v1_compiler_emit_go.rs, v1_compiler_emit_python.rs, v1_compiler_emit_rust.rs

THIS IS NOT A MISSED REGENERATION, and the framing matters because the obvious
reading teaches the wrong lesson. #9486 hand-carried these mirrors alongside its
`.dag` edits and its own PR run was GREEN on regen -- run 33109524057 at head
c84f377, `first_generation_equal=true`. The bytes it committed were exactly
what the emitter produced ON ITS BASE. Nothing about them changed between that
run and the merge.

WHAT THE DRIFT ACTUALLY IS. The emitted call spelling is CLOSURE-DEPENDENT.
Every one of the 28 changed lines here is one class: a bare call name where the
emitter now renders a fully-qualified `crate::<module>::<fn>` path.

  -        let filename_collisions = module_filename_collision_diagnostics(typed.clone());
  +        let filename_collisions =
  +            crate::v1_compiler_emit_core_support::module_filename_collision_diagnostics(
  +                typed.clone(),
  +            );

The bare form compiles -- the file's own `pub use crate::v1_compiler_emit_core_support::{...}`
re-export brings the name into scope -- so the committed mirror was correct Rust
with correct semantics. It simply is not what the emitter renders once the
surrounding closure changed. Two changes, each green on its own base, red in
combination: a semantic merge conflict, which no per-PR gate can see BY
CONSTRUCTION, since each PR's regen runs against a closure the other has not
joined yet.

ONE MEASURED DETAIL THAT NARROWS THE MECHANISM. The `pub use` header in
`v1_compiler_emit_go.rs` is UNCHANGED between committed and candidate -- the
import list still carries `module_filename_collision_diagnostics` -- and the
call renders qualified anyway. So the emitter's qualification decision is not
simply "is this name in the import list", and any future explanation of this
class has to account for that. #9461 (58d10ac, emitter import candidates) is
the natural other half of the pair and three of the emit_rust sites are its
code, but this PR does not assert that as established.

CHANGED-LINE CENSUS, complete, nothing elided:

  v1_compiler_emit_core_support.rs   4  authored_name_at, make_error_node -> crate::v1_std_core::
  v1_compiler_emit_go.rs             5  module_filename_collision_diagnostics
  v1_compiler_emit_python.rs         5  same
  v1_compiler_emit_rust.rs          14  same, plus is_type_def_item / is_coproduct_type / authored_name_at

NO AUTHORITY EDIT. The `.dag` is correct and untouched. Only the emitted mirror
bytes moved, and they were installed VERBATIM from `target/stage0-regen-candidate/src`
-- no hand-editing of emitted output, which is the move that created this
situation and must not be the move that resolves it.

EVIDENCE BY EXECUTION, with a non-stale instrument. `claim_executor` was built
locally from main tip b6003a4 (verified equal
to `origin/main`, not an ancestor), finishing 01:08 UTC -- so it postdates every
commit it measures. Three separate lanes reported findings from stale binaries
on this defect tonight; this one cannot be.

  BEFORE: first_generation_equal=false, FAIL naming those four files
  AFTER:  first_generation_equal=true  planned=138 executed=138

Also verified: `cargo fmt --all --check` clean (no collision with the fmt gate,
which consumes the next formatter pass of this same artifact), and
`cargo build --release -p v1-compiler` clean under `RUSTFLAGS=-D warnings`.

Regen deleted no orphan content and reached nothing under `dag/extdeps`:
`changed_paths` named exactly these four files and every other emitted file in
the 138 already matched.

DURABILITY LIMIT, STATED SO IT IS NOT LATER READ AS AN INEFFECTIVE FIX. This
mirror is correct for the closure at b6003a4. If a PR landing ahead of this
one changes that closure again, the mirror re-drifts and the gate refuses again
through no fault of this change. That is a property of the class -- emitted
bytes are a function of the whole closure, and the merge queue does not
serialize on it -- not a defect in this repair.


Claude-Session: https://claude.ai/code/session_01KQiX51cUPte6qm5zQfrbfr

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