-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
chore(trace ai queries): Human readable queries #97050
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
Changes from all commits
51acc22
fe84a99
14bcb2d
5d1bc57
656a3a5
2454089
ac26da9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,108 @@ | ||
import type {QueryTokensProps} from 'sentry/views/explore/components/seerComboBox/queryTokens'; | ||
|
||
function formatToken(token: string): string { | ||
const isNegated = token.startsWith('!') && token.includes(':'); | ||
const actualToken = isNegated ? token.slice(1) : token; | ||
|
||
const operators = [ | ||
[':>=', 'greater than or equal to'], | ||
[':<=', 'less than or equal to'], | ||
[':!=', 'not'], | ||
[':>', 'greater than'], | ||
[':<', 'less than'], | ||
['>=', 'greater than or equal to'], | ||
['<=', 'less than or equal to'], | ||
['!=', 'not'], | ||
['!:', 'not'], | ||
['>', 'greater than'], | ||
['<', 'less than'], | ||
[':', ''], | ||
] as const; | ||
|
||
for (const [op, desc] of operators) { | ||
if (actualToken.includes(op)) { | ||
const [key, value] = actualToken.split(op); | ||
const cleanKey = key?.trim() || ''; | ||
const cleanVal = value?.trim() || ''; | ||
|
||
const negation = isNegated ? 'not ' : ''; | ||
const description = desc ? `${negation}${desc}` : negation ? 'not' : ''; | ||
|
||
return `${cleanKey} is ${description} ${cleanVal}`.replace(/\s+/g, ' ').trim(); | ||
} | ||
} | ||
|
||
return token; | ||
} | ||
|
||
export function formatQueryToNaturalLanguage(query: string): string { | ||
if (!query.trim()) return ''; | ||
const tokens = query.match(/(?:[^\s"]+|"[^"]*")+/g) || []; | ||
const formattedTokens = tokens.map(formatToken); | ||
|
||
return formattedTokens.reduce((result, token, index) => { | ||
if (index === 0) return token; | ||
|
||
const currentOriginalToken = tokens[index] || ''; | ||
const prevOriginalToken = tokens[index - 1] || ''; | ||
|
||
const isLogicalOp = token.toUpperCase() === 'AND' || token.toUpperCase() === 'OR'; | ||
const prevIsLogicalOp = | ||
formattedTokens[index - 1]?.toUpperCase() === 'AND' || | ||
formattedTokens[index - 1]?.toUpperCase() === 'OR'; | ||
|
||
if (isLogicalOp || prevIsLogicalOp) { | ||
return `${result} ${token}`; | ||
} | ||
|
||
const isCurrentFilter = /[:>=<!]/.test(currentOriginalToken); | ||
const isPrevFilter = /[:>=<!]/.test(prevOriginalToken); | ||
Comment on lines
+58
to
+59
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What exactly is this testing for? Just a heuristic to see if it's a filter? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe to see if there are multiple filters, and instead of giving them a space, it gives a comma than a space afaict. This was just something i moved to be located closer to where it's used. |
||
|
||
if (isCurrentFilter && isPrevFilter) { | ||
return `${result}, ${token}`; | ||
} | ||
|
||
return `${result} ${token}`; | ||
}, ''); | ||
} | ||
|
||
export function generateQueryTokensString({ | ||
groupBys, | ||
query, | ||
sort, | ||
statsPeriod, | ||
visualizations, | ||
}: QueryTokensProps): string { | ||
const parts = []; | ||
|
||
if (query) { | ||
const formattedFilter = formatQueryToNaturalLanguage(query.trim()); | ||
parts.push(`Filter is '${formattedFilter}'`); | ||
} | ||
|
||
if (visualizations && visualizations.length > 0) { | ||
const vizParts = visualizations.flatMap(visualization => | ||
visualization.yAxes.map(yAxis => yAxis) | ||
); | ||
if (vizParts.length > 0) { | ||
const vizText = vizParts.length === 1 ? vizParts[0] : vizParts.join(', '); | ||
parts.push(`visualizations are '${vizText}'`); | ||
} | ||
} | ||
|
||
if (groupBys && groupBys.length > 0) { | ||
const groupByText = groupBys.length === 1 ? groupBys[0] : groupBys.join(', '); | ||
parts.push(`groupBys are '${groupByText}'`); | ||
} | ||
|
||
if (statsPeriod && statsPeriod.length > 0) { | ||
parts.push(`time range is '${statsPeriod}'`); | ||
} | ||
|
||
if (sort && sort.length > 0) { | ||
const sortText = sort[0] === '-' ? `${sort.slice(1)} Desc` : `${sort} Asc`; | ||
parts.push(`sort is '${sortText}'`); | ||
} | ||
|
||
return parts.length > 0 ? parts.join(', ') : 'No query parameters set'; | ||
} |
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.
Bug: Accessibility Issue: Missing
aria-label
PropThe code attempts to access
item['aria-label']
whereitem
is a React StatelyNode<unknown>
object. Thearia-label
prop, passed to the<Item>
component, is not directly available as a property on theNode
object. This causesitem['aria-label']
to beundefined
, preventing thearia-label
attribute from being set on the rendered option and defeating the intended accessibility enhancement.