Skip to content

feat(benchmark): add the evaluator the README already points at - #131

Open
Kairose-master wants to merge 2 commits into
Agent-Field:mainfrom
Kairose-master:feat/benchmark-evaluator
Open

feat(benchmark): add the evaluator the README already points at#131
Kairose-master wants to merge 2 commits into
Agent-Field:mainfrom
Kairose-master:feat/benchmark-evaluator

Conversation

@Kairose-master

Copy link
Copy Markdown

The README says "Benchmark assets, logs, evaluator, and generated projects live in examples/agent-comparison/" — but no evaluator is there, so the score table (95/73/62/59) cannot currently be re-derived from anything in the repo. This adds a deterministic scorer for the published rubric so anyone can re-run the comparison on any agent's output and get the same numbers.

What it scores and how honestly it does it:

  • Structure (20), Hygiene (20), Git (15): recomputed from the project directory, one evidence line per check. A disputed score becomes an argument about a file listing or a git log, not about taste.
  • Functional (30): actually runs npm install + npm test, opt-in via --run-tests because it executes the project's code.
  • Quality (15): never auto-scored. It is a judgment call, so the script reports the inputs to that judgment (README, package metadata, custom error types) and assigns no points. A deterministic scorer that pretended to measure "quality" would just be an opinion with extra steps.
  • Unscorable is never zero: the vendored artifacts had .git stripped when they were copied in, so the Git dimension reports UNSCORABLE on them and drops out of the denominator instead of silently failing.

Stdlib-only, no new dependencies. tests/test_benchmark_scorer.py covers the full-marks fixture, each defect docking exactly its own check, the unscorable-vs-zero distinction, and a smoke run against the vendored claude-code-sonnet artifact (8 tests, pass under the CI env guard).

Summary

  • What changed
  • Why it changed

Validation

  • make check
  • Relevant manual test performed (if needed)

Behavior Impact

  • No behavior change
  • Backward compatible behavior change
  • Breaking change (explain below)

Notes

Any rollout notes, migration notes, or follow-ups.

The README says "Benchmark assets, logs, evaluator, and generated projects
live in examples/agent-comparison/" — but no evaluator is there, so the
score table (95/73/62/59) cannot currently be re-derived from anything in
the repo. This adds a deterministic scorer for the published rubric so
anyone can re-run the comparison on any agent's output and get the same
numbers.

What it scores and how honestly it does it:

- Structure (20), Hygiene (20), Git (15): recomputed from the project
  directory, one evidence line per check. A disputed score becomes an
  argument about a file listing or a git log, not about taste.
- Functional (30): actually runs npm install + npm test, opt-in via
  --run-tests because it executes the project's code.
- Quality (15): never auto-scored. It is a judgment call, so the script
  reports the inputs to that judgment (README, package metadata, custom
  error types) and assigns no points. A deterministic scorer that
  pretended to measure "quality" would just be an opinion with extra steps.
- Unscorable is never zero: the vendored artifacts had .git stripped when
  they were copied in, so the Git dimension reports UNSCORABLE on them and
  drops out of the denominator instead of silently failing.

Stdlib-only, no new dependencies. tests/test_benchmark_scorer.py covers
the full-marks fixture, each defect docking exactly its own check, the
unscorable-vs-zero distinction, and a smoke run against the vendored
claude-code-sonnet artifact (8 tests, pass under the CI env guard).
@CLAassistant

CLAassistant commented Aug 6, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@AbirAbbas AbirAbbas left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks — this fills a real gap (the README does point at an evaluator that doesn't exist), the --run-tests gating is correctly implemented (verified: the default path never invokes npm), it's genuinely stdlib-only with no network access, and all 8 tests pass under the CI env. Requesting a small set of changes:

  1. _list_files prunes only .git and node_modules (score.py:86) while JUNK_DIRS also names coverage. A checked-in coverage/lcov-report/ is simultaneously counted as project source modules (inflating Structure) and penalized as junk (docking Hygiene) — the scorer contradicts itself about one directory, and it's already firing on the vendored artifacts (e.g. claude-code-sonnet reports "6 source modules"; it has 2). Prune everything in JUNK_DIRS.
  2. Uncaught exceptions crash the run: subprocess.run(["npm", ...]) raises FileNotFoundError when npm isn't installed, and the timeout= on the npm/git calls (score.py:93, 212, 218) can raise TimeoutExpired. A scorer whose thesis is "unscorable is never zero" should record these as passed=None/False with evidence, not traceback out.
  3. Cheap hardening, optional but free: --ignore-scripts on the npm install step (test-running is the opt-in, lifecycle scripts don't need to be), and git -c core.fsmonitor= ... in _git since git status/log on an untrusted repo can execute config-supplied commands on the default path.
  4. Nit: "node_modules" in gitignore.read_text() is a substring match — a commented-out line earns the 5 points.

One larger point I'm flagging for a maintainer decision rather than asking you to resolve: running the scorer against the five vendored artifacts doesn't reproduce the README's published table (e.g. Structure gives Codex and CC-Haiku 20/20 where the README says 10, and several published values like Structure=10 aren't reachable under the 7/7/6 check split). Whether the answer is tuning the checks or documenting the published table as human-scored under a finer rubric is a call for the repo owners — please hold off reworking the rubric until that's settled; the four items above are worth doing regardless.

…, harden subprocess calls, parse .gitignore lines

Review items from Agent-Field#131, each pinned by a test:

- _list_files prunes everything JUNK_DIRS names: a checked-in coverage/
  no longer counts as project modules for Structure while Hygiene docks
  it as junk — one directory, one verdict.
- FileNotFoundError/TimeoutExpired can't crash the scorer. Missing npm is
  UNSCORABLE (the scorer's toolchain gap is not the project's failure); a
  hung npm test is a failure with evidence (a suite that never finishes
  is a property of the project). Missing/hung git routes every git-backed
  check to UNSCORABLE — including hygiene's clean-status, whose empty
  stdout would otherwise read as "clean".
- npm install runs with --ignore-scripts (running the suite is the
  opt-in; lifecycle scripts are not part of that deal) and every git call
  clears core.fsmonitor.
- .gitignore coverage is parsed per line: comments excluded, and
  node_modules / /node_modules / node_modules/ / **/node_modules all
  count. A commented-out line no longer earns the 5 points.
@Kairose-master

Copy link
Copy Markdown
Author

Thanks — this fills a real gap (the README does point at an evaluator that doesn't exist), the --run-tests gating is correctly implemented (verified: the default path never invokes npm), it's genuinely stdlib-only with no network access, and all 8 tests pass under the CI env. Requesting a small set of changes:

  1. _list_files prunes only .git and node_modules (score.py:86) while JUNK_DIRS also names coverage. A checked-in coverage/lcov-report/ is simultaneously counted as project source modules (inflating Structure) and penalized as junk (docking Hygiene) — the scorer contradicts itself about one directory, and it's already firing on the vendored artifacts (e.g. claude-code-sonnet reports "6 source modules"; it has 2). Prune everything in JUNK_DIRS.
  2. Uncaught exceptions crash the run: subprocess.run(["npm", ...]) raises FileNotFoundError when npm isn't installed, and the timeout= on the npm/git calls (score.py:93, 212, 218) can raise TimeoutExpired. A scorer whose thesis is "unscorable is never zero" should record these as passed=None/False with evidence, not traceback out.
  3. Cheap hardening, optional but free: --ignore-scripts on the npm install step (test-running is the opt-in, lifecycle scripts don't need to be), and git -c core.fsmonitor= ... in _git since git status/log on an untrusted repo can execute config-supplied commands on the default path.
  4. Nit: "node_modules" in gitignore.read_text() is a substring match — a commented-out line earns the 5 points.

One larger point I'm flagging for a maintainer decision rather than asking you to resolve: running the scorer against the five vendored artifacts doesn't reproduce the README's published table (e.g. Structure gives Codex and CC-Haiku 20/20 where the README says 10, and several published values like Structure=10 aren't reachable under the 7/7/6 check split). Whether the answer is tuning the checks or documenting the published table as human-scored under a finer rubric is a call for the repo owners — please hold off reworking the rubric until that's settled; the four items above are worth doing regardless.

Thanks for the careful review — all four items were real. Fixed in 5b9a1be, each pinned by a test:
1. JUNK_DIRS pruning — _list_files now prunes everything JUNK_DIRS names. A checked-in coverage/ no longer inflates Structure’s module count while Hygiene docks it as junk: one directory, one verdict. (test_coverage_dir_is_pruned_from_structure_not_double_counted)
2. Subprocess exceptions — FileNotFoundError / TimeoutExpired can no longer traceback out. Missing npm → passed=None (“UNSCORABLE: npm is not installed”): the scorer’s own toolchain gap is “couldn’t look”, not a project failure. A hung npm test → passed=False with evidence: a suite that never finishes is a property of the project. Missing/hung git routes every git-backed check to UNSCORABLE — including hygiene’s clean-status check, whose empty stdout would otherwise have read as “clean”. (test_missing_npm_is_unscorable_not_a_crash, test_hung_npm_test_is_a_failure_with_evidence, test_missing_git_binary_is_unscorable_not_a_crash)
3. Hardening — npm install now runs with --ignore-scripts (running the suite is the explicit opt-in; lifecycle scripts aren’t part of that deal), and every git invocation clears core.fsmonitor so scoring a repo can never execute a repo-configured daemon.
4. Gitignore matching — replaced the substring scan with per-line parsing: comments excluded, and the common spellings (node_modules, /node_modules, node_modules/, **/node_modules) all count. A commented-out line no longer earns the 5 points. (test_commented_out_gitignore_line_earns_nothing, test_gitignore_spelling_variants_all_cover)
On the bigger point — you’re right, and with the fixes in I reproduce exactly what you saw: codex and claude-code-haiku score Structure 20/20 where the README table says 10, and 10 isn’t even representable under the current 7/7/6 split. I’d frame that divergence as the reason this PR exists: today the table isn’t derivable from anything in the repo, so there’s no mechanical way to say which side is wrong. Two ways to converge, and I’m fine with either:
• Keep the table, recalibrate the checks — if you can say what Structure=10 was docking for those two, I’ll encode it and iterate until the published values are reachable; or
• Keep the rubric, regenerate the table from the scorer’s output and update the README.
The split and the weights are yours to set — the property I’d hold onto is that the table stays the output of a program anyone can re-run. Scorer suite is now 14 tests, green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants