fix(composer): ring the doorbell for idle Codex frames with no prompt-row animation cell - #53
Merged
Merged
Conversation
…cell Codex can draw an idle frame with all animation cells above and below the prompt row. The prompt-row proof required at least one cell on that row. Thus the bright cells on the next row read as typed text, and fm-send skipped the doorbell. The proof now accepts an empty cell set on the prompt row. The exact dim placeholder proof and all typed-text refusals stay the same. Add four real Codex 0.155.1 frames as regression fixtures in a fork-owned test. Upstream: kunchenguid#4532
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Captain's intent: Follow-up to the idle-Codex doorbell fix (#49, Atlas c666). PR 49 made idle Codex animation frames classify as an empty composer: it accepted clipped dim Codex usage and context footers and inspected lower animation rows before ghost stripping, while keeping the exact dim-placeholder proof and the pending-text safety checks. After it merged, fm-send still skipped the doorbell ("composer visibly holds pending text") for idle Codex workers twice (MAIN typed the doorbell by hand each time). Saved frames: ~/firstmate/data/codex-doorbell-idle-d1/captures/after-fix-* (visible text, detection, and ANSI). One shows a footer like "295K used · Context 1…" with braille decoration on and around the composer line "›⠁Ask Codex to do anything⡀". Atlas ticket c686 on node firstmate/harness-dispatch/steer-delivery. Acceptance: the saved after-fix frames classify as an empty composer and the doorbell rings; real typed text in any harness still blocks the doorbell.
Firstmate spec: Load firstmate-coding-guidelines. Mergeability with official upstream firstmate is a captain priority (upstream-merge rules R1-R14): a generic bug fix stays a minimal, upstream-shaped hunk in the upstream file; fork-only behavior and fork tests live in fork-owned files (tests/fm-local-, docs/local/), and every upstream-owned file with a fork delta has a row in docs/local/seams.tsv; each fork commit touching an upstream path carries an Upstream: trailer. First check whether upstream/main already fixed it (it has: upstream's more general braille-furniture approach from kunchenguid#4532 and later classifies every failing frame as empty; the large upstream reconcile, Atlas c617, is in flight on another branch and is expected to adopt it, so keep this diff small and expect a rebase). TDD: reproduce first as a failing test, then fix. Fix at the composer-classification seam in bin/fm-composer-lib.sh (the owner PR 49 extended) so the frames classify as an empty composer. Keep refusing over real typed text for every harness, keep the full and clipped footer parsers accepting the same set, and do not weaken any other check. Herdr-free change; fixtures are the required proof. Heavy test jobs run under nice -n 10, one at a time.
Accepted decision (firstmate, option A): the saved after-fix ANSI captures already classify empty on the unfixed code, because they were captured seconds after each skip and show a different animation frame. The real failing shape came from read-only sampling of the same idle Codex 0.155.1 pane on Herdr 0.8.2 (pane-cleanup-on-exit-p1): frames with NO animation cell on the › prompt row and bright braille cells on the row below read pending, because _fm_composer_bare_codex_idle_animation required at least one cell on the prompt row. Unfixed main misread 23/120 live frames; the fix misread 0/150. The live frames that misread before the fix are the red fixtures; the saved after-fix captures stay as green regression fixtures. Fixtures live in the fork-owned tests/fm-local-codex-idle-frames.test.sh.
What Changed
_fm_composer_bare_codex_idle_animationinbin/fm-composer-lib.shno longer needs an animation cell on the Codex›prompt row. Some idle frames put all braille cells on the rows above or below the prompt row. These frames now read as an empty composer, andfm-sendrings the doorbell. When the prompt row has dim content after the placeholder, that content must still be animation cells only.tests/fm-local-codex-idle-frames.test.sh, a fork-owned test with real Codex 0.155.1 / Herdr 0.8.2 ANSI frames. Two are live frames with no prompt-row cell, which unfixedmainread as pending. Two are the saved frames from after the skipped doorbells. Each frame must read empty on Herdr, Zellij, and tmux, andunknownwith no styling. Typed drafts, text before the placeholder, bright footer-like rows, text on an animation row, a Claude glyph, and typed drafts in Claude, Codex, and Pi composers must still read as pending.codex-idle-animationrow forbin/fm-composer-lib.shtodocs/local/seams.tsv. The row links the upstream braille-furniture fix (fix(bin): read codex 0.154's idle braille starfield rows as composer furniture kunchenguid/firstmate#4532), which is expected to replace this hunk.Risk Assessment
✅ Low: The code change is a 4-line relaxation in _fm_composer_bare_codex_idle_animation: an empty dim-only strip after the prompt glyph still proves the placeholder was SGR-dim, because FM_COMPOSER_GHOST_LUMA_MAX=0 strips only dim runs. The plain-row check still requires the exact placeholder, and lower rows must still be animation-only or the bounded footer. So typed-text refusals are unchanged, and the single-row path returns the same verdict as before. The fix round's removal of the upstream doc paragraph is verified (runtime-backends.md is identical to base), the seams row budget (155 = 152+3 against the upstream merge base) matches, and the commit carries the Upstream trailer.
Testing
I ran the new fork-owned fixture test on the fix and on the base commit. It fails on base, where the live Codex frame reads pending, and passes with the fix. The existing composer-lib, composer-ghost, and busy-doorbell tests pass with no failures. I also ran an end-to-end check of the real
fm-sendsteer path, using a stub Herdr pane that returns the real idle Codex ANSI frames. On base,fm-sendskipped the doorbell for both live frames that have no prompt-row animation cell ("composer visibly holds pending text"). With the fix, it typed the doorbell and pressed Enter for both live frames and for the saved295K used · Context 1…frame. A typed draft still made it skip the doorbell. The WATCHER DOWN banner in the transcripts is expected: the stub sandbox has no watcher running. This is a CLI and classifier change with no UI, so the evidence is CLI transcripts and not screenshots. I removed the temporary base worktree, and the working tree is clean.Evidence: fm-send E2E transcript with the fix (doorbell rings for idle Codex frames, skipped for a typed draft)
Source: fm-send E2E transcript with the fix (doorbell rings for idle Codex frames, skipped for a typed draft)
Evidence: fm-send E2E transcript on base 5607ed7 (reproduces the bug: doorbell skipped as 'composer visibly holds pending text' on live idle frames)
Source: fm-send E2E transcript on base 5607ed7 (reproduces the bug: doorbell skipped as 'composer visibly holds pending text' on live idle frames)
Evidence: E2E driver script (public fm-send path over a stub Herdr CLI serving the real idle Codex frames)
Source: E2E driver script (public fm-send path over a stub Herdr CLI serving the real idle Codex frames)
Evidence: Fixture test fails on base (red)
Source: Fixture test fails on base (red)
not ok - prompt row without animation cells on Herdr: expected empty, got 'pending'Evidence: Fixture test passes with the fix (green)
Source: Fixture test passes with the fix (green)
ok - real idle Codex frames read empty, with or without prompt-row animation ok - typed text below an idle Codex prompt still blocks the doorbell ok - typed drafts in other harness composers still block the doorbellPipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 1 issue found → auto-fixed ✅
docs/verification/runtime-backends.md:357- Simplification: the change adds a 9-line paragraph to docs/verification/runtime-backends.md. This file is an upstream-owned verification doc, and it has no seams.tsv row. The intent does not require it: it says "fixtures are the required proof", it wants the diff kept small because a rebase onto upstream is expected, and it says fork tests and docs belong in fork-owned files (tests/fm-local-, docs/local/). The paragraph adds a fork delta to an upstream file that the Atlas c617 reconcile will have to resolve. It also gives numbers the intent does not state: "150 frames ... found 27 frames" that read pending before the fix. The intent reports 23 of 120 misread on unfixed main and 0 of 150 misread with the fix. Recommended remedy: remove the paragraph, or move it to a docs/local/ note and fix the counts to match the recorded sampling.🔧 Fix: Drop fork paragraph from upstream runtime-backends doc
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
nice -n 10 bash tests/fm-local-codex-idle-frames.test.shon HEAD 11ba8cf: passes (all 3 cases)Same fixture test run against base 5607ed7 in a temporary detached worktree: fails withprompt row without animation cells on Herdr: expected empty, got 'pending'(the red reproduction)nice -n 10 bash tests/fm-composer-lib.test.sh: passes, nonot oklinesnice -n 10 bash tests/fm-composer-ghost.test.sh: passes, nonot oklinesnice -n 10 bash tests/fm-send-busy-doorbell.test.sh: passes, nonot oklinesManual E2E withfm-send-codex-doorbell-e2e.sh: it runs the publicbin/fm-send.sh t1 <msg>steer for a recordedharness=codexHerdr task, with a stubherdrCLI whosepane readreturns the real idle Codex ANSI frames. It checks 2 live frames without a prompt-row cell, the saved295K used · Context 1…frame, and a live frame with a typed draft. It ran on both HEAD and base.✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.