Skip to content

fix(gate): a bake shortfall names the assembly that declined - #2675

Merged
rbuergi merged 2 commits into
mainfrom
fix/bake-shortfall-names-the-type
Aug 29, 2026
Merged

rbuergi merged 2 commits into
mainfrom
fix/bake-shortfall-names-the-type

Conversation

@rbuergi

@rbuergi rbuergi commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

The failure this makes diagnosable

MeshWeaver.Crm's main has been red since 2026-08-28 21:54 — 23 consecutive runs — on one line:

seed: adopted 86 of 87 baked assembly(ies) for the 87 installed NodeType(s)
FATAL: bake consumption: … 1 were DECLINED and compiled locally
GATE FAILED

Every per-type verdict in that same run is green (Crm/Board: compile=Ok render=ok tests=ok, 12/12 types, 8/8 tests). So the only question the verdict raises is which of 87 assemblies declined, and why. Neither was answerable:

  1. The verdict could not name it. BakeSeedConsumer counted adoptions and compared counts, and said so in a comment — "WHICH assembly was declined is not knowable here" — deferring to "the per-assembly reason is logged by PrebuiltAssemblySeeder".
  2. That log does not exist. Those reasons are logged at Information; the gate mesh runs at Warning. The CI log for that run contains zero ShippedPrebuiltBundles lines. The verdict pointed at evidence the gate never writes.

A red gate that carries no way to diagnose itself is the same shape as the always-attach-the-provider fix one seam over (2026-08-10, "the run therefore carried zero evidence of WHY").

The fix — both halves, at the root

  • The seeder witnesses each path it backs. SeedForTypes(…, Action<string>? onCovered = null) — optional and additive; every existing caller keeps today's behaviour and the IPrebuiltAssemblyConsumer interface is untouched. Both routes to Covered report: adopted in SeedPayloads, already-current in SeedBundle.
  • Shortfall() is now expected.Except(covered) and prints DECLINED: <paths>. This is strictly more correct than the count it replaces: Adopted sums per-call totals, so a path covered under two packages inflated it and could mask a genuinely missing one. A set cannot.
  • The gate raises exactly the bake-seed category to Information while consuming a bake — one line per bundle (34 on that run) — so the reason is in the log the verdict names. MW_LOG_LEVEL still governs the trace levels.

The verdict logic moves into a pure DescribeShortfall(declared, requested, covered, adopted, directory), so the accounting is pinned without standing up a mesh — the same seam PrebuiltAssemblySeeder.DeclineReason exposes for the same reason.

Scope — what this does and does not do

This makes the Crm failure diagnosable; it does not by itself explain which assembly declines there. The next gate run on this build prints the name, and the reason lands beside it. I did not guess at the underlying decline: a faithful local repro needs the sealed upstream publication and the external AI module artifact, and inventing a cause from a count would have been the band-aid.

⚠️ Behaviour change worth reviewing: the set-based check is stricter than the count. If any repo's gate is currently passing because double-counted coverage masked a real gap, this will turn it red — correctly, and now with the name attached.

How it was tested

  • 5 new tests — naming the declined type; the no-overlap staging message; a bake carrying types the run never installs (not a shortfall); and that two runs with identical counts but different missing paths now produce different verdicts, the exact case a count conflated.
  • 113/113 MeshWeaver.PluginTester.Test, including the bake→gate round-trips that exercise the instrumented adoption path (TheGateRunsOnTheBakedBytes_AndCompilesNothingItWasHandedABakeFor, GateInstallsUpstreamPackages, BakeEquivalence).
  • 10/10 ShippedPrebuiltBundlesTest.
  • Release + -warnaserror clean on all three touched projects.

What's New: skipped — CI/gate tooling only, no user-visible effect.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Wt457V2hiV4YkEmcNFNt4N

MeshWeaver.Crm's main has been red since 2026-08-28 21:54 on:

    seed: adopted 86 of 87 baked assembly(ies) for the 87 installed NodeType(s)
    FATAL: bake consumption: ... 1 were DECLINED and compiled locally
    GATE FAILED

Every per-type verdict in that same run is green (12/12 for Crm:
compile=Ok render=ok tests=ok), so the only question the verdict raises is
WHICH of 87 assemblies declined, and why. Neither was answerable:

1. The verdict could not name it. BakeSeedConsumer counted adoptions and
   compared counts, and said so in a comment: "WHICH assembly was declined is
   not knowable here". It deferred to "the per-assembly reason is logged by
   PrebuiltAssemblySeeder".

2. That log does not exist. Those reasons — the framework identity, the
   per-type dependency record, a manifest entry whose payload the bundle does
   not carry — are logged at Information, and the gate mesh runs at Warning.
   The CI log for that run contains ZERO ShippedPrebuiltBundles lines. The
   verdict pointed at evidence the gate never writes.

So a red gate carried no way to diagnose itself, and the failure sat unread
for ~12 hours across 23 consecutive main runs.

Both halves are fixed at the root:

- The seeder now WITNESSES each path it backs (`onCovered`, optional and
  additive — every existing caller keeps today's behaviour). Both ways a path
  becomes Covered report it: adopted in SeedPayloads, and already-current in
  SeedBundle. So the consumer holds the set, not just a count.
- Shortfall() is now `expected.Except(covered)` and prints `DECLINED: <paths>`.
  This is also strictly more correct than the count it replaces: `Adopted` sums
  per-call totals, so a path covered under two packages inflated it and could
  mask a genuinely missing one. The set cannot.
- The gate raises exactly the bake-seed category to Information when it is
  consuming a bake — one line per bundle (34 on that run) — so the REASON is in
  the log the verdict points at. Same argument as the always-attach-the-provider
  fix one seam over: a red verdict whose evidence is never written is not a
  verdict. Trace stays opt-in via MW_LOG_LEVEL.

The verdict logic moves into a pure DescribeShortfall(declared, requested,
covered, adopted, directory) so the accounting is pinned without a mesh — the
same seam PrebuiltAssemblySeeder.DeclineReason exposes for the same reason.

This makes the Crm failure diagnosable; it does not by itself explain which
assembly declines there. The next gate run on this build prints the name.

Tests: 5 new (naming, the no-overlap message, a bake carrying types the run
does not install, and that two runs with identical counts but different missing
paths now produce different verdicts — the case a count conflated).
113/113 PluginTester, 10/10 ShippedPrebuiltBundles, including the
bake→gate round-trips that exercise the instrumented adoption path.
Release + -warnaserror clean on all three touched projects.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Wt457V2hiV4YkEmcNFNt4N
Copilot AI lite review requested due to automatic review settings August 29, 2026 10:27

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 improves the diagnosability of plugin-gate “bake shortfall” failures by tracking which NodeType paths were actually covered during bake consumption and surfacing the missing/declined paths directly in the gate verdict, with supporting log visibility tweaks and tests.

Changes:

  • Add per-NodeType “covered” witnessing during shipped bundle seeding and use set-diff (expected \ covered) to name declined paths in the shortfall verdict.
  • Raise the gate’s bake-consumption logging category to Information while consuming a bake so decline reasons are visible in default CI logs.
  • Add focused unit tests pinning the new shortfall accounting and messaging.

Reviewed changes

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

File Description
tools/MeshWeaver.PluginTester/PluginGateRunner.cs Raises the bake-seed log category to Information when a bake is being consumed.
tools/MeshWeaver.PluginTester/BakeSeed.cs Tracks Covered paths and updates shortfall logic to report explicit declined paths; adds a pure DescribeShortfall seam.
test/MeshWeaver.PluginTester.Test/BakeShortfallNamesTheTypeTest.cs Adds tests to ensure declined paths are named and accounting is set-based (not count-based).
src/MeshWeaver.Hosting/ShippedPrebuiltBundles.cs Threads an optional “covered” callback through bundle seeding and invokes it for adopted and already-current entries.

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

Comment on lines +261 to +263
+ ". The per-assembly reason is logged by PrebuiltAssemblySeeder at Information "
+ "(framework identity, or the per-type dependency record); the gate logs at Warning, "
+ "so raise its level to read it.";

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in dfc2522 — the wording was left over from before I added the automatic raise, so it contradicted the change in the same PR. It now names the bake-seed category, says the gate raises it automatically while consuming a bake, and says what the reason will actually be (framework identity, the per-type dependency record, or a payload the manifest names but the bundle does not carry).

Comment on lines 196 to 200
public static IObservable<int> SeedForTypes(
IMessageHub mesh, IReadOnlyCollection<string> typePaths, ILogger? logger,
string? imageDirectory = null, string? publishedRoot = null)
string? imageDirectory = null, string? publishedRoot = null,
Action<string>? onCovered = null)
=> Observable.Defer(() =>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in dfc2522 — you're right, and it matters more here than the general case: a prebuilt bundle adopted from an earlier framework build is exactly the kind of already-compiled caller that would hit MissingMethodException, and SeedForTypes is the method that serves them.

The witness now rides a separate overload whose parameters are all required, so the shipped 5-argument signature is byte-for-byte unchanged and SeedForTypes(mesh, paths, logger) can't become ambiguous between the two.

@github-actions

github-actions Bot commented Aug 29, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

689 tests   497 ✅  5m 46s ⏱️
  5 suites  192 💤
  5 files      0 ❌

Results for commit dfc2522.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 29, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

913 tests   913 ✅  5m 26s ⏱️
  6 suites    0 💤
  6 files      0 ❌

Results for commit dfc2522.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 29, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

1 546 tests   1 546 ✅  6m 28s ⏱️
    8 suites      0 💤
    8 files        0 ❌

Results for commit dfc2522.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 29, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

879 tests   875 ✅  6m 26s ⏱️
  6 suites    4 💤
  6 files      0 ❌

Results for commit dfc2522.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 29, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

857 tests   857 ✅  6m 48s ⏱️
  6 suites    0 💤
  6 files      0 ❌

Results for commit dfc2522.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 29, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

   10 files     10 suites   5m 44s ⏱️
3 572 tests 3 572 ✅ 0 💤 0 ❌
4 206 runs  4 206 ✅ 0 💤 0 ❌

Results for commit dfc2522.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 29, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

   41 files     41 suites   36m 40s ⏱️
8 456 tests 8 260 ✅ 196 💤 0 ❌
9 090 runs  8 894 ✅ 196 💤 0 ❌

Results for commit dfc2522.

♻️ This comment has been updated with latest results.

…ses the category

Both Copilot findings were right.

1. Adding an optional parameter to a PUBLIC method is a binary break, not a
   source-compatible one: C# bakes a call's full argument list into the CALL
   SITE, so an assembly compiled against the 5-argument form keeps calling it
   and would fail with MissingMethodException. In this framework that assembly
   is a prebuilt bundle adopted from an earlier build — precisely the artifact
   SeedForTypes exists to serve. The witness now rides a separate overload
   whose parameters are all REQUIRED, which is also what keeps
   SeedForTypes(mesh, paths, logger) from becoming ambiguous between the two.

2. The verdict told the reader to raise the log level, which the gate now does
   for them whenever it consumes a bake. It names the category instead, and
   what the reason will say.

Disambiguated the three <see cref="SeedForTypes"/> references the overload made
ambiguous (CS0419 under -warnaserror).

113/113 PluginTester, 10/10 ShippedPrebuiltBundles, Release + -warnaserror
clean on all three touched projects.

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.

2 participants