fix(ralph-loop): keep LLM in smart zone with fresh context per iteration#1348
fix(ralph-loop): keep LLM in smart zone with fresh context per iteration#1348edxeth wants to merge 7 commits intocode-yeongyu:devfrom
Conversation
|
All contributors have signed the CLA. Thank you! ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
There was a problem hiding this comment.
4 issues found across 14 files
Confidence score: 3/5
- Some regression risk: in
src/hooks/ralph-loop/index.ts, a cancelled loop can be re-created after async session reset becausewriteStateruns without re-checking active state, which could cause user-visible loop behavior. - Several configuration/UX inconsistencies are flagged (docs and CLI/prompt strategy defaults) but have lower or uncertain confidence, so impact is less clear.
- Score 3 reflects a concrete runtime behavior risk in loop cancellation plus a few medium-severity inconsistencies that may confuse configuration.
- Pay close attention to
src/hooks/ralph-loop/index.ts,docs/features.md,src/features/builtin-commands/templates/ralph-loop.ts,src/index.ts- loop reactivation risk and strategy configuration inconsistencies.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="docs/features.md">
<violation number="1" location="docs/features.md:265">
P2: Docs use obsolete `context_strategy` key; config schema expects `default_strategy`, so users will configure an invalid option.</violation>
</file>
<file name="src/features/builtin-commands/templates/ralph-loop.ts">
<violation number="1" location="src/features/builtin-commands/templates/ralph-loop.ts:29">
P2: The prompt hard-codes the default strategy as "reset" even though `default_strategy` is configurable, so instructions become inaccurate when users set a different default.</violation>
</file>
<file name="src/hooks/ralph-loop/index.ts">
<violation number="1" location="src/hooks/ralph-loop/index.ts:350">
P2: Cancelled loop state can be recreated after async session reset because writeState runs without re-checking that the loop is still active.</violation>
</file>
<file name="src/index.ts">
<violation number="1" location="src/index.ts:490">
P2: Case-insensitive `--strategy` parsing passes through unnormalized values, so inputs like `--strategy=CONTINUE` won’t match downstream strict comparisons and can fall back to the default strategy.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/index.ts">
<violation number="1" location="src/index.ts:663">
P2: Ralph Loop argument parsing is duplicated in three places (chat.message, /ralph-loop, /ulw-loop). This duplication is already diverging (only slash commands lowercase `strategy`), which risks inconsistent behavior and missed fixes. Consider extracting shared parsing into a helper to keep behavior consistent.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
@code-yeongyu this little feat really takes OMO to another level, it would be great if we could work out a planner for ralph loop-specific SPEC files :) |
There was a problem hiding this comment.
2 issues found across 4 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/hooks/ralph-loop/index.ts">
<violation number="1" location="src/hooks/ralph-loop/index.ts:76">
P2: Reset strategy prompts do not inject the completion promise token, leaving {{COMPLETION_PROMISE}} unresolved and preventing custom completion promises from being honored.</violation>
</file>
<file name="src/index.ts">
<violation number="1" location="src/index.ts:520">
P2: Prompt sanitization was removed; raw CLI flags are now passed into ralphLoop.startLoop and become part of LLM prompts, regressing previous behavior that stripped flags from the task description.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
f0c0eab to
b21d4f8
Compare
… context per iteration - Add ContextStrategySchema with 'reset' and 'continue' options - 'reset' (default): creates new session with fresh context each iteration - 'continue': keeps same session, accumulates context - Implement TUI session switch via raw API call to /tui/select-session - Update tests and documentation This addresses context overflow in long-running Ralph loops by starting each iteration with a clean context window (smart zone).
- Rename context_strategy to default_strategy in config schema - Add --strategy=reset|continue flag to /ralph-loop and /ulw-loop commands - Store strategy in loop state for per-loop override - Parse --strategy from command arguments in index.ts - Skip todo-continuation-enforcer when ralph-loop is active - Update tests for new naming and behavior
…t_strategy config
…ation 1
- Always write strategy field to state file with fallback to DEFAULT_STRATEGY
- Reset strategy now sends full command template (not continuation prompt)
- Preserve raw user task with flags instead of parsed prompt
- Skip startLoop re-initialization when loop already active (prevents infinite loop)
- Rename initial session to 'Ralph Loop - Iteration 1' using session.update API
- Only replace {{PROMISE}} placeholder for continue strategy
Fixes reset strategy creating endless iteration code-yeongyu#2 sessions and ensures
each iteration receives the exact same message format as iteration 1.
Remove hardcoded session rename to 'Ralph Loop - Iteration 1' for the first iteration. Iteration 2+ already use dynamic naming via ralph-loop/index.ts, so this was redundant. Let OpenCode generate session names naturally for iteration 1.
The Problem: Context Window Overflow
The original ralph-loop implementation kept all iterations in a single session, causing the context window to fill up over time. This is exactly the problem described in "Why the Anthropic Ralph plugin sucks":
Every LLM has a smart zone (first ~40% of context) and a dumb zone (remaining ~60%). The original implementation guaranteed you'd enter the dumb zone after just a few iterations:
The Solution: Fresh Context Per Iteration
This PR adds a
default_strategyconfig option with "reset" as the new default:reset⭐continueWith
reset, each iteration starts with a fresh context window. The LLM always operates in the smart zone:This matches how the original bash-loop Ralph works:
Each invocation = fresh context.
Changes
Core Feature
default_strategyconfig withreset(default) andcontinueoptions--strategyflag for per-loop override:/ralph-loop "task" --strategy=continueBug Fixes
todo-continuation-enforcerwhen ralph-loop is active - prevents hook conflictsresetallows keyword detection (new session needs mode injections),continueskips it (mode already applied)Implementation
/tui/select-sessionchat.messageandPreToolUsehandlersUsage
Config (default for all loops):
{ "ralph_loop": { "enabled": true, "default_strategy": "reset" } }Command (per-loop override):
/ralph-loop "Build feature X" --strategy=continue --max-iterations=10Testing
References