Repository navigation
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
Merged
Conversation
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
enabled auto-merge (squash)
September 18, 2026 19:45
erikdarlingdata
force-pushed
the
fix/3541-daily-ghosts-silent-filters
branch
from
September 18, 2026 19:48
c4568b1 to
48e72f8
Compare
…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
force-pushed
the
fix/3541-daily-ghosts-silent-filters
branch
from
September 18, 2026 20:31
4ebfc73 to
390c006
Compare
|
Reviewed the diff in depth (retention-state logic in Findings:
No correctness, security, or Lite/Darling parity issues found. Nice work on the live-Postgres/DuckDB test coverage for the filter-semantics fixes. |
This was referenced Sep 18, 2026
Closed
This was referenced Oct 2, 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 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
UNIONover nine sources, eachLEFT JOINed andCOALESCEd 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-dayget_daily_summary_rangetherefore 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'scollection_runswas 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:StoreConfigProvider.ResolveFleetRetentionDaysover the store'sconfig_collector_schedulesfleet rows, with the purge's own baseline floor oncpu_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 literal3promoted to a named constant, now also the cleanup's default). The clock is the reader's wall clock, deliberately notas_of: a purge is a wall-clock event and a backdated anchor cannot un-purge a table; anchoring the horizon toas_ofwould 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 (theAsOfWindowAnchorTestscensus holds).signal_sources_presentcolumn appended toDailySummarySql.RangeSqland 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.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'sunavailable, 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 asPerformanceCalendarDataTestshas always pinned; what the state says is that the collection-error share has no denominator). The range payload publishesretention_horizon,days_before_horizon,purged_day_count,collected_day_count; every row carriesdata_stateand adata_notesaying 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_FindsRowsPlantedJustBeforeUtcMidnightplants 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_dateisTryParseExact("yyyy-MM-dd")on both SKUs throughMcpHelpers.ParseSummaryDate: the general parser in the same file asas_of's strict allowlist accepted01/02/2026as 2 January for a caller who meant 1 February. Refused, naming the one accepted form.A13 — silent filter semantics
parallel_only/min_dopare 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.Whereover the top-N page the SQL had already cut: a box whose twenty hottest plans were serial answered an empty page underparallel_onlywhile the window held parallel plans — and the engine's own CXPACKET advice sends agents to exactly that call.filter_appliednames the floor on the payload; an empty filtered page is anemptystatus naming the filter, not "no query stats available".get_active_queriespushesdatabase_nameandblocking_onlyinto the SQL, counts the filtered population withCOUNT(*) OVER ()above the parameterisedLIMIT(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 atlimit + 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 inWAITFORwith 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 inblocker_not_shown:not_captured(the blocker held no running request —sys.dm_exec_requestsnever lists an idle open transaction;get_blockinghas 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, becauseblocking_only's description has always promised head blockers and a filter a caller sent is a question they asked.get_collection_log,get_current_waits_trend,get_blocking_stats) refuse a non-positivehours_backthroughMcpHelpers.ValidateUncappedWindow— the shared validator minus its ceiling — instead ofMath.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.get_analysis_factssourceis validated againstFactScorer.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'scoverageis the fifteenth), pinned exactly against everySource = "…"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.GetDailySummaryRangeAsyncreturns a result record (rows + horizon); its two callers are in-file and the tool.DailySummaryReadRow/ Lite'sDailySummaryRowgain init/set members only; positional construction is unchanged,DataStatedefaults toCollected, so the fleet sweep'sGetWindowSignalsAsyncand 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/ LiteGetTopQueriesByCpuAsyncgain an optionalminMaxDop = 0— every existing caller (grids, live tests) is byte-identical in result.DarlingSessionReader.GetActiveQueriesAsyncchanges signature; its only caller is the tool. Lite's grid readGetLatestQuerySnapshotsAsyncis untouched; the MCP read is a sibling.FactScorergains one static list.McpHelpersgains three validators and one const.CollectionBackgroundServicechanges one literal to the new constant.queries/daysarrays.What it does NOT do
past_horizonstate discloses the ambiguity instead).ViewerDataService.DailySummary.cs) — outside this lane's boundary; it reads the same SQL and now receivessignal_sources_present, so its port isHasData = StateFor(...) == Collectedplus the horizon read. Reported to the coordinator.DailyHealthBandCalculator.Describesays "No data collected." for anyHasData = false) —DailyHealthBand.csis 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.Test plan
CI's first execution (head 2) caught three things the Mac harness could not, all fixed on head 3:
PerformanceCalendarDataTestspins an alert-only day Warning (henceno_run_recordkeeps 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);DocCommentHygieneTestscaught the range-read summary block displaced by an insertion;FindingsRetentionHorizonPinTests' literal pin follows the promoted constant. TheXminHorizonPersistenceReadTestsfailure on the same run (expected 4, actual 5) is outside this change — it readspg_xmincapture 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 throwawaytimescale/timescaledb:2.28.1-pg18container: the newMcpFilterSemanticsLivePostgresTests(three serial groups hotter than one parallel group attop = 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_pageeach produced,total_snapshots6/1/5 as the filtered population, truncation pair 4/5; a 45-day-old run record readingpurgedbeside today'scollected, the single-dayunavailable, a surviving deadlock turning itpast_horizonwith1 of 7, a fleet override to 10 days moving the horizon),McpPageContractTests(both live tests + every census, withget_active_queriesnow 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'sMcpMissMessageParityPinTests(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/McpPageContractTestsnew arms,DailyHealthBandTests) are compile-verified here and first executed in CI, being WPF-hosted.New tests:
McpFilterSemanticsLivePostgresTests(3),FactSourceRegistryTests(5),DailySummaryRetentionTestsboth SKUs (4 each),DarlingMcpHealthToolsTests(+6),McpPageContractTestsDarling (+5 A13 census +get_active_queriesin 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?
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
dotnet build -c Debug)Part of #3541 (A9, A13). Does not close it.