Skip to content

Deflake MeetingPromptDetector tests with an explicit settle signal - #1906

Merged
r3dbars merged 4 commits into
mainfrom
claude/deflake-meeting-prompt-detector
Sep 29, 2026
Merged

r3dbars merged 4 commits into
mainfrom
claude/deflake-meeting-prompt-detector

Conversation

@r3dbars

@r3dbars r3dbars commented Sep 29, 2026

Copy link
Copy Markdown
Owner

Why

MeetingPromptDetectorTests failed on and off on a loaded Mac (2 to 52 failures per run at load averages 60–150, clean origin/main included). CI passed it every time. The failing checks all read the detector's result before its async evaluation had finished.

Here's why. Every signal push (updateMicInputUsers, camera, output, requestEvaluation, EventKit change, workspace activate) spawns a Task that runs evaluate(). That awaits an off-main running-apps read, and the tests don't stub it. The tests then waited a fixed ~100 ms (20 × yield + 5 ms sleep). On a busy machine that's not enough time.

What changed

  • MeetingPromptDetector now counts evaluations in flight and exposes waitUntilEvaluationsSettle(). Counted: spawned passes (all signal pushes now go through one scheduleEvaluation()), the first poll pass after start(), browser title reads that re-evaluate when they land, and a timed re-check once its sleep ends. Sleeping re-checks and the 120 s poll aren't counted. The EventKit observer already runs on .main, so it now schedules synchronously via MainActor.assumeIsolated, which means a post can't slip past the counter. There's no change to runtime behavior.
  • Tests await the settle signal instead of sleeping. The extraMilliseconds: waits keep their sleep because it's a minimum gap so a title re-read is allowed. Then they settle. The one test that waits on a real 1 s re-check waits for its condition and settles, with a ~5 s give-up. Deflake the Meet-after-Not-now prompt test #1901's until: workarounds are now plain settles. No wall-clock assertions (check-test-shape.py is clean).
  • DefaultInputDeviceMonitorTests, the ordering test: its 250 ms lookup timeout around a 50 ms fake lookup timed out under load. It then delivered a nil device, so both observers saw false. That timeout isn't what the test checks, so it gets a 30 s bound. The assertion is unchanged.

Checks

  • bash run-tests.sh --filter MeetingPromptDetector: 138/138 on an idle machine.
  • check-test-shape.py and check-source-pins.py --changed-only both pass.
  • Pending: the repro under load (yes × 3 per core, load average ~230–300) is still building locally. I'll post the results here.
  • Not run yet: bash check.sh, independent review.

🤖 Generated with Claude Code

Signal pushes (mic, camera, output, requestEvaluation, EventKit change,
workspace activate) each spawn an evaluate() task that awaits an
off-main running-apps read. The tests waited a fixed ~100 ms for it, so
on a loaded Mac they read the result before evaluation finished and got
nil / 0 prompts.

The detector now counts evaluations in flight (spawned passes, the first
poll pass, title reads that re-evaluate, and timed re-checks once they
fire) and exposes waitUntilEvaluationsSettle(). The tests await that
instead of sleeping. The one test that waits on a 1 s timed re-check
waits for its condition, then settles.

Also: the DefaultInputDeviceNotificationLookupDispatcher ordering test
used a 250 ms lookup timeout around a 50 ms fake lookup. Under load it
timed out, delivered a nil device, and failed the self-write flag. The
timeout isn't what that test checks, so it gets a 30 s bound.
The first loaded run (load avg ~280) failed "an unrecognized site waits
before prompting" at the "not right away" check: with a 1 s wait counted
from the mic push, a first evaluation pass slower than 1 s was already
allowed to prompt. The hold-back check now uses an hour-long wait; a
separate suite keeps the 1 s wait and checks that the detector's own
re-check prompts.
@r3dbars

r3dbars commented Sep 29, 2026

Copy link
Copy Markdown
Owner Author

Loaded run 1 (yes ×3 per core, load avg ~280): 136/138. All the originally reported failures (lines ~320–696) were gone. The 2 left were the "not right away" checks in an unrecognized site waits before prompting: its 1 s evidence wait is counted from the mic push, so a first evaluation pass slower than 1 s was already allowed to prompt. Pushed a split: the hold-back check now uses an hour-long wait, and a separate suite checks that the 1 s re-check prompts on its own. Re-running under load now.

@r3dbars

r3dbars commented Sep 29, 2026

Copy link
Copy Markdown
Owner Author

Independent review (a separate Claude agent, full diff vs origin/main): ship.

  • Counter accounting: checked path by path. No leak and no double-decrement. The title read decrements exactly once via defer, including cancelled and stale-token reads. A timed re-check counts only after its sleep, and nested title reads increment before the parent evaluation finishes.
  • Not counted: the 120 s poll passes, and the off-main bundle read in the workspace observer before it calls scheduleEvaluation. No test depends on either.
  • MainActor.assumeIsolated: safe, because queue: .main always runs the block on main. The refresh flag is now set one hop earlier, which is harmless. Lifetime is unchanged.
  • Deflake the Meet-after-Not-now prompt test #1901's until: waits → plain settles: no coverage lost. Both prompts come from the counted title-read → evaluate chain.
  • Low findings:
    1. The split re-check suite can pass without its re-check under extreme load, when the first pass itself lands past 1 s. That can only give a false pass, never a false failure, and the hold-back suite covers the wait. I fixed the comment to say so.
    2. The until: helper gives up after at least 5 s. That's a timeout, not an assertion on time.
    3. A test stub that never returns would hang waitUntilEvaluationsSettle. Current stubs all return right away.
    4. The hold-back suite leaves a 1-hour re-check sleeping, but it holds the detector weakly, so it's inert.
  • Checks: check-source-pins --changed-only and check-test-shape both pass.

Loaded run 1 on the split code: 138/138 at load average ~384. More runs in progress.

@r3dbars
r3dbars merged commit 62de338 into main Sep 29, 2026
8 checks passed
@r3dbars
r3dbars deleted the claude/deflake-meeting-prompt-detector branch September 29, 2026 02:41
@r3dbars

r3dbars commented Sep 29, 2026

Copy link
Copy Markdown
Owner Author

Loaded repro on the final branch code (yes ×3 per core, same Mac that saw 2–52 failures): run-tests.sh --filter MeetingPromptDetector 5/5 runs at 138/138, load avg 146–384. DefaultInputDevice loaded runs still going.

Heads-up: this merged 27 s after #1907, which touches the same files. I read main's combined result. #1907's makeIsolatedDetector stubs the running-apps read and title reader, and every suite uses it together with these settle helpers, so the two complement each other. I haven't built it locally yet, and Swift CI on main 627d26e8 is pending.

@r3dbars

r3dbars commented Sep 29, 2026

Copy link
Copy Markdown
Owner Author

Loaded DefaultInputDevice runs: the line-246 fix held in all 3. One run failed a different test at line 410, "replacement worker starts", which also had a 2 s bound that was too tight. That's fixed in #1914. bash check.sh on merged main (#1906 + #1907) plus #1914 passes: 19,961/19,961 tests and the deterministic proof.

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.

1 participant