Repository navigation
A configuration lever hanging off a diagnostic chain is attached to that finding instead of orphaned into a card of its own, so a vacuum backlog and the memory bound that caps how fast it can clear arrive as one story (#3691) - #3850
Merged
Merged
Conversation
…haned into a card of its own, so a backlog and the memory bound that caps how fast it can clear are one finding (#3691) The greedy single-path walk follows the highest-severity active edge from each node, so a node mid-path with two active edges reaches only one of them. The vacuum backlog reaches the wraparound trend — and the hold chain behind it — and skips CONFIG_PG_MAINT_WORK_MEM, which is the dead-tuple memory bounding how much of the backlog one autovacuum pass can clear: the lever that would relieve the very thing the story is about. Never visited meant never consumed, and the un-consumed knob then rooted its own one-node card at 0.6 beside the incident it belongs to. The reader saw two cards where the truth is one story with a lever, and nothing on either card said they were the same problem. BuildStories now sweeps SIDE LEAVES after the traversal and before the story is built: for every node on the path, each ACTIVE edge — read through the graph's own GetActiveEdges, so every predicate and the bad-actor alias behave exactly as they do in the walk — whose destination is not on the path, not consumed by a prior story, scored above zero and CONFIG-ADVISORY class is attached to the story as a side leaf and marked consumed. The advisory-class test is by the fact's SOURCE (pg_config, pg_posture, config, database_config) unioned with the advisory-root set, and that is load-bearing rather than incidental: CONFIG_PG_MAINT_WORK_MEM is deliberately NOT an advisory root (D5 — an evidence-gated knob may not root on a quiet server), it takes its 0.4 base from the stamped backlog ratio and the co-fire amplifier lifts it past the ORDINARY 0.5 line, so a root-set membership test would have missed the reported defect entirely. An incident-class side destination is untouched: it is a different story with its own symptom, advice and occurrence history, and ClusterIntoIncidents is what re-merges it into one incident without taking its card away. The leaves ride BESIDE the path, never in it. Path, StoryPath, StoryPathHash, LeafFactKey and FactCount are what they were, so incident identity — the hash every mute row, occurrence count and fingerprint keys on — is byte-identical and no existing story pin moves. A side leaf does not lift severity or confidence either: it is context, not corroboration, and amplifying is the graph's business. The payload gains side_leaves, one card per lever with its advice, through an Attach that returns the caller's object untouched when there is none — the CollectionCaveats discipline, because the tools' serializer writes nulls and a side_leaves: null would be a byte change on every finding of every clean pass. The root's investigation gains ONE sentence naming the lever and pointing at its card; a story with no lever freezes exactly what Compose returned. The SQL Server pass cannot change, structurally rather than by sampling: DB_CONFIG is the only config-advisory destination any SQL Server edge names, its only inbound edge is from LCK_M_S, and LCK_M_S declares exactly that one edge — so DB_CONFIG is always the single candidate and always goes on the path. A pin holds that, and fails the day someone adds a second edge out of LCK_M_S. Tests: Lite.Tests/StorySideLeafTests.cs, thirteen pins — a lever off the middle node attaches and roots nothing; the same lever off the tail is still walked onto the path; an incident-class side destination keeps its own root; a lever at severity 0 is ignored; two levers rank by severity; a lever consumed by an earlier story is not re-swept; the real PG vacuum chain with the real keys carries maintenance_work_mem as a side leaf (with the not-an-advisory-root premise asserted so the pin cannot become a tautology); the SQL Server impossibility; and the payload/prose emitting only when owed. All thirteen were EXECUTED on this machine through an in-worktree net10.0 xunit harness alongside StoryNamedHopsTests, StoryConfidenceTests, TsqlConventionGuardTests, DocCommentHygieneTests and RepoFileAdoptionTests — 137 tests, 0 failures. That execution caught a defect compile-only would have shipped: the SQL Server pin's DB_CONFIG fact lacked the rcsi_off_count metadata the edge predicate reads, so the edge never fired and the assertion was passing for the wrong reason.
… caveat envelopes, so both SKUs must carry it exactly once and the two attachments cannot be confused
erikdarlingdata
deleted the
fix/3691-walk-attaches-side-config-leaves
branch
September 21, 2026 21:08
This was referenced Sep 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The greedy single-path story walk now attaches the config-advisory leaves hanging off its path to the story it belongs to, instead of leaving them to root cards of their own — so a vacuum backlog and the memory bound that caps how fast it can clear arrive as one finding with a lever, not two findings that never mention each other.
Partial for #3691. The checklist line this completes, verbatim:
What was wrong
InferenceEngine.Traversefollows the SINGLE highest-severity active edge from each node until it runs out, and every node it lands on is markedconsumed. That is the right shape for a story — a chain is a chain — but it means a node MID-path with two active edges reaches only one of them.The vacuum chain is exactly that shape.
PG_AUTOVACUUM_BACKLOGhas an active edge toPG_WRAPAROUND_TREND(higher severity, and the hold chain behind it) and another toCONFIG_PG_MAINT_WORK_MEM— the dead-tuple memory each autovacuum pass may use, which bounds how much of the backlog one run can clear. The walk takes the wraparound. The knob is never visited, so nothing consumes it; and because it reaches 0.6, it then roots its OWN one-node story beside the incident it belongs to.So the operator got two cards. One said "public.hot is 5× past its own autovacuum trigger line for four consecutive samples, and a held xmin horizon is why it cannot clear". The other said "maintenance_work_mem is small". Nothing on either card said they were the same problem, and the second one is the lever for the first — the one number you can change tonight.
The shape of the fix, and the one judgement call in it
After
Traversereturns and its path is consumed,SweepSideLeaveswalks the path once more and, for each node, reads that node's ACTIVE edges through the graph's ownGetActiveEdges— the same call the traversal makes, so every predicate, and the bad-actor alias resolution the PostgreSQL graph performs, behave here exactly as they do in the walk. A destination is attached to the story as a SIDE LEAF, and marked consumed, when it is not on the path, not consumed by a prior story, scored above zero, and CONFIG-ADVISORY class.The class test is by the fact's SOURCE, and that is load-bearing rather than tidy. The obvious test is membership of the advisory-root set (
ConfigAdvisoryRootKeys/PgTargetFactKeys.IsConfigAdvisoryRoot) — and it would have missed the reported defect completely. That set answers a different question: "may this fact root BELOW the 0.5 incident line?" Its membership follows D5, so a CONVENTION check (a knob at its shipped default) is in it and an EVIDENCE-gated check is deliberately not —maintenance_work_memmay not root a card on a quiet server. But an evidence-gated knob that DID get its evidence is precisely the orphan here:CONFIG_PG_MAINT_WORK_MEMtakes its 0.4 advisory base from the backlog ratio the vacuum collector stamps onto it, the backlog co-fire amplifier lifts it to 0.6, and it roots on the ORDINARY threshold without ever consulting the advisory-root set. So the sweep asks whether the fact came from a SETTINGS source —pg_config,pg_posture, or the SQL Serverconfig/database_configcollectors — unioned with the advisory-root set, which picks up the keys whose card is an advisory but whose fact rides a measurement source (MISSING_INDEXandPLAN_WARNINGonqueries,PLAN_CACHE_BLOATonmemory,CONFIG_PG_AUTOVACUUM_DISABLEDonpg_vacuum). A source is the collector's own statement about what kind of thing it read, so it cannot drift the way a hand-kept key list does. I checked every emission site on those four sources: each one emits a setting, never a measured symptom.Only config-advisory destinations, and this must not be widened. An incident-class side destination is a DIFFERENT story. It has its own symptom, its own advice, its own occurrence history, and
ClusterIntoIncidentsalready re-merges it into one incident without taking its card away. A config advisory has no incident of its own — it is a standing setting whose entire claim to a card was that nobody else had said it. Widening the sweep to any destination would silently delete findings, which is why the class test carries the longest comment in the method.Rejected alternatives. Appending the leaf to the path would have been three lines: it also changes
StoryPath, and thereforeStoryPathHash, and therefore every mute row, occurrence count and incident fingerprint for the chain — a re-identification of a live incident to make a card prettier. Letting the leaf keep its own card and linking the two cards leaves the "two cards" complaint exactly where it was. Following edges OUT of a side leaf would invent a second path, which is the thing this method exists to avoid.Blast radius
AnalysisStory.SideLeafKeysandAnalysisFinding.SideLeafKeysare new, ephemeral, and not persisted — no schema change, no migration rung.Path,StoryPath,StoryPathHash,LeafFactKeyandFactCountare untouched, so incident identity is byte-identical and every existing story pin holds unchanged (StoryNamedHopsTestsandStoryConfidenceTestswere executed against this build to prove it, not assumed). Severity and confidence are untouched: a lever is context, not corroboration, and amplifying is the graph's business.The payload gains
side_leavesonanalyze_serveronly — one card per lever, carrying that key's advice so a reader told a lever exists does not need a second tool call to learn what it says. It is attached throughStorySideLeaves.Attach, which returns the caller's payload object untouched when there is no lever. That is theCollectionCaveats.Attachdiscipline and it is deliberate: these tools' serializer writes nulls (the payload's ownleaf_factrenders asnullon a one-node story), so aside_leaves: nullproperty would have been a byte change on every finding of every clean pass, for a thing that did not happen.get_analysis_findingsis unchanged — it reads findings back from the store, which carries no side leaves, and inventing them on the read path would be a claim the store cannot support.SQL Server output: byte-identical, structurally rather than by sampling.
DB_CONFIGis the only config-advisory destination any SQL Server edge names; its only inbound edge is fromLCK_M_S; andLCK_M_Sdeclares exactly that one edge. So whenever the walk stands onLCK_M_SwithDB_CONFIGfired,DB_CONFIGis the single candidate and goes ON the path, as it always did. No SQL Server story can carry a side leaf, so no SQL Server payload gains the key and no story text gains the sentence. This is pinned rather than asserted in prose, because it is the kind of claim that stops being true silently — the day someone adds a second edge out ofLCK_M_S, or any edge into aCONFIG_*key, the pin fails and that becomes a review conversation instead of a surprise in a payload diff. (No planted SQL Server store had an orphaned config leaf today, which is the same fact seen from the other side.)What it does not do
No graph edge changes. No severity, amplifier or confidence changes. No Viewer work — the viewer renders findings read back from the store, which carry no side leaves. No migration rung. No new knobs.
get_analysis_findingsunchanged. It does not follow edges out of a side leaf, and it does not attach incident-class destinations.Test plan
New file
Lite.Tests/StorySideLeafTests.cs, thirteen pins, all new: a config leaf off the MIDDLE node attaches as a side leaf, is consumed, and roots no second story (path, hash inputs, leaf and fact count all asserted unchanged); the same leaf off the TAIL is still walked onto the path as today; an incident-class side destination is not swept and still roots its own story; a side leaf at severity 0 is ignored; two levers off two nodes rank by severity with an ordinal tie-break; a lever consumed by an earlier, higher-severity story is not re-swept onto a later one; the real PostgreSQL vacuum chain with the real graph and real keys carriesCONFIG_PG_MAINT_WORK_MEMas a side leaf — withAssert.False(IsConfigAdvisoryRoot(...))asserting the premise, so a future edit to that list cannot quietly turn the pin into a tautology; the SQL Server impossibility above; andToPayload/Attach/Sentence/PopulateStoryTextemitting only when owed, including the byte-identity arm.These were EXECUTED on this machine, per the campaign's execute-your-pins rule, through an in-worktree net10.0 xunit harness (deleted before commit) alongside the existing
StoryNamedHopsTests,StoryConfidenceTests,TsqlConventionGuardTests,DocCommentHygieneTestsandRepoFileAdoptionTests: 137 tests, 0 failures. Running them caught a defect compile-only would have shipped — the SQL Server pin'sDB_CONFIGfact lacked thercsi_off_countmetadata the edge predicate actually reads, so the edge never fired and the assertion was passing for the wrong reason. Fixed and re-executed.CI is still the first execution of these tests in the real suite, and the first execution of anything on the Windows side.
Builds, all zero warnings:
PerformanceMonitor.Analysis,Lite(-p:EnableWindowsTargeting=true),PerformanceMonitor.Darling.Service,Lite.Tests,Darling/Darling.Tests,deprecated/Dashboard.Tests.Which component(s) does this affect?
How was this tested?
Thirteen new unit pins over the real
InferenceEngine, the realRelationshipGraphand the realPgTargetRelationshipGraph, executed locally (137 tests including the neighbouring story suites and three repo censuses, 0 failures) and first executed in the real suite by CI. Pure engine logic: no server contact, no store contact, no fixture. The PostgreSQL scenario is pinned with the production keys and the production vacuum graph; the SQL Server no-change claim is pinned against the graph's own edge table rather than sampled.Checklist
dotnet build -c Debug)