The final train: markdown sections, --verify, and the E1 grader - #37
Conversation
…ore the tier Gate-first (CONTRIBUTING §2): test/mdsectioncheck.sh + test/mdsectionfix/ land BEFORE the grammar, the extraction change, or any recall/emission change. Against the pre-change binary (origin/main 49f4d75) it fails 25 arms of 90 and exits 1 — the recorded red set: setext headings absent, .markdown not indexed, html-block heading leak, no hierarchy scopes, all four link-edge arms, section spans still one heading line (--expand/--for/decoy arms), --recall whole-doc dump, the code-query pollution arm, and the missing deep-blockquote guard. What the gate pins: ATX 1-6 + setext headings as t="sec" symbols · section span = heading to next same-or-higher heading ACROSS heading forms (an H1 setext closes an ATX H1's span) · hierarchy as scope (canonical id path::Parent::Child; Install Steps exists twice, path-qualified, never merged — the churn-keying lesson) · fenced/tilde/ indented/html/front-matter/blockquote non-leak, incl. the double-index guard (a fenced C++ fn must not become a code symbol) · links as edges ([text](x.md) + [ref]: x.md → doc→doc, [text](#anchor) → doc-section→doc-section, each attributed to its enclosing section) · backtick mentions regression · --for ranks the section by a BODY-only token and keeps the sig at the heading line · --recall section-granular with a [sections: disclosure, whole-doc fallback for heading-less docs disclosed · both pollution directions · determinism ×3 + cache transparency + xmllint · the deepquote guard note with a nearlimit non-overbreadth arm. Presence guards up front: 27 greps assert the fixtures still SPELL every construct before any arm searches the map for it. regression.sh and all THREE docs/EVALS.md gate counts move together: 391 -> 392 (lines 24/873/1507), manifestcheck green at 392. Also in this commit, per the round contract: the terminality PRE-REGISTRATION (docs/EVALS.md §4, "Markdown section tier — G2/G3 round") — the residual deleted is "find the section inside the doc"; the registered band is post-call section- localization falling to HALF OR LESS of the pass-2 rate on post-deploy sessions, levels ledger-local. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ith a serialize-bounds patch SHA and ABI both VERIFIED against the tag rather than taken on faith: f969cd3ae3f9fbd4e43205431d0ae286014c05b5 is what v0.5.3 resolves to, and src/parser.c declares LANGUAGE_VERSION 15, inside the runtime's accepted [MIN_COMPATIBLE, LANGUAGE_VERSION] = [13, 15] for core v0.26.9. From tree-sitter-grammars/, not tree-sitter/ — there is no first-party markdown grammar. The upstream repo hosts TWO grammars in subdirectories; only the BLOCK grammar (tree-sitter-markdown/) is vendored, FLATTENED to the house deps/<name>/src layout (LICENSE + src/parser.c + src/scanner.c + src/tree_sitter/*.h) so add_ts_grammar, the vendorpatch scanner sweep and dependencypincheck all see it with their existing shapes. The INLINE grammar is deliberately deferred: inline constructs (links, code spans, wikilinks) are extracted by ingest's own fence-aware line scan, and a second stateful scanner is a cost the tier does not need yet. THE SCANNER FINDING, probed before vendoring (standalone ASan, 2026-08-12): this scanner's serialize() memcpys open_blocks.size * sizeof(Block) bytes after a 5-byte header with NO bounds check AT ALL against the 1024-byte serialization buffer — the yaml OOB's defect class, minus even the bare guard. 300 nested '>' markers, or 300 "- " list markers on ONE LINE, abort ts_assert(length <= 1024) (rc=134); under NDEBUG the write corrupts the heap silently. third_party/patches/markdown/ 001-serialize-bounds.patch clamps the whole write up front (truncation-on-overflow, the yaml patch's accepted semantics), reverse-applies clean, carries its RIPWIRE_VENDOR_PATCH marker, and is classified "upfront" in vendorpatchcheck arm H — the class whose proof obligation is a whole-write bounds check before any write. Verified standalone: both 300-deep shapes parse rc=0 under ASan with the patch; the normal-corpus AST is unchanged. Ingest's pre-parse depth guard (kMaxMdBlockDepth, next commit) is the first of the two layers, same posture as yaml. No tags.scm: extraction will be a custom tree walk (section spans + heading hierarchy need the tree, not a capture list), so the grammar is deliberately absent from _ripwire_query_names; both grammar OBJECT lists and dependencypincheck's vendored- tree enumeration gain markdown. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ng our own CLAUDE.md
The E1 scenario-efficiency round has had one named blocker since Phase 0 closed: analyze.py scores a
PATCH, and the instance bank scores an ANSWER. No amount of budget substitutes for code that does not
exist. This is that code, plus the two harness pieces the pricing memo said the bank needs to reach
it, plus the control-arm hole Phase 0 found in the instrument itself.
bench/agentloop/grade_answers.py — the answer grader. For each row it EXECUTES the row's gt_command
at the pinned sha and derives the key from stdout (protocol §3: ground truth is a command, never a
list), extracts the agent's terminal answer from the retained transcript, and applies the accept rule
mechanically. Three refusals are the point of it:
* NON-CIRCULARITY. A gt_command that INVOKES ripwire is REFUSED — a key produced by the instrument
under test is not a key. The precision matters: nine rows of the real bank name `ripwire/src` as
a PATH argument to grep/ls, and an over-broad test would throw a third of the bank away. Both
directions are gated.
* SEALED JUDGEMENT. Every V row, and any E row whose second half is a classification, REFUSES
without a --key file. The grader never improvises the judgement half.
* HONEST PARTIALS. An accept_rule is prose. A closed clause grammar scores what it recognises and
an unrecognised clause DEMOTES the verdict to PARTIAL rather than being skipped on the way to
PASS. --audit reports that coverage for a whole bank with no transcript and no cost: on the real
28-row bank, 15 rows are fully mechanical, 13 can only reach PARTIAL, and 7 owe a sealed key —
two more than the pricing memo counted (I18 and I30 carry judgement halves it did not flag).
A fourth refusal came out of writing the gate rather than the design. macOS ships bash 3.2, which has
no globstar, so a `**` gt_command silently matches nothing and the key comes back empty — protocol
§4's false zero, wearing the face of a legitimate "no results". Four bank rows use `**`. The grader
probes for a globstar-capable shell once and REFUSED_SHELLs those rows when none exists.
run_agentloop.py — the two pieces the memo named. `--questions TSV` is a local task source that
bypasses tasks.lock and the HuggingFace gold-row fetch entirely, with `--local-corpus` materializing
private trees via `git worktree add --detach` at the pin; the per-tier budget cap and wall timeout
come from the row, never from the observed chain. build_question_prompt() emits the question VERBATIM
plus a scope fence assembled from the protocol's own rules — groundedness stated, read-only, the tier
budget, and the terminal-answer sentinels the grader reads.
F3 — the claude runner had no isolation. codex and opencode have built isolated environments since
they were wired; `claude-code-p`, the DEFAULT --harness, passed child_env = None and inherited
~/.claude whole: CLAUDE.md, both ripwire hooks, all 18 skills, the per-project auto-memory. On this
machine 81 of 84 lines of that CLAUDE.md are a ripwire use-when protocol, so the BASELINE arm was
briefed by name, verb and reflex on the tool it is a control for — and every such run still reported
status=ok. prepare_claude_environment() gives each run a fresh CLAUDE_CONFIG_DIR with only credentials
symlinked, build_claude_command() adds --setting-sources '' (which also kills the settings env.PATH
that would re-prepend the real ripwire after the shim), --strict-mcp-config and
--disable-slash-commands on every arm but ripwire_skills, and the meter is pointed at per-run scratch
so a benchmark can never append to the operator's live substitution telemetry. _claude_metrics() now
retains the raw trailer — without it the terminal answer was parsed for tokens and thrown away.
Also: the --dry-run cost projection now carries its own disclosure. README.md's SAFETY note makes
that number the human approval gate, and it is a literature envelope this harness's own pilot exceeded
by 2.6x on a single run. A gate that under-reports what a human approves is worse than no gate.
GATES, both born red on origin/main (agentloopgradercheck exit 2, agentloopclaudecheck exit 1):
test/agentloopgradercheck.sh the committed fixture bank graded byte-for-byte against
bench/agentloop/fixtures/grader/expected.tsv, all seven verdict
classes exercised, both directions of the circularity test, the
sealed-key refusal with and without the key, the unpinned-fixture
guard, and the exit-code contract.
test/agentloopclaudecheck.sh the F3 canary, modelled on agentloopopencodecheck.sh: no CLAUDE.md /
skills / projects / settings.json under the run home, the three scrub
flags, skills in exactly one arm, the meter pointed at scratch, the
shim first on PATH, transcript retention, and the questions prompt's
scope fence.
Both listed in test/regression.sh in this commit; docs/EVALS.md's three gate counts re-derived from
the loop (392 -> 394). --quality-delta gating=0: run_one's complexity regression and the two
dead-code rows were FIXED (a dispatch dict had hidden the codex/opencode preparers from the
resolver) rather than acked, the three preparers now share ephemeral_run_home()/link_credential();
the residual churn=self and one deliberate 101-token clone are acked with their reasoning.
NOT DONE, and deliberately: no live agent arm was run. The grader's fixtures are the proof; a funded
run is the owner's call, and the dry-run projection understates it 2-5x.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…spans (kParserVer 63) Turns test/mdsectioncheck.sh green (24 red arms → 94 green, both flavours). A heading is to a doc what a function is to a file: - extractMarkdown parses with the vendored tree-sitter-markdown block grammar (custom tree walk, no tags.scm). ATX 1-6 AND setext headings become t="sec" defs whose SPAN runs to the next same-or-higher heading — computed over the MERGED heading list, because the grammar nests `section` nodes on ATX only (probed: a setext H1 does not open a section node); the level rule is the ratified design. bodyByte = the heading construct's end, so the signature is the heading and --expand serves the section body. scope = nearest shallower heading → canonical id path::Parent::Child, identity path-qualified (the churn-keying lesson). - Non-structure stays non-structure: fenced/tilde/indented code, html blocks (the old hand scanner's phantom-heading leak), front-matter and blockquoted headings produce NO symbols, and the mention/link line scan skips those opaque ranges off the AST. - Links are edges over the EXISTING machinery: [text](other.md) + [label]: other.md → the target stem (the resolve ladder's same-dir preference lands the file node, exactly like [[wikilinks]], which are now fence-aware); [text](#anchor) → the slug-matched heading in THIS file (same-file preference) — doc-section→doc-section. `backtick` mentions keep their semantics and now attribute to their enclosing SECTION by the ordinary innermost-span containment. - --recall goes SECTION-GRANULAR where sections matched: buildSectionGranularBody serves the positive-scoring sections (most specific first, overlap-deduped, document order), disclosed per-doc as `[sections: k of n, section-granular; whole doc N B]`; heading-less and docparse-extracted docs keep the whole-doc path, disclosed by the note's absence. - .markdown joins the extension table (kLangTable 36 → 37 rows, compiler-checked). - --doctor gains a parse-probe row for the one grammar with no tags.scm (the honest probe is the pairing ingest uses): loaded="19" expected="19". - mdNestsTooDeep prescan (kMaxMdBlockDepth=200) refuses pathological blockquote/list nesting BEFORE any parse — the first of two layers over the scanner's serialize() OOB (third_party/patches/markdown/001-serialize-bounds.patch is the second). TWO BUGS CAUGHT BY THE ROUND'S OWN GATES, fixed here: - G1: `p-- > 0` in the parent-scope walk wraps at 0 under -fsanitize=integer (cachefuzzcheck's UBSan arm) — rewritten as `p > 0; --p`. - DETERMINISM: the whole-file node's dedup identity was (fileId, nameByte=0), and a SETEXT heading whose paragraph opens the file ALSO puts its name at byte 0 — the two tied on every sort key (same kind, same startByte, equal endByte when the first heading's span runs to EOF), so std::sort's instability flipped the unique() survivor with worker arrival order: bench scoreboards flapped in --merge-scout's changed set (changed= 25/27/29 across runs) and --expand intermittently found no symbol. The file node's identity moves to src.size() (EOF) — no identifier can START there, collision-free by construction. mdsectioncheck's setext0 arm pins it. kParserVer 62 -> 63 AND quality.h's kIngestParserVerMirror in this same commit; qschemetrip re-pinned with its RE-PIN LOG entry (extraction change, kQSnapCacheScheme deliberately unmoved). Quality delta acked at gating=0 (the growth IS the feature, reviewed); symbol-count deltas on this repo: files 1177→1178, symbols 10491→10495, edges 12515→12618 (link edges); docs/ dir: symbols 324→324, edges 29→42. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… bugs earned - golden.xml re-pinned (reviewed diff): the fixture's sections gain hierarchy ids (test/fixture/notes.md::Geometry Fixture::Why it exists / ::Symbols) and notes.md's [[related]] wikilink edge moves from the file node to its enclosing SECTION — same file/symbol/edge counts (6/14/5), est_tokens 647 -> 691 over 1612 -> 1726 B. - fillordercheck: #1's golden-neutral pin RE-PINNED 647 -> 691, reasoning logged in the gate's own re-pin ledger. - docmentioncheck: the --for doc-mention lift now surfaces the SECTION that discusses the symbol (mentions attribute to their enclosing section), not the whole-doc file node — the two lift arms score that row, same wave, plus a non-vacuity arm so a broken row lookup can never make the below-anchor comparison pass at 0. - xmlwellformed: the --around sample walker reads symbol names by LINE — the old unquoted for-loop word-split multi-word section names ("Where to look" became three bogus selectors whose empty error documents failed xmllint). - mdsectioncheck: deepquote.md is GENERATED at gate runtime, never committed (a committed 300-deep file put the guard's stderr note into every repo-wide run — xmlwellformed's --owners arm caught exactly that); new setext0.md fixture + arms pin the byte-0 setext identity collision: BOTH the file node and the opening setext heading must exist, every run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ey were carrying
README's four language sites (above-the-fold line, quickstart parse list, ## Languages,
the documents paragraph) gain the Markdown section tier; the count corrects to
NINETEEN vendored grammars, which --doctor now measures (loaded="19" expected="19").
Two of those sites were ALREADY stale on main — the YAML tier never reached the
above-the-fold line or the count ("seventeen", no YAML) — corrected here rather than
left drifting (the showcase-refresh round had found the class, not the fix). Markdown
graduates out of the documents-only paragraph: its headings are symbols now.
docs/COMMANDS.md regenerated from the binary + the newest committed capture
(docscommandscheck G byte-parity green): the only content change is --help's language
header, which cli.h extended in the feat commit. The README callers example's two
main.cpp line anchors re-pinned by readmeexamplecheck.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…claim bundle
--verify="CLAIM" collapses the manual verification grep-chain (the transcript
mine's largest verb-less intent) into ONE call: a CLOSED claim language —
calls(A,B) · uses(SYM) · unused(SYM) · contains(FILE, "LIT") · defines(FILE, SYM)
· reaches(SYM, "FILE"|LAYER) — answered with a three-valued verdict plus the
evidence inline (the path, the use-sites, the hits, the definition rows).
The verdict grammar is the heart, and it obeys the honesty rules exactly:
- confirmed a witness exists and is printed
- refuted ONLY with complete evidence: a clean uncapped literal-scan
absence carries complete="1" (T1's claim conditions), and an
absence-claim (unused) is refuted by printed witness sites
- not-established the absence is real WITHIN THE MODEL but the model is a
floor; limit= names it (call-graph-floor / reference-floor /
collection-ceiling / scan-degraded / extraction-floor) — it
NEVER means false
calls/reaches can never refute (name-based edges); unused can never confirm
(identifier-based references); complete= and counts_floor= never co-occur.
Built entirely from the sibling machinery — shortestPathAny, collectUseSites,
grepCollect/grepEnrich (with its scan-honesty bits), transitiveCallers — because
the measured gap was never the data, it was the multi-call chain plus manual
reading. An unknown shape refuses loudly with the whole vocabulary (the
--graph-query posture); a typo'd symbol refuses with did-you-mean; a file the
index never saw refuses pointing at --skipped.
Gate red-first: test/verifycheck.sh (44 FAIL / 16 PASS against the pre-feature
binary; 46 arms green after) — per-shape verdict arms, the dynamic-dispatch
mutation arm (a string-keyed call site must yield not-established, never
refuted), complete=/counts_floor= mutual exclusion swept across every captured
root, refusal vocabulary, determinism x3, xmllint per verdict class. Registered
in test/regression.sh same commit; the three EVALS gate counts recomputed from
the actual loop (393). Terminality pre-registration added to docs/EVALS.md
(mechanism public, levels in the operator-local ledger).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- docs/COMMANDS.md regenerated from the binary's --help (docs_commands_build.py; no capture change — the new section carries no sample yet, by design) - README: the advertised flag count 143 -> 144, and the recorded --callers example's main.cpp line numbers re-synced (runVerify shifted them; the rows are pinned by test/readmeexamplecheck.sh) - present/deck5: the three 143-long-flags claims -> 144 (test/deckclaimcheck.sh) - skills/ripwire-navigate: --verify gains its skill home (BODY row only — no frontmatter/description edit, so the two routing floors are untouched by construction; both re-verified green anyway). This satisfies the flag-home gate the same way --run-trace did: verb-first, routing follows the adoption loop. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…y round printUsage verbosity (the standing precedent: the flag catalogue IS the function's content), churn=self on kTotalFlagArms and main (every flag addition touches the arm count and the dispatch chain by design). The clone findings the delta first reported were FIXED, not acked: shapeTag's switch became the declarative kShapeTags table indexed at the single use-site, which dissolved the symTag/refRoleTag/styleTag/generatedReasonTag/colorByLabel clone family memberships. --quality-delta now reports gating=0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts: # .ripwire_quality_acks # README.md # docs/EVALS.md # test/regression.sh
# Conflicts: # .ripwire_quality_acks # README.md # docs/EVALS.md # src/cli.h # test/regression.sh
# Conflicts: # .ripwire_quality_acks # docs/EVALS.md # test/regression.sh
…45, captures regenerated Four lanes' additions unioned (mdsectioncheck, verifycheck, both agentloop gates); acks rebuilt from refs after a filtered-subprocess truncation was caught mid-merge. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis change adds Tree-sitter Markdown indexing with section-level recall, a closed ChangesMarkdown indexing and section recall
Claim verification
E1 agent-loop evaluation
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Ingest
participant Index
participant Verify
Client->>Ingest: index Markdown and source files
Ingest->>Index: store sections, symbols, links, and use sites
Client->>Verify: submit --verify claim
Verify->>Index: query bounded evidence
Index->>Verify: return graph and reference results
Verify->>Client: emit XML verdict
sequenceDiagram
participant QuestionSource
participant AgentLoop
participant Harness
participant Grader
QuestionSource->>AgentLoop: load question row and pinned corpus
AgentLoop->>Harness: run isolated question prompt
Harness->>AgentLoop: retain transcript and metrics
AgentLoop->>Grader: provide transcript result
Grader->>QuestionSource: derive and validate sealed key
Grader->>AgentLoop: return verdict and score
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 14
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (12)
test/verifycheck.sh-37-37 (1)
37-37: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGuard the
cd.If
cd "$ROOT"fails, every arm below runs in the wrong directory and reports misleading failures. The script does not useset -e, so the failure is silent.🛡️ Proposed fix
-cd "$ROOT" +cd "$ROOT" || { echo "verifycheck: cannot cd to $ROOT"; exit 2; }🤖 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 `@test/verifycheck.sh` at line 37, Guard the directory change in the test script by making the cd "$ROOT" command fail immediately when it cannot enter the target directory. Preserve the existing test flow after a successful directory change.Source: Linters/SAST tools
test/verifycheck.sh-222-226 (1)
222-226: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winReport a missing
xmllintas a skip, not a failure.Line 224 sends
xmllintstderr to/dev/null. Ifxmllintis not installed, every file reportsnot well-formedand the gate fails for an environment reason rather than a code reason.🛡️ Proposed fix
xml_ok=1 -for f in c1 c2 n1 n2 n3 u1 u2 u3 d1 d2 d3 r1 r2 r3; do - xmllint --noout "$TMP/$f.xml" 2>/dev/null || { xml_ok=0; no "xmllint: $f.xml is not well-formed"; } -done -[ $xml_ok -eq 1 ] && ok 'xmllint: every verdict class is well-formed XML' +if command -v xmllint >/dev/null 2>&1; then + for f in c1 c2 n1 n2 n3 u1 u2 u3 d1 d2 d3 r1 r2 r3; do + xmllint --noout "$TMP/$f.xml" 2>/dev/null || { xml_ok=0; no "xmllint: $f.xml is not well-formed"; } + done + [ $xml_ok -eq 1 ] && ok 'xmllint: every verdict class is well-formed XML' +else + echo " SKIP xmllint not on PATH — well-formedness unchecked" +fi🤖 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 `@test/verifycheck.sh` around lines 222 - 226, Update the xmllint validation loop in test/verifycheck.sh to detect whether xmllint is available before validating files; report the check as skipped when it is missing, and retain the existing failure reporting and well-formedness gate when the tool is installed.present/deck5_ripwire_build.js-149-149 (1)
149-149: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUpdate the documented-command count to 123.
docs/COMMANDS.mdcontains 123 command headings, including--verify="CLAIM", but the deck states 117.test/deckclaimcheck.shdoes not validate this count.🤖 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 `@present/deck5_ripwire_build.js` at line 149, Update the documented-command count in the foot() call within deck5_ripwire_build.js from 117 to 123, leaving the generated flag count and surrounding wording unchanged.test/verifycheck.sh-181-185 (1)
181-185: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve the
test/path component for this assertion
builtinLayer()maps directory components namedtest,tests, orbenchto thetestlayer. A copied fixture without one of these components makesreaches(leaf_target, test)fail. Keep the fixture undertest/, or add an explicit path precondition.🤖 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 `@test/verifycheck.sh` around lines 181 - 185, Update the fixture setup used by the reaches layer assertion so its path retains a test/, tests/, or bench/ directory component, allowing builtinLayer() to resolve the test layer. Alternatively, add an explicit path precondition before running the verify command, while preserving the existing confirmed-verdict assertion.bench/agentloop/grade_answers.py-82-88 (1)
82-88: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winRow cell count is not validated; extra cells are dropped silently.
Line 86 pads short rows, but a row with more than 13 cells (an embedded tab) is truncated by
zip. That contradicts the fail-closed posture stated in the docstring. Ruff also flags the missingstrict=here (B905).♻️ Proposed change
for line in lines[ 1: ]: if not line.strip(): continue cells = line.split( "\t" ) + if len( cells ) > len( COLUMNS ): + raise SystemExit( f"{path}: row {cells[ 0 ]!r} has {len(cells)} cells, expected " + f"{len(COLUMNS)} — refusing (embedded tab?)" ) cells += [ "" ] * ( len( COLUMNS ) - len( cells ) ) - rows.append( dict( zip( COLUMNS, cells ) ) ) + rows.append( dict( zip( COLUMNS, cells, strict=True ) ) )🤖 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 `@bench/agentloop/grade_answers.py` around lines 82 - 88, Validate the cell count in the row-parsing loop before constructing the dictionary: accept and pad rows with fewer than len(COLUMNS) cells, but reject rows with extra cells instead of allowing zip to truncate them. Update the zip call to use strict=True once the lengths are guaranteed equal, preserving the fail-closed behavior described by the parser’s docstring.Source: Linters/SAST tools
bench/agentloop/grade_answers.py-284-296 (1)
284-296: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
at_pin()accepts traversal segments from agent-authored answers.
PATH_REmatches..as a segment, so an answer containing../../../../etc/hostsresolves outsidepin_root.at_pin()then reports it as grounded, andsymbol_in_file()/verbatim_ok()read that file. The grader consumes untrusted model output, so it should not follow paths out of the pin.🔒 Proposed fix
def at_pin( pin_root, row, rel_path ): + if ".." in pathlib.PurePosixPath( rel_path ).parts or rel_path.startswith( "/" ): + return None for root in [ pathlib.Path( pin_root ) ] + [ pathlib.Path( pin_root ) / r for r, _ in pin_pairs( row ) ]:🤖 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 `@bench/agentloop/grade_answers.py` around lines 284 - 296, Update at_pin() to reject agent-authored rel_path values containing traversal segments before joining them with pin_root or pinned repository roots; require paths to remain within the selected root, while preserving the existing repo-relative and pinned-repository resolution behavior for safe paths.bench/agentloop/grade_answers.py-208-220 (1)
208-220: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAccumulate OpenCode text parts before grading
When OpenCode emits multiple
type: "text"events,text = part["text"]keeps only the last event. This can discard<<<ANSWER>>>and earlier answer content. Accumulate the text parts before callingfenced_answer.🤖 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 `@bench/agentloop/grade_answers.py` around lines 208 - 220, The event parsing logic should accumulate all OpenCode text event parts instead of overwriting text with the latest part. Update the text extraction loop around the text variable and text event handling so earlier content, including answer markers, is preserved before the result is passed to fenced_answer; retain the existing item.completed agent_message handling.bench/agentloop/run_agentloop.py-580-584 (1)
580-584: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winTwo symlink guards use
exists(), which follows the link. In both places the code testsnot link.exists()before callingsymlink_to. For a symlink whose target has been removed,exists()returnsFalse, andsymlink_tothen raisesFileExistsErrorand aborts the run.
bench/agentloop/run_agentloop.py#L580-L584: change the skills-tree guard toif not link.exists() and not link.is_symlink():.bench/agentloop/run_agentloop.py#L596-L599: change the credential guard toif source.exists() and not link.exists() and not link.is_symlink():.🤖 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 `@bench/agentloop/run_agentloop.py` around lines 580 - 584, Update both symlink guards in run_agentloop.py: at lines 580-584, require both not link.exists() and not link.is_symlink() before creating skill links; at lines 596-599, retain source.exists() and additionally require both not link.exists() and not link.is_symlink() before creating credential links. This prevents broken existing symlinks from triggering symlink_to failures.bench/agentloop/run_agentloop.py-1064-1065 (1)
1064-1065: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winSplit the compound statement flagged by Ruff E701.
for arm in arms: - if arm not in ARMS: raise SystemExit( f"unknown arm {arm!r}; expected one of {ARMS}" ) + if arm not in ARMS: + raise SystemExit( f"unknown arm {arm!r}; expected one of {ARMS}" )🤖 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 `@bench/agentloop/run_agentloop.py` around lines 1064 - 1065, In the arms validation loop, split the inline if-and-raise compound statement into a properly indented multi-line conditional while preserving the existing unknown-arm validation and error message.Source: Linters/SAST tools
third_party/patches/README.md-51-51 (1)
51-51: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winFix the MD038 lint hit in the new table row.
The row contains the code span
`- `with a trailing space inside the backticks. markdownlint reports MD038 (no-space-in-code). Change the span so the space is outside it, or describe the marker without the trailing space.📝 Proposed wording fix
-| `markdown/001-serialize-bounds.patch` | `serialize()` memcpys `open_blocks.size * sizeof(Block)` bytes after a 5-byte header with NO bounds check at all against the 1024-byte serialization buffer — the yaml/001 defect class, minus even the bare guard. Measured on v0.5.3 standalone (2026-08-12): 300 nested blockquote markers, or 300 `- ` list markers on ONE line, abort `ts_assert(length <= 1024)` (SIGABRT rc=134); under `NDEBUG` the write corrupts the heap silently. +| `markdown/001-serialize-bounds.patch` | `serialize()` memcpys `open_blocks.size * sizeof(Block)` bytes after a 5-byte header with NO bounds check at all against the 1024-byte serialization buffer — the yaml/001 defect class, minus even the bare guard. Measured on v0.5.3 standalone (2026-08-12): 300 nested blockquote markers, or 300 space-separated `-` list markers on ONE line, abort `ts_assert(length <= 1024)` (SIGABRT rc=134); under `NDEBUG` the write corrupts the heap silently.(The rest of the row is unchanged.)
🤖 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 `@third_party/patches/README.md` at line 51, Update the table row containing the `- ` code span so the trailing space is not inside the backticks, while preserving the row’s meaning and all other wording.Source: Linters/SAST tools
src/ingest.cpp-657-668 (1)
657-668: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCharge tabs at scanner width in the Markdown depth prescan
A nested list with one additional leading tab per line can produce 256 list-marker blocks, while
mdNestsTooDeep()reports only 129. The scanner counts each leading tab as four columns and pushes one block per list marker. This input passeskMaxMdBlockDepth = 200and reaches serialization with truncated scanner state.Charge each tab as four columns, update the
kMaxMdBlockDepthcomment, and add a tab-indented fixture.🤖 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 `@src/ingest.cpp` around lines 657 - 668, Update the leading-whitespace handling in the Markdown depth prescan so each tab increases indent by the scanner width of four columns rather than one. Revise the kMaxMdBlockDepth comment to document this accounting, and add a tab-indented nested-list fixture covering the depth mismatch and expected rejection behavior.docs/EVALS.md-24-26 (1)
24-26: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the duplicate gate-suite text.
The table contains the same gate-suite row three times. The later gate-suite and caveat paragraphs also repeat their new sentence three times. Retain one instance at each location.
As per path instructions, focus on major issues impacting performance, readability, maintainability and security.
Also applies to: 995-997, 1631-1633
🤖 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 `@docs/EVALS.md` around lines 24 - 26, Remove the duplicate gate-suite table rows and repeated gate-suite/caveat sentences in docs/EVALS.md, including the corresponding occurrences near the later referenced sections. Retain exactly one instance of each intended row and sentence, preserving the surrounding documentation and wording.Source: Path instructions
🧹 Nitpick comments (8)
src/main.cpp (2)
6109-6146: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the unused
callSitesOfNamebinding.Line 6116 destructures
callSitesOfNameand line 6117 discards it with a cast. The--usesverb reports it ascall_sites_of_name=, but--verifydoes not. Bind only what the branch uses.♻️ Proposed refactor
- const auto [ sites, callSitesOfName ] = collectUseSites( ing, sel, isChosenCaller ); - (void) callSitesOfName; + const std::vector<UseSite> sites = collectUseSites( ing, sel, isChosenCaller ).first;🤖 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 `@src/main.cpp` around lines 6109 - 6146, Update the uses/unused verification branch to stop binding the unused callSitesOfName result from collectUseSites. Destructure only the sites value needed by the branch and remove the corresponding discard cast, leaving resolveUsesSelector, usesChosenCallers, and site reporting unchanged.
6259-6291: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueCall
builtinLayeronce per node.Line 6287 calls
builtinLayer( ing.files[ fileId ] )twice for every symbol in the reach set. The call performs a path classification per invocation. Bind it once.♻️ Proposed refactor
for( NodeId n : reach ) { const std::uint32_t fileId = ing.symbols[n].fileId; - if( claim.arg2Quoted ? bool( fileFlags[ fileId ] ) : ( builtinLayer( ing.files[ fileId ] ) != nullptr && claim.arg2 == builtinLayer( ing.files[ fileId ] ) ) ) + bool isWitness = false; + if( claim.arg2Quoted ) + { + isWitness = fileFlags[ fileId ] != 0; + } + else if( const char* layer = builtinLayer( ing.files[ fileId ] ) ) + { + isWitness = ( claim.arg2 == layer ); + } + if( isWitness ) { witnesses.push_back( n ); } }🤖 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 `@src/main.cpp` around lines 6259 - 6291, In the reach filtering loop, update the claim.arg2 layer comparison to call builtinLayer once per node, store its result in a local variable, and reuse it for the null check and equality comparison. Keep the quoted-file matching path unchanged.bench/agentloop/grade_answers.py (1)
485-490: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueS rows report
precision=0.0when no symbol line parses.
SYMBOL_RErequires thefile.ext: symbolshape at the start of a line. If an S answer states symbols in prose,pairsis empty, and line 490 replaces the path precision from line 483 with a hard0.0. The report then shows a measured precision of zero for something that was never measured.symbol_okalready staysFalsein that case, so the FAIL is preserved either way. ConsiderNonefor the unmeasured case so the table does not read as a measurement.♻️ Proposed change
scores.update( hits=len( good ), symbol_ok=bool( pairs ) and len( good ) == len( pairs ), - precision=( len( good ) / len( pairs ) ) if pairs else 0.0 ) + precision=( len( good ) / len( pairs ) ) if pairs else None )🤖 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 `@bench/agentloop/grade_answers.py` around lines 485 - 490, Update the S-row scoring logic around grader == "S" so precision remains None when answer_symbols(answer) returns no pairs, rather than overwriting the existing unmeasured path precision with 0.0. Preserve the current measured precision calculation for non-empty pairs and keep symbol_ok false for the empty-pairs case.test/agentloopclaudecheck.sh (1)
49-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe
prepare_environmentbranch assertion is vacuous forclaude-code-p.
if f'"{harness}"' in dispatch or harness == "claude-code-p"passes unconditionally for the default harness, because the claude case is the fallthrough return and carries no literal. The check reports PASS even if that branch is deleted. Assert the returned home instead, for example thatprepare_environment( "claude-code-p", ... )setsCLAUDE_CONFIG_DIR.🤖 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 `@test/agentloopclaudecheck.sh` around lines 49 - 53, Replace the unconditional claude-code-p exception in the prepare_environment branch loop with an assertion on its observable behavior: invoke prepare_environment for "claude-code-p" and verify it sets CLAUDE_CONFIG_DIR appropriately. Keep literal-dispatch checks for the other harnesses and ensure the test fails if the claude-specific behavior is removed.test/agentloopgradercheck.sh (1)
98-103: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThese patterns require a column after the verdict.
'^F11 .* PASS 'and'^F05 .* PASS 'both end with a tab, so they match only ifPASSis not the final field of the row. If the grade table's verdict ever becomes the last column, these two checks fail silently rather than detecting a real regression. Anchor on the verdict field without requiring a trailing separator, or useawkon the field index.Also applies to: 113-117
🤖 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 `@test/agentloopgradercheck.sh` around lines 98 - 103, Update the F11 check and the corresponding F05 check in the grade-table validation to match PASS as the verdict field even when it is the final column, removing the required trailing tab or using field-aware awk matching. Preserve the existing row identifiers and pass/fail behavior.src/ingest.cpp (2)
278-279: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUpdate the two stale "markdown — no grammar/query" comments.
.mdand.markdownnow carry a real grammar pointer. Thele.grammar == nullptrearly returns incompileQueryStandaloneandcompiledQueryForstill say// markdown — no grammar/query. Markdown no longer takes those branches; it is filtered by the emptyquerySubinstead. Behavior is correct, but the comments now point a reader at the wrong lane.🤖 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 `@src/ingest.cpp` around lines 278 - 279, The comments on the markdown early returns in compileQueryStandalone and compiledQueryFor are stale because Markdown now has a grammar and is excluded through the empty querySub filter. Update both comments to describe the actual no-grammar/null-grammar condition without implying Markdown lacks a grammar or query.
5283-5288: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueNormalize the anchor target with
mdSlugOf, and precompute the heading slugs.Two small points in the anchor path:
- The target slug is only lowercased at Line 5286, while the comparison side runs the full
mdSlugOf. An author who writes a raw anchor such as[x](#Node.jsAPI)producesnode.js api, while the headingNode.js APIproducesnodejs-api, so the link drops. Passing the target throughmdSlugOfmakes both sides use one rule.- The resolution loop calls
mdtier::mdSlugOf( h.name )for every (anchor, heading) pair, and each call allocates. Computing each heading's slug once before the loop makes the pass linear in headings.♻️ Proposed change
if( target.front() == '#' ) { - std::string slug; - for( const char c : target.substr( 1 ) ) { slug += ( c >= 'A' && c <= 'Z' ) ? char( c - 'A' + 'a' ) : c; } - pendingAnchors.emplace_back( refByte, std::move( slug ) ); + // one slug rule on both sides — an author may write either the GitHub slug or the raw heading + pendingAnchors.emplace_back( refByte, mdtier::mdSlugOf( target.substr( 1 ) ) ); return; }+ std::vector<std::string> headingSlugs; + headingSlugs.reserve( walk.headings.size() ); + for( const MdHeading& h : walk.headings ) { headingSlugs.push_back( mdtier::mdSlugOf( h.name ) ); } for( const auto& [ refByte, slug ] : pendingAnchors ) { - for( const MdHeading& h : walk.headings ) + for( std::size_t hi = 0; hi < walk.headings.size(); ++hi ) { - if( mdtier::mdSlugOf( h.name ) == slug ) + if( headingSlugs[ hi ] == slug ) { RawRef r; r.fileId = fileId; r.startByte = refByte; r.lang = Lang::Markdown; r.isInherit = false; - r.name = h.name; + r.name = walk.headings[ hi ].name; refs.push_back( std::move( r ) ); break; } } }Also applies to: 5469-5474
🤖 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 `@src/ingest.cpp` around lines 5283 - 5288, Update the raw-anchor handling in the target-processing path to normalize the substring after '#' with mdSlugOf instead of only lowercasing it before storing pendingAnchors. In the anchor resolution loop, precompute each heading’s mdSlugOf(h.name) once before iterating over pending anchors, then compare against the cached slug to avoid recomputing it for every anchor-heading pair.bench/agentloop/README.md (1)
276-278: 📐 Maintainability & Code Quality | 🔵 TrivialTrack the
--bareOAuth blocker.The text records an unresolved blocker:
--bareforcesANTHROPIC_API_KEYand never reads OAuth, so a--live-onerun must confirm that OAuth survives a redirectedCLAUDE_CONFIG_DIRbefore a matrix is booked. An open blocker recorded only in prose is easy to lose between runs.Do you want me to open an issue to track this verification step?
🤖 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 `@bench/agentloop/README.md` around lines 276 - 278, Track the unresolved --bare OAuth verification as an explicit issue or actionable checklist item in the README, including the required --live-one test with redirected CLAUDE_CONFIG_DIR and the prerequisite confirmation before booking a matrix. Keep the existing blocker context and acceptance criteria visible rather than leaving them only in prose.
🤖 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 `@bench/agentloop/grade_answers.py`:
- Around line 119-133: Update verify_pin to reject any empty sha returned by
pin_pairs before comparing it with the repository HEAD. Return an appropriate
validation error for the missing pin, while preserving the existing checkout and
pinned-commit checks for non-empty sha values.
- Around line 65-68: Update CIRCULAR_RE to recognize newline as a
command-position separator, ensuring ripwire or ctxpack immediately following a
newline is detected by is_circular(). Preserve the existing separator handling
and command-prefix matching.
- Around line 537-544: Update the report-printing loop over graded records so
empty scenario_class values do not index into an empty split result. Preserve
the existing first-token output for non-empty values and emit a safe fallback
for blank or missing scenario_class entries, preventing report generation from
failing after grading completes.
- Around line 158-164: Update run_gt to catch subprocess.TimeoutExpired and
FileNotFoundError from subprocess.run, returning the existing refusal
representation for that row instead of propagating the exception. Ensure
grade_instance and its GT_EMPTY handling receive a result that reports the
affected row while preserving verdicts already computed for other rows.
- Around line 606-621: The explicit precondition-error paths currently raise
SystemExit with status 1 instead of the documented status 2. Add a shared helper
that writes the supplied message to stderr and raises SystemExit(2), then
replace the relevant string-based SystemExit calls in main(), load_instances(),
and answers_from_results() with that helper while preserving their existing
messages.
In `@bench/agentloop/run_agentloop.py`:
- Around line 895-897: Update the result-retention flow around retain and
_claude_metrics so the transcript is stored alongside the --results-out file and
events_path is recorded as a relative path, rather than being derived from the
absolute work_dir. Preserve the existing task/arm/seed filename scheme and
ensure the recorded path resolves correctly from the results bundle directory.
- Around line 601-608: Update build_harness_command so an unknown non-empty tier
is rejected before calling build_claude_command, rather than passing None from
TIER_BUDGET_USD.get(tier). Preserve the existing dispatch for codex-exec,
opencode, and known tiers, and raise a clear error identifying the invalid tier
so no uncapped Claude run can proceed.
- Around line 1052-1057: Update the questions lock construction in the run-agent
flow to compute content_sha256 from the actual bytes of a.questions rather than
its basename, using the existing hashlib convention and matching load_tasks_lock
exactly. Preserve the “questions:” prefix and ensure the task-source
verification output reflects the resulting content digest.
- Around line 936-939: Update the repository checkout selection in the
question-run path to refuse execution when is_question is true and local_corpus
is empty, rather than falling through to checkout_repo. Return _fail with an
actionable message directing the caller to provide --local-corpus, while
preserving checkout_local_pin for questions with a local corpus and
checkout_repo for non-question tasks.
- Around line 231-247: Update checkout_local_pin to isolate each run by removing
or resetting the existing worktree at dest / repo before reuse, ensuring no
agent modifications persist between arms or seeds. Also clean up stale worktrees
after use or before recreating them so each local source repo’s git worktree
list remains bounded, while preserving the existing fail-closed behavior.
In `@src/ingest.cpp`:
- Around line 5050-5063: Update mdCleanHeadingText so trailing # characters are
removed only when the hash run is preceded by whitespace; preserve hashes that
are part of heading text such as C# or F#. Keep the existing trailing-whitespace
cleanup and remove the closing sequence only when that whitespace boundary is
present.
In `@src/recall.h`:
- Around line 560-568: Replace the full scan over kept sections in the overlap
check with an ordered interval lookup keyed by sigStartByte: use lower_bound to
find the insertion position for s, then test only the immediate predecessor and
successor intervals for overlap. Maintain the kept intervals in sigStartByte
order as sections are accepted, while preserving the existing overlap semantics.
In `@src/verify.h`:
- Around line 122-189: Strengthen claim parsing before setting Claim::ok:
validate that the outer expression has exactly one closing parenthesis and
reject unmatched or nested trailing structure instead of including it in an
argument. In the two-argument path, require the second argument to use the
quoted-literal form for contains, while preserving bare-token handling only for
shapes that allow it. Also reject nested parenthesized text in the one-argument
path, returning refuse for all syntax outside the supported grammar.
In `@test/agentloopclaudecheck.sh`:
- Around line 233-238: Update the Python contract-check blocks in
test/agentloopclaudecheck.sh (lines 233-238) and test/agentloopgradercheck.sh
(lines 195-199, for the block starting at line 144) to capture the heredoc
interpreter exit status immediately afterward, call no when it is non-zero, and
require a minimum assertion count before reporting success. Ensure both gates
cannot print ALL PASS when the checks produce no assertions or terminate early.
---
Minor comments:
In `@bench/agentloop/grade_answers.py`:
- Around line 82-88: Validate the cell count in the row-parsing loop before
constructing the dictionary: accept and pad rows with fewer than len(COLUMNS)
cells, but reject rows with extra cells instead of allowing zip to truncate
them. Update the zip call to use strict=True once the lengths are guaranteed
equal, preserving the fail-closed behavior described by the parser’s docstring.
- Around line 284-296: Update at_pin() to reject agent-authored rel_path values
containing traversal segments before joining them with pin_root or pinned
repository roots; require paths to remain within the selected root, while
preserving the existing repo-relative and pinned-repository resolution behavior
for safe paths.
- Around line 208-220: The event parsing logic should accumulate all OpenCode
text event parts instead of overwriting text with the latest part. Update the
text extraction loop around the text variable and text event handling so earlier
content, including answer markers, is preserved before the result is passed to
fenced_answer; retain the existing item.completed agent_message handling.
In `@bench/agentloop/run_agentloop.py`:
- Around line 580-584: Update both symlink guards in run_agentloop.py: at lines
580-584, require both not link.exists() and not link.is_symlink() before
creating skill links; at lines 596-599, retain source.exists() and additionally
require both not link.exists() and not link.is_symlink() before creating
credential links. This prevents broken existing symlinks from triggering
symlink_to failures.
- Around line 1064-1065: In the arms validation loop, split the inline
if-and-raise compound statement into a properly indented multi-line conditional
while preserving the existing unknown-arm validation and error message.
In `@docs/EVALS.md`:
- Around line 24-26: Remove the duplicate gate-suite table rows and repeated
gate-suite/caveat sentences in docs/EVALS.md, including the corresponding
occurrences near the later referenced sections. Retain exactly one instance of
each intended row and sentence, preserving the surrounding documentation and
wording.
In `@present/deck5_ripwire_build.js`:
- Line 149: Update the documented-command count in the foot() call within
deck5_ripwire_build.js from 117 to 123, leaving the generated flag count and
surrounding wording unchanged.
In `@src/ingest.cpp`:
- Around line 657-668: Update the leading-whitespace handling in the Markdown
depth prescan so each tab increases indent by the scanner width of four columns
rather than one. Revise the kMaxMdBlockDepth comment to document this
accounting, and add a tab-indented nested-list fixture covering the depth
mismatch and expected rejection behavior.
In `@test/verifycheck.sh`:
- Line 37: Guard the directory change in the test script by making the cd
"$ROOT" command fail immediately when it cannot enter the target directory.
Preserve the existing test flow after a successful directory change.
- Around line 222-226: Update the xmllint validation loop in test/verifycheck.sh
to detect whether xmllint is available before validating files; report the check
as skipped when it is missing, and retain the existing failure reporting and
well-formedness gate when the tool is installed.
- Around line 181-185: Update the fixture setup used by the reaches layer
assertion so its path retains a test/, tests/, or bench/ directory component,
allowing builtinLayer() to resolve the test layer. Alternatively, add an
explicit path precondition before running the verify command, while preserving
the existing confirmed-verdict assertion.
In `@third_party/patches/README.md`:
- Line 51: Update the table row containing the `- ` code span so the trailing
space is not inside the backticks, while preserving the row’s meaning and all
other wording.
---
Nitpick comments:
In `@bench/agentloop/grade_answers.py`:
- Around line 485-490: Update the S-row scoring logic around grader == "S" so
precision remains None when answer_symbols(answer) returns no pairs, rather than
overwriting the existing unmeasured path precision with 0.0. Preserve the
current measured precision calculation for non-empty pairs and keep symbol_ok
false for the empty-pairs case.
In `@bench/agentloop/README.md`:
- Around line 276-278: Track the unresolved --bare OAuth verification as an
explicit issue or actionable checklist item in the README, including the
required --live-one test with redirected CLAUDE_CONFIG_DIR and the prerequisite
confirmation before booking a matrix. Keep the existing blocker context and
acceptance criteria visible rather than leaving them only in prose.
In `@src/ingest.cpp`:
- Around line 278-279: The comments on the markdown early returns in
compileQueryStandalone and compiledQueryFor are stale because Markdown now has a
grammar and is excluded through the empty querySub filter. Update both comments
to describe the actual no-grammar/null-grammar condition without implying
Markdown lacks a grammar or query.
- Around line 5283-5288: Update the raw-anchor handling in the target-processing
path to normalize the substring after '#' with mdSlugOf instead of only
lowercasing it before storing pendingAnchors. In the anchor resolution loop,
precompute each heading’s mdSlugOf(h.name) once before iterating over pending
anchors, then compare against the cached slug to avoid recomputing it for every
anchor-heading pair.
In `@src/main.cpp`:
- Around line 6109-6146: Update the uses/unused verification branch to stop
binding the unused callSitesOfName result from collectUseSites. Destructure only
the sites value needed by the branch and remove the corresponding discard cast,
leaving resolveUsesSelector, usesChosenCallers, and site reporting unchanged.
- Around line 6259-6291: In the reach filtering loop, update the claim.arg2
layer comparison to call builtinLayer once per node, store its result in a local
variable, and reuse it for the null check and equality comparison. Keep the
quoted-file matching path unchanged.
In `@test/agentloopclaudecheck.sh`:
- Around line 49-53: Replace the unconditional claude-code-p exception in the
prepare_environment branch loop with an assertion on its observable behavior:
invoke prepare_environment for "claude-code-p" and verify it sets
CLAUDE_CONFIG_DIR appropriately. Keep literal-dispatch checks for the other
harnesses and ensure the test fails if the claude-specific behavior is removed.
In `@test/agentloopgradercheck.sh`:
- Around line 98-103: Update the F11 check and the corresponding F05 check in
the grade-table validation to match PASS as the verdict field even when it is
the final column, removing the required trailing tab or using field-aware awk
matching. Preserve the existing row identifiers and pass/fail behavior.
🪄 Autofix
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 Plus
Run ID: 98c12763-5396-4016-b55f-9d65ad4c521e
⛔ Files ignored due to path filters (2)
bench/agentloop/fixtures/grader/expected.tsvis excluded by!**/*.tsvbench/agentloop/fixtures/grader/instances.tsvis excluded by!**/*.tsv
📒 Files selected for processing (62)
.ripwire_quality_acksCMakeLists.txtREADME.mdbench/agentloop/README.mdbench/agentloop/fixtures/grader/results.jsonbench/agentloop/fixtures/grader/sealed_key.jsonbench/agentloop/fixtures/grader/transcripts/F01.jsonlbench/agentloop/fixtures/grader/transcripts/F02.jsonlbench/agentloop/fixtures/grader/transcripts/F03.jsonlbench/agentloop/fixtures/grader/transcripts/F04.jsonlbench/agentloop/fixtures/grader/transcripts/F05.jsonlbench/agentloop/fixtures/grader/transcripts/F06.jsonlbench/agentloop/fixtures/grader/transcripts/F07.jsonlbench/agentloop/fixtures/grader/transcripts/F08.jsonlbench/agentloop/fixtures/grader/transcripts/F09.jsonlbench/agentloop/fixtures/grader/transcripts/F10.jsonlbench/agentloop/fixtures/grader/transcripts/F11.jsonlbench/agentloop/grade_answers.pybench/agentloop/run_agentloop.pydocs/COMMANDS.mddocs/EVALS.mdpresent/deck5_ripwire_build.jsskills/ripwire-navigate/SKILL.mdsrc/cli.hsrc/ingest.cppsrc/ingest.hsrc/main.cppsrc/quality.hsrc/recall.hsrc/verify.htest/agentloopclaudecheck.shtest/agentloopgradercheck.shtest/dependencypincheck.shtest/docmentioncheck.shtest/fillordercheck.shtest/golden.xmltest/mdsectioncheck.shtest/mdsectionfix/alt.markdowntest/mdsectionfix/crlf.mdtest/mdsectionfix/decoy.mdtest/mdsectionfix/guide.mdtest/mdsectionfix/helpers.ctest/mdsectionfix/nearlimit.mdtest/mdsectionfix/partner.mdtest/mdsectionfix/plainprose.mdtest/mdsectionfix/setext0.mdtest/qschemetrip.hashtest/qschemetripcheck.shtest/regression.shtest/vendorpatchcheck.shtest/verifycheck.shtest/verifyfix/chain.cpptest/verifyfix/registry.cpptest/xmlwellformed.shthird_party/deps/markdown/LICENSEthird_party/deps/markdown/src/parser.cthird_party/deps/markdown/src/scanner.cthird_party/deps/markdown/src/tree_sitter/alloc.hthird_party/deps/markdown/src/tree_sitter/array.hthird_party/deps/markdown/src/tree_sitter/parser.hthird_party/patches/README.mdthird_party/patches/markdown/001-serialize-bounds.patch
| # ripwire in COMMAND POSITION — start of the command, or after a pipe/;/&&/||/backtick/$( — which is | ||
| # what protocol §3's "no gt_command invokes ripwire" actually forbids. `grep -rn X ripwire/src` names | ||
| # a directory and is fine; nine admitted rows depend on that distinction. | ||
| CIRCULAR_RE = re.compile( r"(?:^|[|;&`]|\$\(|&&|\|\|)\s*(?:[\w./\-]*/)?(ripwire|ctxpack)\b" ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
CIRCULAR_RE misses ripwire after a newline.
The separator class covers |, ;, &, backtick, $(, &&, ||, and string start. A newline is also a shell command separator, but the pattern has no re.MULTILINE and no \n in the class. A gt_command written as:
cd repo
ripwire recall foo
passes is_circular() and gets scored. The non-circularity rule is described as load-bearing, so the gap defeats the check it enforces.
🔒 Proposed fix
-CIRCULAR_RE = re.compile( r"(?:^|[|;&`]|\$\(|&&|\|\|)\s*(?:[\w./\-]*/)?(ripwire|ctxpack)\b" )
+CIRCULAR_RE = re.compile( r"(?:^|[|;&`\n]|\$\(|&&|\|\|)\s*(?:[\w./\-]*/)?(ripwire|ctxpack)\b" )📝 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.
| # ripwire in COMMAND POSITION — start of the command, or after a pipe/;/&&/||/backtick/$( — which is | |
| # what protocol §3's "no gt_command invokes ripwire" actually forbids. `grep -rn X ripwire/src` names | |
| # a directory and is fine; nine admitted rows depend on that distinction. | |
| CIRCULAR_RE = re.compile( r"(?:^|[|;&`]|\$\(|&&|\|\|)\s*(?:[\w./\-]*/)?(ripwire|ctxpack)\b" ) | |
| # ripwire in COMMAND POSITION — start of the command, or after a pipe/;/&&/||/backtick/$( — which is | |
| # what protocol §3's "no gt_command invokes ripwire" actually forbids. `grep -rn X ripwire/src` names | |
| # a directory and is fine; nine admitted rows depend on that distinction. | |
| CIRCULAR_RE = re.compile( r"(?:^|[|;&`\n]|\$\(|&&|\|\|)\s*(?:[\w./\-]*/)?(ripwire|ctxpack)\b" ) |
🤖 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 `@bench/agentloop/grade_answers.py` around lines 65 - 68, Update CIRCULAR_RE to
recognize newline as a command-position separator, ensuring ripwire or ctxpack
immediately following a newline is detected by is_circular(). Preserve the
existing separator handling and command-prefix matching.
| def verify_pin( row, pin_root, allow_unpinned ): | ||
| """None when the tree under pin_root is at the row's pinned sha; an error string otherwise.""" | ||
| if row[ "pin_ref" ].strip() == FIXTURE_PIN: | ||
| return None if allow_unpinned else "pin_ref=FIXTURE requires --allow-unpinned (fixtures only)" | ||
| for repo, sha in pin_pairs( row ): | ||
| tree = pathlib.Path( pin_root ) / repo | ||
| if not tree.is_dir(): | ||
| return f"no checkout at {tree}" | ||
| head = subprocess.run( [ "git", "-C", str( tree ), "rev-parse", "HEAD" ], | ||
| capture_output=True, text=True ) | ||
| if head.returncode != 0: | ||
| return f"{tree} is not a git checkout" | ||
| if not head.stdout.strip().startswith( sha ): | ||
| return f"{tree} is at {head.stdout.strip()[ :12 ]}, not the pinned {sha}" | ||
| return None |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
An empty sha makes the pin check pass for any HEAD.
pin_pairs() returns whatever the pin_ref cell holds. If that cell is empty, or a part is written repo@, sha is "" and head.stdout.strip().startswith( "" ) is always True. The row is then graded against an unverified tree while reporting as pinned. Reject an empty sha explicitly.
🔒 Proposed fix
for repo, sha in pin_pairs( row ):
+ if not sha or not repo:
+ return f"pin_ref {row[ 'pin_ref' ]!r} does not name repo@sha"
tree = pathlib.Path( pin_root ) / repo📝 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.
| def verify_pin( row, pin_root, allow_unpinned ): | |
| """None when the tree under pin_root is at the row's pinned sha; an error string otherwise.""" | |
| if row[ "pin_ref" ].strip() == FIXTURE_PIN: | |
| return None if allow_unpinned else "pin_ref=FIXTURE requires --allow-unpinned (fixtures only)" | |
| for repo, sha in pin_pairs( row ): | |
| tree = pathlib.Path( pin_root ) / repo | |
| if not tree.is_dir(): | |
| return f"no checkout at {tree}" | |
| head = subprocess.run( [ "git", "-C", str( tree ), "rev-parse", "HEAD" ], | |
| capture_output=True, text=True ) | |
| if head.returncode != 0: | |
| return f"{tree} is not a git checkout" | |
| if not head.stdout.strip().startswith( sha ): | |
| return f"{tree} is at {head.stdout.strip()[ :12 ]}, not the pinned {sha}" | |
| return None | |
| def verify_pin( row, pin_root, allow_unpinned ): | |
| """None when the tree under pin_root is at the row's pinned sha; an error string otherwise.""" | |
| if row[ "pin_ref" ].strip() == FIXTURE_PIN: | |
| return None if allow_unpinned else "pin_ref=FIXTURE requires --allow-unpinned (fixtures only)" | |
| for repo, sha in pin_pairs( row ): | |
| if not sha or not repo: | |
| return f"pin_ref {row[ 'pin_ref' ]!r} does not name repo@sha" | |
| tree = pathlib.Path( pin_root ) / repo | |
| if not tree.is_dir(): | |
| return f"no checkout at {tree}" | |
| head = subprocess.run( [ "git", "-C", str( tree ), "rev-parse", "HEAD" ], | |
| capture_output=True, text=True ) | |
| if head.returncode != 0: | |
| return f"{tree} is not a git checkout" | |
| if not head.stdout.strip().startswith( sha ): | |
| return f"{tree} is at {head.stdout.strip()[ :12 ]}, not the pinned {sha}" | |
| return None |
🧰 Tools
🪛 ast-grep (0.45.1)
[error] 126-127: Command coming from incoming request
Context: subprocess.run( [ "git", "-C", str( tree ), "rev-parse", "HEAD" ],
capture_output=True, text=True )
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
🪛 Ruff (0.16.1)
[error] 127-127: subprocess call: check for execution of untrusted input
(S603)
[error] 127-127: Starting a process with a partial executable path
(S607)
🤖 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 `@bench/agentloop/grade_answers.py` around lines 119 - 133, Update verify_pin
to reject any empty sha returned by pin_pairs before comparing it with the
repository HEAD. Return an appropriate validation error for the missing pin,
while preserving the existing checkout and pinned-commit checks for non-empty
sha values.
| def run_gt( gt_command, pin_root, timeout_s=300 ): | ||
| """Execute the derivation command at the pin, under bash (never the operator's zsh — §4).""" | ||
| shell = globstar_shell() | ||
| argv = ( [ shell, "-O", "globstar", "-O", "nullglob", "-c", gt_command ] if shell | ||
| else [ "bash", "-c", gt_command ] ) | ||
| proc = subprocess.run( argv, capture_output=True, text=True, cwd=str( pin_root ), timeout=timeout_s ) | ||
| return proc.stdout, proc.stderr, proc.returncode |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
A timing-out or missing gt_command shell aborts the whole grading run.
subprocess.run(..., timeout=timeout_s) raises subprocess.TimeoutExpired after 300s, and the fallback bash on line 162 raises FileNotFoundError when no bash exists. Neither is caught here nor in grade_instance(). One slow row then discards every verdict already computed for the other rows. Convert both into a refusal verdict for that row.
🛡️ Proposed fix
- proc = subprocess.run( argv, capture_output=True, text=True, cwd=str( pin_root ), timeout=timeout_s )
- return proc.stdout, proc.stderr, proc.returncode
+ try:
+ proc = subprocess.run( argv, capture_output=True, text=True, cwd=str( pin_root ),
+ timeout=timeout_s )
+ except subprocess.TimeoutExpired:
+ return "", f"gt_command exceeded {timeout_s}s", 124
+ except OSError as exc:
+ return "", f"cannot execute the derivation shell: {exc}", 127
+ return proc.stdout, proc.stderr, proc.returncodeThe GT_EMPTY branch at line 432 then reports the row instead of crashing the report.
📝 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.
| def run_gt( gt_command, pin_root, timeout_s=300 ): | |
| """Execute the derivation command at the pin, under bash (never the operator's zsh — §4).""" | |
| shell = globstar_shell() | |
| argv = ( [ shell, "-O", "globstar", "-O", "nullglob", "-c", gt_command ] if shell | |
| else [ "bash", "-c", gt_command ] ) | |
| proc = subprocess.run( argv, capture_output=True, text=True, cwd=str( pin_root ), timeout=timeout_s ) | |
| return proc.stdout, proc.stderr, proc.returncode | |
| def run_gt( gt_command, pin_root, timeout_s=300 ): | |
| """Execute the derivation command at the pin, under bash (never the operator's zsh — §4).""" | |
| shell = globstar_shell() | |
| argv = ( [ shell, "-O", "globstar", "-O", "nullglob", "-c", gt_command ] if shell | |
| else [ "bash", "-c", gt_command ] ) | |
| try: | |
| proc = subprocess.run( argv, capture_output=True, text=True, cwd=str( pin_root ), | |
| timeout=timeout_s ) | |
| except subprocess.TimeoutExpired: | |
| return "", f"gt_command exceeded {timeout_s}s", 124 | |
| except OSError as exc: | |
| return "", f"cannot execute the derivation shell: {exc}", 127 | |
| return proc.stdout, proc.stderr, proc.returncode |
🧰 Tools
🪛 ast-grep (0.45.1)
[error] 162-162: Use of unsanitized data to create processes
Context: subprocess.run( argv, capture_output=True, text=True, cwd=str( pin_root ), timeout=timeout_s )
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(os-system-unsanitized-data)
[error] 162-162: Command coming from incoming request
Context: subprocess.run( argv, capture_output=True, text=True, cwd=str( pin_root ), timeout=timeout_s )
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
🪛 Ruff (0.16.1)
[error] 163-163: subprocess call: check for execution of untrusted input
(S603)
🤖 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 `@bench/agentloop/grade_answers.py` around lines 158 - 164, Update run_gt to
catch subprocess.TimeoutExpired and FileNotFoundError from subprocess.run,
returning the existing refusal representation for that row instead of
propagating the exception. Ensure grade_instance and its GT_EMPTY handling
receive a result that reports the affected row while preserving verdicts already
computed for other rows.
| for g in graded: | ||
| rec = g.get( "record" ) or {} | ||
| print( "\t".join( str( x ) for x in ( | ||
| g[ "id" ], g[ "scenario_class" ].split()[ 0 ], g[ "tier" ], g[ "grader" ], | ||
| g.get( "arm", "-" ), g[ "verdict" ], fmt( g[ "recall" ] ), fmt( g[ "precision" ] ), | ||
| fmt( g[ "tau" ] ), g[ "hallucinated" ], g.get( "cap_hit", 0 ), | ||
| fmt( rec.get( "tokens_in" ) ), fmt( rec.get( "tokens_out" ) ), | ||
| fmt( rec.get( "command_calls" ) ), fmt( rec.get( "wall_seconds" ) ) ) ) ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
scenario_class.split()[ 0 ] raises IndexError on an empty cell.
load_instances() pads short rows with "" (line 86), so scenario_class can be an empty string. "".split() returns [], and line 540 then raises IndexError after every row has already been graded. The whole report is lost.
🛡️ Proposed fix
- g[ "id" ], g[ "scenario_class" ].split()[ 0 ], g[ "tier" ], g[ "grader" ],
+ g[ "id" ], ( g[ "scenario_class" ].split() or [ "-" ] )[ 0 ], g[ "tier" ], g[ "grader" ],📝 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.
| for g in graded: | |
| rec = g.get( "record" ) or {} | |
| print( "\t".join( str( x ) for x in ( | |
| g[ "id" ], g[ "scenario_class" ].split()[ 0 ], g[ "tier" ], g[ "grader" ], | |
| g.get( "arm", "-" ), g[ "verdict" ], fmt( g[ "recall" ] ), fmt( g[ "precision" ] ), | |
| fmt( g[ "tau" ] ), g[ "hallucinated" ], g.get( "cap_hit", 0 ), | |
| fmt( rec.get( "tokens_in" ) ), fmt( rec.get( "tokens_out" ) ), | |
| fmt( rec.get( "command_calls" ) ), fmt( rec.get( "wall_seconds" ) ) ) ) ) | |
| for g in graded: | |
| rec = g.get( "record" ) or {} | |
| print( "\t".join( str( x ) for x in ( | |
| g[ "id" ], ( g[ "scenario_class" ].split() or [ "-" ] )[ 0 ], g[ "tier" ], g[ "grader" ], | |
| g.get( "arm", "-" ), g[ "verdict" ], fmt( g[ "recall" ] ), fmt( g[ "precision" ] ), | |
| fmt( g[ "tau" ] ), g[ "hallucinated" ], g.get( "cap_hit", 0 ), | |
| fmt( rec.get( "tokens_in" ) ), fmt( rec.get( "tokens_out" ) ), | |
| fmt( rec.get( "command_calls" ) ), fmt( rec.get( "wall_seconds" ) ) ) ) ) |
🤖 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 `@bench/agentloop/grade_answers.py` around lines 537 - 544, Update the
report-printing loop over graded records so empty scenario_class values do not
index into an empty split result. Preserve the existing first-token output for
non-empty values and emit a safe fallback for blank or missing scenario_class
entries, preventing report generation from failing after grading completes.
| rows = { r[ "id" ]: r for r in load_instances( a.instances ) } | ||
| if a.audit: | ||
| return audit( list( rows.values() ) ) | ||
| if not a.pin_root: | ||
| raise SystemExit( "--pin-root is required to grade: the key is DERIVED at the pin, never frozen" ) | ||
| keys = json.loads( pathlib.Path( a.key ).read_text() ) if a.key else {} | ||
|
|
||
| if a.transcript: | ||
| if not a.instance_id: | ||
| raise SystemExit( "--transcript needs --instance-id" ) | ||
| blob = pathlib.Path( a.transcript ).read_text( errors="replace" ) | ||
| work = [ ( a.instance_id, "-", None, fenced_answer( transcript_answer_text( blob ) ), None ) ] | ||
| elif a.results: | ||
| work = answers_from_results( a.results ) | ||
| else: | ||
| raise SystemExit( "one of --results or --transcript is required (or --audit)" ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check what exit codes the grader gate expects.
fd -t f 'agentloopgradercheck.sh' --exec cat -n {}Repository: redhat-et/ripwire
Length of output: 11647
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- grade_answers.py: header, error handling, and call sites ---'
sed -n '1,110p' bench/agentloop/grade_answers.py
rg -n -C 2 'SystemExit|usage|precondition|exit 2|return [0-9]' bench/agentloop/grade_answers.py test bench 2>/dev/null | head -240
printf '%s\n' '--- gate references to exit status 2 or precondition failures ---'
rg -n -C 2 'exit 2|status 2|rc.*2|precondition|--transcript needs|--pin-root is required|one of --results' . --glob '!**/.git/**' | head -240Repository: redhat-et/ripwire
Length of output: 50375
🏁 Script executed:
#!/bin/bash
set -u
python3 - <<'PY'
import pathlib
p = pathlib.Path("bench/agentloop/grade_answers.py")
text = p.read_text()
for i, line in enumerate(text.splitlines(), 1):
if 'raise SystemExit' in line:
print(f"{i}: {line}")
PY
printf '%s\n' '--- relevant repository tests and documentation ---'
fd -t f . | rg '(^|/)(test|tests|bench|README|CLAUDE).*' | head -200
rg -n -C 3 'grade_answers|SystemExit|exit.*2|usage/precondition|--pin-root|--transcript' test tests bench README.md CLAUDE.md 2>/dev/null | head -300Repository: redhat-et/ripwire
Length of output: 30760
🏁 Script executed:
#!/bin/bash
set -u
printf '%s\n' '--- source contract and error paths ---'
sed -n '30,95p' bench/agentloop/grade_answers.py
sed -n '580,630p' bench/agentloop/grade_answers.py
python3 - <<'PY'
import pathlib, subprocess, sys
src = pathlib.Path("bench/agentloop/grade_answers.py").read_text()
print("--- SystemExit string semantics ---")
for expression in [
'raise SystemExit("message")',
'raise SystemExit(2)',
]:
code = f"{expression}\n"
p = subprocess.run([sys.executable, "-c", code], capture_output=True, text=True)
print(repr(expression), "returncode=", p.returncode, "stderr=", repr(p.stderr))
print("--- all literal SystemExit forms ---")
for i, line in enumerate(src.splitlines(), 1):
if "SystemExit" in line:
print(f"{i}: {line}")
PY
printf '%s\n' '--- repository assertions for grader precondition exit status ---'
rg -n -C 3 'grade_answers|exit 2|status 2|returncode|--pin-root is required|--transcript needs|one of --results' bench test tests 2>/dev/null | head -300Repository: redhat-et/ripwire
Length of output: 30479
Return status 2 for explicit precondition errors.
The documented exit code is 2, but every explicit raise SystemExit("...") path returns 1. Add a helper that prints the message to stderr and raises SystemExit(2), then use it in main(), load_instances(), and answers_from_results().
🤖 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 `@bench/agentloop/grade_answers.py` around lines 606 - 621, The explicit
precondition-error paths currently raise SystemExit with status 1 instead of the
documented status 2. Add a shared helper that writes the supplied message to
stderr and raises SystemExit(2), then replace the relevant string-based
SystemExit calls in main(), load_instances(), and answers_from_results() with
that helper while preserving their existing messages.
| if a.questions: | ||
| lock = dict( content_sha256=f"questions:{pathlib.Path( a.questions ).name}", | ||
| selected_count=0, selected_repo_count=0 ) | ||
| all_tasks = load_questions( a.questions ) | ||
| lock.update( selected_count=len( all_tasks ), | ||
| selected_repo_count=len( { t["repo"] for t in all_tasks } ) ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Hash the questions TSV content, not its filename.
content_sha256=f"questions:{pathlib.Path( a.questions ).name}" depends only on the basename. Two different banks named instances.tsv produce the same value. The --resume guard at Line 1138 compares this field to decide whether a prior results file was produced against the same task source, so it accepts a merge across two different banks and silently mixes their records.
The printed line at Line 1068 also states "task source verified" for a value that verifies nothing.
♻️ Proposed content hash
if a.questions:
- lock = dict( content_sha256=f"questions:{pathlib.Path( a.questions ).name}",
- selected_count=0, selected_repo_count=0 )
+ digest = hashlib.sha256( pathlib.Path( a.questions ).read_bytes() ).hexdigest()
+ lock = dict( content_sha256=f"questions:{digest}",
+ selected_count=0, selected_repo_count=0 )Confirm hashlib is imported, and that load_tasks_lock uses the same digest convention.
🤖 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 `@bench/agentloop/run_agentloop.py` around lines 1052 - 1057, Update the
questions lock construction in the run-agent flow to compute content_sha256 from
the actual bytes of a.questions rather than its basename, using the existing
hashlib convention and matching load_tasks_lock exactly. Preserve the
“questions:” prefix and ensure the task-source verification output reflects the
resulting content digest.
Source: Linters/SAST tools
| inline std::string mdCleanHeadingText( std::string_view raw ) | ||
| { | ||
| std::string name; | ||
| name.reserve( raw.size() ); | ||
| for( const char c : raw ) | ||
| { | ||
| if( c == '\r' ) { continue; } | ||
| name += ( c == '\n' ) ? ' ' : c; | ||
| } | ||
| while( !name.empty() && ( name.back() == ' ' || name.back() == '\t' ) ) { name.pop_back(); } | ||
| while( !name.empty() && name.back() == '#' ) { name.pop_back(); } // ATX closing sequence — the | ||
| while( !name.empty() && ( name.back() == ' ' || name.back() == '\t' ) ) { name.pop_back(); } // grammar keeps it (probed) | ||
| return name; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Look for heading fixtures ending in '#' and for existing closing-sequence assertions.
set -euo pipefail
echo "=== headings ending in '#' anywhere in the repo's markdown ==="
rg -n --glob '*.md' --glob '*.markdown' -P '^#{1,6}\s.*\S#+\s*$' | head -50
echo
echo "=== mdsectionfix fixtures: heading lines ==="
fd -e md -e markdown . test/mdsectionfix --exec sh -c 'echo "--- $1"; grep -nE "^#{1,6} |^=+$|^-+$" "$1" || true' _ {}
echo
echo "=== does the gate assert anything about closing '#' sequences? ==="
rg -n -C3 'closing|##\s*$|atx' test/mdsectioncheck.sh || trueRepository: redhat-et/ripwire
Length of output: 218
🏁 Script executed:
#!/bin/bash
set -u
echo "=== target function and callers ==="
rg -n -C8 'mdCleanHeadingText|mdSlugOf|canonical|path::' src/ingest.cpp || true
echo
echo "=== relevant source range ==="
sed -n '4980,5105p' src/ingest.cpp
echo
echo "=== markdown grammar and parser references ==="
rg -n -S 'tree_sitter_markdown|markdown_inline|mdsection|heading|atx' --glob '!build/**' --glob '!dist/**' . | head -250 || true
echo
echo "=== corrected fixture scan ==="
matches=0
while IFS= read -r -d '' f; do
echo "--- $f"
grep -nE '^#{1,6} |^=+$|^-+$' "$f" || true
matches=1
done < <(find test/mdsectionfix -type f \( -name '*.md' -o -name '*.markdown' \) -print0 2>/dev/null)
exit 0Repository: redhat-et/ripwire
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -u
echo "=== target function and callers ==="
rg -n -C8 'mdCleanHeadingText|mdSlugOf|canonical|path::' src/ingest.cpp || true
echo
echo "=== relevant source range ==="
sed -n '4980,5105p' src/ingest.cpp
echo
echo "=== markdown grammar and parser references ==="
rg -n -S 'tree_sitter_markdown|markdown_inline|mdsection|heading|atx' --glob '!build/**' --glob '!dist/**' . | head -250 || true
echo
echo "=== corrected fixture scan ==="
while IFS= read -r -d '' f; do
echo "--- $f"
grep -nE '^#{1,6} |^=+$|^-+$' "$f" || true
done < <(find test/mdsectionfix -type f \( -name '*.md' -o -name '*.markdown' \) -print0 2>/dev/null)Repository: redhat-et/ripwire
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -u
echo "=== vendored markdown files ==="
git ls-files | grep -Ei 'markdown|tree.?sitter' | head -200 || true
echo
echo "=== markdown parser setup ==="
sed -n '130,215p' src/ingest.cpp
sed -n '390,425p' CMakeLists.txt
rg -n -C4 'tree_sitter_markdown|TSParser|mdtier::|captureMarkdown|Lang::Markdown' src/ingest.cpp CMakeLists.txt
echo
echo "=== focused markdown fixtures ==="
find test -maxdepth 3 -type f \( -iname '*md*' -o -iname '*markdown*' \) -print 2>/dev/null | head -200
echo
echo "=== headings ending in hash, with safe scan ==="
find . -type f \( -name '*.md' -o -name '*.markdown' \) -not -path './.git/*' -print0 2>/dev/null |
xargs -0 grep -nE '^#{1,6}[[:space:]]+.*[^[:space:]]##?[[:space:]]*$' 2>/dev/null |
head -100 || trueRepository: redhat-et/ripwire
Length of output: 25024
🌐 Web query:
CommonMark ATX heading closing sequence trailing # whitespace before closing sequence empty heading specification
💡 Result:
According to the CommonMark specification, the rules regarding the closing sequence of ATX headings and whitespace are as follows: Closing Sequence Requirements The optional closing sequence of # characters must be preceded by at least one space or tab [1][2][3]. After this required space or tab, the closing sequence may be followed by any number of spaces or tabs, but no other characters [1][2][3]. If a sequence of # characters is not preceded by a space or tab, it is not considered a closing sequence and is instead parsed as part of the heading's contents [2][3]. Empty Headings ATX headings can be empty [3]. For an empty heading, the requirement for at least one space or tab between the opening # characters and the heading's contents is waived [1][2][3]. Summary of Key Rules - Closing Sequence: Must be preceded by a space or tab [1][3]. - Trailing Whitespace: Spaces or tabs are allowed after the closing sequence [1][3]. - Empty Headings: The mandatory space after the opening sequence is only required if the heading contains content; it is not required for empty headings [1][3]. - Parsing: Raw heading contents are stripped of leading and trailing spaces or tabs before being parsed as inline content [1][2].
Citations:
- 1: https://spec.commonmark.org/spec
- 2: https://github.com/commonmark/commonmark-spec/blob/master/spec.txt
- 3: https://raw.githubusercontent.com/commonmark/commonmark-spec/0.31.2/spec.txt
🌐 Web query:
tree-sitter-markdown tree-sitter-markdown grammar inline node ATX heading closing hashes C# heading
💡 Result:
In tree-sitter-markdown grammars, ATX headings are typically parsed as block-level nodes (e.g., atx_heading) that contain a marker (e.g., atx_h1_marker) and an inline content node [1][2][3]. While the CommonMark specification allows for optional closing hashes (e.g., # Heading #) [4], the handling of these closing hashes depends on the specific implementation of the tree-sitter-markdown grammar being used [5]. Current implementations (such as the tree-sitter-grammars/tree-sitter-markdown project) use a C-based scanner to identify ATX markers and content [6]. When parsing ATX headings, these grammars generally focus on capturing the heading level and the main content [1][2]. If closing hashes are present in the source text, they may be treated as part of the inline content node rather than being explicitly structured as a separate syntax node, depending on the complexity of the grammar's scanner [2][6]. If you are working with C# (e.g., using a wrapper like TreeSitter.NET), you will interface with these grammars via the exposed language symbols [7]. Since tree-sitter-markdown relies on a two-pass approach (block grammar followed by inline grammar) [5], you must ensure your parser setup correctly handles the range of the inline content [5]. If closing hashes are not being captured as separate nodes, they will appear within the range of the inline node, where you can further process them using standard string operations or by defining additional query patterns in your tree-sitter queries (e.g., using regex in a local predicate) if necessary to extract or validate the closing sequence [4]. Top results: [2] ophi-dev/tree-sitter-markdown-text@37a826a [6] https://github.com/tree-sitter-grammars/tree-sitter-markdown/blob/f969cd3ae3f9fbd4e43205431d0ae286014c05b5/tree-sitter-markdown/src/scanner.c [5] https://github.com/tree-sitter-grammars/tree-sitter-markdown
Citations:
- 1: https://github.com/ophi-dev/tree-sitter-markdown-text
- 2: ophi-dev/tree-sitter-markdown-text@37a826a
- 3: https://docs.rs/crate/tree-sitter-markdown-text/latest
- 4: https://ota-meshi.github.io/eslint-plugin-markdown-preferences/rules/atx-heading-closing-sequence.html
- 5: https://github.com/tree-sitter-grammars/tree-sitter-markdown
- 6: https://github.com/tree-sitter-grammars/tree-sitter-markdown/blob/f969cd3ae3f9fbd4e43205431d0ae286014c05b5/tree-sitter-markdown/src/scanner.c
- 7: bug: Newline expected at end of single line heading tree-sitter-grammars/tree-sitter-markdown#232
🏁 Script executed:
#!/bin/bash
set -u
echo "=== ATX grammar rules in generated parser ==="
rg -n -C8 'atx_h[1-6]|atx_heading|closing|ATX|hashes|inline' third_party/deps/markdown/src/parser.c third_party/deps/markdown/src/scanner.c | head -300
echo
echo "=== markdown test fixtures ==="
for f in test/mdsectionfix/*.md test/mdsectionfix/*.markdown; do
[ -f "$f" ] || continue
echo "--- $f"
cat -n "$f"
done
echo
echo "=== available parser/compiler tooling (no repository code executed) ==="
command -v cc || true
command -v clang || true
command -v python3 || true
python3 - <<'PY'
try:
import tree_sitter
print("python tree_sitter package: available")
except Exception as exc:
print("python tree_sitter package: unavailable:", type(exc).__name__)
PYRepository: redhat-et/ripwire
Length of output: 25882
🏁 Script executed:
#!/bin/bash
set -u
echo "=== generated grammar symbols and rule metadata ==="
sed -n '100,155p' third_party/deps/markdown/src/parser.c
rg -n -C3 'atx_heading_content|alias_sym_inline|_atx_heading' third_party/deps/markdown/src/parser.c | tail -120
echo
echo "=== all relevant heading implementation comments ==="
rg -n -C5 'closing sequence|closing #|heading text|ATX|atx' src/ingest.cpp test/mdsectioncheck.sh test/mdsectionfix third_party/deps/markdown 2>/dev/null | head -300
echo
echo "=== exact changed-file diff summary ==="
git diff --stat -- src/ingest.cpp
git diff --unified=3 -- src/ingest.cpp | sed -n '1,180p'Repository: redhat-et/ripwire
Length of output: 31498
Guard ATX closing-hash removal with preceding whitespace.
mdCleanHeadingText removes valid heading text from names such as ## C# and ### C# and F#. CommonMark treats trailing # characters as a closing sequence only when whitespace precedes them. Preserve # characters otherwise, because the current behavior changes canonical names and anchor resolution.
🤖 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 `@src/ingest.cpp` around lines 5050 - 5063, Update mdCleanHeadingText so
trailing # characters are removed only when the hash run is preceded by
whitespace; preserve hashes that are part of heading text such as C# or F#. Keep
the existing trailing-whitespace cleanup and remove the closing sequence only
when that whitespace boundary is present.
| for( const std::uint32_t k : kept ) | ||
| { | ||
| const Symbol& o = ing.symbols[ k ]; | ||
| if( s.sigStartByte < o.endByte && o.sigStartByte < s.endByte ) | ||
| { | ||
| overlaps = true; | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Avoid quadratic section-overlap checks.
Each candidate scans every retained section. A Markdown file with many positive-scoring sibling headings makes --recall perform O(N²) comparisons. Keep intervals ordered by sigStartByte and test only the adjacent intervals with lower_bound.
As per path instructions, focus on major issues impacting performance, readability, maintainability and security.
🤖 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 `@src/recall.h` around lines 560 - 568, Replace the full scan over kept
sections in the overlap check with an ordered interval lookup keyed by
sigStartByte: use lower_bound to find the insertion position for s, then test
only the immediate predecessor and successor intervals for overlap. Maintain the
kept intervals in sigStartByte order as sections are accepted, while preserving
the existing overlap semantics.
Source: Path instructions
| if( whole.back() != ')' ) | ||
| { | ||
| return refuse( whole, "no closing ')'" ); | ||
| } | ||
| std::string_view args = whole.substr( open + 1, whole.size() - open - 2 ); // between the parens | ||
|
|
||
| // argument 1 — a bare token up to the first ',' (or the whole args on arity 1) | ||
| if( arity == 1 ) | ||
| { | ||
| c.arg1 = trimWs( args ); | ||
| if( c.arg1.empty() ) | ||
| { | ||
| return refuse( whole, "the shape takes one argument, got none" ); | ||
| } | ||
| if( c.arg1.find( ',' ) != std::string_view::npos ) | ||
| { | ||
| return refuse( whole, "the shape takes one argument, got several" ); | ||
| } | ||
| c.ok = true; | ||
| return c; | ||
| } | ||
| const std::size_t comma = args.find( ',' ); | ||
| if( comma == std::string_view::npos ) | ||
| { | ||
| return refuse( whole, "the shape takes two arguments, got one" ); | ||
| } | ||
| c.arg1 = trimWs( args.substr( 0, comma ) ); | ||
| if( c.arg1.empty() ) | ||
| { | ||
| return refuse( whole, "the first argument is empty" ); | ||
| } | ||
|
|
||
| // argument 2 — QUOTE-FIRST: a leading '"' scans to the closing '"' verbatim (no escapes), so a comma | ||
| // inside a contains() literal never splits; only then may nothing but whitespace remain. | ||
| std::string_view rest = trimWs( args.substr( comma + 1 ) ); | ||
| if( !rest.empty() && rest.front() == '"' ) | ||
| { | ||
| const std::size_t close = rest.find( '"', 1 ); | ||
| if( close == std::string_view::npos ) | ||
| { | ||
| return refuse( whole, "unterminated '\"' in the second argument" ); | ||
| } | ||
| c.arg2 = rest.substr( 1, close - 1 ); | ||
| c.arg2Quoted = true; | ||
| if( !trimWs( rest.substr( close + 1 ) ).empty() ) | ||
| { | ||
| return refuse( whole, "trailing bytes after the quoted argument" ); | ||
| } | ||
| } | ||
| else | ||
| { | ||
| if( rest.find( ',' ) != std::string_view::npos ) | ||
| { | ||
| return refuse( whole, "the shape takes two arguments, got more" ); | ||
| } | ||
| c.arg2 = rest; | ||
| } | ||
| if( c.arg2.empty() && !c.arg2Quoted ) | ||
| { | ||
| return refuse( whole, "the second argument is empty" ); | ||
| } | ||
| // an EMPTY quoted literal is a degenerate contains() ("" occurs everywhere) — refuse it as a user | ||
| // error rather than confirming vacuously. | ||
| if( c.arg2Quoted && c.arg2.empty() ) | ||
| { | ||
| return refuse( whole, "the quoted literal is empty — an empty string occurs everywhere, so the claim measures nothing" ); | ||
| } | ||
| c.ok = true; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reject claim syntax outside the closed grammar.
At Line 122, the final-byte check accepts calls(A,B)) with arg2="B)". At Lines 154-189, contains(path, LIT) succeeds although contains requires a quoted literal. The one-argument path also accepts nested text such as uses(foo(bar)).
Validate the complete outer structure and shape-specific quoting before setting Claim::ok. Otherwise malformed claims can execute instead of producing the documented refusal.
🤖 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 `@src/verify.h` around lines 122 - 189, Strengthen claim parsing before setting
Claim::ok: validate that the outer expression has exactly one closing
parenthesis and reject unmatched or nested trailing structure instead of
including it in an argument. In the two-argument path, require the second
argument to use the quoted-literal form for contains, while preserving
bare-token handling only for shapes that allow it. Also reject nested
parenthesized text in the one-argument path, returning refuse for all syntax
outside the supported grammar.
| done < "$TMP/out.txt" | ||
|
|
||
| grep -q 'Traceback' "$TMP/out.txt" && no "python contract checks raised" | ||
|
|
||
| [ "$fail" = 0 ] && echo "ALL PASS" || echo "FAILURES ABOVE" | ||
| exit $fail |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Both new gates can print ALL PASS after running zero assertions. Each script runs its contract checks in a python3 heredoc, redirects all output to a file, and then parses that file for PASS/FAIL lines. Neither script captures the interpreter's exit status. If the block exits early without printing Traceback, the output file is empty, fail stays 0, and the gate reports success.
test/agentloopclaudecheck.sh#L233-L238: capture the heredoc's exit status into a variable immediately after the block, and callnowhen it is non-zero; also assert a minimum assertion count.test/agentloopgradercheck.sh#L195-L199: apply the same exit-status capture and non-zero check for the block that starts at Line 144.
📍 Affects 2 files
test/agentloopclaudecheck.sh#L233-L238(this comment)test/agentloopgradercheck.sh#L195-L199
🤖 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 `@test/agentloopclaudecheck.sh` around lines 233 - 238, Update the Python
contract-check blocks in test/agentloopclaudecheck.sh (lines 233-238) and
test/agentloopgradercheck.sh (lines 195-199, for the block starting at line 144)
to capture the heredoc interpreter exit status immediately afterward, call no
when it is non-zero, and require a minimum assertion count before reporting
success. Ensure both gates cannot print ALL PASS when the checks produce no
assertions or terminate early.
Three lanes close the collapse queue's buildable column.
Markdown section tier (kParserVer 63) — headings are symbols with real spans and hierarchy;
--forranks sections with the heading as signature,--expandserves section bodies,--recallgoes section-granular with disclosure; links/wikilinks/mentions become doc→doc and doc→code edges. Vendored tree-sitter-markdown with a serialize-bounds patch (the THIRD vendored scanner hole the V1 convention has caught — no bounds check at all; 300 nested blockquotes = silent heap corruption under NDEBUG). Bonus: fixed a pre-existing determinism bug (file-node dedup could tie on all sort keys and flip survivors by worker arrival — the probable cause of historically flapping bench scoreboards). 24 red arms → 94 green.--verify="CLAIM"(G4) — the biggest verb-less intent gets its verb: a closed claim grammar (calls/uses/unused/contains/defines/reaches) with a three-valued verdict grammar — confirmed-with-witness, refuted-only-with-complete-evidence, not-established-with-the-limit-named. calls() can never refute; unused() can never confirm;complete=andcounts_floor=never co-occur (gate-swept); the dynamic-dispatch mutation fixture pins that a string-keyed registry call yields not-established, never a false refuted. 44 red arms → 46 green.E1 answer grader — the scenario proof's named blocker closed: six grader types under the answer-fence contract, groundedness on every type, closed accept-rule grammar (unknown clauses demote, never silently pass), refusal taxonomy including circularity and the bash-3.2
**false-zero hazard. Plus F3: the claude runner gets real env isolation + a 41-assertion canary. Found on first contact: 7 sealed keys owed (not 5), one bank row unsatisfiable at its pin.Integration: four-way loop union → 397 gates; EVALS recomputed; acks rebuilt from refs (a filtered-subprocess truncation was caught mid-merge and corrected); flag count 145; captures regenerated from the union binary. Full plain suite 397 ALL PASS; determinism ×3; xmllint; quality-delta gating=0.
🤖 Generated with Claude Code