fix(provider): backfill Name in pairToolResults for existing tool res…#5353
Merged
SivanCola merged 2 commits intoJun 27, 2026
Merged
Conversation
…ults pairToolResults already set Name on backfill (new) tool results but left it empty when reusing an existing result. If a saved session carried a tool message without Name, SanitizeToolPairing would not repair it, and DeepSeek's API rejects tool-role messages with a missing 'name' field (HTTP 400). GLM's endpoint does not validate this, so the bug only surfaced after switching. Now set r.Name = tc.Name on both the id-matched and positional paths, matching the backfill behaviour.
Treat a tool result whose name does not match its paired tool call as malformed so SanitizeToolPairing takes the slow path and backfills the name before provider serialization. Add regression coverage for saved sessions that carried a tool result with an empty name. Co-authored-by: SivanCola <32437197+SivanCola@users.noreply.github.com>
Collaborator
|
Pushed follow-up commit What changed:
Verified:
|
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
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.
Summary
pairToolResultsalready setNameon backfilled new tool results, but left it empty when reusing an existing result. If a saved session carried a tool message withoutName,SanitizeToolPairingwould not repair it, and DeepSeek's API rejects tool-role messages with a missingnamefield (HTTP 400). GLM's endpoint does not validate this, so the bug only surfaced after switching providers.This PR now sets
r.Name = tc.Nameon both the id-matched and positional result reuse paths, matching the placeholder backfill behavior. A follow-up fix also treats tool results whoseNamedoes not match the paired tool call as malformed in the fast path, so the repair path actually runs for saved sessions with missing tool-result names.Verification
go test ./internal/provider -run TestSanitizeToolPairingBackfillsMissingToolResultName -count=1go test ./internal/provider/... -count=1go test ./...Cache impact
Cache-impact: low - only malformed histories with missing or mismatched tool-result names are rewritten immediately before provider sends; stable system prompts, tool schemas, tool order, and healthy session prefixes stay unchanged. Affected old sessions may get a one-time provider-visible message repair so DeepSeek receives the required
namefield instead of returning HTTP 400.Cache-guard:
TestSanitizeToolPairingBackfillsMissingToolResultNameplusgo test ./internal/provider/... -count=1verify the malformed-history repair path and provider package behavior.System-prompt-review: N/A - no system prompt, memory prefix, output style, or skill index behavior changed.
Authorship note
@liugh-dev is the primary author of this PR. @SivanCola contributed as a collaborator by reviewing the provider fast path, adding the follow-up regression fix, and verifying the final state.