fix(analyzer): detect progress from committed work and embedded RALPH_STATUS#181
fix(analyzer): detect progress from committed work and embedded RALPH_STATUS#181enlego wants to merge 1 commit intofrankbria:mainfrom
Conversation
…_STATUS When Claude commits work atomically after each task, git diff returns zero uncommitted changes. Ralph incorrectly treats this as no progress and opens the circuit breaker after CB_NO_PROGRESS_THRESHOLD loops. Two complementary fixes in the JSON analysis path: - Check git log --since for recent commits, not just git diff - Parse TASKS_COMPLETED_THIS_LOOP and FILES_MODIFIED from the embedded RALPH_STATUS block in the result field (consistent with existing EXIT_SIGNAL extraction added in Bug frankbria#1 Fix) Also fixes literal \\n handling: normalize escape sequences from the jq result field before grep/cut so parsing works in both production (where jq decodes JSON escapes to real newlines) and test environments. All 456 tests pass. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Detect progress in
|
frankbria
left a comment
There was a problem hiding this comment.
Hi @enlego — thanks for tackling this, it's a real problem. When Claude commits atomically after each task, git diff shows zero changes and the circuit breaker trips falsely. Appreciate you digging into it.
That said, there are a few things I'd like to see addressed before we can merge:
1. Link to an issue
Please open a GitHub issue describing the bug and link it to this PR. We track all changes against issues for traceability.
2. The 20-minute heuristic is fragile
git log --oneline -1 --since="20 minutes ago" has edge cases:
- If a loop iteration takes longer than 20 minutes, it misses the commit
- If loops are fast, it can pick up commits from a previous iteration and falsely report progress
A more robust approach would be to use the loop's start timestamp (which Ralph already tracks) instead of a hardcoded window. For example, git log --after="$loop_start_timestamp" would precisely capture commits made during the current iteration.
3. The sed 's/\\n/\n/g' workaround is brittle
The comment explains this well, but having production code include a workaround for test environment behavior is a maintenance risk. Consider either:
- Adjusting the test fixtures to produce real newlines (matching production behavior)
- Using a helper function that abstracts the newline handling
4. claude-review CI is failing
Please investigate — could be flaky CI or something in the diff it flagged.
The core idea is solid. Happy to re-review once these are addressed!
Summary
git diffonly sees uncommitted changes; projects using atomic commits trigger the circuit breaker with no real issueEXIT_SIGNALwas parsed from the embedded RALPH_STATUS block butTASKS_COMPLETED_THIS_LOOPandFILES_MODIFIEDwere notRoot cause
In
analyze_response(JSON path,lib/response_analyzer.sh), progress is detected only viagit diff --name-only. When Claude commits after each loop (a common and encouraged pattern),git diffreturns 0 →has_progress = false→ circuit breaker opens afterCB_NO_PROGRESS_THRESHOLDconsecutive loops.Additionally, the JSON path already extracts
EXIT_SIGNALfrom the embedded---RALPH_STATUS---block in theresultfield, but ignoresTASKS_COMPLETED_THIS_LOOPandFILES_MODIFIEDfrom the same block.Changes
lib/response_analyzer.sh—analyze_response, JSON path (~line 343):Recent-commit detection: After
git diff, checkgit log --oneline -1 --since="20 minutes ago". If a recent commit exists, sethas_progress=trueand populatefiles_modifiedfromgit diff-treeorgit diff HEAD~1.Embedded RALPH_STATUS parsing: Extract
TASKS_COMPLETED_THIS_LOOPandFILES_MODIFIEDfrom theresultfield's---RALPH_STATUS---block, consistent with the existingEXIT_SIGNALextraction. Also normalizes literal\nescape sequences to real newlines before parsing so grep/cut work correctly in both production and test environments.tests/unit/test_json_parsing.bats— 4 new tests (sectionPROGRESS DETECTION WITH COMMITTED WORK TESTS):TASKS_COMPLETED_THIS_LOOPin embedded RALPH_STATUSgit diffis zeroFILES_MODIFIEDfrom RALPH_STATUS whengit diffis zeroTASKS_COMPLETED_THIS_LOOPis zeroTest plan
npm testpasses 100% (456/456 tests)🤖 Generated with Claude Code