fix(coding-agent): seed tool-args reveal with the partial JSON already in hand
ToolArgsRevealController.setTarget initialized new entries with revealed=0, so the first message_update returned { __partialJson: "" } even when the provider had already parsed a complete chunk. For renderers without exposeRawPartialJson (e.g. write), the throttled re-parse + cached displayArgs short-circuited every subsequent setTarget, leaving the preview body blank until tool_execution_end.
Seed revealed with the full incoming partialJson length on entry creation (clamped to a surrogate-safe boundary). The first frame now carries the parsed path/content immediately; subsequent message_updates extend target and the reveal ticks pace only the newly arrived bytes — no field is ever truncated because the seeded prefix is the longest the entry has seen so far.
Fixes #3881
This commit is contained in:
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed streaming tool-call previews (notably `write`) showing an empty body for the entire streaming phase by surfacing the partial JSON already in hand on the first reveal, then pacing only subsequent growth ([#3881](https://github.com/can1357/oh-my-pi/issues/3881)).
|
||||
|
||||
## [16.2.7] - 2026-06-30
|
||||
|
||||
### Breaking Changes
|
||||
|
||||
@@ -144,7 +144,7 @@ export class ToolArgsRevealController {
|
||||
entry = {
|
||||
component: undefined,
|
||||
target: partialJson,
|
||||
revealed: 0,
|
||||
revealed: clampSliceEnd(partialJson, partialJson.length),
|
||||
rawInput,
|
||||
exposeRawPartialJson,
|
||||
parsedArgs: {},
|
||||
|
||||
@@ -91,26 +91,45 @@ describe("EventController paces streamed tool args", () => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("reveals partialJson prefixes per frame, then snaps to final args when the JSON closes", async () => {
|
||||
it("reveals the initial slice immediately, then paces growth across message_updates", async () => {
|
||||
await Settings.init({ inMemory: true, cwd: process.cwd() });
|
||||
vi.useFakeTimers();
|
||||
const updateArgsSpy = vi.spyOn(ToolExecutionComponent.prototype, "updateArgs");
|
||||
const content = "x".repeat(400);
|
||||
const target = `{"path":"/tmp/a.ts","content":"${content}"}`;
|
||||
const streaming = makeStreamingMessage([
|
||||
// Seed includes the complete `path` field (closing quote at byte 20) plus
|
||||
// the opening of `content`, so the rendered preview must show the real
|
||||
// path on the very first dispatch.
|
||||
const seed = target.slice(0, 35);
|
||||
|
||||
// First message_update: only a small slice has arrived. The reveal
|
||||
// MUST surface it as-is (no empty initial frame).
|
||||
const seedStreaming = makeStreamingMessage([
|
||||
{ type: "toolCall", id: "tc-1", name: "write", arguments: {}, [kStreamingPartialJson]: seed },
|
||||
]);
|
||||
const { controller, pendingTools } = createFixture(seedStreaming);
|
||||
await dispatch(controller, seedStreaming);
|
||||
expect(pendingTools.size).toBe(1);
|
||||
|
||||
// Component constructor consumes the initial render args directly; no
|
||||
// updateArgs has been invoked yet, but the seeded prefix is already on
|
||||
// the pending preview.
|
||||
const componentRender = pendingTools.get("tc-1")!.render(80).join("\n");
|
||||
expect(Bun.stripANSI(componentRender)).toContain("/tmp/a.ts");
|
||||
|
||||
// Second message_update: the rest of the payload arrives. The controller
|
||||
// paces the new backlog through reveal ticks.
|
||||
const fullStreaming = makeStreamingMessage([
|
||||
{ type: "toolCall", id: "tc-1", name: "write", arguments: {}, [kStreamingPartialJson]: target },
|
||||
]);
|
||||
const { controller, pendingTools } = createFixture(streaming);
|
||||
|
||||
await dispatch(controller, streaming);
|
||||
expect(pendingTools.size).toBe(1);
|
||||
await dispatch(controller, fullStreaming);
|
||||
|
||||
for (let i = 0; i < 3; i++) {
|
||||
vi.advanceTimersByTime(STREAMING_REVEAL_FRAME_MS);
|
||||
}
|
||||
const pacedFrames = updateArgsSpy.mock.calls.map(call => call[0] as Record<string, unknown>);
|
||||
expect(pacedFrames.length).toBeGreaterThan(0);
|
||||
let previousLength = 0;
|
||||
let previousLength = seed.length;
|
||||
for (const frame of pacedFrames) {
|
||||
const prefix = frame.__partialJson;
|
||||
if (typeof prefix !== "string") throw new Error("Expected __partialJson string on paced frame");
|
||||
@@ -171,12 +190,13 @@ describe("EventController paces streamed tool args", () => {
|
||||
]);
|
||||
const { controller, pendingTools } = createFixture(streaming);
|
||||
|
||||
// Args still streaming: the reveal seeds the preview at an empty prefix, so
|
||||
// the write head shows its `…` path placeholder rather than the real path.
|
||||
// Args still streaming, but the reveal now seeds the preview with the
|
||||
// full available partialJson on the very first message_update — so the
|
||||
// path is already visible before the tool starts executing.
|
||||
await dispatch(controller, streaming);
|
||||
const component = pendingTools.get("tc-1");
|
||||
if (!component) throw new Error("expected a pending write component");
|
||||
expect(Bun.stripANSI(component.render(80).join("\n"))).not.toContain("/tmp/exec.ts");
|
||||
expect(Bun.stripANSI(component.render(80).join("\n"))).toContain("/tmp/exec.ts");
|
||||
|
||||
// The closing full-args message_update never arrives (throttled `arguments`
|
||||
// with smoothing off, an owned-dialect projector, or a superseded turn that
|
||||
|
||||
@@ -51,26 +51,56 @@ describe("tool args reveal", () => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("reveals raw partial JSON monotonically for renderers that consume it", () => {
|
||||
vi.useFakeTimers();
|
||||
const { component, controller } = makeController();
|
||||
const content = "line one\\nline two\\nline three of a streamed write payload";
|
||||
const target = `{"path":"a.ts","content":"${content}"}`;
|
||||
it("reveals what already arrived on the first setTarget call", () => {
|
||||
const { controller } = makeController();
|
||||
const target = `{"path":"a.ts","content":"abc"}`;
|
||||
|
||||
const initial = controller.setTarget(
|
||||
"call-1",
|
||||
target,
|
||||
jsonTarget({ fullArgs: { path: "a.ts" }, exposeRawPartialJson: true }),
|
||||
);
|
||||
expect(partialOf(initial)).toBe("");
|
||||
|
||||
// The provider already delivered a complete partialJson chunk; the
|
||||
// controller MUST surface its parsed fields and raw prefix immediately —
|
||||
// pacing applies only to subsequent growth, never to bytes already in hand.
|
||||
expect(initial.path).toBe("a.ts");
|
||||
expect(initial.content).toBe("abc");
|
||||
expect(partialOf(initial)).toBe(target);
|
||||
});
|
||||
|
||||
it("paces growth across successive setTarget calls for raw-prefix renderers", () => {
|
||||
vi.useFakeTimers();
|
||||
const { component, controller } = makeController();
|
||||
const content = "line one\\nline two\\nline three of a streamed write payload";
|
||||
const target = `{"path":"a.ts","content":"${content}"}`;
|
||||
const seed = target.slice(0, 12);
|
||||
|
||||
const initial = controller.setTarget(
|
||||
"call-1",
|
||||
seed,
|
||||
jsonTarget({ fullArgs: { path: "a.ts" }, exposeRawPartialJson: true }),
|
||||
);
|
||||
// What arrived is exposed immediately, no empty initial frame.
|
||||
expect(partialOf(initial)).toBe(seed);
|
||||
controller.bind("call-1", component);
|
||||
|
||||
// More bytes arrive later — the controller now paces the new backlog.
|
||||
controller.setTarget(
|
||||
"call-1",
|
||||
target,
|
||||
jsonTarget({ fullArgs: { path: "a.ts" }, exposeRawPartialJson: true }),
|
||||
);
|
||||
drain(100);
|
||||
|
||||
const partials = component.frames.map(partialOf);
|
||||
expect(partials.length).toBeGreaterThan(0);
|
||||
expect(partials.at(-1)).toBe(target);
|
||||
for (let i = 1; i < partials.length; i++) {
|
||||
expect(partials[i].length).toBeGreaterThanOrEqual(partials[i - 1].length);
|
||||
expect(target.startsWith(partials[i])).toBe(true);
|
||||
let previous = seed.length;
|
||||
for (const partial of partials) {
|
||||
expect(partial.length).toBeGreaterThanOrEqual(previous);
|
||||
expect(target.startsWith(partial)).toBe(true);
|
||||
previous = partial.length;
|
||||
}
|
||||
});
|
||||
|
||||
@@ -79,15 +109,27 @@ describe("tool args reveal", () => {
|
||||
const requestRender = vi.fn();
|
||||
const { component, controller } = makeController({ requestRender });
|
||||
const target = `{"path":"a.ts","content":"${"x".repeat(1200)}"}`;
|
||||
const seed = target.slice(0, 10);
|
||||
|
||||
const initial = controller.setTarget("call-1", target, jsonTarget());
|
||||
expect(partialOf(initial)).toBe("");
|
||||
// Seed: small initial slice, revealed immediately, sets parsedLen=seed.length.
|
||||
const initial = controller.setTarget("call-1", seed, jsonTarget());
|
||||
expect(partialOf(initial)).toBe(seed);
|
||||
controller.bind("call-1", component);
|
||||
|
||||
// Full payload arrives; the controller paces the new growth.
|
||||
controller.setTarget("call-1", target, jsonTarget());
|
||||
expect(component.frames).toHaveLength(0);
|
||||
|
||||
// First paced tick lands inside the small-prefix window
|
||||
// (< STREAMING_JSON_PARSE_MIN_GROWTH), so a re-parse is forced and a
|
||||
// frame fires.
|
||||
drain(1);
|
||||
expect(component.frames).toHaveLength(1);
|
||||
expect(requestRender).toHaveBeenCalledTimes(1);
|
||||
const firstPartial = partialOf(component.frames[0]);
|
||||
|
||||
// The next tick crosses into the throttled window: growth from the last
|
||||
// parse hasn't yet hit STREAMING_JSON_PARSE_MIN_GROWTH, so no frame fires.
|
||||
drain(1);
|
||||
expect(component.frames).toHaveLength(1);
|
||||
expect(requestRender).toHaveBeenCalledTimes(1);
|
||||
@@ -98,21 +140,6 @@ describe("tool args reveal", () => {
|
||||
expect(secondPartial.length - firstPartial.length).toBeGreaterThanOrEqual(STREAMING_JSON_PARSE_MIN_GROWTH);
|
||||
});
|
||||
|
||||
it("keeps small JSON args visible before completion", () => {
|
||||
vi.useFakeTimers();
|
||||
const { component, controller } = makeController();
|
||||
const target = `{"path":"a.ts","content":"abc"}`;
|
||||
|
||||
controller.setTarget("call-1", target, jsonTarget());
|
||||
controller.bind("call-1", component);
|
||||
drain(20);
|
||||
|
||||
const latest = component.frames.at(-1)!;
|
||||
expect(latest.path).toBe("a.ts");
|
||||
expect(latest.content).toBe("abc");
|
||||
expect(partialOf(latest)).toBe(target);
|
||||
});
|
||||
|
||||
it("passes the full target through untouched when smoothing is disabled", () => {
|
||||
vi.useFakeTimers();
|
||||
const requestRender = vi.fn();
|
||||
@@ -132,11 +159,16 @@ describe("tool args reveal", () => {
|
||||
it("finish drops the reveal so no further frames are pushed", () => {
|
||||
vi.useFakeTimers();
|
||||
const { component, controller } = makeController();
|
||||
const target = `{"path":"a.ts","content":"${"x".repeat(400)}"}`;
|
||||
const seed = target.slice(0, 5);
|
||||
|
||||
controller.setTarget("call-1", `{"path":"a.ts","content":"abcdefghijklmnop"}`, jsonTarget());
|
||||
controller.setTarget("call-1", seed, jsonTarget());
|
||||
controller.bind("call-1", component);
|
||||
// Backlog of new bytes for the reveal loop to advance through.
|
||||
controller.setTarget("call-1", target, jsonTarget());
|
||||
drain(1);
|
||||
const frames = component.frames.length;
|
||||
expect(frames).toBeGreaterThan(0);
|
||||
controller.finish("call-1");
|
||||
drain(10);
|
||||
|
||||
@@ -147,9 +179,11 @@ describe("tool args reveal", () => {
|
||||
vi.useFakeTimers();
|
||||
const { component, controller } = makeController();
|
||||
const target = `{"path":"a.ts","content":"${"x".repeat(500)}"}`;
|
||||
const seed = target.slice(0, 5);
|
||||
|
||||
controller.setTarget("call-1", target, jsonTarget());
|
||||
controller.setTarget("call-1", seed, jsonTarget());
|
||||
controller.bind("call-1", component);
|
||||
controller.setTarget("call-1", target, jsonTarget());
|
||||
drain(1);
|
||||
expect(partialOf(component.frames.at(-1)!).length).toBeLessThan(target.length);
|
||||
controller.flushAll();
|
||||
@@ -164,9 +198,11 @@ describe("tool args reveal", () => {
|
||||
vi.useFakeTimers();
|
||||
const { component, controller } = makeController();
|
||||
const target = `{"content":"${"😀🎉".repeat(40)}"}`;
|
||||
const seed = target.slice(0, 12); // before any surrogate
|
||||
|
||||
controller.setTarget("call-1", target, jsonTarget({ exposeRawPartialJson: true }));
|
||||
controller.setTarget("call-1", seed, jsonTarget({ exposeRawPartialJson: true }));
|
||||
controller.bind("call-1", component);
|
||||
controller.setTarget("call-1", target, jsonTarget({ exposeRawPartialJson: true }));
|
||||
drain(100);
|
||||
|
||||
expect(partialOf(component.frames.at(-1)!)).toBe(target);
|
||||
@@ -179,11 +215,16 @@ describe("tool args reveal", () => {
|
||||
vi.useFakeTimers();
|
||||
const { component, controller } = makeController();
|
||||
const target = "*** Begin Patch\n*** Update File: a.ts\n-old\n+new\n*** End Patch";
|
||||
const seed = target.slice(0, 5);
|
||||
|
||||
controller.setTarget("call-1", target, rawTarget({ input: target }));
|
||||
const initial = controller.setTarget("call-1", seed, rawTarget({ input: seed }));
|
||||
expect(initial.input).toBe(seed);
|
||||
expect(partialOf(initial)).toBe(seed);
|
||||
controller.bind("call-1", component);
|
||||
controller.setTarget("call-1", target, rawTarget({ input: target }));
|
||||
drain(100);
|
||||
|
||||
expect(component.frames.length).toBeGreaterThan(0);
|
||||
for (const frame of component.frames) {
|
||||
expect(frame.input).toBe(partialOf(frame));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user