PR #752 follow-ups: document the deadlines, and derive the import rung from a cold import nf-cas8 ← Beads

closed priority 2 task unassigned

Twinned from https://github.com/cfsh/ninefold/issues/753 by tools/beads/import_github.py's reconcile pass.

Suggestions from ninefold-reviewer's review of #752 (result=CONVERGED). None of these blocked that PR; all three are below the gate and none is urgent. Unclaimed.

1. notes/probes.md does not know the deadlines exist. The file documents budget_ms at length and states flatly that an overrun *"warns"* and does not fail the run (notes/probes.md:80-84). #752 adds a harness contract that does fail the run — import 300 s < probe 600 s < sweep 660 s — and a reader diagnosing a FAIL has no way to learn from the notes that a timeout is now one of the things a FAIL can be, or what the numbers are. It reads naturally as a bullet beside the budget_ms one. Flagged on all three review passes of #752 and never actioned or declined; recording it here so it stops riding on that thread.

2. IMPORT_TIMEOUT_S = 300.0 is derived from the WARM import, and the expensive case is cold. tools/nflib/godot_import.py — the constant's comment says *"Measured at 8.2–10.0 s warm"*, and the module docstring says the same (*"An unconditional --import is not free: 8.2–10.0 s measured warm"*). That measurement is Godot re-verifying source_md5 against a complete cache. A cold tree is a different operation: 422 artifacts and 227.8 MB of sources encoded for the first time, which is what a fresh spawn_dev.py worktree gets on its first sweep — and #619's own finding was three dev worktrees sitting in exactly that state.

Nobody has measured that path, so the ~30× headroom claimed for this rung is 30× the cheap case. If a cold import on a loaded box runs past 300 s the sweep refuses on the merge path with Godot import failed (exit 124), and a killed import leaves a partial cache that the next attempt re-does, so a slow enough box refuses repeatedly. It fails closedEXIT_REFUSED, nothing compared or recorded — so this is a false refusal rather than the false all-clear the code is careful about, but it is #514's shape on the one rung whose number came from the cheap measurement. One cold --headless --import . run settles it; failing that, say in the comment that the cold case is unmeasured and why 300 s is still believed to clear it.

3. The \"wide margin\" is pinned on the wrapper, not on the engine rung it argues about. test_the_deadline_clears_the_slowest_probe_by_a_wide_margin (tools/tests/test_probes.py) asserts probe_all.PROBE_TIMEOUT_S >= 52.7 * 4.2 * 2, and its docstring reasons entirely about grid-scroll's 52.7 s and #620's 4.2× contention — which is now probe.GODOT_TIMEOUT_S's job, since F1 moved the deadline that kills the engine there. GODOT_TIMEOUT_S is bounded from below only transitively, by test_the_deadlines_form_a_LADDER_innermost_first (> IMPORT_TIMEOUT_S, i.e. > 300). That clears the ~220 s contended worst case, so nothing is wrong today — it just is not the 2× margin the constant's own comment claims for it, and lowering it to 310 s keeps the suite green. One more assertGreaterEqual on probe.GODOT_TIMEOUT_S.

Refs #752 · Refs #747

Dependencies

None.

Comments

No comments.

Add a comment