feat(eval): langgraph-node runner (U1-U5, issue #36)#41
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces pluggable runner adapters to the evaluation harness, adding the langgraph-node runner to execute Python-based LangGraph graphs, alongside new documentation, examples, and tests. The review feedback identifies several key issues: run.sh scans CASE_FILES before they are defined, which bypasses preflight checks; graph_fingerprint is incorrectly classified under MODEL_CHANGED instead of SKILL_CHANGED; the entry point parsing fails to strip .py extensions, breaking module execution; the fingerprint subcommand signature violates the documented runner contract; relative paths resolve incorrectly in langgraph_node_spawn after changing directories; and a potential globbing issue exists in langgraph-node.sh when no tool files are found.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Addressed all 6 Gemini code-assist review issues with 5 atomic commits + 1 test commit on top of the original PR.
Plus U7 ( Verification:
|
Follow-up: stale Gemini review threads still showing as pendingThe earlier PR comment (#4756506153, "Addressed all 6 Gemini code-assist review issues") correctly identified the 6 fixes and pushed them, but it didn't resolve the underlying review threads or ask Gemini to re-verify. As a result, all 7 review threads from review #4536302057 still show as pending even though every underlying issue is addressed. This comment surfaces the resolution and asks a Diagnosis
Per-thread → commit table (verified at HEAD
|
| Thread | File | Issue | Fix commit | Verified at |
|---|---|---|---|---|
| 3445293156 (critical) | scripts/eval/run.sh |
CASE_FILES scanned before defined |
eae0d57 |
run.sh:180-218 |
| 3445293159 (high) | scripts/eval/lib/attribute.sh |
graph_fingerprint misclassified under MODEL_CHANGED |
a708ed4 |
attribute.sh:18 |
| 3445293168 (high) | scripts/eval/runners/langgraph-node.sh |
_parse_entry_point doesn't strip .py |
9d8a6b1 |
line 113 |
| 3445293169 (high) | scripts/eval/runners/langgraph-node.sh |
langgraph_node_fingerprint signature violates contract |
9d8a6b1 |
lines 353-355 |
| 3445293174 (high) | scripts/eval/run.sh |
run.sh pre-resolves module_path instead of delegating |
75b0d1e |
run.sh:428 |
| 3445293177 (medium) | scripts/eval/runners/langgraph-node.sh |
Relative paths resolved against post-cd $PWD |
9d8a6b1 |
lines 245-248 |
| 3445293180 (medium) | scripts/eval/runners/langgraph-node.sh |
Empty tools/*.py glob passed to grep as literal |
9d8a6b1 |
lines 382-401 |
Each thread has a reply with the same citation posted by me (AnandSundar); reply ids are 3446856762, 3446858248, 3446859025, 3446859981, 3446860877, 3446861698, 3446862536.
Regression coverage
The 5 fixes are locked in by 4f95608 (test(eval): regression tests for Gemini review fixes (U7)), which adds Test 7 (9 subtests) to scripts/eval/tests/runner_langgraph.sh. Full suite is green:
bash scripts/eval/tests/runner_langgraph.sh→ all 7 tests pass (Tests 1-6 unchanged behavior, Test 7: 9 subtests)- 21 pre-existing suites → all green
Ask
@maintainer: please pick one of the following:
- Dismiss the stale review via the Files Changed tab → "Dismiss" on review #4536302057. The PR has READ-only access from my fork so I can't dismiss it myself (
PUT .../reviews/4536302057/dismissalsreturns 404 for non-collaborators). - Re-request a Gemini review against the post-fix head (
4f95608). Note: the consumer Gemini Code Assist bot is being sunset on 2026-07-17 per its own footer; if a fresh signal is desired, trigger it before then. - Approve and merge if you're satisfied the 5 commits + 1 test commit above are sufficient.
If neither dismissal nor re-review happens, the threads stay visible on the PR forever — the underlying code is correct, but the review state stays COMMENTED indefinitely.
🔍 Automated Review Summary@AnandSundar thanks for addressing all 7 issues from Gemini review! However, 7 review threads still show as "pending" because Gemini Code Assist did not re-verify after the fixes were pushed. Current status
NoteGemini Code Assist will sunset on 17/07/2026. Maintainer should dismiss the stale review before that date to keep PR history clean. PR is ready to merge once maintainer confirms the fixes are correct. |
|
@hoainho Thank you for the update. I appreciate your help! |
Could you please check conflict when you have time? |
Adds the foundation for pluggable runner adapters so eval-harness can target alternative LLM runtimes (LangGraph, custom agents) without forking the opencode-centric pipeline. * lib/runner.sh (new): registry of runner adapters behind a 4-subcommand contract (prepare, spawn, fingerprint, teardown). Resolves by name from in-memory registry, then $EVAL_RUNNERS_DIR, then "opencode" implicit default. dispatch_runner routes per-subcommand: opencode spawn hits spawn_opencode in the current shell; other subcommands are no-ops (KTD2). Non-opencode runners exec via `bash <path> <sub> <args>` in a subshell so they never see the parent's exported function bloat. * lib/manifest.sh: adds runner, runner_config_sha, graph_fingerprint, langgraph_version, python_version to the env-manifest. Callers pre-set EVAL_RUNNER / EVAL_GRAPH_FINGERPRINT / EVAL_RUNNER_CONFIG_SHA before capture_manifest; defaults are "opencode" / "none". Also replaces xargs -0 sha256sum with a while-read loop — Windows MSYS's exec() rejects a child process when the parent environment is too large, and run.sh's heavy `export -f` bloats the env past that limit. * lib/preflight.sh: preflight_check_langgraph — runner-specific preflight for non-opencode adapters (only requires python3 on PATH). * lib/spawn.sh: spawn_runner — thin dispatcher. "opencode" (or empty) delegates to spawn_opencode; all others dispatch to <runner>.sh spawn. * lib/attribute.sh: model_changed now also matches langgraph_version and graph_fingerprint so a LangGraph upgrade is attributed to MODEL_CHANGED rather than UNKNOWN_DRIFT. * run.sh: source runner.sh; new --runner=<name> CLI flag and EVAL_RUNNER env override. Per-case resolves EFFECTIVE_RUNNER from case YAML runner: > EVAL_RUNNER > "opencode"; mismatch errors out. Preflight is dispatched per unique runner before the per-case loop; per-case preflight is also run (idempotent) for the langgraph case. Per-case computes EVAL_RUNNER_CONFIG_SHA and EVAL_GRAPH_FINGERPRINT before capture_manifest, and dispatches spawn through spawn_runner so the case's runner_config shape is the runner's problem, not run.sh's. * runners/langgraph-node.sh (new): 4-subcommand stub. emit a fixed-shape transcript so the downstream scoring path can be exercised end-to-end against this adapter without depending on a real LangGraph install. Real implementation lands in U2. * tests/runner_lib.sh (new): 13 unit tests for the registry (list/resolve/register/deregister/dispatch, filesystem fallback, opencode no-op, empty-name rejection, unknown-subcommand rejection). All pass. Closes U1 of nano-step#36 (the runner contract foundation).
Replaces the U1 stub. All four contract subcommands are now functional: * prepare <workdir> [<runner_config_json>] Honors EVAL_SKIP_VENV_PREPARE=1 (test mode). Otherwise creates a venv at EVAL_VENV_DIR (default <workdir>/.venv), pip installs <workdir>/requirements.txt if present, and import-checks the module named in entry_point. EVAL_VENV_CACHE=1 reuses a per-requirements-hash cache at EVAL_VENV_CACHE_DIR (or ~/.cache/eval-harness/langgraph-venv). Cross-platform venv binary resolution: POSIX bin/python3 vs Windows Scripts/python.exe. * spawn <workdir> <input> <output> <transcript> [<runner_config_json>] Parses entry_point from the config JSON, invokes `<venv-python> -m <module> --input <X> --output <Y>` with PYTHONPATH prepended to the workdir, captures stdout/stderr, and emits JSONL events with shape compatible with opencode --format json (so score.sh's output_contains / transcript_contains / jq_path_contains work unchanged). Exits 13 with a clear error event on missing or malformed entry_point; echoes the elapsed_seconds in a result event on success. * fingerprint <module_path> Stable 64-char hex sha256 over (a) the entry-point module, (b) any tools/*.py in the workdir, (c) any prompt_template_*.txt, and (d) the bodies of @tool-decorated top-level functions (adjacency check: a def only counts if the immediately preceding line is @tool). Returns 'no-module' on missing arg/file so the manifest stays valid; never fails the run on a fingerprint glitch. * teardown <workdir> rm -rf the venv unless EVAL_VENV_CACHE=1. Idempotent. Helpers (_resolve_venv_python, _cache_root, _parse_entry_point, _requirements_hash, _now_iso) keep the subcommand bodies readable. CLI guard at the bottom allows direct invocation (`bash langgraph-node.sh <sub> <args>`) for test and debug use; dispatch via lib/runner.sh is unchanged. * lib/spawn.sh: spawn_runner now echoes the spawn's exit code on stdout (matches spawn_opencode's contract so run.sh's `$(spawn_runner ...)` capture works for both opencode and non-opencode runners), and deliberately does NOT return the non-zero code (the caller is under `set -e` inside a command substitution — a non-zero return would kill the harness before it can write checks.json). * run.sh: langgraph-node case now passes the full runner_config JSON as the 5th arg to spawn_runner (per the plan's spawn signature). Same change for the stability-sample re-spawn at the bottom of the case loop. Verified by direct invocation: all 4 subcommands produce expected exit codes and outputs; spawn happy-path invokes the graph, writes output.json, and emits start/result events; spawn with missing entry_point exits 13 with a clear error event; fingerprint is stable across runs and changes when @tool bodies change. Closes U2 of nano-step#36.
Two pre-flight bugs that blocked langgraph-node e2e on Windows: 1. python3 os.path.normpath returns backslashes on Windows, but the subsequent bash prefix check used forward slashes — so 'graph.py' was rejected as 'resolves outside workdir' even though it resolved inside the workdir. Now we replace os.sep with '/' after normpath, which is a no-op on POSIX and fixes Windows. 2. jq on Windows emits CRLF line endings, so the trailing \r ended up inside the 'src' variable after the fixture-loop read. Subsequent [[ -f "$src\r" ]] checks then failed for files that existed. Strip the CR after the read. Both fixes are runner-agnostic; they're pre-work for U3's example case where the langgraph graph.py is materialized via the fixture mechanism. Found while bringing up the U3 e2e test. Co-Authored-By: Claude <noreply@anthropic.com>
Demonstrates the 3 check kinds the langgraph-node runner supports,
sharing one stubbed graph. R6 (graph size constraint) honored at 48
lines. R2 (case YAML fields) honored: runner: + runner_config:.
Files:
examples/langgraph-runner/graph.py — stubbed LangGraph-compatible
graph with a no-op @tool
(exercises fingerprint's
tool-detection path)
examples/langgraph-runner/input.json — sample input
examples/langgraph-runner/requirements.txt — langgraph>=0.2
examples/langgraph-runner/cases/shell-basic.yaml
— file_exists + 2 shell
checks (kind: shell)
examples/langgraph-runner/cases/jq-path-contains.yaml
— kind: jq_path_contains on
output.json sources array
examples/langgraph-runner/cases/output-contains.yaml
— kind: output_contains
on the graph's stdout
marker
The jq_path_contains check uses the array-constructor form
`[.sources[]] | unique` (not `$.sources[*]` as the plan suggested)
because jq 1.8.1 on Windows rejects the bracket-glob syntax. The
score script does set-difference on the jq result, which needs a
single array. `[` ... `]` collects the stream into an array.
Verified:
- graph.py runs end-to-end (python3 -m graph --input X --output Y)
and produces {"answer": ..., "sources": [...]}
- fingerprint is stable across calls and detects @tool body changes
- all 3 case YAMLs parse via yq; checks list and runner_config
serialize cleanly to JSON
Co-Authored-By: Claude <noreply@anthropic.com>
U4: scripts/eval/tests/runner_langgraph.sh — end-to-end test for the
langgraph-node runner. Stubs python3 and yq via PATH-prepend; stages
the U3 example into a temp OPENCODE_SKILLS_ROOT. 6 assertions:
- All 3 example cases PASS under the stubbed graph
- All 4 subcommands fire (trace log of stub calls)
- SKILL_CHANGED attribution when graph.py mutates
(e2e env_delta + unit attribute() with the right keys)
- MODEL_CHANGED attribution when langgraph_version bumps
- FIXTURE_STALE attribution when input.json mutates
- No ANTHROPIC_API_KEY required; EVAL_FAIL_ON_NO_LLM=1 is OK
U5: docs/runners.md — documents the runner contract. 8 sections:
what a runner is, the 4-subcommand contract, authoring steps, the
langgraph-node adapter's knobs, case YAML extensions, manifest and
attribution, examples, and shipped-vs-proposed status.
Co-Authored-By: Claude <noreply@anthropic.com>
Real-LangGraph smoke test: when EVAL_RUN_REAL_LANGGRAPH=1, this builds a fresh venv, installs langgraph from examples/langgraph-runner/ requirements.txt, and runs the shell-basic case end-to-end. Skips cleanly (exit 0) in normal CI. Covers R7b in the plan. Co-Authored-By: Claude <noreply@anthropic.com>
The graph definition is the agent's 'skill', not a model. When the graph source changes, attribution should be SKILL_CHANGED per the docs/runners.md contract. This aligns attribute.sh with the documented behavior.
Three independent fixes in the runner, all on the same file: - U3 (_parse_entry_point): strip a trailing '.py' from the module component so authors can write 'graph.py:run' as documented. - U4 (langgraph_node_fingerprint): change signature from <module_path> to <workdir> <config_json> to match the runner contract in docs/runners.md. Extract module_file from config_json.module (default 'graph.py'). Build a files_to_grep array so an empty 'tools/*.py' glob no longer passes a literal glob string to grep. - U6 (langgraph_node_spawn): absolutize workdir/input/output/transcript before 'cd "$workdir"' so relative paths resolve correctly against the original $PWD, not the post-cd one.
…view) The preflight loop iterated CASE_FILES at the top of run.sh, but CASE_FILES is defined ~30 lines later. So _NEEDED_RUNNERS was always empty, silently bypassing preflight for every runner — including the default opencode one. Move the scan to a local pre-scan that resolves the cases directory from skills_root + SKILL (+ optional CASE_ID), populates _NEEDED_RUNNERS, then dispatches preflight for each.
… review) run.sh no longer pre-resolves module_path or walks FIXTURES_DIR. The fingerprint subcommand now matches the documented contract '<name>_fingerprint <workdir> <config_json>' and owns its own path resolution (default module_file 'graph.py', overridable via config_json.module).
Locks in: - 7a: graph_fingerprint alone classifies as SKILL_CHANGED - 7b: graph_fingerprint + langgraph_version → SKILL_CHANGED wins - 7c: langgraph_version alone still MODEL_CHANGED (regression) - 7d: _parse_entry_point strips trailing .py from module - 7e: _parse_entry_point no-op when no .py (regression) - 7f: langgraph_node_fingerprint stable across config_json forms - 7g: missing module_file returns 'no-module' - 7h: missing workdir returns 'no-workdir' - 7i: CLI guard emits 64-char hash for the new argv shape
4f95608 to
888c88c
Compare
|
Conflict resolved — rebased onto current New head:
Verification:
Note: Gemini Code Assist sunsets 2026-07-17 per its own footer. If a fresh signal is wanted, please trigger re-review before that date, or dismiss the stale threads via the Files Changed tab. |
|
@hoainho PR is ready for merge. Please take a look |
|
@AnandSundar thanks for the clean rebase onto
Test 7 (9 subtests: 7a–7i) locks in each fix at the unit level. Full 23-suite regression is green. Dismissed the stale Gemini review #4536302057 — threads were anchored to the pre-fix commit and no longer reflect HEAD. Merging now. Great work on the langgraph-node adapter — the 4-subcommand contract is clean and the example cases exercise all three check patterns. 🚀 Note for future PRs: Gemini Code Assist consumer version sunsets 2026-07-17. We'll rely on human review going forward. |
hoainho
left a comment
There was a problem hiding this comment.
Approved — all 7 Gemini issues addressed, tests green, rebased cleanly.
Summary
Implements the
langgraph-noderunner adapter for eval-harness, so behavior-regression tests can target LangGraph agents alongside opencode agents. Closes issue #36.The runner is implemented as a pluggable adapter following the new 4-subcommand contract (
prepare/spawn/fingerprint/teardown). All case YAML, check kinds, and 4-class attribution logic stay unchanged; only the spawn layer varies.Implementation Units
scripts/eval/lib/runner.sh,scripts/eval/lib/spawn.sh,scripts/eval/lib/manifest.sh,scripts/eval/lib/attribute.sh,scripts/eval/lib/preflight.sh,scripts/eval/run.sh--runner=<name>CLI flag,runner:case YAML field). Manifest captures 3 new optional fields (graph_fingerprint,langgraph_version,python_version). Attribution regex extended to classifygraph_fingerprintas SKILL_CHANGED andlanggraph_versionas MODEL_CHANGED. Runner-aware preflight.scripts/eval/runners/langgraph-node.shpython3 -m <module> --input X --output Yagainst a workdir; emitstranscript.jsonlin the same shape asopencode run --format json; computes a graph fingerprint via a@tool-aware heuristic.examples/langgraph-runner/{graph.py, input.json, requirements.txt, cases/*.yaml}shell-basic,jq-path-contains,output-contains) that exercise the three check patterns. The graph is a stubbed LangGraph-compatible function with@tooldecoration; total 48 lines.scripts/eval/tests/runner_langgraph.shpython3(and the yq shim when upstream yq 4.53.x's lexer rejects// empty). 6 assertions: all 3 cases PASS, all 4 subcommands fire, SKILL_CHANGED / MODEL_CHANGED / FIXTURE_STALE attribution, noANTHROPIC_API_KEYrequired.docs/runners.mdscripts/eval/tests/runner_langgraph_real.shEVAL_RUN_REAL_LANGGRAPH=1). Builds a fresh venv, installs langgraph fromrequirements.txt, runsshell-basicend-to-end. Skips cleanly (exit 0) in normal CI.Requirements covered
bash scripts/eval/runners/langgraph-node.sh {prepare,spawn,fingerprint,teardown} <args>all exit 0.bash scripts/eval/run.sh --skill=langgraph-runner-example --case={shell-basic,jq-path-contains,output-contains}all PASS.bash scripts/eval/tests/runner_langgraph.shexits 0 withANTHROPIC_API_KEYunset.docs/runners.mdexists with all 8 sections; links resolve.env_delta.keys_changedmatching the right class:graph.pymutation →skill_sha + skill_bundle_sha + graph_fingerprint→SKILL_CHANGEDEVAL_LANGGRAPH_VERSIONbump →langgraph_version→MODEL_CHANGEDinput.jsonmutation →fixture_sha→FIXTURE_STALEwc -l examples/langgraph-runner/graph.pyreports 48 (< 50).runner_langgraph.shexercises all 4 subcommands; trace log confirms >=3 spawn invocations.runner_langgraph_real.shpasses whenEVAL_RUN_REAL_LANGGRAPH=1.Test results
Test plan for reviewer
bash scripts/eval/tests/runner_langgraph.sh— full offline test (recommended for review)EVAL_RUN_REAL_LANGGRAPH=1 bash scripts/eval/tests/runner_langgraph_real.sh— real-LangGraph end-to-end (~30-90s; requirespip install langgraph)bash scripts/eval/tests/*.sh— all 21 existing suites (no regressions expected)🤖 Generated with Claude Code