feat(eval): add --runs N flag with median scoring#111
feat(eval): add --runs N flag with median scoring#111yogeshchoudhary147 wants to merge 9 commits into
Conversation
|
Warning Review limit reached
Next review available in: 50 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe CLI adds ChangesRepeated eval runs
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant runCli
participant spawnEval
participant collectFromTempFiles
participant aggregateRuns
participant mergeIntoOutput
runCli->>spawnEval: dispatch expanded per-run jobs
spawnEval->>collectFromTempFiles: collect temporary results
runCli->>aggregateRuns: aggregate collected results
aggregateRuns->>mergeIntoOutput: pass aggregated results
mergeIntoOutput->>runCli: save merged output
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/eval/tests/persistence.test.ts (1)
481-481: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFix typographical error in test description.
The word "medianes" is a typo. It should be "medians".
♻️ Proposed refactor
- it('medianes per-dimension scores', () => { + it('medians per-dimension scores', () => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/eval/tests/persistence.test.ts` at line 481, Update the test description in the “medianes per-dimension scores” test case to use the correctly spelled “medians”, without changing the test behavior.packages/eval/src/cli/subprocess-runner.ts (1)
44-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUnify
mergeIntoOutputwith the shared results merge. Its localeval_id|model|mode|toolskey joinstoolswithout sorting or deduping, so it can drift fromresultKey/mergeResults. Reuse the shared persistence helpers instead of maintaining a second dedup path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/eval/src/cli/subprocess-runner.ts` around lines 44 - 53, The local deduplication path in mergeIntoOutput must use the shared resultKey/mergeResults persistence helpers so tools are normalized consistently. Update packages/eval/src/cli/subprocess-runner.ts:44-53 to remove the local key logic and delegate merging to the shared helpers; update packages/eval/src/cli/index.ts:13-13 to use the same shared merge path where this flow is wired, preserving temp-file collection, final output writing, and cleanup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/eval/src/cli/run.ts`:
- Around line 459-466: Update the hasErrors calculation in the run flow to
inspect the pre-aggregation allFresh results rather than merged, preserving
failures.length and treating any fresh result with status "error" as a process
failure even when aggregateRuns removes it.
In `@packages/eval/src/cli/subprocess-runner.ts`:
- Around line 25-42: Add Vitest tests for the exported collectFromTempFiles
function covering flattening valid array payloads, skipping nonexistent temp
files, and ignoring corrupt JSON while cleaning up existing files. Use temporary
file fixtures and assert the returned records for each scenario.
In `@packages/eval/src/persistence/results.ts`:
- Around line 160-177: Update medianBaselineResult so graders_passed and
grader_pass_rate are derived from a single consistent aggregated value rather
than independently calculated medians. Preserve the existing median aggregation
semantics while ensuring the resulting fields cannot imply different pass rates;
use the appropriate shared baseline count or rate relationship for the group.
---
Nitpick comments:
In `@packages/eval/src/cli/subprocess-runner.ts`:
- Around line 44-53: The local deduplication path in mergeIntoOutput must use
the shared resultKey/mergeResults persistence helpers so tools are normalized
consistently. Update packages/eval/src/cli/subprocess-runner.ts:44-53 to remove
the local key logic and delegate merging to the shared helpers; update
packages/eval/src/cli/index.ts:13-13 to use the same shared merge path where
this flow is wired, preserving temp-file collection, final output writing, and
cleanup.
In `@packages/eval/tests/persistence.test.ts`:
- Line 481: Update the test description in the “medianes per-dimension scores”
test case to use the correctly spelled “medians”, without changing the test
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 89af2cdd-4410-4ba0-b19c-c9a5555784f6
📒 Files selected for processing (11)
packages/eval-core/src/types/results.tspackages/eval/src/cli/config.tspackages/eval/src/cli/index.tspackages/eval/src/cli/run.tspackages/eval/src/cli/subprocess-runner.tspackages/eval/src/cli/validators.tspackages/eval/src/index.tspackages/eval/src/persistence/index.tspackages/eval/src/persistence/results.tspackages/eval/tests/cli-config.test.tspackages/eval/tests/persistence.test.ts
|
Re: CodeRabbit comment on CodeRabbit flagged that We held this back intentionally — the fix is technically correct but the right behavior here is a product decision:
Happy to go with whichever direction the team prefers before merging. |
fc4074a to
c2d8d70
Compare
|
Resolved in 0a99fc7 with Option 3. Before |
0a99fc7 to
f715bfe
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
AGENTS.md (1)
369-379: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winKeep the documented
--model allfallback list in sync.The text says the fallback is the framework’s
KNOWN_WORKING_MODELS, but the bullets omitclaude-opus-4-6andclaude-opus-4-7, which are present in that constant. Ifmodels.knownis empty,--model allwill include them despite this documentation. Either list the fallback values accurately or label the bullets as app-specific.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@AGENTS.md` around lines 369 - 379, The documented --model all list conflicts with its stated KNOWN_WORKING_MODELS fallback by omitting claude-opus-4-6 and claude-opus-4-7. Update the documentation around the eval.config.js model list to include those fallback models, or explicitly label the bullets as app-specific and separately document the complete fallback values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/eval/src/cli/validators.ts`:
- Around line 106-115: Update validateRuns to enforce a documented maximum
positive --runs value before returning, rejecting values above that maximum
through the existing invalid-value path. Define and reuse a named maximum
constant, and add tests covering the maximum accepted value and the first value
above it so runCli cannot expand an unbounded job list.
---
Outside diff comments:
In `@AGENTS.md`:
- Around line 369-379: The documented --model all list conflicts with its stated
KNOWN_WORKING_MODELS fallback by omitting claude-opus-4-6 and claude-opus-4-7.
Update the documentation around the eval.config.js model list to include those
fallback models, or explicitly label the bullets as app-specific and separately
document the complete fallback values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c70910cf-eba7-4c43-a615-d5b2ba7bcc89
📒 Files selected for processing (14)
AGENTS.mdpackages/eval-core/src/types/results.tspackages/eval/src/cli/config.tspackages/eval/src/cli/index.tspackages/eval/src/cli/run.tspackages/eval/src/cli/subprocess-runner.tspackages/eval/src/cli/validators.tspackages/eval/src/index.tspackages/eval/src/persistence/index.tspackages/eval/src/persistence/results.tspackages/eval/tests/cli-config.test.tspackages/eval/tests/persistence.test.tspackages/eval/tests/run.test.tspackages/eval/tests/subprocess-runner.test.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/eval/src/persistence/index.ts
- packages/eval/src/index.ts
- packages/eval/tests/subprocess-runner.test.ts
- packages/eval/src/cli/index.ts
- packages/eval/src/cli/config.ts
- packages/eval/tests/cli-config.test.ts
- packages/eval/src/persistence/results.ts
- packages/eval/src/cli/run.ts
- packages/eval/src/cli/subprocess-runner.ts
- packages/eval/tests/persistence.test.ts
Adds a --runs <n> CLI option (default: 1) that runs each eval job N times in parallel and aggregates the results using median scoring. Scores are medianed per-dimension; costs and tokens are summed across all runs. Raw individual run results are embedded in a runs[] field for debugging, and run_count records how many runs were aggregated. When runs > 1, a run index suffix is appended to each temp file name to avoid collisions, and collectFromTempFiles + aggregateRuns replaces the previous mergeIntoOutput call in the subprocess path.
- Check pre-aggregation allFresh for hasErrors so partial run failures are not silently swallowed when aggregateRuns drops error results - Derive grader_pass_rate from graders_passed/graders_total to keep both fields consistent after aggregation - Add Vitest tests for collectFromTempFiles (7 cases) - Fix typo: medianes → medians in test description
The hasErrors change (allFresh vs merged) needs further discussion — reverting until the product decision on partial-run failure semantics is resolved.
Sort and deduplicate tools in the local key function so it stays consistent with the shared resultKey helper in persistence.
Addresses CodeRabbit pre-merge check — docstring coverage was at 77.78% (below the required 80% threshold). Both internal aggregation helpers were missing JSDoc comments.
…rsion When --runs N was passed, buildSubprocessArgs forwarded it to every child process. Each child saw runs > 1, took the subprocess path, and spawned N more children — each of which did the same, causing exponential process explosion until the runner was OOM-killed (exit 137). Fix: add '--runs' to VALUE_FLAGS so it is stripped before forwarding. Subprocesses run a single job once; run-count expansion is the parent's job. Adds a regression test that would have caught this. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
d1a62b6 to
6914817
Compare
…expansion Without an upper bound, --runs 1000 would expand to 1000 subprocesses. Adds a named MAX_RUNS constant (10), rejects values above it through the existing invalid-value path, and adds boundary tests for the max and first-over-max values. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/evals/src/persistence/results.ts (1)
229-234: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
findDroppedErrorsis O(n²) — fine at current scale, but worth a linear rewrite.For every error result it scans the full
resultsarray again to check for a same-key success, andresultKeyre-derives the string each time. Withevals × models × modes × runspotentially in the hundreds/thousands, this becomes a quadratic hot path. A single pass to build aSetof keys with successes would make this linear.♻️ Suggested O(n) rewrite
export function findDroppedErrors(results: JobResult[]): ErrorJobResult[] { - return results.filter( - (r): r is ErrorJobResult => - r.status === 'error' && results.some((s) => s.status !== 'error' && resultKey(s) === resultKey(r)), - ); + const successKeys = new Set(results.filter((r) => r.status !== 'error').map(resultKey)); + return results.filter((r): r is ErrorJobResult => r.status === 'error' && successKeys.has(resultKey(r))); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/evals/src/persistence/results.ts` around lines 229 - 234, Rewrite findDroppedErrors to avoid the nested results.some scan: make one pass to collect resultKey values for non-error results in a Set, then filter error results by membership in that Set. Reuse each computed key within its pass so resultKey is not repeatedly derived, while preserving the existing ErrorJobResult selection behavior.packages/evals/src/cli/subprocess-runner.ts (1)
44-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the unused
mergeIntoOutputexport and keep deduplication in one place.
mergeIntoOutputhas no runtime callers; the CLI now usesloadResults/mergeResults/saveResultsfrom persistence. Since it is still re-exported frompackages/evals/src/index.tsandcli/index.ts, either remove the public export or move actual call sites to the persistence helpers to avoid duplicated tool-normalization behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/evals/src/cli/subprocess-runner.ts` around lines 44 - 56, Remove the unused mergeIntoOutput function and its public re-exports from packages/evals/src/index.ts and cli/index.ts. Keep the CLI flow using loadResults, mergeResults, and saveResults from persistence so tool normalization and deduplication remain centralized.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/evals-core/src/types/results.ts`:
- Around line 83-86: Define one consistent contract for run_count and runs in
aggregateRuns: specify whether they describe requested runs, successful runs, or
all raw outcomes, and ensure partial failures follow that contract. Update both
metadata declarations in packages/evals-core/src/types/results.ts at lines 83-86
and 148-151; add a regression test covering one successful run plus one error,
including the expected metadata and filtered results.
In `@packages/evals/src/persistence/results.ts`:
- Around line 130-160: Update medianAgentResult’s dimension aggregation to
handle runs with missing dimensions without dereferencing undefined entries. For
each representative dimension, filter the group to runs containing a dimension
at the matching index, compute the median from available scores, and preserve
the existing representative-dimension metadata and score-to-grade calculation;
ensure aggregation continues gracefully when some runs omit that dimension.
In `@packages/evals/tests/persistence.test.ts`:
- Line 17: Update the typed fixtures returned by makeBaseline, makeAgent, and
makeError to include the required judge_cost_usd and total_cost_usd fields,
using values consistent with the existing test fixture conventions while
preserving all other result data.
---
Nitpick comments:
In `@packages/evals/src/cli/subprocess-runner.ts`:
- Around line 44-56: Remove the unused mergeIntoOutput function and its public
re-exports from packages/evals/src/index.ts and cli/index.ts. Keep the CLI flow
using loadResults, mergeResults, and saveResults from persistence so tool
normalization and deduplication remain centralized.
In `@packages/evals/src/persistence/results.ts`:
- Around line 229-234: Rewrite findDroppedErrors to avoid the nested
results.some scan: make one pass to collect resultKey values for non-error
results in a Set, then filter error results by membership in that Set. Reuse
each computed key within its pass so resultKey is not repeatedly derived, while
preserving the existing ErrorJobResult selection behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e5954154-a176-4a73-85b6-e031506bc6b0
📒 Files selected for processing (14)
AGENTS.mdpackages/evals-core/src/types/results.tspackages/evals/src/cli/config.tspackages/evals/src/cli/index.tspackages/evals/src/cli/run.tspackages/evals/src/cli/subprocess-runner.tspackages/evals/src/cli/validators.tspackages/evals/src/index.tspackages/evals/src/persistence/index.tspackages/evals/src/persistence/results.tspackages/evals/tests/cli-config.test.tspackages/evals/tests/persistence.test.tspackages/evals/tests/run.test.tspackages/evals/tests/subprocess-runner.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- AGENTS.md
| /** Number of runs aggregated into this result (absent when run_count is 1). */ | ||
| run_count?: number; | ||
| /** Raw individual run results when run_count > 1. */ | ||
| runs?: BaselineJobResult[]; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Define consistent metadata semantics for partial failures. aggregateRuns() filters errors before aggregation, so a requested two-run job with one success returns run_count: 1 and runs: [success]. That contradicts “absent when run_count is 1” and makes runs ambiguous.
packages/evals-core/src/types/results.ts#L83-L86: document or model whether these fields represent requested runs, successful runs, or all raw outcomes.packages/evals-core/src/types/results.ts#L148-L151: apply the same contract and add a regression test for one success plus one error.
As per coding guidelines, “Add a regression test reproducing each bug before its fix and passing after the fix.”
📍 Affects 1 file
packages/evals-core/src/types/results.ts#L83-L86(this comment)packages/evals-core/src/types/results.ts#L148-L151
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/evals-core/src/types/results.ts` around lines 83 - 86, Define one
consistent contract for run_count and runs in aggregateRuns: specify whether
they describe requested runs, successful runs, or all raw outcomes, and ensure
partial failures follow that contract. Update both metadata declarations in
packages/evals-core/src/types/results.ts at lines 83-86 and 148-151; add a
regression test covering one successful run plus one error, including the
expected metadata and filtered results.
Source: Coding guidelines
| /** Aggregates a group of agent results into a single representative result using median scores and summed costs. */ | ||
| function medianAgentResult(group: AgentJobResult[]): AgentJobResult { | ||
| const sorted = [...group].sort((a, b) => a.overall_score - b.overall_score); | ||
| const mid = Math.floor(sorted.length / 2); | ||
| const rep = sorted[mid]!; | ||
|
|
||
| const medianOverallScore = median(group.map((r) => r.overall_score)); | ||
|
|
||
| const dimensions: DimensionSummary[] = rep.dimensions.map((dim, i) => { | ||
| const score = median(group.map((r) => r.dimensions[i]!.score)); | ||
| return { ...dim, score, weighted: score * dim.weight, grade: scoreToGrade(score) }; | ||
| }); | ||
|
|
||
| return { | ||
| ...rep, | ||
| overall_score: medianOverallScore, | ||
| overall_grade: scoreToGrade(medianOverallScore), | ||
| grader_pass_rate: median(group.map((r) => r.grader_pass_rate)), | ||
| wall_time: median(group.map((r) => r.wall_time)), | ||
| active_time: median(group.map((r) => r.active_time)), | ||
| tool_calls: Math.round(median(group.map((r) => r.tool_calls))), | ||
| interruptions: Math.round(median(group.map((r) => r.interruptions))), | ||
| tokens: group.reduce((sum, r) => sum + r.tokens, 0), | ||
| cost_usd: group.reduce((sum, r) => sum + r.cost_usd, 0), | ||
| judge_cost_usd: group.reduce((sum, r) => sum + r.judge_cost_usd, 0), | ||
| total_cost_usd: group.reduce((sum, r) => sum + r.total_cost_usd, 0), | ||
| dimensions, | ||
| run_count: group.length, | ||
| runs: group, | ||
| }; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Unsafe dimensions[i] indexing can crash if runs report different dimension counts.
rep.dimensions.map((dim, i) => median(group.map((r) => r.dimensions[i]!.score))) assumes every run in group has the same dimensions array length/order as rep. If any run's dimensions array is shorter (e.g. a grader failed to produce a dimension for one run), r.dimensions[i] is undefined and .score throws a TypeError, crashing the whole aggregation for that job/eval instead of degrading gracefully.
🐛 Proposed defensive fix
const dimensions: DimensionSummary[] = rep.dimensions.map((dim, i) => {
- const score = median(group.map((r) => r.dimensions[i]!.score));
+ const scores = group
+ .map((r) => r.dimensions[i]?.score)
+ .filter((s): s is number => typeof s === 'number');
+ const score = scores.length > 0 ? median(scores) : dim.score;
return { ...dim, score, weighted: score * dim.weight, grade: scoreToGrade(score) };
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** Aggregates a group of agent results into a single representative result using median scores and summed costs. */ | |
| function medianAgentResult(group: AgentJobResult[]): AgentJobResult { | |
| const sorted = [...group].sort((a, b) => a.overall_score - b.overall_score); | |
| const mid = Math.floor(sorted.length / 2); | |
| const rep = sorted[mid]!; | |
| const medianOverallScore = median(group.map((r) => r.overall_score)); | |
| const dimensions: DimensionSummary[] = rep.dimensions.map((dim, i) => { | |
| const score = median(group.map((r) => r.dimensions[i]!.score)); | |
| return { ...dim, score, weighted: score * dim.weight, grade: scoreToGrade(score) }; | |
| }); | |
| return { | |
| ...rep, | |
| overall_score: medianOverallScore, | |
| overall_grade: scoreToGrade(medianOverallScore), | |
| grader_pass_rate: median(group.map((r) => r.grader_pass_rate)), | |
| wall_time: median(group.map((r) => r.wall_time)), | |
| active_time: median(group.map((r) => r.active_time)), | |
| tool_calls: Math.round(median(group.map((r) => r.tool_calls))), | |
| interruptions: Math.round(median(group.map((r) => r.interruptions))), | |
| tokens: group.reduce((sum, r) => sum + r.tokens, 0), | |
| cost_usd: group.reduce((sum, r) => sum + r.cost_usd, 0), | |
| judge_cost_usd: group.reduce((sum, r) => sum + r.judge_cost_usd, 0), | |
| total_cost_usd: group.reduce((sum, r) => sum + r.total_cost_usd, 0), | |
| dimensions, | |
| run_count: group.length, | |
| runs: group, | |
| }; | |
| } | |
| /** Aggregates a group of agent results into a single representative result using median scores and summed costs. */ | |
| function medianAgentResult(group: AgentJobResult[]): AgentJobResult { | |
| const sorted = [...group].sort((a, b) => a.overall_score - b.overall_score); | |
| const mid = Math.floor(sorted.length / 2); | |
| const rep = sorted[mid]!; | |
| const medianOverallScore = median(group.map((r) => r.overall_score)); | |
| const dimensions: DimensionSummary[] = rep.dimensions.map((dim, i) => { | |
| const scores = group | |
| .map((r) => r.dimensions[i]?.score) | |
| .filter((s): s is number => typeof s === 'number'); | |
| const score = scores.length > 0 ? median(scores) : dim.score; | |
| return { ...dim, score, weighted: score * dim.weight, grade: scoreToGrade(score) }; | |
| }); | |
| return { | |
| ...rep, | |
| overall_score: medianOverallScore, | |
| overall_grade: scoreToGrade(medianOverallScore), | |
| grader_pass_rate: median(group.map((r) => r.grader_pass_rate)), | |
| wall_time: median(group.map((r) => r.wall_time)), | |
| active_time: median(group.map((r) => r.active_time)), | |
| tool_calls: Math.round(median(group.map((r) => r.tool_calls))), | |
| interruptions: Math.round(median(group.map((r) => r.interruptions))), | |
| tokens: group.reduce((sum, r) => sum + r.tokens, 0), | |
| cost_usd: group.reduce((sum, r) => sum + r.cost_usd, 0), | |
| judge_cost_usd: group.reduce((sum, r) => sum + r.judge_cost_usd, 0), | |
| total_cost_usd: group.reduce((sum, r) => sum + r.total_cost_usd, 0), | |
| dimensions, | |
| run_count: group.length, | |
| runs: group, | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/evals/src/persistence/results.ts` around lines 130 - 160, Update
medianAgentResult’s dimension aggregation to handle runs with missing dimensions
without dereferencing undefined entries. For each representative dimension,
filter the group to runs containing a dimension at the matching index, compute
the median from available scores, and preserve the existing
representative-dimension metadata and score-to-grade calculation; ensure
aggregation continues gracefully when some runs omit that dimension.
| aggregateRuns, | ||
| findDroppedErrors, | ||
| } from '../src/persistence/index.js'; | ||
| import type { AgentJobResult, BaselineJobResult, DimensionSummary, ErrorJobResult } from '@a0/evals-core'; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -A48 '^function make(Baseline|Agent)' packages/evals/tests/persistence.test.ts
rg -n -A45 '^export interface (BaselineJobResult|AgentJobResult)' packages/evals-core/src/types/results.tsRepository: auth0/auth0-evals
Length of output: 5808
🏁 Script executed:
#!/bin/bash
set -e
echo "== persistence.test.ts lines 1-85 =="
sed -n '1,85p' packages/evals/tests/persistence.test.ts
echo
echo "== results.ts type declarations relevant to error/job results =="
sed -n '1,220p' packages/evals-core/src/types/results.ts
echo
echo "== package scripts =="
python3 - <<'PY'
import json
from pathlib import Path
for p in ["package.json", "packages/evals/package.json", "packages/evals-core/package.json"]:
path=Path(p)
if path.exists():
data=json.loads(path.read_text())
print(f"{p} scripts:")
print(json.dumps(data.get("scripts", {}), indent=2))
PY
echo
echo "== lockfile ts versions =="
if [ -f package-lock.json ]; then jq '.packages[] | select(.name=="typescript") | {path, version}' package-lock.json | head -80; fiRepository: auth0/auth0-evals
Length of output: 9932
Add the required cost fields to the typed result fixtures.
BaselineJobResult, AgentJobResult, and ErrorJobResult require judge_cost_usd and total_cost_usd, but makeBaseline, makeAgent, and makeError omit them, so this package’s tests won’t type-check.
Proposed fix
function makeBaseline(overrides: Partial<BaselineJobResult> = {}): BaselineJobResult {
return {
// ...
cost_usd: 0.01,
+ judge_cost_usd: 0,
+ total_cost_usd: 0.01,
// ...
};
}
function makeAgent(overrides: Partial<AgentJobResult> = {}): AgentJobResult {
return {
// ...
cost_usd: 0.05,
+ judge_cost_usd: 0,
+ total_cost_usd: 0.05,
// ...
};
}
function makeError(overrides: Partial<ErrorJobResult> = {}): ErrorJobResult {
return {
// ...
cost_usd: 0,
+ judge_cost_usd: 0,
+ total_cost_usd: 0,
// ...
};
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/evals/tests/persistence.test.ts` at line 17, Update the typed
fixtures returned by makeBaseline, makeAgent, and makeError to include the
required judge_cost_usd and total_cost_usd fields, using values consistent with
the existing test fixture conventions while preserving all other result data.
1 participant
Summary
--runs <n>CLI flag (default: 1, fully backward compatible) that runs each eval job N times in parallel and aggregates results using median scoringruns[]field withrun_countfor debuggingTest plan
npm run buildpassesnpm testpasses (734 tests, 0 failures)npm run lintclean on all changed packagesaggregateRuns()(15 cases) and--runsvalidation (6 cases)Summary by CodeRabbit
New Features
--runs <n>option to repeat evaluation jobs and report median-based results.Bug Fixes