Skip to content

ENG-3278: Improve nav search with keyword matching and relevance ranking#7952

Merged
rayharnett merged 10 commits into
mainfrom
jpople/eng-3278/nav-search-improvement
May 6, 2026
Merged

ENG-3278: Improve nav search with keyword matching and relevance ranking#7952
rayharnett merged 10 commits into
mainfrom
jpople/eng-3278/nav-search-improvement

Conversation

@jpople
Copy link
Copy Markdown
Contributor

@jpople jpople commented Apr 17, 2026

Ticket ENG-3278

Description Of Changes

Enhances the admin UI nav search to support keyword/alias matching and multi-tier relevance ranking. Previously, only exact title substring matches surfaced results. Now, users can find pages by common synonyms (e.g. typing "TCF" finds "Vendors", "GDPR" finds "Locations" and "Regulations", "DSR" finds "Request manager").

Also fixes a bug where Ant Design's AutoComplete was applying its own built-in filter on top of our custom filtering, which silently dropped keyword matches from results.

Code Changes

  • Added navMatchRank, matchesNavQuery, and filterAndRankNavItems utilities to useNavSearchItems.ts with a 4-tier relevance ranking (title > parent > group > keyword)
  • Added keywords field to NavConfigRoute, NavConfigTab, and NavGroupChild interfaces
  • Applied keyword aliases to nav items: Vendors (TCF, GVL), Notices (consent, opt-in, opt-out), Experiences (banner, overlay, CMP), Integrations (connectors, connections), Locations/Regulations (GDPR, CCPA), Role Management (RBAC, permissions), Assessments (PIA, DPIA), plus previously added keywords for System inventory, Request manager, and Privacy requests settings
  • Added filterOption={false} to AutoComplete to prevent Ant's built-in filter from dropping keyword-matched results
  • Updated both NavSearchExpanded and NavSearchModal to use filterAndRankNavItems instead of simple title-only filtering
  • Added comprehensive unit tests for matching and ranking logic

Steps to Confirm

  1. Open the admin UI and use the nav search (sidebar search input or Cmd/Ctrl+K)
  2. Type keyword synonyms and verify results appear:
    • "TCF" → Vendors
    • "GDPR" → Locations, Regulations
    • "banner" → Experiences
    • "DSR" → Request manager
    • "connectors" → Integrations
    • "RBAC" → Role Management
  3. Verify direct title searches still work and appear ranked above keyword matches
  4. Verify the collapsed nav modal search (Cmd/Ctrl+K) also surfaces keyword results

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • CHANGELOG.md updated
    • Add a db-migration label
    • Add a high-risk label
    • Updates unreleased work already in Changelog, no new entry necessary
  • UX feedback:
    • All UX related changes have been reviewed by a designer
    • No UX review needed
  • Followup issues:
    • No followup issues
  • Database migrations:
    • No migrations
  • Documentation:
    • No documentation updates required

@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented Apr 17, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
fides-plus-nightly Ready Ready Preview, Comment May 5, 2026 3:26pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
fides-privacy-center Ignored Ignored May 5, 2026 3:26pm

Request Review

@jpople jpople marked this pull request as ready for review April 17, 2026 15:17
@jpople jpople requested a review from a team as a code owner April 17, 2026 15:17
@jpople jpople requested review from kruulik and removed request for a team April 17, 2026 15:17
Copy link
Copy Markdown
Contributor

@claude claude Bot left a comment

Choose a reason for hiding this comment

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

Nav Search Keyword Matching & Relevance Ranking

This is a clean, well-scoped improvement. The implementation is solid and the test coverage is thorough. A few notes:

Strengths

  • filterOption={false} (inline comment): Essential addition — without it, Ant Design's AutoComplete would double-filter results, silently discarding valid keyword/group matches. Easy to miss.
  • Relevance ranking design is sensible: title > parent > group > keyword, with stable ordering within each tier preserving nav-config source order. Using Number.POSITIVE_INFINITY as the "no match" sentinel keeps the sort arithmetic clean.
  • navMatchRank is self-contained — it normalizes the query internally (trim + lowercase), making it safe to call standalone without relying on callers to normalize first.
  • Test suite covers all the meaningful cases: empty query, case-insensitive matching per field, rank ordering, within-tier stability, and filtering. The makeItem helper keeps the tests readable.
  • filterAndRankNavItems uses a stable sort (JS Array.prototype.sort has been stable since ES2019 / V8 7.0), so the "preserves source order within rank" guarantee holds in all supported environments.

Minor observations

  • The if (!q) return 0 guard in navMatchRank is dead code when called via filterAndRankNavItems (callers gate on query ? first), but it's correct for standalone use and makes the function safe to call in isolation — no action needed.
  • "Locations" and "Regulations" both carry ["GDPR", "CCPA"] keywords (see inline comment) — presumably intentional so both surface on those searches.
  • The keyword list in nav-config.tsx will naturally grow over time. If it becomes unwieldy, a co-located keywords comment block or a separate nav-keywords.ts constant map could help, but that's a future concern.

No blocking issues. LGTM.

🔬 Codegraph: connected (46831 nodes)


💡 Write /code-review in a comment to re-run this review.

Comment thread clients/admin-ui/src/features/common/nav/NavSearch.tsx
Comment thread clients/admin-ui/src/features/common/nav/useNavSearchItems.ts
Comment thread clients/admin-ui/src/features/common/nav/nav-config.tsx
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 17, 2026

Title Lines Statements Branches Functions
admin-ui Coverage: 8%
6.63% (3003/45268) 5.9% (1535/26007) 4.69% (627/13351)
fides-js Coverage: 78%
79.43% (2013/2534) 66.13% (1244/1881) 73.09% (345/472)
privacy-center Coverage: 88%
85.97% (331/385) 81.36% (179/220) 78.87% (56/71)

@rayharnett
Copy link
Copy Markdown
Contributor

Code Review: ENG-3278 — Improve nav search with keyword matching and relevance ranking

PR: #7952
Author: @jpople
Base: mainjpople/eng-3278/nav-search-improvement


🚨 Critical Issues (Must Fix)

None. No blocking issues found. The implementation is clean, well-scoped, and correct.


⚠️ Warnings (Should Fix)

1. Keyword list will grow unboundedly — consider a future migration path

Where: nav-config.tsx — ~14 keyword arrays sprinkled across routes/tabs

As the keyword list grows, having them scattered inline with route config becomes harder to maintain. The reviewer's suggestion of a co-located comment block or a separate nav-keywords.ts constant map is worth flagging now.

Recommendation: Not a blocker. Add a TODO comment on the first keyword entry noting this is a future concern:

// TODO: If keywords grow beyond ~20 entries, consider moving to a
// centralized nav-keywords.ts map for easier scanning and dedup.
keywords: ["data map", "data inventory", "assets"],

💡 Suggestions (Nits / Improvements)

1. Add JSDoc to the keywords field on FlatNavItem

Where: useNavSearchItems.ts line ~44

The keywords field on NavConfigRoute, NavConfigTab, and NavGroupChild already has JSDoc ("Optional hand-curated aliases/synonyms used by nav search."), but the keywords on FlatNavItem only has a shorter comment. Since FlatNavItem is the runtime shape consumed by the search logic, a slightly more descriptive JSDoc would help future maintainers:

/**
 * Hand-curated aliases/synonyms for search matching.
 * Populated from nav-config.tsx route definitions at runtime.
 */
keywords?: string[];

2. Consider making navMatchRank return a typed enum instead of magic numbers

Where: useNavSearchItems.tsnavMatchRank function

The rank values 0, 1, 2, 3, Infinity are well-documented in the JSDoc, but using a const enum would make the code self-documenting and prevent accidental typos:

export const NavMatchTier = {
  TITLE: 0,
  PARENT: 1,
  GROUP: 2,
  KEYWORD: 3,
  NONE: Number.POSITIVE_INFINITY,
} as const;

This is a nice-to-have, not a must-have. The current JSDoc approach works fine for this scale. Flagging because if more tiers are added later, the enum prevents silent bugs.

3. Test: consider adding a keyword-only match test

Where: useNavSearchItems.test.ts

The test "matches against hand-curated keywords" only tests a direct keyword hit. It would be valuable to also verify that an item with only a keyword match (no title/group/parent match) still surfaces correctly when the query matches the keyword:

it("surfaces items matched only by keyword", () => {
  const item = makeItem({
    title: "Vendors",
    groupTitle: "Consent Management",
    keywords: ["TCF", "GVL"],
  });
  // "TCF" does NOT appear in title or group — only in keywords
  expect(matchesNavQuery(item, "tcf")).toBe(true);
  expect(navMatchRank(item, "tcf")).toBe(3); // keyword tier
});

This is already partially covered by filterAndRankNavItems tests but worth calling out explicitly.

4. Minor: filterOption={false} deserves a one-line comment

Where: NavSearch.tsx line ~134

The inline comment "prevents Ant's built-in filter from dropping keyword-matched results" is good but it's on the same line as the prop. A dedicated comment above would be more visible:

// Ant Design's default filter only checks title substrings, which would
// silently drop keyword/group matches. Disable it since we filter manually.
filterOption={false}

✅ Good Practices

  • filterOption={false} is an essential fix that's easy to miss — great catch.
  • Relevance ranking design (title > parent > group > keyword) is intuitive and the tier system is clean.
  • navMatchRank is self-contained — normalizes the query internally, making it safe to call standalone.
  • Test suite is comprehensive — covers empty query, case-insensitive matching, rank ordering, within-tier stability, and filtering. The makeItem helper keeps tests readable.
  • filterAndRankNavItems uses a stable sortArray.prototype.sort has been stable since ES2019 / V8 7.0.
  • Type definitions are consistentkeywords?: string[] is added to all three interfaces (NavConfigRoute, NavConfigTab, NavGroupChild) and threaded through configureNavRoute.
  • Test mock fix (...jest.requireActual) in the collapsed/modal test files is the correct approach to ensure the real search logic runs in integration tests.
  • Changelog entry follows the correct format and is properly placed.

Summary

Verdict: LGTM — No blocking issues.

This is a clean, well-scoped improvement. The implementation is solid, the test coverage is thorough, and the relevance ranking design is intuitive. The suggestions above are minor enhancements worth considering but none are blockers.

Copy link
Copy Markdown
Contributor

@rayharnett rayharnett left a comment

Choose a reason for hiding this comment

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

LGTM

@rayharnett rayharnett added this pull request to the merge queue May 6, 2026
Merged via the queue into main with commit 00c6992 May 6, 2026
51 checks passed
@rayharnett rayharnett deleted the jpople/eng-3278/nav-search-improvement branch May 6, 2026 15:28
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.

2 participants