feat(parsec): add parsec as a first-class benchmark + HARBOR_LOCAL_ASIS - #320
feat(parsec): add parsec as a first-class benchmark + HARBOR_LOCAL_ASIS#320bcarmeli wants to merge 1 commit into
Conversation
Parsec (rhpds/parsec) is Red Hat's LLM-agentic troubleshooting tool. Its harbor-tasks/ dir already ships per-task task.toml + tests/verify.py + expected.json + a ubi9 Dockerfile — the harbor adapter's default package_dataset repacking would blow those away. Add HARBOR_LOCAL_ASIS=1 so downstream benchmarks that hand-author rich task dirs can point harbor at the dataset verbatim (16 lines in templates/adapters/harbor/adapter.py, opt-in via env var, backwards compatible). ci/benchmarks/parsec/ adds smoke (5 tasks) + pilot (30 aap2 tasks with seeded 60/20/20 split) + patch-harbor-tasks.sh (shadow-copies the local task tree with docker_image + MCP URLs substituted + verify.py MCP-prefix strip). README documents the 30-task baseline (mean 0.240) and a 2-task N=30 optimization pilot that established the seed prompt is at a local optimum on the aap2-0047/0048 combinatorial-sweep failure mode. ci/benchmarks/lib/run_suite.sh gets a parsec case block alongside swebench/skillsbench (45 lines, no changes to shared logic). parsec-results/ (local intake deliverables) gitignored. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
❌ Automatic Labeling Failed An error occurred while trying to automatically label this pull request. Please check the workflow logs for details and add labels manually. |
OsherElhadad
left a comment
There was a problem hiding this comment.
Review — feat(parsec): add parsec as a first-class benchmark + HARBOR_LOCAL_ASIS
Reviewed the full diff (9 files, +653/-0), checked out the branch, merged it with current main locally, ran the whole core/tests suite against the merge result, and tried to actually run the benchmark end to end. Notes below are ordered blockers → generalizability → honesty → security → docs.
Verdict: request changes. The core design instinct here is right and I want to say that up front (see What this gets right), but the PR is currently red on three deliberate guard tests, the two commands its README gives do not run, and the pilot's held-out test split contains the two tasks the PR itself already optimized against.
What this gets right (and it matters for this repo)
Generalizability was my main worry going in, and on the biggest question the PR lands well:
- No parsec-specific branching in
core/orskills/. I grepped for it; there is none.HARBOR_LOCAL_ASISintemplates/adapters/harbor/adapter.py:707-714,260-266is a generic third branch keyed on an env var — no benchmark name appears in it, and it is opt-in and backwards compatible. That is exactly the shape a new-benchmark need should take when it has to touch shared code. - It reuses the existing harbor adapter instead of writing a parsec adapter. The four-method contract (
tasks()/run_batch()/score()/apply()) is untouched, so dashboard / diagnose / gate keep working with no special-casing. A newtemplates/adapters/parsec/would have been the wrong answer and the PR avoided it. - The
parsec)case inci/benchmarks/lib/run_suite.sh:152-167follows the established per-benchcaseconvention (tau2/swebench/skillsbench/spreadsheetbench) rather than inventing a mechanism. - Both python patch heredocs in
patch-harbor-tasks.share<<'PY'-quoted, the regex substitutions assertn == 1and exit non-zero underset -euo pipefail, and the split file is an exact partition. Careful work.
Blockers
1. CI is red on three benchmark-inventory guard tests, and those tests exist precisely to stop this from landing silently.
Confirmed locally against the PR merged with current main: exactly these three fail, 1099 passed, 12 skipped.
core/tests/test_benchmarks_plan_legs.py:128—assert shipped == ["spreadsheetbench", "swebench"], now['parsec', 'spreadsheetbench', 'swebench']. Its docstring says outright: "Pinned deliberately: if another benchmark adds one, the assertions above need revisiting too."core/tests/test_spreadsheetbench_scoring_and_seed.py:215-222—"a tier gained or changed a committed override — say which and why in the PR".core/tests/test_spreadsheetbench_skillopt_split.py:119-121— "Any third path here means another benchmark's tier just changed meaning from FIT to held-out — which should be a deliberate decision, not a side effect, so this test is the place it gets noticed."
These are not incidental assertions to bump; each one is a written request for a rationale in the PR. Please update all three with the reasoning inline, the way the swebench-pilot addition did in test_which_benches_ship_a_pilot_tier's docstring.
Note that one of the three is free: see the dead-config item below — deleting ci/benchmarks/parsec/pilot/overrides.env clears the overrides guard at zero behavioral cost.
(For completeness: on the raw PR head I also saw test_site_live_panel_tiers.py::test_tier_filter_offers_every_tier fail. That one is pure staleness — the branch is from Aug 17 — and it clears on merge with main. Not yours.)
2. "First-class benchmark" is not actually wired up. The title claims first-class; the plumbing is half-connected:
.github/workflows/benchmarks.yml:191—BENCHES = ["tau2", "swebench", "skillsbench", "spreadsheetbench"], unchanged..github/workflows/benchmarks.yml:32— dispatchoptions: [all, tau2, swebench, skillsbench, spreadsheetbench], unchanged.ci/benchmarks/README.md:3— the benchmark inventory, unchanged.ci/benchmarks/lib/run_suite.sh:17— usage string stillbench (tau2|swebench|skillsbench|spreadsheetbench).
So parsec is undispatchable and unlabelable, while its files do land in the globbed namespace the planner and the guard tests scan. That is the worst of both worlds: invisible to the thing that would run it, visible to the thing that polices it. Either wire it (workflow options + BENCHES + labels + inventory README + usage string) or keep the tier dirs out of ci/benchmarks/*/ until it is ready to be dispatched.
3. Both documented commands fail immediately. README.md:176-180 gives:
BENCH=parsec TIER=smoke bash ci/benchmarks/lib/run_suite.sh
BENCH is positional (run_suite.sh:17, BENCH="${1:?...}"), not an env var. Verified:
ci/benchmarks/lib/run_suite.sh: line 17: 1: bench (tau2|swebench|skillsbench|spreadsheetbench)
Correct form is TIER=smoke bash ci/benchmarks/lib/run_suite.sh parsec.
4. Default paths don't meet, so the flow can't complete even after fixing #3. run_suite.sh reads HARBOR_DATASET="${PARSEC_HARBOR_TASKS_DST:-$REPO/e2e/parsec/harbor-tasks-patched}", but patch-harbor-tasks.sh:22 defaults DST to $(dirname $0)/../harbor-tasks-patched = ci/benchmarks/parsec/harbor-tasks-patched. Run both with defaults, as the README instructs, and the producer writes somewhere the consumer never looks. One of the two defaults should change (I'd move the patcher's default to e2e/parsec/harbor-tasks-patched, matching e2e/ usage by skillsbench/spreadsheetbench).
5. I could not run this, and I don't think anyone outside the author can. I got exactly as far as the dataset guard, which failed cleanly and informatively (credit for the good error message):
::error:: parsec shadow tasks not found at .../e2e/parsec/harbor-tasks-patched
(set PARSEC_HARBOR_TASKS_DST or run ci/benchmarks/parsec/utils/patch-harbor-tasks.sh first)
And the patcher:
Source harbor-tasks/ not found at /Users/boazc/workarea/Python/rhdp-parsec/harbor-tasks
Four independent unobtainable prerequisites:
harbor-tasks/does not exist in the publicrhpds/parsec—GET /repos/rhpds/parsec/contents/harbor-tasksreturns 404. (config/prompts/aap2_agent.mdis public, 200.)patch-harbor-tasks.sh:19defaultsSRCto a personal laptop path,/Users/boazc/workarea/Python/rhdp-parsec/harbor-tasks.- The kaegis sims are on
github.ibm.com/kaegis/simulation-harness— internal. localhost/parsec-agent-base:latest(patch-harbor-tasks.sh:80) has no Dockerfile and no build script anywhere in the PR; grep finds that string only inside the patcher itself.
I'm not asking you to open-source RH's dataset. I am asking the PR to say plainly, in the README, that this benchmark is IBM/RH-internal-only and cannot be run externally — and to ship the parsec-agent-base Dockerfile, which is ours to ship.
Generalizability: the one real coupling risk
HARBOR_LOCAL_ASIS silently changes the capability-delivery contract, and the shared template is the thing left holding the footgun.
In the packaged path, capevolve_harbor/tasks.py:103 → _write_environment() copytrees the entire candidate_dir into environment/capability. The ASIS branch skips package_dataset altogether (adapter.py:260-266), so the candidate now reaches the container only via --extra-instruction-path — and _write_candidate_instruction (adapter.py:391-403) reads nothing but the text of prompt.md or SKILL.md.
Meanwhile templates/adapters/harbor/capevolve.yaml:16 declares the template's own default as capabilities: [skill-package]. A skill-package candidate under HARBOR_LOCAL_ASIS=1 therefore loses its bundled scripts/ and references/ silently: the run completes, the gate runs, numbers get published, and the optimizer's edits to anything but the SKILL.md body were never delivered. That is the failure mode this repo cares most about — a run that looks valid and measures nothing.
parsec's own pilot is safe because run_suite.sh:34 sets CAPS="[system-prompt]". But the flag ships in the shared harbor template, where the next user will pair it with the declared default.
Cheap fix, ~3 lines in the ASIS branch: if candidate_dir contains anything other than prompt.md/SKILL.md, raise with a message saying ASIS can only deliver instruction text. Fail loud instead of quietly under-delivering.
Dead / duplicated config — and it's the cause of one of the three test failures. ci/benchmarks/parsec/pilot/overrides.env is entirely redundant:
HARBOR_LOCAL_ASIS=1—run_suite.shalreadyexports this unconditionally in the parsec case.BACKEND_MCP_URL=http://host.containers.internal:8086/mcp/sse—run_suite.shalready exports the byte-identical default.PARSEC_ADAPTER=harbor— read by nothing. (SWEBENCH_ADAPTERhas the same problem onmain, so this is a copied convention, not an invention — but let's not spread it.)
Delete the file: one guard failure gone, zero behavior change. Related, README.md:196 states "HARBOR_LOCAL_ASIS=1 is set via overrides.env", which contradicts run_suite.sh. Two sources of truth for one flag, and the redundant one is the one tripping a guard.
No test on the new branch. There is no harbor adapter test file today, so this matches repo state rather than regressing it — but the branch is three-way now, and a ~10-line test asserting each branch picks the right dataset_path / dataset_name / include_tasks triple is very cheap relative to how quietly the ASIS path can go wrong.
Honesty and split provenance
Split mechanics check out — I validated the files directly: 30 unique task ids, 18/6/6, exact partition, no overlaps, no strays, smoke ids all present in pilot.
Two problems above that:
Test-split contamination by human-in-the-loop. traces_parsec-aap2-0047 and traces_parsec-aap2-0048 are in the pilot's test split (pilot/split_ids.json:31-32) — and those are precisely the two tasks the README's own "N=30 optimization pilot" ran with train = val = test = {0047, 0048}, 60 trial-cells, whose failure mode ("agent looped combinatorially and never emitted a final answer") was hand-diagnosed and fed to the optimizer across two candidate proposals. Any future pilot run's sealed test number is then measured partly on tasks whose failure mode has already been studied and optimized against by a human. That is a soft leak, but this repo's whole pitch is that the sealed number means what it says. Reseed so those two land in train, or state the contamination in the split file and the README.
No split generator. spreadsheetbench and swebench both ship utils/make_split.py, and test_spreadsheetbench_skillopt_split.py:69 pins the committed file to the generator's byte-exact output. parsec ships a hand-written split_ids.json with no generator, so "Seeded 60/20/20 split" (README.md:110) is currently an unverifiable claim — no seed recorded, not reproducible, and the contamination above can't be cleanly re-rolled. Adding ci/benchmarks/parsec/utils/make_split.py would fix the claim and the contamination in one move.
Smaller notes:
- The results table's
finalize test (best = seed) | 0.326row (README.md:205) will read as held-out to anyone skimming. Thetrain = val = testdisclosure is two paragraphs up. Put "(no holdout — fit)" in the row itself. pilot/tasks.jsonpinsaws/claude-sonnet-4-5per task whilerun_suite.sh:29defaultsAGENT_MODEL=aws/gpt-oss-120b.run_suitedoes warn (tasks.json pins agent(s) [...] but this run uses ...), so this is handled — just flagging that the published 0.240 baseline only reproduces with the pin honored..gitignore:21-22addsparsec-results/, referenced nowhere else in the repo. That's an author-local artifact dir leaking into a shared ignore file — drop it or explain it.- Three of the four new JSON files have no trailing newline. Cosmetic.
On patching the benchmark's own scorer. patch-harbor-tasks.sh rewrites RH's gold tests/verify.py by regex. The rationale is sound and well documented (claude-code emits mcp__<server>__<tool>, expected.json holds bare names; without the strip, trajectory-match is 0 for every task, so the unpatched scorer measures nothing). Two caveats worth a line each in the README: (a) our parsec numbers are consequently not comparable to RH's on the unpatched verifier; (b) the strip makes trajectory matching server-blind — tool X from any of the four sims now satisfies an expected X, so a same-named tool on two sims would be a silent false positive. The fail-loud n != 1 → exit 2 behavior is the right call.
Security
Nothing alarming; the trust posture matches swebench.
- No credentials committed. The parsec case reads
ANTHROPIC_AUTH_TOKENfrom the environment and truncates$WORK/.env(: > "$WORK/.env") exactly like every sibling case.overrides.envholds no secrets. - No injection path from task content — both patch heredocs are
<<'PY'-quoted, so nothing intask.toml/verify.pyis shell-interpolated. - Executing RH-authored
verify.py/task.tomlinside Harbor containers is the same trust model swebench already runs under. Acceptable. - Sim traffic is plain HTTP to
host.containers.internal— loopback only, fine. - Two small hardening asks:
rm -rf "$DST"(:598) is properly quoted and:--defaulted so an empty env var can't widen it, but it is still an unconditional recursive delete of a caller-supplied path — a guard that the path basename isharbor-tasks-patchedis cheap insurance. AndFILTER=${1:-aap2}is unvalidated before being interpolated into a glob (worst case an empty match, so low severity).
Documentation
ci/benchmarks/parsec/README.md is, on its own terms, one of the better benchmark docs in this repo — real numbers, named failure modes, and genuinely honest framing of the N=10 false positive and the stall=2 halt. The "Framework implications for cap-evolve upstream" section (surfacing completion/gate as a side-metric; default optimizer_max_turns for large seeds) is valuable and I'd like those filed as issues regardless of this PR's fate.
But onboarding from the docs alone is currently impossible, and that's the gap against examples/tau2_airline/ (which ships setup.sh, run.sh, capevolve.yaml, seed_capability, DEMO.md):
PROJECT.md— the run book with the full env checklist and open items — is explicitly "in the parsec-intake worktree" (README.md:198), i.e. not in this PR. Same fordocs/container-build.md, referenced atpatch-harbor-tasks.sh:16. Both are dangling references for every reader.- Dataset, sims, and container image all unobtainable (see blocker #5).
- The two commands given don't run (blocker #3).
Minimum bar to land: a setup.sh or an explicit Prerequisites (internal-only) section stating what cannot be obtained externally, the parsec-agent-base Dockerfile, corrected commands, and the two dangling doc references either inlined or dropped.
Summary
Must fix before merge:
- The three guard tests, updated with rationale (one clears free by deleting the redundant
overrides.env). - Wire parsec into
benchmarks.yml(BENCHES+ dispatch options + labels),ci/benchmarks/README.md, and therun_suite.shusage string — or keep the tier dirs out of the guarded glob until it's ready. - Fix the README invocations and the
patch-harbor-tasks.sh↔run_suite.shpath mismatch. - State the internal-only prerequisites, and ship the
parsec-agent-baseDockerfile. - Guard
HARBOR_LOCAL_ASISagainst non-instruction capabilities so a skill-package candidate can't be silently under-delivered. - Move
aap2-0047/0048out of the pilot test split (ideally via a committedmake_split.py), and label thefinalize testrow as no-holdout.
Nice-to-have: a small test pinning the adapter's three-way dataset branch; the two scorer-patch caveats in the README; the rm -rf basename guard; drop PARSEC_ADAPTER and parsec-results/.
Thanks for this — the restraint in extending the harbor adapter generically rather than forking a parsec adapter is the right call, and the honest write-up of a pilot that failed to beat its seed is exactly the kind of result this repo should be publishing. The blockers above are all mechanical; none of them are about the approach.
— Osher-Elhadad
Summary
Adds
rhpds/parsec— Red Hat's LLM-agentic troubleshooting tool — as a first-class cap-evolve benchmark alongsideswebenchandskillsbench. The aap2 sub-agent is the phase-1 pilot subject.HARBOR_LOCAL_ASIS=1— 16-line, backwards-compatible env flag intemplates/adapters/harbor/adapter.py. When set, harbor consumes a local dataset dir verbatim (nopackage_datasetrepacking). Needed because Parsec'sharbor-tasks/already ships per-tasktask.toml+tests/verify.py+expected.json+ aubi9Dockerfile — the default repackaging would destroy semantic content. Downstream benchmarks that hand-author rich task dirs set this in theiroverrides.env.ci/benchmarks/parsec/— smoke (5 tasks), pilot (30 aap2 tasks with seeded 60/20/20 split → 18/6/6),patch-harbor-tasks.shutility, and a README that documents both the Aug-12 30-task baseline (mean 0.240) and a 2-task N=30 optimization pilot (see below).ci/benchmarks/lib/run_suite.sh— parsec case block (45 lines) alongside the existing swebench/skillsbench branches. No changes to shared logic.Results
30-task baseline (Aug 12)
30 aap2 tasks, seed capability unmodified,
aws/claude-sonnet-4-5, 4 sims live, single trial per task: mean 0.240 (stdev 0.281). 5 tasks hittrajectory=1.0(mean 0.730); 25 multi-tool tasks (mean 0.142); 2 gate-failed tasks —aap2-0047/0048combinatorially swept without emitting a final answer.N=30 optimization pilot on aap2-0047 + aap2-0048 (Aug 17)
hill-climb,max_iterations: 5,stall: 2, paired-SE gatek=1.0,optimizer_max_turns: 60.test_delta = 0.000Both optimizer proposals regressed reward →
stall=2triggered → run halted at iter=2 → finalize confirms seed is best. Optimizer spend: $8.61 of $40 budget.Framework observations (surfaced in the README, not blocking this PR):
test_delta = −0.003). With N=30, seed's true mean is 0.317; the N=10 measurement was dominated by an unstable eval window (~95% gate=0).optimizer_max_turns=30was too tight for substantial SKILL.md edits (parsec'saap2_agent.mdis ~24 KB) — it caused byte-identical candidates on retry. Raising to 60 fixed it.Sims
Four kaegis simulation-harness endpoints back the aap2 tool set:
query_aap2fetch_github_file,search_github_repo,search_github_code,search_agnosticv_prslookup_catalog_item,query_babylon_catalogquery_provisions_db,db_describe_tableIcinga (:8089) is optional; not used by aap2 tasks. Its
api.jsoncurrently fails kaegis skill generation (nocomponents.schemas) — tracked for a future upstream fix.Test plan
bash ci/benchmarks/parsec/utils/patch-harbor-tasks.sh— shadow-copies harbor-tasks with docker_image + MCP URLs substituted + verify.py MCP-prefix strip. AssumesPARSEC_HARBOR_TASKS_SRCandPARSEC_HARBOR_TASKS_DSTare set.BENCH=parsec TIER=smoke bash ci/benchmarks/lib/run_suite.sh— 5 tasks, single trial. Should complete without touching the defaultpackage_datasetcode path (verifyHARBOR_LOCAL_ASIS=1branch inrun_batch).BENCH=parsec TIER=pilot bash ci/benchmarks/lib/run_suite.sh— 30 aap2 tasks, seeded 60/20/20 split. Should match the Aug-12 mean 0.240 within a few percent (assuming the sims are alive and the runner has VPC gateway access).HARBOR_LOCAL_ASISdefaults to off; existing swebench runs use theelsebranch inrun_batch(repacking).Related
PalmPalm7/parsec#4— draft PR againstPalmPalm7:migration/full-sdkthat adds thePARSEC_TOOLS_DIRhook to Parsec'ssrc/__init__.py(needed for the phase-2 wrapper architecture; harmless in phase 1).🤖 Generated with Claude Code