Skip to content
Merged
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
3 changes: 3 additions & 0 deletions changelog.d/chat-run-fold-first-part.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
### Fixed
- A chat work block you folded no longer opens again when the words before its tools join it, as
when a later Codex record marks them commentary or a blank commentary is filled in.
23 changes: 23 additions & 0 deletions scripts/chat-fallback-browser-qa.ts
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,29 @@ try {
assert.deepEqual(await opened(), [false, false, false], "a run that splits moves no fold to another block: the folded run stays folded");
await headRows.waitFor({ state: "hidden" });
if (process.env.UI_EVIDENCE_DIR) await page.screenshot({ path: join(process.env.UI_EVIDENCE_DIR, "claude-run-split.png") });
// Words before a run's tools join it later (a Codex record marks them commentary): the run
// keeps the fold the reader gave it (#2063). A history without an ID keeps each row's memory
// by its place, so the turn sits on row 5, which the four rows above left with none.
const sorting = (words: { kind: "text"; text: string; phase?: "commentary" }): ConversationTurn[] => [
...shelves("Shelf a is empty."),
{ role: "user", ts: null, parts: [said("Sort the shelves.")] },
{ role: "assistant", ts: null, parts: [words, bash("sort a")] },
];
turns = sorting(said("Sorting shelf a."));
await page.evaluate(() => window.chatFallbackQA.refresh());
await page.getByText("Sorting shelf a.", { exact: true }).waitFor();
assert.deepEqual(await rows(), ["Sorting shelf a.", "work Working\u2026 \u00b7 1 command"], "the words stand above the working run");
assert.deepEqual(await opened(), [true], "the working run is open");
await heads.last().click();
await headRows.waitFor({ state: "hidden" });
assert.deepEqual(await opened(), [false], "the reader folds the working run");
turns = sorting({ ...said("Sorting shelf a."), phase: "commentary" });
await page.evaluate(() => window.chatFallbackQA.refresh());
// drawn once the words left the turn for its run of work
await page.waitForFunction(() => [...[...document.querySelectorAll(".chat-turn-agent")].at(-1)!.children].every((child) => child.classList.contains("work-block")));
assert.deepEqual(await opened(), [false], "words that join the run keep it folded");
assert.equal(await page.getByText("Sorting shelf a.", { exact: true }).isVisible(), false, "and fold with it");
if (process.env.UI_EVIDENCE_DIR) await page.screenshot({ path: join(process.env.UI_EVIDENCE_DIR, "run-words-join-fold.png") });
await page.evaluate(() => window.chatFallbackQA.status(undefined));
await live.waitFor({ state: "detached" });
console.log("PASS words and runs of work keep the agent's order, settled and live");
Expand Down
2 changes: 1 addition & 1 deletion src/components/ChatTimeline.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ export function ChatTimeline<Row extends ChatTimelineRow>({ rows, scope, source,
const rendered = useCallback(({ item, index }: { item: Row; index: number }) => {
let memory = memories.get(item.key);
if (!memory) {
memory = { toggles: new Map(), outputs: new Map() };
memory = { toggles: new Map(), outputs: new Map(), runs: new Map() };
memories.set(item.key, memory);
}
return <ChatRowMemoryContext.Provider value={memory}>
Expand Down
86 changes: 84 additions & 2 deletions src/components/ChatTurn.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { describe, expect, it } from "bun:test";
import { createElement } from "react";
import { afterEach, beforeEach, describe, expect, it } from "bun:test";
import { act, createElement } from "react";
import { renderToStaticMarkup } from "react-dom/server";
import { create, type ReactTestRenderer } from "react-test-renderer";

import type { ConversationPart, ConversationTurn } from "../../shared/protocol.ts";
import { ChatRowMemoryContext, type ChatRowMemory } from "../lib/chatRowState.tsx";
import { SettingsProvider } from "../lib/settings.ts";
import { Turn } from "./ChatView.tsx";

Expand Down Expand Up @@ -55,3 +57,83 @@ describe("an agent turn's work blocks", () => {
expect(html.indexOf("Worked for 1m")).toBeLessThan(html.indexOf("Done."));
});
});

describe("an agent turn's folded work, drawn again", () => {
const commentary = (value: string): ConversationPart => ({ kind: "text", text: value, phase: "commentary" });
const answer = (value: string): ConversationPart => ({ kind: "text", text: value, phase: "final_answer" });
const descriptors = new Map<string, PropertyDescriptor | undefined>();
let memory: ChatRowMemory;
let renderer: ReactTestRenderer | undefined;

const view = (parts: ConversationPart[]) => createElement(ChatRowMemoryContext.Provider, { value: memory, children: createElement(SettingsProvider, { children: createElement(Turn, {
paneId: "w1:p1", turn: { role: "assistant", ts: "2026-01-01T12:00:00Z", parts }, rowKey: "turn:1", live: true, waiting: false, showThinking: false,
}) }) });
const draw = async (parts: ConversationPart[]) => {
await act(async () => { if (renderer) renderer.update(view(parts)); else renderer = create(view(parts)); });
};
const heads = () => renderer!.root.findAll((node) => node.props.className === "work-block-head");
const opened = () => heads().map((node) => node.props["aria-expanded"] as boolean);
const toggle = async (index: number) => { await act(async () => { heads()[index]!.props.onClick(); }); };
// the row scrolled out of the list and back: only its memory is left
const remount = async (parts: ConversationPart[]) => {
await act(async () => { renderer!.unmount(); });
renderer = undefined;
await draw(parts);
};

beforeEach(() => {
for (const key of ["window", "document", "getComputedStyle", "IS_REACT_ACT_ENVIRONMENT"]) descriptors.set(key, Object.getOwnPropertyDescriptor(globalThis, key));
descriptors.set("languages", Object.getOwnPropertyDescriptor(navigator, "languages"));
const listeners = { addEventListener() {}, removeEventListener() {} };
Object.defineProperty(globalThis, "window", { configurable: true, value: { ...listeners, matchMedia: () => ({ matches: false, ...listeners }) } });
Object.defineProperty(globalThis, "document", { configurable: true, value: { ...listeners, querySelector: () => null, documentElement: { dataset: {}, style: { setProperty() {}, removeProperty() {} } } } });
Object.defineProperty(globalThis, "getComputedStyle", { configurable: true, value: () => ({ getPropertyValue: () => "" }) });
Object.defineProperty(globalThis, "IS_REACT_ACT_ENVIRONMENT", { configurable: true, value: true });
Object.defineProperty(navigator, "languages", { configurable: true, value: ["en"] });
memory = { toggles: new Map(), outputs: new Map(), runs: new Map() };
});

afterEach(async () => {
if (renderer) await act(async () => { renderer!.unmount(); });
renderer = undefined;
for (const [key, descriptor] of descriptors) {
const target = key === "languages" ? navigator : globalThis;
if (descriptor) Object.defineProperty(target, key, descriptor);
else Reflect.deleteProperty(target, key);
}
});

it("keeps a block folded when the words before its tools join it (#2063)", async () => {
// a later Codex record marks the words commentary; an empty commentary record is filled in
for (const [before, after] of [[text("Checking."), commentary("Checking.")], [commentary(""), commentary("Checking.")]] as const) {
memory = { toggles: new Map(), outputs: new Map(), runs: new Map() };
await draw([before, tool("ls")]);
await toggle(0);
expect(opened()).toEqual([false]);
await draw([after, tool("ls")]);
expect(opened()).toEqual([false]);
await remount([after, tool("ls")]);
expect(opened()).toEqual([false]);
await act(async () => { renderer!.unmount(); });
renderer = undefined;
}
});

it("keeps a merged block's fold when a part leaves it", async () => {
await draw([tool("ls"), text("One shelf."), commentary("Checking it.")]);
expect(opened()).toEqual([false, true]);
// the reader folds the working run and opens it again
await toggle(1);
await toggle(1);
// the words between become commentary: one run, which the reader folds
await draw([tool("ls"), commentary("One shelf."), commentary("Checking it.")]);
expect(opened()).toEqual([true]);
await toggle(0);
expect(opened()).toEqual([false]);
// its last words become the answer and leave it
await draw([tool("ls"), commentary("One shelf."), answer("Checking it.")]);
expect(opened()).toEqual([false]);
await remount([tool("ls"), commentary("One shelf."), answer("Checking it.")]);
expect(opened()).toEqual([false]);
});
});
19 changes: 15 additions & 4 deletions src/components/ChatView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import { copyText } from "../lib/clipboard.ts";
import { attachmentFileName, mentionFileName, type GalleryImage } from "../lib/imageViewer.ts";
import { useMachineApi } from "../lib/machineContext.tsx";
import { toTranscriptMessages, type TranscriptMessage } from "../lib/transcript.ts";
import { isLiveWorkTurn, isWaitingWorkTurn, formatWorkDuration, splitTurn, turnSegments, type WorkSegment, todoGroups, visibleWorkParts, workFailed, workStartsOpen, workSummary, type TodoAttempt, type TodoGroup, type ToolPart as ToolPartType } from "../lib/workBlocks.ts";
import { isLiveWorkTurn, isWaitingWorkTurn, formatWorkDuration, runNames, splitTurn, turnSegments, type WorkSegment, todoGroups, visibleWorkParts, workFailed, workStartsOpen, workSummary, type TodoAttempt, type TodoGroup, type ToolPart as ToolPartType } from "../lib/workBlocks.ts";
import { phaseRows, planRows, taskRows, todoRows, type ChecklistRow } from "../lib/checklist.ts";
import { isTodoTool, parseTodoAnswer, todoCallSummary, type TodoItem, type TodoStatus } from "../lib/todos.ts";
import { formatGoalTime, turnGoal, type GoalState, type GoalStatus } from "../lib/goals.ts";
Expand Down Expand Up @@ -1014,6 +1014,15 @@ function noticeLabel(t: ReturnType<typeof useT>, notice: Extract<ConversationPar
// a turn that did not change keeps its object across polls: skip re-rendering it
export const Turn = memo(function Turn({ paneId, turn, rowKey, live, waiting, showThinking, skills = null, userSkills = null, outcome = null, outcomeActions = null, onRewindInTerminal = null }: TurnProps) {
const t = useT();
const memory = useContext(ChatRowMemoryContext);
// what the drawn runs were named, by part index; the row's memory takes it once the turn is drawn
const named = useRef<Map<number, string> | null>(null);
named.current = null;
useLayoutEffect(() => {
if (memory === null || named.current === null) return;
memory.runs.clear();
for (const [index, name] of named.current) memory.runs.set(index, name);
});
// under the turn, not in its meta row: that row fades out once the pointer and focus leave
const [copyFailed, setCopyFailed] = useChatRowToggle("copy-failed", false);
const copyError = copyFailed && <p className="chat-copy-error" role="alert">{t("Couldn't copy. Select the text and copy it manually.")}</p>;
Expand Down Expand Up @@ -1083,12 +1092,14 @@ export const Turn = memo(function Turn({ paneId, turn, rowKey, live, waiting, sh
// The runs before it have the agent's words after them: they are done, read Worked and fold.
const head = runs.filter((run) => visibleWorkParts(run.parts, showThinking).length > 0).at(-1) ?? runs.at(-1);
const work = turnWork(turn.parts, rowKey);
// a run keeps its block and fold while its parts change (runNames); the stand-in is named by the
// part that will start the turn's first run, and is drawn only while the turn has none
const names = runNames(runs, work.at, memory?.runs ?? new Map());
named.current = new Map(runs.flatMap((run, index) => run.parts.map((part) => [work.at.get(part)!, names[index]!] as const)));
// `null`: the head standing in while the running turn has no work yet
const block = (run: WorkSegment | null) => {
const isHead = run === null || run === head;
// a run is named by its first part, so it keeps its block and fold when an earlier run splits
// or grows; the stand-in by the part that will start the turn's first run, which takes its place
const key = `work:${run?.start ?? turn.parts.length}`;
const key = run === null ? `work:${turn.parts.length}` : names[runs.indexOf(run)]!;
return <WorkBlockView key={key} paneId={paneId} parts={run?.parts ?? []} work={work} memoryKey={key}
duration={isHead ? formatWorkDuration(turn.ts, turn.end_ts ?? null) : null} live={live && isHead} waiting={waiting && isHead}
// a settled turn folds its work, unless words the agent said last are in its final run
Expand Down
2 changes: 2 additions & 0 deletions src/lib/chatRowState.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import { createContext, useCallback, useContext, useState, type ComponentProps }
export interface ChatRowMemory {
readonly toggles: Map<string, boolean | null>;
readonly outputs: Map<string, string>;
/** an agent turn's part index → the name of the run of work it was in when last drawn (runNames) */
readonly runs: Map<number, string>;
}

export const ChatRowMemoryContext = createContext<ChatRowMemory | null>(null);
Expand Down
26 changes: 25 additions & 1 deletion src/lib/workBlocks.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { describe, expect, it } from "bun:test";

import { formatWorkDuration, splitTurn, todoGroups, turnSegments, visibleWorkParts, workFailed, workStartsOpen, workSummary, type ToolPart } from "./workBlocks.ts";
import { formatWorkDuration, runNames, splitTurn, todoGroups, turnSegments, visibleWorkParts, workFailed, workStartsOpen, workSummary, type ToolPart } from "./workBlocks.ts";
import type { ConversationPart } from "../../shared/protocol.ts";

const tool = (name: string, summary = name): Extract<ConversationPart, { kind: "tool" }> => ({ kind: "tool", name, summary, input: "{}", output: "" });
Expand Down Expand Up @@ -76,6 +76,30 @@ describe("turnSegments", () => {
});
});

describe("runNames", () => {
// what Turn keeps between draws: each part's run name, by part index
const draw = (parts: ConversationPart[], previous: ReadonlyMap<number, string>) => {
const runs = turnSegments(parts).flatMap((segment) => segment.kind === "work" ? [segment] : []);
const names = runNames(runs, new Map(parts.map((part, index) => [part, index])), previous);
return { names, next: new Map(runs.flatMap((run, index) => run.parts.map((part) => [parts.indexOf(part), names[index]!] as const))) };
};

it("names a run by its first part the first time, and keeps the name while its parts change", () => {
const narration = { ...text("Checking."), phase: "commentary" as const };
const first = draw([text("Checking."), tool("ls")], new Map());
expect(first.names).toEqual(["work:1"]);
expect(draw([narration, tool("ls")], first.next).names).toEqual(["work:1"]);
});

it("gives a split run's first part the old name and never hands a shown name to a new run", () => {
const shown = draw([tool("A"), text(""), tool("B"), text("between"), tool("C")], new Map());
expect(shown.names).toEqual(["work:0", "work:4"]);
expect(draw([tool("A"), text("arrived"), tool("B"), text("between"), tool("C")], shown.next).names).toEqual(["work:0", "work:2", "work:4"]);
// the run named work:0 now starts at 2; a new run at 0 must not take its name
expect(draw([tool("new"), text("between"), tool("A")], new Map([[2, "work:0"]])).names).toEqual(["work:0+", "work:0"]);
});
});

describe("workStartsOpen", () => {
const commentary = (value: string) => ({ ...text(value), phase: "commentary" as const });
const final = (value: string) => ({ ...text(value), phase: "final_answer" as const });
Expand Down
24 changes: 24 additions & 0 deletions src/lib/workBlocks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,30 @@ export function turnSegments(parts: ConversationPart[]): TurnSegment[] {
return segments;
}

/**
* The name of each run's block, which also keeps its fold. A run keeps its name while its parts
* change: words before its tools join it (a record marks them commentary, a blank one is filled
* in), a part leaves it, or it splits or merges with another run (#2063). `previous` holds the name
* of the run each part was in when the turn was last drawn, by part index. A run takes the first
* of those names among its own parts that no earlier run has taken. Only then does a run with none
* get `work:<start>`, so a new run never takes the name of a run already shown.
*/
export function runNames(runs: readonly WorkSegment[], at: ReadonlyMap<ConversationPart, number>, previous: ReadonlyMap<number, string>): string[] {
const used = new Set<string>();
const kept = runs.map((run) => {
const name = run.parts.map((part) => previous.get(at.get(part)!)).find((name) => name !== undefined && !used.has(name));
if (name !== undefined) used.add(name);
return name;
});
return runs.map((run, index) => {
if (kept[index] !== undefined) return kept[index];
let name = `work:${run.start}`;
while (used.has(name)) name += "+";
used.add(name);
return name;
});
}

/**
* A work block is open while its turn runs and folded once the turn settles: the answer stays
* outside the fold, so a finished turn reads as its answer under one "Worked for" row.
Expand Down