Skip to content

fix(composer): a draft taller than 12 rows is still a draft - #57

Merged
Juliusolsson05 merged 5 commits into
mainfrom
fix/composer-tall-draft
Sep 27, 2026
Merged

Juliusolsson05 merged 5 commits into
mainfrom
fix/composer-tall-draft

Conversation

@Juliusolsson05

@Juliusolsson05 Juliusolsson05 commented Sep 27, 2026 •

Copy link
Copy Markdown
Owner

Refs Juliusolsson05/agent-code#1327. App side: Juliusolsson05/agent-code#1343.

Plan: agent-code docs/plans/2026-09-26-codex-tall-draft.md.

Problem

classifyCodexComposerState searched for the › marker only 12 rows above the separator, so a draft of 13 or more composer rows read unknown. unknown is deliberately not occupancy. Agent Code therefore published provider-not-ready, and its own Enter appended to the human's draft and submitted both.

Evidence

New recording testing/fixtures/composer-0157/tall-draft-ctrlc.json: real codex-cli 0.157.1 at 100×30, idle, then a 20-line draft typed with Ctrl+J newlines (no Enter, no model request), then Ctrl+C twice.

  • Codex does not scroll the marker away: the composer grows upward. At 20 lines, › is on row 7 and the draft fills rows 7–26.
  • Replay: drafted while the draft is up to 12 rows, unknown from 13 composer rows (including the empty cursor line right after a Ctrl+J), empty after Ctrl+C.
  • The whole stream was decoded before committing. The only identifying value, the session id in the exit text, is redacted at the same length. The fixture is pretty-printed like its sibling.

Change

The 12-row bounded search is unchanged; blank rows inside it are allowed, since a draft can hold blank lines. When it finds no marker, a second walk starts at the separator and follows only non-blank rows up to the top of the viewport. The one exception is a single trailing blank row, the empty cursor line a Ctrl+J leaves at the bottom of a draft (recorded). A blank row anywhere else, including inside the 12-row window, ends the walk, so a transcript › above a blank row is never reached.

Tests

On the recording:

  • drafted at 11, 15 and 20 rows (red on main from 13 rows);
  • empty after Ctrl+C.

Replays feed recorded chunks in batches of 50, with xterm drained between batches, and the every-height test advances one replay. No timeout is changed; ~0.8 s and ~1.2 s under load.

On constructed rows:

  • negative first: a transcript › across a blank row, whether above the window or inside it at 1/2/3/4/12, stays unknown;
  • two trailing blank rows stay unknown, while the single cursor line does not;
  • the nearest marker wins;
  • the exact 12-row edge: a marker 12 rows up behind a blank is drafted, 13 rows up is unknown;
  • the corrected bound test, which used to expect unknown for 12 unbroken continuation rows. That expectation was the bug.

Mutants killed: bound 11 and 13, no blank stop, no cursor-line skip, a walk starting above the window, deleting the tall walk.

Review

A (Codex), B (Pi) and C (Codex) are all MERGE-READY after verification; C used its final pass. The dispositions are in the PR comment.

Verification

  • src/parsers: 31 tests (23 in the recorded file). tsc --noEmit is clean.
  • The package CI is green.

Residual

  • A draft with a blank line anywhere between its marker and the separator, once it is taller than 12 rows, still reads unknown. So do a draft taller than the viewport (its marker scrolls off) and a draft ending in two or more blank lines. None is recorded. Agent Code's orchestration delivery refuses these, but the pane's own Enter does not (agent-code#1327 stays open).
  • A dense markerless transcript directly above the footer would read drafted. Per Codex's source, the footer is drawn with the composer's marker, so this frame is not expected; it is unrecorded.

🤖 Generated with Claude Code

agent-code#1327. Codex does not scroll a tall draft's marker away; the
composer grows upward (recorded: codex-cli 0.157.1, a 20-line draft with the
marker on row 7, testing/fixtures/composer-0157/tall-draft-ctrlc.json).
The 12-row marker search read every draft over 12 rows as unknown, which is
not occupancy, so Agent Code's own Enter appended to it.

Past the bound the search now goes on through UNBROKEN rows only: a tall
composer is one continuous block, and a blank row still stops the search
before a transcript marker. A draft with a blank line that far up, or taller
than the viewport, stays unknown (unrecorded residual).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Juliusolsson05 and others added 4 commits September 26, 2026 19:30
…plays wait for the parser

- A/C (major): the walk past the 12-row bound started ABOVE the window, so a
  blank row inside the window was never checked and a transcript marker 14
  rows up turned a markerless frame into `drafted` (occupancy). The walk now
  starts at the separator and every row up to the marker must be non-blank,
  except the single empty cursor line a Ctrl+J leaves at the bottom of a
  draft (recorded). Several trailing blank lines stay unknown.
- A/B: replay helpers read whatever frame existed after a 2 s wall-clock
  deadline; under load the tall replay was still parsing and read `unknown`.
  They now wait for xterm to drain.
- C: the exact 12-row edge is pinned (bound 11 and 13 mutants killed); the
  comment now says 13 composer rows, as recorded.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…tall fixture

Pi a/b: the replays dumped ~630 recorded events in one synchronous burst,
which under load left pendingWrites stuck (HeadlessTerminal's documented
write-callback stall) and a half-painted frame read `unknown`. The helpers
now feed one recorded event at a time and let xterm drain between them, as a
PTY delivers them. The tall fixture is pretty-printed like its sibling
(same data, reviewable diffs).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…per drain

One chunk per drain cost ~630 scheduler ticks and ran past the 5 s test
timeout under load (reviews A, B, C round 2). Fifty chunks per drain keeps
xterm's queue shallow (the burst problem from round 1) at ~13 drains; the
tall tests now take ~1.3 s and ~0.24 s at load 36. No timeout was widened.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…not three from scratch

The every-height test replayed the recording from scratch for each of three
heights (~1,450 chunks) and still timed out once under load (review C round
3). It now advances one terminal label by label, feeding only the chunks
since the last check (~620 chunks): 0.78 s at load 41-49. No timeout changed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Juliusolsson05

Copy link
Copy Markdown
Owner Author

Review disposition (final head 0e5ea5d)

Three reviewers: A Codex, B Pi (the GLM slot; opencode would not start on this machine), C Codex. Round 1 was FIX-BEFORE-MERGE from all three. Verification: A MERGE-READY (round 3), B MERGE-READY (round 2), C MERGE-READY (round 4, final pass).

Round 1

  • A1 / C1, major (the walk past the 12-row bound started ABOVE the window, so a blank row inside it was never checked and a transcript › 14 rows up turned a markerless frame into drafted, i.e. occupancy): fixed in 26eccea.
    • The walk now starts at the separator, and every row up to the marker must be non-blank.
    • The one exception, from the recording, is the single empty cursor line a Ctrl+J leaves at the bottom of a draft.
    • Tests: blank rows inside the window at 1/2/3/4/12 stay unknown; two trailing blank rows stay unknown.
  • A2 / B1 / (#1343 A, B) (replays read whatever frame existed after a 2 s wall-clock deadline, so under load a half-parsed frame read unknown): fixed. The replays feed recorded chunks in batches of 50 and wait for xterm to drain. The every-height test advances one replay label by label instead of replaying from scratch (a4842a2, 0e5ea5d). No timeout was widened: the tall tests take ~0.8 s and ~1.2 s at load 41–49. The first attempt (one chunk per drain, 0cd724a) ran past 5 s under load and was replaced.
  • C, surviving mutants:
    • The exact 12-row edge (bound 11 / 13): fixed, with a marker exactly 12 and 13 rows up behind an interior blank.
    • The marker < 0 guard: equivalent, proven. Both searches walk upward from the separator and stop at the first marker, so the long walk can only find the nearest marker or stop and keep the bounded result.
  • C, minor (the threshold is 13 composer rows, not 14): fixed in the comment and body.
  • A, major (declined): a dense markerless transcript, i.e. 13+ unbroken non-blank rows topped by a transcript ›, directly above the status+hint footer, with NO composer marker on screen. Per Codex's source the footer is drawn by ChatComposer together with its › prompt. That shape occurs only when a draft taller than the viewport has scrolled its marker off, and then it is a real draft with no transcript visible. It is unrecorded and stated as a residual.
  • A, suspicions (overlay options, Working rows, image rows above a remote marker): what the pure function accepts; no recording shows Codex painting those frames with this footer.
  • B2, minor (the bound was pinned only to [5, 12]): fixed by the exact-edge tests.
  • B3, minor (the fixture was minified): fixed. Pretty-printed like its sibling, same data.

Later passes

  • Round 2 found only the replay timeout (above), fixed in a4842a2/0e5ea5d. C's round 3 still saw one 5.3 s run under added load; 0e5ea5d made the every-height test advance one replay (~620 chunks instead of ~1,450). C's final pass passed 23/23 on four loaded runs, with the tests at 984 ms and 1,382 ms.

🤖 Generated with Claude Code

@Juliusolsson05
Juliusolsson05 merged commit d42cc1d into main Sep 27, 2026
6 checks passed
@Juliusolsson05

Copy link
Copy Markdown
Owner Author

Manager correction: this merged while its branch was 6 commits behind main (#56 had landed), so its green CI did not cover the combination. There were no conflicts. CI on main's merge commit d42cc1da is being watched; any failure gets a fix-forward PR. Merges now go through a scripted gate that refuses a branch behind its base.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working provider:codex Affects Codex integration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant