Based/new searchbar definition#876
Open
Khalzz wants to merge 7 commits into
Open
Conversation
|
Warning Review limit reached
Next review available in: 38 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR replaces the navbar search bar with a spotlight-style overlay combining Pagefind-indexed static navigation results and streamed MIOT harness AI answers. It adds a harness search API route, a build-time search-index generator, Auth0 access_token propagation through NextAuth, and a shared Markdown rendering utility. ChangesSpotlight Search Feature
Estimated code review effort: 4 (Complex) | ~60 minutes Auth0 access_token propagation
Shared Markdown rendering utility
Sequence Diagram(s)sequenceDiagram
participant User
participant SpotlightSearch
participant usePagefindSearch
participant useHarnessSearch
participant HarnessSearchRoute as "harness/search API"
User->>SpotlightSearch: open overlay (Cmd/Ctrl+K)
User->>SpotlightSearch: type query
SpotlightSearch->>usePagefindSearch: query change
usePagefindSearch-->>SpotlightSearch: static navigation results
User->>SpotlightSearch: select "Ask Harness" prompt
SpotlightSearch->>useHarnessSearch: committedQuery
useHarnessSearch->>HarnessSearchRoute: POST { query }
HarnessSearchRoute-->>useHarnessSearch: { results }
useHarnessSearch-->>SpotlightSearch: harness items (blocks)
SpotlightSearch-->>User: render markdown/goto answer rows
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
turbo-repo/packages/miot-harness-client/src/types.ts (1)
11-18: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winRemove the duplicate
skill_idpropertyturbo-repo/packages/miot-harness-client/src/types.ts:11-18—UserRequestdeclaresskill_idtwice in the same interface body, which breaks TypeScript compilation. Keep the documented field and remove the earlier duplicate.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@turbo-repo/packages/miot-harness-client/src/types.ts` around lines 11 - 18, Remove the duplicate skill_id declaration in UserRequest so the interface only defines it once. In types.ts, keep the documented skill_id field with the SKILL.md guidance comment and delete the earlier duplicate property so the TypeScript type compiles cleanly.
🧹 Nitpick comments (10)
turbo-repo/apps/app/src/features/common/utils/markdown-components.tsx (1)
33-38: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNo distinct styling for fenced code blocks vs inline code.
codeapplies the same inline-style background/padding regardless of whether it's inline code or a fenced block (nopreoverride), so multi-line code blocks won't get a bounded/scrollable container.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@turbo-repo/apps/app/src/features/common/utils/markdown-components.tsx` around lines 33 - 38, The markdown renderer’s `code` component in `markdown-components.tsx` treats all code the same, so fenced blocks render with inline styling instead of a proper block container. Update the markdown component mapping to distinguish inline `code` from fenced code blocks, likely by adding a `pre` override and/or checking the rendered element type from the markdown library. Keep `code` for inline snippets only, and wrap multi-line blocks in a scrollable/bounded container with block-friendly styles.turbo-repo/apps/app/src/app/api/harness/search/route.ts (1)
53-55: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider capping query length before forwarding to the harness.
The raw
queryis sent to the harnessmessagefield with no upper bound, allowing arbitrarily long input to reach the LLM run and drive up cost/latency for a search box use case.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@turbo-repo/apps/app/src/app/api/harness/search/route.ts` around lines 53 - 55, The search route forwards the raw query from route.ts straight into the harness message with no length limit, so cap the input before building the request. Update the query handling in the search route to trim and then truncate or reject overly long values before they reach the harness call, keeping the existing empty-query early return intact. Use the existing route logic around the query variable and the harness request construction to apply the limit consistently.turbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/spotlight-search.tsx (2)
169-176: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueHarness go-to item ids can collide if the API ever returns multiple results.
iresets to 0 for eachresultin theflatMap, soharness-url-0etc. would collide across multiple results. Currently benign sinceroute.tsalways returns a single-elementresultsarray, but worth making the id derivation result-scoped for robustness if that contract changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@turbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/spotlight-search.tsx` around lines 169 - 176, The Harness go-to item IDs in spotlight-search are derived from an index that resets for each result inside harnessGoToItems, so multiple API results could generate duplicate ids. Update the id derivation in spotlight-search.tsx to be result-scoped in the flatMap path, using a stable unique prefix from each harness result together with the block index, and keep the urlBlockToItem mapping unique even if harnessResults contains more than one element.
257-262: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNo-op
onKeyDownstub on a non-interactive<div>used only to gateonClick.
onKeyDown={() => {}}doesn't provide real keyboard equivalence for theonClick={stopPropagation}handler — it just satisfies a11y lint without functional keyboard support. Since this handler only guards against backdrop-close-on-click, consider movingstopPropagationto the parent<dialog>element (already a semantically valid container) instead of adding click semantics to a<div>.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@turbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/spotlight-search.tsx` around lines 257 - 262, The non-interactive div in spotlight-search is using a no-op onKeyDown just to satisfy accessibility lint, but it does not provide real keyboard behavior for the click stopPropagation. Update the spotlight-search component so the stopPropagation logic is handled on the parent dialog element instead of on this div, and remove the fake onKeyDown/onClick semantics from the div while preserving the backdrop-close behavior.Source: Path instructions
turbo-repo/apps/app/next.config.mjs (1)
11-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive
NEXT_PUBLIC_BASE_PATHfrombasePathinstead of duplicating the literal.Two independent
"/app"literals can drift ifbasePathis ever updated.♻️ Proposed fix
- basePath: "/app", - env: { - // Expose basePath so client code can build asset URLs (e.g. /app/pagefind/pagefind.js) - NEXT_PUBLIC_BASE_PATH: "/app", - }, + basePath: "/app", + env: { + // Expose basePath so client code can build asset URLs (e.g. /app/pagefind/pagefind.js) + NEXT_PUBLIC_BASE_PATH: "/app", // keep in sync with `basePath` above + },Consider referencing a single shared constant if
next.config.mjssupports importing one before both usages.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@turbo-repo/apps/app/next.config.mjs` around lines 11 - 15, The next.config.mjs settings duplicate the "/app" value in both basePath and NEXT_PUBLIC_BASE_PATH, which can drift if one changes. Update the config so NEXT_PUBLIC_BASE_PATH is derived from the same shared value used by basePath, ideally via a single constant or import in next.config.mjs, and keep both the config key and env assignment in sync.turbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/types.ts (1)
1-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFile naming and type source deviate from repo convention.
This file is named
types.tsrather than the*.types.tsconvention, and re-exportsHarnessBlockfrom a server API route module instead of a colocated types file. Consider renaming tospotlight.types.tsand movingHarnessBlockinto a shared type file (e.g. undersrc/types/or this same file) that both the route and the client import from.As per coding guidelines, "Colocate types as
*.types.tsfiles; shared types insrc/types/".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@turbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/types.ts` around lines 1 - 24, The Spotlight search types are defined in a nonconforming file name and the shared HarnessBlock type is imported from a server route module. Rename this module to spotlight.types.ts and move HarnessBlock into a colocated shared type source (either this file or src/types/) that both the search route and SpotlightItem-related client code import from; keep SpotlightResultKind and SpotlightItem in the shared types module and update all imports to use the new location.Source: Coding guidelines
turbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/use-pagefind-search.ts (1)
8-15: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate Pagefind type shapes vs.
pagefind.d.ts.
PagefindInstance/PagefindResultDatahere duplicate the shapes already declared insrc/types/pagefind.d.ts. BecausePAGEFIND_PATHis typed as a genericstring(to bypass static module resolution), the ambient declaration never actually applies to this import, so both files must be kept manually in sync. Consider exporting these interfaces frompagefind.d.ts(or a shared*.types.tsfile) and importing them here, or dropping the now-dead ambient declaration.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@turbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/use-pagefind-search.ts` around lines 8 - 15, The Pagefind shape definitions are duplicated between usePagefindSearch and the ambient pagefind.d.ts typing, so keep only one source of truth. Move PagefindInstance and PagefindResultData into a shared exported type location (preferably pagefind.d.ts or a common types file) and update usePagefindSearch to import those symbols instead of redefining them, or remove the unused ambient declaration if it is no longer needed.turbo-repo/apps/app/src/types/pagefind.d.ts (1)
1-17: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLikely unused given the actual import path.
This ambient declaration only applies to the literal specifier
"/pagefind/pagefind.js".use-pagefind-search.tsimports${NEXT_PUBLIC_BASE_PATH}/pagefind/pagefind.jstyped as a plainstringspecifically to bypass static resolution, so this type is never actually applied there, and a separate local interface is duplicated instead. See the paired comment inuse-pagefind-search.tsfor a consolidation suggestion.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@turbo-repo/apps/app/src/types/pagefind.d.ts` around lines 1 - 17, The ambient module declaration for pagefind is only matching a literal specifier and is not being used by the dynamic import in use-pagefind-search.ts, which leads to duplicated local types. Update the pagefind typing setup so the import in use-pagefind-search.ts can reuse the shared PagefindResultData/PagefindResult declarations from pagefind.d.ts, and remove the duplicated local interface there while keeping the dynamic import pattern intact.turbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/spotlight-input.tsx (1)
31-34: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
useLayoutEffect+setTimeoutgains nothing overuseEffect.Since the focus call is deferred via
setTimeout, it no longer runs synchronously before paint, souseLayoutEffectprovides no benefit overuseEffecthere and only adds an SSR-mismatch warning risk in dev.♻️ Suggested change
- useLayoutEffect(() => { + useEffect(() => { const id = setTimeout(() => inputRef.current?.focus(), 60); return () => clearTimeout(id); }, []);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@turbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/spotlight-input.tsx` around lines 31 - 34, The delayed focus logic in spotlight-input’s effect doesn’t need a layout effect because setTimeout already defers it past paint. Replace useLayoutEffect with useEffect in the SpotlightInput component, keeping the same inputRef focus and cleanup behavior, so the effect stays non-blocking and avoids SSR mismatch warnings.turbo-repo/apps/app/src/features/layout/components/secured-navbar/searchbar/search-bar.tsx (1)
1-3: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider deleting the file instead of stubbing it out.
Now that
SearchBaralways returnsnulland is unused (per secured-navbar.tsx), removing the file (and its now-dead siblingkbd-hint.tsx/navegation_params.tsif unused) is cleaner than keeping a no-op stub around.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@turbo-repo/apps/app/src/features/layout/components/secured-navbar/searchbar/search-bar.tsx` around lines 1 - 3, Remove the no-op SearchBar stub instead of keeping a component that always returns null, and update any imports/usage from secured-navbar.tsx or related navbar code to stop referencing SearchBar. Also check the sibling kbd-hint.tsx and navegation_params.ts files for dead references and delete them too if they are no longer used, so the secured-navbar feature only contains active code.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@turbo-repo/apps/app/scripts/build-search-index.mjs`:
- Around line 48-58: The Pagefind records in build-search-index.mjs are tagged
with the wrong locale, causing a mismatch with the app’s Spanish UI set in the
layout. Update the indexing logic around index.addCustomRecord so the record
language matches the app locale (use es instead of en), or otherwise align the
generated index with a shared forceLanguage setting. Keep the change consistent
with the existing page.href, page.label, and meta fields so search results come
from the same locale as the app.
In `@turbo-repo/apps/app/src/app/api/dev/auth0/route.ts`:
- Around line 16-23: The debug response in the auth0 route is exposing raw
credentials by default; update the handler in the dev auth route to return only
non-sensitive flags/metadata unless an explicit local-only debug opt-in is
enabled. In the route’s response object, keep fields like email, hasAccessToken,
hasRawJWT, and hasTicket, but gate accessToken and rawJWT behind a stricter
check than NODE_ENV === "development" (for example, a dedicated opt-in flag or
local-only guard). Make sure the logic is contained in the auth0 route handler
so the default behavior never returns bearer tokens in shared dev deployments.
In `@turbo-repo/apps/app/src/app/api/harness/search/route.ts`:
- Around line 30-36: parseAnswerBlocks currently trusts JSON.parse output and
casts it to HarnessBlock[] without validating structure or URL safety, so add a
lightweight manual guard in parseAnswerBlocks to verify each parsed item has a
known HarnessBlock type and that any value.url only uses http:// or https://
before returning it. Keep malformed or unsafe entries from reaching
use-harness-search.ts and spotlight-search.tsx, where urlBlockToItem and
globalThis.open would otherwise consume the data unchecked.
In `@turbo-repo/apps/app/src/auth.config.ts`:
- Line 252: The token refresh logic in auth.config.ts leaves a stale bearer
token on the token object when the refresh response has no access_token, while
the expiry metadata is still updated. In the refresh handling block around the
token assignment, update the token state so that accessToken is cleared or
otherwise invalidated whenever tokens.access_token is missing, and keep the
existing tokens.access_token assignment path intact for the success case.
- Line 318: The session callback is copying the bearer token into session.user,
which makes it client-visible; remove the accessToken assignment from the
session object in auth.config.ts and keep the token only in the JWT. Update the
relevant session/JWT handling in the auth callbacks so any server-side consumers
read accessToken from the token path instead of session.user.
- Line 235: The refresh check in auth.config.ts is based only on time-to-expiry,
so it can miss tokens that are still far from expiry but already too old for the
harness. Update the refresh logic around shouldRefresh to also consider token
age from the issuance time (or equivalent timestamp available in the
token/session data), and refresh when either the token is near expiry or older
than the harness-safe age threshold. Use the existing auth refresh flow in this
block to keep the behavior consistent.
In `@turbo-repo/apps/app/src/features/common/utils/markdown-components.tsx`:
- Around line 15-20: The list renderers in markdown-components.tsx are
hardcoding text-sm on ul and ol, which overrides inherited sizing from callers
like DescriptionElement. Remove the fixed text size from the ul/ol components
and let them inherit font size so compact/scalable wrappers can control list
typography consistently. Keep the existing layout and spacing classes, and use
the ul/ol render functions as the place to adjust any list-specific styling
only.
- Around line 5-42: Type MARKDOWN_COMPONENTS with react-markdown’s exported
Components type instead of leaving it untyped, and remove any as never
suppression so prop mismatches are checked. Update each renderer (p, strong, em,
ul, ol, li, h1, h2, h3, hr, code, a) to use Readonly<ComponentProps<...> &
ExtraProps> or the matching react-markdown component prop shape so the component
signatures stay compatible and type-safe.
In
`@turbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/spotlight-backdrop.tsx`:
- Around line 20-29: The backdrop in SpotlightBackdrop is a non-interactive div
being used as a dismiss target via onClick, so update it to be
keyboard-accessible. Either switch the overlay in createPortal to a button-based
control, or add the required accessibility affordances on the backdrop element:
a proper role, tabIndex, and onKeyDown handling for Enter/Space to trigger
onClose. Keep the portal structure and children behavior unchanged.
In
`@turbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/spotlight-footer.tsx`:
- Around line 20-22: SpotlightFooter’s props are typed inline instead of using
the required read-only wrapper, so update the SpotlightFooter component
signature to wrap its props in Readonly just like the Key and Hint components in
this file. Keep the memo(function SpotlightFooter...) structure, but change the
parameter type so hasResults is declared through a Readonly<Props> style shape
to match the component prop convention.
In
`@turbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/spotlight-results.tsx`:
- Around line 173-182: The harness empty-state message in SpotlightResults is
hardcoded Spanish text and bypasses i18n. Update the `SpotlightResults`
component to render a passed-in `harnessEmptyLabel` prop instead of the literal
string, following the same pattern used for `recentLabel` and `navigateHeading`.
Then wire `harnessEmptyLabel` through the orchestrator from the i18n service so
the message comes from the locale files rather than being embedded in the
component.
- Around line 105-122: The Spotlight search result action in
spotlight-results.tsx opens an AI-provided URL directly, so update the
SpotlightRow item’s onSelect path to validate/sanitize block.value.url before
opening it. Use the existing harness-url mapping in the urlBlocks.map callback
to reject unsafe schemes like javascript: and only allow trusted http/https
destinations, then open the URL with window.open using noopener,noreferrer to
prevent reverse-tabnabbing and window.opener access.
In
`@turbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/spotlight-search.tsx`:
- Around line 61-69: The `urlBlockToItem` handler opens harness-supplied URLs
with `globalThis.open(..., "_blank")` without `noopener,noreferrer` or scheme
validation. Update the `onSelect` logic in `urlBlockToItem` to only allow safe
URL schemes and open the target with explicit `noopener,noreferrer` behavior so
the new tab cannot access `window.opener`.
In
`@turbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/use-pagefind-search.ts`:
- Line 86: The cancellation guard in usePagefindSearch is shared across effect
reruns, so a newer search can reset cancelRef.current before an older
pagefind.search promise resolves. Replace the hook-level cancelRef with a
per-invocation local cancellation flag inside the effect, and have the cleanup
for that effect set that local flag so each search run in usePagefindSearch has
its own independent guard before calling setResults.
- Around line 25-37: The loadPagefind() helper caches a failed dynamic import
forever because loadPromise is never cleared when the import rejects, so
subsequent calls cannot retry. Update loadPagefind() so that a failed import
resets loadPromise (and leaves pagefindSingleton unset) before returning null,
while still caching only successful imports via pagefindSingleton. This change
should be made in the loadPagefind function used by the spotlight-search
Pagefind loader.
In `@turbo-repo/apps/app/src/features/layout/models/pages-searchable.json`:
- Around line 1-83: The search index is a duplicated, hand-edited copy of the
sidebar page model, so it can drift from the real access rules and hierarchy.
Update the generation flow that builds pages-searchable.json from the same
source used by pages.ts, instead of maintaining a separate static list. Make
sure the generated output preserves each page’s label, href, requiredGroups,
blockedGroups, and nested items recursively so the search index always matches
the sidebar.
---
Outside diff comments:
In `@turbo-repo/packages/miot-harness-client/src/types.ts`:
- Around line 11-18: Remove the duplicate skill_id declaration in UserRequest so
the interface only defines it once. In types.ts, keep the documented skill_id
field with the SKILL.md guidance comment and delete the earlier duplicate
property so the TypeScript type compiles cleanly.
---
Nitpick comments:
In `@turbo-repo/apps/app/next.config.mjs`:
- Around line 11-15: The next.config.mjs settings duplicate the "/app" value in
both basePath and NEXT_PUBLIC_BASE_PATH, which can drift if one changes. Update
the config so NEXT_PUBLIC_BASE_PATH is derived from the same shared value used
by basePath, ideally via a single constant or import in next.config.mjs, and
keep both the config key and env assignment in sync.
In `@turbo-repo/apps/app/src/app/api/harness/search/route.ts`:
- Around line 53-55: The search route forwards the raw query from route.ts
straight into the harness message with no length limit, so cap the input before
building the request. Update the query handling in the search route to trim and
then truncate or reject overly long values before they reach the harness call,
keeping the existing empty-query early return intact. Use the existing route
logic around the query variable and the harness request construction to apply
the limit consistently.
In `@turbo-repo/apps/app/src/features/common/utils/markdown-components.tsx`:
- Around line 33-38: The markdown renderer’s `code` component in
`markdown-components.tsx` treats all code the same, so fenced blocks render with
inline styling instead of a proper block container. Update the markdown
component mapping to distinguish inline `code` from fenced code blocks, likely
by adding a `pre` override and/or checking the rendered element type from the
markdown library. Keep `code` for inline snippets only, and wrap multi-line
blocks in a scrollable/bounded container with block-friendly styles.
In
`@turbo-repo/apps/app/src/features/layout/components/secured-navbar/searchbar/search-bar.tsx`:
- Around line 1-3: Remove the no-op SearchBar stub instead of keeping a
component that always returns null, and update any imports/usage from
secured-navbar.tsx or related navbar code to stop referencing SearchBar. Also
check the sibling kbd-hint.tsx and navegation_params.ts files for dead
references and delete them too if they are no longer used, so the secured-navbar
feature only contains active code.
In
`@turbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/spotlight-input.tsx`:
- Around line 31-34: The delayed focus logic in spotlight-input’s effect doesn’t
need a layout effect because setTimeout already defers it past paint. Replace
useLayoutEffect with useEffect in the SpotlightInput component, keeping the same
inputRef focus and cleanup behavior, so the effect stays non-blocking and avoids
SSR mismatch warnings.
In
`@turbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/spotlight-search.tsx`:
- Around line 169-176: The Harness go-to item IDs in spotlight-search are
derived from an index that resets for each result inside harnessGoToItems, so
multiple API results could generate duplicate ids. Update the id derivation in
spotlight-search.tsx to be result-scoped in the flatMap path, using a stable
unique prefix from each harness result together with the block index, and keep
the urlBlockToItem mapping unique even if harnessResults contains more than one
element.
- Around line 257-262: The non-interactive div in spotlight-search is using a
no-op onKeyDown just to satisfy accessibility lint, but it does not provide real
keyboard behavior for the click stopPropagation. Update the spotlight-search
component so the stopPropagation logic is handled on the parent dialog element
instead of on this div, and remove the fake onKeyDown/onClick semantics from the
div while preserving the backdrop-close behavior.
In
`@turbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/types.ts`:
- Around line 1-24: The Spotlight search types are defined in a nonconforming
file name and the shared HarnessBlock type is imported from a server route
module. Rename this module to spotlight.types.ts and move HarnessBlock into a
colocated shared type source (either this file or src/types/) that both the
search route and SpotlightItem-related client code import from; keep
SpotlightResultKind and SpotlightItem in the shared types module and update all
imports to use the new location.
In
`@turbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/use-pagefind-search.ts`:
- Around line 8-15: The Pagefind shape definitions are duplicated between
usePagefindSearch and the ambient pagefind.d.ts typing, so keep only one source
of truth. Move PagefindInstance and PagefindResultData into a shared exported
type location (preferably pagefind.d.ts or a common types file) and update
usePagefindSearch to import those symbols instead of redefining them, or remove
the unused ambient declaration if it is no longer needed.
In `@turbo-repo/apps/app/src/types/pagefind.d.ts`:
- Around line 1-17: The ambient module declaration for pagefind is only matching
a literal specifier and is not being used by the dynamic import in
use-pagefind-search.ts, which leads to duplicated local types. Update the
pagefind typing setup so the import in use-pagefind-search.ts can reuse the
shared PagefindResultData/PagefindResult declarations from pagefind.d.ts, and
remove the duplicated local interface there while keeping the dynamic import
pattern intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 55fb78a6-7399-4b52-ae10-51459a54c8ff
⛔ Files ignored due to path filters (1)
turbo-repo/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (34)
turbo-repo/apps/app/.env.exampleturbo-repo/apps/app/.gitignoreturbo-repo/apps/app/next.config.mjsturbo-repo/apps/app/package.jsonturbo-repo/apps/app/scripts/build-search-index.mjsturbo-repo/apps/app/src/app/api/dev/auth0/route.tsturbo-repo/apps/app/src/app/api/harness/search/route.tsturbo-repo/apps/app/src/app/globals.cssturbo-repo/apps/app/src/auth.config.tsturbo-repo/apps/app/src/features/common/components/kpi-stat/kpi-stat.tsxturbo-repo/apps/app/src/features/common/utils/markdown-components.tsxturbo-repo/apps/app/src/features/layout/components/secured-layout.tsxturbo-repo/apps/app/src/features/layout/components/secured-navbar/searchbar/kbd-hint.tsxturbo-repo/apps/app/src/features/layout/components/secured-navbar/searchbar/navegation_params.tsturbo-repo/apps/app/src/features/layout/components/secured-navbar/searchbar/search-bar.tsxturbo-repo/apps/app/src/features/layout/components/secured-navbar/secured-navbar.tsxturbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/navigate-actions.tsturbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/spotlight-backdrop.tsxturbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/spotlight-empty-state.tsxturbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/spotlight-footer.tsxturbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/spotlight-input.tsxturbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/spotlight-result-item.tsxturbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/spotlight-results.tsxturbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/spotlight-row.tsxturbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/spotlight-search.tsxturbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/types.tsturbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/use-harness-search.tsturbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/use-pagefind-search.tsturbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/use-spotlight-state.tsturbo-repo/apps/app/src/features/layout/models/pages-searchable.jsonturbo-repo/apps/app/src/lang/en.jsonturbo-repo/apps/app/src/lang/es.jsonturbo-repo/apps/app/src/types/pagefind.d.tsturbo-repo/packages/miot-harness-client/src/types.ts
💤 Files with no reviewable changes (1)
- turbo-repo/apps/app/src/features/layout/components/secured-navbar/searchbar/navegation_params.ts
| if (token.refreshToken && !account && !token.ticket) { | ||
| const expiresAt = Number(token.accessTokenExpiresAt ?? 0) * 1000; | ||
| const shouldRefresh = expiresAt - Date.now() < 5 * 60 * 1000; // 5 min before expiry | ||
| const shouldRefresh = expiresAt - Date.now() < 3 * 60 * 60 * 1000; // 3 h — harness rejects tokens older than ~4 h |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Refresh based on token age, not only time-to-expiry.
Line 235 says the harness rejects tokens older than ~4h, but this condition only refreshes near expiry. Any token lifetime over ~7h can become too old for harness while still being more than 3h from expiry.
🐛 Proposed fix
token.rawJWT = account.id_token;
token.accessToken = account.access_token;
token.accessTokenExpiresAt = account.expires_at;
+ token.accessTokenIssuedAt = Math.floor(Date.now() / 1000);
token.refreshToken = account.refresh_token; const expiresAt = Number(token.accessTokenExpiresAt ?? 0) * 1000;
- const shouldRefresh = expiresAt - Date.now() < 3 * 60 * 60 * 1000; // 3 h — harness rejects tokens older than ~4 h
+ const now = Date.now();
+ const issuedAt = Number(token.accessTokenIssuedAt ?? 0) * 1000;
+ const isNearExpiry = expiresAt - now < 5 * 60 * 1000;
+ const isTooOldForHarness =
+ issuedAt === 0 || now - issuedAt > 3 * 60 * 60 * 1000;
+ const shouldRefresh = isNearExpiry || isTooOldForHarness; if (tokens.access_token) token.accessToken = tokens.access_token;
token.accessTokenExpiresAt = Math.floor(Date.now() / 1000) + tokens.expires_in;
+ token.accessTokenIssuedAt = Math.floor(Date.now() / 1000);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const shouldRefresh = expiresAt - Date.now() < 3 * 60 * 60 * 1000; // 3 h — harness rejects tokens older than ~4 h | |
| token.accessTokenIssuedAt = Math.floor(Date.now() / 1000); | |
| const expiresAt = Number(token.accessTokenExpiresAt ?? 0) * 1000; | |
| const now = Date.now(); | |
| const issuedAt = Number(token.accessTokenIssuedAt ?? 0) * 1000; | |
| const isNearExpiry = expiresAt - now < 5 * 60 * 1000; | |
| const isTooOldForHarness = | |
| issuedAt === 0 || now - issuedAt > 3 * 60 * 60 * 1000; | |
| const shouldRefresh = isNearExpiry || isTooOldForHarness; | |
| token.accessTokenIssuedAt = Math.floor(Date.now() / 1000); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@turbo-repo/apps/app/src/auth.config.ts` at line 235, The refresh check in
auth.config.ts is based only on time-to-expiry, so it can miss tokens that are
still far from expiry but already too old for the harness. Update the refresh
logic around shouldRefresh to also consider token age from the issuance time (or
equivalent timestamp available in the token/session data), and refresh when
either the token is near expiry or older than the harness-safe age threshold.
Use the existing auth refresh flow in this block to keep the behavior
consistent.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
turbo-repo/apps/app/src/features/layout/models/pages.ts (1)
29-34: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winPrefer
satisfiesoverasto keep compile-time shape checking.The trailing
as SidebarItem[]silences structural mismatches between the JSON-derived shape andSidebarItem(e.g., atargetstring that isn't a validHTMLAttributeAnchorTarget, or a missing required field) that would otherwise be caught by the compiler. As per coding guidelines: "Avoidanyandunknowntypes; prefer type inference" and "Don't cast types when the type is already correct - avoid unnecessary type assertions."♻️ Proposed refactor
-export const pages: SidebarItem[] = pagesConfig.map((p) => ({ +export const pages = pagesConfig.map((p) => ({ ...p, icon: PAGE_ICONS[p.label], totals: {}, items: (p.items ?? []).map((c) => ({ ...c, totals: {} })), -})) as SidebarItem[]; +})) satisfies SidebarItem[];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@turbo-repo/apps/app/src/features/layout/models/pages.ts` around lines 29 - 34, The `pages` mapping currently ends with a broad `as SidebarItem[]` cast, which bypasses structural checks on the JSON-derived objects. Update the `pages` export in `pages.ts` to use `satisfies SidebarItem[]` on the mapped result (or an equivalent compile-time shape check) so `PAGE_ICONS`, `totals`, and each item in the `map` path are validated without suppressing type errors. Keep the `pagesConfig.map` and `items` transformation intact, but remove the unsafe assertion so mismatches in `SidebarItem` are caught by the compiler.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@turbo-repo/apps/app/src/features/layout/models/pages.ts`:
- Around line 29-34: The `pages` mapping currently ends with a broad `as
SidebarItem[]` cast, which bypasses structural checks on the JSON-derived
objects. Update the `pages` export in `pages.ts` to use `satisfies
SidebarItem[]` on the mapped result (or an equivalent compile-time shape check)
so `PAGE_ICONS`, `totals`, and each item in the `map` path are validated without
suppressing type errors. Keep the `pagesConfig.map` and `items` transformation
intact, but remove the unsafe assertion so mismatches in `SidebarItem` are
caught by the compiler.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3bbaaf47-04d2-4043-a588-6328734b7ce6
📒 Files selected for processing (9)
turbo-repo/apps/app/scripts/build-search-index.mjsturbo-repo/apps/app/src/app/api/dev/auth0/route.tsturbo-repo/apps/app/src/app/api/harness/search/route.tsturbo-repo/apps/app/src/auth.config.tsturbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/spotlight-results.tsxturbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/spotlight-search.tsxturbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/use-pagefind-search.tsturbo-repo/apps/app/src/features/layout/models/pages-config.jsonturbo-repo/apps/app/src/features/layout/models/pages.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- turbo-repo/apps/app/src/app/api/harness/search/route.ts
- turbo-repo/apps/app/scripts/build-search-index.mjs
- turbo-repo/apps/app/src/app/api/dev/auth0/route.ts
- turbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/use-pagefind-search.ts
- turbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/spotlight-results.tsx
- turbo-repo/apps/app/src/features/layout/components/secured-navbar/spotlight-search/spotlight-search.tsx
|
Contributor
App preview imageThe latest app preview image for this PR is ready.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added new searchbar, connected to the harness allowing:
"Ask": Harness consults with responses on url and markdown format comming as ai responses.
"Navigate": As movement capabilities once refeering a keyword found on the pagefind
"Build": WIP
Summary by CodeRabbit
New Features
Bug Fixes