Repository navigation
fix: a deleted NodeType gates portal readiness forever (#1391) - #1396
Conversation
"No sources matched" is not the same as "sources matched and did not compile."
The first is a CONTENT fact and must not gate readiness; the second is a verdict
about this image and must. PreWarmStatus.NoSources exists for exactly the first
case and is deliberately non-gating — but almost nothing could reach it.
Both classifiers additionally required `d.Sources is { Count: > 0 }`:
DynamicTypePreWarmer.ClassifyCompileFailure (activation-driven sweep)
NodeTypeBatchBake.StampAndReport (batch bake)
An empty NodeTypeDefinition.Sources does NOT mean "configuration-only". It means
"uses the DEFAULT queries" — `namespace:{path}/Source scope:subtree`, see
CodeQueryResolver.DefaultSources — which is how very nearly every NodeType in a real
mesh is authored. So the conjunct made NoSources unreachable for essentially the whole
population: delete a type's Source/ subtree and it is compiled against nothing, the
configuration lambda's resulting CS0246/CS1061 are recorded as an IMAGE regression, and
a node that no longer exists holds portal readiness on every pod boot.
That is what Edu/Course was doing to memex — deleted in the 2026-08-12 Edu rework,
failing on 100% of 23 sampled boots, with a diagnostic naming code that no longer exists:
Executed source queries (2):
- namespace:Edu/Course/Source scope:subtree nodeType:Code
- namespace:Edu/Course/Test scope:subtree nodeType:Code
Matched Code nodes (0):
(none) — the configuration lambda cannot reference types because no source
files were included.
CS0246: The type or namespace name 'CourseContent' could not be found
CS1061: 'LayoutDefinition' does not contain a definition for 'AddCourseLayoutAreas'
This restores the three-outcome contract SourceSnapshot.cs already documents — an
established-but-EMPTY snapshot is a content fact "(the sources were deleted, or the type
is configuration-only)" and classifies as NoSources — from which the extra conjunct had
silently drifted. A genuinely configuration-only type is folded in on purpose: with no
Code nodes there is nothing for an image to regress, so a failure in its configuration
lambda is content drift for its owner, not a reason to stall everyone else's rollout.
What is deliberately NOT changed:
- The GATE. This is a classification fix, never "compile errors stop gating". A type
whose sources are still present and do not compile keeps a non-empty snapshot and
keeps gating.
- The two-witness rule (#1216). The batch still requires BOTH its own resolved snapshot
AND the persisted CurrentSourceVersions, so a starved discovery cannot reclassify
itself as deleted content.
- A NULL snapshot still gates — "the watcher never seeded" is not "the sources are gone".
- NodeTypeBatchBake.Assemble's `unestablished` guard keeps its DeclaresSources exemption.
That one is load-bearing: a legitimately source-less type must not abandon the whole
batch, and the persisted snapshot is what tells the two apart at classification time.
The existing ClassifyCompileFailure_NoDeclaredSources_StaysCompileError asserted the old
behaviour on the premise that an empty Sources means the type "compiles from its
Configuration alone". That premise is wrong about the domain, so the test is replaced
rather than kept — with the reasoning recorded where the next reader will find it.
Verification (fail-before / pass-after):
before — ClassifyCompileFailure_DefaultQueriesWithEmptySnapshot_IsNoSources FAIL
DeletedDefaultQueryType_DoesNotGateReadiness FAIL
after — 46/46 NodeTypeBakeGateStateTest; 55/55 including
SourceDiscoveryUnavailableTest, SourceSnapshotEstablishmentTest,
NodeTypeBatchBakeDiscoveryInvariantTest
Both regression guards pass in BOTH states, which is what a guard should do.
Release -warnaserror clean: MeshWeaver.Hosting, Hosting.Monolith.Test.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes how NodeType compile failures are classified during pre-warm/batch bake so that “no sources matched” (deleted or genuinely source-less content) is treated as a non-gating content condition (PreWarmStatus.NoSources), rather than a gating image regression (CompileError). This prevents deleted/default-query NodeTypes from holding portal readiness indefinitely.
Changes:
- Adjust
DynamicTypePreWarmer.ClassifyCompileFailureto key solely offCurrentSourceVersionsbeing explicitly empty, removing the (incorrect) dependency onNodeTypeDefinition.Sourcesbeing non-empty. - Align batch bake classification (
NodeTypeBatchBake.StampAndReport) with the activation path so default-query NodeTypes can reachNoSourceswhen their resolved source set is empty and the persisted snapshot corroborates it. - Update/replace regression tests and add an end-to-end gate assertion for deleted default-query NodeTypes; add a WhatsNew entry describing the user-visible fix.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| test/MeshWeaver.Hosting.Monolith.Test/NodeTypeBakeGateStateTest.cs | Replaces the old assumption about empty Sources and adds end-to-end gate coverage for deleted vs. broken default-query NodeTypes. |
| src/MeshWeaver.Hosting/NodeTypeBatchBake.cs | Updates batch bake’s compile-failure classification to treat empty resolved sources as NoSources without requiring declared queries, while keeping the two-witness rule. |
| src/MeshWeaver.Hosting/DynamicTypePreWarmer.cs | Fixes the activation-path classifier so explicitly empty snapshots classify as NoSources regardless of whether queries were declared. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-13-a-deleted-page-type-no-longer-holds-up-the-portal-starting.md | Documents the behavioral fix as a user-facing “Fix” entry. |
Suppressed comments (1)
src/MeshWeaver.Hosting/DynamicTypePreWarmer.cs:832
- The XML doc cref
Graph.Configuration.SourceSnapshotdoesn’t resolve (no such namespace). Also,SourceSnapshotis internal toMeshWeaver.Graph.Configuration, so a cross-assembly<see cref>may not bind even with the correct namespace; using<c>SourceSnapshot</c>avoids broken cref warnings.
///
/// <para>This restores the three-outcome contract <see cref="Graph.Configuration.SourceSnapshot"/>
/// already documents — an established-but-EMPTY snapshot is a content fact "(the sources were
/// deleted, or the type is configuration-only)" and classifies as <c>NoSources</c> — from which
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| /// does not mean "configuration-only" — it means <b>"uses the DEFAULT queries"</b> | ||
| /// (<c>namespace:{path}/Source scope:subtree</c>, see | ||
| /// <see cref="Graph.Configuration.CodeQueryResolver.DefaultSources"/>), which is how very | ||
| /// nearly every NodeType in a real mesh is authored. Requiring declared queries therefore made |
…lt (#1391) The member doc still said 'the type's DECLARED source queries', which is the premise the classifier fix removes. Restates it as the effective queries and records why the declared-only condition must not come back. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Test Results (shard 1)1 474 tests +21 1 474 ✅ +21 7m 34s ⏱️ +26s Results for commit 6346405. ± Comparison against base commit b073c6b. This pull request removes 1 and adds 22 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Test Results 66 files ± 0 66 suites ±0 43m 41s ⏱️ - 2m 1s Results for commit 6346405. ± Comparison against base commit b073c6b. This pull request removes 1 and adds 42 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
…ST (#1391) Three DynamicTypePreWarmerTest failures on CI were caused by my diff, and they were RIGHT. This is the classifier's fault, not the fixtures'. An empty CurrentSourceVersions has two very different causes, and the previous commit conflated them: Sources were DELETED (Edu/Course) → content verdict, must NOT gate NEVER had sources, own Configuration is broken → authored defect, MUST gate All three fixtures are the second kind — `Configuration = "config => this is not valid C# at all (("` with no sources — so reclassifying them to NoSources also downgraded their dependents' cascade from the gating UpstreamFailed to the non-gating UpstreamContentBroken. That is a real weakening of the gate, which is precisely what those tests exist to catch. Their assertions are untouched. The second witness is LastCompileSucceededAt: a type that has NEVER built cannot have LOST anything. It survives a failure — ApplyCompileFailure clears CompiledSources but never the success stamp — so it is readable exactly where the verdict is formed. Verified against production, 11/11, on two independent durable signals: Edu/Course + 3 × Doc/* non-null → reclassify (their sources WERE deleted) 6 × FutuRe/{region}/* null → gate. No `sources` key and no Source/ subdirectory in the repo, yet their lambdas reference LineOfBusiness / AddLineOfBusinessLayoutAreas. Broken as authored — the real-world twin of the test fixtures. Type/article null → gate. Never compiled: the importer derives the node path from the FILENAME while the Code node lands under the id (capital A). 2 × Northwind (live sources) n/a → gate, on the non-empty snapshot as before. mesh_node_history corroborates the same 3/7 split — those 7 never recorded a source across 44-47 versions each. Blast radius: 4 of 16 failures reclassify; memex-cloud keeps 9 gating, memex keeps 1 (BinaryClickerV2/BinaryToggle, null, correctly gating). Two traps recorded in the code so they are not "simplified" back: - THREE shapes of CurrentSourceVersions exist in production, not two: populated, explicitly {}, and ABSENT (SQL NULL, on public.mesh_nodes rows). Only the middle may reclassify. `is { Count: 0 }` is right BY CONSTRUCTION — a C# property pattern never matches null — so absent gates. Do not rewrite it as a bare `.Count == 0`, which throws. - CompiledSources would be the more precise evidence but is useless as history: written only on success and nulled on failure, so every currently-failing type in production lacks it, INCLUDING the three that demonstrably compiled in June. Also documents the inverse of the deletion hazard in NodeTypeCompilation.md: ADDING a framework symbol that in-mesh source calls has the same invisibility. #1386 moved an Article extractor to MarkdownBody.Of and the in-mesh callers shipped before the image, leaving three types at CS0103 in prod until the portal self-updated. The framework half must land first. Verification: 67/67 across DynamicTypePreWarmerTest (all three previously-red, fixtures UNCHANGED), NodeTypeBakeGateStateTest, SourceDiscoveryUnavailableTest, SourceSnapshotEstablishmentTest, NodeTypeBatchBakeDiscoveryInvariantTest. Doc link + What's New integrity green. Release -warnaserror clean: Hosting, Hosting.Monolith.Test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…es' into fix/1391-nosources-default-queries
Part of #1391. Independent of #1393 (#1387) — different root, evidence in the issue.
The rule, stated
"No sources matched" is not the same as "sources matched and did not compile." The first is a CONTENT fact and must not gate readiness; the second is a verdict about this image and must.
PreWarmStatus.NoSourcesexists for exactly the first case and is deliberately non-gating. Almost nothing could reach it.🚨 And "no sources matched" needs a second witness, which the first revision of this PR got wrong — see Round two below. The sources must have been LOST, not merely absent.
The defect
Both classifiers additionally required
d.Sources is { Count: > 0 }:DynamicTypePreWarmer.ClassifyCompileFailureNodeTypeBatchBake.StampAndReportAn empty
NodeTypeDefinition.Sourcesdoes not mean "configuration-only". It means "uses the DEFAULT queries" —namespace:{path}/Source scope:subtree, seeCodeQueryResolver.DefaultSources— which is how very nearly every NodeType in a real mesh is authored.So the conjunct made
NoSourcesunreachable for essentially the whole population. Delete a type'sSource/subtree and it is compiled against nothing, the configuration lambda's resultingCS0246/CS1061are recorded as an image regression, and a node that no longer exists holds portal readiness on every pod boot.That is what
Edu/Coursewas doing tomemex— failing on 100% of 23 sampled boots, with a diagnostic naming code that no longer exists:Not a new policy — a restored one
SourceSnapshot.csalready documents the intended three-outcome contract, and it is unambiguous:The extra conjunct had silently drifted from that. A genuinely configuration-only type is folded in on purpose, exactly as that contract says: with no Code nodes at all there is nothing for an image to regress, so a failure in its configuration lambda is content drift for its owner to fix, not a reason to stall everyone else's rollout.
What is deliberately NOT changed
CurrentSourceVersions, so a starved discovery cannot reclassify itself as deleted content.NodeTypeBatchBake.Assemble'sunestablishedguard keeps itsDeclaresSourcesexemption. That one is load-bearing and I left it alone on purpose: a legitimately source-less type must not abandon the whole batch, and the persisted snapshot is what tells the two apart at classification time. Fixing it too would make any configuration-only type abandon every batch.A tested behaviour is being changed, deliberately
ClassifyCompileFailure_NoDeclaredSources_StaysCompileErrorasserted the old behaviour, on the premise that an emptySourcesmeans the type "compiles from its Configuration alone". That premise is wrong about the domain — it describes a tiny minority as if it were the definition. The test is replaced rather than kept, with the reasoning recorded where the next reader will find it.Round two: an empty snapshot alone was too coarse
The first revision keyed only on
CurrentSourceVersionsbeing empty. CI caught that with three reds inDynamicTypePreWarmerTest, and they were right — this was a defect in my classifier, not in the fixtures.All three build their "broken" type as
Configuration = "config => this is not valid C# at all (("with no sources. That is not deleted content; it is a type that is broken as authored, and it must gate. Reclassifying it also downgraded its dependents' cascade from the gatingUpstreamFailedto the non-gatingUpstreamContentBroken— a real weakening of the gate, which is exactly what those tests exist to catch.The second witness is
LastCompileSucceededAt: a type that has NEVER built cannot have LOST anything. It survives a failure —ApplyCompileFailureclearsCompiledSourcesbut never the success stamp — so it is readable exactly where the verdict is formed.The three failing tests now pass with their assertions and fixtures completely untouched (
git diff origin/main...HEAD -- .../DynamicTypePreWarmerTest.csis empty). The production rule got stricter; nothing was loosened to make a test green.Verified against production, 11/11, on two independent durable signals
lastCompileSucceededAtEdu/Course+ 3 ×Doc/*FutuRe/{AmericasIns,AsiaRe,EuropeRe}/{LineOfBusiness,TransactionMapping}sourceskey and noSource/subdirectory in the repo, yet their lambdas referenceLineOfBusiness/AddLineOfBusinessLayoutAreas. Broken as authored: the real-world twin of the test fixturesType/articleid(Type/Article/Source/Article, capital A)Northwind/{AnalyticsCatalog,Product}mesh_node_historycorroborates the same 3/7 split independently — those 7 never recorded a source across 44–47 versions each.Blast radius: 4 of 16 failures reclassify. memex-cloud keeps 9 gating, memex keeps 1 (
BinaryClickerV2/BinaryToggle, null snapshot, correctly gating). The ones left gating deserve to.Two traps recorded in the code so they are not "simplified" back
CurrentSourceVersionsexist in production, not two: populated, explicitly{}, and ABSENT (SQL NULL, onpublic.mesh_nodesrows). Only the middle may reclassify.is { Count: 0 }is right by construction — a C# property pattern never matches null — so absent gates. Do not rewrite it as a bare.Count == 0, which throws.CompiledSourceswould be the more precise evidence but is useless as history: written only on success and nulled on failure, so every currently-failing type in production lacks it — including the three that demonstrably compiled in June.Also: the inverse hazard, documented
NodeTypeCompilation.mdcovered deleting a framework symbol in-mesh code calls. Adding one has the same invisibility: #1386 moved an Article extractor into compiled framework code asMarkdownBody.Of, and the in-mesh callers shipped before the image did —ACME/Article,Cornerstone/ArticleandNorthwind/Articlesat atCS0103: The name 'MarkdownBody' does not existon memex-cloud until the portal self-updated toci.3435, at which point all three returned toOkon their own. No action needed; the rule ("the framework half must land first") is now written down.Verification
Fail-before / pass-after:
ClassifyCompileFailure_DefaultQueriesWithEmptySnapshot_IsNoSourcesDeletedDefaultQueryType_DoesNotGateReadiness(end-to-end through the gate)ClassifyCompileFailure_DefaultQueriesWithSourcesStillPresent_StaysCompileErrorBrokenDefaultQueryTypeWithLiveSources_StillGatesReadiness(end-to-end)Both regression guards pass in both states — which is what a guard should do; they exist so the fix cannot widen later.
Plus the round-two boundary, both directions:
ClassifyCompileFailure_EmptySnapshotButNeverBuilt_StaysCompileErrorNeverBuiltSourcelessBrokenType_StillGatesReadiness(end-to-end)DynamicTypePreWarmerTestreds, fixtures UNTOUCHEDDynamicTypePreWarmerTest,NodeTypeBakeGateStateTest,SourceDiscoveryUnavailableTest,SourceSnapshotEstablishmentTest,NodeTypeBatchBakeDiscoveryInvariantTest.DocumentationLinkIntegrityTest+WhatsNewEntryIntegrityTestgreen.-warnaserrorclean:MeshWeaver.Hosting,MeshWeaver.Hosting.Monolith.Test.SourceDiscoveryUnavailableTest.StarvedSourceDiscovery_IsNotACompileVerdict_WhileARealCompileErrorStillIswas raised as possibly sharing a root: it does not. Its broken control does create a{path}/Source/codenode, so its snapshot is non-empty and this change cannot reach it — green locally throughout.Confirmed against the durable row — the discriminator is 11 for 11
The one thing that could have made this fix a no-op for the real case is
Edu/Coursecarrying a stale non-emptycurrentSourceVersionsinstead of an explicitly empty one. It does not. Fromedu.mesh_nodes(dbmemex, read-only SELECT):Type
object, key count0— andsourcesis SQL NULL, i.e. default queries. So this fix does reclassify it.The node itself is
state=2(ACTIVE); what is gone is itsSource/subtree — 0 rows inedu.code, and zero rows inedu.mesh_node_historyfor those paths, so a hard delete rather than a recycle.Edu/_GitSyncdates it: the last version with a non-empty map is v282 at2026-07-11 18:49:45.545+00, matchinglastSyncedAt 2026-07-11T18:49:45.618+00(sha4ce635b). The GitSync import of the Plugins repo'sEdu/subdirectory removed the sources and left the NodeType node Active. v283→v301 are bake retries against nothing.Across both portals the discriminator separates cleanly, with the DB
LIKE '{path}/Source/%'count and the runtimeMatched Code nodes (N)agreeing on every row:currentSourceVersionsEdu/Course(memex)MeshWeaver/samples/Graph/Data/*/Article,Northwind/{AnalyticsCatalog,Product})CS0103/CS0246breakage11 for 11, zero false positives, no overlap. The five REAL failures keep gating, which is the property the regression guards exist to protect.
The third shape, already handled
public.mesh_nodesholds rows whosecurrentSourceVersionsis absent / SQL NULL — a third shape beyond "empty" and "populated". No change needed:d.CurrentSourceVersions is { Count: 0 }is a C# property pattern, which never matches null, so absent ⇒CompileError⇒ still gates. That is the intended "the watcher never seeded is not the sources are gone" rule, pinned byClassifyCompileFailure_DefaultQueriesWithNullSnapshot_StaysCompileError.Still open on #1391 (not addressed here)
The GitSync importer leaves a NodeType node behind when its sources go away or land at a different path.
Edu/Courseis one instance;MeshWeaver/samples/Graph/Data/Type/articleis another and is a plain platform bug — the importer derives the node path from the FILENAME (article.json) while the Code node lands under theid(Type/Article/Source/Article), so the default query can never match.This PR stops those from holding readiness hostage. It does not stop the importer creating the condition, and it does not clean up the existing rows. Both are separate, and any data repair is the owner's call to run — nothing was mutated during this investigation (SELECT-only). The repair options are written up on the issue.
🤖 Generated with Claude Code