Repository navigation
Investigation requests: a viewer panel for open, claimed and reported requests (#5904) - #5907
Conversation
| // The journal keeps the read's order (newest first) and the claim time comes from its claimed event. | ||
| var journal = picked.GetProperty("journalRows").EnumerateArray().Select(Strings).ToArray(); | ||
| Assert.Equal(new[] { "reported", "claimed", "opened" }, journal.Select(r => r[1])); | ||
| Assert.Matches("claimed .*6:05", text); |
There was a problem hiding this comment.
This assertion is pinned to your machine's timezone, not to behaviour.
The claimed event in the harness is 2026-10-14T10:05:00Z (investigations-harness.mjs, the journal fixture). The page renders it through localTime → util.js:504, d.toLocaleString(), which formats in the Node process's local zone. Neither Run() here nor the harness sets TZ.
- On your rig (US Eastern, UTC−4 in October)
10:05Zrenders6:05:00 AM, soclaimed .*6:05matches. - On a GitHub-hosted runner, which is UTC, it renders
10/14/2026, 10:05:00 AM. There is no6:05anywhere in the pane text — the journal grid'sformat: "time"cells are localized the same way, the brief carries no times, and the evidence ishead blocker 412 s— soRegex.IsMatchfails andPickingARequest_ReadsThatOneRequest_AndDrawsItsReportBriefAndJournal_AsTextgoes red.
.github/workflows/build.yml sets no runner timezone (the only log_timezone lines are PostgreSQL server settings), so this is a hard red on CI rather than a flake. The surrounding assertions in this same fact are zone-independent by construction ("Verdict: needs_person", "agent-a", the createdImgOrScript count, the journal's event order), which is the shape to follow.
The behaviour actually worth pinning here — "the claim time comes from the claimed event, not from the row" — is already pinned zone-independently by out.claimedAt/out.claimedAtNever, asserted at ARequestWithNoReport_SaysSo_... (Assert.Equal("2026-10-14T10:05:00Z", run.GetProperty("claimedAt").GetString())). Either drop this line as redundant, or have the harness emit the rendered fact string and compare it against localTime applied to the same instant, so the test states the mapping rather than one zone's output of it.
| store and masks at the level the MCP investigation tools serve now (_mcpDataDetail), read when each request is picked. */ | ||
| private void InvestigationsButton_Click(object sender, RoutedEventArgs e) | ||
| { | ||
| var reader = new PerformanceMonitorLite.Investigations.InvestigationPanelReader(InvestigationStore, () => _mcpDataDetail); |
There was a problem hiding this comment.
The two halves of this PR apply opposite masking rules to the same operator-facing surface, and Lite's half makes the panel's readability depend on whether the MCP host happens to be running.
The PR body's "## Masking" section states the rule: "The web viewer shows operators the full report and brief, like the WPF viewer … mcp.dataDetail masks agents over MCP only, so no web read is masked." That is what the Darling half does — DarlingWebEndpoints.cs:6161 hard-codes McpDataDetail.Full, and the Darling Viewer's existing InvestigationsWindow (Darling/PerformanceMonitor.Darling.Viewer/InvestigationsWindow.xaml.cs, ViewerDataService.Investigations.cs) contains no masker at all. The new page's own header comment says it too (pages/investigations.js:14).
Lite's half does the opposite. _mcpDataDetail is the MCP host's agent-facing level: it is Full at construction, set to mcpSettings.DataDetail when the host starts (MainWindow.xaml.cs:729) and reset to Full when it stops (:752). Every other consumer of that field is agent-facing — it is handed to EmailAlertService for the webhook token's brief (:168), i.e. the brief an agent fetches.
Concretely, for an operator who has hardened mcp_data_detail to metadataOnly — the reason the setting exists:
- Start the MCP host. Open Investigations, pick a reported request.
InvestigationToolCore.ReportsAnswerrunsMcpDataDetailMasker.ForAnswer(MetadataOnly);InvestigationFieldMasking.Maskreplaces the whole report node with{"withheld": …}(InvestigationFieldMasking.cs:43).InvestigationPanelReader.Detailthen yieldsSummary = "(withheld) …"and — because onlysummaryandreasongo throughWithheld()—VerdictandRoot causecome back as empty strings (InvestigationPanelReader.cs:1118-1119,Text(node["verdict"])/Text(node["root_cause"])on a node that has neither key). The operator's own desktop app will not show them their own agent's findings.- Stop the MCP host. The same request now renders in full, because the field flipped back to
Full.
So the level is governed by an unrelated service's lifecycle, and a Darling operator in the identical configuration sees everything. InvestigationPanelReaderTests.ABriefStoredFullIsServedMaskedAtTheLevelTheToolsServeAndStaysMasked pins this behaviour, so it is intentional — but it pins the mechanism, not the question of whether the local operator is an agent. Per CONTRIBUTING's two-store rule and the PR's own stated rule, this panel should read at Full like its Darling twins; if the masking is genuinely wanted, it needs its own setting rather than the MCP host's, and the Verdict/Root cause blanking needs the Withheld() treatment Summary already gets.
| .filter(Boolean).join(" · "); | ||
| const parts = [el("div", { class: "muted", text: head })]; | ||
| if (rep.verdict != null) parts.push(el("div", {}, [el("strong", { text: "Verdict: " }), el("span", { text: valueText(rep.verdict) })])); | ||
| if (rep.summary != null) parts.push(el("div", {}, [el("strong", { text: "Summary: " }), el("pre", { class: "inv-text", text: valueText(rep.summary) })])); |
There was a problem hiding this comment.
Every inv-* class this page invents exists in no stylesheet, and the one on these <pre>s is load-bearing.
grep -rn "inv-" Darling/PerformanceMonitor.Darling.Service/wwwroot/css/ returns nothing: inv-text, inv-brief, inv-report, inv-section, inv-detail (:203) and inv-detail-head (:153) are all undefined across app.css, theme.css, editor.css and time-range.css.
For most of them that is only missing chrome. For inv-text it is a layout defect, because a bare <pre> is white-space: pre. The page this one is modelled on spells the same thing with a class that is defined — job-history.js:366 uses jh-detail-message, and app.css:1325 gives it exactly what a <pre> holding agent free text needs:
.jh-detail-message { margin: 8px 0 0; max-height: 24rem; overflow: auto; white-space: pre-wrap; overflow-wrap: anywhere; }summary is capped at InvestigationToolCore.MaxSummaryChars = 4000 and root_cause at MaxRootCauseChars = 8000, and neither cap requires a newline — an agent that writes one paragraph produces a single 4,000-character line. With white-space: pre that line does not wrap, so the <pre> overflows and the page gets a horizontal scrollbar; with no max-height/overflow the stored brief at :169 pushes the Journal section off-screen instead of scrolling in place.
Two related misses from the same copy: inv-detail lacks the .jh-detail pane's border/background and its app.css:1322 :empty { display: none; }, so after Close the empty slot still occupies layout; and inv-detail-head lacks .jh-detail-head's display: flex; justify-content: space-between, so the Close button does not sit at the right edge.
The Node harness cannot see any of this — it renders into a node tree with no styling, which the test class says outright ("Behavior only; the spelling of the page's source is not pinned"). Simplest fix is to reuse jh-detail/jh-detail-head/jh-detail-message (the page already reuses jh-detail-close at :155), or to add the inv-* rules to app.css beside them.
| <DataGridTextColumn Header="State" Binding="{Binding State}" Width="80"/> | ||
| <DataGridTextColumn Header="Server" Binding="{Binding ServerName}" Width="160"/> | ||
| <DataGridTextColumn Header="Metric" Binding="{Binding FirstMetric}" Width="180"/> | ||
| <DataGridTextColumn Header="Family" Binding="{Binding Family}" Width="130"/> | ||
| <DataGridTextColumn Header="Opened" Binding="{Binding Opened}" SortMemberPath="OpenedAtUtc" Width="140"/> | ||
| <DataGridTextColumn Header="Agent" Binding="{Binding Agent}" Width="130"/> | ||
| <DataGridTextColumn Header="Lease until" Binding="{Binding LeaseUntil}" SortMemberPath="LeaseUntilUtc" Width="140"/> | ||
| <DataGridTextColumn Header="Reported" Binding="{Binding Reported}" SortMemberPath="ReportedAtUtc" Width="140"/> | ||
| <DataGridTextColumn Header="Route" Binding="{Binding RouteId}" ElementStyle="{StaticResource NumericCell}" Width="60"/> |
There was a problem hiding this comment.
The alert count is missing, and the comment eight lines up says these columns match the web page's.
:11 claims "The same columns and grouping as the Investigations page in the Darling web UI." The web page's columns are REQUEST_COLUMNS = READ_FIELDS.list_investigation_requests.table.columns (read-fields.js:783-794), ten of them, and the PR body's web bullet lists them including alert count. This grid has nine: alert_count is the one that is gone.
It is not a cosmetic omission — it is how many alerts joined the request, which is what distinguishes a one-off from a server that has fired the same family eleven times in the grouping window. The list tool's own guide text names it as a row field ("Each row: id, server, family, first metric, alert count, state, lease and expiry", DarlingMcpInvestigationTools.cs:73), the web page shows it, and the Darling Viewer's existing Investigations window shows it too (Darling/PerformanceMonitor.Darling.Viewer/InvestigationsWindow.xaml:70, Header="Alerts") — so Lite is the only one of the three surfaces without it.
The reader has the value but does not surface it: InvestigationPanelRequest exposes ClaimCount (InvestigationPanelReader.cs:33), which no column in this file binds, and exposes no AlertCount at all, while Row.AlertCount is right there on the record. Adding public int AlertCount => Row.AlertCount; and an Alerts column (and dropping or binding ClaimCount) closes both halves.
Either way the comment at :11 needs to stop asserting parity it does not have — the order also differs (the web leads with Opened, this leads with State), which is fine on its own but makes the sentence false twice over.
| export const JOURNAL_COLUMNS = [...READ_FIELDS.get_investigation_reports.table.columns, { key: "detail_text", label: "Detail", wrap: true }]; | ||
|
|
||
| /* The rows of a reply under `key`; [] for anything else. */ | ||
| function rowsUnder(data, key) { |
There was a problem hiding this comment.
Non-blocking: rowsUnder is a copy of alerts.js:730, minus its "or the reply itself when it is an array" branch.
The traps file's line on this is "Two copies of one definition pinned EQUAL to each other fail only after BOTH have drifted (#3684). Alias, do not copy." These two are not even pinned equal, so a fix to one (the array branch is exactly the kind of thing that gets added) silently leaves the other behind. alerts.js already exports investigationRows, so exporting rowsUnder beside it and importing it here costs one line.
The same applies one level up, and the PR body already raises it under "Existing panel left in place": the collapsed disclosure at alerts.js:786-820 and this page are now two Investigations surfaces over the same two reads, with different columns (INVESTIGATION_COLUMNS carries verdict/summary and a per-row journal disclosure; REQUEST_COLUMNS carries lease_until/reported_at/route_id), different journal rendering (journalLine joined with · vs a VIZ.table) and different truncation copy. Since you asked: yes, I'd replace the disclosure with a link to #/investigations — but that is your call and it is disclosed, so it is not what the verdict rests on.
|
Review posted inline. Four findings; the verdict follows in a formal review. |
There was a problem hiding this comment.
Verdict: CHANGES REQUESTED
Four findings, inline. The one that stops CI is a timezone-pinned assertion in InvestigationsPageTests; the one that matters most is that the two halves apply opposite masking rules to the same operator-facing surface.
Claims
- VERIFIED — New page at
#/investigations, nav under Alert History, routed and rendered —index.html:39,app.js:48,:91,:186. - VERIFIED — Lists over
list_investigation_requestsat limit 100 in the MCP list order with those ten columns, State and Family sent to the read so it filters above its LIMIT —investigations.js:56,:103,read-fields.js:783-794,DarlingMcpInvestigationTools.cs:108. - VERIFIED — A picked row reads one request by
request_idand draws reports, stored brief and journal in the read's order, claim time from the journal'sclaimedevent; the unreviewed/self-attributed note is on the reports —investigations.js:75,:142,:160,:181. - VERIFIED — Empty, error, list-truncated and journal-truncated lines; no write action (only
readToolwith the two read names) —:171,:186,:223,:226,:231,:239. - VERIFIED — The poll rebuild keeps the filters and the open request — module-scope
choices(:104);app.js:479does not list the route as no-poll. - VERIFIED — Text only — every value takes
el()'stext:path or apre, andutil.js:36throws on anhtmlprop; pinned bycreatedImgOrScript == 0. - VERIFIED — The web report read passes
McpDataDetail.Full, so no web read is masked —DarlingWebEndpoints.cs:6161; the Darling Viewer's window holds no masker either (ViewerDataService.Investigations.csnamesMcpDataDetailnowhere). - VERIFIED — The
alerts.jsdisclosure is untouched (absent from the diff, still atalerts.js:709-820), and the nav/router pins hold with no new ones:job-historyis still last, and bothAdminPageTestspins scan spans the new lines sit above. - VERIFIED — The catalog scan looked only in the
Mcpfolder and these reads build their payload inInvestigationToolCore.cs—PayloadSourceFor(AlertNotebookTableCatalogTests.cs:284-312); every catalog key resolves there (ReportsAnswer:321-332,RequestPayload:336-354). - VERIFIED — Lite reuses the store's list at limit + 1, already ordered
opened_at DESC, request_id, and reads one request on its own connection with no write lock —InvestigationPanelReader.cs:69-76,ListSql:167,ReadReportsAsync:661-667(passingutcNowassinceUtcis harmless;ReportsRequestsSql:223gates that predicate on a non-null request id). - VERIFIED — The pane comes from the same
ReportsAnswerthe tool serves, times and events row for row —InvestigationPanelReader.cs:88-104; the positional node mapping is sound becauseReportsAnswerappends one node per row in order (InvestigationToolCore.cs:266,:304). - VERIFIED — Lite's nine list columns and its UTC
SortMemberPaths —InvestigationsWindow.xaml:51-59,:115. The body's list is accurate; the comment at:11claiming "the same columns" as the web page is not. - VERIFIED — Lite masks at
MainWindow._mcpDataDetail, the MCP host's setting,Fullwhile it is not running —MainWindow.xaml.cs:125,:729,:752,:1789. Finding 2 rests on this. - VERIFIED —
investigation_reports.request_idis UNIQUE, so the two-report case must be tested at the mapping level —Lite/Database/Schema.cs:374. - UNVERIFIED — The gate and class-filter counts (4181 / 1147 / 845 / 950, all
Failed: 0) — rig results; finding 1 says the UTC outcome differs for at least one class. - UNVERIFIED —
InvestigationToolsLiveTests.TheWebReads_...passes — needsDARLING_TEST_PGand a live store. It compiles against real symbols; itsreports/briefcomparison is near-tautological (dispatch and tool called with identical arguments), so what it pins is that the dispatch forwardsFullandrequest_id. - UNVERIFIED — "XAML compiles", window never opened by hand — needs a Windows build and a run.
NumericCellat:59does resolve asProcedureHistoryWindow.xamlresolves it.
Riskiest lines
Darling/Darling.Tests/InvestigationsPageTests.cs:95—Assert.Matches("claimed .*6:05", text)compares againstlocalTime'stoLocaleString()of2026-10-14T10:05:00Z, encoding a UTC-4 rendering; a UTC runner renders10:05and no6:05exists in the pane. It IS the pin, pinned to the wrong thing.Lite/MainWindow.xaml.cs:1789— the_mcpDataDetailclosure makes an operator panel's readability a function of the MCP host's lifecycle and its agent-facing hardening level — pinned byABriefStoredFullIsServedMaskedAtTheLevelTheToolsServeAndStaysMasked, which pins the mechanism, not whether the local operator is an agent.wwwroot/js/pages/investigations.js:132— apreon classinv-text, defined in no stylesheet, so a 4,000-character newline-free summary renders withoutwhite-space: pre-wrap— UNPINNED; the harness renders into an unstyled node tree.
Traps near this diff
- "Truncation inferred from a full page ... Fetch
limit + 1; the flag isfetched.Count > limit." — does not apply:InvestigationPanelReader.cs:71-73fetchesListLimit + 1and flagsrows.Count > ListLimit; the page readstruncatedoff the tool, same shape atDarlingMcpInvestigationTools.cs:108-110. - "A
total_*assigned from a.Countis a count of the page unless the set is provably whole." — does not apply:investigations.js:240says "N requests shown" and the notice says "the newest N requests; older ones are not listed". - "Two copies of one definition pinned EQUAL to each other fail only after BOTH have drifted (#3684). Alias, do not copy." — applies:
rowsUnderatinvestigations.js:63is a copy ofalerts.js:730with a branch dropped, and the surface itself duplicates thealerts.js:786disclosure. Non-blocking; the body raises the second half and asks for a decision. - "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." — applies twice: the masking policy (Lite masks; the Darling page and Viewer do not) and the column set (
alert_countis atread-fields.js:789and atPerformanceMonitor.Darling.Viewer/InvestigationsWindow.xaml:70, on neither Lite list). - "Ruling (#5833): tests are load-bearing ... never answer a trap with a test that reads source text or asserts how code is spelled." — does not apply: the three new test files are behaviour tests, and the only source-text read added is one entry in the pre-existing census at
AlertNotebookTableCatalogTests.cs:259-265. - "A JSON number above 2^53 is rounded by every JavaScript client." — does not apply:
request_idis a GUID serialized as a string (InvestigationToolCore.cs:289,:338); the other ids are small ints.
Divergence (blind read vs PR body)
- The masking rule is stated for one half and inverted in the other. "## Masking" states the product rule —
mcp.dataDetailmasks agents over MCP only, so no web read is masked — and the Darling half implements it. The Lite half routes the same operator-facing panel through the MCP host'smcp_data_detail, described accurately but never reconciled with that rule. The consequence the body omits: atmetadataOnlythe operator's own panel shows(withheld)for the summary and empty Verdict and Root cause, because onlysummaryandreasonpass throughWithheld()(InvestigationPanelReader.cs:1118-1119against:1146-1147), and it becomes readable again when the MCP host stops. - "Verified ... Failed: 0" against a timezone-pinned assertion. The body reports both suites green on the author's rig and leaves the rest to CI.
InvestigationsPageTests.cs:95can only pass at UTC-4 or UTC+8, and CI runs at UTC with noTZset anywhere inbuild.yml. - "The same columns" is asserted in the code, not the body. The body's two column lists differ honestly, so nothing in the body is refuted — but
InvestigationsWindow.xaml:11claims parity with the web page's columns, and the missing one is the column both other surfaces carry.
Reviewer checklist
- Run the harness under a UTC clock and then
InvestigationsPageTestson a UTC machine, and confirm the6:05assertion fails as read — Node could not be executed here, so finding 1 is confirmed by readinglocalTime(util.js:502-505) and the fixture instant, not by a run. - Load
#/investigationsagainst a store holding a report whose summary is one long newline-free paragraph: does theprewrap or push a horizontal scrollbar, and does the empty detail slot still take layout after Close? - On Lite, start the MCP host at
mcp_data_detail = metadataOnly, open Investigations, pick a reported request and see what the operator can read; then stop the host and reopen it. - Build Lite on Windows and open the window by hand — the report template, the
NumericCelllookup and the row split are unexercised. - Decide the duplicate-surface question the body asks: keep the
alerts.jsdisclosure, or replace it with a link.
The Lite panel reads at Full like the web page and the Viewer (mcp.dataDetail masks agents over MCP only), gains the Alerts column, the web page reuses the jh-detail classes and shares rowsUnder from util.js, and the zone-pinned 6:05 assertion is dropped.
|
Follow-up review (range Verified independently, before reading the PR body:
All of this matches the PR body's "Round 1 review" section point for point — no divergence found. |
There was a problem hiding this comment.
Verdict: LGTM
Claims
- VERIFIED — "The panel no longer reads
_mcpDataDetail;InvestigationPanelReaderhas no data-detail parameter and builds the pane atFull" —Lite/Investigations/InvestigationPanelReader.cs:77,99and the single caller update atLite/MainWindow.xaml.cs:1789 - VERIFIED — "Added the
Alertscolumn (InvestigationPanelRequest.AlertCount)... the grid now has the web page's ten columns" —InvestigationsWindow.xaml:52-61(10DataGridTextColumns), matching the web harness's 10-header string inInvestigationsPageTests.cs:66 - VERIFIED — "
ClaimCountstays on the record, unbound (harmless, no column wants it)" — grep confirms noClaimCountbinding inInvestigationsWindow.xaml - VERIFIED — "I reused the defined classes: the detail slot is
jh-detail... the harness now finds the slot byjh-detail" —investigations.js:148,150,198,app.css:1326-1328,investigations-harness.mjs:166 - VERIFIED — "One definition now:
rowsUnderis exported fromutil.js... bothalerts.jsandinvestigations.jsimport it" —util.js:502,alerts.js:27,investigations.js:29; confirmed the merged superset behavior is dead code forinvestigations.js's callers since both MCP reads it consumes always reply with an object, never a bare array - VERIFIED — "Dropped [the
claimed .*6:05assertion]; the claim-time mapping stays pinned zone-independently byclaimedAt" —InvestigationsPageTests.cs:92(comment) and:130-131(theclaimedAtassertions) - UNVERIFIED — the cited local test-run counts (
Total: 154,Total: 6,Total: 1045, etc.) and the laptop gate runs — these are live build/test executions a read-only tree can't reproduce
Riskiest lines
Lite/Investigations/InvestigationPanelReader.cs:99— panel now always servesFullregardless ofmcp.dataDetail, a deliberate parity change (Darling's web panel already did this viaDarlingWebEndpoints.cs:6161, unchanged in this range) — pinned byThePanelShowsSummaryVerdictRootCauseAndBriefInFull_WhileTheMcpHostRunsAtMetadataOnly(Lite.Tests/InvestigationPanelReaderTests.cs:111)Darling/PerformanceMonitor.Darling.Service/wwwroot/js/pages/investigations.js:198— detail slot renamed tojh-detail, which carries.jh-detail:empty { display: none; }from the Job History rules — pinned byinvestigations-harness.mjs:166'sslotText()lookup andInvestigationsPageTests.TheRebuild_KeepsTheOpenRequest_AndCloseEmptiesThePaneDarling/PerformanceMonitor.Darling.Service/wwwroot/js/util.js:502— sharedrowsUndernow used by bothalerts.jsandinvestigations.js— UNPINNED directly (no test imports/callsrowsUnderby name), but exercised transitively through both pages' existing render tests
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, not a nit" — does not apply: this commit is the fix that brings Lite's
InvestigationPanelReaderinto parity with Darling's pre-existingMcpDataDetail.Fullpanel read (DarlingWebEndpoints.cs:6161, unchanged in this range), not a new one-sided change - "Two copies of one definition pinned EQUAL to each other fail only after BOTH have drifted" — does not apply:
rowsUnderhad two copies with slightly different behavior (bare-array branch), and this commit collapses them to one definition inutil.jsrather than pinning two copies equal
Divergence (blind read vs PR body)
None — the body's "Round 1 review" section matches the diff point for point on every claim graded above.
Reviewer checklist
- Open the Lite
InvestigationsWindowby hand once (never opened by hand per the PR body) to confirm the newAlertscolumn renders and theFull-detail report/brief/journal actually display with real data
Claude posting for Erik Darling. Round 1's findings were fixed in the next push and round 2 answered LGTM on the current head; this stale changes-requested verdict is dismissed so the branch rule reads the current one.
Claude posting for Erik Darling.
Fixes #5904.
Both halves are on this branch: the Darling web page (sections below, unchanged) and the Lite WPF panel (
## Lite half).What changed
#/investigations, nav entry "Investigations" under Alert History). It lists investigation requests over/api/read/list_investigation_requests(limit 100, in the MCP list order,opened_at DESC, request_id): opened, state, server, metric, family, alert count, agent, lease until, reported, route. The State filter (all, open, claimed, reported, closed, expired) and the Family filter go to the read, so the read applies them before its limit. Picking a row reads that one request over/api/read/get_investigation_reports?request_id=and shows its report(s) (verdict, summary, root cause, evidence, agent, time), the stored brief as served, and the journal in the read's order (event_time DESC, journal_id DESC) with the claim time taken from the journal'sclaimedevent. Reports carry the note that they are an agent's unreviewed, self-attributed claim. Empty, error and truncation lines (list truncated, journal truncated) are shown. No write action. The 60 s poll rebuild keeps the filters and the open request.el()'s text path or a<pre>; nothing is built as HTML.list_investigation_requestsandget_investigation_reports(journal rows) entries inwwwroot/js/read-fields.js; the page reads its columns from there.Masking
One rule for both halves:
mcp.dataDetailmasks agents over MCP only; an operator's own window reads atFull. The Darling web page and the Darling Viewer show the full report, brief and journal (the webget_investigation_reportsread passesMcpDataDetail.Full, as on dev), and the Lite panel now does the same: it builds its pane fromInvestigationToolCore.ReportsAnsweratFulland takes no data-detail input at all, so the MCP host's setting and its start/stop no longer change what the operator sees. The MCP tools themselves still mask for agents at the configured level.Existing panel left in place
pages/alerts.jsalready has a collapsed read-only "Investigations" disclosure under the alert table (#5632). I left it untouched (its tests pin it); the new page is the full viewer. Say if you want the disclosure removed in favor of a link.Tests (behavior only; no new source-text pins)
InvestigationsPageTests(Node harnessinvestigations-harness.mjsruns the shipped page against a recording fetch): list columns and order, the state and family params, a picked request's report/brief/journal, markup in the data stays text (noimgorscriptelement is created), the rebuild keeps the open request, Close, no-report line, empty, error and truncation lines.InvestigationToolsLiveTests.TheWebReads_CarryTheFieldsThePageDraws_InTheMcpOrder_AndTheReportIsServedInFull(live store): both web reads carry every field the page uses, the list isopened_at DESC, the journalevent_time DESC, and the web report and brief equal the MCP tool's full-level answer.AlertNotebookTableCatalogTests: its "every catalog key is a key the read's payload builds" scan looked only in the Mcp folder; the two reads build their payload inPerformanceMonitor.Common/Investigations/InvestigationToolCore.cs, so I added that file as the source for those two reads (the rule still holds).Verification
pre-push.sh, Git Bash):guard-darling Total: 4181, Errors: 0, Failed: 0, Skipped: 0andguard-lite Total: 1147, Errors: 0, Failed: 0, Skipped: 0. The selection stage then stopped ("selection is the whole suite ... run the brief's classes; CI decides"), so no diff-selected classes ran from the script.WebReadCancellationPinTests,EffectiveReadWindowTests, the investigation tool and contract live classes:Darling.Tests Total: 845, Errors: 0, Failed: 0, Skipped: 0, Not Run: 0on my own rig (port 55904, stopped).guard-darling Total: 4181, Failed: 0,guard-lite Total: 1147, Failed: 0) before the follow-up that put the report read back toFull; after it I rebuilt and reran the class list above (Total: 845, Errors: 0, Failed: 0), not the gate. The only product files that changed are back to dev's text.playwright-cli(the page needs a running service with a store; the Node harness runs the shipped page script instead).Leftovers
## Lite half).deprecated/touched; no CHANGELOG edit.Lite half
Read-only WPF panel over the DuckDB investigation tables #5902 added. No MCP tool, no action on a request (claim, release and approve are not offered), nothing new for agents.
Lite/Investigations/InvestigationPanelReader.cs(new): the read side.ListAsynccalls the store's existingListAsync(limit 100 + 1 for the truncated flag), which already returnsopened_at DESC, request_id, so no new store read was needed.ReadDetailAsynccallsReadReportsAsyncfor one request (its own connection, no write lock) and builds the pane from the sameInvestigationToolCore.ReportsAnswertheget_investigation_reportstool serves, so the report, journal and stored brief come from the one builder the tool uses, atFull(see## Masking). Times and event names come from the read row for row. Claimed time is the newestclaimedjournal event (the list carries agent and lease end), the same split the Darling page uses.Lite/Windows/InvestigationsWindow.xaml(.cs)(new): list columns state, server, metric, family, alerts, opened, agent, lease until, reported, route (the web page's ten columns; State leads here and Opened leads there) (State and Family filters as on the Darling page, Refresh button); picking a row shows the reports (agent, time, verdict, summary, root cause, evidence), the stored brief and the journal grid. Reads run inTask.Run; a stale pick is dropped by a sequence number. Time columns sort by their UTCDateTime(GridTimeColumnSortMemberTestsrequires it).Lite/MainWindow.xaml(.cs): an "Investigations" button in the sidebar under Manage Tags opens the window modeless. The reader is built with the store only;MainWindow._mcpDataDetail(the MCP host's agent-facing level) is not consulted.Lite.Tests/InvestigationPanelReaderTests.cs(new, 6 tests, throwaway DuckDB): the list order (ties onopened_atbreak byrequest_id, equal to the store's list); the alert count each request carries; state and family filters; the summary, verdict, root cause and stored brief show in full while the tool's answer atmetadataOnlyfor the same request withholds them; a submitted report and its journal render under the request with the report text in full; a request with two reports and three journal rows renders all of them in order with the claimed time from the journal.Notes for the reviewer:
investigation_reports.request_idis UNIQUE, so the store never holds two reports for one request; the two-report case is tested at the mapping level (InvestigationPanelReader.Detailover a constructedInvestigationReportsRead), because the MCP read shape allows a list.Verified (
Lite.Tests.exe, one run with every class filter):*Census*,*Ratchet*,*Investigation*(includesDuckDbInvestigationStoreTests, the new reader tests and the MCP investigation tools classes),*GridTimeColumnSort*,*ThemeColorOverride*,*CrossAppGuard*,*TabLoads*,*McpWritePolicy*,*McpPageContract*,*AlertReadFailureSurface*,*DocCommentHygiene*,*McpPayloadContract*,*Inventory*,*AlertLegendParity*,*SettingsSaveReport*,*Wpf*,*Smoke*,*Xaml*,*Window*:Total: 950, Errors: 0, Failed: 0. BuildBuild succeeded.The first run failedGridTimeColumnSortMemberTestson the journal Time column; fixed withSortMemberPathto UTC members. Full suite not run (laptop rule; CI decides).CHANGELOG entry
SECTION: Added
ENTRY:
mcp.dataDetailmasks agents over MCP only.REF:
[Investigation requests: a viewer panel for open, claimed and reported requests (#5904) #5907]: Investigation requests: a viewer panel for open, claimed and reported requests (#5904) #5907
Round 1 review
CI run 38845482264 and the bot's review (CHANGES REQUESTED on 63fddd1). Fixed in the push after it; the last round for this PR.
Red tests
InvestigationsPageTests.PickingARequest_ReadsThatOneRequest_AndDrawsItsReportBriefAndJournal_AsText: theclaimed .*6:05line pinned the UTC-4 rendering of10:05Z. Dropped; the claim-time mapping stays pinned zone-independently byclaimedAtinARequestWithNoReport_SaysSo_AndTheEmptyErrorAndTruncationLinesAreDrawn. The class passes under the laptop's zone and withTZ=UTC.QueryStoreComposeStampWriterGuardLiveTests.AnUnrelatedLongWriter_ATempTableAndAWriteToAnotherTable_NoLongerDefersTheHour(shard 5, not in this PR's diff): not reproduced. The whole class ran three times on a throwaway UTC PostgreSQL rig and passed every time (5 of 5 tests, Skipped 0, three runs). Left to CI's rerun; no change.Findings
MainWindow.xaml.cs:1789). The panel no longer reads_mcpDataDetail;InvestigationPanelReaderhas no data-detail parameter and builds the pane atFull.InvestigationPanelReaderTests.ABriefStoredFullIsServedMaskedAtTheLevelTheToolsServeAndStaysMaskedis deleted and replaced byThePanelShowsSummaryVerdictRootCauseAndBriefInFull_WhileTheMcpHostRunsAtMetadataOnly(the same stored report throughReportsAnsweratmetadataOnlywithholds the literal; the panel shows summary, verdict, root cause and brief in full).AReportAndItsJournalComeBackUnderTheRequestWithTheReportTextInFullis the old masked-report test, flipped. The blank Verdict/Root cause atmetadataOnlythe review describes can no longer happen: nothing is masked in the panel. The## Maskingrule above states it for both halves. The window's header comments (xaml,.xaml.cs, reader, test class) are updated to say the same.InvestigationsWindow.xaml:59). Added theAlertscolumn (InvestigationPanelRequest.AlertCount, right-aligned like Route); the grid now has the web page's ten columns, and the comment at:11says so, including that State leads here and Opened leads there. Pinned byTheListCarriesTheAlertCountOfEachRequest(a joined request shows 2, a single one 1).ClaimCountstays on the record, unbound (harmless, no column wants it).investigations.js:132). I reused the defined classes: the detail slot isjh-detail(border, background,:empty { display: none }), its head isjh-detail-head(flex, Close at the right edge), and the summary, root cause, evidence and stored brief<pre>s arejh-detail-message(pre-wrap, max-height, scroll). The two remaining hooks,inv-sectionandinv-report, now have small spacing rules inapp.cssbeside thejh-block. The Node harness cannot pin CSS, so this is by construction, not by a test; the harness now finds the slot byjh-detail.rowsUndercopy (investigations.js:63). One definition now:rowsUnderis exported fromwwwroot/js/util.js(the array-reply branch from thealerts.jsversion kept) and bothalerts.jsandinvestigations.jsimport it; neither keeps a copy. The existing AlertHistory and Investigations page classes pass.alerts.jsdisclosure (non-blocking in the review, and the question this body raised): kept as is. Replacing it with a link to#/investigationsis still Erik's call; it needs its tests rewritten and is not part of this fix round.limit + 1,total_*from a.Count, 2^53 ids, Tests that assert spelling, not behavior: cap them, stop the review asking for pins, delete the shape ones #5833 source-text pins): no change; no source-text test was added this round.NumericCellresolves as inProcedureHistoryWindow.xaml); no browser check of the page's wrapping/empty-pane CSS (reuse of the existing classes is the answer). The earlier gate counts are superseded by the runs below.Verified this round
Lite.Tests.exe, buildsBuild succeeded.(0 warnings):*Investigation*,*Census*,*Ratchet*:Total: 154, Failed: 0;*InvestigationPanelReader*alone:Total: 6, Failed: 0;*Census*+*Ratchet*alone:Total: 32, Failed: 0.Darling.Tests.exe(buildBuild succeeded., 0 warnings):*InvestigationsPageTests*Total: 5, Failed: 0(also withTZ=UTC);*PageTests*,*Census*,*Ratchet*,*InvestigationsPage*,*WebAssets*,*Alerts*,*Javascript*:Total: 1045, Failed: 0;*AlertHistory*,*AlertSampleAge*,*WebClickthrough*,*Alert*:Total: 2661, Failed: 0, Skipped: 150;QueryStoreComposeStampWriterGuardLiveTeststhree runs, 5 of 5 each.