fix(tui): add spacing between status line and composer#768
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c497ec61bd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| this.ui.addChild(this.todoContainer); | ||
| this.ui.addChild(this.btwContainer); | ||
| this.ui.addChild(this.statusLine); // Main status rail + hook statuses; composer chrome is rendered by the editor. | ||
| this.ui.addChild(new Spacer(COMPOSER_STATUS_SPACING_ROWS)); |
There was a problem hiding this comment.
Avoid adding a second composer spacer
In a normal initialized session with no above-editor hook widget, hookWidgetContainerAbove already contributes one blank row (it is initialized with new Spacer(1) and rebuilt with spacerWhenEmpty=true). Adding this separate spacer before that container makes the default layout render two blank rows between the status line and the composer, which contradicts the new test's expected single row and adds more vertical gap than intended.
Useful? React with 👍 / 👎.
| }); | ||
|
|
||
| it("renders one blank row between status line and composer", () => { | ||
| const rendered = mode.ui.render(48).map(line => stripVTControlCharacters(line)); |
There was a problem hiding this comment.
Initialize the UI before asserting its layout
This test only constructs new InteractiveMode(...); the UI component tree is populated later in InteractiveMode.init() where the status line, hook container, and editor container are added. With the current setup mode.ui.render(48) renders an empty tree, so both indices stay -1 and the test fails before it can validate composer spacing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9cd78f3480
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| it("renders one blank row immediately above the composer", async () => { | ||
| vi.spyOn(mode.ui, "start").mockImplementation(() => {}); | ||
|
|
||
| await mode.init(); |
There was a problem hiding this comment.
Disable launch side effects before full init
In environments where gh is installed and authenticated for a user who has not starred the repo, this full init() call uses the default starReminder.enabled=true and startup.quiet=false path to schedule scheduleLaunchStarReminderAfterFirstRender, which can start an async gh check and potentially open a confirm prompt after the test assertions/teardown. This makes the layout unit test depend on developer/CI GitHub CLI state; disable that setting or mock the reminder path before invoking full initialization.
Useful? React with 👍 / 👎.
|
REQUEST_CHANGES Blockers:
Verification performed:
This is not safe to squash-merge until the layout contract is corrected and the test asserts the real rendered spacing instead of the internal child order that currently proves two blank rows. — |
|
Addressed the review blockers in c26e7bf.
Verification:
— |
|
MERGE_READY Re-reviewed head Confirmed:
Safe to squash-merge to — |
Summary
Verification
bun test packages/coding-agent/test/interactive-mode-editor-component.test.tsand package-scoped variant; local harness failed before assertions due missing workspace dependency/linkage (@gajae-code/agent-core/ test harness dependency setup). CI should validate in the PR environment.Closes #758.
—
[repo owner's gaebal-gajae (clawdbot) 🦞]