Skip to content

Feature: design of eval harness - #220

Open
sanjayankur31 wants to merge 28 commits into
developmentfrom
feat-eval-harness
Open

sanjayankur31 wants to merge 28 commits into
developmentfrom
feat-eval-harness

Conversation

@sanjayankur31

Copy link
Copy Markdown
Member
  • includes the ADRs that document the design of the evaluation harness. (ADRs 0056-0060)

ADR-0056 (Test Case Format): update Date from 2025-01-01 to 2026-10-27
ADR-0057 (Run Manager & Isolation): update Date from 2025-01-01 to 2026-10-27
AGENTS.md: add guidance that ADRs must use today date in YYYY-MM-DD format

This ensures ADRs use current dates and prevents future use of placeholder dates.
…S.md

- ADR files use YYYY-MM-DD format for the Date field in MADR header
- Session log files use YYYY-MM-DD-HHMM format where HHMM is 24-hour time
- Separate files per session avoid merge conflicts when multiple sessions run the same day
- Updated documentation to explicitly state these conventions for clarity
Convert the metrics collection ADR to the devdocs/adr-template.md structure:
- add YAML frontmatter with status, date and decision-makers
- rename Problem to Context and Problem Statement
- add Decision Drivers and Considered Options sections
- rename Decision to Decision Outcome with explicit chosen option
- demote Consequences under the Decision Outcome and rephrase as Good/Bad bullets
- convert the comparison tables into Pros and Cons of the Options with one heading per option
- fold Rationale, Implementation Notes, Examples, Related Decisions and References under a final More Information section
Convert the run manager ADR to the devdocs/adr-template.md structure:
- add YAML frontmatter with status, date and decision-makers
- rename Problem to Context and Problem Statement
- add Decision Drivers and Considered Options sections
- rename Decision to Decision Outcome with explicit chosen option
- demote Consequences under the Decision Outcome and rephrase as Good/Bad bullets
- convert the comparison tables into Pros and Cons of the Options with one heading per option
- fold Rationale, Implementation Notes, Examples, Related Decisions and References under a final More Information section
Convert the test case format ADR to the devdocs/adr-template.md structure:
- add YAML frontmatter with status, date and decision-makers
- rename Problem to Context and Problem Statement
- add Decision Drivers and Considered Options sections
- rename Decision to Decision Outcome with explicit Chosen option
- demote Consequences under the Decision Outcome and rephrase as Good/Bad bullets
- convert the alternatives comparison table into Pros and Cons of the Options with one heading per option
- fold Rationale, Implementation Notes, Related Decisions and References under a final More Information section
Update ADR-0056 and ADR-0057 references to the current ADR titles (ADR-0057 Run Manager & Isolation, ADR-0058 Metrics Collection & Aggregation) and drop the duplicated metrics schema in ADR-0057 in favour of ADR-0058.
Specify a filesystem directory-per-run result layout (run.json, metrics.jsonl, transcript.jsonl, artifacts/, COMPLETE marker) with an append-only runs/index.jsonl index, atomic rename-on-completion writes, schema-version stamping per ADR-0058, and configurable artifact retention.
Move ADR-0056 (test case format), ADR-0057 (run manager & isolation), ADR-0058 (metrics collection & aggregation), ADR-0059 (result storage) and ADR-0060 (harness execution) from proposed to accepted.

Also revise the ADR-0058 primary vs. secondary metrics hierarchy: task_success, pass@k and turn_count are primary evaluative metrics, while token_usage, latency and cost_estimate are secondary/optional; latency is explicitly not an evaluation signal.
@sanjayankur31 sanjayankur31 added documentation Improvements or additions to documentation enhancement New feature or request labels Oct 28, 2026
@sanjayankur31 sanjayankur31 moved this from 🆕 New to 👀 In review in NeuroML backlog Oct 28, 2026
@sanjayankur31 sanjayankur31 linked an issue Oct 28, 2026 that may be closed by this pull request
@sanjayankur31

Copy link
Copy Markdown
Member Author

@khanak0509 : this is what I've come up with for the evaluation harness---what do you think? im slowly beginning to work on the code now---starting with the package scaffold + jsonl bits.

@sanjayankur31

Copy link
Copy Markdown
Member Author

(will merge this in a couple of days---just up here for us to review once)

@khanak0509

Copy link
Copy Markdown
Collaborator

Hii, looking solid. Will take a deeper look, just need a bit of time.

Add a streaming JSONL test-case loader (klea_eval.cases) that validates every case against the ADR-0056 schema and invariants, raising CaseValidationError (klea_eval.errors) with source path, line number and case_id. Cover the loader and every rejection rule with tests in eval_pkg/tests/test_cases.py, and align the ADR field names (output_format, rubric_type).
@khanak0509

Copy link
Copy Markdown
Collaborator

Hii, took a deeper look. The overall direction really looks good, and the eval_pkg scaffold plus the JSONL loader are a solid start.

A few things though that I think are worth fixing before the run manager lands hard on the accepted ADRs. So like run_id is only case_id + seed right now, which means topology, model and pass@k repeats all collide on the same id, and then skip COMPLETE would just keep the old results after the agent changes. Also chat_id is per turn, but in klea conversation state is per chat, so multi-turn would lose continuity unless it's one chat_id per run. And the determinism claim feels a bit strong, I think it should cover IDs, config and ordering, not identical LLM outputs, because even at temperature 0 that is not really true.

Also ADR-0058 assumes the SSE tool/usage events carry args, duration and errors, but streams.md does not have that today, so either the stream contract needs extending or those metrics need to wait. Cases also don't have interrupt answers, mode or access_level yet, so HITL runs would hang. And how the agent actually gets pointed at runs//workspace/ is not named, maybe client_context.cwd?

One more small thing, 0057 and 0059 still disagree a bit on the run directory layout, like metadata.json vs run.json and when the dir is created.

What do you think?

Reconcile the five evaluation harness ADRs so ownership boundaries are stated once instead of contradicting each other.

- 0057/0059: evaluation workspace root outside the source tree; run dir is runs/<run_id>/ with run.json, metrics.jsonl at the run root, transcript.jsonl, workspace/ and COMPLETE marker

- 0058/0059/0060: ADR-0059 is the single owner of the write protocol, ADR-0058 owns the metric record schema and per-run aggregation, ADR-0060 owns orchestration and suite-level aggregation (pass@k)

- 0056: roles renamed to plan/execute/eval/chat plus user; fixtures resolve against the case file directory

- 0058: metrics.jsonl is one JSON object per line with schema_version and record_type; pass_at_k removed from the per-run envelope

- 0057/0058/0060: determinism scoped to identities, ordering and derived metrics; LLM sampling makes turn content and latency non-identical

- 0057: collision-free identity tuple (run_id, user_id, chat_id) with chat_id derived from run_id + user_id; run_config.model_overrides stated explicitly

- 0059/0060: uniform date 2026-10-27
Fix the residual contradictions left after the ADR-0056..0060 reconciliation.

- 0057 context Model Resolution: name the turn role set (user, plan, execute, eval, chat) instead of the retired assistant/evaluator roles, and state that only model-backed roles are resolved

- 0058 Evaluation Result Record: drop the per-run pass_at_k field, noting it is a suite-level aggregate produced by the ADR-0060 Aggregate stage

- 0058 Related Decisions: describe the run-scoped identity (run_id, case_id) correctly instead of the malformed 'case_id's run_dir'

- 0056 Related Decisions: stop claiming ADR-0057 resolves the user role; only plan/execute/eval/chat are model-resolved
Do this separately, because we don't want this included in the
requirements file---it's only a dev dep

This branch has not been deployed

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

Labels

documentation Improvements or additions to documentation enhancement New feature or request

Projects

Status: 👀 In review

Development

Successfully merging this pull request may close these issues.

Design: Agent: evaluation harness

2 participants