Skip to content

The mesh compiled identical content into a different assembly on every process - #2007

Closed
rbuergi wants to merge 1 commit into
mainfrom
fix/deterministic-source-join-order
Closed

rbuergi wants to merge 1 commit into
mainfrom
fix/deterministic-source-join-order

Conversation

@rbuergi

@rbuergi rbuergi commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

Lifted out of #1994 so it can land on its own — it is an independent bug fix with consequences well beyond the content key it was found by, and it should not wait on that PR's open decision.

What was wrong

CollectCompileSources kept "the order the snapshot delivered them". On the mesh path that delivery is map.Values.ToList() over an ImmutableDictionary — hash-bucket order over per-process randomised string hashes. The join order is in the emitted bytes.

So the mesh compiled identical content into a different assembly on every process — not reproducible against the compiler-driven producer, and not against itself:

run mesh compiler
1 ic920384a… i3494e62e073f2a551796fa08440330ed
2 iead1b585… i3494e62e073f2a551796fa08440330ed
3 i70db762d… i3494e62e073f2a551796fa08440330ed

Now ordinal by path, inside the toolchain function that already owns the join order, so both producers agree by construction.

What this explains

  • Cross-replica dedupe in FileSystemAssemblyStore was structurally impossible. PutWithLocation is first-write-wins on a content-hashed file name, and its own comment reads "The bytes are identical by construction — the hash IS the name." With per-process source order they were not. Two silos compiling the same (path, version) produced different hashes, so different names — never a collision, but never a share either. Every replica wrote its own generation into /data/assembly-cache, which is retention pressure AssemblyCacheGenerations has been sweeping ever since. That is now genuinely dedupable and worth measuring.
  • The store's digest-of-emitted-bytes could never dedupe, for the same reason.
  • BakeEquivalenceTest's class remarks are now stale. They document the non-determinism as a fact of life and explain why the test compares the type/member surface rather than bytes. That constraint is largely lifted, so byte equality between the two producers is within reach — which would prove the bake produces what the mesh would, rather than something merely surface-compatible with it. Not attempted here.

Deploy note

It changes emitted bytes for every NodeType with no identity change. That is safe: existing cached DLLs stay valid for their content, and recompiles simply write new content-hashed names.

Verification

CompileSourceOrderTest — 2/2, verified non-vacuous by reverting the fix. Cherry-picks onto main cleanly.

🤖 Generated with Claude Code

…slice 4)

`BakeEquivalenceTest` went red on the content key and the divergence was real:
the compiler-driven bake produced the same key on three consecutive runs while
the mesh-driven bake produced a different one every time.

    run 1   mesh ic920384a…   tree i3494e62e…
    run 2   mesh iead1b585…   tree i3494e62e…
    run 3   mesh i70db762d…   tree i3494e62e…

`CollectCompileSources` kept "the order the snapshot delivered them", and on the
mesh path that delivery is `map.Values.ToList()` over an `ImmutableDictionary`
(MeshNodeCompilationService's source probe) — hash-bucket order over string
hashes that .NET randomises PER PROCESS. The join order is part of the emitted
bytes, so the mesh compiled identical content into a different assembly on every
process: not reproducible against the compiler-driven bake, and not reproducible
against ITSELF. That is why the assembly store's digest of the emitted bytes
could never dedupe, and it would have made the content key move on every compile
— an identity that invalidates everything on every build, which is strictly
worse than the proxy it replaces.

So the order becomes a function of the SOURCE SET: ordinal by path, inside the
toolchain function that already owns the join order, so both producers agree by
construction rather than by convention. C# does not care — top-level
declarations in a concatenated unit are order-independent, which is exactly what
BakeEquivalenceTest's type-and-member SURFACE comparison proves, and it is what
made the old non-determinism invisible.

One-time effect: every NodeType's bytes change once, as they would on any
identity move.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 21, 2026 16:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes non-deterministic source ordering in the dynamic NodeType compile pipeline so identical source sets produce identical emitted bytes across processes. It makes CollectCompileSources deterministic by sorting input nodes by path before folding/combining, and adds a focused unit test to pin the behavior.

Changes:

  • Sort compile sources by mesh path (ordinal) in NodeCompileShaping.CollectCompileSources, making emitted assembly bytes reproducible across processes.
  • Add CompileSourceOrderTest to verify both the ordered paths and the combined compile unit are invariant to delivery order.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
test/MeshWeaver.Graph.Test/CompileSourceOrderTest.cs Adds unit tests that pin deterministic ordering and stable combined source output.
src/MeshWeaver.Compiler/NodeCompileShaping.cs Changes compile source collection to sort by path (ordinal) to eliminate per-process hash-order nondeterminism.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

   10 files  ±0     10 suites  ±0   5m 22s ⏱️ +20s
1 707 tests ±0  1 706 ✅  - 1  0 💤 ±0  1 ❌ +1 
2 176 runs  ±0  2 175 ✅  - 1  0 💤 ±0  1 ❌ +1 

For more details on these failures, see this check.

Results for commit c9e2115. ± Comparison against base commit 00e8435.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

1 790 tests  ±0   1 682 ✅ ±0   7m 14s ⏱️ - 1m 43s
   10 suites ±0     108 💤 ±0 
   10 files   ±0       0 ❌ ±0 

Results for commit c9e2115. ± Comparison against base commit 00e8435.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

1 229 tests  ±0   1 225 ✅ ±0   11m 12s ⏱️ -25s
    9 suites ±0       4 💤 ±0 
    9 files   ±0       0 ❌ ±0 

Results for commit c9e2115. ± Comparison against base commit 00e8435.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

1 215 tests  ±0   1 214 ✅ ±0   6m 42s ⏱️ -26s
   11 suites ±0       1 💤 ±0 
   11 files   ±0       0 ❌ ±0 

Results for commit c9e2115. ± Comparison against base commit 00e8435.

@github-actions

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

3 007 tests  +2   3 007 ✅ +2   8m 2s ⏱️ ±0s
    9 suites ±0       0 💤 ±0 
    9 files   ±0       0 ❌ ±0 

Results for commit c9e2115. ± Comparison against base commit 00e8435.

@rbuergi

rbuergi commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

Closing as included: this was a cherry-pick of c9eccc76a out of #1994, opened while that PR still looked blocked on the full-MVID decision. That decision is settled — the rule stays, with the reason pinned as a test — so #1994 merged whole and carries this commit. Its analysis (the FileSystemAssemblyStore dedupe consequence, the stale BakeEquivalenceTest remarks, and determinism as a silent precondition of the content key) is recorded on #1994 so it lives with the commit that landed.

@rbuergi rbuergi closed this Aug 21, 2026
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.

2 participants