Practice requests with a score per agent (#5918)

[erikdarlingdata]

Claude posting for Erik Darling.

Fixes #5918. Part of #5632 (Phase B, wave 3).

Why

Before an operator trusts an agent with Level 2, they can watch it work cases they already judged. A practice request replays a case that has feedback, without the answers, and scores the agent's report against that feedback. The score is advisory.

What changes

1. Start from the Viewer. New Practice tab in the Investigations window (own UserControl, InvestigationPracticePanel): lists judged cases (reported requests with feedback), Start Practice Request, and the scores. InvestigationPracticeStore.OpenAsync copies the case's route, server, family, first metric, story path, alert times, alert count and alert-row values into a new request carrying practice_of, stamped level 1, journaled opened by person:<login>. It refuses (writing nothing) an unknown case, a practice request, a case with no report, a case with no feedback, and a case that already has a live practice request.
2. Practice brief, no answers. InvestigationBriefSources gets Practice (default false). When true the composer sets request.practice, adds one read rule, and drops earlier reports and prior links. The Darling reader also skips notes whose source_request_id is the replayed case and skips the two reads. A brief that is not practice is byte for byte unchanged (pinned by a test comparing it with and without the new argument); Lite's reader never sets it.
3. Scoring (InvestigationPracticeScoring, pure): newest corrected verdict is the answer (full credit); else right makes the original report's verdict the answer (full), partly the same at half credit; wrong with no correction has no answer (shown as "No answer", not scored). Newest run per agent per case counts. Cases whose feedback names a missed step are counted for a person to read against; Darling does not judge the step text.
4. Scores show in the Viewer's Practice tab and on the web Admin page (new Practice Scores tab, own buildPractice, GET /api/investigation-practice-scores, list-only).

Shared-SQL fixes the feature needs (a practice request must not behave like live traffic): FindJoinSql skips practice requests (a live alert opens its own request), and CountRouteSql and RouteUseSql skip them (they do not eat a route's hourly or daily limit). Review round 1 adds the other real-traffic reads: the Report Feedback list (ReportedSql) and the per-family counts (FamilyCountsSql) skip practice requests too.

Provisioning: the Viewer is the admin role, which could not INSERT requests. Added GRANT INSERT (<the open columns>) ON investigation_requests and GRANT INSERT ON investigation_request_alerts to admin, in managed section 10e and provision-roles step 3j (they name practice_of, a V191 column, so they sit in the V191-guarded block, not in 10d/3i). No migration.

5. The agent cannot read the replayed case while its practice is live. Two reads return reports or feedback to an investigations-arm agent: get_investigation_reports and the brief's earlier_reports and prior_links (see round 1 below). Under the investigations arm (mcp.allowInvestigations, writes off) it now leaves out the report, the feedback and the journal rows of every case a live practice request replays (a practice request that reads open or claimed, computed as the list computes it: not submitted, not expired, a lapsed lease reading open again). The request row stays, and so does every other report and its feedback. Once the practice report is submitted, or the practice expires, the case shows again. The writes arm (operators) and a host serving neither arm read everything, and so does the web Admin page. One request by id and the window read both apply it. Family counts in a window read still count the case among many; I left them. The seam (neither Level 2 spine: the agent_action gate and its journal (#5912) #5925's DarlingMcpWritePolicy nor Per-server threshold overrides: loader, PostgreSQL counts, set_alert_threshold_override (#5916) #5933 gives a read tool its arm, since the policy only filters tool names at start): InvestigationReadPolicy(HideLivePracticeAnswers), a service parameter of get_investigation_reports, registered by ConfigureMcpServices from writes.Investigations. It adds one entry to the pinned service-parameter list in McpServiceParameterDiSeatCensusTests (updated deliberately, with the reason in its comment). InvestigationRequestStore.LivePracticeReplaysAsync reads the replayed ids.
6. Practice marker. The shared request row (InvestigationRequestRow) gains an optional trailing PracticeOf. list_investigation_requests and the report read's requests carry practice_of (the replayed case's id) on a practice row; every other row has no such key (absent, not null), so Lite's rows and its goldens are unchanged (LiteInvestigationGoldenTests pass with no edit). The Viewer's Requests tab reads practice_of only when the store is at V191 or later (through SkewAsync) and shows a Practice column then; an older store lists as before with no practice column, so the tab keeps its V184 floor.
7. Admin grant pinned live. InvestigationPracticeGrantLiveTests provisions the roles on a throwaway store both ways (managed batch, the shipped provision-roles.sql), connects as the real admin login, opens a practice request, and checks the row and its alert links; the viewer login is refused with 42501 and writes nothing.
8. UI tests. InvestigationPracticePanelStaTests and InvestigationPracticePanelLiveStaTests (STA, through the Investigations window that seats the panel) and AdminPracticeScoresBehaviourTests (Node, through the Admin page's pure scenario).

The score stays advisory.

Test plan

• InvestigationPracticeTests (pure, 11 tests): practice brief holds none of the earlier report or link; non-practice brief byte-identical; answer follows right/partly/wrong/corrected; newest corrected wins; per-agent score; nothing scored gives no percent.
• InvestigationPracticeLiveTests (5, live PG on a throwaway rig): brief for a case with every feedback field filled in (verdict, corrected verdict, missed step, comment, author) plus a proposed-and-approved note contains none of it, while a live request of the same story does (control); never-judged cases are refused and write nothing; the gate refuses a claimed practice request with practice even stamped level 2; a live alert never joins it and route use excludes it; a practice report is scored.
• Targeted: brief composer, Pg brief reader live, DarlingManagedRoles, InvestigationRequestStore (live), AgentActionGate, AgentLevel2 (live, incl. the BYO script), InvestigationFeedback, AdminReportFeedback, Viewer investigations, ViewerGridTimeColumn, InvestigationTools/Contract live, all *Census*, *Ratchet*, DocCommentHygiene, StorageCommandTimeout, McpPayloadContract, ViewerSchemaVersionGate, WebRouteStatementCensus, ReadAdmissionCoverage: 900 run, 3 failures, all three fixed (below) and re-run green (53 of the affected classes). Lite.Tests LiteInvestigationGoldenTests + DuckDbInvestigationBriefSourceReader: 10 of 10, no golden edited.
• Fixed along the way: WebRouteStatementCensusTests wanted the route in DarlingWebEndpoints.cs and a swept error answer (moved there); AgentLevel2StoreGrantLiveTests BYO script ran my grant on a store below V191 (moved into 3j); TheByoScriptsV184Blocks... counts blocks naming investigation_requests (now excludes the V191 block, with a comment); DarlingManagedRolesTests asserted nobody INSERTs the alert-link table (now: admin only).
• Item 1 (hide the answers), InvestigationPracticeLiveTests.WhileAPracticeIsLive_...: one claimed-then-submitted practice; under the investigations arm the replayed case's report, feedback (both ZEBRA- markers) and journal are absent while the other case's report and feedback show, by request id and in the window; the writes arm shows everything; after the practice report is submitted the case shows again. Pure: TheReportReadHidesTheReplayedCase_OnlyUnderTheInvestigationsArm (4 cases: the DI seat follows the arm) and WithoutReplayedAnswers_....
• Item 2, InvestigationPracticeGrantLiveTests (2, live): admin opens, viewer refused (42501), through both grant sources.
• Item 3, InvestigationPracticePanelStaTests (3), InvestigationPracticePanelLiveStaTests (1, lists only the judged case, Start opens its practice, the Requests read marks it, the scores grid shows the agent), AdminPracticeScoresBehaviourTests (2, Node ran).
• Item 4, TheRequestRow_NamesWhatAPracticeReplays_... plus the live facts above; Lite: Lite.Tests investigation classes 134 of 134, no golden edited.
• This round, on a throwaway rig: InvestigationPractice*, ToolsLive, Contract, WritePolicy, ViewerInvestigations, AdminPage, EffectiveReadWindow, AgentLevel2, DarlingManagedRoles, InvestigationFeedback and the others in one run: 294 run, 0 failed after the DI seat pin was updated (it failed once, as it should); *Census*, *Ratchet*, McpToolsListBudget, McpPayloadContract, StorageCommandTimeout, ViewerSchemaVersion: 647 run, 0 failed.
• Not run: the Viewer window and the web tab by hand or in a browser (no playwright, no screenshot); the full suite (CI decides).

Review round 1

Review findings (CHANGES REQUESTED: 3 blockers, 3 notes, 1 refuted CHANGELOG claim) and what changed. Commit Practice requests: review round 1 (#5918).

Blocker 1, the answer key reached other briefs through earlier_reports. Fixed. InvestigationRequestStore.LivePracticeRequestsSql(now) (InvestigationRequestStore.cs, the block above LivePracticeReplaysSql) is now the one definition of a live practice request (the computed state, as ListSql computes it). LivePracticeReplaysSql (the hider), PgInvestigationBriefSourceReader.EarlierReportsSql and PriorLinksSql, and InvestigationPracticeStore.LiveSql all alias it, none copies it. EarlierReportsSql and PriorLinksSql now exclude every report and link of a practice request, and the report and links of any case a live practice request replays (rq.practice_of IS NULL AND NOT EXISTS (live practice of this case)); now is a new trailing parameter ($5, $6). Feedback is never in a brief, so the case's feedback needed no extra work there. Prior links had the same leak, so they got the same filter. Test: InvestigationPracticeLiveTests.AnotherBrief_OnTheSameServerAndFamily_ReadsNoPracticeReport_AndNoReportOfACaseALivePracticeReplays. It fails on the old SQL (checked by removing the two filters, 2 of the new facts failed), and on the new shape it asserts, in order: (a) while the practice is open, a live brief on the same server and family lists only the bystander's report and link, not the case's; (b) after the practice report is submitted, neither the practice report nor its link is in the live brief; (c) a practice that is open but dead (claim count 3) hides nothing. One deviation from the ruling's literal test: once the practice report is submitted the practice is no longer live (same definition as the hider), so the replayed case's report is visible again; the test asserts that, not its absence. Approved notes that the case proposed still reach other briefs (a person approved them for exactly that); I left that alone and said so in the README.

Blocker 2, practice requests counted as real traffic. Fixed. InvestigationFeedbackStore.ReportedSql and FamilyCountsSql add r.practice_of IS NULL (InvestigationFeedbackStore.cs). FindJoinSql, CountRouteSql and RouteUseSql already had it. The remaining reads of investigation_requests are the Requests lists (ListSql, the Viewer's select, ReportsRequestsSql) which show practice rows on purpose with their practice_of marker, the long-poll catch-up (OpenedSinceSql, agents claim practice requests), and the Practice tab's own RunsSql. The Practice tab's judged-case list reads the feedback list, so it now lists real cases only. Test: APracticeRequest_IsNotRealTraffic_ForTheFeedbackList_AndTheFamilyCounts_AndItCopiesTheAlertCount (a practice that reports and gets feedback does not appear in ListReportedAsync and does not move the family counts; it does appear in the Practice scores). It fails on the old SQL.

Blocker 3, the Requests tab's store floor. Fixed. ViewerDataService.Investigations.cs: InvestigationRequestsSelectSql is now a method taking withPractice; GetInvestigationRequestsAsync reads the store version (_cachedStoreSchemaVersion, like the collection caveats read) and asks for practice_of only at V191 or later, through SkewAsync; an older store, or one whose probe could not answer, lists as before with PracticeOf null. New PracticeStoreSchemaVersion = 191. Test: ViewerInvestigationsLiveTests.TheRequestsTab_KeepsItsV184Floor_ListingAStoreBeforeV191AsBefore_WithNoPracticeColumn (drops the feedback table and the column, then lists).

Notes.

• Stored versus computed state: right. InvestigationPracticeStore.LiveSql now aliases the computed definition, so a practice claimed three times no longer answers AlreadyLive. Test: ADeadPractice_DoesNotBlockAFreshReplayOfTheCase.
• alert_count: copied. OpenSql and SourceSql carry it, and the admin INSERT column list gains alert_count in DarlingManagedRoles.cs (10e) and tools/provision-roles.sql (3j). The grant live tests (both sources) pass. The class summary says so.
• FOR SHARE OF r comment: corrected on SourceSql (it locks the case's request row so the case cannot be deleted under the copy; it does not lock the feedback table, and the copy reads no feedback).

CHANGELOG claims. Both refuted claims are corrected: the README (items 2 and 3) and the entry below no longer say a practice request "changes nothing"; the entry says what ships. ABriefThatIsNotPractice_IsUnchanged_ByteForByte_... pins the new argument's default; the byte-for-byte claim rests on the three sources.Practice branches in InvestigationBriefComposer, as the reviewer found. The body no longer says the investigations arm has one read that returns reports.

CI. The PR's own run was green except the Claude review guard (this review) and the Darling web UI smoke. Smoke: four baselines differed (fleet, ui-sql-overview, ui-sql-queries, admin-servers). The first three also failed on the unrelated fix/5916-threshold-overrides PR with the same pixel counts, which is dev baseline drift; #5930 (#5929) refreshed those baselines on dev and this branch now includes it (merged origin/dev, no code conflicts). admin-servers still shifts here by the new Practice Scores tab button.

UI smoke

Per the coordinator's ruling the tools/ui-smoke baselines are not re-rendered here. The admin-servers baseline shifts by the Practice Scores tab button on the Admin page; refresh it when this merges.

Round 1 tests

• Throwaway PostgreSQL rig, DARLING_TEST_PG: practice classes (InvestigationPracticeTests, ...LiveTests, ...GrantLiveTests, panel, admin) plus ViewerInvestigationsLiveTests, InvestigationFeedback*, InvestigationRequestStore*, PgInvestigationBriefSourceReader*, DarlingManagedRolesTests, ViewerSchemaVersionGateTests: 129 run, 0 failed. Investigation brief, contract, dispatch, note-decision, tools, masker and Investigations classes: 176 run, 0 failed. *Census*, *Ratchet*, *Pin*, DocCommentHygiene, AlertReadFailureSurface: 1151 run, 0 failed, 2 skipped. Lite *Investigation*: 134 of 134, no golden touched. Build 0 warnings.
• Not run: the full suite, the Windows STA window by hand, the browser (CI decides).

CHANGELOG entry

SECTION: Added
ENTRY: - Practice requests with a score per agent ([#5937]) - Pick a case you already gave feedback on, in the Viewer's new Practice tab, and Darling replays it for an agent without the original report, verdict or comment. The agent's report is scored against your feedback (a corrected verdict, or right and partly right) and the score shows per agent in the Viewer and on the web Admin page. A practice request never joins a live alert, never counts against a route's limit, and is left out of the Report Feedback list and the per-family counts; its report is not shown to other briefs. While a practice is live, the replayed case's report is also left out of the other briefs' earlier reports and prior links, and out of get_investigation_reports for an agent on the investigations arm. The score is advisory.
REF: [#5937]: #5937

[claude]

The answer key is still reachable from every other brief on the same server and family.

This suppresses earlier_reports in the practice brief, and WithoutReplayedAnswers suppresses the case's report in get_investigation_reports. But EarlierReportsSql (line 83) selects every report on rp.server_id = $1 AND rq.family = $2 with only rp.request_id <> $3 as the carve-out — it knows nothing about practice, in either direction:

1. The case's report leaks back to the practising agent. A practice request is by construction on the same server and family as the case it replays. If any other live request opens on that server+family while the practice is live and the same agent claims it, the brief it gets lists the replayed case's report in earlier_reports — verdict, root_cause, root_cause_found and summary, i.e. the answer key, through EarlierReportNode. The PR body states "get_investigation_reports is the only investigations-arm read that returns reports or feedback"; claim_investigation_request's brief returns them too.
2. The practice report leaks into live briefs. Once the practice agent submits, that report is written with the case's server_id and the practice request's family, so it becomes the newest row of EarlierReportsSql for that server+family. The next real investigation is told "an agent concluded X here 20 minutes ago" when that was a replay of a three-day-old incident — and it eats one of the ten MaxEarlierReports slots. The README's "A practice request changes nothing" does not hold here.

PriorLinksSql (line 160) has the same shape for link_recorded rows, so a practice report's links ride into live briefs too.

Both arms want the same filter on the join: AND rq.practice_of IS NULL for arm 2, and for arm 1 something like AND rp.request_id NOT IN (SELECT practice_of FROM config.investigation_requests WHERE practice_of IS NOT NULL AND <live>) — or, more simply, reuse LivePracticeReplaysAsync's set as a parameter to the brief reader.

[claude]

Two more reads of this table count practice requests as real traffic.

practice_of IS NULL lands on FindJoinSql, CountRouteSql and RouteUseSql here, but InvestigationFeedbackStore reads config.investigation_requests twice more and neither was filtered:

• ReportedSql (InvestigationFeedbackStore.cs:69) — "reported requests opened since $1, joined to their report". A practice request that an agent reported on matches. It therefore shows up in GET /api/investigation-feedback (the Admin page's Report Feedback tab), in the Viewer's Feedback tab, and — because the new Practice tab reads the same list (InvestigationPracticePanel.LoadAsync → GetInvestigationFeedbackListAsync) — as a replayable "judged case" in the Practice tab itself once somebody gives it feedback. Selecting it then gets IsPractice, which is safe but is a list of rows that cannot be used.
• FamilyCountsSql (InvestigationFeedbackStore.cs:82) — the per-family reported, with_feedback, marked_wrong, agent_false_positives and false_positives that People give feedback on investigation reports #5913 added for alert tuning, served by both the web route and get_investigation_reports. A practice report counts as a fresh report in its family, and a practice of a false_positive case that the agent gets right adds a false positive. Replay one judged false_positive case three times and the blocking family's false_positives rises by three with no new alert behind it.

The PR body says "Family counts in a window read still count the case among many; I left them" — that is about the replayed case. The practice rows entering these two statements is a different thing, and it makes an operational number drift with how much practice the operator runs.

[claude]

This raises the Requests tab's store floor from V184 to V191 without the house skew message.

GetInvestigationRequestsAsync (line 167) runs this SQL directly — it is not wrapped in SkewAsync, unlike every other V191-dependent read this PR adds (OpenInvestigationPracticeAsync, GetInvestigationPracticeScoresAsync, and #5913's feedback reads). SkewAsync's own summary is "a store without the V191 table or a column is the house schema-skew message" (line 555).

Against a V184–V190 store — a Viewer upgraded before the service has migrated, which is the case ViewerSchemaSkewException exists for — this read now throws a raw PostgresException 42703, LoadRequestsAsync's blanket catch prints Could not read the investigation requests: column "practice_of" does not exist, and the Requests tab lists nothing where it previously worked. Wrapping the call in SkewAsync restores the house sentence:

var rows = await SkewAsync(() => ...);

[claude]

Non-blocking: this reads the stored state, while LivePracticeReplaysSql (InvestigationRequestStore.cs:276) reads the computed one. They disagree on a practice request that was claimed three times: the hider calls it expired (so the case's answers are visible again, correctly), but this says it is still open/claimed and OpenAsync answers AlreadyLive. The operator then cannot start a fresh replay of that case until expires_at passes — up to the copied life of the original request — even though the dead practice is hiding nothing. Computing the state here the same way, or adding AND claim_count < 3, would keep the two answers in step.

[claude]

Non-blocking: alert_count is not in the column list, so every practice request takes the table default of 1 while CopyAlertsSql copies all of the case's alert rows. The brief reads r.alert_count (PgInvestigationBriefSourceReader.RequestSql), so a replay of a five-alert case hands the agent request.alert_count = 1 above an alerts array holding the first and the latest three — a tell, and a small departure from the class summary's "the brief's alerts section reads what the original brief read". Copying r.alert_count would need it added to the admin INSERT grant in section 10e / step 3j as well.

[claude]

Non-blocking, comment accuracy: FOR SHARE OF r locks the request row, which is what stops the case being deleted under the copy (the feedback and report tables reference it ON DELETE CASCADE). It does not lock config.investigation_feedback, so a direct DELETE FROM config.investigation_feedback WHERE request_id = … (admin holds DELETE on that table, 10e) can still commit between this read and the INSERT. The summary would be true as "locked so the case cannot be deleted under the copy".

[claude]

Verdict: CHANGES REQUESTED

Claims

• VERIFIED — OpenAsync copies route/server/family/first metric/story path/alert times and alert rows into a request with practice_of, level 1, journaled opened by person:<login> — InvestigationPracticeStore.cs:170-182,283-289; parameter order matches InvestigationRequestStore.AgentAction.cs:86. alert_count is the one request column not copied (inline note).
• VERIFIED — refuses, writing nothing, an unknown case / a practice request / no report / no feedback / an already-live practice — InvestigationPracticeStore.cs:224-268: every arm returns before the INSERT inside a transaction that is never committed.
• VERIFIED — the composer sets request.practice, adds one read rule, drops earlier reports and prior links — InvestigationBriefComposer.cs:120,165-170,213.
• VERIFIED — the Darling reader skips the case's own note and the two reads — PgInvestigationBriefSourceReader.cs:103,257,271.
• VERIFIED — a non-practice brief is byte for byte unchanged, but not by the test cited: ABriefThatIsNotPractice_IsUnchanged_ByteForByte_… composes the same Practice = false sources twice, so it pins the new argument's default, not the pre-change bytes. What proves it is that all three branches sit behind sources.Practice at InvestigationBriefComposer.cs:120,165,213.
• VERIFIED — Lite's reader never sets it — DuckDbInvestigationBriefSourceReader.cs:273 passes seven positional arguments, so Practice defaults false.
• VERIFIED — every other row has no practice_of key, so Lite's payload is unchanged — InvestigationToolCore.cs:457-463; McpHelpers.JsonOptions is new() { WriteIndented = false } with no naming policy, so the extra SerializeToNode round trip cannot move a byte.
• VERIFIED — no migration, and the grants sit in the V191-guarded block — PgMigrations.cs:1458 already adds the column; grants land in managed 10e (DarlingManagedRoles.cs:1739-1742, after 10d's REVOKE … CASCADE) and provision-roles.sql step 3j. The granted column list equals OpenSql's exactly.
• VERIFIED — FindJoinSql, CountRouteSql, RouteUseSql skip practice — InvestigationRequestStore.cs:62,93,220.
• VERIFIED — the new windows refuse out of range rather than clamp — DarlingWebEndpoints.cs:891-896 (1-8760, default 720), ViewerDataService.InvestigationPractice.cs:2005-2006 (1-365).
• REFUTED — "get_investigation_reports is the only investigations-arm read that returns reports or feedback". claim_investigation_request returns a brief, and EarlierReportsSql (PgInvestigationBriefSourceReader.cs:83-91) selects every report on the same server_id and family, carving out only the current request. The replayed case's verdict, root cause and summary therefore sit in earlier_reports of any other live brief on that server and family, including one the practising agent claims. Inline at :257.
• REFUTED — CHANGELOG "A practice request changes nothing". Its report becomes the newest earlier_reports row for that server and family (above), and InvestigationFeedbackStore.ReportedSql:69 and FamilyCountsSql:82 count practice rows, so the Report Feedback list and the per-family reported / agent_false_positives / false_positives move with how much practice is run. Inline at InvestigationRequestStore.cs:220.
• UNVERIFIED — every test-plan run (900/294/647 figures, the Node and STA runs, Lite 134 of 134): the live classes gate on DARLING_TEST_PG (InvestigationPracticeTests.cs:264,505,670) and the rest need a Windows/Node rig.
• UNVERIFIED — "the viewer login is refused with 42501 and writes nothing", through both grant sources: the assertions exist (InvestigationPracticeTests.cs:537-541) but need a live cluster with the fixed roles.

Riskiest lines

1. Darling/PerformanceMonitor.Darling.Service/Investigations/PgInvestigationBriefSourceReader.cs:257 — the whole "no answer in the brief" guarantee; it holds for the practice brief and the statement it guards leaks the same report into every other brief on that server and family — pinned by APracticeBrief_HoldsNoAnswerFromTheCase_EvenWithEveryFeedbackFieldFilledIn for the practice brief only, UNPINNED for the other-brief path.
2. Darling/PerformanceMonitor.Darling.Storage/InvestigationPracticeStore.cs:170-174 — the OpenSql column list must equal the admin column grant or every Viewer gets 42501; pinned by InvestigationPracticeGrantLiveTests.TheManagedBatch_… and TheBringYourOwnScript_…, both skipped without DARLING_TEST_PG.
3. Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.Investigations.cs:62 — makes an unwrapped V184-era read V191-only; UNPINNED.

Traps near this diff

• "Lite and Darling ship in parity … a fix to one SKU's reader, tool, alert or description without the counterpart's is a finding" — does not apply: practice is Darling-only (PgMigrations.cs:1420, "No Lite twin: Lite has no investigation store"), Practice defaults false at Lite's only construction site (DuckDbInvestigationBriefSourceReader.cs:273), practice_of is emitted only when non-null (InvestigationToolCore.cs:461), and the shared tool contract skips service parameters (InvestigationToolContract.cs:111).
• "Truncation inferred from a full page" — does not apply: WithoutReplayedAnswers never touches Requests (DarlingMcpInvestigationTools.cs:645-650), so requests_returned and truncated still describe the page the store read.
• "A total_* assigned from a .Count is a count of the page unless the set is provably whole" — does not apply: the scores come from RunsSql, which has no LIMIT (InvestigationPracticeStore.cs:185-192).
• "hours_back, limit, top, days_back are refused when out of range, never clamped" — does not apply: both new windows refuse (DarlingWebEndpoints.cs:893, ViewerDataService.InvestigationPractice.cs:2005).
• "CASE WHEN interval > 0 THEN delta / interval ELSE 0 END spells 'unknowable' as 0" — does not apply: possible > 0 ? points / possible : null (InvestigationPracticeStore.cs:127) carries the null to both surfaces (admin.js:1397, InvestigationPracticePanel.xaml.cs:1802).
• "Integer division in a rate column" — does not apply: Points and Possible are double (InvestigationPracticeStore.cs:57).
• "Ruling (#5833) … never add a census, and never answer a trap with a test that reads source text" — does not apply as a finding: no census is added, the two edits are rows in existing rosters (McpServiceParameterDiSeatCensusTests.cs:819, ReadAdmissionCoverageCensusTests.cs:231), and the GRANT-wording assertion (DarlingManagedRolesTests.cs:573-575) is backed by a behaviour test that opens a practice request as the real admin login.

Divergence (blind read vs PR body)

The feature is well sealed at the two seams the body names and unsealed at three it does not.

1. The body says get_investigation_reports is the only investigations-arm read returning reports or feedback. The brief does too: EarlierReportsSql is practice-blind, so the answer key reaches the practising agent through any other brief on the same server and family, and the practice agent's report then reaches live briefs as the newest earlier report, eating one of ten slots.
2. The body treats the family counts only as "they still count the case among many". It does not mention that practice requests themselves now enter ReportedSql and FamilyCountsSql, putting practice rows in the Report Feedback list, in the Practice tab's own case list, and in the per-family false-positive numbers.
3. The body notes the Viewer's Requests tab reads practice_of without noting that this makes a V184-era read V191-only; the PR wraps its new reads in SkewAsync and leaves this one bare.
   No mechanism the body names is missing from the diff, and nothing in the diff goes unmentioned.

Reviewer checklist

• Run the rig-gated classes with DARLING_TEST_PG set — InvestigationPracticeLiveTests, InvestigationPracticeGrantLiveTests (both grant sources, on a cluster with no pre-existing admin/viewer/mcp role), InvestigationPracticePanelLiveStaTests.
• Live: open a practice request, submit its report, then open a live request on the same server and family and read earlier_reports — neither the practice report nor the replayed case's report should be there.
• Live: note the Admin page's family counts, run three replays of one judged false_positive case, re-read them.
• Point a current Viewer at a store pinned at V190 and open Manage Investigations…; see what the Requests tab says.
• Drive the Investigations window by hand: Practice tab on a read-only seat, the window combo, and whether the list and scores refresh after Start Practice Request.
• Load the Admin page in a browser: the new Practice Scores tab, and whether the admin-servers baseline still matches with the extra tab button.

Claude posting for Erik Darling. All three blockers, the three notes and the refuted claims are answered in the body's Review round 1 section (head 4d1b2e8); the next round reviews that head.

[claude]

Follow-up pass over 4d1b2e894 (the one content commit in this range; 965dcff17 is a merge of origin/dev that only brought in unrelated CHANGELOG/UI-smoke housekeeping, left alone per instructions).

This commit answers all three CHANGES REQUESTED items from the prior full review:

1. The brief-leak seal. EarlierReportsSql and PriorLinksSql (PgInvestigationBriefSourceReader.cs:84-97,163-178) now add rq.practice_of IS NULL (never surface a practice request's own report/link as "earlier history") and NOT EXISTS (... LivePracticeRequestsSql ...) (never surface the report/link of a case a LIVE practice is replaying, to ANY brief on that server+family, not just the practicing agent's). Verified against the new test AnotherBrief_OnTheSameServerAndFamily_ReadsNoPracticeReport_AndNoReportOfACaseALivePracticeReplays, which walks exactly this: hidden while live, visible again once the practice reports (state leaves open/claimed) or dies (claim_count hits 3).
2. The not-real-traffic seal. InvestigationFeedbackStore.ReportedSql and FamilyCountsSql (InvestigationFeedbackStore.cs:69,92) now add r.practice_of IS NULL, keeping practice rows out of the Report Feedback list and per-family counts. Verified against APracticeRequest_IsNotRealTraffic_ForTheFeedbackList_AndTheFamilyCounts_AndItCopiesTheAlertCount.
3. The V184-floor seal. GetInvestigationRequestsAsync now branches on _cachedStoreSchemaVersion >= PracticeStoreSchemaVersion (191): a pre-V191 store gets the old column list read bare (unwrapped — matches its pre-existing V184 behavior), a V191+ store gets practice_of added and the read wrapped in SkewAsync like the rest of the V191 surface. Verified against the new TheRequestsTab_KeepsItsV184Floor_... test, which drops investigation_feedback (the V191 probe sentinel, confirmed at ViewerDataService.cs StoreSchemaProbeSql) and the column, and checks the version probe actually reads below 191 before asserting the row comes back with PracticeOf = null.

One more seal not called out by name in the prior review but needed for #1 to hold: LiveSql in InvestigationPracticeStore.cs and LivePracticeReplaysSql in InvestigationRequestStore.cs are now both generated from one shared InvestigationRequestStore.LivePracticeRequestsSql(nowParameter), so "live" can no longer drift between the three readers of it — directly answering the "two copies of one definition pinned equal to each other fail only after both have drifted" trap (#3684). Parameter numbering checked by hand against each Command(...) call site ($5/$6 in the brief reader, $1/$2 in the store) — all line up.

alert_count is now copied into the practice row (InvestigationPracticeStore.cs SourceSql/OpenSql, DarlingManagedRoles.cs/provision-roles.sql grants extended to match) — closes the one gap the prior review noted in the copy list.

No parity finding: practice/Level-2 investigations remain Darling-only: Lite has no practice_of concept to carry, confirmed unchanged in this range (Lite files are not in the diff).

[claude]

Verdict: LGTM

Claims

• VERIFIED — LivePracticeRequestsSql(now) is now the one definition of a live practice request, and LivePracticeReplaysSql, EarlierReportsSql/PriorLinksSql, and InvestigationPracticeStore.LiveSql all alias it rather than copying it — InvestigationRequestStore.cs:279-297, PgInvestigationBriefSourceReader.cs:86,95,176, InvestigationPracticeStore.cs:169-171.
• VERIFIED — EarlierReportsSql/PriorLinksSql exclude a practice request's own report/link and the report/link of any case a LIVE practice replays, via a new trailing now parameter ($5/$6) — PgInvestigationBriefSourceReader.cs:84-97,163-178; parameter order checked against both Command(...) call sites.
• VERIFIED — once a practice's report is submitted, the case is no longer "live" by the same definition, so the replayed case's report becomes visible again (the one named deviation from a literal "always hidden" reading) — matches AnotherBrief_OnTheSameServerAndFamily_ReadsNoPracticeReport_AndNoReportOfACaseALivePracticeReplays's afterReport assertions in InvestigationPracticeTests.cs.
• VERIFIED — ReportedSql/FamilyCountsSql add r.practice_of IS NULL, so practice rows no longer count as real traffic — InvestigationFeedbackStore.cs:69,92.
• VERIFIED — InvestigationRequestsSelectSql takes withPractice; GetInvestigationRequestsAsync asks for practice_of only when the cached store version is ≥ the new PracticeStoreSchemaVersion = 191, wrapping that read in SkewAsync, and reads the pre-V191 shape bare otherwise — ViewerDataService.Investigations.cs:56-62,170-213.
• VERIFIED — the V184-floor test drops config.investigation_feedback (the actual V191 probe sentinel, confirmed at ViewerDataService.cs's StoreSchemaProbeSql) and the practice_of column, then asserts the probed version reads below 191 before checking the row — ViewerInvestigationsLiveTests.cs TheRequestsTab_KeepsItsV184Floor_....
• VERIFIED — InvestigationPracticeStore.LiveSql now aliases the computed "live" definition, so a practice claimed three times no longer blocks a fresh replay — matches ADeadPractice_DoesNotBlockAFreshReplayOfTheCase.
• VERIFIED — alert_count is now copied (SourceSql/OpenSql ordinal 12/$14) and the admin grant column lists in DarlingManagedRoles.cs and provision-roles.sql both gained it, in the same position as OpenSql's insert list.
• VERIFIED — the README no longer claims "a practice request changes nothing"; it now lists what a practice request is excluded from — Darling/README.md.
• UNVERIFIED — "the test fails on the old SQL (checked by removing the two filters)" and "the grant live tests (both sources) pass": all of this needs DARLING_TEST_PG against a live PostgreSQL/TimescaleDB cluster, which this read-only tree can't run.
• REFUTED — none.

Riskiest lines

1. Darling/PerformanceMonitor.Darling.Service/Investigations/PgInvestigationBriefSourceReader.cs:95,97 — the NOT EXISTS/practice_of IS NULL pair is the entire fix for the prior review's blocker 1 (the answer-key leak); a mistake here reopens that leak silently — pinned by AnotherBrief_OnTheSameServerAndFamily_ReadsNoPracticeReport_AndNoReportOfACaseALivePracticeReplays (live-rig gated).
2. Darling/PerformanceMonitor.Darling.Viewer/ViewerDataService.Investigations.cs:177-182,213 — withPractice must stay false exactly when the SQL omits the practice_of column, or reader.GetGuid(17) reads past the result set; protected here only by && short-circuit evaluation, not by a defensive bounds check — pinned by TheRequestsTab_KeepsItsV184Floor_ListingAStoreBeforeV191AsBefore_WithNoPracticeColumn (live-rig gated).
3. Darling/PerformanceMonitor.Darling.Storage/InvestigationPracticeStore.cs:275-277 — the OpenSql parameter list grew by one (alert_count at $14) and shifted practice_of to $15; an off-by-one here silently swaps alert_count and the practice marker — pinned by APracticeRequest_IsNotRealTraffic_ForTheFeedbackList_AndTheFamilyCounts_AndItCopiesTheAlertCount (live-rig gated).

Traps near this diff

• "Two copies of one definition pinned EQUAL to each other fail only after BOTH have drifted (#3684). Alias, do not copy." — applies, and this commit is the fix for it: LiveSql/LivePracticeReplaysSql collapsed into one shared LivePracticeRequestsSql generator (InvestigationRequestStore.cs:279), closing exactly the gap the prior review found between the brief reader's ad hoc "live" and the store's.
• "Lite and Darling ship in parity … a fix to one SKU's reader, tool, alert or description without the counterpart's is a finding" — does not apply: no Lite file is touched in this range (git diff --name-only has none under Lite/), and practice/Level-2 investigations remain Darling-only as the prior full review already established.

Divergence (blind read vs PR body)

None — the "Review round 1" section of the PR body names exactly the three blockers fixed, the one deviation (case reappears once the practice is no longer live), and the alert_count/FOR SHARE OF notes, and the diff implements each one as described.

Reviewer checklist

• Run the rig-gated classes with DARLING_TEST_PG set: InvestigationPracticeLiveTests (including the two new facts in AnotherBrief_...), APracticeRequest_IsNotRealTraffic_..., ADeadPractice_DoesNotBlockAFreshReplayOfTheCase, ViewerInvestigationsLiveTests.TheRequestsTab_KeepsItsV184Floor_..., and the grant live tests.
• Live: open a practice request, submit its report, then read a live brief on the same server/family twice — once while the practice is open (case hidden) and once after submission (case visible again).