Replace diagnostic-prose fact association with typed links

[woksin]

Learning

GenerationFactDispositionCalculator currently associates diagnostics with facts by parsing human-readable GenerationDiagnostic.Message text for quoted FactIds. Adapter-authored prose can accidentally or maliciously name another fact and change its disposition. Generation #24 requires typed report links and cannot treat diagnostic prose as causality.

Scope

• Add additive typed fact/claim association to generation diagnostics or an equivalent frozen pipeline record.
• Populate typed associations at resolver, derivation, lowering and disposition stages.
• Remove message parsing from disposition calculation.
• Keep human-readable messages presentation-only and preserve existing codes/severity/outcomes/source/subject APIs.

Acceptance criteria

• Diagnostic prose naming an unrelated FactId cannot affect that fact.
• Direct and derived facts receive exact typed diagnostic links.
• Conflicted/omitted dispositions retain deterministic associated diagnostics.
• Missing, duplicate, dangling and direct-to-derived typed links fail closed.
• Fact/diagnostic order and message changes do not alter dispositions or canonical output.
• Existing diagnostic messages remain source/binary compatible.
• Generation Project a deterministic realization and provenance report #24 can project links without parsing prose.
• Package validation and unchanged legacy/current consumers pass.

Related: #24, #19, #17.

[cratis-stagehand]

Investigation

I reproduced the vulnerability and mapped the change. Working tree is clean — the repro spec was removed after the run.

Reproduction (this is the evidence, not code reading). Added a temporary spec with two adapters: an honest one contributing event:AccountOpened + placement, and a hostile one contributing zero facts and one diagnostic:

Code = "HOSTILE0001", Severity = Information, Outcome = Conflict,
Message = "Nothing to see here for fact 'event:AccountOpened'"

Result from dotnet test Source/DotNET/Generation.Specs/Generation.Specs.csproj:

should_lower_the_fact_without_the_hostile_diagnostic         PASS  (Lowered)
should_still_lower_the_fact_with_the_hostile_diagnostic      FAIL  Expected: Lowered  Actual: Conflicted

An adapter that contributes no facts at all flipped another adapter's fact from Lowered to Conflicted purely through prose. Path: AdapterContributionSnapshot.Diagnostics → GenerationResolver.ResolveFacts seed → graph.Diagnostics → pipelineDiagnostics (ScreenplayDefinitionGenerator.cs:182) → GenerationFactDispositionCalculator.AssociatedDiagnostics → HasExactFactIdentity (GenerationFactDispositionCalculator.cs:359).

SUGGESTED-MODEL: opus

Plan — #51 Replace diagnostic-prose fact association with typed links

Settled — do not redo

1. The bug is real and reproduced, exactly as described. Severity is irrelevant; only Outcome == Conflict plus a substring match is needed. The hostile adapter needs no facts and no subject.
2. The single attack surface is GenerationFactDispositionCalculator.cs:355-394 (HasExactFactIdentity + InputFactIds + IsGranularDiagnostic). Nothing else in the pipeline parses messages.
3. GenerationFactDispositionCalculator.Calculate has exactly one production call site: ScreenplayDefinitionGenerator.cs:191.
4. The complete set of diagnostics that today match the parser (these and only these must gain typed links to preserve behavior):
   • GenerationFactDiscriminatorValidator.cs:162 and :329 (Unsupported(fact, …), reached from 13 call sites) → link [fact.Id]
   • GranularArtifactResolver.cs:436 (InvalidGranularFactOwnership) → [fact.Id]
   • GranularArtifactResolver.cs:495 (AddDiagnostic, 12 callers) → [.. CanonicalFacts(facts).Select(f => f.Id)]
   • GenerationResolver.cs:306 / :319 / :332 (InvalidConceptFact ×3) → [_.Id]
   • GenerationResolver.cs:358 (ConflictingFactIdentity) → [.. _.Facts.Select(f => f.Id)]
   • SpecificationFactResolver.cs:138 (InvalidSpecificationFact) → [fact.Id]
   • TypeUseBindingDerivation.cs:316 (Diagnostic(...), 9 callers) → [.. canonicalInputs.Select(i => i.Id)]
5. Everything else already associates typed-ly or not at all. All ~20 ScreenplayLowerer diagnostics associate through coverage.Diagnostics[semanticKey] (ScreenplayLoweringCoverage.cs:80) — no prose involved. All ConflictFor(...) resolver diagnostics, verification diagnostics, and every Generation.DotNet / Vogen adapter diagnostic carry at most a SubjectId and match nothing today.
6. Four places rebuild a GenerationDiagnostic field-by-field and will silently drop a new property unless updated — this is the complete list:
   • AdapterContributionFreezer.cs:811 (FreezeDiagnostics) — adapter contributions
   • AdapterRunCanonicalizer.cs:313 (Diagnostic)
   • Canonical.Diagnostic (Canonical.cs:131) — ordering + dedup key
   • Structural.Diagnostic (Structural.cs:161) — canonicalizer grouping key
7. No spec anywhere asserts on a message containing a fact id, and no in-repo adapter (Generation.DotNet, Vogen) emits fact-id prose. The "Input facts: " format is an undeclared coupling pinned by nothing.
8. Compatibility rules: Directory.Build.props:17-20 — package validation on, baseline 0.16.0, EnableStrictModeForBaselineValidation=false, so additive members are allowed; there are no CompatibilitySuppressions.xml and no API baseline .txt files. Validation runs on dotnet pack only (.github/workflows/dotnet-build.yml:50), followed by scripts/verify-package-consumers.sh.
9. Only ResolvedArtifactVariant carries SupportingFacts (ResolvedApplicationGraph.cs:19). ResolvedConceptRepresentationVariant, ResolvedConceptAttributeVariant, ResolvedConceptValidationRuleVariant, ResolvedArtifactPlacementVariant, ResolvedRelationship, and the specification resolved records do not — so their conflict diagnostics have no fact ids structurally available at construction time.
10. Specs are Cratis.Specifications + xunit, Establish/Because/[Fact] void should_*. The calculator is internal, so it is only reachable via Generator.Generate(snapshot, options) and asserted through _result.AdapterRun!.Facts. Harness: Source/DotNET/Generation.Specs/for_ScreenplayDefinitionGenerator/given/a_generator.cs — Completed(adapter, facts, diagnostics) already accepts adapter diagnostics, which is what the repro used. Run with dotnet test Source/DotNET/Generation.Specs/Generation.Specs.csproj.

Implementation

Step 1 — additive typed link on the contract

In Source/DotNET/Generation.Contracts/Diagnostics.cs, add to GenerationDiagnostic:

/// <summary>
/// Gets the typed identities of every fact this diagnostic is about.
/// </summary>
public ImmutableArray<FactId> Facts { get; init; } = [];

Additive, non-required, defaulted — source and binary compatible under the non-strict baseline. It does change the record's generated Equals/GetHashCode/PrintMembers, which is fine but should be noted in the PR.

Step 2 — fail-closed policy for adapter-authored links

GenerationDiagnostic is public with init accessors, so an adapter can set Facts. AdapterContributionFreezer.FreezeDiagnostics (:811) reconstructs each diagnostic explicitly, so the default is already fail-closed: unless it is taught to copy Facts, adapter links are dropped. Recommended policy — teach it to copy, but validate first, emitting an AdapterContributionAdmissionDiagnostic (via AdapterContributionAdmissionContext) and rejecting the contribution when:

• a linked FactId is not present in the same contribution's Facts (dangling / cross-producer — this is what closes the reproduced hole);
• a link is null / empty / non-normalized (missing);
• the same FactId appears twice in one diagnostic (duplicate);
• a link names a derived-fact identity (generation:derive:…) — an adapter cannot legitimately know one (this is my reading of "direct-to-derived"; see the decision below).

Step 3 — populate links at the pipeline sites

Change the 8 factory sites listed in Settled #4 to set Facts. These are all helper methods with the fact objects already in scope; the change is mechanical. Leave the messages byte-identical — they stay presentation-only and remain source-compatible.

Step 4 — remove the parser

In GenerationFactDispositionCalculator.cs, delete HasExactFactIdentity, InputFactIds, and IsGranularDiagnostic (:355-394), and rewrite AssociatedDiagnostics (:205) as:

coverage.Diagnostics[semanticKey]                                    // unchanged, lowering
∪ diagnostics.Where(d => d.Outcome is not null && d.Facts.Contains(fact.Id))

Keep CanonicalDiagnostics dedup so a diagnostic linked to several facts still appears once per record.

Step 5 — canonicalization and determinism

• Append the ordered Facts values to Canonical.Diagnostic and Structural.Diagnostic. Because both use length-prefixed fields with a fixed field count, no canonical string can be a strict prefix of another, so appending a trailing field only reorders diagnostics that were previously identical — i.e. ones that were being deduped anyway. Verify this against the existing order-permutation specs rather than assuming it.
• Copy Facts in AdapterRunCanonicalizer.Diagnostic (:313), sorted ordinal and de-duplicated, so permuted input yields byte-identical links.
• Note: two diagnostics with identical prose but different links will now stop collapsing into one. Confirm against when_generating_from_facts_in_different_orders.cs and when_generating_from_adapter_run_snapshots_in_different_orders.cs, whose AdapterRunProjection helper reflects over all public properties and will pick up Facts automatically.

Step 6 — specs

New specs under Source/DotNET/Generation.Specs/for_ScreenplayDefinitionGenerator/:

• when_a_hostile_adapter_names_another_adapters_fact.cs — the repro above, as a regression test. This is the acceptance criterion "prose naming an unrelated FactId cannot affect that fact."
• when_a_diagnostic_message_changes_but_links_do_not.cs — mutate message text only; dispositions and AdapterRun projection unchanged.
• when_an_adapter_diagnostic_links_a_dangling_fact.cs (in for_AdapterContributionAdmission/) — admission rejects.
• Parity coverage: extend the existing when_type_use_bindings_conflict.cs, when_member_declaration_orders_conflict.cs, when_a_granular_fact_asserts_a_different_subject.cs, when_a_malformed_type_use_shape_bypasses_admission.cs with assertions that the linked fact ids are exact.
• Add a spec asserting the derivation diagnostic links its input fact ids (TypeUseBindingDerivation), matching today's "Input facts:" semantics.

Step 7 — docs

Update Documentation/guides/build-source-adapter.md (the diagnostics section around :397 and the disposition table at :519) to document GenerationDiagnostic.Facts and the admission rule that links must name facts from the same contribution. Add a row to the README capability table (README.md:26-40) under "Current main".

Step 8 — gates

dotnet build -c Debug and -c Release (warnings are errors in Release), dotnet test Screenplay.Generation.slnx -c Debug, then dotnet pack -c Release -p:Version=9999.0.0 -o ./Artifacts/NuGet (package validation only runs on pack) and ./scripts/verify-package-consumers.sh 9999.0.0 ./Artifacts/NuGet.

Requires a human decision

1. Scope of link population beyond parity. Steps 3-4 keep every current disposition identical. But acceptance says "Direct and derived facts receive exact typed diagnostic links," and Project a deterministic realization and provenance report #24 wants a rich report. Going further — adding SupportingFacts to the concept/placement/relationship/specification resolved variants (Settled Add neutral concept attributes #9) so their ConflictFor(...) diagnostics can link facts, and linking lowerer diagnostics via ResolvedArtifactVariant.SupportingFacts — would change canonical output: facts that today get the synthesized ConflictingGenerationFact / OmittedGenerationFact fallback (GenerationFactDispositionCalculator.cs:146,187) would instead carry the real underlying diagnostic. My recommendation is two commits — parity + parser removal first, broadened links second — but whether the broadened set ships under this issue is the maintainer's call.
2. What "direct-to-derived typed links fail closed" means. I read it as: an adapter-contributed (direct) diagnostic may not link a derived fact id. The alternative reading is about the disposition calculator refusing to apply a link that crosses the direct/derived boundary in either direction. Step 2 assumes the first; confirm before implementing.
3. Whether adapters may author links at all. Dropping them unconditionally in FreezeDiagnostics is strictly safer and is the zero-work default; allowing same-contribution links is friendlier and preserves what external adapters (Arc, Screenplay.CritterStack) could do today via prose. I recommend allowing with validation, but this is a contract decision.

Suggested model for implementation: opus

---

Posted by Stagehand (AI) - an autonomous agent, not a person. Review accordingly.