Repository navigation
Make the Viewer match the web on dropped databases, labels, job history, wording and strip gaps (#6651) - #6667
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
|
||
| /* #6651 (#5644 W7): a stretch with no samples (the service host was off overnight) is a break in the strip, not a ramp or a flat | ||
| line across it. The break rule is the one the Viewer's line charts use (TimeSeriesGaps, three times the median spacing). */ | ||
| var runs = SplitAtGaps(_data.Select(d => d.BucketTime).ToList()); |
There was a problem hiding this comment.
SplitAtGaps here runs on _data, and _data is always the output of FillEmptyBuckets (same file, lines 211-235), which synthesizes a zero-value TimeSliceBucket for every hour between floorStart and floorEnd — including hours where the service was off. Every one of the six LoadData(...) call sites (ViewerServerTab.ActiveQueries.cs:111, ViewerServerTab.Queries.cs:583,614,646, ViewerServerTab.Blocking.cs:355,365) passes non-null startUtc/endUtc, so this fill always runs.
The consequence: by the time Redraw() calls SplitAtGaps(_data.Select(d => d.BucketTime)...), consecutive BucketTime deltas are always exactly one hour — there is no longer a genuine time gap for a service-off stretch to show up as, since the empty hours were already backfilled with zero-value points before this method ever sees the data. GapThreshold (3× the median spacing) can never be exceeded when every spacing equals the median, so SplitAtGaps will always return one single run here, identical to the old unconditional-draw behavior.
In other words, the new "break at the service-off gap" drawing logic is unreachable through every real call path — the strip will never actually break. The new test (ViewerWebParityWordsTests.Strip_BreaksWhereTheServiceWasOff_InsteadOfDrawingAcrossIt) only calls SplitAtGaps directly on raw, unfilled timestamps, so it can't catch this — it never goes through LoadData/FillEmptyBuckets the way Redraw() does.
To actually fix the drawn strip, the break needs to survive past FillEmptyBuckets — e.g. by having it mark a synthesized bucket (a flag on TimeSliceBucket, or simply not filling hours with no data and instead passing the raw timestamps straight to the gap splitter) instead of relying on a uniform 1-hour-spaced series to still show a timing gap it no longer has.
|
The PR body's "Viewer only: Lite has no twin of these surfaces" is refuted for at least four of the six items. Lite ships its own, separate copies of these same UI surfaces (by this repo's established pattern of a Darling/Lite twin pair per control), and each one has the identical thing this PR fixes in Darling, left untouched:
Item 4 (Settings wording) and item 6 look genuinely Darling-only (no matching |
There was a problem hiding this comment.
Verdict: CHANGES REQUESTED
Claims
- VERIFIED — "An existing test,
HourlySql_BindsTheCeilingOnlyWhenOneIsKnown_OnBothGrids, now expects the procedures hourly arm's ceiling at$7" —ViewerDataService.ProcedureStats.cs:638callsHourlyCeilingSql(ceiling, 7), matching the updated test inViewerHourlyWindowEdgesTests.cs. - VERIFIED — "Checked the Queries tab's other per-database reads: the Query Store clutter reads already go through
DarlingQueryStoreClutterReader... which appliesOmitDroppedAsync" — confirmed atDarlingQueryStoreClutterReader.cs:275,415,496. - VERIFIED — "Grepped the Viewer XAML ... for other
get_*and*_sourcenames: the only others are code comments" — spot-checkedCollectionLogWindow.xaml:95,FinOpsTab.xaml:708,ViewerServerTab.xaml:1882,2384,2453,3038,4083,4157,4586, all inside<!-- -->comments. - REFUTED — "All under
Darling/PerformanceMonitor.Darling.Viewer/. Viewer only: Lite has no twin of these surfaces." — Lite ships its own twin of four of the six items (dropped-database ranking, the Job History "Last Success" column, the FinOps/Plan-Cache labels, andTimeRangeSlicerControl's gap handling), none of them fixed; see the posted top-level comment for file:line evidence on each. - UNVERIFIED — "Targeted run on a throwaway PostgreSQL with TimescaleDB: ... Total: 436, Failed: 0, Skipped: 0" and the "Revert-proof" claims — these are live test-run results that need a real PostgreSQL/TimescaleDB instance and a Windows build; not checkable from a read-only tree.
Riskiest lines
Darling/PerformanceMonitor.Darling.Viewer/TimeRangeSlicerControl.xaml.cs:332—SplitAtGapsruns on_data, whichFillEmptyBuckets(lines 211-235, unchanged, called from every one of the 6LoadDatasites with non-null bounds) has already backfilled to one zero-value bucket per hour across the whole window, so consecutiveBucketTimedeltas are always uniform and the 3×-median-spacing threshold can never trip — the "break where the service was off" feature this PR advertises cannot fire through any real call path. UNCOVERED — the only new test,ViewerWebParityWordsTests.Strip_BreaksWhereTheServiceWasOff_InsteadOfDrawingAcrossIt, callsSplitAtGapsdirectly on raw, unfilled timestamps and never exercisesLoadData/Redraw()together, so it can't catch this.Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.ProcedureStats.cs:573— the new$6dropped-database exclusion clause inTopProceduresSql, added beforeGROUP BY/LIMITso it narrows the ranking population rather than post-filtering the page. Covered byViewerTopProceduresDroppedDatabaseTests.RawTier_LeavesTheDroppedDatabaseOut_RanksTheLiveOneAtTop1_AndReadsANamedDroppedDatabaseAsAsked(live, needsDARLING_TEST_PG; runs in the "Darling PostgreSQL tests" CI job).Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.ProcedureStats.cs:628(databases.ExcludeParameter()) together withViewerDataService.RollupAvailability.cs:99'sHourlyCeilingSql(ceiling, index)— the hourly arm now binds exclusion at$6and the optional ceiling at$7, so a parameter-count/position slip between the built SQL text and the boundNpgsqlParameters would misfire silently. Covered byHourlySql_BindsTheCeilingOnlyWhenOneIsKnown_OnBothGrids(asserts the built SQL's slot positions) and byViewerTopProceduresDroppedDatabaseTests.HourlyTier_LeavesTheDroppedDatabaseOut_AtTop1(live, exercises real parameter binding end to end).
Traps near this diff
- "Every MCP filter is part of the query, not a post-filter on the page:
parallel_only/min_dop/blocking_onlycut before theLIMIT" — does not apply as a defect here: the new dropped-database exclusion is correctly placed in the SQL'sWHERE/filter clause beforeGROUP BY/LIMITin both the raw (ViewerDataService.ProcedureStats.cs:573) and hourly (ViewerDataService.ProcedureStats.cs:639) arms, not applied as a post-filter on the fetched page. - "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" — does not apply: tier routing (
RetentionTierRouter.Resolve) is unchanged by this diff, andLeftOutDatabasesis attached after the tier is chosen in both the raw and hourly branches (ViewerDataService.ProcedureStats.cs:213,610), so the response still names its tier as before. - "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." — applies: see the REFUTED claim above and the posted top-level comment; Lite has its own unfixed twin of 4 of the 6 items this PR addresses in Darling.
Divergence (blind read vs PR body)
The body frames this entire PR as Darling-Viewer-only because "Lite has no twin of these surfaces," but Lite has its own separate implementations of the Top Procedures grid, the Job History grid, the FinOps labels, and TimeRangeSlicerControl, each carrying the same issue being fixed here (see the REFUTED claim and the top-level comment for file:line evidence). Separately, the body's and CHANGELOG's claim that "the Top Queries and Top Procedures strip breaks where the service was off instead of drawing across it" does not hold for the shipped Redraw() path, because the pre-existing FillEmptyBuckets step erases the real time gap before the new SplitAtGaps call ever sees it (see the inline comment on TimeRangeSlicerControl.xaml.cs:332) — the unit-level SplitAtGaps function is correct and tested, but the integration claimed in the PR body and CHANGELOG is not reachable. Minor: the body says "Two LastSuccessfulRunLocal assertions ... went with the removed column," but the diff removes four such Assert.Equal lines across two test methods in StoredServerClockRowTextTests.cs; non-blocking.
Reviewer checklist
- Run the Viewer against a server with a genuine multi-hour gap in
procedure_stats/query_statscollection and confirm whether the Top Queries/Top Procedures strip actually breaks, or draws a continuous line through the zero-filled gap as described above. - Decide and record whether Lite's twins (Top Procedures dropped-database ranking, Job History "Last Success" column, FinOps/Plan-Cache labels,
TimeRangeSlicerControlgap handling) are in scope for a follow-up, or intentionally left behind, and fix the PR body's "no twin" claim either way.
… follow the Settings note into ViewerLabels, break the strip on real-bucket gaps (#6651)
| internal static List<(int Start, int Count)> RunsOverFilledBuckets(IReadOnlyList<DateTime> bucketTimes, IReadOnlyList<DateTime> realTimes) | ||
| { | ||
| var real = realTimes.Select(t => FloorToHour(t).Ticks).Distinct().OrderBy(t => t).ToList(); | ||
| var threshold = GapThresholdTicks(real); | ||
| var gaps = new List<(long Lo, long Hi)>(); | ||
| if (threshold.HasValue) | ||
| { | ||
| for (var i = 0; i + 1 < real.Count; i++) | ||
| { | ||
| if (real[i + 1] - real[i] > threshold.Value) | ||
| { | ||
| gaps.Add((real[i], real[i + 1])); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| var runs = new List<(int Start, int Count)>(); | ||
| var start = -1; | ||
| var count = 0; | ||
| for (var i = 0; i < bucketTimes.Count; i++) | ||
| { | ||
| var t = FloorToHour(bucketTimes[i]).Ticks; | ||
| var missing = gaps.Exists(g => t > g.Lo && t < g.Hi); | ||
| if (!missing) | ||
| { | ||
| if (count == 0) | ||
| { | ||
| start = i; | ||
| } | ||
|
|
||
| count++; | ||
| } | ||
|
|
||
| var breaksAfter = i + 1 < bucketTimes.Count | ||
| && gaps.Exists(g => t >= g.Lo && FloorToHour(bucketTimes[i + 1]).Ticks <= g.Hi); | ||
| if ((missing || breaksAfter || i + 1 == bucketTimes.Count) && count > 0) | ||
| { | ||
| runs.Add((start, count)); | ||
| start = -1; | ||
| count = 0; | ||
| } | ||
| } | ||
|
|
||
| return runs; | ||
| } |
There was a problem hiding this comment.
Non-blocking nit: when a single real sample is flanked by two gaps that each exceed the threshold (e.g. one isolated sample between two multi-hour holes), RunsOverFilledBuckets emits a run of (start, 1). In Redraw, that degenerate run draws BeginFigure → LineTo to the same point → LineTo straight down to the baseline, i.e. a zero-width spike rather than a visible point. Cosmetic only (rare in practice), not a correctness bug in the gap math itself — the run boundaries matched both manual tracing and ViewerWebParityWordsTests for the realistic cases. No action needed unless you want a visible marker for lone isolated samples.
There was a problem hiding this comment.
Verdict: LGTM
Claims
- VERIFIED — "The new
CurrentDatabaseScope.NarrowAsynccall ... passed no logger. It now passeslogger: null, as every other Viewer call into these readers does" — confirmed at ViewerDataService.ProcedureStats.cs:198; every other Viewerlogger: nullcall site (ViewerDataService.Findings.cs:136, QueryTrends.cs:651, RollupAvailability.cs:142, QueryStore.cs:539, FinOps.Recommendations.cs:197, QueryStoreRegressions.cs:262) does the same, and the guard regex inCurrentDatabaseScopeLoggerThreadingTests.EveryNonTestCaller_OfACurrentDatabaseScopeLookup_PassesALoggerrequires the literalloggertoken in the call expression. - VERIFIED — "The test now checks the XAML binds that label and that the label keeps the precedence and sweep words and names the Alert Settings page, with no key name" —
UncorroboratedRouteStoreKnobTests.cs:333-337asserts the{x:Static local:ViewerLabels.UncorroboratedRouteNote}binding plus the label's own prose, andViewerLabels.UncorroboratedRouteNotewas already wired intoSettingsWindow.xamlby commit04b10bdd0, which predates this follow-up's base. - VERIFIED — "
Redrawsplit_dataafterFillEmptyBucketshad given every hour a bucket, so no jump ever showed. The decision is now made inLoadDatafrom the real buckets" — confirmed:TimeRangeSlicerControl.xaml.cs:114now computes_runsinLoadDatafromdata.Select(d => d.BucketTime)(pre-fill) viaRunsOverFilledBuckets, vs. the old code'sSplitAtGaps(_data...)called inRedrawon the already hour-filled list, which is uniformly 1-hour spaced and so could never exceed a gap threshold. - VERIFIED — "Tests ... 18:00-21:00 plus 07:00-10:00 in an 18:00-10:30 window gives runs (0,4) and (13,4); a two-hour hole, two buckets 9 hours apart, and a steady series stay one run" — hand-traced
RunsOverFilledBuckets/GapThresholdTicksagainst all fourViewerWebParityWordsTestscases; each produces exactly the asserted runs. - UNVERIFIED — "Build
Darling.Tests: 0 Warning(s), 0 Error(s)" and the listed live/guard test run totals — these are build/test-run results from the author's machine; a read-only tree can't execute the suite or the live-PostgreSQL tests.
Riskiest lines
- Darling/PerformanceMonitor.Darling.Viewer/TimeRangeSlicerControl.xaml.cs:300-344 (
RunsOverFilledBuckets) — the core gap-detection logic that decides where the strip breaks; a defect here silently draws a line across a real outage (the exact bug this commit fixes) or breaks a steady series — covered byViewerWebParityWordsTests.Strip_BreaksWhereTheServiceWasOff_InsteadOfDrawingAcrossItandStrip_WithAShortHole_OrFewBuckets_StaysOneRun, which fail on a regression to the old "split the filled data" behaviour or on an incorrect threshold. - Darling/PerformanceMonitor.Darling.Viewer/TimeRangeSlicerControl.xaml.cs:264-291 (
GapThresholdTicks) — the quantile/median threshold math; a defect (e.g. wrong quantile index, wrong floor) would misclassify a steady series as gapped or vice versa — covered by the same two tests above (the "short hole" case specifically exercises the 3x-median-vs-floor boundary). - Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.ProcedureStats.cs:198 (
logger: null) — low risk; functionally a no-op (the parameter already defaulted to null), its only effect is satisfying the repo's source-text guard — UNCOVERED by any behaviour test (the guard itself is a source-text census per the traps file's tests-and-censuses ruling, so it does not count as coverage), but the PR body states why no behaviour test applies (the Viewer has no host logger to assert against) and the change carries no behavioural risk.
Traps near this diff
- Ruling (#5833): tests are load-bearing. ... never answer a trap with a test that reads source text or asserts how code is spelled — applies to
CurrentDatabaseScopeLoggerThreadingTests.EveryNonTestCaller_OfACurrentDatabaseScopeLookup_PassesALogger(walks the repo tree with a regex over source text), which is what this commit's logger fix satisfies — but that test pre-dates this follow-up range (unchanged by these commits) and is not newly added here, so this commit doesn't violate the ruling; it's just answering an existing census-shaped guard that this diff didn't introduce. - 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 — does not apply to the
TimeRangeSlicerControlgap-break logic: Lite's copy (Lite/Controls/TimeRangeSlicerControl.xaml.cs) has noSplitAtGaps/gap-break logic at all, and the Darling file's own header comment ("W1e copy-parity; the viewer owns its own copy, Lite/Dashboard are untouched") documents this as a pre-existing, already-accepted divergence from before this follow-up's base commit, not something newly introduced here. ORDER BY 3 + 4 DESC/ integer division / percent-of-page / etc. (SQL traps) — do not apply; this follow-up touches no SQL.truncated = rows.Count >= limit/ page-vs-population counts (storage/MCP traps) — do not apply; no paged reader or MCP payload changed in this range.
Divergence (blind read vs PR body)
None — the "Round 1 fixes (part 1)" section of the PR body accurately describes all three changes in this range (the logger pass-through, the ViewerLabels test fix, and moving the gap decision from Redraw to LoadData on the real buckets), and the blind read confirms each mechanism as described.
Reviewer checklist
- Run the Darling Viewer against a live PostgreSQL store with an overnight outage in the Top Queries/Top Procedures window and visually confirm the strip breaks at the real gap instead of drawing a flat/ramped line across it (a read-only tree can't render WPF).
…ess, label and strip-gap fixes (#6651) Lite's Top Procedures grid leaves out dropped databases and says so, the Job History Last Success column is gone, the FinOps and Memory labels use the web's words, and the time strip breaks where the collector was off.
…wer and Lite (#6667) Both apps held identical copies of ProcedureLeftOutText and of the strip gap rule. The banner text is now DatabasesLeftOut.ProcedureBanner (Common) and the gap rule is StripGaps (Ui, beside TimeSeriesGaps).
Claude posting for Erik Darling. Round 1 findings fixed; round 2 (5637085836) is LGTM.
Claude posting for Erik Darling.
Closes #6651.
Why
#5704 and #5705 changed the web pages and the MCP tools. The Darling Viewer kept the old behaviour for six rows of the issue's table, so the Viewer and the web disagreed for the same server and window.
What changes
The Viewer changes are under
Darling/PerformanceMonitor.Darling.Viewer/. Lite ships its own copies of items 1, 2, 3 and 5; round 1 part 2 fixes each of them the same way underLite/(see "Round 1 fixes (part 2, Lite)" below).CurrentDatabaseScope.NarrowAsync, the one rule the service and web share, before it reads. With no database chosen, the dropped names are excluded in the statement ($6, raw and hourly arms), so the rank's LIMIT is not filled by them. A database the user names is read as asked; an unknown set leaves out nothing; a custom range that ended in the past is anchored at its end (the MCPas_ofbehaviour). The read result carriesLeftOutDatabases. A new banner above the grid names the omitted databases with the sharedDatabasesLeftOut.Note. On an empty page it shows the sharedCollectorRunFacts.ProcedureStatsEmptysentence ("No procedure ran in the last Nh in a database this server still has ...") built from the collector's own run history (a small viewer-side read ofv_collection_log, the same three counts the service reads); the MCP pointer "(see databases_left_out)" is dropped from that sentence because the names are in the same banner. The slicer-range handler updates the banner too.Checked the Queries tab's other per-database reads: the Query Store clutter reads already go through
DarlingQueryStoreClutterReaderwith the default (all databases) filter, which appliesOmitDroppedAsync. The web applies the narrowing to top procedures only, so Top Queries is left alone.LastSuccessfulRunLocalview-model property. The issue and brief point to a second column atViewerServerTab.xaml(LastSuccessFormatted); that one is the Collection Health grid's per-collector "Last Success", a different fact (it is not a job's repeated success), so it stays.ViewerLabelsclass, bound with{x:Static}, so a test reads the value the screen shows.get_alert_settingsoranalysis.uncorroborated_route_source; it says the Alert Settings page shows which value is in force. Grepped the Viewer XAML and strings for otherget_*and*_sourcenames: the only others are code comments, plus the Ask-AI prompt text, which is sent to an AI and names tools on purpose.TimeRangeSlicerControl, shared by the other slicers) is drawn as separate runs. Runs are split byTimeSeriesGaps.BreakAtGaps(the Viewer's line-chart rule: 3x the median spacing, no break for fewer than three points), through a new staticSplitAtGaps.Test plan
Darling.Tests: 0 Warning(s), 0 Error(s).ViewerTopProceduresDroppedDatabaseTests(live raw and hourly, top 1 with the dropped database heavier, named-database case, unknown set, banner text) andViewerWebParityWordsTests, plusViewerHourlyWindowEdgesTests,ViewerHourlyWindowEdgesLiveTests,StoredServerClockRowTextTests,ViewerGridTimeColumnSortMemberTests,RollupWindowEndBoundTests,ViewerQueriesTests,ViewerQueryTabLoadTests,ViewerIoRollupRouteTests,ViewerQueryTagColumnLiveTests, every*Census*and*Ratchet*class, andStage=Guard:Total: 436, Failed: 0, Skipped: 0.ViewerLabels; the tests pin the words, but the{x:Static}binding in XAML is not exercised (no screenshot: the Viewer cannot be run against the real config).pre-push.sh(CI decides).An existing test,
HourlySql_BindsTheCeilingOnlyWhenOneIsKnown_OnBothGrids, now expects the procedures hourly arm's ceiling at$7(the exclusion takes$6); the rule it pins (a ceiling bound only when one is known, beside the window end) still holds. TwoLastSuccessfulRunLocalassertions and one sort-member table row went with the removed column.Considered, not added
CHANGELOG entry
SECTION: Fixed
ENTRY:
REF:
[Make the Viewer match the web on dropped databases, labels, job history, wording and strip gaps (#6651) #6667]: Make the Viewer match the web on dropped databases, labels, job history, wording and strip gaps (#6651) #6667
Round 1 fixes (part 1)
CurrentDatabaseScopeLoggerThreadingTests). The newCurrentDatabaseScope.NarrowAsynccall inViewerDataService.ProcedureStats.cspassed no logger. It now passeslogger: null, as every other Viewer call into these readers does (PgSecondaryReplicaScope.ReadAsync, the Query Store readers; the Viewer has no host logger).UncorroboratedRouteStoreKnobTests.TheSettingsWindowCombo_PrefillsSavesAndRestores_ThroughTheThreeStates). The test pinned the old XAML sentence, including the MCP keyanalysis.uncorroborated_route_source. This PR moved the note intoViewerLabels.UncorroboratedRouteNoteon purpose (no key name in prose for a person). The test now checks the XAML binds that label and that the label keeps the precedence and sweep words and names the Alert Settings page, with no key name. Prefill, save and restore were not broken; those assertions pass unchanged.Redrawsplit_dataafterFillEmptyBucketshad given every hour a bucket, so no jump ever showed. The decision is now made inLoadDatafrom the real buckets (RunsOverFilledBuckets), with the web strip's rule (chart-gaps.jsgapThresholdMs: a jump over 3 times the median spacing, floor 5 minutes, no break for fewer than two spacings or a series with no steady cadence, 75th percentile over 3 times the 25th). The filled hours inside a gap are left out of the line and area, which break there; the fill still serves the bars and axis. A hole shorter than the threshold keeps its zero buckets in the line, as before.SplitAtGaps(which usedTimeSeriesGaps, the line-chart rule) is removed.Tests (
ViewerWebParityWordsTests, throughLoadDataand the control'sDrawnRuns, on the STA thread): 18:00-21:00 plus 07:00-10:00 in an 18:00-10:30 window gives runs (0,4) and (13,4); a two-hour hole, two buckets 9 hours apart, and a steady series stay one run.Ran (Debug build, 0 errors; throwaway PostgreSQL stopped after):
CurrentDatabaseScopeLoggerThreadingTests,UncorroboratedRouteStoreKnobTests,ViewerWebParityWordsTests,ViewerTopProceduresDroppedDatabaseTests,ViewerHourlyWindowEdgesTests,*TimeRangeSlicer*, every*Census*and*Ratchet*: Total 546, Failed 0, Skipped 0. Not run:pre-push.shand the full suite (CI decides). The wrapper scripts for CPU capping were refused by the harness, so the build ran plain with-m:1.Considered, not added: the web's 5-minute floor never binds for hourly buckets; kept anyway so the rule matches the web word for word.
Round 1 fixes (part 2, Lite)
I confirmed each Lite twin at the lines the review named before changing it; all four were real (line numbers had moved by a few lines). Lite and the Viewer now behave the same for 1 to 4.
ServerTab.Refresh.cs, slicer change inServerTab.Slicers.cs) now go through oneServerTab.ReadTopProceduresAsync. With no database chosen it asksLocalDataService.GetDatabasesLeftOutOfRankingAsync(new, inLocalDataService.CurrentDatabases.cs, over the existingGetDroppedDatabasesAsync) and passes the result asexcludeDatabaseNamestoGetTopProceduresByCpuAsync, as theget_top_procedures_by_cpuMCP tool does. A database the user names is read as asked, an unknown set (no recent snapshot) leaves out nothing, and a window that ended in the past is anchored at its end (same rule as the Viewer). A newProcStatsLeftOutBanner(inServerTab.xaml, text fromServerTab.ProcedureLeftOutText) names the databases and carries the sharedDatabasesLeftOut.Note; on an empty page it usesCollectorRunFacts.ProcedureStatsEmptyso it claims only the databases the server still has. To count the collector's successful runs inside the grid's exact window,GetCollectorRunFactsAsyncgot an optionalwindowparameter (existing callers unchanged). Both reads run on the pool insideReadTopProceduresAsync, which the callers now await directly (theTabLoadsOffUiThreadTestscensus wants every_dataServicecall inside aTask.Run).JobHistoryTab.xamlandLastSuccessfulRunLocalremoved fromLocalDataService.JobHistory.cs. Nothing else read the property (grep overLite,Lite.Tests);LastSuccessfulRunandLastSuccessfulRunUtcstay, as in Darling. The two test pins on the property (GridTimeColumnSortMemberTestsroster row,StoredServerClockRowTextTestsassert) were removed with it.FinOpsTab.xaml: "Avg/P95/Max CPU" and "... CPU %" now read "Avg/P95/Max SQL Server CPU" and "... SQL Server CPU %".ServerTab.xamlMemory tab: "Plan cache (all cache stores)". Plain text in the XAML; Lite has no label constants class and a test would only read source text (Tests that assert spelling, not behavior: cap them, stop the review asking for pins, delete the shape ones #5833).TimeRangeSlicerControl.xaml.csmirrors Darling's commit 223a06e:RunsOverFilledBucketsdecides the gaps from the real buckets beforeFillEmptyBuckets, with the web rule (3x median spacing, 5-minute floor, no break for an irregular cadence or under two spacings);DrawnRunsexposes the pieces;Redrawdraws one figure per run.LoadDataandShowEmptyset them. No use ofTimeSeriesGaps.Tests (new):
TopProceduresGridDroppedDatabaseTests(the grid's two reads against a DuckDB store: dropped database left out and top-1 not filled by it; named dropped database read as asked; a past window anchored at its end vs the same days read to now; no snapshot leaves nothing out; banner text with rows, empty page, cap and absent),TimeRangeSlicerGapTests(throughnew TimeRangeSlicerControl(),LoadData,DrawnRunson the STA thread: the same two cases as Darling's),JobHistoryLastSuccessColumnTests. Changed:SecondOccurrenceReadTestspin string forOnProcStatsSlicerChangednow namesReadTopProceduresAsync(0, e.StartUtc, e.EndUtc)(its rule, that the handler passes the event's UTC bounds straight to its read, still holds).Ran (Debug build of Lite.Tests, 0 errors; 2 warnings, both the existing
EmailAlertService.csCA1068 in a file this PR does not touch):-trait Stage=GuardTotal 1184, Failed 0; the classes*TimeRangeSlicer*,*JobHistory*,*FinOps*,*TopProcedures*,*DroppedDatabase*,*CurrentDatabases*,*StoredServerClock*,*GridTimeColumn*,*CollectorRun*,*SecondOccurrenceRead*,*TabLoadsOff*,*ServerTabEmpty*,*MultiServerTime*,*ScreenReaderRowNames*, every*Census*and*Ratchet*: Total 582, Failed 0. Also the classesci-select.py local-classesnamed that were not in those: Total 326, Failed 1 (theSecondOccurrenceReadTestspin above, fixed and re-run green). Not run:pre-push.sh, the full suite and the Darling classes (CI decides). I did not revert the fix to watch the new tests fail:DrawnRunsandGetDatabasesLeftOutOfRankingAsyncdo not exist before the change, so the gap and left-out tests cannot compile against the old code. The wrapper scripts for CPU capping were refused for the build (the harness refusedcpucap.sh), so the build ran plain with-m:1; the test slot wrapper worked.Considered, not added: a Lite test of the banner XAML binding (source text, #5833); a revert-and-fail run of the new tests (they cannot compile on the old code, see above).
One definition each
The round-1 fixes left two pairs of identical code in the Viewer and in Lite. Each pair is now one definition (REVIEW-TRAPS #3684, alias, do not copy):
DatabasesLeftOut.ProcedureBannerinPerformanceMonitor.Common/DatabasesLeftOut.cs(wasViewerServerTab.ProcedureLeftOutTextandServerTab.ProcedureLeftOutText). Both tabs call it; the two existing banner test classes now call the shared method with the same assertions.StripGapsinPerformanceMonitor.Ui/StripGaps.cs, besideTimeSeriesGaps(unchanged). It holdsRunsOverFilledBuckets,GapThresholdTicks,GapFactorandGapFloorTicks, with the comment naming the web'schart-gaps.jsgapThresholdMs. BothTimeRangeSlicerControls callStripGaps.RunsOverFilledBuckets. The two copies were diffed first: identical apart from the parameter type (IReadOnlyList<long>vsList<long>) and one comment sentence.-trait Stage=Guard1184 passed; Lite.TestsTimeRangeSlicerGapTests,TopProceduresGridDroppedDatabaseTests,TimeSeriesGapsTests,*LeftOut*,*Census*,*Ratchet*52 passed; Darling.TestsViewerWebParityWordsTests,ViewerTopProceduresDroppedDatabaseTests(live, on a throwaway PostgreSQL, stopped),*LeftOut*,*Census*,*Ratchet*512 passed. The full suite andpre-push.shwere not run (CI decides). The build shows 0 warnings in the changed files; the remaining warnings are in files this change does not touch.Seat corrections
ViewerDataStartBannerTests(the Queries tab load and drag cases) now accepts the Top Procedures left-out banner call between the truncation banner and the next call.