Skip to content

refactor(tui): simplify TUI layer (-299 net lines)#809

Open
kelsonpw wants to merge 7 commits into
mainfrom
refactor/simplify-tui
Open

refactor(tui): simplify TUI layer (-299 net lines)#809
kelsonpw wants to merge 7 commits into
mainfrom
refactor/simplify-tui

Conversation

@kelsonpw
Copy link
Copy Markdown
Collaborator

@kelsonpw kelsonpw commented May 17, 2026

Summary

Seven low-risk simplifications to src/ui/tui/ with no behavior changes. 380 lines deleted, 81 added — net -299 lines, 4469 tests pass (unchanged from baseline).

Commits (each reviewable independently)

  1. Extract KeyHintInline primitive for [K] label rows — replaces 8 verbose six-line <Box><Text>[</Text><Text bold>K</Text><Text>] label</Text></Box> blocks in AuthScreen, DataSetupScreen, DataIngestionCheckScreen with a single one-line component. Refactors KeyHintBar to share the same primitive.
  2. Share EMPTY_HINTS between hint hooksuseEscapeBack had its own private copy of the frozen empty array; now imports from useScreenHints. Also hoists SignupFullNameScreen's useMemo (empty deps) to a module-level frozen const, matching the existing pattern in RegionSelectScreen / IntroScreen.
  3. Lookup tables for JourneyStepper glyph/color — replaces the nested ternary chains with two Record<StepState, ...> lookups. The doneSuccess state-overlay stays explicit.
  4. Delete dead primitivesCardLayout and SplitView were exported via primitives/index.ts but never imported anywhere; same for the HAlign / VAlign enums in styles.ts.
  5. Delete dead AmplitudeLogo / AnimatedAmplitudeLogo — replaced by DiscoveryFeed in PR feat(tui): polish Welcome screen — tighter copy, inline workspace picks, smaller logo on returning runs #677; only references left are historical comments. 151 lines of gradient/wave animation math.
  6. Pre-build hint constants in SignupEmailScreen — the per-render useMemo switched on a single boolean; replaced with two module-level frozen hint arrays + a ternary.
  7. Merge duplicate primitives import in ConsoleView — two separate import { X } from '../primitives/index.js' lines collapsed into one.

Patterns identified but deferred

  • Signup-flow signupRequiredFields.includes('terms_acceptance') / 'full_name' checks repeat in flows.ts — load-bearing flow predicates with subtle correctness guarantees around back-nav walls and isWall predicates. Worth a dedicated refactor with tests targeting those specific invariants; not a drop-in win.
  • Dynamic import('../../../lib/registry.js') in IntroScreen's FrameworkPicker — appears twice, but both are gated on user action (entering the picker / picking a framework) and serve startup-cost goals. Merging would require a module-level cache.
  • isPickerAction typeguard in AuthScreen — only used twice in the same file; not worth abstracting further.
  • Duplicate /help registration in console-commands.ts — looks like a regression from PR feat(tui): register /help slash command and Tab autocomplete in palette #769 but removing the dupe could change which descriptor COMMANDS.find() returns; better handled as a discrete bug-fix PR with its own test.

Tests

  • Before: 4469 passed (297 files)
  • After: 4469 passed (297 files)
  • Snapshots regenerated: 0 (rendered output is byte-identical for every replaced block — KeyHintInline produces the same <Box><Text>[</Text>…<Text>]…</Text></Box> tree)

Verification

  • pnpm tsc --noEmit — green
  • pnpm lint — green
  • pnpm test — green, same count
  • src/utils/wizard-abort.ts — untouched
  • src/lib/ — untouched (sibling agent territory)

Test plan

  • pnpm tsc --noEmit passes
  • pnpm lint passes (prettier + eslint)
  • pnpm test — 4469 tests pass
  • pnpm exec vitest run --pool=forks --maxWorkers=1 src/ui/tui/ — 1019 tests pass

Note

Low Risk
Low risk refactor limited to TUI rendering and imports, mainly deleting unused components and replacing repetitive UI fragments with shared helpers. Primary risk is minor visual regressions in key-hint/stepper presentation due to refactoring.

Overview
Simplifies TUI UI plumbing and removes dead code by extracting a reusable KeyHintInline (and updating KeyHintBar plus several screens to use it) and by centralizing the shared EMPTY_HINTS constant for screen hint hooks.

Refactors JourneyStepper to use state→glyph/color lookup tables instead of nested ternaries, collapses duplicate imports in ConsoleView, and deletes unused/obsolete components (AmplitudeLogo, CardLayout, SplitView, plus HAlign/VAlign tokens) and their barrel exports.

Reviewed by Cursor Bugbot for commit 4fa7077. Bugbot is set up for automated code reviews on this repo. Configure here.

kelsonpw added 7 commits May 16, 2026 20:36
Replaces 8 verbose 6-line inline blocks in AuthScreen / DataSetupScreen /
DataIngestionCheckScreen with a one-line component. Also refactors the
KeyHintBar internals to share the same primitive so the bracketed-hint
visual contract has one definition.

No rendered output changes — every replaced block produced the exact
same Box+Text tree.
- useEscapeBack now imports EMPTY_HINTS from useScreenHints instead of
  duplicating the same frozen-empty-array constant locally.
- SignupFullNameScreen's hint list had no dependencies, so the useMemo
  wrapper was dead weight; replaced with a module-level frozen const,
  matching the existing pattern in RegionSelectScreen and IntroScreen.

No behavioral change.
Replace the nested ternary chains in JourneyStepper.render with two
Record<StepState, ...> lookup tables. The doneSuccess override
remains an explicit if, since it's a state-overlay rather than a
distinct step state.

No rendered output changes; existing snapshot tests pass.
…Align

Both primitives and their associated alignment enums were exported via
src/ui/tui/primitives/index.ts and src/ui/tui/styles.ts but never
imported anywhere in src/. No tests reference them. Removing.

- src/ui/tui/primitives/CardLayout.tsx (30 lines)
- src/ui/tui/primitives/SplitView.tsx (40 lines)
- HAlign / VAlign enums from styles.ts (only CardLayout used them)

No behavior change.
Both exports are orphaned — RunScreen replaced the animated logo with a
DiscoveryFeed panel back in #677, and no other file imports either
component. Only references left in the repo are historical context
inside comments. Removing 151 lines of gradient/wave animation math.

No behavior change.
Replace per-render useMemo with two module-level frozen hint arrays.
The hint set switches purely on canGoBack(), which has two discrete
outcomes — pre-building both avoids the closure + useMemo allocation
and matches the established pattern in RegionSelectScreen and
SignupFullNameScreen.

No behavioral change.
Two separate imports from primitives/index.js for SlashCommandInput
and PickerMenu — collapse into one.
@kelsonpw kelsonpw requested a review from a team as a code owner May 17, 2026 03:47
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Celebration hint loses gap={1} spacing between elements
    • Added an optional gap prop to KeyHintInline and passed gap={1} at the celebration Continue hint call site to restore the original inter-element spacing.

Create PR

Or push these changes by commenting:

@cursor push 3c758ec12d
Preview (3c758ec12d)
diff --git a/src/ui/tui/components/KeyHintBar.tsx b/src/ui/tui/components/KeyHintBar.tsx
--- a/src/ui/tui/components/KeyHintBar.tsx
+++ b/src/ui/tui/components/KeyHintBar.tsx
@@ -34,8 +34,16 @@
  * one-off hotkey advertisements; the persistent footer bar at the
  * bottom of the layout is `KeyHintBar` itself.
  */
-export const KeyHintInline = ({ hint, label }: { hint: string; label: string }) => (
-  <Box>
+export const KeyHintInline = ({
+  hint,
+  label,
+  gap,
+}: {
+  hint: string;
+  label: string;
+  gap?: number;
+}) => (
+  <Box gap={gap}>
     <Text color={Colors.muted}>[</Text>
     <Text color={Colors.body} bold>
       {hint}

diff --git a/src/ui/tui/screens/DataIngestionCheckScreen.tsx b/src/ui/tui/screens/DataIngestionCheckScreen.tsx
--- a/src/ui/tui/screens/DataIngestionCheckScreen.tsx
+++ b/src/ui/tui/screens/DataIngestionCheckScreen.tsx
@@ -325,13 +325,11 @@
    */
   async function refreshToken(force = false): Promise<boolean> {
     try {
-      const { getStoredToken, getStoredUser, storeToken } = await import(
-        '../../../utils/ampli-settings.js'
-      );
+      const { getStoredToken, getStoredUser, storeToken } =
+        await import('../../../utils/ampli-settings.js');
       const { refreshAccessToken } = await import('../../../utils/oauth.js');
-      const { EXPIRY_BUFFER_MS } = await import(
-        '../../../utils/token-refresh.js'
-      );
+      const { EXPIRY_BUFFER_MS } =
+        await import('../../../utils/token-refresh.js');
       const user = getStoredUser();
       const stored = getStoredToken(user?.id, user?.zone);
       if (!stored || !user) return false;
@@ -450,15 +448,15 @@
         );
         // Fall back to the first org if the stored ID doesn't match (stale checkpoint).
         const org = currentSession.selectedOrgId
-          ? userInfo.orgs.find((o) => o.id === currentSession.selectedOrgId) ??
-            userInfo.orgs[0]
+          ? (userInfo.orgs.find((o) => o.id === currentSession.selectedOrgId) ??
+            userInfo.orgs[0])
           : userInfo.orgs[0];
         // Fall back to the first project if the stored ID doesn't match.
         const project =
           org && currentSession.selectedProjectId
-            ? org.projects.find(
+            ? (org.projects.find(
                 (p) => p.id === currentSession.selectedProjectId,
-              ) ?? org.projects[0]
+              ) ?? org.projects[0])
             : org?.projects[0];
 
         const restoredFields: Parameters<typeof store.restoreSessionIds>[0] =
@@ -948,7 +946,7 @@
 
         <Box marginTop={1}>
           {celebrationReady ? (
-            <KeyHintInline hint="Enter" label="Continue" />
+            <KeyHintInline hint="Enter" label="Continue" gap={1} />
           ) : (
             <Text color={Colors.body}>Verifying{Icons.ellipsis}</Text>
           )}

You can send follow-ups to the cloud agent here.

Reviewed by Cursor Bugbot for commit 4fa7077. Configure here.

</Text>
<Text color={Colors.muted}>] Continue</Text>
</Box>
<KeyHintInline hint="Enter" label="Continue" />
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Celebration hint loses gap={1} spacing between elements

Low Severity

The old celebration-state "Continue" hint used <Box gap={1}>, which adds 1 character of spacing between each <Text> child (rendering as [ Enter ] Continue). The replacement KeyHintInline uses a plain <Box> with no gap prop (defaults to 0), rendering as [Enter] Continue. This is a subtle visual change despite the PR claiming byte-identical output for every replaced block.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 4fa7077. Configure here.

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