Skip to content

The daily summary stops painting purged months green, and every MCP filter is part of the query: parallel_only/min_dop/blocking_only cut before the page, a negative window is refused, an unknown source names the accepted set (#3541 A9/A13) - #3641

Merged
erikdarlingdata merged 3 commits into
devfrom
fix/3541-daily-ghosts-silent-filters
Sep 18, 2026

Conversation

@erikdarlingdata

@erikdarlingdata erikdarlingdata commented Sep 18, 2026 •

Copy link
Copy Markdown
Owner

The daily summary stops painting purged months green, and every MCP filter is part of the query

Two of #3541's remaining items, on both SKUs. PARTIAL — the issue stays open by design (A10, A12, A15/A16 and the contract census remain).

A9 — retention ghosts

The lie. The daily aggregate's day spine is a UNION over nine sources, each LEFT JOINed and COALESCEd to zero — and the sources age out at different horizons: the seven per-signal collector tables at their 30-day default, the collection log at twice that, the alert log at ninety days. For every day between the shortest horizon and the longest, the spine still had the day (a run record or an alert still named it) while every signal the band reads — deadlocks, blocking, CPU, memory — was gone, and each rendered as a measured zero. Measured zeros band Healthy. A 366-day get_daily_summary_range therefore painted the purged months green, under a description that said a returned day meant collection happened and a quiet day was a healthy one. #3596's collection_runs was half the fix: it distinguishes a day nobody ran from a day somebody did — but a purged day has runs, because the run record is the thing that outlived the signals.

The mechanism. Three facts now decide each returned day, in one shared place (DailySummaryRetention.StateFor, PerformanceMonitor.Common), so the two SKUs — and the MCP tools and the Lite calendar within them — make the same call from the same inputs:

  1. The horizon, computed in the READER: Darling takes the shortest effective fleet retention among the seven signal collectors (StoreConfigProvider.ResolveFleetRetentionDays over the store's config_collector_schedules fleet rows, with the purge's own baseline floor on cpu_utilization) and the two log constants (DarlingRetention.CollectionLogRetentionDays / AlertHistoryRetentionDays) — so an operator-shortened retention moves the horizon, and the horizon the tool publishes is the horizon the purge enforces. Lite's is its one archive horizon, RetentionService.ArchiveRetentionMonths (a literal 3 promoted to a named constant, now also the cleanup's default). The clock is the reader's wall clock, deliberately not as_of: a purge is a wall-clock event and a backdated anchor cannot un-purge a table; anchoring the horizon to as_of would let "as_of 25 days ago, days_back 30" paint the purged stretch green again — the exact defect. The anchor still governs the window and the still-forming day's clamp; the tool bodies name no clock (the AsOfWindowAnchorTests census holds).
  2. Signal presence, a trailing signal_sources_present column appended to DailySummarySql.RangeSql and Lite's copy (the retention arm the brief allowed; Blocking and CPU bands are rates over the window they were measured in, tiered from 14 days of fleet data, so a week of blocking and an hour of blocking stop getting the same colour (#3539 A2/A3/A8d) #3596's band CTEs are untouched — it counts which of the seven signal joins produced a row). This is what keeps the horizon honest: the horizon is arithmetic, a purge is an event that may not have happened yet (daily sweep, whole-chunk drops, the Ungated catalog sweep bypasses the #1680 coverage gate: drop_chunks destroys rollup-uncovered raw history at 30 days #1784 coverage gate holding a table for weeks), and "the tables no longer hold it" must not be said of a day whose rows are demonstrably there.
  3. collection_runs, Blocking and CPU bands are rates over the window they were measured in, tiered from 14 days of fleet data, so a week of blocking and an hour of blocking stop getting the same colour (#3539 A2/A3/A8d) #3596's column.

The states: collected (a verdict), purged (past the horizon, no signal rows — NoData, never Healthy; the single-day tool answers it with the miss vocabulary's unavailable, hints carrying what the spine still holds), past_horizon (past the horizon but some signal rows survive — rows real, verdict withheld, the row says "N of 7 signal sources still hold rows"), no_run_record (inside retention, a signal row or an alert but no run record — a DISCLOSURE, not a withheld verdict: inside retention every zero is a measurement, so the band stands and an alert-only day is Warning as PerformanceCalendarDataTests has always pinned; what the state says is that the collection-error share has no denominator). The range payload publishes retention_horizon, days_before_horizon, purged_day_count, collected_day_count; every row carries data_state and a data_note saying what its zeros are. Both descriptions and both instruction rows say so.

Why this shape and not others. Two states (purged/collected on the horizon alone): rejected by executing it — the existing DailySummary_ExplicitDate_FindsRowsPlantedJustBeforeUtcMidnight plants a deadlock on a fixed day two months back, which the arithmetic calls purged while the row sits in the table; a rule that says "no longer holds it" of a held row is a new lie. Per-signal horizons and per-signal nulling: the honest end state, but it changes the aggregate's projection shape for three consumers and is a lane of its own; the single horizon plus presence gives the conservative answer (never Healthy past the horizon) without asserting a per-signal fact the SQL does not carry. Data-driven purge detection (no horizon): cannot tell a quiet collected day from a purged one, which is the defect.

summary_date is TryParseExact("yyyy-MM-dd") on both SKUs through McpHelpers.ParseSummaryDate: the general parser in the same file as as_of's strict allowlist accepted 01/02/2026 as 2 January for a caller who meant 1 February. Refused, naming the one accepted form.

A13 — silent filter semantics

  1. parallel_only / min_dop are a HAVING floor (COALESCE(MAX(max_dop), 0) >= $6) on the grouped population BEFORE the CPU ordering and the cap, in both Darling grouping reads and Lite's, so the page is the top-N of the filtered population. It was a .Where over the top-N page the SQL had already cut: a box whose twenty hottest plans were serial answered an empty page under parallel_only while the window held parallel plans — and the engine's own CXPACKET advice sends agents to exactly that call. filter_applied names the floor on the payload; an empty filtered page is an empty status naming the filter, not "no query stats available".
  2. get_active_queries pushes database_name and blocking_only into the SQL, counts the filtered population with COUNT(*) OVER () above the parameterised LIMIT (MCP percents name their denominator: shares are of the window or say they are of the page, so a three-row page stops summing to 100% of everything (#3541 A7) #3613's idiom), pages at limit + 1, and joins the paged dialect on both SKUs (snapshots_returned / truncated / oldest_returned_collection_time / newest_returned_collection_time / order) — it was the census's stated exclusion; the census comment moves with it. Head blockers are never stripped: the WAITFOR trim dropped the classic head blocker (a session sitting in WAITFOR with an open transaction) while its victims pointed at it; a row now stays whatever its text when a row in the same capture names it (is_head_blocker) — same capture, not same window, because session ids are reused and the old C# arm matched across the window. A victim whose blocker is not on the page says the one reason in blocker_not_shown: not_captured (the blocker held no running request — sys.dm_exec_requests never lists an idle open transaction; get_blocking has its input buffer), filtered (the caller's own database filter excluded it — honoured, and said), past_page. The contract decision: the caller's filters are honoured literally and the row explains, because blocking_only's description has always promised head blockers and a filter a caller sent is a question they asked.
  3. The three uncapped reads (get_collection_log, get_current_waits_trend, get_blocking_stats) refuse a non-positive hours_back through McpHelpers.ValidateUncappedWindow — the shared validator minus its ceiling — instead of Math.Abs-ing it: a window that ends before it starts is a caller error, and flipping the sign answered a different question with nothing to say so.
  4. get_analysis_facts source is validated against FactScorer.KnownSources — the registry the brief asked for, in the scorer, fifteen entries (the campaign counted fourteen; Analysis-engine campaign: coverage honesty, measured thresholds, config attribution (adversarial review, 11 findings) #3538's coverage is the fifteenth), pinned exactly against every Source = "…" literal in the three collector assemblies and against the scorer's switch arms — and refused with the whole set. Both descriptions spell the set out (pinned equal to the list).

Blast radius

  • DarlingHealthReader.GetDailySummaryRangeAsync returns a result record (rows + horizon); its two callers are in-file and the tool. DailySummaryReadRow / Lite's DailySummaryRow gain init/set members only; positional construction is unchanged, DataState defaults to Collected, so the fleet sweep's GetWindowSignalsAsync and every hand-built test row band as before. The Lite calendar (ServerTab.DailySummary) reads the same rows, so its cells go grey where the MCP says No Data.
  • DarlingDataReader.GetTopQueriesByCpuAsync / Lite GetTopQueriesByCpuAsync gain an optional minMaxDop = 0 — every existing caller (grids, live tests) is byte-identical in result. DarlingSessionReader.GetActiveQueriesAsync changes signature; its only caller is the tool. Lite's grid read GetLatestQuerySnapshotsAsync is untouched; the MCP read is a sibling.
  • FactScorer gains one static list. McpHelpers gains three validators and one const. CollectionBackgroundService changes one literal to the new constant.
  • Web dispatch signatures are unchanged; the web pages read only the queries / days arrays.

What it does NOT do

Test plan

CI's first execution (head 2) caught three things the Mac harness could not, all fixed on head 3: PerformanceCalendarDataTests pins an alert-only day Warning (hence no_run_record keeps its band), and its fixed July 2026 fixture month would have drifted past the three-month horizon within weeks (now the calendar month two months before the current one); DocCommentHygieneTests caught the range-read summary block displaced by an insertion; FindingsRetentionHorizonPinTests' literal pin follows the promoted constant. The XminHorizonPersistenceReadTests failure on the same run (expected 4, actual 5) is outside this change — it reads pg_xmin capture rows for its own server id and nothing here touches that path.

Executed locally before push (the shared libraries and the Darling service are net10.0): a throwaway harness compiling the real test files against the built assemblies ran 310 tests green, including the live-PostgreSQL classes against a throwaway timescale/timescaledb:2.28.1-pg18 container: the new McpFilterSemanticsLivePostgresTests (three serial groups hotter than one parallel group at top = 2 — the old code's empty page vs the parallel row; the six-row blocking capture with WAITFOR head kept and flagged, not_captured / filtered / past_page each produced, total_snapshots 6/1/5 as the filtered population, truncation pair 4/5; a 45-day-old run record reading purged beside today's collected, the single-day unavailable, a surviving deadlock turning it past_horizon with 1 of 7, a fleet override to 10 days moving the horizon), McpPageContractTests (both live tests + every census, with get_active_queries now a member and the A13 pins), DarlingMcpHealthToolsTests (new retention unit tests; the boundary-day live test updated for the day it plants now being before the horizon), DarlingMcpSessionToolsTests, DarlingMcpDataToolsTests, DarlingMcpToolsTests, AsOfWindowAnchorTests (both SKUs' tool bodies), FactSourceRegistryTests, DailyHealthBandTests + DailySummaryRetentionTests, and Lite's McpMissMessageParityPinTests (source-only). Lite's new DuckDB SQL (the CTE/EXISTS/COUNT(*) OVER () page read and the HAVING floor) was executed against DuckDB 1.5.5 in a throwaway console using the shipped statement text and produced the expected rows and counts. Lite's tool tests (Lite.Tests/McpPageContractTests new arms, DailyHealthBandTests) are compile-verified here and first executed in CI, being WPF-hosted.

New tests: McpFilterSemanticsLivePostgresTests (3), FactSourceRegistryTests (5), DailySummaryRetentionTests both SKUs (4 each), DarlingMcpHealthToolsTests (+6), McpPageContractTests Darling (+5 A13 census + get_active_queries in the paged census) and Lite (+5 DuckDB + census), McpMissMessageParityPinTests (+3 shared sentences). Builds: PerformanceMonitor.Darling.Service, PerformanceMonitorLite (-p:EnableWindowsTargeting=true), Darling.Tests, Lite.Tests, deprecated/Dashboard.Tests — 0 warnings, 0 errors.

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?

See the test plan above: 310 tests executed locally including the live PostgreSQL classes against a throwaway TimescaleDB 2.28.1 / PG18 container; DuckDB 1.5.5 execution of the new Lite SQL; zero-warning builds of every touched project and both test projects plus deprecated/Dashboard.Tests. No monitored SQL Server is involved — every read here is a stored read.

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 (stored reads only; live PostgreSQL + DuckDB executed)
  • I have not introduced any hardcoded credentials or server names

Part of #3541 (A9, A13). Does not close it.

@erikdarlingdata
erikdarlingdata enabled auto-merge (squash) September 18, 2026 19:19
auto-merge was automatically disabled September 18, 2026 19:45

Pull request was closed

@erikdarlingdata
erikdarlingdata enabled auto-merge (squash) September 18, 2026 19:45
@erikdarlingdata
erikdarlingdata force-pushed the fix/3541-daily-ghosts-silent-filters branch from c4568b1 to 48e72f8 Compare September 18, 2026 19:48
…ilter is part of the query (#3541 A9/A13)

A9: the daily aggregate's day spine outlives its signals (collection log 60 d, alert log 90 d,
signals 30 d) and COALESCEs each purged signal to a measured zero, so a year-long range banded the
purged months Healthy. Both SKUs' readers now compute the store's retention horizon (Darling: the
shortest EFFECTIVE fleet retention among the seven signal collectors and the two log constants;
Lite: RetentionService.ArchiveRetentionMonths) on the reader's wall clock, the SQL projects how
many signal sources still hold rows for the day, and the shared DailySummaryRetention.StateFor
judges each day: purged (past horizon, no signal rows -> No Data, never Healthy), past_horizon
(rows survive, verdict withheld), no_run_record, collected. The range tool publishes
retention_horizon / days_before_horizon / purged_day_count / collected_day_count and each row its
data_state + data_note; the single-day tool answers a purged day with the unavailable envelope.
summary_date is TryParseExact yyyy-MM-dd on both SKUs (McpHelpers.ParseSummaryDate).

A13: parallel_only / min_dop are a HAVING floor on the grouped population before the CPU ranking
and the cap (both SKUs, both Darling grouping reads), with filter_applied on the payload and a
filtered-miss sentence instead of "no query stats". get_active_queries pushes database_name and
blocking_only into the SQL, counts the filtered population with COUNT(*) OVER (), pages at
limit + 1 (snapshots_returned / truncated / bounds / order), never strips a head blocker some row
in the same capture names (is_head_blocker), and names why a victim's blocker is absent
(blocker_not_shown: not_captured / filtered / past_page). The three uncapped reads refuse a
non-positive hours_back through McpHelpers.ValidateUncappedWindow instead of Math.Abs. The
get_analysis_facts source filter is validated against FactScorer.KnownSources (15, pinned to every
collector literal) and refused with the whole set.

Census: get_active_queries joins McpPageContractTests' paged dialect on both SKUs; new A13 pins
(no .Where after the read, predicates before ORDER BY/LIMIT, no Math.Abs on a parameter anywhere,
the three uncapped reads on the shared validator); live PostgreSQL round-trips for the filter,
head-blocker, purged/past-horizon/override arms; Lite DuckDB twins.
…ed range-read summary goes back to its member; the two fixed-date Lite fixtures follow the horizon

CI's first execution caught three things the Mac harness could not. (1) PerformanceCalendarDataTests has always
pinned an alert-only day (no collection-log run) as Warning — the alert is real and inside retention every zero
is a measurement — so no_run_record is now a DISCLOSURE (error share has no denominator) that keeps the band,
and only the two past-horizon states withhold it; both SKUs' ToSignals, the descriptions, the instructions
rows and the pins say so. (2) The same fixture's fixed July 2026 month would have drifted past the three-month
horizon within weeks; it is now the calendar month two months before the current one. (3) DocCommentHygiene:
the DailySummaryRangeReadResult insertion had displaced GetDailySummaryRangeAsync's summary block. Plus the
FindingsRetentionHorizonPinTests literal pin follows the promoted RetentionService.ArchiveRetentionMonths.
@erikdarlingdata
erikdarlingdata force-pushed the fix/3541-daily-ghosts-silent-filters branch from 4ebfc73 to 390c006 Compare September 18, 2026 20:31
@claude

claude Bot commented Sep 18, 2026

Copy link
Copy Markdown

Reviewed the diff in depth (retention-state logic in PerformanceMonitor.Common/DailySummaryDataState.cs, the Darling/Lite daily-summary SQL and readers, the A13 filter-in-query changes for get_top_queries_by_cpu/get_active_queries, McpHelpers.ValidateUncappedWindow/ParseSummaryDate/ValidateChoice, and the FactScorer.KnownSources registry), plus cross-checked Lite vs Darling for parity on every surface touched.

Findings:

  • Correctness: DailySummaryRetention.StateFor's horizon-then-run-count ordering is sound and matches the documented invariants (purged/past_horizon/no_run_record/collected); boundary cases (horizon day itself, zero vs positive signal presence) are covered by both unit and live-Postgres tests. The A13 parallel_only/min_dop HAVING floor and the get_active_queries head-blocker logic (same-capture matching, blocker_not_shown ladder) check out against the SQL and the C# consumers on both SKUs.
  • Parity: Every A9/A13 surface has a matching Lite (DuckDB) and Darling (Postgres) implementation with the same vocabulary, JSON field names, and miss-message wording (pinned by McpMissMessageParityPinTests/McpPageContractTests). Parameter offsets in the DuckDB queries (e.g. GetTopQueriesByCpuAsync's minMaxDop shifting the database_name IN-clause start index from 6→7, and GetActiveQueriesPageAsync's from 5) line up correctly with the parameter-add order.
  • Security: All new predicates (database_name, blocking_only, min_dop/parallel_only floor, source) are bound as typed parameters ($N) on the Postgres side and DuckDBParameter on the Lite side — no string concatenation of caller input into SQL text.
  • Style: Postgres/DuckDB SQL here isn't T-SQL, so the CONTRIBUTING T-SQL rules (OPTION(RECOMPILE), etc.) don't apply; naive-UTC timestamp binding correctly strips DateTimeKind via DarlingMcpReadParameters.AddTimestamp.

No correctness, security, or Lite/Darling parity issues found. Nice work on the live-Postgres/DuckDB test coverage for the filter-semantics fixes.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — reviewed retention-state logic, A9/A13 SQL and reader changes, Lite/Darling parity, and parameter binding/security; no substantive issues found.

@erikdarlingdata
erikdarlingdata merged commit f681802 into dev Sep 18, 2026
10 of 13 checks passed
@erikdarlingdata
erikdarlingdata deleted the fix/3541-daily-ghosts-silent-filters branch September 18, 2026 21:00
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