feat(benchmarks): EvalManifestV1 — unified eval description schema + first-party manifests - #3
feat(benchmarks): EvalManifestV1 — unified eval description schema + first-party manifests#3LukasParke wants to merge 1 commit into
Conversation
…first-party manifests Ports openrouter-web#31084 + #31085 (all review rounds): - EvalManifestV1: one document schema describing any eval — dataset binding (inline | hf | builtin), solver/scorer bindings (spec | builtin | capsule), model roles with override policy, epochs/reducer, sampling, provenance. Capsule images must be digest-pinned (lowercase sha256 — registries reject uppercase, and case variants would fork the identity digest). - manifestIdentityDigest: structural hash over score-changing fields; the comparability-series key. Model roles are sorted before hashing. - validateManifest: cross-field rules Zod can't express (candidate role required, judge role for spec-judge scorers, capsule/tier gating). - FIRST_PARTY_MANIFESTS: gpqa_diamond + tau3_bench_banking described in the schema, with drift-guard tests pinning them to the registered benchmarks' actual datasets/options — including gpqa's HF revision pin (enforced at run time by the datasets layer; fails closed if upstream moves).
There was a problem hiding this comment.
"## Perry's Review\n\nVerdict: 💬 Comments / questions\n\n> Note: The maintainer app is not installed on OpenRouterTeam, so I can't post an APPROVE. The review is clean — please approve manually once CI is green.\n\nRisk: 🟢 Low\n\n
PR #3 — EvalManifestV1 schema + first-party manifests
\n\nWell-designed unified manifest schema. The identity digest, cross-field validation, and first-party manifest drift guards are all solid. This is a clean foundation for the manifest architecture.\n\nKey observations:\n\n- ThemanifestIdentityDigest correctly excludes display/metadata fields (name, reports, provenance) and includes only comparability-bearing fields. The model-role sorting ensures declaration order can't split a series.\n- The validateManifest cross-field rules are comprehensive: candidate-role requirement, unique roles, judge-role for spec-judge, pass@k coherence, capsule-needs-rootless. All properly tested.\n- The PINNED_IMAGE_RE regex correctly enforces lowercase-only digest hex, matching registry conventions.\n- The first-party manifests (GPQA, tau3-banking) are well-documented with comparability notes explaining deliberate divergences from reference protocols.\n- The drift guards in manifests.test.ts are the right safety net for the M2 phase: manifests and meta/registry constants must agree until manifests become the source of truth.\n\nOne inline question below.\n| //#region Identity | ||
|
|
||
| function stableJson(value: unknown): string { | ||
| if (value === null || typeof value !== "object") { |
There was a problem hiding this comment.
The stableJson function falls through to JSON.stringify(value) for primitives, but JSON.stringify(undefined) returns undefined (not a string), which would produce key:undefined in the output. In practice, the identity object normalizes optionals with ?? null, but nested optional fields on solver/scorer/sampling objects could still carry undefined. While the digest remains deterministic (same input always produces the same string), undefined and null would produce different digests for semantically equivalent manifests. Should stableJson filter out undefined values (or coerce them to null) to produce canonical JSON?
▶ Prompt for agents: If this is intentional (Zod parsing normalizes all optionals), add a brief comment noting that Zod-validated inputs never carry undefined so the undefined path is unreachable in practice. If not, add a guard in stableJson to skip or coerce undefined values.
Stacked on #2. Ports openrouter-web#31084 + #31085 (all review rounds).
What
src/benchmarks/eval-manifest.ts): one document schema describing any eval — dataset binding (inline|hf|builtin), solver/scorer bindings (spec|builtin|capsule), model roles with override policy, epochs + reducer, sampling, provenance. Capsule images must be digest-pinned (lowercase sha256 — registries reject uppercase, and case variants would fork the identity digest).manifestIdentityDigest: structural hash over score-changing fields — the comparability-series key. Model roles are sorted before hashing so declaration order can't fork identity; undefined-valued fields canonicalize with absent ones.validateManifest: cross-field rules Zod can't express (candidate role required; judge role required for spec-judge scorers; capsule bindings gated by isolation tier).FIRST_PARTY_MANIFESTS(src/benchmarks/manifests.ts):gpqa_diamondandtau3_bench_bankingdescribed in the schema, with drift-guard tests pinning every documented field to the registered benchmark's actual dataset/options — including gpqa's HF revision pin (now enforced at run time by the datasets layer; upstream moving the dataset fails the run closed with the observed SHA; re-pin, never un-pin).Downstream,
FIRST_PARTY_MANIFESTS+ the digest are what gate builtin-solver resolution in the monorepo dispatcher (a user manifest self-declaring a first-party id must digest-match or be rejected — security finding from the review).Testing
Manifest schema round-trips, digest canonicalization, validateManifest rules, drift guards; full suite green.