fix: auto-resolve tool-call stream drop (#475) + JSON workspace-registry collision (#469) - #480
Draft
TYRMars wants to merge 2 commits into
Draft
fix: auto-resolve tool-call stream drop (#475) + JSON workspace-registry collision (#469)#480TYRMars wants to merge 2 commits into
TYRMars wants to merge 2 commits into
Conversation
StreamAccumulator.#finalise dropped any real tool call whose streamed
deltas never carried an `id`, since the guard required both `id` and
`name`. OpenAI-compatible backends (Kimi/Ollama) commonly stream
`{index, function:{name, arguments}}` with no id, so the call was
silently discarded while finish_reason stayed "tool_calls" — the turn
ended with an empty assistant message and the tool never ran, whereas
the same backend worked over non-streaming /v1/chat/completions.
Skip only placeholder padding slots (no name) and synthesise
`call_<index>` when the id is missing, mirroring the Google provider's
`gem_<index>`. Adds regression tests for the missing-id and padding-slot
cases.
Fixes #475
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AjcBxxZuarAPFtYuz7CsaC
JsonFileWorkspaceStore wrote workspaces.json flat into the base dir it shares with JsonFileConversationStore's <id>.json rows. JsonFileConversation Store.list accepted any *.json and dereferenced stored.messages.length, so after the first POST /v1/workspaces (or workspaces.bind on resume) the stray workspaces.json crashed GET /v1/conversations permanently — the SPA sidebar went dead for the lifetime of the data dir. The reverse collision let a conversation with id "workspaces" clobber the registry. - Move the registry to <base>/workspaces/workspaces.json, matching every other domain store (projects/, workflows/, …). A legacy flat file is migrated in on first open (new file written before the old one is removed, so a mid-migration crash can't lose it). - Harden JsonFileConversationStore.list to skip any *.json without a messages array, so a stray/legacy non-conversation file can never crash the sidebar again. Regression tests for the migration, the new subdir layout, and the conversation-store guard. Fixes #469 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AjcBxxZuarAPFtYuz7CsaC
TYRMars
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Auto-resolves two open bug reports. Both fixes are isolated to their respective packages with regression tests.
Fixes #475 —
@jarvis/llmstreaming tool-call dropStreamAccumulator.#finalise(packages/llm/src/openai.ts) reconstructed tool calls behind a guard requiring both anidand aname. The guard was only meant to skip placeholder padding slots, but it also silently discarded any real tool call whose streamed deltas never carried anid. OpenAI-compatible backends (Kimi/Ollama, explicitly in scope) commonly stream{index, function:{name, arguments}}with noid, so the call vanished whilefinish_reasonstayed"tool_calls"— the turn ended with an empty assistant message and the tool never ran, whereas the same backend worked over non-streamingPOST /v1/chat/completions.Change: skip only slots that never carried a
name; synthesisecall_<index>when theidis missing (mirrors the Google provider'sgem_<index>). Adds regression tests for the missing-id and padding-slot cases.Fixes #469 —
@jarvis/storeJSON workspace-registry collisionJsonFileWorkspaceStorewroteworkspaces.jsonflat into the base dir it shares withJsonFileConversationStore's<id>.jsonrows.JsonFileConversationStore.listaccepted any*.jsonand dereferencedstored.messages.length, so after the firstPOST /v1/workspaces(orworkspaces.bindon resume) the stray file crashedGET /v1/conversationspermanently — the SPA sidebar went dead for the lifetime of the data dir. The reverse collision let a conversation with id"workspaces"clobber the registry.Change:
<base>/workspaces/workspaces.json, matching every other domain store (projects/,workflows/, …). A legacy flat file is migrated in on first open — the new file is written before the old one is removed, so a mid-migration crash can't lose it.JsonFileConversationStore.listto skip any*.jsonwithout amessagesarray, so a stray/legacy non-conversation file can never crash the sidebar again.Verification (local, all green)
pnpm --filter @jarvis/llm test— 158/158pnpm --filter @jarvis/store test— 438/438 (incl. SQLite once the native binding is built)@jarvis/llm,@jarvis/store) — cleaneslint packages/llm packages/store— clean🤖 Generated with Claude Code
https://claude.ai/code/session_01AjcBxxZuarAPFtYuz7CsaC