Data family: 7/10 MCP tool descriptions converted, 3 large ones handed off (#3898)

[erikdarlingdata]

Refs #3898.

Why

Issue #3898: Darling's MCP tools/list catalog is huge before the first question. This lane's brief covered 10
"Data" family tools (Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpDataTools.cs and their Lite
twins). Base branch: feature/3898-content-data was branched from origin/feature/3898-parallel-safe-budget
(PR #4054 was still open/unmerged at dispatch time; this diff will show #4054's files until it merges into dev,
which is expected per the brief).

What changes (7 of 10 tools converted, both SKUs)

• get_wait_stats, get_file_io_stats, get_perfmon_stats, get_server_properties: Darling and Lite carry
  byte-identical original text, so the same head/tail split applies verbatim to both.
• get_top_procedures_by_cpu and get_top_queries_by_cpu: shared an identical ~460-char sentence about
  min/max_cpu_ms lifetime extremes and cpu_attribution's omitted-not-invented ratio. That sentence is now the
  cpu_time_extremes_and_attribution topic in PerformanceMonitor.Common/Mcp/McpToolGuideTopics.Data.cs,
  appended by both tools' tails on both SKUs. get_top_queries_by_cpu also had two over-cap parameters
  (group_by 616 chars, parallel_only 371 chars); both trimmed to their guardrail sentence (<=200 chars),
  remainder moved into the tool's own tail. Lite has no group_by parameter (no host_object rollup feature) so
  its tail omits that content; Lite's distinct_texts zero-meaning sentence differs slightly from Darling's
  ("no stored text for the group" vs "only rows predating the text dimension") and each side's own original
  wording was preserved rather than reconciled (did not verify which is more accurate against the read path;
  flagging rather than guessing per D9 rule 9).
• list_servers (D10 entry tool): Darling and Lite bodies diverge for real, not just wording -- Darling's status
  is derived purely from collection recency (no live connection); Lite's status IS a live connection check
  (ServerManager.GetConnectionStatus, confirmed via LocalDataService.Overview.cs's doc comment). The shared
  head states both facts in one clause per D6 ("Darling has no live connection...; Lite's status IS a live
  connection check..."). Darling-only detail (engine_kind/sql_version deprecated-alias trap, peer_fleets) stays
  in Darling's own tail only; Lite's tail is just its original "Use this first..." sentence.

Per-tool before/after served chars (both SKUs identical except list_servers, noted):

Tool	Darling before -> after	Lite before -> after
get_wait_stats	644 -> 316	644 -> 316
get_file_io_stats	690 -> 425	690 -> 425
get_perfmon_stats	1034 -> 509	1034 -> 509
get_server_properties	894 -> 559	894 -> 559
get_top_procedures_by_cpu	821 -> 406	821 -> 406
get_top_queries_by_cpu	1837 -> 474	1606 -> 474
list_servers	1228 -> 382	160 -> 382 (deliberate growth, banked)

Corrections

None identified in the converted tools.

D4 removals

None of the 7 converted tools carried issue-number references or anecdotes in their original text.

Re-pointed pins

• Darling.Tests/McpToolGuideTests.GetToolGuide_ServesTails_AndTopics_AndReportsUnknownNames and
  GetToolGuide_WithNoArguments_IsTheIndex: this fixture's HostCatalog() wires up DarlingMcpDataTools
  directly, so once any of its tools gained a real guide the hardcoded index/no-separate-guide expectations broke.
  Swapped the probe tool from list_servers (now guide-bearing) to get_cpu_utilization (still unconverted),
  and added the 7 newly-guided tool names to the expected index, in ordinal sort order. Same fix mirrored in
  Lite.Tests/McpToolGuideTests for its list_servers-only overlap.
• New McpToolGuideHeadsDataTests (both SKUs, pattern copied from McpToolGuideHeadsHealthParserTests): pins
  each converted head's guardrail phrase, the 620-char/200-char caps, the shared topic riding both top-by-cpu
  tails, and that list_servers' Darling-only peer_fleets/engine_kind detail never reaches the shared head.

D9 traps (for the 7 converted tools)

1. Q: get_wait_stats returned fewer rows than expected: were there really only that many wait types?
   Correct: limit bounds the page; truncated says whether more existed.
   Prevents it: head's "Bounded by limit: wait_types_returned is how many you got; truncated means the window held more."
2. Q: Can I sum total_wait_time_ms across the page to get the server total?
   Correct: no -- the page holds only the heaviest waits.
   Prevents it: head's "a sum over the page is not a sum over the server."
3. Q: get_file_io_stats/get_perfmon_stats/get_server_properties -- is hours_back relevant here?
   Correct: these are latest-snapshot reads, not windowed; there is no hours_back parameter at all.
   Prevents it: head's "Reads the newest snapshot, not a window; captured_at is when it was collected."
4. Q: get_file_io_stats shows avg_read_latency_ms: null -- is that a 0ms read?
   Correct: sample_interval_seconds: 0 means no delta was knowable (first sighting/counter reset/gap); null, not 0.
   Prevents it: head's explicit null-vs-0 sentence.
5. Q: get_perfmon_stats shows delta_value: null on a row -- is that missing data?
   Correct: only if counter_kind is "gauge" (a level has no delta); check counter_kind first.
   Prevents it: head's gauge/rate/other breakdown.
6. Q: get_top_queries_by_cpu with parallel_only=true returned zero rows -- does this server have no query stats?
   Correct: the filter ran window-wide before the cap; zero means no group ever ran parallel, not a collection miss.
   Prevents it: head's "an empty page under it is the window's real answer, not a miss."
7. Q: min_cpu_ms/max_cpu_ms on a get_top_queries_by_cpu/get_top_procedures_by_cpu row look inconsistent with total_cpu_ms for the window -- bug?
   Correct: they're lifetime extremes for the plan's whole cache lifetime, not windowed.
   Prevents it: head's "min/max_cpu_ms and min/max_elapsed_ms are LIFETIME extremes, not windowed."
8. Q: cpu_attribution.attributed_cpu_ratio is missing from the payload -- is that a bug?
   Correct: omitted deliberately when the CPU series or core count is missing/insufficient, never invented.
   Prevents it: head's "cpu_attribution's ratio is omitted, not invented, when its inputs are missing."
9. Q: list_servers shows a server Offline on Darling -- does that mean the collector can't reach it right now?
   Correct: Darling never live-connects; Offline means stale collection history, nothing about reachability now.
   Prevents it: head's "Darling has no live connection to monitored servers: status is derived from how recently each was collected."
10. Q: Same Offline status read on Lite -- same meaning?
    Correct: no -- Lite's status IS a live connection check, independent of whether anything is being collected.
    Prevents it: head's explicit Lite clause, stated beside Darling's in the same sentence.

Test plan

• dotnet build Darling/Darling.Tests/Darling.Tests.csproj -c Debug -- 0 Warning(s), 0 Error(s).
• dotnet build Lite.Tests/Lite.Tests.csproj -c Debug -- 0 Warning(s), 0 Error(s).
• git merge origin/dev done (fix round 1; see below).
• dropcheck.py origin/dev HEAD on all 7 tools, both SKUs: all 14 lines missing=0.
• Full Darling.Tests: Total 13120, Errors 0, Failed 0, Skipped 631 (no local PG rig, expected).
• Full Lite.Tests: Total 5224, Errors 0, Failed 0, Skipped 0.

Fix round 1

Merged origin/dev (#4054 had landed). The merge conflicted on 9 add/add files (budget txt files, the shared
McpToolGuideTopics.Data.cs) and 2 content conflicts (McpToolGuideTests.cs fixture lists) -- all mechanical:
dev's copies of these files were still the pre-conversion placeholders this PR fills in (dev hasn't merged this
PR yet), so every conflict resolved to this branch's already-converted side. Verified with git show on both
refs before resolving, not just picked blindly.

Restored 14 dropped sentences the first pass had paraphrased into the head instead of moving (dropcheck
missing count went from 22 to 0 across the 14 tool/SKU rows). The head's paraphrase is new text and never
replaces an original sentence, so each original went back into its tail verbatim, next to the paraphrase where
one already existed:

• get_wait_stats (Darling + Lite): 1 each -- the opening sentence.
• get_file_io_stats / get_perfmon_stats (Darling + Lite): 2 each -- the opening sentence, and the tool's
  original full "LATEST IS A TIME: this reads the newest ... snapshot..." sentence (the head now uses the
  shorter catalog convention phrase, so the original full sentence lives only in the tail).
• get_server_properties (Darling + Lite): 2 each; the tail's two truncated fragments were replaced with the
  two full original sentences so no partial duplicate prose remains.
• get_top_queries_by_cpu: Darling 3 (opening sentence; Set group_by='host_object'..., which Lite's original
  never had; min/max_cpu_ms...extremes_note, reinserted adjacent to max_dop comes from... as in the
  original), Lite 2 (opening sentence; the same min/max_cpu_ms...extremes_note sentence).
• list_servers: Darling 2 (the full "Lists all monitored servers..." sentence, replacing a truncated
  paraphrase; "The service has no live connection..."), Lite 1 (the full "Lists all monitored SQL Server
  instances with their current status and last collection time." sentence, replacing a truncated fragment).
• get_top_procedures_by_cpu: 0, already clean.

Head wording (task 2): get_file_io_stats, get_perfmon_stats, get_server_properties (both SKUs) now open
their latest-snapshot clause with the catalog convention phrase: "Reads the newest snapshot, not a window" ->
"LATEST IS A TIME: the newest snapshot, not a window" (rest of the sentence and captured_at unchanged). Heads
stayed byte-identical Darling/Lite, served at 437 / 521 / 571 chars respectively (all under the 620 target).
Updated McpToolsListBudget/DarlingMcpDataTools.txt, .../McpIoTools.txt, .../McpPerfmonTools.txt,
.../McpServerInfoTools.txt (both products, exact values from the test failure messages) and the three
HeadFacts entries in McpToolGuideHeads.Data.cs (both products) to the new phrase.

The failing pin: Darling.Tests.McpLatestSnapshotStampTests.EveryLatestSnapshotTool_SaysLatestIsATime_InItsDescription_OnBothSkus
(a Darling-side cross-check that also reads Lite's source as text -- not the Lite.Tests copy the Handoff
guessed at). Its DescriptionOf helper's regex only understood a single string literal or a single const
identifier, not this PR's "head" + McpToolGuide.Marker + "tail" concatenation, so it threw "could not locate
the tool's Description" for every converted latest-snapshot tool. Added a fallback that walks the attribute text
and joins quoted literal chunks (skipping the marker/topic identifiers between them) when the simple forms don't
match; the head-wording fix above then satisfies the pin's Assert.Contains. Found and fixed the identical
defect in McpPageContractTests.EveryLitePagedTool_SaysWhatBoundsItsPage_InItsDescription (it now silently
matched a parameter's [Description(...)] instead of the tool's, once get_wait_stats converted) with the
same join approach (DescriptionLiteralConcat). Both are small, mechanical parser fixes confined to test-only
helper code, not behavior changes -- fixed in-lane rather than deferred, since they're a direct, small
consequence of this PR's own description-shape change.

Handoff

Ran out of context mid-task (hit the 300k hard wall). Three tools from the original 10-tool brief are NOT
converted -- left with their original, unmarked descriptions, no test or budget changes needed for them:

1. get_query_store_top (609 chars). Non-trivial: it appends the SHARED constant
   McpHelpers.WindowTruncatedDescription (also used by DarlingMcpTrendTools.cs, DarlingMcpQueryStoreClutterTools.cs,
   Lite's McpQueryTools.cs -- other lanes' tools). Darling.Tests/McpPayloadContractCensusTests.EveryWindowFloorTool_CarriesTheSharedClause_AndNoOtherToolDoes
   already anticipates conversion: it requires (a) the source still literally reference
   McpHelpers.WindowTruncatedDescription by name (don't inline/duplicate its text), (b) the tool's FULL
   description still contain the constant's value as one CONTIGUOUS substring (so the marker must NOT split
   the constant itself), and (c) the served HEAD still contain the literal substrings window_truncated and
   not a page cut. The way to satisfy all three without touching the shared constant (which would ripple into
   other lanes' tools): write your OWN short head sentence containing those two exact substrings (e.g.
   "window_truncated marks this window floor (not a page cut; no limit changes it); effective_start /
   effective_hours_back give the reach actually served."), place McpToolGuide.Marker after that, then in the
   tail keep the tool's other original prose and append + McpHelpers.WindowTruncatedDescription UNCHANGED
   (so the constant's full text, wire-change notice included, lands entirely in the tail as one contiguous
   block). Do not edit McpHelpers.cs.
2. get_collection_log (6,995 chars). Three parameters exceed the 200-char D2 cap and need trimming (guardrail
   part kept in the parameter description, remainder moved to the tool's tail): collector_name (244 chars),
   min_duration_ms (382 chars), status (481 chars). The description itself has no issue-number refs to
   remove. Otherwise a normal (large) head/tail split.
3. get_collection_health (21,026 chars, the largest description in the catalog). Needs careful D4 pruning.
   Only one issue reference found: "before The alert pass retries a store read once, two seconds later, before counting it failed — every alert-read kill this week was a transient inside a write band that a retry survives #3848" in the retried_reads paragraph (delete the ref, keep "Every
   one of those was a swallowed failure, so read the two together..."). Several anecdotes carry a rule that must
   survive with the anecdote's specific numbers removed: the ".453 install" pg_statement_stats story, the
   "query_store on one dogfood server averaged 13,834ms..." story (keep "Read the three together: avg close to
   p95..." rule), the "measured on two real instances, dominance 3.58 over 72 items..." story (keep "remediation
   routes through SHARE, not dominance" rule), the "pg_deadlocks...49,258,335 ms...stored zero rows" story (keep
   "A verdict keyed on cost-plus-zero-rows would fire on the healthy quiet install" rule), and the "pg_deadlocks
   moved from an in-database route to an AWS API route...six days later" story (keep "Do not infer a live
   condition from last_error alone" rule). The shared constant AlertReadFailureCounter.FleetScopedReads (used
   in the alert_read_health block) is already resolvable by the generic identity test via KnownConstantClasses
   in Darling.Tests/McpToolGuideTests -- do not edit that constant or its owning file (PerformanceMonitor.Alerting/AlertReadFailureCounter.cs),
   it's shared alerting-subsystem code with its own issue refs, out of scope here. Tail will comfortably fit
   under the 24,000-char get_tool_guide budget even after a terse head is carved out.

Fix needed for the one failing test before this PR is mergeable: read
Lite.Tests/McpLatestSnapshotStampTests.cs around line 296 to see the exact phrase
EveryLatestTool_SaysLatestIsATime_AndNamesCapturedAt checks for, then adjust the heads of get_file_io_stats,
get_perfmon_stats and get_server_properties (both SKUs, to stay byte-identical) to include whatever exact
wording that pin requires (possibly "LATEST IS A TIME" verbatim, or a look at the assertion will show the actual
substring test). This same pin should be re-checked for get_server_properties/get_perfmon_stats too even
though only get_file_io_stats was reported (the run may have stopped at the first failure, or those two may
already coincidentally pass -- verify both).

🤖 Generated with Claude Code

https://claude.ai/code/session_01M7r1CXmEcBmsrTV34CCLfF

[erikdarlingdata]

Coordinator D9 misreading check (haiku, served tools/list text only, no get_tool_guide; the 7 converted tools): 10/10 correct, with correct reasoning, each answer quoting its evidence from a head (traps 1-10 in the body). Separately, a dropped-sentence check (plans/3898-d9/dropcheck.py, old description vs new head + tail) found 13 original sentences paraphrased in the heads instead of moved verbatim to the tails. Among them are the three pinned 'LATEST IS A TIME' sentences, which is why Lite's McpLatestSnapshotStampTests fails. Fix round 1 restores them, then runs the test classes that pin these tools and both full suites. The 3 unconverted tools (get_collection_health, get_collection_log, get_query_store_top) go to a separate lane after this merges.

[erikdarlingdata]

Coordinator, after fix round 1: the seven Data tools wrote the split as a concatenated McpToolGuide.Marker. That compiles to the same string, but the generic cross-SKU identity test finds markers by scanning source for the literal <>, so it read all seven as unconverted and never compared their heads. 806d97e rewrites them as the literal (Split trims both sides, so the served text is byte-identical and no budget line moved), and ScanToolsUnder now refuses a concatenated marker. Red-watch: putting one back fails with 'get_wait_stats: write <> inside the Description literal...'. With the heads now actually compared, the identity test passes, so the twins' heads are byte-identical. Targeted, both SKUs, green: McpToolGuide*, McpToolsListBudget*, McpLatestSnapshotStamp*, McpPageContract*, McpDescription*, McpZeroIsAMeasurement*, McpPayloadContractCensus*. dropcheck: missing=0 on all 14 tool/SKU rows.