fix: skip Claude's <synthetic> placeholder when reading transcript models - #1056
fix: skip Claude's <synthetic> placeholder when reading transcript models#1056MetalZealot wants to merge 2 commits into
Conversation
…cript models After an API error (e.g. 529 Overloaded), Claude Code appends assistant rows stamped model "<synthetic>" (the error notice, "No response requested.", session-limit messages). The transcript scanner adopted that placeholder as the session's model, and since transcript-sourced models feed the send path, the next turn was sent with model "<synthetic>" and failed with "There's an issue with the selected model (<synthetic>)" until the user switched surfaces. extractClaudeEventModel now rejects angle-bracketed placeholder values for both event.model and message.model, so the backward scan continues to the last genuine turn and recovers the session's real model. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughClaude model extraction now ignores synthetic ChangesClaude model filtering
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…iteboon#1056 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
server/modules/providers/tests/claude-models.test.ts (1)
90-105: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover placeholders in
event.modeltoo.The test currently validates only
message.model; addmodel: '<synthetic>'to one synthetic row so both changed extraction paths are protected.Proposed test update
{ type: 'assistant', sessionId: PROVIDER_SESSION_ID, + model: '<synthetic>', message: { model: '<synthetic>', content: [{ type: 'text', text: 'API Error: 529 Overloaded.' }],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/modules/providers/tests/claude-models.test.ts` around lines 90 - 105, Update the synthetic assistant event fixtures in the test to include model: '<synthetic>' on one row, while retaining message.model coverage on another row, so extraction from both event.model and message.model is validated.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@server/modules/providers/tests/claude-models.test.ts`:
- Around line 90-105: Update the synthetic assistant event fixtures in the test
to include model: '<synthetic>' on one row, while retaining message.model
coverage on another row, so extraction from both event.model and message.model
is validated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 37ebf5f8-de5e-45c0-a5de-72cd3a8fd748
📒 Files selected for processing (2)
server/modules/providers/list/claude/claude-models.provider.tsserver/modules/providers/tests/claude-models.test.ts
CodeRabbit review on siteboon#1056: the guard checks both event.model and message.model, but the fixtures only set message.model. Stamp the 529 row with a top-level placeholder too so both paths are exercised. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Mirrors the CodeRabbit-suggested coverage pushed to PR siteboon#1056 (351f987 on fix/synthetic-model-guard): the guard checks both event.model and message.model, but fixtures only set message.model. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Hello @MetalZealot, thanks for the PR. I tried replicating the issue but it doesn't seem to occur. Can you check and verify again? If so, can you share a more clear reproduction step? |
|
Thanks for checking. I re-verified this against current The trigger is narrower than “any 529”: it occurs only when Claude Code appends a locally generated assistant row whose A deterministic reproduction is:
{"type":"assistant","sessionId":"<session-id>","message":{"model":"claude-sonnet-5","content":[]}}
{"type":"assistant","sessionId":"<session-id>","message":{"model":"<synthetic>","content":[{"type":"text","text":"You've hit your session limit"}]}}
Actual on current main: the active model resolves to Expected: the scan skips that placeholder row and keeps walking back to The same rows are produced naturally by session-limit notices, some API/auth errors, and I also merged this PR locally into current upstream npx tsx --tsconfig server/tsconfig.json --test server/modules/providers/tests/claude-models.test.tsAll 3 tests pass on the current merge result. |
Problem
After an API error (e.g. 529 Overloaded), Claude Code appends locally-fabricated assistant rows to the session transcript stamped with the placeholder model
"<synthetic>"— the API-error notice,"No response requested.", and session-limit messages.extractClaudeEventModelaccepts whatever model the last assistant row carries, so the backward transcript scan ingetCurrentActiveModeladopts the placeholder and reports<synthetic>as the session's active model until a genuine turn lands.Repro
"model": "<synthetic>"to the session JSONL.<synthetic>instead of the model the session was actually running.Fix
extractClaudeEventModelnow rejects angle-bracketed placeholder values for bothevent.modelandmessage.model, so the scan keeps walking back to the last genuine turn and recovers the session's real model. A transcript containing only synthetic rows falls back to the catalog default, same as before when no model was found.Tests
New
server/modules/providers/tests/claude-models.test.ts(node:test, same isolated-database harness asopencode-sessions.test.ts):Run with
npx tsx --tsconfig server/tsconfig.json --test server/modules/providers/tests/claude-models.test.ts— 3/3 pass with the fix; the two guard tests fail on currentmain.tsc --noEmit -p server/tsconfig.jsonand eslint are clean.🤖 Generated with Claude Code
Summary by CodeRabbit