Repository navigation
fix(chat): reasoning_content parsing, image passthrough, model-change race - #104
Conversation
… race - Parse delta.reasoning_content ?? delta.reasoning in the desktop chat loop, coding-agent loop, and subagent dispatch loop (streaming + non-stream), and strip reasoning_content before resend so Qwen/DeepSeek/GLM thinking shows in the panel and doesn't trigger 400s on resend. - Always forward attached images to the model instead of guessing vision support from a hardcoded model-name allow-list; images now also flow through the subagent dispatch path. Fixes images being silently dropped for custom endpoints and vision-capable on-device models (e.g. Gemma). - Prefer the live in-memory aiConfig over the backend cache on refresh hydrates so a model change applies to the next message (no "clear twice").
|
Warning Review limit reached
Next review available in: 53 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughChangesChat runtime fixes
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
calculatePromptBreakdown stringified the whole multimodal message content, tokenising the image's base64 data URL and inflating the conversation count / context ring by orders of magnitude. Count only text parts and add a flat per-image estimate instead, matching mobile's messageText() behaviour.
|
@coderabbitai please review |
|
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@electron/lib/token-breakdown-images.test.ts`:
- Around line 51-64: Update the test around calculatePromptBreakdown to compare
the image-bearing message with an equivalent text-only message, and assert that
the image-bearing conversation count is greater by the expected nonzero flat
image cost. Preserve the existing payload-size comparison so the test verifies
both nonzero contribution and independence from base64 payload length.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 76555151-886e-4b78-b1b9-96c65067790d
📒 Files selected for processing (8)
changelogs/v2.5.17.mdelectron/ipc/chat.tselectron/lib/chat-loop.tselectron/lib/chat-subagent-loop.tselectron/lib/llm.tselectron/lib/pi-agent-loop.tselectron/lib/token-breakdown-images.test.tssrc/store/index.ts
1 participant
What does this PR do?
Fixes three chat issues on desktop: (1) Qwen/DeepSeek/GLM-style reasoning is now parsed from
reasoning_contentand shown in the thinking panel; (2) attached images are always forwarded to the model instead of being stripped by a hardcoded vision allow-list; and (3) a stale-cache race that made a model change require clearing the chat twice.Type of change
Screenshots / recording
Not applicable — no UI changes.
Details
1.
reasoning_contentparsing (Qwen/DeepSeek/GLM)The desktop stream parsers only read
delta.reasoning, but these models emit thinking ondelta.reasoning_content(andmessage.reasoning_contenton non-streaming responses). Now readsreasoning_content ?? reasoningin:electron/lib/chat-loop.ts— streaming + non-streamelectron/lib/pi-agent-loop.ts— streamingelectron/lib/chat-subagent-loop.ts— non-stream dispatchBoth
reasoningandreasoning_contentare stripped before assistant messages are re-sent (re-sendingreasoning_contenttriggers 400s on DeepSeek). Matches the existing mobile behaviour inmobile/src/chat/providers/openai.ts.2. Image passthrough
electron/ipc/chat.tspreviously guessed vision support from a hardcoded model-name allow-list (gpt-4o,claude-3,gemini, …) and silently dropped images for anything else — including custom OpenAI-compatible endpoints and vision-capable on-device models like Gemma. The gate is removed; images are always forwarded and the model decides. The llama.cpp local router forwardscontentverbatim, so vision-capable on-device models get theimage_urlparts. Images now also flow through the subagent dispatch path, which previously dropped them.3. Model-change race ("clear chat twice")
A refresh
hydrateFromElectron(fired after write-tool turns and everydb:changed) read the backend settings cache and layered it over localStorage. Because the model-change write is fire-and-forget, a refresh firing before it landed reverted the selection. On refresh hydrates the live in-memoryaiConfignow takes precedence over the backend cache (src/store/index.ts); the initial hydrate still trusts the cache.Checklist
npm run type-check:allpassesnpm run lintpassesnpm testpasses (runsnpm run compilefirst soelectron/bundle-guard.test.tsactually executes —npm run test:bundleto run just that)npm run test:e2epasses (run before merging UI changes or cutting a release)text-[Npx]pixel font classes (no styling changes)handle()and returnIpcResult<T>— N/A, no new handlersschema.ts— N/Aelectron/db/queries.ts— N/Adependencies/devDependencies— N/A--external:<pkg>compile flags — unchangedNotes for reviewer
db:changedrefresh — an acceptable trade for fixing the revert bug, since the initial hydrate still reads the cache.npm test/npm run test:e2enot run locally in this session — please confirm in CI.Summary by CodeRabbit
Bug Fixes
Tests