test(brainbar): prove the watcher probe left the main actor with an event, not a 0.3s clock (XS) - #786
Conversation
…vent, not a 0.3s clock (XS) `testWatcherProbeDoesNotBlockMainActorRefresh` asserted `elapsed < 0.3` while its stub probe slept 0.6s. On #784 a cold macos-15 runner measured 0.336s: the property (refresh returns without waiting for the probe) HELD, the proxy threshold did not. swift (macos-15) is a required check, so that flake blocks releases. The probe now parks its first `sample()` on a semaphore only the test releases, so "the refresh did not wait" is proven by an event -- reaching the next line while the probe is still blocked is the property itself -- with no wall-clock margin anywhere. The park is bounded (15s) so a main-actor regression fails the assertion instead of hanging the suite, and the tail wait is a generous 30s liveness poll for the released result: a slow runner makes this test slower, never red. Verified both directions locally, brain-bar/Tests only: - green: `swift test --filter StatsCollectorTests` -> 16/16, 8 consecutive runs - red: moving `watcherProcessProbe.sample()` out of the detached task onto the main actor fails it on the property assertion (StatsCollectorTests.swift:756); reverted - green under 24x CPU load, where the old 0.3s assert's replacement tail (5s) had failed; this test passes twice under that load Agent: brainlayerClaude-9fcc41a6 (Claude Opus 5, 1M) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_75745832-8387-4aec-aa23-e23fa33c9da4) |
|
@coderabbitai review Focus: this is a test-only change to a flaky wall-clock assert on a required check ( — brainlayerClaude-9fcc41a6 (Claude Opus 5, 1M context) |
|
Warning Review limit reachedNext included review available in 34 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
 |
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| Python |  |
Sep 5, 2026 5:15p.m. | Review ↗ |
| Swift | |
Sep 5, 2026 5:15p.m. | Review ↗ |
| JavaScript | |
Sep 5, 2026 5:15p.m. | Review ↗ |
| Shell | |
Sep 5, 2026 5:15p.m. | Review ↗ |
| Secrets | |
Sep 5, 2026 5:15p.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
BrainLayer ratchetEvery Value below was measured by this run. A row this machine cannot measure says
🟢 GREEN measured, within budget · 🔴 RED measured, out of budget — a finding to clear before merge · ⚪ n/a not measurable on this machine, never guessed. No RED rows. Measured on Linux/x86_64 · measured |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- PR diff summary ---'
git diff --stat HEAD^ HEAD
echo '--- changed files ---'
git diff --name-only HEAD^ HEAD
echo '--- candidate test files ---'
fd -a 'StatsCollectorTests\.swift$' .
echo '--- Swift declaration outline ---'
while IFS= read -r file; do
ast-grep outline "$file" --lang swift --items all --match 'GatedWatcherProbe|testWatcherProbeDoesNotBlockMainActorRefresh|WatcherProbe|StatsCollectorTests' --view expanded || true
done < <(fd -a 'StatsCollectorTests\.swift$' .)
echo '--- PR diff ---'
git diff --find-renames --find-copies HEAD^ HEAD -- ':*.swift'Length of output: 1343 🏁 Script executed: #!/bin/bash
set -euo pipefail
file="brain-bar/Tests/BrainBarTests/StatsCollectorTests.swift"
echo '--- diff ---'
git diff --find-renames HEAD^ HEAD -- "$file"
echo '--- relevant symbol locations ---'
rg -n -C 8 'GatedWatcherProbe|testWatcherProbeDoesNotBlockMainActorRefresh|hasCompletedASample|wait\(|timeout|semaphore|continuation|sample\s*\(' "$file"
echo '--- file outline ---'
ast-grep outline "$file" --lang swift --items all --view expandedLength of output: 23815 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- StatsCollector candidates ---'
fd -a 'StatsCollector\.swift$' .
echo '--- refresh and watcher-probe call sites ---'
rg -n -C 12 -g '*.swift' '\bfunc\s+refresh\s*\(|watcherProcessProbe\.sample\s*\(|watcherProcessProbe|isRefreshing' .
echo '--- target test complete ---'
sed -n '733,787p' brain-bar/Tests/BrainBarTests/StatsCollectorTests.swiftLength of output: 171210
The 10-second start wait confirms liveness. The 15-second gate bound prevents a hung suite. The 30-second result wait confirms that the released detached task publishes its result. None of these values is used as a performance assertion. Static review only. This environment cannot run the Swift test suite.
|
…ove (review round 1) Cursor low on #786: reaching the line after `refresh(force: true)` does not by itself prove anything -- at that instant the probe may not have entered `sample()` yet, and the park is bounded by `maxBlock`, so "only this test can release it" was also too strong. The proof is the PAIR of assertions: the probe entered `sample()`, AND it has not completed while the test runs on. Comment now says exactly that. Comment-only; no assertion, helper, or production behaviour changed. Agent: brainlayerClaude-9fcc41a6 (Claude Opus 5, 1M) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_f7563fd2-af42-4485-b63d-3c47181611eb) |
All six sites: pyproject.toml, src/brainlayer/__init__.py, server.json (root + packages[0]), brain-bar/bundle/Info.plist (short, bundle, release). Casks/brainbar.rb stays 1.5.9: no Swift change this release, declared to scripts/brainlayer-version-check.sh via BRAINLAYER_VERSION_CHECK_CASK_LAG_REASON="no BrainBar release for 1.5.16" (kept in the PR body, never exported into the push env — two tests assert the check fails without a reason). Why a release, 22 minutes after the last one. v1.5.15 was tagged at 51a72a0 (20:12:29). #778 — the index runtime watchdog — merged at 78d92bc (20:34:55), so it is NOT in 1.5.15. Verified: `git merge-base --is-ancestor 78d92bc v1.5.15^{}` -> false, and the installed keg's python raises `ModuleNotFoundError: No module named 'brainlayer.index_watchdog'`. That watchdog is the fix for the M1's nightly `brainlayer index` job, which ran 14h03m at ~100% CPU on 09-05, 10h past its own 4h cap. Both Macs' 03:15 index jobs are `launchctl disable`d as a stopgap and come back ON only after 1.5.16 is installed and the watchdog is proven present. Nine commits ride along: #774 #777 #780 #778 #783 #785 #786 #787 #788. Co-authored-by: brainlayerClaude-c1601b03 running claude-opus-5 <noreply@anthropic.com>
1 participant
The defect
StatsCollectorTests.testWatcherProbeDoesNotBlockMainActorRefreshassertedXCTAssertLessThan(elapsed, 0.3)while its stub probe slept0.6s. Found by seat ebe88b04 on #784: a cold macos-15 runner measured 0.336 s. The property under test — refresh returns without waiting for the probe — HELD; the proxy threshold did not.swift (macos-15)is a required check, so that flake blocks releases.Per the plan, this does not nudge
0.3 → 0.5. It removes the clock from the property.The fix (brain-bar/Tests only)
GatedWatcherProbeparks its firstsample()on a semaphore that only the test releases. Reaching the line aftercollector.refresh(force: true)is the property: a probe sampled on the main actor could not have handed control back while it is still parked.XCTAssertFalse(probe.hasCompletedASample)states that directly. No wall-clock margin is involved anywhere in the assertion.Two timeouts remain, both liveness rather than thresholds — a slow runner makes the test slower, never red:
Verification (local, both directions)
swift test --filter StatsCollectorTestswatcherProcessProbe.sample()out of the detached task onto the main actorStatsCollectorTests.swift:756on the property assertion — reverted,git statusclean of itClosure: 5 consecutive local runs at 797e587 (M4,
nice -n 15)swift test --filter StatsCollectorTestsThe old assert's 0.3 s budget sat inside a test that legitimately takes ~0.6 s of stub delay; the spread above (0.565-0.845 s) is stub delay plus scheduling, and none of it is load-bearing any more.
CI on the first pushed SHA (041962f):
swift (macos-15)pass, 5m53s -- the required check this PR exists to protect.Review round 1 (Cursor low, addressed in 797e587)
The comment at ~749 overclaimed: reaching the line after
refresh(force: true)proves nothing on its own (the probe may not have enteredsample()yet at that instant), and the park is bounded bymaxBlock, so "only this test can release it" was too strong. Reworded to name the actual proof -- the PAIR of assertions. Comment-only; no assertion or helper changed.Out of scope — pre-existing, filed separately
Under that same synthetic 24× load (far harsher than a cold CI runner), two other tests in the file still fail on their own 2 s/3 s deadlines:
testOlderFullRefreshPreservesPublishedTruthWhileNewerStandaloneIsPendingandtestSuccessfulDashboardRefreshRefetchesSelectedWiderWindow. Not touched here — canon rule 9, one defect one PR. I have not seen either fail in CI; the evidence is synthetic-load only.Size: XS — one test file, no production change (
74 insertions(+), 16 deletions(-)).Agent: brainlayerClaude-9fcc41a6 (Claude Opus 5, 1M context)
Note
Low Risk
Test-only changes in
StatsCollectorTests.swift; no production or runtime behavior is modified.Overview
Fixes a flaky CI failure in
testWatcherProbeDoesNotBlockMainActorRefreshwhere a 0.3s elapsed-time cap could fail on slow macOS runners even when refresh correctly did not wait on the watcher probe.The test drops the in-class
BlockingWatcherProbe(fixed 0.6s sleep) and uses a newGatedWatcherProbethat blocks the firstsample()on a semaphore until the test releases it. Aftercollector.refresh(force: true), it asserts the probe has started sampling but not finished—proving refresh returned without awaiting the probe, with no timing margin on that property.deferalways releases the parked sample; the tail uses a 30s liveness wait (up from 3s) for the probe result to publish.Reviewed by Cursor Bugbot for commit 797e587. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Replace delay-based watcher probe test with event-based
GatedWatcherProbeThe test for non-blocking main-actor refresh previously inferred non-blocking behavior from a 0.3s wall-clock threshold. It now uses a gated probe that blocks the first sample until explicitly released, so the test can directly assert that refresh returned while the sample is still in progress.
GatedWatcherProbe, a thread-safe test double that records sample calls and blocks the first sample until released or a bounded timeout expiresBlockingWatcherProbeand its elapsed-time assertion in StatsCollectorTests.swifttestWatcherProbeDoesNotBlockMainActorRefreshnow verifies the collector is refreshing after refresh returns, releases the blocked sample, and waits for the running-process result with a longer liveness deadlineMacroscope summarized 797e587.