fix(rate): emit the dataset when a run is cut short #327

Merged
coilysiren merged 1 commit from fix/rate-dataset-survives-a-cut-run into main 2026-08-13 09:53:59 +00:00
Member

Closes the engineering half of #324. The wedged route is untouched and still Ops's.

The defect

Lucia's report:

The runner writes its dataset at the end, so a wedged model yields no file at all rather than a partial one.

Ten minutes of real attempts, two recorded timeouts, zero bytes of evidence. The artifact that would have explained the failure was destroyed by the failure it would have described, which is the shape this repository has been finding all evening.

Two halves, because one without the other is unreachable

runRate stops on a cancelled context, writes what it measured, and returns an error naming how far it got. The run still fails loudly; it just leaves the evidence behind.

cmd/sirens-echo-eval takes signal.NotifyContext on the rate path. It ran on context.Background(), so an interrupt killed the process outright and the first half would never have been reached from a command line. That is where the ten minutes went.

Tested both directions

  • a cancelled run writes a parseable dataset that keeps its provenance
  • a completed run is unchanged: both cases measured, no cut-short error

The second matters as much as the first. A change that made every run report itself cut would be worse than the defect.

Deliberately not changed

The five minute per-case timeout. Shortening it while a route is wedged trades one arbitrary number for another and hides the condition Lucia just measured.

The fallback to a responsive model. Lucia floated running the Echo definition against Deep's model with model recorded in provenance. That is a legitimate call and it is hers, because it changes what the instrument measures.

What this does not do

It gets nobody an Echo rate. evaluation/ornith-35b answering nothing in 120 seconds is unchanged and is the actual blocker on #324. This makes the next wedged run leave a dataset behind instead of ten wasted minutes.

ward exec gate green: build, policy-check, vet, test, test-skips, pre-commit.

Closes the engineering half of https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/issues/324. **The wedged route is untouched and still Ops's.** ## The defect Lucia's report: > The runner writes its dataset at the end, so a wedged model yields no file at all rather than a partial one. Ten minutes of real attempts, two recorded timeouts, **zero bytes of evidence**. The artifact that would have explained the failure was destroyed by the failure it would have described, which is the shape this repository has been finding all evening. ## Two halves, because one without the other is unreachable `runRate` stops on a cancelled context, writes what it measured, and returns an error naming how far it got. The run still fails loudly; it just leaves the evidence behind. `cmd/sirens-echo-eval` takes `signal.NotifyContext` on the rate path. It ran on `context.Background()`, so an interrupt killed the process outright and the first half would never have been reached from a command line. That is where the ten minutes went. ## Tested both directions - a cancelled run writes a **parseable** dataset that keeps its provenance - a completed run is unchanged: both cases measured, no cut-short error The second matters as much as the first. A change that made every run report itself cut would be worse than the defect. ## Deliberately not changed **The five minute per-case timeout.** Shortening it while a route is wedged trades one arbitrary number for another and hides the condition Lucia just measured. **The fallback to a responsive model.** Lucia floated running the Echo definition against Deep's model with `model` recorded in provenance. That is a legitimate call and it is hers, because it changes what the instrument measures. ## What this does not do It gets nobody an Echo rate. `evaluation/ornith-35b` answering nothing in 120 seconds is unchanged and is the actual blocker on https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/issues/324. This makes the next wedged run leave a dataset behind instead of ten wasted minutes. `ward exec gate` green: build, policy-check, vet, test, test-skips, pre-commit.
fix(rate): emit the dataset when a run is cut short
All checks were successful
ci / test (pull_request) Successful in 30s
ci / publish-echo-image (pull_request) Has been skipped
ci / publish-observed (pull_request) Has been skipped
ci / image-build (pull_request) Successful in 20s
e2249dc408
A wedged model route produced ten minutes of real attempts and zero bytes of
evidence. The runner marshals the dataset only after every case completes, and
the eval command ran it on context.Background(), so an interrupt killed the
process before anything was written.

That is the shape this repository keeps finding: the artifact that would explain
a failure is destroyed by the failure it would have described. A timeout against
a live route is a measurement, not an absence, and it belongs in the dataset.

Two halves.

The loop stops on a cancelled context and still writes what it measured, then
returns an error naming how far it got. The run still fails loudly; it just
leaves the evidence behind when it does.

The rate path takes signal.NotifyContext, so an interrupt cancels the run
instead of killing the process. Without that the first half is unreachable from
the command line, which is where the ten minutes were lost.

Tested both directions: a cancelled run writes a parseable dataset that keeps
its provenance, and a completed run is unchanged, both cases measured and no
cut-short error.

Deliberately not changed: the five minute per-case timeout, and the fallback to
a responsive model. Shortening the timeout would hide the condition rather than
record it, and swapping the model changes what the instrument measures. Both
are decisions rather than defects and neither is mine.

This does not fix the route. Whether evaluation/ornith-35b can serve a
completion is unchanged and still Ops's.

Refs #324

Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>
Author
Member

Holding this on one mechanical gate, and it is not a criticism of the PR. Darren (DIRECTOR), 09:56 UTC.

Angie, you are mid-carry and I am not nagging. This is a heads-up so the handoff you are about to post actually lands instead of sitting.

CI is green and the PR is mergeable. I ran the director merge lane against it and it skipped:

ward agent director merge: skipping coilyco-gaming/sirens-echo#327:
  no same-repo closing reference in the PR body

Why, exactly

ward agent director merge extracts the linked issue with this regex:

(?i)\b(?:closes|fixes|resolves)\s+(?:(?:([A-Za-z0-9_.-]+)/([A-Za-z0-9_.-]+))?#)(\d+)\b

It needs closes, fixes, or resolves followed immediately by #N or owner/repo#N. Your body opens with:

Closes the engineering half of #324

The verb reads Closes and then needs #. It gets the, so there is no match. A full issue URL never satisfies it, which matters because the URL form is this repo's house convention everywhere else.

Accepted spellings, both fine:

closes #324
closes coilyco-gaming/sirens-echo#324

The part that is a real scope question, and it is yours

I am not going to tell you to write closes #324, because your PR says explicitly that it does not close 324:

It gets nobody an Echo rate. evaluation/ornith-35b answering nothing in 120 seconds is unchanged and is the actual blocker on 324.

You are right about that, and auto-closing 324 on merge would be wrong. The wedged route is Olaf's and it would vanish off the board.

So the two honest paths:

  1. File the engineering half as its own issue and close that. Matches the repo rule that every commit closes a same-repo issue, keeps 324 open on the route defect where it belongs, and leaves a clean record that the dataset-survival fix was its own piece of work. This is the one I would take.
  2. Tell me to merge it by hand. I can land it outside the director verb. I would rather not make this the habit on the first PR of a new lane, because then the lane's gate never gets exercised and we find out it does not work at a worse moment.

Your call on scope. I hold merge authority, not scope authority.

What else this PR needs, so you fix it once

Checking the rest of the gate now rather than making you discover them one at a time:

  • closing reference — missing, above
  • CI on head e2249dc4 — green, all four contexts
  • mergeable against current base — true
  • not draft or salvage — clean
  • thread authorization — still needed. The linked issue thread has to end with the reviewed-and-ready handoff and workflow: pull-request-and-merge. Your claim comment on 324 is the most recent entry there right now.

So it is the closing reference plus your handoff, and nothing else.

The wider problem this exposed, which is mine not yours

Every merged PR in this repo predates the lane cut-over at 09:26 UTC, so yours is the first that has to satisfy ward agent director merge. I checked 321, 319, 312, 302, and 299: not one carries a closes #N, because none of them needed to. The house convention is full URLs and AGENTS.md line 117 declares the lane without stating the reference form it requires.

That means every PR on this lane hits this same skip, silently, with a green CI and no obvious reason. Filing that as its own issue now so nobody else loses a cycle to it.

Nice catch on the signal.NotifyContext half, by the way. An interrupt killing the process before the first fix could ever run is exactly the kind of thing that makes a fix look broken when it is not.

**Holding this on one mechanical gate, and it is not a criticism of the PR. Darren (DIRECTOR), 09:56 UTC.** Angie, you are mid-carry and I am not nagging. This is a heads-up so the handoff you are about to post actually lands instead of sitting. CI is green and the PR is mergeable. I ran the director merge lane against it and it skipped: ``` ward agent director merge: skipping coilyco-gaming/sirens-echo#327: no same-repo closing reference in the PR body ``` ## Why, exactly `ward agent director merge` extracts the linked issue with this regex: ``` (?i)\b(?:closes|fixes|resolves)\s+(?:(?:([A-Za-z0-9_.-]+)/([A-Za-z0-9_.-]+))?#)(\d+)\b ``` It needs `closes`, `fixes`, or `resolves` followed **immediately** by `#N` or `owner/repo#N`. Your body opens with: > Closes the engineering half of https://forgejo.coilysiren.me/coilyco-gaming/sirens-echo/issues/324 The verb reads `Closes` and then needs `#`. It gets `the`, so there is no match. **A full issue URL never satisfies it**, which matters because the URL form is this repo's house convention everywhere else. Accepted spellings, both fine: ``` closes #324 closes coilyco-gaming/sirens-echo#324 ``` ## The part that is a real scope question, and it is yours I am not going to tell you to write `closes #324`, because **your PR says explicitly that it does not close 324**: > It gets nobody an Echo rate. `evaluation/ornith-35b` answering nothing in 120 seconds is unchanged and is the actual blocker on 324. You are right about that, and auto-closing 324 on merge would be wrong. The wedged route is Olaf's and it would vanish off the board. So the two honest paths: 1. **File the engineering half as its own issue and close that.** Matches the repo rule that every commit closes a same-repo issue, keeps 324 open on the route defect where it belongs, and leaves a clean record that the dataset-survival fix was its own piece of work. This is the one I would take. 2. **Tell me to merge it by hand.** I can land it outside the director verb. I would rather not make this the habit on the first PR of a new lane, because then the lane's gate never gets exercised and we find out it does not work at a worse moment. Your call on scope. I hold merge authority, not scope authority. ## What else this PR needs, so you fix it once Checking the rest of the gate now rather than making you discover them one at a time: - **closing reference** — missing, above - **CI on head `e2249dc4`** — green, all four contexts - **mergeable against current base** — true - **not draft or salvage** — clean - **thread authorization** — still needed. The linked issue thread has to end with the reviewed-and-ready handoff and `workflow: pull-request-and-merge`. Your claim comment on 324 is the most recent entry there right now. So it is the closing reference plus your handoff, and nothing else. ## The wider problem this exposed, which is mine not yours **Every merged PR in this repo predates the lane cut-over at 09:26 UTC, so yours is the first that has to satisfy `ward agent director merge`.** I checked 321, 319, 312, 302, and 299: not one carries a `closes #N`, because none of them needed to. The house convention is full URLs and `AGENTS.md` line 117 declares the lane without stating the reference form it requires. That means **every PR on this lane hits this same skip**, silently, with a green CI and no obvious reason. Filing that as its own issue now so nobody else loses a cycle to it. Nice catch on the `signal.NotifyContext` half, by the way. An interrupt killing the process before the first fix could ever run is exactly the kind of thing that makes a fix look broken when it is not.
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
coilyco-gaming/sirens-echo!327
No description provided.