fix: route Claude partial tool input through canonical mapping (#108) (#122)

* fix: show loading skeleton for pending tool call details

* fix: surface claude tool previews before full json

* fix: route claude partial tool input through canonical mapping

* fix: remove pending tool-call summary placeholder
This commit is contained in:
Mohamed Boudra
2026-03-17 14:42:39 +08:00
committed by GitHub
parent 9364da3414
commit 11f6494c20
10 changed files with 723 additions and 24 deletions
+29 -13
View File
@@ -71,6 +71,10 @@ import {
buildToolCallDisplayModel,
} from "@/utils/tool-call-display";
import { resolveToolCallIcon } from "@/utils/tool-call-icon";
import {
hasMeaningfulToolCallDetail,
isPendingToolCallDetail,
} from "@/utils/tool-call-detail-state";
import {
parseAssistantFileLink,
parseInlinePathToken,
@@ -1923,31 +1927,42 @@ export const ToolCall = memo(function ToolCall({
const summary = displayModel.summary;
const errorText = displayModel.errorText;
const IconComponent = resolveToolCallIcon(toolName, effectiveDetail);
const isLoadingDetails = isPendingToolCallDetail({
detail: effectiveDetail,
status,
error,
});
const secondaryLabel = summary;
// Check if there's any content to display
const hasDetails =
Boolean(error) ||
(effectiveDetail
? effectiveDetail.type === "plain_text"
? Boolean(effectiveDetail.text)
: effectiveDetail.type !== "unknown" ||
effectiveDetail.input !== null ||
effectiveDetail.output !== null
: false);
hasMeaningfulToolCallDetail(effectiveDetail);
const canOpenDetails = hasDetails || isLoadingDetails;
const handleToggle = useCallback(() => {
if (isMobile) {
openToolCall({
toolName,
displayName,
summary,
summary: secondaryLabel,
detail: effectiveDetail,
errorText,
showLoadingSkeleton: isLoadingDetails,
});
} else {
setIsExpanded((prev) => !prev);
}
}, [isMobile, openToolCall, toolName, displayName, summary, effectiveDetail, errorText]);
}, [
isMobile,
openToolCall,
toolName,
displayName,
secondaryLabel,
effectiveDetail,
errorText,
isLoadingDetails,
]);
useEffect(() => {
if (!onInlineDetailsHoverChange || isMobile || isExpanded) {
@@ -1984,19 +1999,20 @@ export const ToolCall = memo(function ToolCall({
detail={effectiveDetail}
errorText={errorText}
maxHeight={400}
showLoadingSkeleton={isLoadingDetails}
/>
);
}, [isMobile, effectiveDetail, errorText]);
}, [isMobile, effectiveDetail, errorText, isLoadingDetails]);
return (
<ExpandableBadge
testID="tool-call-badge"
label={displayName}
secondaryLabel={summary}
secondaryLabel={secondaryLabel}
icon={IconComponent}
isExpanded={!isMobile && isExpanded}
onToggle={hasDetails ? handleToggle : undefined}
renderDetails={hasDetails && !isMobile ? renderDetails : undefined}
onToggle={canOpenDetails ? handleToggle : undefined}
renderDetails={canOpenDetails && !isMobile ? renderDetails : undefined}
isLoading={status === "running" || status === "executing"}
isError={status === "failed"}
isLastInSequence={isLastInSequence}
@@ -8,6 +8,7 @@ import {
buildLineDiff,
parseUnifiedDiff,
} from "@/utils/tool-call-parsers";
import { hasMeaningfulToolCallDetail } from "@/utils/tool-call-detail-state";
import { DiffViewer } from "./diff-viewer";
import { getCodeInsets } from "./code-insets";
@@ -20,6 +21,7 @@ interface ToolCallDetailsContentProps {
errorText?: string;
maxHeight?: number;
fillAvailableHeight?: boolean;
showLoadingSkeleton?: boolean;
}
export function ToolCallDetailsContent({
@@ -27,6 +29,7 @@ export function ToolCallDetailsContent({
errorText,
maxHeight,
fillAvailableHeight = false,
showLoadingSkeleton = false,
}: ToolCallDetailsContentProps) {
const resolvedMaxHeight = fillAvailableHeight ? undefined : (maxHeight ?? 300);
@@ -269,7 +272,11 @@ export function ToolCallDetailsContent({
const sectionsFromTopLevel = [
{ title: "Input", value: detail.input },
{ title: "Output", value: detail.output },
].filter((entry) => entry.value !== null && entry.value !== undefined);
].filter((entry) => hasMeaningfulToolCallDetail({
type: "unknown",
input: entry.value ?? null,
output: null,
}));
for (const section of sectionsFromTopLevel) {
let value = "";
@@ -327,6 +334,20 @@ export function ToolCallDetailsContent({
}
if (sections.length === 0) {
if (showLoadingSkeleton) {
return (
<View
style={[
styles.loadingContainer,
fillAvailableHeight && styles.fillHeight,
]}
>
<View style={styles.loadingLineWide} />
<View style={styles.loadingLineMedium} />
<View style={styles.loadingLineShort} />
</View>
);
}
return (
<Text style={styles.emptyStateText}>No additional details available</Text>
);
@@ -471,5 +492,27 @@ const styles = StyleSheet.create((theme) => {
fontSize: theme.fontSize.sm,
fontStyle: "italic",
},
loadingContainer: {
gap: theme.spacing[2],
padding: theme.spacing[3],
},
loadingLineWide: {
height: 12,
width: "100%",
borderRadius: theme.borderRadius.full,
backgroundColor: theme.colors.surface3,
},
loadingLineMedium: {
height: 12,
width: "72%",
borderRadius: theme.borderRadius.full,
backgroundColor: theme.colors.surface3,
},
loadingLineShort: {
height: 12,
width: "48%",
borderRadius: theme.borderRadius.full,
backgroundColor: theme.colors.surface3,
},
};
});
@@ -28,6 +28,7 @@ export type ToolCallSheetData = {
summary?: string;
detail?: ToolCallDetail;
errorText?: string;
showLoadingSkeleton?: boolean;
};
interface ToolCallSheetContextValue {
@@ -131,7 +132,7 @@ interface ToolCallSheetContentProps {
}
function ToolCallSheetContent({ data, onClose }: ToolCallSheetContentProps) {
const { toolName, displayName, detail, errorText } = data;
const { toolName, displayName, detail, errorText, showLoadingSkeleton } = data;
const IconComponent = resolveToolCallIcon(toolName, detail);
@@ -159,6 +160,7 @@ function ToolCallSheetContent({ data, onClose }: ToolCallSheetContentProps) {
detail={detail}
errorText={errorText}
fillAvailableHeight
showLoadingSkeleton={showLoadingSkeleton}
/>
</BottomSheetScrollView>
</View>
@@ -0,0 +1,60 @@
import assert from "node:assert/strict";
import { describe, it } from "vitest";
import type { ToolCallDetail } from "@server/server/agent/agent-sdk-types";
import {
hasMeaningfulToolCallDetail,
isPendingToolCallDetail,
} from "./tool-call-detail-state";
describe("tool-call detail state", () => {
it("treats empty unknown payloads as not meaningful", () => {
const detail: ToolCallDetail = {
type: "unknown",
input: {},
output: null,
};
assert.strictEqual(hasMeaningfulToolCallDetail(detail), false);
});
it("treats partial unknown payloads with real values as meaningful", () => {
const detail: ToolCallDetail = {
type: "unknown",
input: { path: "src/index.ts" },
output: null,
};
assert.strictEqual(hasMeaningfulToolCallDetail(detail), true);
});
it("marks running calls with no meaningful detail as pending", () => {
assert.strictEqual(
isPendingToolCallDetail({
detail: {
type: "unknown",
input: {},
output: null,
},
status: "running",
error: null,
}),
true
);
});
it("does not mark completed calls as pending", () => {
assert.strictEqual(
isPendingToolCallDetail({
detail: {
type: "unknown",
input: {},
output: null,
},
status: "completed",
error: null,
}),
false
);
});
});
@@ -0,0 +1,69 @@
import type { ToolCallDetail } from "@server/server/agent/agent-sdk-types";
function hasMeaningfulUnknownValue(value: unknown): boolean {
if (value === null || value === undefined) {
return false;
}
if (typeof value === "string") {
return value.trim().length > 0;
}
if (typeof value === "number" || typeof value === "boolean") {
return true;
}
if (Array.isArray(value)) {
return value.some(hasMeaningfulUnknownValue);
}
if (typeof value === "object") {
return Object.values(value).some(hasMeaningfulUnknownValue);
}
return true;
}
export function hasMeaningfulToolCallDetail(
detail: ToolCallDetail | undefined
): boolean {
if (!detail) {
return false;
}
switch (detail.type) {
case "shell":
return true;
case "read":
return Boolean(detail.filePath || detail.content);
case "edit":
return Boolean(
detail.filePath || detail.unifiedDiff || detail.oldString || detail.newString
);
case "write":
return Boolean(detail.filePath || detail.content);
case "search":
return detail.query.trim().length > 0;
case "worktree_setup":
return Boolean(detail.branchName || detail.worktreePath || detail.log);
case "sub_agent":
return Boolean(
detail.subAgentType ||
detail.description ||
detail.log ||
detail.actions.length > 0
);
case "plain_text":
return Boolean(detail.label || detail.text);
case "unknown":
return (
hasMeaningfulUnknownValue(detail.input) ||
hasMeaningfulUnknownValue(detail.output)
);
}
}
export function isPendingToolCallDetail(params: {
detail: ToolCallDetail | undefined;
status: "executing" | "running" | "completed" | "failed" | "canceled";
error: unknown | null | undefined;
}): boolean {
const isRunning =
params.status === "running" || params.status === "executing";
return isRunning && params.error == null && !hasMeaningfulToolCallDetail(params.detail);
}
@@ -447,7 +447,7 @@ describe("ClaudeAgentSession redesign invariants", () => {
}
});
test("applies input_json_delta updates only when buffered JSON is complete", async () => {
test("routes input_json_delta through partial parsing before buffered JSON is complete", async () => {
const session = await createSession();
const internal = session as unknown as {
mapPartialEvent: (event: Record<string, unknown>) => AgentTimelineItem[];
@@ -485,7 +485,7 @@ describe("ClaudeAgentSession redesign invariants", () => {
partial_json: "{\"command\":\"echo ",
},
},
expectedCommand: "echo seed",
expectedCommand: "echo ",
},
{
event: {
@@ -528,6 +528,45 @@ describe("ClaudeAgentSession redesign invariants", () => {
}
});
test("surfaces canonical partial tool input from input_json_delta before JSON is complete", async () => {
const session = await createSession();
const internal = session as unknown as {
mapPartialEvent: (event: Record<string, unknown>) => AgentTimelineItem[];
toolUseCache: Map<string, { input?: Record<string, unknown> }>;
};
const toolUseId = "tool-input-preview";
const index = 8;
try {
internal.mapPartialEvent({
type: "content_block_start",
index,
content_block: {
type: "tool_use",
id: toolUseId,
name: "Edit",
},
});
internal.mapPartialEvent({
type: "content_block_delta",
index,
delta: {
type: "input_json_delta",
partial_json:
"{\"file_path\":\"src/message.tsx\",\"old_string\":\"before",
},
});
expect(internal.toolUseCache.get(toolUseId)?.input).toEqual({
file_path: "src/message.tsx",
old_string: "before",
});
} finally {
await session.close();
}
});
test("maps tool_result content shapes into deterministic string output", async () => {
const session = await createSession();
const internal = session as unknown as {
@@ -39,6 +39,7 @@ import {
listClaudeCatalogModels,
type ClaudeModelFamily,
} from "./claude/model-catalog.js";
import { parsePartialJsonObject } from "./claude/partial-json.js";
import { buildToolCallDisplayModel } from "../../../shared/tool-call-display.js";
import type {
@@ -4398,15 +4399,19 @@ class ClaudeAgentSession implements AgentSession {
}
const buffer = (this.toolUseInputBuffers.get(toolId) ?? "") + partialJson;
this.toolUseInputBuffers.set(toolId, buffer);
let parsed: unknown;
try {
parsed = JSON.parse(buffer);
} catch {
const entry = this.toolUseCache.get(toolId);
const parsed = parsePartialJsonObject(buffer);
if (!entry || !parsed) {
return;
}
const entry = this.toolUseCache.get(toolId);
const normalized = this.normalizeToolInput(parsed);
if (!entry || !normalized) {
const normalized = this.normalizeToolInput(parsed.value);
if (!normalized) {
return;
}
if (!parsed.complete && Object.keys(normalized).length === 0) {
return;
}
if (this.areToolInputsEqual(entry.input ?? undefined, normalized)) {
return;
}
this.applyToolInput(entry, normalized);
@@ -4428,6 +4433,21 @@ class ClaudeAgentSession implements AgentSession {
return input;
}
private areToolInputsEqual(
left: AgentMetadata | undefined,
right: AgentMetadata
): boolean {
if (!left) {
return false;
}
const leftKeys = Object.keys(left);
const rightKeys = Object.keys(right);
if (leftKeys.length !== rightKeys.length) {
return false;
}
return rightKeys.every((key) => left[key] === right[key]);
}
private applyToolInput(entry: ToolUseCacheEntry, input: AgentMetadata): void {
entry.input = input;
if (this.isCommandTool(entry.name, input)) {
@@ -0,0 +1,60 @@
import { describe, expect, it } from "vitest";
import { parsePartialJsonObject } from "./partial-json.js";
describe("parsePartialJsonObject", () => {
it("parses complete objects", () => {
expect(
parsePartialJsonObject("{\"command\":\"pwd\",\"cwd\":\"/tmp/repo\"}")
).toEqual({
value: {
command: "pwd",
cwd: "/tmp/repo",
},
complete: true,
});
});
it("keeps partial string values without field-specific logic", () => {
expect(parsePartialJsonObject("{\"command\":\"echo ")).toEqual({
value: {
command: "echo ",
},
complete: false,
});
});
it("returns parsed prefix fields from incomplete objects", () => {
expect(
parsePartialJsonObject(
"{\"file_path\":\"src/message.tsx\",\"old_string\":\"before"
)
).toEqual({
value: {
file_path: "src/message.tsx",
old_string: "before",
},
complete: false,
});
});
it("parses nested partial values generically", () => {
expect(
parsePartialJsonObject(
"{\"payload\":{\"path\":\"src/index.ts\",\"content\":\"hello"
)
).toEqual({
value: {
payload: {
path: "src/index.ts",
content: "hello",
},
},
complete: false,
});
});
it("returns null for non-object payloads", () => {
expect(parsePartialJsonObject("\"text\"")).toBeNull();
});
});
@@ -0,0 +1,374 @@
type PartialParseResult<T> = {
value: T;
nextIndex: number;
complete: boolean;
};
function skipWhitespace(input: string, index: number): number {
let currentIndex = index;
while (currentIndex < input.length && /\s/u.test(input[currentIndex] ?? "")) {
currentIndex += 1;
}
return currentIndex;
}
function parsePartialString(
input: string,
index: number
): PartialParseResult<string> | null {
if (input[index] !== "\"") {
return null;
}
let currentIndex = index + 1;
let value = "";
while (currentIndex < input.length) {
const character = input[currentIndex];
if (character === "\"") {
return {
value,
nextIndex: currentIndex + 1,
complete: true,
};
}
if (character === "\\") {
const escapeCharacter = input[currentIndex + 1];
if (escapeCharacter === undefined) {
return {
value,
nextIndex: input.length,
complete: false,
};
}
switch (escapeCharacter) {
case "\"":
case "\\":
case "/":
value += escapeCharacter;
currentIndex += 2;
continue;
case "b":
value += "\b";
currentIndex += 2;
continue;
case "f":
value += "\f";
currentIndex += 2;
continue;
case "n":
value += "\n";
currentIndex += 2;
continue;
case "r":
value += "\r";
currentIndex += 2;
continue;
case "t":
value += "\t";
currentIndex += 2;
continue;
case "u": {
const hex = input.slice(currentIndex + 2, currentIndex + 6);
if (hex.length < 4 || !/^[0-9a-fA-F]{4}$/u.test(hex)) {
return {
value,
nextIndex: input.length,
complete: false,
};
}
value += String.fromCodePoint(Number.parseInt(hex, 16));
currentIndex += 6;
continue;
}
default:
return {
value,
nextIndex: input.length,
complete: false,
};
}
}
value += character;
currentIndex += 1;
}
return {
value,
nextIndex: input.length,
complete: false,
};
}
function parsePartialLiteral(
input: string,
index: number
): PartialParseResult<boolean | null> | null {
const remaining = input.slice(index);
if (remaining.startsWith("true")) {
return { value: true, nextIndex: index + 4, complete: true };
}
if (remaining.startsWith("false")) {
return { value: false, nextIndex: index + 5, complete: true };
}
if (remaining.startsWith("null")) {
return { value: null, nextIndex: index + 4, complete: true };
}
return null;
}
function parsePartialNumber(
input: string,
index: number
): PartialParseResult<number> | null {
const match = /^-?(?:0|[1-9]\d*)(?:\.\d+)?(?:[eE][+-]?\d+)?/u.exec(
input.slice(index)
);
if (!match?.[0]) {
return null;
}
return {
value: Number(match[0]),
nextIndex: index + match[0].length,
complete: true,
};
}
function parsePartialArray(
input: string,
index: number
): PartialParseResult<unknown[]> | null {
if (input[index] !== "[") {
return null;
}
const values: unknown[] = [];
let currentIndex = index + 1;
while (currentIndex <= input.length) {
currentIndex = skipWhitespace(input, currentIndex);
const nextCharacter = input[currentIndex];
if (nextCharacter === "]") {
return {
value: values,
nextIndex: currentIndex + 1,
complete: true,
};
}
if (nextCharacter === undefined) {
return {
value: values,
nextIndex: input.length,
complete: false,
};
}
const parsedValue = parsePartialJsonValue(input, currentIndex);
if (!parsedValue) {
return {
value: values,
nextIndex: currentIndex,
complete: false,
};
}
values.push(parsedValue.value);
currentIndex = skipWhitespace(input, parsedValue.nextIndex);
if (!parsedValue.complete) {
return {
value: values,
nextIndex: currentIndex,
complete: false,
};
}
const delimiter = input[currentIndex];
if (delimiter === ",") {
currentIndex += 1;
continue;
}
if (delimiter === "]") {
return {
value: values,
nextIndex: currentIndex + 1,
complete: true,
};
}
if (delimiter === undefined) {
return {
value: values,
nextIndex: input.length,
complete: false,
};
}
return {
value: values,
nextIndex: currentIndex,
complete: false,
};
}
return {
value: values,
nextIndex: input.length,
complete: false,
};
}
function parsePartialObject(
input: string,
index: number
): PartialParseResult<Record<string, unknown>> | null {
if (input[index] !== "{") {
return null;
}
const value: Record<string, unknown> = {};
let currentIndex = index + 1;
while (currentIndex <= input.length) {
currentIndex = skipWhitespace(input, currentIndex);
const nextCharacter = input[currentIndex];
if (nextCharacter === "}") {
return {
value,
nextIndex: currentIndex + 1,
complete: true,
};
}
if (nextCharacter === undefined) {
return {
value,
nextIndex: input.length,
complete: false,
};
}
const parsedKey = parsePartialString(input, currentIndex);
if (!parsedKey?.complete) {
return {
value,
nextIndex: parsedKey?.nextIndex ?? currentIndex,
complete: false,
};
}
currentIndex = skipWhitespace(input, parsedKey.nextIndex);
if (input[currentIndex] !== ":") {
return {
value,
nextIndex: currentIndex,
complete: false,
};
}
currentIndex = skipWhitespace(input, currentIndex + 1);
const parsedMemberValue = parsePartialJsonValue(input, currentIndex);
if (!parsedMemberValue) {
return {
value,
nextIndex: currentIndex,
complete: false,
};
}
value[parsedKey.value] = parsedMemberValue.value;
currentIndex = skipWhitespace(input, parsedMemberValue.nextIndex);
if (!parsedMemberValue.complete) {
return {
value,
nextIndex: currentIndex,
complete: false,
};
}
const delimiter = input[currentIndex];
if (delimiter === ",") {
currentIndex += 1;
continue;
}
if (delimiter === "}") {
return {
value,
nextIndex: currentIndex + 1,
complete: true,
};
}
if (delimiter === undefined) {
return {
value,
nextIndex: input.length,
complete: false,
};
}
return {
value,
nextIndex: currentIndex,
complete: false,
};
}
return {
value,
nextIndex: input.length,
complete: false,
};
}
function parsePartialJsonValue(
input: string,
index: number
): PartialParseResult<unknown> | null {
const currentIndex = skipWhitespace(input, index);
const character = input[currentIndex];
if (character === undefined) {
return null;
}
if (character === "\"") {
return parsePartialString(input, currentIndex);
}
if (character === "{") {
return parsePartialObject(input, currentIndex);
}
if (character === "[") {
return parsePartialArray(input, currentIndex);
}
if (character === "t" || character === "f" || character === "n") {
return parsePartialLiteral(input, currentIndex);
}
if (character === "-" || /\d/u.test(character)) {
return parsePartialNumber(input, currentIndex);
}
return null;
}
export function parsePartialJsonObject(
input: string
): { value: Record<string, unknown>; complete: boolean } | null {
const currentIndex = skipWhitespace(input, 0);
const parsed = parsePartialObject(input, currentIndex);
if (!parsed) {
return null;
}
return {
value: parsed.value,
complete:
parsed.complete && skipWhitespace(input, parsed.nextIndex) === input.length,
};
}
@@ -36,6 +36,22 @@ describe("claude tool-call mapper", () => {
}
});
it("maps partial running input through the same canonical detail path", () => {
const item = expectMapped(
mapClaudeRunningToolCall({
callId: "claude-call-partial-1",
name: "Bash",
input: { command: "echo " },
output: null,
})
);
expect(item.detail).toEqual({
type: "shell",
command: "echo ",
});
});
it("maps running known tool variants with detail for early summaries", () => {
const readItem = expectMapped(
mapClaudeRunningToolCall({