[erikdarlingdata]

Fixes #5834

Claude posting for Erik Darling.

Closes #5834.

Why

Two findings from PR #5819's third review round, both ruled real.

1. get_investigation_reports read its window with LIMIT 100 and answered request_count: read.Requests.Count. A window holding 140 requests answered request_count: 100, and nothing in the answer said the window held more. The reports and the journal are scoped to those 100 ids, so the omission was silent in three places. The journal had the same shape under its 200-row limit, and list_investigation_requests had the milder form (count = rows.Count under the caller's limit, no flag).
2. InvestigationBriefComposer.CutCounter.Cap cut each field at a raw UTF-16 index (value.AsSpan(0, FieldCharCap - CutMarker.Length)). When the cut landed between the two halves of an emoji's surrogate pair, the kept lone high surrogate serialized as U+FFFD, so the agent read a replacement glyph in the field it was briefed on.

What changes

Commit d124514.

Finding 1, the #3699 shape (fetch limit + 1, return the page, flag the rest):

• InvestigationRequestStore.ReadReportsAsync reads limit + 1 requests and journalLimit + 1 journal rows, trims each to its page, and returns RequestsTruncated and JournalTruncated on InvestigationReportsRead (two new optional record members, default false). The reports and the journal are still read for the returned requests only. No second statement and no COUNT(*): the issue asks for the flag, not the window's true count.
• get_investigation_reports answers requests_returned (renamed from request_count), truncated and journal_truncated. Its description, after the guide marker so the 300-character head is unchanged, names the 100-request bound and what to do when truncated is true.
• list_investigation_requests reads limit + 1 and answers requests_returned (renamed from count) and truncated. Its description names both.
• Every reader moved. No reader of request_count or the list's count existed outside the tool (git grep). The web Investigations panel (wwwroot/js/pages/alerts.js) now shows a notice when the list, the 168-hour reports read, or a request's journal was cut. The desktop viewer reads its own SQL (ViewerDataService.Investigations.cs), not these answers, so it had nothing to move. The served instructions name neither field.

Finding 2: CutCounter.Cap keeps McpHelpers.TextElementCutLength(value, FieldCharCap - CutMarker.Length) units, the #3625 rule already in PerformanceMonitor.Common. An emoji that straddles the cut is dropped whole, and one that ends at the cut is kept whole. truncated.chars counts the units not kept. That is the unit FieldCharCap and TotalCharCap count (UTF-16 length), so the count matches what was dropped, including a dropped emoji's two units.

Lite parity: git grep -n 'FieldCharCap\|ReadReportsAsync\|ReportsRequestsSql' -- Lite prints nothing. Lite has no investigation tools and no brief composer, so there was nothing to mirror.

Test plan

Behavior tests only, no source-text pins.

• InvestigationToolsLiveTests.ReportsAndList_SayWhenTheWindowHeldMoreThanThePage (live, own scratch store): 101 requests in the window answer requests_returned 100 and truncated true on both tools; 100 answer 100 and false. 200 journal rows on the page's requests answer 200 and journal_truncated false, and 201 answer 200 and true. With the store's limit + 1 reverted to limit, it fails (Assert.True() Failure, Actual: False).
• InvestigationBriefComposerTests.FieldCut_NeverSplitsASurrogatePair (two rows): with the cut index inside the emoji, the note composes with no U+FFFD and the emoji dropped whole. With the emoji ending at the cut index, it is kept whole. fields_cut is 1, and chars is the units not kept. With the raw cut restored, the straddle row fails (Failed: 1 of 20).
• DarlingMcpInvestigationToolsTests.ReportsAnswer_SaysWhenTheWindowOrTheJournalHeldMore (Guard): the answer carries requests_returned and both flags, and no request_count.
• AlertHistoryRangePageTests.TheInvestigationsPanel_SaysWhenAReadWasCut_AndNothingWhenNoneWas (node harness): the three notices appear when the reads say truncated / journal_truncated, and none appears when they do not.
• Targeted classes (InvestigationBriefComposerTests, DarlingMcpInvestigationToolsTests, AlertHistoryRangePageTests, EffectiveReadWindow*, McpToolGuideHeads*, McpHelpers*, InvestigationDispatcherTests): Total: 259, Errors: 0, Failed: 0, Skipped: 0.
• Live on a throwaway PostgreSQL rig (UTC, CI's preload libraries; stopped afterward): InvestigationToolsLiveTests, InvestigationRequestStoreLiveTests, InvestigationContractLiveTests: Total: 24, Errors: 0, Failed: 0, Skipped: 0. InvestigationToolsLiveTests, ViewerInvestigationsLiveTests, PgInvestigationBriefSourceReaderLiveTests: Total: 11, Failed: 0, Skipped: 0.
• Gate (.github/scripts/pre-push.sh): GATE GREEN. Guard: Darling Total: 4157, Failed: 0, Lite Total: 1134, Failed: 0. Selected classes: Darling Total: 2275, Failed: 0, Skipped: 84 (live classes; the gate ran without a rig), Lite Total: 701, Failed: 0.
• Full suite: not run (laptop rule). CI decides.

For the coordinator

• request_count (reports) and count (list) are renamed to requests_returned, as the issue asks. An external agent that read either key sees a missing key. AGENTS.example.md reads only status.
• The web panel's notices are new user-visible text. Please check their wording.

Not done

Nothing from the brief.

SECTION: Fixed
ENTRY: - get_investigation_reports says when its window was cut, and the brief's field cut keeps emoji whole ([#PR]) - get_investigation_reports reported the 100-request page as the window's request count, and nothing said the window held more; list_investigation_requests and the journal had the same gap. Both tools now answer requests_returned with a truncated flag (journal_truncated for the journal), and the web Investigations panel says when a read was cut. A brief field cut could also split an emoji into a replacement glyph, and now keeps or drops it whole.

[claude]

Non-blocking (reuse): these three lines are McpHelpers.BoundPage re-spelled — McpHelpers.cs:660-665 is

var truncated = fetched.Count > limit;
IReadOnlyList<T> page = truncated ? fetched.Take(limit).ToList() : fetched;

identical, down to the .Take(limit).ToList() on the truncated branch only. The body calls this "the #3699 shape", and #3699 bound its sites through that helper (McpPageContractTests.PgPagedToolsThroughTheHelper, and McpPayloadContractCensusTests' own failure message says "fetch cap + 1, bind through McpHelpers.BoundPage and publish truncated"). Behaviour is the same today, so this is a cleanup, not a defect:

Suggested change

	/* #5834, the #3699 shape: read one row past the limit, return the page, and say whether more matched. */
	var fetched = await Store(postgres).ListAsync(state, serverId, family, limit + 1, cancellationToken);
	var truncated = fetched.Count > limit;
	IReadOnlyList<InvestigationRequestRow> rows = truncated ? fetched.Take(limit).ToList() : fetched;
	/* #5834, the #3699 shape: read one row past the limit, return the page, and say whether more matched. */
	var (rows, truncated) = McpHelpers.BoundPage(
	await Store(postgres).ListAsync(state, serverId, family, limit + 1, cancellationToken), limit);

[claude]

Non-blocking (wording an agent acts on): the remedy list reads "narrow hours_back or pass server_name or request_id", and one of the three cannot reach what was cut.

ReportsRequestsSql is … AND r.opened_at >= $4 ORDER BY r.opened_at DESC, r.request_id LIMIT $5 (InvestigationRequestStore.cs:411-413), so the page keeps the newest 100 and the rows it dropped are the oldest in the window. Narrowing hours_back removes more of the old end: it clears truncated without ever showing the requests the flag was about. server_name genuinely partitions the set, and request_id addresses a row directly — those two work. And since the tool declares no limit parameter, the "raise limit" escape the list tool's sentence offers (:91-92) does not exist here.

The sentence is defensible read as "here is how to get an answer that is complete for what you asked", which all three do; it is misleading read as "here is how to see the rest". Since there is no cursor on this read, saying so outright is probably the honest version — something like "the dropped requests are the oldest in the window and no argument pages to them; pass server_name to split the window, or request_id for one request".

[claude]

Non-blocking — the wording check you asked for in the PR body. All three notices are accurate about their own read:

• the list notice (:806) matches ListSql's ORDER BY s.opened_at DESC + LIMIT, so "the newest N; older ones are not listed" is exactly what was served;
• the journal notice (:772) matches ReportsJournalSql's ORDER BY event_time DESC, journal_id DESC;
• this one correctly attributes a missing verdict to the reports page being cut.

One imprecision worth a word, pre-existing and only surfaced by this notice: the two reads do not cover the same set. The list read (:793) passes limit: 100 and no window — it is the newest 100 requests of any age — while the reports read (:794) is windowed at INVESTIGATION_HOURS (168). So a listed request older than seven days shows no verdict whether or not truncated fired, and nothing on the panel says why. With this notice present, a reader who sees a verdict-less row and no notice will conclude the request has no report; with the notice present, they will attribute an older row's blank verdict to the cut. Either way the 168-hour edge is the real cause and is undisclosed.

Cheapest honest fix is to say the window in the un-cut case too (the panel already knows INVESTIGATION_HOURS), e.g. a standing line that verdicts are read over the last 168 hours — or to window the list read to the same span so the two sets agree. Your call; not blocking this PR.

[claude]

Note: journal_truncated is a second-bound cut key the vocabulary census cannot see

Not a defect in this PR, and not a request to add a pin — recording it because the census reads as complete and is not, which is the same shape as the roster notes in McpPayloadContractCensusTests itself.

journal_truncated (DarlingMcpInvestigationTools.cs:685) is a textbook <bound>_truncated second bound: a page cut beside the payload's own truncated, exactly what SecondBoundCutKeys exists to classify (it rosters databases_truncated, points_truncated, plans_truncated, scan_truncated, processes_truncated, window_truncated). It is not in that roster, and EveryCutKey_IsThePageDialect_OrClassified_OnBothSkus stays green anyway — so the green is not evidence the key was classified.

The reason is the matcher. PayloadKey (McpPayloadContractCensusTests.cs:1643-1645) accepts two shapes:

• an initializer key — ^\s+name = or a bare name,
• an envelope write — ^\s+identifier["name"] =

ReportsAnswer builds its payload as a JsonObject collection initializer, so the lines are ["journal_truncated"] = read.JournalTruncated, with no identifier before the [. Neither alternative matches, and every key in that object — requests_returned, truncated, journal_truncated, hours_back, brief — is invisible to the census. (Compare the trend disclosure's envelope["window_truncated"] = …, which the second alternative was written for and which does match.) The spelling here is the right one regardless; it just was not checked by anything.

Two consequences, both for whoever owns that lane rather than for this PR:

1. The roster's claim ("a second bound joins SecondBoundCutKeys as <bound>_truncated") is now untrue for one key, and the census will not notice the next one either as long as the tool builds a bare JsonObject.
2. Under the Tests that assert spelling, not behavior: cap them, stop the review asking for pins, delete the shape ones #5833 ruling the answer is probably not a wider matcher or a roster row — the behaviour pin for this key already exists in DarlingMcpInvestigationToolsTests.ReportsAnswer_SaysWhenTheWindowOrTheJournalHeldMore and InvestigationToolsLiveTests.ReportsAndList_SayWhenTheWindowHeldMoreThanThePage, which is the instrument that ruling prefers. Worth one line in that file's doc comment saying the matcher does not read bare collection-initializer keys, so the next reader does not take its silence for coverage.

[claude]

Verdict: LGTM

Claims

• VERIFIED — the store reads limit + 1 requests and journalLimit + 1 journal rows, trims each, returns two new optional record members defaulting to false — InvestigationRequestStore.cs:848, :868-872, :893, :905-911, record :154-159.
• VERIFIED — reports and journal still scoped to the returned requests, no second statement, no COUNT(*) — ids taken after the trim (:874), both statements filter request_id = ANY($1), all three unchanged.
• VERIFIED — get_investigation_reports answers requests_returned (was request_count), truncated, journal_truncated, with the new prose after the guide marker so the 300-char head is unchanged — DarlingMcpInvestigationTools.cs:683-685, marker :540, text :547-549; the pins in McpToolGuideHeadsInvestigationsTests are untouched.
• VERIFIED — list_investigation_requests reads limit + 1, answers requests_returned (was count) and truncated, both named in the description — :119-128, :91-92.
• VERIFIED — no reader of the old keys existed outside the tool — grepped C#, wwwroot/js, the .mjs harness, README.md, AGENTS.example.md; other request_count hits are the unrelated CpuSchedulerStatsCollector columns.
• VERIFIED — the panel says when the list, the 168-hour reports read or a journal was cut — alerts.js:806, :807-809, :772, pinned by TheInvestigationsPanel_SaysWhenAReadWasCut_AndNothingWhenNoneWas.
• VERIFIED — the viewer reads its own SQL and the instructions name neither field — ViewerDataService.Investigations.cs has its own LIMIT 500/200 and calls neither tool.
• VERIFIED — Cap keeps TextElementCutLength units, drops a straddling element whole, keeps one ending at the cut, counts in the caps' UTF-16 unit — InvestigationBriefComposer.cs:423-425, unit :413; both boundaries run by FieldCut_NeverSplitsASurrogatePair, and the older exact-FieldCharCap pin still holds (its text is a run of x).
• VERIFIED — Lite has nothing to mirror — nothing under Lite/ names these tools, ReadReportsAsync or FieldCharCap; McpToolGuideHeads.Investigations.cs:18 says so too.
• VERIFIED — behaviour tests only — the four new tests execute the composer, the answer builder, the live store and the page module; no census or roster added.
• UNVERIFIED — the gate and targeted figures (259, Darling 4157, Lite 1134, GATE GREEN) — needs a build; no .NET SDK here.
• UNVERIFIED — the live-rig figures (24, 11) — needs PostgreSQL at rung V184.
• UNVERIFIED — the two mutation checks — need a run on a reverted tree; both plausible from source.

Riskiest lines

1. InvestigationRequestStore.cs:848 — P(limit + 1) is on the statement the single-request arm shares, and the trim at :871 runs on the list the brief was read beside (:861-864); both arms hold only because requestId is not null bounds the result at one row — pinned by ReportsAndList_SayWhenTheWindowHeldMoreThanThePage and by InvestigationRequestStoreLiveTests line 352 for the request-id arm.
2. InvestigationBriefComposer.cs:423-425 — kept length and omission count are two quantities from one call; pair them wrong and the marker overruns FieldCharCap or chars lies — pinned by FieldCut_NeverSplitsASurrogatePair.
3. DarlingMcpInvestigationTools.cs:683-685 — the wire rename; an agent that read request_count sees a missing key — pinned by ReportsAnswer_SaysWhenTheWindowOrTheJournalHeldMore, which asserts the old key is gone.

Traps near this diff

• "Truncation inferred from a full page … Fetch limit + 1; the flag is fetched.Count > limit." — applies, and is what the diff implements (:120-121, store :868, :905).
• "A total_* assigned from a .Count is a count of the page … Otherwise it is *_returned." — applies: both page counts moved to requests_returned beside truncated.
• "A count taken off the page and named as the population's." — applies, honoured the stricter way: no population count is published at all, so none sits above the LIMIT to get wrong.
• "A text cut at a UTF-16 index can land between the halves of a surrogate pair … Cut on StringInfo text elements and count omitted characters in the same unit." — applies, and is the second half of the fix; the count is UTF-16 units, the unit the caps measure and the section budget spends (:204).
• "hours_back, limit, top, days_back are refused when out of range, never clamped." — does not apply: limit refused at :105-106, hours_back at :578; the + 1 is on the fetch, after validation.
• "ORDER BY 3 + 4 DESC is not "ordinal 3 plus ordinal 4" … Sort by named columns." — does not apply: named columns with a tiebreak at store :355, :412, :425 — which matters more now, since without them limit + 1 could return a different 100.
• "Lite and Darling ship in parity … without the counterpart's is a finding, not a nit." — does not apply: the counterpart does not exist (McpToolGuideHeads.Investigations.cs:18).
• "Two copies of one definition pinned EQUAL to each other fail only after BOTH have drifted. Alias, do not copy." — not the trap, but the smell: :121-122 re-spells McpHelpers.BoundPage (McpHelpers.cs:660-665) line for line. Nothing pins them equal; reuse cleanup, commented inline, non-blocking.
• "Ruling (#5833) … A trap is answered with a behaviour test that fails on the defect." — honoured: both tools sit outside McpPageContractTests.PagedTools (no Lite twin) and carry behaviour tests instead. Where the censuses read thinner than they look is journal_truncated — separate note, not a request for a pin.

Divergence (blind read vs PR body)

Three, none a behaviour difference; all posted as non-blocking comments.

1. "what to do when truncated is true" overstates one of its three options. :547-549 offers "narrow hours_back or pass server_name or request_id". The page keeps the newest 100, so the dropped rows are the oldest: narrowing hours_back removes more of that end and clears the flag without showing what it was about. The other two reach those rows; hours_back cannot, and there is no limit to raise.
2. "the #3699 shape" without #3699's helper — the list arm hand-rolls BoundPage; same behaviour, but the census's failure text names the helper as the binding.
3. The reports notice is not the only cause of a blank verdict — the list read is unwindowed (alerts.js:793), the reports read is 168 hours (:794), so a request older than seven days shows no verdict either way and nothing says so. Pre-existing, made noticeable by the new notice.

Noted, not divergence: reports is the one array here with no bound of its own (ReportsSql, no LIMIT). Honest — an uncut array needs no flag — but the new enumeration of bounds is the whole disclosure while size is driven by up to 300 reports.

Reviewer checklist

• Build and run the suite: the test-plan figures and both mutation checks could not be executed here.
• Run the new live test against a real V184 store — the insert reads sound by eye, but only a live run proves the 100/101 and 200/201 boundaries.
• Page a busy window (hours_back: 168, 100+ requests with reports) and measure the answer's size.
• View each notice live and judge the wording in place, including the 168-hour edge.
• Decide whether the viewer's Investigations window owes the same disclosure (LIMIT 500/200, no flag).