Single-source the Query Store liveness touch guard and derive its width from the margin it spends (#3189)

[erikdarlingdata]

What changes

The Query Store liveness touch guard becomes one named constant and widens from one hour to six.

Three statements guard a last_seen touch, and the third is in a different file from the other two:

site	statement
QueryStorePlanMap.TouchAndProbeSql	the touched CTE
QueryStorePlanMap.TouchAndProbeSql	dim_touch
QueryStoreTextStore.TouchAndProbeSql	the touched CTE

All three now interpolate QueryStoreLivenessTouchGuard.GuardInterval, which is the only place the width is written. That is the primary change, not the width. QueryStorePlanMap.TouchAndProbeSql's own summary says the map row and the dimension row cannot age out at different times and calls that structurally impossible rather than carefully avoided; with a literal per site it was neither — it was copies that happened to agree, and widening two of them and missing the third is exactly the divergence that claim rules out.

The width is derived from the MARGIN, not from the retention horizon

GuardHours = StampSkewMarginHours / MarginShareDivisor = 24 / 4 = 6, rendering interval '6 hours'.

StampSkewMarginDays is the smaller of the two prune margins the touched tables reserve — QueryStorePlanMap.PruneMarginDays (1) against QueryStoreTextStore.PruneMarginDays (2) — so one shared width is safe on both sides, conservatively so on the text side. It is a ternary over those two constants, not a restatement, so tightening either margin narrows the guard rather than silently eating its headroom.

The retention horizon is the wrong anchor, and expressing the guard as a fraction of it would be a relationship that does not hold. DarlingRetention.ComputeMapCutoff prunes at now - (widestFactRetention + PruneMarginDays) and ComputeDimensionCutoff takes the dimension at now - (widestFactRetention + ChunkIntervalDays + 1). Both of those trailing margins are documented, in DarlingRetention's own words, as covering the last_seen refresh guard. The retention term is the operator's: it varies per collector, moves with plan_content_retention_days, and lends the guard nothing it can rely on. The margin is the part reserved for exactly this. A guard sized as a fraction of the horizon would widen whenever an operator widened retention, while the budget it actually spends from would not move at all — that is #3199's defect shape wearing a derivation's clothes.

So: the magnitude is derived and moves with the margin; MarginShareDivisor = 4 is CHOSEN and its doc says so in those words. Nothing measured picks it. Every value from 1 to 24 keeps the guard inside the margin, and the whole range sits orders of magnitude below the horizons seen in the field. What the divisor trades is write volume against stamp freshness; a quarter leaves 4x headroom inside the margin for the collection cadence, for clock skew between the service and the store, and for a sweep that runs late. There is no measurement that prefers 4 to 3 or to 6, and the constant is documented so nobody reads 6 as a measured quantity.

QueryStoreLivenessTouchGuard.GuardStaysInsideStampSkewMargin() pins the invariant in the shape QueryStorePlanMap.MarginOrderingHolds already uses: at least an hour, and the divisor must divide the margin exactly, so the share stated is the share taken.

The anchor is a conservative proxy for the room, and the precondition is named

PruneMarginDays is the map's margin only while ComputeMapCutoff's coupled arm wins. With the plan-content knob enabled — and its default is 21 — the dedicated arm governs and the map's cutoff is the knob itself, with no margin term in it at all. So this constant is not "the room available"; it is the smallest stamp-trailing allowance the retention arithmetic names anywhere, and it is strictly smaller than the least room any reachable configuration leaves.

Enumerated over the shipped arithmetic rather than argued: 48 combinations of dim-feeding retention (floored at 1 day by Math.Max(1, …)) against the knob (off, or clamped to 7–365). Least room 2 days, at retention 1 with the knob off. 21 days in the default configuration. Six hours is 12.5% of the worst case and 1.2% of the default.

Anchoring on the named 1-day allowance rather than on a floor computed from three interacting knobs is a deliberate trade, stated as one: it keeps the derivation readable and it keeps the guard inside the true room by a further factor of two at the worst point.

The min's membership is derived, not stated

StampSkewMarginDays is a min over an enumeration, and an enumeration with no inclusion criterion is the next defect. The criterion: a table contributes a term exactly when this guard's touch writes its last_seen. That is three tables, and it is emphatically not "every table with a PruneMarginDays".

The tree holds three PruneMarginDays constants and only two qualify. PgStatementText.PruneMarginDays is 2, so folding it into the min would change no value at all — min(1, 2, 2) is still 1 — while being a genuine category error: PgStatementText has no guard site, its UpsertSql conflict arm advances last_seen on every conflict under a MONOTONICITY guard (EXCLUDED.last_seen >= …) rather than a staleness one, and its margin exists so text outlives the statistics rows referencing it. A fact-outliving margin, not a guard-trailing one, wearing the same constant name. A term that is wrong to include and cannot move the answer is the worst kind to leave unenforced, so it is enforced rather than documented: TheGuardedStoreTypesAreDerivedFromTheGuardSites sweeps the whole storage project for guard sites and requires the set of PruneMarginDays terms in the initializer's own source to equal the set of store types that scan finds. PgStatementText.cs is asserted to be inside the swept population and outside the guarded set, so the exclusion is measured rather than claimed about a file the scan might be blind to.

The third guarded table needs no term of its own, and that follows from an invariant already in the repo. dim_touch writes query_plan_dim.last_seen, and query_plan_dim has no PruneMarginDays — its room above the fact horizon is ChunkIntervalDays + 1 in ComputeDimensionCutoff, whose trailing day is documented there as covering this very guard. QueryStorePlanMap.MarginOrderingHolds already pins PruneMarginDays < ChunkIntervalDays + 1, so guard ≤ map margin < dim margin follows without a second copy of the arithmetic; the test asserts that ordering so an inversion reds instead of silently putting the dimension inside the guard's reach.

Why it is safe

The guard is a per-row predicate, not a statement-level skip. A row that keeps being referenced has its stamp refreshed at least once per window, so the width is precisely the bound on how stale a live row's last_seen can be. Orphaning content requires a stamp as stale as the GC horizon, which requires that many hours of no references — the intended behaviour, and not something a guard width can manufacture.

Measured on one production store: the dimension holds 15.58 days while the fact tables that reference these digests retain 4, roughly 11.6 days of slack on top of the margin. Only 21.2% of dimension rows had been touched in the last day and 7.0% in the last two hours, so about four fifths of them survive on the horizon rather than on liveness and cannot be orphaned by touching less often.

The hard bound is tighter than the field measurement, and it is worth stating. The tightest cutoff any configuration can reach is the map prune's widestFactRetentionDays + PruneMarginDays. One day is genuinely reachable for the first term and the enforcement points say so: StoreConfigProvider.ValidRetention honours a fleet override at >= 1, and DarlingRetention then floors the fold at Math.Max(1, …) from a seed of 1. With PruneMarginDays = 1 and ComputeMapCutoff returning the NEWER of its coupled and dedicated arms, a 1-day dim-feeding retention puts the map's prune cutoff at now - 2 days even with the plan-content knob at its MinPlanContentRetentionDays floor of 7. So the guard has to clear 2 days in the tightest reachable configuration, not 15.58. Six hours takes an eighth of that. 15.58 days is one store's observed hold; quoting it as the bound overstates the headroom by about 8x, in the direction that grants permission.

Scoping: this reduces WRITES, not examinations

The guard gates the two UPDATEs. The verdict SELECT at the bottom of each statement joins unnest($1..$4) against the store and returns resolved / hash_stale for every reference in the batch regardless of the guard. Nothing about what gets examined changes.

The saving is on non-HOT update volume. last_seen is indexed on all three touched tables and is the column the touch writes, so a heap-only update is impossible: every touch is a new heap tuple, a new b-tree entry, a dead one left behind, WAL for both, and autovacuum work afterwards. Measured on one production store at the one-hour width, 22.1 M non-HOT last_seen updates a day across the three tables. The per-row touch ceiling is inversely proportional to the width, so 1h → 6h drops that ceiling by 1 - 1/6 = 83.3%, from 22.1 M/day to about 3.7 M/day.

That 83.3% is an upper bound on the saving, not a point estimate: it is the reduction for rows referenced often enough to hit the guard every window. A row referenced twice a day takes two touches at either width and contributes no saving at all. Given that only 21.2% of dimension rows were touched in the last day, the realised saving is somewhere below the ceiling and this change does not claim to know where.

The churn the round-trip figure does not include

A measurement posted to #3189 after this branch was cut names a second cost on the same axis, and it is the axis this change acts on. collect.query_store_text was carrying 1,070,974 dead tuples against 5,272,953 live — 16.9% — with n_tup_del at ZERO. Nothing has ever deleted a row from that table, so every one of those dead tuples is an UPDATE, and the touch is the only thing that updates it on a cycle. None of that appears in text_fetch_probe_ms: the round trip is billed, the tuple version it leaves behind and the autovacuum pass that reclaims it are not.

One attribution in that comment does not follow, and it matters because it would price a saving this change will not deliver. The same comment reports the text table's TOAST relation at 3,443,398 live against 35,028,552 deleted tuples and reads it as the touch's churn too. It cannot be. PostgreSQL does not rewrite an unchanged out-of-line value on UPDATE — the new heap tuple carries the same TOAST pointer, and toast_delete_external runs only for values present in the old tuple and absent from the new one. A last_seen-only update therefore generates no TOAST churn. The candidate that does is QueryStoreTextStore.UpsertSql's conflict arm, which writes query_sql_text = EXCLUDED.query_sql_text unconditionally on every text refetch — deliberately, for the Query Store reset-renumbering reason that method's own doc gives. So expect the heap churn to fall with the guard and the TOAST churn not to. Whoever measures the effect of this change should split those two before reading a result.

Two ~83% figures are in play and they measure different things

• 1 − 1/6 = 83.3% is the write reduction from the guard ratio. Arithmetic, not measurement.
• 1 − 1/6.02 = 83.4% is a peer's measurement of the share of probe work that re-examines a reference already examined in the same six hours.

They coincide only because the width under discussion happens to equal that measurement's window; change either and they separate. The second one is not this change's payoff — it prices a decoupling of the touch from the probe that nobody has scoped, and it is itself an upper bound rather than a point estimate: its denominator counts rows whose last_seen lands in the window, and a row stamped just before the window opens and re-referenced within the hour is examined inside it but suppressed by the guard, so it never enters the denominator while its examinations count in the numerator.

#3189's headline and this remedy address different quantities

#3189 is titled on examinations — "31M references a day to find 1.4% missing" — and this change addresses writes. Both are worth doing and they are not the same change. Saying so here because the issue as filed reads as though widening the guard were the fix for its headline, and it is not.

Also not done, and deliberately: memoizing "already present" to skip the probe. That was recommended on #3189 and withdrawn there. The touch and the probe are the same round trip, so not sending a reference means not touching it, and QueryStorePlanMap's summary states this table's last_seen is the only protection Query Store digests have from the dimension GC. Widening the guard does not unlock verdict caching.

The fourth INTERVAL '1 hour', and why it stays

PayloadDimensions.UpsertSql carries the same literal twice — the dimension upsert's ON CONFLICT arm — and it is not migrated onto this constant. It is a different guard on a different statement: it fires when a fetch lands content, which happens on a minority of runs, so its write volume is a rounding error next to a per-cycle touch. (#3189 gives two figures for that minority and they disagree — "~78% of runs fetch nothing" against "2,894 plan fetches across 10,955 runs", which is 26.4% fetching. Nothing here turns on which; both are a minority.) It spends from the same one-day margin and sits strictly inside the new width, so it is conservative rather than inconsistent. Coupling the two would tie two independent decisions to one number for no measured gain.

The single-source mechanism, and its partial-migration proof

QueryStoreTouchGuardSingleSourceTests pins the property, not the value. A value pin ("the width is six hours") passes a partial migration the moment its own expected value is updated in the same commit, so the assertion that has to hold is the single-sourcing.

• NoGuardSiteSpellsItsOwnWidth is the control. It reads both files' string literals through CSharpSourceWalker and requires that none of them spells an interval with a number in it. It is the only instrument that can see a site typed back to a literal, and it is deliberately the source scan rather than an assertion over the shipped strings — once all three sites interpolate one constant they cannot disagree, so a composed-SQL agreement check is true of the implementation rather than evidence about it.
• EveryGuardSiteInTheShippedSqlCarriesTheOneWidth corroborates: exactly three guard sites across the two statements, two in one and one in the other, all carrying the same width. It catches a site appearing or disappearing, which the source scan does not price.
• TheWidthIsAShareOfTheStampSkewMarginAndFollowsIt holds the derivation, and its load-bearing half is a source assertion that GuardHours' initializer is the division. The arithmetic assertion alone passes a hard-coded 6 for as long as the margin happens to be one day — see mutation I3 below, which is exactly that failure.

The three files that were coupled to the old width and would otherwise have gone quietly wrong:

• QueryStorePlanFetchTests' two guard assertions now compare against the constant rather than counting a literal, which would have been a fourth copy of the width.
• QueryStoreFetchProbeLivePostgresTests lands its fixture rows GuardHours + 1 hours old, derived rather than typed. At the hard-coded -3 hours the rows would have landed inside the new guard, no UPDATE would have fired, and the freshness assertions this test exists for would have read 0/0.
• DarlingRetention's four prose sites that described the guard as hourly. The fourth was found by the review bot, not by me: my sweep was line-oriented and that phrase spans a line break ("…margin for the hourly\n last_seen refresh…"), so grep 'hourly last_seen' could not match it. A multi-line re-sweep confirms there is now exactly one remaining hourly-plus-last_seen phrase in the tree and it is the fixed one. One of the four was already wrong before this change: ComputeMapCutoff justified its one-day margin by an asymmetry — "TouchAndProbeSql refreshes the map's stamp eagerly while the dim's refresh is hourly-guarded" — that the shipped SQL does not have. Both stamps are guarded, at the same width, now provably so. The margin was and is safe; the reason given for it did not exist.

Mutation table

Harness: the shipped .cs files compiled into a net10.0 xunit.v3 console project named Darling.Tests (so InternalsVisibleTo applies), .csproj held outside the repo tree with build output redirected into it. Every mutation was applied by byte-level replace with a count == 1 anchor, confirmed present by git diff --numstat and by grepping the mutated content, rebuilt, run, restored with git checkout --, and confirmed restored by content.

Baselines, printed, unmutated. M1–M9 and I1–I4 ran at b7c622ac9: Total: 117, Errors: 0, Failed: 0, Skipped: 0, Not Run: 0. M10–M12 and I5 ran at f0da3c9b8, after the membership test landed: Total: 118, Errors: 0, Failed: 0, Skipped: 0, Not Run: 0. The merge commit re-runs the same 118 green.

One process note worth recording rather than hiding: the first attempt at the M10–M12 pass ran with the membership work still UNCOMMITTED, and the loop's git checkout -- Darling/ restore deleted it along with the mutation — a mutation harness cannot tell your work from its own. M10's red was real and reproduced, but M11, M12 and I5 from that pass were run against a tree that no longer held the test and are discarded; the table reports the re-run after f0da3c9b8 was committed.

#	Mutation	Result	What it establishes
M1	one plan-map site → interval '1 hour'	3 failed	partial migration inside the two-site file
M2	the text-store site → interval '1 hour'	3 failed	partial migration in the file an editor misses
M3	both plan-map sites hard-coded to the current correct width	1 failed — only NoGuardSiteSpellsItsOwnWidth	the value pins are blind to a correct-looking literal; the source scan is the instrument
M9	the text-store site hard-coded to the current correct width	1 failed — only NoGuardSiteSpellsItsOwnWidth	same, in the other file
M4	dim_touch's guard deleted outright	2 failed	an unguarded touch is caught
M5	GuardHours hard-coded to 6	1 failed	the derivation is required in source, not just in arithmetic
M6	GuardHours hard-coded and PruneMarginDays 1 → 2	1 failed	a width that stops following its margin
M7	MarginShareDivisor 4 → 5 (24/5 truncates)	1 failed	the share stated must be the share taken
M8	guard rewritten to twice the margin	1 failed	the guard cannot escape its budget
M10	PgStatementText.PruneMarginDays folded into the min — value unchanged at 1	1 failed	the category error is caught although no value moves
M11	QueryStoreTextStore's term dropped, leaving min(1) — value unchanged at 1	1 failed	the other direction of the same value-invisible error
M12	PgStatementText.cs declared as a guarded file, which has no guard site	1 failed	the declared set cannot drift from the scan

Instrument mutations

#	Mutation	Result	What it establishes
I1 + M9	the text file removed from the scanned set, then the text site hard-coded	0 failed — ALL GREEN	the two-file list is the whole of the control. Crippled, it reports a clean bill of health on a partial source migration in exactly the file this change exists to protect
I3 + M5	the derivation source assertion weakened to match only GuardHours, then GuardHours hard-coded	0 failed — ALL GREEN	the arithmetic assertion alone is a pin that cannot fail while the margin is one day; the source assertion is what makes it a control
I2 + M4	the exact guard-site total turned into a floor, then a guard site deleted	2 failed	the total count is redundant with the per-file counts for a deletion — stated rather than claimed as coverage
I4 + M4	the total and both per-file counts turned into floors, then a guard site deleted	1 failed — only the neighbouring QueryStorePlanFetchTests pin	the counts are what make this test independent of that one
I5 + M10	the initializer-terms set equality weakened to NotEmpty, then PgStatementText's term folded in	0 failed — ALL GREEN	that set equality is the whole control on membership; every value assertion in the class passes the category error unchanged

Verification

All 8 checks green on 8abfb5e6f, latest run of each: check-branches 4 s, description-drift 5 s, Darling whole-tree guards 17 s, Darling Linux build 114 s, review 146 s, verify 160 s, Darling PostgreSQL tests 309 s, build 466 s. Darling.Tests Total: 8295, Errors: 0, Failed: 0, Skipped: 335, Not Run: 0 with zero [FAIL] lines and none of this branch's test names in the SKIP list.

• Darling.Tests targets net10.0-windows and cannot run on macOS. Everything above is the real shipped .cs compiled into a net10.0 xunit.v3 harness named Darling.Tests; CI is the arbiter for the rest of the suite.
• The suite in that harness is QueryStoreTouchGuardSingleSourceTests, QueryStorePlanFetchTests, RepoFileAdoptionTests, CommentFilterAdoptionTests and DocCommentHygieneTests — the last three because a new source-scanning test file and a new production file are exactly what those censuses judge. All 117 pass. RepoFileAdoptionTests' LF-reader set equality is why this pin reads raw rather than LF-normalised: every anchor in it is single-line, and the one place a newline could reach an anchor collapses whitespace itself.
• The composed SQL was dumped from the built assembly, not read from source, and diffed against the base commit's verbatim literal with only interval '1 hour' → interval '6 hours' substituted. Both statements are byte-identical (1,698 and 1,451 bytes, LF-normalised), so the concatenation changed the width and nothing else — no whitespace, no line, no structure.
• QueryStoreFetchProbeLivePostgresTests exercises the guard against a real store and runs in CI's Darling PostgreSQL tests; its fixture age now derives from the constant, so that job is the live check on this change. It ran 223 s on the first pushed SHA and passed — a duration in the range that means the live-PostgreSQL pins executed rather than skipped.
• build failed once, on b7c622ac9, and it was not this change. One test:
  DarlingDeployRollbackRetentionTests.TheDeployScriptAndTheService_AgreeOnWhatARollbackBackupIsCalled, with "powershell.exe did not exit within 60 seconds running the extracted functions". That test reads Darling/tools/upgrade-darling.ps1 and the Service .csproj — neither is in this diff — and spawns Windows PowerShell under a self-imposed 60-second budget on a runner that had just finished 3,535 Lite tests and 8,294 Darling ones. Darling.Tests reported Total: 8294, Failed: 1, so this branch's own tests ran and are absent from both the failure list and the SKIP list. Recorded rather than dismissed: the re-runs since carry a superset of the change, and if it recurs it is real.

CHANGELOG entry (not committed — a batch pass owns that file)

- **The Query Store liveness touch guard is one constant, six hours wide** ([#3189]) - the `last_seen` touch that keeps Query Store plan XML alive against the dimension GC was guarded by `interval '1 hour'` written THREE times, twice in `QueryStorePlanMap.TouchAndProbeSql` (the `touched` CTE and `dim_touch`) and once in `QueryStoreTextStore.TouchAndProbeSql` - so the map row and the dimension row were protected by copies that happened to agree, in a statement whose own summary calls their divergence structurally impossible. `QueryStoreLivenessTouchGuard` is now the only place the width is written, and it is DERIVED: the smaller of the two tables' prune margins (`QueryStorePlanMap.PruneMarginDays` = 1 day against `QueryStoreTextStore.PruneMarginDays` = 2) divided by a stated `MarginShareDivisor` of 4. Derived from the MARGIN and not from the retention horizon, deliberately - the margins are what `DarlingRetention`'s cutoffs reserve for a trailing stamp, while the retention term is the operator's and moves with `plan_content_retention_days`, so a guard sized as a fraction of the horizon would widen when retention widened while its actual budget did not move. The divisor is CHOSEN, not measured, and its doc says so: safety picks nothing here, since anything from 1 to 24 hours stays inside the margin and the hard bound is 2 days in the tightest reachable configuration. What it buys is WRITES, not examinations - the verdict SELECT returns a verdict for every reference in the batch regardless of the guard - and `last_seen` is indexed on all three tables, so every touch is a full non-HOT update; measured at the one-hour width, 22.1 M of them a day across the three tables, whose per-row ceiling the six-hour width divides by six. `ComputeMapCutoff`'s margin rationale is corrected with it: it justified the one-day margin by `TouchAndProbeSql` refreshing the map's stamp eagerly while the dim's was hourly-guarded, and the shipped SQL guards both, now at one width by construction. The `min`'s MEMBERSHIP is derived rather than stated, because an enumeration with no inclusion criterion is the next defect: a table contributes a term exactly when this touch writes its `last_seen`, and `PgStatementText.PruneMarginDays` is deliberately excluded - it is 2, so folding it in could not change the value, while `PgStatementText` has no guard site at all (its upsert advances `last_seen` on every conflict under a monotonicity guard) and its margin is a fact-outliving one wearing the same constant name. `query_plan_dim`, the third table the touch writes, needs no term because `MarginOrderingHolds` already pins the map's margin strictly inside the dimension's. The pin is the single-sourcing and the membership rather than the value - a source scan over both statement files that reds on any hard-coded interval, plus a project-wide guard-site sweep whose derived store types must equal the terms in the constant's own initializer. Both are mutation-proved on changes that move NO value: a correct-looking literal in the text store reds only the literal scan, folding in `PgStatementText`'s margin reds only the membership equality, and crippling either control makes its own mutation pass silently.

[claude]

Reviewed this against CONTRIBUTING.md conventions (T-SQL style N/A here — this is Darling's PostgreSQL store), Lite/Darling parity, correctness, security, and performance.

Correctness — checked and holds:

• GuardHours = StampSkewMarginHours / MarginShareDivisor = 24/4 = 6, and StampSkewMarginDays correctly resolves to min(QueryStorePlanMap.PruneMarginDays=1, QueryStoreTextStore.PruneMarginDays=2) = 1 day. GuardStaysInsideStampSkewMargin() (6 ≥ 1 and 6·4 == 24) holds.
• The const string TouchAndProbeSql → static readonly string TouchAndProbeSql change in both QueryStorePlanMap.cs/QueryStoreTextStore.cs is required, not incidental — concatenating a non-constant field (GuardInterval, which calls .ToString()) can no longer be a compile-time constant.
• No circular static-initializer dependency: QueryStoreLivenessTouchGuard's consts depend only on PruneMarginDays consts from the other two classes, never on their static readonly SQL fields, so the cross-type field access at class-init time is a DAG, not a cycle.
• Grepped for stray interval '1 hour' across Darling/ — the only survivor is in QueryStoreTrendRouting.cs, which is an unrelated 1-hour lookback window, not a guard site. No fourth guard site was missed.
• The comment updates in DarlingRetention.cs correctly distinguish the separate, untouched 1-hour dim-upsert conflict-arm guard (PayloadDimensions.cs) from this widened 6-hour Query Store guard, and correctly note the existing 1-day prune margins comfortably absorb either (6h < 24h) — no numeric changes were needed there, only documentation, which checks out.
• Walked the live-Postgres test's guard-crossing math by hand (landedAt = now - 7h, first touch fires since 7h > 6h guard; second call at now+2h does not re-fire the guard since now < now+2h-6h is false — but the test doesn't assert freshness there, only the verdict SELECT, which runs unconditionally by design). Consistent.

Lite/Darling parity: No drift. This digest-based Query Store plan/text map is a Darling-only central-store mechanism (multi-server dedup against one PostgreSQL store); grepping Lite turned up no equivalent TouchAndProbeSql/liveness-guard concept to keep in sync — Lite queries Query Store DMVs directly per-server and has no analogous dedup store.

Security: No new input handling, no secrets, no file/network/process surface changes.

Performance: This is a net improvement — widening the guard from 1h to 6h cuts non-HOT last_seen update volume roughly 6x on a hot, indexed column (documented as ~22.1M/day at the old width), with no change to probe/verdict correctness since the verdict SELECT is unconditional.

No findings to flag. The single-sourcing tests (QueryStoreTouchGuardSingleSourceTests) are a solid belt-and-suspenders design: a source-literal control that would catch a reverted site, and a shipped-SQL corroborator that would catch a site appearing/disappearing.

🤖 Generated with Claude Code

[claude]

Review

Reviewed the diff against dev. This is a Darling-only change (PostgreSQL storage layer); Lite has no equivalent Query Store touch-and-probe/dedup store, so there's no Lite/Darling parity concern here — the Two-Store Parity rules in CONTRIBUTING.md apply to shared interface implementations (IAlertStateStore and friends), and this guard isn't behind one. No T-SQL was added or changed (only C#/PostgreSQL and doc comments), so the T-SQL style section doesn't apply to this diff.

Manually traced the SQL composition for both TouchAndProbeSql properties (QueryStorePlanMap.cs, QueryStoreTextStore.cs) — the const string → static readonly string change is required and correct now that the literal is built from a non-const (GuardInterval is a runtime-composed static readonly), and no other call site depends on either property being a compile-time constant. The StampSkewMarginDays/GuardHours const derivation resolves at compile time with no circular runtime dependency, and GuardStaysInsideStampSkewMargin()'s divisibility invariant is safe as long as MarginShareDivisor divides 24 evenly, which the new mutation-style tests (M7) do pin.

One finding, not inline because the line isn't part of this PR's diff hunk:

Darling/PerformanceMonitor.Darling.Service/DarlingRetention.cs:1056 — stale "hourly" reference. The PR description states three prose sites in this file describing the guard as hourly were updated to reflect the new single-sourced, 6-hour-wide guard, but this comment (in the row-capped drain logging block) still reads "a one-day margin for the hourly last_seen refresh". It should reference QueryStoreLivenessTouchGuard.GuardHours like the other updated sites in this file, otherwise it's a fourth copy of exactly the drift this PR exists to eliminate.

No other correctness, security, or performance issues found in the diff.

[claude]

Reviewed. Summary: this is a Darling-only change (PostgreSQL storage layer for the Query Store plan/text cache) — Lite has no equivalent TouchAndProbeSql/liveness-guard mechanism, so there's no Lite/Darling parity drift to flag here.

Correctness — checked and consistent:

• StampSkewMarginDays = min(QueryStorePlanMap.PruneMarginDays=1, QueryStoreTextStore.PruneMarginDays=2) = 1 day → StampSkewMarginHours = 24 → GuardHours = 24 / MarginShareDivisor(4) = 6, matching the PR description's derivation.
• GuardStaysInsideStampSkewMargin() correctly enforces GuardHours >= 1 and exact divisibility, so integer-division truncation can't silently under-spend the margin.
• The TouchAndProbeSql fields had to move from const string to static readonly string since they now interpolate QueryStoreLivenessTouchGuard.GuardInterval (itself static readonly, built from a const int) — correct, and there's no circular static-init dependency since GuardInterval's inputs are all plain consts.
• QueryStorePlanMap.PruneMarginDays and QueryStoreTextStore.PruneMarginDays are referenced as consts from QueryStoreLivenessTouchGuard's own const initializer (StampSkewMarginDays), which is fine at compile time — no ordering hazard.
• Verified PgStatementText.PruneMarginDays == 2 and that its UpsertSql conflict arm only has a monotonicity guard (EXCLUDED.last_seen >=), not a staleness guard — so the doc's claim that it's rightly excluded from the min holds.
• Verified neither QueryStorePlanMap.cs nor QueryStoreTextStore.cs has any other numeric interval literal that could produce a false positive/negative in the new NumericIntervalLiteral regex scan (the only other INTERVAL ' sites are PruneSql's concatenation, where the digit isn't inside the same string literal as the quote).
• The widened guard (1h → 6h) stays safely inside both the day-margin (PruneMarginDays) and the dimension GC's ChunkIntervalDays + 1 margin, per MarginOrderingHolds; this is a write-amplification improvement, not a regression.
• TouchAndProbeSql is now built via runtime string concatenation instead of being a compile-time literal, but it's still computed once into a static readonly field, so no added per-call cost.
• All call sites (QueryStoreFetchProbe.cs) pass these fields as plain method arguments, so the const → static readonly change doesn't break anything requiring a compile-time constant.

Style: PostgreSQL SQL text consistently uses AS on aliases already (AS m, AS batch, AS t, AS d, AS x), matching repo convention. New test file (QueryStoreTouchGuardSingleSourceTests.cs) reuses the existing CSharpSourceWalker/RepoFile helpers rather than adding new copies, consistent with CONTRIBUTING.md's de-duplication guidance.

No correctness, security, or parity issues found.