Skip to content

Commit 2aacd26

Browse files
committed
fix(webapp,dashboard-agent): keep investigation cards, and stop a broken request reading as an answer
An investigation card could vanish from a reply that contained step separators: the winning revision was keyed by its position in the untouched reply, while rendering walked the stripped parts. Both now index the same stripped list. A host-written `data-view` card counted towards winning a revision but was never drawn as a card, so it could suppress the tool-rendered card it competed with into nothing. Both carriers now render. `apiGet` left `fetch` and `res.json()` unguarded, so a connection reset threw out of the tool, and a transport failure that did land could read as a definite 404. It now returns a transport failure the way the sibling paths do. Also: guard the card's Hypotheses section on length, as Evidence already is, and move the reports auth resource out of the serializer so rendering a report doesn't pull the route builder in.
1 parent d40e9fe commit 2aacd26

13 files changed

Lines changed: 408 additions & 59 deletions

apps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsx

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,10 @@ function hostViewBlocks(part: UIMessage["parts"][number]): unknown[] | null {
2929
return Array.isArray(p.data?.blocks) ? p.data!.blocks! : null;
3030
}
3131

32-
function investigationBlocksFor(part: UIMessage["parts"][number]): unknown[] | null {
32+
// Both carriers render as cards, so whichever one wins a revision is one the panel
33+
// can actually draw — a host-written card can never suppress a tool-rendered one
34+
// into nothing.
35+
function viewBlocksFor(part: UIMessage["parts"][number]): unknown[] | null {
3336
return viewSpecFor(part)?.blocks ?? hostViewBlocks(part);
3437
}
3538

@@ -41,12 +44,15 @@ function investigationRef(block: unknown): InvestigationRef | null {
4144
return { id: b.id, revision: typeof b.revision === "number" ? b.revision : 0 };
4245
}
4346

44-
/** Per investigation id, the one `messageId:partIndex` allowed to render: highest revision. */
47+
/**
48+
* Per investigation id, the one `messageId:partIndex` allowed to render: highest revision.
49+
* Indexed over the same stripped parts the renderer walks, so the two agree on what part 0 is.
50+
*/
4551
export function winningInvestigationOccurrences(messages: UIMessage[]): Map<string, string> {
4652
const best = new Map<string, { revision: number; occurrence: string }>();
47-
for (const message of messages) {
53+
for (const message of messages.map(stripStepParts)) {
4854
(message.parts ?? []).forEach((part, partIndex) => {
49-
for (const block of investigationBlocksFor(part) ?? []) {
55+
for (const block of viewBlocksFor(part) ?? []) {
5056
const ref = investigationRef(block);
5157
if (!ref) continue;
5258
const current = best.get(ref.id);
@@ -71,37 +77,41 @@ function withoutSupersededInvestigations(
7177
});
7278
}
7379

74-
// Renders one message. Assistant messages that include a completed render_view
75-
// part get the catalog cards (plus the gather tool rows / lead-in text for
76-
// transparency); everything else uses the shared MessageBubble unchanged, so
77-
// its streaming memoization is preserved for the common case.
78-
const DashboardAgentMessageBubble = memo(function DashboardAgentMessageBubble({
80+
// Renders one message. Assistant messages carrying a view spec get the catalog
81+
// cards (plus the gather tool rows / lead-in text for transparency); everything
82+
// else uses the shared MessageBubble unchanged, so its streaming memoization is
83+
// preserved for the common case.
84+
export function DashboardAgentMessageBubble({
7985
message,
8086
investigationWinners,
8187
}: {
8288
message: UIMessage;
8389
/** See {@link winningInvestigationOccurrences}. */
8490
investigationWinners?: Map<string, string>;
8591
}) {
86-
if (message.role !== "assistant" || !message.parts?.some((p) => viewSpecFor(p))) {
92+
if (message.role !== "assistant" || !message.parts?.some((p) => viewBlocksFor(p))) {
8793
return <MessageBubble message={message} />;
8894
}
8995
return (
9096
<div className="space-y-2">
9197
{message.parts.map((part, i) => {
92-
const spec = viewSpecFor(part);
98+
const spec = viewBlocksFor(part);
9399
if (!spec) return renderPart(part, i);
94100
const blocks = withoutSupersededInvestigations(
95-
spec.blocks,
101+
spec,
96102
`${message.id}:${i}`,
97103
investigationWinners
98104
);
99105
if (blocks.length === 0) return null;
106+
// No `onIntent`: nothing here can act on one yet, so the cards drop their
107+
// action rows rather than offer buttons that would do nothing.
100108
return <ViewBlocks key={i} blocks={blocks as never} />;
101109
})}
102110
</div>
103111
);
104-
});
112+
}
113+
114+
const MemoizedMessageBubble = memo(DashboardAgentMessageBubble);
105115

106116
// Renders the conversation with the shared agent message renderer — the same
107117
// MessageBubble the run inspector and playground use, so agent output looks
@@ -123,7 +133,7 @@ export function DashboardAgentMessages({
123133
<div className="flex-1 overflow-y-auto scrollbar-thin scrollbar-track-transparent scrollbar-thumb-surface-control">
124134
<div ref={rootRef} className="space-y-4 p-4">
125135
{messages.map((message) => (
126-
<DashboardAgentMessageBubble
136+
<MemoizedMessageBubble
127137
key={message.id}
128138
message={stripStepParts(message)}
129139
investigationWinners={investigationWinners}
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
import type { InvestigationBlock } from "@internal/dashboard-agent-contracts";
2+
import { createElement } from "react";
3+
import { renderToStaticMarkup } from "react-dom/server";
4+
import { describe, expect, it } from "vitest";
5+
import { OperatingSystemContextProvider } from "~/components/primitives/OperatingSystemProvider";
6+
import { ShortcutsProvider } from "~/components/primitives/ShortcutsProvider";
7+
import { InvestigationCard } from "./InvestigationCard";
8+
9+
/**
10+
* What the card actually puts on the page, rather than what its source says. Static markup,
11+
* so it proves the rendered output and nothing about interaction: a click is not exercised.
12+
*/
13+
14+
const HYPOTHESIS = {
15+
id: "h1",
16+
statement: "The receipt builder is handed a null order id.",
17+
verdict: "validated" as const,
18+
evidence: [],
19+
};
20+
21+
function block(overrides: {
22+
hypotheses?: InvestigationBlock["investigation"]["hypotheses"];
23+
actions?: NonNullable<InvestigationBlock["capabilities"]>["actions"];
24+
}): InvestigationBlock {
25+
return {
26+
type: "investigation",
27+
id: "inv_1",
28+
revision: 0,
29+
version: 1,
30+
investigation: {
31+
outcome: "concluded",
32+
severity: "crit",
33+
confidence: "high",
34+
title: "send-order-receipt fails on every retry",
35+
headline: "Every attempt dies on a null order id.",
36+
remediation: "Guard the receipt builder against a missing order.",
37+
hypotheses: overrides.hypotheses ?? [],
38+
evidence: [],
39+
},
40+
...(overrides.actions
41+
? { capabilities: { version: 1, actions: overrides.actions } }
42+
: undefined),
43+
} as InvestigationBlock;
44+
}
45+
46+
// The Button primitive reads both of these for its shortcut hints.
47+
function markup(props: Parameters<typeof InvestigationCard>[0]) {
48+
return renderToStaticMarkup(
49+
createElement(
50+
OperatingSystemContextProvider,
51+
{ platform: "mac" },
52+
createElement(ShortcutsProvider, null, createElement(InvestigationCard, props))
53+
)
54+
);
55+
}
56+
57+
describe("the card's sections appear only when they have something in them", () => {
58+
it("leaves out an empty Hypotheses heading, the way Evidence already does", () => {
59+
const html = markup({ block: block({}), defaultExpanded: true });
60+
expect(html).not.toContain("Hypotheses");
61+
expect(html).not.toContain("Evidence");
62+
});
63+
64+
it("shows the heading once there is a hypothesis under it", () => {
65+
const html = markup({ block: block({ hypotheses: [HYPOTHESIS] }), defaultExpanded: true });
66+
expect(html).toContain("Hypotheses");
67+
expect(html).toContain("The receipt builder is handed a null order id.");
68+
});
69+
});
70+
71+
describe("action buttons need a host to hand the intent to", () => {
72+
const actions = [
73+
{
74+
kind: "ask_follow_up" as const,
75+
label: "Keep digging",
76+
intent: { kind: "ask" as const, prompt: "Keep digging into the receipt failures." },
77+
},
78+
];
79+
80+
it("renders no button when the host passes no onIntent, rather than a dead one", () => {
81+
const html = markup({ block: block({ actions }), defaultExpanded: true });
82+
expect(html).not.toContain("Keep digging");
83+
});
84+
85+
it("renders the same action once a host can act on it", () => {
86+
const html = markup({ block: block({ actions }), defaultExpanded: true, onIntent: () => {} });
87+
expect(html).toContain("Keep digging");
88+
});
89+
});

apps/webapp/app/components/dashboard-agent/InvestigationCard.tsx

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -215,17 +215,19 @@ export function InvestigationCard({
215215

216216
{expanded ? (
217217
<div className="space-y-5 pt-1">
218-
<Section title="Hypotheses">
219-
<ul className="space-y-5">
220-
{investigation.hypotheses.map((hypothesis) => (
221-
<HypothesisRow
222-
key={hypothesis.id}
223-
hypothesis={hypothesis}
224-
resolveUri={resolveUri}
225-
/>
226-
))}
227-
</ul>
228-
</Section>
218+
{investigation.hypotheses.length > 0 ? (
219+
<Section title="Hypotheses">
220+
<ul className="space-y-5">
221+
{investigation.hypotheses.map((hypothesis) => (
222+
<HypothesisRow
223+
key={hypothesis.id}
224+
hypothesis={hypothesis}
225+
resolveUri={resolveUri}
226+
/>
227+
))}
228+
</ul>
229+
</Section>
230+
) : null}
229231

230232
{investigation.evidence.length > 0 ? (
231233
<Section title="Evidence">

apps/webapp/app/presenters/v3/reports/reportsApi.server.ts

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,8 @@
22
import { json } from "@remix-run/server-runtime";
33
import { ReportFormatSchema, ReportPeriodSchema } from "@trigger.dev/core/v3/schemas";
44
import { z } from "zod";
5-
import { reportQueryTables } from "~/presenters/v3/reports/report-registry";
65
import { renderReportAnsi, renderReportMarkdown } from "~/presenters/v3/reports/renderMarkdown";
76
import { type ReportViewModel } from "~/presenters/v3/reports/report-view-model";
8-
import { everyResource } from "~/services/routeBuilders/apiBuilder.server";
97

108
export const ReportParamsSchema = z.object({
119
key: z.string(),
@@ -36,11 +34,3 @@ export function reportResponse(vm: ReportViewModel, format: ReportFormatParam):
3634
});
3735
}
3836
}
39-
40-
/**
41-
* Per-table, not the permissive `{ type: "query", id: "all" }`: a JWT must be scoped to every table
42-
* the report reads, so a partially scoped token can't reach the others.
43-
*/
44-
export function reportAuthResource(key: string) {
45-
return everyResource(reportQueryTables(key).map((id) => ({ type: "query", id })));
46-
}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
// Apart from `reportsApi.server.ts` so that rendering a report doesn't drag the route
2+
// builder — and `env.server` behind it — into everything that serializes one.
3+
import { reportQueryTables } from "~/presenters/v3/reports/report-registry";
4+
import { everyResource } from "~/services/routeBuilders/apiBuilder.server";
5+
6+
/**
7+
* Per-table, not the permissive `{ type: "query", id: "all" }`: a JWT must be scoped to every table
8+
* the report reads, so a partially scoped token can't reach the others.
9+
*/
10+
export function reportAuthResource(key: string) {
11+
return everyResource(reportQueryTables(key).map((id) => ({ type: "query", id })));
12+
}

apps/webapp/app/routes/api.v1.dashboard-agent.eval-policy.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@ import { authenticateUatOrApiRequest } from "~/services/uatRoutePreamble.server"
1212

1313
const QuerySchema = z.object({ organizationId: z.string().min(1) });
1414

15+
// obs-map-disable request-context -- the only call here that can throw catches its own failure
16+
// and logs it with organizationId, in dashboardAgentEvalPolicy.server.ts; the rest are early
17+
// returns, and the one failure that does reach the boundary is auth, where no tenant is known yet.
1518
export async function loader({ request }: LoaderFunctionArgs) {
1619
const authentication = await authenticateUatOrApiRequest(request);
1720
if (!authentication?.userActor) {

apps/webapp/app/routes/api.v1.reports.$key.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@ import { isReportKey, REPORT_KEYS } from "~/presenters/v3/reports/report-registr
44
import {
55
ReportParamsSchema,
66
ReportSearchParamsSchema,
7-
reportAuthResource,
87
reportResponse,
98
} from "~/presenters/v3/reports/reportsApi.server";
9+
import { reportAuthResource } from "~/presenters/v3/reports/reportsApiAuth.server";
1010
import { logger } from "~/services/logger.server";
1111
import { createLoaderApiRoute } from "~/services/routeBuilders/apiBuilder.server";
1212

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
import type { UIMessage } from "ai";
2+
import { describe, expect, it } from "vitest";
3+
import {
4+
DashboardAgentMessageBubble,
5+
winningInvestigationOccurrences,
6+
} from "~/components/dashboard-agent/DashboardAgentMessages";
7+
8+
/**
9+
* The two halves of card rendering that have to agree: which occurrence of an investigation
10+
* wins, and which part index the renderer is standing on when it asks. Structural — these call
11+
* the component as a function and read the returned element tree, so they prove what is handed
12+
* to `ViewBlocks`, not what the browser paints.
13+
*/
14+
15+
function investigationBlock(id: string, revision: number) {
16+
return {
17+
type: "investigation",
18+
id,
19+
revision,
20+
version: 1,
21+
investigation: {
22+
outcome: "concluded",
23+
severity: "warn",
24+
confidence: "medium",
25+
title: id,
26+
headline: `${id} — what we have so far.`,
27+
hypotheses: [],
28+
evidence: [],
29+
},
30+
};
31+
}
32+
33+
const stepStart = { type: "step-start" } as never;
34+
35+
function toolCard(id: string, revision: number) {
36+
return {
37+
type: "tool-render_view",
38+
toolCallId: `call-${id}-${revision}`,
39+
state: "output-available",
40+
output: { blocks: [investigationBlock(id, revision)] },
41+
} as never;
42+
}
43+
44+
function hostCard(id: string, revision: number) {
45+
return { type: "data-view", data: { blocks: [investigationBlock(id, revision)] } } as never;
46+
}
47+
48+
function message(id: string, parts: UIMessage["parts"]): UIMessage {
49+
return { id, role: "assistant", parts };
50+
}
51+
52+
/** Every `ViewBlocks` element in the rendered tree, with the blocks it was handed. */
53+
function renderedBlocks(node: unknown): unknown[][] {
54+
const found: unknown[][] = [];
55+
const walk = (value: unknown) => {
56+
if (Array.isArray(value)) return value.forEach(walk);
57+
const element = value as { props?: { blocks?: unknown[]; children?: unknown } } | null;
58+
if (!element || typeof element !== "object") return;
59+
if (Array.isArray(element.props?.blocks)) found.push(element.props.blocks);
60+
if (element.props?.children) walk(element.props.children);
61+
};
62+
walk(node);
63+
return found;
64+
}
65+
66+
function renderBubble(msg: UIMessage, winners?: Map<string, string>) {
67+
return renderedBlocks(
68+
DashboardAgentMessageBubble({ message: msg, investigationWinners: winners })
69+
);
70+
}
71+
72+
describe("the winner's part index and the renderer's part index are the same index", () => {
73+
it("counts the parts the renderer walks, not the ones it drops", () => {
74+
// The step separator is stripped before rendering, so the card is part 0 on screen.
75+
const withSeparator = message("msg-1", [stepStart, toolCard("inv_1", 0)]);
76+
expect(winningInvestigationOccurrences([withSeparator]).get("inv_1")).toBe("msg-1:0");
77+
});
78+
79+
it("keeps the card a reply with step separators would otherwise lose", () => {
80+
const withSeparator = message("msg-1", [stepStart, toolCard("inv_1", 0)]);
81+
const winners = winningInvestigationOccurrences([withSeparator]);
82+
83+
// What DashboardAgentMessages renders: the stripped message, against those winners.
84+
const stripped = message("msg-1", [toolCard("inv_1", 0)]);
85+
const blocks = renderBubble(stripped, winners);
86+
87+
expect(blocks).toHaveLength(1);
88+
expect(blocks[0]).toHaveLength(1);
89+
});
90+
});
91+
92+
describe("a host-written card and a tool-rendered card compete on equal terms", () => {
93+
it("renders a host-written card, so winning one cannot mean rendering nothing", () => {
94+
const blocks = renderBubble(message("msg-host", [hostCard("inv_2", 1)]));
95+
expect(blocks).toHaveLength(1);
96+
expect((blocks[0][0] as { id: string }).id).toBe("inv_2");
97+
});
98+
99+
it("drops the superseded revision and keeps the winner, whichever carrier it arrived in", () => {
100+
const older = message("msg-tool", [toolCard("inv_3", 1)]);
101+
const newer = message("msg-host", [hostCard("inv_3", 2)]);
102+
const winners = winningInvestigationOccurrences([older, newer]);
103+
104+
expect(winners.get("inv_3")).toBe("msg-host:0");
105+
expect(renderBubble(older, winners)).toHaveLength(0);
106+
expect(renderBubble(newer, winners)).toHaveLength(1);
107+
});
108+
});

0 commit comments

Comments
 (0)