- 
                Notifications
    You must be signed in to change notification settings 
- Fork 14
Deduplicate generative direct answer calls #554
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
Conversation
The effect hook that triggers a GDA call has two dependencies: the search results and the search ID. These both change when a search is made. In React 18, these updates are batched together so only one call to GDA gets made when the search is updated. However, in React 17, no batching is performed and updates are handled in order, meaning a call to GDA is made when the search results are updated and when the search ID is updated, even if the same search triggers both updates. This change tracks the search results and only executes a new GDA call if the search results have changed. J=WAT-4861 TEST=manual Created a local test site that runs on React 17. Made a local tarball with my changes to search-ui-react using npm pack and pointed the local test site's import of search-ui-react to the tarball. Confirmed that only one GDA call is made per-seasrch. Also spun up the search-ui-react test-site (which runs on React 18) and confirmed that GDA calls are still made once per search as expected.
| Current unit coverage is 91.48841354723707% | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we could use src/hooks/useDebouncedFunction.ts instead?
Also, do we want to update the package version in this change or does an automated GH workflow action do that?
| 
 We could, as the delta between the searchResults object getting updated and the searchId object getting updated is likely minimal. However, explicitly processing just one of the updates strikes me as better practice. I think that my usage of "debounce" here is incorrect and misleading - I meant "deduplicate". We absolutely want to bump the patch version, good call. Updating. | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a bug where generative direct answer (GDA) calls were duplicated in React 17 due to unbatched state updates, while maintaining single GDA calls in React 18. The fix tracks the previous search results to prevent unnecessary duplicate calls when both search results and search ID change for the same search operation.
- Introduced state tracking to compare current search results with previously executed search results
- Updated the effect hook dependency array and logic to prevent duplicate GDA calls
- Bumped package version from 1.9.3 to 1.9.4
Reviewed Changes
Copilot reviewed 2 out of 4 changed files in this pull request and generated 1 comment.
| File | Description | 
|---|---|
| src/components/GenerativeDirectAnswer.tsx | Added state tracking to prevent duplicate GDA calls by comparing search results | 
| package.json | Version bump to 1.9.4 | 
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
The effect hook that triggers a GDA call has two dependencies: the search results and the search ID. These both change when a search is made. In React 18, these updates are batched together so only one call to GDA gets made when the search is updated. However, in React 17, no batching is performed and updates are handled in order, meaning a call to GDA is made when the search results are updated and when the search ID is updated, even if the same search triggers both updates. This change tracks the search results and only executes a new GDA call if the search results have changed.
J=WAT-4861
TEST=manual
Created a local test site that runs on React 17. Made a local tarball with my changes to search-ui-react using npm pack and pointed the local test site's import of search-ui-react to the tarball. Confirmed that only one GDA call is made per-seasrch.
Also spun up the search-ui-react test-site (which runs on React 18) and confirmed that GDA calls are still made once per search as expected.