ci(docs): record the -n 8 worker-count trial, which is worse on every leg - #412
Merged
Merged
Conversation
… leg Comment-only. No functional change -- the three matrix values are untouched. Recorded so the next person does not spend a dispatch re-running it. The argument for raising the worker count is genuinely tempting: this suite is process-spawn heavy, so a worker blocked on a pwsh or git child is not holding a core, which reads as 4 workers under-subscribing 4 vCPU. Measured, it is wrong. leg -n 4 baseline (n=2) -n 8 (n=1) delta outcome ubuntu-latest 630 / 633s 731s +16% RED windows-2022 967 / 1004s 1067s +8% green windows-2025 1084 / 1092s 1366s +25% RED Slower everywhere and red on two of three. The failures are the mechanism rather than noise: they are exactly the tests that measure elapsed time or bind real ports, which is what contention degrades first. Trial branch claude/ci-worker-count-trial at 2bb7339, run 31968796353, cited so the raw data outlives the branch.
wshallwshall
enabled auto-merge (squash)
August 16, 2026 20:17
wshallwshall
added a commit
that referenced
this pull request
Aug 17, 2026
…t is not (#415) * feat(coord): make work durable without publishing it, and measure what is not Committing is a session's own judgment; pushing is not, because origin IS the published artifact. So a session can create work it is not permitted to make durable, and one that stops at a usage cap takes the only copy with it. Measured on this machine 2026-08-16: 802 commits across 239 branches existed on no remote at all, the oldest 17 days. Nothing reported it, because git raises no signal for correct-but-unpublished -- it conflicts on concurrent edits, never on an unpublished divergence. durability_push.sh (post-commit) pushes HEAD to a PRIVATE remote as refs/tags/rescue/auto/<branch>. That buys durability without review and without disclosure -- no PR, no auto-merge, nothing visible outside the nominated remote -- so it needs no approval, which matters because the sessions that most need it cannot stop and ask. Opt-in via mefor.durabilityRemote and a no-op when unset, so a clone, a CI checkout or a fork never pushes anywhere; it hard-refuses a URL matching the public canonical repo; it always exits 0 and pushes in the background, because a durability control that can block a commit gets disabled by the first person it inconveniences. unbacked_check.ps1 is the missing instrument. It measures per REF, never per HEAD: rev-list --count HEAD --not --remotes returned 0 for every worktree here and was read as "nothing is unbacked", while the same machine held 802 unbacked commits on branches no worktree had checked out. It probes remote reachability as a separate axis, because a repository whose origin no longer answers reported 0 unbacked across 7 refs -- refs/remotes/* are local copies and still exclude. It prints coverage on every run including a clean one, since a bare "clean" cannot be told apart from a run that examined nothing. install-git-hooks.ps1 installs the hook so it is versioned and re-installable from a known ref rather than hand-placed, and reports INSTALLED and ARMED as two different states: an installed hook with no nominated remote protects nothing. * test(coord): pin the two ways unbacked_check reported clean when it was not Every case here pins a mistake that was made, not one that was imagined. Per REF, never per HEAD: `rev-list --count HEAD --not --remotes` per worktree returned 0 for every repository on this machine and was read as "nothing is unbacked", while the same machine held 802 unbacked commits across 239 branches. The test leaves HEAD on a pushed branch and puts the work on one no checkout points at -- the ordinary state of a branch, and precisely what a per-HEAD sweep cannot see. It asserts the per-HEAD count is 0 first, so the test would pass against the broken version only if the fix were absent. Configured is not reachable: a repository whose origin no longer answers reported 0 unbacked across 7 refs, because refs/remotes/* are local copies and still exclude. That case now asserts UNVERIFIABLE, a non-zero exit, and specifically that the string "0 unbacked commits" does NOT appear -- a check that cannot tell must not print the word that means it can. Also pinned: a clean run still states its coverage, because a run that examined nothing and one that examined everything must not print the same reassuring line; NO-REMOTE is reported rather than skipped and says "needs a remote first", since that remedy differs from an ordinary unpushed branch; -SkipReachability announces itself rather than silently meaning something weaker; and a comma list through `pwsh -File` is split rather than failing with "Path does not exist". Fixtures build throwaway repositories under tmp_path and a local bare repo as the remote. Nothing touches the real .git/hooks or the live claim registry: the engine suite runs under pytest-xdist at -n 4 --dist loadfile and four workers would race shared state. No assertion depends on elapsed time, for the same reason. * docs(coord): cite a ref-independent identifier beside a sha that will not land Contributed by the CI performance session and verified here rather than taken on trust. PR #412 merged a comment into .github/workflows/ci.yml recording the -n 8 worker-count trial. Line 994 of origin/main reads: # 2026-08-16 on branch claude/ci-worker-count-trial at 2bb7339 (run 31968796353) That branch was deliberately deleted from the public remote because its content must never merge. Measured: `git ls-remote origin` returns ZERO matches for 2bb7339, and exactly three refs hold it -- one local branch on one machine, plus the rescue tag and its remote-tracking copy created by this morning's sweep. A single `git branch -D` would leave a citation on the canonical public branch resolving to nothing. The citation survives anyway, because its author named the CI run id alongside the sha and GitHub retains runs independently of any git ref. That is the practice worth generalising: when prose cites work that will deliberately not land, pair the sha with something whose retention does not depend on a git ref. The sha is exactly the half this tool exists to rescue, which makes it the wrong half to depend on alone. No behaviour change; this is guidance in the DESCRIPTION where an operator reading the tool will meet it. * docs(hooks): the safe remote is not the default one, which is the sharper trap Contributed by the tracker-board session after applying the visibility rule for the first time, and it found something the rule as written did not cover. This checkout carries two remotes that differ in KIND: origin MEFORORG/MessageFoundry PUBLIC private wshallwshall/MessageFoundry PRIVATE `git push` with no remote named resolves to `origin`. So the dangerous target is the one a hand reaches by default, and the safe one has to be typed. The rule this file already stated -- verify visibility before nominating -- guards against assuming a remote is private and being wrong. It does not guard against the case where nobody assumed anything: the wrong destination is simply what happens when no decision is made. That is also the reason this hook takes an explicitly nominated remote and treats an unset key as a no-op, rather than defaulting to one. There is no default that is safe to guess here, and guessing `origin` would publish. Measured today rather than reasoned about: five rescue tags were pushed to a public repository earlier in this pass before its visibility was checked. The content proved harmless, which was luck of selection rather than a property of the method -- the same shape of error, one layer up. * fix(coord): a branch scan cannot see a detached HEAD, and 23 of 44 were detached Third false negative this script has shipped, same shape as the first two. Scanning refs/heads answers "does any BRANCH carry unbacked commits". A worktree with a detached HEAD is on no branch, so it is structurally invisible to that scan however thorough the scan is. Detached is not an edge case here: measured 2026-08-16, 23 of one repository's 44 registered worktrees were detached, and 11 of them carried 30 commits that existed on no remote while this script reported zero. So every worktree HEAD is now checked as well, via git worktree list, and a detached one is reported against its PATH because it has no branch name to report against. Coverage counts refs AND worktree checkouts rather than a single number, because one number cannot say which of the two it meant and the gap between them is exactly where those 30 commits were. The pattern across all three defects is worth stating once, and it is written into the script: each earlier version answered a question adjacent to the one a reader would ask. Per HEAD instead of per ref; per local ref instead of per reachable remote; per branch instead of per checkout. Each fix widened what "everything" means. That this happened three times in a tool built to prevent exactly this failure is the argument for the coverage line, not against it. The test deletes the branch after committing, so the commit is reachable ONLY from the worktree's detached HEAD, and asserts first that every ref is clean -- so it fails against the previous version rather than passing for the wrong reason. Two existing assertions were updated for the new coverage wording and the singular "1 branch"; both were caught by the suite rather than by review. * fix(coord): name the primary in the coverage line, and say why a claim sweep is not this Both corrections came from a peer session that verified the tool's output rather than reading it. THE PRIMARY CHECKOUT IS A REGISTERED WORKTREE. `git worktree list` includes it, so two people counting worktrees differ by one and each believes the other has found something -- which is what happened: one session read 45 and this one read 44, and the whole gap was the primary plus a worktree created between the two scans. Checking it is correct, since the primary carries unbacked commits like any other checkout, so the coverage line now says "(incl. primary)" rather than subtracting it. A silent off-by-one in a reconciliation invites exactly the wrong conclusion. A CLAIM-BASED SWEEP IS BLIND TO THE WORKTREE MOST WORTH FINDING. This project has a coordination registry recording which checkout holds which work, and reconciling from it is the obvious shortcut. Measured 2026-08-16: a worktree created by scripts/worktree/new.ps1 held a committed-but-unpushed commit and NO claim named it, because that script does not take one. So the registry cannot see a checkout holding work nobody registered -- the exact case a sweep exists to catch. This check reads git rather than the registry and is unaffected; the header now says so, because the shortcut will look tempting again.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Comment-only change to
ci.yml. No functional change -- the threepytest_workersmatrix values are untouched.Why record a negative result
The argument for raising the xdist worker count is genuinely tempting, and someone will reach for it again: this suite is process-spawn heavy, so a worker blocked waiting on a
pwshorgitchild is not holding a core, which reads as 4 workers under-subscribing a 4-vCPU runner.Measured, it is wrong -- worse on every leg and red on two of three.
-n 4baseline (n=2)-n 8(n=1)-n 8outcomeThe
-n 4baselines are two runs agreeing within 4 percent: PR #411 and its post-merge push tomain.The failures are the mechanism, not noise
They cluster on exactly the tests that measure elapsed time or bind real ports, which is what contention degrades first:
test_outbound_batch.py's pacing floor and a super-linear-scaling ratio assertion.connscale_smoke,multishard_smoke, andsession_mail's claim-verdict test -- the three heaviest multi-process tests, the last being the slowest single test in the suite.Raw data is cited in the comment (trial branch
claude/ci-worker-count-trialat2bb733915, run31968796353) so it outlives the branch, which is being deleted.