Add BenchPress score-matrix adapter - #197
Conversation
Aggregator adapter for microsoft/benchpress-score-matrix, modeled on llm_stats:
source_type=documentation, source_role=aggregator, output logs split by
evaluator_relationship (derived per score from BenchPress source_type). Reads the
public CSV mirror from HuggingFace (--input-json for offline replay).
- Follows BenchPress's documented update manifest (metadata.json): generated_at_utc
-> retrieved_timestamp, and source_git_commit/generated_at_utc recorded on every
record for snapshot/update tracking.
- Metric bounds are the metric's true bounds with +/-inf where unbounded; written
as the JSON Infinity token (EEE's json.loads + pydantic loader reads float('inf');
model_dump_json would null it, so write_log serializes the record itself).
- Per-score citation -> source_data.url; per-score harness/settings -> additional_details.
Adds utils/benchpress/README.md, a utils/README.md row, and
tests/test_benchpress_adapter.py (8 tests). Verified: live fetch -> 366 logs, all
pass 'every_eval_ever validate'; unit tests pass.
|
GPT 5.6 Review (3m 49s) FindingsHigh — Do not identify tau-bench
|
mrshu
left a comment
There was a problem hiding this comment.
⚒️ review-anvil report
Review decision: COMMENT — The adapter is well documented and tested, and four data-selection or portability details would benefit from follow-up.
Result: One high-priority audit-status issue and three medium-priority provenance or serialization issues remain.
Scope: This PR adds a BenchPress score-matrix aggregator with citations, bounds, and snapshot metadata.
Checks: 3 concerns checked; 0 broad claims kept, 3 narrowed, 0 ruled out, 0 set aside.
Second check: targeted, 2 reviewers; 4 kept, 4 fix paths clarified, 0 set aside, 0 removed.
Earlier review comments
None.
What I noticed
- [high] data selection
utils/benchpress/adapter.py:432— The adapter exports every numeric row fromscores_all, including rows outside BenchPress's accepted audit statuses. BenchPress currently reduces 4,903 audit rows to 4,493 accepted rows before canonical processing. (RAVF001; inline) - [medium] evaluator relationship
utils/benchpress/adapter.py:69—tech_reportalways maps tofirst_party, although document type does not establish who evaluated the model. Cross-provider report rows can receive incorrect relationship provenance. (RAVF002; inline) - [medium] JSON portability
utils/benchpress/adapter.py:494— Logs with inferred unbounded metrics contain bareInfinitytokens. Python accepts them, but standards-compliant JSON parsers reject the entire file. (RAVF003; inline) - [medium] snapshot provenance
utils/benchpress/adapter.py:223— Metadata and three CSVs are fetched independently from mutablemain. Metadata errors are ignored, so one run can mix revisions or lose its version anchor while still succeeding. (RAVF004; inline)
Things to try
Things to try (4 items)
- [high] data selection — Keeping
scores_allwhile admitting onlyverifiedandverified_third_partyrows would fix the narrow issue without changing canonical-selection scope. (RAVF001) - [medium] evaluator relationship — Explicit report ownership can be compared with model provider. When ownership is absent,
otheravoids guessing from an arXiv or publisher host. (RAVF002) - [medium] JSON portability — A shared schema representation for unbounded values can keep the normal writer and strict JSON compatibility. Until then, unsupported unbounded results can be counted and skipped. (
RAVF003) - [medium] snapshot provenance — One resolved Hugging Face dataset revision can pin all four live reads. Its revision can stay separate from the manifest's upstream source commit. (
RAVF004)
Run details
- Target: PR #197 (
add-benchpress-adapter, 5 files, +744/-0) - Rounds: 1/1 completed; adaptive off; one reviewer returned empty output
- Mix: 3 codex-exec; 2 succeeded and 1 failed the empty-output check
- Focus: metric semantics, provenance, serialization, maintainability, and constructive suggestion-oriented language
- Earlier review comments: none
- Finding counts: 0 critical, 1 high, 3 medium, 0 low, 0 nit
- Checks: concerns=3; narrowed=3/ruled-out=0/set-aside=0
- Second check: targeted; reviewers=2; kept=4/clarified=4/set-aside=0/removed=0; approval changed no
- Set aside: 0 items
Reviewed with review-anvil.
|
|
||
| groups: dict[tuple[str, str, str], list[EvaluationResult]] = defaultdict(list) | ||
| model_infos: dict[tuple[str, str, str], ModelInfo] = {} | ||
| for score in payload['scores']: |
There was a problem hiding this comment.
[high] data selection — Rows outside BenchPress's accepted audit statuses become normal results
The adapter reads scores_all and accepts every linked numeric row. BenchPress applies a verified or verified_third_party status filter before its canonical pipeline; preserving audit_status only as metadata does not enforce that boundary.
Keeping scores_all while applying the accepted-status allowlist would fix this narrow issue without changing representative or fill semantics.
| # blank/unknown -> other. | ||
| RELATIONSHIP_BY_SOURCE_TYPE = { | ||
| 'official_blog': 'first_party', | ||
| 'tech_report': 'first_party', |
There was a problem hiding this comment.
[medium] evaluator relationship — Document type alone does not prove first-party evaluation
Every tech_report row maps to first_party, but BenchPress permits a report from one provider to contain scores for another provider's model. The relationship field describes the evaluator's relationship to the model.
Explicit report ownership can be compared with the model provider. When that signal is absent, other avoids guessing from a publisher or arXiv URL.
| filepath = dir_path / f'{uuid.uuid4()}.json' | ||
| data = log.model_dump(mode='python', exclude_none=True) | ||
| filepath.write_text( | ||
| json.dumps(data, indent=2, allow_nan=True, default=_json_default), |
There was a problem hiding this comment.
[medium] JSON portability — Unbounded metrics produce non-standard Infinity tokens
allow_nan=True writes bare Infinity or -Infinity. Python's loader accepts these tokens, but RFC-compliant JSON parsers reject affected files.
A shared schema representation for unbounded values can preserve the meaning and the standard writer. An adapter-local wire convention would create a second serialization policy.
| def fetch_payload() -> dict[str, Any]: | ||
| """Fetch the live BenchPress CSV mirror + metadata.json from HuggingFace.""" | ||
| try: | ||
| metadata = fetch_json(f'{HF_BASE}/metadata.json') |
There was a problem hiding this comment.
[medium] snapshot provenance — One export can combine files from different revisions
Metadata and the three CSVs are separate reads from mutable main. Metadata errors become an empty object, after which the adapter uses the current time and omits its promised version anchor.
Resolving one Hugging Face dataset revision can pin every live read. The dataset revision and manifest source_git_commit can remain separate provenance fields.
main removed utils/ and moved every adapter to every_eval_ever/adapters/<name>/. The benchpress adapter moves with them, and utils/README.md is deleted rather than edited; its adapter entry now lives in every_eval_ever/adapters/README.md.
Export only the rows BenchPress accepts (audit_status verified / verified_third_party); the dropped, needs_review and flagged rows are outside its own canonical matrix and are now reported as exclusions. --include-unaccepted exports them. Derive evaluator_relationship from citation breadth, not source_type alone. A provider-authored document routinely carries a comparison table of competitors and BenchPress scrapes those cells too, so a citation supplying scores for several providers is `other` instead of first_party. 2,519 of the 3,476 rows previously called first_party came from such a document. Publish through the shared save_evaluation_logs instead of a local writer that emitted bare `Infinity`, which the validator now rejects. Batch publication also means a late failure leaves no partial tree. Read all four export files at one resolved dataset commit, recorded as benchpress_dataset_revision; --revision replays a snapshot. Reject a score that falls outside the range its own benchmark declares: the export mixes scales inside a benchmark (mt_bench_101 declares 1-10 and carries values up to 90.2), so the record cannot state both and rescaling would be a guess. Report failures and exclusions through the shared SourceConversionResult, so unconvertible rows land in adapter_reports/ and the command exits non-zero.
|
Thanks — all four are fixed, and running the conversion against the live snapshot RAVF001 (high, audit status) — fixed as suggested. One thing that came out of checking this, in case it's useful elsewhere: RAVF002 (relationship) — fixed, and it was broader than the finding. You scoped
2,519 of 3,476. The concrete case: Claude scores cited to Your "when ownership is absent, That's a material change in what the same input produces, so I've moved the PR to RAVF003 (bare RAVF004 (snapshot provenance) — fixed as suggested. The dataset commit sha is Fifth issue, not in the review: the export mixes score scales inside one
A record can't state both, and rescaling would be a guess about which cells are on Verification. 4,903 source scores → 332 logs / 4,475 scores; 410 excluded, 18 |
mrshu
left a comment
There was a problem hiding this comment.
⚒️ review-anvil report
Review decision: COMMENT — This pass is comment-only, and one high-priority row-accounting gap remains.
Result: The adapter now handles the earlier selection, JSON, and default snapshot concerns well. Four focused suggestions remain.
Scope: Add a BenchPress score-matrix adapter with snapshot pinning, provenance policy, range checks, and complete source-row accounting.
Checks: 3 concerns checked; all 3 confirmed.
Second check: 2 reviewers checked 4 findings; all 4 stayed, and 3 fix paths were clarified.
Earlier review comments
- Fixed: accepted audit-status selection and strict JSON publication now follow the intended source and shared writer.
- Fixed: the default fetch resolves one immutable dataset SHA before reading all four files.
- Still present: the earlier evaluator-relationship concern remains because citation breadth does not identify document ownership.
What I noticed
| ID | Priority | Topic | Code location | What I noticed |
|---|---|---|---|---|
| RAV-RUN2-R1-F001 | high | failure accounting | every_eval_ever/adapters/benchpress/adapter.py:214 |
One malformed score or row-level conversion error can stop the full run before that source row is recorded. (inline) |
| RAV-RUN2-R1-F002 | medium | snapshot provenance | every_eval_ever/adapters/benchpress/adapter.py:246 |
The default path is pinned, but an explicit symbolic --revision can still mix snapshots and records the symbol instead of a SHA. (inline) |
| RAV-RUN2-R1-F003 | medium | evaluator relationship | every_eval_ever/adapters/benchpress/adapter.py:299 |
A one-provider citation does not prove that the document owner matches the scored model provider. |
| RAV-RUN2-R1-F004 | medium | exclusion accounting | every_eval_ever/adapters/benchpress/adapter.py:641 |
An exclusions-only run prints a count but does not preserve each excluded row and reason in the report. (inline) |
Things to try
Suggested next steps (4 items)
- [high] failure accounting — Expected parsing and schema errors can stay inside a per-row boundary. Unexpected program errors can remain visible. (
RAV-RUN2-R1-P001; coversRAV-RUN2-R1-F001) - [medium] snapshot provenance — Every supplied reference can resolve once to a SHA, preserving current branch and tag support. (
RAV-RUN2-R1-P002; coversRAV-RUN2-R1-F002) - [medium] evaluator relationship — Ownership-unknown documents can use
otheruntil the source exposes comparable publisher evidence. (RAV-RUN2-R1-P003; coversRAV-RUN2-R1-F003) - [medium] exclusion accounting — The shared report can be saved for failures or exclusions. The exit remains nonzero only for failures. (
RAV-RUN2-R1-P004; coversRAV-RUN2-R1-F004)
Run details
- Target: PR #197 (
add-benchpress-adapter, 5 files, +942/-0) at3d624b536f87ffd74d3f135bf43c7044df673cdc - Run ordinal: 2
- Rounds: 1/1 completed; adaptive off in review-only mode
- Mix: 3 codex-exec; 2 completed, 1 failed after DNS transport retries
- Focus: correctness, maintainability, simplicity, production blast radius, and constructive suggestions
- Earlier review comments: 4 grouped concerns; 3 fixed and 1 still present
- Finding counts: 0 critical, 1 high, 3 medium, 0 low, 0 nit
- Checks: concerns=3; confirmed=3
- Second check: targeted; reviewers=2; kept=4, clarified=3, removed=0; approval unchanged
Reviewed with review-anvil.
| def _parse_scores(rows: list[dict]) -> list[dict]: | ||
| return [{ | ||
| 'model_id': r['model_id'], 'benchmark_id': r['benchmark_id'], | ||
| 'score': _to_float(r.get('score')), | ||
| 'reference_url': _clean(r.get('reference_url')), | ||
| 'source_type': _clean(r.get('source_type')), | ||
| 'audit_status': _clean(r.get('audit_status')), | ||
| 'matches_canonical': _clean(r.get('matches_canonical')), | ||
| 'reported_setting': _json_obj(r.get('reported_setting_json')), | ||
| 'notes': _clean(r.get('notes')), | ||
| 'n_candidates': _clean(r.get('n_candidates')), | ||
| } for r in rows] |
There was a problem hiding this comment.
RAV-RUN2-R1-F001 [high] failure-accounting — Score parsing happens before SourceConversionResult exists, and later row construction has no row-level error boundary. One malformed source value can stop every valid row without a source-row failure record.
A narrow boundary can catch expected parsing and schema errors. Unexpected program or infrastructure errors can remain visible.
| revision = revision or resolve_revision() | ||
| base = f'https://huggingface.co/datasets/{HF_REPO}/resolve/{revision}' | ||
| metadata = fetch_json(f'{base}/metadata.json') | ||
| return { | ||
| 'models': _parse_models(fetch_csv(f'{base}/data/models.csv')), | ||
| 'benchmarks': _parse_benchmarks(fetch_csv(f'{base}/data/benchmarks.csv')), | ||
| 'scores': _parse_scores(fetch_csv(f'{base}/data/scores_all.csv')), | ||
| 'metadata': {**metadata, 'dataset_revision': revision}, |
There was a problem hiding this comment.
RAV-RUN2-R1-F002 [medium] snapshot-provenance — The default fetch resolves one SHA, but a supplied symbolic revision bypasses that step. A moving branch can resolve differently across four requests, while metadata records only the symbol.
Resolving every supplied reference once would keep branch and tag support while making all file URLs reproducible.
| print(path) | ||
| print(f'{result.total_records} source scores -> {len(paths)} log(s); ' | ||
| f'{len(result.exclusions)} excluded, {len(result.failures)} failed') | ||
| if result.failures: |
There was a problem hiding this comment.
RAV-RUN2-R1-F004 [medium] exclusion-accounting — The report is saved only when result.failures is nonempty. An exclusions-only run succeeds but loses each excluded source reference and reason.
The shared report already supports exclusions. It can be saved for either collection while the nonzero exit stays conditional on failures.
…e domain Score values are no longer parsed while the CSV is read, where one unusable cell raised before SourceConversionResult existed and ended the run without recording the row it came from. The score stays as written and is parsed inside a per-row boundary in make_logs, so a value that will not parse or a field the schema rejects is reported against its own row while the rest publish. The boundary catches only what bad source data raises; anything else still surfaces. The descriptive numerics (params, num_problems) only ever reach a record as text, so they keep the text rather than failing. A second, differing result for a benchmark a log already carries was dropped silently by a de-dup pass. evaluation_result_id is the join key for instance records, so one log still carries one result per benchmark, but the row that cannot be represented is now reported instead of vanishing. An identical repeat carries nothing new and still collapses. No live row hits either path. resolve_revision now takes a reference, so a branch or tag is resolved to a commit once instead of being spliced into four file URLs and recorded as provenance that can move. A full SHA is already immutable and costs no request. evaluator_relationship takes first_party from publisher evidence rather than document type. source_type says what kind of document a citation is, not who published it, and the one-provider citation heuristic conflated the two: on the live snapshot it labelled a Google gemini-2.5-flash score first_party from Qwen's model card, while demoting OpenAI's own scores on openai.com whenever the page also tabulated competitors. first_party now takes a provider-authored type and a citation on a domain carrying the model provider's name, because a domain has an owner and a path on a shared host does not. 841 cells across 49 logs qualify, from 12 hosts that are all the provider's own; the rest are `other`. No score value changed. The conversion report is written on every run rather than only when a row failed, so an exclusions-only or clean run no longer leaves an earlier run's report to be read as its own, and the swap is atomic. The non-zero exit stays conditional on failures.
cd95054 to
c8b0f7c
Compare
|
All four findings are fixed in Re-verified live on this head, by a fresh fetch and an RAV-RUN2-R1-F001 · failure accounting — one bad cell ended the runCorrect, and it was the failure mode the accounting exists to prevent. The score now stays as the source wrote it and is parsed inside Two details are judgement calls rather than mechanics:
Four tests: a junk score and a RAV-RUN2-R1-F002 · snapshot provenance — a symbolic
|
| cells | host | developer |
|---|---|---|
| 476 | openai.com |
openai |
| 120 | www.anthropic.com |
anthropic |
| 99 | deepmind.google |
|
| 28 | cohere.com |
cohere |
| 26 | www-cdn.anthropic.com |
anthropic |
| 25 | ai.meta.com |
meta |
| 17 | cdn.amazon.science |
amazon |
| 15 | mistral.ai |
mistral |
| 15 | moonshotai.github.io |
moonshot-ai |
| 15 | x.ai |
xai |
| 4 | anthropic.com |
anthropic |
| 1 | cdn.openai.com |
openai |
Full distribution {other: 2,654, third_party: 980, first_party: 841} across
{third_party: 139, other: 138, first_party: 49} logs, from
{other: 2,550, third_party: 980, first_party: 945} / {third_party: 139, other: 119, first_party: 74} before. 2,614 published cells carry a provider-authored type whose
publisher the export does not identify, and all of them are other. No score value changes and
no cell is dropped.
Three tests: the split, a unit test on the host rule (including the two shared-host
negatives), and one asserting a provider-authored type on a shared host is not a
first_party claim.
Fifth defect, found while fixing F003 — a silent de-dup
Not in your ledger, and the same shape as F001/F004: a de-dup pass collapsed a log's
results by evaluation_result_id, keeping whichever row arrived first and dropping the
other with no entry anywhere — not a failure, not an exclusion. The report would
still reconcile against total_source_records, because the loss happened after
counting.
evaluation_result_id is the schema's "recommended deterministic join key", so one log
genuinely cannot carry two results for one benchmark. Now: an identical repeat still
collapses (it carries nothing new), and a second differing result is reported as a
failure naming the citation already in the log. Giving the id a citation suffix was the
alternative, but it would change the join key on all 4,475 cells to fix a case that
does not occur — or make identity depend on whether a sibling row exists, which is the
mistake #203/#204 were pulled up on. Zero live rows hit either path
(4,903 − 410 − 18 = 4,475 exactly), so this closes a hole rather than repairing
observed loss. test_a_second_result_for_one_benchmark_is_reported_not_dropped pins
both halves.
RAV-RUN2-R1-F004 · exclusion accounting — one fix, one deliberate divergence
Both defects are real and fixed: the write was conditional on result.failures, so an
exclusions-only run printed a count and preserved nothing, and a clean run left the
previous run's report standing to be read as its own. write_conversion_report now
writes on every run and swaps the file in with os.replace, so an interrupted write
cannot leave a truncated report where a complete one was. The exit stays non-zero for
failures only, as P004 specifies — an excluded row is BenchPress's own audit decision.
The divergence is when: the report is written before publication, not after. It
accounts for the conversion, so a publication error is precisely when it is most worth
having on disk. test_the_report_survives_a_publication_error makes export_logs
raise and asserts the failures are on disk anyway. Two more tests cover the defects you
named: an exclusions-only run exits 0 and itemizes all seven excluded rows with
reasons, and a seeded stale report is replaced with no partial file left beside it.
Happy to move the write after publication if you would rather have it gated.
This is the same shape as the fix in #204, and deliberately not factored out: a shared
helpers/io.py helper would couple the two PRs into a merge order and needs
cross-package agreement per AGENTS.md. It stays adapter-local in both, and I will
propose the shared home as one follow-up covering both once these land.
One housekeeping note: the "GPT 5.6 Review" comment above is about tau-bench —
utils/tau_bench/adapter.py:389, Pass^k labelled pass_at_k — and does not apply
here. This PR touches only the five adapters/benchpress/ + test files and never that
path. It looks like it was meant for #192, so the high finding on it should not be read
as outstanding on this branch.
The provenance rule, the snapshot pinning and the report-write rationale were argued twice, in the README and in the PR description. Keep the rule a record consumer needs; drop the argument for it.
Keep the invariant an editor could break as a comment on the rename.
mrshu
left a comment
There was a problem hiding this comment.
⚒️ review-anvil report
Review decision: COMMENT — The earlier findings are addressed, and seven focused suggestions can strengthen the new adapter.
Result: Two high-priority and five medium-priority findings remain.
Scope: Add a BenchPress score-matrix adapter with stable identity, correct metadata, complete row accounting, and safe publication.
Checks: 7 concerns checked and confirmed.
Second check: 2 reviewers kept all 7 findings and clarified 5 fix paths.
Earlier review comments
Earlier review comments (23 ledger entries)
- Audit-status filtering, expected row failures, immutable revision reads, evaluator ownership, strict JSON publication, and exclusion-report persistence are fixed.
- Old
utils/benchpressanchors are no longer relevant after the adapter move. - The findings below are distinct residual cases on the current head.
What I noticed
| ID | Priority | Topic | Code location | What I noticed |
|---|---|---|---|---|
| RAV-RUN3-R1-F001 | high | data selection | every_eval_ever/adapters/benchpress/adapter.py:557 |
A missing audit_status column turns every row into a normal exclusion. The run can publish no logs and still exit successfully. |
| RAV-RUN3-R1-F002 | high | stable identity | every_eval_ever/adapters/benchpress/adapter.py:628 |
evaluation_id omits the immutable dataset revision. A replay can use current time, while a lagging manifest time can collide across revisions. |
| RAV-RUN3-R1-F003 | medium | model metadata | every_eval_ever/adapters/benchpress/adapter.py:352 |
BenchPress supplies open_weights, but the typed model_availability field remains unknown. |
| RAV-RUN3-R1-F004 | medium | metric semantics | every_eval_ever/adapters/benchpress/adapter.py:470 |
Every metric receives a benchmark-specific .score ID. Known global metrics such as percent-correct results cannot join on canonical identity and scale. |
| RAV-RUN3-R1-F005 | medium | exclusion accounting | every_eval_ever/adapters/benchpress/adapter.py:559 |
Exclusions omit the source row. Repeated model and benchmark references cannot be distinguished in the report. |
| RAV-RUN3-R1-F006 | medium | evaluation metadata | every_eval_ever/adapters/benchpress/adapter.py:631 |
The log names BenchPress as the evaluation library although BenchPress aggregates results from varied harnesses. |
| RAV-RUN3-R1-F007 | medium | duplicate accounting | every_eval_ever/adapters/benchpress/adapter.py:603 |
Equal duplicate rows collapse to one result without a failure or exclusion entry. The source-row ledger cannot reconcile the duplicate. |
Things to try
Suggested changes (7 items)
- [high] data selection — Missing or blank status values can become row failures before policy filtering. A fresh report and non-zero exit can preserve the incompatible export evidence. (
RAV-RUN3-R1-P001; coversRAV-RUN3-R1-F001) - [high] stable identity — The raw model, relationship, and required immutable dataset revision can define the ID. Generated and retrieved times can stay as metadata. (
RAV-RUN3-R1-P002; coversRAV-RUN3-R1-F002) - [medium] model metadata — Recognized source booleans can map to
open_weightsorclosed_weights. Unknown tokens can remain typedunknown, with raw text preserved. (RAV-RUN3-R1-P003; coversRAV-RUN3-R1-F003) - [medium] metric semantics — A pinned mapping can canonicalize only metrics with established semantics. Score, bounds, unit, and direction can convert together while ambiguous ratings remain namespaced. (
RAV-RUN3-R1-P004; coversRAV-RUN3-R1-F004) - [medium] exclusion accounting — Each exclusion can retain its complete score row. Repeated references then preserve distinct scores, citations, and statuses. (
RAV-RUN3-R1-P005; coversRAV-RUN3-R1-F005) - [medium] evaluation metadata — The log-level library can be set only when every result has the same recognized harness. Mixed groups can use
unknownwhile preserving each result claim. (RAV-RUN3-R1-P006; coversRAV-RUN3-R1-F006) - [medium] duplicate accounting — Exact source-cell duplicates can keep one emitted result and add exclusions for later rows. Differing rows with the same result ID can remain failures. (
RAV-RUN3-R1-P007; coversRAV-RUN3-R1-F007)
Run details
- Target: PR #197 at
c3d16fe138396b433a9393ab61a7ad1391b546e0 - Run ordinal: 3
- Rounds: 1/1 completed; adaptive off; material findings
- Mix: 3 codex-exec; one reviewer could not access the exact head, and two completed substantive reviews
- Focus: correctness, maintainability, simplicity, production blast-radius, and constructive optional suggestions
- Earlier review comments: 23 ledger entries; all fixed or no longer relevant
- Finding counts: 0 critical, 2 high, 5 medium, 0 low, 0 nit
- Checks: concerns=7; confirmed=7; set-aside=0
- Second check: targeted; reviewers=2; kept=7; clarified=5 plans; removed=0; approval unchanged
- Set aside: 0 items
Reviewed with review-anvil.
| ] = defaultdict(dict) | ||
| model_infos: dict[tuple[str, str, str], ModelInfo] = {} | ||
| for score in payload['scores']: | ||
| audit_status = score.get('audit_status') or 'missing' |
There was a problem hiding this comment.
RAV-RUN3-R1-F001 [high] data-selection — A missing audit_status column can produce a successful empty run
_parse_scores() reads this field with get(). This branch then treats every absent value as a normal exclusion. With all rows excluded, no failure reaches raise_if_incomplete().
Missing or blank status values can become row failures before policy filtering. A fresh report and non-zero exit would preserve the incompatible export evidence.
| key=lambda r: r.evaluation_result_id or '') | ||
| log = EvaluationLog( | ||
| schema_version=SCHEMA_VERSION, | ||
| evaluation_id=f'benchpress/{relationship}/{sanitized}/{timestamp}', |
There was a problem hiding this comment.
RAV-RUN3-R1-F002 [high] stable-identity — The ID does not include the immutable score-matrix revision
This timestamp can fall back to conversion time, and the source manifest can lag the pinned CSV revision. Replays can change identity, while distinct revisions can collide.
The raw model, relationship, and required dataset_revision can define the ID. Generated and retrieved times can stay as metadata.
| name=model.get('name') or slug, | ||
| id=f'{org}/{slug}', | ||
| developer=provider, | ||
| additional_details=_str_map({ |
There was a problem hiding this comment.
RAV-RUN3-R1-F003 [medium] model-metadata — Typed availability remains unknown despite source evidence
The source provides open_weights values, and this code keeps them only as raw additional detail. ModelInfo then fills model_availability with unknown.
Recognized booleans can map to open_weights or closed_weights. Unknown tokens can remain typed unknown, with their raw text preserved.
| metric_config=MetricConfig( | ||
| evaluation_description=( | ||
| f'{benchmark.get("name") or benchmark["id"]} score reported via BenchPress.'), | ||
| metric_id=f'benchpress.{bslug}.score', |
There was a problem hiding this comment.
RAV-RUN3-R1-F004 [medium] metric-semantics — Known global metrics receive benchmark-specific IDs and source scales
The AIME fixture declares percent correct, but this path emits benchpress.aime-2025.score on a 0–100 scale. That result cannot join canonical accuracy records.
A pinned mapping can canonicalize metrics only when source metadata proves their meaning. Score, bounds, unit, and direction can convert together; ambiguous ratings can remain namespaced.
| for score in payload['scores']: | ||
| audit_status = score.get('audit_status') or 'missing' | ||
| if not include_unaccepted and audit_status not in ACCEPTED_AUDIT_STATUSES: | ||
| exclusions.append(SourceRecordExclusion( |
There was a problem hiding this comment.
RAV-RUN3-R1-F005 [medium] exclusion-accounting — Repeated exclusion references lose row-level detail
This exclusion stores only the model and benchmark reference. The fixture contains repeated references, so the report cannot distinguish their scores, citations, or status values.
Each exclusion can retain its complete score row through the existing source_record field.
| evaluation_id=f'benchpress/{relationship}/{sanitized}/{timestamp}', | ||
| retrieved_timestamp=timestamp, | ||
| source_metadata=source_metadata(relationship, version), | ||
| eval_library=EvalLibrary(name='BenchPress', version='unknown'), |
There was a problem hiding this comment.
RAV-RUN3-R1-F006 [medium] evaluation-metadata — The aggregator is recorded as the evaluation harness
BenchPress re-reports scores from varied harnesses, which the results already retain. This log-level value states that BenchPress ran every evaluation in the group.
A common harness can populate this field only when every result agrees. Mixed groups can use unknown while keeping each result's source claim.
| relationship = relationship_from_score(score, model) | ||
| key = (org, slug, relationship) | ||
| kept = groups[key] | ||
| previous = kept.get(result.evaluation_result_id) |
There was a problem hiding this comment.
RAV-RUN3-R1-F007 [medium] duplicate-accounting — Equal duplicate rows disappear from the source ledger
This branch keeps one result and silently skips the later equal row. total_records still includes both, but neither failures nor exclusions identifies the duplicate.
Exact source-cell duplicates can remain deduplicated while later rows become exclusions with their complete source_record.
- Derive model_availability from the source open_weights flag instead of leaving the library's auto-filled blanket 'unknown', and set a considered deployment_type='unknown' (BenchPress records no serving platform). The raw open_weights value is kept alongside. - Fold the immutable dataset_revision into evaluation_id: the manifest can lag the CSVs, so two content-differing snapshots could otherwise share one retrieved_timestamp and collide. - Treat a missing audit_status column as a structural mismatch (strict subscript like the id columns) so a dropped column fails loud instead of silently excluding every row and exiting 0. - Carry the excluded source row on the audit-status SourceRecordExclusion, like a failure, so repeated model/benchmark refs are distinguishable in the report.
What / source
Converts the BenchPress score matrix (
microsoft/benchpress-score-matrix) intoaggregate EEE records under
data/benchpress/.BenchPress is an aggregator: it re-reports scores scraped from provider blogs,
tech reports, model cards, leaderboards and third-party aggregators, each cell
carrying its own citation (
reference_url) and provenance (source_type). So it ishandled like
llm_stats—source_type=documentation,source_role=aggregator, andlogs split by
evaluator_relationship.Adapter lives at
every_eval_ever/adapters/benchpress/(rebased onto the post-#218 layout;
utils/no longer exists).Review lane
evaluator_relationshipis a provenance claim on everyrecord, and this adapter takes it from who published the citation, not from
what kind of document it is.
first_partyneeds a provider-authoredsource_typeand a citation on a domain carrying the model provider'sname, which gives
{other: 2,654, third_party: 980, first_party: 841}scoresacross
{third_party: 139, other: 138, first_party: 49}logs. Worth amaintainer's eyes: the evidence rule is the substance of the change, not the
mapping table.
Design agreed in: n/a — the mapping questions are answered inline in the review
thread below.
Checklist
python -m every_eval_ever validateclean at the finaldata/benchpress/<dev>/<model>/path — 326 files, 0 invalid, 0 errors,0 warnings, exit 0
adapter_reports/benchpress_failures.json— unconvertible rows with anon-zero exit, excluded rows itemized without one. The report is written on
every run, before publication, and swapped in atomically, so no earlier run's
report can be read as this one's and an interrupted write cannot truncate a
complete one.
tests/test_benchpress_adapter.py);full
pytest testsgreen (435 passed, 20 skipped)ruff checkcleanmodel_info.idis<provider-slug>/<benchpress-slug>and the raw slug is kept inadditional_details.benchpress_model_id; ids become canonical downstream viathe eval-card-registry, same as the other aggregator adapters.
documents they claim, no benchmark is double-counted across relationship
splits,
evaluation_idis stable for a given (relationship, model, snapshot)Decisions & coverage
Decision / where:
evaluator_relationship,relationship_from_score+_provider_publishes.Chose:
third_partyfor an independentsource_type;first_partyonly for aprovider-authored
source_typewhose citation is hosted on a domain carryingthe scored model's provider name (
openai.com,cdn.amazon.science,moonshotai.github.io,x.ai);otherotherwise.Instead of: trusting
source_typealone (the first version), or trusting it whenthe citation covers exactly one provider (the second).
Why:
source_typerecords what kind of document a citation is, not whopublished it, and the one-provider heuristic conflated "cited for one provider's
models" with "published by that provider". The live snapshot falsifies it in both
directions: a Google
gemini-2.5-flashscore cited toQwen's model card was
labelled
first_party, whileopenai.com/index/introducing-gpt-5-5
— 46 OpenAI cells plus 8 competitor cells on one page — was demoted wholesale
because the page tabulates competitors. A domain has an owner, so it names a
publisher; a path on a shared host does not (the org in
huggingface.co/Qwen/…is path, and
storage.googleapis.comserves anyone's bucket, which is why its68 cells stay
other). Every one of the 12 hosts that now yieldsfirst_partyis the provider's own. The rule under-claims rather than guesses: a provider
publishing on a hostname that does not spell its name
(
lf3-static.bytednsdoc.comfor a ByteDance model) isother. On that samepage, BenchPress already tags the competitor cells
source_type=third_partyitself, so they come out
third_party— the export made the distinction and theold heuristic discarded it. No score value changes and no cell is dropped.
Confidence: high. General? yes — any aggregator carrying per-cell citations
has this problem, and document type is never document ownership.
Decision / where: duplicate results,
make_logsgrouping.Chose: key each log's results by
evaluation_result_id; an identical repeatcollapses, a second differing result for the same benchmark is a reported
failure.
Instead of: the de-dup pass this adapter had, which kept whichever row came
first and dropped the other with no entry anywhere.
Why:
evaluation_result_idis the schema's "recommended deterministic join key",so one log carrying two results for one benchmark would break the join — but a
row that cannot be represented is a row to report, not to lose, and the silent
version made
total_source_recordsreconcile against a count that had quietlyshrunk. Zero live rows hit either path (4,903 − 410 − 18 = 4,475 exactly), so
this closes a latent hole rather than repairing observed loss. Giving the id a
citation suffix was the alternative; it would change the join key on all 4,475
cells to fix a zero-occurrence case, or make identity depend on whether a
sibling row exists.
Confidence: high. General? yes — the same silent de-dup shape is easy to write
in any adapter that groups cells into logs.
Decision / where: row filtering,
ACCEPTED_AUDIT_STATUSES.Chose: export only
audit_status in {verified, verified_third_party}.Instead of: exporting every row.
Why: BenchPress excludes
dropped/needs_review/flaggedfrom its owncanonical matrix, so re-publishing them would assert more than the source does.
These are recorded as exclusions (not failures — a policy exclusion must not fail
the run);
--include-unacceptedexports them.Confidence: high. General? no.
Decision / where: score-vs-declared-range disagreement,
_within_bounds.Chose: report the row as unconvertible.
Instead of: rescaling, or widening
MetricConfigto fit the value.Why: the export mixes scales inside a single benchmark —
mt_bench_101declares1–10 and carries values up to 90.2,
fleursWER declares [0,1] and carries up to86.4,
creative_writing_v3Elo declares [1000,2000] and carries 87.5. The recordcannot state both, and a rescale would be a guess about which cells are on which
scale.
Confidence: high. General? yes — worth a note in the conversion skill.
Decision / where: snapshot pinning,
resolve_revision/fetch_payload.Chose: resolve whatever was asked for — the default tip, a branch, a tag — to
one commit sha, read all four files at it, and record that sha as
benchpress_dataset_revision;--revisionreplays a snapshot. A--revisionthat is already a full sha is used as given.
Instead of: reading
mainper file, or splicing a supplied symbol straight intothe four file URLs.
Why:
mainmoves, and four files are four requests, so a run could otherwise mixrevisions — and a supplied branch or tag has exactly the same problem, plus it
would be recorded as provenance that can later point somewhere else.
metadata.jsonalone was not enough — the manifest can lag the CSVs. Resolving afull sha would only cost a request, since it cannot move.
Confidence: high. General? yes (already the pattern in
vectara).Decision / where: per-row failure boundary,
_parse_scores/make_logs.Chose: keep the score as the source wrote it and parse it inside a per-row
boundary that catches only what bad source data raises (
ValueError,TypeError, pydanticValidationError).Instead of: parsing during CSV read (the previous behaviour), or catching
Exceptionper row.Why: parsing at read time happened before
SourceConversionResultexisted, so oneunusable cell ended the run without recording the row it came from — the failure
mode the accounting is meant to prevent. Catching broadly would file adapter bugs
as bad source data, so anything else still surfaces as a crash.
total_recordsstays the full source row count, so nothing is silently dropped to make the
numbers reconcile. The descriptive numerics (
params_total_M,num_problems)only ever reach a record as text via
_str_map, so they keep unparseable textrather than failing.
Confidence: high. General? yes — every adapter that parses before it can
record has this.
Decision / where: when the accounting report is written,
write_conversion_report.Chose: unconditionally, atomically, and before publication. The non-zero exit
stays conditional on failures.
Instead of: only when a row failed (the previous behaviour).
Why: an exclusions-only run printed a count and preserved nothing, and a clean run
left an earlier run's report standing to be read as its own. It accounts for the
conversion, so a publication error is when it is most worth having on disk.
Confidence: high. General? yes — a stale ledger reads as current in any adapter
that writes one. It is adapter-local in the same shape as Add Open Medical-LLM Leaderboard adapter #204's; the shared home
is
helpers/io.py, proposed as one follow-up covering both rather than slippedinto either PR.
Decision / where: publication,
export_logs.Chose: the shared
save_evaluation_logs, deleting the adapter's own writer.Why: that writer emitted bare
Infinity, whichstrict_json_loadsnow rejects,so its output was no longer loadable; the shared path writes the JSON string
"Infinity"and batches publication so a late failure leaves no partial tree.Confidence: high. General? no — it was this adapter's bug.
Coverage: 4,903 source scores → 326 logs / 4,475 scores. 410 excluded
(BenchPress's own non-accepted rows: 342
dropped, 61needs_review, 7flagged) ·18 failed (7 rows whose model or benchmark id is absent from the export, 11 rows
outside their benchmark's declared range) — each itemized with its own row and
reason in
adapter_reports/benchpress_failures.json, the failures with a non-zeroexit. No caps, no sampling. Re-verified live on this head (both a fresh fetch and
an
--input-jsonreplay, which agree): 4,903 → 326 logs / 4,475 scores,validateall 326 passed, and the same 18 failures and 410 exclusions as before — no new
failure of any kind, including the new duplicate check.
Operator asked about policy calls? Yes, three, all resolved above and in the
review thread: the
first_partyprovenance claim (changed), the 410-row policyexclusion (kept, made visible), and the mixed-scale rows (rejected rather than
guessed at).