Skip to content

Keep the leaked solver daemons off the known-lane pass under pytest - #406

Merged
jehanazad merged 1 commit into
mainfrom
fix/known-lane-interval-flake
Sep 15, 2026
Merged

jehanazad merged 1 commit into
mainfrom
fix/known-lane-interval-flake

Conversation

@Babissimo

Copy link
Copy Markdown
Contributor

ClickUp test_known_lane interval-gate test still flakes under xdist despite its own mitigation.

test_interval_gate_holds_between_passes fails now and then under xdist with its first assertion reading 0 attempts (2026-09-06, on #389, and twice in a row on #403 today, none of them backend diffs). The ticket had the mechanism right, and review found why the mitigation did not hold: conftest turns KNOWN_LANE_MODE off for the suite, but DARK_FOLLOW_MODE keeps its shadow default, lanes_armed() is true when either lane is on, and maybe_run_pass returns early only when both are off. So every solver worker daemon a TestClient lifespan leaks takes the pass lock, stamps the pass clock and runs an empty dark-follow pass every two seconds, and one landing between the fixture's reset and the test's first call leaves that call gated.

What changes

  • backend/tests/conftest.py: DARK_FOLLOW_MODE defaults to off under pytest, beside the existing KNOWN_LANE_MODE line, so no leaked daemon is armed at thread start.
  • backend/tests/test_known_lane.py: the autouse fixture turns DARK_FOLLOW_MODE off per test as well (a daemon armed while another file had a lane on keeps polling), before the reset that drains any pass in flight, and restores it afterwards. Its clock comment is trimmed to the standing reason for the reset.

Tests that exercise dark following already set the mode themselves (test_dark_follow.py's own autouse fixture does the same per test). The ticket's other option, asserting on a return value, is not available: maybe_run_pass returns None. The leaked daemons themselves stay with the ticket.

Verification: the full backend suite locally against this branch: 3608 passed, 2 skipped, 0 failed; pre-commit on both files: all hooks passed.

🤖 Generated with Claude Code

test_interval_gate_holds_between_passes asserts that its first
maybe_run_pass runs, and under xdist that assertion fails now and then
with the attempt count still 0 (2026-09-06, 2026-09-15 on #389, and
twice in a row on #403, all diffs that never touched the backend).

conftest turns KNOWN_LANE_MODE off for the suite so the solver worker
daemons every TestClient lifespan leaks do not poll the pass, but
DARK_FOLLOW_MODE keeps its "shadow" default, lanes_armed() is true when
either lane is on, and maybe_run_pass returns early only when both are
off. So every leaked daemon takes the pass lock, stamps the pass clock
and runs an empty dark-follow pass every two seconds, and one landing
between the fixture's clock reset and the test's first call leaves that
call gated.

DARK_FOLLOW_MODE is now off for the suite the way KNOWN_LANE_MODE is,
and the known-lane fixture turns it off again per test for a daemon
armed while another file had a lane on, before the reset that drains
any pass in flight. Tests that exercise dark following already set the
mode themselves. The fixture's clock comment is trimmed to the reason
the reset stands; the ordering story it told was the first guess at
this.

ClickUp: 123zgec3130

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@claude

claude Bot commented Sep 15, 2026

Copy link
Copy Markdown

Claude finished @Babissimo's task in 1m 45s —— View job


Review: PR #406 — Keep the leaked solver daemons off the known-lane pass under pytest

  • Gather context (PR diff, CLAUDE.md, ONBOARDING.md)
  • Read changed files (backend/tests/conftest.py, backend/tests/test_known_lane.py)
  • Review for correctness, race conditions, test hygiene
  • Post review feedback

Verdict: looks correct, no blocking issues found.

I traced the mechanism the PR describes against the actual source and it holds up:

  • lanes_armed() (backend/services/tasks/known_lane.py:918-925) is True when either KNOWN_LANE_MODE or dark_follow.mode() is not "off", and it's read exactly once per solver worker thread at start (backend/services/tasks/solver.py:4125).
  • maybe_run_pass() (known_lane.py:928-961) only early-returns when both modes read off, and stamps _last_pass_ts under the pass lock otherwise.
  • core/state.py:139 defaults DARK_FOLLOW_MODE to "shadow" from the environment — so before this fix, conftest.py silencing only KNOWN_LANE_MODE left every leaked TestClient-lifespan daemon still armed via the dark-follow lane, free to take the lock and stamp the clock between the test_known_lane.py fixture's reset and a test's first maybe_run_pass call. That's a real, narrow window matching the reported "0 attempts" flake.

The fix is minimal and consistent with the existing pattern:

  • conftest.py:31 adds os.environ.setdefault("DARK_FOLLOW_MODE", "off") right beside the existing KNOWN_LANE_MODE line, before any backend module import — same placement discipline the file already documents (RETINA_ENV comment at the top).
  • test_known_lane.py's autouse fixture now also pins state.DARK_FOLLOW_MODE = "off" before calling known_lane._reset_for_tests() (which itself blocks on _PASS_LOCK, so any daemon pass already in flight drains before the reset), and restores the prior value afterward — mirroring the existing KNOWN_LANE_MODE save/restore exactly.
  • Checked for regressions: every other test file that needs dark-follow behavior sets DARK_FOLLOW_MODE explicitly per-test (test_dark_follow.py's own autouse fixture, test_dark_key_track_continuity.py:81), and the one non-test reader that compares the raw attribute (frame_processor.py:506, == "binding") treats "off" and "shadow" identically, so defaulting to off changes nothing for it.
  • No other test file installs a conflicting DARK_FOLLOW_MODE autouse fixture that would fight this one.

Minor, non-blocking observations:

  • The trimmed docstring/comment in test_known_lane.py reads well and accurately reflects the new dual-flag rationale.
  • I couldn't run pre-commit or the test suite myself in this sandbox (no backend/.venv), so I verified statically against the source instead of reproducing the PR author's local run; nothing in the diff looks like it would fail linting (both changed lines stay well under the repo's 120-col ruff limit).
  • This doesn't touch anything under /v1/nodes or backend/services/node_config.py, so the generated-contract rule in CLAUDE.md doesn't apply here.

Nice, precise root-cause fix — matches the two-flag semantics of maybe_run_pass correctly rather than papering over the symptom.

@jehanazad
jehanazad merged commit f1af5c0 into main Sep 15, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants