feat: Engineer Harness release evaluation — telemetry, budgets, Terminal-Bench canary, hosts, and release gates - #38
Open
noodlemind wants to merge 98 commits into
Open
Conversation
Model/provider profiles with pinned routing and pricing, code-enforced budget ceilings with pre-request refusal, structured run telemetry, and an extended OpenAI-compatible driver: usage/cost capture, fallback detection, reasoning metadata round-trip, tool-result truncation events, explicit stop reasons, and network-vs-billable failure classification.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughSummaryAdded provider accounting and telemetry, secure Terminal-Bench execution, release evaluation orchestration, schemas, host adapters, configurations, and extensive tests. ChangesEvaluation platform controls
Terminal-Bench execution
Release evaluation orchestration
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Harbor-based execution of terminal-bench@2.0 cobol-modernization: fail-closed task pinning with tree checksums, documented-flag CLI construction behind an injected spawn, official verifier artifact reading with pytest counts and end-state tree hashing, generic/harness A/B condition builders with parity invariants, a Node stdio bridge agent with explicit stop reasons, and the thin Harbor BaseAgent Python wrapper.
Controlled Kimi/OpenRouter host with fail-closed credentials, local Gemma/Ollama host with Docker endpoint rewrite, Codex and Claude subscription A/B contracts with transcript preservation and null-not- estimated telemetry normalization, and Copilot/Grok compatibility smoke checklists with fail-closed evaluation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
packages/harness/test/eval-model-profiles.test.mjs (1)
57-60: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe
'use strict'line is a no-op here.ES modules are always strict, so the directive adds nothing and implies strictness is opt-in. Also worth asserting the nested
provider.orderarray is frozen, since that is whatdeepFreeze's array recursion actually covers.♻️ Tighten the freeze assertions
assert.ok(Object.isFrozen(p.provider)); + assert.ok(Object.isFrozen(p.provider.order)); assert.throws(() => { - 'use strict'; p.pricing.outputPerM = 0; }, TypeError);🤖 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/harness/test/eval-model-profiles.test.mjs` around lines 57 - 60, Remove the redundant 'use strict' directive from the assert.throws callback in the freeze assertions, and add an assertion that the nested provider.order array is frozen to verify deepFreeze recursively freezes arrays.packages/harness/test/eval-budget.test.mjs (2)
89-96: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winOnce the parent-refusal flag asymmetry flagged in
evals/lib/budget.mjs(Lines 71-74) is fixed, extend this test to asserttrial.exhausted === trueand that the trial's ownevents()records the refusal.🤖 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/harness/test/eval-budget.test.mjs` around lines 89 - 96, Extend the test around trial.precheck in the “precheck refuses when the trial fits but the release ceiling would be crossed” case to assert trial.exhausted is true and trial.events() contains a record for the refusal, while preserving the existing denied verdict and release-reason assertions.
60-77: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd the exact-ceiling boundary case.
precheckuses a strict>, so a request landing exactly on the ceiling is allowed. That boundary is the one most likely to flip in a future refactor and no test pins it.💚 Suggested addition
+test('precheck allows a request that lands exactly on the ceiling', () => { + const budget = createBudget({ ceilingUsd: 5, label: 'trial' }); + budget.charge(4, 'earlier'); + assert.equal(budget.precheck(1).allowed, true); + assert.equal(budget.exhausted, false); +});🤖 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/harness/test/eval-budget.test.mjs` around lines 60 - 77, Add a test alongside the existing precheck tests that charges an amount leaving room exactly equal to the requested estimate, then assert precheck allows it and does not mark the budget exhausted. Use createBudget, charge, and precheck consistently with the surrounding tests to pin the inclusive ceiling boundary.evals/lib/budget.mjs (1)
17-19: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueName says integer, predicate accepts any finite non-negative number.
nonNegativeInt(1.5)istrue. Either rename tononNegativeNumberor addNumber.isInteger; token counts are integral, so the stricter check would also catch garbage like1e-3from a provider.🤖 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 `@evals/lib/budget.mjs` around lines 17 - 19, Update nonNegativeInt to require Number.isInteger(value) in addition to its existing numeric, finite, and non-negative checks, ensuring fractional values such as 1.5 and 1e-3 are rejected while valid non-negative integers remain accepted.evals/lib/model-profiles.mjs (1)
52-58: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueGuard the lookup against inherited keys.
getProfile('constructor')(or'toString') resolves throughObject.prototypeand returns a truthy non-profile instead of throwing.♻️ Use an own-property check
export function getProfile(id) { - const profile = PROFILES[id]; - if (!profile) { + const profile = Object.hasOwn(PROFILES, id) ? PROFILES[id] : undefined; + if (!profile) { throw new Error(`unknown model profile: ${id} (known: ${Object.keys(PROFILES).join(', ')})`); } return profile; }🤖 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 `@evals/lib/model-profiles.mjs` around lines 52 - 58, Update getProfile to validate that id is an own key of PROFILES before returning its value, rather than relying on the truthiness of PROFILES[id]. Preserve the existing unknown-profile error message and normal lookup behavior for declared profiles.evals/lib/telemetry.mjs (1)
30-34: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winPayload keys can overwrite
seq/type, and the stored event is returned by reference.
{ seq, type, ...data }lets adata.typeordata.seqsilently rewrite the transcript's own metadata, which defeats the append-only audit guarantee. Spreadingdatafirst fixes it. Returning the stored object also lets a caller mutate recorded history after the fact — return a copy if that matters.🛡️ Make metadata authoritative
record(type, data = {}) { - const event = { seq: seq++, type, ...data }; + const event = { ...data, seq: seq++, type }; events.push(event); - return event; + return { ...event }; },🤖 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 `@evals/lib/telemetry.mjs` around lines 30 - 34, Update the record method so payload fields cannot overwrite the authoritative seq and type metadata: spread data before assigning the generated sequence and supplied type. Return a copy of the event rather than the stored object, while preserving the existing event pushed into events.
🤖 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 `@evals/lib/budget.mjs`:
- Around line 49-52: Update createBudget to validate ceilingUsd at construction
time, rejecting missing, non-numeric, NaN, or otherwise invalid ceiling values
with an error instead of allowing an unlimited budget. Ensure valid ceilings
continue through the existing spent, precheck, and remainingUsd logic unchanged.
- Around line 71-74: Update the parent refusal path in the child budget precheck
flow so the child marks itself exhausted and records the refusal event before
returning the parent’s refusal result. Preserve the existing parent result while
ensuring trial.exhausted and events() reflect the blocked request.
In `@evals/lib/drivers.mjs`:
- Around line 158-201: Add per-request timeout handling to callApi using the
profile’s timeoutMs value, creating an AbortController and passing its signal to
both fetchImpl and res.json(). Keep the timer active until JSON parsing
completes, clear it on every outcome, and convert aborts caused by the timeout
into a classified ProviderError with appropriate telemetry. Update production
driver callers to pass the profile/timeout configuration into callApi.
- Line 88: Update the driver state reset logic to assign fallbackDetected =
false within reset(), alongside the other driver-state fields. Ensure reused
drivers start each retry or trial without carrying fallback state from previous
attempts.
In `@evals/lib/model-profiles.mjs`:
- Around line 23-49: Update the pricing values in the kimi-k2.7-code profile to
match the exact pinned moonshotai provider endpoint rather than the model-level
listing. Keep the provider pin in the profile unchanged and ensure drivers.mjs
consumers receive the endpoint-specific input, cached-input, and output rates
for cost totals, budget charges, and prechecks.
---
Nitpick comments:
In `@evals/lib/budget.mjs`:
- Around line 17-19: Update nonNegativeInt to require Number.isInteger(value) in
addition to its existing numeric, finite, and non-negative checks, ensuring
fractional values such as 1.5 and 1e-3 are rejected while valid non-negative
integers remain accepted.
In `@evals/lib/model-profiles.mjs`:
- Around line 52-58: Update getProfile to validate that id is an own key of
PROFILES before returning its value, rather than relying on the truthiness of
PROFILES[id]. Preserve the existing unknown-profile error message and normal
lookup behavior for declared profiles.
In `@evals/lib/telemetry.mjs`:
- Around line 30-34: Update the record method so payload fields cannot overwrite
the authoritative seq and type metadata: spread data before assigning the
generated sequence and supplied type. Return a copy of the event rather than the
stored object, while preserving the existing event pushed into events.
In `@packages/harness/test/eval-budget.test.mjs`:
- Around line 89-96: Extend the test around trial.precheck in the “precheck
refuses when the trial fits but the release ceiling would be crossed” case to
assert trial.exhausted is true and trial.events() contains a record for the
refusal, while preserving the existing denied verdict and release-reason
assertions.
- Around line 60-77: Add a test alongside the existing precheck tests that
charges an amount leaving room exactly equal to the requested estimate, then
assert precheck allows it and does not mark the budget exhausted. Use
createBudget, charge, and precheck consistently with the surrounding tests to
pin the inclusive ceiling boundary.
In `@packages/harness/test/eval-model-profiles.test.mjs`:
- Around line 57-60: Remove the redundant 'use strict' directive from the
assert.throws callback in the freeze assertions, and add an assertion that the
nested provider.order array is frozen to verify deepFreeze recursively freezes
arrays.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 49f36e5f-81f8-465e-a4a7-c6228882481d
📒 Files selected for processing (8)
evals/lib/budget.mjsevals/lib/drivers.mjsevals/lib/model-profiles.mjsevals/lib/telemetry.mjspackages/harness/test/eval-budget.test.mjspackages/harness/test/eval-driver-telemetry.test.mjspackages/harness/test/eval-model-profiles.test.mjspackages/harness/test/eval-telemetry.test.mjs
Release runner sequencing deterministic evals, preflight, A/B pairs, smokes, gate policy, and reporting: §8 pair classification with safety/infrastructure/budget precedence, §9 gates with calibration softening and full-pair conditional rerun, §10 chained budgets with a reason-gated reserve, eval-run.v1 and eval-report.v1 schema contracts with a dependency-free validator, release-canary config, markdown eval card, and README runbook. Paid steps never run after a failed preflight.
noodlemind
changed the title
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
Budgets refuse construction without a valid ceiling instead of failing open, and a parent-ceiling refusal marks the child budget exhausted with its own audit event. Kimi profile pricing now matches the pinned Moonshot AI endpoint rates rather than the model-level floor. The driver clears fallback state on reset and aborts hung requests at the profile timeout, classifying them as unknown-billing timeout failures.
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (8)
packages/harness/test/eval-tb-agent.test.mjs (1)
53-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDead expression in the scripted result, and the value it produces is never asserted.
JSON.parse('{}')is always truthy, so the ternary is noise. Also consider adding a case whereinputends (input.end()) while an exec is outstanding — that path is currently uncovered and is where the hang flagged inagent.mjssurfaces.♻️ Simplify
- const { input, output, lines } = pump({ resultFor: (line) => ({ code: 0, stdout: `ran:${JSON.parse('{}') ? line.command : ''}`, stderr: '' }) }); + const { input, output, lines } = pump({ resultFor: (line) => ({ code: 0, stdout: `ran:${line.command}`, stderr: '' }) });🤖 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/harness/test/eval-tb-agent.test.mjs` at line 53, Simplify the scripted result callback in pump by removing the always-truthy JSON.parse ternary and directly using line.command for stdout. Also add coverage for calling input.end() while an exec remains outstanding, asserting the expected completion behavior and preventing the hang path in agent.mjs.evals/external/terminal-bench/verifier.mjs (1)
68-77: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
hashTreereads every artifact fully into memory and the tree is walked twice per collection.
collectVerifierEvidencewalks the tree, thenhashTreewalks it again andreadFileSyncs each file. Harbor artifact trees include full container/verifier logs, so this can spike memory on a large trial. Accepting a pre-computed file list and streaming contents into the hash would remove both costs without changing the digest.🤖 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 `@evals/external/terminal-bench/verifier.mjs` around lines 68 - 77, Update hashTree to accept the already-collected file list from collectVerifierEvidence instead of walking the directory again, and hash each file via a streaming read rather than readFileSync so full artifacts are not loaded into memory. Preserve the existing relative-path, separator, and SHA-256 digest ordering to keep the resulting tree hash unchanged.evals/external/terminal-bench/harbor-adapter.mjs (1)
60-62: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider validating the lock before building args.
buildHarborRunArgstrustslock.datasetRef/lock.taskunvalidated; callers that skipverifyTaskAgainstLockwould spawn an unpinned run. A cheapvalidateTaskLockassertion here makes the fail-closed guarantee independent of call order.🤖 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 `@evals/external/terminal-bench/harbor-adapter.mjs` around lines 60 - 62, Update buildHarborRunArgs to call the existing validateTaskLock assertion before constructing or returning the Harbor arguments, using the provided lock and task context. Ensure invalid or unpinned lock values fail closed even when verifyTaskAgainstLock was not called by the caller.evals/external/terminal-bench/agent.mjs (1)
126-133: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
?? 'local'defeats the clean-skip contract for hosted providers.
openAiToolDriverreturnsnullwhen the API key is missing so the caller can skip cleanly (evals/lib/drivers.mjs:80). Substituting'local'turns a missingOPENROUTER_API_KEYinto a run that spends a trial slot on guaranteed 401s. Keep the placeholder only for keyless local endpoints (e.g. Ollama) rather than unconditionally.♻️ Suggested gating
- const apiKey = process.env[condition.apiKeyEnv ?? 'OPENROUTER_API_KEY'] ?? 'local'; + const apiKeyEnv = condition.apiKeyEnv ?? 'OPENROUTER_API_KEY'; + const apiKey = process.env[apiKeyEnv] ?? (profile.requiresKey === false ? 'local' : null); + if (!apiKey) throw new Error(`missing API key: set ${apiKeyEnv} for profile ${profile.id ?? condition.profileId}`);🤖 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 `@evals/external/terminal-bench/agent.mjs` around lines 126 - 133, Update the API-key selection in the trial setup around openAiToolDriver so a missing environment key remains missing for hosted providers, allowing the driver’s null result to trigger a clean skip. Only use the 'local' placeholder when the selected profile targets a keyless local endpoint such as Ollama, while preserving explicit configured keys and the existing driver-not-configured handling.evals/release.mjs (1)
151-155: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGate activity is hardcoded while the profile declares it in data.
evals/config/release-canary.yamldeclarespairs[].gate(after-calibration,informational),enabled,timeoutMs, andcalibrationReleases: 3, but the CLI only readsprofile,task.lockFile, andbudget.*. Editing those YAML keys silently has no effect, which undermines the file's "matrix … in data" premise. Consider deriving gate activity fromconfig.pairs, falling back to the current mapping when a host is absent.♻️ Sketch
-function gateActiveFor(host, calibrationRelease) { - if (host === 'openrouter-kimi') return !calibrationRelease; // gate: after-calibration - if (host === 'ollama-gemma') return false; // gate: informational - return true; // frontier rotation gates when scheduled +function gateActiveFor(host, calibrationRelease, pairs = []) { + const gate = pairs.find((p) => p.host === host)?.gate; + if (gate === 'informational') return false; + if (gate === 'after-calibration') return !calibrationRelease; + return true; // frontier rotation gates when scheduled }🤖 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 `@evals/release.mjs` around lines 151 - 155, Update the release configuration loading and gateActiveFor flow to derive each host’s gate behavior from config.pairs, including gate, enabled, timeoutMs, and calibrationReleases where applicable, so edits to release-canary.yaml take effect. Preserve the current host-specific behavior as a fallback when a host has no matching pair, including after-calibration handling for openrouter-kimi and informational handling for ollama-gemma.packages/harness/test/eval-hosts.test.mjs (1)
78-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtend the transcript/instructions test to cover the Codex host too.
This test only exercises
createClaudeHost();createCodexHost()'s parallel instructions (e.g. "do not assume the subscription resolves to the same version") go unchecked. Line 48'sfor (const host of [createCodexHost(), createClaudeHost()])pattern could be reused here.🤖 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/harness/test/eval-hosts.test.mjs` around lines 78 - 88, Extend the test around createClaudeHost to also exercise createCodexHost, preferably by iterating over both hosts as in the existing host loop. Preserve the transcript persistence assertions for each host, and verify each host’s runInstructions includes the manual A/B guidance and resolved-model requirement, including Codex-specific subscription-version wording.evals/hosts/copilot-smoke.mjs (1)
8-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract shared smoke-host boilerplate.
copilot-smoke.mjsandgrok-smoke.mjsduplicate theCHECKLISTshape,createHost(), andevaluate(); onlyidand the discovery-item wording differ.
evals/hosts/copilot-smoke.mjs#L8-L28: move this into a factory (e.g.createSmokeHost({ id, checklist })) and call it with the Copilot checklist.evals/hosts/grok-smoke.mjs#L7-L27: call the same shared factory with the Grok checklist.🤖 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 `@evals/hosts/copilot-smoke.mjs` around lines 8 - 28, Extract the duplicated CHECKLIST/createHost/evaluate implementation into a shared createSmokeHost factory. In evals/hosts/copilot-smoke.mjs lines 8-28, call the factory with the Copilot id and checklist; in evals/hosts/grok-smoke.mjs lines 7-27, replace the duplicate host implementation with the same factory using the Grok id and checklist, preserving each host’s discovery wording.evals/hosts/claude-subscription.mjs (1)
13-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract shared subscription-host boilerplate.
claude-subscription.mjsandcodex-subscription.mjsduplicateTELEMETRY_FIELDS,NUMERIC_FIELDS,telemetryTemplate(),normalizeHostReport(), andpreserveTranscript()verbatim; onlyid,gate, andrunInstructionstext differ.
evals/hosts/claude-subscription.mjs#L13-L65: move this shared logic into a factory (e.g.createSubscriptionHost({ id, gate, runInstructions })) in a new shared module and call it here with Claude-specific params.evals/hosts/codex-subscription.mjs#L15-L67: call the same shared factory with Codex-specificid/runInstructions.♻️ Proposed shared factory sketch
// evals/hosts/subscription-host-base.mjs export function createSubscriptionHost({ id, gate, runInstructions }) { const TELEMETRY_FIELDS = [ 'premiumRequestsConsumed', 'rateLimitEvents', 'hostReportedPromptTokens', 'hostReportedOutputTokens', 'hostReportedModel', 'fallbackObserved', ]; const NUMERIC_FIELDS = new Set([ 'premiumRequestsConsumed', 'rateLimitEvents', 'hostReportedPromptTokens', 'hostReportedOutputTokens', ]); return { id, kind: 'subscription', gate, runInstructions, telemetryTemplate() { return Object.fromEntries(TELEMETRY_FIELDS.map((f) => [f, null])); }, normalizeHostReport(raw = {}) { /* ...shared body... */ }, preserveTranscript({ transcript, dir, label = 'run' }) { /* ...shared body... */ }, }; }🤖 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 `@evals/hosts/claude-subscription.mjs` around lines 13 - 65, Extract the duplicated subscription-host logic from evals/hosts/claude-subscription.mjs lines 13-65 and evals/hosts/codex-subscription.mjs lines 15-67 into a shared createSubscriptionHost factory in a new shared module, including telemetryTemplate, normalizeHostReport, preserveTranscript, and the telemetry constants. Update both host modules to call the factory with their respective id, gate, and runInstructions while preserving their existing host-specific behavior.
🤖 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 `@evals/external/terminal-bench/agent.mjs`:
- Around line 55-73: Update the readline setup around nextLine to handle input
stream close and error events, settling every outstanding waiter with a
protocol_error result so pending reads cannot hang. Reuse the existing waiter
queue and preserve normal line parsing and pendingLines behavior; ensure
runStdioAgent can continue to report completion after the stream terminates.
In `@evals/external/terminal-bench/harbor_agent.py`:
- Around line 43-46: Update the setup loop in async setup to capture the exit
code returned by _exec and stop setup by raising an error when any setup command
fails; only continue to run the trial when every command completes successfully.
- Around line 92-105: Update _exec to honor the per-command timeoutMs supplied
by agent.mjs, without assuming a specific Harbor keyword or dependency version.
Pass the timeout through when the Harbor exec surface supports it, otherwise
wrap the awaitable with asyncio.wait_for; on timeout, return a non-zero result
with appropriate stderr instead of awaiting indefinitely, while preserving
existing output and exit-code handling.
In `@evals/external/terminal-bench/harbor-adapter.mjs`:
- Around line 77-92: Update evals/external/terminal-bench/harbor-adapter.mjs
lines 77-92 in findLatestJobDir to accept a run-specific baseline such as since
or excludeNames captured before runHarbor, and return null when no new job
directory exists; update lines 104-109 in classifyFailure to use run.code and
classify a non-zero exit without a fresh job directory as infrastructure,
preventing stale results from reaching readTrialResult or verifier
classification.
In `@evals/external/terminal-bench/verifier.mjs`:
- Around line 30-32: Update the numeric parsing in verdictFromReward so the
entire trimmed content must be a valid numeric representation, rejecting
trailing or leading non-numeric text such as “1 of 2” and returning null when
unusable. Preserve the existing finite-value check and reward/metrics result for
valid numbers.
In `@evals/release.mjs`:
- Line 348: Update the runRelease invocation in the release flow so
harnessVersion comes from the version field in packages/harness/package.json
rather than raw.profile. Preserve raw.profile for its existing profile-related
uses and pass the package version into the report and Eval Card metadata.
- Line 190: Update the release evaluation flow around stepFn so each host uses
its own budget from release rather than always using budgets.kimiPair; keep
kimiPairUsd exclusively for the kimi-pair experiment and provide separate
ceilings for codex-subscription and ollama-gemma under release. Apply the same
host-specific selection to the additional stepFn calls noted in the comment.
- Around line 94-101: Update summarizeRun’s matrix lookup to handle verdict
pairs not present in matrix, including unexpected values such as “error,”
without destructuring undefined. Return a classification that preserves the
blocking behavior for missing or invalid telemetry so malformed run documents do
not throw before the schema check.
- Around line 199-208: Update the rerun handling around rerunFn and classifyPair
so a null or otherwise unavailable rerun is treated as unresolved, not as
evidence that the harness regression is flaky. Only set reproduced to false and
relabel classification as flaky-inconclusive after a valid fresh pair is
classified as non-regression; preserve the blocking regression outcome when no
rerun result exists.
- Around line 312-320: Validate the value assigned to budget.releaseCeilingUsd
in the config construction before passing it to createBudget. Reject missing,
non-numeric, or otherwise invalid --budget-usd values instead of allowing NaN,
while preserving the configured raw.budget.releaseCeilingUsd fallback and
ensuring valid numeric ceilings continue to enforce the budget.
- Around line 351-353: Replace the immediate process.exit(exitCode) after report
output with process.exitCode = exitCode so stdout can flush before natural
termination, and update the top-level catch to set process.exitCode = 2 instead
of exiting immediately.
---
Nitpick comments:
In `@evals/external/terminal-bench/agent.mjs`:
- Around line 126-133: Update the API-key selection in the trial setup around
openAiToolDriver so a missing environment key remains missing for hosted
providers, allowing the driver’s null result to trigger a clean skip. Only use
the 'local' placeholder when the selected profile targets a keyless local
endpoint such as Ollama, while preserving explicit configured keys and the
existing driver-not-configured handling.
In `@evals/external/terminal-bench/harbor-adapter.mjs`:
- Around line 60-62: Update buildHarborRunArgs to call the existing
validateTaskLock assertion before constructing or returning the Harbor
arguments, using the provided lock and task context. Ensure invalid or unpinned
lock values fail closed even when verifyTaskAgainstLock was not called by the
caller.
In `@evals/external/terminal-bench/verifier.mjs`:
- Around line 68-77: Update hashTree to accept the already-collected file list
from collectVerifierEvidence instead of walking the directory again, and hash
each file via a streaming read rather than readFileSync so full artifacts are
not loaded into memory. Preserve the existing relative-path, separator, and
SHA-256 digest ordering to keep the resulting tree hash unchanged.
In `@evals/hosts/claude-subscription.mjs`:
- Around line 13-65: Extract the duplicated subscription-host logic from
evals/hosts/claude-subscription.mjs lines 13-65 and
evals/hosts/codex-subscription.mjs lines 15-67 into a shared
createSubscriptionHost factory in a new shared module, including
telemetryTemplate, normalizeHostReport, preserveTranscript, and the telemetry
constants. Update both host modules to call the factory with their respective
id, gate, and runInstructions while preserving their existing host-specific
behavior.
In `@evals/hosts/copilot-smoke.mjs`:
- Around line 8-28: Extract the duplicated CHECKLIST/createHost/evaluate
implementation into a shared createSmokeHost factory. In
evals/hosts/copilot-smoke.mjs lines 8-28, call the factory with the Copilot id
and checklist; in evals/hosts/grok-smoke.mjs lines 7-27, replace the duplicate
host implementation with the same factory using the Grok id and checklist,
preserving each host’s discovery wording.
In `@evals/release.mjs`:
- Around line 151-155: Update the release configuration loading and
gateActiveFor flow to derive each host’s gate behavior from config.pairs,
including gate, enabled, timeoutMs, and calibrationReleases where applicable, so
edits to release-canary.yaml take effect. Preserve the current host-specific
behavior as a fallback when a host has no matching pair, including
after-calibration handling for openrouter-kimi and informational handling for
ollama-gemma.
In `@packages/harness/test/eval-hosts.test.mjs`:
- Around line 78-88: Extend the test around createClaudeHost to also exercise
createCodexHost, preferably by iterating over both hosts as in the existing host
loop. Preserve the transcript persistence assertions for each host, and verify
each host’s runInstructions includes the manual A/B guidance and resolved-model
requirement, including Codex-specific subscription-version wording.
In `@packages/harness/test/eval-tb-agent.test.mjs`:
- Line 53: Simplify the scripted result callback in pump by removing the
always-truthy JSON.parse ternary and directly using line.command for stdout.
Also add coverage for calling input.end() while an exec remains outstanding,
asserting the expected completion behavior and preventing the hang path in
agent.mjs.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2531f8ad-9832-4aa2-918c-1b49f9145ec4
📒 Files selected for processing (25)
.gitignoreevals/README.mdevals/config/release-canary.yamlevals/external/terminal-bench/agent.mjsevals/external/terminal-bench/generic-condition.mjsevals/external/terminal-bench/harbor-adapter.mjsevals/external/terminal-bench/harbor_agent.pyevals/external/terminal-bench/harness-condition.mjsevals/external/terminal-bench/task-lock.jsonevals/external/terminal-bench/verifier.mjsevals/hosts/claude-subscription.mjsevals/hosts/codex-subscription.mjsevals/hosts/copilot-smoke.mjsevals/hosts/grok-smoke.mjsevals/hosts/ollama-gemma.mjsevals/hosts/openrouter-kimi.mjsevals/release.mjsevals/schema/eval-report.v1.schema.jsonevals/schema/eval-run.v1.schema.jsonpackages/harness/test/eval-hosts.test.mjspackages/harness/test/eval-release.test.mjspackages/harness/test/eval-tb-adapter.test.mjspackages/harness/test/eval-tb-agent.test.mjspackages/harness/test/eval-tb-conditions.test.mjspackages/harness/test/eval-tb-verifier.test.mjs
Bridge: a closed input stream settles the loop as protocol_error instead of hanging without a done message. Adapter: job discovery anchors to the current run so stale directories are never graded, and a run with no fresh job directory classifies as infrastructure. Verifier: reward.txt must be wholly numeric. Release runner: malformed verdicts classify safely and block via the schema gate, only the kimi pair draws from the kimi allowance, an unavailable rerun leaves a regression unresolved rather than flaky, --budget-usd is validated, the report carries the real harness version, and exit uses process.exitCode so piped JSON is never truncated. Python bridge: setup failures raise instead of running a contaminated treatment, and per-command timeouts are honored via asyncio.wait_for.
Harbor args now use the real 0.20.0 flags (-i/--include-task-name; --n-attempts for attempts; -n is concurrency) and pin job identity with --job-name/--jobs-dir, with jobDirFor replacing newest-directory guessing. The agent module is an importable package (evals.external.terminal_bench, verified against the real harbor BaseAgent) and context population is Pydantic-safe via metadata. The task lock commits the checksum of the pinned terminal-bench@2.0 download. Paid budgeted drivers now stop immediately on unusable usage; the kimi host auto-creates the profile trial ceiling when no budget is passed. Verifier evidence is restricted to the official logs/verifier directory, and a nonzero harbor exit is classified before any reward is trusted. Adds provider-free contract smokes (python import, stamped lock, harbor run --help flags when the CLI is installed).
The release CLI now has two explicit modes: --deterministic-only (free per-PR path) and release-candidate (default), where the live Kimi A/B pair is required and wired end to end: task bytes verified against the committed lock before any provider call, conditions written with trial ceilings capped by the pair's remaining allowance, harbor invoked with deterministic job identity and a mounted harness bundle (Node runtime + harness package, activated only in the treatment, fail-closed), verifier evidence and bridge telemetry assembled into schema-valid eval-run documents, and provider-reported cost charged to the chained release ledger. Gates now block skipped or invalid required pairs, failed smokes, and all-null metered telemetry; provider/verifier failures are infrastructure-invalid, never capability results. Driver prechecks use observed usage as a floor; zero-priced local profiles are exempt from paid fail-closed metering. Verifier falls back past ambiguous reward.json. Python bridge raises when the node side dies without a done message and reports cached tokens as n_cache_tokens. End-to-end CLI tests run the whole pipeline against a fake harbor on PATH, including the credential-less blocking path.
Mounts use Docker Compose service-volume format (type/source/target/ read_only) as harbor's ServiceVolumeConfig requires; harbor spawns get PYTHONPATH with the repo root since --agent resolves via plain importlib; the Python bridge passes exec's native timeout_sec (wait_for fallback for other versions); HARNESS_EVAL_TB_ENV overrides the configured sandbox environment for local Docker debugging runs.
…ier layout, arch-aware bundle Verified against live harbor 0.20.0 + Docker + OpenRouter runs: --ae agent env arrives via BaseAgent extra_env (not os.environ) and is forwarded to the Node bridge; the done payload is persisted before the stdout done line so harbor's immediate terminate() cannot truncate it; verifier evidence accepts harbor's host-side trial layout (trial-root verifier/ dir) while still rejecting anything nested in agent artifacts; activation uses a real command (harness help) through a PATH-proof /usr/bin link; the bundle carries node runtimes per architecture selected by uname -m (the pinned task image is amd64-only even on arm64 hosts); npm install runs --ignore-scripts inside the bundle copy; setup failures report both output streams.
A trial whose verifier already passed is definitive evidence even when a provider error (e.g. exhausted credits during post-verification review) ends the agent loop afterwards; an interrupted fail remains a provider failure. Observed live: the harness arm solved and verified the task, then hit OpenRouter 402 — the pair is parity, not infrastructure-invalid.
…s, and gates task-lock.json becomes schema 2 with a stamped task list (tasksOf normalizes legacy single-task locks); stampTaskLock appends candidates or re-stamps entries by name; taskLock preflight verifies every pinned task's bytes; live steps run one fresh generic+harness pair per task with task-scoped job identities and artifact files; runRelease classifies, reruns, and gates each task independently with task-labeled reasons; the dataset location env is HARNESS_EVAL_TB_DATASET_DIR.
Seeds per condition per task (3 for calibration, 1 routine, config- driven): majority verdict over all attempted seeds (a null-reward seed never counts toward a pass), median reward and efficiency over valid seeds, pair validity by seed majority, seed-multiplied trials all charging the pair budget, and §9 reruns pinned to a single fresh pair. The lock now pins cancel-async-tasks, git-leak-recovery, and custom-memory-heap-crash alongside the cobol-modernization anchor, checksums stamped from the verified terminal-bench@2.0 download.
A 429 is a confirmed-unbilled transient failure — the §10 retry policy permits retrying it. The driver now retries up to transientRetries times within one request, honoring Retry-After and otherwise backing off exponentially (capped 30s), recording each retry in telemetry. Terminal statuses (402, 5xx) never retry. Observed live: 17 of 23 calibration trials died on burst 429s from the pinned endpoint. Adapter and CLI fixtures updated to harbor's real trial-root verifier layout.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements the Engineer Harness release-evaluation plan end to end: a model-resilient A/B canary on the pinned Terminal-Bench task (
terminal-bench@2.0/cobol-modernization) with code-enforced budgets, structured telemetry, Harbor-based execution, host adapters, and release orchestration with gates and eval-card reporting. Four commits, one per workstream; everything is deterministic and provider-free in CI, with live infrastructure (Harbor CLI, sandboxes, provider keys) behind injected boundaries exercised at release time.1. Driver telemetry, budgets, and model profiles (
evals/lib/)model-profiles.mjs— deep-frozen profiles: Kimi K2.7 Code via OpenRouter (pinned provider,allow_fallbacks: false, cached/uncached pricing, $5 trial ceiling, 15-min timeout) and Gemma 4 26B local via Ollama (zero pricing, 30-min timeout).budget.mjs— cost from provider-reported usage only (malformed usage →null, never a silent estimate), worst-case pre-request estimation including max output tokens, chained trial→release ceilings that refuse before sending and recordbudget_exhausted.telemetry.mjs— sequenced structured transcript events plus token/cost accumulation with missing-usage tracking (costComplete).drivers.mjs— the live OpenAI-compatible driver gains profiles, model-default temperature, reasoning config, provider pinning on the wire, fallback detection (model and provider, normalized names), usage/cost/generation-id capture, reasoning-metadata round-tripping, per-request budget prechecks, configurable tool-result truncation with recorded events, explicit stop reasons, andProviderErrorclassifying network vs http vs billable failures. Legacy construction unchanged.2. Terminal-Bench adapter (
evals/external/terminal_bench/)task-lock.json+harbor-adapter.mjs— fail-closed pinning with a committed checksum of the pinnedterminal-bench@2.0download (harbor 0.20.0, verified locally), tamper detection,harbor runargv using flags verified against the harbor 0.20.0 CLI source (-i/--include-task-name,--n-attempts,--n-concurrent,--job-name/--jobs-dirfor deterministic job identity), injected-spawn execution with timeout, and failure classification: infrastructure (including any nonzero harbor exit — a reward read out of a failed invocation is not evidence) vs provider vs verifier vs a validly graded trial (reward 0 is a fail, not a failure).verifier.mjs— official verifier artifacts trusted only from the trial'slogs/verifierdirectory (a reward-named file dropped anywhere else in the tree cannot spoof the verdict), strict whole-content numeric parsing, pytest assertion counts, sha256 artifact-tree fingerprint. A missing reward staysnull.generic-condition.mjs/harness-condition.mjs— the A/B arms: byte-identical instruction and limits; the neutral baseline is fair (encourages exploring/testing/verifying) with zero harness vocabulary, enforced by test; the treatment layers the engineer contract, skill guidance, and activation commands.agent.mjs+harbor_agent.py— a stdio JSON-lines bridge: all decisions (driver, budget, telemetry) stay in tested Node; the thin HarborBaseAgentwrapper pumps exec/result lines in the sandbox. Explicit stop reasons on every exit path, EOF-safe (a dying Python side settles asprotocol_error, never a hang). The module is an importable Python package (evals.external.terminal_bench.harbor_agent:StdioBridgeAgent, verified as a realBaseAgentsubclass under harbor 0.20.0); context population is Pydantic-safe (bridge payload inmetadata, token/cost fields attempted individually); setup failures raise so a treatment arm can never silently run without the Harness.3. Model and host adapters (
evals/hosts/)Controlled Kimi host with fail-closed credentials (no key → no driver → no spend); local Gemma host with the
host.docker.internalendpoint rewrite; Codex/Claude subscription A/B contracts (fresh sandboxes, record the exact resolved model, transcripts preserved, unavailable telemetry recorded asnull— never estimated, never mixed numerically with the API result); Copilot/Grok compatibility smoke checklists with fail-closed evaluation.4. Live release path (
evals/external/terminal_bench/live-steps.mjs,provision.mjs)node evals/release.mjs(release-candidate mode, default) runs the required live Kimi A/B end to end with no external glue: task bytes verified against the committed lock before any provider call; per-condition condition files with trial ceilings capped by the pair's remaining allowance;harbor runwith deterministic job identity and the harness bundle mounted read-only into both conditions (a host-prepared bundle of a Linux Node runtime + the harness package at the evaluated SHA — the pinned COBOL image has neither — activated only in the treatment, fail-closed); verifier evidence + bridge telemetry assembled into schema-valideval-run.v1documents; provider-reported cost charged to the chained release ledger across processes.--deterministic-onlyis the free per-PR mode. A release candidate missing harbor, credentials, task verification, or run evidence blocks — it never greens.5. Release orchestration and reporting (
evals/release.mjs,evals/config/,evals/schema/)eval-run.v1/eval-report.v1schema contracts with a dependency-free validator; every run document is validated and the report validates itself before printing.release-canary.yamlprofile, markdown Eval Card + JSON report, README runbook (configuration, interpretation, costs, troubleshooting).Review hardening
Four review rounds are folded in (commits
84cfb9a,089b500,dd1fe28,40798bb). Round 4 closed the fail-open gate paths: skipped or infrastructure-invalid required pairs block, failed smokes block, all-null metered telemetry on an API pair blocks, provider/verifier failures classify as infrastructure-invalid (never "model capability"), and the driver's precheck uses observed usage as a floor on top of the character heuristic. Earlier rounds: pinned-endpoint pricing ($0.95/$0.19/$4.00 per M for the pinned Moonshot AI endpoint), budgets that refuse to construct without a valid ceiling and stop immediately when a paid response's usage is unusable (spend that cannot be metered is never continued), per-request abort timeouts, fallback state cleared on reset, per-pair budget scoping, unresolved-rerun semantics,--budget-usdvalidation, and the kimi host auto-creating its profile trial ceiling so an unbudgeted paid driver cannot exist.Verification
Strict TDD throughout — every module's tests were watched failing before implementation.
harbor run --help)--deterministic-onlyexits 0; release-candidate mode with no harbor/credentials exits 1 (fail closed)harbor download terminal-bench@2.0(checksum committed in the lock), Python import of the exact--agentreference under the real harbor packagenode evals/release.mjs --jsonruns end to end on a clean checkout: deterministic suite executes, paid pairs report as safely skipped, the report validates againsteval-report.v1, exit 0Summary by CodeRabbit
New Features
Documentation
Tests