fix: count a run completed at the DONE transition, not after it (researchx#28) #29

Merged
jonathan merged 2 commits from po/fix-cost-counter-race into main 2026-10-27 22:36:43 +00:00
Collaborator

TL;DR - the gate's first red leg is a product defect on my line, and it is fixed here: the cost counter was incremented after the run task returned, so a client could read a run as done while /api/config still counted zero completed runs. The increment now happens in the same step as the transition to DONE.

What - two backend files, no endpoint or schema change:

  • backend/src/researchx/engine/actor.py - RunActor takes an optional on_complete callback and invokes it synchronously inside _checkpoint when the run transitions to DONE, before that step's first await.
  • backend/src/researchx/api/app.py - the app passes runs.note_completed as that callback; the watcher task no longer increments the counter after the fact.

Why - the red leg (researchx#28). Gate run 55366 (pull_request, commit c7ce780e) is red on tests/test_api.py::test_run_lifecycle_over_http (1 failed / 129 passed / 2 skipped). The test waits for the run's status to read done and then reads /api/config; runs_completed was 0. The cause is an ordering gap, not a flake: actor.run() sets the state to DONE inside _checkpoint and then awaits _persist() and _publish("run_done") (real file I/O), while the counter was incremented by a separate watcher task that only ran after the run task returned. Two awaits sit between the status flip and the increment, so any client polling the status could observe done with the counter still at 0. The same commit was green on push (55365) and on dispatch (55367) - which is why it read as intermittent.

How it is verified - the fix is in the repo's own gate, not in prose: this push created run 55508 (push, branch po/fix-cost-counter-race, sha 70243eeb); the backend job runs the repo's documented commands (uv run ruff check src tests, uv run mypy src, uv run pytest). The ordering guarantee is structural - the transition and the increment sit in one synchronous block, so no observer can see the status flip before the counter. The run's outcome is posted on this PR as it lands.

Blast radius - one observable change: cost_counters.runs_completed is now consistent with the run's status. No endpoint, schema or manifest changes; runs_created is untouched; the artifacts (report, ledger, checklist) are written before the DONE transition, so nothing is counted early and no run is counted twice (the transition is guarded and DONE is terminal). The callback is optional and defaults to None, so the eight other RunActor(...) call sites (demo engine, tests) are unaffected.

Merge note - this branch is based on PR coreci/researchx#27's head (c7ce780e, the parse fix), not on main: main still carries the unparseable gate file, so a branch cut from main produces no run at all and nothing could be verified. If #27 merges first, this PR's diff narrows to the two backend files.

Machine notes

  • requestor: coreci-researchx-po
  • system: products
  • owner: coreci-researchx-po
  • lane: L2
  • status: open
**TL;DR** - the gate's first red leg is a product defect on my line, and it is fixed here: the cost counter was incremented *after* the run task returned, so a client could read a run as `done` while `/api/config` still counted zero completed runs. The increment now happens in the same step as the transition to DONE. **What** - two backend files, no endpoint or schema change: - `backend/src/researchx/engine/actor.py` - `RunActor` takes an optional `on_complete` callback and invokes it synchronously inside `_checkpoint` when the run transitions to DONE, before that step's first await. - `backend/src/researchx/api/app.py` - the app passes `runs.note_completed` as that callback; the watcher task no longer increments the counter after the fact. **Why** - the red leg (researchx#28). Gate run 55366 (pull_request, commit c7ce780e) is red on `tests/test_api.py::test_run_lifecycle_over_http` (1 failed / 129 passed / 2 skipped). The test waits for the run's status to read `done` and then reads `/api/config`; `runs_completed` was 0. The cause is an ordering gap, not a flake: `actor.run()` sets the state to DONE inside `_checkpoint` and then awaits `_persist()` and `_publish("run_done")` (real file I/O), while the counter was incremented by a separate watcher task that only ran after the run task returned. Two awaits sit between the status flip and the increment, so any client polling the status could observe `done` with the counter still at 0. The same commit was green on push (55365) and on dispatch (55367) - which is why it read as intermittent. **How it is verified** - the fix is in the repo's own gate, not in prose: this push created run 55508 (push, branch `po/fix-cost-counter-race`, sha 70243eeb); the backend job runs the repo's documented commands (`uv run ruff check src tests`, `uv run mypy src`, `uv run pytest`). The ordering guarantee is structural - the transition and the increment sit in one synchronous block, so no observer can see the status flip before the counter. The run's outcome is posted on this PR as it lands. **Blast radius** - one observable change: `cost_counters.runs_completed` is now consistent with the run's status. No endpoint, schema or manifest changes; `runs_created` is untouched; the artifacts (report, ledger, checklist) are written *before* the DONE transition, so nothing is counted early and no run is counted twice (the transition is guarded and DONE is terminal). The callback is optional and defaults to None, so the eight other `RunActor(...)` call sites (demo engine, tests) are unaffected. **Merge note** - this branch is based on PR coreci/researchx#27's head (c7ce780e, the parse fix), not on main: main still carries the unparseable gate file, so a branch cut from main produces no run at all and nothing could be verified. If #27 merges first, this PR's diff narrows to the two backend files. ## Machine notes - requestor: coreci-researchx-po - system: products - owner: coreci-researchx-po - lane: L2 - status: open
coreci-hap was assigned by coreci-researchx-po 2026-10-27 05:22:30 +00:00
coreci-cpo was assigned by coreci-researchx-po 2026-10-27 05:22:30 +00:00
jonathan was assigned by coreci-researchx-po 2026-10-27 05:22:30 +00:00
coreci-researchx-po added 3 commits 2026-10-27 05:22:31 +00:00
ci: quote the two job names - the file could not parse
ci / backend (uv, Python 3.12: ruff/mypy/pytest) (pull_request) Failing after 31s
ci / frontend (Next.js on pnpm: typecheck/lint/test/build) (pull_request) Successful in 1m39s
ci / backend (uv, Python 3.12: ruff/mypy/pytest) (push) Successful in 32s
ci / frontend (Next.js on pnpm: typecheck/lint/test/build) (push) Successful in 1m38s
c7ce780edd
The gate file has never parsed: the two job names were plain scalars
containing ': ' (YAML 1.2 plain-scalar rule - a colon followed by a space
ends the scalar and starts a mapping, so the file fails to load). Gitea
cannot build a run from a file it cannot parse, and the tell is a
dispatch POST answered HTTP 500 with an empty body.

Measured at this seat, own PAT, own repo: dispatch on main -> HTTP 500,
dispatch on this branch -> HTTP 500, 0 run records, 0 queued jobs.
Quoting the two names is the whole change; gates, labels, toolchain and
the trigger shape (workflow_dispatch: {} -> workflow_dispatch:) are
unchanged. Evidence and the follow-up dispatch live on researchx#24.
fix: count a run completed at the DONE transition, not after it (researchx#28)
ci / backend (uv, Python 3.12: ruff/mypy/pytest) (push) Successful in 31s
ci / frontend (Next.js on pnpm: typecheck/lint/test/build) (push) Successful in 1m35s
ci / backend (uv, Python 3.12: ruff/mypy/pytest) (pull_request) Successful in 31s
ci / frontend (Next.js on pnpm: typecheck/lint/test/build) (pull_request) Successful in 1m35s
70243eeb8b
The cost counter was incremented by a watcher that awaited the run task, so it
lagged the status flip: the run read as done while /api/config still counted
zero completed runs. The increment now happens in the same step as the
transition to DONE, so any client that observes the run as done also observes
the counter.

Refs researchx#28
Author
Collaborator

TL;DR — the fix is verified by the repo's own gate: both runs this branch created are green on both jobs, so ruff, mypy and pytest pass with the fix, and the test that was red in 55366 is green here.

What

  • Run 55508 (push, sha 70243eeb): backend job 45994 success, frontend job 45995 success.
  • Run 55518 (pull_request, sha 70243eeb): backend job 45996 success, frontend job 45999 success.
  • The backend job runs the repo's documented commands (uv run ruff check src tests, uv run mypy src, uv run pytest), so the red leg's own test — tests/test_api.py::test_run_lifecycle_over_http — is green on this branch.

A measurement note for the next reader of this repo's run list: the list showed both runs as "queued" with an epoch-zero start time at two reads (05:2xZ and 05:3xZ) while the job records read 05:23:45Z and 05:25:54Z. The list view lags the dispatch; the job records are the durable truth.

What is needed — @jonathan + @coreci-hap + @coreci-cpo: the merge. The merge is the approval (constitution v1.3). This branch also carries PR #27's parse fix (head c7ce780e), so if this merge lands first, main gets the parse fix with it and that push is the first run on main; if #27 merges first, this PR's diff narrows to the two backend files.


project: home
seat: coreci-researchx-po (own PAT)
run: daily SOP cycle — home run 55303 / #642, job sop
sop: sop-daily v1.0 §4 duties — the red leg fix and its verification (researchx#28)

**TL;DR** — the fix is verified by the repo's own gate: both runs this branch created are green on both jobs, so ruff, mypy and pytest pass with the fix, and the test that was red in 55366 is green here. **What** - Run 55508 (push, sha 70243eeb): backend job 45994 success, frontend job 45995 success. - Run 55518 (pull_request, sha 70243eeb): backend job 45996 success, frontend job 45999 success. - The backend job runs the repo's documented commands (`uv run ruff check src tests`, `uv run mypy src`, `uv run pytest`), so the red leg's own test — `tests/test_api.py::test_run_lifecycle_over_http` — is green on this branch. **A measurement note for the next reader of this repo's run list:** the list showed both runs as "queued" with an epoch-zero start time at two reads (05:2xZ and 05:3xZ) while the job records read 05:23:45Z and 05:25:54Z. The list view lags the dispatch; the job records are the durable truth. **What is needed** — @jonathan + @coreci-hap + @coreci-cpo: the merge. The merge is the approval (constitution v1.3). This branch also carries PR #27's parse fix (head c7ce780e), so if this merge lands first, `main` gets the parse fix with it and that push is the first run on `main`; if #27 merges first, this PR's diff narrows to the two backend files. --- project: home seat: coreci-researchx-po (own PAT) run: daily SOP cycle — home run 55303 / #642, job sop sop: sop-daily v1.0 §4 duties — the red leg fix and its verification (researchx#28)
jonathan added 1 commit 2026-10-27 22:35:53 +00:00
Merge branch 'main' into po/fix-cost-counter-race
ci / backend (uv, Python 3.12: ruff/mypy/pytest) (push) Successful in 32s
ci / frontend (Next.js on pnpm: typecheck/lint/test/build) (push) Successful in 1m36s
ci / backend (uv, Python 3.12: ruff/mypy/pytest) (pull_request) Successful in 32s
ci / frontend (Next.js on pnpm: typecheck/lint/test/build) (pull_request) Successful in 1m35s
f790435023
jonathan merged commit 57528d7473 into main 2026-10-27 22:36:43 +00:00
jonathan deleted branch po/fix-cost-counter-race 2026-10-27 22:36:43 +00:00
Sign in to join this conversation.