fix(providers): OpenRouter reasoning-model content:null fallback - #1155
fix(providers): OpenRouter reasoning-model content:null fallback#1155dizhaky wants to merge 1 commit into
Conversation
OpenRouterProvider.call() read only message.content. Reasoning models on OpenRouter (DeepSeek V4/R1, etc.) return content:null with the answer in message.reasoning / message.reasoning_details when finish_reason:"length" (reasoning exhausts the token budget before producing final content). This caused mem::summarize chunk calls to hard-fail with empty_provider_response (absorbed by the chunk+skip+reduce layer, but wasteful ~227 retries). Fall back to reasoning_details[].text ?? message.reasoning when content is null so the chunk gets degraded-but-usable output instead of a hard throw. The reasoning text is truncated in the length case, but the chunk+reduce layer tolerates partial output better than a throw; when the model finishes (finish_reason:"stop") content is populated and the fallback is not used. Also adds a deploy-time dist patch (deploy/coolify/openrouter-reasoning-patch.mjs + Dockerfile RUN) for the coolify Docker image, which installs the pre-built npm package rather than building from source — so the fix reaches running containers before a new npm release ships the compiled source fix. Verified live on mfc1: forced content:null response (max_tokens=20 -> finish_reason:"length") -> patched extraction returns reasoning (89 chars) instead of throwing. Container rebuilt + healthy. DAN-2499 Co-Authored-By: Claude <noreply@anthropic.com>
|
@dizhaky is attempting to deploy a commit to the rohitg00's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe OpenRouter provider now falls back to reasoning fields when response content is absent. The Coolify Docker build patches compiled AgentMemory files with the same behavior. ChangesOpenRouter reasoning fallback
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/providers/openrouter.ts (1)
72-78: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove implementation comments from the TypeScript provider.
Lines 72-78 explain the code behavior. Remove them and keep the fallback control flow self-explanatory.
As per coding guidelines, “Do not add comments explaining what code does; use clear naming instead.”
🤖 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 `@src/providers/openrouter.ts` around lines 72 - 78, Remove the explanatory comment above the fallback logic in the OpenRouter provider, leaving the existing control flow unchanged and relying on its naming and structure to convey the behavior.Source: Coding guidelines
🤖 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 `@src/providers/openrouter.ts`:
- Around line 72-78: Remove the explanatory comment above the fallback logic in
the OpenRouter provider, leaving the existing control flow unchanged and relying
on its naming and structure to convey the behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4c716131-a7fe-4903-9003-60d9b85e7b3c
📒 Files selected for processing (3)
deploy/coolify/Dockerfiledeploy/coolify/openrouter-reasoning-patch.mjssrc/providers/openrouter.ts
Problem
OpenRouterProvider.call()reads onlymessage.content:Reasoning models on OpenRouter (DeepSeek V4/R1, etc.) return the final answer in
message.content, but when reasoning exhausts the token budget (finish_reason: "length"),contentisnulland onlymessage.reasoning/message.reasoning_detailsare populated. This makes the provider throw"returned unexpected response", surfacing asempty_provider_responsefrommem::summarizechunk calls. The chunk+skip+reduce layer absorbs the failures (sessions still produce valid summaries), but it wastes a retry per failing chunk — observed ~227 chunk-levelmem::summarizefailures over ~26h on one deployment.Fix
Fall back to
reasoning_details[].text ?? message.reasoningwhencontentis null, so the chunk gets degraded-but-usable output instead of a hard throw:The reasoning text is truncated in the
lengthcase, but the chunk+reduce layer tolerates partial output better than a hard throw; when the model finishes (finish_reason: "stop")contentis populated and the fallback is not used.Verification
Forced the
content:nullcase against the live OpenRouter API (deepseek-v4-pro,max_tokens=20→finish_reason: "length",content: null,reasoning_detailspopulated). The patched extraction returns the reasoning text instead of throwing. Deployed in a coolify Docker image on mfc1; container rebuilt + healthy,code_memory_consolidatesucceeds.Deploy-time dist patch (included)
The coolify Dockerfile installs the pre-built npm package (
@agentmemory/agentmemory@<ver>) rather than building from source, so this PR also addsdeploy/coolify/openrouter-reasoning-patch.mjs+ aDockerfileRUNthat applies the same fix to the bundleddist/*.mjsat image-build time. This lets existing coolify deploys get the fix before a new npm release ships the compiled source change. The source fix is the real fix; the dist patch is a stopgap for pre-built-package deploys.🤖 Generated with Claude Code
Summary by CodeRabbit