get_pvs_stats moves reading guidance off tools/list (#3898)

[erikdarlingdata]

Refs #3898.

Why

Issue #3898: a client that loads Darling's whole MCP catalog pays for every tool description before it asks a
question. get_pvs_stats is one of the twin tools that carries that cost on both products (1,156 served
characters each). This PR moves its reading guidance off tools/list and into the per-tool guide that
get_tool_guide(tools[], topics[]) serves on demand. Nothing is deleted. The full original text now lives in the
tail, and get_tool_guide returns it by tool name.

What changes

• Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPvsTools.cs and Lite/Mcp/McpPvsTools.cs: split
  get_pvs_stats's Description(...) into a head (served on tools/list) and a tail (served by
  get_tool_guide), using the literal <<GUIDE>> marker. The two products' descriptions were byte-identical
  before this change (confirmed with desc.py) and stay byte-identical after it.
• Darling/Darling.Tests/McpToolGuideHeads.Pvs.cs and Lite.Tests/McpToolGuideHeads.Pvs.cs (new): head pins for
  the tool, following the HealthParser and Data family pattern.
• Darling/Darling.Tests/McpToolsListBudget/DarlingMcpPvsTools.txt and
  Lite.Tests/McpToolsListBudget/McpPvsTools.txt: updated the tool get_pvs_stats line to the new served length.
  The two param lines are unchanged. No parameter was renamed, added, or removed.
• No SQL, payload shape, or tool behavior changed. This PR touches descriptions only.

Before/after served characters

Tool	Darling before	Darling after	Lite before	Lite after
get_pvs_stats	1,156	620	1,156	620

The served head (620 chars) is the new head text plus the 31-character pointer that tools/list appends
(" Reading guide: get_tool_guide."). The cap is 620.

Corrections

One, made during coordinator verification. The tail said pct_of_database "is null only when the numerator was
not measured or the denominator is absent". PctReason also returns a reason, and the share is null, when the
database size is 0 (DarlingMcpPvsTools.cs PctReason, and Lite's twin in McpPvsTools.cs). The tail now
says "the denominator is absent or zero". dropcheck.py lists that one sentence as changed on each product.

D4 removals

None. The served text (head and tail) carries no issue reference and no anecdote to remove.

Re-pointed pins

One pin was re-pointed during coordinator verification.

Darling/Darling.Tests/McpZeroIsAMeasurementTests.cs already tests this tool
through two helpers, DescriptionOf and ToolDescription. Both already resolved to
McpToolGuide.Split(...).Head before this PR. For an unconverted tool the head is the whole literal, so the pins
passed either way. The pin Assert.Contains("pvs_measured says whether the DMV reported a size", darling, ...)
in BothSkusPvsDescriptions_StayByteIdentical_AndTeachPvsMeasured now exercises the real head/tail split for the
first time.

The coordinator's head rewrite teaches the flag inside the pct_of_database rule instead, so the pin now checks
"PVS size is unmeasured (pvs_measured false)". It still checks that the served head teaches pvs_measured. The
tail keeps the original "pvs_measured says whether the DMV reported a size" sentence.

D9 traps

The served head alone must answer each of these correctly. Each phrase in "Prevents it" is in the head, word for
word.

• Q: get_pvs_stats returns pct_of_database: null for a database. Does that mean the database has 0 percent
  PVS usage?
  Correct: No. Null means the size was not measured, or the database size is missing or 0. It never means
  zero.
  Prevents it: "pct_of_database is null only if PVS size is unmeasured (pvs_measured false) or database size is missing or 0"
• Q: A row's pvs_size_mb is 0. Is that the same as pct_of_database coming back null?
  Correct: No. A measured 0 MB against a known database size is a real, healthy value. pct_of_database
  comes back 0.00, not null.
  Prevents it: "else measured 0 MB = 0.00"
• Q: aborted_version_cleaner_end_time is null while aborted_version_cleaner_start_time has a value. Is that
  missing data?
  Correct: No. That pairing means the cleaner is mid-run. It is a normal state, not a gap.
  Prevents it: "cleaner times (a start with no end = mid-run)"
• Q: Is trend_hours_back anchored to the same as_of as the snapshot, the way most windowed tools anchor to
  a supplied point in time?
  Correct: No. It looks back from the call's current time. This tool takes no as_of parameter at all.
  Prevents it: "looks back from now, not from as_of"
• Q: I ask for a longer trend_hours_back. Will a database that fell out of today's top 5 still show up for
  the period when it was bigger?
  Correct: No. The top-5 filter is evaluated by current size. A database that has since shrunk out of the top
  5 never appears in the trend, even for its earlier high points.
  Prevents it: "over the top-5 databases by current size"
• Q: The four cleaner timestamps look like they need the same UTC offset correction as other server-local
  fields in this system. Do I need to adjust them myself?
  Correct: No. They are already de-skewed, so they compare directly against as_of with no adjustment.
  Prevents it: "Cleaner times are de-skewed to UTC, like as_of."
• Q: A server returns status: "empty" for get_pvs_stats. Does that mean this target cannot collect PVS
  data at all?
  Correct: Not necessarily. empty means the engine can collect PVS, but no PVS row for this server is in the
  store, for example on a fresh registration. When the engine truly cannot collect PVS, the status is not_collected instead.
  Prevents it: "No rows: not_collected if this engine can't collect PVS, else empty"
• Q: Is pct_of_database a fraction from 0 to 1, the way some other ratio fields in this system are?
  Correct: No. The name and the healthy-case value both say percent on a 0 to 100 scale, not a fraction.
  Prevents it: "% of database" and "else measured 0 MB = 0.00"
• Q: Does trend_hours_back default to some standard window, the way hours_back defaults to 1 or 24 hours
  on other tools?
  Correct: No. The default is 0, which turns the trend off. A default call returns only the latest snapshot.
  Prevents it: "trend_hours_back (0 = off)", and the parameter's "0 (default) returns the latest snapshot only"
• Q: Is the per-database snapshot itself bounded by a rolling window, the way most Darling reads are?
  Correct: No. The main read has no window. It is the single newest capture, stamped as_of. Only the
  optional trend adds a window, and only for the top-5 databases.
  Prevents it: "LATEST IS A TIME: the newest snapshot, not a window"
• Q: Is aborted_transaction_count a delta over some period, like a per-window count elsewhere in this
  system?
  Correct: No. This tool reports a single snapshot at as_of. Every field, including the aborted transaction
  count, is the value at that moment, not a change over a period.
  Prevents it: "LATEST IS A TIME: the newest snapshot, not a window; as_of is when it was taken."
• Q: When not_collected is the answer, did the collector fail to run this cycle?
  Correct: No. not_collected means this server's engine cannot run PVS collection at all. Examples are a
  PostgreSQL target, and a SQL Server edition too old for Accelerated Database Recovery. It is a permanent fact
  about the target, not a transient miss.
  Prevents it: "not_collected if this engine can't collect PVS"
• Q: A row shows pvs_size_mb: 0, pvs_measured: true, and pct_of_database: null. Is that a
  contradiction?
  Correct: No. The database size is missing or 0, so the share has no denominator.
  pct_of_database_reason says which.
  Prevents it: "or database size is missing or 0 (pct_of_database_reason says why)"
• Q: Can you pass as_of to read the PVS state as it was during last night's incident?
  Correct: No. The tool takes no as_of parameter. It reads the newest snapshot, and as_of in the answer
  says when that snapshot was taken.
  Prevents it: "LATEST IS A TIME: the newest snapshot, not a window; as_of is when it was taken."
• Q: Both aborted_version_cleaner_start_time and aborted_version_cleaner_end_time are null. Is the cleaner
  mid-run?
  Correct: No. Mid-run needs a start time with no end time.
  Prevents it: "a start with no end = mid-run"
• Q: The answer's as_of is three days old. Is the pvs_size_mb shown the database's size right now?
  Correct: No. It is the size at as_of, three days ago.
  Prevents it: "LATEST IS A TIME: the newest snapshot, not a window; as_of is when it was taken."

Coordinator verification

• Head facts fixed: 4. The head is now 589 characters, served 620, and byte-identical on both products.
  1. "measured 0 MB = pct_of_database 0.00, never null" was false. pct_of_database is null when the database size
     is missing or 0 (DarlingMcpPvsTools.cs and McpPvsTools.cs: r.DatabaseDataSizeMb is > 0, and
     PctReason). The rule now reads "pct_of_database is null only if PVS size is unmeasured (pvs_measured false)
     or database size is missing or 0 (pct_of_database_reason says why); else measured 0 MB = 0.00".
  2. "null = pvs size unmeasured or database size missing" left out the zero denominator. The same rewrite covers
     it.
  3. "no end time = still running" dropped the original's condition, "a start time without an end time". It now
     reads "a start with no end = mid-run".
  4. Lesson 5: this is a latest-snapshot tool on both products. GetPvsStatsLatestAsync reads the rows at
     MAX(collection_time), and the head lacked "LATEST IS A TIME". It also said "at as_of", which reads like an
     as_of parameter. The head now says "LATEST IS A TIME: the newest snapshot, not a window; as_of is when it was
     taken."
• Checked and true on both products: trend_hours_back looks back from DateTime.UtcNow, not from as_of. The
  trend's top 5 is by the newest snapshot's size, DESC NULLS LAST (DarlingPvsReader.PvsTrendSql,
  LocalDataService.FinOps.Pvs.cs). The four cleaner times are de-skewed to UTC. No rows gives not_collected
  first, else empty.
• Tail correction: 1 (see Corrections). Re-pointed pin: 1 (see Re-pointed pins).
• D9 round 1 (haiku, 16 questions: the 12 above plus 4 new ones on the fixes): 0 misreads. Q10, Q11, Q12, Q14
  and Q15 were answered "can't tell". Every other answer was correct.
• Targeted tests after the fix: Darling 373 total, 0 failed, 11 skipped, after the pin re-point. Lite 211
  total, 0 failed. The classes: McpToolGuide*, McpToolsListBudget*, McpDescription*,
  McpZeroIsAMeasurement*, McpPayloadContractCensus*, MeasurementContractCensus*, McpPageContract*,
  *Pvs*, AsOfWindowAnchor*, McpLatestSnapshotStamp*, McpPayloadClockFrameDiscipline*,
  McpMissMessageParityPin*, ConsumedTimestampFrameDiscipline*.

Test plan

• dotnet build Darling/Darling.Tests/Darling.Tests.csproj -c Debug: Build succeeded, 0 Warning(s), 0
  Error(s).
• dotnet build Lite.Tests/Lite.Tests.csproj -c Debug: Build succeeded, 0 Warning(s), 0 Error(s).
• Targeted Darling classes (McpToolGuideHeadsPvsTests, McpToolGuideTests, McpToolsListBudgetTests,
  McpZeroIsAMeasurementTests, McpPayloadContractCensusTests, DarlingMcpPvsToolsTests,
  McpLatestSnapshotStampTests, AsOfWindowAnchorTests, ConsumedTimestampFrameDisciplineTests,
  McpPayloadClockFrameDisciplineTests): Total 201, Failed 0.
• Targeted Lite classes (McpToolGuideHeadsPvsTests, McpToolGuideTests, McpToolsListBudgetTests,
  McpMissMessageParityPinTests, McpDescriptionTruthPinTests): Total 87, Failed 0.
• Red-watch: broke the pvs_measured head phrase, confirmed McpToolGuideHeadsPvsTests failed, restored it,
  confirmed green again.
• Dropped-sentence check: python dropcheck.py origin/dev HEAD get_pvs_stats reports
  get_pvs_stats [Darling] old=1156 new=1742 sentences=5 missing=0 and
  get_pvs_stats [Lite] old=1156 new=1742 sentences=5 missing=0.
• git merge origin/dev: clean, no conflicts.
• Full Darling suite (Darling.Tests.exe, after the merge): Total 13393, Errors 0, Failed 0, Skipped 637, Not
  Run 1. The known non-elevated DarlingInstallLocationTests failures did not occur in this run.
• Full Lite suite (Lite.Tests.exe, after the merge): Total 5270, Errors 0, Failed 2. The two failures,
  AnalysisPassTokenThreadingTests.TheReadLockWaitIsAbandonableWhileAWriterHoldsIt and
  StatusBarSizeReadLockTests.GetUsedDataSizeMb_WhenTheWriteLockIsHeld_GivesUpInsteadOfBlocking, are lock-timing
  tests unrelated to this tool. Re-run alone, both classes passed (Total 7, Failed 0), which confirms they are
  contention flakes from the full parallel run, not caused by this change.
• Plain English checker on this PR body: see summary line below.

Status routes (for the "not_collected" and "empty" words the head states)

Status	Darling	Lite	Condition
not_collected	DarlingMcpPvsTools.cs:54	McpPvsTools.cs:55	rows.Count == 0 and this server's engine cannot run PVS collection at all (checked first)
empty	DarlingMcpPvsTools.cs:55	McpPvsTools.cs:56	rows.Count == 0 and the engine can collect PVS, but no PVS row for this server is in the store
error	DarlingMcpPvsTools.cs:127	McpPvsTools.cs:125	any exception in the try block, via McpHelpers.FormatError (same on both products)

Handoff

None. get_pvs_stats is fully converted and green on both products.

🤖 Generated with Claude Code

https://claude.ai/code/session_01QQh8LczP1HGLXQYAx4oTZC