fix(monitor): tolerate corrupt lines in cycle-history.jsonl (no false "All clear") - #13
fix(monitor): tolerate corrupt lines in cycle-history.jsonl (no false "All clear")#13jodunk wants to merge 1 commit into
Conversation
A single stray/malformed line in logs/cycle-history.jsonl aborted every slurp-mode jq call, so --alerts/--costs/--compare/--trend/--dashboard/ --health all emitted 'parse error' and reported empty/zero cycle counts. Worst case: --alerts printed 'All clear. No alerts across cycles.' -- a false-negative in the monitoring function users rely on. monitor.sh now routes every cycle-history read through a per-line filter (jq -R 'fromjson? // empty') that skips corrupt lines independently, instead of bare 'jq -s' over the whole file. Verified end-to-end: a real repo with one stray line went from 'All clear... cycles' (empty) to '2 alerts across 115 cycles' (correct). Complements the writer-side fix (append_cycle_history never emits a corrupt line): the writer prevents future corruption; this reader tolerates any line that still slips through (manual edits, partial writes, pre-existing stray lines). Defense-in-depth. Adds tests/test-monitor-resilience.sh (5 checks: bug reproduced, per-line filter counts valid lines without swallowing neighbours, fail-detection survives the filter, helper wired, no bare jq -s over HISTORY_FILE remains). Co-Authored-By: Claude <noreply@anthropic.com>
📝 WalkthroughWalkthrough
ChangesHistory parsing resilience
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@monitor.sh`:
- Line 53: Update the history handling around total_cycles=$(jqh 'length') so
zero valid records are treated as unavailable history in every mode, including
--alerts, --trend, and --health. Guard downstream integer comparisons, division,
and cost parsing from empty or null telemetry, preserving normal behavior when
valid records exist, and add a regression case covering an entirely malformed
history.
- Around line 39-40: Update the jqh and jqhr helper filters to retain only
parsed cycle record objects, excluding JSON scalars and other non-cycle values
before analytics queries run. Validate the required cycle fields used by
downstream queries, such as status, cost, duration_s, and model, when partial
records may be present.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ead63b9a-f5c9-48e4-a229-b748e7a4653c
📒 Files selected for processing (2)
monitor.shtests/test-monitor-resilience.sh
| jqh() { jq -R 'fromjson? // empty' "$HISTORY_FILE" | jq -s "$@"; } | ||
| jqhr() { jq -R 'fromjson? // empty' "$HISTORY_FILE" | jq -r "$@"; } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '"partial record"' '{"cycle":1}' |
jq -R 'fromjson? // empty' |
jq -s '[.[] | type]'
# Current result: ["string","object"]; the string must not reach analytics.Repository: NikitaDmitrieff/auto-co-meta
Length of output: 193
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Files named monitor.sh:\n'
fd -a '^monitor\.sh$' . || true
printf '\nmonitor.sh outline if present:\n'
if [ -f monitor.sh ]; then
ast-grep outline monitor.sh || true
echo "\nmonitor.sh relevant content (first 220 lines):\n"
sed -n '1,220p' monitor.sh | nl -ba
fi
printf '\nSearch jqh/jqhr usages:\n'
rg -n --hidden --glob '!node_modules' 'jqh|jqhr' . || true
printf '\nSearch history file references:\n'
rg -n --hidden --glob '!node_modules' 'HISTORY_FILE|history' . || trueRepository: NikitaDmitrieff/auto-co-meta
Length of output: 369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'monitor.sh line count:\n'
wc -l monitor.sh
printf '\nmonitor.sh lines 1-260:\n'
awk '{printf "%5d\t%s\n", NR, $0}' monitor.sh | sed -n '1,260p'
printf '\nSearch jqh/jqhr usages:\n'
awk 'NF && (NR>=1 && NR<=260)' monitor.sh | grep -nE 'jqh|jqhr|HISTORY_FILE|model|status|cost|group' -n || true
printf '\nShell behavior probe for common downstream selectors:\n'
tmp="$(mktemp)"
printf '%s\n' '"partial record"' '42' 'true' '{"partial":1}' '{"status":"ok","cost":0,"model":"a"}' > "$tmp"
jq -R 'fromjson? // empty' "$tmp" | \
jq -R 'fromjson? // empty' "$tmp" | \
jq -s '
["status", "cost", "model"] as $keys |
{
valid_cycle: [.[] | select(type=="object" and has("status") and has("cost") and has("model"))],
only_string: ["partial record"],
object_no_keys: [{partial:1}],
all: [.[]]
}
'
rm -f "$tmp"Repository: NikitaDmitrieff/auto-co-meta
Length of output: 14776
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Remaining monitor.sh content from line 260 onward:\n'
awk '{printf "%5d\t%s\n", NR, $0}' monitor.sh | sed -n '260,534p'
printf '\njq error probes mirroring monitor.sh downstream selectors:\n'
tmp="$(mktemp)"
printf '%s\n' \
'{"status":"ok","cost":0,"model":"a"}' \
'"partial record"' \
'42' \
'{"cycle":1}' \
'{"status":"ok","cost":0,"model":"a"}' \
> "$tmp"
for expr in \
'[.[-10:] | .[] | select(.status=="fail")]' \
'select(.status=="fail")' \
'.[-10:] | .[] | select(.status=="fail") | " Cycle \(.cycle): exit \(.exit_code), \(.duration_s)s"' \
'[.[].cost] | add' \
'select(.status=="ok")' \
'select(.status=="fail")' \
'[.[].duration_s] | add' \
'group_by(.model) | map({model: .[0].model, cost: ([.[].cost]|add)})'
do
echo "--- $expr ---"
jq -R 'fromjson? // empty' "$tmp" | jq "$expr" 2>&1 || true
done
rm -f "$tmp"
printf '\nProgrammatic extraction of jqh/jqhr call locations in monitor.sh:\n'
python3 - <<'PY'
import re
from pathlib import Path
p=Path("monitor.sh")
text=p.read_text()
for m in re.finditer(r'\b(jqh|jqhr)\b', text):
lineno=text.count('\n',0,m.start())+1
start=max(0,m.start()-50); end=min(len(text),m.end()+120)
snippet=text[start:end].replace('\n','|')
print(f"{lineno}: {snippet}")
PYRepository: NikitaDmitrieff/auto-co-meta
Length of output: 24255
Filter JSON-valid non-cycle values in jqh/jqhr.
fromjson? skips malformed lines but keeps JSON scalars like "partial record" or 42; analytics calls then try .status, .cost, .duration_s, .model, etc. and fail. Restrict parsed values to cycle objects, and validate required fields if partial records can be logged.
Proposed fix
-jqh() { jq -R 'fromjson? // empty' "$HISTORY_FILE" | jq -s "$@"; }
-jqhr() { jq -R 'fromjson? // empty' "$HISTORY_FILE" | jq -r "$@"; }
+jqh() { jq -R '(fromjson? // empty) | select(type == "object")' "$HISTORY_FILE" | jq -s "$@"; }
+jqhr() { jq -R '(fromjson? // empty) | select(type == "object")' "$HISTORY_FILE" | jq -r "$@"; }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| jqh() { jq -R 'fromjson? // empty' "$HISTORY_FILE" | jq -s "$@"; } | |
| jqhr() { jq -R 'fromjson? // empty' "$HISTORY_FILE" | jq -r "$@"; } | |
| jqh() { jq -R '(fromjson? // empty) | select(type == "object")' "$HISTORY_FILE" | jq -s "$@"; } | |
| jqhr() { jq -R '(fromjson? // empty) | select(type == "object")' "$HISTORY_FILE" | jq -r "$@"; } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@monitor.sh` around lines 39 - 40, Update the jqh and jqhr helper filters to
retain only parsed cycle record objects, excluding JSON scalars and other
non-cycle values before analytics queries run. Validate the required cycle
fields used by downstream queries, such as status, cost, duration_s, and model,
when partial records may be present.
|
|
||
| local total_cycles | ||
| total_cycles=$(jq -s 'length' "$history_file") | ||
| total_cycles=$(jqh 'length') |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files named monitor.sh:"
fd -a '^monitor\.sh$' . || true
echo
echo "Relevant line ranges:"
if [ -f monitor.sh ]; then
sed -n '1,110p' monitor.sh
echo "---"
sed -n '220,320p' monitor.sh
echo "---"
sed -n '350,425p' monitor.sh
echo "---"
sed -n '505,525p' monitor.sh
else
echo "monitor.sh not found at repo root"
fi
echo
echo "Search for total_cycles / jqh / jq:"
rg -n "total_cycles|jqh|jq |jq\\(" monitor.sh || true
echo
echo "Behavioral probe for jq/jqh expression semantics:"
python3 - <<'PY'
import json, subprocess, sys, tempfile, os
cases = [
("corrupt only", "{garbage}\n{bad"),
("empty", ""),
("valid line plus corrupt", '{"cycle":1}\n{bad'),
]
for name, lines in cases:
with tempfile.NamedTemporaryFile("w+", delete=False) as f:
f.write(lines)
path = f.name
out = subprocess.run(["jqh","if . then length else 0 end", "<", path], text=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE, shell=True)
print(name, repr(lines).replace('\n', "\\n"), "jqh_rc=", out.returncode, "stdout=", repr(out.stdout), "stderr=", repr(out.stderr))
PY
echo
echo "Check jqh binary availability:"
command -v jqh || true
jqh --help 2>/dev/null | head -n 20 || trueRepository: NikitaDmitrieff/auto-co-meta
Length of output: 20311
🌐 Web query:
jq length null array length return 0 division by zero add empty behavior
💡 Result:
In jq, the length function, division operator, and add filter behave as follows: Length Function The length of a null value is 0 [1][2]. The length of an empty array is also 0 [1][2]. Division by Zero Division by zero (e.g., x / 0) raises a division by zero error [3][4][5]. Historically, implementations of constant folding occasionally caused variations in behavior (such as NaN instead of an error), but current stable versions consistently throw an error when a divisor is zero [3][5]. Add Filter The add filter takes an array as input and sums, concatenates, or merges its elements depending on their types [6][7]. If the input is an empty array, add returns null [6][7]. This is distinct from empty, which produces no results at all [8][6].
Citations:
- 1: https://devdocs.io/jq/
- 2: https://github.com/jqlang/jq/blob/33b3a68f/docs/content/manual/v1.6/manual.yml
- 3: Make 0 divided by 0 results in NaN consistently jqlang/jq#2253
- 4: Make 0 divided by 0 results in NaN consistently jqlang/jq#2253
- 5: Fix constant folding of division and reminder with zero divisor jqlang/jq#2797
- 6: https://jqlang.org/manual/
- 7: https://manpages.debian.org/bookworm/jq/jq.1.en.html
- 8: https://manned.org/man/arch/jq.1
🌐 Web query:
jq division by zero division by zero error jq documentation
💡 Result:
In jq, division by zero raises a runtime error [1][2]. When a division operation encounters a zero divisor, jq terminates the operation and reports an error, typically displaying: jq: error (at ): number () and number (0) cannot be divided because the divisor is zero [3]. Key aspects of this behavior include: - Error Handling: Because this raises a runtime error, it can be caught using the ? operator [1][2]. For example, the expression .[] | (1 / .)? will gracefully return 1 and -1 when processing an input array like [1, 0, -1], effectively suppressing the error caused by the division by zero [1][2]. - 0/0 Behavior: While division by zero generally raises an error, historical versions of jq had inconsistencies regarding how 0/0 was handled compared to other zero-division operations. Recent updates have standardized behavior to ensure 0/0 also triggers a division error [4]. - Constant Folding: Improvements in the jq engine (specifically around constant folding) have been implemented to ensure that zero-division errors are handled consistently, whether the operation is a literal calculation in the code or a dynamic operation performed on input data [4]. For robust scripting, if you expect a divisor might be zero, you should either validate the input data before performing the division or use the ? operator to catch and ignore the resulting error [1][2].
Citations:
- 1: https://github.com/stedolan/jq/blob/90bc29c1b544c0436ec44246e180fdbb6d6384df/docs/content/3.manual/v1.5/manual.yml
- 2: https://manpages.debian.org/trixie/jq/jq.1.en.html
- 3: Make 0 divided by 0 results in NaN consistently jqlang/jq#2253
- 4: Fix constant folding of division and reminder with zero divisor jqlang/jq#2797
Treat all-corrupt histories as unavailable telemetry.
When every history line is malformed, jqh reduces it to an empty array, so --alerts can abort on integer comparison and --trend / --health hit zero-cycle division or null-cost parsing. Normalize zero valid records to “no history available” in every mode and add a zero-valid-record regression case.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@monitor.sh` at line 53, Update the history handling around total_cycles=$(jqh
'length') so zero valid records are treated as unavailable history in every
mode, including --alerts, --trend, and --health. Guard downstream integer
comparisons, division, and cost parsing from empty or null telemetry, preserving
normal behavior when valid records exist, and add a regression case covering an
entirely malformed history.
1 participant
Problem
A single stray/malformed line in
logs/cycle-history.jsonlaborted every slurp-modejqcall inmonitor.sh. Result:--alerts,--costs,--compare,--trend,--dashboard, and--healthall emittedparse error: Invalid numeric literaland reported empty / zero cycle counts.Worst case —
--alertsprinted:That empty count + "All clear" is a false-negative in the monitoring function users rely on to catch failures and cost spikes.
Root cause
Every history read was a bare
jq -s '...' "$HISTORY_FILE"(orjq -r).jq -sslurps the whole file as one JSON stream, so one bad line (e.g. a stray pipe-delimited log line like2026-03-08T... | cycle=123 | blog_post=...) aborts the entire parse. The downstream[ ... -gt 0 ]integer comparisons then fail withinteger expression expected.This is the same bug class as the
--templateflag (#12): advertised behaviour silently broken on the real install path.Fix
Route every cycle-history read through a per-line filter that parses each line independently:
fromjson? // emptyskips a corrupt line without swallowing its valid neighbours (jq's stream-resync would drop ~9 valid cycles after a bad line —jq -c '.'was tried first and rejected for exactly this reason).Verification
End-to-end on a real repo with one stray line:
--alertsAll clear... across cycles(empty)2 alerts across 115 cycles(correct)--costsTotal: $228 across 115 cycles--healthScope
Reader-side only (
monitor.sh). Complements — does not duplicate — the writer-side fix in919bbcd(append_cycle_historynever emits a corrupt line): the writer prevents future corruption; this reader tolerates any line that still slips through (manual edits, partial writes, pre-existing stray lines). Defense-in-depth.Diff:
monitor.sh(+53/-43) +tests/test-monitor-resilience.sh(new, self-contained).🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests