Add an evaluation harness for file selection and content generation

[AshwinUgale]

Implements the eval harness from #14 . PR 1 of 2 (harness only; CI is PR 2). Strictly additive — all new under eval/, no existing file touched.

What it does: runs the real pipeline against test cases and scores both decisions — file selection (precision/recall/F1, index and full-scan paths scored separately) and content generation (deterministic assertions + a no-fence check).

Contents (~93 files, all under eval/):

Harness — execute.py, prep_index.py, aggregate.py, eval.yaml, judges/
Scorer — scoring/: verbatim copy of agent-eval-harness's score.py + support modules; PROVENANCE.md records source/commit + "modifications: none"
Corpus — 10 hand-authored cases (5 positive / 5 negative)
README.md + .gitignore

Design choices (per our discussion):

Copy, not pip — traced score.py's imports: the deterministic path needs only config.py + events.py (stdlib + pyyaml); LLM/MLflow machinery is lazy and unused. Byte-identical, no rewiring.
Frozen indexes (option c) — pre-built .doc-index/ per case + stubbed git fns. (a) mock loses the real path; (b) build-every-run is costly × N; (c) is cheapest and matches the cached-on-main production path. Diff is fed as a string — no git repo.
Two paths scored separately — a blended score hides which regressed.
Assertions, not LLM-judge (v1) — schema reserves the judge slot for v2.
N runs → pass-rate for LLM non-determinism (N=3 PR / N=10 nightly).

Run:
export MODEL_API_BASE=... MODEL_API_KEY=... MODEL_NAME=gpt-4o-mini
python eval/prep_index.py --all
python eval/execute.py --run-id myrun --samples 3
python eval/aggregate.py --run-id myrun --samples 3

Out of scope: CI (PR 2), LLM-judge (v2), mined cases (no corpus exists yet).
Cost + preliminary findings in comments below.

@csoceanu

[AshwinUgale]

Per-run model calls (counts from the actual corpus; $ is order-of-magnitude on gpt-4o-mini):

Run	Calls	~Cost
per case / sample	~4-5
per sample (10 cases)	~41	~$0.02
N=3 (PR)	~123	< $0.10
N=10 (nightly)	~410	~$0.20
index prep (one-time, committed)	~18	~$0.01

Per case ≈ index discovery (~2) + full-scan (~1) + content gen (1 per annotated file). Indexes are committed, so normal runs pay $0 for indexing.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 5:01 PM UTC · Completed 5:15 PM UTC
Commit: 06f62db · View workflow run →

[AshwinUgale]

gpt-4o-mini, N=3 — sharing for interest; the harness is the deliverable, not these numbers:

Judge	Overall
selection - index path	40%
selection - full-scan path	43%
content assertions	60%
no-fence-wrapper	100%
answer-key guard	100%

Discovery looks lexically driven: renaming one code file (cli.py→main.py), nothing else, dropped a case's recall 3/3→1/3 — without name overlap between the changed file and the doc path, positives mostly miss (the 40%/43% is mostly correct negatives). Conservatism holds on negatives bar one vocab-trap; content-gen tends to break conservatism on name-overlap. Can write up as a separate issue if useful.

[fullsend-ai-review]

Review

Findings

Medium

• [logic-error] eval/dataset/cases/001_add_cli_flag/.doc-index/manifest.json — All 10 committed manifest.json files store doc_hashes keys with Windows backslash separators (e.g., "cli\\reference.md"), because prep_index.py was run on Windows. When execute.py runs on Linux, update_indexes_if_needed() calls get_folder_doc_hashes() which uses str(rel_path) — on Linux this produces forward slashes. The hash-key mismatch causes folder_needs_reindex() to return True, silently triggering an LLM-cost index rebuild on every eval run on Linux/macOS, defeating the frozen-index guarantee.
  Remediation: Re-run prep_index.py on Linux so committed manifests use forward slashes, or patch the workspace setup in execute.py to normalize manifest doc_hashes keys to forward slashes before writing .doc-index/ into the workspace.

• [code-organization] eval/scoring/score.py:4605 — The vendored scorer uses exec() with full __builtins__ to compile inline check: snippets from eval.yaml, and eval() to evaluate if: condition strings. The trust model is appropriate (eval.yaml is committed and review-gated), and the README explicitly documents the security/trust boundary. This is accepted risk for vendored, unmodified code.

Low

• [edge-case] eval/execute.py:2150 — The _workspace context manager uses process-global os.chdir() and _capture_stdout replaces process-global sys.stdout. Both would break under parallelization. The protection is purely documentary. The monkeypatching of discovery.fetch_indexes_from_main and discovery.commit_indexes_to_repo creates coupling to pipeline internals, though it is well-documented with detailed comments.

• [missing-test] eval/tests/test_judges.py — Tests cover judge_selection and judge_annotations_present but not the two inline check: judges (content_assertions, no_fence_wrapper) defined in eval.yaml, which contain non-trivial logic.

• [stale-reference] eval/aggregate.py:236 — Calls score._normalize_result(), a private function from the vendored scorer. If the vendored code is re-synced from upstream, this private API could change. Document this dependency in PROVENANCE.md.

• [missing-doc] README.md — The root README has no section for development, testing, or evaluation. The new eval/ directory is not discoverable from main documentation.

• [pattern-inconsistency] eval/aggregate.py:163 — Some helper functions across eval/ files lack docstrings, inconsistent with existing codebase patterns (though several cited functions do have docstrings).

• [pattern-inconsistency] eval/execute.py:2059 — Uses # --- style comment separators; existing codebase uses # === for major sections.

• [missing-doc] RELEASING.md — Does not mention quality gates or the new evaluation harness. Consider adding a note about running the eval harness before releases.

Info

• [pattern-inconsistency] eval/execute.py:2265 — Error messages use plain print() without emoji prefix; existing codebase uses emoji markers. Appropriate for a developer-only tool namespace.

Previous run

Review

Findings

Medium

• [injection-vuln] eval/scoring/score.py:881 — The vendored _make_inline_check compiles and executes arbitrary Python from eval.yaml's check field using exec() with full __builtins__ access. While eval.yaml is a committed config file requiring PR review to modify (limiting the practical attack surface), the trust boundary should be documented explicitly — anyone with write access to eval.yaml gains arbitrary code execution during scoring.
  Remediation: Document in eval/README.md or PROVENANCE.md that eval.yaml is a trusted input and its check: fields execute arbitrary Python. Consider restricting the exec() namespace to a minimal set of safe builtins in a future upstream PR.

• [logic-error] eval/judges/meta.py:16 — The annotations_present meta judge only checks for the expected_files key but not the content key. If a case author omits the content block in annotations.yaml, the content_assertions judge vacuously passes (iterates zero rules, returns True). The meta judge was specifically designed to prevent vacuous passes (per its docstring) but only guards the selection dimension, not content generation.
  Remediation: Extend judge_annotations_present to also verify that when content output files exist, a content key is present in annotations. Alternatively, add a second meta judge content_annotations_present.

Low

• [injection-vuln] eval/scoring/score.py:774 — The vendored score_cases uses eval() with a bypassable {"__builtins__": {}} sandbox for judge conditions. Currently unused in this PR's eval.yaml (no if: conditions), but the trust model should be documented.

• [injection-vuln] eval/scoring/score.py:893 — The vendored _load_code_judge uses importlib.import_module() to load arbitrary modules from eval.yaml's module: field. This is by design (it loads judges.meta and judges.selection), but the trust boundary is implicit.

• [injection-vuln] eval/scoring/agent_eval/_bootstrap.py:42 — Vendored bootstrap calls os.execv() for venv auto-activation. The check at line 19 guards against non-existent venv, but a malicious .eval-venv/bin/python3 placed via future PR would be executed. No .eval-venv/ exists currently.

• [edge-case] eval/execute.py:144 — _workspace uses process-global os.chdir(). Cases run sequentially (safe), but discovery's internal threads depend on cwd. Add a comment documenting the sequential-only constraint.

• [logic-error] eval/aggregate.py:34 — Flake exclusion is over-aggressive: if content generation for ANY doc flakes, both content_assertions and no_fence_wrapper judges are excluded for the entire sample, losing signal from non-flaked docs. Acceptable v1 limitation but worth documenting.

• [missing-doc] README.md — eval/ adds a complete evaluation harness with comprehensive eval/README.md, but the main README has no mention of it. Consider adding a cross-reference in PR 2 (CI wiring).

• [architectural-conflict] eval/scoring/ — 1,700+ lines vendored from agent-eval-harness. PROVENANCE.md documents source/commit/re-sync, which is good. Consider adding expected update cadence or upstream watch process.

• [edge-case] eval/execute.py:60 — If openai package is absent, _API_ERRORS becomes () and the except clause catches nothing. In practice, openai is required for the pipeline, so this path is unreachable.

• [misplaced-abstraction] eval/execute.py:54 — Monkeypatching discovery.fetch_indexes_from_main and discovery.commit_indexes_to_repo creates tight coupling with production internals. Well-documented but fragile if discovery's imports change.

Info

• [naming-convention] eval/execute.py:237 — ArgumentParser variable named ap (also in aggregate.py:77, prep_index.py:73) while existing codebase uses parser. Internally consistent within eval/.

• [naming-convention] eval/ — Introduces eval-specific terminology (judges, cases, samples, runs). eval/README.md explains terms in context.

• [test-inadequate] eval/dataset/cases — No unit tests for the harness code itself (judges, aggregator, path normalization, flake detection). Only testable via end-to-end runs requiring a model API key. Could follow in a future PR.

[fullsend-ai-review]

See the review comment for full details.

[AshwinUgale]

Thanks for the thorough pass — addressed it:

• content-guard gap (logic-error): extended judge_annotations_present to require a content block, so a case that omits it now fails loudly instead of letting content_assertions pass vacuously. ("Require content key only when content output exists" wouldn't catch it here — content is generated only for annotated files, so an omitted block also means no output; the guard now requires it unconditionally.)
• trust boundary (injection-vuln): documented in eval/README.md — eval.yaml is a trusted input whose check:/if:/module: fields execute code during scoring. scoring/ is copied byte-identical from agent-eval-harness, so documented rather than patched; hardening the exec() namespace is an upstream change.
• sequential chdir (edge-case): documented the sequential-only constraint in _workspace.
• no judge unit tests (test-inadequate): added eval/tests/test_judges.py — normalize_path, selection F1, and the guard (incl. the new content check). No model key needed.
• flake-exclusion coarseness / main-README cross-ref: noted as v1 and PR-2 items respectively.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 6:16 PM UTC · Completed 6:29 PM UTC
Commit: 06f62db · View workflow run →

[csoceanu]

Hi @AshwinUgale, thanks for the PR!

This is a really big PR. Since we have to manually review everything, could you please split it into smaller, more focused PRs? It'll help us review it much faster. Thanks!

[AshwinUgale]

Hi @csoceanu ,

Splitting this into three smaller, focused PRs per your request. They stack (each is a dependency of the next), so they land in sequence:

1. Scorer — # — the vendored scoring engine. Open now.
2. Engine — runner + judges + config. Follows once first merges.
3. Corpus — the 10 test cases. Follows once second merges.

I'm closing this one.
Thanks for the steer on breaking it up.