Conversation
fix: don't abort on thinking-only responses from reasoning models
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughPackage version bumped to 0.260330.13. rlmLoop’s empty-response handling now treats empty text with output tokens as "thinking-only" (doesn't increment consecutiveEmpty or trigger warnings); only responses with empty text and zero output tokens count toward abort logic. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b81b4f57fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (response.usage.outputTokens > 0) { | ||
| // Thinking-only iteration — model is reasoning, not stuck | ||
| if (opts.verbose) { | ||
| logVerbose(iteration, "thinking-only response (no visible text yet)"); | ||
| } |
There was a problem hiding this comment.
Reset empty-response streak for thinking-only replies
When responseText.length === 0 and outputTokens > 0, this branch treats the turn as non-empty but does not clear consecutiveEmpty. That means a sequence like empty(0 tokens) → thinking-only(>0 tokens) → empty(0 tokens) still advances the "consecutive" counter and can trigger the 3-empty abort even though the empties were interrupted by a valid thinking turn. In rlmLoop, this can prematurely terminate runs for reasoning models that interleave hidden-thinking and blank-text responses.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/rlm.ts (1)
302-305: Remove duplicate comment.Line 302 and 303 contain identical comments. The second one (line 303) appears to be a copy-paste artifact.
🧹 Proposed fix
// Check for empty LLM response (issue `#14`) - // Check for empty LLM response (issue `#14`) // Thinking-only responses (output tokens > 0 but no visible text) are normal // for reasoning models warming up — don't count as empty.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/rlm.ts` around lines 302 - 305, Remove the duplicated comment line "// Check for empty LLM response (issue `#14`)" that appears twice in src/rlm.ts (the redundant copy directly above the block about "Thinking-only responses..."); keep a single instance of that comment immediately before the explanation about thinking-only responses so the comment is not repeated. Ensure only one "// Check for empty LLM response (issue `#14`)" remains and surrounding comments/spacing are preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/rlm.ts`:
- Around line 302-305: Remove the duplicated comment line "// Check for empty
LLM response (issue `#14`)" that appears twice in src/rlm.ts (the redundant copy
directly above the block about "Thinking-only responses..."); keep a single
instance of that comment immediately before the explanation about thinking-only
responses so the comment is not repeated. Ensure only one "// Check for empty
LLM response (issue `#14`)" remains and surrounding comments/spacing are
preserved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0f453ba3-9f45-4edd-9af8-b61774949db8
📒 Files selected for processing (3)
package.jsonsrc/rlm.tssrc/version.ts
…hinking fix: reset empty streak on thinking-only responses
Rolling Promotion PR
Auto-maintained rolling promotion PR from `dev` to `main`.
Process:
Summary by CodeRabbit
Chores
0.260330.13Bug Fixes