[Srinidhi444]

Description

Fixes #1820

Lazy-loads ink-big-text and ink-gradient instead of importing them at startup. Both the welcome banner and Nanocoder Shape settings preview now use a shared, cached asynchronous loader with a lightweight text fallback.

Startup module comparison:

Metric	Before	After
Total modules	1,504	1,461
Banner-related modules	29	0

This removes 43 modules (~2.9%) from the startup import graph.

Type of Change

• Bug fix
• New feature
• Breaking change
• Documentation update

Changeset

• Added a changeset (pnpm changeset) describing this change for the changelog

Docs-only or internal chores need no changeset (or run pnpm changeset --empty to note that intentionally).

Testing

Automated Tests

• New features include passing tests in .spec.ts/tsx files
• All existing tests pass (pnpm test:all completes successfully)
• Tests cover both success and error scenarios

Manual Testing

• Tested with Ollama
• Tested with OpenRouter
• Tested with OpenAI-compatible API
• Tested MCP integration (if applicable)

Checklist

• If this was for an open issue, I was assigned to it
• Code follows project style guidelines
• Self-review completed
• Documentation updated (if needed)
• No breaking changes (or clearly documented)
• Appropriate logging added using structured logging (see CONTRIBUTING.md)

[github-actions]

nc-review: comments — 2 important, 3 nits

@Srinidhi444 — a few things worth a look, none blocking.

The PR replaces the eager ink-big-text / ink-gradient imports in welcome-message.tsx and the Nanocoder Shape settings panel with a shared cached useBannerModules hook that dynamically imports both and falls back to plain bold text, and adds a changeset plus two test files. The approach is correct and matches what issue #1820 asked for; the loader memoises the promise, resets it on failure so a transient error cannot poison later mounts, and both call sites sit before any early return so hook order is stable. What holds it back is test quality: several pre-existing banner assertions were left reading lastFrame() synchronously, which after this change pass because the art has not loaded yet rather than for the reason they were written to check, and the whole welcome-message.spec.tsx file was mechanically converted to test.serial even though AVA is already configured with "serial": true globally.

🟠 important · tests · source/components/welcome-message.spec.tsx:719

WelcomeMessage drops the wordmark when the font is wider than the terminal was left untouched by the diff — still test(...) and still asserting against a synchronous lastFrame():

savePreferences({nanocoderShape: '3d'});
const output = stripAnsi(
	renderWithTheme(<WelcomeMessage availableRows={40} />).lastFrame() ?? '',
);
t.notRegex(output, THREE_D_GLYPHS, 'a wordmark that cannot fit must be dropped');
t.notRegex(output, BLOCK_GLYPHS, 'and no other font stands in for it');

At 30 columns logoText is null, so useBannerModules(false) never loads the modules and both notRegex assertions now pass against the plain-text fallback — i.e. the test no longer distinguishes "the wordmark was correctly dropped for being too wide" from "no wordmark was ever rendered". This is the exact failure mode the project rubric calls out: a test that can go green while proving nothing, and it makes the suite more dangerous, not less. Same class of problem in the second half of sizes the row budget to the chosen font, where the withBlock render is still read synchronously and asserted with t.notRegex(withBlock, BLOCK_GLYPHS, 'the taller default does not') — that too now passes purely because the dynamic import has not resolved. Give both a waitForWordmark(lastFrame, BLOCK_GLYPHS)-style wait (or assert on the loaded art explicitly) so the assertions bite.

🟠 important · tests · source/components/welcome-message.spec.tsx

The waitForWordmark helper threads the load correctly through most of the file, but the tests it cannot rescue now depend on module-cache warm-up order. useBannerModules seeds state from the module-level loadedBannerModules (useState(() => loadedBannerModules)), so whether a given synchronous lastFrame() sees the art depends on whether an earlier test in the same process already triggered the import. That makes several of the converted assertions order-dependent in a way a reader cannot see from the test body. Making the whole file test.serial (which the diff does) papers over this by fixing ordering globally rather than making each test self-sufficient — a fresh-process or filtered-run test would behave differently. Consider asserting through the wait helper everywhere the art matters, so each test stands on its own.

⚪ nit · scope · source/components/welcome-message.spec.tsx

Roughly thirty tests were mechanically rewritten from test( to test.serial(, but package.json already sets the AVA config to "serial": true (line 165), so every one of those conversions is a no-op. It inflates the diff and makes the real edits hard to spot. Dropping the redundant conversions and keeping only the lines that genuinely changed would leave a reviewable diff.

⚪ nit · design · source/hooks/use-banner-modules.ts

The rejection handler swallows the error entirely:

() => {
	// Keep the plain-text fallback when the optional modules fail.
},

The comment describes the user-visible behaviour, not why the failure is invisible. A module that cannot resolve is a real (if unlikely) install/packaging problem, and per CONTRIBUTING's logging section a logger.debug/warn here would make it diagnosable instead of a silent permanent plain-text banner. There is also no retry: bannerModulesPromise is reset but nothing re-runs the effect, so a transient failure leaves the component plain until it unmounts. Both are defensible as-is — just worth an explicit decision rather than an empty block.

⚪ nit · tests · source/hooks/banner-lazy-loading.spec.tsx

The eager-import guard is a good regression test and correctly excludes import type and import() expressions, but it hard-codes a three-file list. Any new module that statically imports ink-big-text will sail through. Walking the source/ tree and matching the package names (excluding type-only imports, as it already does) would make the guard actually comprehensive — and it would still be cheap, since it is pure file reading.

---

_{🔴 blocking · 🟠 a reviewer would ask for a change · ⚪ optional}

_{Automated code review — correctness, security, design, tests, plus duplicates and scope. A human still decides; this is not a substitute for review and is not exhaustive. The required status checks separately cover lint, formatting, types, unused dependencies, the test suite and the build. This bot never merges. Maintainers can rerun with /re-review.}

[Srinidhi444]

hey @addyCooks please review it and let me know if there are any issues

[Srinidhi444]

hey @addyCooks any updates