Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
} from "./chatStore";
import { deriveLiveStatus, type LiveStatusModel } from "./liveStatusModel";
import { StreamingOutput } from "./StreamingOutput";
import { buildStreamTools } from "./streamState";
import { buildStreamTools, filterPendingStreamState } from "./streamState";
import type { MergedTool, StreamState } from "./types";

const shouldRenderStreamingSection = (liveStatus: LiveStatusModel): boolean =>
Expand Down Expand Up @@ -101,6 +101,7 @@ interface LiveStreamTailProps {
subagentVariants?: Map<string, SubagentVariant>;
urlTransform?: UrlTransform;
mcpServers?: readonly TypesGen.MCPServerConfig[];
pendingToolCallIDs?: ReadonlySet<string>;
}

export const LiveStreamTail = ({
Expand All @@ -111,6 +112,7 @@ export const LiveStreamTail = ({
subagentVariants,
urlTransform,
mcpServers,
pendingToolCallIDs,
}: LiveStreamTailProps) => {
const streamState = useChatSelector(store, selectStreamState);
const streamError = useChatSelector(store, selectStreamError);
Expand All @@ -124,12 +126,18 @@ export const LiveStreamTail = ({
store,
selectSubagentStatusOverrides,
);
// Hide result-only rows whose pending call is already rendered in the
// durable transcript, so the live tail does not flash a duplicate row.
const visibleStreamState = filterPendingStreamState(
streamState,
pendingToolCallIDs,
);
Comment on lines +131 to +134

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Add Storybook coverage for the filtered live tail

When a durable pending tool call and a result-only stream entry share an ID, this changes user-visible rendering through ChatPageTimeline and LiveStreamTail, but the added unit tests exercise only the pure filter helper. Add a Storybook story with a play assertion proving that only the durable row remains while a parallel call with a different ID stays visible; otherwise the prop wiring, live-status derivation, and rendered timeline behavior remain untested.

AGENTS.md reference: site/AGENTS.md:L9-L10

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 64e6939. Added the story StreamedResultForPendingToolDoesNotDuplicate in ChatPageContent.stories.tsx, which renders ChatPageTimeline with a real chat store: a durable pending read_file call (tc-1) plus its streamed result-only part, and a parallel in-stream call (tc-2). The play function asserts both sides of the invariant: tc-1 renders exactly once as the durable pending row with the duplicate suppressed, and the tc-2 row stays visible. Verified in chromium via the storybook test project.

const streamTools = buildStreamTools(
streamState?.toolCalls,
streamState?.toolResults,
visibleStreamState?.toolCalls,
visibleStreamState?.toolResults,
);
const liveStatus = deriveLiveStatus({
streamState,
streamState: visibleStreamState,
retryState,
reconnectState,
streamError,
Expand All @@ -140,7 +148,7 @@ export const LiveStreamTail = ({
return (
<LiveStreamTailContent
isTranscriptEmpty={isTranscriptEmpty}
streamState={streamState}
streamState={visibleStreamState}
streamTools={streamTools}
liveStatus={liveStatus}
subagentTitles={subagentTitles}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
applyMessagePartToStreamState,
buildStreamTools,
createEmptyStreamState,
filterPendingStreamState,
} from "./streamState";
import type { StreamState } from "./types";

Expand Down Expand Up @@ -702,6 +703,203 @@ describe("applyMessagePartToStreamState", () => {
});
});

describe("filterPendingStreamState", () => {
const pendingReadFileState = (): StreamState => ({
blocks: [{ type: "tool", id: "tc-1" }],
toolCalls: {},
toolResults: {
"tc-1": {
id: "tc-1",
name: "read_file",
result: { content: "file body" },
isError: false,
},
},
sources: [],
});

it("normalizes a fully filtered state to null", () => {
const filtered = filterPendingStreamState(
pendingReadFileState(),
new Set(["tc-1"]),
);
expect(filtered).toBeNull();
});

it("keeps a result and block that share an id with an in-stream call", () => {
const state: StreamState = {
blocks: [{ type: "tool", id: "tc-1" }],
toolCalls: {
"tc-1": { id: "tc-1", name: "read_file", args: { path: "a.ts" } },
},
toolResults: {
"tc-1": {
id: "tc-1",
name: "read_file",
result: { content: "file body" },
isError: false,
},
},
sources: [],
};
// The normal streaming merge path wins even when the same id is
// pending in the durable transcript.
const filtered = filterPendingStreamState(state, new Set(["tc-1"]));
expect(filtered).toBe(state);
const tools = buildStreamTools(filtered?.toolCalls, filtered?.toolResults);
expect(tools).toHaveLength(1);
expect(tools[0].status).toBe("completed");
});

it("keeps a result-only entry for a parallel call to the same tool with a different id", () => {
const state: StreamState = {
blocks: [
{ type: "tool", id: "tc-1" },
{ type: "tool", id: "tc-2" },
],
toolCalls: {},
toolResults: {
"tc-1": {
id: "tc-1",
name: "read_file",
result: { content: "first" },
isError: false,
},
"tc-2": {
id: "tc-2",
name: "read_file",
result: { content: "second" },
isError: false,
},
},
sources: [],
};
const filtered = filterPendingStreamState(state, new Set(["tc-1"]));
expect(Object.keys(filtered?.toolResults ?? {})).toEqual(["tc-2"]);
expect(filtered?.blocks).toEqual([{ type: "tool", id: "tc-2" }]);
});

it("keeps a partially filtered state non-null when other content remains", () => {
const state: StreamState = {
blocks: [
{ type: "response", text: "Reading the file now." },
{ type: "tool", id: "tc-1" },
],
toolCalls: {},
toolResults: {
"tc-1": {
id: "tc-1",
name: "read_file",
result: { content: "file body" },
isError: false,
},
},
sources: [],
};
const filtered = filterPendingStreamState(state, new Set(["tc-1"]));
expect(filtered).not.toBeNull();
expect(filtered?.toolResults).toEqual({});
expect(filtered?.blocks).toEqual([
{ type: "response", text: "Reading the file now." },
]);
});

it("keeps non-tool blocks and other tool blocks while dropping the pending one", () => {
const state: StreamState = {
blocks: [
{ type: "response", text: "Reading the file now." },
{ type: "tool", id: "tc-1" },
{ type: "tool", id: "tc-2" },
],
toolCalls: {
"tc-2": { id: "tc-2", name: "bash", args: { command: "ls" } },
},
toolResults: {
"tc-1": {
id: "tc-1",
name: "read_file",
result: { content: "file body" },
isError: false,
},
},
sources: [],
};
const filtered = filterPendingStreamState(state, new Set(["tc-1"]));
expect(filtered?.blocks).toEqual([
{ type: "response", text: "Reading the file now." },
{ type: "tool", id: "tc-2" },
]);
expect(filtered?.toolResults).toEqual({});
expect(filtered?.toolCalls).toBe(state.toolCalls);
});

it("returns the input reference when nothing is dropped", () => {
const state = pendingReadFileState();
expect(filterPendingStreamState(state, undefined)).toBe(state);
expect(filterPendingStreamState(state, new Set())).toBe(state);
expect(filterPendingStreamState(state, new Set(["tc-other"]))).toBe(state);
});

it("returns null for null stream state", () => {
expect(filterPendingStreamState(null, new Set(["tc-1"]))).toBeNull();
});

it("keeps a still-streaming result for a pending durable call", () => {
const state: StreamState = {
blocks: [{ type: "tool", id: "tc-1" }],
toolCalls: {},
toolResults: {
"tc-1": {
id: "tc-1",
name: "advisor",
result: "Use small steps.",
resultRaw: "Use small steps.",
isError: false,
isStreaming: true,
},
},
sources: [],
};
const filtered = filterPendingStreamState(state, new Set(["tc-1"]));
expect(filtered).toBe(state);
const tools = buildStreamTools(filtered?.toolCalls, filtered?.toolResults);
expect(tools).toHaveLength(1);
expect(tools[0].status).toBe("running");
});

it("drops the final result for a pending durable call once streaming completes", () => {
let state: StreamState | null = null;
state = applyMessagePartToStreamState(state, {
type: "tool-result",
tool_name: "advisor",
tool_call_id: "tc-1",
result_delta: "Use ",
});
state = applyMessagePartToStreamState(state, {
type: "tool-result",
tool_name: "advisor",
tool_call_id: "tc-1",
result_delta: "small steps.",
});
// While the delta is still streaming the entry is kept.
expect(filterPendingStreamState(state, new Set(["tc-1"]))).toBe(state);

state = applyMessagePartToStreamState(state, {
type: "tool-result",
tool_name: "advisor",
tool_call_id: "tc-1",
result: {
type: "advice",
advice: "Use small steps.",
advisor_model: "test-provider/test-model",
remaining_uses: "2",
},
});
const filtered = filterPendingStreamState(state, new Set(["tc-1"]));
expect(filtered).toBeNull();
});
});

describe("buildStreamTools", () => {
it("returns empty array for null toolCalls", () => {
expect(buildStreamTools(null, null)).toEqual([]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,64 @@ const getStreamToolStatus = (
return result.isError ? "error" : "completed";
};

/**
* Drops result-only stream entries whose tool call is already rendered as
* pending in the durable transcript, so the live tail does not show a
* duplicate row before the tool step commits. Still-streaming results are
* kept so progressive output keeps rendering, and a state left with no
* visible content becomes null so the tail renders nothing. See the
* filterPendingStreamState tests for the full behavior contract.
*/
export const filterPendingStreamState = (
streamState: StreamState | null,
pendingToolCallIDs: ReadonlySet<string> | undefined,
): StreamState | null => {
if (!streamState || !pendingToolCallIDs || pendingToolCallIDs.size === 0) {
return streamState;
}
const toolResults: StreamState["toolResults"] = {};
let dropped = false;
for (const [id, result] of Object.entries(streamState.toolResults)) {
// Still-streaming results stay visible while they accumulate; the
// duplicate is only suppressed once the final result arrives.
if (
pendingToolCallIDs.has(id) &&
!streamState.toolCalls[id] &&
!result.isStreaming
Comment on lines +258 to +260

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep final advisor advice visible until durable commit

When an advisor finishes, its final tool-result contains result, which makes applyMessagePartToStreamState clear isStreaming before the durable tool message commits; this predicate then removes the result and its block even though the durable pending row contains only the question and still says "Consulting advisor…". The advice therefore disappears between the final SSE event and the durable commit. Fresh evidence after the prior thread is the new drops the final result for a pending durable call once streaming completes test, which explicitly expects this state to become null, while the Storybook story covers only result_delta events. Preserve the advisor's final result until the durable result is available, and cover that final-event window in the story.

AGENTS.md reference: site/AGENTS.md:L9-L10

Useful? React with 👍 / 👎.

) {
dropped = true;
continue;
}
toolResults[id] = result;
}
if (!dropped) {
return streamState;
}
const blocks = streamState.blocks.filter(
(block) =>
block.type !== "tool" ||
!(
pendingToolCallIDs.has(block.id) &&
!streamState.toolCalls[block.id] &&
!streamState.toolResults[block.id]?.isStreaming
),
);
// Nothing visible remains.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Remove the control-flow narration

The Nothing visible remains comment only paraphrases the zero-length and empty-map guard immediately below it, so it violates FE4's explicit prohibition on comments that restate control flow. Remove it rather than narrating what the condition already expresses.

AGENTS.md reference: site/AGENTS.md:L15-L15

Useful? React with 👍 / 👎.

if (
blocks.length === 0 &&
Object.keys(streamState.toolCalls).length === 0 &&
Object.keys(toolResults).length === 0 &&
streamState.sources.length === 0
) {
return null;
}
return {
...streamState,
blocks,
toolResults,
};
};

export const buildStreamTools = (
toolCalls: StreamState["toolCalls"] | null | undefined,
toolResults: StreamState["toolResults"] | null | undefined,
Expand Down
Loading
Loading