Skip to content

A compile problem now says whether it is your code or a timeout — and links the log - #940

Merged
rbuergi merged 2 commits into
mainfrom
fix/641-compile-overlay-timeouts
Aug 8, 2026
Merged

rbuergi merged 2 commits into
mainfrom
fix/641-compile-overlay-timeouts

Conversation

@rbuergi

@rbuergi rbuergi commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Fixes #641.

Already on main (not redone here)

The audit on the issue established that the compiler output itself is already
surfaced: BuildCompilationErrorMarkdownText's diagnostics fence plus the
per-source Monaco with Roslyn markers (NodeTypeLayoutAreas), the
CompileProgressIndicator, the {TypeName}: {Message} treatment for a
non-Roslyn abort in RunCompile, and ONE timeout path that already reads
correctly — the enrichment settle timeout ("has not finished compilation yet…
no code change is needed"). None of that is touched.

What this PR adds

1. The broken INSTANCE page never linked the compile activity

WithCompilationErrorOverlay never received LastCompilationActivityPath, so
the "View full compile log →" anchor existed only on the NodeType's own
Overview — the instance page, the one a user actually lands on, had no route
to the diagnostics at all. The path is now threaded through to the markdown
builder and rendered as a markdown link (never an embedded LayoutAreaControl:
the activity is history and may be unaddressable — subscribing to an inexistent
address is the resubscribe storm). It rides along on every overlay that has a
type node in hand, and it matters most on a timeout, where the log is the only
evidence of how far the build got.

2. Timeouts and lookup misses no longer render as compile failures

Every overlay call site that passed no guidance silently inherited "There was
a compilation error in this item's code… Please correct the code."

Four causes reached it that way, none of them the author's code:

  • the type-registration probe (~3 s) folded a FAULT into "type is not
    registered", so an indeterminate lookup and a genuinely missing type produced
    identical copy. Split into Registered / Missing / Indeterminate;
  • pinned-release / assembly-store misses — an assembly this node cannot
    fetch is a lookup miss, not broken source;
  • framework-stale — whose guidance already said "no source changes are
    needed" under a lead-in demanding a code fix;
  • an enrichment chain that faulted outright, before any build was consulted
    (it now also names the exception TYPE, like RunCompile does).

The wording is no longer implicit: NodeTypeEnrichmentHelpers.OverlayCause +
OverlayCopy are the single place that decides it, and ImpliesCodeFix pins
which causes may ask for an edit. Every availability cause leads with "No code
change is needed for this."

The contract handler's 60 s settle timeout fell straight to the Subscribe error
sink as Fail("The operation has timed out.") — naming neither what timed out
nor that it was a timeout — and once that reached the write-back it persisted
CompilationStatus = Error. It now folds into the same write-back with a
message that names the type and the budget.

3. Structural: CompilationStatus.Unavailable

The enum had no way to say "could not determine", so a timeout could only be
persisted as Error. Unavailable is appended last (every existing
persisted ordinal unchanged) and is a MARKER, never an answer: the write-back
refuses to let it overwrite a state another driver owns (Pending/Compiling —
that compile writes its own terminal state; clobbering it would strand the
dispatch), a strictly better one (Ok), or a never-compiled null the
first-build kickoff gates on. EnsureCompileDispatched treats it as "never
determined", so the next request — never a timer — dispatches a fresh
compile rather than leaving the type parked.

Every reader was checked (grep CompilationStatus) so the new member cannot be
mistaken for a failure or hang a waiter:

Reader Handling
enrichment settled predicate, IsRecompileSettled terminal — waiting would burn the whole no-progress budget for a write that is not coming
ApplyStreamResult dedicated branch: retry copy + compile-log link
MCP GetDiagnostics / compile (MeshOperations) settled, and reported as "NOT a compile failure" instead of hanging or blaming the source
NodeType progress page / status badges / compile panel (NodeTypeLayoutAreas) own undetermined state (warning tint, "Retry compile"), never the error tint
DynamicTypePreWarmer terminal; reported TimedOut, never CompileError
NodeTypeBakeStatus deliberately unchanged — Unavailable is not PreviouslyBroken and stays probeable
AwaitCompilationSettled (MeshDataSource) already correct (only Pending/Compiling are in-flight)

Tests

  • CompileErrorPageTest — the copy invariant is enumerated over
    OverlayCause
    , so a cause added later cannot quietly inherit the code-fix
    wording; plus the settle-timeout page, the genuine-failure page, and the
    compile-log link (present / absent).
  • RecompileSettlePredicateTest — Unavailable is terminal, but never
    satisfies requireUsableBuild.
  • CompileErrorOverviewTest — end to end: an Unavailable type's instance
    Overview reads as retry and links the log and never says "correct the code";
    a genuine Roslyn failure still shows its diagnostics, still asks for the fix,
    and now links its log too.

Local runs (Release): Graph.Test 831 passed · Content.Test/CompileErrorOverview
4 passed · Messaging.Hub.Test Localization 33 passed ·
Documentation link integrity passed · Monolith CompileSingleDriverConsistency +
MeshNodeCompilationIntegration passed. All touched projects build clean with
-c Release -warnaserror.

Known gap

The overlay's prose (headline, guidance paragraphs) remains English-only, as it
was before this PR — localizing it means deferring the whole copy to render time
and migrating the pre-existing constants, which is a separate change. The two
NEW discrete strings this PR adds (ui.viewCompileLog,
ui.compileStateUnknown) are in both strings.en.json and strings.de.json,
and the previously hard-coded "View full compile log →" on the NodeType page now
reads from the catalog. CompileProgressIndicator (the global banner) still
surfaces only in-flight compiles and hard failures — an Unavailable type shows
nothing there rather than an error-styled banner, which the page overlay covers.

🤖 Generated with Claude Code

Every emergency overlay that passed no `guidance` silently inherited the
genuine-failure copy — "There was a **compilation error** in this item's code…
**Please correct the code.**" Four causes reached it that way:

  * a ~3 s type-registration probe that TIMED OUT (folded into "type is not
    registered", so an indeterminate lookup and a genuinely missing type were
    the same answer),
  * a pinned release that could not be resolved, or whose assembly bytes this
    node's store does not hold,
  * an assembly built against a previous framework — whose own guidance already
    said "no source changes are needed" under a lead-in demanding a code fix,
  * an enrichment chain that faulted outright, before any build was consulted.

None of those is the author's code. The 60 s settle wait in
NodeTypeContractHandler was worse still: it fell to the Subscribe error sink as
Fail("The operation has timed out.") — a message naming neither what timed out
nor that it WAS a timeout — and once that reached the write-back it persisted
CompilationStatus = Error, so every instance page of a perfectly healthy type
read as broken code.

Root cause, not copy: the status enum had no way to say "could not determine".
`CompilationStatus.Unavailable` (appended last, existing ordinals unchanged) now
records it, and it is a MARKER, never an answer — the write-back refuses to let
it overwrite a state another driver owns (Pending/Compiling, whose compile will
write its own terminal state), a strictly better one (Ok), or a never-compiled
null the first-build kickoff gates on. EnsureCompileDispatched treats it as
"never determined", so the next REQUEST — never a timer — dispatches a fresh
compile instead of leaving the type parked on an undetermined state.

The wording is no longer implicit either: NodeTypeEnrichmentHelpers.OverlayCause
+ OverlayCopy are the single place that decides it, and ImpliesCodeFix pins which
causes may ask for an edit. Every availability cause leads with "No code change
is needed for this."

And the broken INSTANCE page now links the compile log. "View full compile log →"
existed only on the NodeType's own Overview; on a timeout that log is the only
evidence of how far the build got, so it is exactly the page that needed it.
Rendered as a markdown link, never an embedded LayoutAreaControl — the activity
is history and may be unaddressable, and subscribing to an inexistent address is
the resubscribe storm.

Readers updated so the new member cannot be mistaken for a failure: the
enrichment settle predicate and IsRecompileSettled treat it as terminal (waiting
would burn the whole no-progress budget for a write that is not coming), MCP
GetDiagnostics/compile report it as "NOT a compile failure" instead of hanging or
blaming the source, the NodeType progress page / status badges / compile panel
get their own undetermined state, and the pre-warmer reports TimedOut rather than
CompileError. NodeTypeBakeStatus deliberately unchanged: Unavailable is not
PreviouslyBroken and stays probeable.

Tests: the copy invariant is enumerated over OverlayCause, so a cause added later
cannot quietly inherit the code-fix wording; integration tests assert an
Unavailable type's instance page reads as retry and links the log, and that a
genuine Roslyn failure still shows its diagnostics and still asks for the fix.

Fixes #641.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 8, 2026 06:46
@rbuergi

rbuergi commented Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

On the rolling-deploy exposure you flagged (an old pod deserializing "Unavailable" fails the NodeTypeDefinition parse): that one is worth closing rather than accepting, because the blast radius is not limited to the new field — System.Text.Json's enum converter throws on an unknown value, so the whole node fails to materialize, and a NodeTypeDefinition that will not deserialize is exactly the poisoned-content shape #891 just had to defend the compile watchers against.

Two options, either fine:

  1. Make CompilationStatus deserialization tolerant — an unrecognized name maps to Unknown (which every reader already treats as 'never determined'), so an old pod degrades to 'don't know' instead of failing the parse. This is the schema-evolution twin of [JsonExtensionData] UnknownMembers, which already protects unknown members on that same type.
  2. Or ship the reader tolerance one release BEFORE anything writes the new value, and note the ordering in the PR.

Option 1 is self-contained and removes the window entirely; worth doing here unless it turns out the converter is shared in a way that makes it risky. Not blocking my read of the rest — the enum-as-marker design (never overwrites Pending/Compiling/Ok/null, request-driven redispatch, no timer) is the right shape.

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 “page can’t be displayed” compile-failure overlay and related diagnostics so users can tell whether they need to fix their code vs. retry/wait due to timeouts/availability issues, and it adds a direct link to the compile activity log from the instance overlay.

Changes:

  • Add CompilationStatus.Unavailable to represent “could not determine compile state” (distinct from real compile failures) and thread this through UI, enrichment, pre-warming, and AI diagnostics.
  • Ensure instance overlays can link to the last compile activity log, and centralize overlay wording by cause so non-code faults don’t instruct users to “correct the code”.
  • Add/extend tests and a What’s New entry; add localization keys for the new “unknown” and “view log” labels.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
test/MeshWeaver.Graph.Test/RecompileSettlePredicateTest.cs Adds coverage that Unavailable is treated as terminal for settle predicates.
test/MeshWeaver.Graph.Test/CompileErrorPageTest.cs Verifies overlay copy per-cause and compile-log link rendering behavior.
test/MeshWeaver.Content.Test/CompileErrorOverviewTest.cs End-to-end assertions for instance overview behavior on Unavailable vs real failures.
src/MeshWeaver.Messaging.Hub/Localization/strings.en.json Adds ui.compileStateUnknown and ui.viewCompileLog.
src/MeshWeaver.Messaging.Hub/Localization/strings.de.json German translations for the new UI keys.
src/MeshWeaver.Mesh.Contract/Services/CompilationStatus.cs Introduces CompilationStatus.Unavailable with detailed contract docs.
src/MeshWeaver.Hosting/DynamicTypePreWarmer.cs Treats Unavailable as terminal and reports it as timed out (not compile error).
src/MeshWeaver.Graph/NodeTypeLayoutAreas.cs Updates NodeType UI for Unavailable state and localizes “view log” link label.
src/MeshWeaver.Graph/Configuration/NodeTypeEnrichmentHelpers.cs Adds explicit overlay causes + compile-log link plumbing into the overlay markdown builder.
src/MeshWeaver.Graph/Configuration/NodeTypeContractHandler.cs Marks timeout/lookup failures as “Unavailable” and attempts to persist that state.
src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-08-compile-error-clarity.md What’s New documentation for the improved overlay/diagnostics behavior.
src/MeshWeaver.AI/MeshOperations.cs Treats Unavailable as settled/terminal for diagnostics and compile reporting.

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

Comment on lines +1961 to +1963
var activityHref = "/" + def.LastCompilationActivityPath;
panel = panel.WithView(Controls.Html(
$"<a href=\"{System.Net.WebUtility.HtmlEncode(activityHref)}\" style=\"font-size: 12px; color: var(--neutral-foreground-hint);\">View full compile log →</a>"));
$"<a href=\"{System.Net.WebUtility.HtmlEncode(activityHref)}\" style=\"font-size: 12px; color: var(--neutral-foreground-hint);\">{System.Net.WebUtility.HtmlEncode(LocalizationCatalog.Get("ui.viewCompileLog", locale))}</a>"));
Comment on lines +319 to +323
CompilationStatus.Unavailable => ("❔", "Compile state unknown",
(string.IsNullOrEmpty(def.CompilationError)
? "The last attempt to determine this type's build state did not complete."
: $"```text\n{def.CompilationError}\n```")
+ "\n\nNothing is known to be wrong with the code — click **Recompile** to try again."),
Comment on lines +273 to +286
if (resolved.Unavailable)
return def.CompilationStatus is null
or CompilationStatus.Pending
or CompilationStatus.Compiling
or CompilationStatus.Ok
? curr
: curr with
{
Content = def with
{
CompilationStatus = CompilationStatus.Unavailable,
CompilationError = response.Error
}
};
@github-actions

github-actions Bot commented Aug 8, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

1 142 tests   1 135 ✅  6m 31s ⏱️
   11 suites      7 💤
   11 files        0 ❌

Results for commit 6116854.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 8, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

685 tests   685 ✅  5m 17s ⏱️
 10 suites    0 💤
 10 files      0 ❌

Results for commit 6116854.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 8, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

1 796 tests   1 794 ✅  6m 7s ⏱️
   11 suites      2 💤
   11 files        0 ❌

Results for commit 6116854.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 8, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

   11 files     11 suites   5m 1s ⏱️
1 837 tests 1 731 ✅ 106 💤 0 ❌
1 937 runs  1 831 ✅ 106 💤 0 ❌

Results for commit 6116854.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 8, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

1 676 tests   1 485 ✅  5m 38s ⏱️
   11 suites    191 💤
   11 files        0 ❌

Results for commit 6116854.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 8, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

1 186 tests   1 186 ✅  8m 13s ⏱️
   11 suites      0 💤
   11 files        0 ❌

Results for commit 6116854.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 8, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

   65 files     65 suites   36m 50s ⏱️
8 322 tests 8 016 ✅ 306 💤 0 ❌
8 422 runs  8 116 ✅ 306 💤 0 ❌

Results for commit 6116854.

♻️ This comment has been updated with latest results.

Review caught a real violation: this PR added a hand-built <a> element inside
an existing Controls.Html call. AGENTS.md is absolute that HTML strings are
never emitted — the framework's controls render the markup — and the hand-built
form also forced manual HtmlEncode of both the href and the label, which is
exactly the escaping drift the rule exists to prevent.

Now Controls.Markdown with the styling on the control. Same anchor, escaped by
the renderer. 19/19 green (CompileErrorPageTest + RecompileSettlePredicateTest),
Release -warnaserror clean.

Note the two SIBLING Controls.Html anchors in this method (the release-published
link, the assembly-store line) are pre-existing and left alone here; they are
the same violation and want their own sweep rather than an unrelated widening of
this PR.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@rbuergi

rbuergi commented Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

Worked all three review findings; verdicts differ per finding.

1. Hand-built HTML anchor — VALID, fixed (6116854). You were right: this PR added an <a> string inside an existing Controls.Html call, which AGENTS.md forbids absolutely, and it forced manual HtmlEncode on both href and label — the escaping drift the rule exists to prevent. Now Controls.Markdown with the styling on the control; 19/19 green. Noting for the record that the two SIBLING Controls.Html anchors in the same method (release-published link, assembly-store line) are pre-existing and deliberately left alone — same violation, but they want their own sweep rather than widening this PR.

2. Hard-coded English in the Unavailable status copy — VALID but pre-existing style, deliberately not fixed here. The whole switch it joins is English constants (Compiling, Pending, Error, Waiting, No-compile-required all hard-code their label and prose). Localizing only the new arm would leave the block half-translated and inconsistent; localizing the block means deferring every constant to render time, which is a separate change the PR body already discloses. The genuinely NEW discrete strings (ui.viewCompileLog, ui.compileStateUnknown) ARE in both catalogs.

3. The write-back guard — accurate observation, deliberate design. You are right that in the most likely settle-timeout case the status is Pending/Compiling and nothing is persisted. That is the intent: a compile IS in flight and owns the terminal write, so stamping Unavailable over it would strand the dispatch. Unavailable earns its keep on the paths where no compile is running (the type-existence probe, pinned-release misses). Overwriting a stale Error is likewise intended — the latest attempt could not determine the state, so 'unknown' is truer than a stale 'failed'.

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.

Compile-failure overlay: show the compiler log; do not render lookup timeouts as compile failures

2 participants