feat(harbor): upload standard verifier output - #724
Merged
Abhijeet Prasad (AbhiPrasad) merged 5 commits intoSep 1, 2026
Conversation
`max_total_attachment_bytes` was one shared pot consumed in source order, so artifacts and trajectory images could starve verifier output and reward-details. Same trial, one bigger screenshot, different evidence. Attachments are meant to be large and `Attachment` uploads lazily from a path, so both caps and the budget plumbing are gone; `_read_bounded_file` becomes `_read_safe_file`, keeping the symlink, file-type and dev/ino checks. `trajectory.json` is parsed in-process rather than uploaded, so it keeps a bound of its own via `max_trajectory_bytes`. Two fixes from the same review: - Raw `test-stdout.txt`/`test-stderr.txt` now require `attachments=all`. `normalize_json` redacts by key name, which raw text lacks, so `redact_patterns` — empty by default — was the only protection on a payload the default config uploaded. `ctrf.json` stays in `verifier-details`. - `json.loads` raises `RecursionError`, not `JSONDecodeError`, on deeply nested input. A 400 KB `ctrf.json` aborted `_sync_final_result` after the spans started but before output, scores and metadata, leaving a half-written row. Both decoders now fall back to text. Tests use the real span harness (`init_test_exp` + `with_memory_logger`) instead of `RecordingSpan` fakes, so they assert the stored `AttachmentReference` and the queued upload. BREAKING CHANGE: `max_attachment_bytes` and `max_total_attachment_bytes` are removed, and unknown options are silently ignored, so a config setting them now uploads with no ceiling. `verifier-details` no longer ships raw verifier stdout/stderr; use `attachments=all` with `redact_patterns`.
Follow-up to 3306c05d, from a quality pass over the branch. `RecursionError` from `json.loads` was guarded at two call sites but not on task-written `manifest.json` or either `trajectory.json` reader. The guard is now `identity.try_parse_json`, used at all five. Those readers also moved to `read_bytes`, closing a second hole: `read_text` raises `UnicodeDecodeError`, a `ValueError`, which the old `except (OSError, JSONDecodeError)` never caught either. Span previews used `normalize_json`'s default `max_depth=8` while their attachment used 20, so a 12-deep `ctrf.json` came out `[DROPPED: depth limit]` in the span and whole in the attachment. That default was incidental. `_bounded_summary` now passes 20 for both verifier output and reward details, and skips the walk when the payload already fits — its serialized size is known from the attachment, so re-walking and re-measuring a multi-MB payload bought nothing.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
The verifier-output tests wrote raw stdout/stderr fixtures with Path.write_text, which translates "\n" to "\r\n" in text mode on Windows. The plugin decodes the file's bytes verbatim, so the summary carried CRLF and the assertions failed on Windows only. Write those fixtures as bytes so the on-disk content matches what is asserted. Also skip the fifo parametrization of the unsafe-file-type test where os.mkfifo does not exist. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Stephen Belanger (Qard)
approved these changes
Sep 1, 2026
Abhijeet Prasad (AbhiPrasad)
deleted the
codex/harbor-verifier-evidence
branch
September 1, 2026 15:30
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
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.
Supersedes #716
AI Summary
Braintrust already received the reward numbers for each Harbor trial. Now it also receives the evidence the verifier wrote, so you can see why a trial failed without leaving the UI.
Verifier evidence shows up in two places: a bounded inline preview and one complete normalized
verifier-output.jsonattachment.Attachment modes
Harbor runs are evaluation environments, where complete failure evidence is generally more useful than omitting raw logs. The default therefore captures all standard verifier output.
attachments=ctrf.jsontest-stdout/stderr.txtnonestructuredall(default)artifact_include)structuredis the JSON-only tier for environments that should not upload raw verifier logs. Task artifacts remain opt-in even underall: they require an explicitartifact_includeglob.Structured redaction matches sensitive key names, while plain text has no keys:
If an eval environment contains sensitive values, configure
redact_patternsor useattachments="structured".redact_patternsis the only redaction applied to raw verifier logs and is empty by default.Size limits are gone
There used to be a 5 MB per-file cap and a 20 MB per-trial cap. The per-trial cap was one shared pot, spent in source order:
That meant a few screenshots could push out the score evidence, and the same trial with a slightly larger screenshot could upload different evidence. Attachments are intended for large payloads, so both attachment caps and their shared-budget plumbing are removed and every accepted file uploads whole.
trajectory.jsonkeeps a separate limit (max_trajectory_bytes, 20 MB) because it is parsed in-process rather than treated as an attachment.Configuration changes
attachments="all"is now the default.attachments="verifier-details"is renamed toattachments="structured".max_attachment_bytesandmax_total_attachment_bytesare removed and no longer impose an upload ceiling.