diff --git a/.verify-floor.json b/.verify-floor.json index 0bb4c00..1d1c024 100644 --- a/.verify-floor.json +++ b/.verify-floor.json @@ -1,8 +1,8 @@ { - "collected": 410, - "passed": 410, + "collected": 411, + "passed": 411, "skipped_max": 26, "selftest_skipped_max": 7, "gate_skipped_max": 2, - "note": "Recorded by verify.py --update-floor, except the *_max ceilings, which are edited BY HAND and never re-measured. `collected` catches tests that stopped being collected; `passed` is compared against passed+skipped, so a check may move between passing and consciously-skipped but the two together may never shrink. The *_max ceilings bound the skipped side: 24/7/2 is exactly what a machine with none of this instance's local prerequisites skips (a GitHub runner: no agent CLIs, no ~/.codex/skills, no /Applications/ChatGPT.app, no populated capability ledger), measured 2026-08-21. On the owner's machine all prerequisites exist and nothing skips at all. Raising a ceiling is a deliberate act: it means agreeing that one more thing is allowed to go unchecked, so say which and why in the commit. LOWERED 26 -> 24 on 2026-08-22, reverting the raise made earlier the same day. The two kill-switch exemption tests no longer need to skip on a bare runner: their declarations moved out of the running instance's ledger and into capabilities.KNOWN_DECLARATIONS, so they assert code-derived truth and run everywhere. Moving a test back below the ceiling is the preferred way to lower it -- fix what made it machine-dependent, rather than agreeing to check less. FLOOR 345 -> 353 on 2026-08-22: 345 was measured on a branch cut before #13 (research panels/rounds/domain studies) merged, so the recorded floor sat 8 tests BELOW what main actually collects. A floor below reality is the permissive direction -- those 8 could have silently stopped being collected and still cleared the check, which is exactly the hole this file exists to close. Measure the floor on the merge result, not on the branch. Raised again on 2026-08-22 by the producer-identity-scope branch, which adds tests on top of the 353 recorded by #15; re-measured after rebasing rather than assumed. NOTE: `verify.py --update-floor` REPLACES this note with a generic one, so it must be restored by hand after every use \u2014 the ceiling rationale is the only record of which prerequisite justifies each skip. FLOOR 365 -> 366 on 2026-08-22 (heartbeat-ordering work, PR #18): exactly one new test, test_capabilities.test_no_tick_producer_runs_above_the_heartbeat_export. No ceiling moved and nothing new is skipped -- it reads source files rather than a populated ledger, so it runs on any machine. The branch recorded 354 because it was cut before #16 merged; re-measured on the MERGE RESULT per the rule above, which is exactly the mistake that put the floor 8 below reality last time. FLOOR 366 -> 368 on 2026-08-23: main collected 368 while this file recorded 366, drift left by #34 (evidence-acquisition landed, +1) and #37 (tick capability evidence, +1) whose authors each measured against a branch cut before the other merged. A floor BELOW reality is the permissive direction this file exists to close -- those two could have silently stopped being collected and still cleared the check. Measured on the merge result per the rule above: 368 passed, 0 failed, 0 skipped, 83/83 selftests, 43/43 can-fire, 5/5 gates. CEILING 24 -> 26 and FLOOR 368 -> 387 on 2026-08-23 (profiles/provenance branch, PR #42). This file CONFLICTED with #50, which raised the floor 366 -> 368 on main while this branch raised it to 387; resolved as the UNION rather than by taking a side -- #50's rationale is retained above and the count was RE-MEASURED on the new merge result instead of keeping either number. 368 (main) + 19 (this branch's net new tests) = 387; #50 corrected recorded drift rather than adding coverage, which is why 387 is unchanged from the pre-conflict measurement. Measured in a runner sandbox reproducing CI exactly (361 passed, 26 skipped, 387 collected) AND on the owner's machine (387 passed, 0 skipped, 5/5 gates). The two new skips are drift detectors against a REAL installed agent runtime, so neither can be moved below the ceiling -- the preferred way to lower one: (1) agy advertised-models cache absent, since comparing declared model ids against the catalogue agy actually advertises needs that catalogue, and a fixture would exercise the comparison while detecting no real drift; (2) vibe config absent (~/.vibe/config.toml), since active_model cannot be read to check for drift when there is no config to read. Both name their missing prerequisite, so a green run still states what it did not check. A third candidate skip was REFUSED: dispatcher's per-run agy-log assertion failed on a bare runner because adapters.advertised_models shells out to `agy models` when its disk cache is cold, and that probe landed inside a monkeypatched subprocess.run and overwrote the captured command. That is a stub leak, so it was fixed by ISOLATING the double rather than by skipping -- which makes CI run MORE. FLOOR 387 -> 391 on 2026-08-23 (improvement-log accessor, PR #59): exactly four new tests, all in test_improvement_log.py -- three read tracked files in the tree (the pointer's size and content, and that CLAUDE.md 0 step 3 and 5 name the accessor rather than a bare path) and one runs the accessor as a subprocess against a path that cannot exist. None reads a populated ledger, an agent CLI or ~/.codex, so all four RUN on a bare runner and NO ceiling moved: nothing new is skipped. Measured on the MERGE RESULT after rebasing onto origin/main af6654d, which collected 387 -- not on the branch base, per the rule above. FLOOR 391 -> 402 on 2026-08-23 (Gate python-ci configuration, the PR that adds the missing .github/workflows/autofix-versions.env): exactly 11 new tests, all in test_ci_gate_config.py, which read committed files only -- the pin file, ruff.toml, mypy.ini, pr-00-gate.yml's toggle annotations and docs/CI_LINT_BASELINE.md. NO ceiling moved. On any CHECKOUT -- CI, the owner's tree, a second instance -- all 11 run: they need no installed linter and no populated ledger. In the EXEC-MIRROR layout all 11 skip with one named reason, because orch-sync-mirror.sh copies root-level *.py only, so .github/workflows, docs/ and scripts/ are genuinely absent there (env_prereq.repo_files_absent). That lands at 11/26 on a machine that otherwise skips nothing, and CI stays at 26/26, so no ceiling needed raising. The skip gate is the presence of those DIRECTORIES, never of the pin file itself -- gating on the file would have made the test that checks for it unable to fail. Measured on the merge result, twice: the branch was rebuilt on origin/main after #42 and #59 merged, and re-measured after #61 merged and was merged in -- 393 passed + 9 skipped = 402 collected both times, so #61 added no collected tests and this floor is not sitting below reality. #61 itself left main's floor at 391, which is exactly main-without-these-11, so there is no inherited drift to correct. RULE CHANGE 2026-08-23: `collected` is now an EQUALITY, not a minimum. Every floor entry above this one records the number being found BELOW reality and hand-raised after the fact -- 21 low at the worst, then 8, then 1, then 2 -- because nothing ever required a test-adding PR to touch this file, so the permissive direction was silent by construction and the rule 'measure on the merge result' had to be restated three times with nothing enforcing it. verify.py now FAILS when collected exceeds the floor, printing the two integers to write. That also makes the concurrency case self-enforcing: once every test-adding branch must edit these same two lines, two concurrent branches CONFLICT IN GIT, so the second cannot merge without rebasing onto the first and re-measuring on the actual merge result. Demonstrated repeatedly on the change itself: six merges landed on main in the two hours it took to write, moving this file 368 -> 387 -> 391 -> 402, and every one would have left the floor below reality under the old one-directional rule. `passed` deliberately stays a MINIMUM on passed+skipped: only collection is machine-invariant (a skipped test is still collected), measured across machines at 391 collected on both, with pass/skip splits of 365/26 on CI against 391/0 locally. The *_max ceilings are untouched by this change and nothing new is skipped. `--update-floor` also stops REPLACING this note -- it appends -- so the warning above about restoring it by hand no longer applies; and drift does NOT block --update-floor, since a gate that forbade its own only remedy would be a deadlock (the first draft was exactly that). FLOOR 402 -> 407 on 2026-08-23 (findability admission requirement). (findability admission requirement). (findability admission requirement). (findability admission requirement). Exactly five new pytest tests, all in test_capability_admission.py: test_findability_distinguishes_its_three_sub_causes, test_findability_blocks_new_capabilities_and_reports_older_ones_as_debt, test_unreadable_reach_is_not_evaluated_and_never_a_failure, test_findability_exemption_is_declared_in_code_not_in_a_live_ledger, test_consult_sites_are_falsifiable_claims_about_real_callers. NO CEILING MOVED and nothing new skips: all five build synthetic ledgers in a tempdir or read committed tables, so none needs a populated capability ledger, an agent CLI or ~/.claude/skills. The one machine-dependent thing they touch -- an external consult site declared in capability_advisor.CONSULT_SITES whose skill prompt is not on this machine -- is reported as UNVERIFIED rather than skipped, because absence of the caller is not refutation of the claim; the in-tree site (tick) is asserted verified on every machine so the check can never degrade into 'everything unverified, nothing tested'. Measured on the merge result per the rule above: this file CONFLICTED three times while the branch was open, as main went 387 -> 391 -> 402 (#61, #64, #65, #60). Each time it was resolved as the UNION rather than by taking a side, and the count was RE-MEASURED on the new merge result rather than either number being carried forward: 402 (main at bd6da2e) + 5 (this branch's new tests) = 407. That is the rule this file already states -- measure the floor on the merge result, not on the branch -- and it mattered here, because #60 both deleted test_ci_gate_config.py and added more than it removed, so guessing in either direction would have been wrong. -> re-measured on 2026-08-23 (PR #62, the four deferred #42 review findings): three new tests, all machine-independent (each builds its own tmp_path Brain and manifests), so NO CEILING MOVED and nothing new is skipped. Fourth conflict for this branch, and the first one under the EQUALITY -- which is the point: the equality's own rationale says git conflict detection is what enforces 'measure on the merge result', and that is exactly what happened here. Under the old minimum the three earlier conflicts could each have been resolved by keeping the larger number; under the equality the count MUST be measured, and it was. RESOLVED AGAINST #68 (findability admission requirement) on 2026-08-23: taken as the UNION per the rule this file states -- #68's five-test entry is retained above and this branch's three-test entry beside it -- and the count RE-MEASURED on the merge result rather than keeping either side's number. main fc1fd42 collects 407; this branch adds 3; 410 measured with `pytest --collect-only -q` on the merge result, not assumed. Ceilings untouched at 26/7/2 and nothing new is skipped. Also resolved in the same merge: langsmith-fleet-worker-attempt.json, a CI-emitted `langsmith-fleet/v1` worker-attempt record whose two sides differed only in `emitted_at` and `pr_number` (62 here, 68 on main). Main's NEWER record was kept rather than this branch's older one -- discarding a newer provenance observation to win a merge would corrupt exactly the causal-provenance evidence CLAUDE.md 2 protects, and this branch's own run re-emits its record anyway." + "note": "Recorded by verify.py --update-floor, except the *_max ceilings, which are edited BY HAND and never re-measured. `collected` catches tests that stopped being collected; `passed` is compared against passed+skipped, so a check may move between passing and consciously-skipped but the two together may never shrink. The *_max ceilings bound the skipped side: 24/7/2 is exactly what a machine with none of this instance's local prerequisites skips (a GitHub runner: no agent CLIs, no ~/.codex/skills, no /Applications/ChatGPT.app, no populated capability ledger), measured 2026-08-21. On the owner's machine all prerequisites exist and nothing skips at all. Raising a ceiling is a deliberate act: it means agreeing that one more thing is allowed to go unchecked, so say which and why in the commit. LOWERED 26 -> 24 on 2026-08-22, reverting the raise made earlier the same day. The two kill-switch exemption tests no longer need to skip on a bare runner: their declarations moved out of the running instance's ledger and into capabilities.KNOWN_DECLARATIONS, so they assert code-derived truth and run everywhere. Moving a test back below the ceiling is the preferred way to lower it -- fix what made it machine-dependent, rather than agreeing to check less. FLOOR 345 -> 353 on 2026-08-22: 345 was measured on a branch cut before #13 (research panels/rounds/domain studies) merged, so the recorded floor sat 8 tests BELOW what main actually collects. A floor below reality is the permissive direction -- those 8 could have silently stopped being collected and still cleared the check, which is exactly the hole this file exists to close. Measure the floor on the merge result, not on the branch. Raised again on 2026-08-22 by the producer-identity-scope branch, which adds tests on top of the 353 recorded by #15; re-measured after rebasing rather than assumed. NOTE: `verify.py --update-floor` REPLACES this note with a generic one, so it must be restored by hand after every use \u2014 the ceiling rationale is the only record of which prerequisite justifies each skip. FLOOR 365 -> 366 on 2026-08-22 (heartbeat-ordering work, PR #18): exactly one new test, test_capabilities.test_no_tick_producer_runs_above_the_heartbeat_export. No ceiling moved and nothing new is skipped -- it reads source files rather than a populated ledger, so it runs on any machine. The branch recorded 354 because it was cut before #16 merged; re-measured on the MERGE RESULT per the rule above, which is exactly the mistake that put the floor 8 below reality last time. FLOOR 366 -> 368 on 2026-08-23: main collected 368 while this file recorded 366, drift left by #34 (evidence-acquisition landed, +1) and #37 (tick capability evidence, +1) whose authors each measured against a branch cut before the other merged. A floor BELOW reality is the permissive direction this file exists to close -- those two could have silently stopped being collected and still cleared the check. Measured on the merge result per the rule above: 368 passed, 0 failed, 0 skipped, 83/83 selftests, 43/43 can-fire, 5/5 gates. CEILING 24 -> 26 and FLOOR 368 -> 387 on 2026-08-23 (profiles/provenance branch, PR #42). This file CONFLICTED with #50, which raised the floor 366 -> 368 on main while this branch raised it to 387; resolved as the UNION rather than by taking a side -- #50's rationale is retained above and the count was RE-MEASURED on the new merge result instead of keeping either number. 368 (main) + 19 (this branch's net new tests) = 387; #50 corrected recorded drift rather than adding coverage, which is why 387 is unchanged from the pre-conflict measurement. Measured in a runner sandbox reproducing CI exactly (361 passed, 26 skipped, 387 collected) AND on the owner's machine (387 passed, 0 skipped, 5/5 gates). The two new skips are drift detectors against a REAL installed agent runtime, so neither can be moved below the ceiling -- the preferred way to lower one: (1) agy advertised-models cache absent, since comparing declared model ids against the catalogue agy actually advertises needs that catalogue, and a fixture would exercise the comparison while detecting no real drift; (2) vibe config absent (~/.vibe/config.toml), since active_model cannot be read to check for drift when there is no config to read. Both name their missing prerequisite, so a green run still states what it did not check. A third candidate skip was REFUSED: dispatcher's per-run agy-log assertion failed on a bare runner because adapters.advertised_models shells out to `agy models` when its disk cache is cold, and that probe landed inside a monkeypatched subprocess.run and overwrote the captured command. That is a stub leak, so it was fixed by ISOLATING the double rather than by skipping -- which makes CI run MORE. FLOOR 387 -> 391 on 2026-08-23 (improvement-log accessor, PR #59): exactly four new tests, all in test_improvement_log.py -- three read tracked files in the tree (the pointer's size and content, and that CLAUDE.md 0 step 3 and 5 name the accessor rather than a bare path) and one runs the accessor as a subprocess against a path that cannot exist. None reads a populated ledger, an agent CLI or ~/.codex, so all four RUN on a bare runner and NO ceiling moved: nothing new is skipped. Measured on the MERGE RESULT after rebasing onto origin/main af6654d, which collected 387 -- not on the branch base, per the rule above. FLOOR 391 -> 402 on 2026-08-23 (Gate python-ci configuration, the PR that adds the missing .github/workflows/autofix-versions.env): exactly 11 new tests, all in test_ci_gate_config.py, which read committed files only -- the pin file, ruff.toml, mypy.ini, pr-00-gate.yml's toggle annotations and docs/CI_LINT_BASELINE.md. NO ceiling moved. On any CHECKOUT -- CI, the owner's tree, a second instance -- all 11 run: they need no installed linter and no populated ledger. In the EXEC-MIRROR layout all 11 skip with one named reason, because orch-sync-mirror.sh copies root-level *.py only, so .github/workflows, docs/ and scripts/ are genuinely absent there (env_prereq.repo_files_absent). That lands at 11/26 on a machine that otherwise skips nothing, and CI stays at 26/26, so no ceiling needed raising. The skip gate is the presence of those DIRECTORIES, never of the pin file itself -- gating on the file would have made the test that checks for it unable to fail. Measured on the merge result, twice: the branch was rebuilt on origin/main after #42 and #59 merged, and re-measured after #61 merged and was merged in -- 393 passed + 9 skipped = 402 collected both times, so #61 added no collected tests and this floor is not sitting below reality. #61 itself left main's floor at 391, which is exactly main-without-these-11, so there is no inherited drift to correct. RULE CHANGE 2026-08-23: `collected` is now an EQUALITY, not a minimum. Every floor entry above this one records the number being found BELOW reality and hand-raised after the fact -- 21 low at the worst, then 8, then 1, then 2 -- because nothing ever required a test-adding PR to touch this file, so the permissive direction was silent by construction and the rule 'measure on the merge result' had to be restated three times with nothing enforcing it. verify.py now FAILS when collected exceeds the floor, printing the two integers to write. That also makes the concurrency case self-enforcing: once every test-adding branch must edit these same two lines, two concurrent branches CONFLICT IN GIT, so the second cannot merge without rebasing onto the first and re-measuring on the actual merge result. Demonstrated repeatedly on the change itself: six merges landed on main in the two hours it took to write, moving this file 368 -> 387 -> 391 -> 402, and every one would have left the floor below reality under the old one-directional rule. `passed` deliberately stays a MINIMUM on passed+skipped: only collection is machine-invariant (a skipped test is still collected), measured across machines at 391 collected on both, with pass/skip splits of 365/26 on CI against 391/0 locally. The *_max ceilings are untouched by this change and nothing new is skipped. `--update-floor` also stops REPLACING this note -- it appends -- so the warning above about restoring it by hand no longer applies; and drift does NOT block --update-floor, since a gate that forbade its own only remedy would be a deadlock (the first draft was exactly that). FLOOR 402 -> 407 on 2026-08-23 (findability admission requirement). (findability admission requirement). (findability admission requirement). (findability admission requirement). Exactly five new pytest tests, all in test_capability_admission.py: test_findability_distinguishes_its_three_sub_causes, test_findability_blocks_new_capabilities_and_reports_older_ones_as_debt, test_unreadable_reach_is_not_evaluated_and_never_a_failure, test_findability_exemption_is_declared_in_code_not_in_a_live_ledger, test_consult_sites_are_falsifiable_claims_about_real_callers. NO CEILING MOVED and nothing new skips: all five build synthetic ledgers in a tempdir or read committed tables, so none needs a populated capability ledger, an agent CLI or ~/.claude/skills. The one machine-dependent thing they touch -- an external consult site declared in capability_advisor.CONSULT_SITES whose skill prompt is not on this machine -- is reported as UNVERIFIED rather than skipped, because absence of the caller is not refutation of the claim; the in-tree site (tick) is asserted verified on every machine so the check can never degrade into 'everything unverified, nothing tested'. Measured on the merge result per the rule above: this file CONFLICTED three times while the branch was open, as main went 387 -> 391 -> 402 (#61, #64, #65, #60). Each time it was resolved as the UNION rather than by taking a side, and the count was RE-MEASURED on the new merge result rather than either number being carried forward: 402 (main at bd6da2e) + 5 (this branch's new tests) = 407. That is the rule this file already states -- measure the floor on the merge result, not on the branch -- and it mattered here, because #60 both deleted test_ci_gate_config.py and added more than it removed, so guessing in either direction would have been wrong. -> re-measured on 2026-08-23 (PR #62, the four deferred #42 review findings): three new tests, all machine-independent (each builds its own tmp_path Brain and manifests), so NO CEILING MOVED and nothing new is skipped. Fourth conflict for this branch, and the first one under the EQUALITY -- which is the point: the equality's own rationale says git conflict detection is what enforces 'measure on the merge result', and that is exactly what happened here. Under the old minimum the three earlier conflicts could each have been resolved by keeping the larger number; under the equality the count MUST be measured, and it was. RESOLVED AGAINST #68 (findability admission requirement) on 2026-08-23: taken as the UNION per the rule this file states -- #68's five-test entry is retained above and this branch's three-test entry beside it -- and the count RE-MEASURED on the merge result rather than keeping either side's number. main fc1fd42 collects 407; this branch adds 3; 410 measured with `pytest --collect-only -q` on the merge result, not assumed. Ceilings untouched at 26/7/2 and nothing new is skipped. Also resolved in the same merge: langsmith-fleet-worker-attempt.json, a CI-emitted `langsmith-fleet/v1` worker-attempt record whose two sides differed only in `emitted_at` and `pr_number` (62 here, 68 on main). Main's NEWER record was kept rather than this branch's older one -- discarding a newer provenance observation to win a merge would corrupt exactly the causal-provenance evidence CLAUDE.md 2 protects, and this branch's own run re-emits its record anyway. FLOOR 410 -> 411 on 2026-08-23 (CodeRabbit follow-up on PR #42, thread 3837879039; re-measured again after #56 made `collected` an EQUALITY, which makes an assumed number a hard RED rather than a quiet pass -- main stayed at 402 across #56, and the merge result measures 403, so #56 added no collected tests and this is main's 402 plus this branch's one): exactly one new test, test_feedback_model_provenance.test_late_sweep_completes_terminal_attempts_never_one_in_flight, which pins that ledger_reconcile.resolve_unresolved_worker_attempts completes only TERMINAL unresolved worker attempts and never one still in flight. No ceiling moved and nothing new is skipped -- the test builds its own tmp ledger and codex rollout fixture and monkeypatches adapters.CODEX_SESSIONS, so it needs no agent CLI and no populated capability ledger and runs on a bare runner. RESOLVED AGAINST #59 (improvement-log accessor), which raised the floor 387 -> 391 on main while this branch raised it to 388: taken as the UNION -- #59's rationale is retained above and the count was RE-MEASURED on the new merge result rather than keeping either number, which is the rule this file states and the mistake that once put the floor 8 below reality. 391 (main, incl. #59's four tests) + 1 (this branch's one new test) = 392 measured, not assumed: 392 passed, 0 failed, 0 skipped, 83/83 selftests, 43/43 can-fire, 5/5 gates. Three sibling follow-up branches are in flight against this same main (CI/ruff config, arm-attribution + durability, adapters label->ID); if this file conflicts with one of them, resolve as the UNION and RE-MEASURE on the new merge result rather than taking either number -- that is what #42 and #50 did, and taking a side is what put the floor 8 below reality earlier. RESOLVED AGAINST #68 (findability admission requirement) on 2026-08-23: taken as the UNION per the rule this file states -- #68's five-test entry is retained above and this branch's one-test entry beside it -- and the count RE-MEASURED on the merge result. main fc1fd42 collects 407; this branch adds 1; 408 measured with `pytest --collect-only -q` on the merge result, not assumed. Ceilings untouched at 26/7/2 and nothing new is skipped -- the one new test builds its own tmp ledger and codex rollout fixture, so it runs on a bare runner. Also resolved in the same merge: langsmith-fleet-worker-attempt.json, a CI-emitted `langsmith-fleet/v1` worker-attempt record differing only in `emitted_at` and `pr_number`; main's NEWER record was kept, since discarding a newer provenance observation to win a merge would corrupt the causal-provenance evidence CLAUDE.md 2 protects." } diff --git a/ledger_reconcile.py b/ledger_reconcile.py index 3ae4452..6886c70 100644 --- a/ledger_reconcile.py +++ b/ledger_reconcile.py @@ -270,15 +270,50 @@ def resolve_unresolved_worker_attempts(*, apply: bool = False, limit: int = 5000 keeps no per-session log stays unresolved with its reason NAMED, and nothing is ever inferred from the requested model. Dry-run by default; reports the per-agent breakdown either way, so "resolved 37" always arrives next to "16 cannot report and here is why". + + Only TERMINAL attempts are eligible: `status='unresolved'`. An in-flight (`started`) + attempt is never completed here, and neither is one that never ran (`failed`) — see the + eligibility comment below for why each exclusion is required and why neither starves the + drain. """ + # TERMINAL ROWS ONLY -- `status='unresolved'` is the whole eligibility rule, and it is doing two + # jobs. A worker attempt's profile row is written `started` BEFORE the subprocess is spawned + # (`dispatcher`/`exp_abcd` pre-dispatch), so an IN-FLIGHT attempt matches every other clause + # here: role worker, profile set, resolved_model still NULL. `cli_reported_model` reads the + # FIRST model in the session log within a 2h window of `started_ts`, and that log exists as soon + # as the CLI starts -- so a run still executing probes clean, and `--apply` stamped it + # `complete` with a resolved model and a `completed_ts` of the sweep's own clock. That is the + # one row shape allowed to support an exact-model claim, manufactured for a worker that had not + # finished and could still fall back, retry onto another model, or fail outright. + # It also excludes `failed` (dispatcher's `profile_process_start_failed`), which is terminal but + # never ran: there is no served model to recover, so resolving it from a neighbouring session in + # the window would be pure invention. + # Not a starved drain (the trap this repo keeps falling into): a `started` row is excluded only + # while it is in flight. Its own completion closes it to `complete` (resolved -- no sweep + # needed) or `unresolved` (eligible on the next pass), so the exclusion clears itself without + # the sweep's help. Measured on the live ledger when this filter landed: 56 candidates before, + # 56 after -- every genuinely drainable row is already `unresolved`. with feedback._conn() as c: rows = c.execute( "SELECT ea.run_id, ea.profile_id, r.agent, r.target, r.ts " "FROM execution_attempts ea JOIN runs r ON r.run_id=ea.run_id " - "WHERE ea.operation_role='worker' AND ea.resolved_model IS NULL " + "WHERE ea.operation_role='worker' AND ea.status='unresolved' " + "AND ea.resolved_model IS NULL " "AND ea.profile_id IS NOT NULL ORDER BY r.ts DESC LIMIT ?", (int(limit),), ).fetchall() + # Counted, not silently narrowed. `candidates: 0` beside `not_terminal: {started: 3}` reads + # as "wait for those runs to finish"; `candidates: 0` alone reads as "the sweep is broken". + not_terminal = { + str(status or ""): int(count) + for status, count in c.execute( + "SELECT ea.status, COUNT(*) " + "FROM execution_attempts ea JOIN runs r ON r.run_id=ea.run_id " + "WHERE ea.operation_role='worker' AND ea.resolved_model IS NULL " + "AND ea.profile_id IS NOT NULL " + "AND (ea.status IS NULL OR ea.status<>'unresolved') GROUP BY ea.status" + ).fetchall() + } resolved: dict[str, int] = {} blocked: dict[str, int] = {} failed: dict[str, str] = {} @@ -349,6 +384,9 @@ def resolve_unresolved_worker_attempts(*, apply: bool = False, limit: int = 5000 # Reported beside it, never inside it: rows excluded because the seat can never report. # Naming them keeps the exclusion auditable, and keeps `candidates` an honest backlog. "excluded_unreportable": unreportable, + # Rows excluded as not-terminal, keyed by the status that excluded them. `started` clears + # itself when the run completes; `failed` never ran and is permanently and correctly out. + "excluded_not_terminal": not_terminal, "failed": failed, } diff --git a/test_feedback_model_provenance.py b/test_feedback_model_provenance.py index 23e9ab6..55da99c 100644 --- a/test_feedback_model_provenance.py +++ b/test_feedback_model_provenance.py @@ -837,3 +837,159 @@ def test_every_seat_with_a_session_store_can_report_and_is_read_from_its_own_sto ) == "Gemini 3.6 Flash (High)" ) + + +def test_late_sweep_completes_terminal_attempts_never_one_in_flight(tmp_path, monkeypatch): + """The sweep may finish a TERMINAL unresolved attempt; it must not finish a running one. + + The profile attempt row is written `started` BEFORE the subprocess is spawned, so an in-flight + attempt matches every other clause of the sweep's query -- worker role, profile set, + `resolved_model` still NULL. `cli_reported_model` reads the first model in the session log + within a 2h window of `started_ts`, and that log exists from the moment the CLI starts, so a + run that is still executing probes CLEAN. Before the status filter, `--apply` stamped it + `complete` with a resolved model and a `completed_ts` taken from the sweep's own clock: the one + row shape allowed to support an exact-model claim, minted for a worker that had not finished and + could still fall back, retry onto another model, or fail outright. + + All three runs below share one workspace, so the probe resolves for ALL of them. The status + filter is therefore the only thing that can protect the two ineligible rows. + """ + import json + + import adapters + import ledger_reconcile + + old_db = feedback.DB_PATH + feedback.DB_PATH = tmp_path / "feedback.db" + try: + workspace = tmp_path / "offloads" / "ws-sweep" + workspace.mkdir(parents=True) + sessions = tmp_path / "sessions" + (sessions / "2026").mkdir(parents=True) + started = int(time.time()) + stamp = time.strftime("%Y-%m-%dT%H-%M-%S", time.localtime(started)) + (sessions / "2026" / f"rollout-{stamp}-x.jsonl").write_text( + json.dumps({"type": "session_meta", "payload": {"cwd": str(workspace.resolve())}}) + + "\n" + + json.dumps({"type": "turn_context", "payload": {"model": "gpt-5.6-terra"}}) + + "\n" + ) + monkeypatch.setattr(adapters, "CODEX_SESSIONS", sessions) + + def _pending(run_id): + """A pre-dispatch worker attempt: exactly what the dispatcher writes before spawning.""" + feedback.record_run(run_id, f"offload:{workspace}", "offload", "codex") + feedback.record_execution_attempt( + run_id, + attempt_id=f"attempt:profile:{run_id}", + operation_role="worker", + profile_id="codex-5.6-terra-high", + requested_provider="openai", + requested_model="gpt-5.6-terra", + status="started", + source="orchestrator-profile-decision", + started_ts=started, + ) + + def _row(run_id): + return ( + sqlite3.connect(feedback.DB_PATH) + .execute( + "SELECT status, resolved_model, completed_ts FROM execution_attempts " + "WHERE run_id=?", + (run_id,), + ) + .fetchone() + ) + + # TERMINAL: the completion path closed it unresolved. This is the sweep's whole purpose. + _pending("sweep-terminal") + feedback.complete_profile_attempt_unresolved( + "sweep-terminal", + selected_profile_id="codex-5.6-terra-high", + fallback_reason="resolved_model_not_reported_by_completion", + ) + # IN FLIGHT: the worker is still running, so nothing has closed it. + _pending("sweep-inflight") + # TERMINAL BUT NEVER RAN: the process failed to start, so no model ever served it. + _pending("sweep-failed") + feedback.complete_profile_attempt_unresolved( + "sweep-failed", + selected_profile_id="codex-5.6-terra-high", + fallback_reason="profile_process_start_failed", + status="failed", + ) + + report = ledger_reconcile.resolve_unresolved_worker_attempts(apply=True) + + # The terminal unresolved row gains the identity its log always carried. + assert _row("sweep-terminal")[:2] == ("complete", "gpt-5.6-terra") + # The in-flight row is untouched: no resolved model, no invented completion timestamp. + assert _row("sweep-inflight") == ("started", None, None) + # The never-ran row keeps its terminal failure rather than borrowing a neighbour's model. + assert _row("sweep-failed")[:2] == ("failed", None) + assert report["resolved_by_agent"] == {"codex": 1} + # `candidates` counts only what a reader could still drain. + assert report["candidates"] == 1 + # The exclusions are NAMED, not silently narrowed away: `candidates: 0` next to + # `{"started": 1}` reads as "wait for that run"; `candidates: 0` alone reads as "broken". + assert report["excluded_not_terminal"] == {"started": 1, "failed": 1} + + # DELIBERATE BREAK: drop the status clause, restoring the pre-fix query exactly. + # + # This is keyed on an EXACT substring, trailing space included, so it has to fail loudly + # when that substring stops matching. A reformatted query would make `replace` a silent + # no-op: the filter would keep protecting the row, and the corruption assertion below would + # fail while blaming the fix rather than this fixture. Two guards, because there are two + # ways to go stale -- the clause moves within a query still recognisable (caught per + # statement), or the query itself becomes unrecognisable so nothing is ever stripped + # (caught by `stripped` after the run). Only the second covers an aliased or re-ordered + # rewrite, which is why the per-statement assert alone is not enough. + real_conn = feedback._conn + clause = "AND ea.status='unresolved' " + stripped = [] + + class _Unfiltered: + def __init__(self, c): + self._c = c + + def execute(self, sql, *a): + # The candidate query is the only statement carrying the clause. Identify it by + # the FROM/JOIN and ORDER BY fragments -- the not-terminal count query shares the + # FROM/JOIN but ends in GROUP BY, so it is not mistaken for the candidate. + if "FROM execution_attempts ea JOIN runs r" in sql and "ORDER BY r.ts DESC" in sql: + assert clause in sql, ( + f"deliberate break is STALE: the candidate query no longer contains " + f"{clause!r}, so stripping it does nothing and the corruption assertion " + f"below would blame the fix. Update this fixture. SQL: {sql!r}" + ) + stripped.append(sql) + return self._c.execute(sql.replace(clause, ""), *a) + return self._c.execute(sql, *a) + + def __getattr__(self, name): + return getattr(self._c, name) + + def __enter__(self): + self._c.__enter__() + return self + + def __exit__(self, *exc): + return self._c.__exit__(*exc) + + monkeypatch.setattr(feedback, "_conn", lambda: _Unfiltered(real_conn())) + ledger_reconcile.resolve_unresolved_worker_attempts(apply=True) + assert stripped, ( + "deliberate break never fired: nothing matched the candidate query's FROM/JOIN + " + "ORDER BY signature, so no clause was stripped and the assertions below prove " + "nothing. Update this fixture to match the current query." + ) + broken_status, broken_model, broken_completed = _row("sweep-inflight") + assert (broken_status, broken_model) == ("complete", "gpt-5.6-terra"), ( + "the break must reproduce the corruption: a running worker stamped with an " + "exact resolved model" + ) + assert broken_completed is not None, "and with a completion timestamp it never earned" + # REVERTED by monkeypatch teardown; the filtered assertions above are the guard. + finally: + feedback.DB_PATH = old_db