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

[erikdarlingdata]

Refs #3898.

Why

Issue #3898: get_plan_corrections put its full 1,672-character description into every tools/list reply on
both products. That text carries reading guidance a model only needs when it actually calls the tool. It covers
what bounds the page, when empty beats not_collected, and why timestamps line up against
get_query_store_regressions. Nothing is deleted. The guidance moves into a per-tool reading guide that
get_tool_guide(tools[], topics[]) serves on demand, and a short guardrail head stays in tools/list.

What changes

• get_plan_corrections converts on both products (D6 twin, byte-identical head): Darling/PerformanceMonitor.Darling.Service/Mcp/DarlingMcpPlanCorrectionTools.cs
  and Lite/Mcp/McpPlanCorrectionTools.cs.
• New head-pin test class McpToolGuideHeadsSqlTailPlanCorrectionsTests, appended to
  Darling/Darling.Tests/McpToolGuideHeads.SqlTail.cs and Lite.Tests/McpToolGuideHeads.SqlTail.cs.
• Budget ceiling updated in Darling/Darling.Tests/McpToolsListBudget/DarlingMcpPlanCorrectionTools.txt and
  Lite.Tests/McpToolsListBudget/McpPlanCorrectionTools.txt. Only the tool line changed. No parameter was over
  200 chars, so none needed a cut.
• No topic added: this lane converts one tool, and no sentence repeats across two or more tools in this drop.

Served characters

Tool	Darling before	Darling after	Lite before	Lite after
get_plan_corrections	1,672	611	1,672	611

Corrections

None. The original description matched the tool body on both products (verified against
DarlingMcpPlanCorrectionTools.cs and McpPlanCorrectionTools.cs, and their shared status routes below).

D4 removals

None. The original text carried no issue-number references and no anecdote.

Re-pointed pins

None needed. git grep -l get_plan_corrections -- Darling/Darling.Tests Lite.Tests found five files beyond this
PR's own new tests: DarlingMcpPlanCorrectionToolsTests.cs, McpLatestSnapshotStampTests.cs,
McpPageContractTests.cs, PlanCorrectionFrameLiveTests.cs and RepoFileAdoptionTests.cs. None of them pins
description prose that moved.

McpPageContractTests reads the raw DescriptionAttribute.Description, which still holds the head, the
<<GUIDE>> marker and the full tail together. It only asserts the text contains "truncated" and "limit".
Both are still true. McpLatestSnapshotStampTests and RepoFileAdoptionTests reference the tool name in
comments and rosters, not its text. PlanCorrectionFrameLiveTests never reads Description.

Status routes (for the D9 traps below)

• not_collected: DarlingMcpPlanCorrectionTools.cs:61-63 and McpPlanCorrectionTools.cs:56-58. The
  condition is tuning.Count == 0 && rows.Count == 0, AND the engine-capability probe finds this server's engine
  can never produce plan_correction rows, for example a PostgreSQL target. The probe is
  DarlingEngineCapability.NotCollectedStatusAsync (DarlingEngineCapability.cs:62-82) on Darling. Lite's twin
  is McpEngineCapability.NotCollectedStatusAsync (McpEngineCapability.cs:43).
• empty: same file and line, same AND condition, but the engine-capability probe returns null. This engine
  CAN collect the data. There is just nothing collected or open right now.
• A row in recommendations[] but an empty automatic_tuning[], or the reverse, is a normal 200 payload with one
  side as an empty array. Neither status word fires unless BOTH are empty.
• error: DarlingMcpPlanCorrectionTools.cs:123 / McpPlanCorrectionTools.cs:124, the catch block's
  McpHelpers.FormatError, an unhandled exception during the read.

D9 traps

• Q: get_plan_corrections returns recommendations_returned: 40. Does that mean 40 distinct
  FORCE_LAST_GOOD_PLAN recommendations fired in the window?
  Correct: No. The collector re-captures every still-open recommendation on every cycle, so one
  recommendation that stays open counts once per capture.
  Prevents it: "Rows recur per capture, not per distinct recommendation."
• Q: Two rows in recommendations[] share the same query_id and regressed_plan_id. Is that two separate
  findings, or one?
  Correct: Almost certainly one. They are most likely the same recommendation, captured on two different
  cycles while it stayed open. Rows are not unique findings.
  Prevents it: "Rows recur per capture, not per distinct recommendation."
• Q: hours_back=168 (a week) was requested, truncated came back true, and oldest_returned_collection_time
  is only 6 hours before newest_returned_collection_time. Is the reader broken?
  Correct: No. The page is bounded by limit, not by hours_back. A server with several open recommendations
  can fill the page in hours. Raise limit or narrow hours_back to reach further back.
  Prevents it: THE PAGE IS BOUNDED BY limit, NOT hours_back: truncated means more rows existed; oldest/newest_returned_collection_time bound the page.
• Q: hours_back=1 was requested, but automatic_tuning[] lists a database whose snapshot was captured three
  days ago. Is that a bug?
  Correct: No. automatic_tuning ignores hours_back and as_of completely. It is always the latest
  captured snapshot, however old.
  Prevents it: automatic_tuning ignores the window: the latest snapshot; each row's as_of says when.
• Q: The tool call omitted the as_of parameter. Does automatic_tuning[]'s own as_of field then come
  back null?
  Correct: No. The response field as_of (per row, when that snapshot was captured) is a different thing
  from the request parameter as_of (the window anchor). The field is populated whenever a tuning row exists,
  whether or not the caller passed the parameter.
  Prevents it: automatic_tuning ignores the window: the latest snapshot; each row's as_of says when.
• Q: recommendations[] came back empty but automatic_tuning[] has three rows. Does the payload carry
  status: "empty"?
  Correct: No. empty only fires when both are empty. Here the call returns an ordinary payload with
  recommendations: [] and the populated automatic_tuning array, no status wrapper at all.
  Prevents it: "No rows and no automatic_tuning: empty (not_collected checked first)."
• Q: Both recommendations[] and automatic_tuning[] came back empty. Is the status always "empty"?
  Correct: No. not_collected is checked first. On an engine that can never produce this data, for example a
  PostgreSQL target, the answer is not_collected, not empty.
  Prevents it: "No rows and no automatic_tuning: empty (not_collected checked first)."
• Q: A call returned status: "error". Does that mean this server's engine cannot collect plan corrections?
  Correct: No. error only comes from an unhandled exception during the read. An engine that cannot collect
  this data answers not_collected earlier in the function, on a path that never reaches the code that raises
  that exception.
  Prevents it: "No rows and no automatic_tuning: empty (not_collected checked first)" (states the
  not_collected-first order the error path sits outside of).
• Q: Is valid_since, last_refresh, execute_action_initiated_time or revert_action_initiated_time in the
  monitored server's local time zone, the way some other DMV-sourced columns are?
  Correct: No. All four, like every timestamp on this payload, are UTC, stored and returned unconverted. They
  compare directly against collection_time, with no offset math.
  Prevents it: "All timestamps are UTC."
• Q: A caller wants the oldest plan-correction activity first for a timeline view. Can recommendations[] be
  read in that order directly?
  Correct: No. The array is served newest capture first (order: "collection_time_desc"). A chronological
  timeline has to reverse it, or re-sort by collection_time ascending.
  Prevents it: "newest capture first" (head's opening clause).

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).
• dropcheck, Darling: get_plan_corrections old=1672 new=2250 sentences=7 missing=0.
• dropcheck, Lite: get_plan_corrections old=1672 new=2250 sentences=7 missing=0.
• Red-watch: broke "All timestamps are UTC." to "All timestamps are LOCAL." in the head, confirmed
  McpToolGuideHeadsSqlTailPlanCorrectionsTests.EveryConvertedHead_CarriesItsGuardrailFact_AndThePointer failed
  (Total: 2, Failed: 1), restored, confirmed green again.
• Targeted, Darling: McpToolGuideHeadsSqlTail*, McpToolsListBudget*, McpPageContract*,
  McpLatestSnapshotStamp*, DarlingMcpPlanCorrectionToolsTests, McpToolGuideTests,
  McpDescriptionTruthPinTests, McpZeroIsAMeasurementTests, McpPayloadContractCensusTests,
  MeasurementContractCensusTests - Total: 217, Failed: 0 (across the two runs).
• Targeted, Lite: McpToolGuideHeadsSqlTail*, McpToolsListBudget*, McpPageContract*, McpToolGuideTests,
  McpDescriptionTruthPinTests, McpZeroIsAMeasurementTests, McpPayloadContractCensusTests,
  MeasurementContractCensusTests - Total: 64, Failed: 0 (across the two runs).
• Full suite, Darling.Tests: Total 13,309, Errors 0, Failed 0, Skipped 636 (live-PostgreSQL classes, no rig).
• Full suite, Lite.Tests: Total 5,263, Errors 0, Failed 0, Skipped 0.
• Plain-English checker on this PR body: body_4105.md: 21 hit(s), doc mode (20 D9 labels and 1 emoji). The remaining hits are the D9 labels and the attribution emoji, which are allowed.

Coordinator verification

• I merged origin/dev (c51706c9) into the branch. The dropped-sentence check shows missing=0 on both products. No parameter description changed.
• I checked every head fact against the code on both products. The window is collection_time >= start AND collection_time <= as_of. Rows sort by collection_time DESC, score DESC. truncated comes from a limit + 1 fetch. automatic_tuning reads the server's newest capture (MAX(collection_time)) with no time filter. empty comes back only when both lists are empty, and only after NotCollectedStatusAsync returns null. Every head fact is correct. Head facts fixed: 0.
• D9 round 1: a fresh haiku reader answered the 10 questions above from the served head and the parameter text only. Misreads: 0. Seven answers were correct. Q6, Q7 and Q8 came back as "can't tell" because the head does not state them, and that is not a misread.
• Q5 got the right answer for the wrong reason. The reader took the head's as_of to mean the request parameter. So I changed a latest snapshot, as_of says when to the latest snapshot; each row's as_of says when on both products. I updated the two head pins and the two budget lines. The served head went from 598 to 611 characters.
• The changelog buffer entry now says "fewer than 620 characters" instead of "598 characters".
• After the edit, both builds have 0 warnings. Targeted classes: Darling 310 tests and Lite 125 tests, 0 failed. Full suites: Darling ran 13,310 tests with 2 failed and 636 skipped. Lite ran 5,263 tests with 0 failed. The 2 Darling failures are the known DarlingInstallLocationTests pair from Install scripts: the service account gets Read & Execute on the install root and Modify only where it writes (#4052) #4090. They fail on any non-elevated local run, and CI runs elevated.

Handoff

None. One tool, fully converted, both products in lockstep, no unconverted work left for this lane.

🤖 Generated with Claude Code

https://claude.ai/code/session_01QQh8LczP1HGLXQYAx4oTZC