Skip to content

The root card names the leaves it consumed: a story's root advice now says who was behind the chain — the parked application, the lead blocker, the job — so a CRITICAL leaf walked under a higher root is no longer nameless (both engines, partial #3691) - #3759

Merged
erikdarlingdata merged 1 commit into
devfrom
fix/3691-root-names-leaves
Sep 19, 2026

Conversation

@erikdarlingdata

Copy link
Copy Markdown
Owner

What does this PR do?

Headline mechanism. A story's ROOT card now names the leaves the traversal consumed under it. InferenceEngine.BuildStories records, on each story, the hops after the root whose fact names a thing — the application that parked a transaction, the lead-blocker session, the Agent job, the table, the statement — highest severity first, at most three (AnalysisStory.NamedHops); FactAdvice.PopulateStoryText then composes one sentence per hop onto the root's frozen investigation: Behind it: PG_IDLE_IN_TRANSACTION(severity 1.20) —billing-worker as billingonappdb: billing-worker as billing idle in transaction for 15 min — the application never committed or rolled back, and it is holding the xmin horizon back. Both engines by construction — the change is in the shared engine and composer, and the roster of which keys name something is explicit per engine in the new FactIdentity. PARTIAL for #3691 — checklist line: **A CRITICAL leaf consumed mid-chain is never NAMED by the root**. Do not close.

The defect (v2 exit check §WRONG 2)

PG_IDLE_IN_TRANSACTION at 1.2 names its holder — application, role, database — in its own metadata and advice (lane 14, #3733). But when the saturation fact outranks it (1.25) and the share bar is met, the greedy traversal walks saturation → idle and CONSUMES the idle fact: it roots no card, and the saturation card's advice never said who was behind the parked sessions. grep billing over the whole analyze_server + get_analysis_findings payloads returned nothing. An operator reading the only card the pass produced could not find the application. The SQL Server engine has exactly the same shape: LCK → BLOCKING_CHAIN hides the lead-blocker session behind the wait's card; SCH_M / WRITELOG / IO_WRITE_LATENCY_MS → RUNNING_JOBS the job behind the symptom's (the SCH_M composer's LinkedJobClause already worked around it for that one key — this makes the rule general).

Why this shape

Identity is a contract over the seams the collectors already fill, not a new field. The facts that name something already carry the name: Fact.ObjectName (the one string seam the doubles-only metadata cannot replace — "application as role", a table, a slot), Fact.DatabaseName, the key's own suffix for the two per-statement families (BAD_ACTOR_<query_hash>, PG_BAD_ACTOR_<queryid>), and one numeric seam (BLOCKING_CHAIN's worst_apex_spid). A third string property on Fact would be a second copy of the same name that sixteen collector partials owned by sixteen lanes had to remember to fill. And "has an ObjectName" is NOT the contract: the PostgreSQL wait facts carry their event's display name there (the key restated, not a who), the config-change fact its setting names. So FactIdentity holds an explicit roster per engine — SqlServerKeys (4 + the BAD_ACTOR_ prefix), PostgresKeys (11 + PG_BAD_ACTOR_) — and Describe(fact) reads each key's identity off its seam; an empty seam (a lock rollup with no top relation, a job fact with no name) is no identity and the hop is simply not named. Both rosters are pinned by exact set.

Recorded in the engine, rendered by the composer. BuildStory is the one place the traversal's raw path and the >0 working set are both in hand, so the engine decides WHICH hops are named, in what order, how many (FactIdentity.NamedHopsOf: hops after the root, Severity descending with path order on ties, Take(3), root excluded because its own advice names it). PopulateStoryText is the one place the full fact set and the story meet, so it renders — reading each hop's own composed headline from the full set the same way the root's values are read. The sentence goes on Investigation, not Remediation: the hop's name is where to look; what to do about the sibling stays the sibling family's words (the reverse links the composers already carry keep saying it in their own voice).

One sentence shape, always identity-then-headline. I tried a dedupe rule (drop the identity clause when the hop's headline already names its subject — lane 14's idle headline and the job composer both do) and rejected it: it dropped the database, which the idle headline does not carry and which is part of the identity this lane exists to surface, and its behaviour depended on how a sibling lane happened to phrase a headline. So the holder appears twice in the idle sentence — as the label and inside the finding — and FactIdentity.Sentence says why that is accepted.

Cap of three, highest first. The longest traversal is MaxPathDepth (10) hops; a chain naming ten things would bury the root's own investigation under its leaves. Path still lists every hop.

Where the name lands. StoryText — the compact {h,i,r} blob both finding stores already copy onto the persisted row and both MCP tools already render (analyze_server line ~209 and get_analysis_findings line ~925 in DarlingMcpTools.cs emit advice.Investigation from FactAdvice.GetForFinding / GetComposedForFinding; Lite's twins the same). So the holder reaches every card, e-mail and payload without the MCP payload files changing — lane 21 owns them and they are untouched. RootFactMetadata is Dictionary<string, double> and cannot carry a string, so the brief's named_hops: [{key, identity, severity}] metadata form is not available; the typed AnalysisStory.NamedHops list is the record of which hops were named (ephemeral, like MatchedAmplifiers), and the story text is what persists.

Blast radius — SQL Server story text CHANGES where a consumed hop has an identity (intended, stated)

Every SQL Server chain whose hop is BLOCKING_CHAIN (with an apex spid), RUNNING_JOBS (with a job name), BAD_ACTOR_*, or an object anomaly with a table gains sentences on the root's investigation; today's graph reaches the first two as hops. Headline and Remediation are byte-identical everywhere. No existing test pinned the exact investigation of such a chain through BuildStories + PopulateStoryText (the pipeline tests that pin investigation text — AnomalyIncidentReconcilerTests, RecurrenceLabelerTests, FindingStoreRecurrenceReadTests — build their stories by hand or walk chains with no identity-bearing hop), so no existing pin needed weakening; the new tests pin the new text. Byte-identity is pinned for everything else: a chain none of whose hops names anything, a one-node story, an identity-bearing ROOT (not its own hop), an absolution, a hand-built story with no NamedHops, and the PostgreSQL saturation-alone and idle-alone cards — each freezes exactly SerializeForStoryText(Compose(root)).

Severity, confidence, path, story hash, incident clustering, muting: untouched — the traversal is not changed, only what is recorded off it after the fact. No schema change; nothing new is persisted.

What it does NOT do

  • Does not touch co_fired or any other MCP payload field (DarlingMcpTools.cs, McpAnalysisTools.cs — lane 21's). If lane 21 wants a typed named_hops array on the payload, it needs an ephemeral NamedHops on AnalysisFinding copied in the two stores' story → finding maps (PgFindingStore.cs ~387, Lite/Analysis/FindingStore.cs ~191) — reported to the coordinator as a one-liner.
  • Does not put identity on the wait facts (PG_WAIT_* / SQL wait keys) — their ObjectName is the event's display name, not a who.
  • Does not change any collector, scorer, graph, PgTargetFactKeys, the root partials, the reconciler, RecurrenceLabeler, or deprecated/Dashboard (which consumes the shared composer and inherits the sentence).
  • No rung; no production read.

Test plan

New — Lite.Tests/StoryNamedHopsTests.cs (SQL Server side, 7 tests): the SQL Server roster by exact set and prefix; Describe for each rostered key off its seam (spid with/without sleeping, job name, table in database, query hash in database); null for an empty seam and for an off-roster key with an ObjectName (CONFIG_CHANGED, a wait); through a test graph, a six-node chain records the three highest-severity identity hops in severity order, drops the fourth, skips the hop that names nothing and never the root, with path/severity/hash inputs unchanged; an identity-bearing root and an absolution have none; PopulateStoryText appends exactly the two expected sentences in engine order with headline/remediation untouched, plus Sentence's two edge shapes (no headline; headline already terminated); a no-identity chain and a hand-built story are byte-identical to SerializeForStoryText(Compose(root)); and the real shape — LCK 1.0 → BLOCKING_CHAIN 0.7 (depth 3, sleeping apex ×1.4) through the real FactScorer and RelationshipGraph — names lead-blocker session 57, sleeping on the LCK card.

New — Darling/Darling.Tests/PgTargetNamedHopsTests.cs (4 tests): the PostgreSQL roster by exact set (every member IsPgKey; the PG_BAD_ACTOR alias, a wait key and the saturation key are not identity-bearing); Describe for all eleven families off the seam each collector partial fills (lanes 3/14, 15, 17, 7, 4, 13, 12); the exit shape — lane 14's fixture (30 of 90 parked, 90/97 → 1.25; billing-worker as billing in appdb, 15 min, pinning, six captures → 1.2) through the real scorer, PgTargetRelationshipGraph and composer: one story [saturation, idle], one NamedHop, the saturation composer's investigation contains no "billing" alone and exactly the expected sentence after — grep billing-worker and grep appdb hit the root card; and byte-identity for saturation alone and for an idle ROOT under the share bar (its own card names the holder as before, no marker).

Executed here, not only compiled: net10.0 harness (AssemblyName=Darling.Tests) outside the worktree over both new files plus PgTargetSessionsTests, PgTargetWaitTests, PgTargetBlockingTests, PgTargetSharedSwitchRoutingTests, PgTargetMcpSurfaceTests, PgTargetFactCollectorTests, RepoFileAdoptionTests, CommentFilterAdoptionTests, StoryConfidenceTests — 167 / 167 green (4 skipped = DARLING_TEST_PG-gated). DocCommentHygieneTests run from a throwaway project inside the worktree: 76 / 77, the one red being the harness directory itself appearing in the project census (artifact; removed). The composed sentences were printed and read for both engines. Builds, zero warnings of ours: PerformanceMonitor.Analysis (--no-incremental), PerformanceMonitor.Darling.Analysis, PerformanceMonitor.Darling.Service, Darling.Tests, Lite.Tests (the one xUnit2000 in LogTailOverlapThresholdPinTests.cs:95 is pre-existing on dev), deprecated/Dashboard.Tests. CI's Windows job is the first execution of Lite.Tests as a project and of the rest of the repo's censuses.

Which component(s) does this affect?

  • Lite
  • Darling
  • Lite Tests
  • Darling Tests
  • SQL collection scripts
  • Documentation
  • Full Dashboard (deprecated)
  • CLI Installer (deprecated)

How was this tested?

Above. No production read; no container needed (pure engine logic).

Checklist

  • I have read the contributing guide
  • My code builds with zero warnings (dotnet build -c Debug)
  • I have tested my changes against at least one SQL Server version (engine-level change over scored facts; executed through the real scorer and graph for both engines, no live server involved)
  • I have not introduced any hardcoded credentials or server names

… says who was behind the chain (both engines, partial #3691)

InferenceEngine.BuildStories records the identity-bearing hops a traversal consumed on the story (AnalysisStory.NamedHops, highest severity first, capped at three, root excluded) and FactAdvice.PopulateStoryText composes one 'Behind it:' sentence per hop onto the root's frozen investigation. FactIdentity is the contract: an explicit roster per engine over the string seams the collectors already fill (ObjectName, DatabaseName, the per-statement key suffix, BLOCKING_CHAIN's apex spid). A chain none of whose hops names anything is byte-identical. MCP payload files untouched; the sentence rides in StoryText, which both stores already round-trip.
@erikdarlingdata
erikdarlingdata merged commit 0197990 into dev Sep 19, 2026
7 of 8 checks passed
@erikdarlingdata
erikdarlingdata deleted the fix/3691-root-names-leaves branch September 19, 2026 17:46
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.

1 participant