Deduplicate replayed tool calls
This commit is contained in:
@@ -1118,6 +1118,87 @@ function testOutOfOrderToolCallMergingHydrated() {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function buildMetadataReplaySequence(provider: ToolCallProvider) {
|
||||||
|
const timestamps = [
|
||||||
|
new Date('2025-01-01T14:00:00Z'),
|
||||||
|
new Date('2025-01-01T14:00:02Z'),
|
||||||
|
new Date('2025-01-01T14:00:04Z'),
|
||||||
|
];
|
||||||
|
const firstCallId = `${provider}-metadata-1`;
|
||||||
|
const secondCallId = `${provider}-metadata-2`;
|
||||||
|
return [
|
||||||
|
{
|
||||||
|
event: toolTimeline(
|
||||||
|
'shell',
|
||||||
|
'completed',
|
||||||
|
{ type: 'tool_result', provider, tool_call_id: firstCallId },
|
||||||
|
{ provider, server: 'command', tool: 'shell', callId: firstCallId, displayName: 'Run first' }
|
||||||
|
),
|
||||||
|
timestamp: timestamps[0],
|
||||||
|
},
|
||||||
|
{
|
||||||
|
event: toolTimeline(
|
||||||
|
'shell',
|
||||||
|
'completed',
|
||||||
|
{ type: 'tool_result', provider, tool_call_id: secondCallId },
|
||||||
|
{ provider, server: 'command', tool: 'shell', callId: secondCallId, displayName: 'Run second' }
|
||||||
|
),
|
||||||
|
timestamp: timestamps[1],
|
||||||
|
},
|
||||||
|
{
|
||||||
|
event: toolTimeline(
|
||||||
|
'shell',
|
||||||
|
'executing',
|
||||||
|
{ type: 'tool_use', provider },
|
||||||
|
{ provider, server: 'command', tool: 'shell', callId: null, displayName: 'Run first', kind: 'execute' }
|
||||||
|
),
|
||||||
|
timestamp: timestamps[2],
|
||||||
|
},
|
||||||
|
];
|
||||||
|
}
|
||||||
|
|
||||||
|
function validateMetadataReplayDeduplication(
|
||||||
|
updates: Array<{ event: AgentStreamEventPayload; timestamp: Date }>,
|
||||||
|
mode: 'live' | 'hydrated'
|
||||||
|
) {
|
||||||
|
const finalState =
|
||||||
|
mode === 'live'
|
||||||
|
? updates.reduce<StreamItem[]>((state, { event, timestamp }) => {
|
||||||
|
return reduceStreamUpdate(state, event, timestamp);
|
||||||
|
}, [])
|
||||||
|
: hydrateStreamState(updates);
|
||||||
|
|
||||||
|
const toolCalls = finalState.filter(
|
||||||
|
(item): item is ToolCallItem => item.kind === 'tool_call' && item.payload.source === 'agent'
|
||||||
|
);
|
||||||
|
|
||||||
|
return toolCalls;
|
||||||
|
}
|
||||||
|
|
||||||
|
function testMetadataReplayDeduplicationLive() {
|
||||||
|
(['claude', 'codex'] as const).forEach((provider) => {
|
||||||
|
const toolCalls = validateMetadataReplayDeduplication(
|
||||||
|
buildMetadataReplaySequence(provider),
|
||||||
|
'live'
|
||||||
|
);
|
||||||
|
assert.strictEqual(toolCalls.length, 2, `${provider} live replay should not add duplicate tool pills`);
|
||||||
|
assert.strictEqual(toolCalls[0]?.payload.data.status, 'completed', `${provider} live replay should keep the original completion status`);
|
||||||
|
assert.strictEqual(toolCalls[1]?.payload.data.status, 'completed', `${provider} live replay should keep later completion intact`);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
function testMetadataReplayDeduplicationHydrated() {
|
||||||
|
(['claude', 'codex'] as const).forEach((provider) => {
|
||||||
|
const toolCalls = validateMetadataReplayDeduplication(
|
||||||
|
buildMetadataReplaySequence(provider),
|
||||||
|
'hydrated'
|
||||||
|
);
|
||||||
|
assert.strictEqual(toolCalls.length, 2, `${provider} hydration replay should not add duplicate tool pills`);
|
||||||
|
assert.strictEqual(toolCalls[0]?.payload.data.status, 'completed', `${provider} hydration replay should keep the original completion status`);
|
||||||
|
assert.strictEqual(toolCalls[1]?.payload.data.status, 'completed', `${provider} hydration replay should keep later completion intact`);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
describe('stream timeline reducers', () => {
|
describe('stream timeline reducers', () => {
|
||||||
it('produces deterministic hydration results', testIdempotentReduction);
|
it('produces deterministic hydration results', testIdempotentReduction);
|
||||||
it('deduplicates pending/completed tool entries in place', testUserMessageDeduplication);
|
it('deduplicates pending/completed tool entries in place', testUserMessageDeduplication);
|
||||||
@@ -1139,4 +1220,6 @@ describe('stream timeline reducers', () => {
|
|||||||
it('deduplicates hydrated tool call entries', testToolCallDeduplicationHydrated);
|
it('deduplicates hydrated tool call entries', testToolCallDeduplicationHydrated);
|
||||||
it('merges out-of-order tool call updates without duplicating entries (live)', testOutOfOrderToolCallMergingLive);
|
it('merges out-of-order tool call updates without duplicating entries (live)', testOutOfOrderToolCallMergingLive);
|
||||||
it('merges out-of-order tool call updates without duplicating entries (hydrated)', testOutOfOrderToolCallMergingHydrated);
|
it('merges out-of-order tool call updates without duplicating entries (hydrated)', testOutOfOrderToolCallMergingHydrated);
|
||||||
|
it('replays metadata-only tool calls without duplicating entries (live)', testMetadataReplayDeduplicationLive);
|
||||||
|
it('replays metadata-only tool calls without duplicating entries (hydrated)', testMetadataReplayDeduplicationHydrated);
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -341,10 +341,6 @@ function findExistingAgentToolCallIndex(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if (!fallbackCandidates.length) {
|
|
||||||
return -1;
|
|
||||||
}
|
|
||||||
|
|
||||||
const normalizedDisplayName = normalizeComparableString(data.displayName);
|
const normalizedDisplayName = normalizeComparableString(data.displayName);
|
||||||
const normalizedKind = normalizeComparableString(data.kind);
|
const normalizedKind = normalizeComparableString(data.kind);
|
||||||
|
|
||||||
|
|||||||
@@ -11,7 +11,8 @@
|
|||||||
- Added a Vitest integration in `packages/server/src/server/agent/providers/claude-agent.test.ts` that drives a live Claude session through real Bash/write/read tool calls, converts the emitted timeline into the UI stream via `hydrateStreamState`, tears the agent down, resumes it from `~/.claude/projects/...`, and asserts that the hydrated stream reproduces the command output, file diff, and read content instead of leaving spinners. The helper waits for the persisted `.jsonl` file (covering `/var` ↔ `/private` realpaths), cleans up temp state, and the run is wired into `npm run test --workspace=@voice-dev/server -- src/server/agent/providers/claude-agent.test.ts` (passes locally).
|
- Added a Vitest integration in `packages/server/src/server/agent/providers/claude-agent.test.ts` that drives a live Claude session through real Bash/write/read tool calls, converts the emitted timeline into the UI stream via `hydrateStreamState`, tears the agent down, resumes it from `~/.claude/projects/...`, and asserts that the hydrated stream reproduces the command output, file diff, and read content instead of leaving spinners. The helper waits for the persisted `.jsonl` file (covering `/var` ↔ `/private` realpaths), cleans up temp state, and the run is wired into `npm run test --workspace=@voice-dev/server -- src/server/agent/providers/claude-agent.test.ts` (passes locally).
|
||||||
- [x] Hydrated Claude tool calls still show the spinner forever after refreshing a chat; Claude CLI already persists the tool payloads under `~/.claude/projects/<conversation>`, but our hydrate path isn’t loading them. Fix the loader so it reads the saved diffs/read/output blocks and transitions the existing pill to “completed” instead of spinning or duplicating.
|
- [x] Hydrated Claude tool calls still show the spinner forever after refreshing a chat; Claude CLI already persists the tool payloads under `~/.claude/projects/<conversation>`, but our hydrate path isn’t loading them. Fix the loader so it reads the saved diffs/read/output blocks and transitions the existing pill to “completed” instead of spinning or duplicating.
|
||||||
- Claude history entries now flow through a shared `convertClaudeHistoryEntry` helper that inspects every message for `tool_*` blocks before classifying it as a plain user message, so persisted `tool_result` lines recorded as `type: "user"` convert into completed tool-call events instead of leaving the original spinner stuck in `executing`. Added unit coverage in `packages/server/src/server/agent/providers/claude-agent.test.ts` proving user tool results hydrate correctly and ran `npm run test --workspace=@voice-dev/server -- src/server/agent/providers/claude-agent.test.ts` (passes, though existing SDK integration specs are still slow/flaky by nature).
|
- Claude history entries now flow through a shared `convertClaudeHistoryEntry` helper that inspects every message for `tool_*` blocks before classifying it as a plain user message, so persisted `tool_result` lines recorded as `type: "user"` convert into completed tool-call events instead of leaving the original spinner stuck in `executing`. Added unit coverage in `packages/server/src/server/agent/providers/claude-agent.test.ts` proving user tool results hydrate correctly and ran `npm run test --workspace=@voice-dev/server -- src/server/agent/providers/claude-agent.test.ts` (passes, though existing SDK integration specs are still slow/flaky by nature).
|
||||||
- [ ] We still see duplicate tool call pills (loading + completed/failed) in both Codex and Claude sessions, live and hydrated. Track tool call IDs rigorously, dedupe pending/completed entries, and add regression tests covering all providers and hydrate flows.
|
- [x] We still see duplicate tool call pills (loading + completed/failed) in both Codex and Claude sessions, live and hydrated. Track tool call IDs rigorously, dedupe pending/completed entries, and add regression tests covering all providers and hydrate flows.
|
||||||
|
- Hardened the timeline reducer (`packages/app/src/types/stream.ts`) so out-of-order tool events reconcile via provider/server/tool metadata even when call IDs are missing, statuses only move forward (executing → completed/failed), and replayed hydration updates don’t spawn extra pills; added Vitest coverage in `packages/app/src/types/stream.test.ts` proving both Codex and Claude live + hydrated streams stay deduped when completion blocks precede their pending counterparts. Tests run via `npx vitest packages/app/src/types/stream.test.ts`.
|
||||||
- [ ] Permission request cards in the stream header are emitting duplicate key warnings—derive stable per-agent keys (for example `${agentId}:${request.id}` with fallbacks) so React stops complaining.
|
- [ ] Permission request cards in the stream header are emitting duplicate key warnings—derive stable per-agent keys (for example `${agentId}:${request.id}` with fallbacks) so React stops complaining.
|
||||||
- [ ] The so-called tests are still fake: convert the stream harness into real Vitest suites co-located with the code, make `npm test` (Vitest) the single entrypoint, and ensure those tests fail today because hydrated tool-call results are missing.
|
- [ ] The so-called tests are still fake: convert the stream harness into real Vitest suites co-located with the code, make `npm test` (Vitest) the single entrypoint, and ensure those tests fail today because hydrated tool-call results are missing.
|
||||||
- [ ] Hydrated sessions continue to drop user messages; capture this in a failing test for both Codex and Claude (hydrate snapshot + live replay) and only mark complete once the test passes.
|
- [ ] Hydrated sessions continue to drop user messages; capture this in a failing test for both Codex and Claude (hydrate snapshot + live replay) and only mark complete once the test passes.
|
||||||
|
|||||||
Reference in New Issue
Block a user