feat(search): use quickstarts backend fuzzy search in help panel and #268
Open
aferd wants to merge 9 commits intoRedHatInsights:masterfrom
Open
feat(search): use quickstarts backend fuzzy search in help panel and #268aferd wants to merge 9 commits intoRedHatInsights:masterfrom
aferd wants to merge 9 commits intoRedHatInsights:masterfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds backend fuzzy-search integration: new docs and tests; Changes
Sequence DiagramsequenceDiagram
actor User
participant Client as SearchPanel/Filters
participant API as fetchQuickstarts
participant Backend as Quickstarts API
participant Processor as Result Processor
User->>Client: Enter search query
Client->>Client: Trim input -> set fuzzy flag
Client->>API: fetchQuickstarts(display-name=X, fuzzy=true)
API->>Backend: GET /quickstarts?display-name=X&fuzzy=true
Backend-->>API: Return ordered quickstarts
API-->>Client: Return quickstarts results
Client->>Processor: Map quickstarts to results (preserve order -> relevanceScore)
Client->>Processor: Build services & API lists
Processor->>Processor: Filter services/API via matchesQuery (client-side)
Processor-->>Client: Combine results (quickstarts + services + api)
Client->>User: Render ranked search results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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: 1
🧹 Nitpick comments (2)
src/utils/fetchQuickstarts.test.ts (1)
7-10: Consider strengthening the mock type.The
mockGetUserfunction returns a minimal object that doesn't fully match theChromeAPI['auth']['getUser']return type. While theas nevercast at call sites works, a more complete mock would catch type mismatches earlier.♻️ Optional: More complete mock
-const mockGetUser = () => - Promise.resolve({ - identity: { internal: { account_id: '123' } }, - }); +const mockGetUser = jest.fn().mockResolvedValue({ + identity: { + internal: { account_id: '123' }, + account_number: '12345', + type: 'User', + user: { username: 'test-user' }, + }, +});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/fetchQuickstarts.test.ts` around lines 7 - 10, The mockGetUser helper returns a minimal object that doesn't match ChromeAPI['auth']['getUser']'s full return shape; update mockGetUser to return a more complete shaped object (including identity, internal.account_id and any other expected fields like external/subject info or metadata present in ChromeAPI['auth']['getUser']) so tests can use its real type instead of relying on "as never" casts; ensure the function signature reflects ChromeAPI['auth']['getUser'] (or a typed partial/DeepPartial of it) and update call sites to remove the unsafe casts.src/components/HelpPanel/HelpPanelTabs/SearchPanel/SearchPanel.tsx (1)
277-292: Potential negative relevanceScore for services and API docs.Unlike quickstarts (line 254) which use
Math.max(0, ...), the service and API doc relevance scores can go negative with many results:
- Services:
80 - igoes negative after 80 items- API docs:
70 - igoes negative after 70 itemsWhile this may not cause functional issues if results aren't sorted by relevanceScore elsewhere, it's inconsistent with the quickstarts handling.
♻️ Optional: Consistent non-negative relevanceScore
const filteredServiceResults = serviceResults .filter((r) => matchesQuery(r, query)) - .map((r, i) => ({ ...r, relevanceScore: 80 - i })); + .map((r, i) => ({ ...r, relevanceScore: Math.max(0, 80 - i) })); // API docs: build then filter client-side by query const apiDocResults: SearchResult[] = apiDocs.map((apiDoc, index) => ({ id: `api-${index}`, title: apiDoc.name, description: `API documentation for ${apiDoc.services.join(', ')}`, type: 'api', url: apiDoc.url, tags: apiDoc.services, })); const filteredApiResults = apiDocResults .filter((r) => matchesQuery(r, query)) - .map((r, i) => ({ ...r, relevanceScore: 70 - i })); + .map((r, i) => ({ ...r, relevanceScore: Math.max(0, 70 - i) }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/HelpPanel/HelpPanelTabs/SearchPanel/SearchPanel.tsx` around lines 277 - 292, The relevanceScore for filteredServiceResults and filteredApiResults can go negative (using 80 - i and 70 - i); update the map calls that set relevanceScore for serviceResults => filteredServiceResults and apiDocResults => filteredApiResults to clamp to non-negative values (e.g., use Math.max(0, 80 - i) and Math.max(0, 70 - i)) so relevanceScore never drops below 0 and remains consistent with the quickstarts logic that already uses Math.max.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/backend-fuzzy-search-integration.md`:
- Line 31: Update the documentation sentence in
docs/backend-fuzzy-search-integration.md that describes SearchPanel behavior:
change the present-tense claim that SearchPanel "runs Fuse.js over quickstarts +
services + API docs" to indicate this was the previous behavior being replaced
(e.g., "previously ran Fuse.js" or "formerly ran Fuse.js") and/or explicitly
state that Fuse.js has been removed and replaced by the backend fuzzy search;
reference the SearchPanel component and the fetchAllData(getUser, {}) call in
the updated sentence so readers understand which code path the doc is referring
to.
---
Nitpick comments:
In `@src/components/HelpPanel/HelpPanelTabs/SearchPanel/SearchPanel.tsx`:
- Around line 277-292: The relevanceScore for filteredServiceResults and
filteredApiResults can go negative (using 80 - i and 70 - i); update the map
calls that set relevanceScore for serviceResults => filteredServiceResults and
apiDocResults => filteredApiResults to clamp to non-negative values (e.g., use
Math.max(0, 80 - i) and Math.max(0, 70 - i)) so relevanceScore never drops below
0 and remains consistent with the quickstarts logic that already uses Math.max.
In `@src/utils/fetchQuickstarts.test.ts`:
- Around line 7-10: The mockGetUser helper returns a minimal object that doesn't
match ChromeAPI['auth']['getUser']'s full return shape; update mockGetUser to
return a more complete shaped object (including identity, internal.account_id
and any other expected fields like external/subject info or metadata present in
ChromeAPI['auth']['getUser']) so tests can use its real type instead of relying
on "as never" casts; ensure the function signature reflects
ChromeAPI['auth']['getUser'] (or a typed partial/DeepPartial of it) and update
call sites to remove the unsafe casts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cbefd804-8473-4eac-aef2-507616ae2ad0
📒 Files selected for processing (9)
docs/backend-fuzzy-search-integration.mdpackage.jsonsrc/components/GlobalLearningResourcesPage/GlobalLearningResourcesFilters.tsxsrc/components/GlobalLearningResourcesPage/GlobalLearningResourcesFiltersMobile.tsxsrc/components/HelpPanel/HelpPanelTabs/SearchPanel/SearchPanel.tsxsrc/user-journeys/_shared/helpPanelJourneyHelpers.tssrc/utils/FiltersCategoryInterface.tssrc/utils/fetchQuickstarts.test.tssrc/utils/fetchQuickstarts.ts
💤 Files with no reviewable changes (1)
- package.json
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/backend-fuzzy-search-integration.md`:
- Line 56: Update the sentence about Fuse.js so it unambiguously states current
behavior: remove any conditional that suggests a Fuse fallback and explicitly
state that Fuse.js is not used; describe that search for services/apiDocs uses
simple substring matching and that no global Fuse index or Fuse.js dependency
exists in the codebase (remove the phrase “use Fuse only … or simple substring
match” and replace with an explicit line like “No Fuse.js fallback:
services/apiDocs use simple substring matching; Fuse.js is not used”).
- Around line 81-84: Update the "Optional cleanup" section to reflect that
fuse.js has been removed by this PR: replace the line stating removal is
optional with a definitive statement that fuse.js has been removed (mentioning
"fuse.js" explicitly and that it is no longer a project dependency), and keep or
adjust the note about FUZZY_SEARCH_DISTANCE_THRESHOLD so it describes that typo
tolerance is now controlled by the backend env var
(FUZZY_SEARCH_DISTANCE_THRESHOLD) after these changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f1674237-913e-4e42-b55d-04810f798091
📒 Files selected for processing (1)
docs/backend-fuzzy-search-integration.md
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/utils/FiltersCategoryInterface.ts (1)
18-19: Prefer deriving filter keys by value type instead of hard-coding'fuzzy'.This works now, but it is brittle if more boolean/non-filter flags are added to
FetchQuickstartsOptions. Consider derivingFilterCategoryIDfrom option value types so it stays correct automatically.♻️ Suggested type-safe refactor
export type CategoryID = keyof FetchQuickstartsOptions; -/** Category IDs that have array/string values (excludes 'fuzzy', which is boolean). */ -export type FilterCategoryID = Exclude<CategoryID, 'fuzzy'>; +/** Category IDs eligible for filter chips (excludes boolean flags like `fuzzy`). */ +export type FilterCategoryID = { + [K in CategoryID]: FetchQuickstartsOptions[K] extends boolean | undefined + ? never + : K; +}[CategoryID];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/FiltersCategoryInterface.ts` around lines 18 - 19, The current FilterCategoryID hard-codes removal of 'fuzzy' and is brittle; replace it with a conditional/mapped type derived from FetchQuickstartsOptions so keys whose value types are filterable (e.g., string or string[]) are selected automatically. Specifically, create FilterCategoryID by mapping over keyof FetchQuickstartsOptions and keeping keys where FetchQuickstartsOptions[K] extends string | string[] (or the appropriate filter value types), then index the mapped type to produce the union and intersect/ensure it is constrained to CategoryID; update any uses of FilterCategoryID accordingly.src/components/GlobalLearningResourcesPage/AppliedFilters.tsx (1)
43-50: Small cleanup: remove redundant alias and metadata key cast.After the
FilterCategoryIDguard,categoryIdis already the narrowed key. You can simplify this block.🧹 Suggested simplification
- .map((categoryId) => { - const categoryKey = categoryId; - const filters = loaderOptions[categoryKey]; + .map((categoryId) => { + const filters = loaderOptions[categoryId]; if (!Array.isArray(filters) || filters.length === 0) return null; - const categoryName = - FiltersCategoryMetadata[ - categoryId as keyof typeof FiltersCategoryMetadata - ]; + const categoryName = FiltersCategoryMetadata[categoryId]; ... - onClose={() => removeFilter(categoryKey, filterId)} + onClose={() => removeFilter(categoryId, filterId)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/GlobalLearningResourcesPage/AppliedFilters.tsx` around lines 43 - 50, The code creates an unnecessary alias and cast: remove the redundant categoryKey variable and use categoryId directly when reading loaderOptions and FiltersCategoryMetadata; replace loaderOptions[categoryKey] with loaderOptions[categoryId] and access FiltersCategoryMetadata[categoryId] without the "as keyof typeof FiltersCategoryMetadata" cast, relying on the prior FilterCategoryID type guard to ensure correctness.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/components/GlobalLearningResourcesPage/AppliedFilters.tsx`:
- Around line 43-50: The code creates an unnecessary alias and cast: remove the
redundant categoryKey variable and use categoryId directly when reading
loaderOptions and FiltersCategoryMetadata; replace loaderOptions[categoryKey]
with loaderOptions[categoryId] and access FiltersCategoryMetadata[categoryId]
without the "as keyof typeof FiltersCategoryMetadata" cast, relying on the prior
FilterCategoryID type guard to ensure correctness.
In `@src/utils/FiltersCategoryInterface.ts`:
- Around line 18-19: The current FilterCategoryID hard-codes removal of 'fuzzy'
and is brittle; replace it with a conditional/mapped type derived from
FetchQuickstartsOptions so keys whose value types are filterable (e.g., string
or string[]) are selected automatically. Specifically, create FilterCategoryID
by mapping over keyof FetchQuickstartsOptions and keeping keys where
FetchQuickstartsOptions[K] extends string | string[] (or the appropriate filter
value types), then index the mapped type to produce the union and
intersect/ensure it is constrained to CategoryID; update any uses of
FilterCategoryID accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 791607b6-fcf1-48ab-aaaa-9b281105c5c3
📒 Files selected for processing (3)
docs/backend-fuzzy-search-integration.mdsrc/components/GlobalLearningResourcesPage/AppliedFilters.tsxsrc/utils/FiltersCategoryInterface.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/backend-fuzzy-search-integration.md
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/utils/FiltersCategoryInterface.ts (1)
41-48: Consider keeping backend-only params out of UI metadata typing.
FiltersCategoryMetadatais UI-facing, butRecord<CategoryID, string>forces entries likefuzzy: ''. Typing this asRecord<FilterCategoryID, string>avoids carrying technical flags in display metadata.♻️ Suggested refactor
-export const FiltersCategoryMetadata: Record<CategoryID, string> = { +export const FiltersCategoryMetadata: Record<FilterCategoryID, string> = { 'product-families': 'Product families', content: 'Content type', 'use-case': 'Use case', 'display-name': 'Display name', bundle: '', - fuzzy: '', };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/FiltersCategoryInterface.ts` around lines 41 - 48, Change the UI-facing metadata type from Record<CategoryID, string> to Record<FilterCategoryID, string> and update FiltersCategoryMetadata accordingly so backend-only keys (e.g., "fuzzy") are not forced into UI metadata; locate the constant FiltersCategoryMetadata and replace its declared type with Record<FilterCategoryID, string>, and remove or move any backend-only entries (like 'fuzzy') out of this object so the metadata only contains UI display categories.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/utils/FiltersCategoryInterface.ts`:
- Around line 41-48: Change the UI-facing metadata type from Record<CategoryID,
string> to Record<FilterCategoryID, string> and update FiltersCategoryMetadata
accordingly so backend-only keys (e.g., "fuzzy") are not forced into UI
metadata; locate the constant FiltersCategoryMetadata and replace its declared
type with Record<FilterCategoryID, string>, and remove or move any backend-only
entries (like 'fuzzy') out of this object so the metadata only contains UI
display categories.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 01298baf-0aa8-4ddd-b7fd-06c15c99b746
📒 Files selected for processing (2)
src/components/GlobalLearningResourcesPage/AppliedFilters.tsxsrc/utils/FiltersCategoryInterface.ts
Collaborator
Author
|
/retest |
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.
https://redhat.atlassian.net/browse/RHCLOUD-45384
feat(search): use quickstarts backend fuzzy search in help panel and catalog
display-name is set so the quickstarts service uses Levenshtein search.
loading all and filtering with Fuse.js; filter services and API docs
client-side with simple substring match.