Repository navigation
Trajectory step 2: measurement provenance; pending ledger reader tolerates new fields - #586
Conversation
There was a problem hiding this comment.
Round 1 — reviewed head b619c331 — reviewer summarizer:hermes/gpt-5.6-terra over coverage+credentials+deployment+general+lifecycle+prose.
terra
Advisory findings from outerloop — the code owner decides. Reply to disagree; the outerloop:no-review label opts this PR out.
Verdict: 2 blocking, 1 advisory.
2 findings attached to the lines below.
Completed measurements lose their result-file refs. [lifecycle] The final AttemptResult omits outcome.eval_files, so the measure event at lines 2516-2520 receives an empty tuple after a non-parked successful measurement and cannot record the files it read. (src/outerloop/orchestrator.py:2742; high confidence)
Verdict: changes requested. Blocking: the image digest is captured too late for parked dispatched runs, and synchronous completed measurements discard result-file provenance refs. Non-blocking: the provenance test stubs out the production collector. Rejected: none; no duplicated, mistaken, or prose findings were supplied.
|
Summary finding (credited result drops eval_files): fixed in 7a9767f. attempt_once's credited AttemptResult now carries outcome.eval_files. The measure event inside attempt_once already had the files; only the returned result lacked them. |
There was a problem hiding this comment.
Round 4 — reviewed head 7a9767f0 — reviewer hermes/gpt-5.6-terra.
terra
Advisory findings from outerloop — the code owner decides. Reply to disagree; the outerloop:no-review label opts this PR out.
Verdict: 2 blocking, 0 advisory.
2 findings attached to the lines below.
Image provenance can be wrong after a dispatch crash or when a cached baseline used an earlier image.
There was a problem hiding this comment.
Round 5 — reviewed head 3ba194d9 — reviewer hermes/gpt-5.6-terra.
terra
Advisory findings from outerloop — the code owner decides. Reply to disagree; the outerloop:no-review label opts this PR out.
Verdict: nothing blocking — 1 advisory note.
1 finding attached to the lines below.
The ruler provenance digest changes for solver-only edits.
Trajectory step 2:
measureevents say what produced the number.ledger_events.ruler_identity()now computes the ruler digest that pending submissions already carry: the measurement signature plus the non-solver tree entries.measurement_pendingand every measure event call it, so the event and the ledger row hold the same value. It's best-effort: a git failure logs and records""; it never fails a wake.image_sha256is the SHA-256 of the configured container image file, cached in-process by (path, size, mtime) so a tick hashes it at most once.refshold{path, sha256}for the evaluation'sexit-code,stdoutandstderrunder the run directory, the files the kernel reads to get the metric. Contents are never inlined.Ledger link deferred, with forward compatibility added now. The plan also puts the trajectory chain head in the pending ledger record. But
parse_pendingbuilt the record withPendingSubmission(**raw), so any unknown key made an older kernel fail its whole ledger read. This PR makesparse_pendingignore unknown fields, and adds a test that a newer record still parses. The chain-head field can then land in a later release without breaking this one. Pending records written by this kernel are unchanged.Compatibility: no persisted format changes. Older kernels ignore
trajectory.jsonl. This kernel reads every pending record an older kernel writes, and will tolerate fields a newer one adds.Not verified live: a dispatched run with a real image (unit tests use a small temp file).
Gate: ruff check, ruff format --check, mypy, pytest (3143 passed, 7 skipped).
Built by DeepSeek-V4.1-Flash, with the forward-compatibility change by Claude; reviewed by Claude.