fix(ask-user-question): stop the active-row marker shifting the preview box - #2541
Conversation
…ew box OVERLAY_ACTIVE_ROW_MARKER is an APC string terminated by BEL. ECMA-48 lets BEL terminate OSC but not APC, so a conforming terminal — tmux among them — keeps consuming bytes as APC payload. PreviewPane's side-by-side join glues the marked option row and the preview column onto one physical line, so the terminal swallowed the pad and the column gap and drew the box border 13 columns early with option 1 selected and 11 with option 2, on the cursor row alone. ReservedBottomOverlay.takeActiveRow was the only stripper in the tree, and the workflows stage-chat custom-UI slot paints a mounted component directly without it, so the mark reached the terminal on that path. Terminate the mark with ST as the spec requires, and strip it centrally in applyLineResets on both interactive renderers — the last step before pi-tui turns the composited screen into bytes, and where it already removes its own CURSOR_MARKER. Every component tree either renderer paints converges there, so overlay, inline, widget, and stage-chat mounts are all covered, including hosts that never call takeActiveRow. Assistant-model: Claude Opus 5
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
Implementation Notes —
|
| # | Contract clause | Verification | Status |
|---|---|---|---|
| 1 | Fix implemented in the designated worktree, on fix/askq-preview-active-row-align, never main |
git branch --show-current → fix/askq-preview-active-row-align; all edits under that path |
pass |
| 2 | Root cause identified from a REAL tmux capture of a REAL running atomic CLI |
tmux pipe-pane -o on a live CLI: APC atomic:active occurrences: 2 | BEL-term: 2 | ST-term: 0, mark mid-line followed by 13 pad+gap spaces |
pass |
| 3 | Before- and after-fix tmux capture-pane -p output stored verbatim in TMUX-EVIDENCE.md in the worktree |
TMUX-EVIDENCE.md present at 216 lines / 15 324 bytes, 16 balanced fence lines, both signature capture lines found by grep -F. Deliberately untracked per Amendment B: the criterion says in the worktree, not committed |
pass |
| 4 | After-fix capture shows the box aligned identically on every row, for every selection | Machine-measured border column: after = [33,33,33,33,33,33] for both selections; before = [20,33,…] / [33,33,22,…] |
pass |
| 5 | Regression tests covering PreviewPane exist and pass; there were none before |
test/unit/ask-user-question-preview-pane.test.ts (12 tests) + packages/coding-agent/test/overlay-active-row-marker-strip.test.ts (3 tests) |
pass |
| 6 | npm run check passes; the vitest suites covering the touched code pass |
npm run check clean; npm run test:unit 685 files / 6756 passed; coding-agent suite 475 files / 3911 passed |
pass |
| 7 | ## [Unreleased] entry in packages/coding-agent/CHANGELOG.md |
appended to the existing ### Fixed subsection, no duplicate heading |
pass |
| 8 | No scratch or repro harness files remain in the tree | Post-commit git status --porcelain --untracked-files=all → exactly ?? TMUX-EVIDENCE.md. Sweep verified clean: no packages/workflows/dist, no tsconfig.build.json, no bun.lock/yarn.lock/pnpm-lock.yaml, no issues.md; harness lives in /tmp/askq-preview-repro |
pass |
| 9 | User-facing docs describing this behavior stay accurate | packages/coding-agent/docs/extensions.md:2832 rewritten |
pass |
| 10 | Reviewed delta committed on the branch, six paths, evidence file excluded (Amendment B 1-2) | git log --oneline origin/main..HEAD → one commit 192a9b8fc3; git show --stat HEAD → 6 files, +388/−5; git ls-files TMUX-EVIDENCE.md → empty |
pass |
| 11 | npm run check passes on the committed state (Amendment B 3) |
re-run after the commit: biome clean, tsc --noEmit clean, tsgo -p tsconfig.build.json --noEmit clean, shrinkwrap up to date, exit 0 |
pass |
| — | PR body carries the evidence verbatim | out of this stage's scope (Ignore requests to submit a PR); the branch is committed but not pushed, and TMUX-EVIDENCE.md is the body's source text |
deferred to the authorized final action |
Root cause — confirmed first-hand, not inherited
Reproduced live at tmux 3.7b, session 140×44, atomic from worktree source. tmux pipe-pane -o over the painted frames:
\x1b[31;1H\x1b[2K\x1b[0m\x1b]8;;\x07\x1b[38;2;138;190;183m❯ 1. Encrypt at rest\x1b[39m\x1b_atomic:active\x07 \x1b[38;2;138;190;183m┌──────…
OVERLAY_ACTIVE_ROW_MARKER was "\u001B_atomic:active\u0007" — an APC string terminated by BEL. ECMA-48 allows BEL for OSC but not APC, so tmux kept consuming the pad and the column gap as payload and resumed drawing at the border's SGR. The shift equals leftWidth − visibleWidth(leftClamped) + PREVIEW_COLUMN_GAP + PREVIEW_PADDING_LEFT: 13 columns for option 1, 11 for option 2, both matching the captures exactly.
PreviewPane.renderSideBySide is what buries the mark mid-line (WrappingSelect appends it at end-of-line; the join glues the preview column behind it), which is why only side-by-side layout is affected.
The frame that leaked was painted through the workflows stage-chat custom-UI slot (footer ctrl+x return to graph), a path that never calls ReservedBottomOverlay.takeActiveRow — the repository's only stripper before this change.
Fix
Two layers, three edits, no restyle of surrounding code.
packages/coding-agent/src/core/extensions/ui-types.ts— terminator changed to ST:"\u001B_atomic:active\u001B\\". Safe because pi-tui'sextractAnsiCodealready accepts APC terminated by BEL or ST (dist/utils.js:382-394); verified at runtime thatvisibleWidthis still 0 andtruncateToWidthstill preserves the mark.- Same file —
stripOverlayActiveRowMarker(lines), one definition of the removal. packages/coding-agent/src/modes/interactive/interactive-tui.ts—applyLineResetsoverridden onAtomicTuiAltScreenand on a newAtomicTuiMainScreen, with the main-screen construction site repointed. That seam is pi-tui's last transform over the composited screen before it builds the byte buffer, and is where pi-tui already removes its ownCURSOR_MARKER, so it covers every component tree either renderer paints.
ReservedBottomOverlay.takeActiveRow was deliberately left alone: it needs the mark to pick the crop window and runs on the component tree long before this seam.
preview-pane.ts was deliberately not modified. Hoisting the mark to end-of-line there is redundant once the central strip lands, and editing those lines risks exactly the restyle the objective forbids.
Validation
| command | outcome |
|---|---|
npm run check |
pass (biome, tsc --noEmit, tsgo -p tsconfig.build.json --noEmit, shrinkwrap up to date) |
npm run test:unit |
685 files passed, 6756 passed / 2 skipped |
npm run test --workspace=@bastani/atomic |
475 files passed / 4 skipped, 3911 passed / 39 skipped |
npx vitest --run --project unit test/unit/ask-user-question-preview-pane.test.ts |
12 passed |
npx vitest --run test/overlay-active-row-marker-strip.test.ts (coding-agent) |
3 passed |
npx vitest --run test/ask-user-question-transcript-scroll.test.ts (owns the mark / boundOverlayLines contract) |
76 passed |
Red-green proof. With interactive-tui.ts reverted to HEAD and everything else in place, test/overlay-active-row-marker-strip.test.ts reports 2 failed / 1 passed — both renderer tests fail on written.includes(OVERLAY_ACTIVE_ROW_MARKER) === false. Restoring the file returns 3 passed.
Honest limit on the PreviewPane column tests. The pane's own arithmetic was already self-consistent before the fix, so the column-invariance assertions in ask-user-question-preview-pane.test.ts do not fail pre-fix — they pin the invariant the fix must not break. The tests that genuinely fail pre-fix are the two renderer-level ones plus the terminator-shape assertion.
Loop 2 — delivery (commit only, no push, no PR)
No file under Amendment B's "do not modify" list was touched. git diff origin/main --stat before staging was byte-identical to the reviewed delta: 4 files, +41 / −5.
Staged by explicit path, never git add -A / git add . / git commit -a, because TMUX-EVIDENCE.md is untracked and not gitignored (git check-ignore exits 1) and a blanket stage would sweep it in. git diff --cached --name-only confirmed six paths and zero matches for TMUX-EVIDENCE.md before committing.
Commit: 192a9b8fc34d776e17e07f8bdce25f5e4a4f61f8 — fix(ask-user-question): stop the active-row marker shifting the preview box, with the Assistant-model: Claude Opus 5 trailer. Six files, +388 / −5 (the +347 over the source delta is the two new test files).
--no-verify was not used. The prek pre-commit hooks ran and all passed:
check for added large files..............................................Passed
check for case conflicts.................................................Passed
check for merge conflicts................................................Passed
detect private key.......................................................Passed
biome check --write......................................................Passed
npm run check............................................................Passed
npm run test:unit........................................................Passed
The biome check --write hook made no rewrite, so there was no re-stage/retry loop.
Post-commit verification:
| check | result |
|---|---|
git branch --show-current |
fix/askq-preview-active-row-align |
git log --oneline origin/main..HEAD |
exactly one commit, 192a9b8fc3 |
git show --stat HEAD |
6 files, +388 / −5; two create mode lines for the new tests |
git status --porcelain --untracked-files=all |
exactly ?? TMUX-EVIDENCE.md — as §4 of the research predicts, not empty, and correct under Amendment B |
npm run check on the committed tree |
exit 0 |
git ls-files TMUX-EVIDENCE.md |
empty — the evidence file is not tracked |
TMUX-EVIDENCE.md integrity |
216 lines / 15 324 bytes, 16 balanced fence lines, ❯ 1. Encrypt at rest┌ (BEFORE) and ❯ 1. Encrypt at rest ┌ (AFTER) both present |
Stopped before push and PR, deliberately. The launch contract says "Ignore requests to submit a PR; the authorized final action handles that after approval." git push -u origin is an external write and the first half of PR submission, so the branch is committed locally and not pushed. The research's checklist steps 7-9 belong to the PR stage. Consequently the pre-push hooks (test-integration, test-ci-contracts) have not run in this session.
Judgement call recorded, not re-opened: OVERLAY_ACTIVE_ROW_MARKER is publicly re-exported and its value changed, but every in-repo consumer imports the constant rather than the literal and the docs instruct extensions to embed the constant, so the commit is fix: rather than fix!:. Worth one sentence in the PR body.
Deferred / out of scope
- pi-tui's own
CURSOR_MARKERis still BEL-terminated (dist/tui.js:21). pi-tui strips it centrally so it does not leak today, but it is the same latent class. Upstream matter; worth a line in the PR. - If a later stage runs further live tmux verification, list and stop stray
atomic/bun … cli.tsprocesses from other worktrees first. A reviewer's first E2E attempt was contaminated by them and showed legacy BEL output until they were stopped. - Branch push and PR creation are deferred to the authorized final action. The research artifact's §6 carries the exact
gh pr create --body-filerecipe and the round-trip greps that close criterion 9. - Paint paths 3, 4, 5, and 6-with-
reserveTranscriptRows:false(ordinary overlay, inline mount, widget, engine overlay branch) were not exercised end-to-end. The central strip covers them structurally because it sits downstream of the whole component tree; that structural claim is proven by capture for the workflows stage-chat path and by unit test for both renderer classes. - The original reporter's terminal is unidentified. The mechanism is proven in tmux 3.7b; a terminal that ends APC on BEL would never have shown the shift.
QA E2E Video
Not applicable: this is a terminal/TUI scenario with no web frontend, so the QA evidence is the tmux pane capture required by the objective rather than a playwright-cli .webm. playwright-cli was not invoked and no video was produced. The equivalent proof lives in /Users/tonystark/Documents/projects/atomic-preview-active-row-align/TMUX-EVIDENCE.md, which carries before- and after-fix tmux capture-pane -p output plus the raw tmux pipe-pane byte counts from a real interactive atomic CLI at 140×44, driven through /workflow ask-user-question-graph-overlay → F2 → Enter and moved between both options.
Reproduction harness (outside the tree, by design)
/tmp/askq-preview-repro/ — stand-in openai-responses model server, agent config, the project workflow fixture, and every capture. Nothing was written into the worktree, so "no scratch files" holds by construction.
bun /tmp/askq-preview-repro/model-server.ts /tmp/askq-preview-repro/state
tmux new-session -d -s askqprev -x 140 -y 44 -c /tmp/askq-preview-repro/project
# in the pane:
cd /tmp/askq-preview-repro/project && NODE_ENV=production \
ATOMIC_CODING_AGENT_DIR=/tmp/askq-preview-repro/agent ATOMIC_SKIP_VERSION_CHECK=1 \
bun <worktree>/packages/coding-agent/src/cli.ts --approve --offline --no-session
# then: /workflow ask-user-question-graph-overlay → ↵ → F2 → ↵ → ↓ / ↑
tmux capture-pane -p -t askqprevNODE_ENV=production is load-bearing: under a test runner the workflows extension hands stages a stub session that never calls a tool. A fresh worktree also needs packages/natives/native/atomic_natives.<platform>.node (gitignored, built only by test/global-setup-natives.ts); it was already present in this worktree.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| type TuiInputListener, | ||
| TuiMainScreen, | ||
| } from "@earendil-works/pi-tui"; | ||
| import { stripOverlayActiveRowMarker } from "../../core/extensions/ui-types.ts"; |
There was a problem hiding this comment.
This new source import uses a .ts suffix instead of the repository-required TypeScript ESM .js convention. Reference ../../core/extensions/ui-types.js so the source consistently targets the emitted module specifier.
Context Used: AGENTS.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/coding-agent/src/modes/interactive/interactive-tui.ts
Line: 11
Comment:
**ESM import suffix is inconsistent**
This new source import uses a `.ts` suffix instead of the repository-required TypeScript ESM `.js` convention. Reference `../../core/extensions/ui-types.js` so the source consistently targets the emitted module specifier.
**Context Used:** AGENTS.md ([source](https://github.com/bastani-inc/atomic/blob/main/AGENTS.md))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Declining this one — the .js convention does not govern packages/coding-agent.
AGENTS.md states the rule and its rationale together: "Source files use .js import extensions… The repo ships as .ts files; Bun resolves .js specifiers to the underlying .ts source directly — no loader hook required." That reasoning applies to the raw-TypeScript companion packages, which ship unbuilt. The very next bullet carves this package out: "packages/coding-agent is copied from upstream pi and keeps its existing build setup."
The code agrees:
| scope | .ts relative imports |
.js relative imports |
|---|---|---|
packages/coding-agent/src |
2449 | 161 |
packages/workflows/src |
0 | 1735 |
tsconfig.base.json sets allowImportingTsExtensions: true and rewriteRelativeImportExtensions: true, which is precisely the configuration that makes a .ts specifier correct here and rewrites it at emit.
And within this file, lines 12-14 are clipboard.ts, open-browser.ts, transcript-follow-indicator.ts. Taking the suggestion would make line 11 the only .js import in the file, and it would point at a path that does not exist on disk — ui-types.js is not a file, only ui-types.ts is.
Please remember this distinction: .js specifiers for the raw-TypeScript packages (workflows, subagents, mcp, web-access, intercom), .ts specifiers inside packages/coding-agent.
1 participant
Summary
The
ask_user_questionpreview box drew its border 11-13 columns early on the active option row only, in side-by-side layout.OVERLAY_ACTIVE_ROW_MARKERwas an APC string terminated by BEL; ECMA-48 permits BEL to terminate OSC but not APC, so tmux kept consuming the row padding and the preview column gap as APC payload and resumed drawing at the box border's SGR.Two layers, per the reviewed plan:
ESC \) at its definition, as the spec requires.applyLineResetson both interactive renderers — pi-tui's last transform over the composited screen, and where it already removes its ownCURSOR_MARKER. Every component tree either renderer paints converges there, so overlay, inline, widget, and workflows stage-chat mounts are covered, including hosts that never callReservedBottomOverlay.takeActiveRow(previously the only stripper in the tree, and the reason the defect escaped on the stage-chat path).Changes
packages/coding-agent/src/core/extensions/ui-types.tsOVERLAY_ACTIVE_ROW_MARKER; newstripOverlayActiveRowMarkerhelperpackages/coding-agent/src/modes/interactive/interactive-tui.tsapplyLineResetsoverrides onAtomicTuiAltScreenand a newAtomicTuiMainScreen; main-screen construction site repointedpackages/coding-agent/test/overlay-active-row-marker-strip.test.tstest/unit/ask-user-question-preview-pane.test.tsPreviewPane(12 tests)packages/coding-agent/CHANGELOG.md### Fixedsubsection under## [Unreleased]packages/coding-agent/docs/extensions.mdSix files, +388 / −5.
preview-pane.ts,wrapping-select.ts, andreserved-bottom-overlay.tsare deliberately untouched.Validation
npm run check— clean (biome,tsc --noEmit,tsgo -p tsconfig.build.json --noEmit, shrinkwrap up to date).npm run test:unit— 685 files, 6756 passed / 2 skipped.npm run test --workspace=@bastani/atomic— 475 files, 3911 passed / 39 skipped.origin/main, the renderer test is 3 failed / 3 and the PreviewPane test 6 failed / 6; restoring the fix returns both to green.[33,33,33,33,33,33]after the fix for option 1, option 2, and option 1 again; before the fix[20,33,...]and[33,33,22,...]. Full captures below.Notes for the reviewer
OVERLAY_ACTIVE_ROW_MARKERis publicly re-exported and its value changed, but every in-repo consumer imports the constant rather than the literal, and the docs instruct extensions to embed the constant — so this isfix:, notfix!:.CURSOR_MARKERis still BEL-terminated (dist/tui.js:21). It does not leak today because pi-tui strips it centrally, but it is the same latent class. Upstream matter.TMUX-EVIDENCE.mdis intentionally not committed — it is evidence, not shipped content, and AGENTS.md keeps scratch artifacts out of the tree. It is carried verbatim in this body instead.Implementation notes
Implementation Notes —
ask_user_questionpreview box misaligns on the active option rowWorktree:
/Users/tonystark/Documents/projects/atomic-preview-active-row-alignBranch:
fix/askq-preview-active-row-align(already checked out at stage start; base commit3646705921, nevermain)Contract amendments received
Amendment A — inherited from loop 1 (user-authored, verbatim):
Implemented as required behavior in loop 1: both layers landed (ST terminator at the definition; central strip in
applyLineResetson both renderers), and the fix was verified in the same real tmux + real CLI scenario.Amendment B — received for loop 2 (user-authored, verbatim):
Amendment B was executed literally in loop 2. Nothing under its "do not modify" list was touched:
git diff origin/mainfor the four source/docs files is byte-identical to the reviewed+41/−5, and both new test files were committed unchanged.Amendment B vs. Amendment B's own step 3 and the "do not re-run the suites" clause — resolved. The repository's prek
pre-commithook runsnpm run checkandnpm run test:unitand cannot be skipped without--no-verify, which thegh-commitskill forbids in capitals. Those runs are mandated, not discretionary, so committing without--no-verifyhonours both clauses at once. Recorded rather than treated as a conflict.No new user steering arrived during loop 2 beyond Amendment B.
Acceptance matrix
fix/askq-preview-active-row-align, nevermaingit branch --show-current→fix/askq-preview-active-row-align; all edits under that pathatomicCLItmux pipe-pane -oon a live CLI:APC atomic:active occurrences: 2 | BEL-term: 2 | ST-term: 0, mark mid-line followed by 13 pad+gap spacestmux capture-pane -poutput stored verbatim inTMUX-EVIDENCE.mdin the worktreeTMUX-EVIDENCE.mdpresent at 216 lines / 15 324 bytes, 16 balanced fence lines, both signature capture lines found bygrep -F. Deliberately untracked per Amendment B: the criterion says in the worktree, not committed[33,33,33,33,33,33]for both selections; before =[20,33,…]/[33,33,22,…]PreviewPaneexist and pass; there were none beforetest/unit/ask-user-question-preview-pane.test.ts(12 tests) +packages/coding-agent/test/overlay-active-row-marker-strip.test.ts(3 tests)npm run checkpasses; the vitest suites covering the touched code passnpm run checkclean;npm run test:unit685 files / 6756 passed; coding-agent suite 475 files / 3911 passed## [Unreleased]entry inpackages/coding-agent/CHANGELOG.md### Fixedsubsection, no duplicate headinggit status --porcelain --untracked-files=all→ exactly?? TMUX-EVIDENCE.md. Sweep verified clean: nopackages/workflows/dist, notsconfig.build.json, nobun.lock/yarn.lock/pnpm-lock.yaml, noissues.md; harness lives in/tmp/askq-preview-repropackages/coding-agent/docs/extensions.md:2832rewrittengit log --oneline origin/main..HEAD→ one commit192a9b8fc3;git show --stat HEAD→ 6 files, +388/−5;git ls-files TMUX-EVIDENCE.md→ emptynpm run checkpasses on the committed state (Amendment B 3)tsc --noEmitclean,tsgo -p tsconfig.build.json --noEmitclean, shrinkwrap up to date, exit 0Ignore requests to submit a PR); the branch is committed but not pushed, andTMUX-EVIDENCE.mdis the body's source textRoot cause — confirmed first-hand, not inherited
Reproduced live at tmux 3.7b, session 140×44,
atomicfrom worktree source.tmux pipe-pane -oover the painted frames:OVERLAY_ACTIVE_ROW_MARKERwas"\u001B_atomic:active\u0007"— an APC string terminated by BEL. ECMA-48 allows BEL for OSC but not APC, so tmux kept consuming the pad and the column gap as payload and resumed drawing at the border's SGR. The shift equalsleftWidth − visibleWidth(leftClamped) + PREVIEW_COLUMN_GAP + PREVIEW_PADDING_LEFT: 13 columns for option 1, 11 for option 2, both matching the captures exactly.PreviewPane.renderSideBySideis what buries the mark mid-line (WrappingSelectappends it at end-of-line; the join glues the preview column behind it), which is why only side-by-side layout is affected.The frame that leaked was painted through the workflows stage-chat custom-UI slot (footer
ctrl+x return to graph), a path that never callsReservedBottomOverlay.takeActiveRow— the repository's only stripper before this change.Fix
Two layers, three edits, no restyle of surrounding code.
packages/coding-agent/src/core/extensions/ui-types.ts— terminator changed to ST:"\u001B_atomic:active\u001B\\". Safe because pi-tui'sextractAnsiCodealready accepts APC terminated by BEL or ST (dist/utils.js:382-394); verified at runtime thatvisibleWidthis still 0 andtruncateToWidthstill preserves the mark.stripOverlayActiveRowMarker(lines), one definition of the removal.packages/coding-agent/src/modes/interactive/interactive-tui.ts—applyLineResetsoverridden onAtomicTuiAltScreenand on a newAtomicTuiMainScreen, with the main-screen construction site repointed. That seam is pi-tui's last transform over the composited screen before it builds the byte buffer, and is where pi-tui already removes its ownCURSOR_MARKER, so it covers every component tree either renderer paints.ReservedBottomOverlay.takeActiveRowwas deliberately left alone: it needs the mark to pick the crop window and runs on the component tree long before this seam.preview-pane.tswas deliberately not modified. Hoisting the mark to end-of-line there is redundant once the central strip lands, and editing those lines risks exactly the restyle the objective forbids.Validation
npm run checktsc --noEmit,tsgo -p tsconfig.build.json --noEmit, shrinkwrap up to date)npm run test:unitnpm run test --workspace=@bastani/atomicnpx vitest --run --project unit test/unit/ask-user-question-preview-pane.test.tsnpx vitest --run test/overlay-active-row-marker-strip.test.ts(coding-agent)npx vitest --run test/ask-user-question-transcript-scroll.test.ts(owns the mark /boundOverlayLinescontract)Red-green proof. With
interactive-tui.tsreverted to HEAD and everything else in place,test/overlay-active-row-marker-strip.test.tsreports 2 failed / 1 passed — both renderer tests fail onwritten.includes(OVERLAY_ACTIVE_ROW_MARKER) === false. Restoring the file returns 3 passed.Honest limit on the PreviewPane column tests. The pane's own arithmetic was already self-consistent before the fix, so the column-invariance assertions in
ask-user-question-preview-pane.test.tsdo not fail pre-fix — they pin the invariant the fix must not break. The tests that genuinely fail pre-fix are the two renderer-level ones plus the terminator-shape assertion.Loop 2 — delivery (commit only, no push, no PR)
No file under Amendment B's "do not modify" list was touched.
git diff origin/main --statbefore staging was byte-identical to the reviewed delta: 4 files, +41 / −5.Staged by explicit path, never
git add -A/git add ./git commit -a, becauseTMUX-EVIDENCE.mdis untracked and not gitignored (git check-ignoreexits 1) and a blanket stage would sweep it in.git diff --cached --name-onlyconfirmed six paths and zero matches forTMUX-EVIDENCE.mdbefore committing.Commit:
192a9b8fc34d776e17e07f8bdce25f5e4a4f61f8—fix(ask-user-question): stop the active-row marker shifting the preview box, with theAssistant-model: Claude Opus 5trailer. Six files, +388 / −5 (the +347 over the source delta is the two new test files).--no-verifywas not used. The prekpre-commithooks ran and all passed:The
biome check --writehook made no rewrite, so there was no re-stage/retry loop.Post-commit verification:
git branch --show-currentfix/askq-preview-active-row-aligngit log --oneline origin/main..HEAD192a9b8fc3git show --stat HEADcreate modelines for the new testsgit status --porcelain --untracked-files=all?? TMUX-EVIDENCE.md— as §4 of the research predicts, not empty, and correct under Amendment Bnpm run checkon the committed treegit ls-files TMUX-EVIDENCE.mdTMUX-EVIDENCE.mdintegrity❯ 1. Encrypt at rest┌(BEFORE) and❯ 1. Encrypt at rest ┌(AFTER) both presentStopped before push and PR, deliberately. The launch contract says "Ignore requests to submit a PR; the authorized final action handles that after approval."
git push -u originis an external write and the first half of PR submission, so the branch is committed locally and not pushed. The research's checklist steps 7-9 belong to the PR stage. Consequently the pre-push hooks (test-integration,test-ci-contracts) have not run in this session.Judgement call recorded, not re-opened:
OVERLAY_ACTIVE_ROW_MARKERis publicly re-exported and its value changed, but every in-repo consumer imports the constant rather than the literal and the docs instruct extensions to embed the constant, so the commit isfix:rather thanfix!:. Worth one sentence in the PR body.Deferred / out of scope
CURSOR_MARKERis still BEL-terminated (dist/tui.js:21). pi-tui strips it centrally so it does not leak today, but it is the same latent class. Upstream matter; worth a line in the PR.atomic/bun … cli.tsprocesses from other worktrees first. A reviewer's first E2E attempt was contaminated by them and showed legacy BEL output until they were stopped.gh pr create --body-filerecipe and the round-trip greps that close criterion 9.reserveTranscriptRows:false(ordinary overlay, inline mount, widget, engine overlay branch) were not exercised end-to-end. The central strip covers them structurally because it sits downstream of the whole component tree; that structural claim is proven by capture for the workflows stage-chat path and by unit test for both renderer classes.QA E2E Video
Not applicable: this is a terminal/TUI scenario with no web frontend, so the QA evidence is the tmux pane capture required by the objective rather than a
playwright-cli.webm.playwright-cliwas not invoked and no video was produced. The equivalent proof lives in/Users/tonystark/Documents/projects/atomic-preview-active-row-align/TMUX-EVIDENCE.md, which carries before- and after-fixtmux capture-pane -poutput plus the rawtmux pipe-panebyte counts from a real interactiveatomicCLI at 140×44, driven through/workflow ask-user-question-graph-overlay→F2→Enterand moved between both options.Reproduction harness (outside the tree, by design)
/tmp/askq-preview-repro/— stand-inopenai-responsesmodel server, agent config, the project workflow fixture, and every capture. Nothing was written into the worktree, so "no scratch files" holds by construction.NODE_ENV=productionis load-bearing: under a test runner the workflows extension hands stages a stub session that never calls a tool. A fresh worktree also needspackages/natives/native/atomic_natives.<platform>.node(gitignored, built only bytest/global-setup-natives.ts); it was already present in this worktree.TMUX evidence —
ask_user_questionpreview box misaligned on the active option rowVerbatim
tmux capture-pane -poutput from a real interactiveatomicCLI, before and after the fix.Environment (identical for every capture)
TERM=tmux-256color, sessionaskqprev, geometry 140 columns x 44 rowsatomicbuilt from this worktree's source (packages/coding-agent/src/cli.ts), version 0.0.0 (versionless release base), base commit 3646705/workflow ask-user-question-graph-overlay->F2(graph viewer) ->Enter(stage chat) -> the stage's realask_user_questiondialog, whose two options carry box-drawing previews. Footer readsctrl+x return to graph, so the frame is painted through the workflows stage-chat custom-UI slot.tmux capture-pane -p -t askqprev, pane rows 25-44 (the dialog region), reproduced verbatim below.Root cause, from the raw byte stream
tmux pipe-pane -oover the same painted frames, before the fix. The active row carriesOVERLAY_ACTIVE_ROW_MARKERmid-line, terminated by BEL:Measured on that capture:
APC atomic:active occurrences: 2 | BEL-term: 2 | ST-term: 0.ECMA-48 lets BEL terminate OSC but not APC; tmux's own
input.csays so explicitly. So tmuxkeeps consuming bytes as APC payload, eats the pad and the column gap, and resumes drawing at the
border's SGR — the border lands early by exactly
leftWidth - visibleWidth(leftClamped) + PREVIEW_COLUMN_GAP + PREVIEW_PADDING_LEFT: 13 columns for option 1, 11 for option 2. Both match thecaptures below.
PreviewPane.renderSideBySideis what buries the mark mid-line —WrappingSelectappends it atend-of-line and the side-by-side join glues the preview column after it. That is why only
side-by-side layout is affected.
BEFORE the fix
BEFORE — option 1 selected (
❯ 1. Encrypt at rest)BEFORE — option 2 selected (
❯ 2. Plaintext storage)BEFORE — cursor moved back to option 1 (
❯ 1. Encrypt at rest)AFTER the fix
AFTER — option 1 selected (
❯ 1. Encrypt at rest)AFTER — option 2 selected (
❯ 2. Plaintext storage)AFTER — cursor moved back to option 1 (
❯ 1. Encrypt at rest)Machine-measured border column
Column index of the first box-drawing character (
┌,│,└) on each box row of the capture:[20, 33, 33, 33, 33, 33][33, 33, 22, 33, 33, 33][20, 33, 33, 33, 33, 33][33, 33, 33, 33, 33, 33][33, 33, 33, 33, 33, 33][33, 33, 33, 33, 33, 33]Every after-fix row is column 33. Before the fix the active row was 20 (option 1 selected) and 22 (option 2 selected) — 13 and 11 columns early, matching the formula above.
Raw byte stream after the fix
Same scenario,
tmux pipe-pane -oover the same two selections (2064 bytes captured):That zero is the load-bearing result: the mark no longer reaches the terminal on a paint path that
never calls
ReservedBottomOverlay.takeActiveRow.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Greptile Summary
This change removes the internal active-row marker from final interactive terminal output and adds regression coverage for renderer and preview-pane behavior.
One repository import-convention issue remains: the new
interactive-tui.tsimport uses a.tssuffix rather than the required emitted.jssuffix.Confidence Score: 4/5
The rendering behavior is covered, but the new import should follow the repository’s required ESM suffix convention before merging.
There is one non-security P2 finding and no P0 or P1 findings, which corresponds to a score of 4.
Files Needing Attention: packages/coding-agent/src/modes/interactive/interactive-tui.ts
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix(ask-user-question): stop the active-..." | Re-trigger Greptile
Context used: