-
Notifications
You must be signed in to change notification settings - Fork 7
GIE-226: Add LLM-friendly errors and handle empty response cases #6
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
base: main
Are you sure you want to change the base?
Conversation
This commit transforms query errors to be LLM-friendly with guidance on how to proceed, and also detects and adds guidance for LLMs on empty query responses. Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
|
|
||
| // Check for empty results and add guidance separately | ||
| if emptyWarning := checkEmptyResult(result, query); emptyWarning != "" { | ||
| response["emptyResultGuidance"] = emptyWarning |
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 better target the case when the metric is purely hallucinated and not present in the system at all. Rather than giving the guidance to the LLM to check the list_metrics, we could then right away return query as error, if the targeted time-series doesn't exist, both saving tokens and getting more deterministic behavior.
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.
There's no real way of doing this here directly when we are executing the query, but...I forgot we already do check for this in the guardrails method. I can move those errors into isSafeQuery method then
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.
Yes, that's what I was thinking of.
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.
Tried to address this in b923bf5. This is a bit more complex now, but essentially we check parsing, metric and label name existence in guardrails (as an always-on check). And then we check if query response is still empty post execution and guide accordingly
Signed-off-by: Saswata Mukherjee <saswataminsta@yahoo.com>
|
I've encountered 2 issues during the testing:
The list_metrics definitely has the metric. But the TSDB stats contain only small subset: Is there some pagination going when loading the data? |
|
Hmm yes this is probably due to the specific setup on OpenShift, where we use Querier on top of Thanos sidecars. I have an idea of addressing this, will follow up |
|
@saswatamcode what's the plan with this PR? Revisit after #15 gets in? |
|
Yes exactly! |
This commit transforms query errors to be LLM-friendly with guidance on how to proceed, and also detects and adds guidance for LLMs on empty query responses.