Skip to content

promote NAV_COMMANDS to the structural source of truth for the sidebar, leaving Layout.jsx presentation-only #5053

Description

@atomantic

Problem

Navigation is defined twice and the two copies must be hand-kept in sync:

  • server/lib/navManifest.jsNAV_COMMANDS, the source for ⌘K and voice ui_navigate.
  • client/src/components/Layout.jsx — the navItems tree the sidebar renders.

Every navigation attribute that matters to both has to be written in both places. Instance feature gating just added another one (feature), joining path, label, and section. Today that is held together by source-scrape guards in server/lib/navManifest.test.js (path→feature parity, section coverage, App.jsx route coverage) rather than by a shared structure.

Scrape guards catch drift after the fact and only for what they scrape. A guard cannot catch an attribute nobody thought to scrape.

Decision already made (do not re-litigate)

Tagging both lists plus a parity guard was the right altitude for the PR that introduced feature gating — it matched the repo's established convention and kept that change small. This issue is the follow-up to remove the duplication itself, not a revisit of that call.

Why it is not a trivial merge

The two lists genuinely differ, and the sidebar carries presentation the manifest has no business holding:

  • lucide icon components per row (icon: Dog)
  • external rows with href + dynamicHost (Autofixer at //:5560) that have no manifest entry at all
  • dynamic children (apps, pipeline series, universes) hydrated at runtime
  • defaultTo, single, separator, moreLabel, end, activePathPrefix, grandchildren
  • deliberate ordering (alphabetical within a section, after the Dashboard/OpenWorld block) that differs from manifest order

Proposed shape

Make NAV_COMMANDS the structural source of truth — path, label, section, feature — and let Layout.jsx supply presentation only, keyed by path:

const NAV_PRESENTATION = { '/devtools/jira': { icon: Ticket } };

Rows with no manifest entry (external href, dynamic groups) stay declared locally and are explicitly marked as such. Then:

  • feature is read from the manifest — the sidebar cannot drift on it, and the path→feature parity guard becomes unnecessary.
  • A new page added to NAV_COMMANDS shows up in the sidebar once someone gives it an icon, instead of being silently missing.
  • Adding an attribute later means adding it once.

Verification

  • The existing guards in server/lib/navManifest.test.js must keep passing throughout; delete only the ones the structure makes impossible to violate, and say which in the PR.
  • client/src/components/Layout.test.jsx covers ordering, section expansion, pinned/recent resolution, and feature gating — all must stay green.
  • Confirm the sidebar renders identically before/after on a full install: same rows, same order, same sections expanded.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    effort:highDispatch reasoning effort: highmodel:heavyDispatch capability: strongest available coding modelplanTracked by /do:replan

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions