Twinned from https://github.com/cfsh/ninefold/issues/984 by tools/beads/import_github.py's reconcile pass.
Loop step 4 tells every session to rebuild f5 after every dev.py pr and after
every merge. For a tooling PR — the majority case; affected_probes.py's
docstring cites the #847 measurement that 43% of merges are Godot-invisible and
most of the rest touch only tools/ — that rebuild changes **nothing Aedan
plays**, yet still pays the full verify phase in rebuild_f5.py: `dotnet
build Ninefold.sln` + the whole xUnit suite + a headless Godot import. That is
10–12s on an idle box and minutes when several sessions compete — and
tooling-heavy stretches are exactly when they do.
The result is that the decision "does *my* change need an f5 rebuild?" lives in
each session's head, steered only by .md guidance. Sessions either pay the
minutes pointlessly or quietly skip the step and drift.
The obvious reading of this issue — "only rebuild for games changes" — is wrong as an implementation, and rejecting it is part of the spec:
f5/integration is *defined* as origin/main + every open PR head. If a
tooling merge advances main and nobody rebuilds:
* f5.py's warn_if_behind_main asks whether origin/main is an ancestor of
the branch. It is not, so Aedan gets a false BEHIND main warning on every
launch.
* rebuild_f5.py --health (f5_status) counts *every* commit main has that f5
lacks, so it cries STALE-F5 forever — and poll.py surfaces that to the
f5/integration owner as an outage.
* The -f5 worktree's *tools* go stale, and f5.py itself lives on that
branch.
Teaching all three consumers the Godot-visible distinction is strictly more surface (and more drift risk) than making the rebuild itself cheap. **Keep the rebuild unconditional; make the expensive part conditional.**
Reuse design 005's classifier — nflib.godot_build.changed_paths +
godot_inert — exactly as probe_gate's #637 fast-skip and rebuild_f5's own
#599 hold-out already do. Do not write a new path classifier; the whole
point of godot_build.py is that there is one, and BuildPremiseTest pins it
to what Ninefold.csproj and .gdignore actually say.
rebuild_f5.py: fast verify when nothing Godot-visible changedIn integrate(), after the merge loop and the provenance stamp, immediately
before verify(tree_path, runner):
```python visible = changed_paths(tree, "origin/f5/integration", "HEAD") ```
* Two shas, tree against tree — the same contract as
affected_probes.py and probe_gate.sweep_input_changed, and for the same
reason: three dots is merge-base-relative and would hide what the old
integration carried and the new one does not.
* Filter with godot_inert. If every changed path is inert → run the **fast
verify**: dotnet build only, skipping the test suite and the headless
import. Print the decision loudly, e.g.
→ Verify (fast): nothing Godot-visible changed since <short-sha> — build only.
* Otherwise (any visible path, changed_paths returned None, or
origin/f5/integration does not exist / is unreadable) → the full verify,
unchanged. Every ambiguity fails toward verifying, mirroring
affected_probes.py's unknown-diff rule: "I could not tell" must never
become "don't check".
* Add --full-verify to force the full phase regardless (the
RUN_PROBE_GATE=1 idea, as a flag).
Why each piece of the split is sound:
* Headless import reads only what the Godot build can see — that is
godot_inert's literal definition — so an unchanged visible tree cannot
change its outcome. Safe to skip.
* dotnet build is kept even on the fast path. Ninefold.sln includes
tests/Ninefold.Tests.csproj, which cross-links tools/tests/*.cs and pure
shared logic — all Godot-*inert* paths — so a compile-level semantic conflict
between a fresh tooling commit on main and an open mixed branch is exactly
what the diff check cannot rule out. The build is also what f5.py runs at
Aedan's desk; keeping it guarantees his F5 never hits a compile error we
chose not to look at. It is incremental in the reused -f5 worktree, so it
is the cheap third of the phase.
* The test suite is skipped on the fast path. In principle inert files can
break tests, but every inert delta in the integration arrived through a
presubmit-gated PR or presubmit-gated main; the only unverified object is
the *combination*, a test-level (not compile-level) conflict between two
green tooling changes with zero game-file movement. The blast radius is a
test suite Aedan never runs during a playtest, and the next visible rebuild
re-runs everything. If the reviewer or Aedan judges even that too loose,
the fallback is "fast = build + tests, skip import only" — still saves the
Godot launch — but implement the build-only version first.
Transitivity note for the docstring: the comparison is against the last *pushed* integration, whose own verify may itself have been fast. That is sound because each fast push asserted visible-tree equality with its predecessor, so equality chains back to the last fully-verified build — and the build check ran on every link.
Tests (in tools/tests/test_rebuild_f5.py, same seams the suite already uses —
runner injection, scripted Git):
* Inert-only diff → run_tests.sh and the Godot import are not invoked;
dotnet build is; push still happens; the fast-path line is printed.
* One visible path among many inert → full verify.
* changed_paths → None (scripted git failure) → full verify.
* No origin/f5/integration ref (first-ever build) → full verify.
* --full-verify → full verify on an inert diff.
The manual loop-step-4 is the part that makes sessions *feel* the decision is
theirs. Move it into the front door (design 006). f5/integration is main +
every open PR head, so it changes on exactly three session-driven events, and
each one's verb ends by invoking ./tools/pr/rebuild_f5.py — foreground,
echoed like every other process a verb runs:
* dev.py pr — a new head enters the frontier.
* dev.py merge — main advances, a head leaves.
* dev.py push (new verb) — an existing head *moves*. This is the
iterate-on-feedback / prototyping case ("move that thing a bit to the
right", pre-review, Aedan playtesting each round via ./f5.py): today that
loop is edit → commit → raw git push, which touches no verb, so hooking
only pr and merge would leave every feedback round invisible to f5 —
and worse, teach sessions that rebuilds are automatic right as the one
remaining manual case becomes the common one. The verb: **gate, push,
rebuild** — run the gate through devcli's existing one-spelling-of-it
helper (presubmit.py --all, streamed), record the result in the presubmit
ledger exactly as dev.py check does, push the current branch (refuse on
main), then the same rebuild tail as the other two.
notes/pair-programming.md's mechanics list ("commit, open PRs, …, rebuild
f5") switches to naming dev.py push as the iterate step.
Why the gate is in the verb (this is presubmit disappearing behind the front door, same move as the rebuild):
* The presubmit ledger stays true for every head. _record_presubmit is
what merge_pr.py --why reads; a head pushed with raw git push reports
"presubmit: UNKNOWN — no run recorded" forever. A gating push verb is the
only way dev.py status stays honest during iteration.
* In a rev:off pairing worktree, merge_pr.py is not used (Aedan merges
from the UI), so its --all never runs — push-time is the only
automated gate on that path.
* A red gate refuses the push (and therefore the rebuild), same posture as
pr. Escape hatch: --ungated, loudly named, for vibing rounds where
Aedan has said speed beats the suite; it skips gate *and* ledger record,
so --why honestly reports no run.
⚠ Do not re-introduce path→suite routing to make the push gate cheaper.
presubmit.py's plan() docstring records the #458 measurement and the
agreed removal; the gate is --all or skipped-loudly, nothing between. And
do not skip the rebuild's own verify because the gate just passed — the gate
checks the branch tree, the verify checks the merged integration; they are
different objects and both stay.
Note the fast verify (T1) deliberately does not fire for that prototyping
loop — a game edit is Godot-visible, so each round pays the full verify, as it
must: that build is the thing Aedan is about to play. If iteration latency
under load turns out to matter, a --build-only escape for vibing rounds is a
possible follow-up, but it is a policy call (publishing an f5 the tests never
saw) — file it separately if wanted; do not fold it in here.
* Unconditional. No path inspection in devcli — the decision about what
the rebuild costs lives in rebuild_f5.py (T1), in one place. After a
tooling PR/merge the invocation is seconds; after a games one it was
required anyway.
* Lock contention is already handled: rebuild() stands down with exit 0
when another session holds the publish lock, precisely so callers chaining
it don't escalate routine contention. A rebuild failure should be *reported*
by the verb but must not turn a completed merge into a failed verb — the
merge happened; say the rebuild didn't, and print the command to re-run.
* Respect the existing separation: merge_pr.py itself stays rebuild-free
(its _gate_worktree docstring explains the backgrounded-gate collision
with <topic>-f5; the *foreground* invocation after merge_pr.py returns,
from devcli, has no such overlap — note that in a comment).
* Extend the devcli contract tests the same way the merge verb's
merge_pr.py invocation is pinned today.
* CLAUDE.md loop step 4 shrinks from an instruction to a description: `dev.py
pr/merge` rebuild f5 for you; it is cheap when your change is
Godot-inert. Do not grow the file — this should be a net wording swap.
* notes/merging.md / notes/operating.md: same adjustment where they
narrate the manual step; keep --health guidance as-is (its semantics are
deliberately untouched by this issue).
* notes/pair-programming.md: the iterate loop pushes with dev.py push, so
every feedback round Aedan playtests is in f5 by construction.
* Mind tools/tests/test_doc_pointers.py — pointer phrases in the touched
docstrings are checked.
* No change to which *branches* enter the frontier — #599's hold-out already
covers that and is untouched.
* No change to f5_status / --health / warn_if_behind_main semantics —
the design keeps their invariant ("f5 contains main") true instead of
teaching them exceptions.
* No probe-selection changes — design 005b already owns that side.
PR 1: T1 (rebuild_f5 fast verify + tests). PR 2: T2 + T3 (devcli invocations, contract tests, doc wording). PR 2 stacks on PR 1 so the auto-invocation never lands without the thing that makes it cheap.
None.
No comments.