Repository navigation
feat: diagnostics name the assembly a NodeType's content type resolved to (#4158 ask 2) - #4236
Conversation
…d to (#4158 ask 2) Every identity on a `get_diagnostics` reply was a CLAIM some build wrote. `mvid` is `NodeTypeDefinition.LatestAssemblyMvid` — the bytes a BUILD produced — and the record beside it says in as many words that `LatestAssemblyPath` "is an ADDRESS, not an identity". Neither is the statement the serializer and `/schema/<Type>` act on: which CLR type this NodeType's content resolves to HERE, and which assembly that type came from. Those diverge whenever two builds of one assembly NAME are in the process (#3732 — a module bundle on the shelf and a runtime-compiled collectible build), and that is exactly the state in which a stale ADOPTED build and a stale registry SHELF read identically from a consumer (#1585, a night of diagnosis). So the reply carries a `contentType` block, read from the live IMeshContentTypeRegistry at the moment of the call: `status`, `typeName`, `assembly` (Assembly.Location), `mvid` (ManifestModule.ModuleVersionId, the value the compiler stamps INTO the bytes) and `collectible`. Present on every branch — Ok, Error, Compiling, Unavailable, Foreign, Unknown and the degraded-JsonElement path, which is the one that most needs it. Nothing gates on it; it is a reading, like `built-from=` from ask 1. Watched failing on unmodified `main` (both cases, 2/2, payload quoted in the PR): the block must be present whatever the answer is. Got: {"status":"Ok","nodeTypePath":"DiagUntyped","message":"Compile SUCCEEDED …"} Three rules, each with a case that fails without it: - Absence is PRINTED, never inferred. `unresolved` (asked, nothing registered) and `not-asked` (no registry in this process) are two different sentences — a missing instrument may not read as a clean result. The `unresolved` sentence names the consequence, "its views render empty", because that is what the reader is looking at. - Every key is written, always. The block is an explicit JsonObject, not a serialised anonymous type — measured on the first attempt: the hub's options stamp a polymorphic `$type` on a NESTED object (putting `<>f__AnonymousType15`6[…]` into an envelope people read) and their ignore condition DROPS null and default members, so `typeName`, `assembly`, `mvid` and a `false` `collectible` all vanished, leaving a reader unable to tell "not reported" from "reported as absent". - It is read from the TYPE, never from the path. The control loads the test assembly's own bytes into a collectible AssemblyLoadContext so the same full name is present twice; the two readings must differ. If the answer were a function of the NodeType path — or of the node's record — they would be identical and that case goes red. `FormatDiagnostics` grows an OVERLOAD, never a defaulted parameter: a defaulted one replaces the signature a previously-built module calls (the same rule #2471's overload was added under). Conserved: Doc/Architecture/Modules → "…and the RECORD is not the resolution", beside ask 1's "The file is not the source"; a row on BuildIdentityAdmission's reporting table; a What's New entry. Fixes #4158 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Test Results (shard 0)262 tests 262 ✅ 2m 16s ⏱️ Results for commit 59ab7ac. |
Test Results (shard 1)1 443 tests 1 443 ✅ 2m 19s ⏱️ Results for commit 59ab7ac. |
Test Results (shard 2)898 tests 898 ✅ 3m 28s ⏱️ Results for commit 59ab7ac. |
Test Results (shard 4)1 831 tests 1 639 ✅ 4m 26s ⏱️ Results for commit 59ab7ac. |
Test Results (shard 5) 6 files 6 suites 5m 19s ⏱️ Results for commit 59ab7ac. |
Test Results (shard 3)1 047 tests 1 045 ✅ 5m 57s ⏱️ Results for commit 59ab7ac. |
Test Results 18 files 18 suites 23m 47s ⏱️ Results for commit 59ab7ac. |
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved diagnostic coverage and type-source issues remain, along with inaccurate assembly-provenance documentation.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Adds contentType diagnostics identifying the CLR type and assembly resolved for a NodeType.
Changes:
- Extends diagnostic responses with resolved type, assembly, MVID, and load-context data.
- Adds assembly-resolution and wire-format tests.
- Updates architecture and What's New documentation.
File summaries
| File | Summary |
|---|---|
test/MeshWeaver.Messaging.Hub.Test/ResolvedContentTypeNamesTheLoadedAssemblyTest.cs |
Tests assembly identity across load contexts. |
test/Memex.Portal.Shared.Test/DiagnosticsNameTheContentTypeAssemblyTest.cs |
Tests diagnostic wire output. |
src/MeshWeaver.Messaging.Hub/Serialization/ResolvedContentType.cs |
Defines resolved content-type metadata. |
src/MeshWeaver.Mesh.Operations/MeshOperations.cs |
Integrates content-type diagnostics. |
src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-13-diagnostics-say-which-assembly-a-types-content-comes-from.md |
Announces the diagnostic feature. |
src/MeshWeaver.Documentation/Data/Architecture/Modules.md |
Documents the diagnostic payload. |
src/MeshWeaver.Documentation/Data/Architecture/BuildIdentityAdmission.md |
Adds the diagnostic surface to identity documentation. |
Review details
Suppressed comments (5)
src/MeshWeaver.Documentation/Data/Architecture/Modules.md:662
- This newly added table and explanation present
collectibleplus a null location as a definitive shipped-vs-runtime-compiled classifier. That is not valid in this codebase: shipped module assemblies are loaded in a collectibleModulesAssemblyLoadContext, and compiled assemblies can be loaded from file-backed cache paths byNodeAssemblyLoadContext. The documentation should describe the observed location/MVID/load-context state and avoid claiming that those fields establish origin.
| `collectible` | `true` ⇒ a runtime compile in this process; `false` ⇒ a module shipped with it |
`collectible` + a null `assembly` is what separates the two builds of one name: an in-process compile
has no file and a collectible context, a shipped module has both the other way round. Nothing gates on
src/MeshWeaver.Documentation/Data/WhatsNew/2026-09-13-diagnostics-say-which-assembly-a-types-content-comes-from.md:34
- This What's New entry repeats an origin inference that the runtime cannot make: shipped modules are loaded by
ModulesAssemblyLoadContextwithisCollectible: true, and runtime-compiled NodeType assemblies can be loaded from a file byNodeAssemblyLoadContext. Consequently, “whether it was compiled in this installation” and “a type compiled here has no file and is marked as such” are false for valid load paths. Describe these as the assembly's observed location and collectibility instead of presenting them as shipped-vs-compiled provenance.
- whether it was **compiled in this installation** or arrived as part of one.
That last pair is what separates the two cases that used to look the same. A type compiled here has no
file and is marked as such; a type that shipped with a module has a file and is not. When both are
present at once — which happens whenever a piece of code is being replaced — you can now see which of
src/MeshWeaver.Mesh.Operations/MeshOperations.cs:4167
- This resolver does not read the type that the claimed
/schema/<Type>consumer actually uses.ResolvedContentType.Ofasks the mesh-wide path map, whileDataExtensions.GenerateJsonSchemaresolvestypeDefinitionfrom the schema handler's hub-localITypeRegistrybefore callingGetJsonSchemaAsNode(src/MeshWeaver.Data/DataExtensions.cs:3824-3844). During a pinned/older NodeType hub or simultaneous rebuild, those maps can contain different CLRTypeobjects, so diagnostics can report Type B while schema serializes Type A. Use the type held by the same schema/serializer boundary andResolvedContentType.Describe(Type), or narrow the contract to explicitly say this is only the mesh-wide registry reading.
private ResolvedContentType ResolveContentType(string nodeTypePath)
=> ResolvedContentType.Of(
hub.ServiceProvider.GetService<IMeshContentTypeRegistry>(), nodeTypePath);
test/Memex.Portal.Shared.Test/DiagnosticsNameTheContentTypeAssemblyTest.cs:73
- The integration coverage exercises only the
Ok/resolved andOk/unresolved cases. The new formatter is also wired forCompiling,Error,Foreign,Unavailable, andUnknown, and the implementation has direct-return branches that bypass the overload; without assertions for those wire shapes, a regression can silently removecontentTypefrom exactly the cases this PR says it covers. Add cases (or a table-driven formatter test) for every status and early exit.
[Fact(Timeout = 120_000)]
public async Task ARegisteredContentTypeIsNamedByItsOwnAssemblyAndMvid()
test/Memex.Portal.Shared.Test/DiagnosticsNameTheContentTypeAssemblyTest.cs:67
- This new test bridges the Rx sequence with
FirstAsync()directly.FirstAsync()completes its task from inside the Rx signalling path, so the test continuation can resume inline on the trampoline; use the repository's.Await()/ObservableAwaitbridge here (as intest/Memex.Portal.Shared.Test/CatalogCategoryFirstTest.cs:283-287) rather than awaiting the raw Rx task.
var json = await new MeshOperations(Mesh).GetDiagnostics(nodeTypePath)
.Timeout(TestTimeouts.CrossSilo).FirstAsync();
- Files reviewed: 7/7 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| return Observable.Return(FormatDiagnostics( | ||
| CompilationStatus.Ok, nodeTypePath, | ||
| error: null, startedAt: null, lastCompiledAt: null, | ||
| hub.JsonSerializerOptions)); | ||
| hub.JsonSerializerOptions, | ||
| publishedAssemblyMvid: null, | ||
| ResolveContentType(nodeTypePath))); |
| Resolved => $"{TypeName} from " | ||
| + (Assembly ?? "an assembly loaded from bytes (no file)") | ||
| + $" (mvid {Mvid ?? "unreadable"}" | ||
| + (Collectible ? ", collectible — compiled in this process)" : ")"), |
| is what an operator is actually looking at. | ||
| - **Every key is written, always.** The block is composed as an explicit `JsonObject`, not serialised | ||
| from an anonymous type. Measured on the first attempt: the hub's options stamp a polymorphic `$type` | ||
| on a NESTED object (putting `<>f__AnonymousType15`6[…]` into an envelope people read) and their |
The question nobody could ask
get_diagnosticsanswered entirely in claims.mvidisNodeTypeDefinition.LatestAssemblyMvid— the identity of the bytes a BUILD produced — and the record beside it says in as many words that
LatestAssemblyPath"is an ADDRESS, not an identity".[ModuleLoad]names the assembly a moduleLOADED. None of them is the statement the serializer and
/schema/<Type>actually act on: whichCLR type this NodeType's content resolves to here, and which assembly that type came from.
Those are different statements whenever two builds of one assembly NAME are in the process (#3732 —
a module bundle on the shelf and a runtime-compiled collectible build), and that is exactly the state
in which a stale ADOPTED build and a stale registry SHELF read identically from the consumer. That is
the night #4158 was filed about (Plugins#1585: newest generation, newest
written=, types predatingtwo merged PRs; three
RefreshModulesand two restarts spent on the wrong reading).What the source is — and why it is a property of the BYTES
The brief was to find a source that is a property of the bytes rather than of the run. It exists and
was already in hand, discarded:
ContentDegradationRegistry.IsResolvableresolves theTypeandthrows it away with
out _, andGenerateJsonSchemaholds the veryTypeit is about to serialisefrom. From a resolved
Type:mvidAssembly.ManifestModule.ModuleVersionIdassemblyAssembly.LocationcollectibleAssembly.IsCollectibleSo the reply now carries a
contentTypeblock, read from the liveIMeshContentTypeRegistryat themoment of the call — on every branch (Ok, Error, Compiling, Unavailable, Foreign, Unknown, and
the degraded-
JsonElementpath, which is the one that most needs it). Nothing gates on it; it is areading, exactly like ask 1's
built-from=.Watched failing on unmodified
mainThe control was built and run in a detached worktree at
origin/main(225c886dd4) with only thetest file added — it compiles there (it asserts the WIRE literals
"resolved"/"unresolved", not thenew constants) and both cases fail, 2/2:
On this branch: 2/2 pass, and the payload is the answer the issue asked for:
The negative control — quoted
Two, one per level.
The wire control (same mesh, same call, a NodeType with no registered content type). If the block
were a constant of the reply, or read off the node's record rather than the live registry, both cases
would answer the same way and this one fails:
The two-builds control (
ResolvedContentTypeNamesTheLoadedAssemblyTest) — the #3732 shape, builtfor real: the test assembly's own bytes loaded into a collectible
AssemblyLoadContext, so the SAMEfull name is present twice in the process.
The precondition of that control is asserted, never skipped — a skip would make the one case that
can falsify this reading silently vacuous, which is the failure mode the file is about.
Two things measured on the first attempt, and fixed rather than worked around
Both would have shipped a block that looks right and reads wrong:
$typeon a NESTED object. The hub's serializer put<>f__AnonymousType15\6[String,String,String,String,Boolean,String]` — a compiler-generated name —into a diagnostics envelope people read.
typeName,assembly,mvidand afalsecollectibleall vanished, so a reader could not tell not reported from reported asabsent — the one distinction
statusexists to make.The block is therefore composed as an explicit
JsonObject: every key written, always, independent ofhow any hub configures its serializer.
Shape
FormatDiagnosticsgrows an OVERLOAD, never a defaulted parameter — a defaulted one replaces thesignature a previously-built module calls (the rule #2471's overload was added under). The existing
overloads are byte-for-byte unchanged in output (
contentType: null⇒ no block), so no existingconsumer moves.
Conserved
Doc/Architecture/Modules→ "…and the RECORD is not the resolution ([ModuleLoad] cannot name the commit a bundle was built from, nor the assembly a NodeType's content type resolved to — a stale adopted build and a stale registry shelf are indistinguishable from the consumer #4158, ask 2)", beside ask1's "The file is not the source".
BuildIdentityAdmission's Where the honest status is reported table.Category: Feature).dotnet test MeshWeaver.Documentation.Test→ 459/459, link integrity included.Verification
Fixes #4158 · Pairs-with: none — this adds a public record and an overload; nothing leaves
src/.🤖 Generated with Claude Code