Skip to content

[code-quality-layout-manifest-fetch] Layout fetches the palette manifest it already imports statically #6362

Description

@atomantic

Slice audited: navigation backbone — server/lib/navManifest.js, server/routes/palette.js, client/src/components/Layout.jsx and the section tab headers. Audit date: 2026-09-06.

Problem

client/src/components/Layout.jsx:134 imports NAV_COMMANDS statically from server/lib/navManifest.js (since #5053, 2026-08-25, when the sidebar started deriving its rows from the manifest). The same component still fetches the manifest over HTTP on every mount:

  • Layout.jsx:761-769api.getPaletteManifest() into manifestNav state (added 2026-06-04 for Collapse sidebar to 8–10 top-level domains + Pinned + Recent #713, when the sidebar had no static access to the manifest).
  • Layout.jsx:771-779manifestEntryByPath is built from the fetched copy.
  • Layout.jsx:876-889resolveNavEntry calls migrateLegacyNavPath(path, manifestNav) with the fetched copy.
  • Layout.jsx:163 — the api import exists only for this call.

server/routes/palette.js:64-66 builds the nav half of the response from the very same NAV_COMMANDS object the client bundle already contains.

Impact

  • One redundant request per page load, for a payload the bundle already carries.
  • Until the fetch resolves, resolveNavEntry returns null for the 63 manifest entries that have no sidebar presentation (/wiki/log, /goals/tree, …) and for every stored path that must go through previousPaths migration. Pinned/Recent rows for those paths render nothing on first paint and pop in later, and useNavWorkingSet's normalize effect (client/src/hooks/useNavWorkingSet.js:96-107) re-runs when the fetch lands.
  • If the fetch fails (network, expired auth on a password-gated install), those rows stay missing for the whole session with only a console warning.
  • client/src/components/Layout.test.jsx:71-78 has to mock getPaletteManifest with a hand-written one-entry manifest, so the tests exercise a fake manifest rather than the real one.

Fix

Use the static import for everything:

  1. Delete the fetch and the manifestNav state. Build manifestEntryByPath from NAV_COMMANDS (a module-level map; keep the feature filter inside the existing useMemo on isFeatureEnabled).
  2. Pass NAV_COMMANDS to migrateLegacyNavPath in resolveNavEntry.
  3. Remove the api import from Layout.jsx and the getPaletteManifest mock from Layout.test.jsx; keep the eidoverse previousPaths assertion, now against the real manifest.

Leave client/src/components/CmdKSearch.jsx:133 and client/src/components/voice/VoiceWidget.jsx:175 alone — they need the actions half, which only the server can hydrate from the voice tool registry — and leave the endpoint itself as-is (documented for companion apps in docs/COMPANION_APP_API.md:138).

Acceptance criteria

  • Layout.jsx contains no getPaletteManifest call and no manifestNav state; git grep getPaletteManifest client/src/components/Layout.jsx is empty.
  • A stored pinned path with no sidebar presentation (e.g. /wiki/log) renders its Pinned row on the first render with no network mock — add this case to Layout.test.jsx.
  • A stored path listed in some command's previousPaths (the existing eidoverse case) still resolves to the current path on first render without the getPaletteManifest mock.
  • cd client && npm test -- Layout passes; server/lib/navManifest.test.js and server/routes/palette.test.js are unchanged and pass.

Scope: small

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

code-qualityProposed from a code-quality auditeffort:lowEffort: lowmodel:lightModel size: lightplanTracked by /do:replanplanner:fable-5-1Plan authored by the fable-5-1 model

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions