Skip to content

Commit a2e7e93

Browse files
author
Test User
committed
fix(analyzer): address code review feedback
- Fix BSD date parsing to handle milliseconds in ISO timestamps (e.g., 2026-01-09T10:30:00.123+00:00) - Document error_count mapping behavior when only has_errors=true is present - Remove unused has_session_id_field variable - Add debug logging for session persistence (controlled by VERBOSE_PROGRESS) - Standardize session filename to .claude_session_id across all files All 239 tests passing.
1 parent fdff095 commit a2e7e93

3 files changed

Lines changed: 32 additions & 20 deletions

File tree

CLAUDE.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ The system uses a modular architecture with reusable components in the `lib/` di
3636
- Supports both flat JSON format and Claude CLI format (`result`, `sessionId`, `metadata`)
3737
- Extracts structured fields: status, exit_signal, work_type, files_modified
3838
- **Session management**: `store_session_id()`, `get_last_session_id()`, `should_resume_session()`
39-
- Automatic session persistence to `.session_id` file with 24-hour expiration
39+
- Automatic session persistence to `.claude_session_id` file with 24-hour expiration
4040
- Detects test-only loops and stuck error patterns
4141
- Two-stage error filtering to eliminate false positives
4242
- Multi-line error matching for accurate stuck loop detection
@@ -314,7 +314,7 @@ bats tests/unit/test_cli_parsing.bats
314314
- `get_last_session_id()` - Retrieves stored session ID
315315
- `should_resume_session()` - Checks session validity (24-hour expiration)
316316
- Added `get_epoch_seconds()` to date_utils.sh for cross-platform epoch time
317-
- Auto-persists sessionId to `.session_id` file during response analysis
317+
- Auto-persists sessionId to `.claude_session_id` file during response analysis
318318
- Added 16 new tests covering Claude CLI format and session management
319319
- Test count: 239 (up from 223)
320320

lib/response_analyzer.sh

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,6 @@ parse_json_response() {
7272

7373
# Detect JSON format by checking for Claude CLI fields
7474
local has_result_field=$(jq -r 'has("result")' "$output_file" 2>/dev/null)
75-
local has_session_id_field=$(jq -r 'has("sessionId")' "$output_file" 2>/dev/null)
7675

7776
# Extract fields - support both flat format and Claude CLI format
7877
# Priority: Claude CLI fields first, then flat format fields
@@ -94,6 +93,10 @@ parse_json_response() {
9493
local files_modified=$(jq -r '.metadata.files_changed // .files_modified // 0' "$output_file" 2>/dev/null)
9594

9695
# Error count: from flat format OR derived from metadata.has_errors
96+
# Note: When only has_errors=true is present (without explicit error_count),
97+
# we set error_count=1 as a minimum. This is defensive programming since
98+
# the stuck detection threshold is >5 errors, so 1 error won't trigger it.
99+
# Actual error count may be higher, but precise count isn't critical for our logic.
97100
local error_count=$(jq -r '.error_count // 0' "$output_file" 2>/dev/null)
98101
local has_errors=$(jq -r '.metadata.has_errors // false' "$output_file" 2>/dev/null)
99102
if [[ "$has_errors" == "true" && "$error_count" == "0" ]]; then
@@ -232,9 +235,10 @@ analyze_response() {
232235
local json_confidence=$(jq -r '.confidence' .json_parse_result 2>/dev/null || echo "0")
233236
local session_id=$(jq -r '.session_id' .json_parse_result 2>/dev/null || echo "")
234237

235-
# Persist session ID if present
238+
# Persist session ID if present (for session continuity across loop iterations)
236239
if [[ -n "$session_id" && "$session_id" != "null" ]]; then
237240
store_session_id "$session_id"
241+
[[ "${VERBOSE_PROGRESS:-}" == "true" ]] && echo "DEBUG: Persisted session ID: $session_id" >&2
238242
fi
239243

240244
# JSON parsing provides high confidence
@@ -567,8 +571,8 @@ detect_stuck_loop() {
567571
# SESSION MANAGEMENT FUNCTIONS
568572
# =============================================================================
569573

570-
# Session file location
571-
SESSION_FILE=".session_id"
574+
# Session file location - standardized across ralph_loop.sh and response_analyzer.sh
575+
SESSION_FILE=".claude_session_id"
572576
# Session expiration time in seconds (24 hours)
573577
SESSION_EXPIRATION_SECONDS=86400
574578

@@ -628,16 +632,24 @@ should_resume_session() {
628632
local session_time
629633

630634
# Parse ISO timestamp to epoch - try multiple formats for cross-platform compatibility
635+
# Strip milliseconds if present (e.g., 2026-01-09T10:30:00.123+00:00 → 2026-01-09T10:30:00+00:00)
636+
local clean_timestamp="${timestamp}"
637+
if [[ "$timestamp" =~ \.[0-9]+[+-Z] ]]; then
638+
clean_timestamp=$(echo "$timestamp" | sed 's/\.[0-9]*\([+-Z]\)/\1/')
639+
fi
640+
631641
if command -v gdate &>/dev/null; then
632642
# macOS with coreutils
633-
session_time=$(gdate -d "$timestamp" +%s 2>/dev/null)
643+
session_time=$(gdate -d "$clean_timestamp" +%s 2>/dev/null)
634644
elif date --version 2>&1 | grep -q GNU; then
635645
# GNU date (Linux)
636-
session_time=$(date -d "$timestamp" +%s 2>/dev/null)
646+
session_time=$(date -d "$clean_timestamp" +%s 2>/dev/null)
637647
else
638648
# BSD date (macOS without coreutils) - try parsing ISO format
639-
# Format: 2026-01-09T10:30:00+00:00 or similar
640-
session_time=$(date -j -f "%Y-%m-%dT%H:%M:%S" "${timestamp%[+-]*}" +%s 2>/dev/null)
649+
# Format: 2026-01-09T10:30:00+00:00 or 2026-01-09T10:30:00Z
650+
# Strip timezone suffix for BSD date parsing
651+
local date_only="${clean_timestamp%[+-Z]*}"
652+
session_time=$(date -j -f "%Y-%m-%dT%H:%M:%S" "$date_only" +%s 2>/dev/null)
641653
fi
642654

643655
# If we couldn't parse the timestamp, consider session expired

tests/unit/test_json_parsing.bats

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -657,7 +657,7 @@ EOF
657657
assert_equal "$output_format" "json"
658658
}
659659

660-
@test "analyze_response persists sessionId to .session_id file" {
660+
@test "analyze_response persists sessionId to .claude_session_id file" {
661661
local output_file="$LOG_DIR/test_output.log"
662662

663663
cat > "$output_file" << 'EOF'
@@ -670,9 +670,9 @@ EOF
670670
analyze_response "$output_file" 1
671671

672672
# Session ID should be persisted for continuity
673-
[[ -f ".session_id" ]] || skip "Session persistence not yet implemented"
673+
[[ -f ".claude_session_id" ]] || skip "Session persistence not yet implemented"
674674

675-
local stored_session=$(cat .session_id)
675+
local stored_session=$(cat .claude_session_id)
676676
[[ "$stored_session" == *"session-persist-test-123"* ]]
677677
}
678678

@@ -683,23 +683,23 @@ EOF
683683
@test "store_session_id writes session to file with timestamp" {
684684
run store_session_id "session-test-abc"
685685

686-
[[ -f ".session_id" ]] || skip "store_session_id not yet implemented"
686+
[[ -f ".claude_session_id" ]] || skip "store_session_id not yet implemented"
687687

688-
local content=$(cat .session_id)
688+
local content=$(cat .claude_session_id)
689689
[[ "$content" == *"session-test-abc"* ]]
690690
}
691691

692692
@test "get_last_session_id retrieves stored session" {
693693
# First store a session
694-
echo '{"session_id": "session-retrieve-test", "timestamp": "2026-01-09T10:00:00Z"}' > .session_id
694+
echo '{"session_id": "session-retrieve-test", "timestamp": "2026-01-09T10:00:00Z"}' > .claude_session_id
695695

696696
run get_last_session_id
697697

698698
[[ "$output" == *"session-retrieve-test"* ]] || skip "get_last_session_id not yet implemented"
699699
}
700700

701701
@test "get_last_session_id returns empty when no session file" {
702-
rm -f .session_id
702+
rm -f .claude_session_id
703703

704704
run get_last_session_id
705705

@@ -711,7 +711,7 @@ EOF
711711
@test "should_resume_session returns true for recent session" {
712712
# Store a recent session (simulated as current timestamp)
713713
local now=$(date +%s)
714-
echo "{\"session_id\": \"session-recent\", \"timestamp\": \"$(date -Iseconds)\"}" > .session_id
714+
echo "{\"session_id\": \"session-recent\", \"timestamp\": \"$(date -Iseconds)\"}" > .claude_session_id
715715

716716
run should_resume_session
717717

@@ -721,7 +721,7 @@ EOF
721721

722722
@test "should_resume_session returns false for old session" {
723723
# Store an old session (24+ hours ago)
724-
echo '{"session_id": "session-old", "timestamp": "2020-01-01T00:00:00Z"}' > .session_id
724+
echo '{"session_id": "session-old", "timestamp": "2020-01-01T00:00:00Z"}' > .claude_session_id
725725

726726
run should_resume_session
727727

@@ -730,7 +730,7 @@ EOF
730730
}
731731

732732
@test "should_resume_session returns false when no session file" {
733-
rm -f .session_id
733+
rm -f .claude_session_id
734734

735735
run should_resume_session
736736

0 commit comments

Comments
 (0)