-
Notifications
You must be signed in to change notification settings - Fork 27
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
feature/pass-query-to-event-page #137
feature/pass-query-to-event-page #137
Conversation
The selected gram was "rent control" and the selected context span was ""We know 25% rent increases are Unconsciousable and Seattle needs...". For highlighting in the event card, it will highlight "rent control" or "Renters rights and rent control". (I didn't tokenize these two phrases for highlighting)
Agree. I think the strict match feature is very common amongst searchers. You might not even need the helper text. I think you only need to change something in here:
|
Also should we use weight datetme relevance here?: //use user's selected ranking algo or the default value ? we might need to pass an optional sorting option to eventSearchService.searchEvents |
Made a new issue for search operators for general search. #138
Hmmmm dang. I think it would be better if Yes to strict matching. Agree it should be easy to do for the intra-event search but the general search is a bit of a different challenge. |
So the datetime weighted is useful for search but that function is asking for "what is the most valuable search gram used in the search for this document".. Keep it as it is, its useful for context not search if that makes sense? |
Sure, but the real fix would be to get the selected gram and context span in sync right? |
Yes and no... it's a hack because it's hard to find the right part of the context span to pull from: https://github.com/CouncilDataProject/cdp-backend/blob/main/cdp_backend/pipeline/event_index_pipeline.py#L237 The full sentence found is:
Which has rent control in it but just later and the similarity match for getting the context span simply found the wrong section of the sentence. |
New commit does this:
|
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.
Looks good to me. Thanks for making the change.
Want to include the strict match in transcript search in this PR or no?
Let's wait for the other PR. |
Link to Relevant Issue
This pull request resolves #123
Description of Changes
Link to Forked Storybook Site
https://tohuynh.github.io/cdp-frontend/?path=/story/library-cards-event--meeting-search-result (highlight even card)