fix(exit-detection): use explicit EXIT_SIGNAL instead of confidence threshold

[frankbria]

Summary

• Fix premature exit after exactly 5 loops in JSON output mode
• Replace confidence-based heuristic with explicit EXIT_SIGNAL checking
• Add 4 TDD tests validating the fix

Problem

In JSON output mode, update_exit_signals() used a confidence threshold (≥60) to populate completion_indicators. JSON mode always has confidence ≥70 due to deterministic scoring:

• +50 for successful JSON parsing
• +20 for having a result field

This caused every successful JSON response to increment completion_indicators, triggering the safety circuit breaker after exactly 5 loops—even when Claude explicitly set EXIT_SIGNAL: false.

Solution

Replace confidence-based logic in update_exit_signals() with explicit EXIT_SIGNAL checking:

# Before (buggy)
local confidence=$(jq -r '.analysis.confidence_score' "$analysis_file")
if [[ $confidence -ge 60 ]]; then
    signals=$(echo "$signals" | jq ".completion_indicators += [$loop_number]")
fi

# After (fixed)
local exit_signal=$(jq -r '.analysis.exit_signal // false' "$analysis_file")
if [[ "$exit_signal" == "true" ]]; then
    signals=$(echo "$signals" | jq ".completion_indicators += [$loop_number]")
fi

Test plan

• Test 32: Verifies completion_indicators NOT updated when exit_signal=false
• Test 33: Verifies completion_indicators IS updated when exit_signal=true
• Test 34: Verifies accumulation only on exit_signal=true
• Test 35: Simulates 5 JSON mode loops with exit_signal=false (exact bug scenario)
• All 424 tests pass (100% pass rate)

Summary by CodeRabbit

Release Notes v0.11.1

• Bug Fixes

  • Fixed completion indicator detection to use explicit exit signals instead of confidence thresholds, reducing premature exits and improving reliability.

• Tests

  • Added 4 new unit tests validating exit signal detection and safety circuit breaker behavior.

• Chores

  • Version bumped to v0.11.1.
  • Updated documentation reflecting behavioral improvements.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

The PR replaces a confidence-threshold heuristic with explicit EXIT_SIGNAL checking for completion indicator accumulation. The core logic change shifts from checking confidence_score >= 60 to reading analysis.exit_signal, with corresponding documentation and safety message updates plus new test coverage for the revised behavior.

Changes

Cohort / File(s)	Summary
Documentation Updates CLAUDE.md	Version bumped to v0.11.1; test matrix updated from 20 to 35 tests for test_exit_detection.bats; added "Completion Indicators Fix" section explaining shift from confidence threshold to explicit EXIT_SIGNAL checking.
Core Logic Changes lib/response_analyzer.sh	Modified update_exit_signals function: replaced confidence_score >= 60 check with explicit exit_signal == "true" condition to determine when completion_indicators accumulate.
Safety Circuit Messaging ralph_loop.sh	Updated should_exit_gracefully function messages: changed references from "completion indicators" heuristic to "EXIT_SIGNAL=true responses" to clarify that accumulation occurs only when exit_signal is explicitly true.
Test Coverage tests/unit/test_exit_detection.bats	Added Tests 32–35: verify that completion_indicators only increment when exit_signal=true, test cumulative behavior with mixed true/false signals, and validate behavior in JSON-mode scenarios with high confidence but exit_signal=false.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• PR #47: Adds the exit_signal field to analysis data and JSON-output parsing in response_analyzer.sh, directly enabling the explicit signal checking introduced here.
• PR #62: Modifies response_analyzer.sh's exit_signal/completion detection logic with JSON parsing that populates the exit_signal field leveraged by this PR's update logic.
• PR #90: Updates both response_analyzer.sh and ralph_loop.sh to respect explicit EXIT_SIGNAL handling and adds corresponding test coverage for the same behavior change.

Poem

🐰 No more guessing by confidence's hue,
Exit signals now guide us true!
Completion waits for Claude's intent,
No premature exits to lament! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely summarizes the main change: replacing a confidence-threshold heuristic with explicit EXIT_SIGNAL checking in exit detection logic.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/completion-indicators-exit-signal

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[macroscopeapp]

Switch exit detection to explicit bash lib/response_analyzer.sh:update_exit_signals EXIT_SIGNAL=true checks and update ralph_loop.sh:should_exit_gracefully warning to reference 5 consecutive EXIT_SIGNAL=true responses

Replace confidence threshold logic with explicit EXIT_SIGNAL checks in update_exit_signals and update warning text; add unit tests validating EXIT_SIGNAL-driven accumulation. See lib/response_analyzer.sh, ralph_loop.sh, and tests/unit/test_exit_detection.bats.

📍Where to Start

Start with update_exit_signals in lib/response_analyzer.sh, then review should_exit_gracefully in ralph_loop.sh and the new tests in tests/unit/test_exit_detection.bats.

---

Macroscope summarized 6485b05.

[github-actions]

Code Review: Fix Exit Detection

This is a well-executed fix for the premature exit bug in JSON mode. The root cause analysis is accurate and the solution is elegant.

Strengths

1. Correct Fix - Replacing confidence-based heuristic (confidence >= 60) with explicit EXIT_SIGNAL checking is the right approach. The old logic conflated "high confidence in parsing" with "Claude explicit intent to exit", which were semantically different.

2. Comprehensive Tests - Tests 32-35 are excellent TDD examples that validate the fix thoroughly.

3. Good Documentation - Comments in response_analyzer.sh clearly explain why the change was needed.

4. Consistency - Aligns with the documented behavior in ralph_loop.sh that already checks claude_exit_signal == "true" for the main exit condition.

Minor Observations

1. The safety breaker at line 393 now triggers only on EXIT_SIGNAL=true responses, which is correct. Consider documenting why 5 was chosen as the threshold - it is higher than the normal threshold of 2.

2. The fix retains confidence scoring for analysis and logging purposes, which is good practice for debugging.

Suggestions

Consider adding an integration test that simulates a full multi-loop scenario where Claude works on a feature across several iterations with exit_signal=false, then signals completion with exit_signal=true. This would verify the complete flow end-to-end.

Verdict

Approved - The fix is correct, well-tested, and maintains backward compatibility with existing behavior while fixing the critical bug in JSON mode.

[github-actions]

Review complete. I've posted a comprehensive code review to PR #132: #132 (comment)

The fix is well-executed - it correctly replaces a flawed confidence-based heuristic with explicit EXIT_SIGNAL checking, preventing premature exits in JSON mode. The 4 new TDD tests thoroughly validate the fix, and documentation updates are appropriate.

opencode session  |  github run

[claude]

Code Review - PR 132: Exit Detection Fix

Overall Assessment

APPROVED - This is an excellent bug fix with comprehensive test coverage and clear documentation. The fix addresses a critical issue where JSON mode caused premature exits after exactly 5 loops.

---

Code Quality (5/5 stars)

Strengths:

1. Root Cause Analysis: The PR description clearly identifies the issue - confidence-based logic caused false positives in JSON mode
2. Surgical Fix: Changes are minimal and targeted - only touching the problematic logic
3. Test-Driven: Added 4 comprehensive tests (32-35) that validate the fix and prevent regression
4. Documentation: Updated both inline comments and CLAUDE.md with clear explanations

lib/response_analyzer.sh Changes:

• Replaced confidence threshold with explicit exit_signal check
• Added excellent inline comment explaining the rationale
• Maintains backward compatibility with fallback for missing field

ralph_loop.sh Changes:

• Updated safety circuit breaker comment to reflect new behavior
• Clarified that completion_indicators now tracks EXIT_SIGNAL=true responses, not confidence

---

Test Coverage (5/5 stars)

4 New Tests (Tests 32-35):

Test 32: Verifies completion_indicators NOT updated when exit_signal=false with high confidence (70)

• Critical test - This is the exact bug scenario
• Validates that high confidence alone doesn't trigger accumulation

Test 33: Verifies completion_indicators IS updated when exit_signal=true

• Tests the positive case to ensure the fix doesn't break intended behavior

Test 34: Tests accumulation logic across multiple loops

• 2 loops with exit_signal=false (should NOT add)
• 1 loop with exit_signal=true (SHOULD add)
• Verifies selective accumulation

Test 35: EXACT BUG SIMULATION

• 5 consecutive JSON mode loops with confidence=70 and exit_signal=false
• Validates that completion_indicators remains empty (not filled to 5)
• This is the precise scenario that triggered the premature exit bug

Test Quality:

• Direct function testing via sourcing response_analyzer.sh
• Uses realistic JSON structures with all required fields
• Clear assertions with helpful variable names
• Tests both positive and negative cases

---

Potential Issues and Suggestions

Minor: Missing null safety check

Location: lib/response_analyzer.sh:567

The fix uses jq with a default fallback which provides safety. Current code works correctly - if jq fails or analysis_file is malformed, exit_signal could be empty string, but the == check handles this safely (empty string != "true"), so this is just a minor observation, not a blocker.

Documentation Clarity

The inline comment in response_analyzer.sh is excellent. Consider also updating the function-level docstring for update_exit_signals() to mention the EXIT_SIGNAL requirement (if docstrings exist).

---

Performance Considerations

No performance impact - the change replaces one jq query with another of similar complexity.

---

Security Considerations

No security concerns. The fix doesn't introduce any new attack vectors or vulnerabilities.

---

Breaking Changes

None - This is a bug fix that aligns behavior with documented intent. The change makes the system MORE reliable, not less.

---

Alignment with Repository Standards

Per CLAUDE.md Feature Development Quality Standards:

• Test Pass Rate: 100% (424/424 tests passing)
• Test Types: Added unit tests for bash script functions
• Test Quality: Tests validate behavior with clear scenarios
• Git Workflow: Uses conventional commit format (fix(exit-detection):)
• Documentation: Updated CLAUDE.md with v0.11.1 entry
• Inline Comments: Added explanatory comments in modified functions

---

Recommendations

Before Merge:

1. All tests pass (confirmed: 424/424)
2. Documentation updated (CLAUDE.md includes v0.11.1 entry)
3. Conventional commit format used
4. Test coverage validates the fix

Optional Enhancements (Post-Merge):

• Consider adding integration test simulating full 5-loop scenario with mock Claude responses
• Add metric tracking for EXIT_SIGNAL usage patterns (for future debugging)

---

Final Verdict

APPROVED - Ready to Merge

This PR exemplifies excellent bug fix practices:

• Clear problem identification
• Minimal, targeted changes
• Comprehensive test coverage
• Updated documentation
• Follows repository conventions

The fix addresses a critical usability issue where JSON mode users experienced premature exits after exactly 5 loops, even when work was ongoing. The solution correctly aligns implementation with documented behavior - respecting Claude's explicit EXIT_SIGNAL intent rather than relying on confidence heuristics.

Test Count: 424 passing (100% pass rate)
Impact: High - fixes critical JSON mode exit detection bug
Risk: Low - surgical change with comprehensive test coverage