Skip to content

test: stabilize Pi follow-up duplicate-answer assertion - #2937

Open
bingb0t5 wants to merge 1 commit into
kunchenguid:mainfrom
bingb0t5:fm/fm-pi-followup-duplicate-answer
Open

test: stabilize Pi follow-up duplicate-answer assertion#2937
bingb0t5 wants to merge 1 commit into
kunchenguid:mainfrom
bingb0t5:fm/fm-pi-followup-duplicate-answer

Conversation

@bingb0t5

Copy link
Copy Markdown

Intent

Fix the pre-existing Firstmate main-branch failure where tests/fm-calm-pi-extension.test.sh's Pi follow-up adjacent case (two followUp deliveries queued back-to-back in message_start) intermittently reported a rendered duplicate captain answer. Diagnosis via repeated E2E reproduction and fine-grained pane probing (many runs, up to 150 probes at 10-20ms granularity) showed the underlying session file was always correct (exactly one CAPTAIN_ANSWER, one merged MONITOR_HANDLED_..._ONE_TWO turn) and the rendered pane count never exceeded 1 -- it only ever raced between 0 (not yet redrawn) and 1 (settled), never 2. Git history confirmed the pane capture for this specific assertion has been a single, unpolled snapshot taken immediately after the session-file readiness loop since the check was introduced in commit 65ad47b, unlike every other readiness check in this test which polls. Sending two adjacent followUp deliveries queues more Calm presentation work (an extra operational-user row plus its Calm-hiding invalidation) than a single delivery, so the already-correct captain answer's TUI redraw could still be in flight the instant the session file confirmed processing, making this one assertion flaky specifically in the adjacent case. The fix polls the pane the same way the adjacent session-file wait already does (same idiom, bounded iterations, same sleep granularity) before evaluating the duplicate-answer count, and additionally tracks the peak count observed during that poll so a genuine transient duplicate frame would still fail the assertion even if a later redraw self-corrected -- this strengthens the check rather than weakening it. The original exactly-one assertion and its failure message are unchanged; no product/extension code (fm-calm.ts, fm-calm-assistant-layout.ts, fm-calm-visibility.ts, fm-calm-operational-user-layout.ts, fm-calm-working-ship.ts) was touched, since the defect was entirely in the test's own settle-wait omission. Reviewed pi and pi-signed integration surfaces: pi-signed execs the identical Pi engine/TUI as pi (per harness-adapters skill) and has no separate Calm extension surface, so this test (which already runs against whatever pi binary is installed) covers both; no separate pi-signed test path exists or is needed. Verified the fix with 8+ consecutive clean runs of the full tests/fm-calm-pi-extension.test.sh file (all sub-cases, not just adjacent) and a clean bin/fm-lint.sh pass on the changed file.

What Changed

  • Poll the Pi pane until the adjacent follow-up result is rendered before checking the captain answer count.
  • Track the peak captain answer count during polling so transient duplicate frames still fail the assertion.
  • Preserve the final exactly-one captain answer check and existing failure message.

Risk Assessment

✅ Low: The change is narrowly scoped to test synchronization, preserves the exactly-one pane assertion, records the peak observed captain-answer count to catch sampled transient duplicates, and conforms to the stated durable test-fix intent without changing product code.

Testing

Three consecutive full fm-calm-pi-extension E2E runs passed against the installed Pi binary, exercising the real tmux TUI, adjacent back-to-back follow-ups, peak and settled duplicate-answer detection, persisted session semantics, Calm visibility, and restart behavior. Two executable transcripts were retained as evidence. No screenshot was captured because the terminal test automatically tears down its ephemeral tmux session; its pane contents and JSONL session state are asserted directly before teardown.

Evidence: Pi Calm E2E run 2 transcript

Source: Pi Calm E2E run 2 transcript

ok - Pi calm resolves its persistent home independently of Pi's launch directory
ok - Pi calm compatibility evidence never rejects a Pi version for being newer than 0.82.0, and still fails closed on a missing or malformed version
ok - a missing collapsed-thinking presentation API degrades only that Calm adapter with a clear skip reason, while the rest of Calm still registers
ok - missing Pi presentation class exports reach the independent adapter degradation path
ok - Calm registers none of its 7 built-in tool wrappers at load while config/calm is off, and all 7 synchronously at load while config/calm is on
ok - Calm's first same-session /calm activation claims every uncontested built-in, leaves a foreign bash tool fully intact and callable, warns prominently and logs the contested name, and only rows constructed before that activation - the documented bound - fail to retroactively collapse
ok - Pi calm centralizes transcript visibility, preserves execution/export data, keeps Pi's stock working row visible while no run is active, and persists its choice across session starts
ok - Pi calm on collapses mid-turn assistant working notes to zero height while Calm off keeps them, leaves streaming, truncated-final, and genuine final replies untouched, never mutates the messages, ignores every /calm argument, and restores a legacy persisted max as ordinary Calm on
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
ok - Pi Calm native /skill:ahoy geometry keeps every collapsed thinking and tool block at zero height while preserving expansion, history, restart, and Calm-off rendering
ok - Pi Calm working ship moves on a slow independent cadence over faster fixed-cell blue water, paints the complete boat standard yellow with balanced resets, keeps ANSI-stripped width exact, flips the directional sail on the exact bounce at both edges and every width, clamps visible and hidden resizes, falls back deterministically when narrow, freezes and resumes column/direction across settle/start without hidden-time jumps or duplicate timers, resets only on a fresh session, and installs and removes one scheduler-owning widget across starts, settle, abort, failure, shutdown, reload, replacement, and Calm toggles while leaving Calm-off visibility untouched
ok - Pi calm native E2E replaces the stock working row with a moving, resize-clamped working ship that freezes and resumes across two working periods in one Pi session, clears on abort, keeps captain turns visible, hides exact operational user rows without changing persistence, restores stock rendering Calm-off, survives restart, and preserves export plus Ctrl+O behavior
Evidence: Pi Calm E2E run 3 transcript

Source: Pi Calm E2E run 3 transcript

ok - Pi calm resolves its persistent home independently of Pi's launch directory
ok - Pi calm compatibility evidence never rejects a Pi version for being newer than 0.82.0, and still fails closed on a missing or malformed version
ok - a missing collapsed-thinking presentation API degrades only that Calm adapter with a clear skip reason, while the rest of Calm still registers
ok - missing Pi presentation class exports reach the independent adapter degradation path
ok - Calm registers none of its 7 built-in tool wrappers at load while config/calm is off, and all 7 synchronously at load while config/calm is on
ok - Calm's first same-session /calm activation claims every uncontested built-in, leaves a foreign bash tool fully intact and callable, warns prominently and logs the contested name, and only rows constructed before that activation - the documented bound - fail to retroactively collapse
ok - Pi calm centralizes transcript visibility, preserves execution/export data, keeps Pi's stock working row visible while no run is active, and persists its choice across session starts
ok - Pi calm on collapses mid-turn assistant working notes to zero height while Calm off keeps them, leaves streaming, truncated-final, and genuine final replies untouched, never mutates the messages, ignores every /calm argument, and restores a legacy persisted max as ordinary Calm on
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
ok - Pi Calm native /skill:ahoy geometry keeps every collapsed thinking and tool block at zero height while preserving expansion, history, restart, and Calm-off rendering
ok - Pi Calm working ship moves on a slow independent cadence over faster fixed-cell blue water, paints the complete boat standard yellow with balanced resets, keeps ANSI-stripped width exact, flips the directional sail on the exact bounce at both edges and every width, clamps visible and hidden resizes, falls back deterministically when narrow, freezes and resumes column/direction across settle/start without hidden-time jumps or duplicate timers, resets only on a fresh session, and installs and removes one scheduler-owning widget across starts, settle, abort, failure, shutdown, reload, replacement, and Calm toggles while leaving Calm-off visibility untouched
ok - Pi calm native E2E replaces the stock working row with a moving, resize-clamped working ship that freezes and resumes across two working periods in one Pi session, clears on abort, keeps captain turns visible, hides exact operational user rows without changing persistence, restores stock rendering Calm-off, survives restart, and preserves export plus Ctrl+O behavior

Pipeline

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.

  • Inspected git diff 52ff62e8253623b4fafd125c52cdd4c13a3a84b1..f5c983cb389874995f1bc7fd48cf64456784e962 -- tests/fm-calm-pi-extension.test.sh
  • Ran bash tests/fm-calm-pi-extension.test.sh three consecutive times
  • Captured runs 2 and 3 with bash tests/fm-calm-pi-extension.test.sh 2>&1 | tee <evidence-log>
  • Verified cleanup with git status --short
✅ **Document** - passed

✅ No issues found.

⏭️ **Lint** - skipped
  • ⚠️ linter found issues (exit code 1)

🔧 Fix: Verify lint with pinned toolchain
1 warning still open:

  • ⚠️ linter found issues (exit code 1)
✅ **Push** - passed

✅ No issues found.

…er check

The adjacent-follow-up E2E case captured the tmux pane for its
duplicate-captain-answer assertion immediately after the session file
confirmed processing, with no settle wait, unlike every other
readiness check in this test. Sending two followUp deliveries queues
more Calm presentation work (an extra operational-user row plus its
hiding invalidation) than a single one, so the already-settled captain
answer's redraw could still be in flight at that instant, making the
check flaky. Poll the pane the same way the session-file wait already
does, and track the peak count seen along the way so a captain answer
that is genuinely rendered twice for even one frame still fails even
if a later redraw were to self-correct.
@greptile-apps

greptile-apps Bot commented Aug 24, 2026

Copy link
Copy Markdown

Confidence Score: 5/5

The PR appears safe to merge because the narrowly scoped test synchronization change preserves the existing assertions without introducing a concrete failure.

The changed test waits for the later rendered follow-up marker, tracks sampled duplicate answers, and still requires exactly one captain answer in the settled pane.

Reviews (1): Last reviewed commit: "fix(test): settle Pi follow-up pane befo..." | Re-trigger Greptile

@kunchenguid

Copy link
Copy Markdown
Owner

Speaking as Kun's firstmate:

Scheduled 3:10am PT 8/24 pass. VISION.md read in full from current main 8fa0505e48a155da78a9aeeb50911719dc558710 (#2939). First look at this unstamped PR. No captain comment authorizing a merge.

VISION (inspected the adjacent-follow-up pane poll in tests/fm-calm-pi-extension.test.sh: 100×50ms wait for MONITOR_HANDLED_${label}_ONE, peak CAPTAIN_ANSWER count, original exactly-one assertion preserved; no product/extension files). Per-rule:

  • One captain, one interface: aligns (a flake that reports a duplicate captain answer when the pane is mid-redraw is dishonest).
  • Authority is explicit and never inferred: aligns (test-only; no runtime grant).
  • Scripts own the mechanics: aligns (same poll idiom the session-file wait already uses).
  • A restart is a non-event: cannot tell from tests alone.
  • Delegation with a spine: aligns (field incident → regression coverage).
  • The fleet outlives any vendor: cannot tell (Pi TUI test only).
  • Scope: aligns (one assertion's settle-wait).

Class: corrective (test-only). Security: none; tests only. No .github files. First-time fork bingb0t5 (prior unmerged PRs; GitHub still FIRST_TIME_CONTRIBUTOR). Full diff reviewed before workflow approval; no security reason to withhold CI.

Workflow-approval this pass (captain gh):

  • 32708214923 CI — approved; now in_progress
  • 32708214996 Require no-mistakes — approved; completed success (GitHub body-compliance only)

CI / NM: HEAD f5c983cb389874995f1bc7fd48cf64456784e962. MERGEABLE / UNSTABLE, ahead 1 / behind 2. Body no-mistakes-pipeline-attestation:v1 matches THIS HEAD (f5c983cb389874995f1bc7fd48cf64456784e962); lint step skipped in the pipeline JSON. GitHub NM SUCCESS is not the SHA match; the body attestation is.

Overlap / HOLD: none of the standing spawn/herdr/lock files. Same test file as #2783 (home-language). #2783 is not land-eligible; if this lands first, #2783 rebases. Not a named hold.

CI is not green yet this pass, so not land-eligible today. Test-only corrective + matching attestation + no hold files would be YES once CI is green. Waiting on CI, not the captain. Do not rebase.

Land-eligible: NO. Captain-flag NOW: no.

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