test(fm-calm-pi): poll pane for render signal before asserting duplicate-answer count - #54
Conversation
… rows (robots-eboq) test_operational_followup_turn_e2e waited on the session file — written the moment the core persists a turn — and then captured the tmux pane immediately. The Pi TUI repaints on its own schedule, so the capture regularly caught a frame drawn before the turn reached the screen: zero CAPTAIN_ANSWER rows, not two. The `-eq 1` check then failed with "rendered a duplicate captain answer", which misnamed the defect and made the whole case nondeterministic on main. Poll the pane for MONITOR_HANDLED_<label>_ONE — the last row of the flow, drawn after the captain answer — before asserting, matching what replay_exact_case already does. A genuine duplicate is still on screen by then, so the assertion keeps its teeth. Report the observed row count in the failure message so a not-yet-rendered pane can never again be reported as a duplicate. Verified: 5/5 green after the fix (2/2 failures before it), full file green.
|
Warning Review limit reached
Next review available in: 42 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus 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 |
1 participant
Intent
Fix the pre-existing nondeterministic failure on main tracked as robots-eboq: tests/fm-calm-pi-extension.test.sh, test_operational_followup_turn_e2e, intermittently fails with 'Pi follow-up rendered a duplicate captain answer' (observed on loaded_default and loaded_off across runs).
Diagnosis established before the change: the assertion message was misleading. run_followup_case waited on the SESSION FILE (written the moment the core persists a turn) and then captured the tmux pane immediately. The Pi TUI repaints on its own schedule, so that capture regularly caught a frame drawn BEFORE the turn reached the screen. Instrumented reproduction showed the pane count was 0, not 2 - nothing was ever duplicated; the '-eq 1' check simply failed on a not-yet-rendered pane and reported it as a duplicate.
The fix, deliberately chosen: poll the PANE for MONITOR_HANDLED__ONE before asserting anything about pane contents. That marker is the last row of the flow and is drawn after the captain answer, so a genuine duplicate captain row is still on screen by the time the count runs - the assertion keeps its teeth rather than being weakened into a tautology. This is exactly the pattern the sibling replay_exact_case in the same file already uses; run_followup_case was the outlier. Also report the observed row count in the failure message so a not-yet-rendered pane can never again be misreported as a duplicate.
Explicitly ruled out: adding a blind sleep (papers over the race without a signal), and waiting for the pane to show exactly one CAPTAIN_ANSWER row (would make the assertion vacuous and hide a real duplicate-render regression).
Scope is deliberately test-only - no product/extension code is touched, because the product behavior was never wrong. This is a test-harness timing defect. Verification performed on Pi 0.82.1 / tmux 3.7b: reproduced the failure 2/2 runs before the change; after the change test_operational_followup_turn_e2e is green 5/5 consecutive runs and the full tests/fm-calm-pi-extension.test.sh file is green. No new test is added because the change repairs an existing regression test rather than covering new behavior.
What Changed
run_followup_casenow polls the tmux pane for theMONITOR_HANDLED_<label>_ONEsentinel (the last row of the flow, drawn after the captain answer) before asserting onCAPTAIN_ANSWERrow count, matching the pattern already used byreplay_exact_casetmux capture-panethat fired right after the session file was written, which regularly caught a pre-render frame and falsely reported zero rows as a duplicate$captain_rowsinstead of exactly one) so a not-yet-rendered pane can never be misreported as a duplicateRisk Assessment
✅ Low: The change is a minimal, test-only timing fix that aligns run_followup_case with the established replay_exact_case pattern, preserves the assertion's diagnostic strength, and introduces no new product or logic risk.
Testing
Ran test_operational_followup_turn_e2e end-to-end on Pi 0.82.1 / tmux 3.7b (exactly the versions cited in the user intent); all cases including the previously-intermittent loaded_default and loaded_off passed, producing the expected
ok - Pi operational follow-up E2E ...pass line with exit 0.Evidence: test_operational_followup_turn_e2e output
ok - Pi operational follow-up E2E processes exact user-role notifications once while Calm hides current and adjacent rows, Calm off and absent render them, and restart preserves semantics EXIT: 0Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
✅ **Review** - passed
✅ No issues found.
✅ **Test** - passed
✅ No issues found.
bash wrapper.sh(extracted single-test wrapper that calls only test_operational_followup_turn_e2e with Pi 0.82.1 / tmux 3.7b) — ran all 9 run_followup_case invocations plus replay_exact_case end-to-end`✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.