fix(importer): count samples when an eval log has no results block by sjawhar · Pull Request #1426 · METR/hawk

sjawhar · 2026-08-23T17:19:51Z

Overview

When an eval log has no results block, the importer wrote total_samples=0 and completed_samples=0 on the eval row while still writing every sample row, so hawk list evals showed 0/0 for finished evals whose samples were all imported. This PR counts the written samples instead when a finished log has no results block.

Approach

A present results block is still trusted; it can legitimately differ from the file (a resumed run, say). Without one, the importer counts the log's recorded sample summaries, the list its sample loop iterates. completed_samples credits only samples marked complete (or with completed_at on older logs) and without an error.

The count reads the file being parsed, not location_override. For an S3 import that is the downloaded copy, so a replaced object cannot make the count disagree with the rows written.

Live polls do not count: a running eval has no results yet and is parsed on every poll (the header-only buffer-sync parse and the per-file import), so counting would re-read every sample summary each time. Its row keeps 0 until the import after it finishes.

Updating the row after the sample loop instead would leave it wrong until the loop ends; counting up front keeps it right from the first write.

Risks

Rows already imported with 0/0 are not backfilled; a forced re-import rewrites them.
A log that never leaves status started (a run killed before it finished) still shows 0/0.
A results-less import of a finished log reads the file's sample summaries once more.

Testing & validation

New converter tests cover a present results block winning, counting without one, errored samples, counting from the parsed file rather than location_override, and live polls not counting. A Postgres test checks that the persisted counts equal the sample rows written.

uv run pytest tests/core/importer/eval -n auto: 414 passed.
Verified the change works (commands / manual steps described above)
Added or updated tests where it makes sense

Code quality

pre-commit run --all-files passes (ruff, basedpyright/mypy, eslint/prettier/tsc, shellcheck — what CI's Lint job runs)

Before merging

PR title is a Conventional Commit with a lower-case subject — it becomes the squash-merge commit subject and drives the SemVer bump
All commits are signed and show as Verified on GitHub — see Commit signing

Copilot

Pull request overview

Fixes incorrect sample totals when importing eval logs without a results block.

Changes:

Counts recorded samples when results are absent.
Preserves result-provided counts when present.
Adds converter and PostgreSQL integration coverage.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
`hawk/hawk/core/importer/eval/converter.py`	Adds fallback sample counting.
`hawk/tests/core/importer/eval/test_converter.py`	Tests count selection behavior.
`hawk/tests/core/importer/eval/test_writer_postgres.py`	Tests persisted counts and sample rows.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…from the actual parsed file Addresses METR#1426 Copilot review findings: - completed_samples was set to the full recorded-sample count, which reports every recorded sample as completed even when some errored. Derive it from summaries marked completed (mirroring eval_status._is_completed) that do not carry an error, per the Eval.completed_samples contract ("samples completed without error"). - the no-results sample count was read from the persisted location (location_override for S3 imports), not the file EvalConverter is actually parsing -- defeating the S3 download optimization and risking a stale/replaced-object mismatch. Read from EvalConverter.eval_source via a new sample_source parameter instead. Omp-Session: 01a05ad7-ca0d-7000-a3a3-7187810518dd

An eval log without a `results` block (e.g. one written before scoring finished, or a hand-repaired eval that never got one back) persisted total_samples=0 and completed_samples=0 on the eval row, even though the importer went on to write every sample row from the log. The warehouse row and its own sample rows silently disagreed, and 'hawk list evals' showed 0/0 for evals that genuinely held hundreds of samples. When results is present its numbers are still trusted verbatim (this can legitimately differ from the raw sample count, e.g. a resumed run). When it is absent, count the samples the importer will actually write (the same sample_summaries the write loop iterates) instead of persisting 0.

…from the actual parsed file Addresses METR#1426 Copilot review findings: - completed_samples was set to the full recorded-sample count, which reports every recorded sample as completed even when some errored. Derive it from summaries marked completed (mirroring eval_status._is_completed) that do not carry an error, per the Eval.completed_samples contract ("samples completed without error"). - the no-results sample count was read from the persisted location (location_override for S3 imports), not the file EvalConverter is actually parsing -- defeating the S3 download optimization and risking a stale/replaced-object mismatch. Read from EvalConverter.eval_source via a new sample_source parameter instead. Omp-Session: 01a05ad7-ca0d-7000-a3a3-7187810518dd

Resolve the two conflicts in hawk/core/importer/eval/converter.py: - keep main's _model_event_names helper next to this branch's _is_sample_completed/_count_recorded_samples helpers; - EvalConverter.parse_eval_log passes both this branch's sample_source and main's new file_metadata to build_eval_rec_from_log. The test files merged without conflicts. Omp-Session: 01a081d4-2514-7000-b8e2-ff8548776146

Live sample import added two paths that build the eval record of a running eval on every poll: the header-only parse for buffer syncs (parse_eval_log_header_only) and the per-file import of an eval whose status is still "started". A running eval has no results block yet, so the no-results fallback in build_eval_rec_from_log re-read every recorded sample summary on each of those polls, including on the header-only path, which exists to skip per-sample work. Count only for a finished log, and only when the caller passes sample_source, the file it is actually parsing. EvalConverter.parse_eval_log passes it, so a finished results-less import still gets its real counts; a running eval and the header-only parse keep 0, which the import after the eval finishes overwrites. Counting also no longer falls back to eval_source, the persisted location. Red on the merge parent, green with this change: test_header_only_does_not_count_samples_when_results_absent (read_log_sample_summaries called once by the header-only parse) and test_converter_leaves_running_eval_counts_to_the_final_import (total_samples 4 for a running eval). Omp-Session: 01a081d4-2514-7000-b8e2-ff8548776146

…esults

rasmusfaber · 2026-10-05T12:17:55Z

+        for summary in sample_summaries
+        if _is_sample_completed(summary) and not summary.error
+    )
+    return len(sample_summaries), completed_samples


total_samples is documented to be the planned count of samples. This instead stores the recorded count.

Copilot AI balanced review requested due to automatic review settings August 23, 2026 17:19

sjawhar requested a review from a team as a code owner August 23, 2026 17:19

sjawhar requested a review from kai-metr August 23, 2026 17:19

Copilot started reviewing on behalf of sjawhar August 23, 2026 17:20 View session

Copilot AI reviewed Aug 23, 2026

View reviewed changes

Comment thread hawk/hawk/core/importer/eval/converter.py Outdated

Comment thread hawk/hawk/core/importer/eval/converter.py Outdated

legion-implementer Bot force-pushed the fix/eval-log-importer-sample-count-without-results branch from d33a102 to f41804d Compare September 3, 2026 02:48

legion-implementer Bot force-pushed the fix/eval-log-importer-sample-count-without-results branch from f41804d to 5420f38 Compare September 5, 2026 16:14

legion-implementer Bot force-pushed the fix/eval-log-importer-sample-count-without-results branch from 5420f38 to 7160d9f Compare September 8, 2026 20:18

sjawhar added 2 commits September 24, 2026 04:21

sjawhar-agent Bot force-pushed the fix/eval-log-importer-sample-count-without-results branch from 7160d9f to 40dbd23 Compare September 24, 2026 04:49

sjawhar and others added 3 commits October 4, 2026 02:23

Merge branch 'main' into fix/eval-log-importer-sample-count-without-r…

2b5fc33

…esults

rasmusfaber requested changes Oct 5, 2026

View reviewed changes

Navigation Menu

Repository navigation

fix(importer): count samples when an eval log has no results block - #1426

fix(importer): count samples when an eval log has no results block#1426
sjawhar wants to merge 5 commits into
METR:mainfrom
sjawhar:fix/eval-log-importer-sample-count-without-results

sjawhar commented Aug 23, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

rasmusfaber Oct 5, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

sjawhar commented Aug 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Approach

Risks

Testing & validation

Code quality

Before merging

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

rasmusfaber Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sjawhar commented Aug 23, 2026 •

edited

Loading