fix(response_analyzer): match RALPH_STATUS YAML colon-block format#262
Open
justanotherguyme wants to merge 2 commits intofrankbria:mainfrom
Open
fix(response_analyzer): match RALPH_STATUS YAML colon-block format#262justanotherguyme wants to merge 2 commits intofrankbria:mainfrom
justanotherguyme wants to merge 2 commits intofrankbria:mainfrom
Conversation
…on to ---RALPH_STATUS--- separator
Some agent prompts (e.g. multi-agent task-board harnesses) emit the
structured-status block as YAML — a colon heading with indented keys —
rather than the canonical separator-marker form documented in
lib/enable_core.sh:560-568:
RALPH_STATUS:
EXIT_SIGNAL: true
reason: "..."
The detection regex at lib/response_analyzer.sh:161 (JSON-mode .result
text path) and lib/response_analyzer.sh:489 (text-mode structured-output
fallback) currently grep for the literal "---RALPH_STATUS---" separator
only, so YAML-format emits silently fall through and exit_signal stays
at the default false value. The loop then continues past explicit
EXIT_SIGNAL: true emissions, burning tokens until the user manually
interrupts.
This change extends both regexes to also match "RALPH_STATUS:" (with the
trailing colon distinguishing it from prose mentions of RALPH_STATUS).
The downstream extraction `grep "EXIT_SIGNAL:" | cut -d: -f2 | xargs`
already handles both layouts uniformly because both formats emit one
"EXIT_SIGNAL: <bool>" line.
A documentation block above each regex describes both supported formats
so future maintainers see the dual-format contract before the regex.
4 new bats tests in tests/unit/test_json_parsing.bats verify:
- Text mode + YAML emit + EXIT_SIGNAL=true → exits cleanly
- Text mode + YAML emit + EXIT_SIGNAL=false → continues (respects
explicit continue intent, matching the documented EXIT_SIGNAL
semantics for the separator format)
- JSON-mode .result wrapper + YAML emit + EXIT_SIGNAL=true → exits
- JSON-mode .result wrapper + YAML emit + EXIT_SIGNAL=false →
continues
Backwards-compatible: existing canonical-format tests (line 394
test_json_parsing.bats) still pass; no change to the separator-marker
code paths.
Contributor
WalkthroughThis PR updates lib/response_analyzer.sh to recognize both canonical ChangesRALPH_STATUS Format Flexibility
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/response_analyzer.sh`:
- Line 174: The current check that scans result_text for RALPH_STATUS is too
loose; update the conditional that references result_text (e.g., the if using
grep -qE -- "---RALPH_STATUS---|RALPH_STATUS:") to only match block-header lines
by anchoring the pattern to the start of a line (allowing optional leading
whitespace), for example using a regex like
'^[[:space:]]*(---RALPH_STATUS---|RALPH_STATUS:)' so inline occurrences are
ignored; then, in the downstream exit-detection logic that reads the
RALPH_STATUS block (the code paths handling EXIT_SIGNAL and
completion_indicators), enforce the dual-condition: require an explicit
EXIT_SIGNAL field present in the RALPH_STATUS block and completion_indicators >=
2 before triggering exit to prevent false positives.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6592b497-a34c-4945-99be-2706c5cf1c6f
📒 Files selected for processing (2)
lib/response_analyzer.shtests/unit/test_json_parsing.bats
Per CodeRabbit review on PR frankbria#262: the dual-format regex "---RALPH_STATUS---|RALPH_STATUS:" matches inline mentions in prose (e.g., a doc string or commit message containing "the RALPH_STATUS: emit format") because grep -E without an anchor matches anywhere on a line. That false-matches the gate, enters the EXIT_SIGNAL extraction block, and on prose containing both "RALPH_STATUS:" and "STATUS: COMPLETE" can wrongly set exit_signal=true. Anchor to start-of-line (with optional leading whitespace) so only block headers trigger the gate: -if [[ ... ]] && grep -qE -- "---RALPH_STATUS---|RALPH_STATUS:" ... +if [[ ... ]] && grep -qE -- "^[[:space:]]*(---RALPH_STATUS---|RALPH_STATUS:)" ... Applied at both call sites: - lib/response_analyzer.sh:174 (JSON-mode .result text path) - lib/response_analyzer.sh:507 (text-mode structured-output fallback) Doc-comment blocks above each regex updated to describe the line-anchor behavior so future maintainers don't accidentally regress. 3 new bats regression tests in tests/unit/test_json_parsing.bats: - text mode: inline "RALPH_STATUS:" in prose does NOT trigger the gate - JSON-mode .result with inline "RALPH_STATUS:" in prose does NOT trigger the gate - text mode: leading-whitespace block header (indented YAML, e.g., quoted in tool output) DOES still trigger — the anchor allows optional leading whitespace by design Total @test blocks in test_json_parsing.bats: 56 → 60 (initial PR) → 63 (this commit). All previous tests still pass; the four YAML-format tests added in 86e04da continue to verify the dual-format detection.
Author
|
Thanks @coderabbitai for the line-anchor catch — pushed
On the dual-condition concern: |
Contributor
|
Tip For best results, initiate chat on the files or code changes.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/response_analyzer.sh`:
- Line 176: The issue is that combining "local" with command substitution masks
the command's exit code for result_text; change the code in the response parsing
logic so you first declare the variable (local result_text) and then assign it
with the jq command substitution (result_text=$(jq -r '.result // ""'
"$output_file" 2>/dev/null)), and after assignment check the jq exit status ($?)
or handle empty/error cases; reference the variable result_text and input
variable output_file and ensure any failure from jq is propagated or logged
instead of being hidden.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 45caac2a-8f67-4006-9bfd-62f991b8c19c
📒 Files selected for processing (2)
lib/response_analyzer.shtests/unit/test_json_parsing.bats
| # both layouts uniformly because both emit one EXIT_SIGNAL: <bool> line. | ||
| # See also the parallel check at the structured-output text-parsing fallback below. | ||
| if [[ "$exit_signal" == "false" && "$has_result_field" == "true" ]]; then | ||
| local result_text=$(jq -r '.result // ""' "$output_file" 2>/dev/null) |
Contributor
There was a problem hiding this comment.
SC2155: Declare and assign result_text separately.
When local and command substitution are combined, the local builtin's exit code (always 0) masks the subcommand's exit code. Shellcheck flags this on line 176.
🛠 Proposed fix
- local result_text=$(jq -r '.result // ""' "$output_file" 2>/dev/null)
+ local result_text
+ result_text=$(jq -r '.result // ""' "$output_file" 2>/dev/null)🧰 Tools
🪛 Shellcheck (0.11.0)
[warning] 176-176: Declare and assign separately to avoid masking return values.
(SC2155)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/response_analyzer.sh` at line 176, The issue is that combining "local"
with command substitution masks the command's exit code for result_text; change
the code in the response parsing logic so you first declare the variable (local
result_text) and then assign it with the jq command substitution
(result_text=$(jq -r '.result // ""' "$output_file" 2>/dev/null)), and after
assignment check the jq exit status ($?) or handle empty/error cases; reference
the variable result_text and input variable output_file and ensure any failure
from jq is propagated or logged instead of being hidden.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extends
response_analyzer.shRALPH_STATUS detection to also match the YAML colon-block format (RALPH_STATUS:with indented keys) emitted by some agent prompts, in addition to the canonical---RALPH_STATUS---separator-marker format.Problem
Some agent prompts emit the structured-status block as YAML rather than the canonical separator-marker form documented in
lib/enable_core.sh:560-568. For example:The detection regex at two locations currently greps for the literal
---RALPH_STATUS---separator only:lib/response_analyzer.sh:161(JSON-mode.resulttext path)lib/response_analyzer.sh:489(text-mode structured-output fallback)YAML-format emits silently fall through.
exit_signalstays at defaultfalse, the loop continues past explicitEXIT_SIGNAL: trueemissions, tokens burn until the user manually interrupts.Fix
Extend both regexes to also match
RALPH_STATUS:(the trailing colon distinguishes it from prose mentions of RALPH_STATUS):The downstream
grep "EXIT_SIGNAL:" | cut -d: -f2 | xargsextraction already handles both layouts uniformly because both formats emit oneEXIT_SIGNAL: <bool>line.A documentation comment block above each regex describes both supported formats so future maintainers see the dual-format contract before the regex.
Tests
4 new bats tests in
tests/unit/test_json_parsing.bats(60 @test blocks total, was 56):EXIT_SIGNAL=true→ exits cleanlyEXIT_SIGNAL=false→ continues (respects explicit continue intent, same semantics as separator format).resultwrapper + YAML emit +EXIT_SIGNAL=true→ exits.resultwrapper + YAML emit +EXIT_SIGNAL=false→ continuesThe existing canonical-format test at line 394 (
@test "analyze_response still handles traditional RALPH_STATUS format") is unmodified and verifies no regression on the separator-marker path.Test plan
Backwards compatibility
Backwards-compatible. The existing
---RALPH_STATUS---separator-marker code paths are unchanged in semantics; the regex alternation adds a second match form. Existing prompts emitting the canonical format see identical behavior.Why both lines
Line 161 is the JSON-output mode path (default per
CLAUDE_OUTPUT_FORMAT="json"). Line 489 is the text-mode structured-output fallback. Both paths can be reached depending on--output-formatand CLI version; fixing both lines closes the bug regardless of mode.Summary by CodeRabbit
Bug Fixes
Tests