Skip to content

Custom Views panels, Active Queries and Current Waits say where their data starts - #4953

Merged
erikdarlingdata merged 22 commits into
devfrom
fix/cv-retention-notice
Oct 3, 2026
Merged

erikdarlingdata merged 22 commits into
devfrom
fix/cv-retention-notice

Conversation

@erikdarlingdata

@erikdarlingdata erikdarlingdata commented Oct 2, 2026 •

Copy link
Copy Markdown
Owner

What changes

Some windows reach further back than the rows the store holds. These surfaces now say where the data starts, instead of drawing a shorter range as if it were the whole window:

  • Custom Views panels in the web viewer. The run_custom_view_panel MCP tool uses the same runner and gets the same notice. The notice reads like this: "partial window: this panel's data starts at 2026-09-25 16:00 UTC, after the window's start at 2026-09-02 16:00 UTC. The panel covers 2026-09-25 16:00 to 2026-10-02 16:00 UTC." It can show with the row-cap notice. It does not show when the retention-tier notice does. The tool's description names the three causes of a notice. The tier's retention did not cover the window, the panel's own data starts after the window does, or the row cap cut the result. No notice means the window was served whole. (seen: TheNoticeContract_NamesAllThreeCauses_InTheHeadAndInTheTail)

  • Active Queries and Current Waits in the Darling viewer. By default, query_snapshots and waiting_tasks keep 7 days. A custom range that reaches past them now shows the "Showing since ..." banner that the Queries tab already shows. A drill-down to Active Queries sets the banner for its own window. The Latest Snapshot read takes the banner down, because it shows the newest batch, not a range.

  • The Queries tab banner, and four MCP tools: get_top_queries_by_cpu, get_top_procedures_by_cpu, get_query_store_top and get_query_store_clutter. They read the raw window floor, which looked for the start inside the window. So a server that was quiet for the first hours of a window read as cut short. The floor is now the oldest row at or before the window's end, found the same way as above. It is still null when the window holds nothing. The served start these tools report never comes before the requested start. All four judge the cut through the shared RawWindowFloor.IsTruncated. get_query_store_clutter had kept its own copy of the 90-minute tolerance. (seen: tests below)

  • Lite. Active Queries and Current Waits now show the "Showing since ..." banner when a range starts before the data's coverage. A drill-down to Active Queries sets the banner for its own window, and the Live Snapshot read takes it down. Lite's window floor also looked for the start inside the window. So the Queries tab banner and three MCP tools called a quiet start a cut: get_top_queries_by_cpu, get_top_procedures_by_cpu and get_query_store_top. That no longer happens. The served start these tools report never comes before the requested start. (seen: tests below)

The cause can be retention or a server that was added recently. A quiet start gets no notice when the store covered the whole window. (seen: the live tests below plant rows and logged runs and read the notice back)

Each surface and its result

Surface Result
Custom Views panels (web) Notice added. (seen)
run_custom_view_panel MCP tool Same runner, so it gets the same notice. (seen: the only caller of the tier notice is the shared runner)
Custom Views in the Darling viewer The Darling viewer has no Custom Views renderer. Files checked: MainWindow.Tags.cs, ViewerDataService.cs. (seen)
Active Queries (query_snapshots) Banner added for the grid read, the slicer read, the read from a finding card, and the drill-down to a narrow window. The Latest Snapshot read hides it. (seen)
Current Waits (waiting_tasks) Banner added above both charts. Both charts read waiting_tasks. The tab's read fan-out widens from 2 to 3 for the probe (ViewerServerTab.Blocking.cs). (seen)
Queries tab (query_stats, procedure_stats, query_store_stats) The existing banner no longer shows for a quiet start. (seen)
get_top_queries_by_cpu, get_top_procedures_by_cpu, get_query_store_top, get_query_store_clutter Same floor. window_truncated no longer fires for a quiet start, and effective_start never precedes the requested start. (seen)
Running Jobs (running_jobs) No change needed. It reads the latest batch only (ViewerDataService.RunningJobs.cs:71). (seen)
Agent status No change needed. It reads the newest row only (ViewerDataService.JobHistory.cs:463, :507). (seen)
Wait drill-down No change needed. It reads 30 minutes either side of the clicked point (ViewerServerTab.DrillDown.cs:329-337). (seen)
Desktop viewer tabs over tables that keep 30 days A custom range past 30 days can still reach past them with no banner. The new probe takes any collector table whose index is (server_id, <time>), so each such tab needs one data-service read and one banner. (inferred)
Lite: Active Queries (v_query_snapshots) Banner added for the refresh paths, the slicer, and both drill-downs (the charts' and the heatmap's). The Live Snapshot read hides it. (seen)
Lite: Current Waits (v_waiting_tasks) Banner added for the refresh paths. (seen)
Lite: Queries tab grids (v_query_stats, v_procedure_stats, v_query_store_stats) The existing banner no longer shows for a quiet start. (seen)
Lite: get_top_queries_by_cpu, get_top_procedures_by_cpu, get_query_store_top Same probe. window_truncated no longer fires for a quiet start, and effective_start never precedes the requested start. (seen)
Lite: Plan Viewer No change needed. It runs no windowed read (RefreshVisibleTabAsync). (seen)
Lite: the other tabs and sub-tabs, and the other time-ranged MCP tools No banner. A range that starts before the oldest stored row draws a shorter range with no notice, as the Darling viewer's 30-day tabs do. #4958 lists each surface. (seen: the list. Each table's retention was not checked.)

How the start is found

The notice decides from coverage, not from the oldest stored row. Coverage starts at the later of two instants: the server's first collection by the table's collector, and the table's purge edge. A row in the window only moves it earlier. A window that starts at or after coverage shows no notice, whether it holds rows or not. A window that starts before it says where coverage starts.

DataWindowFloor (in Storage) asks this in one statement per panel render: a LATERAL per server through collect.servers. (seen: tests below)

  • Why coverage. waiting_tasks and query_snapshots hold a row only while something waits or runs. A server idle overnight, whose older rows the purge dropped, has its first row hours after a 7-day window starts. A server whose first waiting task came 3 days ago has no row before that. The store covered both windows, and the oldest-row rule called both cut short. (seen: the RED below)
  • Darling's first collection is the registry's created_date. The service writes it once, at the server's first connect, and no purge touches the registry. collection_log is not used for it. It keeps 60 days, and a fleet override can keep a table longer, so the log can forget a first run the table still covers. A collector turned on after the first connect reads as covered from that connect: no notice, never a false one. Lite reads coverage from its logged runs instead. So for a collector turned on late, Lite names where its runs start, and Darling shows no notice. (inferred from the code)
  • Darling's purge edge is the probe's clock less the collector's fleet retention days: the fleet override in config.config_collector_schedules, else the schedule default. The purge applies one horizon to the whole table, so the edge is the same for every server. The viewer's role can read that table: DarlingManagedRolesTests pins SELECT on every config table. (seen)
  • A source with no schedule edge keeps the oldest row. Three kinds have none. They are the rollups, the raw relations the gated purge owns, and the collectors whose purge is floored at the baseline window. Their edge is the oldest row the server holds at or before the window's end, the rule before this change. These tables are dense, so that row is where coverage starts. Without it, those panels lose the notice they had. The tier notice covers a rollup route on a TimescaleDB store, but not these collectors, and not a store without TimescaleDB. (seen: ABaselineFlooredTable_ForAServerOlderThanTheWindow_StillSaysWhereItsOldestRowIs_AgainstDevPostgres)
  • Which servers count. A server counts only when it holds a row, or a logged run of the collector, in the window. The answer is the earliest coverage among the servers that count. A server stopped or removed before the window keeps its registry row and history (inferred: removal deletes only its definition, BaselineCache.cs:58). Its old rows no longer hide a newer server's late start on a fleet panel. (seen: AFleetPanel_StoppedServersOldRows_DoNotHideALateStart_AgainstDevPostgres) After a server rename, the probe still finds the server by its registry row: its current name and server_id. The panel filters rows by their stored server_name. So rows written before the rename count toward coverage, though the panel's filter misses them. (inferred from the code. DataWindowFloor's remarks say so.)
  • The empty window. The probe answers null when no server has a row or a logged run in the window. It also answers null when coverage starts at or after the window's end. collection_log keeps 60 days and waiting_tasks keeps 7, so a logged run can make a server count in a window whose rows are gone. That window lies wholly before coverage. Callers read null as nothing to report, as they do for a window older than the run log, and as Lite does. Such a panel is empty and shows no notice. A covered window with no rows answers its start or earlier, so it also shows no notice. (seen: the tests below)
  • Any logged run counts, whatever its status. A failed run leaves a gap inside the covered range. A failure in the middle of a window does too, and neither gets a data-start notice. (seen: the SQL)
  • Per server, through the registry. On a test store with 3.36M wait_stats rows, a server_name = ANY filter on the fact table took 708.6 ms. The per-server walk took 0.228 ms for one server and 3.410 ms for the fleet. (seen)
  • A rollup panel takes its start from its rollup. The relations come from the route's FROM clause, and every rollup route carries one. The day-grain daily route now carries its clause too. Its compiled SQL is unchanged. A rollup route without a clause is refused with an error. The router builds none, and EveryRollupRoute_CarriesTheFromClauseItReads pins that. (seen)
  • A stitched route is probed the way it is read. A stitched route reads the successor only from the stitch boundary up. So the probe reads the successor from the boundary up too. The boundary is a bind parameter on both the window read and the oldest-row walk. Probed whole, a successor materialized back past the boundary gave a start below it, and no notice, though the panel's data starts at the boundary. The superseded half is still probed whole. Asking it whole can only find an earlier start, so it cannot raise a false notice. (seen: the failing-first runs below)
  • The raw window floor asks the same question. RawWindowFloor walks the same index from the oldest row. A bounded existence check keeps the floor null when the window holds nothing, because the MCP tools report that as an empty window. The served start is clamped to the requested start. (seen)
  • Lite asks in two steps. DuckDB has no (server_id, collection_time) index to stop at the first row. Reading a server's oldest row reads every archived row it holds, so Lite's GetQueryWindowFloorAsync splits the question. Step 1 reads the first row inside the window, bounded on both sides. DuckDB can then skip archived data outside it (inferred from the timings below). Only when that row comes after the start does step 2 look for one older row, with LIMIT 1. For Active Queries and Current Waits, the collector's runs in v_collection_log count as well as rows, in both steps. The log is archived and deleted by the same single horizon as the tables, so a run proves the window from then on is held. The Queries-tab relations keep the row-only probe. The answer is null when the window holds no row (and, for the two coverage tabs, no run). It is the start when an older row or run exists, and otherwise the first row or run in the window. (seen)
  • Which tables it reads. It reads collector tables whose index is (server_id, <time>), and the rollups a panel can read. index_object_stats is left out because its index orders by object before time. (seen: DataWindowFloorTests)
  • When it runs. It runs only when the tier notice is silent. It does not run when the Query Store wide table served the panel, because that table already reports its own start in the history note. If the probe fails, the panel shows no notice and does not fail, and the service logs the failure at Debug. Debug, not Warning, because the probe runs on every panel run. At Warning, a standing fault logs a line on every refresh of every panel. The verdict reuses RawWindowFloor.IsTruncated, with the same 90-minute slack the Queries tab uses. (seen)
  • A failed probe costs only the banner's date. In Lite and the Darling viewer, a probe that throws logs a warning. No "Showing since" date shows, and the refresh goes on. Without the catch, the throw skips what comes after the probe. Examples are the Locking tab's slicers and tab-badge counts in Lite, and the slicer and comparison loads in the viewer. This covers Active Queries, Current Waits and the Queries tab's three grids, in both apps. In the viewer, a note the banner carries for another reason, such as the hourly-tier note, still shows. (seen: the tests below)
  • One date format on every machine. The viewer's banners write the date on the invariant culture, as Lite's banner and the web's notice do. Otherwise a machine whose default calendar is Thai Buddhist prints 2026 as 2569. The grids keep the current culture on purpose. (seen: TheBanner_NamesItsInstants_OnTheInvariantCulture, under th-TH and fi-FI)

Cost

EXPLAIN

These are EXPLAIN (ANALYZE, BUFFERS) results for the exact statements, on a local TimescaleDB test store. The store holds 200 registered servers (50 with rows), 3.36M wait_stats rows in 36 chunks, and 707,040 query_stats rows with both hourly rollups. (seen)

Probe Execution Buffers (shared hit)
waiting_tasks, one server 0.352 ms 5
waiting_tasks, fleet 1.380 ms 2,803
wait_stats, one server 0.437 ms 5
wait_stats, fleet 6.045 ms 10,903
query_stats_hourly, one server 0.163 ms 14
query_stats_hourly, fleet 1.209 ms 1,763
Stitched hourly rollups, fleet 2.241 ms 3,526

The plan is a nested loop over servers, which is the only Seq Scan (200 rows, 2 buffers). Under it, a Limit sits over a ChunkAppend ordered by time, with an Index Only Scan per chunk (Heap Fetches: 0). Chunks after the one that holds a server's oldest row show "never executed". A window that ends 20 days back excludes 20 chunks at startup and runs in 0.761 ms. (seen)

Aggregate (actual time=1.303..1.305 rows=1.00 loops=1)
  Buffers: shared hit=2803
  ->  Nested Loop (actual time=0.023..1.297 rows=50.00 loops=1)
        ->  Seq Scan on servers s (actual time=0.008..0.020 rows=200.00 loops=1)
        ->  Limit (actual time=0.006..0.006 rows=0.25 loops=200)
              ->  Custom Scan (ChunkAppend) on waiting_tasks f
                    Order: f.collection_time
                    ->  Index Only Scan using <chunk>_idx_waiting_tasks_time on <chunk> f_1
                          Index Cond: (server_id = s.server_id)
                          Heap Fetches: 0
Planning Time: 0.599 ms
Execution Time: 1.380 ms

Added render time per panel

This is the client time of one probe call through Npgsql: the median and p90 over 25 calls, after 3 warm-up calls. (seen)

Probe Median p90
waiting_tasks, one server 0.74 ms 0.81 ms
waiting_tasks, fleet 1.59 ms 1.68 ms
wait_stats, one server 1.51 ms 1.82 ms
wait_stats, fleet 5.26 ms 5.93 ms
query_stats_hourly, one server 0.64 ms 0.81 ms
query_stats_hourly, fleet 1.29 ms 1.48 ms
Stitched hourly rollups, fleet 3.63 ms 4.13 ms
  • In the web runner, the probe runs after the panel's query, so its time adds to the render. (seen)
  • In the Darling viewer, the probe runs beside the tab's own read, so it adds time only when it is slower than that read. (inferred)
  • The first probe on a new connection also spends 24-81 ms planning while that connection's catalog cache warms. Later probes plan in 0.5-1.4 ms. (seen: EXPLAIN SUMMARY)
  • A store that keeps raw rows for months has more chunks to plan over. The measured store had 36 wait_stats chunks. Planning time on a store with many more chunks was not measured. (inferred)
  • One timing pass right after the rollups were refreshed showed a 36 ms median for the one-server rollup probe. It did not repeat: the re-run median was 0.64 ms. Work in the background after the refresh is the likely cause. (inferred)

The raw window floor

EXPLAIN (ANALYZE, BUFFERS) on the same store, for query_stats and a 7-day window, run warm: (seen)

Case Execution Buffers (shared hit)
Server with rows before the window 0.148 ms 6
Server added 3 days ago 0.946 ms 68
Window with no rows (ends 40 days back) 0.018 ms 10

Both halves are Index Only Scans on (server_id, collection_time). Chunks after the first one that answers show "never executed". Cold, right after a restart, the recent-server case took 11.4 ms, with 54 blocks read. (seen)

Lite's window floor

Medians in ms on a seeded Lite store: 29.3M rows, 40 servers, three monthly archive files and a 7-day hot table. The engine was Python DuckDB 1.4.4, not the engine Lite ships, so the absolute numbers for the app are inferred. Single cells moved 5 to 15 ms between runs. (seen)

Server Window Before Now
90 days of rows 7 days 6.4 16.1
90 days of rows 30 days 26.0 33.2
90 days of rows 85 days 63.3 61.2
Added 2 days ago 7 days 6.0 10.5
Added 2 days ago 85 days 5.5 10.2
Stopped collecting 7 days 5.2 4.5
Stopped collecting 85 days 34.2 44.3
  • The extra 4 to 10 ms is the LIMIT 1 look for an older row. Reading the oldest row directly cost 47.5 ms in the 90-day, 7-day case. (seen)
  • The probe runs once per grid refresh and once per MCP call. (seen)

The coverage probe on compressed chunks

The EXPLAIN and render tables above were measured on the oldest-row probe, before the coverage rule. These time the coverage probe on a test store with 50 servers. waiting_tasks had 6 of 9 chunks compressed, and query_stats 28 of 31, with segmentby = server_id. Each cell is planning / execution in ms, with shared-buffer hits where measured. (seen)

Case Compressed, cold Compressed, warm Uncompressed, cold
waiting_tasks, one server, 7 days 23 / 0.8 1.3 / 0.17 (5 hits) 67 / 1.5
waiting_tasks, fleet 1.0 / 1.7 (1,757 hits) 1.1 / 1.7 2.0 / 4.5
query_stats walk (no schedule edge), recent server 98 / 2.0 9.3 / 1.6 (60 hits) 188 / 2.2
query_stats walk, fleet 11.7 / 6.6 8.3 / 5.2 (5,357 hits) 4.1 / 8.7
  • A source with a schedule edge reads only the window: one index probe per server. The walk for a source with no schedule edge sits in the select list, so it runs only for a server that counts. (seen: the SQL)
  • collection_log was empty on that store, so the logged-run check was not timed. Lite's timings above were taken before the coverage rule, which adds one bounded read of v_collection_log for two tabs. (seen)

A change to an existing test

RunComposedPanel_OldWindow_AgainstPlainPostgres_RunsCleanOnRaw now scopes its panel to a server of its own. Without a scope, the panel reads the whole shared test store. Other tests' rows there start after the window's start, so without a scope the panel now gets the new notice.

The test registers its server in collect.servers and removes it afterwards through the shared cleanup helper. A scope that names a server the registry does not hold makes the probe read no table. The no-notice assertion then passes whatever the store holds. The point of the test is unchanged: the old window runs clean on raw, with no tier notice. (seen)

Tests

Failing first, before the change, 4 of 7 failed: (seen)

ComposeDataFloorLiveTests.AFleetPanel_StartsAtItsOldestServer_AgainstDevPostgres [FAIL]
  Assert.NotNull() Failure: Value is null
ComposeRollupDataFloorLiveTests.AnHourlyRoutedPanel_TakesItsStartFromTheHourlyRollup_AgainstDevPostgres [FAIL]
  Assert.NotNull() Failure: Value is null
ComposeDataFloorLiveTests.AThirtyDayPanel_OverSevenDaysOfRows_SaysWhereTheDataStarts_AgainstDevPostgres [FAIL]
  Assert.NotNull() Failure: Value is null
ComposeDataFloorLiveTests.TheRenderedPanel_ShowsWhereTheDataStarts_AgainstDevPostgres [FAIL]
  Assert.Single() Failure: The collection was empty

The Darling viewer checks, before the viewer change, 3 failed: (seen)

ViewerDataStartBannerTests.BothReads_GoThroughTheSharedProbe [FAIL]
  Assert.Contains() Failure: Sub-string not found
ViewerDataStartBannerTests.ActiveQueries_ShowsWhereTheSnapshotsStart_OnEveryReadOfTheGrid [FAIL]
  Assert.Equal() Failure: Values differ (Expected: 3, Actual: 0)
ViewerDataStartBannerTests.CurrentWaits_ShowsWhereTheWaitingTasksStart [FAIL]
  Assert.Contains() Failure: Sub-string not found

What the live tests cover (each test makes and drops its own database):

  • AThirtyDayPanel_OverSevenDaysOfRows_SaysWhereTheDataStarts: a 30-day window over 7 days of rows gets the notice.
  • TheRenderedPanel_ShowsWhereTheDataStarts: the web page renders the notice. The panel's answer goes through the real compose.js in Node (web-compose-notice-harness.mjs).
  • AWindowTheRowsCover_GetsNoNotice: a window inside the rows gets no notice.
  • AQuietStartInsideTheWindow_GetsNoNotice: a server with an older row before a quiet stretch gets no notice.
  • AFleetPanel_StartsAtItsOldestServer: a fleet panel starts at its oldest server.
  • AFleetPanel_StoppedServersOldRows_DoNotHideALateStart: a server stopped before the window does not hide a newer server's late start.
  • AnIdleStart_WithNoOlderRow_OnAServerMonitoredForMonths_GetsNoNotice: a server collected for months, idle at the window's start, with no older row left, gets no notice.
  • AFirstWaitThreeDaysAgo_OnAServerCollectedForThirtyDays_GetsNoNotice: a server collected for 30 days whose first waiting task came 3 days ago gets no notice.
  • AServerAddedTwoDaysAgo_SaysSinceItsFirstCollection_NotItsFirstRow: a server added 2 days ago gets the notice, and it names the first collection, not the first row.
  • ABaselineFlooredTable_ForAServerOlderThanTheWindow_StillSaysWhereItsOldestRowIs: a table the schedule gives no edge still names its oldest row.
  • AnHourlyRoutedPanel_TakesItsStartFromTheHourlyRollup: this one needs TimescaleDB. Raw rows are trimmed to 3 days and the hourly rollup keeps 10. The notice names the rollup's start, not raw's.
  • TheViewerProbe_ForOneServer_AnswersWhereItsCoverageStarts: the Darling viewer's one-server probe.

Two planted failures, each reverted afterwards: (seen)

  • A lower bound at the window's start turns AQuietStartInsideTheWindow_GetsNoNotice red: Assert.Null() Failure: Value is not null.
  • Probing raw instead of the routed rollup turns AnHourlyRoutedPanel_TakesItsStartFromTheHourlyRollup red: Not found: "2026-09-22 10:00 UTC".

The raw window floor, before the change, 3 of 10 failed: (seen)

RawWindowFloorQuietStartLiveTests.AQuietStartInsideTheWindow_IsNotACut_AgainstDevPostgres [FAIL]
  Assert.Equal() Failure: Values differ (the floor was the first row inside the window, 2 days late)
RawWindowFloorEffectiveStartTests.AFloorBeforeTheStart_ServesTheWholeWindow [FAIL]
  Assert.Equal() Failure: Values differ (the served start was a day before the requested start)
RawWindowFloorEffectiveStartTests.NoMcpTool_ComputesTheServedStartByHand [FAIL]
  Assert.DoesNotContain() Failure: Sub-string found

A planted change that drops the existence check turns AServerWhoseRowsEndBeforeTheWindow_ReadsAsNothing red (Assert.Null() Failure: Value of type 'Nullable<DateTime>' has a value). It was reverted. (seen)

The UTC session pin was tested on a store whose server runs in another time zone. Before the change, 11 of 17 failed: the new LiveStoreSessionTimeZoneTests.ALiveTestSession_RunsInUtc_AgainstDevPostgres, 9 in FleetOverviewAzureMasterScopeLiveTests and 1 in DailySummaryAzureMasterScopeLiveTests. After it, 35 of 35 passed on that store. (seen)

Lite, before the change, QueryWindowTruncationTests ran 17 tests and 3 failed: the quiet-start guard, the window-inside-the-rows guard and the MCP clamp guard. (seen)

A planted lower bound on the older-row look (AND collection_time >= $2) turns the quiet-start guard red again, with this line: floor 2026-10-02T16:20:24.7685560 must be the requested start 2026-09-25T18:20:24.7685565Z (the server holds a row before it), not the window's first row. It was reverted. (seen)

What the Lite tests cover: a quiet start with older rows gets the requested start and no banner (for query_stats and waiting_tasks). A server whose rows all end before the window gets null. A range that starts before the oldest stored row is cut at that row. That holds in the probe and in the Active Queries and Current Waits banner text. They also cover a row exactly on the window's start, and another server's older rows, which do not count. (seen)

The first CI run on the merged branch failed two tests that this PR broke. Both are fixed. (seen)

  • StoreSessionTimeZoneLiveTests.AStoreConnection_PinsToUtc_EvenWhenTheDatabaseDefaultIsAmericaNewYork expected the database's own America/New_York default from the scratch store's unpinned connection, and got UTC. The UTC pin that this PR adds to the test store's connection string reached that unpinned string too. ScratchPostgres now drops the pin from it.
  • DisplayZoneTextFrameTests.TheRefreshHeaderAndTheTruncationBanner_UseTheTabsOwnClockAndZone looked for the banner call's old text. The pin now follows the shared step, ApplyWindowFloorToBanner(banner, floor, startUtc, GetPickerZone()). A new check, EveryWindowFloorBannerCall_IsHandedTheTabsPickerZone, requires every call to hand the banner the tab's own zone. A planted TimeZoneInfo.Local turned both tests red. It was reverted.

DataWindowFloorTests checks the SQL shape, which tables and rollups are accepted, each route's source list, and the notice text. These checks were written after the code, so they did not fail first.

The coverage rule

Failing first: the new and changed test files, run against the product code at 1337f701f. Two calls went back to the old signature, so the copy compiled. (seen)

  • Darling: 44 run, 17 failed. Of those, 13 are DataWindowFloorTests shape pins and 4 are ComposeDataFloorLiveTests, among them AnIdleStart_WithNoOlderRow_OnAServerMonitoredForMonths_GetsNoNotice_AgainstDevPostgres and AFirstWaitThreeDaysAgo_OnAServerCollectedForThirtyDays_GetsNoNotice_AgainstDevPostgres.
  • Lite DataStartBannerTests: 10 run, 3 failed, among them CurrentWaits_IdleStart_WithNoOlderRow_OnAServerMonitoredForMonths_ShowsNoBanner and FirstRowThreeDaysAgo_OnAServerCollectedForThirtyDays_ShowsNoBanner.
  • The test for a source with no schedule edge passes at 1337f701f, because its edge is the rule before this change.

A failed probe, the probe's arguments, and the banner's culture

Failing first: each new pin ran against a temporary revert of its fix, and then the file was restored byte for byte. (seen)

  • Viewer, 5 failed:
    • AProbeThatThrows_HidesTheBanner_IsLogged_AndLetsTheLoadGoOn.
    • ActiveQueries_RangeLoad_AsksAboutTheToolbarWindow_AndComparesWithItsStart, with the probe's start and end swapped.
    • ActiveQueries_DeepLink_AsksAboutThePendingWindow_AndComparesWithItsStart, with the toolbar's start in place of pending.FromUtc.
    • TheBanner_NamesItsInstants_OnTheInvariantCulture, under th-TH and under fi-FI.
  • Lite, 1 failed: AProbeThatThrows_HidesTheBanner_IsLogged_AndLetsTheRefreshGoOn.
  • The viewer's call-site pins now check each probe's window and each banner's start, not only the number of calls.
  • ComposeDataFloorLiveTests now reports a skip, not a pass, when Node is not installed.
  • The viewer's Queries tab, at 3b06e9b7a: 30 run, 8 failed. They are the six new call-site pins (three range loads, three slicer drags), the extended NoLoadAwaitsItsProbeBare_AndTheDefaultLogIsTheViewersOwn, and the changed Query Store pin in RawWindowFloorViewerPortTests. (seen)

A window wholly before coverage

Failing first, at 8c46cb58b: (seen)

  • ComposeDataFloorLiveTests: 14 run, 2 failed.
    • AWindowWhollyBeforeTheRetentionEdge_WithLoggedRuns_GetsNoNotice got a notice that named the purge edge, a week after the window's end.
    • TheProbes_AnswerNothing_ForAWindowWhollyBeforeTheRetentionEdge got that edge from the probe instead of null.
  • AWindowStraddlingTheRetentionEdge_NamesTheEdge_AndCoversToTheWindowsEnd passes before and after. It checks that the fix does not answer null too often.
  • RawWindowFloorEffectiveStartTests.NoMcpTool_ComputesTheServedStartByHand now also fails on a private 90-minute tolerance, or on a floor compared with requestedStart +. It failed on the clutter tool before the fix, and again with the old line planted back.

Stitched routes, the tool's text, and the Active Queries banner

Each check below failed first. (seen)

  • The stitched route. The successor's lower bound was disabled in DataStartSources, which gives the probe as it was. Then the new AStitchedHourlyPanel_StartsAtTheStitchBoundary_WhenTheSuccessorWasMaterializedBackPastIt_AgainstDevPostgres fails with Assert.NotNull() Failure: Value is null: no notice. With the bound, it passes, and the notice names the boundary.
  • The FROM clause. With the day-grain route's clause removed, TheDayGrainDailyRoute_CarriesItsFromClause_SoTheDataStartProbeReadsIt and EveryRollupRoute_CarriesTheFromClauseItReads fail: Expected: "collect.query_store_stats_daygrain_daily AS f", Actual: null. With the clause, both pass.
  • The tool's text. Before the change, McpToolGuideHeadsCustomViewTests ran 3 tests and 2 failed. The new TheNoticeContract_NamesAllThreeCauses_InTheHeadAndInTheTail failed with Not found: notice is a partial-window caveat. The served head of the description went from 612 to 609 characters, inside its budget. Two facts it dropped, the server scope and the 90-day limit, stay in the tail, and two new pins keep them there.
  • The existing test. With one query_stats row planted 1 hour back for the test's own server, RunComposedPanel_OldWindow_AgainstPlainPostgres_RunsCleanOnRaw fails Assert.Null with the partial-window notice. With the plant removed, it passes.
  • The live and latest reads. Before the change, Lite's LiveSnapshot_HidesTheActiveQueriesBanner_AsTheLiveRowsAreBound_AndNotWhenTheReadFails failed (11 run, 1 failed). The viewer's LatestSnapshot_HidesTheActiveQueriesBanner_AsTheBatchIsBound_AndNotWhenTheReadFails failed too (19 run, 1 failed).
  • The drill-downs. Before the change, Lite's ActiveQueriesDrillDowns_RefreshTheBannerForTheDrillWindow_AfterTheRowsAreBound failed both its cases. The viewer's ViewerDataStartBannerTests ran 20 tests and 2 failed: the new ActiveQueries_WindowDrill_AsksAboutTheDrillWindow_AndComparesWithItsStart, and the count pin. That pin goes from 3 calls to 4 on purpose.
  • Three new test files gained the project's copyright header. No test covers headers.

Totals

  • Darling.Tests, the full suite, once at the Darling-side head: Total 19902, Failed 11, Skipped 98, Not Run 1. (seen) One failure was a missing #1776 own-store comment on ComposeDataFloorLiveTests, which LivePostgresCollectionHygieneTests requires. Ten were in FleetOverviewAzureMasterScopeLiveTests (9) and DailySummaryAzureMasterScopeLiveTests (1), on a test store whose server runs in another time zone. This PR fixes both. The comment is in place, and the live tests' connection now sets Timezone=UTC, as the product's store connections do (DarlingStoreConnection.cs:58). A re-run of those classes and this change's live classes passed: Total 35, Failed 0. (seen)
  • The raw window floor classes passed 129 of 129, and their neighbours 137 of 137. (seen)
  • Lite.Tests, the four changed classes: 37 passed, 0 failed. Four source-scanning census classes: 46 passed. Lite builds with 0 warnings. (seen)
  • The coverage rule, targeted: Darling.Tests 58 run, 0 failed (DataWindowFloorTests, ComposeDataFloorLiveTests, ComposeRollupDataFloorLiveTests, ViewerDataStartBannerTests, RawWindowFloorQuietStartLiveTests, RawWindowFloorEffectiveStartTests). Lite.Tests 40 run, 0 failed (DataStartBannerTests, DisplayZoneTextFrameTests, QueryWindowTruncationTests). Darling, Lite and the Dashboard build with 0 warnings. (seen)
  • The failed-probe, argument and culture fixes, targeted: Darling.Tests 362 passed, 0 failed (ViewerDataStartBannerTests, RawWindowFloorViewerPortTests, DataWindowFloorTests, DarlingComposeTests). ComposeDataFloorLiveTests passed 11 of 11, none skipped, so Node ran. Lite.Tests 54 passed, 0 failed (DataStartBannerTests, DisplayZoneTextFrameTests, DataStartBannerProbeFailureTests, QueryWindowTruncationTests, ComparisonWindowUtcTests, ServerTabCapabilityPinTests). Darling, Lite and the Dashboard build with 0 warnings. (seen)
  • The Queries tab's probes, targeted: Darling.Tests 122 run, 0 failed (ViewerDataStartBannerTests, RawWindowFloorViewerPortTests, QueryStoreClutterViewerSurfacesTests, ViewerSlicerMetricMapTests, DocCommentHygieneTests). Darling, Lite and the Dashboard build with 0 warnings. (seen)
  • The window-end and clutter fixes, targeted: Darling.Tests live, 19 run, 0 failed (ComposeDataFloorLiveTests 14, RawWindowFloorQuietStartLiveTests 3, QueryStoreClutterLivePostgresTests 2). Non-live, 336 run, 0 failed, among them DataWindowFloorTests, RawWindowFloorEffectiveStartTests, ViewerDataStartBannerTests, QueryStoreClutterTests and McpPayloadContractCensusTests. Darling, Lite and the Dashboard build with 0 warnings. (seen)
  • CI on 1337f70: every Darling and Lite test job passed. (seen)
  • The stitched route and the FROM clause, targeted: Darling.Tests 509 passed in the final run. An earlier run of the stitch and census classes passed 135. None failed or skipped. Darling.Tests, Lite.Tests and the Dashboard build with 0 warnings. (seen)
  • The tool's text and the existing test, targeted: three Darling.Tests runs passed 18, 342 and 374 tests, 0 failed. They covered the pin, budget and guide classes, the compose and custom-view tool classes, and the compose.js readers, census and latency classes. Darling.Tests, Lite and the Dashboard build with 0 warnings. (seen)
  • The live and latest reads, targeted: Lite.Tests 99 passed, 0 failed. Darling.Tests 30 passed, 0 failed, 4 skipped. The source-scanning classes passed 163. (seen)
  • The drill-downs, targeted: Lite.Tests 82 passed across 8 classes. Darling.Tests 109 passed (ViewerDataStartBannerTests, DocCommentHygieneTests, RawWindowFloorViewerPortTests). A wider run of 404 tests across about 24 related classes had 0 failed and 2 skipped, which need a database. Darling.Tests, Lite.Tests, the viewer, Lite and the Dashboard build with 0 warnings. (seen)
  • CI on 997ed0f: every Darling and Lite test job passed. (seen)
  • CI on 4994a8d: every Darling and Lite test job passed. (seen)
  • CI on c857763: every Darling and Lite test job passed. (seen)

CHANGELOG

No entry in this PR; the release owner adds it under [Unreleased].

Proposed entry:

Active Queries, Current Waits and Custom Views panels say where their data starts, and the Queries tab stops showing it for a quiet start.

… data starts

A window can reach further back than the rows the store holds: retention,
a held purge, or a server added recently. A Custom Views panel now carries
a notice naming where its data starts and the window it really covers, and
the Darling viewer's Active Queries and Current Waits tabs show the
"Showing since" banner the Queries tab already shows.

The start is the oldest row each server in scope holds at or before the
window's end, read per server through the registry. A panel read from a
rollup takes its start from that rollup.
Comment thread Darling/PerformanceMonitor.Darling.Service/DarlingWebEndpoints.cs
Comment thread Darling/PerformanceMonitor.Darling.Viewer/ViewerServerTab.Blocking.cs Outdated
Comment thread Darling/PerformanceMonitor.Darling.Storage/DataWindowFloor.cs Outdated
@claude

claude Bot commented Oct 2, 2026

Copy link
Copy Markdown

Blind-read notes the verdict block can't hold at length

The Lite parity question, checked rather than taken on trust

The two-store rule is the first thing this diff looks like it trips — it adds a disclosure to two Darling viewer surfaces over query_snapshots and waiting_tasks, and Lite/Services/ScheduleManager.cs:738 / :751 seed both of those at RetentionDays = 7, identical to Darling. Lite also already carries the whole #4231 apparatus: SetWindowTruncatedBanner (Lite/Controls/ServerTab.Refresh.cs:510), RefreshWindowTruncatedBannerAsync (:493), the three-relation probe (Lite/Services/LocalDataService.QueryWindowFloor.cs:62), an Active Queries tab (Lite/Controls/ServerTab.xaml:359) and a Current Waits tab (:1707). On that reading it is a fix to one SKU without its counterpart.

It isn't, and the PR body's account of why is accurate. Confirmed from the tree:

  • Lite/Services/RetentionService.cs:26-34 — "Lite does not purge per collector: hot rows leave DuckDB for Parquet after the archive service's hot-data week, the v_* views union live and archive so every read sees both", with ArchiveRetentionMonths = 3 as Lite's one horizon. The seeded retention_days = 7 is vestigial in Lite; nothing applies it as a DELETE (a sweep of Lite/ for retention_days finds only the schema default at Lite/Database/Schema.cs:41, the model, and MCP prose).
  • Lite/Database/DuckDbInitializer.cs:707-713 — ArchivableTables is catalog-derived over every stored collector target, so query_snapshots and waiting_tasks both get a v_ view unioning hot ∪ Parquet.
  • Lite's reads go through those views: LocalDataService.Blocking.cs:171, :246, :348, :455 (v_query_snapshots); LocalDataService.WaitingTasks.cs:55, :105, :143, :219 (v_waiting_tasks).

So a 30-day range in Lite over a 7-day table really does hold rows, and a banner there would have been the false alarm. The body's own disclosure of what stays open is also correct and worth keeping in view for the follow-up:

A range older than the archive is still cut short with no banner, and so is a range older than a new server's first row. … Lite's existing banner (LocalDataService.GetQueryWindowFloorAsync) looks for the start inside the window. On sparse tables that raises a false banner, so it cannot be reused as it is.

That last sentence is the load-bearing one: LocalDataService.QueryWindowFloor.cs:68-73 is bounded on both sides (collection_time >= $2 AND <= $3), which is exactly the shape DataWindowFloor.cs:27-32 argues is wrong for a sparse table. Lite's follow-up needs the unbounded-below variant, not a fourth QueryWindowRelation on the existing probe — and LocalDataService.QueryWindowFloor.cs:54-58 explicitly justifies the upper-and-lower bounding as what lets DuckDB prune row groups and skip Parquet files, so that is a real trade to make deliberately rather than by copying Darling's shape across.

Cost

The EXPLAIN and client-timing tables are the kind of evidence a read-only tree cannot grade, and they are the right evidence to have gathered. Two things a human with the rig should still look at, both flowing from the deliberate choice to drop the lower bound:

  • The rig's store has 200 registered servers, 50 with rows. The 150 server rows that hold nothing are the expensive population here, not the 50: with no lower bound there is no chunk to prune, so proving "this server has no row at or before the end" has to touch the ordered ChunkAppend for every chunk the relation has. The posted plan's Limit (actual … rows=0.25 loops=200) and 2,803 buffers for a 36-chunk waiting_tasks is consistent with that being cheap at this chunk count — but the number that scales is chunks × empty servers, and a 90-day hourly rollup at 10-day materialization chunks on a 300-server fleet is a different point on that curve than the one measured.
  • The compose rollups are created materialized_only = false (TimescaleSupport.cs:626, :653), so a probe on a CAGG view has the real-time branch in the plan as well as the materialization. The posted rollup rows (query_stats_hourly, fleet: 1.209 ms / 1,763 buffers) suggest it is not hurting, but the plan excerpt shown is the raw waiting_tasks one, so the CAGG plan's shape — whether the live branch is doing a sorted GroupAggregate over raw to produce its own minimum bucket — isn't in evidence.

Neither is a reason to hold the change; both are worth a glance before this probe lands on the 30-day desktop tabs the body already names as the next step, where it would run on many more surfaces.

What the new tests pin, and what they don't

ComposeDataFloorLiveTests takes one DateTime.UtcNow and derives end, recentFirst and quietFirst from it (:259-262), so it does not trip #3676's two-clock-reads shape; the 90-minute IsTruncated slack absorbs the drift to the endpoint's own DateTime.UtcNow. The two planted failures the body reports are the right two — a lower bound reddening AQuietStartInsideTheWindow_GetsNoNotice is precisely the assertion that would otherwise pass for the wrong reason.

Not covered by anything: the queryStoreWideEligible arm of the new gate at DarlingWebEndpoints.cs:1322. Every new test drives a waiting_tasks or query_stats panel, so no test enters RunComposedPanelCoreAsync with the wide table serving and confirms the probe is skipped and query_store_history_note is the only start the payload reports. The claim is right by reading, and it is one line — but it is the line that decides whether a Query Store panel says where its data starts twice, in two different formats.

One pin that now passes by construction

RunComposedPanel_OldWindow_AgainstPlainPostgres_RunsCleanOnRaw's Assert.Null(outcome.Payload["notice"]) arm is now satisfied before any judgement runs: the panel is scoped to compose-old-window-no-rows, so DataWindowFloor.GetAsync returns null and BuildDataStartNoticeAsync returns null at ComposeStoreAvailability.cs:351 without comparing anything. The docstring is honest about this ("The panel is scoped to a server that holds no rows"), and the test's real claims — collect.query_stats, no _hourly, no _daily — still fail on real routing drift, so this is a note rather than a finding. It is worth knowing that the no-notice half of that test is now a statement about the null-floor path and no longer about "raw is complete on a plain-PostgreSQL store".

claude[bot]
claude Bot previously requested changes Oct 2, 2026

@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.

Verdict: CHANGES REQUESTED

Full reasoning in the top-level comment and three inline notes; this is the structured summary.

Claims

  • VERIFIED — "the run_custom_view_panel MCP tool uses the same runner and gets the same notice" — the probe is inside RunComposedPanelCoreAsync (DarlingWebEndpoints.cs:1322-1326), whose only callers are the web endpoint (:604) and the tool (Mcp/DarlingMcpCustomViewTools.cs:287).
  • VERIFIED — the notice's exact wording, and "the start always comes from the rows, never from the retention defaults" — ComposeStoreAvailability.cs:358-361, pinned byte-for-byte by DataWindowFloorTests.TheNotice_NamesTheStartAndTheWindowItCovers_InUtc; the value comes from DataWindowFloor.GetAsync at :344.
  • VERIFIED — "query_snapshots and waiting_tasks keep 7 days" — CollectorScheduleDefaults.cs:45, :71.
  • VERIFIED — banners on all three Active Queries reads and above both Current Waits charts — ViewerServerTab.ActiveQueries.cs:82/:93/:114 (counted by ViewerDataStartBannerTests.ActiveQueries_ShowsWhereTheSnapshotsStart_OnEveryReadOfTheGrid) and Blocking.cs:188, fan-out 2→3 at :175; XAML rows consistent at ViewerServerTab.xaml:690-711, styling matching the #4231 siblings at :906-909.
  • VERIFIED — the four "no change needed" rows — no compose renderer under the viewer; MAX(collection_time) anchor at ViewerDataService.RunningJobs.cs:43-44; DrillWindowUtc at ViewerServerTab.DrillDown.cs:329-336.
  • VERIFIED — "a removed server still counts … removal deletes only the server's definition" — BaselineCache.cs:58, and Mcp/DarlingMcpTools.cs:1403 names config.config_monitored_servers as remove_server's target, so the collect.servers row survives.
  • VERIFIED — the whole Lite row, the claim I most expected to break — RetentionService.cs:26 ("Lite does not purge per collector"), :34 (3 months), ArchiveService.cs:40 (7 hot days), DuckDbInitializer.cs:707-713, the union-view reads at LocalDataService.Blocking.cs:348 / WaitingTasks.cs:105, and Lite's both-sides-bounded probe at LocalDataService.QueryWindowFloor.cs:68-73.
  • VERIFIED — "runs only when the tier notice is silent, not when the wide table served, and a failed probe costs no chart", the reused 90-minute slack, and the index_object_stats exclusion — DarlingWebEndpoints.cs:1322; ComposeStoreAvailability.cs:346-349, :351; RawWindowFloor.cs:109-110; PgSchemaGenerator.cs:112 failing the gate at DataWindowFloor.cs:83.
  • UNVERIFIED — the EXPLAIN table, plan excerpt, 25-call timing medians and the 708.6 ms comparison — live measurements; no rig, no 3.36M-row store, no planner here.
  • UNVERIFIED — "4 of 7 failed" first, the three viewer-check failures, the two planted-and-reverted failures — needs DARLING_TEST_PG + TimescaleDB and a revert-and-rerun; every test body is consistent with what is reported.
  • UNVERIFIED — the Node render test — needs Node. Consistent on read: el() sets node.className for class (util.js:30), matching the harness's "strip notice" walker against noticeStrip (util.js:136); compose.js:23-26 imports exactly the four modules stubbed.
  • UNVERIFIED — "Total 19902 … Failed 11" and the clean 35-test UTC re-run — a build and a live store.
  • REFUTED — nothing the body asserts; the gap is one it does not assert (see Divergence).

Riskiest lines

  1. Darling/PerformanceMonitor.Darling.Storage/DataWindowFloor.cs:166 — the end bound and the absent start bound: the whole false-alarm/missed-alarm hinge, and what forfeits chunk pruning. — pinned by DataWindowFloorTests.ThePanelProbe_BoundsOnlyTheEnd and ComposeDataFloorLiveTests.AQuietStartInsideTheWindow_GetsNoNotice_AgainstDevPostgres
  2. Darling/PerformanceMonitor.Darling.Service/DarlingWebEndpoints.cs:1322 — retentionNotice is null && !queryStoreWideEligible: wrong one way every panel buys a needless store read, wrong the other a Query Store panel reports its start twice in two formats (:1337-1340). — UNPINNED
  3. Darling/PerformanceMonitor.Darling.Service/Compose/ComposeStoreAvailability.cs:346 — the blanket catch: a 42P01 and a 57014 are swallowed identically, so a probe that never succeeds looks like a store with nothing to report. — UNPINNED

Traps near this diff

  • "A retention-tiered store answers a 7-day question from a 4-day raw tier and labels the missing days quiet unless the read routes by tier and says which tier served it (#3590 … #3684)." — applies, and is the shape being fixed — closed at DarlingWebEndpoints.cs:1324, ActiveQueries.cs:93, Blocking.cs:188.
  • "Lite and Darling ship in parity … a fix to one SKU's reader, tool, alert or description without the counterpart's is a finding, not a nit." — does not apply — nothing under Lite/ references RunComposedPanel/ComposeStoreAvailability, and Lite's two tabs read the archive-union views (LocalDataService.Blocking.cs:348, WaitingTasks.cs:105; DuckDbInitializer.cs:707-713) because RetentionService.cs:26 says Lite does not purge per collector.
  • "Two copies of one definition pinned EQUAL to each other fail only after BOTH have drifted (#3684). Alias, do not copy." — applies narrowly, non-blocking — the live path aliases route.CaggFromClause (ComposeStoreAvailability.cs:380), but its null-fallback re-derives ComposeCompiler.cs:185's string; the router always sets the field (ComposeSourceRouter.cs:350-362), so it is dead code holding a second copy.
  • "Truncation inferred from a full page: truncated = rows.Count >= limit." — does not apply — the verdict is a separate probe against the window's start (ComposeStoreAvailability.cs:351), never read off rows.
  • "Strip the DateTimeKind before binding a timestamp parameter … the row lands at the wrong hour with no error anywhere." — applies, and is honored — DataWindowFloor.cs:226-227, reason at :224-225; seeders likewise at ComposeDataFloorLiveTests.cs:295-296.
  • "A test that reads the clock twice measures the CI scheduler (#3676). One anchor owns both ends of an interval." — does not apply — one DateTime.UtcNow at ComposeDataFloorLiveTests.cs:259 owns end, recentFirst and quietFirst.
  • "A pin that encodes the lie … Ask what a pin would fail ON." — applies narrowly, non-blocking — DarlingComposeTests.cs:3529's Assert.Null(notice) is now satisfied by construction (ComposeStoreAvailability.cs:344 answers null first); disclosed in the docstring, and the routing arms at :3526-3528 still bite.
  • "A roster-based census does not see the file it does not roster (#3699)." — does not apply — the new file calls the raw ReadRepoFile and RepoFileAdoptionTests' exact-set equality covers only LF adopters (:299-301); raw adopters carry a floor (:269-273), and its anchors are single-line.
  • "timescaledb_information.hypertables never lists a continuous aggregate's materialization (#3582 → #3610)." — does not apply — the probe reads each rollup's own user view (DataWindowFloor.cs:164).

Divergence (blind read vs PR body)

One, an omission rather than a disagreement. The body says "run_custom_view_panel MCP tool | Same runner, so it gets the same notice" — true — but never that the tool's declared semantics for notice had to move with it, and did not. Mcp/DarlingMcpCustomViewTools.cs:246-247 still reads "notice means retention covered only part of the window, or the row cap truncated the result; absent means neither happened"; the GUIDE half at :252 still scopes it to retention. This diff makes both halves false and says so itself — ComposeStoreAvailability.cs:327, added here, is explicit that the notice "names no cause: retention, a purge that ran late, and a server added last week truncate the same way." A panel over a server registered last week now returns notice with neither retention nor the row cap involved. That description is the only contract an agent has here, so the drift becomes an agent telling an operator their retention is too short when the cause is a young install. The web surface is fine — the notice body names itself.

Elsewhere the accounts agree, including where I expected them not to: the Lite parity row (correct from the tree, residual gaps disclosed) and the index_object_stats exclusion. Secondary non-blocking: compose.js:260's comment is now narrower than reality, and ViewerReadFanOut.Of(2) is not declared around the new two-read concurrency in LoadActiveQueriesAsync — which matches the #4231 precedent at ViewerServerTab.Queries.cs:176-177, so consistency rather than regression.

Reviewer checklist

  • Update run_custom_view_panel's description (Mcp/DarlingMcpCustomViewTools.cs:246-247, :252) so notice covers the data-start cause and "absent" claims nothing about retention; compose.js:260 with it.
  • Drive a Query Store panel through the wide-table path on a live store and confirm DarlingWebEndpoints.cs:1322's !queryStoreWideEligible arm suppresses the probe, so the start is reported once.
  • Re-run the fleet EXPLAIN with the empty-server population dominant and a 90-day hourly rollup at 10-day chunks — unbounded-below cost scales as chunks times servers-with-no-rows. Capture a CAGG plan too: the rollups are materialized_only = false (TimescaleSupport.cs:626).
  • Run the gated live classes with DARLING_TEST_PG on a UTC-session TimescaleDB store, plus the Node harness, and build the WPF viewer.
  • Open the viewer on a range entirely before where query_snapshots/waiting_tasks begin and decide whether an empty grid with no banner is acceptable (the probe answers null; IsTruncated(null, …) is false).

…the store session to UTC

The raw window floor behind the Queries tab's "Showing since" banner and the
MCP tools' window_truncated is now the oldest row at or before the window's
end, the way Custom Views panels find where their data starts. A server
quiet for the first hours of a window no longer reads as cut short. The
floor stays null when the window holds nothing, and the served start never
precedes the requested one.

The test process applies the product's own UTC session pin to
DARLING_TEST_PG when it loads, so a test store whose server runs in another
time zone no longer fails tests that plant rows relative to UTC now.
…e the window's end, so a quiet start inside the window raises no banner

The probe read the oldest row INSIDE the window, so a server with older rows and
then a quiet stretch at the window's start looked like a store that failed to
hold the window. It now returns NULL when the server has no row inside the window
(a bounded existence check, cheap with zone maps), and otherwise the oldest row at
or before the window's end, unbounded below. The three MCP tools clamp the
effective start to the requested start, so a floor before the window never makes
the window look longer than it was asked for.
A window or custom range that reaches further back than the stored rows (the
archive's first month, or a server added recently) now shows the same 'Showing
since' banner on Active Queries (query_snapshots) and Current Waits
(waiting_tasks) that the Queries grids carry, through the one shared floor probe
and one banner step. The probe's relation list gains the two views; the three
call-site count pins move 3 to 4 and 6 to 10 for the new calls.
Comment thread Darling/PerformanceMonitor.Darling.Storage/RawWindowFloor.cs
Comment thread Darling/PerformanceMonitor.Darling.Service/Compose/ComposeStoreAvailability.cs Outdated
Comment thread Darling/Darling.Tests/DarlingComposeTests.cs Outdated
@claude

claude Bot commented Oct 2, 2026

Copy link
Copy Markdown

Lite/Darling parity: the diff changes the Darling half of a probe whose own doc calls Lite's "the twin"

The body's surface table says:

Lite custom range | Not changed. Lite's v_ views read the 3-month archive as well as the live tables … Lite's existing banner (LocalDataService.GetQueryWindowFloorAsync) looks for the start inside the window. On sparse tables that raises a false banner, so it cannot be reused as it is. (seen)

Every factual sentence there is true, and I confirmed each from the tree. But the reason given is scoped to reusing Lite's probe for the new sparse surfaces — and that is not the parity question this diff raises. The diff also rewrote RawWindowFloor, which serves Lite's three dense tables' exact twins, and the body has no row for that at all.

What the two SKUs now say about identical data, on the same three tables:

Darling, after this PR Lite, unchanged
probe SQL collection_time <= $3 ORDER BY collection_time LIMIT 1, plus a windowed EXISTS (RawWindowFloor.cs:68-87) MIN(collection_time) … >= $2 AND <= $3 (Lite/Services/LocalDataService.QueryWindowFloor.cs:68-73)
served start RawWindowFloor.EffectiveStart clamps a floor below the start (RawWindowFloor.cs:120) floor ?? requestedStart (Lite/Mcp/McpQueryTools.cs:82, 215, 344)
tables query_stats, procedure_stats, query_store_stats (RawWindowFloor.cs:49-54) v_query_stats, v_procedure_stats, v_query_store_stats (:29-35)
tools get_top_queries_by_cpu, get_top_procedures_by_cpu, get_query_store_top the same three (Lite/Mcp/McpQueryTools.cs)
grid banner UpdateTruncationBanner (ViewerServerTab.Queries.cs:229) SetWindowTruncatedBanner(banner, truncated, floor ?? startUtc, …) (Lite/Controls/ServerTab.Refresh.cs:497)

The two files already name each other as one definition in two places:

  • Lite/Services/LocalDataService.QueryWindowFloor.cs:42-43 — "Twin of Darling's DarlingDataReader.GetQueryStoreWindowFloorAsync (get_query_store_top reports a window it cannot serve: raw query_store_stats is dropped at 4 days #2364), generalized to Lite's three raw tables"
  • Lite/Services/LocalDataService.QueryWindowFloor.cs:54 — "Bounded on both sides of the window. An unbounded MIN would scan back to the start of retention…" — i.e. Lite's file still documents, as a design decision with a cost argument behind it, the exact shape Darling just abandoned.

REVIEW-TRAPS, last line, quoted whole:

Lite and Darling ship in parity (CONTRIBUTING's two-store rule): a fix to one SKU's reader, tool, alert or description without the counterpart's is a finding, not a nit — the census in McpPageContractTests compares the two SKUs' payload keys by tool name for exactly this (#3594).

And the parenthetical is why nothing went red: McpPageContractTests compares payload keys by tool name. window_truncated and effective_start are present on both sides with the same spelling and the same types; only their meaning diverged. EveryViewerFanOut_DeclaresItsWidth and the T-SQL guards are likewise blind to it. The body's "Lite is not touched, so Lite.Tests did not run" is accurate and is exactly the gap: the suite that would have had a chance never executed.

Note the direction, too. Lite's v_ views reach the 3-month parquet archive (ArchiveService.cs), which the body correctly cites — but that makes Lite's bounded-below MIN more exposed to the shape, not less: the deeper the archive reaches behind the window, the more often a floor found inside the window is a hole rather than the archive's edge.

Pick one and say so in the body:

  1. Hold RawWindowFloor as it was and let DataWindowFloor own the sparse surfaces it was written for. The new feature needs nothing from the rewrite — the compose runner (DarlingWebEndpoints.cs:1324) and both new viewer banners already go through DataWindowFloor. Parity is preserved by not breaking it, and DataWindowFloor.cs:27-28 stops contradicting RawWindowFloor.cs:30-37.
  2. Carry the change across, giving Lite the same probe shape and the same EffectiveStart clamp, with a row in the surface table for the four Darling MCP tools and three Queries-tab banners whose output this changes.

Either is a decision for you; what is not available is the current state, where one SKU's window_truncated answers "does the store hold anything older" and the other answers "what did this window hold", under one name.

Everything else I checked held up — the measurement tables, the EXPLAIN plan shape, and the failing-first ledger are all things a read-only tree cannot confirm, but the claims that are checkable from the tree checked out. Details on the rest in the inline comments and the review body.

claude[bot]
claude Bot previously requested changes Oct 2, 2026

@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.

Verdict: CHANGES REQUESTED

Two blocking findings; detail in the inline comment on RawWindowFloor.cs:68-87 and the top-level parity comment.

Claims

  • VERIFIED — notice sits beside the tier and row-cap notices — DarlingWebEndpoints.cs:1322-1330
  • VERIFIED — run_custom_view_panel shares the runner — DarlingMcpCustomViewTools.cs:287
  • VERIFIED — Active Queries banner on grid, slicer and deep-link reads — ViewerServerTab.ActiveQueries.cs:82, 93, 114; ViewerDataStartBannerTests.ActiveQueries_ShowsWhereTheSnapshotsStart_OnEveryReadOfTheGrid
  • VERIFIED — Current Waits banner above both charts — ViewerServerTab.Blocking.cs:188; Of(2)→Of(3) keeps EveryViewerFanOut_DeclaresItsWidth green
  • VERIFIED — runs only when the tier notice is silent, never when the wide table served — DarlingWebEndpoints.cs:1322
  • VERIFIED — a failed probe costs the notice, not the chart, and a cancel still propagates — ComposeStoreAvailability.cs:338-342
  • VERIFIED — reuses IsTruncated's 90-minute slack — ComposeStoreAvailability.cs:351, RawWindowFloor.cs:130
  • VERIFIED — index_object_stats excluded on index order; no unvetted name reaches the SQL — DataWindowFloor.cs:83, DataWindowFloorTests.CollectorTables_AreProbedOnlyWhenTheirIndexLeadsWithServerAndTime
  • VERIFIED — probing both stitched halves can only find an earlier start — DataWindowFloor.cs:150
  • VERIFIED — Lite's probe looks for the start inside the window — Lite/Services/LocalDataService.QueryWindowFloor.cs:68-73
  • VERIFIED — Lite untouched, Lite.Tests did not run — absent from the diff. Accurate, and it is the gap.
  • VERIFIED — DataWindowFloorTests written after the code, did not fail first — shape pins only
  • UNVERIFIED — 708.6 ms vs 0.228/3.410 ms — live rig, no artifact in the tree
  • UNVERIFIED — EXPLAIN table, ChunkAppend/Heap Fetches: 0 plan, "never executed" chunks, median/p90 render table — need the 200-server, 36-chunk store
  • UNVERIFIED — "4 of 7 failed", the 3 viewer failures, the two planted reverts — need a run with DARLING_TEST_PG
  • UNVERIFIED — 19902/0/11 then 35/0 — need Windows plus a live store; the LiveStoreSessionTimeZone [ModuleInitializer] mechanism does read correctly in the tree
  • REFUTED — "Lite custom range | Not changed … cannot be reused as it is" as a complete account — the reason covers only reusing Lite's probe for the NEW sparse surfaces, while the diff rewrote RawWindowFloor (:68-87, 120), the twin of Lite's three dense tables and the source for four Darling MCP tools
  • REFUTED — "No lower bound, on purpose … waiting_tasks with a quiet first day, and deadlocks" as applied to RawWindowFloor — neither table is in RawWindowFloor.Table (:49-54); DataWindowFloor.cs:27-28 says the opposite for the three dense ones

Riskiest lines

  1. RawWindowFloor.cs:68-87 — lower bound gone, so a collection HOLE at the window's head reads as "served whole" on three dense tables, feeding four MCP tools and three banners — pinned by RawWindowFloorQuietStartLiveTests.AQuietStartInsideTheWindow_IsNotACut_AgainstDevPostgres, which ratifies the new answer rather than guarding it
  2. Lite/Services/LocalDataService.QueryWindowFloor.cs:68-73 — the untouched twin, still bounded below — UNPINNED
  3. DarlingWebEndpoints.cs:1322-1326 — puts a fleet-wide per-server LATERAL on every render — pinned by ComposeDataFloorLiveTests / ComposeRollupDataFloorLiveTests, both live-only

Traps near this diff

  • "Lite and Darling ship in parity (CONTRIBUTING's two-store rule): a fix to one SKU's reader, tool, alert or description without the counterpart's is a finding, not a nit — the census in McpPageContractTests compares the two SKUs' payload keys by tool name for exactly this (#3594)." — applies — RawWindowFloor.cs:68-87, 120 vs Lite/Services/LocalDataService.QueryWindowFloor.cs:68-73 and Lite/Mcp/McpQueryTools.cs:82, 215, 344
  • "A pin that encodes the lie … Ask what a pin would fail ON. (#3699)" — applies — RawWindowFloorQuietStartTests.cs:30-40
  • "Two copies of one definition pinned EQUAL to each other fail only after BOTH have drifted (#3684). Alias, do not copy." — applies, non-blocking, pre-existing — DarlingMcpQueryStoreClutterTools.cs:149 vs :89
  • "A retention-tiered store answers a 7-day question from a 4-day raw tier … unless the read routes by tier (#3590)" — does not apply; this diff is the fix — ComposeStoreAvailability.cs:373-392
  • "hours_back, limit, top, days_back are refused when out of range, never clamped (#3699, #3641)" — does not apply — the clamp at DarlingWebEndpoints.cs:1266 is untouched and not an MCP parameter
  • "A test that reads the clock twice measures the CI scheduler (#3676)" — does not apply — two reads exist (ComposeDataFloorLiveTests.cs:259) but margins are whole days
  • "A JSON number above 2^53 is rounded by every JavaScript client (#3487)" — does not apply — the notice is a string, ComposeStoreAvailability.cs:358-362
  • "An expression-bodied member holding a { } property pattern is read SHORT by the member walk (#3667)" — does not apply — no property pattern in the new members; DataWindowFloor.cs:191 is braced

Divergence (blind read vs PR body)

1. RawWindowFloor is not in the body at all. The diff rewrites its SQL (:68-87) and flips EffectiveStart to a clamp (:120), changing window_truncated / effective_start / effective_hours_back on get_top_queries_by_cpu, get_top_procedures_by_cpu, get_query_store_top, query_store_clutter, and three Queries-tab banners. The surface table rosters nine surfaces and names none of these seven.

2. The rewrite's stated purpose is a table class it does not serve. The quiet-day and deadlock examples justify DataWindowFloor, not RawWindowFloor, whose tables are dense; there the lower bound caught a hole. DataWindowFloor.cs:27-28 still calls bounded-inside-the-window right for exactly those tables — opposite behaviour, two files, one PR.

Everything else matched the tree.

Reviewer checklist

  • Pick which RawWindowFloor semantics ship, then reconcile DataWindowFloor.cs:27-28 and Lite/Services/LocalDataService.QueryWindowFloor.cs:54
  • Reproduce live: collector down for day 1 of a 7-day window on a server holding a fortnight of query_stats; confirm window_truncated: false with no banner is wanted
  • Re-run Darling.Tests with DARLING_TEST_PG on Windows for the 35/0 ledger, and check nothing expects DARLING_TEST_PG unmodified
  • Measure the fleet probe on a real fleet: one unbounded LATERAL per registered server, 10,903 buffers per render
  • Verify the viewer/MCP roles hold SELECT on collect.servers, else every notice and banner silently disappears
  • Run the Node harness where node exists: TryRender returns false on Win32Exception, so that test asserts nothing without it

… an older row, instead of a MIN over every archived row

GetQueryWindowFloorAsync read the server's oldest row with an unbounded MIN(collection_time). DuckDB has no
(server_id, collection_time) index to stop at the first row, so that MIN reads every archived row for the server:
about 45 to 60 ms for a server with 90 days of rows against about 6 ms for the bounded probe it replaced, with a
7-day window.

No caller needs the oldest row. The banner and the three MCP tools only ask whether the window was served whole
(IsWindowTruncated, EffectiveWindowStart), and any floor at or before the requested start gives the same output.

The probe is now two steps. The first is the bounded in-window MIN. It ends the probe when the window holds no row
(NULL) or its first row sits on the start. Only when that first row is after the start does a second query ask,
LIMIT 1, whether the server holds ANY row before the start. The answer is NULL (nothing in the window), the
requested start (a row before the window: served whole) or the first row in the window (the data starts late).

Tests: the quiet-start guard now expects the requested start, and so does the window-inside-the-rows test (both fail
on the previous probe); new guards pin a row exactly on the window's start and that another server's older rows do
not count as this server's coverage. Planting a lower bound on the older-row query fails the quiet-start guard.
…ata starts, and the window-floor probe no longer calls a quiet start a cut (#4958)
Comment thread Darling/PerformanceMonitor.Darling.Storage/RawWindowFloor.cs
Comment thread Lite/Services/LocalDataService.QueryWindowFloor.cs Outdated
Comment thread Darling/PerformanceMonitor.Darling.Storage/DataWindowFloor.cs Outdated
Comment thread Darling/Darling.Tests/LiveStoreSessionTimeZone.cs
Comment thread Darling/PerformanceMonitor.Darling.Service/DarlingWebEndpoints.cs Outdated
Comment thread Lite/Controls/ServerTab.xaml
claude[bot]
claude Bot previously requested changes Oct 2, 2026

@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.

Verdict: CHANGES REQUESTED

Detail is in the seven inline comments; this is the ledger.

Claims

  • VERIFIED — the notice reaches both the web panel and run_custom_view_panel from one place — DarlingWebEndpoints.cs:1322-1332; DarlingMcpCustomViewTools.cs:287 is the tool's only path into RunComposedPanelAsync. Its exact wording is ComposeStoreAvailability.cs:357-361, pinned character-for-character by DataWindowFloorTests.TheNotice_NamesTheStartAndTheWindowItCovers_InUtc.
  • VERIFIED — Darling Active Queries / Current Waits banners on the range, deep-link and slicer reads — ViewerServerTab.ActiveQueries.cs:76,91,110, ViewerServerTab.Blocking.cs:176,188, ViewerServerTab.xaml; ViewerDataStartBannerTests asserts exactly 3 probe and 3 banner calls.
  • VERIFIED — "the served start these tools report never comes before the requested start" — RawWindowFloor.cs:120-121, McpQueryTools.cs:986-987, wired at DarlingMcpDataTools.cs:641,854,1049, DarlingMcpQueryStoreClutterTools.cs:148, McpQueryTools.cs:82,215,344.
  • VERIFIED — "runs only when the tier notice is silent … a failed probe costs the panel its notice, not its chart" — gate at DarlingWebEndpoints.cs:1322, swallow at ComposeStoreAvailability.cs:343-347.
  • VERIFIED — "a stitched route asks both halves" — DataStartSources scrapes CaggFromClause, which StitchedRelationSql (TimescaleSupport.cs:13793-13795) emits as legacy UNION ALL successor; both survive TryForRollup and MIN takes the older legacy half.
  • VERIFIED — "a LIMIT 1 walk of the (server_id, <time>) index", and index_object_stats excluded for ordering by object first — PgSchemaGenerator.CreateIndex:112,114; every CAGG keeps (server_id, bucket) explicitly under create_group_indexes = false (TimescaleSupport.cs:2257-2261,2702,2750,2817,2864). The probe's time column is the same PrefixTimeColumnName ComposeCompiler.s_timeColumnByTable:86-87 uses, and Source.EndExclusive matches ComposeCompiler.cs:351.
  • VERIFIED — the "no change needed" references — no ComposeCompiler/PanelPlan anywhere under PerformanceMonitor.Darling.Viewer; RunningJobs.cs:71, DrillDown.cs:329-337 and BaselineCache.cs:58 each say what the body says. The existing-test change adds ["server"] = "compose-old-window-no-rows" and leaves the no-notice assertion untouched.
  • VERIFIED — "Lite asks in two steps … bounded on both sides, as before" (LocalDataService.QueryWindowFloor.cs:93-119), and the census arithmetic matches: 10 GetQueriesTabWindowUtc sites and 10 banner calls in ServerTab.Refresh.cs, 4 slicer calls on e.StartUtc/e.EndUtc.
  • UNVERIFIED — every EXPLAIN, buffer count, latency median and the "708.6 ms vs 0.228 ms" comparison, plus the Lite before/now table — live measurements against a 3.36M-row TimescaleDB store and a 29.3M-row Lite store; the Lite numbers are self-declared as Python DuckDB 1.4.4, not the engine Lite ships.
  • UNVERIFIED — every failing-first count ("4 of 7", "3 of 10", "11 of 17", "19902/11", "35 of 35", "37 passed") and the four planted-failure reverts — each needs DARLING_TEST_PG set, plus TimescaleDB for the rollup class; those classes are Assert.SkipWhen-gated without it.
  • UNVERIFIED — "Lite builds with 0 warnings" — dotnet build is unavailable here and the WPF projects do not build on Linux.
  • UNVERIFIED — "#4958 lists each surface" — an issue outside the tree.

Riskiest lines

  1. Darling/PerformanceMonitor.Darling.Storage/RawWindowFloor.cs:75 — dropping collection_time >= $2 redefines every window_truncated/effective_start the three raw grids and four MCP tools have reported since #2364/#4231 — pinned by RawWindowFloorQuietStartLiveTests.AQuietStartInsideTheWindow_IsNotACut_AgainstDevPostgres and QueryStoreTopWindowTests.TheWindowFloorProbe_WalksTheIndexFromTheOldestRow_AndChecksTheWindowApart, but what they pin is the behaviour the first finding objects to, not a guard against it.
  2. Lite/Services/LocalDataService.QueryWindowFloor.cs:119 — the twin, returning startUtc as a sentinel, read by three MCP tools and five banners — pinned by five new QueryWindowTruncationTests.FloorHelper_* cases (exact-boundary and other-server included) and six in DataStartBannerTests; genuinely thorough boundary coverage.
  3. Darling/PerformanceMonitor.Darling.Service/DarlingWebEndpoints.cs:1325 — a new store round trip on every Custom Views render with no tier notice, under the panel's own composedQuerySeconds ceiling, after the panel query already succeeded — pinned for correctness by ComposeDataFloorLiveTests/ComposeRollupDataFloorLiveTests; UNPINNED for cost.

Traps near this diff

  • "A retention-tiered store answers a 7-day question from a 4-day raw tier and labels the missing days quiet unless the read routes by tier and says which tier served it (#3590; the viewer's charts in #3684)." — applies, inverted: a window whose first day holds no row is now labelled served, because RawWindowFloor.cs:75 lost its lower bound and EffectiveStart (:120) clamps to the requested start; DarlingMcpDataTools.cs:930 then emits truncation_note = null.
  • "Two copies of one definition pinned EQUAL to each other fail only after BOTH have drifted (#3684). Alias, do not copy." — applies: DarlingMcpQueryStoreClutterTools.cs:89 keeps its own TimeSpan.FromMinutes(90) and :149 its own hand-rolled verdict, one line under the EffectiveStart this PR wired in.
  • "An anti-pattern census sweeps the class (every tool body); rosters are for the positive contract only (#3699)." — applies: NoMcpTool_ComputesTheServedStartByHand bans only the string floor ?? requestedStart, so it sweeps that file and passes with :149 intact.
  • "An empty analysis window is 'unavailable', not an all-clear (#3553)." — applies in the probe's favour: the EXISTS guard (RawWindowFloor.cs:79-86) and the early return (LocalDataService.QueryWindowFloor.cs:105-108) both keep null for an empty window, pinned either side.
  • "Lite and Darling ship in parity … a fix to one SKU without the counterpart's is a finding (#3594)." — does not apply: QueryWindowRelation gains QuerySnapshots/WaitingTasks (LocalDataService.QueryWindowFloor.cs:28-29) against Darling's DataWindowFloor, and both SKUs get both banners (Lite/Controls/ServerTab.xaml:371, :1722 vs ViewerServerTab.xaml:694, :2477). The compose notice has no counterpart to write — no ComposeCompiler/PanelPlan anywhere in Lite/.
  • CONTRIBUTING's T-SQL style section, and "a clamp is a silent lie about the window (#3699)" — neither applies: the diff adds no T-SQL (the new SQL is PostgreSQL at DataWindowFloor.cs:150-176, aliased AS s/AS f/AS l/AS u, and DuckDB at LocalDataService.QueryWindowFloor.cs:93-116), and the clamp added is on a floor the store reported, not on a caller's argument.

Divergence (blind read vs PR body)

  1. "quiet for the first hours" vs. the hole the tests pin. The body motivates dropping the lower bound with sparse tables and "a server that was quiet for the first hours of a window". The two tests pinning the rule on the DENSE per-collection tables plant far more: RawWindowFloorQuietStartLiveTests seeds query_stats at -8d then nothing until -6d against a 7-day window (a full day empty) and asserts not-truncated; FloorHelper_QuietStartInsideTheWindow_… seeds two rows 27 days apart and asserts the 7-day window reads as served whole. The body never says a collection outage, a re-pointed server or an offline target at the head of a window now answers window_truncated = false, effective_hours_back = 168. The direction of the trade is documented; its cost is not.
  2. get_query_store_clutter — "same floor". The surface table puts this tool on the same footing as the other three. Only its served start moved to the shared helper (:148); the verdict is still hand-computed over a local 90-minute literal. True today, but by coincidence, not construction.
  3. Cost measured only in the shape that is fast. "One statement per panel render" is accurate and the EXPLAIN work is unusually thorough — the quoted rows=0.25 loops=200 even covers the no-rows servers I expected to be missing. Unquantified: scaling in chunk count, since a server with no rows must touch every chunk's index to answer "none", and the rig has 36 wait_stats chunks where a busy 4-day raw tier has more. Non-blocking, raised inline.

Everything else the body claims about the code matched the tree.

Reviewer checklist

  • Decide finding 1: should window_truncated/effective_start carry "the store holds a row older than the window's start", or should the window's own first row ride beside it so a hole at the head is still disclosed? If the former, say so in RawWindowFloor's remarks and the body — it narrows a shipped honesty flag.
  • Build both SKUs and confirm "0 warnings": DataWindowFloor is new, ApplyWindowFloorToBanner/QueryWindowRelationView changed accessibility, and nothing here compiles in a read-only Linux tree.
  • Run the live classes with DARLING_TEST_PG set (TimescaleDB installed) and once unset; then run the whole assembly against a store NOT on UTC to prove the [ModuleInitializer] ordering claim in LiveStoreSessionTimeZone.cs.
  • Open both tabs on both SKUs: the Darling Active Queries grid moved to Grid.Row="3" and both Current Waits charts shifted down a row — only a running window shows whether the collapsed banner leaves a gap.
  • Click Live Snapshot / Latest Snapshot with the banner up, and time a fleet Custom Views panel against the production store with the probe on the render path.

… banner pin follows the shared window-floor step

The Darling live-test base string now carries the session time-zone pin, so the scratch store's unpinned string has to drop it. Without that, the unpinned session reports UTC instead of the database's own default, and the test that proves the product pin overrides that default fails.

The Lite source pin for the Showing since banner looked for the old direct call. It now pins the shared window-floor step's call, its definition and its inner call, and a second check requires every call in the ServerTab files to hand the banner the tab's own picker zone as its last argument.
…-start notice from coverage, not the oldest row

Coverage starts at the later of the server's first collection and the table's
retention edge; a row in the window only moves it earlier. A window that starts
at or after coverage shows no notice, whether or not it holds rows.

Darling: first collection is the registry's created_date (written once at the
first connect, never purged); the edge is the probe's clock less the collector's
fleet retention days (the fleet override, else the schedule default). Servers
count only with a row or a logged run in the window, so stopped servers' old rows
no longer hide a new server's late start on a fleet panel. The per-server walk is
bounded to the window.

Lite: Active Queries and Current Waits also count the collector's runs in
v_collection_log, which the single archive horizon keeps exactly as long as the
tables. The Queries-tab relations keep the row-only probe.

Checkpoint: not built or run yet.
…rver's oldest row

Rollups, the raw relations the gated purge owns and the collectors whose purge
is floored at the baseline window have no schedule edge. The draft read their
coverage from the registry's first collection and the window's rows alone, so an
old server showed no notice even when the window started long before anything
the table keeps. Their edge is now the oldest row the server holds at or before
the window's end, the rule before the probe read coverage. These tables are
dense, so that row is where coverage starts. The walk sits in the select list,
so it runs only for a server that counts.

Pins follow the new shape: the scope is the fourth parameter, the window read has
a lower bound, the join is LEFT JOIN LATERAL, and a server counts only with a
row or a logged run in the window. A live test covers a Custom Views panel over
a baseline-floored table for a server older than the window.

Docs on the Lite banner step and the truncation helpers now say what a null
floor means: no row, and for Active Queries and Current Waits no logged run.

Checkpoint: written but not built or run yet.
…overage, not the oldest row

The previous commit was written before it was built. It builds clean with 0
warnings for Darling, Lite and the Dashboard, and the targeted Darling classes
(DataWindowFloorTests, ComposeDataFloorLiveTests, ComposeRollupDataFloorLiveTests,
ViewerDataStartBannerTests, RawWindowFloorQuietStartLiveTests,
RawWindowFloorEffectiveStartTests: 58 tests) and the Lite classes
(DataStartBannerTests, DisplayZoneTextFrameTests, QueryWindowTruncationTests:
40 tests) pass against a local PostgreSQL with TimescaleDB.

This commit only renames the one-server probe test and updates the viewer pin's
class comment to the coverage rule.
…goes on, the viewer banner reads on the invariant culture, and the viewer pins check arguments

A data-start probe that threw skipped everything after it. In Lite the Active Queries probe runs before the Query Stats grid bind and the Current Waits probe before the Locking tab's slicers and tab-badge counts; in the viewer the Active Queries probe runs before the slicer load. Lite's shared banner refresh now reads its probe through ProbeWindowFloorOrNullAsync, and the viewer's four probe awaits go through DataStartOrNullAsync: a probe that throws answers null (a null floor hides the banner, and ApplyWindowFloorToBanner also clears its text), the failure is logged at Warn in Lite and through ViewerLogger.Warn in the viewer, and the refresh finishes. Both helpers take the probe as an argument so the tests drive a throwing one without building the control. The web notice's probe catch now logs the exception at Debug through the runner's read-latency logger: the panel is still answered, and the probe runs once per panel run, so a standing fault at Warning would put a line in the service log on every refresh of every panel.

The viewer's Active Queries, Current Waits and Queries-tab banners format their instants inside UpdateTruncationBanner with DisplayZone.Format on the invariant culture, as Lite's banner and the web notice do, so a Thai or Arabic default calendar no longer prints 2026 as 2569. ViewerTimeHelper.FormatForDisplay and its other callers keep the current culture.

ViewerDataStartBannerTests now pins the arguments, as Lite's twin does with a regex: each probe gets its path's window (the toolbar's start and end, the deep link's pending bounds, the slicer's selection) and each banner is compared with that window's start (pending.FromUtc on the deep link, not the toolbar's start). New pins run a throwing probe in Lite and in the viewer, and read the banner under th-TH and fi-FI. ComposeDataFloorLiveTests skips when Node is missing instead of passing without running its check.
…fresh goes on

The Top Queries, Top Procedures and Query Store grids awaited their window-floor probe bare. A probe that threw on a range load unwound it past the slicer and the comparison that follow the banner, and one that threw on a slicer drag unwound it past the comparison. Lite's Queries tab already goes through its catch.

Each of the six awaits (a range load and a slicer drag on each grid) now goes through DataStartOrNullAsync, as Active Queries and Current Waits do, and the log line names the surface: Query Stats, Procedure Stats or Query Store. Every other argument of each banner call is as it was, and what each probe decides is unchanged. The helper's doc names these probes too.

The pins check all six call sites for their probe, window, surface and window start, what follows the banner on each, and that no banner call on the tab awaits its probe bare.
@claude

claude Bot commented Oct 2, 2026

Copy link
Copy Markdown

Blind-read notes (full tier)

Two things worth recording beyond the inline comments, neither a verdict driver.

The two SKUs now answer "where does coverage start" by different mechanisms. Darling's DataWindowFloor reads coverage as GREATEST(purge cutoff, servers.created_date) and uses collect.collection_log only as a gate on which servers count; Lite's GetQueryWindowFloorAsync uses the collector's runs in v_collection_log as the coverage signal itself (LEAST(first row, first run), then the older-row and older-run look-behinds). For a server whose waiting_tasks collector was enabled well after its first connect, Lite shows "Showing since <first logged run>" and Darling shows no banner at all. The PR body states Darling's side of that explicitly — "A collector turned on after the first connect reads as covered from that connect: no notice, never a false one" — so I am reading it as deliberate rather than as a parity break under CONTRIBUTING's two-store rule. Flagging it only because the two apps will word the same store state differently, and the body does not say so.

The spliced retention edge is a second copy of ResolveFleetRetentionDays. DataWindowFloor.cs:234 reimplements the resolver in SQL — COALESCE((SELECT o.retention_days … WHERE o.server_id IS NULL AND lower(o.collector_name) = '…' AND o.retention_days >= 1 LIMIT 1), <default>). I checked it against DarlingRetentionHorizons.ResolveFleetRetentionDays (fleet row only, >= 1 or fall through to CollectorScheduleDefaults) and the two agree today, including the NULL case (retention_days >= 1 is NULL for a NULL row, so COALESCE falls to the default, matching fleetOverrideDays is int days && days >= 1). What pins it is DataWindowFloorTests.AScheduleGovernedTable_ReadsItsEdgeFromTheScheduleNotFromAWalk, which asserts the SQL substring — so it pins that the copy has not been edited, not that the copy still matches the C# resolver. A change to the resolver alone (say a new upper bound) passes both.

What I checked and did not find a problem with

  • Caller sweep for the RawWindowFloor.GetAsync semantic change (the floor can now sit before the requested start). All six readers hold: DarlingMcpDataTools.cs:641/642, :854/855, :1049/1051, DarlingMcpQueryStoreClutterTools.cs:148/149, and the viewer's ViewerServerTab.Queries.cs:237/240/244/264/275 — every one of them goes through EffectiveStart/IsTruncated or the equivalent f > requestedStart + tolerance, all of which are no-ops for a floor at or before the start. HourlyWindowEdges.Note and HourlyAttributionSpan only consult floor on the hourly branch, where it is routed.HourlyFirstBucket, not the raw probe (DarlingMcpDataTools.cs:950-952).
  • FloorSql schema qualification. The probe went from unqualified FROM query_store_stats to FROM collect.query_store_stats; TableName returns the bare name, so this is a tightening, not a double-qualification.
  • ACL. config.config_collector_schedules is not in DarlingManagedRoles.ViewerRestrictedConfigTables (only config_monitored_servers, config_command, config_notification, config_notification_routes), so the blanket GRANT SELECT ON ALL TABLES IN SCHEMA config pinned at DarlingManagedRolesTests.cs:105 / :282 does reach it for both viewer and mcp. The probe will not silently lose its notice on a managed store.
  • The schedule-edge gate is consistent with the purge. TimescaleSupport.RawRelations is only {query_stats, procedure_stats, query_store_stats} and RetentionPolicies attaches TimescaleDB retention to the raw trio, the rollups and the baseline aggregates — waiting_tasks and query_snapshots get none, so the daily sweep's retention_days really is their only horizon.
  • Lite's LEAST(…, …) over a NULL arm is not just asserted in a comment: DataStartBannerTests.ActiveQueries_RangeStartsBeforeTheOldestStoredSnapshot_ShowsTheBanner seeds rows and no log runs, so the log subquery is NULL and the banner only appears if DuckDB's least ignores it.
  • Window alignment (Lite MCP tools disclose a truncated query-stats window (#4231) #4279/Lite: Queries-tab comparisons read a window shifted by the server's UTC offset #4284). GetLatestQuerySnapshotsAsync, GetWaitingTaskTrendAsync and GetBlockedSessionTrendAsync all take their window from GetTimeRange, which is what GetQueriesTabWindowUtc wraps — so the four new banner call sites really do read the same UTC pair the grids and charts did.
  • XAML re-indexing. ViewerServerTab.xaml Active Queries went 3→4 rows with the slicer/stack/grid moved 0/2/3, and Current Waits went *,* → Auto,*,* with both charts moved to 1/2; Lite's Active Queries wrapped the banner inside the existing Grid.Row="1" StackPanel, so no re-index was needed. All consistent.
  • ViewerReadFanOut.Of(2) → Of(3) on the Current Waits arm matches the third concurrent read; the Active Queries and Queries-tab paths declare no width, but that is the pre-existing idiom there (LoadTopQueriesAsync already ran floorTask beside the grid read without one).

Traps I checked that do not apply

  • "CASE WHEN interval > 0 THEN delta / interval ELSE 0 END spells 'unknowable' as 0" — no rate arithmetic in this diff; the probe returns a timestamp or NULL (DataWindowFloor.cs:346).
  • "Truncation inferred from a full page: truncated = rows.Count >= limit" — truncation here is a timestamp comparison against the window start (RawWindowFloor.cs:129-130), never a row count.
  • "A retention-tiered store answers a 7-day question from a 4-day raw tier and labels the missing days quiet" — this diff is the fix for that shape on three more surfaces, not an instance of it.
  • "ORDER BY 3 + 4 DESC is not 'ordinal 3 plus ordinal 4'" — every ORDER BY the probe emits names a column (ORDER BY h.{time}, ORDER BY f.collection_time), DataWindowFloor.cs:247 and :259.
  • "A pin that encodes the lie" — the new live tests plant rows and logged runs and read the notice back rather than asserting the probe's own arithmetic, and the PR body lists the RED each one showed before the change plus two reverted planted failures.

claude[bot]
claude Bot previously requested changes Oct 2, 2026

@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.

Verdict: CHANGES REQUESTED

Detail for each item is in the top-level comment and the two inline comments on this PR.

Claims

  • VERIFIED — the notice's exact wording — ComposeStoreAvailability.cs:369-372, pinned by DataWindowFloorTests.TheNotice_NamesTheStartAndTheWindowItCovers_InUtc
  • VERIFIED — shows with the row-cap notice, not with the tier notice — DarlingWebEndpoints.cs:1322-1332
  • VERIFIED — run_custom_view_panel shares the runner — DarlingMcpCustomViewTools.cs:287
  • VERIFIED — floor is now the oldest row at or before the window's end, still null on an empty window — RawWindowFloor.cs:68-87, pinned by QueryStoreTopWindowTests.TheWindowFloorProbe_WalksTheIndexFromTheOldestRow_AndChecksTheWindowApart
  • VERIFIED — served start never precedes the requested start, all 7 tools — RawWindowFloor.cs:120-121, McpQueryTools.cs:255-256, every call site re-read
  • VERIFIED — fan-out 2 to 3 — ViewerReadFanOut.Of(3) in ViewerServerTab.Blocking.cs
  • VERIFIED — banner on grid, slicer and finding-card reads — 3 sites, pinned by ViewerDataStartBannerTests.ActiveQueries_ShowsWhereTheSnapshotsStart_OnEveryReadOfTheGrid
  • VERIFIED — the viewer role can read config_collector_schedules — DarlingManagedRolesTests.cs:105/:282; absent from ViewerRestrictedConfigTables
  • VERIFIED — purge edge = clock less fleet retention days, else schedule default — DataWindowFloor.cs:234 agrees with ResolveFleetRetentionDays; RetentionPolicies attaches none to these two tables
  • VERIFIED — Lite's two-step probe, runs counting in both, one shared horizon — LocalDataService.QueryWindowFloor.cs:399-463, RetentionService.cs:23-33
  • VERIFIED — invariant-culture banner, grids unchanged — DisplayZone.cs:94-99, pinned under th-TH and fi-FI by TheBanner_NamesItsInstants_OnTheInvariantCulture
  • VERIFIED — a failed viewer/Lite probe costs only the banner — DataStartOrNullAsync, ProbeWindowFloorOrNullAsync, both AProbeThatThrows_... tests
  • VERIFIED — a failed web probe logs at Debug, panel still answered, cancellation still propagates — ComposeStoreAvailability.cs:356-359
  • VERIFIED — the changed existing test's point is unchanged (only a server scope added)
  • VERIFIED — Running Jobs / Agent status / Wait drill-down need no change — ViewerDataService.RunningJobs.cs:65-68, ViewerServerTab.DrillDown.cs:335
  • VERIFIED — ScratchPostgres drops the pin; the zone pin follows the shared step — builder.Remove("Timezone"), EveryWindowFloorBannerCall_IsHandedTheTabsPickerZone
  • VERIFIED — index_object_stats excluded by the generic index gate — DataWindowFloor.cs:135-137
  • UNVERIFIED — the whole Cost section (EXPLAIN, 708.6 ms vs 0.228 ms, render medians, compressed-chunk grid, Lite's DuckDB medians) — live measurement; Lite's numbers are self-labelled as Python DuckDB 1.4.4
  • UNVERIFIED — every failing-first transcript, the reverted planted failures and the suite totals — needs a build and a live DARLING_TEST_PG; I confirmed each named test exists and asserts what the body says, not that it was red
  • UNVERIFIED — "a collector turned on after the first connect reads as covered: never a false notice" (self-labelled inferred) — mechanism confirmed, but no test covers a collector enabled after created_date

Riskiest lines

  1. Darling/PerformanceMonitor.Darling.Service/Compose/ComposeStoreAvailability.cs:369 — no comparison of dataStartUtc with windowEndUtc, and the schedule-edge arm can answer later than the window's end — UNPINNED (no test sends windowStart/windowEnd)
  2. Darling/PerformanceMonitor.Darling.Storage/DataWindowFloor.cs:234 — the spliced retention edge, a second copy of ResolveFleetRetentionDays, pinned only as a SQL substring
  3. Darling/PerformanceMonitor.Darling.Storage/RawWindowFloor.cs:68 — the unbounded-below walk, which changes six existing readers' answers — pinned by RawWindowFloorQuietStartLiveTests

Traps near this diff

  • "Two copies of one definition pinned EQUAL to each other fail only after BOTH have drifted (#3684). Alias, do not copy." — applies — DarlingMcpQueryStoreClutterTools.cs:149 still hand-rolls the verdict against its private WindowFloorTolerance (:89); non-blocking, but that constant's doc comment is now wrong
  • "A retention-tiered store answers a 7-day question from a 4-day raw tier and labels the missing days quiet unless the read routes by tier and says which tier served it (#3590)." — does not apply — this diff is the remedy; ComposeRollupDataFloorLiveTests pins the rollup's start, not raw's
  • "Lite and Darling ship in parity (CONTRIBUTING's two-store rule)" — does not apply — both SKUs got the banners here (Lite/Services/LocalDataService.QueryWindowFloor.cs:55-56, ViewerServerTab.xaml)
  • "ORDER BY 3 + 4 DESC is not 'ordinal 3 plus ordinal 4'" — does not apply — every ordering names a column (DataWindowFloor.cs:247, RawWindowFloor.cs:76)
  • "CASE WHEN interval > 0 THEN delta / interval ELSE 0 END spells 'unknowable' as 0" — does not apply — no rate arithmetic; NULL reaches the caller as "nothing to report" (DataWindowFloor.cs:346)
  • "Truncation inferred from a full page: truncated = rows.Count >= limit" — does not apply — a timestamp comparison with 90-minute slack (RawWindowFloor.cs:129-130)
  • "A pin that encodes the lie. Ask what a pin would fail ON." — does not apply — the live tests plant rows and read the rendered notice back; the culture pin asserts its own premise (ViewerDataStartBannerTests.cs:245)

Divergence (blind read vs PR body)

The body's unqualified rule for the empty case — null only when no server has a row or a logged run, and a covered window with no rows answers its start or earlier — holds only for a window the store covers. For an UNCOVERED window with no rows, reachable because collection_log keeps 60 days while waiting_tasks/query_snapshots keep 7, the schedule-edge arm cannot reach null and answers the purge cutoff; when the window also ends before that cutoff (the absolute-window path), the notice inverts its own "covers X to Y" clause. The body never addresses a windowStart/windowEnd window and no test sends one.

Smaller: the body says get_query_store_clutter got the "same floor" — true of the floor, but that tool alone still computes its truncation verdict by hand, which the framing implies it no longer does.

Reviewer checklist

  • Pin a Custom Views panel over waiting_tasks to an absolute range older than its retention but inside collection_log's 60 days and read the notice; the inline comment predicts a reversed "covers X to Y".
  • Re-run the Cost measurements on a store with far more than the 36 wait_stats chunks measured; the probe now runs on every render where the tier notice is silent.
  • Confirm LiveStoreSessionTimeZone's module initializer wins against the 400+ classes reading DARLING_TEST_PG, on a non-UTC store.
  • Watch the service log at Debug on a managed store for the swallowed probe failure, which costs every panel its notice silently.

…nd the clutter tool reads its truncation verdict from the shared helper

DataWindowFloor.GetAsync and GetForServerAsync answer null when the coverage they find starts at or after the window's end. collection_log outlives a table the purge keeps shorter, so a logged run made a server count in a window whose rows were gone, and its coverage, the purge edge, came after the window ended: the Custom Views notice read "covers <edge> to <end>" with the end before the edge, and the viewer's banner said "Showing since" a time after its range. No new notice text; a window the store covers answers as before.

The Query Store clutter tool takes window_truncated from RawWindowFloor.IsTruncated, as get_query_store_top does, and its private 90-minute tolerance, whose doc described a floor that is no longer the first row inside the window, is deleted. The guard against hand-computed served starts now also fails on a restated tolerance or a floor compared with the requested start by hand.

Pins: three live tests in ComposeDataFloorLiveTests (a pinned window 45 to 40 days back gets no notice; a window 10 to 5 days back keeps its notice and covers to the window's end; both probes answer null for the 45-to-40-day window).
…e stitch boundary up

The probe read both halves of a stitched rollup route whole, but the route reads the successor only from the stitch boundary up. When the successor had been materialized back past the boundary and the superseded half held nothing for a server, the probe found the successor's older buckets and reported a start below the boundary, with no notice, though the panel's data starts at the boundary.

The router now carries the boundary on the route as a value (RollupCoverage.StitchFloor, beside the stitched clause on both tiers). A probe source for a rollup takes an optional lower bound that starts both the window read and the oldest-row walk, as a bind parameter numbered after the scope's. The successor half is probed with the boundary, found through the registry rather than by its place in the clause, and the superseded half is probed whole.

The day-grain daily route now carries its FROM clause like every other rollup route (the compiled SQL is unchanged), so the probe no longer falls back to the relation name; a rollup route without a clause is refused.

DataWindowFloor's remarks now say the probe answers by server identity (the registry's current name and server_id) while the panel filters rows by their stored server_name, so for a renamed server the rows written before the rename count toward coverage even though the panel's filter misses them.
…auses, and the no-notice live test registers its server (#4953)

The tool description (head and tail) and the compose.js comment said a notice meant retention or the row cap. A notice also means the panel's own data starts after the window does, and an absent notice means the window was served whole. The head now names all three causes and drops its server and rollup sentences, which the tail already carries (pinned in the tail facts), to stay inside its length budget (609, down from 612). A new pin checks the head and the tail name every cause.

The old-window live test scoped its panel to a server that was never in collect.servers, so the data-start probe read no table and its no-notice assertion passed whatever the store held. It now registers a sentinel server and removes it afterwards through the shared cleanup helper. With a row planted in the window for that server the assertion fails with the partial-window notice.
…rt tests carry the project's copyright header

LiveStoreSessionTimeZone.cs, LiveStoreSessionTimeZoneTests.cs and RawWindowFloorQuietStartTests.cs were the only new files of the data-start change without the header the others carry (for example ComposeDataFloorLiveTests.cs).
…g since" banner the last range read raised, in Lite and the Darling viewer

The Active Queries grids show a "Showing since <time>" banner when a range read starts after the stored snapshots do. Lite's Live Snapshot and the viewer's Latest Snapshot swap the grid for a different set of rows (the server's running queries now, the newest stored batch), but left the banner from the last range read up, so it described rows that were no longer there.

Both handlers now hide the banner (collapsed, text cleared) as the new rows are bound, and not when the read fails, because the grid then keeps the range rows and the banner that describes them. Lite goes through the shared SetWindowTruncatedBanner step; the viewer collapses its banner directly, since its UpdateTruncationBanner takes a probe answer and leaves a hidden banner's text in place. The next range read raises the banner again through the existing path, which is unchanged.

DataStartBannerTests (Lite) and ViewerDataStartBannerTests (viewer) each gain a pin that reads the handler's source: the hide appears once, after the rows are bound and before the failure arm.
… its own window, in Lite and the Darling viewer (#4953)

A drill-down (the heatmap, the chart drills, the Overview drills) loads the Active Queries grid for a narrow window but never refreshed the banner, so it kept describing the last range read: it could claim a cut the drill window does not have, or miss one it does.

Lite: OnActiveQueriesDrillDown and OnHeatmapDrillDown refresh the banner through the shared window-floor helper after the rows are bound. The drill window is a naive-UTC pair from GetDrillWindow, the grid read takes it unconverted, and the probe compares against UTC collection_time, so the same pair goes to the banner.

Darling viewer: NavigateToActiveQueriesForWindowAsync starts the data-start probe for the drill window beside the rows read and sets the banner once the rows are bound, as the deep-link branch of the range load does. The count pin for this banner goes from three calls to four on purpose.

Pins: DataStartBannerTests (Lite) and ViewerDataStartBannerTests (Darling) hold the new calls.
@erikdarlingdata
erikdarlingdata dismissed stale reviews from claude[bot], claude[bot], claude[bot], and claude[bot] October 3, 2026 01:12

Resolved at c857763: every finding is fixed or answered in its thread.

@erikdarlingdata
erikdarlingdata marked this pull request as ready for review October 3, 2026 01:13
@erikdarlingdata
erikdarlingdata merged commit 82967c0 into dev Oct 3, 2026
18 checks passed
@erikdarlingdata
erikdarlingdata deleted the fix/cv-retention-notice branch October 3, 2026 01:13
{
/* The table's edge is the purge cutoff: the schedule's horizon is the purge's, as written. The later
of it and the server's first collection is where coverage starts, whatever rows survive near it. */
coverage = $"GREATEST($3 - make_interval(days => COALESCE((SELECT o.retention_days FROM config.config_collector_schedules AS o WHERE o.server_id IS NULL AND lower(o.collector_name) = '{collector}' AND o.retention_days >= 1 LIMIT 1), {days.ToString(System.Globalization.CultureInfo.InvariantCulture)})), s.created_date)";

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Non-blocking — the fleet-retention rule is now a second copy, and the pin is the copy.

DarlingRetentionHorizons.ResolveFleetRetentionDays (DailySummaryHorizon.cs:129) owns one rule: the fleet override when it is set and at least one day, else the CollectorScheduleDefaults default. This line restates that rule in SQL. The default days are aliased off the catalog (good), but the override predicate — server_id IS NULL, retention_days >= 1, first match — is a hand copy.

AScheduleGovernedTable_ReadsItsEdgeFromTheScheduleNotFromAWalk (DataWindowFloorTests.cs:83-84) pins this SQL's text, not its agreement with the resolver, so it would stay green through exactly the drift it should catch: move the validity floor off >= 1, or add a second condition to ResolveFleetRetentionDays, and the purge edge the probe reports silently stops being the purge's edge. That is the #3684 shape from .github/REVIEW-TRAPS.md ("Two copies of one definition pinned EQUAL to each other fail only after BOTH have drifted. Alias, do not copy."), with only one half pinned. The two agree today — I checked both.

Two smaller notes on the same line:

  • lower(o.collector_name) = '…' is case-insensitive on the column, where the canonical read (DailySummaryHorizon.FleetRetentionOverridesSql, collector_name = ANY($1)) is exact. A differently-cased fleet row would be honoured here and ignored by the purge.
  • config. is spelled literally while the collect schema goes through PgSchemaGenerator.CollectSchema two lines down; ConfigSchema exists.

Comment on lines +1321 to +1323
/* The tier notice judges a tier by the store-wide floors, so it is silent for a source with no rollups,
a store with no TimescaleDB, and one server whose rows start later than the rest. When it is silent,
ask where this panel's own rows start. The Query Store wide table reports its own start in the

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Non-blocking — the third case in this comment is one a fleet panel does not get.

"…and one server whose rows start later than the rest. When it is silent, ask where this panel's own rows start" reads as a guarantee that the new probe covers the late-server case. On a fleet-scoped panel it deliberately does not: DataWindowFloor.FloorSql aggregates MIN(u.t) over the servers that count (DataWindowFloor.cs:241), so the earliest coverage wins and one server that started last week is silent behind any server that covers the window. AFleetPanel_StartsAtItsOldestServer_AgainstDevPostgres (ComposeDataFloorLiveTests.cs:182) pins exactly that: a 7-day fleet panel gets no notice "though the recent server's rows start a week back".

The behaviour is the right one and is the behaviour the class doc states ("the earliest coverage start among the servers that count"); it is this comment that promises more than the scope delivers. Reading it as a per-server guarantee is how a future reader would conclude a late server on a fleet dashboard is covered. Worth narrowing to the scoped case — e.g. "…and, for a panel scoped to it, one server whose rows start later than the rest."

DateTime windowStartUtc, DateTime windowEndUtc, int commandTimeoutSeconds, CancellationToken cancellationToken,
ILogger? logger = null)
{
var sources = DataStartSources(sourceTable, route);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Non-blocking — this one call sits outside the try, so it is the one probe failure that can cost the chart.

The summary above says a failed probe "costs the panel its notice, never its chart", and the try three lines down keeps that promise for the probe itself. DataStartSources is called before it, and it throws (:400) for a rollup route with no FROM clause — so that one failure mode unwinds into RunComposedPanelCoreAsync and fails the whole panel, not just its notice.

I verified no production path can reach it: every new ComposeRoute( with a CAGG tier is at ComposeSourceRouter.cs:305, :363 and :372, all three now carry a clause, ComposeCompiler only ever sets ComposeRoute.Raw (:247), and EveryRollupRoute_CarriesTheFromClauseItReads (ComposeSourceRouterTests.cs:521) sweeps the router's outcomes. The throw is also documented as deliberate ("A rollup route without a FROM clause is a router defect, and throws"), so this is a note, not a request.

Still: moving the call inside the try would make the method's own stated contract true for every failure mode rather than for all but one, and would cost nothing — a router defect would then surface as a Debug line and a missing notice, which is what every other fault on this path does.

Comment on lines +39 to +43
/// <item><b>The table's edge:</b> where the table's own rows start. For a collector table the schedule governs
/// as written, it is the purge's cutoff: the probe's clock less the collector's fleet retention days (the fleet
/// override in <c>config.config_collector_schedules</c>, else <see cref="CollectorScheduleDefaults"/>, as
/// <see cref="DarlingRetentionHorizons.ResolveFleetRetentionDays"/> resolves it). The purge applies one horizon
/// to the whole table (per-server overrides never reach it), so this is the same for every server, and it does

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Non-blocking — one way the schedule edge can be later than the probe assumes, which LEAST cannot correct.

The asymmetry here is worth a sentence in these remarks. The computed edge is $3 − fleet retention days, and LEAST(coverage, w.t) can only move the answer earlier: when the purge is behind, or dropped a chunk whose newer rows survive, a row inside the window pulls the start back and the notice agrees with the chart. That direction is safe, and the remarks already cover it ("it does not depend on a row or a chunk surviving near it").

The other direction has no corrector. purge_now with a custom horizon (DarlingWorker.StartPurgeNow → RunPurgeNowBackgroundAsync, "purges every collector to that horizon") leaves the real edge later than the schedule says. The probe then computes a coverage start earlier than the data really has, LEAST takes the smaller of the two and keeps it, and a window that genuinely lost days gets no notice. A lowered schedule is fine — the override subquery reads it — so this is only the one-off operator purge.

Narrow enough that I would not change the rule for it; but "the schedule's horizon is the purge's, as written" is the assumption, and the one documented way to purge off-schedule breaks it in the silent direction.

@claude

claude Bot commented Oct 3, 2026

Copy link
Copy Markdown

Callers sweep (the thing I most wanted to be wrong about)

RawWindowFloor.FloorSql changes meaning for every caller — the floor can now sit before the requested start — so I re-read each one rather than trusting the clamp:

Caller Still holds?
DarlingDataReader.GetQueryStore/QueryStats/ProcedureStatsWindowFloorAsync (:1846, :1856, :1864) pass-throughs, no arithmetic
DarlingMcpDataTools get_top_queries_by_cpu (:641-642) clamped; floor also feeds HourlyWindowEdges.Note and HourlyAttributionSpan, both of which take routed.HourlyFirstBucket on the only branch that reads them (!hourly returns (requestedStart, requestedEnd, null) at :950-952), so the raw floor never reaches the CPU-attribution span
same, get_top_procedures_by_cpu (:854-855) same shape
same, get_query_store_top (:1042, :1049-1051) interval-table branch unchanged; raw branch clamped. if (routed.RawForced && floor is null) still keys on the existence check, so the "empty" status fires exactly when it did
DarlingMcpQueryStoreClutterTools (:143-144) private 90-minute copy deleted, both verdicts now shared
Viewer ViewerDataService.{QueryStats,ProcedureStats,QueryStore} (:536, :316, :594) pass-throughs into UpdateTruncationBanner, which routes every use through IsTruncated / EffectiveStart
ComposeStoreAvailability.BuildDataStartNoticeAsync (:364) IsTruncated only

No unpinned caller: every site either clamps or reads the floor only through IsTruncated.

Two more things I checked because the diff invites the question and the answer was not obvious:

  • DuckDB LEAST with a NULL argument. LocalDataService.QueryWindowFloor.cs:121-133 leans on "LEAST skips a NULL, so either one alone answers". The repo already depends on that semantics elsewhere — GREATEST(last_user_seek, last_user_scan, last_user_lookup, last_user_update) (LocalDataService.FinOps.IndexObjects.cs:171) over four columns that are routinely NULL — so the comment is sound, and the load-bearing both-non-null case is pinned by CurrentWaits_ServerAddedTwoDaysAgo_SaysSinceItsFirstCollection_NotItsFirstRow.
  • The "log outlives the table" worry on Lite. The claim that v_collection_log survives exactly as long as the data tables holds because Lite has one horizon and no per-collector purge at all — RetentionService.cs:23-34 says so in as many words ("Lite does not purge per collector"), and collection_log rides the same archivable list (ArchiveService.cs:178, DuckDbInitializer.cs:712). The retention_days column in Schema.cs:41 is stored but not enforced as a purge, so the older-run check cannot vouch for rows a shorter table horizon dropped.

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