Skip to content

MCP tool audit fixes: spills, empty reasons, FinOps, dropped databases, object stats (#5633) - #5704

Merged
erikdarlingdata merged 71 commits into
devfrom
fix/5633-mcp-tool-audit
Oct 13, 2026
Merged

erikdarlingdata merged 71 commits into
devfrom
fix/5633-mcp-tool-audit

Conversation

@erikdarlingdata

@erikdarlingdata erikdarlingdata commented Oct 12, 2026 •

Copy link
Copy Markdown
Owner

Claude posting for Erik Darling.

Closes #5633.

This PR fixes the defects from the MCP tool audit in #5633. Several lanes build it on one branch. Each lane writes only its own section below, between its markers.

Items 13 and 14 changed files that PRs #5700 and #5697 also changed. Both of those have merged, so items 13 and 14 are in the second wave (W2A, W2B).

L1: advice text for spills, cross joins and anomaly sigma (items 2, 5, 8)

Why

  • Item 2: QUERY_SPILLS is SUM(delta_spills) over v_query_stats (Darling PgFactCollector.QueryPerf.cs and Lite DuckDbFactCollector.QueryPerf.cs, same SQL). That column is sys.dm_exec_query_stats.total_spills, which counts 8 KB pages, not spill events. The headline said "N query spills". The fact already carries spilling_query_count (rows with delta_spills > 0), so no collector or store change was needed.
  • Item 5: "accidental cross joops" in the PLAN_WARNING investigation. grep -rI joops over the repo now finds nothing.
  • Item 8: ComposeAnomaly printed deviation_sigma beside baseline_mean. When AnomalyGate.Decide runs in the robust frame (bucket has a median or MAD), both sigmas are measured from baseline.Median in MAD-derived sigmas, and both are clamped at AnomalyThresholds.SigmaDisplayCap (25).

What changes (all in PerformanceMonitor.Analysis/FactAdvice.cs unless noted)

  • ComposeQuerySpills: headline and investigation now read "12,800 pages (100 MB) spilled to tempdb" (pages x 8 / 1024), via new SpilledPagesText. The static QUERY_SPILLS block said drill-down is "ranked by spill count"; it now says "ranked by pages spilled (8 KB each)".
  • FactScorer.cs: comment only (the 100/1000 thresholds are pages). No threshold change.
  • Web: server-tabs.js column labels "Total/Min/Max/Avg Spills" and the trend "Spills" are now "... Spilled Pages". JSON keys unchanged.
  • ComposeAnomaly: new AnomalyUsedRobustFrame (median > 0 or MAD > 0, which is exactly when EffectiveRobustSigma is positive; the detectors stamp both as zeros for classical-frame metrics). In the robust frame the sentence names "baseline median" and the median value and says "robust σ"; the classical frame keeps the old wording and the mean. New SigmaText: a sigma at the cap reads "at least 25 robust σ" (constant, not a literal). Applied to peak sigma, window-mean sigma and the headline.
  • ComposeAnomalyWaitProfile: the mean's "robust σ" clause goes through SigmaText (cap handled). ANOMALY_MEMORY_PRESSURE goes through ComposeAnomaly. Ratio, object-growth and object-contention composers print no sigma.
  • Config-change comparison verdict (delta_sigma, clamped by ComparisonBanding): at the cap it now says "at least +25σ" / "at least −25σ".
  • Test: Darling/Darling.Tests/McpToolAuditFactAdviceTextTests.cs (8 tests).

Test plan

  • Build Darling.Tests: 0 Warning(s), 0 Error(s).
  • Reverted FactAdvice.cs to the old text (plus a stub for the new helper): 7 of 8 new tests fail; the classical-frame control passes. Restored.
  • -class McpToolAuditFactAdviceTextTests, FactAdvice, Anomaly, Census, Ratchet, DocCommentHygiene, McpToolsListBudget: Total 731, Failed 1, Skipped 13. The one failure, SlowClassCensusTests.APullRequestIntoMain_SkipsNothing_AndOneIntoDevSkipsTheSameAsBefore, passes alone (43 s); it timed out under the load of the 283 s run.
  • Lite.Tests not run (no Lite file touched; FactAdvice is shared and the Darling test exercises it).
  • Full suite not run (CI decides).

Lite parity: FactAdvice and FactScorer are shared by Lite and Darling, so the composer fixes apply to both. Checked Lite-only text: Lite/Mcp/McpAnalysisTools.cs QUERY_SPILLS recommendations say "Find queries with spills" (a query that spills, correct), Lite DuckDbFactCollector.QueryPerf.cs builds the same page sum. Lite WPF grid headers ("Total Spills", "Min/Max Spills", "Spills Delta" in Lite/Controls/ServerTab.xaml, Lite/Windows/*HistoryWindow.xaml, and the Darling Viewer twins) still carry the old noun; not changed here because they are many hand-written XAML headers with filter-button tags and sort keys.

What the seat should double-check

  • Parked: relabel the WPF grid headers (Lite and Darling Viewer) to "Spilled Pages", and the MCP JSON keys total_spills/avg_spills/min_spills (contract change, deliberately untouched). Tool descriptions that mention "spills" name the DMV column, left as is.
  • spilling_query_count counts v_query_stats rows with spills, which the headline still calls "distinct queries"; not touched.
  • Robust-frame detection reads the stamped baseline_median/baseline_mad; a fact from before Darling: anomaly-engine statistical depth (MAD/EWMA/changepoint; real confidence) #1743 (no such keys) renders the classical wording, same as before.

Changelog:
SECTION: Fixed
ENTRY:

L2: truthful empty reasons for top procedures and trace flags, min server memory 0 vs 16 (items 3, 19, 9)

Why. Three MCP reads told the wrong story. get_top_procedures_by_cpu gave the "two collection cycles" reason for every empty page. get_trace_flag_changes said "fewer than two config snapshots" with snapshot_count 0 after the trace_flags collector had run clean and stored nothing (no flag is on). min server memory (MB) configured 0 beside in-use 16 showed values_match false, though SQL Server always runs 0 as 16.

What changes.

  • New PerformanceMonitor.Common/CollectorRunFacts.cs: the shared "collector ran and found nothing" evidence (successful runs capped at 2, runs inside the window, newest success time and rows_collected) and the shared sentences. Darling reads it from v_collection_log (DarlingCollectorRunFactsReader), Lite from DuckDB (LocalDataService.GetCollectorRunFactsAsync). Only SUCCESS rows count. The cap is a bound parameter (the terminal-LIMIT census forbids a literal).
  • Item 3: fewer than two successful procedure_stats runs gives unavailable with the two-cycles text (now also saying the collector has not completed two runs). Two or more runs with at least one inside the window gives empty: "No procedure executed in the last Nh that matches this request ...". Two or more runs but none in the window gives unavailable ("no successful run inside the last Nh", a collection gap, not an all-clear). I added that third branch so "nothing ran" is never claimed for a window nobody measured.
  • Item 19: get_trace_flag_changes and get_trace_flags (both products): when the newest successful trace_flags run stored zero rows, the answer is "No trace flags are enabled as of the trace_flags collector's latest successful run (T UTC)." The "fewer than two snapshots" text stays only when there is no such run (or the run stored rows). snapshot_count is unchanged (captures that listed a flag).
  • Item 9: new PerformanceMonitor.Common/ConfigValuesMatch.cs; (0, 16) for min server memory (MB) only counts as a match. Used by Darling's DarlingCurrentConfigReader.ServerConfigReadRow, the Darling viewer's ServerConfigRow, and Lite's ServerConfigRow. The web grid reads values_match from the MCP payload, so it follows. ConfigChangeDiff does not compute this flag (it diffs changes), so I left it.
  • Tool descriptions and guide tails are untouched, so McpToolsListBudgetTests pins did not move. get_collection_health note text and the expected-zero list were not touched.

Test plan.

  • Darling.Tests.CollectorRunEmptyReasonTests: 16 passed. Lite.Tests CollectorRunEmptyReasonTests: 11 passed (DuckDB-backed: seeds collection_log and calls the real tools).
  • Reverted-shape proof (Lite): with HasDeltaBaseline/LastRunFoundNothing forced false and the match rule reduced to equality, 4 of 11 failed (after-run procedures, window gap, trace flags after an empty run, and the (0,16) case). Restored afterwards.
  • Darling census, ratchet, McpToolsListBudget and my class, before the last merge: 536 run, 1 failure (SlowClassCensusTests.APullRequestIntoMain_SkipsNothing..., which passed alone, 35 s; load-sensitive, unrelated). It had also caught two real problems in my first draft, both fixed: a literal LIMIT 2 (census wants a bound cap) and my helper placed between a tool and its body (the bounded-parameter census lost the tool).
  • NOT RUN: the Lite census/ratchet/EngineCapabilityMiss classes after the final changes (the 60-minute wall hit as the Lite build started). Lite's own class passed earlier, before the bound-LIMIT change.
  • NOT RUN: a build and tests after the final merge of origin/fix/5633-mcp-tool-audit (other lanes' code came in). Please rebuild both test projects; I expect no conflict, but it is unverified.
  • NOT RUN: a live PostgreSQL test of DarlingCollectorRunFactsReader (SQL shape is pinned; both statements are plain reads of v_collection_log). No rig was started.

Lite parity. Item 3: McpQueryTools.GetTopProceduresByCpu. Item 19: McpConfigHistoryTools.GetTraceFlagChanges and McpConfigTools.GetTraceFlags. Item 9: LocalDataService.Config.cs ServerConfigRow. Lite's old trace-flag-changes text never said "fewer than two ... captured" with a count; it said "at least two snapshots are needed", which was also wrong after an empty run, so it uses the same shared sentence.

What the seat should double-check.

  • The defect this lane found (turning off the LAST trace flag never showed as a change, in either product) is fixed by F4 on this PR; see the F4 section.
  • A new third procedure_stats outcome (unavailable, runs exist but none in the window) is a behavior addition beyond the ruling.
  • A failed collection_log read on these empty paths now propagates to the tool's error envelope rather than falling back to the old (false) text.

Changelog:
SECTION: Fixed
ENTRY:

L3: audit_config edition, compare_analysis overlapping windows, get_read_latency percentiles above max (items 10, 12, 23)

Why

  • Item 10: audit_config printed "edition": "Enterprise" for a Developer Edition server, because it named engine edition 3 (Enterprise, Developer and Evaluation all report 3).
  • Item 12: compare_analysis only checked baseline_hours_back > hours_back, so the baseline window (same length as the comparison) could overlap it. The default stayed 28 whatever hours_back was.
  • Item 23: get_read_latency returned p50/p95/p99 as histogram bucket upper bounds, so they could sit above max_ms (max 3,568 with every percentile 5,000).

What changes

  • Item 10 (DarlingMcpTools.AuditConfig, McpAnalysisTools.AuditConfig in Lite): edition is now the stored Edition string from the latest server-properties snapshot (the value get_server_properties returns). New AuditEditionLabel(storedEdition, engineEdition) falls back to the engine-edition label only when the store has no Edition string, and then a new edition_note field says it is the engine edition, not the SKU. edition_note is null otherwise. No audit rule keys on the label (checked: editionName is only echoed in the payload, and the pins forbid branching on edition), so Developer and Evaluation advice is unaffected. The tool description is unchanged.
  • Item 12: new shared McpHelpers.ValidateBaselineWindow (PerformanceMonitor.Common), called by both products. baseline_hours_back is now int?. Omitted: hours_back + 24 (4 still gives 28), raised to 2 x hours_back when that is larger (hours_back above 24), and refused with a "no room" message past hours_back 84 because the 168-hour ceiling cannot hold a disjoint baseline. Explicit and below 2 x hours_back: refused as overlapping, naming both windows ("the last 4 hours" and "hours 7 to 3 back", baseline_hours_back 7 is below 2 x hours_back (8)). Darling web: the compare_analysis route now passes null when the query string omits baseline_hours_back (new QueryNullableInt), and the catalog entry no longer advertises a default of 28. Tool head now reads "(>= 2 x hours_back, else refused)" and the parameter text "Baseline start, hours back from the comparison's end. Default hours_back + 24; at least 2 x hours_back." (shorter than before, so the budget pins went DOWN: tool 611 to 606, parameter 139 to 103, in both products). The guide tail states no default, so it is unchanged.
  • Item 23: ReadLatencyPercentiles.FromBuckets takes an optional maxMs; a bucket bound above it is capped to it. This is the only caller path (MCP tool; the web route calls the tool), and Lite has no get_read_latency. A capped value is no longer a bucket bound but is still an upper-bound estimate (the true percentile is at or below max), so *_is_at_least stays false and the overflow-bucket "at least" floor is untouched. BucketEstimateNote and the tool guide now say a bound above max_ms is capped to max_ms. No new response keys.
  • Existing pins updated deliberately: AuditConfigEditionNameTests source pin, McpToolGuideHeads.SqlCore.cs (Darling and Lite) guardrail fact, McpPayloadContractCensusTests shared-producer regex (adds ValidateBaselineWindow), GetReadLatencyLiveTests (p99 90,000 becomes 61,000 for max 61,000; p95/p99 100 becomes 90 for max 90), Lite AuditConfigEditionTests (was asserting the engine name over a seeded Edition of "Test Edition"; now asserts the stored string and a null note).

Test plan

  • New McpToolAudit5633L3Tests (Darling and Lite): stored edition reported, fallback label plus note, source pins on the tool bodies, ValidateBaselineWindow accept/refuse/default theories, percentile cap (3,568 case: old shape gives the bucket bound above max, new gives 3,568), bound at or below max untouched, overflow floor kept.
  • Darling targeted run before the final merges: 955 total, 0 failed (new tests, AuditConfigEditionNameTests, DarlingMcpToolsTests, ReadLatencyPercentilesTests, McpToolsListBudgetTests, all *Census*, *Ratchet*, DarlingWebEndpointsTests, McpToolGuide*, DarlingCustomViewsTests, AnalysisAsOfAnchorTests).
  • Lite targeted: *AuditConfig* plus the new class, 13 total, 0 failed. Lite budget, guide, Census, Ratchet and truth-pin classes ran once earlier with only the AuditConfigEditionTests failures that I then fixed.
  • Live on a UTC rig (port 5683, stopped afterwards): GetReadLatencyLiveTests, EngineCapabilityMissTests, PgTargetAuditConfigTests, AnalysisCoverageLivePostgresTests, CompareAnalysisPgBandingLiveTests, SecondaryReplicaScopeLiveTests: 23 total, 0 failed, 0 skipped.
  • Old-shape proof: the percentile test asserts the uncapped call is above 3,568 (the old output); the Lite AuditConfigEditionTests run failed with "Test Edition" vs "Enterprise" on the old expectation, showing the tool now reports the stored string. I did not revert the production code to prove the other pins.
  • After the last two merges of the shared branch and origin/dev (code from other lanes only), I rebuilt Darling.Tests and Lite.Tests (0 warnings) but hit the 60-minute wall before re-running the test classes. Please re-run the budget, guide-head, census and new classes.
  • Full suites not run (laptop rule); CI decides.

Lite parity: audit_config (same edition fix and edition_note), compare_analysis (shared validator, same description and param text, same pins) done. Lite has no get_read_latency, so item 23 is Darling only. Checked by grep: ReadLatencyPercentiles and BucketUpperBoundsMs have no other caller in Lite, the viewer, or the web.

What the seat should double-check

Changelog:
SECTION: Fixed
ENTRY:

L4: percent growth, NULL-last table sizes, heap and clustered usage class (#5633 items 7, 6, 15, 16)

Why. Item 6: get_database_sizes published only auto_growth_mb, so a percent-growth file read as null. Item 15: get_table_index_sizes sorted current_reserved_mb DESC without NULLS LAST (PostgreSQL puts NULLs first), showed an unsized table as 0, and listed the collector's own tempdb # tables. Item 16: get_index_usage labelled heaps and clustered indexes "Unused", a drop-candidate label for the table itself. Item 7 was verify-only.

What changes.

  • Item 7: already pinned. Lite.Tests/CollectorSelfQueryMarkerCensusTests builds every SQL Server collector query over six target shapes and also checks each inner sp_executesql batch on its own; it passes (ran in the Lite census set). DarlingSelfQueryMarkerTests covers Darling's by-id Query Store fetches, detection query and job reads. No statement was missing the marker; no code change.
  • Item 6: DarlingObjectStatsReader.DatabaseSizeLatestSql and the DatabaseSizeRow record gain is_percent_growth and growth_pct; DarlingMcpObjectStatsTools.FilePayload publishes both. Lite: LocalDataService.GetLatestDatabaseSizeStatsAsync, DatabaseSizeStatsRow and McpServerInfoTools.FilePayload the same.
  • Item 15: ObjectSizeGrowthSql (Darling) and the DuckDB twin order current_reserved_mb DESC NULLS LAST plus database, schema and table names as tie-break, and drop #-prefixed tables from the latest-snapshot CTE. CurrentReservedMb and CurrentUsedMb are now nullable on both products' rows, so the payload says null, not 0. IndexObjectStatsCollector adds AND o.name NOT LIKE N'#%' (shared by Lite and Darling); stored rows stay.
  • Item 16: the classification CASE gets a first branch: a HEAP or CLUSTERED% index (includes clustered columnstore) with no reads is Table not read; with reads it falls to Active. Nonclustered classes unchanged. Applied in Darling's IndexUsageSql, Darling's FinOps storage-growth reader, and both Lite CASEs (usage and per-table detail). The unused-first sort key now ranks only nonclustered zero-read indexes first, so table storage cannot crowd drop candidates out of the cap. IndexUsageTruncationTests pins that key, updated deliberately for this. Tool text (tail only, head untouched so the 493-char cap and McpToolsListBudgetTests pins did not move): the classification rule names Table not read; the reset caveat now reads "clears on an instance restart, a database restore, a detach and attach, the database going offline, and (for that index) a rebuild"; the "cumulative since the last instance restart" sentence is softened. Per-database counter age is NOT reported: the store holds only the instance start time (sqlserver_start_time), no per-database one.

Test plan.

  • New Darling.Tests/McpToolAudit5633Tests.cs (4 pins plus 3 live tests on a UTC rig, port 5684) and Lite.Tests/McpToolAudit5633Tests.cs (5 tests on DuckDB). Reverted the NULLS LAST, # filter and Table not read pieces once: 3 Darling tests failed (SQL pin, table sizes live, usage live); restored, all pass.
  • Darling: McpToolAudit5633, IndexUsageTruncation, GuardStageCensus, McpToolsListBudget, *ObjectStats*, *Census*, *Ratchet*, DocCommentHygiene: 656 passed, 0 failed. Earlier broader set (McpToolGuide, IndexUsage, StorageGrowth, DatabaseSize, ZeroIsAMeasurement): 921 total, the 2 failures (truncation pin, guard-stage trait on my new class) fixed.
  • Lite: McpToolAudit5633, McpToolsListBudget, McpToolGuide, IndexObjectStats, IndexUsage, Census, Ratchet, AzureSiblingSize, ObjectStats, CollectorSelfQueryMarker: 235 passed, 0 failed. Builds 0 warnings.
  • After the final merge of origin/dev and the shared branch (clean merges, no conflicts) I rebuilt both projects (0 warnings) but the wall-clock limit hit before rerunning the classes. The seat should rerun *McpToolAudit5633* in both.
  • Not run: full suites (CI decides).

Lite parity. Items 6, 15, 16 each done in Lite (reads, payload, tool text); item 15's collector filter is shared code. DuckDB already sorts NULLs last by default; NULLS LAST made explicit.

Double-check.

  • The # exclusion also applies to any database, not only tempdb (a user table whose name starts with # would vanish from the read).
  • Table not read is a new classification value; any client matching only Unused/Write-only/Active sees it for heaps and clustered indexes. The Viewer grid shows the class text as is.
  • The PG rig at C:/Users/edarl/AppData/Local/Temp/pmrig-5684 is still running (lane hit the wall-clock tool lock before it could stop it). Stop it with pg_ctl stop -D .../pmrig-5684/data.
  • The Darling Viewer / FinOps storage_growth reader CASE was changed too (same defect).

Changelog:
SECTION: Fixed
ENTRY:

L5: one CPU right-sizing verdict, null inventory cost, Parallelism waits (#5633 items 11, 20, 22; fixes #5644 W18)

Status: Darling and Lite are done and green. This lane hit its 60-minute wall before the Lite tests; F1 updated them (see F1).

Why

  • Item 11 and [BUG] Web UI audit on 3.9.0-nightly.20261006.515: 22 defects (blocking counts, CPU duplicates, FinOps, charts, UI wording) #5644 W18: the 24-hour compute rule fired at P95 under 30%, the VM rule read its own 7-day P95, and the Utilization verdict used a third test (average under 15% and max under 40%). One server could read RIGHT_SIZED (P95 24%) beside "reduce from 16 to 8" (7-day P95 16%) beside "about 4 cores" (24-hour P95 7%).
  • Item 20: the inventory printed monthly and annual cost 0 for a server with no cost. Its guide said 0 meant no budget.
  • Item 22: CXPACKET, CXCONSUMER, CXSYNC_PORT and CXSYNC_CONSUMER were counted as CPU, and HTDELETE, HTBUILD and HTREPARTITION as Other.

What changes

  • Item 11 / W18: new FinOpsRecommendationFigures.CpuVerdictAllowsDownsizing(util) is true only when the Utilization read has a CPU sample and its own verdict is OVER_PROVISIONED. CpuRightSizingTargetCores (the compute row) requires it, and the reader's VM row requires it too. The VM CPU row now reads the Utilization view's 24-hour window and P95 (the same vmUtil read), not a separate 7-day P95; its text says "the last 24 hours". Both rows wait for Cpu24HourWindowWatched. The existing "the row that keeps more cores wins" rule is unchanged, so the list still holds one CPU row. The memory row keeps its 7-day window. ServerHardwareScope.CpuCoreNoun now returns "logical CPUs" off Azure SQL Database (vCores stays on it); the VM row text says "logical CPUs" too.
  • Item 20 cost: inventory rows give monthly_cost_usd and annual_cost_usd null (left out of the row) and cost_reason "monthly cost not set" when the cost is 0. Guide text updated.
  • Item 20 health score: I compared both paths. The inventory's FleetHealthScoreFor and the Utilization tool use the same inputs and window (24-hour P95 CPU, the newest memory row's buffer pool over physical memory, the newest database-size snapshot's free share) and the same FinOpsHealthCalculator. The code already agreed (fixed earlier in b5bc69a, before the audit's base), so I could not reproduce 92 versus 98 without the audit's store. The guide now says the score is from the same inputs and that two calls made across a collection can differ by what that collection changed. If the seat sees a gap on a quiet store, that is a real bug I did not find.
  • Item 22: DarlingFinOpsOptimizationReader.WaitCategorySummarySql now maps SOS_SCHEDULER_YIELD to CPU and CXPACKET, CXCONSUMER, CXSYNC_PORT, CXSYNC_CONSUMER, HTDELETE, HTBUILD, HTREPARTITION to "Parallelism". The get_finops_optimization guide lists the categories.

Test plan

  • Darling.Tests builds, 0 warnings, 0 errors.
  • Targeted Darling classes (FinOpsOptimizationView/GoldenLive, FinOpsRecommendation* incl. golden, tool parity, secondary replica, reads live, ViewerFinOpsRecommendationsTests, FinOpsClickThroughRules, DarlingMcpFinOpsInventoryTools, AzureSqlDatabase*, McpToolsListBudget): all pass after fixes (last run 63 of 63 on the five live and golden classes; the earlier 348-test run had only those 7 expected failures, now fixed). Rig on port 5685, UTC.
  • New pins: CpuRightSizing_FollowsTheUtilizationVerdict_SoItCannotDisagreeWithIt (RIGHT_SIZED at P95 24% gives no row), Row_WithNoCostSet_IsNullWithTheSameReasonTheOtherFinOpsToolsGive, live CpuRightSizing_FollowsTheUtilizationVerdict_OneRowOneWindowOneP95, and the seeded wait test now has HTBUILD/HTREPARTITION/HTDELETE and expects Parallelism first (CXPACKET is no longer CPU). Goldens FinOpsOptimization/golden.json and FinOpsRecommendations/golden.json regenerated (the recommendations golden loses the two CPU rows, because the seed's 22% CPU is RIGHT_SIZED).
  • Not run: the revert-to-old-shape proof; -class "*Census*" and -class "*Ratchet*" in both projects; the whole Lite.Tests project (Lite production code was edited but not built or run).
  • McpToolsListBudgetTests was in the run list for Darling and passed, but the guide text grew by about 600 characters; confirm the total ceiling on CI.

Lite parity

  • Lite/Services/LocalDataService.FinOps.Recommendations.cs: rule 2 requires OVER_PROVISIONED; rule 12 CPU row uses vmUtil.P95CpuPct, requires OVER_PROVISIONED and the 24-hour watch, 7-day CPU query and cpuSpanEnough removed; "logical CPUs" text.
  • Lite/Services/LocalDataService.FinOps.Workload.cs: both wait-category CASE copies now have Parallelism.
  • Lite has no get_finops_inventory tool (item 20 is Darling only). CpuCoreNoun is shared (Common).
  • Lite tests: the edits this lane listed (the "logical CPUs" noun, the VM detail text without the 7-day window, the CPU seed data) were made by F1, which also deleted the CPU helpers the recommendations stopped using (Lite CpuSamplesSpanEnough, Darling GetCpuP95Async/GetCpuSampleSpanAsync/CpuSamplesSpanEnough). Lite's AzureSqlDatabaseOwnFiguresTests noun pin was already updated; the Azure vCores tests were unaffected.

What the seat should double-check

  • Ruling reading: I made the Utilization verdict a necessary condition of both CPU rows rather than merging the two target formulas. At P95 24% on 16 CPUs the verdict RIGHT_SIZED gives no row; at avg under 15% and max under 40% the more conservative target wins.
  • The health-score finding above (no divergence found in code).
  • Line endings: sed-edited files may show LF-to-CRLF warnings.

Changelog:
SECTION: Fixed
ENTRY:

L6: dropped databases are no longer reported as current (item 4)

Why. The Query Store clutter tool listed 13 databases, several dropped, one with a Critical verdict. FinOps recommendations and index analysis listed dropped databases, "compress dbo.r" named no database, and get_top_procedures_by_cpu over 168 hours returned only a dropped database's rows. The backfill retry part was already fixed by #5486.

What changes.

  • New CurrentDatabaseScope (Darling.Storage): the current-database set is the newest database_states snapshot per server, read with QueryStoreBackfillState.NewestSnapshotSql, the same SQL text The Query Store backfill stops retrying databases that were dropped from the server (#5483) #5486's DropGoneDatabasesAsync uses (no second source). It returns null (filter nothing) with no snapshot (older store, Azure SQL Database), a snapshot older than 6 hours, a snapshot with no names, or a failed read.
  • Query Store clutter (GetReadCostAsync, GetPlanChurnAsync, GetConfigAsync): rows of a dropped database are OMITTED when no database is chosen, so a dropped database cannot carry a verdict, a count or a recommendation. Omitted rather than marked database_dropped, because the verdict counts and fleet rollups are built from these rows and a mark would need every consumer to learn it.
  • FinOps index analysis (ReadIndexObjectStatsAsync, the one read the analysis, the desktop tab and the compression rule share): rows of a dropped database are omitted (same reason).
  • FinOps recommendations: idle, dev/test, TDE-blocker and storage-tier rules skip dropped databases; compression text now names the database (SalesA.dbo.T1 (2.0GB), falls back to schema.table when the input has no database). Golden fixture updated for it.
  • get_top_procedures_by_cpu (Darling): with no database named, the filter is narrowed to the current set, so the SQL is unchanged and the rank's LIMIT no longer fills with a dropped database. A database the caller NAMES is read as asked (a question about its history).
  • Edits to the FinOps recommendations function are limited to the filter lines and one read at the top; L5 conflicts should keep both sides.

Plan cost. The lookup is one indexed read per call (MAX(collection_time) on idx_database_states_time (server_id, collection_time) with a 6-hour lower bound, then one row per database). It never touches a fact table. I did not run EXPLAIN on a large store; no fact-table SQL was changed.

Lite parity. Lite's get_top_procedures_by_cpu gets the same narrowing (LocalDataService.GetCurrentDatabasesAsync, v_database_states, same SQL text and horizon). NOT done (parked, out of time): Lite FinOps has no equivalent of Darling's index analysis read; its compression rule is a live sys.partitions scan of the monitored server, which cannot return a dropped database, and its recommendation text names no database there either (same text "schema.table"; left as is). Lite has no clutter tool I checked.

Test plan.

  • DroppedDatabaseScopeLiveTests (4 tests, rig port 5686): FinOps (no snapshot, stale snapshot, newest snapshot dropping two databases; recommendations, index inputs, index analysis rollups), top procedures (top 1 page, named dropped database, no snapshot), clutter config and plan churn, rule unit tests, compression text. Total 4, 0 failed. Proven to fail on the old shape: with ReadAsync forced to return null, 2 of 4 fail.
  • Lite CurrentDatabasesTests 1 pass; Lite *Census* 67 pass; DatabaseStateNoVerdict 10 pass.
  • Darling *QueryStoreClutter* 67 pass, *TopProcedures* 16 pass.
  • *FinOpsRecommendations*: 87 of 89 passed; the 2 golden tests failed only on the new SalesA. prefix; fixture updated in the second commit and the golden class re-run: 2 of 2 pass. *FinOpsIndexAnalysis* 50 of 50 pass.
  • Darling *Census*, *Ratchet*, *StorageCommandTimeout*, *DocCommentHygiene*, *LivePostgresCollectionHygiene*: started, results not read before the wall. CI decides.
  • Merged origin/fix/5633-mcp-tool-audit (no conflicts) but did not merge origin/dev or rebuild after it.

Seat should double-check. (1) The golden fixture edit (golden.json, server a). (2) Any Viewer test asserting compression Detail with a non-empty database name. (3) A database created after the newest snapshot is hidden until the next snapshot (30 minutes by default); the backfill's "row since the snapshot" guard was not copied. (4) Lite index/FinOps parity above.

Changelog:
SECTION: Fixed
ENTRY:

L7: panel server names, waiting-task rollup, plan DOP (items 17, 18, 21)

Why

  • 17: run_custom_view_panel matched the spec's server only against the stored server_name, so a display name returned 0 rows. Re-checked on current code: the unknown-server half is already answered by W13 (not_found, "No server named 'X' is registered", pinned in DarlingMcpCustomViewToolsLivePostgresTests), so only the display-name half was still broken.
  • 18: get_waiting_tasks returned one row per waiting thread, so a parallel query looked like 30 duplicates, 29 "blocked by" their own session.
  • 21: cache and Query Store plans carry no QueryPlan/@DegreeOfParallelism; the parser cast the missing value to 0, so the three plan tools said dop: 0 for a parallel plan.

What changes

  • 17: ComposeServerScope.ResolveNames[Async] runs once per panel run (shared runner, so MCP and web alike) and places each scoped name through DarlingServerResolver.ResolveOrError (exact, display name, partial, case-insensitive). An exact storage name is kept as written. A name matching several servers is refused with the resolver's own sentence. A name matching nothing is left as written, so a removed or disabled server's stored history (Custom Views and custom alert rules filter raw reads by server name, so every compressed chunk is read in full #5525) and W13's unknown-server answer both keep working. Registry read fault: fail open, like the unregistered-name lookup.
  • 18: WaitingTasksSql (Darling) and Lite's GetWaitingTasksAsync group by capture, session, wait type, database: wait_duration_ms is the max, thread_count is new, blocking_session_id is only a blocker other than the session itself, and new intra_query_parallelism flags threads waiting on their own session. No new column, no migration. Tool description (both products) gets one GUIDE sentence.
  • 21: StatementInfo.HasDegreeOfParallelism (new); the formatter emits dop as null when the attribute is absent and a new parallel flag from operators marked Parallel="1". The int DegreeOfParallelism stays 0 when absent, so every analyzer/benefit/UI reader (> 1, > 0, == 2, stmt.DegreeOfParallelism > 0 ? ... : 1) behaves exactly as before. PlanAnalyzer.cs and PlanRegex.cs are untouched.

Test plan

  • PlanDopParallel5633Tests (4), WaitingTasksRollup5633Tests (SQL pin + live), ComposeServerScopeResolveNamesTests (4), live CustomViewTools_CrudAndRunRoundTrip extended (display name and upper-case spelling return rows): 16/16 pass on a UTC rig (port 5687), after merging the shared branch.
  • Reverted the three product files: the 4 plan tests, the live rollup test and the live panel test fail (6 of 7 failed); restored.
  • Darling: McpToolsListBudget, every *Census*, *Ratchet*, McpToolGuide, McpPageContract, PlanSync*, McpPlanAnalysis*, BenefitScorer*, DarlingMcpPlanTools*: 1168 passed, 0 failed (before the final merge). The description change fit the existing budget pins; none changed.
  • Lite: new WaitingTasks_ParallelQueryThreads_... plus WaitingBlockedDatabaseFilterToolTests, McpToolsListBudget, Census, Ratchet, McpPageContract, McpToolGuide, McpWindowNoticeTool, DataStartBanner: 453 passed, 0 failed (before the final merge).
  • Not run: full suites, origin/dev merge (shared branch merged twice, dev not merged for time), Lite rebuild after the final merge.

Lite parity

  • 18: done (DuckDB v_waiting_tasks, same grouping, McpWaitTools output, twin test, same description sentence).
  • 21: the formatter and parser are shared, so Lite's plan tools get it. 17: Lite has no custom-view panel runner.

Parked

  • 17, validate_custom_view flagging an unknown server: a view definition carries no server scope (the server is the run spec's server or the $server variable at run time), and the tool is a pure, store-less validator. Flagging would need a store parameter on the tool, which touches the DI-seat census. Not done; the run itself already answers unknown names.
  • 17, the ruling's "refuse a name that matches no server" is met by W13's existing not_found (only when nothing is stored under the name), not by an up-front invalid refusal, to keep Custom Views and custom alert rules filter raw reads by server name, so every compressed chunk is read in full #5525's removed-server history readable. Say if the seat wants the harder refusal.
  • 21: EffectiveDOP still casts a missing attribute to 0; nothing emits it (UI shows it only when > 0), so no payload change was needed.

What the seat should double-check

  • Item 18 changes get_waiting_tasks row counts and limit meaning (the cap now counts rolled-up rows); the aggregate runs over the whole window before the cap.
  • Item 17 adds one registry read per scoped panel run.

Changelog:
SECTION: Fixed
ENTRY:

F1: make the merged branch build and pass (integration lane)

Why. Seven wave-1 lanes merged onto one branch. Lane L5 changed the CPU right-sizing rules (Utilization verdict gate, one 24-hour window and P95, "logical CPUs" wording) and left Lite's tests, dead readers and a few pins behind; other lanes' new rows and new test class broke Darling tests and a census. Merging origin/dev was a no-op (already up to date at head aa7415e).

What changes (head 3a2729e, three commits, pushed to fix/5633-mcp-tool-audit)

  • Build: one warning on the merged tree, CA1068 on CurrentDatabaseScope.ReadAsync (CancellationToken not last). nowUtc now comes before the token; the two internal callers and DarlingFinOpsRecommendationsReader pass cancellationToken: by name.
  • Lite FinOpsTests: "cores" became "logical CPUs" in the four CPU-row assertions, the Azure no-vCores check also rejects "32 logical CPUs", and the Azure vCores test also asserts the text does not say "logical CPUs". The two tests that asserted the removed 7-day detail are replaced:
    • VmRightSizing_AWeekOfSamples_CpuDetailNamesTheUtilizationWindow_NotTheSevenDaysObserved: a week of sparse samples plus a covered day; the detail starts "From the last 24 hours, P95 CPU" and has no "samples over".
    • VmRightSizing_BusyClusterFourDaysAgo_DoesNotMoveTheTwentyFourHourP95_SoItCannotDisagreeWithTheUtilizationView: 90% CPU four days ago, 8% in the last day; the row must quote P95 8.0%. A separate 7-day read would blend the busy cluster in.
    • The seeds already met the OVER_PROVISIONED rule (avg 8%, max 8%, so under 15% and 40%); only the text failed. Deleted CpuSamplesSpanEnough_NeedsAFullDay (covered only the dead helper).
  • Dead code, confirmed by grep (zero callers anywhere) and a clean compile, then deleted:
    • Darling: GetCpuP95Async, GetCpuSampleSpanAsync, CpuP95Sql, ViewerDataService.RecommendationsCpuP95Sql, FinOpsRecommendationFigures.CpuSamplesSpanEnough and CpuRightSizingMinSpan.
    • Lite: CpuSamplesSpanEnough, CpuRightSizingMinSpan, GetCpuSampleSpanAsync.
    • Tests that covered only those: the CPU P95 block of FinOpsRecommendationsReadsLiveTests, CpuRightSizing_NeedsAFullDayOfSamples, the CpuP95Sql InlineData rows and one delegate assertion.
  • Darling pins updated for the wave's own changes (reason each):
    • RightSizingWindowTests.ViewerRightSizing_...: the "no literal 'the last '" guard now allows exactly the one "the last 24 hours" window the CPU row names, and nothing else.
    • RightSizingWindowTests.Readers_TakeTheWindowFromThoseColumns_...: renamed ..._AndTheCpuRowNamesTheUtilizationWindow; it asserts var cpuWindow = "the last 24 hours", no "recent samples", no cpuReader.
    • FinOpsTabServerInventoryPageTests.EveryShownColumnKeyIsEmittedByTheInventoryRow: 27 to 28 emitted keys (item 20 added cost_reason).
    • FinOpsOptimizationViewLiveTests: wait category rows 5 to 6 and 9 to 10 (item 22 added Parallelism as its own category).
  • WaitingTasksRollup5633Tests (live, shared store) was missing [Collection("live-postgres")]; LivePostgresCollectionHygieneTests.EveryClassUsingTheSharedStore_IsSerializedOrDocumentsWhyNot failed on it. Added.
  • New Lite pin WaitCategorySummary_ParallelismWaits_AreTheirOwnCategory_NotCpuOrOther (Lite had the Parallelism SQL change from item 22 with no test).

Revert proofs (item 4). Reverted the product change once, rebuilt, ran, restored (git checkout -- on those files, then rebuilt):

  • Gate back to != NotApplicable: CpuRightSizing_FollowsTheUtilizationVerdict_SoItCannotDisagreeWithIt and ..._OneRowOneWindowOneP95 (live) both FAIL.
  • monthly_cost_usd = s.MonthlyCost: Row_WithNoCostSet_IsNullWithTheSameReasonTheOtherFinOpsToolsGive FAILS.
  • CXPACKET back into CPU, HT* out of Parallelism (Darling reader): the Parallelism wait-category tests FAIL (OneRowPerSection_ReadsFieldByField_AgainstLiteralValues, HoursBack_WidensWaitsAndQueries..., plus the count tests).
  • Lite Parallelism SQL reverted: my new Lite pin FAILS (restored, passes).
  • Not reverted: my two replacement Lite CPU tests (reasoned: the old 7-day read printed "From 17 samples over 6 days" and blended the old busy cluster into its P95).

Test plan

  • Darling.Tests and Lite.Tests build: 0 Warning(s), 0 Error(s).
  • Darling, live rig (PG on port 5681, UTC, timescaledb + pg_stat_statements preloaded; stopped): all classes named in the brief plus *McpTool*, *Hygiene*, *Timeout*: 2183 run, 1 failed (the collection attribute above), then fixed. Hygiene, DocComment, Pin, Meta, AlertReadFailureSurface, ViewerSchemaVersion, Inventory, Ratchet, WaitingTasksRollup5633 after the fix: 942 run, 0 failed, 2 skipped.
  • First Darling run (before fixes): 1644 run, 7 failed: the two RightSizingWindow pins, the inventory key count, and four FinOpsOptimizationViewLiveTests (three count assertions, plus ReadRoute_ReturnsTheToolsBody). ReadRoute_ReturnsTheToolsBody passed alone and in the later full run; it compares two reads with a 10 second tolerance on effective_start, so I read it as load-sensitive on this laptop, not a code failure. Not seen failing a second time.
  • Lite: 378 run (first pass after Lite edits) then 261 + 394 on re-runs including the new pin: 0 failed at the end. One failure in a re-run was my own stale binary after a revert experiment (rebuilt, passes).
  • Full suites not run (brief: targeted classes plus every Census and Ratchet class; CI decides).
  • Installer.Tests, deprecated/ untouched.

Line endings (item 6). For every file the branch changes against origin/dev (72 modified files), compared the CR/LF mix of the blob on the branch with the blob on dev: no file flipped. No added or modified blob is CRLF or mixed (git ls-files --eol), and no file has a whole-file diff.

Lite parity. Dead-code removal in both products; Lite FinOps tests brought to the new rule; Parallelism wait category now pinned in Lite as well as Darling.

What the seat should double-check

  • CurrentDatabaseScope.ReadAsync parameter order changed (only caller is the recommendations reader; pass cancellationToken: by name).
  • I left ViewerDataService.RecommendationsMemoryP95Sql and the memory reads alone (still used, 7-day by design).
  • The first Darling run's ReadRoute_ReturnsTheToolsBody failure: say if you want that tolerance widened; I did not touch it.
  • Browser tools and playwright-cli: not used (no web UI diff).

Changelog:
(none: test, pin and dead-code changes only; no user-visible change beyond what the other lanes' entries already name)

F2: Compose server scope is exact-match only; the current-database set honours as_of and keeps newer rows; Lite recommendations skip dropped databases

Fixes review round 1 findings M1, M2 (plus the new-database leftover from L6), M3 and L-b, and verifies the item 17 ruling.

Why

  • M1: a panel spec naming a removed or disabled server ("SQL01", history still stored, Custom Views and custom alert rules filter raw reads by server name, so every compressed chunk is read in full #5525) was remapped by the resolver's partial tier to an enabled "SQL01-DR", so the panel showed another server's rows.
  • M2: the current-database set was always today's snapshot, so get_top_procedures_by_cpu with an as_of days back hid a database dropped since. Also a database created after the newest snapshot (up to 30 minutes old) was hidden by the clutter reads until the next snapshot.
  • M3: Lite's dormant-database rule (6) and storage-tier rule (13) read DuckDB history with no current-database filter, so a dropped database was still listed and costed; Darling filters the same rules.
  • L-b: two UTC stamps in CollectorRunFacts used the host culture's time separator.

What changes

  • ComposeServerScope.ResolveNames: a scoped name resolves only by an exact match (stored server_name, then display name, case-insensitive). No exact match leaves the name as written (the Custom Views and custom alert rules filter raw reads by server name, so every compressed chunk is read in full #5525 history path). Several exact matches still get the resolver's refusal. Item 17 check: an unknown name goes through the shared run path (ComposeServerScope.FindUnregisteredAsync then DarlingWebEndpoints.UnknownServerMessage), which returns not_found naming the name; the web panel run and the MCP run_custom_view_panel both use that one runner, so both give the message (test UnknownName_IsReportedNotFound_NamingIt).
  • CurrentDatabaseScope: new ReadSnapshotAsync (names plus the snapshot's collection time); ReadAsync and NarrowAsync take asOfUtc. With an anchor the read uses the new QueryStoreBackfillState.NewestSnapshotAtOrBeforeSql (the same text plus collection_time <= $3) with the 6-hour horizon measured from the anchor. DarlingMcpDataTools passes windowEnd only when as_of was supplied. OmitDroppedAsync takes an optional timeOf; a row stamped after the snapshot is kept (no extra query). Wired for the three clutter reads (last slowest / last collection / captured at) and the index-analysis read (collection time). Parameter order follows the branch's CA1068 rule (cancellation token last).
  • Lite: GetCurrentDatabasesAsync(serverId, nowUtc, asOfUtc) uses the same at-or-before SQL against v_database_states; McpQueryTools passes the anchor when as_of is set. GetRecommendationsWithNoteAsync reads the set once and drops dropped databases from the idle-database rule (6) and the low-IO-latency rule (13), unknown means no filter, name-only like Darling's reader.
  • CollectorRunFacts: both stamps format with CultureInfo.InvariantCulture.

Test plan

  • Darling targeted: ComposeServerScope*, CollectorRunEmptyReasonTests, DroppedDatabaseScopeLiveTests (live, rig port 5682, UTC, not skipped), ComposeUnknownServerMessageTests, plus every *Census*, *Ratchet*, DocCommentHygiene, AlertReadFailureSurfaceTests, LivePostgresCollectionHygieneTests, StartupCommandTimeoutTests, StorageCommandTimeoutTests, *QueryStoreBackfill*, *FinOpsRecommendations*: 871 run, 1 failed. The failure was LivePostgresCollectionHygieneTests on WaitingTasksRollup5633Tests (another lane's class, missing its collection attribute); the branch has since taken a fix for it in a later merge, not re-run here.
  • Lite: CurrentDatabasesTests, every *FinOps*, *Census*, *Ratchet*, McpToolsListBudget*, DocCommentHygiene: 254 run, 0 failed.
  • Builds: Darling.Tests and Lite.Tests, --no-incremental, 0 Warning(s) (a first CA1068 warning on OmitDroppedAsync was fixed).
  • Old shape fails (proved by reverting the source, tests kept): M1 (4 tests fail: the review's SQL01/SQL01-DR case and three partial names), L-b (culture test fails), M2 as_of half and the new-database half of the live test (each fails alone: Expected ["LiveDb","NewDb"], Actual ["LiveDb"]), Lite M3 (both new recommendation tests fail). The Lite AsOf_UsesTheSnapshotAtOrBeforeIt and the live ReadAsync(asOfUtc) assertions use a new parameter, so on the old shape they fail to compile rather than at runtime.
  • Not run: the full suites (the brief replaces them with targeted plus Census and Ratchet; CI decides).

Lite parity
Checked each: M1 (compose is Darling only; Lite has no compose), M2 as_of (Lite McpQueryTools and GetCurrentDatabasesAsync changed), M3 (Lite is where the bug was; rules 1, 4, 5, 7 are live queries and already current, so only 6 and 13 needed it), L-b (shared Common file, both products). Item 3's keep-newer-rows is Darling only: Lite's set is applied as a name filter before the ranking read and as a name filter on the two recommendation rules, as Darling's NarrowAsync and recommendations reader do, and Darling's recommendations reader is also name-only, so Lite omits no rows by timestamp.

What the seat should double-check

  • A database created in the last 30 minutes is still invisible to the ranking narrowing (NarrowAsync filters by name before the read); only the omitted-row reads keep it. Stated in the class comment.
  • M1 behavior change: a partial name in a panel spec no longer resolves to a server (it did before). The existing "ambiguous partial is refused" test was replaced by "partial stays as written" and "several exact matches are refused".
  • NewestSnapshotAtOrBeforeSql is a new const beside the shared one in PerformanceMonitor.Collectors/QueryStoreBackfillState.cs; the backfill itself still uses the original.

Changelog:
SECTION: Fixed
ENTRY:

F3: waiting-task rollup from one thread, source-grep tests replaced, spilled-pages wording

Covers #5633 items 2, 12 and 18 (review round 1 lows L-a and L-c, and lane L1's item 2 leftovers).

Why

  • L-a: the parallel-thread rollup (item 18) took MAX(wait_duration_ms), MAX(resource_description) and MAX(blocking_session_id) independently, so a row could show one thread's wait beside another thread's resource and hide a second, lower-numbered blocker.
  • L-c: two L3 tests only checked that DarlingMcpTools.cs contained certain strings, so a refactor that kept the text but broke the wiring would pass.
  • Item 2 leftovers: the WPF grids still said "Total Spills" and the like for sys.dm_exec_query_stats spill columns that count 8 KB pages; the MCP guide text did not say pages; and spilling_query_count was read as "distinct queries" by the advice text but was a row count.

What changes

  • get_waiting_tasks, both products. wait_duration_ms, resource_description and blocking_session_id now come from the longest-waiting thread of the group (Darling: ordered array_agg; Lite: arg_max over a struct, so a NULL resource or blocker on the winning thread is not skipped). Ties break on the lower blocker, then the greater resource text, the same way in both products. If the longest thread waits on its own session (parallelism), blocking_session_id is the blocker of the longest-waiting thread that has another session as its blocker, so a real blocker is never dropped. New blocking_session_ids (sorted distinct blockers other than the session itself), set only when there is more than one, otherwise null. thread_count and intra_query_parallelism are unchanged. Guide tail of the tool says so; the served head did not change.
  • L-c: AuditConfig_ReadsTheStoredEditionProperty and CompareAnalysis_NoLongerDefaultsBaselineHoursBackTo28 are now live tests that call AuditConfig and CompareAnalysis on seeded PostgreSQL data. The audit test stores Developer Edition (64-bit) with engine edition 3 and expects that string and no edition_note. The compare test expects the overlap and "no room" refusals, and with hours_back 24 and no baseline expects the baseline to start 48 hours before the anchor (and not 28).
  • Spilled pages. The six WPF grids/windows (Lite ServerTab.xaml, Lite and Darling Viewer ProcedureHistoryWindow.xaml and QueryStatsHistoryWindow.xaml, ViewerServerTab.xaml) now say "Total/Avg/Min/Max Spilled Pages", "Spilled Pages" and "Spilled Pages Delta", matching server-tabs.js. Only header text and, for the longer labels, column widths changed; every binding path, sort tag and filter tag is as before. get_top_queries_by_cpu guide tail (both products) says total_spills counts 8 KB pages written to tempdb, not spill events (Darling's detail='full' sentence too). The JSON keys are unchanged.
  • spilling_query_count: it was COUNT(CASE WHEN delta_spills > 0 ...) over v_query_stats, one row per query per snapshot, so one query spilling in thirty snapshots counted thirty times and the investigation text said "across 30 distinct queries". I chose to count distinct queries rather than reword the text, because the text and the fact name already say distinct queries and the drill-down groups by (database, query_hash): it is now COUNT(DISTINCT database_name || '/' || query_hash) over rows with a spill, in both collectors. A row with a NULL hash counts once per database.

Test plan

  • Darling.Tests, classes WaitingTasksRollup5633Tests, McpToolAudit5633L3Tests, QuerySpillsDistinctQueryCountLiveTests, McpToolsListBudget*, every *Census* and *Ratchet* class, McpToolAuditFactAdviceTextTests, ViewerHistoryWindowTests: 572 run, 0 failed, 0 skipped (live rig on port 5683, UTC, both preload libraries).
  • Lite.Tests, classes WaitingBlockedDatabaseFilterToolTests, QuerySpillsDistinctQueryCountTests, McpToolsListBudget*, every *Census* and *Ratchet* class: 90 run, 0 failed. Earlier, with *FactCollector* and *QueryHighDop* added: 169 run, 0 failed.
  • Fails on the old shape, proved by reverting: with MAX(resource_description) back in the Darling SQL, DisagreeingThreads_... and the SQL-text test fail; with AuditEditionLabel(null, edition) in AuditConfig, AuditConfig_EchoesTheStoredEditionString_... fails; with the old row count, both spill-count tests fail with 4 against the expected 2.
  • Full suite not run on this laptop (brief's rule); CI decides.
  • WPF headers not looked at on screen; the wider columns (140, or 120 for "Spilled Pages") are a judgment, build is clean.
  • The L-c compare test proves the baseline instant, not a data-bearing comparison.

Lite parity

  • Waiting-task rollup: LocalDataService.WaitingTasks.cs (DuckDB arg_max) and WaitingTaskRow.BlockingSessionIds, McpWaitTools.GetWaitingTasks payload and guide; test WaitingTasks_DisagreeingThreads_... in WaitingBlockedDatabaseFilterToolTests.
  • Spilled pages: Lite ServerTab.xaml plus its two history windows, McpQueryTools guide, DuckDbFactCollector.QueryPerf.cs, test QuerySpillsDistinctQueryCountTests.
  • L-c tests are Darling-only (Lite has no such source-grep test pair).

What the seat should double-check

  • blocking_session_ids is always present in the payload and null unless there is more than one other blocker. If you would rather omit the key when null, say so.
  • The fallback for a self-blocked longest thread picks another thread's blocker, so blocking_session_id and resource_description can then come from different threads (the resource stays the longest thread's). I judged showing a real blocker better than hiding it.
  • The grid column widths in the Lite and Darling Viewer XAML.
  • WPF grids elsewhere (Dashboard is deprecated and untouched) that may still say "Spills" in a place I did not grep, such as a code-built header.
  • server-tabs.js was only read for its labels; not touched.

Changelog:
SECTION: Fixed
ENTRY:

F4: turning off the last enabled trace flag now shows as a change (item 19)

Why. ConfigChangeDiff.DiffTraceFlagChanges set-diffs consecutive trace_flags captures, but the collector stores one row per enabled flag and nothing at all when none is enabled. A run that finds no flag therefore left no capture to compare, so turning off the LAST enabled flag (and turning on the first one after an empty run) never appeared as a change, in either product, in the tool, the UI or the analysis findings.

What changes.

  • PerformanceMonitor.Common/ConfigChangeDiff.cs: new overload DiffTraceFlagChanges(snapshots, emptyRunTimes, start, end). Each time in emptyRunTimes is an empty snapshot (a capture with no flags); a time that already holds flag rows is ignored. The old two-argument signature delegates with an empty list, so nothing else changes.
  • The empty runs are the SUCCESS rows of collection_log for trace_flags with rows_collected = 0 (a failed run, or a NULL count, is never an empty snapshot). The read is bounded like the change read: the runs inside the window plus the single newest run before the window (the baseline for a change on the window's first capture), never a scan of the log.
    • Darling MCP get_trace_flag_changes: DarlingCollectorRunFactsReader.ReadEmptyTraceFlagRunsAsync.
    • Darling viewer: ViewerDataService.GetTraceFlagChangesAsync (EmptyTraceFlagRunsSql).
    • Darling analysis (CONFIG_CHANGED attribution): DarlingAnalysisService.EmptyTraceFlagRunsForAttributionSql; the empty run times also join the capture times given to GroupIntoEvents, so an event's previous capture is right.
    • Lite: LocalDataService.GetEmptyTraceFlagRunsAsync (feeds the tool and the UI through GetTraceFlagChangesAsync), and AnalysisService with the same SQL.
  • The empty answer text of get_trace_flag_changes (both products) used to say that turning off the last flag "does not appear as a change here". That is no longer true, so it now says a successful empty run counts as an empty capture and no turn-off falls in this window.
  • ConfigChangeAttribution remarks: blind spot (2) is closed.

Test plan.

  • Pure diff, Darling (ViewerConfigChangesDiffTests): on then empty run is "disabled" at the run's time; on then no empty run is no change; empty then on is "enabled"; an empty run stamped at a capture that has rows is ignored and repeated empties add one turn-off.
  • Lite reader (ConfigChangesReaderTests, real DuckDB): on then SUCCESS/0 run gives "DISABLED"; on then ERROR, PERMISSIONS and SUCCESS/NULL runs give no change; SUCCESS/0 run 30 h ago (before the 24 h window) then on gives "ENABLED" (baseline arm).
  • Darling live (TraceFlagEmptyRunLivePostgresTests, rig on 5684): the MCP reader, the viewer SQL and the analysis SQL return the same two times (the in-window empty run and the newest one before the window; not the ERROR, PERMISSIONS, NULL-count or other-collector rows, not the older pre-window run); the tool returns "disabled" then "enabled" newest first; on then only a failed run is no change.
  • Failed on the old shape once: with the new overload ignoring emptyRunTimes, the Darling live test and 2 of the 3 Lite tests failed (the no-change test passes either way, by design).
  • Darling: *ConfigChange*, *ViewerConfigChanges*, *TraceFlag*, *CollectorRun*, *Census*, *Ratchet*, DocCommentHygiene, Startup/StorageCommandTimeout, LivePostgresCollectionHygiene, AlertReadFailureSurface, McpToolsListBudget: 1045 run, 0 failed, 3 skipped. Lite: *ConfigChange*, *TraceFlag*, *CollectorRun*, Census, Ratchet, DocCommentHygiene, McpToolsListBudget: 243, 0 failed. Builds 0 warnings. After the last merge of the shared branch I re-ran only my own classes (Darling 64, Lite 12; 0 failed).
  • Full suites (CI decides). Analysis-pass end to end with an empty run (the SQL is covered live, the diff call is shared; no new analysis test).

Lite parity. Checked all three Lite sites: LocalDataService.GetTraceFlagChangesAsync (tool and UI), AnalysisService.AttributeConfigChanges trace-flag family, and the get_trace_flag_changes empty message. SQL is the same text with v_collection_log.

Double-check.

  • The trace_flags collector runs once on connect, so empty runs are rare; a server with no flags ever has a handful of SUCCESS/0 rows, and the window query reads only those in the window plus one.
  • A "turned off" at the empty run's time means "first successful run that saw no flag", not the exact moment of the DBCC.
  • LivePostgresCollectionHygieneTests failed once mid-lane on another lane's WaitingTasksRollup5633Tests (no live-postgres collection); it was fixed on the shared branch and passed after my merge.

Changelog:
SECTION: Fixed
ENTRY:

F5: trace-flag diff - bound the empty-run baseline at the retention, never log a failed trace_flags read SUCCESS (item 19, round 2 Medium-1, Medium-2, Low-3)

Why. Lane F4 made a successful trace_flags run with rows_collected = 0 an empty snapshot in the trace-flag change diff. Review round 2 found two holes in that. (1) Both trace_flags collectors caught every SqlException and returned 0, so a denied DBCC, a timeout or a dropped connection was logged SUCCESS with 0 rows and read as "every flag turned off". (2) The baseline arm ("the newest empty run before the window") had no lower bound: an empty run older than the oldest trace_flags rows the store can still hold paired with a later capture and read as a flag turned on that had really been on for weeks. In Lite the same arm scanned the whole v_collection_log history on each call.

What changes.

  • Medium-2, Lite: RemoteCollectorService.ServerConfig.cs CollectTraceFlagsAsync no longer has a catch. A failure reaches RunCollectorAsync, which already records PERMISSIONS (SqlServerPermissionErrors.IsPermissionDenied) or ERROR for every other collector.
  • Medium-2, Darling: RunTraceFlagsTolerantAsync is deleted and the trace_flags dispatch entry is the plain r.RunAsync(TraceFlagsCollector.Instance, s, ct). RunOneAsync then records PERMISSIONS or ERROR. DarlingWorker.cs edit (also touched by open PR PostgreSQL log text keeps row values out of the stored text, in their slot #5698 near line 10738, log masking): three hunks, all in the trace_flags collector only - the dispatch line, the deleted wrapper at the end of the file, and two doc-comment sentences that described the tolerance. No masking code touched.
  • The permanent engine limit (Azure SQL Database has no DBCC TRACESTATUS) is not a catch case any more: TraceFlagsCollector.AppliesTo already gates it (!target.IsAzureSqlDb) and both products record SKIPPED / skip it before the run, so collection health does not turn red there. What the old catch still covered was a permission denial, and that is the existing PERMISSIONS pattern (a yellow, deliberate least-privilege state, not a per-cycle ERROR).
  • Medium-1 and Low-3, Darling: new DarlingRetentionHorizons.FleetRetentionDaysSql / TraceFlagsBaselineFloorSql in Storage (DailySummaryHorizon.cs). The retention is the fleet override config.config_collector_schedules.retention_days for trace_flags when set and at least 1, else CollectorScheduleDefaults (30 days) - the same rule as the purge and DataWindowFloor. The baseline arm of all three copies (DarlingCollectorRunFactsReader.EmptyTraceFlagRunsSql, ViewerDataService.EmptyTraceFlagRunsSql, DarlingAnalysisService.EmptyTraceFlagRunsForAttributionSql) gets AND collection_time >= (window start - retention). The three constants are now static readonly (interpolated); every caller (MCP tool, viewer, analysis) goes through them.
  • Medium-1 and Low-3, Lite: Lite has one horizon for every table, RetentionService.ArchiveRetentionMonths (3), and its per-collector retention_days purges nothing (grep: only the Query Store backfill reads it), so the floor is windowStart.AddMonths(-3), bound as $4 in LocalDataService.EmptyTraceFlagRunsSql (EmptyTraceFlagRunsFloor). In the SQL, so DuckDB prunes the archive by zone map. Used by GetTraceFlagChangesAsync and AnalysisService through the one shared reader.

Test plan.

  • Darling live (rig, port 5685, UTC, both preload libraries): TraceFlagEmptyRunLivePostgresTests 3/3 - new AnEmptyRunOlderThanTheRetention_IsNotABaseline_ByEveryDarlingRead_AndAFleetOverrideMovesTheBound (day -50 empty run + capture now: reader, viewer SQL, analysis SQL all empty and the tool shows no "enabled"; with a fleet retention_days = 90 the same run is the baseline again); AFlagOnThenOnlyAFailedRun_IsNoChange now plants both ERROR and PERMISSIONS.
  • Lite: ConfigChangesReaderTests new ...AnEmptyRunOlderThanTheRetention_IsNotABaseline..., ...AnEmptyRunInsideTheRetention_StillIsTheBaseline, TheEmptyRunFloor_IsBoundInTheSql....
  • Source pins for the collectors (a failure needs a live monitored server): TraceFlagsFailedRunPinTests in both projects (Darling: plain dispatch, no tolerant wrapper, one TraceFlagsCollector.Instance in the worker; Lite: CollectTraceFlagsAsync has no catch and no return 0).
  • Proved on the old shape: with the floor reverted to unbounded, the Lite retention test, the Lite SQL-bound test and the Darling live test fail; restored and green. The pins fail on the old code by construction (it had RunTraceFlagsTolerantAsync and a catch).
  • Darling: *TraceFlag*, *Census*, *Ratchet* 564/564 pass; plus DarlingWorkerTests, CollectionLogDrainForensics*, CollectorRunEmptyReasonTests, McpToolsListBudget* pass. Lite: *TraceFlag*, *ConfigChanges*, *Census*, *Ratchet*, *CollectorGateSurface*, *McpToolsListBudget* 106/106. Both builds 0 warnings.
  • LivePostgresCollectionHygieneTests FAILS, but not on my class: it names McpToolAudit5633L3Tests.cs:29 (lane L-a's file, no live-postgres collection or own-store comment). Re-run it after that lane's fix; my live class already carries [Collection("live-postgres")].
  • Full suites not run (laptop rule; CI decides).

Lite parity. Both collectors, both baseline SQLs and both test sets changed together. Checked: Lite AnalysisService.ReadEmptyTraceFlagRunsAsync goes through LocalDataService.ReadEmptyTraceFlagRunsAsync, so the new $4 parameter reaches it with no second edit; the Darling analysis, viewer and MCP copies each carry the floor.

What the seat should double-check.

  • The floor is window start minus retention, as ruled. The purge cuts at now minus retention, so for a long window the bound is slightly more permissive than the strict purge edge; it still rules out the review's scenario.
  • Lite: the 3-month constant is the archive horizon (month files go whole), not a configurable value; Lite has no configurable trace_flags retention to read.
  • DarlingWorker.cs overlap with PostgreSQL log text keeps row values out of the stored text, in their slot #5698: confirm the merge keeps both (my hunks are far from line 10738).
  • A server whose login is denied DBCC now shows PERMISSIONS for trace_flags (before: SUCCESS/0 rows). That is the intended change.

Changelog:
SECTION: Fixed
ENTRY:

F6: dropped-database scope follow-ups and compose server names (round 2 Medium-3, Low-1, F2 leftover)

Why. (1) get_query_store_clutter accepted as_of but always dropped databases using today's snapshot, so an incident review of a window before a drop lost that database. (2) Ranking reads narrowed to an include list of the newest database_states snapshot, hiding a database created after that snapshot for up to 30 minutes. (3) The compose scope's exact-name test saw only enabled servers and did not trim, so an enabled server's display name could take a disabled server's name, and a padded name stopped resolving.

What changes.

  • Medium-3: CurrentDatabaseScope.OmitDroppedAsync has an asOfUtc parameter (passed to ReadSnapshotAsync). The three clutter readers (GetReadCostAsync, GetPlanChurnAsync, GetConfigAsync) take asOfUtc; the tool passes the anchor when as_of is set, on the per-server and the fleet-median reads. Other callers checked: FinOps index analysis and FinOps recommendations (Darling) and GetCurrentDatabasesAsync in the FinOps recommendations (Lite) are current-state tools with no as_of, so nothing to pass. get_top_procedures_by_cpu already passed it (both products).
  • Leftover: NarrowAsync now EXCLUDES only true drops. New CurrentDatabaseScope.ReadDroppedAsync runs one grouped read of database_states (MAX(collection_time) per database since a floor); a name whose newest stamp is older than the newest overall is dropped. Floor = the window start, never nearer than the 6-hour horizon and never further than 7 days (DropLookBack: a drop only matters if it falls inside the ranking window; 7 days bounds the scan to about 336 snapshots). A name no snapshot lists is never excluded. A newest snapshot older than 6 hours is unknown (nothing excluded). No fact-table probe.
  • DatabaseFilter gets the smallest exclusion form: Excluding(...), ExcludedNames, ExcludeParameter(), ExcludeClause(column, index) = ($n::text[] IS NULL OR col IS NULL OR col <> ALL($n)) (NULL-safe on the column). The filter stays IsAll, so a reader that does not apply it reads every database as before. Readers checked: only the top-procedures path takes a narrowed filter (the single NarrowAsync caller). TopProceduresSql (both passes) and TopProceduresHourlySql carry the new $6 line; the hourly ceiling moved from $6 to $7; both bind sites add ExcludeParameter(). The existing $5 list predicate text is unchanged. The viewer has its own copy of the statement and never narrows.
  • Low-1: ComposeServerScope loads EVERY registered server (LoadAllServersSql, no is_enabled filter), trims the name, keeps an exact stored server_name (any registration) before trying display names, then resolves as before (the resolver already prefers a stored-name match over a display-name match, case-insensitively). A name with no exact match stays as written (raw), so removed servers keep the "left as written" path.
  • Lite: QueryStoreBackfillState.LastSeenPerDatabase*Sql (shared text), GetDroppedDatabasesAsync (same rules), GetTopProceduresByCpuAsync(excludeDatabaseNames:) with BuildDbExcludeClause; get_top_procedures_by_cpu uses them. Lite has no clutter tool, so no twin for Medium-3.

Test plan.

  • Build Darling.Tests and Lite.Tests: 0 errors; 0 new warnings (the one warning, xUnit2013 in DarlingOperatorGroupScriptTests.cs, is another lane's file).
  • Reverted once to prove the tests fail on the old shape: trim, enabled-only load and clutter without asOfUtc gave 7 failures (padded names x3, stored-beats-display, the registry-read pin, the live disabled-server test, the clutter as_of test); restored, they pass.
  • New: QueryStoreClutter_WithAsOf_KeepsADatabaseDroppedAfterTheWindow, DroppedFrom_NamesOnlyWhatAnEarlierSnapshotListedAndTheNewestDoesNot, NewDb-ranks assertions added to TopProcedures_AndTheClutterReads_... (heaviest, in no snapshot, takes top 1), compose padded / stored-beats-display / registry-read pin / live ResolveNamesAsync_SeesDisabledRegistrations; Lite Dropped_IsWhatAnEarlierSnapshotListedAndTheNewestDoesNot, TopProcedures_LeaveOutADroppedDatabase_AndRankADatabaseNoSnapshotNames.
  • Darling.Tests (rig port 5686, UTC): all *Census*, *Ratchet*, LivePostgresCollectionHygieneTests, my classes, DatabaseFilter and top-procedure SQL pin classes: 833 run, 3 failed. (a) ComposeServerScopeResolveNamesTests.ResolveNamesAsync_SeesDisabledRegistrations: connect timeout under the heavy parallel run; passes alone after the run. (b) ObjectStatsDatabaseFilterLiveTests... JSON parse failure under the same load; passes alone. (c) LivePostgresCollectionHygieneTests: missing own-store comment on my class (fixed and re-run) and on another lane's McpToolAudit5633L3Tests, which still fails: that class is not mine.
  • Lite.Tests Census, Ratchet, CurrentDatabasesTests, DatabaseFilterTests, QueryStatsCpuRankingReaderTests: 81 run, 0 failed.
  • Not run: the full suites; the merge of the last origin/fix/5633-mcp-tool-audit commits (Lane L3's McpToolAudit5633L3LivePostgresTests and others) was pushed without a rebuild; the rig was stopped before the 60-minute wall.
  • get_top_procedures_by_cpu against a fleet-size database_states (about 336 snapshots x many databases at the 7-day cap) was not timed.

Lite parity. Checked: the only Lite use of the current-database set was get_top_procedures_by_cpu (replaced) and FinOps recommendations (post-read, current state, kept). Lite v_database_states reads use the same shared SQL text.

What the seat should double-check.

  • McpToolAudit5633L3Tests (lane L3) lacks the #1776 own-store comment; hygiene fails on it.
  • The 7-day DropLookBack and the window-start floor are my call (the ruling allowed "a bounded look-back you justify").
  • OmitDroppedAsync (post-read, used by the clutter tool and FinOps) still judges by the newest snapshot only, as before: a database created after it is kept by the row-time rule, not by "never seen". The ruling covered the Narrow reads only.
  • McpToolsListBudgetTests / CrossAppMcpToolInventoryPinTests: no tool text or inventory changed here.

Changelog:
SECTION: Fixed
ENTRY:

F7: CI guard failure, distinct high-DOP count, Lite waiting-task tie order, Darling CPU window behaviour test

Why. Review round 2 findings Medium-4, Low-2, Low-4, plus the red Guard tests job (run 38308174340): LivePostgresCollectionHygieneTests.EveryClassUsingTheSharedStore_IsSerializedOrDocumentsWhyNot listed McpToolAudit5633L3Tests, whose two tests drive tools against the shared PostgreSQL store.

What changes.

  1. CI fix: the two live tests (AuditConfig_EchoesTheStoredEditionString_..., CompareAnalysis_RefusesOverlap_...) moved unchanged into Darling/Darling.Tests/McpToolAudit5633L3LivePostgresTests.cs, a new class with [Collection("live-postgres")] and the existing LiveStoreCleanup.RunAsync cleanup. The pure tests stay in McpToolAudit5633L3Tests with no collection.
  2. Medium-4: high_dop_queries is now COUNT(DISTINCT CASE WHEN <same DOP/MAXDOP predicate> THEN COALESCE(database_name,'') || '/' || COALESCE(query_hash,'') END) in PgFactCollector.QueryPerf.cs and DuckDbFactCollector.QueryPerf.cs, the same key and NULL-hash rule as spilling_queries (a NULL hash collapses to one per database). Bands decision: the 5/20 bands stay. The fact's text says "N queries", and the count now is literally queries, so the bands now mean what they say; a server that used to hit full severity from one query seen in 20 snapshots now needs 20 distinct queries. The stale-MAXDOP cross-check parity pins (QueryHighDopStaleMaxDopParityTests, both projects) keyed on COUNT(CASE ... THEN 1 END); I updated their canonical string, census regex and guard regex to the DISTINCT form (the guard regex now requires DISTINCT, so reverting to a row count fails the pin). The unguarded-count negative sample still lands in the census and misses the guard.
  3. Low-2: Lite's waiting-task rollup replaces the arg_max struct keys with DuckDB ordered aggregates using the PostgreSQL ordering exactly: blocker first(CASE WHEN blocker <> session THEN blocker END ORDER BY wait DESC NULLS LAST, blocker ASC NULLS LAST), fallback first(blocker ORDER BY wait DESC NULLS LAST, blocker ASC NULLS LAST) FILTER (WHERE blocker <> session), resource first(resource ORDER BY wait DESC NULLS LAST, blocker ASC NULLS LAST, resource DESC NULLS LAST). Darling's SQL is unchanged.
  4. Low-4: new FinOpsCpuRightSizingWindowLiveTests (Darling, own scratch database) runs DarlingFinOpsRecommendationsReader.GetRecommendationsAsync on seeded data. Test 1: 32 cores, last 24 hours at 8%, a busy 90% stretch four days back; the CPU row must read "CPU: reduce from 32 ... (P95 CPU 8.0%)" and the detail must start "From the last 24 hours, P95 CPU utilization was 8.0%". Test 2: P95 8% but one 60% spike, so the Utilization verdict is RIGHT_SIZED; neither CPU row may appear. The source pin in RightSizingWindowTests stays.

Test plan.

  • Tie and NULL-wait tests added in both products with the same expected rows (Lite WaitingBlockedDatabaseFilterToolTests.WaitingTasks_TiedAndNullWaitThreads_PickTheSameRowAsDarling, Darling WaitingTasksRollup5633Tests.TiedAndNullWaitThreads_PickTheSameRowAsLite). Session 80: self-blocked longest thread, tie at 300 ms between blockers 50 and 40 gives blocker 40; session 81: the only other-blocked thread has a NULL wait and still supplies blocker 70; session 82: equal waits give blocker 22, resource of blocker 22.
  • Revert proofs (each fails on the old shape): Lite old arg_max SQL fails the tie/NULL test and old row count fails HighDopQueryCount_CountsDistinctQueries_NotSnapshotRows; Darling old row count fails its HighDopQueryCount... live test; a 7-day utilization window (AddDays(-7)) fails the busy-cluster CPU test. The Darling tie/NULL test passes on both old and new Darling code by design (it pins parity with Lite; Darling's SQL was already the reference).
  • Darling rig (PostgreSQL port 5687, UTC, both preload libraries), -trait Stage=Guard: 1588 total, 0 failed (this includes LivePostgresCollectionHygieneTests).
  • Darling *Census*, *Ratchet*, *LivePostgresCollectionHygiene*, *QueryHighDop*, *QuerySpillsDistinct*, *WaitingTasksRollup5633*, *FinOpsCpuRightSizing*, *McpToolAudit5633*, *RightSizingWindow*, *McpToolsListBudget*: 635 total, 0 failed.
  • Lite -trait Stage=Guard -failSkips: 496 total, 0 failed. Lite Census, Ratchet, QueryHighDop, QuerySpillsDistinct, WaitingBlockedDatabaseFilter, McpToolAudit5633, McpToolsListBudget, FinOpsTests: 177 total, 0 failed.
  • Builds: Lite 0 warnings; Darling 0 errors, 1 warning (xUnit2013 in DarlingOperatorGroupScriptTests.cs, from the dev merge, not this lane).
  • Not run: the Darling Stage=Guard classes with DARLING_TEST_PG unset and -failSkips (CI's exact condition; the run above had the rig set), the full suites, a mutation proof that the RIGHT_SIZED test fails if the CPU rows ignore the verdict.

Lite parity. Medium-4 and Low-2 changed in both products with the same tests; the Low-4 behaviour test already existed in Lite (FinOpsTests.VmRightSizing_BusyClusterFourDaysAgo...), this adds the Darling one. The parity pins were updated in both projects together.

What the seat should double-check.

  • The QueryHighDopStaleMaxDopParityTests edits in both projects (canonical string, census regex, guard regex): I changed pins deliberately because the guarded count shape changed; the guard regex is stricter than before (requires DISTINCT and the key).
  • Severity effect of Medium-4: counts that used to be snapshot rows are now distinct queries, so QUERY_HIGH_DOP severity drops for servers with a few high-DOP queries seen repeatedly. Intended.
  • Files from the brief's do-not-touch list: none.

Changelog:
SECTION: Fixed
ENTRY:

F8: fix lane (wave 2 review: collection caveats are written by the scheduled pass only)

This section replaces W2A's change. W2A (commit 2a688bf) granted the mcp role INSERT, DELETE and a column UPDATE on collect.analysis_collection_caveats, plus a "caveats NOT saved" sentence in persistence_note. The seat ruled against it (least privilege). Ignore the W2A section above for the grants, the bool return, LastCollectionCaveatWriteFailed and the persistence_note text: all of that is gone. The ruling, in two sentences: collect.analysis_collection_caveats is the scheduled analysis pass's state, so only the scheduled pass (the worker, running as the service role) writes it. Every on-demand pass (MCP analyze_server, with or without as_of) computes its caveats for its own response and the in-process ledger and never writes the table.

Why. Review findings W2A-1 to W2A-6 (handoffs/h6/review-w2.md): an as_of run over a past window could replace the standing "current" rows (W2A-1); any MCP window choice could clear or forge them (W2A-2, W2A-3); the grant comments overstated their protection (W2A-3); the role docs, the privilege census and the ApplyPassAsync doc were wrong or thin (W2A-4 to W2A-6). Withdrawing the write answers all of them. Plus one CI failure: run 38315771866, TsqlConventionGuardTests.TheMemberScan_ReadsEveryDeclarationWhole (CurrentDatabaseScope.cs Narrow).

What changes.

  • DarlingManagedRoles.cs and Darling/tools/provision-roles.sql: W2A's two GRANT statements and its doc-comment edits are reverted, so the comments say "exactly two narrow analysis INSERTs" again. Nothing else in provisioning changed. The role SQL that runs on every start never granted this table to mcp on dev (git grep of origin/dev finds no analysis_collection_caveats in either file), so there is nothing to revoke on a released store.
  • DarlingAnalysisService.AnalyzeAsync (both overloads) takes bool persistCollectionCaveats = false; the caveat write is if (persistCollectionCaveats). There was no scheduled-versus-on-demand flag to reuse (AnalysisContext.PersistFindings only means "not anchored", and unanchored MCP runs do persist findings), so this is the one new parameter. It defaults to false so a new on-demand caller cannot write the table by forgetting a flag. DarlingWorker.cs passes true (one call site, one line plus a comment; the only other AnalyzeAsync caller in the product is the MCP tool). No web-triggered analysis exists.
  • ApplyPassAsync returns Task again (the worker has no use for a bool). Its doc is fixed (W2A-6): it always opens a connection and always runs the DELETE; the upsert is skipped when nothing is unread. It now says scheduled passes only.
  • DarlingMcpTools.cs: the CollectionCaveatsNotSavedNote constant, ComposePersistenceNote and the call are removed. persistence_note is back to its dev shape, since an on-demand pass never tries to write.
  • TsqlConventionGuardTests: Darling/PerformanceMonitor.Darling.Storage/CurrentDatabaseScope.cs Narrow added to KnownTruncatedRanges. Narrow is filter.IsAll && dropped is { Count: > 0 } ? ... : filter, the An RDS or Aurora target can name the AWS role Darling assumes for it, so one host can monitor clusters in several AWS accounts #5452 property-pattern shape: the brace walk closes the range at the pattern's brace and strands only the ternary arms, which hold no string literal. I checked that the file's SQL sits in members the walk reads whole, so no census counts a stranded site.
  • Three source/reflection pins that matched the exact text AnalyzeAsync(AnalysisContext context) now match the prefix (CollectionCaveatsParityTests, AnalysisWindowEmptyRuleParityTests, DarlingAnalysisServiceEngineRoutingTests), and DarlingAnalysisPipelineTests looks the overload up with the extra bool.

Privileges the mcp role now holds on collect tables (census on a real role, real managed batch): SELECT on every table (schema-wide grant plus the default privilege for later tables), and INSERT on analysis_findings. Nothing else: no INSERT, UPDATE, DELETE, TRUNCATE, TRIGGER, REFERENCES (table or column level), no MAINTAIN (checked when the server has it), no sequence USAGE/UPDATE. On analysis_collection_caveats it holds SELECT only (the web panel and get_collection_health read it).

Test plan.

  • McpCollectionCaveatGrantLiveTests rewritten (3 live tests, real managed batch on a scratch store): (1) the strict census above, asserting the mcp role holds NO write on the caveat table and analysis_findings:INSERT is its only collect write (W2A-5: TRIGGER, REFERENCES, MAINTAIN where present, sequences); (2) analyze_server as mcp, with and without as_of, leaves three seeded caveat rows (two for the server, one for another) exactly unchanged, with the old grants put back so the privilege is not what protects them; (3) on the owner pool, the default pass and the anchored pass leave the table unchanged and a pass with persistCollectionCaveats: true (the worker's) still clears the stale row.
  • DarlingManagedRolesTests.McpRole_HoldsNoWriteOnTheCaveatTable_InTheManagedBatchOrTheByoScript (no database): no non-comment statement in the managed batch or provision-roles.sql names the table; GRANT ALL ... TO mcp and a collect-wide non-SELECT grant are caught (W2A-5's regex gap; the test also asserts the patterns match sample GRANT ALL ON collect.analysis_collection_caveats TO mcp; shapes). DarlingSecuritySplitLiveTests now asserts mcp lacks INSERT, UPDATE, DELETE, TRUNCATE, TRIGGER and REFERENCES on the table.
  • Fails on the W2A shape: with W2A's grants put back in both role files and the flag forced on (|| true), all 3 live tests and the unit test failed (4 of 4); restored, they pass.
  • Rig (port 5683, UTC, timescaledb + pg_stat_statements): *Census*, *Ratchet*, *Role*, *Grant*, *CollectionCaveat*, *AnalysisAsOfAnchor*, *TsqlConventionGuardTests*, *LivePostgresCollectionHygieneTests*, *AnalyzeServer*, *DarlingAnalysisBudget*: 1082 run, 1 failed (CollectionCaveatsParityTests, a source pin on the old signature, fixed above), 17 skipped (rig has no DARLING_TEST_PGRUNTIME). Then *Parity*, *EngineRouting*, *DarlingAnalysis* plus my classes: 188 run, 1 failed (DarlingAnalysisPipelineTests reflection pin, fixed above). Final re-run of the fixed classes: 26 passed, 0 failed. TsqlConventionGuardTests passed (the CI failure is fixed). Build: 0 warnings, 0 errors, after merging origin/dev.
  • Not run: the full suite (CI decides), Installer.Tests, the runtime-bound role tests that need DARLING_TEST_PGRUNTIME.

Lite parity. Lite has no store roles (DuckDB, no GRANTs) and no collection-caveat table; Lite's AnalyzeAsync(AnalysisContext) is untouched (the shared parity pins match by prefix, so they still read both products). Nothing to mirror.

What the seat should double-check. (1) The W2A section above is now obsolete; its Changelog entry should be dropped in favour of the one below. (2) After this change an MCP analyze_server run never updates the caveat panel; only the next scheduled pass does (5-360 min, or never if Analysis.Enabled is false, then the 7-day prune). (3) The one-line DarlingWorker.cs edit sits in a file other PRs touch; keep both sides on a conflict.

Changelog:
SECTION: Fixed
ENTRY:

F9: size the background workers by host RAM (item 14, wave 2 review findings W2B-1 to W2B-4, plus the README pin)

Why

Lane W2B raised timescaledb.max_background_workers to 2 x hypertables + 16 (160) for every host. The wave 2 review (W2B-1) estimated that 139 concurrent job backends can exceed RAM on a 4 GB host, and found the formula keyed on the wrong count (W2B-4), a v8 legacy fallback that accepted each worker line alone (W2B-3), and two comments that disagreed with the facts (W2B-2, W2B-4). CI run 38315771866 also failed ReadmeDerivedCountPinTests because the pin derived 74 while the README said 160.

Measurement (rig on port 5684, PostgreSQL 18 with the bundled TimescaleDB, a store built by the real migrations plus the job-creating convergence steps)

The store held 142 jobs (94 compression, 23 refresh, 23 retention, 2 system). With every scheduled job set overdue (alter_job(..., next_start => now() - 1 minute)), the rig was restarted at each sizing while a sampler summed PrivateMemorySize64 of the postgres processes whose executable is under the rig directory, every 250 ms for 45 s (script kept out of the repo):

workers / processes idle (12 processes) peak private memory processes at peak extra backends per-backend increment out of background workers lines
74 / 85 (old) 45.0 MB 101.9 MB 21 9 6.3 MB 68
160 / 171 (W2B) 46.1 MB 163.3 MB 41 29 4.0 MB 0

Reading it honestly:

  • The burst is bounded by process spawn rate and job duration, not by the slot count: even with 160 slots only 29 job backends overlapped at the sampled peak (the store is empty, so jobs finish in milliseconds; samples were 156-166 in 45 s). The 74-slot launch failures come from slots held by workers that are still being spawned, not from memory.
  • The increment is about 4-6 MB private per job backend, so the measured peak is far below a quarter of RAM at any size. It is an EMPTY store: a compression job on a real chunk sorts at up to maintenance_work_mem (1 to 1.5 GB) and a refresh sorts at work_mem, which this does not measure. So I did not raise the factor on the strength of the small number.
  • Verdict: the measurement is consistent with, but cannot prove, a high factor, so I used the ruling's fallback of 16 slots per GB. At the review's 15 MB per backend that bounds a full burst of backends at 16 x 15 = 240 MB per GB (24%), and at the measured 4-6 MB at 64-100 MB per GB.

What changes

  • DarlingManagedPostgres.DeriveWorkerSettings(hypertables, quantizedRamBytes): workers = clamp(16 x RAM GB, hypertables + 2, ceiling), max_worker_processes = 3 + workers + 8 (unchanged). RAM is the same whole-GB figure the v8 fingerprint already records and DeriveMemorySettings uses (QuantizeRam), so no new input and no fingerprint change. The v8 block, DarlingStoreHostProfile (--check-settings) and the every-start v8 heal log all pass it.
  • Floor DeriveWorkerFloor = hypertables + 2 (74 / 85), the shape of every release up to 3.10.0. The v2 block (no RAM input) writes the floor; v8 is later in the file and wins, as the review said.
  • Ceiling DeriveWorkerCeiling = hypertables + 3 x continuous aggregates + 2 + 16 = 177 at 72 hypertables and 29 aggregates (W2B-4: counts hypertables AND aggregates and every policy kind; the real store has 142 jobs, the ceiling covers 143 with room). New TimescaleSupport.ContinuousAggregateCount (the distinct views of the hourly, daily, baseline, off-grid and frozen tiers); constants WorkerSlotsPerGb, PolicyJobsPerContinuousAggregate, NonObjectPolicyJobs replace PolicyJobsPerHypertable.
  • Values at the measured schema (72 hypertables, 29 aggregates):
host RAM workers max_worker_processes
4 GB (and less) 74 (floor) 85
6 GB 96 107
8 GB 128 139
16 GB (12 GB and up) 177 (ceiling) 188
  • ManagedConfMigration.cs (W2B-3): the v8 legacy fallback accepts a worker line only when BOTH assignments in the same span equal the legacy PAIR (hypertables + 2, + 13), read the way v2 pairs them. The rebuiltCurrent and rebuiltLegacy checks in the v2 classifier were dead (the earlier M == N + 11 test already implies both) and are removed with a comment saying why.
  • Comments (W2B-2, W2B-4): the compose memory budget now counts the background workers (74 slots = up to 73 job backends at about 4-6 MB each, plus 23 refresh jobs x 3 x 16 MB of sort memory if they overlap) and says a mem_limit under about 4 GB has no headroom for both; the two comments on an idle slot's cost now say about 25-40 KB of shared memory and no process, and that a slot a restart FILLS is a backend.
  • docker-compose.yml goes back to the floor, 74 / 85: a container has no host-RAM figure and a 2 GB container is at the floor of the rule anyway; the comment says a mem_limit of 12 GB or more may use 177 / 188. The README worker-sizing section states the rule, the ceiling and an 8 GB example (128 / 139) with the 4 GB and 12 GB values; the runbook carries the same example. The README process-count FAQ no longer says "hypertable count + 2 (about 57)".
  • CI guard failure: ReadmeDerivedCountPinTests derived its expected pair as hypertables + 2 while the README (W2B) said 160 / 171. The pin now derives the 8 GB example from DeriveWorkerSettings and matches the sentence "For an 8 GB host that is 128 and 139 for 72 hypertables (the 71 collector tables plus", so the README and the pin agree and cannot go stale on other hosts. PgLogEventsPipelineTests (runbook and README sentences) and CiComposeWorkerSizingTests (floor) follow.

Test plan

  • Measurement above, on this laptop, no other load, about 10 minutes.
  • JobCountWithinWorkerSizingTests (2): the pure pin at 4 / 8 / 16 GB, the empty store, one more aggregate, and the clamp bounds at 1 GB to 1 TB; the live fact now asserts the formula's aggregate and hypertable counts equal the migrated store's real ones, that the ceiling covers the real 142 jobs + 1 scheduler and sits within 40 slots of them, and that a 16 GB host gets a worker per job.
  • ManagedConfMigrationTests: ClassifyLines_V8Block_With74And171_IsNotServiceOwned (both lines HandEdit) and ClassifyLines_V8Block_WithTheLegacyWorkerPair_IsOurs; HardwareSizingConfAppend_WorkerCountsAreTheRamSizedPair_NeverBelowTheV2Floor (4 / 8 / 16 / 32 GB, v2 floor vs v8).
  • Classes run (rig 5684, DARLING_TEST_PG and DARLING_TEST_PGRUNTIME set): JobCountWithinWorkerSizingTests 2/0 failed; ReadmeDerivedCountPinTests 5/0; CiComposeWorkerSizingTests 4/0; PgLogEventsPipelineTests 94/0; CiClusterWorkerSizing (both) and every ManagedConf class together 245 total, 0 failed, 1 skipped (includes the rehearsal fixture's legacy v2 and v8 blocks); every *Census*, *Ratchet*, *DocCommentHygiene* class 638 total, 0 failed. DarlingManagedPostgresTests (full class): 231 total, 1 failed (HardwareSizingConfAppend_EmitsHostDerivedSettings still pinned W2B's 96 / 107 at 40 hypertables; now 145 / 156, the 32 GB host at the ceiling), 3 skipped; after the fix and git merge origin/dev the HardwareSizingConfAppend and WorkerSizingConfAppend methods (14) pass, and the other classes above re-ran green on the merged tree: 808 total, 0 failed. The full class was not re-run after the merge.
  • Proven by reverting: with the v8 fallback reduced to the worker line alone, ClassifyLines_V8Block_With74And171_IsNotServiceOwned fails; restored.
  • Full suite not run (laptop; CI decides). Lite has no PostgreSQL or TimescaleDB, so there is nothing to mirror.

What the seat should double-check

  • The 16 per GB factor is the ruling's fallback, not a measured limit; the measurement is on an empty store.
  • Existing stores: a store whose conf holds the unreleased W2B 160 / 171 (v2 or v8) heals on its next start (v8 content differs from what this build writes); no release carried 160 / 171, so nothing in the field is orphaned.
  • Hosts with RAM from the fallback reading (not authoritative) quantize to 4 GB and get the floor.
  • The CI darling-pg cluster still runs 74 / 85 (workflow untouched, per the brief).

Changelog:
SECTION: Fixed
ENTRY:

F10: Darling CI pin failures from run 38321499493 (head 4535bb3)

Why

CI run 38321499493 failed 7 Darling tests (plus 2 in shard 5 found when its log finished). Every one is a pin or guard that reads source an earlier lane on this branch changed; the lanes ran only their own classes plus Census and Ratchet. All nine are test-side fixes: in each case the product code is right and the pin's literal text no longer described the right shape, and each pin was updated to the new shape (not loosened) with the reason in a comment. No product code changed in this part.

What changes (per failure: cause commit, code or pin, fix)

  1. McpLatestSnapshotStampTests.EveryLatestReadTool_IsInTheRoster_... (audit_config). Cause: 894159d (item 10, L3) made audit_config call GetLatestServerPropertiesAsync to echo the stored Edition string. Code right: the payload is the analysis pass's facts plus an edition label, not a latest snapshot, so it publishes no captured_at and stamping it would imply a snapshot time the facts do not have. It is the same case as the two CPU rankings, which read the newest server_properties row as an input. Fix: audit_config joins LatestLookupInsideAnotherRead (the list's own control fails if the tool ever gains captured_at) and the doc comment names it.
  2. QueryStoreClutterTests.TheToolBody_ReadsTheFleetOverTheSameAnchoredWindow. Cause: 59feb9b (item 4 dropped databases, L6/F6) added asOfUtc: asOfUtc to the fleet reads. Code right and better: the fleet read keeps the same requestedStart/now window as the tool's own read and now also the same as_of anchor for dropped-database handling, so the fleet median and the server answer see the same databases. Fix: the pin matches GetReadCostAsync(postgres, fleetIds, requestedStart, now, asOfUtc: asOfUtc, cancellationToken: cancellationToken) and the churn twin, so dropping the anchor from a fleet read now fails.
  3. AzureSiblingSizePayloadTests.TheLatestSnapshotSql_SelectsTheFileId_.... Cause: eb656f3 (item 6, L4) appended is_percent_growth, growth_pct after file_id. I checked the reader's ordinals first: GetLatestDatabaseSizesAsync maps file_id at 11, is_percent_growth at 12, growth_pct at 13, matching the SQL order, so no data bug. Fix: the regex pins the full tail volume_free_mb, file_id, is_percent_growth, CAST(growth_pct ...) AS growth_pct FROM v_database_size_stats, so a column slipped in before file_id still fails.
  4. NoServiceObjectiveToResizeTests.RecommendationRules_StandDownOnTheSharedPredicate. Cause: 1a0bf37 (item 11, L5) replaced the per-rule ProvisioningStatus == NotApplicable test with one shared predicate CpuVerdictAllowsDownsizing, true only for OVER_PROVISIONED, so NotApplicable (and RIGHT_SIZED, and no-sample) stand down by construction. Code right: both CPU rows use it (the compute row through CpuRightSizingTargetCores, the prescriptive row in DarlingFinOpsRecommendationsReader). Fix: the pin asserts the predicate is defined on OverProvisioned alone, the compute row gates on it, the reader's prescriptive row gates on it, and no inverted test exists. The sibling behavior test (NotApplicable returns null, OverProvisioned returns a row) still executes the stand-down and passes.
  5. Shard 5 (job 115052871146), two failures, both get_trace_flag_changes, Cause: item 19 (9d195d6, 5f794e5).
    • WebDataStartNoteConfigAndLogTests.TheAdmittedReads_AnswerTheirWord_...: the source-text check looked for NoChanges( in the tool body; the empty path now goes through NoTraceFlagChanges(, which returns NoChanges unless a successful empty run says no flag is on (then status empty itself). Pin now accepts NoTraceFlagChanges( too.
    • DarlingMcpConfigHistoryToolsLivePostgresTests.APreWindowSnapshot_IsNotCovered_... (KeyNotFoundException on hints): its seed logs a SUCCESS run with rows_collected = 0 for trace_flags. Since item 19 that is an empty snapshot, which turns the seeded flag off inside the window, so the tool correctly answers with that change (top-level effective_start, no hints). Code right. The case is about coverage, so the seed's second run now stores one row (runRows: 1, a new optional seed argument, default 0 so the other cases are unchanged). Reproduced on my rig before the fix and green after.

Test plan

  • Reproduced the 2 shard-5 failures on my rig (UTC PostgreSQL, both preload libraries) and the 4 pin failures locally before fixing.
  • After the fix: the six classes above (140 tests) ran green, rig-backed: McpLatestSnapshotStampTests, QueryStoreClutterTests, AzureSiblingSizePayloadTests, NoServiceObjectiveToResizeTests, WebDataStartNoteConfigAndLogTests, DarlingMcpConfigHistoryToolsLivePostgresTests.
  • Build: 0 Warning(s).
  • Sweep: the 53 Darling-side source files this branch changes (the diff against origin/dev, excluding Lite and Darling.Tests), mapped to the test classes that name them (any test file naming the file by its .cs leaf, plus every file naming a rarely used basename; 13 very common basenames such as McpHelpers, DarlingWorker and DarlingMcpTools matched by leaf path only): 581 test files, 703 classes, run in 3 chunks on the rig against one database. 10,377 tests, 7 failed, 44 skipped. The 7 were all live classes (FinOpsRecommendationsSecondaryReplicaLiveTests, QueryStoreClutterLivePostgresTests, FleetPartialDatabaseFailureBandTests, QueryStoreBackfillSkipFailingDatabaseTests, FinOpsStorageGrowthGoldenLiveTests, FinOpsRecommendationsToolParityLiveTests, GatedCollectorRowsLiveTests); re-run together on a freshly created database they passed (53 tests, 0 failed), so the failures were shared-database interference from running 700 classes in one database, not code. The sweep found no new pin failure beyond the nine listed above.
  • After merging origin/fix/5633-mcp-tool-audit (F11's push touched DarlingMcpConfigHistoryTools.cs and DarlingCollectorRunFactsReader.cs): rebuilt, 0 Warning(s), and re-ran my six classes plus CollectorRunEmptyReasonTests and AsOfWindowAnchorTests (187 tests, 0 failed).
  • Not run: the sweep chunks ran on the build from before that last merge; Lite.Tests; the full suite (CI decides).

Lite parity

Test-only change on the Darling side. F11 owns the Lite half of the same CI run (ArchivableTableBareRead, AsOfWindowAnchor on Lite/Mcp, McpWindowNotice trace flags). Lite.Tests was not touched here. The Darling-directory leg of AsOfWindowAnchorTests passed in that CI run and is in the sweep.

What the seat should double-check

  • Item 1 is a judgment call: audit_config is listed as "latest lookup inside another read" rather than stamped. If the seat prefers a captured_at on audit_config (the properties row's own collection time), that is a product change, not a pin edit.
  • The live trace-flag coverage seed now needs a non-zero row run; a future trace_flags coverage case with zero-row runs will hit the same item-19 behavior.

Changelog: none (test or internal only)

F11: Lite-side CI failures on run 38321499493 (3 tests)

Why. Three Lite-side tests failed on head 4535bb3. Each reads source or seeds data that an earlier lane on this branch changed, and those lanes ran only their own classes.

What changes, per failure

  1. ArchivableTableBareReadSweepTests.EveryBareReadOfAnArchivableTable_IsOnTheBareOnPurposeList (LocalDataService.CurrentDatabases.cs, 2 bare reads at lines 43 and 122). Cause: 6cfa952 (item 4), kept by 3b779b1 and 59feb9b. The code was right and the sweep was seeing a false positive. Both reads already run v_database_states (hot plus archive): the shared backfill SQL names database_states, and the call site swaps it with .Replace("FROM database_states", "FROM v_database_states"). The sweep matches FROM database_states in that Replace's first literal. Fix: one ThroughArchiveView helper whose match text is database_states WHERE, so no bare FROM literal is left. No allowance was added: the reads are not bare, and the 6 h to 7 d look-back can reach archived rows, which is why the view is right.
  2. AsOfWindowAnchorTests.EveryToolThatTakesTheAnchor_ActuallyUsesIt(Lite/Mcp). Cause: 9d195d6 (L2 items 3 and 19). The pin was right, and the code was wrong in two ways.
    • get_trace_flag_changes called GetCollectorRunFactsAsync with no anchor at all.
    • get_top_procedures_by_cpu passed windowEnd positionally, which did anchor the window count. But the newest-run read inside GetCollectorRunFactsAsync had no time bound in either tool.
    • The effect: an as_of call before a collector's first run read "the newest successful run ever" and gave an empty reason as of now. For trace flags that was "No trace flags are enabled" from a run that had not happened yet.
    • Fix: both tools pass asOfUtc: windowEnd. GetCollectorRunFactsAsync bounds the newest-run read at the anchor (new CollectorLatestSuccessAtOrBeforeSql, collection_time <= $4).
    • Darling twin, same bug and fixed here: DarlingCollectorRunFactsReader.ReadAsync bounds the newest run at windowEndUtc when it is given (new LatestSuccessAtOrBeforeSql). The trace-flag changes tool now passes its window end. This is in F10's side of the tree, but the files (DarlingCollectorRunFactsReader.cs, one line in DarlingMcpConfigHistoryTools.cs) are not on F10's list. The Darling AsOfWindowAnchorTests case passed before my change because it only checks dataService. calls.
  3. McpWindowNoticeConfigAndLogToolTests.AnEmptyWindow_PastCoverage_CarriesTheFloorAndTheNote_UnderHints(TraceFlags) (KeyNotFoundException on status). Cause: 5f794e5 (item 19, trace-flag diff reads zero-row runs as empty snapshots). The code was right and the test seed was wrong. The test seeds flag 1117 and then 28 h of SUCCESS runs with rows_collected = 0. Under item 19 that is the flag turned off, a real change, so the tool correctly returned a change payload, not the empty envelope. The shared window-notice shape is intact (the payload still carries it). Fix: SeedCoverageAsync and SeedLogRunsAsync take a rowsCollected argument, and this one test's trace-flag runs store one row (the collector found the same flag each time). Nothing else changed.

Test plan

  • Sweep: 180 Lite.Tests classes (every class in the 166 test files that name a changed Lite file's basename, or the test files this branch changed, plus every *Census* and *Ratchet* class): Total 2425, Failed 0.
  • Darling.Tests: AsOfWindowAnchorTests, CollectorRunEmptyReasonTests, *Census*, *Ratchet*, *McpToolsListBudget*, *McpConfigHistory*: Total 649, Failed 0, Skipped 8 (live, no rig). Both products' anchor cases pass.
  • The three originally failing tests pass.
  • New tests: Lite TraceFlagChanges_AsOfBeforeTheOnlyRun_AnswersAsOfThen_NotAsOfNow (fails with the trace-flag tool call reverted, proven once) and CollectorRunFacts_WithAnAnchor_IgnoreRunsAfterIt; Darling SQL pin for LatestSuccessAtOrBeforeSql.
  • Builds: 0 warnings, 0 errors, both test projects.
  • Darling live tests (no rig started: the Darling twin fix is a one-bound change to an existing query, pinned by text only). CI decides.

Lite parity. Items 2 and 3 are Lite and Darling both; the Darling twin of item 2 is fixed (above). Item 1 is Lite only (Darling has no such sweep).

What the seat should double-check

  • The Darling fix for item 2 is not exercised by a live test. The bound reuses the same naive-UTC timestamp parameter as the window count beside it.
  • With no as_of, the newest-run read is now bounded at the call's now. A run committed after the call started is not seen, which is the intended anchor semantics.
  • Item 3 changed a test seed, not code. If you read the tool's payload for that case as wrong, the item 19 semantics (a zero-row SUCCESS run after a seen flag is a change) would need a ruling.

Changelog:
SECTION: Fixed
ENTRY:

F12: top procedures (and two sibling reads) say which databases they left out; the silent catch now logs

Why. claude[bot] finding D: with no database_name, get_top_procedures_by_cpu narrows the read to "databases the server still has" (#5633 item 4). If the only activity was in a database dropped since, the empty page said "the procedure_stats collector ran ... and recorded nothing for it", which is false (it recorded rows there) and pointed at a filter the caller never made. Second finding: CurrentDatabaseScope had two catch ... { return null; } blocks that dropped the exception, so a SQL or ordinal mistake would silently turn all of item 4 off.

What changes

  • Darling and Lite, same words (PerformanceMonitor.Common/CollectorRunFacts.cs, new DatabasesLeftOut.cs, McpHelpers.WithDatabasesLeftOut):
    • When the narrowing left at least one database out, the envelope carries databases_left_out (sorted names, first 20) and databases_left_out_note (says the newest database snapshot no longer lists them, dropped or detached since; the full count when over 20; "name one in database_name to read its history"). Both keys ride an empty page and a page with rows. A page that left nothing out has the keys null on the data page and absent on an empty page.
    • The empty sentence is no longer "recorded nothing for it": it now says "No procedure ran in the last 168h in a database this server still has: the procedure_stats collector ran successfully N time(s) in the window and recorded nothing for them. 1 database(s) the server no longer lists were left out of this read (see databases_left_out)." The "unavailable" branches (new collector, no run in window) are unchanged.
    • Payload example (empty page): {"status":"empty","message":"No procedure ran in the last 168h in a database this server still has: ... 1 database(s) the server no longer lists were left out of this read (see databases_left_out).","databases_left_out":["GoneDb"],"databases_left_out_note":"These databases were left out of this read because the server's newest database snapshot no longer lists them (dropped or detached since an earlier snapshot did). Name one in database_name to read its history."}
  • Other item-4 reads checked:
    • Query Store clutter (Darling): same false-nothing possibility (its empty ladder ends "... has it OFF everywhere"). OmitDroppedAsync now reports the omitted names (omittedDatabases); the tool names them in both shapes, and when only omitted databases had rows the miss is empty ("No Query Store rows ... for a database this server still has ...", database echo kept).
    • FinOps index analysis (Darling): same (fallback "No index statistics were collected for this server yet"). The shared index read reports omitted names on IndexAnalysisWithSnapshotTimes.OmittedDatabases; the tool adds the envelope keys and the empty sentence claims only the databases still present.
    • FinOps recommendations (Darling): checked, no change. Its envelope is a count plus a list with no "nothing was collected" sentence, so an empty list does not claim absence of data.
    • Lite has no clutter, index-analysis or recommendation twin using this scope (its only item-4 read is top procedures).
  • Logging: CurrentDatabaseScope methods take an optional ILogger? logger (before the CancellationToken, CA1068) and the two catches now call logger?.LogWarning(...) with the failing read, server id and exception message, once per occurrence, the pattern the rest of Darling.Storage uses (CollectionCaveatStore, DarlingModuleMap). Fail-open kept. The logger is threaded from get_top_procedures_by_cpu and get_query_store_clutter (both now take the DI ILogger, already an allowed seat). Not threaded: FinOps index analysis and recommendations, whose tools have no logger parameter, so a failure there is still not logged (the parameter exists; threading it needs a DI parameter on those tools). Lite's twin catches (LocalDataService.CurrentDatabases.cs) now AppLogger.Warn.
  • Pins updated on purpose: WebTopProceduresColumnsTests (the tool signature now has ILogger? logger = null between detail and the token), EmptyAnswerDatabaseEchoTests (clutter has a third StatusForDatabase empty answer, count 2 to 3).

Test plan

  • Bot's scenario on both products (DroppedDatabaseScopeLiveTests.TopProcedures_SaysWhichDatabasesItLeftOut_... on a rig, LiteMcpServerClockTests.TopProcedures_SaysWhichDatabasesItLeftOut_...): GoneDb has rows, LiveDb none, newest snapshot names LiveDb. Asserts the sentence, the names, the page with rows, the named-database read (nothing left out), and for Darling the clutter empty page. Both fail on the old shape (reverted product file once on each side: Assert.DoesNotContain() Failure).
  • Unit tests: CollectorRunEmptyReasonTests (new words, unavailable branches unchanged, cap at 20 with full count), FinOpsIndexAnalysisViewTests (empty sentence plus names).
  • Sweep (every test class naming a changed source file, plus all Census and Ratchet classes and McpToolsListBudgetTests): Darling 111 classes, 1965 tests, 2 failures, both stale pins from my change, fixed (above), re-run green; after merging the branch again, 643 targeted tests green. Lite 28 classes plus Census/Ratchet, 761 tests green; post-merge Lite 82 green. Builds 0 warnings.
  • Not run: the full suites (CI decides), a live index-analysis page with rows (unit-tested only).

Lite parity. Top procedures done and tested; Lite shares CollectorRunFacts, DatabasesLeftOut and McpHelpers.WithDatabasesLeftOut with Darling, so the words are identical. No tool description changed (budget pins untouched).

Double-check. get_top_procedures_by_cpu and get_query_store_clutter gained an ILogger? logger DI parameter (schema unchanged; census green). OmitDroppedAsync and the three clutter readers gained an optional omittedDatabases collector before logger. The index analysis and recommendations reads still fail open without logging (see above).

Changelog:
SECTION: Fixed
ENTRY:

F13: bot-review smaller findings and stale PR sections

Why. The claude[bot] review of 168d822 left four smaller findings plus two PR sections that F4 and F1 had made stale. Pushed at 0095611.

What changes.

  1. DarlingSessionReader.WaitingTasksSql now groups and projects rtrim(wait_type) (it grouped on the raw column). Rows written before Wait names lose their trailing space, and two Hyperscale timer waits are ignored #4884 carry a trailing space, so a session's padded and trimmed rows used to split into two rows and the padded name was shown. Lite and ViewerDataService.BlockingTrends already did this. The GROUP BY repeats the rtrim(...) expression because a bare wait_type in PostgreSQL would bind to the input column. Lite's DuckDB fixture the bot asked for already exists (WaitingBlockedDatabaseFilterToolTests.WaitingTasks_TiedAndNullWaitThreads_PickTheSameRowAsDarling: self-blocked longest thread, tied other-session blockers expect the lower id, NULL-duration blocker still reported), so I added nothing there.
  2. One definition of the empty trace-flag runs SQL: DarlingRetentionHorizons.EmptyTraceFlagRunsSql (Darling.Storage, in DailySummaryHorizon.cs beside TraceFlagsBaselineFloorSql, which it calls). It is static readonly, not const, because the floor is built by a method call. DarlingCollectorRunFactsReader and ViewerDataService.ConfigChanges.cs lost their byte-identical copies and reference it. TraceFlagEmptyRunLivePostgresTests now compares the shared text against the analysis copy instead of the viewer copy. DarlingAnalysisService.EmptyTraceFlagRunsForAttributionSql stays separate (it reads the base collection_log, not v_collection_log); its doc now says why. Lite's DuckDB copy is untouched except for a doc cref.
  3. FinOpsRecommendationFigures.VmRightSizing and VmCpuTargetCores: the CPU parameter is p95Cpu24h (the caller already named its local that). The memory parameter keeps its 7-day name.
  4. Stale sections: L2 now points to F4 instead of "defect found and NOT fixed", and its changelog entry says the trace-flag sentence applies "when the window holds no change" (F4 owns the turn-off entry). L5's status line now says the Lite tests were updated by F1, and its Lite-tests bullet says F1 updated them and deleted the CPU helpers (checked: the helpers are gone from Lite/Services and Darling.Storage/FinOps). L5's changelog claimed neither, so it is unchanged.

Pinned by.

  • WaitingTasksRollup5633Tests.PaddedAndTrimmedWaitType_AreOneRow_ProjectedTrimmed (live, pre-Wait names lose their trailing space, and two Hyperscale timer waits are ignored #4884 padded LCK_M_X plus trimmed LCK_M_X on one session, and a lone padded PAGEIOLATCH_SH ) and the SQL pin now asserting GROUP BY ... rtrim(wait_type) and rtrim(wait_type) AS wait_type.
  • Proved on the old shape: with the SQL reverted to raw wait_type, the class gave Total: 5, Failed: 2 (the new live test and the SQL pin); restored afterwards.
  • TraceFlagEmptyRunLivePostgresTests keeps proving the reader, the viewer read and the analysis read return the same run times.

Test plan.

  • Build Darling.Tests and Lite.Tests: 0 Warning(s), 0 Error(s), before and after merging origin/dev.
  • The sweep (every test class in files that name DarlingSessionReader, DarlingCollectorRunFactsReader, ViewerDataService.ConfigChanges, DailySummaryHorizon, DarlingRetentionHorizons, DarlingAnalysisService, FinOpsRecommendationFigures, DarlingFinOpsRecommendationsReader, EmptyTraceFlagRunsSql, WaitingTasksSql): 118 Darling.Tests files, 138 classes, plus every *Census* and *Ratchet* class, on a UTC rig with both preload libraries: Total: 2522, Failed: 0, Skipped: 0. Lite: 2 classes naming the changed Lite file (CollectorRunEmptyReasonTests, ConfigChangesReaderTests) plus Census, Ratchet and WaitingBlockedDatabaseFilterToolTests: Total: 113, Failed: 0.
  • After merging origin/dev once: Darling 646 tests (the touched classes, DarlingMcpSessionToolsTests, McpPageContractTests, Census, Ratchet), Failed: 0, Skipped: 0; Lite 95 tests, Failed: 0.
  • Full suite not run on this laptop; CI decides the rest.

Lite parity. Waiting tasks: Lite already groups and projects rtrim(wait_type) and has the DuckDB tie and NULL fixture. Empty trace-flag runs: Lite's DuckDB SQL is a separate dialect and project and takes a different floor parameter, so it keeps its own copy. FinOps: Lite computes the same value inline as p95Cpu24h.

What the seat should double-check.

  • EmptyTraceFlagRunsSql is static readonly, not const as the brief said: it embeds TraceFlagsBaselineFloorSql("$2"), a method call, so it cannot be a constant. The existing ViewerDataService.EmptyTraceFlagRunsSql and DarlingCollectorRunFactsReader.EmptyTraceFlagRunsSql names are gone; the one test that used the viewer name was updated. No other file referenced them.
  • No tool description changed, so the tool-list budget pins are untouched.
  • The rtrim change is visible only for rows stored before Wait names lose their trailing space, and two Hyperscale timer waits are ignored #4884.

Changelog:
SECTION: Fixed
ENTRY:

F14: the FinOps index analysis and recommendations tools hand the host logger to the current-database lookup

Pushed to fix/5633-mcp-tool-audit as f491226 (merged with origin/dev, no conflicts).

Why. F12 (e401d9b) gave the CurrentDatabaseScope methods an optional ILogger? logger so their fail-open catch logs a Warning ("a SQL or ordinal mistake in it turns all of item 4 off"), and threaded the tool's logger from get_top_procedures_by_cpu and get_query_store_clutter. Two Darling callers still passed none: the FinOps index analysis read (get_finops view index_analysis) and get_finops_recommendations, so a failure of the dropped-database lookup there was silent.

What changes.

  • Tools: GetFinOps and GetFinOpsRecommendations take ILogger? logger = null just before CancellationToken cancellationToken, the same position and form as get_wait_stats. No Description changed (the logger is a DI seat with no description, so the tool list and the budget pins are untouched).
  • GetFinOps passes it to ReadIndexAnalysisAsync, then to DarlingFinOpsIndexAnalysisReader.GetIndexAnalysisWithSnapshotTimesAsync, GetIndexAnalysisAsync, GetIndexCleanupInputsAsync, and the private ReadIndexObjectStatsAsync, which hands it to CurrentDatabaseScope.OmitDroppedAsync.
  • GetFinOpsRecommendations passes it to DarlingFinOpsRecommendationsReader.GetRecommendationsAsync, which gives it to CurrentDatabaseScope.ReadAsync and to the compression check's GetIndexCleanupInputsAsync. The tool's PgSecondaryReplicaScope.ReadAsync call, which had logger: null, now gets the same logger.
  • Reader methods take ILogger? logger = null before cancellationToken (CA1068 requires the token last). That moved the token for positional callers, so those now name it: ViewerDataService.FinOps.IndexAnalysis.cs (3 calls) and the live tests (DroppedDatabaseScopeLiveTests, FinOpsIndexAnalysisGoldenLiveTests, FinOpsIndexAnalysisLatestSnapshotLiveTests, FinOpsIndexAnalysisViewLiveTests, FinOpsRecommendationsSecondaryReplicaLiveTests, FinOpsRecommendationsToolParityLiveTests).
  • Web dispatch (DarlingWebEndpoints.BuildReadDispatch, which already holds the host logger) now passes it for get_finops, get_finops_recommendations, and also for get_query_store_clutter and get_top_procedures_by_cpu, whose web entries F12 left on the default null (the same gap, one line each).

Caller list of CurrentDatabaseScope (non-test), all pass a logger now.

  • DarlingMcpDataTools.GetTopProceduresRanked (NarrowAsync): F12.
  • DarlingQueryStoreClutterReader read cost, plan churn, config (OmitDroppedAsync x3): F12.
  • DarlingFinOpsIndexAnalysisReader.ReadIndexObjectStatsAsync (OmitDroppedAsync): this lane.
  • DarlingFinOpsRecommendationsReader.GetRecommendationsAsync (ReadAsync): this lane.
  • CurrentDatabaseScope.NarrowAsync / OmitDroppedAsync calling ReadDroppedAsync / ReadSnapshotAsync inside the class: pass their own logger.
  • Tests only (no logger, by the brief): DroppedDatabaseScopeLiveTests direct calls.
  • Not callers of CurrentDatabaseScope, still no logger: the Viewer (ViewerDataService.FinOps.*) calls the readers with no logger. A WPF client has no host ILogger (no ILogger anywhere in PerformanceMonitor.Darling.Viewer), so it uses the optional default (null, silent), as before.

Test plan.

  • New CurrentDatabaseScopeLoggerThreadingTests: (1) reflection: both tools have logger (an ILogger, optional) right before the CancellationToken; (2) a source scan of every non-test .cs under Darling/ that every CurrentDatabaseScope.(ReadAsync|ReadSnapshotAsync|ReadDroppedAsync|NarrowAsync|OmitDroppedAsync)( call expression mentions logger (floor: at least 5 calls found). Proved once on the old shape: removing logger: logger from ReadIndexObjectStatsAsync makes the scan fail (Total: 3, Failed: 1); restored, 3 of 3 pass.
  • Pin updated deliberately: DarlingMcpFinOpsRecommendationsToolsTests.ToolSurface_ExactlyGetFinOpsRecommendations_WithOnlyServerName pinned the exact parameter list postgres, server_name, cancellationToken; it now pins postgres, server_name, logger, cancellationToken, and still asserts only server_name carries a Description (the client-visible surface is unchanged).
  • Build Darling.Tests with --no-incremental: 0 Warning(s), 0 Error(s).
  • Darling.Tests, classes *CurrentDatabaseScope* *FinOps* *DroppedDatabaseScope* *DarlingMcp* *DarlingWebEndpoints* *WebDatabaseFilter* *WebReadCancellation* *WebReadReach* *WebExceptionText* *Census* *Ratchet* *McpToolsListBudget* *McpServiceParameter* *InventoryPin* *QueryStoreClutter* *TopRanking* *McpLatestSnapshotStamp* *EngineCapabilityMiss* *McpReadCommandTimeout* *WebTopProcedures* *ReadLatencyWeb* *Viewer* *DocCommentHygiene* *TsqlConventionGuard* *LiteReadsTrait*, after merging origin/dev: Total: 7233, Failed: 0, Skipped: 789. The skips are the live classes (no DARLING_TEST_PG).
  • Live Postgres classes (DroppedDatabaseScopeLiveTests, the FinOps index and recommendations live classes): compile with the new signatures but were not run, since no rig was started for a signature-only change. CI runs them.
  • Full suite: not run on this laptop; CI decides.

Lite parity. Nothing to mirror: Lite logs through AppLogger.Warn already (brief). No Lite file changed.

What the seat should double-check.

  • The reader parameter order (logger before cancellationToken) is why six live tests and three Viewer lines changed to a named token; that is mechanical.
  • The web dispatch now passes the logger for four tools; check that you want the two F12 entries (get_query_store_clutter, get_top_procedures_by_cpu) done here rather than in F12's section.

Changelog: none (internal logging).

F15: merge origin/dev (V179 Client Timeouts, #5640/#5714)

Merge commit 888911038e928c217dbac1c447514f95c6dfd0de, pushed to fix/5633-mcp-tool-audit. No code changed beyond the two conflict files.

Conflicts (2), both kept both sides

  • Darling/README.md, worker-sizing paragraph: kept the branch's RAM-bounded paragraph (8 GB example, floor and ceiling, 142 measured jobs) and added dev's parenthetical to it: "72 hypertables (the 71 collector tables plus collection_log; the 72nd collector table, attention_events, is a plain table with its own 30-day delete and is not counted)". Dev's "Today that is 74 and 85" lead-in was dropped, because the branch states the same 74 and 85 as the 4 GB floor. Dev's 71 -> 72 collector count (43 SQL Server) merged cleanly.
  • Darling/Darling.Tests/ReadmeDerivedCountPinTests.cs, worker-sizing pin: the branch's regex ("For an 8 GB host that is ...", figures from DarlingManagedPostgres.DeriveWorkerSettings) with dev's 4th figure, TimescaleSupport.HypertableTables.Count, and dev's comment.

Recounted figures (the pin produced them; nothing hand-typed): worker sizing did not change. 8 GB host is 128 background workers and 139 worker processes for 72 hypertables (HypertableCount, 71 collector hypertables + collection_log) and 29 aggregates. The attention table is plain, so the hypertable count is unchanged. Collector total 72 (43 SQL Server, 29 PostgreSQL).

Test plan (rig on port 5684, UTC, both preload libraries, stopped afterwards)

  • Darling.Tests and Lite.Tests build, 0 warnings, 0 errors.
  • Darling: ReadmeDerivedCountPin 5/5, DarlingManagedPostgresTests 231 (3 skipped), JobCountWithinWorkerSizingTests 2/2 (live), CiClusterWorkerSizingTests 5/5, CiComposeWorkerSizingTests 4/4, *Ratchet* 15/15, *Census* 548/548.
  • Darling classes that read README.md: RefreshCeilingProvenancePin 16, PgExtensionDependencyContract 3, CommentFilterAdoption 4, StoreObjectConvergence 14, AlertMasterSwitchSurface 9, BuiltinAlertPersistenceRung 11, FleetIdentifierScrub 3, UncorroboratedRouteStoreKnob 21, DarlingCliCommandsTests 34, DropXeSessionsVerb 106: all passed.
  • Lite: *Census* 67/67, *Ratchet* matches 0 tests, CrossAppMcpToolInventoryPin 7, CrossAppGuardCiGate 20, CiSelectMapSelectGuard 31, CiSelectReplayGuard 4: all passed.
  • Full suites not run on this laptop (CI decides).

Seat should double-check: the other README-reading test classes not in the list above were not run; CI covers them.

Changelog: none.

Answers to the bot review of 168d822

Claude posting for Erik Darling. Each finding from review 5496066203 and its five inline comments, with where it was answered.

Refuted claims

  • L2's "Defect found and NOT fixed" note: stale. F4 fixed the trace-flag turn-off in the five places the note named. F13 rewrote the note and L2's changelog entry.
  • L5's status line (Lite tests not updated, CPU helpers left in place): stale. F1 did both. F13 corrected the section.
  • F3's "the same way in both products" for the blocker tie-break: true when the review ran, fixed since. Lite's fallback now uses first(blocking_session_id ORDER BY wait_duration_ms DESC NULLS LAST, blocking_session_id ASC NULLS LAST), filtered to other-session blockers (989495f). That gives PostgreSQL's row on a tie and keeps a blocker whose wait time is NULL. Pinned by WaitingTasks_TiedAndNullWaitThreads_PickTheSameRowAsDarling, the three-row case the review asked for.

Inline findings

  • get_top_procedures_by_cpu claims "recorded nothing" after it narrowed the filter: fixed in F12, both products. Both page shapes now carry databases_left_out and a note, and the empty sentence covers only the databases the server still has. F12 also fixed the same problem in Query Store clutter and FinOps index analysis. FinOps recommendations has no "nothing found" sentence, so it needed no change.
  • Lite blocker fallback: see the third refuted claim.
  • Darling groups on the raw wait_type: fixed in F13. Darling now groups on and returns rtrim(wait_type), as Lite does, pinned by a live test with a pre-Wait names lose their trailing space, and two Hyperscale timer waits are ignored #4884 padded row.
  • Three copies of the empty trace-flag runs SQL: fixed in F13. There is now one definition (DarlingRetentionHorizons.EmptyTraceFlagRunsSql), used by the MCP reader and the viewer. The analysis copy reads collection_log, not v_collection_log, and its doc says why. Lite's copy is DuckDB SQL in another project.
  • p95Cpu7d now fed a 24-hour P95: renamed p95Cpu24h in F13.

Riskiest lines

  1. CurrentDatabaseScope's catch-all: it still fails open, by design, but it now logs a Warning each time. Lite logs through AppLogger.Warn. Every Darling MCP tool and web read entry that reaches it passes its logger (F12, F14). The web viewer's readers have no logger, as before, and keep the silent default. The live classes that pin the read run in CI's Darling PostgreSQL shards.
  2. The waiting-tasks array_agg rollup: pinned by WaitingTasksRollup5633Tests, live in CI. F13 added the padded wait-type case.
  3. The compare_analysis default baseline: intended (item 12). Unit-pinned by McpToolAudit5633L3Tests.ValidateBaselineWindow_*.

Trap: a latest-snapshot read must say when: this does not apply as written. Here the snapshot only decides which databases to leave out. The rows are windowed procedure stats with their own times. The payload now names what it left out and why.

Reviewer checklist

  • Live classes and censuses on the merged head: CI run 38327692216 on 47b64fe ran them with a PostgreSQL store and passed. The final head gets its own run.
  • DuckDB semantics for the blocker fallback: settled by the fixture above.
  • get_waiting_tasks on pre-Wait names lose their trailing space, and two Hyperscale timer waits are ignored #4884 rows: F13's live test.
  • get_top_procedures_by_cpu over 168 h with only dropped-database activity: F12's live test (Darling) and tool test (Lite), each shown failing on the old shape.
  • The rig on port 5684: no rig listens on ports 5681-5687.
  • The six WPF grids, whose header now reads "Spilled Pages" at widths 140/120: not looked at on screen. This is left for Erik at merge.

Not re-measured: L6's plan claim (one indexed read per call, look-back capped at 7 days) was never timed on a large database_states.

W2A: the MCP role can write the collection caveats it persists (item 13) - replaced by F8

Lane W2A first fixed item 13 by granting the mcp role INSERT, UPDATE and DELETE on collect.analysis_collection_caveats. The wave 2 security review found that this let an as_of run of analyze_server overwrite the current caveats, and let any MCP client rewrite caveat rows for any server. F8 removed the grant and took a narrower approach: only the scheduled analysis pass writes that table, so the mcp role needs no new privilege. See the F8 section. This section has no changelog entry; F8's entry replaces it.

W2B: enough background workers for the store's jobs (item 14) - sizing replaced by F9

Lane W2B measured the problem on a rig. A store built by the migrations carries 142 TimescaleDB jobs (94 compression, 23 refresh, 23 retention, 2 built in). With the old 74 workers, a restart with every job overdue logged 61 "out of background worker slots" launch failures. They were retried and every job ran, so the cost was a noisy, late first pass. W2B raised the workers to a fixed 160. The wave 2 review asked for the memory cost of that on a small host, so F9 measured the restart burst and replaced the fixed number with sizing by host memory. See the F9 section. This section has no changelog entry; F9's entry replaces it.

…temp-table exclusion in get_table_index_sizes, Table not read class in get_index_usage (#5633 items 6, 15, 16)
…entory, Parallelism wait category (#5633 items 11, 20, 22)
…ead_latency percentiles above max (#5633 items 10, 12, 23)
…erver memory 0 vs 16 is a match (#5633 items 3, 19, 9)
…e dead CPU P95 and span reads, pin updates, CA1068
… as_of and keeps newer rows; Lite recommendations skip dropped databases (#5633)
…icy job (#5633 item 14)

A migrated store carries 142 policy jobs, all overdue at any restart, against the old hypertables + 2 (74) slots: 61 launch failures logged on one store. Workers are now 2 x hypertables + 16 (160 / 171 at 72 hypertables); compose, README and the runbook follow, and the conf migration still recognises the legacy blocks as ours.
…s writes collect.analysis_collection_caveats (#5633 item 13)
…old floor and the policy-job ceiling (#5633 item 14)
…it_config latest lookup, clutter fleet anchor, database size column order, shared CPU stand-down predicate, trace flag empty path (#5633)
…s no longer read as bare database_states; trace-flag window test seeds runs that stored the flag (#5633)
…ped databases they left out; the current-database lookup logs its fail-open (#5633)
@erikdarlingdata
erikdarlingdata dismissed claude[bot]’s stale review October 13, 2026 03:03

Claude posting for Erik Darling. Review of 168d822: every finding is answered in the PR body's RV section and fixed on the branch by F12, F13 and F14 (head f491226, own CI run 38336000805 green).

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