feat(eval-harness): replay_post_fork host-agent JSONL split (closes #600) - #601
Conversation
Reviewer's GuideImplements a host-agent eval-replay path for the context-rebuilder eval harness by threading an optional per-run directory through the harness, emitting JSONL replay requests, rejoining host-provided responses, and extending tests and docs to cover the new flow while preserving the legacy stub behavior by default. Sequence diagram for host-agent eval-replay flow with run_dirsequenceDiagram
actor Operator
participant EvalHarness
participant FileSystem
participant HostAgent
participant ModelAPI
Operator->>EvalHarness: run eval_harness --mode sweep --run-dir base_run_dir
EvalHarness->>FileSystem: write replay_requests.jsonl
EvalHarness-->>Operator: results (reason=pending_replay, matched=False)
Operator->>HostAgent: start replay dispatcher over base_run_dir
HostAgent->>FileSystem: read replay_requests.jsonl per case_run_dir
loop per_request_row
HostAgent->>ModelAPI: call with rebuilt_block + "\n---\n" + user_turn
ModelAPI-->>HostAgent: actual
HostAgent->>FileSystem: append {turn_idx, actual} to replay_responses.jsonl
end
Operator->>EvalHarness: rerun eval_harness --mode sweep --run-dir base_run_dir
EvalHarness->>FileSystem: read replay_responses.jsonl per case_run_dir
EvalHarness-->>Operator: results (matched | needs_llm_judge | pending_replay)
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR implements host-agent eval-replay: writing per-eval-turn ChangesHost-Agent Eval-Replay Integration
Sequence Diagram(s)sequenceDiagram
participant CLI as main CLI
participant Sweep as sweep_thresholds/sweep_budgets
participant RunOne as run_one
participant Replay as replay_post_fork
participant Operator as Host Operator
CLI->>+Sweep: invoke with --run-dir
Sweep->>+RunOne: forward run_dir
RunOne->>+Replay: call replay_post_fork(run_dir=subdir)
Replay->>Replay: write replay_requests.jsonl
Operator->>Operator: (external) read requests, invoke models
Operator->>+Replay: write replay_responses.jsonl into run_dir
Replay->>RunOne: read responses, join by turn_idx, set matched/reason
RunOne-->>-Sweep: return RunResult
Sweep-->>-CLI: return SweepResult
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
robotrocketscience
added
author-Planck
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="benchmarks/context-rebuilder/eval_harness.py" line_range="236-245" />
<code_context>
+def _read_user_and_expected(
</code_context>
<issue_to_address>
**issue (bug_risk):** The OSError sentinel ({}, {}) from _read_user_and_expected is indistinguishable from other empty results, changing behavior for non-OSError cases.
In `replay_post_fork`, treating both `expected_by_idx` and `user_turn_by_idx` as empty (with `case.eval_turns` non-empty) as an OSError sentinel:
```python
expected_by_idx, user_turn_by_idx = _read_user_and_expected(case)
if case.eval_turns and not expected_by_idx and not user_turn_by_idx:
return []
```
is incorrect because `_read_user_and_expected` can return `({}, {})` both when an OSError occurs and when all lines are malformed / unusable. Previously, the latter case still produced placeholder rows (e.g., `expected=""`); now it returns an empty list, changing harness behavior for those edge cases. To preserve prior semantics, use a distinct flag/sentinel to differentiate OSError from "no usable lines found" instead of relying solely on the dicts being empty.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
benchmarks/context-rebuilder/README.md (2)
293-293: 💤 Low valueAdd language specifier to fenced code block.
The fenced code block should specify
bashas the language for proper syntax highlighting.📝 Proposed fix
- ``` + ```bash uv run python benchmarks/context-rebuilder/eval_harness.py \ --mode threshold-sweep \ --corpus benchmarks/context-rebuilder/fixtures/synthetic/ \ --out /tmp/sweep1.json \ --run-dir /tmp/sweep1/ ```As per coding guidelines, the static analysis tool markdownlint-cli2 flagged this fenced code block as missing a language specification.
🤖 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 `@benchmarks/context-rebuilder/README.md` at line 293, The fenced code block containing the command starting with "uv run python benchmarks/context-rebuilder/eval_harness.py \\" should be updated to specify the language for syntax highlighting—replace the opening triple backticks ("```") with "```bash" so the block becomes a bash code block; no other changes to the contents are needed.
303-303: 💤 Low valueUse fenced code block instead of indented style.
The indented code block at line 303 should be converted to a fenced code block for consistency and to avoid static analysis warnings.
📝 Proposed fix
- For each (case, threshold, budget) cell, the harness writes: - - /tmp/sweep1/<case_stem>__t<threshold>__b<budget>/replay_requests.jsonl + For each (case, threshold, budget) cell, the harness writes: + + ``` + /tmp/sweep1/<case_stem>__t<threshold>__b<budget>/replay_requests.jsonl + ```As per coding guidelines, the static analysis tool markdownlint-cli2 flagged this as using indented code style instead of fenced blocks.
🤖 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 `@benchmarks/context-rebuilder/README.md` at line 303, Replace the indented markdown code block containing the path string "/tmp/sweep1/<case_stem>__t<threshold>__b<budget>/replay_requests.jsonl" with a fenced code block: add a triple-backtick line before the path and a triple-backtick line after it so the path is enclosed in a fenced block rather than indented style; ensure there are no extra leading spaces on the path line itself.
🤖 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.
Nitpick comments:
In `@benchmarks/context-rebuilder/README.md`:
- Line 293: The fenced code block containing the command starting with "uv run
python benchmarks/context-rebuilder/eval_harness.py \\" should be updated to
specify the language for syntax highlighting—replace the opening triple
backticks ("```") with "```bash" so the block becomes a bash code block; no
other changes to the contents are needed.
- Line 303: Replace the indented markdown code block containing the path string
"/tmp/sweep1/<case_stem>__t<threshold>__b<budget>/replay_requests.jsonl" with a
fenced code block: add a triple-backtick line before the path and a
triple-backtick line after it so the path is enclosed in a fenced block rather
than indented style; ensure there are no extra leading spaces on the path line
itself.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f9e00f39-896e-43f9-8335-6b7236406586
⛔ Files ignored due to path filters (1)
CHANGELOG.mdis excluded by!**/CHANGELOG.md
📒 Files selected for processing (3)
benchmarks/context-rebuilder/README.mdbenchmarks/context-rebuilder/eval_harness.pytests/test_context_rebuilder_eval_harness_wiring.py
|
[claim:review:Maxwell:2026-05-10T17:14:44Z] |
|
[claim:review:Einstein:2026-05-10T17:14:48Z] |
|
[release:review:Einstein:2026-05-10T17:14:53Z] |
|
[claim:review:godel:2026-05-10T17:15:14Z] |
|
[release:review:godel:2026-05-10T17:15:19Z] |
|
[claim:review:planck:2026-05-10T17:15:43Z] |
|
[release:review:planck:2026-05-10T17:15:47Z] |
|
Review: blocker, single-line fix. Code is sound — polymorphic split via Blocker: the discretion grep on the diff vs The word lives in the new Requested: amend / fixup the scrub commit (or stack a 7th Removing |
robotrocketscience
removed
the
attn:review
|
[release:review:Maxwell:2026-05-10T17:17:00Z] |
|
This PR is now behind Auto-rebase was removed because the bot has no signing key; rebasing as the bot strips author signatures and the |
|
[claim:review:godel:2026-05-10T23:00:27Z] |
b3178de to
72adc99
Compare
|
Rebased onto current |
PR-size soft capThis PR is over the advisory size threshold:
Bigger PRs collide with more open work, which under the parallel-session workflow tends to produce repeated This is advisory only — nothing is blocked. If the size is intentional (large refactor, module removal, generated code), apply the |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@benchmarks/context-rebuilder/README.md`:
- Around line 293-303: The fenced code block showing the uv run command (the
eval_harness.py example) must declare a language (use "bash") and the subsequent
indented example path line
(/tmp/sweep1/<case_stem>__t<threshold>__b<budget>/replay_requests.jsonl) should
be converted to a fenced code block as well (use "text") so both blocks conform
to markdownlint (MD040/MD046); update the opening fence for the command block to
"```bash" and replace the indented path line with its own "```text" fenced
block.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: eb399668-7800-4dc2-acc3-99bd1618654a
⛔ Files ignored due to path filters (1)
CHANGELOG.mdis excluded by!**/CHANGELOG.md
📒 Files selected for processing (3)
benchmarks/context-rebuilder/README.mdbenchmarks/context-rebuilder/eval_harness.pytests/test_context_rebuilder_eval_harness_wiring.py
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/test_context_rebuilder_eval_harness_wiring.py
- benchmarks/context-rebuilder/eval_harness.py
|
This PR is now behind Auto-rebase was removed because the bot has no signing key; rebasing as the bot strips author signatures and the |
|
Rebased onto current Verification:
Push: Unlabeling |
|
[release:review:leibniz:2026-05-11T14:50:36Z] |
|
Note on parallel work: after pushing I noticed three sister worktrees at One sister worktree ( Protocol gap noted for handoff: I should have run |
robotrocketscience
added
the
ready-to-merge
|
merge-train: blocked FF push to The |
github-actions
Bot
removed
the
ready-to-merge
|
Resolved two unresolved bot-review threads that triggered
Re-adding |
robotrocketscience
added
the
ready-to-merge
|
merge-train: blocked branch is not fast-forward on The |
github-actions
Bot
removed
the
ready-to-merge
|
[claim:review:leibniz:2026-05-11T15:01:06Z] |
Extends `replay_post_fork` with an optional `run_dir: Path | None`
parameter implementing the polymorphic eval-replay pattern that
mirrors `/aelf:onboard`'s LLM-classify split. The aelfrice repo
never imports the `anthropic` SDK; the model call is the host's
responsibility.
When `run_dir` is None (default), behaviour is unchanged: one
placeholder row per eval_turn with `reason=REPLAY_PENDING_REASON`
("needs_replay_client"). Threshold/budget sweeps continue to
produce valid latency + token-cost numbers without dispatching
subagents.
When `run_dir` is provided:
* Writes `<run_dir>/replay_requests.jsonl` — one row per eval_turn
with `{turn_idx, rebuilt_block, user_turn, expected}`. An
operator-driven dispatcher (Claude Code session, MCP host, or
private `aelf:replay-eval` skill) spawns one subagent per row.
* Reads `<run_dir>/replay_responses.jsonl` (if present) and joins
by `turn_idx`. Substring match (`expected.lower() in
actual.lower()`) short-circuits to matched=True. Filled rows
without substring match drop to NEEDS_LLM_JUDGE_REASON for
commit-3 of #592 to pick up. Missing/empty `actual` rows hold
PENDING_REPLAY_REASON.
Best-effort file IO: mkdir/write/read failures squash so the
harness's hook contract (never block a sweep) is preserved.
The single-pass `_read_user_and_expected` walks the case once to
collect both the expected text at the eval index and the most
recent user-role turn at-or-before that index — the prompt the
host-agent dispatcher will replay through the rebuilt context.
Default-path callers (no run_dir) retain identical behaviour;
the existing 15 wiring tests pass unchanged.
) Adds an optional `run_dir: Path | None = None` to `run_one`, `sweep_thresholds`, and `sweep_budgets`, plus the matching `--run-dir` argparse flag on `main()`. Threads the value into `replay_post_fork` so an operator-driven sweep produces per- (case, config) request files at: <run_dir>/<case_stem>__t<threshold>__b<budget>/replay_requests.jsonl The slug embeds threshold + budget so each cell of a sweep gets its own request file rather than overwriting a shared one. Default is None — bare callers (existing tests, the v1.2.0 stub path) get unchanged behaviour. The 15 wiring tests pass without modification. Default-path note: the `--run-dir` flag is opt-in. Without it, sweeps continue to emit placeholder rows (reason=needs_replay_client) and skip the file IO, matching the v1.2.0 stub contract.
Five new tests covering the host-agent eval-replay path: - `test_replay_post_fork_writes_requests_jsonl` — request file schema (turn_idx, rebuilt_block, user_turn, expected) and the most-recent-user-at-or-before semantics for `user_turn`. - `test_replay_post_fork_pending_reason_when_no_responses` — run_dir set but response file absent → `pending_replay`, distinct from the bare-stub `needs_replay_client`. - `test_replay_pending_round_trip` — acceptance bullet from #600. Two-pass flow: emit requests, hand-author responses (no subagent invoked in CI), re-invoke, verify substring-match short-circuits to matched=True and non-match drops to `needs_llm_judge`. Asserts score_fidelity reports the substring half (0.5 on the seeded fixture). - `test_replay_pending_partial_response_keeps_others_pending` — acceptance bullet `missing rows stay pending_replay`. Partial response file must not promote uncovered rows. - `test_run_one_threads_run_dir_to_replay_post_fork` — verifies the per-(case, config) subdir naming (`<stem>__t<threshold>__b<budget>`) and that the request file appears under it after a `run_one` call with `run_dir`. All 20 wiring tests pass.
Documents the polymorphic eval-replay split in benchmarks/context-rebuilder/README.md: * Three-step flow: sweep emits request files, operator dispatches subagents (Claude Code or MCP host), re-running joins responses. * Per-(case, config) subdir naming (`<stem>__t<threshold>__b<budget>`) so each cell of a sweep gets its own request/response pair. * Substring-match short-circuit in `replay_post_fork`; non-match drops to `needs_llm_judge` for the LLM-judge follow-up (commit-3 of #592). * Default behaviour without `--run-dir` is unchanged (`needs_replay_client`, fidelity=0, no file IO). The dispatch step is operator-driven; aelfrice never imports the `anthropic` SDK and never holds API keys. A private `aelf:replay-eval` skill in `~/.claude/skills/` can automate the for-loop, but the contract is the on-disk request/response files.
The discretion grep gate (~/.claude/scripts/aelf-pr-open.sh) flagged banned vocabulary in the diff. The original spec issue (#600) uses host/CLI-vendor names that are appropriate in the issue body but not in shipped repo content. Sanitize: * `Claude Code session` / `Claude Desktop` → `host session` / generic host references. * `subagent` → `child task` / `dispatched task`. * Specific vendor names dropped from the eval_harness.py docstring, README, and CHANGELOG entry. Pure docs/comment scrub. No code behaviour change. The 20 wiring tests pass unchanged.
Planck scrub commit (1796723) sanitized vendor / banned-vocab terms in the `**With `run_dir`**` paragraph but missed the matching `**No `run_dir` (v1.2.0 stub default)**` paragraph one line up, which still read "without dispatching subagents". Replaces with "without dispatching child tasks" to match the surrounding scrub vocabulary. Pure docs change.
cde44f6 to
db9f6ed
Compare
robotrocketscience
added
the
ready-to-merge
|
[release:review:leibniz:2026-05-11T15:01:52Z] |
github-actions
Bot
removed
the
ready-to-merge
|
merge-train: merged db9f6ed → |
1 participant
Closes #600 (commit-2 of #592 umbrella).
What lands
The host-agent eval-replay path for the context-rebuilder eval harness, applied to the polymorphic split that mirrors
/aelf:onboard's LLM-classify pattern. aelfrice still imports zero from any vendor model SDK on this path; the model invocation is the host's responsibility (an MCP-enabled host CLI, an operator-driven loop, or a privateaelf:replay-evalskill).Atomic commits
All signed.
Code shape
replay_post_forkgains an optionalrun_dir: Path | Noneparameter:Default
None— unchanged stub behaviour. Each row carriesreason="needs_replay_client",matched=False. Threshold/budget sweeps still produce valid latency + token-cost numbers.run_dirset — writes one JSON row per eval_turn to<run_dir>/replay_requests.jsonl:{"turn_idx": 8, "rebuilt_block": "...", "user_turn": "...", "expected": "..."}Reads
<run_dir>/replay_responses.jsonlif present, joining byturn_idx. Substring-match (expected.lower() in actual.lower()) short-circuits tomatched=True. Filled rows without substring match drop toreason="needs_llm_judge"for commit-3 of feat(eval-harness): wire context-rebuilder eval harness for #587 hot-start scoring #592 (LLM judge). Missing/emptyactualrows holdreason="pending_replay".run_one,sweep_thresholds,sweep_budgetsall gain matchingrun_dirparameters;main()gains--run-dir. Per-(case, config) subdirs are computed deterministically:<base>/<case_stem>__t<threshold>__b<budget>/.The single-pass
_read_user_and_expectedwalks each case once, collecting both the expected text at the eval index and the most-recent user-role text at-or-before that index — the prompt the host-agent dispatcher will replay through the rebuilt context.Verification
uv run pytest -x -q→ 3291 passed, 52 skipped (full suite). 5 new tests intests/test_context_rebuilder_eval_harness_wiring.pycover request emit, response join, substring match, partial-coveragepending_replay, andrun_oneper-(case, config) subdir naming.github/main→ clean.Acceptance criteria
replay_post_forkwritesreplay_requests.jsonlto a per-run directory and returns rows withreason="pending_replay".replay_responses.jsonl(if present) and joins byturn_idx; missing rows staypending_replay.benchmarks/context-rebuilder/README.md(the "or equivalent skill" branch of the AC; the dispatch step is operator-driven, not aelfrice — see "Why this and not the SDK" in feat(eval-harness): wire replay_post_fork via host-agent subagent (commit-2 of #592) #600).score_fidelityproduces non-zero scores once the operator has run the replay phase (asserted intest_replay_pending_round_trip: 0.5 verdict on the seeded round-trip fixture).tests/test_context_rebuilder_eval_harness_wiring.py::test_replay_pending_round_trip.Out of scope (per #600)
reason="needs_llm_judge"for that follow-up.Summary by Sourcery
Add a host-agent eval-replay path to the context-rebuilder eval harness that persists replay requests and joins host-provided responses while keeping the default stub behaviour unchanged.
New Features:
Enhancements:
Documentation:
Tests:
Summary by CodeRabbit
New Features
Documentation
Tests