smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(tui): preserve grouped read request order

Browse Source 
(cherry picked from commit eb6767e3248ad0fe3ea371e5226a4af6e0b8f97b)
This commit is contained in:
joshrzemien
2026-07-28 22:46:07 -04:00
committed by can1357
parent a3e90b3d95
commit 57cb290fef
3 changed files with 132 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/src/modes/components/read-tool-group.ts
+73 -46
View File
@@ -43,27 +43,30 @@ export function readArgsCollapseIntoGroup(args: unknown): boolean {
}
/**
* Return the collapsed read calls that can own a turn's usage row. Visible
* assistant content and mixed-tool turns keep their usage as a standalone row
* so the metrics are not misleadingly attributed to one tool.
* Return the collapsed read calls that can own a turn's usage row. Mixed-tool
* turns and visible content after a read keep the standalone row so request
* metrics retain their transcript ordering.
*/
export function groupedReadUsageCallIds(message: AssistantMessage): string[] | undefined {
const hasVisibleContent = message.content.some(
content =>
content.type === "image" ||
(content.type === "text" && canonicalizeMessage(content.text)) ||
(content.type === "thinking" && canonicalizeMessage(content.thinking)),
);
if (hasVisibleContent) return undefined;
const toolCalls = message.content.filter(content => content.type === "toolCall");
if (
toolCalls.length === 0 ||
toolCalls.some(content => content.name !== "read" || !readArgsCollapseIntoGroup(content.arguments))
) {
return undefined;
const toolCallIds: string[] = [];
let sawToolCall = false;
for (const content of message.content) {
if (content.type === "toolCall") {
if (content.name !== "read" || !readArgsCollapseIntoGroup(content.arguments)) return undefined;
sawToolCall = true;
toolCallIds.push(content.id);
continue;
}
if (
sawToolCall &&
(content.type === "image" ||
(content.type === "text" && canonicalizeMessage(content.text)) ||
(content.type === "thinking" && canonicalizeMessage(content.thinking)))
) {
return undefined;
}
}
return toolCalls.map(content => content.id);
return toolCallIds.length > 0 ? toolCallIds : undefined;
}
type ReadRenderArgs = {
@@ -122,6 +125,7 @@ type ReadEntry = {
};
type ReadUsageRow = {
toolCallIds: readonly string[];
usage: Usage;
durationMs?: number;
ttftMs?: number;
@@ -325,6 +329,7 @@ function formatMergedSelectorParts(selectors: string[]): string {
export class ReadToolGroupComponent extends Container implements ToolExecutionHandle {
#entries = new Map<string, ReadEntry>();
#usageRows = new Map<string, ReadUsageRow>();
#usageBatchByToolCallId = new Map<string, string>();
#text: Text;
#expanded = false;
#showContentPreview: boolean;
@@ -440,16 +445,18 @@ export class ReadToolGroupComponent extends Container implements ToolExecutionHa
ttftMs?: number,
timestamp?: number,
): boolean {
const attachedToolCallIds: string[] = [];
let anchorId: string | undefined;
for (let index = toolCallIds.length - 1; index >= 0; index--) {
const toolCallId = toolCallIds[index]!;
if (this.#entries.has(toolCallId)) {
anchorId = toolCallId;
break;
}
for (const toolCallId of toolCallIds) {
if (!this.#entries.has(toolCallId)) continue;
attachedToolCallIds.push(toolCallId);
anchorId = toolCallId;
}
if (!anchorId) return false;
this.#usageRows.set(anchorId, { usage, durationMs, ttftMs, timestamp });
for (const toolCallId of attachedToolCallIds) {
this.#usageBatchByToolCallId.set(toolCallId, anchorId);
}
this.#usageRows.set(anchorId, { toolCallIds: attachedToolCallIds, usage, durationMs, ttftMs, timestamp });
this.#updateDisplay();
return true;
}
@@ -544,27 +551,37 @@ export class ReadToolGroupComponent extends Container implements ToolExecutionHa
}
#buildSummaryRows(targets: ReadDisplayTarget[]): ReadSummaryRow[] {
const selectorTargetsByBasePath = new Map<string, ReadDisplayTarget[]>();
const selectorTargetsByBasePathAndBatch = new Map<string, Map<string | undefined, ReadDisplayTarget[]>>();
for (const target of targets) {
if (!target.selector) continue;
const existing = selectorTargetsByBasePath.get(target.basePath);
let targetsByBatch = selectorTargetsByBasePathAndBatch.get(target.basePath);
if (!targetsByBatch) {
targetsByBatch = new Map<string | undefined, ReadDisplayTarget[]>();
selectorTargetsByBasePathAndBatch.set(target.basePath, targetsByBatch);
}
const batchId = this.#usageBatchByToolCallId.get(target.entry.toolCallId);
const existing = targetsByBatch.get(batchId);
if (existing) existing.push(target);
else selectorTargetsByBasePath.set(target.basePath, [target]);
else targetsByBatch.set(batchId, [target]);
}
const mergeableBasePaths = new Set<string>();
for (const [basePath, baseTargets] of selectorTargetsByBasePath) {
if (basePath && baseTargets.length > 1) {
mergeableBasePaths.add(basePath);
const mergedTargetsByTarget = new Map<ReadDisplayTarget, ReadDisplayTarget[]>();
for (const [basePath, targetsByBatch] of selectorTargetsByBasePathAndBatch) {
if (!basePath) continue;
for (const groupedTargets of targetsByBatch.values()) {
if (groupedTargets.length <= 1) continue;
for (const target of groupedTargets) {
mergedTargetsByTarget.set(target, groupedTargets);
}
}
}
const emittedMergedRows = new Set<string>();
const emittedMergedTargets = new Set<ReadDisplayTarget[]>();
const rows: ReadSummaryRow[] = [];
for (const target of targets) {
if (target.selector && mergeableBasePaths.has(target.basePath)) {
if (!emittedMergedRows.has(target.basePath)) {
const mergedTargets = selectorTargetsByBasePath.get(target.basePath) ?? [target];
const mergedTargets = mergedTargetsByTarget.get(target);
if (mergedTargets) {
if (!emittedMergedTargets.has(mergedTargets)) {
rows.push({
targetPath: `${target.basePath}:${formatMergedSelectorParts(
mergedTargets
@@ -574,7 +591,7 @@ export class ReadToolGroupComponent extends Container implements ToolExecutionHa
basePath: target.basePath,
targets: mergedTargets,
});
emittedMergedRows.add(target.basePath);
emittedMergedTargets.add(mergedTargets);
}
continue;
}
@@ -602,17 +619,27 @@ export class ReadToolGroupComponent extends Container implements ToolExecutionHa
}
#usageRowsBySummaryRow(rows: ReadSummaryRow[]): Map<number, ReadUsageRow[]> {
const usageRowsByIndex = new Map<number, ReadUsageRow[]>();
for (const [toolCallId, usageRow] of this.#usageRows) {
for (let index = rows.length - 1; index >= 0; index--) {
const row = rows[index]!;
if (!row.targets.some(target => target.entry.toolCallId === toolCallId)) continue;
const usageRows = usageRowsByIndex.get(index);
if (usageRows) usageRows.push(usageRow);
else usageRowsByIndex.set(index, [usageRow]);
break;
const lastRowIndexByToolCallId = new Map<string, number>();
for (const [index, row] of rows.entries()) {
for (const target of row.targets) {
lastRowIndexByToolCallId.set(target.entry.toolCallId, index);
}
}
const usageRowsByIndex = new Map<number, ReadUsageRow[]>();
for (const usageRow of this.#usageRows.values()) {
let lastRowIndex: number | undefined;
for (const toolCallId of usageRow.toolCallIds) {
const index = lastRowIndexByToolCallId.get(toolCallId);
if (index !== undefined && (lastRowIndex === undefined || index > lastRowIndex)) {
lastRowIndex = index;
}
}
if (lastRowIndex === undefined) continue;
const usageRows = usageRowsByIndex.get(lastRowIndex);
if (usageRows) usageRows.push(usageRow);
else usageRowsByIndex.set(lastRowIndex, [usageRow]);
}
return usageRowsByIndex;
}
packages/coding-agent/test/modes/controllers/event-controller-read-grouping.test.ts
+28 -1
View File
@@ -135,7 +135,7 @@ describe("EventController read-group accretion", () => {
it("nests a read-only completion's usage inside the active group", async () => {
settings.set("display.showTokenUsage", true);
const { controller, chatContainer } = createFixture();
const message = assistantMessage([read("usage.ts:1-50")]);
const message = assistantMessage([thinking("Reviewing the target"), read("usage.ts:1-50")]);
message.usage = {
input: 1234,
output: 7,
@@ -158,6 +158,33 @@ describe("EventController read-group accretion", () => {
expect(usageBlocks).toEqual([group!]);
});
it("keeps usage standalone when visible content follows a read", async () => {
settings.set("display.showTokenUsage", true);
const { controller, chatContainer } = createFixture();
const message = assistantMessage([read("usage.ts:1-50"), thinking("Read complete")]);
message.usage = {
input: 1234,
output: 7,
cacheRead: 0,
cacheWrite: 0,
totalTokens: 1241,
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 },
};
message.timestamp = new Date(2026, 0, 2, 3, 4, 5).getTime();
await controller.handleEvent({ type: "message_start", message } as AgentSessionEvent);
await controller.handleEvent({ type: "message_update", message } as AgentSessionEvent);
await controller.handleEvent({ type: "message_end", message } as AgentSessionEvent);
const [group] = readGroups(chatContainer);
expect(group).toBeDefined();
const usageBlocks = chatContainer.children.filter(component =>
Bun.stripANSI(component.render(120).join("\n")).includes("2026-01-02 03:04:05"),
);
expect(usageBlocks).toHaveLength(1);
expect(usageBlocks[0]).not.toBe(group!);
});
it("starts a new group after a completion that renders visible reasoning", async () => {
const { controller, chatContainer } = createFixture();
packages/coding-agent/test/read-tool-group.test.ts
+31 -2
View File
@@ -101,8 +101,8 @@ describe("ReadToolGroupComponent", () => {
const twoPath = path.resolve("/tmp/two.ts");
const threePath = path.resolve("/tmp/three.ts");
component.updateArgs({ path: onePath }, "read-one");
component.updateArgs({ path: twoPath }, "read-two");
component.updateArgs({ path: threePath }, "read-three");
component.updateArgs({ path: `${twoPath}:1-2,${threePath}:1-2` }, "read-two");
component.updateArgs({ path: `${twoPath}:3-4` }, "read-three");
component.updateResult({ content: [{ type: "text", text: "one" }] }, false, "read-one");
component.updateResult({ content: [{ type: "text", text: "two" }] }, false, "read-two");
component.updateResult({ content: [{ type: "text", text: "three" }] }, false, "read-three");
@@ -280,6 +280,35 @@ describe("ReadToolGroupComponent", () => {
expect(matches).toBe(1);
});
it("keeps usage below an inline preview when the summary row is suppressed", () => {
const component = new ReadToolGroupComponent({ showContentPreview: true });
const examplePath = path.resolve("/tmp/example.ts");
component.updateArgs({ path: examplePath }, "read-preview");
component.updateResult({ content: [{ type: "text", text: "line 1\nline 2" }] }, false, "read-preview");
component.attachUsage(
["read-preview"],
{
input: 1234,
output: 7,
cacheRead: 0,
cacheWrite: 0,
totalTokens: 1241,
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 },
},
1000,
500,
new Date(2026, 0, 2, 3, 4, 5).getTime(),
);
const lines = Bun.stripANSI(component.render(120).join("\n")).split("\n");
const previewIndex = lines.findIndex(line => line.includes("line 2"));
const usageIndices = lines
.map((line, index) => (line.includes("2026-01-02 03:04:05") ? index : -1))
.filter(index => index >= 0);
expect(usageIndices).toHaveLength(1);
expect(usageIndices[0]).toBeGreaterThan(previewIndex);
});
it("links grouped summary paths to resolved filesystem paths and selector lines", () => {
settings.override("tui.hyperlinks", "always");
const component = new ReadToolGroupComponent();