fix: count a run completed at the DONE transition, not after it (researchx#28) #29
Reference in New Issue
Block a user
Delete Branch "po/fix-cost-counter-race"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
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
donewhile/api/configstill 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-RunActortakes an optionalon_completecallback and invokes it synchronously inside_checkpointwhen the run transitions to DONE, before that step's first await.backend/src/researchx/api/app.py- the app passesruns.note_completedas 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 ontests/test_api.py::test_run_lifecycle_over_http(1 failed / 129 passed / 2 skipped). The test waits for the run's status to readdoneand then reads/api/config;runs_completedwas 0. The cause is an ordering gap, not a flake:actor.run()sets the state to DONE inside_checkpointand 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 observedonewith 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, sha70243eeb); 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_completedis now consistent with the run's status. No endpoint, schema or manifest changes;runs_createdis 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 otherRunActor(...)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
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.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
70243eeb): backend job 45994 success, frontend job 45995 success.70243eeb): backend job 45996 success, frontend job 45999 success.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,maingets the parse fix with it and that push is the first run onmain; 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)