Update non prod feature flag to 0.6 threshold - #6571
Conversation
| { | ||
| "tiles": [ | ||
| { | ||
| "type": "PLACE_OVERVIEW" |
There was a problem hiding this comment.
verified the change is reasonable, it will filter out all SV with threshold 0.7 and able to capture SV with threshold 0.6
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request updates feature flag configurations for non-production environments to enable the spanner embedding threshold. It also includes comprehensive updates to various integration test golden files to ensure they remain consistent with the current state of the application. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
| @@ -10,308 +10,115 @@ | |||
| "tiles": [ | |||
There was a problem hiding this comment.
restores back to two SVs as the test intended
| @@ -10,55 +10,28 @@ | |||
| "tiles": [ | |||
There was a problem hiding this comment.
properly filter-in MultiSV after threshold decrease
| @@ -9,29 +9,21 @@ | |||
| { | |||
| "tiles": [ | |||
There was a problem hiding this comment.
heuristic dection type can already find SVs above threshold and didn't goes to LLM detection like before
| @@ -131,6 +131,38 @@ | |||
| "denom": "Count_Person", | |||
There was a problem hiding this comment.
we have more SVs to show with lower threshold and not showing the related like before
| @@ -9,20 +9,23 @@ | |||
| { | |||
| "tiles": [ | |||
There was a problem hiding this comment.
we are able to capture the 2 SV like the NL server
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates several configuration files to include new data visualizations, such as unemployment rates, criminal activity statistics, and housing metrics. The changes involve adding new chart blocks and updating statistical variable specifications across multiple test data files. My review identified that some chart titles are repetitive, which could hinder user experience, and noted a discrepancy in the user message for a specific query regarding California counties.
| } | ||
| ], | ||
| "denom": "Count_Person", | ||
| "title": "Crimes by Type in Cities of San Francisco Bay Area" |
There was a problem hiding this comment.
The title "Crimes by Type in Cities of San Francisco Bay Area" is repeated for multiple chart blocks (for Aggravated Assault, Arson, Burglary, etc.). This will create several sections with the exact same title in the UI, which can be confusing for users.
To improve clarity, each block should have a more specific title that reflects its content. For example, this block could be titled "Aggravated Assault" and the next one "Arson", and so on.
| "svSource": "CURRENT_QUERY", | ||
| "svSource": "UNKNOWN", | ||
| "test": "filter_test", | ||
| "userMessages": [] |
There was a problem hiding this comment.
The user message "Sorry, there were no relevant statistics for cities in California. See results for United States." seems incorrect for this query. The original query was about "counties in California", not cities. Furthermore, the results shown are for California counties, not for the United States. The message is misleading to the user.
| @@ -73,10 +73,335 @@ | |||
| { | |||
There was a problem hiding this comment.
The lower threshold causing the valid char vars (Topic + SVs) goes from 2 to >40, and changes the max number of chart shown for given var from 10 to 3 and cause the chart difference
| @@ -103,10 +103,40 @@ | |||
| { | |||
| "tiles": [ | |||
There was a problem hiding this comment.
this is just a change in place for query_2 and follows the same comment above
1 participant
This PR update the feature flag for autopush, staging, custom, dev and local environment to set detect-and-fulfill threshold to 0.6 from 0.7
It also include the goldens rebases that was done and reverted from #6533