Skip to content

RI-7195: Add search screen #4788

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

pawelangelow
Copy link
Collaborator

@pawelangelow pawelangelow commented Aug 1, 2025

This PR introduces the Search screen, which is intended to be displayed when there are existing indexes.

Since the functionality is quite similar to Workbench, I attempted to reuse some of its logic. However, due to structural differences, full reuse isn't straightforward at this stage. Here's a breakdown of what's included in this change:

  • useQuery – This is the core piece of the PR and deserves the most attention during review. It replicates the same command execution flow as Workbench, but implemented as a React hook rather than going through Redux.
    Note: Unit tests are not included in this PR but will be added later in the flow once the behavior stabilizes.
  • VectorSearchQuery – A resizable layout wrapper combining the Query and Results panes.
  • Extended API for QueryCardHeader – Added a hideFields prop to control the visibility of specific elements like "Profiling" and the "View type" toggle.
  • Temporary duplication of QueryCard and CommandsView components – Similar to RI-7195: Extend Query component #4794, this is a workaround to avoid breaking Workbench while the Search feature evolves. Once we finalize some open design decisions, we can revisit this and consolidate the components properly.

If you want to test with the provided data, use these commands:

  • FT.SEARCH idx:bikes_vss "@brand:Nord" SORTBY price ASC
  • FT.SEARCH idx:bikes_vss "@material:{alloy} @weight:[0 20]"

@pawelangelow pawelangelow self-assigned this Aug 1, 2025
Copy link
Contributor

github-actions bot commented Aug 1, 2025

Code Coverage - Frontend unit tests

St.
Category Percentage Covered / Total
🟢 Statements 80.9% 19182/23711
🟡 Branches 66.32% 8336/12570
🟡 Functions 74.59% 5035/6750
🟢 Lines 81.33% 18774/23084

Test suite run success

4923 tests passing in 650 suites.

Report generated by 🧪jest coverage report action from ceb38aa

@pawelangelow pawelangelow force-pushed the fe/RI-7195/add-search-screen branch from a57abf3 to 872aefb Compare August 1, 2025 14:39
@pawelangelow pawelangelow changed the base branch from feature/RI-6855/vector-search to fe/RI-7195/extend-query August 1, 2025 14:40
@pawelangelow pawelangelow marked this pull request as ready for review August 1, 2025 14:52
@pawelangelow pawelangelow force-pushed the fe/RI-7195/add-search-screen branch from 872aefb to e9ebec0 Compare August 1, 2025 14:57
Base automatically changed from fe/RI-7195/extend-query to feature/RI-6855/vector-search August 4, 2025 08:09
@pawelangelow pawelangelow force-pushed the fe/RI-7195/add-search-screen branch from e9ebec0 to afaf8c4 Compare August 4, 2025 08:12
Copy link
Contributor

github-actions bot commented Aug 4, 2025

Code Coverage - Integration Tests

Status Category Percentage Covered / Total
🟢 Statements 81.71% 16239/19873
🟡 Branches 64.87% 7347/11325
🟡 Functions 70.78% 2282/3224
🟢 Lines 81.35% 15277/18778

Copy link
Contributor

github-actions bot commented Aug 4, 2025

Code Coverage - Backend unit tests

St.
Category Percentage Covered / Total
🟢 Statements 92.34% 13797/14942
🟡 Branches 74.02% 4157/5616
🟢 Functions 85.86% 2125/2475
🟢 Lines 92.13% 13190/14317

Test suite run success

2948 tests passing in 286 suites.

Report generated by 🧪jest coverage report action from 71bcb66

<QueryWrapper
query={query}
activeMode={activeMode}
resultsMode={resultsMode}
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure whether it's an issue with the way I test it, but I've noticed the following issue.

If we run queries on the Workbench page, I see a button with the option to toggle the way we display the results - via text, or render them in a table.

Image

However, when I go to the new Vector Search page and run a query, I don't see the button for toggling the view.

Image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, this is intended - the designs use a very lite version of that component (the one that displays the command + the controls for it - re-reun, delete, expand, etc..). So I just copied the one from the workbench and disabled some stuff - the view type changer + profile button. If you see the last methods of the useQuery hook - we don't support them at all for this page.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the information. I asked because we have telemetry events planned for these buttons, and SEARCH_RESULT_VIEW_CHANGED is also included. Anyway, if it's not part of the design, I can omit this one for now, and cover the rest.

Copy link
Collaborator

@KrumTy KrumTy left a comment

Choose a reason for hiding this comment

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

overall the useQuery hook seems really complex, it lacks tests as well
can it be restructured/split in a way or the functionality has to be together?

loadHistory()
}, [instanceId])

const prepareNewItems = (
Copy link
Collaborator

Choose a reason for hiding this comment

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

can extract from hook

valkirilov
valkirilov previously approved these changes Aug 5, 2025
@pawelangelow pawelangelow merged commit bb18d5d into feature/RI-6855/vector-search Aug 5, 2025
17 checks passed
@pawelangelow pawelangelow deleted the fe/RI-7195/add-search-screen branch August 5, 2025 13:15
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.

3 participants