[BUG] MCP tool audit on 3.9.0-nightly.20261006.515: 23 defects (blocking, spills, FinOps, store, tool payloads) #5633

Component

Darling

Performance Monitor Version

3.9.0

SQL Server Version

MCP, Not SQL Server

Windows Version

Windows

Describe the Bug

I ran an audit of the MCP tools against a live install. This issue lists every defect it found, 23 in total.
Each item says what the tool returned, the cause where I could find it in the code, and the expected
behavior.

Setup

• Build: 3.9.0-nightly.20261006.515, a headless service with the managed PostgreSQL 18 and TimescaleDB store.
• Fleet: four SQL Server 2022 Developer Edition instances. Most results come from one 16-CPU test instance with
  about 3 days of data.
• The service host is on a power schedule. It is off every night and on weekends, so collection has a gap of
  about 7 hours each night.
• Coverage: 153 of the 177 tools were called. The 22 tools that add, change or delete configuration were not
  called.
• get_procedure_plan_xml and analyze_procedure_plan could not be tested. No user procedure ran in the window,
  so there was no procedure sql_handle to pass.
• Code references are against dev at 71e66d9. Line numbers are approximate.

Index

2. The spill headline counts pages as spills

The headline said "186,014 query spills to tempdb — memory grants too small for their sorts/hashes". The value is
the sum of total_spills deltas from sys.dm_exec_query_stats. That column counts pages spilled, not spill
events. So 186,014 is about 1.4 GB of spilled pages over the window. No tempdb sample in the window was above
300 MB.

Where: PerformanceMonitor.Analysis/FactAdvice.cs, ComposeQuerySpills, line 1601. Its headline and investigation text
use the nouns "spill" and "query spill" for that value.

Expected: call the value pages, or convert it to MB. Alternatively, count spilling executions.

3. get_top_procedures_by_cpu gives the wrong reason for an empty result

With hours_back = 48, the tool said: "No procedure stats available. Delta-based collection requires at least two
collection cycles (~30 minutes) to produce non-zero values." At the same time, get_collection_health showed
procedure_stats as HEALTHY, with 2,395 runs and 162 stored rows over 7 days. The result itself was correct,
because no user-database procedure ran in the window.

Cause: Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpDataTools.cs, line 1059, returns that text for
every empty page. Lite/Mcp/McpQueryTools.cs, line 209, has the same text.

Expected: give the "two cycles" reason only when the collector is new. Otherwise, say that no procedure ran in
the window.

4. Dropped databases are reported as current

get_query_store_clutter (hours_back = 48) returned 13 databases. Several of them were scratch databases that had
been dropped before the call. One dropped database got the verdict Critical (qs_read_only), with the text "New
runtime statistics are being dropped". get_query_store_health correctly listed only the 4 databases that exist.

Other tools show dropped databases in the same way:

• get_finops_recommendations lists a dropped database among databases with low I/O latency.
• get_finops_recommendations also says to compress dbo.r (1.8 GB). It does not name the database, and no
  current database has a table with that name. The table was in a dropped database.
• get_finops_index_analysis lists four dropped databases.
• get_top_procedures_by_cpu with hours_back = 168 returns only rows from a dropped database.

The query_store backfill also kept retrying dropped databases every 5 minutes. This may be the same cause.

Expected: mark a database that no longer exists as dropped, and keep it out of verdicts and
recommendations. Name the database in every object-level recommendation.

5. Typo in the plan-warning advice

PerformanceMonitor.Analysis/FactAdvice.cs:2202 says "accidental cross joops". It should say "accidental cross
joins". The text appears in the PLAN_WARNING investigation from analyze_server.

6. get_database_sizes shows percent growth as null

The tool returned "auto_growth_mb": null, with no other growth field, for the four master and msdb files.
sys.master_files shows all four at 10% growth (growth 10 with is_percent_growth set). A reader cannot tell
this from "unknown".

Cause: the store has is_percent_growth and growth_pct, and other readers use them
(DarlingAlertReadAdapter.cs, line 1054). DarlingObjectStatsReader.DatabaseSizeLatestSql (line 667) selects only
auto_growth_mb. DarlingMcpObjectStatsTools.FilePayload (line 645) publishes only that field.

Expected: add is_percent_growth and growth_pct to the payload, as DarlingMcpFinOpsTools.DatabaseSizes.cs
already does (line 102).

7. The monitor's own queries are not filtered out of query_stats

In the CPU heatmap for the busiest user database, 46 of 97 cells had one of the monitor's own collector queries
as the most-executed query. Examples:

• the plan correction collector's read of the automatic tuning options
• the plan correction collector's read of Query Store query text
• the database size collector's load of its #file_space temp table

Cause: PerformanceMonitor.Collectors/QueryStatsCollector.cs (lines 241 and 263) removes the monitor's own
statements with a text-match filter on the batch text. The filter looks for the marker comment
PerformanceMonitorLite. Only QueryStatsCollector.cs and QueryStoreCollector.cs put that marker in their SQL.
PlanCorrectionCollector.cs, DatabaseSizeStatsCollector.cs and most other collectors have no marker, so their
statements are kept.

Expected: put the marker on every collector statement. Alternatively, the collector runner could add it to
every batch it sends.

8. Anomaly text compares a robust, capped σ with the mean

The ANOMALY_QUERY_DURATION investigation text said:

Total query duration reached 15.6 s this window, 25σ above its 3.8 s baseline for this hour-of-week (over 23
baseline samples); the window's mean of 1.7 s sat 25σ above it ...

A mean of 1.7 s cannot be 25σ above a 3.8 s baseline.

Cause:

• When the bucket has robust statistics, AnomalyGate.Decide measures both σ values from baseline.Median, with
  the MAD-derived sigma (PerformanceMonitor.Analysis/AnomalyGate.cs, line 371).
• Both values are capped at SigmaDisplayCap, which is 25.0 (Baselines/AnomalyThresholds.cs:105).
• ComposeAnomaly (FactAdvice.cs, line 1857) prints them next to baseline_mean. It never shows the median,
  and it does not say that 25 is the cap.

The wait-profile composer already handles this case. It says "robust σ above the baseline median"
(FactAdvice.cs, line 2043).

Expected: name the median as the baseline when the robust frame was used. Say "at least 25σ" when the value
is at the cap.

9. min server memory is flagged as a mismatch (minor)

get_server_config returns min server memory (MB) with value_configured 0, value_in_use 16 and
values_match: false. SQL Server always runs a configured 0 as 16. It was the only mismatch among the 97
settings. A reader or an agent will take values_match: false as a pending change.

Expected: treat 0 against 16 as a match for this setting, or add a note to that row.

10. audit_config reports Developer Edition as "Enterprise"

audit_config returned "edition": "Enterprise". get_server_properties, get_finops_inventory and the FinOps
views all say "Developer Edition (64-bit)", with engine_edition 3. Engine edition 3 covers Enterprise,
Developer and Evaluation, so it cannot name one edition by itself. For FinOps this matters, because Developer
Edition has no license cost.

Expected: use the Edition server property, as get_server_properties does.

11. FinOps gives two different CPU recommendations in one response

get_finops_recommendations returned both of these for the same server:

Cause: two rules fire independently in Darling/PerformanceMonitor.Darling.Storage/FinOps/FinOpsRecommendationFigures.cs:

• CpuRightSizing (line 286) uses the utilization P95.
• VmRightSizing (line 334) uses p95Cpu7d. Its text says "From 4,206 samples over 2 days".

The two rules use different windows and different formulas. Also, "16 cores" is 16 logical CPUs.
get_server_properties shows 1 socket with 8 cores.

Expected: one CPU recommendation per server, from one window. Say whether the count is cores or logical
CPUs.

12. compare_analysis accepts overlapping windows

With hours_back = 24 and the default baseline_hours_back of 28, the windows were:

The two windows share 20 of their 24 hours. All 90 facts came back "stable", which overlapping data always gives.

The tool checks only that baseline_hours_back is greater than hours_back. The windows stop overlapping only
when baseline_hours_back is at least twice hours_back. The default of 28 fits the default hours_back of 4
("yesterday, same time"). It is not adjusted when the caller changes hours_back.

Expected: refuse or warn about overlapping windows. When the caller does not set baseline_hours_back,
default it to hours_back plus 24.

13. analyze_server over MCP gets "permission denied" on analysis_collection_caveats

get_store_log shows this ERROR at the same second as an analyze_server call:

PerformanceMonitorDarling-Mcp ERROR: permission denied for table analysis_collection_caveats

The failed statement was the delete of that server's rows from collect.analysis_collection_caveats. The
analyze_server response still said "persisted": true.

Cause:

• The analysis pass writes collection caveats (Darling/PerformanceMonitor.Darling.Analysis/DarlingAnalysisService.cs,
  line 455).
• The MCP role has insert rights on collect.analysis_findings (DarlingManagedRoles.cs:953), but no grant on
  collect.analysis_collection_caveats.
• CollectionCaveatStore logs the failure and continues, by design. So every analysis run from MCP leaves the
  caveat table unchanged and writes an ERROR to the store log.

Expected: give the MCP role insert, update and delete rights on collect.analysis_collection_caveats, or
skip the caveat write when the pass runs as the MCP role. Report a partial write in persistence_note.

14. The store runs out of background workers at every service start

At the service start, the store logged this 22 times in one second:

WARNING: failed to launch job 1070 "Columnstore Policy [1070]": out of background workers

get_store_metrics shows 136 background jobs. get_store_host shows timescaledb.max_background_workers = 74 and
max_worker_processes = 85, both from the managed settings block. This host starts every weekday morning, so the
warning appears every day. The jobs run later on their next schedule, so the effect may be only a delay.

Expected: size the worker settings for the number of jobs, or stagger the job start times after a restart.

15. get_table_index_sizes sorts NULL sizes first, and includes the collector's own temp tables

The first 7 of the 100 rows were tempdb temp tables, shown as 0 MB: #foreign_key_referenced…,
#index_compression…, #index_definitions…, #ops…, #index_columns…, #foreign_key_supporting… and
#usage…. Row 8 was the largest table on the server, at 310 GB.

Cause:

• These are the temp tables that PerformanceMonitor.Collectors/IndexObjectStatsCollector.cs creates for itself,
  for example #usage, #ops and #index_columns, at lines 161, 187 and 209. The collector then reads them
  back from tempdb with the user tables.
• Their size is NULL. DarlingObjectStatsReader.cs (line 215) orders by current_reserved_mb descending, without
  NULLS LAST. PostgreSQL puts NULLs first in a descending sort.
• The payload then shows the NULL as 0. The get_index_usage query in the same file already uses NULLS LAST.

Expected: add NULLS LAST, and leave the collector's own # tables out of the snapshot.

16. get_index_usage labels clustered indexes "Unused", and the reset caveat is incomplete

44 of the 75 rows were CLUSTERED indexes classified as "Unused". The first row was the clustered primary key of
the largest table, at 308 GB. A clustered index is the table itself, so it is never a drop candidate.

The tool description says "Counters reset at the last restart or index rebuild". SQL Server on this instance had
been up for three weeks. The usage counters reset five days earlier, when the database was restored. A restore,
a detach and attach, or taking the database offline also resets them. A reader who uses the restart date will
think that the counters cover three weeks.

Cause: the classification CASE in DarlingObjectStatsReader.cs (line 310) does not check index_type_desc.

Expected: classify heaps and clustered indexes separately, for example "table not read since …". Name
restore and offline in the reset caveat. Report the age of the counters for each database.

17. run_custom_view_panel returns no rows for a display name or an unknown server, with no message

The panel spec passed validate_custom_view. The results:

The generated SQL matches the server value against the stored server_name column only. Every other tool accepts
"Server name or display name" and refuses an unknown name.

Expected: resolve display names the way the other tools do, and refuse a name that matches no server.

18. get_waiting_tasks rows cannot be told apart for parallel queries

Of the 30 rows returned, only 19 were distinct. 29 of the 30 showed a session waiting on itself, for example
session 97, CXCONSUMER, blocking_session_id 97. The payload has no exec_context_id or
blocking_exec_context_id. So the threads of one parallel query look like exact duplicates, and like a session
that blocks itself. This is the same pattern as item 1.

Expected: return the execution context ids, or roll parallel threads up to one row per session and wait type,
with a thread count.

19. get_trace_flag_changes gives the wrong reason for an empty result (minor)

The tool said "No configuration change history yet: fewer than two config snapshots have been captured", with
snapshot_count 0. The trace_flags collector had run 6 times with no errors. It stored nothing because no
trace flags were on, which a global DBCC TRACESTATUS confirmed. The server-config and database-config change tools had
7 snapshots for the same period.

Expected: when the collector ran and found no flags, say that no trace flags are enabled.

20. FinOps inventory shows $0 when no cost is set, and a second health score (minor)

get_finops_inventory returned monthly_cost_usd: 0 and annual_cost_usd: 0 for all four servers.
get_finops (utilization) and get_finops_recommendations returned null with "monthly cost not set" for the
same server. A value of 0 reads as "free", and null reads as "unknown". For the same server at the same time,
health_score was 92 in the inventory and 98 in the utilization view.

Expected: return null with the reason in the inventory too. Use one health score, or say how the two differ.

21. Plan analyzers return "dop": 0 for parallel estimated plans (minor)

analyze_query_plan, analyze_plan_xml and analyze_query_store_plan all returned "dop": 0 and
"serial_reason": null for one query. Its plan has 64 operators marked parallel, and the plan cache shows
that it ran at DOP 6. ShowPlanParser.cs:665 reads the DegreeOfParallelism attribute of the QueryPlan element. Estimated plans from the plan
cache and Query Store do not carry that attribute. So the 0 means "not recorded", but it reads as a measurement.

Expected: return null and a parallel flag, from the operators, when the plan does not record DOP.

22. FinOps counts parallelism waits as CPU (minor)

In get_finops (optimization, wait_categories), DarlingFinOpsOptimizationReader.cs:190 maps CXPACKET,
CXCONSUMER, CXSYNC_PORT and CXSYNC_CONSUMER to "CPU", together with SOS_SCHEDULER_YIELD. It maps
HTDELETE, HTBUILD and HTREPARTITION to "Other". On the test instance, "CPU" was 80.7% of all waits, and
CXCONSUMER was the top wait. Query Store's own wait categories have a Parallelism category that includes
CXPACKET and the HT waits.

Expected: add a Parallelism category, as Query Store does.

23. get_read_latency shows percentiles above the maximum (minor)

For the analyze_server route, max_ms was 3,568, and p50_ms, p95_ms and p99_ms were all 5,000. The note
says these are bucket upper bounds. Still, a p50 above the maximum of the same reads looks like an error.

Expected: cap each percentile at max_ms.

🤖 Generated with Claude Code

Steps to Reproduce

Run the MCP tools listed above.

Expected Behavior

That this stuff gets fixed, Erik.

Actual Behavior

It's not fixed, Erik.

Error Messages / Log Output

Screenshots

No response

Additional Context

No response