Skip to content

fix(dashboard): bundle backdrop filler image via Vite import (#24355) - #24713

Closed
Tranquil-Flow wants to merge 1 commit into
NousResearch:mainfrom
Tranquil-Flow:fix/24355-bundle-filler-bg-via-vite
Closed

fix(dashboard): bundle backdrop filler image via Vite import (#24355)#24713
Tranquil-Flow wants to merge 1 commit into
NousResearch:mainfrom
Tranquil-Flow:fix/24355-bundle-filler-bg-via-vite

Conversation

@Tranquil-Flow

@Tranquil-Flow Tranquil-Flow commented May 13, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

When this PR was first opened, web/src/components/Backdrop.tsx still imported the filler image via the fragile /ds-assets/filler-bg0.* public path that depended on the sync-assets prebuild hook firing. Direct vite build, partial prebuild failures, or _build_web_ui's stale-dist fallback (#23817) would silently 404 the asset and the default backdrop would disappear. The PR switched the import to @nous-research/ui/assets/filler-bg0.webp so Vite bundles a content-hashed copy into web_dist/assets/.

That Backdrop.tsx change has since landed on main independently (along with removal of the redundant sync-assets / predev / prebuild scripts), so the original code change is no longer needed.

This PR now carries the two remaining pieces around that landed change:

  1. Regression contract test (tests/tools/test_backdrop_filler_bundling.py) — asserts Backdrop.tsx does NOT reference the fragile public path and DOES import via the design-system package's ./assets/* export. The import-pattern assertion uses a regex that tolerates either quote style and either .webp / .jpg extension so future ds-asset bumps or reformats don't false-positive the contract.

  2. @nous-research/ui bump 0.14.0 → 0.15.0 per @austinpickett's request — the breaking changes that surfaced the original regression originate in the 0.15.0 release. Verified that 0.15.0 still ships dist/assets/filler-bg0.webp + dist/hooks/use-gpu-tier + every dist/ui/components/{button,list-item,selection-switcher,spinner,badge,select,tabs,command-block} import used in web/src/. npm run build passes clean.

Related Issue

Addresses #24355 (original code change already on main; this PR adds the regression test + the upstream-requested ui bump)

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • tests/tools/test_backdrop_filler_bundling.py (new, 63 LOC, 2 tests) — contract guard against re-introducing the /ds-assets/ public-path dependency in Backdrop.tsx.
  • web/package.json — bump @nous-research/ui from 0.14.0 to 0.15.0.
  • web/package-lock.json — regenerated against 0.15.0.

How to Test

  1. pytest tests/tools/test_backdrop_filler_bundling.py -q → 2 passed.
  2. cd web && npm install && npm run build → vite build passes clean; web_dist/assets/filler-bg0-<hash>.webp emitted.
  3. Reverting either Backdrop.tsx import on a local branch should fail one of the two contract assertions.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 15 (Darwin 24.6.0)

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Screenshots / Logs

$ pytest tests/tools/test_backdrop_filler_bundling.py -q
..                                                                       [100%]
2 passed in 8.68s

$ cd web && npm run build
vite v7.3.2 building client environment for production...
✓ 2067 modules transformed.
../hermes_cli/web_dist/assets/filler-bg0-RwOqXSl_.webp    931.56 kB
../hermes_cli/web_dist/assets/index-DK2Uerqs.js         1,581.97 kB │ gzip: 460.21 kB
✓ built in 6.08s

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/tui Terminal UI (ui-tui/ + tui_gateway/) labels May 13, 2026
@austinpickett
austinpickett requested a review from Copilot May 18, 2026 14:49
Comment thread web/src/components/Backdrop.tsx Outdated
// Import directly so Vite bundles the asset; the prior `/ds-assets/...`
// path 404s whenever the `npm run sync-assets` prebuild hook is bypassed
// (issue #24355).
import fillerBg from "@nous-research/ui/assets/filler-bg0.jpg";

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

fillerBg has moved to filler-bg0.webp

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed — import now references filler-bg0.webp (verified via npm pack: the @nous-research/ui@0.10.0 lockfile-pinned version ships .jpg, but 0.15.0 ships only .webp so any lockfile bump would have broken the old path). New head: 798ec38.

Copilot AI left a comment

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.

Pull request overview

This PR fixes a dashboard regression where the default backdrop filler image could 404 when sync-assets didn’t populate web/public/ds-assets/ (e.g., direct vite build, partial prebuild failure, or stale-dist fallback). It switches the filler image to be bundled via a Vite-resolved module import so the asset is emitted into web_dist/assets/ with a content-hashed name.

Changes:

  • Update Backdrop to import the filler image via @nous-research/ui/assets/filler-bg0.jpg and use the bundled URL in <img src>.
  • Add a contract test ensuring Backdrop.tsx doesn’t reference the fragile /ds-assets/... path and does import via the design system package export.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
web/src/components/Backdrop.tsx Replaces a fragile public-path image reference with a Vite-bundled asset import.
tests/tools/test_backdrop_filler_bundling.py Adds a regression/contract test to prevent reintroducing the /ds-assets/ dependency for the default backdrop filler.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +50 to +55
source = BACKDROP.read_text(encoding="utf-8")
assert '"@nous-research/ui/assets/filler-bg0.jpg"' in source, (
"Backdrop.tsx must import the filler image from "
"`@nous-research/ui/assets/filler-bg0.jpg` so Vite bundles it. "
"See #24355."
)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Loosened — the assertion is now a regex tolerating either quote style and either extension:

pattern = re.compile(
    r'''from\s+['"]@nous-research/ui/assets/filler-bg0\.(?:webp|jpg)['"]'''
)
assert pattern.search(source), ...

Formatting changes (single vs double quotes, whitespace, multi-line import) no longer break the contract. Negative check on /ds-assets/ is unchanged. New head: 798ec38.

@Tranquil-Flow

Copy link
Copy Markdown
Contributor Author

@austinpickett — fixed the filler-bg rename: import now references filler-bg0.webp (matching the moved asset).

Also addressed the Copilot inline note on test_backdrop_filler_bundling.py:55 — the contract assertion is now a regex that tolerates either quote style and either extension, so future formatting changes don't break it.

New head: 798ec38. Ready for another look.

@austinpickett

Copy link
Copy Markdown
Collaborator

It's important to bump the @nous-research/ui package first, 0.15.0. There's some breaking changes that were caused from that.

https://design.nousresearch.com

@Tranquil-Flow

Copy link
Copy Markdown
Contributor Author

It's important to bump the @nous-research/ui package first, 0.15.0. There's some breaking changes that were caused from that.

https://design.nousresearch.com

Copy that, will integrate this.

@austinpickett

Copy link
Copy Markdown
Collaborator

@copilot resolve the merge conflicts in this pull request

@Tranquil-Flow
Tranquil-Flow force-pushed the fix/24355-bundle-filler-bg-via-vite branch from 798ec38 to 81f7083 Compare May 19, 2026 13:40
@Tranquil-Flow
Tranquil-Flow requested a review from a team May 19, 2026 13:40
@Tranquil-Flow

Copy link
Copy Markdown
Contributor Author

@austinpickett — done, and the PR is now narrower than it was.

Turns out the Backdrop.tsx .webp import change itself has already landed on main independently (and the redundant sync-assets / predev / prebuild scripts were removed) — so the code change this PR was originally about is no longer needed.

Reformulated the PR around the two remaining pieces:

  1. @nous-research/ui bump 0.14.0 → 0.15.0 (your ask). Verified 0.15.0 ships every component + hook + asset path that web/src/ imports; npm run build passes clean with the bumped version.
  2. tests/tools/test_backdrop_filler_bundling.py — regression contract test (2 cases) that ensures Backdrop.tsx can't silently regress back to the /ds-assets/ public-path approach. Tolerant regex on the import pattern so it accepts either .webp / .jpg and either quote style.

New head: 81f70832. MERGEABLE. Diff is now +292 / −1975 (the deletions are the redundant 0.10.0 lockfile entries the rebase cleaned up against the 0.14.0-baseline main).

Happy to split the ui bump out as its own PR if you'd prefer the test landing separately.

austinpickett
austinpickett previously approved these changes May 20, 2026

@austinpickett austinpickett left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review: fix(dashboard): bundle backdrop filler image via Vite import

Contract test + @nous-research/ui version bump. The actual Vite import fix landed in a prior commit; this PR adds the regression guard.

(item 1) Contract test is well-designed — negative check (no /ds-assets/filler-bg0 literal) + positive check (import from @nous-research/ui/assets/). Regex tolerates quote style and extension changes. Thorough docstring.

(item 2) Version bump 0.14.0 → 0.15.0 is a minor bump that presumably adds the ./assets/* package export map entry Vite needs. Pinned version appropriate for internal design system.

(item 3) 4500+ line package-lock.json diff is mechanical.

Verdict: Approve ✅ — Clean regression guard.

Reviewed by Hermes Agent

@Tranquil-Flow
Tranquil-Flow force-pushed the fix/24355-bundle-filler-bg-via-vite branch from 81f7083 to 09b386f Compare May 25, 2026 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/tui Terminal UI (ui-tui/ + tui_gateway/) P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants