Six small honesty riders from the brains-review residue: a retired knob says so on the wire, get_pg_server_config says when it was captured, Lite's latch/spinlock pages bind to the caller's limit, the ORDER-BY-integer fold gets a census, and two rung docs stop asserting superseded facts (#3653)

[erikdarlingdata]

What does this PR do?

Six SMALL items from #3653's residue queue (the brains-review campaign's deferred list), one PR, each item named so the checklist can be ticked. Partial for #3653 — it does not close it. Every one is the same kind of lie: a surface that says a little less than it knows, or a little more than is true.

1. poison_wait.threshold_ms says it is retired, on the wire (both SKUs)

#3593 moved the Poison Wait alert to ACCUMULATED wait over a ten-minute window; IAlertEngineSettings.PoisonWaitThresholdMs survives on the contract because both SKUs persist it and their Settings windows show it, but nothing evaluates it. get_alert_settings kept reporting it as though it were a knob, and Darling's update_alert_settings kept accepting it as though setting it did something.

The key STAYS — removing a published field is a contract break for a value that costs nothing to carry. Beside it, both SKUs now emit threshold_ms_note: "retired by #3593 — the alert grades accumulated wait over a ten-minute window; this value is stored and reported but not consulted" (one constant, DarlingMcpAlertTools.PoisonWaitThresholdMsNote; Lite carries the text by hand and its test reads Darling's declaration to hold the two byte-equal, the cpu.mode vocabulary idiom). update_alert_settings becomes accept-but-warn: the value is validated under its old bounds first (0 is still invalid), stored, listed in updated_fields, and named in a new always-present warnings: [] array on the success envelope. The note itself, handed back by a caller who round-tripped the whole read payload, is accepted with no column and no warning — refusing it would fail the read → modify → write invariant EveryColumnRead_IsEmittedByThePayload_AndAcceptedByTheWriter holds. Both descriptions state the retirement.

While in that description, one stale clause got fixed: get_alert_settings still said "a PostgreSQL server has no deadlock or blocking health band" — #3638 gave it a measured deadlock band, and update_alert_settings' own description already said so two paragraphs down. It now says the blocking band is what PG lacks.

2. get_pg_server_config says when it was captured

McpLatestSnapshotStampTests' reader-call regex named GetLatest*, *Latest*, *Snapshot* and a list of specific readers, but not the GetCurrent*Async family — so DarlingPgServerConfigReader.GetCurrentConfigAsync, a MAX(collection_time)-anchored latest read, sat outside #3637's census, and a sibling lane recorded the gap (the DarlingOnlyStamped allowance) rather than closing it. The regex now carries GetCurrent\w+Async; the tool joins DarlingOnlyStamped (Lite has no PostgreSQL target, so there is no pair for the roster); CurrentConfigSql selects c.collection_time ON THE ROW STATEMENT (no second MAX() that could stamp the next capture — the EveryStampedRead_SelectsItsStampColumn_OnTheRowStatement theory gains the row); PgConfigRow grows CollectionTimeUtc; the payload carries captured_at. The description says LATEST IS A TIME and that the collector runs HOURLY (the brief said "captured on connect, can be weeks old" — that is the SQL Server config family's shape; CollectorScheduleDefaults puts pg_server_config at 60 minutes, so the description says hourly). The sweep's population went 27 → 28 examined bodies; the ONLY newly matched tool is get_pg_server_config, verified by running the widened and unwidened regex side by side over the tree. GetConfigChangesAsync (a windowed history read in the same reader) is witnessed as NOT matching.

3. Lite's latch / spinlock pages bind to the caller's limit

LocalDataService.LatchSpinlock.cs capped both latest-snapshot reads at a literal LIMIT 20 while get_latch_stats / get_spinlock_stats published latch_count / spinlock_count as if they were the snapshot's population — and sys.dm_os_spinlock_stats carries well over a hundred spinlocks, so the spinlock page was always the hot tail dressed as the whole. #3594's pattern, exactly as get_wait_stats did it: the readers take limit defaulting to a named LatchSpinlockGridRowCap = 20 (the grid passes nothing and reads exactly what it always read — the #3613 idiom); the tools take limit (validated by ValidateTop, ceiling 1,000), fetch limit + 1, and publish latches_returned / spinlocks_returned, truncated (observed, never count >= limit), and order. limit sits before as_of, as on every other Lite paged tool; the two positional test callers were re-bound by name. McpInstructions rows updated. Darling's get_latch_stats is a different shape (a windowed top-N rate read with top) and is untouched.

4. The ORDER-BY-integer fold gets a census

ORDER BY 3 + 4 DESC is not "ordinal 3 plus ordinal 4"; PostgreSQL and DuckDB fold it to the constant 7 and the sort is no sort — #3613 found get_pg_kernel_stats had never been ranked. Parse analysis cannot see it (the statement is valid; it means the wrong thing), so DarlingPgReadSqlParsesLiveTests gains a no-server fact beside the live one: every *Reader string field in the Storage assembly (all of them, not only DarlingPg* — SQL Server-side reads fold identically; static readonly composed text included) and every Lite/Services/*.cs (DuckDB SQL lives inline there) are swept for ORDER BY <int> <op> <int> as the leading item or any later comma item on the clause, with SQL comments blanked first so a rationale that quotes the defect is not a hit. Zero hits today, on both SKUs — the regex was run against the tree before the fact was written. Population floors (≥ 40 Darling fields with an ORDER BY, ≥ 150 Lite clauses; measured 48 and 225) and a witnessed defect/fixed pair (three shapes that must match, six that must not, including ORDER BY 3 DESC, 4 DESC, ORDER BY total_ms * 1.5, and coalesce(x, 0)) keep the sweep from passing over nothing.

5. V79 / V122 rung docs stop asserting superseded facts

Comment-only, in each rung's own voice, one parenthetical clause: V79's "this file grew N MB inside the window" carries "(superseded by #3631 — file_growth_rise_mb is a RATE, MB per hour, same column, same default)"; V122's "a PostgreSQL server has no deadlock band to agree with" carries "(superseded by #3638 — the PostgreSQL card bands its own pg_stat_database.deadlocks counter through the SAME health_bands tiers; the columns stay separate for the first reason, the instruments differ)". No SQL const, no rung ordering, no StorageVersion touched; MigrationLadderPins / DocCommentHygieneTests pass by construction (no cref added, no summary opened).

6. ToolRecommendations learns HADR_SYNC_COMMIT (Lite)

#3616 taught the scorer to grade the fleet's second-largest wait, and the next_tools table had no entry, so the finding arrived on the card with nothing to do next. Lite's entry: get_wait_trend (wait_type: HADR_SYNC_COMMIT), get_alert_history naming AgAlertPolicy's AG Sync Fell Behind / AG Replica Disconnected rows as the secondary-side evidence (Lite has NO AG-health MCP read — Darling's get_ag_health has no twin here — so this is the honest pointer), get_perfmon_trend (Transactions/sec), get_file_io_stats. Every tool is a read: synchronous commit is a durability policy and the remediation is never "switch to async" (FactAdvice says so). A new ToolRecommendationsTests holds the specific entry AND the structural property nothing had held: every recommended tool is a Lite [McpServerTool] and every suggested parameter is on that tool's signature, reflected off the assembly. Darling's twin table lives in DarlingMcpTools.cs, which the #3542 plumbing lane owns tonight — deliberately NOT edited; reported to the coordinator as a one-line follow-up for that lane. The deprecated Dashboard twin is the frozen-twin batch's.

What it does NOT do

• Does not close Brains-review campaign: deferred structural residue (from #3538 / #3539 / #3540 / #3541) #3653 (six of its items; the rest are structural or other lanes'). Does not touch FactScorer / FactAdvice / InferenceEngine / RelationshipGraph / AnomalyIncidentReconciler / DarlingMcpTools / DarlingAnalysisService / DarlingWorker (the concurrent lane's files).
• Does not remove poison_wait.threshold_ms from any surface, Settings window, JSON or column.
• Does not change Darling's get_latch_stats / get_spinlock_stats (a different, windowed shape) or the Lite viewer grid's row count.
• get_pg_server_config keeps truncated = rows.Count >= limit and a page-scoped non_default_count — that is MCP pages now say what bounded them: caps bind to the caller's limit, truncation is detected not inferred, and no page count is called a total (#3541 A3) #3594's class on a Darling-only tool, noted for the coordinator, not folded in here.

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?

Builds (-c Debug -v q -nologo, test projects with -p:EnableWindowsTargeting=true): PerformanceMonitorLite, PerformanceMonitor.Darling.Service, Darling.Tests, Lite.Tests — 0 warnings, 0 errors each.

Tests are net10.0-windows and cannot run on the authoring Mac; CI is their first execution. The pure-logic pins were EXECUTED here through a throwaway console harness against the built PerformanceMonitor.Darling.Service / .Storage DLLs, calling the real methods: BuildAlertSettingsPayload serialises poison_wait: {enabled, threshold_ms: 1000, threshold_ms_note: "retired by #3593 …"}; BuildAlertSettingsUpdate returns one target + one warning for threshold_ms: 700, an invalid and no warning for 0, no target and no warning for the note key, one target and no warning for enabled; both descriptions carry the pinned phrases; the ORDER BY sweep finds 48 Darling fields / 225 Lite clauses and zero hits, and all nine witnesses land as asserted; the widened latest-reader regex examines 28 bodies and newly matches exactly get_pg_server_config, with captured_at present. Harness deleted.

New tests (compile-verified here, first executed in CI): DarlingMcpAlertToolsTests.PoisonWaitThresholdMs_IsReportedWithItsRetirementNote_AndWrittenWithAWarning; McpAlertSettingsKeyTests.PoisonWaitThresholdMs_CarriesDarlingsRetirementNote_ByteForByte; McpLatestSnapshotStampTests (Darling) — DarlingOnlyStamped + CurrentConfigSql row + two regex witnesses; McpPageContractTests (Lite) — GetLatchStats_CapBindsToLimit_AndTruncationIsObserved, GetSpinlockStats_CapBindsToLimit_AndTruncationIsObserved, roster +2; DarlingPgReadSqlParsesLiveTests.NoShippedRead_OrdersByAnIntegerExpression_OnEitherSku; Lite.Tests/ToolRecommendationsTests (new file, two facts). Existing censuses pre-checked: EveryColumnRead_IsEmittedByThePayload_AndAcceptedByTheWriter (the note key is accepted and claims no column), Lite's GetAlertSettings_ReportsEveryGroupDarlingDoes_SpelledDarlingsWay (both SKUs emit threshold_ms_note), CommentFilterAdoptionTests (no .StartsWith( prefix filter added), RepoFileAdoptionTests (raw reader only, no LF-anchored pin).

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 (no SQL Server surface changed; pins executed against the built assemblies, see above)
• I have not introduced any hardcoded credentials or server names

[claude]

Reviewed the diff against CONTRIBUTING.md and checked for Lite/Darling parity drift, correctness, and security. Summary of what I verified:

• DarlingPgServerConfigReader.CurrentConfigSql: new c.collection_time column lands at ordinal 13, and GetCurrentConfigAsync reads reader.GetDateTime(13) — matches. rows.Count == 0 is checked before rows[0].CollectionTimeUtc is dereferenced in GetPgServerConfig, so no null/empty-row risk.
• BuildAlertSettingsUpdate: signature widened from a 2-tuple to a 3-tuple (Warnings added); confirmed the only production call site (UpdateAlertSettings) was updated, and the bound check for threshold_ms still runs before the warning is appended, so an invalid value is refused rather than stored-with-a-warning.
• Lite get_latch_stats/get_spinlock_stats: limit was inserted before as_of (matching the rest of Lite's paged-tool convention), the DuckDB LIMIT $4 parameterization follows the same pattern already used throughout LocalDataService.*.cs, and the limit + 1/truncate-by-slicing logic matches the get_wait_stats precedent. No positional (non-test, non-MCP-reflection) callers of these two methods exist outside the test file, so the parameter reorder is safe.
• Parity: Darling's get_latch_stats/get_spinlock_stats are a genuinely different (windowed, top-based) shape and correctly untouched. The ToolRecommendations / HADR_SYNC_COMMIT entry is Lite-only by design — the PR body explicitly flags Darling's twin table in DarlingMcpTools.cs as deliberately deferred to the PostgreSQL-target analysis engine: design and v1 build #3542 lane, which is the right call rather than a silent drift.
• PgMigrations.cs: confirmed the V79/V122 changes are comment-only (inside /// doc comments) — no migration SQL body or StorageVersion was touched, which matters since these are already-executed migrations.
• All new SQL is parameterized; no injection surface.

Nothing substantive to flag — this is a tightly-scoped, well-tested PR.

[claude]

LGTM — verified ordinal/reader mapping for the new pg_server_config timestamp column, the BuildAlertSettingsUpdate tuple-widening's single call site, Lite paged-tool limit/truncation logic against the get_wait_stats precedent, Lite/Darling parity (including the deliberately-deferred Darling ToolRecommendations twin), and that the PgMigrations.cs V79/V122 changes are comment-only. No correctness, security, or parity issues found.

[claude]

LGTM — reviewed all 6 items (poison_wait retirement note on both SKUs, get_pg_server_config captured_at stamp, Lite latch/spinlock paging, ORDER-BY-integer-fold sweep, doc-comment updates, HADR_SYNC_COMMIT recommendation entry). Checked: parameter binding/ordering on the new limit params matches the established get_wait_stats idiom (limit before as_of, limit+1 fetch, truncated observed not inferred), DuckDB LIMIT $4 parameterization matches existing patterns elsewhere in LocalDataService, the two positional test call sites that would've silently mis-bound to the new limit slot were correctly re-bound by name, rows[0] access in get_pg_server_config is guarded by the prior rows.Count == 0 early return, and the Lite/Darling parity claims in the PR body (Darling's get_latch_stats/get_spinlock_stats being a different windowed top-based shape, DarlingMcpTools.cs deliberately left untouched) check out against the actual code. No correctness, security, or parity issues found.