People can give feedback on investigation reports: store, web tab, false-positive counts (#5913)

[erikdarlingdata]

Claude posting for Erik Darling.

Fixes #5913. Part of #5632.

Why

An operator can read an investigation report but cannot tell Darling whether it was right. This lets people mark a report right, partly right or wrong, say what the agent missed, and see how often each alert family's investigations end as a false positive. It works from the web Admin page and from the Viewer.

What changed

The store write and read (Darling.Storage/InvestigationFeedbackStore.cs, Common/Investigations/InvestigationFeedback.cs, Common/Investigations/InvestigationContracts.cs). The shared InvestigationReportsRead (the return type of IInvestigationRequestStore.ReadReportsAsync) gains two optional trailing members, Feedback and FamilyCounts, that only a feedback-keeping store (Darling's) fills. Lite's store never sets them, InvestigationToolCore.ReportsAnswer guards both keys on null, so Lite's answers are unchanged and no Lite file is touched.

• RecordAsync is one upsert on V191's unique index (request_id, lower(author)): a second record by the same author (any letter case) replaces the first, and the author's spelling and time are the new record's.
• Rules in InvestigationFeedbackRules mirror V191's CHECKs and refuse in plain words (no 500): a verdict outside right, partly or wrong, a corrected verdict outside real, false_positive or needs_person, text over 4000 characters, an author outside [A-Za-z0-9._:-]{1,64}. A request with no report is NoReport; an unknown id is NotFound.
• Reads: ReadAsync (feedback of request ids), ListReportedAsync, FamilyCountsAsync, CanRecordAsync (has_table_privilege INSERT and UPDATE on the table). Each read has an overload that takes the caller's open NpgsqlConnection; the data-source ones delegate.
• Author shaping is now injective. AuthorFromIdentity keeps a sign-in that V191 already accepts exactly as typed. Any other sign-in has each refused character turned into an underscore and gets - plus 8 hex characters of a SHA-256 of the whole name (lower-cased, because the unique index ignores case), so DOMAIN\erik reads DOMAIN_erik-<hash> and no longer collides with DOMAIN/erik, DOMAIN erik or a plain DOMAIN_erik. Names over 64 characters keep 55 characters plus the suffix, so two long names that differ only past the 64th stay apart. I chose this over refusing the identity because a Windows or e-mail sign-in would otherwise be locked out of giving feedback at all.

The web (DarlingWebEndpoints.cs, wwwroot/js/pages/admin.js).

• GET /api/investigation-feedback?hours_back= (default 168, 1 to 2160): reported requests (newest 200) with their feedback, plus the counts per family, plus can_record. Lists for everyone. It reads the list, the counts and can_record on one connection.
• PUT /api/investigation-feedback/{requestId}: JSON required (415), author is the seat's EditorPrincipal. 400 for a refused value, 404 unknown request, 409 no report, and 403 with FeedbackRefusedMessage when the store refuses for privilege (SQLSTATE 42501, the viewer role). Nothing is written under the viewer login and there is no new viewer grant.
• The PUT is swept now. The 200 receipt, the sentences for a refused body, 403, 404 and 409 go through DarlingWebStatementSweep.JsonText; the bad-GUID and oversize-body 400s go through ErrorResult, the 415 through UnsupportedMediaTypeResult, and the 500/503 through DarlingWebFailureLog.Body (all fixed sentences the census accepts). So the route is no longer on WebRouteStatementCensusTests' named list; the entry is removed. The scanner accepts ErrorResult(...) as a fixed writer only for a literal message or bodyError, which is why the previous attempt was not recognised. The refusal sentences echo a clipped caller-typed value, so sweeping them costs nothing and is the honest classification. The handler reads the store result by deconstruction, because WebExceptionTextCensusTests flags any x.Message in web code.
• New "Report Feedback" tab on the Admin page: false-positive counts table, a card per report with its feedback, and a form shown only for a sign-in that can make changes AND when the list's can_record is true (the store says the dashboard's own login may write the table). On managed and compose stores the web connects as the viewer role, so can_record is false there: no form, and the tab says it is list-only and feedback is given in the Viewer.

The Viewer (ViewerDataService.Investigations.cs, InvestigationsWindow.xaml and .xaml.cs). A new Report Feedback tab on the Investigations window, shaped like the Requests tab:

• Window picker (7, 30 or 90 days) with a label stating the window of the counts and the settled-verdict rule, the false-positive counts per alert family, the reported requests (newest report first, at most 200, with a note when the window held more), and for the selected report the report text and the feedback so far.
• A form for admin logins: the report was right, partly or wrong; what it should have said (or keep the report's verdict); the step the agent missed; a comment; Save. The author is the signed-in login, shaped as above. A second save by the same login replaces the first and the status line says so.
• GetInvestigationFeedbackListAsync, GetInvestigationFamilyCountsAsync and RecordInvestigationFeedbackAsync reuse InvestigationFeedbackStore on the Viewer's data source (no new grant: the Viewer writes under the admin login). A refused value is an ArgumentException carrying the rules' sentence; SQLSTATE 42501 becomes ViewerReadOnlyException (the message the Viewer already uses for refused writes), and a store below V191 becomes ViewerSchemaSkewException. A read-only seat sees the lists and counts, the form is hidden and the status line says feedback cannot be changed here. The window also checks the values before the store, as its other commands do.

The reports list and the false-positive counts. get_investigation_reports returns, per report, a feedback array (free text masked like the report's, author masked as a login), and for a window read a family_counts array: reported, with_feedback, marked_wrong, agent_false_positives (reports that say false_positive) and false_positives (the settled verdict after feedback). The settled verdict of a request is the corrected verdict of its newest feedback that gave one, else the report's verdict, so feedback moves the count both ways. A store without feedback (Lite) sends neither key, so its answer keeps its shape.

Guide text. Darling/README.md (the investigations section) says where feedback is given (the Viewer's Investigations window and the web Admin page, both on a Report Feedback tab), that it is recorded only under the admin login, that one person has one feedback per report, and what it feeds (get_investigation_reports and the per-family false-positive counts). docs/tabs.md lists the new Admin tab.

Tests

Behaviour tests, no new census, roster or source-text test.

• InvestigationFeedbackLiveTests (6, live): refusal in plain words with nothing written; one record per author in any letter case; the counts move in both directions and only for that family; a deleted request deletes its feedback; the web routes as the managed admin role and as the managed viewer role (grants replayed from the shipped provisioning SQL).
• AdminReportFeedbackBehaviourTests (5, Node harness): the record goes to the route as JSON, a 403 or 400 shows the service's sentence, a 404 rereads, a read-only seat gets no form.
• InvestigationFeedbackAuthorTests (new, 7 cases): a valid name is kept; sign-ins differing only in refused characters, or only past the 64th, stay distinct; letter case is one author; every result passes V191's rule.
• ViewerInvestigationsLiveTests gained two live facts: an admin seat records, replaces on a second save by the same login (any case), adds a second author, reads the list and the counts, and gets the rules' sentence for a bad verdict or a blank login; a viewer seat reads the list and counts and a save throws ViewerReadOnlyException with nothing written (grants replayed from the shipped provisioning SQL). ViewerInvestigationsValidationTests and InvestigationsWindowStaTests gained the refusals before the store, the feedback checks, and the hidden form on a read-only seat.

Ran

On a throwaway PostgreSQL rig (UTC, port 55913, stopped afterwards), built the Viewer (Build succeeded, 0 warnings) and Darling.Tests (0 warnings), then all passing: ViewerInvestigations* 9, InvestigationsWindowStaTests 7, InvestigationFeedback* 13, AdminReportFeedbackBehaviourTests 5, WebRouteStatementCensusTests 13, ReadAdmissionCoverageCensusTests 10, *Census* 499 (after fixing one WebExceptionTextCensusTests hit my first edit caused), the whole Stage=Guard trait 4182, StorageCommandTimeoutTests 24, StartupCommandTimeoutTests 15, AlertReadFailureSurfaceTests 27, LivePostgresCollectionHygieneTests 10, ViewerSchemaVersionGateTests 34, InvestigationRequestStoreLiveTests 13, DarlingMcpInvestigationToolsTests 42, AdminPageBehaviourTests 14, ViewerServerTabCapabilityPinTests 6. There are no *Ratchet*, DocComment* or SlowClass* classes in Darling.Tests (0 tests). Not run: the full suite, Lite.Tests (Lite untouched), and the new Viewer tab was not looked at on screen (WPF; the STA tests construct the window, they never lay it out). The round-1 layout change below is likewise unseen.

Round 1 review

Head after this push: 9d35e89d8 (on top of 70e6b9e3b). The red CI job and every finding of review 5543800209:

• Red CI job (ViewerGridTimeColumnSortMemberTests.EveryTimeColumnInTheViewer_SortsByADateTime_NotByItsText): the two new time columns sorted by Row.ReportedAtUtc / Row.CreatedAtUtc, a nested path the roster does not check. FeedbackReportItem now has ReportedAtUtc and FeedbackGivenItem has CreatedAtUtc (DateTime properties the display text is formatted from), the columns' SortMemberPath names them, and both columns are in the test's Columns roster. The class passes.
• 1. Form gated on the seat, not the store role. GET /api/investigation-feedback now carries can_record (via FeedbackListBody), from InvestigationFeedbackStore.CanRecordAsync: has_table_privilege('config.investigation_feedback','INSERT') AND ...'UPDATE' on the list's own connection (the record is an upsert, so both). It is a separate scalar read on that connection, not a column of the counts read, because the counts read returns no row for a window with no reports. The page shows the form on session.can_edit === true && data.can_record === true; when can_record is not true it shows FEEDBACK_LIST_ONLY (the old note's sentence, now true), and FEEDBACK_NOTE no longer promises list-only. The module comment, the note and Darling/README.md now say the same thing. Pinned in the Node harness (feedback:nostore: seat can edit, can_record false, zero forms, the sentence shown; feedback:recorded with can_record true: the form, no list-only sentence) and live (can_record true under the managed admin role, false under the managed viewer role).
• 2. Raw UTC stamp. The Reported paragraph goes through localizeIsoText; each record line now ends with given <local time> (also localized) and the card says reports are newest first (note); .feedback-card, .feedback-form and .tag-status have rules in wwwroot/css/theme.css next to .tag-field. Pinned in the harness: the Reported line carries no ISO T/.0000000Z text and ends by agent-a: real.
• 3. Window and caps. Both surfaces now state the window of the counts: the web (feedbackWindowLabel(hours_back): "Counts and reports cover the requests opened in the last 7 days that have a report.") and the Viewer (FeedbackWindowLabel(days), next to the picker, also stating the settled-verdict rule). The caps are aligned: the web list limit went from 100 to 200 to match the Viewer's. The windows still differ on purpose (web 7 days fixed, Viewer 30 days default with a picker) and each says so in its label and in the README. Settled-verdict rule: kept, and stated (my call): a later feedback that gives no corrected verdict does not displace an earlier one that did, because a person who only says "partly right" has not withdrawn the correction before it. It is said in the web note, the Viewer label and the README. Pinned: harness checks the web window label, ViewerInvestigationsTests checks the Viewer label.
• 4. Clamp. Math.Clamp is gone: GetInvestigationFeedbackListAsync and GetInvestigationFamilyCountsAsync call ArgumentOutOfRangeException.ThrowIfLessThan(daysBack, 1) and ThrowIfGreaterThan(daysBack, 365) (new InvestigationFeedbackMaxDays), and the XML summary says so. Live fact: -1, 0, 366 and 4000 throw for both reads; 1 and 365 read.
• 5. Second connection. InvestigationFeedbackStore.ReadAsync, ListReportedAsync and FamilyCountsAsync have overloads taking the caller's NpgsqlConnection (the data-source ones open one and delegate, no statement changed). ReadReportsAsync uses them, so it holds one connection for the whole read, and the web list route does the same for its three reads. The early return when the window holds no requests now carries Feedback = [] and FamilyCounts = [] (null for a one-request read), so a Darling read never answers without the key. Live fact added: an empty window gives empty, non-null lists.
• 6. Body wording. Fixed in ## What changed: the shared InvestigationReportsRead gains two optional members that only a feedback-keeping store fills, and InvestigationContracts.cs is in the file list. The PUT answer enumeration is corrected (above).
• 7. Access keys and layout. The tab header is Report Feed_back (Alt+B, was Alt+F, which the Requests tab's Re_fresh holds) and the new tab's Refresh is Refres_h (Alt+H); no other key in the window uses B or H. The fixed Height="110"/"130"/"150" and Width="760": the counts grid has MinHeight/MaxHeight, the right column is a grid with star rows (report text 2*, feedback grid 3*, each with a minimum), the verdict/missed-step row is a WrapPanel, and the comment box takes the remaining width. Not looked at on screen (no way to run WPF here); the STA tests construct the window but do not lay it out.

Ran (rig on port 55913, UTC, stopped afterwards; build Build succeeded, 0 warnings): ViewerGridTimeColumnSortMemberTests, InvestigationFeedbackLiveTests, ViewerInvestigationsLiveTests, InvestigationsWindowStaTests, AdminReportFeedbackBehaviourTests, AdminPageBehaviourTests, ViewerInvestigationsTests, InvestigationFeedbackAuthorTests (162 total, 0 failed, 0 skipped); every Census class in Darling.Tests plus WebRouteStatementCensusTests, ReadAdmissionCoverageCensusTests, WebExceptionTextCensusTests, DarlingMcpInvestigationToolsTests, the command-timeout and live-hygiene pins (541, 0 failed); AdminPageTests, AdminServerEditBehaviourTests, AwsRoleEditTests, DarlingMcpInvestigationToolsTests, ViewerInvestigationsTests (347, 0 failed); the sort-member, STA and ViewerInvestigationsTests classes again after the last edit (123, 0 failed). There is no *Ratchet* class besides LiveCleanupConversionRatchetTests, which passed. Not run: the full suite (CI decides), Lite.Tests (untouched).
Dev (6c26226: #5907, #5919, #5922, #5925, #5908) was merged in (5eb3166); the five conflicts (admin.js, admin-server-edit-harness.mjs, ReadAdmissionCoverageCensusTests.cs, Darling/README.md, InvestigationToolCore.cs) were resolved by keeping both sides, with no other change: the Admin page now has five tabs (Report Feedback and Pending Notes), both routes sets sit in the census pin, and the Common file keeps dev's LinkNode beside this PR's FamilyCountsPayload. After the merge: Darling.Tests build and Viewer build and Lite.Tests build Build succeeded, 0 warnings; the brief's Darling classes plus every Census, Ratchet, AgentAction* and investigation tool-contract class ran 499 passed, 0 failed (live classes on a throwaway PostgreSQL rig, stopped afterwards); Lite.Tests *Investigation* and LiteInvestigationGoldenTests 134 passed and *Census* 32 passed.

CHANGELOG entry

SECTION: Added
ENTRY: - People can give feedback on investigation reports ([#5913]) - A report can be marked right, partly right or wrong, with a corrected verdict, the step the agent missed and a comment, from the Viewer's Investigations window (Report Feedback tab, admin login) or the web Admin page's new Report Feedback tab (recorded only under an admin store login; list-only otherwise). get_investigation_reports now returns each report's feedback and, per alert family, how many investigations ended as false positives before and after people's corrections.
REF: [#5913]: #5926

For the coordinator to double-check

• The false-positive definition (the settled verdict above) and the hours_back window on opened_at; both are my call, the issue does not define them.
• A feedback requires the request to hold a report (409 on the web, NoReport in the Viewer); the issue does not say, I judged feedback on an unreported request meaningless.
• The author shaping now appends a hash suffix to any sign-in V191 would refuse. The suffix is visible in the feedback list (DOMAIN_erik-1a2b3c4d); a plain erik stays erik. Feedback already recorded under the previous shaping (only possible on a dev build of this branch) would not be replaced by a new save.
• The PUT is swept, not named. If you would rather keep a fixed-receipt route on the named list as /api/alert-history/dismiss is, the change is a two-line revert, but the sweep is the stronger gate.
• No journal row is written for feedback (the journal event domain was not widened; no migration allowed).
• The Viewer tab was built and tested through the STA window and live store tests but not looked at with eyes; after the round-1 layout change (star rows, wrap panel) a quick open of Settings, Investigations, Report Feedback at a narrow and a wide window is worth doing.
• Round 1: can_record is a separate has_table_privilege scalar on the list's connection rather than a column of the counts read (the counts read has no row for an empty window); it checks INSERT and UPDATE because the record is an upsert.
• Round 1: the web list limit moved from 100 to 200 to match the Viewer; the two windows (web 7 days fixed, Viewer 30 days with a picker) still differ and are now labeled. The settled-verdict rule is unchanged and now stated.
• Round 1: the CHANGELOG entry above is unchanged; if you want the user-visible round-1 changes in it, the new facts are the window labels and that the web form hides on a viewer-role store.

[claude]

A raw UTC machine stamp reaches the page. r.reported_at_utc is interpolated straight into the paragraph's text. The route writes it with Iso() = ToString("o") (DarlingWebEndpoints.cs, FeedbackListBody), so what a person reads is Reported 2026-10-01T00:10:00.0000000Z by agent-a: real.

Every other instant this dashboard prints is shown in the reader's own clock: localizeIsoText (util.js:199) exists for exactly this shape — its own test fixture pins the seven-fraction-digit form (web-partial-window-harness.mjs:120, frac7) — noticeStrip/emptyStrip apply it to every strip they build (util.js:178, util.js:183), panels.js:1321 applies it to a localizeText column, and sweeps.js:300 does it by hand for a note, pinned by WebClampCellBehaviourTests. The Viewer twin of this very tab localizes too — InvestigationsWindow.xaml.cs, FeedbackReportItem.ReportedDisplay is Row.ReportedAtUtc.ToLocalTime().ToString("yyyy-MM-dd HH:mm", ...). So the two surfaces that are supposed to show the same list disagree, and the web one shows the hour in UTC to a reader whose grids beside it are local.

Suggested change

	el("p", { class: "mute-label", text: "Reported " + r.reported_at_utc + " by " + r.agent_id + ": " + r.report_verdict }),
	el("p", { class: "mute-label", text: localizeIsoText("Reported " + r.reported_at_utc + " by " + r.agent_id + ": " + r.report_verdict) }),

(localizeIsoText needs adding to the util.js import on line 24.)

Two related gaps in the same card, while you are here:

• feedbackLine (line 1280) never renders recorded_at_utc, though the route returns it per record. A web reader cannot tell when a record was given, or which of two records by different authors is the newer one — the <ul> is newest-first but nothing says so. The Viewer shows it (FeedbackGivenItem.WhenDisplay).
• .feedback-card (line 1332), .feedback-form (line 1320) and .tag-status (line 1307) have no rule anywhere under wwwroot/css/; grep -rn 'feedback-card\|feedback-form\|tag-status' wwwroot/css/ is empty. .tag-field and .mute-label beside them are styled (theme.css:81), so the three new names style nothing — the counts table and the fields render, the card and the form are bare blocks.

[claude]

The form is gated on the web seat, not on the store role, so "list-only there" is not what the page does.

session.can_edit is DarlingWebSeat.CanEdit — the shared token or an OIDC admin seat (DarlingWebSeat.cs:38, :87). It says nothing about which PostgreSQL role postgres.webConnectionString logs in as. On a managed or compose store the web host connects as the viewer role, which holds only SELECT on config.investigation_feedback (DarlingManagedRoles.cs:1734-1737), so every record there answers 403.

But three places claim the page is list-only on exactly those stores:

• FEEDBACK_NOTE, line 1116: "on managed and compose stores the web dashboard connects as the viewer role, so this page is list-only there and feedback is given in the desktop Viewer."
• the module comment, line 1218: "The page lists for everyone." then line 1219-1221 repeats the viewer-role 403.
• Darling/README.md: "on a store where the web connects as the read-only viewer role (managed and compose stores) the web tab lists feedback and gives it in the viewer instead."

What an admin seat on a managed store actually gets is four filled-in fields and a Record feedback button that always 403s — which is the path AdminReportFeedbackBehaviourTests.ARefusedOrInvalidRecord_ShowsTheServicesSentence_AndDoesNotReadTheListAgain("refused") pins, so the behaviour is tested; it is the three sentences that are wrong. On the default deployment shape every person who finds this tab types a verdict, a missed step and a comment before being told to go use a different application.

Two ways out, either is fine:

1. Let the answer carry the capability. GET /api/investigation-feedback already runs on the web login; have FeedbackListBody add a can_record the route sets from a cheap probe (has_table_privilege('config.investigation_feedback', 'INSERT') on the same connection, one more column on the counts read), and gate line 1360 on session.can_edit === true && data.can_record === true, with the note's sentence shown when it is false. That makes the note true and costs no round trip.
2. Keep the form unconditional and reword all three sentences to say what happens — the form is shown to any sign-in that can make changes, and a record is refused with the service's sentence unless the web dashboard's own store login holds the admin role.

The 403 sentence itself (FeedbackRefusedMessage) is good and does name the Viewer; it is the promise made before the click that the page does not keep.

[claude]

Non-blocking, but it is a shape this repo has a standing rule about. .github/REVIEW-TRAPS.md, Storage and MCP payloads:

hours_back, limit, top, days_back are refused when out of range, never clamped — a clamp is a silent lie about the window (#3699's census pins zero clamps across 156 methods; a negative window is refused, #3641).

Math.Clamp(daysBack, 1, 365) here is the only window clamp in ViewerDataService.*: every neighbour takes its bound as given (ViewerDataService.FinOps.Workload.cs:54, :96, :108 — DateTime.UtcNow.AddHours(-hoursBack), no clamp), and ValidateInvestigationNote just below (line 564) is the file's own refuse-don't-clamp example.

It is unreachable today — the one caller is InvestigationsWindow.FeedbackDays, which yields 7, 30 or 90 from FeedbackWindowBox's Tag, or 30 when the parse fails — so no caller can hand it an out-of-range day count, which is why I am not blocking on it. Two things would still be worth doing: the XML summary says "the last daysBack days" and does not mention that daysBack = 0 silently reads one day and daysBack = 4000 silently reads a year; and a public method on a public service is reachable by the next caller, who will not know. ArgumentOutOfRangeException.ThrowIfLessThan(daysBack, 1) plus a ThrowIfGreaterThan(daysBack, 365) keeps the two surfaces honest and matches the file.

[claude]

Non-blocking, an observation about where these two reads sit rather than about what they compute.

ReadReportsAsync opens one connection at line 695 and holds it for the whole method — the requests read, the reports read and the journal read all pass that same connection (lines 698, 727, 738). These two calls break that: InvestigationFeedbackStore holds a data source, not a connection, so ReadAsync and FamilyCountsAsync each open a second pooled connection (InvestigationFeedbackStore.cs:1423, :1484) while line 695's is still open and unused. So the method's pool demand doubles, and get_investigation_reports — the one production caller (DarlingMcpInvestigationTools.cs:592) — goes from three round trips to five on every window read.

Neither matters at today's concurrency, and the reads are correct: sinceUtc and serverId are the same values the requests read used (ReportsRequestsSql's r.opened_at >= $4 and r.server_id = $3 are FamilyCountsSql's $1 and $2), so the counts describe exactly the population the page was cut from, which is the right side of the "a count taken off the page" trap. If you want the one-connection shape back, a ReadAsync(NpgsqlConnection, ...) / FamilyCountsAsync(NpgsqlConnection, ...) overload on the feedback store that takes the caller's connection, with the data-source ones delegating, would do it without changing a statement.

One shape note while this is open: the early return at line 724 leaves Feedback and FamilyCounts null, so a Darling window read with no requests at all answers without a family_counts key, while the same read with one request answers with family_counts: []. It is unreachable through the tool — DarlingMcpInvestigationTools.cs:595 short-circuits Requests.Count == 0 to status: empty before ReportsAnswer is ever called — so nothing is wrong today. It is one line (new InvestigationReportsRead(requests, reports, journal, brief, requestsTruncated, false, [], requestId is null ? [] : null)) if you would rather the record never carry the ambiguity for the next caller, since the contract comment reads that null means "a store that keeps no feedback".

[claude]

Non-blocking, a note on the PR body rather than on the code. The body says:

The store write and read (Darling.Storage/InvestigationFeedbackStore.cs, Common/Investigations/InvestigationFeedback.cs). No change to the shared IInvestigationRequestStore contract, so Lite is untouched.

The record changed here is that contract's return type — IInvestigationRequestStore.cs:61 declares Task<InvestigationReportsRead> ReadReportsAsync(...), and InvestigationReportsRead gains Feedback and FamilyCounts. The interface's own signature is untouched, and the file list the body gives for the store work omits this one.

The conclusion the body draws from it does hold, which is why this is not a finding: both members are trailing optional parameters, so Lite's positional construction still compiles; Lite's store never sets them; and InvestigationToolCore.ReportsAnswer guards both keys on null (:317, :353), so Lite's get_investigation_reports and its InvestigationPanelReader answer byte-for-byte what they answered before. No Lite file is in the diff. Worth stating as "the shared InvestigationReportsRead gains two optional members that only a feedback-keeping store fills" rather than "no change to the shared contract" — a reviewer checking the two-store parity rule reads the first and stops, and reads the second and goes looking.

[claude]

Verdict: CHANGES REQUESTED

Claims

• VERIFIED — "RecordAsync is one upsert on V191's unique index (request_id, lower(author)); a second record by the same author replaces the first, and the author's spelling and time are the new record's" — InvestigationFeedbackStore.cs RecordSql against PgMigrations.cs:1506.
• VERIFIED — "Rules in InvestigationFeedbackRules mirror V191's CHECKs; no report is NoReport, an unknown id NotFound" — the domains, the 4000-char caps and IsValidAuthor → InvestigationRules.IsValidAgentId all equal PgMigrations.cs:1499-1503; the split is RequestExistsSql.
• VERIFIED — "Author shaping is now injective; names over 64 keep 55 characters plus the suffix" — AuthorFromIdentity: 55 + '-' + 8 hex = 64, hashed over ToLowerInvariant(). Pinned by InvestigationFeedbackAuthorTests.SignInsThatDifferOnlyInRefusedCharacters_StayDistinctAuthors and .LongNames_StayDistinct_PastTheSixtyFourthCharacter_AndFit.
• VERIFIED — "415, 400, 404, 409, and 403 with FeedbackRefusedMessage (42501)", and "no new viewer grant" — MapInvestigationFeedback with FeedbackRefusalStatus / IsFeedbackPrivilegeRefusal; grants already ship at DarlingManagedRoles.cs:1734-1737 and tools/provision-roles.sql:1724-1728.
• VERIFIED — "The PUT is swept now; the entry is removed" — net effect holds with no census file in the diff: the answer paths return DarlingWebStatementSweep.JsonText (WebRouteStatementCensusTests.cs:115), Combine at :305 makes a mixed route Swept, the rest are on FixedWriters (:83-88), and the handler deconstructs instead of reading ex.Message.
• VERIFIED — "Lite sends neither key"; the settled verdict moving the count both ways; a read-only Viewer seat keeping the lists and losing the form; the 200-row cap with its note; no journal row; docs/tabs.md — InvestigationToolCore.cs:317/:353, FamilyCountsSql's newest_corrected CTE (pinned by InvestigationFeedbackLiveTests.Feedback_ChangesTheFalsePositiveCounts_OfThatAlertFamilyOnly_InBothDirections), GiveFeedbackPanel collapsed (pinned by ViewerInvestigationsTests.AReadOnlySeat_HasNoWriteControls_AndSaysWhy), ListReportedAsync's limit + 1, docs/tabs.md:39.
• UNVERIFIED — "built the Viewer (0 warnings) ... all passing ... Stage=Guard 4182" — nothing here runs: WPF needs Windows, the live classes skip without DARLING_TEST_PG, the harness needs Node, the window tests a desktop.
• UNVERIFIED — "the new Viewer tab was not looked at on screen" — a self-reported omission a read-only tree cannot settle.
• REFUTED — "No change to the shared IInvestigationRequestStore contract, so Lite is untouched" — InvestigationContracts.cs:171-178: InvestigationReportsRead, that interface's return type (IInvestigationRequestStore.cs:61), gains Feedback and FamilyCounts, and the body's file list omits the file. The conclusion holds (trailing optionals, Lite sets neither, both keys guarded on null); the sentence is what a parity reviewer reads before stopping. Inline.
• REFUTED — "On managed and compose stores the tab is list-only there" (also admin.js:1116, :1218, Darling/README.md) — admin.js:1360 gates the form on session.can_edit, i.e. DarlingWebSeat.CanEdit (DarlingWebSeat.cs:38), which knows nothing about the store role. An admin seat there gets the full form and a Record button that always 403s — the path AdminReportFeedbackBehaviourTests.ARefusedOrInvalidRecord_...("refused") pins. Inline, with two fixes.

Riskiest lines

1. Darling/PerformanceMonitor.Darling.Storage/InvestigationFeedbackStore.cs:56 — RETURNING (xmax = 0) decides Recorded from Replaced off a heap detail, and both surfaces' status sentences ride on it. House knowledge supports it (QueryStoreWatermarksWriterLiveTests.cs:567) — pinned by InvestigationFeedbackLiveTests.ASecondRecordByTheSameAuthor_InAnyLetterCase_ReplacesTheFirst_AndAnotherAuthorAddsOne, live-only.
2. PerformanceMonitor.Common/Investigations/InvestigationFeedback.cs:2052 — two sign-ins collapsing to one author is one person silently overwriting another's verdict through the unique index; and 55 + 1 + 8 must be exactly 64 or V191's CHECK refuses what the rules passed. Pinned by InvestigationFeedbackAuthorTests.LongNames_StayDistinct_PastTheSixtyFourthCharacter_AndFit.
3. Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/admin.js:1334 — the one changed line whose output reaches a person uninspected: Reported 2026-10-01T00:10:00.0000000Z. UNPINNED — the harness asserts tabs, form count, the write, the reread count, the status and the feedback line; nothing asserts this paragraph.

Traps near this diff

• "Truncation inferred from a full page ... Fetch limit + 1; the flag is fetched.Count > limit." — does not apply — ListReportedAsync binds limit + 1 and sets var truncated = heads.Count > limit;.
• "A count taken off the page and named as the population's ... the page's count is *_returned." — does not apply — the counts come from FamilyCountsSql, no LIMIT, same opened_at/server_id population; the page's count is requests_returned.
• "hours_back, limit, top, days_back are refused when out of range, never clamped — a clamp is a silent lie about the window (#3699; a negative window is refused, #3641)." — applies (unreachable today) — ViewerDataService.Investigations.cs:487 and :496 clamp to 1-365, the only window clamp in ViewerDataService.* (compare ViewerDataService.FinOps.Workload.cs:54). The web half refuses, 400, pinned live; FeedbackDays yields only 7/30/90, so this is an inline note, not part of the verdict.
• "Lite and Darling ship in parity ... a fix to one SKU's reader, tool, alert or description without the counterpart's is a finding, not a nit" — does not apply — Darling-only by construction (PgMigrations.cs:1425, "No Lite twin"); Lite's answer is unchanged key-for-key and the new description text lands after <<GUIDE>> (DarlingMcpInvestigationTools.cs:534).
• "Ruling (#5833): tests are load-bearing. ... Never add one, and never answer a trap with a test that reads source text" — does not apply — no new census; the two ReadAdmissionCoverageCensusTests rows are what its existing roster demands for a new route.

Divergence (blind read vs PR body)

Beyond the two REFUTED claims, two.

"Every answer of the PUT goes through DarlingWebStatementSweep.JsonText" — three do not: the bad-GUID and oversize-body 400s via ErrorResult, the 415, and the 500/503 via DarlingWebFailureLog.Body. All are fixed sentences the census accepts, so the conclusion is right and only the enumeration is not.

"Shaped like the Requests tab" hides a window difference: the web lists a fixed 168 hours with no picker and caps at 100, the Viewer defaults to 30 days with a 7/30/90 picker and caps at 200. Neither surface says over what window the counts are taken. Related and also unstated: a later feedback giving no corrected verdict does not displace an earlier one that did.

Reviewer checklist

• Open the Viewer on a live V191 store under connectAs = "admin": the STA tests construct the window but never lay it out, so the fixed Height="110"/Height="150" grids and Width="760" box are unmeasured against the 3*/2* split.
• On the web Report Feedback tab against a managed or compose store, confirm what an admin seat sees (per the comment on admin.js:1360: the full form, 403 on save).
• Run InvestigationFeedbackLiveTests and the two new ViewerInvestigationsLiveTests facts on Windows with DARLING_TEST_PG: RETURNING (xmax = 0) is unexercised offline.
• Re-run WebRouteStatementCensusTests and ReadAdmissionCoverageCensusTests; I graded them by reading the same source text they read.
• Check the access key: Report _Feedback and the Requests tab's Re_fresh both claim Alt+F, and the new tab's Refresh has none.

Claude posting for Erik Darling. Round 1's findings were fixed in 9d35e89 (the PR body's Round 1 review section) and the round-2 review run on that head ended at its turn cap without a verdict; this stale changes-requested verdict is dismissed so the branch rule reads the current state.