Skip to content

feat(cli): offer AI build debug in onboarding TUI#2328

Draft
WcaleNieWolny wants to merge 14 commits into
mainfrom
claude/charming-ramanujan-df5074
Draft

feat(cli): offer AI build debug in onboarding TUI#2328
WcaleNieWolny wants to merge 14 commits into
mainfrom
claude/charming-ramanujan-df5074

Conversation

@WcaleNieWolny
Copy link
Copy Markdown
Contributor

Summary

  • The Ink-based build init wizard renders build output through React state, but on failure the underlying requestBuildInternal calls @clack/prompts (confirm / select / spinner) to offer AI analysis. Clack and Ink fight for the terminal, so the AI offer either corrupts the TUI or hangs — users in onboarding never get a usable AI debug option.
  • Adds aiAnalysisMode: 'auto-prompt' | 'caller-handled' | 'skip' to BuildRequestOptions so callers like the onboarding wizard can suppress the clack flow and run the AI step themselves with Ink-native components. Default 'auto-prompt' keeps the direct-CLI matrix (and CI/CD detection) untouched.
  • Both the iOS (cli/src/build/onboarding/ui/app.tsx) and Android (cli/src/build/onboarding/android/ui/app.tsx) wizards gain three new Ink steps — ai-analysis-prompt (Select: Debug with AI / Skip), ai-analysis-running (SpinnerLine), ai-analysis-result (renders the markdown analysis + AI safety warning, then exits).
  • PostHog: extends AiAnalysisTriggeredBy with 'onboarding' (additive) so the existing dashboard can segment onboarding-driven AI usage. Reuses trackBuilderOnboardingStep, trackAiAnalysisChoice, trackAiAnalysisResult unchanged.
  • Build infra: TypeScript 6.0.3 turns the deprecated baseUrl into a hard error, breaking bun run build on main. Adding "ignoreDeprecations": "6.0" per TS's own recommendation unblocks the build. Unrelated to the feature but bundled here so the binary is buildable on the branch.

Design plan lives in the wiki at projects/capgo/plans/2026-05-22-onboarding-ai-debug.md (intentionally not committed to the repo).

CI/CD safety

No new detection layer. Two existing constraints handle CI by construction:

Caller aiAnalysisMode TTY? Outcome
Direct CLI 'auto-prompt' (default) yes Existing clack flow (show_menu / ask_then_menu) — unchanged
Direct CLI 'auto-prompt' (default) no (CI) decideAnalyzeBehaviorskip or auto_upload — unchanged
Onboarding TUI 'caller-handled' yes (Ink needs TTY) Logs captured → result.aiAnalysis.ready=true → Ink prompts user
Onboarding TUI 'caller-handled' no (CI) aiAnalysis undefined → wizard goes straight to build-complete

shouldCaptureLogs() is already TTY-gated, and Ink itself requires a TTY to render anything.

Test plan

  • Unit: cli/test/test-ai-onboarding-mode.mjs — 9 tests, all green
    • decideAnalyzeBehavior matrix unchanged (regression guard for direct CLI)
    • runCapgoAiAnalysis happy path posts to /build/ai_analyze with correct shape
    • runCapgoAiAnalysis returns too_big over 10 MB without calling backend
    • runCapgoAiAnalysis returns error when the captured log is missing
    • releaseCapturedLogs deletes the log and is best-effort on missing files
  • Regression: existing test-ai-analyze-flow, test-ai-log-capture, test-ai-render-markdown, test-onboarding-progress, test-onboarding-recovery all pass
  • Build: bun run build (CLI workspace) produces a dist/index.js binary
  • Manual: trigger a failing iOS build inside build init → verify Ink prompt renders cleanly, AI flow runs without TUI corruption, analysis displays readably
  • Manual: same on Android
  • Manual: direct CLI build request on a failing build → confirm clack-based menu still appears (no regression)
  • Manual: CI sanity — npx @capgo/cli build request --platform ios … | cat → no prompts, no Ink, no crash

Out of scope (intentional)

  • Dashboard-side AI debug button in BuildTable.vue (the dashboard doesn't have full logs; would require a server-side log fetch endpoint).
  • build ai-debug <jobId> re-run subcommand.
  • Local AI option in the Ink prompt — kept to two options (Debug / Skip) for the simplest first cut.

🤖 Generated with Claude Code

The Ink-based `build init` wizard runs `requestBuildInternal` with a custom
logger so build output flows through React state. On failure today the build
code still tries to drive the AI flow with `@clack/prompts` (`confirm`,
`select`, `spinner`) — but Ink owns the terminal, so those clack writes/reads
collide with React redraws and the AI prompt either corrupts the TUI or hangs.
Net effect: users in the onboarding wizard never get a usable AI debug offer.

This change adds a `caller-handled` mode so the wizard can drive the AI UX
with Ink-native components.

Layering (matches docs/superpowers/specs in wiki):

- `cli/src/ai/analyze.ts`
  - `runCapgoAiAnalysis({ apiHost, apikey, jobId, appId })` reads the
    `/tmp/capgo-builds/<jobId>.log` file, returns `too_big` over 10 MB,
    `error { message: 'log_unavailable' }` if missing, otherwise delegates
    to `postAnalyzeRequest`.
  - `releaseCapturedLogs(jobId)` thin wrapper around
    `cleanupCapturedJobFiles({ keepAiPromptFile: false })`.

- `cli/src/ai/telemetry.ts`
  - Extend `AiAnalysisTriggeredBy` with `'onboarding'` so PostHog can
    segment onboarding-driven AI usage from CLI menu / CI flag.

- `cli/src/schemas/build.ts`
  - `BuildRequestOptions.aiAnalysisMode?: 'auto-prompt' | 'caller-handled' | 'skip'`
    (default `'auto-prompt'` — direct CLI invocation matrix unchanged).
  - `BuildRequestResult.aiAnalysis?: { jobId, capturedLogPath, ready }`.

- `cli/src/build/request.ts`
  - Branch the failure-AI block on `aiAnalysisMode`. `'caller-handled'`
    keeps the captured log alive (via `keepPromptFile=true`) and surfaces
    it on the result so callers can run `runCapgoAiAnalysis`. `'skip'`
    no-ops. `'auto-prompt'` keeps the existing clack flow.
  - Capture stays enabled in caller-handled mode regardless of TTY so the
    captured log is always available to the caller; explicit `'skip'`
    suppresses capture entirely.

- `cli/src/build/onboarding/{types,android/types}.ts`
  - New `OnboardingStep`/`AndroidOnboardingStep`: `'ai-analysis-prompt'`,
    `'ai-analysis-running'`, `'ai-analysis-result'`. `STEP_PROGRESS` +
    `getPhaseLabel` entries.

- `cli/src/build/onboarding/{ui,android/ui}/app.tsx`
  - Pass `aiAnalysisMode: 'caller-handled'` to `requestBuildInternal`. On
    failure with `aiAnalysis.ready === true`, transition to
    `'ai-analysis-prompt'` (Ink `<Select>` with Debug-with-AI / Skip).
    Pick "Debug" -> `'ai-analysis-running'` (`<SpinnerLine>`) which calls
    `runCapgoAiAnalysis` and transitions to `'ai-analysis-result'`. Render
    the analysis via existing `renderMarkdown` (ANSI passes through Ink's
    `<Text>`). On exit, call `releaseCapturedLogs(jobId)`.
  - PostHog: `trackAiAnalysisChoice({ ..., triggeredBy: 'onboarding' })`
    fires when the user picks Debug or Skip; `trackAiAnalysisResult` fires
    after `postAnalyzeRequest` resolves. Existing
    `trackBuilderOnboardingStep` continues to fire on each new step.

CI/CD safety: no new detection. The two existing layers handle it —
`shouldCaptureLogs()` gates on TTY, and Ink itself requires a TTY to render.
In CI, capture is off, `result.aiAnalysis` is undefined, and the wizard goes
straight to `build-complete`.

Build infra: TypeScript 6.0.3 turns the deprecated `baseUrl` into a hard
error. Adding `"ignoreDeprecations": "6.0"` per TS recommendation unblocks
`bun run build` on main. Unrelated to the feature but bundled here so the
binary is buildable on the branch.

Test coverage:
- `cli/test/test-ai-onboarding-mode.mjs` (9 tests, all passing):
  - `decideAnalyzeBehavior` matrix unchanged (regression guard)
  - `runCapgoAiAnalysis` happy path, too-big, missing log
  - `releaseCapturedLogs` deletes the log + is best-effort
- Existing `test-ai-analyze-flow`, `test-ai-log-capture`,
  `test-ai-render-markdown`, `test-onboarding-progress`, and
  `test-onboarding-recovery` all still pass.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 22, 2026

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: c0f92b81-6b44-44cd-b2d7-0be0d44fa79d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands and usage tips.

@socket-security
Copy link
Copy Markdown

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff Package Supply Chain
Security
Vulnerability Quality Maintenance License
Addedemulate@​0.5.07110010093100
Addedgit-format-staged@​4.0.1961009483100
Addeddayjs@​1.11.2010010010084100
Addedeslint-plugin-format@​2.0.19910010085100
Addeddrizzle-orm@​1.0.0-rc.1971008899100
Addedeslint@​10.2.18910010096100
Addeddompurify@​3.4.2981001009490
Addeddotenv@​17.4.29910010092100
Addeddiscord-api-types@​0.38.4710010010095100

View full report

@codspeed-hq
Copy link
Copy Markdown
Contributor

codspeed-hq Bot commented May 22, 2026

Merging this PR will not alter performance

✅ 43 untouched benchmarks
⏩ 2 skipped benchmarks1


Comparing claude/charming-ramanujan-df5074 (79fae03) with main (6ab0cec)

Open in CodSpeed

Footnotes

  1. 2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

After the AI analysis renders, the user often needs to apply the suggested
fix in their editor and re-run the build. Previously the only option from
'ai-analysis-result' was "Continue" -> exit, which forced the user to
re-launch the wizard (and re-run credential discovery) just to re-attempt
the build.

This change lets the user retry directly from inside the wizard:

- 'ai-analysis-result' now shows two options when retries remain:
    🔄  I fixed it, retry build (N retries left)
    ⏭   Continue (skip retry)
- On retry: release the captured log for the current jobId, reset AI state
  (aiJobId / aiAnalysisText / aiResultMessage), increment aiRetryCount,
  and transition back to 'requesting-build'. The existing useEffect for
  that step re-runs `requestBuildInternal` and (on failure) routes back
  through the AI flow with the new jobId.
- After MAX_AI_RETRIES (= 2) the retry option is removed and a single
  "Continue" is shown. A dim note explains how many retries were used.
- Total wizard-internal attempts capped at 3: initial + 2 retries.

PostHog: extend `AiAnalysisChoice` with 'retry' (additive). The Choice
event fires per-retry-attempt, carrying the current jobId — so the
funnel can measure "AI suggestion was actionable enough to merit a
retry" and "retry led to a successful subsequent build" as separate
metrics by joining on app/job.

Mirrored to both iOS (cli/src/build/onboarding/ui/app.tsx) and Android
(cli/src/build/onboarding/android/ui/app.tsx) wizards.

Privacy: as before, only closed-enum choice values cross the telemetry
boundary; the AI diagnosis text is never observed.
If the on-failure AI diagnosis is taller than the user's terminal viewport,
rendering it inline causes the earlier lines to scroll off the top before the
retry/skip picker appears — which we explicitly do not want in the onboarding
wizard. Earlier lines scrolled into the terminal history aren't the "active"
screen, so the user can't see both the diagnosis and the choices at the same
time and the wizard ends up in a confusing state.

This change adds a conservative fit estimator and routes overflowing analyses
through a new fullscreen scrollable viewer modeled on the existing
`FullscreenDiffViewer` from main.

### Fit estimator — `cli/src/build/onboarding/ai-fit.ts`

`isAiAnalysisTooTall(text, terminalRows, terminalCols)` estimates rendered
rows (per logical line × wrap factor, ignoring ANSI escapes for length) and
compares against `terminalRows - AI_RESULT_CHROME_ROWS` (default 20). The
20-row chrome reserve is deliberately generous: prefer a false-positive
scroll (one extra keystroke) over a false-negative inline render (lines
disappear off the top).

### Scrollable viewer — `cli/src/build/onboarding/ui/components.tsx`

`FullscreenAiViewer` mirrors the shape of `FullscreenDiffViewer` on main but
takes pre-rendered ANSI text lines instead of structured diff lines.
Keybindings: ↑/↓ or j/k (line), PgUp/PgDn or u/d/Space (page), g/G (top/
bottom), Esc/Enter (dismiss). Shows "Showing N-M of TOTAL lines" footer and a
status-aware exit hint.

### New step — `'ai-analysis-result-scroll'` (iOS + Android)

Added to both `OnboardingStep` and `AndroidOnboardingStep`, plus
`STEP_PROGRESS` and `getPhaseLabel`. The outer wizard Header / progress bar
/ log panel are hidden during this step so the viewer gets the full screen
(same pattern main uses for `view-workflow-diff`).

### Wizard wiring (both iOS and Android `ui/app.tsx`)

- New `aiViewedFull` state. The step useEffect for `ai-analysis-result`
  checks fit and transitions to `ai-analysis-result-scroll` when needed.
- The scroll step renders `<FullscreenAiViewer />`; on exit it sets
  `aiViewedFull=true` and returns to `ai-analysis-result`.
- The inline `ai-analysis-result` render now branches on `aiViewedFull`:
  if true, replaces the body dump with a compact "📖 Analysis already
  shown above (scroll your terminal back to re-read it)" marker so the
  retry/skip picker is always visible.
- Retry handler resets `aiViewedFull` so the next attempt re-evaluates
  fit against the new analysis text.

### Test coverage

`cli/test/test-ai-fit.mjs` (12 tests, all green):
- ANSI strip
- Empty / short / overflowing inputs
- Borderline conservative case
- Single very long wrapping line

Existing `test-ai-onboarding-mode`, `test-ai-analyze-flow`,
`test-ai-log-capture`, `test-ai-render-markdown` continue to pass — no
regression in the underlying flow.
Two bugs caused the FullscreenAiViewer to push content into the terminal's
scrollback history, forcing the user to scroll the terminal emulator (not
the viewer) to see the analysis:

1. Dimensions were captured at mount time. When the terminal was resized
   the viewer kept its original `viewportRows` and chrome reserve, so the
   actual rendered output could be taller (or shorter) than the live screen.

2. Both `viewportRows` and `maxScrollOffset` treated each logical line as
   one terminal row. Long lines wrap, so a "10-row" slice could actually
   render as 30+ rows on a narrow terminal — overflowing the viewport.

This change addresses both:

- `FullscreenAiViewer` now subscribes to `stdout.on('resize', ...)` and
  stores `{ rows, cols }` in state. Every resize triggers a re-render that
  recomputes the viewport, max scroll offset, divider width, and visible
  slice against the live dimensions.

- Two new helpers in `ai-fit.ts`:
    `pickVisibleLines(lines, scrollOffset, viewportRows, terminalCols)`
       — wrap-aware slice. Stops adding logical lines once their cumulative
         wrapped row count would exceed the viewport. Always includes at
         least one line so the viewer body is never empty.
    `computeMaxScrollOffset(lines, viewportRows, terminalCols)`
       — wrap-aware scroll bound. Walks backwards from the last line,
         packing as many tail lines as fit (counting wrap), and returns the
         offset of the first fully-visible tail line.

- Chrome reserve bumped from 8 to 10 rows to absorb chrome lines that
  themselves wrap on narrow terminals (subtitle, position line, exit hint
  are all sentence-length and can wrap on <60-col terminals).

- Dividers now scale to `min(60, cols - 1)` so they never wrap and silently
  eat a viewport row.

Test coverage in `test-ai-fit.mjs` (22 tests, all green):
- ANSI strip, simple row counting, wrap accounting (preserved)
- `pickVisibleLines`: empty input, scroll past end, simple slice, wrap-
  triggered early stop, one-line floor for hostile single lines, offset.
- `computeMaxScrollOffset`: empty input, viewport-larger-than-content,
  simple packing, wrap-aware tail packing.
…hrome

Ink renders in normal-terminal mode, so each step transition leaves the
previous frame in the user's scrollback. When the AI prompt step's Select
fires and the wizard transitions to `ai-analysis-running` (or beyond), the
new frame previously rendered its own Header — producing the visible
artifact of two stacked "🚀 Capgo Cloud Build · Onboarding" boxes:

  ╔══════════════════════════╗   ← frozen above by the previous frame
  ║  Capgo Cloud Build       ║
  ║  · Onboarding            ║
  ╚══════════════════════════╝
  AI debug · 92%
  ...
  > Debug with AI ✔
    Skip
  ╔══════════════════════════╗   ← duplicate, rendered by the new frame
  ║  Capgo Cloud Build       ║
  ║  · Onboarding            ║
  ╚══════════════════════════╝
  AI debug · 95%
  ...
  Analyzing build log with Capgo AI (Kimi K2.5)...

This was already partly addressed for `ai-analysis-result` and
`ai-analysis-result-scroll`. Extending the same suppression to
`ai-analysis-prompt` and `ai-analysis-running` keeps the entire AI
sub-flow Header-free, so the most recent Header in scrollback is the
one tied to the user's current AI step instead of a stale earlier one.

The frozen "Debug with AI ✔ / Skip" line is left visible — that's
normal Ink behavior in non-alternative-screen mode (the previous
frame's content stays in scrollback) and would require a fullscreen
mode switch to address, out of scope here.

Mirrored to both iOS (cli/src/build/onboarding/ui/app.tsx) and Android
(cli/src/build/onboarding/android/ui/app.tsx) wizards.
… steps)

The previous fix went too far and hid the Header on every AI sub-flow
step including the entry one (`ai-analysis-prompt`). That left the user
seeing a chrome-less "Build failed." prompt with no wizard anchoring,
which felt jarring — the screenshot showed an isolated `AI debug`
phase label + progress bar over a bare error message and Select.

Refining: keep the Header on `ai-analysis-prompt` (the entry into the
AI sub-flow — first thing the user sees, needs wizard anchoring) and
only suppress it on the steps that render AFTER the prompt's Select
fires (`ai-analysis-running`, `ai-analysis-result`,
`ai-analysis-result-scroll`). Those are the ones at risk of stacking a
fresh Header below the frozen previous frame in the user's scrollback.

Net result:
- First entry to AI flow: Header + AI debug + progress + "Build failed"
  + Select  — looks like a normal wizard step, no missing chrome.
- User picks: Select freezes that frame into scrollback.
- New frame (running): no Header — frozen Header from prompt remains the
  most recent one visible, no duplicate.
- Same for result / result-scroll.

Mirrored to both iOS and Android wizards.
…plicates

The previous "hide Header on post-Select AI steps" fix removed the
duplicate banner artifact, but at the cost of leaving the AI-running
step Header-less — the wizard then looked decapitated as soon as the
user picked "Debug with AI".

The right tool for this is Ink's `<Static>`. Static items are written
to the terminal exactly once, above the dynamic render area, and never
re-rendered. Putting the Header inside `<Static>` gives us:

- A single persistent "🚀 Capgo Cloud Build · Onboarding" banner that
  the user sees on every step (including the whole AI sub-flow).
- Zero risk of a duplicate banner ever appearing — Ink does not re-emit
  static content on subsequent renders, so step transitions, terminal
  resizes, and the @inkjs/ui Select committing its selected line into
  scrollback all leave the banner untouched.

The previous `showHeader` conditional and the `isPostPromptAiStep`
helper are no longer needed; removed in both wizards. `showLog` and
`showProgress` keep their existing per-step semantics — only the Header
moved to Static.

Module-level `STATIC_HEADER_ITEMS = ['header']` keeps the items array
reference stable across renders so Static doesn't ever decide a new
item appeared. Ink's Static items prop is typed as a mutable `string[]`,
so the constant is declared accordingly (the array is never mutated in
practice).

Mirrored to both iOS (cli/src/build/onboarding/ui/app.tsx) and Android
(cli/src/build/onboarding/android/ui/app.tsx) wizards.
This replaces the previous Ink-Static-Header approach (which made the
banner permanent at the cost of losing it on `requesting-build` and the
scrollable AI viewer) with terminal alt-screen mode for the whole wizard.

In alt-screen mode the terminal uses a separate buffer where each Ink
frame fully replaces the previous one. That removes the underlying cause
of the duplicate-Header artifact (Ink committing each step's frame into
scrollback) and unlocks the original `showHeader` conditional, so we can
go back to:

- Header VISIBLE on every interactive step including the entire AI
  sub-flow (`ai-analysis-prompt`, `ai-analysis-running`,
  `ai-analysis-result`) — fixes the "no banner on Analyzing build log"
  regression.
- Header HIDDEN on `requesting-build` and `ai-analysis-result-scroll`
  — those steps get the full terminal height for build output and the
  scrollable AI viewer, as before the Static experiment.

### Mechanics

`command.ts`:
- `enterAltScreen()` writes `ESC[?1049h ESC[H` (enter alt buffer + cursor
  home) before `render()`.
- `exitAltScreen()` writes `ESC[?1049l` after `waitUntilExit()` so the
  user's pre-wizard terminal content is restored on clean exit.
- Process-level cleanup handlers (`exit` / `SIGINT` / `SIGTERM` /
  `uncaughtException`) also call `exitAltScreen()` so the user is never
  stranded in an alt buffer if something crashes.
- After exit, prints a one-line summary
  `✔ Capgo onboarding complete for <appId> (<platform>).` so the user
  has a visible breadcrumb in the normal terminal flow that the wizard
  finished (the wizard's last frame is wiped by the buffer restore —
  same UX as vim/htop/less, which is the expected behavior for a TUI).

### iOS / Android wizards

- Reverted commit 2d7ea70 (Ink Static for Header).
- `showHeader = step !== 'requesting-build' && !isAiResultScroll` — the
  same conditional intent as before the duplicate-Header bug ever
  surfaced.
- Removed the now-redundant `isPostPromptAiStep` helper.

### Trade-offs

- The wizard's per-step output is no longer left in scrollback when the
  user exits — that's the expected TUI behavior and is documented by the
  completion summary line.
- Power users who wanted to scroll back through wizard steps DURING the
  flow lose that ability (alt buffer doesn't keep scrollback). They can
  use the wizard's own back/retry affordances instead.
The Static-Header approach kept the banner visible across every step
(including the entire AI sub-flow), with no duplicate-banner artifact at
step transitions and no need for alt-screen mode. The remaining gripe
was the row cost on `requesting-build` and the scrollable AI viewer —
the double-bordered box was 4-5 rows tall, eating real estate where
vertical space matters.

This change replaces the bordered box with a compact two-row banner:

  🚀  Capgo Cloud Build · Onboarding
  ────────────────────────────────

A single bold line + a thin dim divider gives the same "I'm in the Capgo
wizard" anchoring at one third of the row cost. Heavy steps now lose
~2 rows instead of ~4-5.

No code changes outside `components.tsx` — `<Static>` and the call sites
already render whatever the Header component returns.
Static was rejected by the user. Restoring conditional Header rendering:

- `showHeader = step !== 'requesting-build' && !isAiResultScroll`
- Header is shown on every interactive step including the full AI
  sub-flow (prompt → running → result), giving the user the "Capgo
  Cloud Build · Onboarding" anchoring they explicitly asked for on the
  AI running step.
- Header is hidden on `requesting-build` and on the scrollable AI
  viewer so those steps get the full terminal height as before.

The Header itself stays in its compact two-row form (bold line + thin
divider) introduced earlier — when visible it only costs ~2 rows, and
when hidden the user gets the full screen on heavy steps.

Static is gone from both `ui/app.tsx` files: no more `<Static>` wrapper,
no `STATIC_HEADER_ITEMS` constant, and the `Static` symbol is dropped
from the `ink` import.

Trade-off acknowledged in earlier reviews: if Ink's render diff hits a
size-change edge case (e.g. terminal narrow enough that the prompt
frame scrolls), the previous frame's content can persist in the user's
scrollback. We're accepting that artifact rather than going back to
Static or to alternative-screen mode, both of which the user has
explicitly rejected.
The compact two-row variant was introduced solely to reduce the row cost when the Header was always visible (via Static). Now that Header is conditional again and hidden on heavy steps (requesting-build, scroll viewer), the original double-bordered banner is back — same visual identity as the rest of the wizard.
…place

The scrollable AI viewer's content area used `pickVisibleLines`, which
greedily packs logical lines into `viewportRows` rendered rows but stops
short if the next line would overflow. That means the visible content's
rendered row count is variable: scrolling can pick lines whose wrap
totals are different from the previous slice. Each render then has a
slightly different total height.

Ink renders in normal-terminal mode, so a 1-row height change between
renders writes the new frame below the old one and the user perceives
"scrolling just added an extra line at the bottom" — the previous frame
stays in scrollback right above the new one.

Fix: after picking the visible lines, compute how many rendered rows
they actually used (`totalRenderedRows`) and pad with empty <Text> rows
to fill the remainder of `viewportRows`. The viewer's chrome was already
constant; padding the body makes the WHOLE frame constant, so Ink's
diff replaces the previous frame in-place on every scroll keystroke.

Added `totalRenderedRows` to `ai-fit.ts` (sum of `renderedRowsForLine`
across an array of lines) as a small shared helper. All 22 existing
fit tests still pass.
`isAiAnalysisTooTall` is deliberately conservative — it routes the
analysis through the scrollable viewer whenever the estimate is close
to the chrome-adjusted row budget. In practice that means we sometimes
end up in the viewer with an analysis that fits the viewport outright:
`maxScrollOffset === 0`, every logical line visible at once, no
scrolling possible.

Previously the chrome still advertised scrolling in that case:

- Subtitle: "15 lines — scrollable because the analysis is taller than
  your terminal"  (← lie when all 15 fit)
- Footer:   "Showing 1-15 of 15 lines. ↑/↓ or PgUp/PgDn to scroll."
            (← pointless when there's nothing to scroll past)

Now those strings are conditional on `maxScrollOffset > 0`:

- Subtitle is omitted when scrolling would be a no-op (its only job
  was the "scrollable" advisory, which doesn't apply).
- Footer reads "Showing all N lines." instead.
- Exit hint collapses to the "Press Esc or Enter to continue to the
  retry/skip prompt." variant — the "when done" wording only makes
  sense if there's something the user is "done with".

No behavior change when scrolling IS needed; only the false-positive
copy is suppressed.
@sonarqubecloud
Copy link
Copy Markdown

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.

1 participant