Repository navigation
fix(continuation): detect non-Claude continuation intent; scope completion guard to final sentence - #1712
fix(continuation): detect non-Claude continuation intent; scope completion guard to final sentence#17120xghost42 wants to merge 1 commit into
Conversation
…etion guard to final sentence analyzeContinuationIntent was biased toward Claude-style output (Twigpine#1707): - The COMPLETION_MARKERS guard tested the entire response, so a marker buried mid-text ('The download is complete. Now installing the deps') suppressed an obvious later continuation intent. It now inspects only the final sentence — a genuinely completed last sentence is still suppressed, a mid-text marker no longer is. - Added action verbs common to non-Claude models (process, download, compile, deploy, install, train, refactor, ...) gated behind explicit first-person intent framing, plus present-progressive action statements ('Now downloading the dataset') as a continuation signal. Left out the issue's tuning suggestions (raising MAX_CONTINUATION_NUDGES, inverting the default shouldNudge) as product decisions.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📜 Recent review details⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🧰 Additional context used📓 Path-based instructions (10)src/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,js,jsx,py,json,md}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.test.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.test.{ts,tsx,js}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{ts,tsx,js,jsx,py}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (CONTRIBUTING.md)
Files:
**/*⚙️ CodeRabbit configuration file
Files:
{src/**/*.test.ts,src/**/*.test.tsx,tests/**,scripts/**/*.test.ts,vscode-extension/**/*.test.js}⚙️ CodeRabbit configuration file
Files:
**⚙️ CodeRabbit configuration file
Files:
{src/services/**/*.ts,src/utils/**/*.ts}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🔇 Additional comments (2)
📝 WalkthroughWalkthroughExpands ChangesContinuation detection for non-Claude models
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
Hey — nice work isolating the bug. I ran the test cases you added against the implementation and 5 of 7 fail on const { analyzeContinuationIntent } = await import('./src/utils/continuation.ts')
// expected true
analyzeContinuationIntent('The download is complete. Now I will install the dependencies').shouldNudge // false
analyzeContinuationIntent('Step one finished. I will now deploy the service').shouldNudge // false
analyzeContinuationIntent('Let me compile the project').shouldNudge // false
analyzeContinuationIntent('I will now train the model').shouldNudge // false
analyzeContinuationIntent('Now downloading the dataset').shouldNudge // false
// expected false — these pass
analyzeContinuationIntent('Finished installing the packages. All set.').shouldNudge // false ✓
analyzeContinuationIntent('Downloaded the files and the migration is complete.').shouldNudge // false ✓Likely causes:
Suggest: add the new verb list to the strong-intent regex on line 102, and promote the present-progressive match to its own early-return above the punctuation check. Then re-run the probes; all 5 should flip to Happy to push a fix to your branch if you want — just say the word. |
|
this pr appears to also try to slip in before the original reporter landed his pr at #1713 |
|
Agreed — #1707 is @rayss868's report and #1713 is their own fix for it, so this should defer to that. Closing in favor of #1713 so review can focus on one. @rayss868 — the cases I added in |
Closes #1707.
Problem
analyzeContinuationIntent(src/utils/continuation.ts) is biased toward Claude-style output, so non-Claude models (OpenAI/Gemini/local) frequently stop after a turn instead of continuing. Two concrete causes:COMPLETION_MARKERSguard tested the entire response, so a marker buried mid-text — e.g.The download is complete. Now installing the deps— suppressed an obvious later continuation intent.Fix
shouldNudge: false.I will … deploy,Let me … install,Now I'll … compile) so they only fire as a continuation signal, plus present-progressive action statements (Now downloading the dataset) for the unpunctuated/truncated case. These do not override terminal punctuation, so a punctuated completion likeFinished installing.stays suppressed by the marker guard.Out of scope (deliberately)
The issue also suggests raising
MAX_CONTINUATION_NUDGES(3 → 20) and inverting the defaultshouldNudgetotrue. Those are product/tuning decisions with broader blast radius, so they're left for a maintainer call rather than bundled here.Tests
Extended
src/__tests__/bugfixes.test.tswith mid-text-marker, intent-gated-verb, gerund, and still-suppressed-completion cases. Full file: 32 pass / 0 fail.tsc --noEmitclean;bun run smokegreen.Summary by CodeRabbit