Scott/follow up fixes#386
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a structured followUpContext to track conversation history (original query and Q&A chain) for follow-up queries, replacing simple query string concatenation. It also defaults the target place to "Earth" when no explicit place is provided, stripping follow-up options in this scenario to prompt the user for a location. A critical issue was identified in route.ts where the resolved places array is not assigned back to parsed.places after fallback logic, which would prevent the default place from propagating to the client and downstream history nodes.
…llowups to send to api
…ample questions to return if the api returns no options, edit discovery skill to only show follow up if there is no data
6397485 to
70d2a1a
Compare
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
nick-nlb
left a comment
There was a problem hiding this comment.
Thank you Scott! Some comments, but once the actionable ones have been addressed we could merge this in. Some of them will require experimentation and discussion (such as how the model now responds to gibberish query).
We should also make sure we keep onto the radar (te tracker) the movement of "feedback" toasts, such as for adversarial queries)into the followUp loop. It doesn't have to give options, but should give text, and let the user type another query. It should do this for adversarial, nonsense or non-data related queries.
| */ | ||
| const stripNoDataLinks = (md: string, withData: Set<string>): string => { | ||
| return md.replace( | ||
| /\[([^\]]+)\]\(#fetch=([^&]+)&.*?placeName=[^)]*\)/g, |
There was a problem hiding this comment.
This is highly specific (and mirrors the generated links) but also couples this string detection to that generation. If we ever change the order of the parameters, or add other parameters, this becomes fragile.
Since the only thing we need here is the dcid, we could simplify this and make it less reliant on the strict parameter order.
There was a problem hiding this comment.
Ah true, simplified this one!
| - **`options`**: Exactly 3 to 5 concise labels. Each should represent a distinct analytical direction relevant to the place/topic. Phrase them from the user's perspective (e.g. "Economic impact", "Health outcomes for its people", not "GDP variables"). | ||
| 5. **Follow-up (disambiguation signal)**: The `followUp` field is a signal consumed by the orchestrator — it will NOT appear directly in the final per-place result sent to the user. Instead, the orchestrator collects follow-up signals from all places and synthesizes a single follow-up message for the entire conversation round. Include the `followUp` field ONLY as a fallback when you cannot return any meaningful data — for example, the query is gibberish, adversarial, too broad to map to any specific variables, not data-related, or `search_indicators` returned no usable results for the place. In all other cases (including broad exploratory queries like "tell me about Seychelles"), find and return variables directly — do NOT use `followUp` as a disambiguation step. | ||
| - **When to include**: The query is unintelligible, nonsensical, or adversarial; the query has no data-related interpretation; no variables were found after searching; the place name is ambiguous and cannot be resolved. | ||
| - **When to omit**: Any time you can find and return at least one relevant variable with data. Even for broad queries, select the most representative variables across domains and present them directly. |
There was a problem hiding this comment.
This logic pushes the model very hard to avoid asking a "follow-up" - or even in interpreting a nonsense question.
Even pure gibberish such as "asdfasdf" causes it to think it needs to pull something. I had to try very hard to have it ever give any sort of follow-up. This gives the feeling that the agent is disconnected from the questions being asked.
There was a problem hiding this comment.
Yep totally went too far in the 'always provide data' direction :D
I updated this so that:
– if the query fails the safety check, you get a generic follow-up with suggested questions
– if the query fails the parse query step (is gibberish, non-data related, or can't find any places or variables) you get a follow-up that is provided by the llm and is more tailored to the actual question you asked, with suggested questions
– if the data_discovery step doesn't get any good data you get the follow-up plus custom follow-up options that came from the mcp
| }; | ||
|
|
||
| // If multiple places triggered disambiguation, merge summaries. | ||
| if (disambiguationEntries.length > 1) { |
There was a problem hiding this comment.
I worry that this will result in a long list of potentially repetitive or disjointed text, as well as duplicated options. Perhaps this should be run through a follow-up synthesis pass through the LLM to make it a single, coherent summary. However, I would put that aside for now until we have a chance to experiement.
Separately (and more actionable right away), right now we are not dedupping, which would help (but not totally remove the issue, because the two summaries might have similar, but not identical follow-up options).
There was a problem hiding this comment.
I added deduping, but I think you're right this could warrant another llm pass
Overview
Improves the follow-up question flow and related queries experience in DataWeaver. Previously, broad/exploratory queries triggered a follow-up disambiguation step instead of returning data directly, follow-up context (the user's original question and selected option) was lost between turns, and multi-place queries could emit multiple conflicting follow-ups. This PR restructures how follow-up context is passed through the pipeline, tightens the data discovery skill to prefer returning data over asking clarifying questions, consolidates per-place follow-ups into a single synthesized message, adds related query suggestions to card footers, strips dead hyperlinks for variables with no data, and introduces a shuffled pool of example prompts.
Changes Made
FollowUpContexttype that chains the original query and each follow-up Q&A pair, passing it from the client store through the API route into bothparseQueryandrunToolLoopso Gemini has full conversation history when resolving follow-ups.follow_upstream event type. Follow-ups are no longer embedded inside per-placeQueryResultobjects — they're collected across all places in the loop, merged into a single disambiguation message, and emitted once at the end of the stream. This fixes the issue of multiple follow-ups appearing for multi-place queries.nodeSetFollowUpstore action: Added a new store action to write the synthesized follow-up directly onto theHistoryNode, decoupling it from query results.PageHomenow readslatestNode.followUpdirectly instead of scanning through results.data_discovery.mdto only usefollowUpas a last-resort fallback (no data found / unintelligible / adversarial query) rather than a disambiguation step for broad queries. Clarified thatfollowUpis an orchestrator signal, not user-facing output. Added rules for data verification, DCID accuracy, place passing, and the newrelatedQueriesoutput field.relatedQuery(single string) →relatedQueries(string array) acrossCardChart,CardText, shape props, helpers, andsync_store. Card footers now render multiple related-query buttons in a vertical stack.footer.module.scssto use column flexbox with gap for stacked related-query buttons.render_result_html.tsnow identifies which variables actually have time-series data and converts#fetch=markdown links for empty variables into plain text — both in the table rows and in the introduction/coverage/insights prose.dismissedNodeIdstate inPageHomewith anodeDismissFollowUpstore action that removes the follow-up from the node, preventing reappearance on re-renders.Introinto a sharedconfigs/example_prompts.tspool of 20 prompts. The page server component shuffles and slices 4 at render time. The follow-up component also pulls from this pool as a fallback when the API returns no options."Earth"instead of using the raw query string as a place, and strips follow-up options so the model asks the user to specify a location.parse_query.mdandparse_query.tsto extract places from the full follow-up conversation context and to be more conservative about marking queries as follow-ups when they contain explicit places/topics.query_provider— results now always register cards, and follow-ups arrive separately via the new stream event.