[erikdarlingdata]

Three small verified honesty defects from the 2026-09 brains-review campaign, plus two lead-approved extensions folded in during review.

#3529 — get_memory_trend published a hardcoded total_granted_mb = 0.0 on both SKUs

The payload promised granted-memory data and shipped a literal zero; an agent investigating RESOURCE_SEMAPHORE could read "granted was 0 all window" and rule out memory grants at exactly the wrong moment.

• total_granted_mb is now an explicit null on both SKUs (identical shape), with an envelope-level granted_note naming get_memory_grants as the source of the grants series.
• Both tool descriptions corrected: they no longer promise granted memory and point at get_memory_grants.
• Pinned by: a Lite real-DuckDB payload test (null per point + note), the Darling gated-live test's extended payload assertions, and an ungated description pin per SKU.
• The grants-series join (the issue's "complete" option) is deliberately out of scope — filed as get_memory_trend: join the grants series so total_granted_mb carries real data (complete fix for #3529) #3548.

#3530 — Lite QueryStore time-slice read TotalReads and TotalLogicalReads from the same ordinal

The SELECT computes total_reads (logical) at ordinal 4, total_writes at 5, and total_physical_reads at 6. The reader mapped both read fields to 4 — a deliberate alias for the logical aggregate, matching the QueryStats slicer — but never read ordinal 6, so the physical column was computed and dropped. TotalPhysicalReads now maps ordinal 6.

• Pinned by a new test whose fixture rows carry pairwise-distinct values per I/O column (logical 110, writes 30, physical 50) — equal fixture values are exactly how the slip stayed invisible.

#3531 — LCK_M_IS advice handed out the RCSI ALTER with no caveats

The LCK_M_S twin closed with the brief-exclusive-lock timing note and the test-on-a-copy warning; LCK_M_IS recommended the identical ALTER calling RCSI "the strictly better mechanism" with neither. Both twins now carry the same three counter-objectives — the brief exclusive lock the ALTER takes, the tempdb version-store cost (phrased after FactRiskDisclosure's RCSI risk items, not duplicated from them), and the test-on-a-copy warning. Pinned by a shared advice-content test over both keys.

Extensions folded in per team-lead instruction

Both were initially left out under this PR's file boundary and filed as issues; the wave lead extended the boundary and directed they land here.

• QueryStore slicer plots logical reads under the "Avg Physical Reads" label #3547 — QueryStore slicer plotted logical reads under an "Avg Physical Reads" label. The sorting handler's physical arm was a forced workaround while the reader dropped the physical column; with Lite QueryStore time-slice reads TotalReads and TotalLogicalReads from the same column ordinal #3530 fixed, the mapping now targets TotalPhysicalReads (label "Total Physical Reads", mirroring the QueryStats slicer), with the matching bucket.Value case. The selected-row overlay follows: the timeline read carries avg_physical_io_reads through the same deduped CTE (no new dedup site), QueryStoreItemTimelinePoint gains PhysicalReads, and the overlay selector plots it — bars-only would have traded the label lie for a bars-vs-overlay mismatch (elapsed overlay on physical bars). The distinct-per-column test covers the timeline read path too; the overlay's metric→field switch itself is WPF code-behind and not unit-testable, like its sibling arms.
• Two stale doc lines still describe get_memory_trend's total_granted_mb as an always-0 parity placeholder #3549 — two doc lines still described the old 0-placeholder. Darling/README.md and DarlingTrendReader's doc comment now describe the shipped null + granted_note behavior.

Tests

• Lite.Tests (QueryStoreDedupReadTests + TrendEmptyParityToolTests): 21/21 pass, including the extended distinct-per-column pin over the slicer bucket AND the overlay timeline
• Dashboard.Tests (FactAdviceCorrectnessTests + FactAdviceComposeTests): 49/49 pass
• Darling.Tests (DarlingMcpTrendToolsSurfaceAndSqlTests + DarlingTrendEmptyTests): 35 pass, 1 skip (the gated _AgainstDevPostgres test; CI's darling-pg job runs it)
• All touched test projects build with 0 warnings, 0 errors

Fixes #3529, fixes #3530, fixes #3531, fixes #3547, fixes #3549

🤖 Generated with Claude Code

https://claude.ai/code/session_01QMYFB4qv5tqGwMmdPsa4Zx

[claude]

Reviewed the diff against CONTRIBUTING.md's C#/Two-Store-Parity conventions, focusing on correctness, Lite/Darling parity, security, and perf.

• get_memory_trend publishes a hardcoded total_granted_mb of 0.0 on both SKUs #3529 (fake granted-memory zero): total_granted_mb now nulls out identically in Lite/Mcp/McpMemoryTools.cs and Darling/.../DarlingMcpTrendTools.cs, with a matching granted_note in both envelopes. Confirmed McpHelpers.JsonOptions has no DefaultIgnoreCondition, so the (double?)null actually serializes as JSON null rather than being dropped — the tests asserting JsonValueKind.Null are checking something real, not a no-op.
• Lite QueryStore time-slice reads TotalReads and TotalLogicalReads from the same column ordinal #3530 (dropped physical-reads column): Verified against the SQL in Lite/Services/LocalDataService.QueryStore.cs — total_physical_reads is genuinely ordinal 6 in the SELECT list, so the new TotalPhysicalReads = reader.IsDBNull(6) ? 0 : ToDouble(reader.GetValue(6)) mapping is correct. Checked Darling's counterpart (ViewerDataService.QueryStore.cs → ReadQueryStatsSlicerAsync in ViewerDataService.QueryStats.cs) — it already read ordinal 6 correctly, so this was a Lite-only bug and there's no parity drift to fix on the Darling side.
• LCK_M_IS advice hands out the RCSI ALTER without the caveats its LCK_M_S twin carries #3531 (RCSI caveats): FactAdvice.cs lives in the shared PerformanceMonitor.Analysis project referenced by both Lite and Darling, so this fix applies to both SKUs automatically — no parity gap.
• Noticed DarlingTrendReader.cs:43-45 still has a doc comment describing the old "always-0 ... Lite parity placeholder" behavior, but that's exactly one of the two stale doc lines the PR description already calls out as filed under Two stale doc lines still describe get_memory_trend's total_granted_mb as an always-0 parity placeholder #3549 (outside this PR's file boundary), so not flagging it as new.

No correctness, security, or Lite/Darling parity issues found in the actual changed code.

[claude]

LGTM — verified the memory-trend null/granted_note change serializes correctly (no DefaultIgnoreCondition on JsonOptions), confirmed the QueryStore ordinal-6 fix matches the SQL and that Darling's equivalent reader already had it right (no parity drift), and confirmed FactAdvice.cs is shared so the RCSI caveat fix covers both SKUs.

[claude]

LGTM — reviewed the memory-trend null/granted_note change (Lite + Darling wording is byte-identical), the QueryStore physical-reads ordinal fix (verified ordinal 6 in the SELECT is total_physical_reads, and confirmed Darling's equivalent slicer already read it correctly so no parity gap existed), and the LCK_M_S/LCK_M_IS advice-caveat parity (shared FactAdvice.cs, wording consistent with FactRiskDisclosure's existing tempdb version-store language). No correctness, parity, or security issues found; tests added are appropriately targeted (distinct fixture values per I/O column, both SKUs pinned).

[claude]

Reviewed the diff in detail (Lite/Darling parity, the QueryStore ordinal fix, the RCSI advice caveats, and the memory-grant nulling). Everything in the diff itself checks out:

• GetQueryStoreSlicerDataAsync's ordinal 6 → TotalPhysicalReads mapping matches the SELECT's actual column order.
• The new QueryStoreItemTimelinePoint.PhysicalReads plumbing (SQL → record → grid overlay selector) is wired correctly end to end, and the new distinct-per-column test (SlicerBucket_MapsWritesAndPhysicalReads_ToTheirOwnColumns) would genuinely catch a logical/physical swap on either the bars or the overlay.
• Lite and Darling's get_memory_trend descriptions, the granted_note envelope field, and the null-vs-0 payload change are word-for-word identical between SKUs — no parity drift there. PerformanceMonitor.Analysis (FactAdvice) is shared between Lite and Darling, so the RCSI caveat fix for LCK_M_S/LCK_M_IS covers both automatically.
• McpHelpers.JsonOptions has no DefaultIgnoreCondition, so (double?)null correctly serializes to JSON null rather than being omitted — the tests asserting JsonValueKind.Null are checking real behavior.

One thing worth a look, not in this diff's blast radius but directly adjacent to it: Lite/Controls/ServerTab.Grids.cs's ProcedureStatsGrid_Sorting (around line 421) still has the exact bug this PR fixes for Query Store. It maps the TotalPhysicalReads column to the TotalReads (logical) metric under a "Total Physical Reads" label:

"TotalPhysicalReads" => ("TotalReads", "Total Physical Reads"),

And its data source, GetProcStatsSlicerDataAsync (Lite/Services/LocalDataService.QueryStats.cs ~line 824-869), computes total_physical_reads in the SELECT at ordinal 6 but never reads it into TotalPhysicalReads on the TimeSliceBucket — same drop-on-the-floor pattern as #3530. Interestingly, ComputeProcOverlayPoints (same file, ~line 117) already has a "TotalPhysReads" => h.DeltaPhysicalReads arm ready to go, so the overlay-selection plumbing is half-built and just needs the reader + sorting-handler fix to match what this PR just did for Query Store.

Not asking to block on this since it's outside the file regions this PR touches and wasn't called out as one of the deliberately-deferred items (unlike #3548), but given the PR's whole premise is "don't silently drop the physical-reads column or mislabel the series," it seems worth a fast follow-up issue so it doesn't get lost.

[claude]

LGTM — verified the ordinal-6 physical-reads mapping against the actual SELECT column order, traced the new PhysicalReads field through the SQL → record → grid overlay selector, confirmed Lite/Darling get_memory_trend descriptions and granted_note payloads are word-for-word identical (shared FactAdvice covers the RCSI caveat fix for both SKUs automatically), and confirmed JsonSerializerOptions has no null-ignore so the null-payload tests reflect real behavior. Flagged one non-blocking adjacent issue (ProcedureStatsGrid_Sorting has the same dropped-physical-reads bug, untouched by this PR) as a PR comment for a follow-up.