Skip to content

ci(test-sqlite): set the job budget from its measured p90 (#594) - #595

Merged
cdeust merged 1 commit into
mainfrom
ci/sqlite-job-budget
Sep 16, 2026
Merged

ci(test-sqlite): set the job budget from its measured p90 (#594)#595
cdeust merged 1 commit into
mainfrom
ci/sqlite-job-budget

Conversation

@cdeust

@cdeust cdeust commented Sep 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Summary

main went red at a981f935: Test (SQLite backend) hit its timeout-minutes: 10 after 616 s, at 74 % of the suite, and CI Green failed on test-sqlite='cancelled' (run 35092942280). Re-running the failed jobs turned main green with no code change, and the same tree had taken 266 s as the pull-request run of 33dc284a, the commit that merge squashed. One runner took at least 2.3× another for identical work.

Measured over that day's 21 job executions, every attempt counted: 18 successes between 234 s and 416 s, median 328 s, p90 348 s, and three non-successes. Two are real timeouts, confirmed attempt by attempt: main at 616 s, passing on its rerun in 416 s, and PR #584's first attempt at 610 s, passing on its rerun in 276 s. The third is a 47 s cancellation from a superseded push. The slowest success used 69 % of the 600 s budget, and two executions in 21 exceeded it.

The budget is now 20 minutes, 3.4× that p90, about twice the point where both timeouts fired, and one eighteenth of GitHub's 6-hour ceiling, and the workflow carries the measurement beside the number (ADR-1074). The Windows job at 15 minutes and the Python matrix at 33 to 65 minutes are untouched: they have not been observed to cancel and nothing measured here applies to them.

Closes #594

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Test plan

  • The change is one workflow value plus its rationale; the external signal is this PR's own Test (SQLite backend) run completing under the new budget.
  • .github/workflows/ci.yml parses (yaml.safe_load).
  • scripts/check_doc_claims.py: OK. scripts/check_craftsmanship.py --base origin/main: OK. scripts/check_project_wiki.py: mirrors match.
  • No source file changed, so the suite is unaffected; the last full run on this tree was 8736 passed, 8 skipped.

Audit notes

  • Splitting the job across workers would cut the wall clock instead of raising the ceiling, but the suite shares SQLite and PostgreSQL fixtures whose isolation under pytest-xdist is unproven here. That is a measurement, not a timeout decision, and ADR-1074 says so.
  • ADR-1074 records the p90 to watch, 348 s: if a future measurement puts it near 600 s, the suite has grown into the new headroom and the answer is to split the job, not to raise the number again.

Coding-standards compliance

  • §8 The number carries its measurement, in the workflow and in the ADR.
  • §9 No dead code, no TODO.

Breaking changes

None. CI Green still requires the job to succeed, so a real failure blocks a merge exactly as before; a hung suite now burns 20 minutes instead of 10 before failing.

Reviewer checklist

  • CHANGELOG.md updated under Unreleased / Fixed.
  • Documentation updated (ADR-1074).
  • No secrets or PII in the diff.
  • CI passes on the latest commit.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Cz7A6goxVzfKL9NT7k4zDH

@cdeust

cdeust commented Sep 16, 2026

Copy link
Copy Markdown
Owner Author

ZETETIC-REVIEW: REQUEST_CHANGES

Summary

The decision (raise Test (SQLite backend) from 10 to 20 minutes rather than split the job) is defensible on the evidence I independently reproduced, but the specific measurement the PR and ADR-1074 cite: "29 CI runs of 2026-09-16: 27 successes... the two non-successes are 47 s and 150 s cancellations from superseded pushes": does not reproduce against GitHub's own run history, and materially mischaracterizes a second real timeout event as a fast superseded-push cancel. Fix the numbers before merge; they are the entire justification this ADR exists to record (§8: "the number carries its measurement").

Stakes

Medium: CI-only change (workflow timeout value), no source/runtime code touched, but it gates every merge via ci-green, and it makes an empirical claim that must be checkable.

Move 0: ledger / seen-defect check

No unmapped branches; no "pre-existing/unrelated/out of scope" rationalization. Proceeds normally.

(1) Independent re-measurement (gh run list / gh api .../jobs)

gh run list --repo cdeust/Cortex --workflow ci.yml --created 2026-09-16 returns 19 workflow runs total for that UTC date (verified this is the complete CI workflow's runs for the day: no other workflow among the day's 102 total runs carries a Test (SQLite backend) job). Two of those 19 runs were rerun once each (35092942280, 35062950954), giving 21 total Test (SQLite backend) job executions, not 29.

Pulling every job execution's started_at/completed_at via gh api repos/cdeust/Cortex/actions/runs/<id>/attempts/<n>/jobs:

  • 18 successes, 234s–416s, median 327s, p90 ≈347s (close to but not the claimed 27/321/343: computed from 9 fewer runs than claimed).
  • 3 non-successes, not 2:
    • 35092942280 attempt 1 (main, a981f935): 616s cancelled: this one matches the PR's own headline claim.
    • 35071025388 attempt 1: 47s cancelled, step Install dependencies (no postgresql extra): genuinely fast, consistent with a superseded-push cancel.
    • 35062950954 attempt 1 (PR fix(launcher,wiki): install without a pip module, refuse an empty refine (#582, #583) #584): 610s cancelled, step Verify hook-free MCP host contract (a step deep into the suite, started at +9m50s into the job): this is a second real timeout, not a "150s cancellation from superseded push." No 150s-duration cancellation exists anywhere in the day's run history.

ADR-1074 itself contradicts its own "150s superseded push" framing one sentence later: "The same job was cancelled the same way on PR #584 earlier that day" (docs/adr/ADR-1074-...md:21): "the same way" as main's 616s timeout is exactly what the 610s duration shows, not a fast supersede-cancel.

(2) Is 20 minutes defensible, or should the job split?

On the corrected numbers, 20 minutes (1200s) is still defensible: arguably more so: two independent runs hit ~610-616s against the 600s cap in a sample of only 21 executions (~9.5%, not ~3.4% as the "one incident" framing implies), so the slow tail is not a one-off fluke, but 1200s still gives >3x headroom over both observed timeout events and would have passed both without masking a truly hung suite (GitHub's own 6h ceiling remains the backstop for that). I would take the same "raise the budget, don't split yet" decision the PR takes, given the unproven pytest-xdist fixture isolation the PR/ADR already flags as the reason not to split: but say so on the real incident rate (2 timeouts in ~21 runs, not 1 in 29), not the current text's.

(3) ADR-1073 / ADR-1074 collision (parallel branches, same base a981f935)

No numeric ID collision: ADR-1073 (PR #593) and ADR-1074 (PR #595) are distinct numbers. But both wiki/manifest.json diffs insert their new "ADR-10NN": {...} entry at the identical anchor point, immediately after the existing "ADR-1072" entry (verified: git diff origin/main..8239287d -- wiki/manifest.json and git diff origin/main..26eb49d3 -- wiki/manifest.json both show a 4-line insertion right after the same ADR-1072 block, no other manifest changes in either). Merging both sequentially without a rebase will produce a git merge conflict at that exact location (insert/insert at the same line): trivial to resolve (keep both keys) but not automatic.

Safe merge order: merge #593 (ADR-1073) first: it is fully green, smaller, and has no outstanding findings. Then rebase #595 (ADR-1074) onto the new main before merging; the rebase will show the wiki/manifest.json conflict directly (resolve by keeping both ADR-1073 and ADR-1074 entries), and the corrected measurement numbers (below) should go in the same rebase pass rather than as a follow-up PR.

(4) Workflow still parses / ci-green still requires this job

python -c "import yaml; yaml.safe_load(open('.github/workflows/ci.yml'))" parses cleanly. ci-green's needs: list (.github/workflows/ci.yml:1016) still includes test-sqlite, unchanged by this diff: a real failure still blocks merge exactly as before.

(5) Gates on this worktree

  • uv run python scripts/check_craftsmanship.py --base origin/main → OK.
  • uv run python scripts/check_doc_claims.py → OK (1 pre-existing declared exemption, unrelated to this diff).
  • uv run python scripts/check_project_wiki.py → "Project wiki mirrors match their canonical sources."
  • git status --short clean throughout (no edits left behind).

Issues

Blocking

  • .github/workflows/ci.yml:133-134 and docs/adr/ADR-1074-...md:21 (mirrored at wiki/adr/cortex/1074-...md:19): the cited measurement ("29 CI runs... 27 successes... two non-successes are 47s and 150s") does not reproduce against gh run list/gh api for 2026-09-16 (21 job executions found, 18 successes, and the second non-success is a 610s timeout, not 150s). Required change: re-run the measurement against reproducible gh output and correct the workflow comment and ADR body (both docs/ and wiki/ mirrors) to the actual counts, and reclassify the PR fix(launcher,wiki): install without a pip module, refuse an empty refine (#582, #583) #584 cancellation as a second timeout event rather than a superseded-push cancel. The 20-minute decision itself can stand: the correction strengthens rather than weakens the case: but the numbers as written are not what happened. (Move 0/zetetic evidence duty: a citation must match its source.)

Non-blocking

  • ADR-1074 "What to watch" (docs/adr/ADR-1074-...md, Consequences section) tracks only the p90; given two near-cap runs already observed in one day, consider also watching for recurrence of runs landing within ~2x the p90 (i.e., near the old 600s cap), since that is the actual leading indicator that was missed here.

Verdict

REQUEST_CHANGES. Correct the measurement in .github/workflows/ci.yml and both docs/adr/ADR-1074-...md / wiki/adr/cortex/1074-...md mirrors to match reproducible gh output (21 runs, 18 successes, second non-success is a 610s timeout). The timeout-minutes: 20 value and the "don't split yet" call do not need to change.

cdeust added a commit that referenced this pull request Sep 16, 2026
@cdeust @claude
ci(test-sqlite): set the job budget from its measured p90 (#594)
120a5ea 
The 10-minute budget left 1.7x headroom over this job's p90, and two runs
of 2026-09-16 exceeded it: main at a981f93, cancelled at 616s having
reached 74% of the suite, which made CI Green fail on
test-sqlite='cancelled'; and PR #584's first attempt at 610s. Both passed
on a rerun, in 416s and 276s, with no code change. The main run's own
tree had taken 266s as the pull request of the commit it squashed.

Measured over that day's 21 job executions, every attempt counted: 18
successes from 234s to 416s, median 328s, p90 348s, and three
non-successes: the two timeouts above and a 47s cancellation from a
superseded push. The budget is now 20 minutes, 3.4x that p90, about twice
the point where both timeouts fired, and one eighteenth of GitHub's own
ceiling, with the measurement recorded beside the number so suite growth
into the new headroom stays visible (ADR-1074). The Windows and matrix
budgets are untouched: nothing measured here applies to them.

An earlier draft of this commit, its ADR and its CHANGELOG entry cited 29
runs, 27 successes and a second cancellation from a superseded push. That
sample crossed several days and counted only each run's latest attempt,
so it hid the two reruns and invented the 150s cancellation. The review
of PR #595 caught it; the numbers above are the reproducible ones.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Cz7A6goxVzfKL9NT7k4zDH
@cdeust
cdeust force-pushed the ci/sqlite-job-budget branch from 26eb49d to 120a5ea Compare September 16, 2026 16:02
@cdeust

cdeust commented Sep 16, 2026

Copy link
Copy Markdown
Owner Author

Corrected, and the review was right: my sample was wrong, not just imprecise.

I had listed the last 30 CI runs regardless of date and read only each run's latest attempt, so the sample crossed several days and hid every first attempt that was rerun. That is where "29 runs, 27 successes, median 321 s, p90 343 s" and the imaginary "150 s cancellation from a superseded push" came from.

Re-measured with gh api repos/cdeust/Cortex/actions/runs/<id>/attempts/<n>/jobs, scoped to 2026-09-16, every attempt counted: 21 job executions, 18 successes from 234 s to 416 s, median 328 s, p90 348 s, and three non-successes. Two are real timeouts, confirmed attempt by attempt: main at a981f935 cancelled at 616 s and passing on its rerun in 416 s, and PR #584's first attempt cancelled at 610 s and passing on its rerun in 276 s. The third is the 47 s supersede-cancel.

So the incident rate is two timeouts in 21 executions, and the former budget left 1.7x over the p90 rather than 1.4x. The value stays at 20 minutes, now stated as 3.4x that p90 and about twice the point where both timeouts fired. Corrected in the workflow comment, both ADR-1074 copies, the CHANGELOG entry, the commit message and the PR body.

The branch is rebased onto main at 6a7e2550 (PR #593 merged first, as the review's merge order said). The wiki/manifest.json insert conflict was real and is resolved with both keys, ADR-1073 and ADR-1074, and scripts/check_project_wiki.py confirms the mirrors match.

@cdeust cdeust mentioned this pull request Sep 16, 2026
Closed
@cdeust

cdeust commented Sep 16, 2026

Copy link
Copy Markdown
Owner Author

ZETETIC-REVIEW: REQUEST_CHANGES

Summary

Re-verified PR #595 at commit 120a5ea. The measurement correction is real and independently reproducible: my own re-derivation from the raw job timestamps (gathered before this round, from the same public gh api data) matches the corrected figures exactly on every count that matters (21 job executions, 18 successes 234-416s, three non-successes at 616s/610s/47s, two of them genuine timeouts). The manifest conflict is resolved correctly, gates pass, and the workflow still parses with ci-green still requiring this job. One stale number from the earlier draft survives in both ADR-1074 mirrors and needs a follow-up fix before this is clean.

Move 0

No seen-defect rationalization anywhere in the PR body or commit message; the commit message states plainly what the earlier draft got wrong, which is the opposite of a bypass. Proceeds normally.

Independent re-verification of the corrected measurement

Recomputed from the raw started_at/completed_at timestamps I pulled via gh api repos/cdeust/Cortex/actions/runs/<id>/attempts/<n>/jobs for every one of the 19 workflow runs and their reruns on 2026-09-16 (same underlying dataset as the round-1 review, not re-fetched from the coordinator's numbers):

  • 21 total job executions, 18 successes ranging 234s-416s, median 327s (statistics.median) or 328s (index-based median, both reasonable), p90 346.6s (interpolated) or 348s (nearest-rank), matching the PR's stated 328/348 within normal percentile-method variance.
  • 3 non-successes, exactly as claimed: 35092942280 attempt 1 at 616s (main, a981f935), 35062950954 attempt 1 at 610s (PR fix(launcher,wiki): install without a pip module, refuse an empty refine (#582, #583) #584), 35071025388 attempt 1 at 47s. No 150s entry exists anywhere in the data; the "150s superseded-push cancellation" is correctly gone from every surface I checked.
  • .github/workflows/ci.yml:133-138, both docs/adr/ADR-1074-...md/wiki/adr/cortex/1074-...md context paragraphs, the CHANGELOG entry, the commit message, and the PR body all now state the same 21/18/328/348/616/610/47 figures consistently.

Stale-number sweep

Grepped the full tree at 120a5ea for 29, 27 successes, 321, 343, 150 s/150s, 1.4x/1.4×. All hits outside the ADR-1074 files are unrelated pre-existing content in other documents (numeric-equivalence-tolerance.md, phase-0.4.5-backfill-design.md, an unrelated CHANGELOG symbol count). One real hit: docs/adr/ADR-1074-the-sqlite-test-job-s-budget-is-set-from-its-measured-p90.md:23 and its wiki mirror wiki/adr/cortex/1074-the-sqlite-test-job-s-budget-is-set-from-its-measured-p90.md:21 both still read "At 1.4× the p90 it was doing the second," a leftover from the pre-correction draft. Every other surface in this same commit (workflow comment line 135, CHANGELOG.md:32, the commit message, the PR body) states the corrected 1.7x (600s / 348s ≈ 1.73). The ADR's own Context section is now internally inconsistent: it states the correct 21/18/616/610/47 figures in the sentence right before, then reverts to the pre-correction 1.4x ratio in the very next sentence.

Manifest, parse, gate checks

  • wiki/manifest.json:3463-3470: ADR-1073 and ADR-1074 entries both present, correctly resolved side by side, no truncation or duplication.
  • python -c "import yaml; yaml.safe_load(open('.github/workflows/ci.yml'))" parses cleanly.
  • ci-green's needs: list still includes test-sqlite, unchanged.
  • scripts/check_craftsmanship.py --base origin/main -> OK. scripts/check_doc_claims.py -> OK (1 pre-existing declared exemption). scripts/check_project_wiki.py -> "Project wiki mirrors match their canonical sources."
  • git status --short clean throughout; no edits made in the worktree.

Issues

Blocking

  • docs/adr/ADR-1074-the-sqlite-test-job-s-budget-is-set-from-its-measured-p90.md:23 and wiki/adr/cortex/1074-the-sqlite-test-job-s-budget-is-set-from-its-measured-p90.md:21: "At 1.4× the p90 it was doing the second" is the pre-correction ratio, inconsistent with the 1.7x figure this same commit states everywhere else (workflow comment, CHANGELOG, commit message, PR body). Required change: update both mirror copies to 1.7x (or drop the redundant restatement, since the ratio is already given in the Decision section as 3.4x for the new budget and the workflow comment already carries the 1.7x figure for the old one).

Non-blocking

None beyond the one blocking item; everything else re-verified clean.

Verdict

REQUEST_CHANGES. Fix the leftover "1.4x" in both ADR-1074 mirrors to match the corrected 1.7x used everywhere else in this commit. No other change needed: the 21/18/616/610/47 measurement, the manifest resolution, and the 20-minute value all check out against independent re-verification.

@cdeust @claude
ci(test-sqlite): set the job budget from its measured p90 (#594)
6522f45 
The 10-minute budget left 1.7x headroom over this job's p90, and two runs
of 2026-09-16 exceeded it: main at a981f93, cancelled at 616s having
reached 74% of the suite, which made CI Green fail on
test-sqlite='cancelled'; and PR #584's first attempt at 610s. Both passed
on a rerun, in 416s and 276s, with no code change. The main run's own
tree had taken 266s as the pull request of the commit it squashed.

Measured over that day's 21 job executions, every attempt counted: 18
successes from 234s to 416s, median 328s, p90 348s, and three
non-successes: the two timeouts above and a 47s cancellation from a
superseded push. The budget is now 20 minutes, 3.4x that p90, about twice
the point where both timeouts fired, and one eighteenth of GitHub's own
ceiling, with the measurement recorded beside the number so suite growth
into the new headroom stays visible (ADR-1074). The Windows and matrix
budgets are untouched: nothing measured here applies to them.

An earlier draft of this commit, its ADR and its CHANGELOG entry cited 29
runs, 27 successes and a second cancellation from a superseded push. That
sample crossed several days and counted only each run's latest attempt,
so it hid the two reruns and invented the 150s cancellation. The review
of PR #595 caught it; the numbers above are the reproducible ones.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01Cz7A6goxVzfKL9NT7k4zDH
@cdeust
cdeust force-pushed the ci/sqlite-job-budget branch from 120a5ea to 6522f45 Compare September 16, 2026 16:07
@cdeust

cdeust commented Sep 16, 2026

Copy link
Copy Markdown
Owner Author

ZETETIC-REVIEW: APPROVE

Summary

Re-verified PR #595 at commit 6522f45. The diff from 120a5ea is exactly the two-line fix requested: both ADR-1074 mirrors now read "At 1.7x the p90," matching every other surface in the commit. No stale or internally-disagreeing figure remains anywhere in the branch.

Move 0

No seen-defect rationalization in the PR body or commit history. Proceeds normally.

Diff scope

git diff 120a5ea6..6522f453 touches exactly two files, docs/adr/ADR-1074-the-sqlite-test-job-s-budget-is-set-from-its-measured-p90.md and its wiki mirror wiki/adr/cortex/1074-the-sqlite-test-job-s-budget-is-set-from-its-measured-p90.md, one line each, 1.4x to 1.7x. Nothing else changed.

Stale/derived-figure sweep

Grepped the full tree at 6522f45 for the raw-count tokens (29, 27 successes, 321, 343, 150s) and for 1.4 and 1.7x/3.4x. The only 1.4 hits left are unrelated pre-existing content, pyright 1.1.410 and an unrelated .bestpractices.json sentence. 1.7x now appears in exactly four places and all four agree: the workflow comment (ci.yml:135), CHANGELOG.md:32, docs/adr/ADR-1074-...md:23, and wiki/adr/cortex/1074-...md:21. 3.4x (the new-budget ratio) likewise appears in the same four places, all agreeing. diff docs/adr/... wiki/adr/... shows the two files are identical besides the generated-mirror header comment, as expected.

Percentile-method note

No objection to keeping the document's own median/p90 (328s/348s). My earlier 327/346.6 was interpolated; the branch's figures use a different, equally defensible method, and the underlying raw data (21 executions, 18 successes 234-416s, three non-successes at 616s/610s/47s) is what the decision rests on and is unchanged and correct. No reason to prefer one over the other, so no change requested here.

Gates (doc-only delta)

  • python -c "import yaml; yaml.safe_load(open('.github/workflows/ci.yml'))" parses cleanly.
  • scripts/check_project_wiki.py -> "Project wiki mirrors match their canonical sources."
  • scripts/check_doc_claims.py -> OK (1 pre-existing declared exemption, unrelated to this diff).
  • scripts/check_craftsmanship.py --base origin/main -> OK.
  • git status --short clean throughout; no edits made in the worktree.

Issues

Blocking

None.

Non-blocking

None.

Verdict

APPROVE. The stale ratio from round 2 is fixed, nothing else in the branch disagrees with the raw measurement, mirrors match, workflow parses, ci-green is unaffected by this doc-only delta, and gates are green.

@cdeust
cdeust merged commit 10b98d0 into main Sep 16, 2026
28 checks passed
@cdeust
cdeust deleted the ci/sqlite-job-budget branch September 16, 2026 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[bug] a slow runner turns main red: the SQLite job's 10-minute budget has 1.4x headroom over its p90

1 participant

@cdeust