diff --git a/static/app/components/core/chat/thinkingBlock.tsx b/static/app/components/core/chat/thinkingBlock.tsx index 53ef7386b591..5d8d323eab42 100644 --- a/static/app/components/core/chat/thinkingBlock.tsx +++ b/static/app/components/core/chat/thinkingBlock.tsx @@ -91,6 +91,7 @@ export function ThinkingBlock({title, startTime, endTime, children}: ThinkingBlo size="sm" variant="outline" flex={1} + minWidth={0} > @@ -32,11 +34,13 @@ A tool call collapses to a single title line, with an optional trailing title="Read trace waterfall" status="success" reference={{label: 'Trace', value: 'a3805648'}} + durationMs={6400} /> @@ -46,20 +50,29 @@ A tool call collapses to a single title line, with an optional trailing title="Read trace waterfall" status="success" reference={{label: 'Trace', value: 'a3805648'}} + durationMs={6400} /> ``` -## Output +## Input -When a call produces a primary result, pass `output` to surface it as a chip -under an `Output:` label. +Pass `input` to render the call's request under an `Input:` label. It is a slot, +so give it a decomposed view — typically a `FormattedQuery`, which parses a +Sentry search string (including boolean and parenthesized grouping) into query +chips. + dataset:spans project:ml-service span.description:DSL + + } /> @@ -68,20 +81,58 @@ under an `Output:` label. } +/> +``` + +## Output + +Pass `output` to render the call's result under an `Output:` label — a result +value, or on failure the error itself. Like `input`, it is a slot. + + + + + Returned HTTP 502 + + } + /> + + + +```jsx +Returned HTTP 502} /> ``` ## Status -The leading glyph reflects the call's lifecycle. +The leading glyph reflects the call's lifecycle. A `failure` keeps that glyph and +additionally hoists a chip into the trailing result slot — where a successful +call shows its `reference` — carrying the `failureLabel` (typically the HTTP +status code, e.g. `502`), so the outcome reads on the right rather than only as a +small glyph on the far left. - - + + @@ -101,9 +152,8 @@ Pass `notifications` to surface short status lines beneath a call. ## Links -Give a `reference` or `output` chip a `to` to render it as a real link (an -anchor supporting middle/cmd-click and keyboard access) rather than an -`onClick` button. +Give a `reference` chip a `to` to render it as a real link (an anchor supporting +middle/cmd-click and keyboard access) rather than an `onClick` button. @@ -125,9 +175,9 @@ anchor supporting middle/cmd-click and keyboard access) rather than an ## Detail -Pass `children` to tuck supplementary detail beneath the call — for example an -expandable request/response. It lives in the collapsible panel, revealed by -toggling the title. +Pass `children` to tuck supplementary detail beneath the call — for example a +request body. It renders inline, indented under the title; a tool call has no +disclosure of its own. diff --git a/static/app/components/core/chat/toolCall.spec.tsx b/static/app/components/core/chat/toolCall.spec.tsx index 6a7cb34d855d..862891b3a9e3 100644 --- a/static/app/components/core/chat/toolCall.spec.tsx +++ b/static/app/components/core/chat/toolCall.spec.tsx @@ -1,4 +1,4 @@ -import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary'; +import {render, screen} from 'sentry-test/reactTestingLibrary'; import {ToolCall} from '@sentry/scraps/chat'; @@ -17,18 +17,18 @@ describe('ToolCall', () => { expect(screen.queryByText('Output:')).not.toBeInTheDocument(); }); - it('renders an output chip when output is provided', () => { + it('renders an output slot under an Output label', () => { render( Returned HTTP 502} /> ); expect(screen.getByText('Query spans')).toBeInTheDocument(); expect(screen.getByText('Output:')).toBeInTheDocument(); - expect(screen.getByText('a3805648')).toBeInTheDocument(); + expect(screen.getByText('Returned HTTP 502')).toBeInTheDocument(); }); it('communicates status via the leading glyph', () => { @@ -42,6 +42,39 @@ describe('ToolCall', () => { expect(screen.getByLabelText('Running')).toBeInTheDocument(); }); + it('keeps the leading glyph and hoists a Failed chip beside the result on failure', () => { + render(); + + // Leading glyph (accessible label) plus a visible trailing chip. + expect(screen.getByLabelText('Failed')).toBeInTheDocument(); + expect(screen.getByText('Failed')).toBeInTheDocument(); + }); + + it('shows the failureLabel (e.g. HTTP status) in the trailing chip', () => { + render(); + + expect(screen.getByText('502')).toBeInTheDocument(); + expect(screen.queryByText('Failed')).not.toBeInTheDocument(); + }); + + it('renders a duration in the meta slot', () => { + render(); + expect(screen.getByText('9.4s')).toBeInTheDocument(); + }); + + it('renders the input slot under an Input label', () => { + render( + dataset is spans} + /> + ); + + expect(screen.getByText('Input:')).toBeInTheDocument(); + expect(screen.getByText('dataset is spans')).toBeInTheDocument(); + }); + it('surfaces notifications', () => { render( { ); }); - it('reveals supplementary detail children when expanded', async () => { + it('renders supplementary detail children inline, always visible', () => { render(
GET /api/0/traces/a3805648/
 ); - // Detail lives in the collapsible panel, so it is hidden until the title is toggled. - const detail = screen.getByText('GET /api/0/traces/a3805648/'); - expect(detail).not.toBeVisible(); - - await userEvent.click(screen.getByRole('button', {name: /Query spans/})); - - expect(detail).toBeVisible(); + // A tool call is not a disclosure: its detail is not tucked behind a toggle. + expect(screen.getByText('GET /api/0/traces/a3805648/')).toBeVisible(); + expect(screen.queryByRole('button', {name: /Query spans/})).not.toBeInTheDocument(); }); }); diff --git a/static/app/components/core/chat/toolCall.tsx b/static/app/components/core/chat/toolCall.tsx index 63a5ebfa4c71..778871fb5771 100644 --- a/static/app/components/core/chat/toolCall.tsx +++ b/static/app/components/core/chat/toolCall.tsx @@ -2,12 +2,13 @@ import type {MouseEvent, ReactNode} from 'react'; import type {LocationDescriptor} from 'history'; import {Button, LinkButton} from '@sentry/scraps/button'; -import {Disclosure} from '@sentry/scraps/disclosure'; -import {Container, Flex} from '@sentry/scraps/layout'; +import {Container, Flex, Stack} from '@sentry/scraps/layout'; import {Text} from '@sentry/scraps/text'; import {IconSpan} from 'sentry/icons'; import {t} from 'sentry/locale'; +import {getDuration} from 'sentry/utils/duration/getDuration'; +import {SECOND} from 'sentry/utils/formatters'; import {unreachable} from 'sentry/utils/unreachable'; import {ToolCallIndicator, type ToolCallStatus} from './toolCallIndicator'; @@ -46,7 +47,8 @@ export interface ToolCallReference { interface ToolCallProps { /** * Lifecycle status. Drives the leading glyph (spinner while running, semantic - * icon once settled) via `ToolCallIndicator`. + * icon once settled) via `ToolCallIndicator`. A `failure` also surfaces a + * trailing `Failed` chip in the result area so the outcome reads on the right. */ status: ToolCallStatus; /** @@ -54,26 +56,47 @@ interface ToolCallProps { */ title: string; /** - * Supplementary detail rendered beneath the title and output — e.g. an - * expandable request/response for the call. Kept in the title's column so it - * aligns under the headline rather than the status glyph. + * Supplementary detail rendered beneath the title — e.g. the request body. + * Always visible: a nested tool call has no disclosure of its own. */ children?: ReactNode; + /** + * How long the call took, in milliseconds. Rendered right-aligned in the + * trailing meta slot. Omit when the duration is unknown. + */ + durationMs?: number; + /** + * The trailing danger chip's text when `status` is `failure` (e.g. the HTTP + * status code `502`). Defaults to `Failed`. + */ + failureLabel?: string; + /** + * The call's request, rendered under an `Input:` label. Pass a decomposed + * view (e.g. a `FormattedQuery`) so the request reads as its filters rather + * than a raw URL string. + */ + input?: ReactNode; /** * Short status lines surfaced beneath the call (e.g. "Truncated to 100 rows"). */ notifications?: string[]; /** - * The primary result of the call, rendered as a chip under an `Output:` label. + * The call's result, rendered under an `Output:` label — a result value, or on + * failure the error itself. A slot, mirroring `input`. */ - output?: ToolCallReference; + output?: ReactNode; /** * A trailing chip shown inline with the title. Typically the entity the call - * acted on. + * acted on — the call's result. */ reference?: ToolCallReference; } +// The leading status glyph and the indent of every row beneath the title are +// pinned to this width so detail (input chips, notifications, children) aligns +// under the headline rather than under the glyph. +const GLYPH_SLOT_WIDTH = '16px'; + function ChipContent({label, value}: {value: string; label?: string}) { return label ? ( @@ -111,10 +134,65 @@ function ReferenceChip({reference}: {reference: ToolCallReference}) { ); } -function SecondaryBox({children}: {children: ReactNode}) { +/** + * The hoisted failure marker. A failed call keeps its leading glyph but also + * surfaces this danger chip in the trailing result slot, where a successful call + * would show its `reference` — so the outcome is legible on the right rather than + * only as a small glyph on the far left. The `label` is typically the HTTP status + * code (e.g. `502`). + */ +function FailureChip({label}: {label: ReactNode}) { + return ( + + + {label} + + + ); +} + +function ToolCallDuration({durationMs}: {durationMs: number}) { + return ( + + {getDuration(durationMs / 1000, 1, true, false, false, SECOND)} + + ); +} + +function InputBox({input}: {input: ReactNode}) { + return ( + + + + {t('Input:')} + + {input} + + + ); +} + +function OutputBox({output}: {output: ReactNode}) { return ( - {children} + + + {t('Output:')} + + {output} + ); } @@ -141,51 +219,67 @@ function getStatusLabel(status: ToolCallStatus): string | undefined { /** * A single agent tool call within a `ThinkingBlock`. * - * Built on the same outline `Disclosure` as `ThinkingBlock`: the lifecycle glyph - * (`ToolCallIndicator`) is the leading item, the `title` is the toggle, and an - * optional `reference` chip trails it. `output` and `notifications` sit under the - * title and stay visible; pass `children` to tuck supplementary detail (e.g. the - * request/response) into the collapsible panel. + * Unlike the collapsible `ThinkingBlock` it lives in, a tool call is not itself a + * disclosure — its detail is always visible. The lifecycle glyph + * (`ToolCallIndicator`) leads the title; an optional `reference` chip and, on + * failure, a `failureLabel` chip (the HTTP status) trail it; and a `durationMs` + * reads right-aligned in the meta slot. `input`, `output`, `notifications`, and + * `children` stack beneath the title, indented to align under the headline. */ export function ToolCall({ title, status, + durationMs, + failureLabel, + input, output, reference, notifications, children, }: ToolCallProps) { + const isFailure = status === 'failure'; + const hasTrailing = Boolean(reference) || isFailure; + const hasDetail = + Boolean(input) || + Boolean(output) || + Boolean(notifications?.length) || + Boolean(children); + return ( - - + + - } - trailingItems={reference ? : undefined} - > - - {title} - - - - {output ? ( - - - - {t('Output:')} - - - - - ) : null} - - {notifications?.map((note, i) => ( - - {note} - - ))} + + + + {title} + + {hasTrailing ? ( + + {reference ? : null} + {isFailure ? : null} + + ) : null} + + {durationMs === undefined ? null : } + - {children ? {children} : null} - + {hasDetail ? ( + + + + {input ? : null} + {output ? : null} + {notifications?.map((note, i) => ( + + {note} + + ))} + {children} + + + ) : null} + ); } diff --git a/static/app/views/seerExplorer/callRecords.tsx b/static/app/views/seerExplorer/callRecords.tsx index edd654ec3443..7373e09a251d 100644 --- a/static/app/views/seerExplorer/callRecords.tsx +++ b/static/app/views/seerExplorer/callRecords.tsx @@ -1,3 +1,5 @@ +import {parseSearch, Token} from 'sentry/components/searchSyntax/parser'; +import {getKeyName} from 'sentry/components/searchSyntax/utils'; import {t} from 'sentry/locale'; import type {CallRecord} from 'sentry/views/seerExplorer/types'; @@ -88,6 +90,121 @@ export function callRecordDetail(record: CallRecord): { }; } +// Query params that scope or format a request rather than describe what it looked for. Decomposing +// these into chips would bury the meaningful filters (dataset, project, the search itself) under +// pagination and field-selection noise, so they are dropped. +const NON_FILTER_PARAMS = new Set([ + 'referrer', + 'per_page', + 'cursor', + 'sort', + 'field', + 'useRpc', + 'sampling', + 'noPagination', + 'partial', + 'utc', +]); + +/** + * A filter key's specificity, for ordering the `Input:` chips from broadest scope to narrowest + * identifier. An id (`trace_id`, `ai_conversation.id`) pins one record; a namespaced attribute + * (`span.description`) narrows within a dataset; a plain key (`dataset`, `project`) scopes broadly. + * Higher sorts later. + */ +function keySpecificity(key: string): number { + if (key === 'id' || key.endsWith('.id') || key.endsWith('_id')) { + return 3; + } + return key.includes('.') ? 2 : 1; +} + +// Wrap a value that would otherwise re-tokenize wrong (spaces, quotes, parens) so the assembled +// query parses back to the same filter. +function quoteValue(value: string): string { + return /[\s"()]/.test(value) + ? `"${value.replace(/\\/g, '\\\\').replace(/"/g, '\\"')}"` + : value; +} + +/** + * Reorder a flat query into canonical least→most specific order. + * + * Only safe for a flat conjunction: boolean/parenthesized grouping makes order meaningful, so a + * grouped query is returned untouched. Otherwise each term is sorted by its key's specificity + * (ties broken by text) so the same request always reads the same way regardless of the order the + * params happened to arrive in. + */ +function canonicalizeQuery(query: string): string { + const parsed = parseSearch(query); + if (!parsed) { + return query; + } + + // Anything beyond flat filters and whitespace — a logic group `(a OR b)`, a bare boolean, a + // stray paren — makes token order meaningful, so the query is left exactly as written. + const hasGrouping = parsed.some( + token => + token.type !== Token.FILTER && + token.type !== Token.FREE_TEXT && + token.type !== Token.SPACES + ); + if (hasGrouping) { + return query; + } + + const terms = parsed.flatMap(token => { + if (token.type === Token.FILTER) { + return [ + {text: token.text.trim(), specificity: keySpecificity(getKeyName(token.key))}, + ]; + } + // Free text has no key to rank, so it sorts first (least specific). + if (token.type === Token.FREE_TEXT && token.text.trim()) { + return [{text: token.text.trim(), specificity: 0}]; + } + return []; + }); + + terms.sort((a, b) => a.specificity - b.specificity || a.text.localeCompare(b.text)); + return terms.map(term => term.text).join(' '); +} + +/** + * The call's request as a single canonical query string for the `Input:` row, or null when there + * is nothing to show. + * + * Reads the query string off `resolved_path` (the literal URL requested): each meaningful param + * becomes a `key:value` term and a Sentry `query` param is folded in as its own filters, then the + * whole thing is canonicalized (see `canonicalizeQuery`). The Explorer hands the result to + * `FormattedQuery`, which parses grouping and renders the chips — so this only has to assemble and + * order the terms. Scope/format params are dropped (`NON_FILTER_PARAMS`). + */ +export function callRecordInputQuery(record: CallRecord): string | null { + const path = record.resolved_path ?? record.path; + const queryIndex = path?.indexOf('?') ?? -1; + if (!path || queryIndex === -1) { + return null; + } + + const params = new URLSearchParams(path.slice(queryIndex + 1)); + const terms: string[] = []; + let search = ''; + for (const [key, value] of params) { + if (!value || NON_FILTER_PARAMS.has(key)) { + continue; + } + if (key === 'query') { + search = value; + continue; + } + terms.push(`${key}:${quoteValue(value)}`); + } + + const raw = [...terms, search].filter(Boolean).join(' ').trim(); + return raw ? canonicalizeQuery(raw) : null; +} + /** Mark a cut-short preview so the box does not read as the whole payload. */ function withEllipsis( text: string | undefined, diff --git a/static/app/views/seerExplorer/components/chat/callRecords.spec.tsx b/static/app/views/seerExplorer/components/chat/callRecords.spec.tsx index 0f06f3f38958..41f60ff4d3a2 100644 --- a/static/app/views/seerExplorer/components/chat/callRecords.spec.tsx +++ b/static/app/views/seerExplorer/components/chat/callRecords.spec.tsx @@ -1,7 +1,8 @@ -import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary'; +import {render, screen} from 'sentry-test/reactTestingLibrary'; import { callRecordDetail, + callRecordInputQuery, callRecordLabel, callRecordStatus, } from 'sentry/views/seerExplorer/callRecords'; @@ -98,14 +99,16 @@ describe('call record rendering', () => { ]); render(); - expect(screen.getByRole('link', {name: /Retrieve an Issue/})).toHaveAttribute( + // The navigable resource is a trailing result chip (a LinkButton: role button + href), labeled + // by its kind rather than by the tool's own title. + expect(screen.getByRole('button', {name: /View issue/})).toHaveAttribute( 'href', expect.stringContaining('/issues/139458447/') ); }); - describe('a row that both expands and navigates', () => { - /** An api call with a destination *and* a request to show — both affordances on one row. */ + describe('a row that navigates', () => { + /** An api call with a destination to link to. */ function linkable(): CallRecord { return apiRecord({ path: '/api/0/organizations/{organization_id_or_slug}/issues/{issue_id}/', @@ -115,24 +118,37 @@ describe('call record rendering', () => { }); } - it('keeps the link out of the disclosure button', () => { + it('surfaces the resource as a linked result chip, not a disclosure toggle', () => { render(); - // An anchor inside a button is invalid HTML, and it leaves expand and navigate sharing one - // click target and one tab stop. - const link = screen.getByRole('link', {name: /Retrieve an Issue/}); - const expander = screen.getByRole('button', {name: /Retrieve an Issue/}); - expect(expander).not.toContainElement(link); + // A tool call is not a disclosure: the title is plain text, not an expand toggle, so nothing + // shares a click target with the link. + expect( + screen.queryByRole('button', {name: /Retrieve an Issue/}) + ).not.toBeInTheDocument(); + expect(screen.getByRole('button', {name: /View issue/})).toHaveAttribute( + 'href', + expect.stringContaining('/issues/139458447/') + ); }); - it('still expands to the request it made', async () => { - render(); - - await userEvent.click(screen.getByRole('button', {name: /Retrieve an Issue/})); - - expect( - screen.getByText('GET /api/0/organizations/acme/issues/139458447/') - ).toBeInTheDocument(); + it('decomposes the request query string into readonly input chips', () => { + const block = codeModeBlock([ + apiRecord({ + resolved_path: + '/api/0/organizations/sentry/events/?dataset=spans&project=ml-service&query=ai_conversation.id%3A28193042', + }), + ]); + render(); + + expect(screen.getByText('Input:')).toBeInTheDocument(); + expect(screen.getByText('dataset')).toBeInTheDocument(); + expect(screen.getByText('spans')).toBeInTheDocument(); + expect(screen.getByText('project')).toBeInTheDocument(); + expect(screen.getByText('ml-service')).toBeInTheDocument(); + // The Sentry search string is expanded into its own filter chips rather than shown raw. + expect(screen.getByText('ai_conversation.id')).toBeInTheDocument(); + expect(screen.getByText('28193042')).toBeInTheDocument(); }); }); @@ -399,6 +415,78 @@ describe('callRecordDetail', () => { }); }); +describe('callRecordInputQuery', () => { + it('has no input for a request with no query string', () => { + expect( + callRecordInputQuery(apiRecord({resolved_path: '/api/0/issues/54/'})) + ).toBeNull(); + }); + + it('turns each meaningful query param into a key:value term', () => { + expect( + callRecordInputQuery( + apiRecord({ + resolved_path: + '/api/0/organizations/sentry/events/?dataset=spans&project=ml-service', + }) + ) + ).toBe('dataset:spans project:ml-service'); + }); + + it('folds a Sentry query param in as its own filters', () => { + expect( + callRecordInputQuery( + apiRecord({ + resolved_path: + '/api/0/organizations/sentry/events/?query=span.description%3ADSL', + }) + ) + ).toBe('span.description:DSL'); + }); + + it('quotes a query value that would re-tokenize wrong', () => { + expect( + callRecordInputQuery(apiRecord({resolved_path: '/api/0/x/?transaction=GET /foo'})) + ).toBe('transaction:"GET /foo"'); + }); + + it('drops scope and formatting params so filters are not buried in noise', () => { + expect( + callRecordInputQuery( + apiRecord({ + resolved_path: + '/api/0/x/?dataset=spans&field=id&field=title&per_page=100&referrer=api&sort=-timestamp', + }) + ) + ).toBe('dataset:spans'); + }); + + it('sorts a flat query least → most specific (scope, attribute, id)', () => { + expect( + callRecordInputQuery( + apiRecord({ + resolved_path: + '/api/0/organizations/sentry/events/?dataset=spans&project=ml-service&query=ai_conversation.id%3A28193042%20span.description%3ADSL', + }) + ) + ).toBe( + 'dataset:spans project:ml-service span.description:DSL ai_conversation.id:28193042' + ); + }); + + it('preserves order when the query uses boolean/paren grouping', () => { + // Grouping makes order meaningful, so it must not be reordered by specificity. + expect( + callRecordInputQuery( + apiRecord({ + resolved_path: + '/api/0/x/?query=(ai_conversation.id%3A28193042%20OR%20dataset%3Aspans)', + }) + ) + ).toBe('(ai_conversation.id:28193042 OR dataset:spans)'); + }); +}); + describe('live call rendering', () => { function liveBlock(overrides?: Partial): Block { return { diff --git a/static/app/views/seerExplorer/components/chat/responseGroup.spec.tsx b/static/app/views/seerExplorer/components/chat/responseGroup.spec.tsx new file mode 100644 index 000000000000..eff3e4dc937b --- /dev/null +++ b/static/app/views/seerExplorer/components/chat/responseGroup.spec.tsx @@ -0,0 +1,171 @@ +import {OrganizationFixture} from 'sentry-fixture/organization'; + +import {render, screen, userEvent} from 'sentry-test/reactTestingLibrary'; + +import type {Block} from 'sentry/views/seerExplorer/types'; + +import {groupTranscript, deriveThinkingTitle, ResponseGroup} from './responseGroup'; + +function userBlock(id: string, content: string): Block { + return { + id, + message: {role: 'user', content}, + timestamp: '2024-01-01T00:00:00Z', + loading: false, + }; +} + +function toolUseBlock( + id: string, + overrides?: Partial & {loading?: boolean} +): Block { + const {loading = false, ...message} = overrides ?? {}; + return { + id, + message: { + role: 'tool_use', + content: null, + tool_calls: [{id: `${id}-call`, function: 'telemetry_live_search', args: '{}'}], + ...message, + }, + timestamp: '2024-01-01T00:01:00Z', + loading, + tool_results: [ + { + tool_call_id: `${id}-call`, + tool_call_function: 'telemetry_live_search', + content: '{}', + }, + ], + tool_links: [{kind: 'telemetry_live_search', params: {}}], + }; +} + +function assistantBlock(id: string, content: string, loading = false): Block { + return { + id, + message: {role: 'assistant', content, tool_calls: null}, + timestamp: '2024-01-01T00:02:00Z', + loading, + }; +} + +describe('groupTranscript', () => { + it('keeps user blocks as their own segments', () => { + const segments = groupTranscript([userBlock('u1', 'hi')]); + expect(segments).toEqual([{kind: 'user', block: expect.anything(), index: 0}]); + }); + + it('groups a run of tool_use + assistant blocks after a user block into one response', () => { + const blocks = [ + userBlock('u1', 'hi'), + toolUseBlock('t1'), + toolUseBlock('t2'), + assistantBlock('a1', 'the answer'), + ]; + + const segments = groupTranscript(blocks); + + expect(segments).toHaveLength(2); + expect(segments[0]!.kind).toBe('user'); + const response = segments[1]!; + expect(response.kind).toBe('response'); + expect(response.kind === 'response' && response.indices).toEqual([1, 2, 3]); + }); + + it('starts a new response after each user block', () => { + const blocks = [ + userBlock('u1', 'q1'), + assistantBlock('a1', 'a1'), + userBlock('u2', 'q2'), + toolUseBlock('t1'), + assistantBlock('a2', 'a2'), + ]; + + const segments = groupTranscript(blocks); + + expect(segments.map(s => s.kind)).toEqual(['user', 'response', 'user', 'response']); + }); +}); + +describe('deriveThinkingTitle', () => { + it('summarizes the response with the latest tool activity', () => { + const group = [toolUseBlock('t1'), assistantBlock('a1', 'answer')]; + // telemetry_live_search settles to "Queried spans" (see getToolsStringFromBlock). + expect(deriveThinkingTitle(group)).toMatch(/Queried spans/); + }); + + it('falls back to "Thinking" before any tool has run', () => { + expect(deriveThinkingTitle([assistantBlock('a1', 'answer')])).toBe('Thinking'); + }); +}); + +describe('ResponseGroup', () => { + const organization = OrganizationFixture(); + + it('renders a single reasoning block titled by the latest activity, answer outside it', () => { + const group = [ + toolUseBlock('t1'), + toolUseBlock('t2'), + assistantBlock('a1', 'The final answer'), + ]; + + render(, { + organization, + }); + + // One consolidated reasoning toggle for the whole response, titled by the latest activity. + expect(screen.getByRole('button', {name: /Queried spans/})).toBeInTheDocument(); + // The final answer is hoisted out of the collapsible reasoning. + expect(screen.getByText('The final answer')).toBeInTheDocument(); + }); + + it('collapses the reasoning until it is expanded', async () => { + const group = [ + toolUseBlock('t1', {thinking_content: 'my private reasoning'}), + assistantBlock('a1', 'Done'), + ]; + + render(, { + organization, + }); + + // A completed response's reasoning starts collapsed, so the thinking prose is hidden. + expect(screen.getByText('my private reasoning')).not.toBeVisible(); + + await userEvent.click(screen.getByRole('button', {name: /Queried spans/})); + + expect(screen.getByText('my private reasoning')).toBeVisible(); + }); + + it('renders no reasoning block when the response is a direct answer', () => { + const group = [assistantBlock('a1', 'Just an answer')]; + + render(, { + organization, + }); + + expect( + screen.queryByRole('button', {name: /Thinking|Queried/}) + ).not.toBeInTheDocument(); + expect(screen.getByText('Just an answer')).toBeInTheDocument(); + }); + + it('gates thinking prose on the showThinking toggle but keeps tool calls', async () => { + const group = [ + toolUseBlock('t1', {thinking_content: 'my private reasoning'}), + assistantBlock('a1', 'Answer'), + ]; + + render( + , + {organization} + ); + + await userEvent.click(screen.getByRole('button', {name: /Queried spans/})); + + expect(screen.queryByText('my private reasoning')).not.toBeInTheDocument(); + // The tool call row still renders (as its own link), just without the reasoning prose. + expect(screen.getByRole('link', {name: /Queried spans/})).toBeInTheDocument(); + }); +}); diff --git a/static/app/views/seerExplorer/components/chat/responseGroup.tsx b/static/app/views/seerExplorer/components/chat/responseGroup.tsx new file mode 100644 index 000000000000..7b9c7236826f --- /dev/null +++ b/static/app/views/seerExplorer/components/chat/responseGroup.tsx @@ -0,0 +1,267 @@ +import {Fragment} from 'react'; +import styled from '@emotion/styled'; +import {motion} from 'framer-motion'; + +import {MessageRow, ThinkingBlock} from '@sentry/scraps/chat'; +import {Container} from '@sentry/scraps/layout'; + +import {SeerMarkdown} from 'sentry/components/seer/markdown'; +import {AgentWriteApprovalProvider} from 'sentry/components/seer/markdown/embeds/components/agentWriteApproval'; +import {t} from 'sentry/locale'; +import {callRecordLabel, visibleCallRecords} from 'sentry/views/seerExplorer/callRecords'; +import type { + Block, + PendingUserInput, + SeerExplorerRunId, +} from 'sentry/views/seerExplorer/types'; +import {getToolsStringFromBlock} from 'sentry/views/seerExplorer/utils'; + +import {AssistantBlock} from './assistant'; +import {hasValidContent} from './shared'; +import {CODE_MODE_TOOLS, ToolCallList} from './toolUse'; + +/** + * One assistant response: a run of consecutive `assistant`/`tool_use` blocks that follows a user + * message. The server emits a turn as many blocks (a `tool_use` block per reasoning+tool step, then + * a terminating `assistant` block with the answer), so grouping them here is what lets a whole + * response collapse into a single `ThinkingBlock` instead of one row per step. + */ +interface ResponseSegment { + blocks: Block[]; + /** Indices into the original flat block array, for stable keys and ref bookkeeping. */ + indices: number[]; + kind: 'response'; +} + +interface UserSegment { + block: Block; + index: number; + kind: 'user'; +} + +export type TranscriptSegment = ResponseSegment | UserSegment; + +/** + * Partition the flat block list into user messages and assistant responses. + * + * A user block is its own segment; every maximal run of `assistant`/`tool_use` blocks after it is + * one response. This mirrors how the run itself is streamed — see `useSeerExplorer`'s + * `serverHasResponse`, which treats either role as "the assistant has started responding". + */ +export function groupTranscript(blocks: Block[]): TranscriptSegment[] { + const segments: TranscriptSegment[] = []; + let current: ResponseSegment | null = null; + + blocks.forEach((block, index) => { + if (block.message.role === 'user') { + current = null; + segments.push({kind: 'user', block, index}); + return; + } + if (current) { + current.blocks.push(block); + current.indices.push(index); + return; + } + current = {kind: 'response', blocks: [block], indices: [index]}; + segments.push(current); + }); + + return segments; +} + +/** + * The terminal answer of a response, if any: the last block, when it is an `assistant` block that + * carries real content. Its reasoning still belongs in the ThinkingBlock; only its content is + * hoisted out as the visible answer. + */ +function finalAnswer(group: Block[]): Block | null { + const last = group[group.length - 1]; + return last?.message.role === 'assistant' && hasValidContent(last.message.content) + ? last + : null; +} + +/** + * The most recent user-facing action within a block, or null when it did nothing worth naming. + * + * Prefers the Code Mode call records (their labels are what the rows show), then falls back to a + * classic tool's label — skipping Code Mode's own tool names, which name nothing ("Used + * sentry_api_execute tool"). Deliberately never reads `thinking_content`: the title is visible even + * when the reasoning is toggled off, so it must not leak it. + */ +function latestBlockActivity(block: Block): string | null { + const finished = (block.tool_results ?? []).flatMap( + result => result?.structuredContent?.calls ?? [] + ); + const records = visibleCallRecords( + finished.length ? finished : (block.live_calls ?? []) + ); + for (let i = records.length - 1; i >= 0; i--) { + const label = callRecordLabel(records[i]!); + if (label) { + return label; + } + } + + const calls = block.message.tool_calls ?? []; + const labels = getToolsStringFromBlock(block); + for (let i = labels.length - 1; i >= 0; i--) { + if (labels[i] && !CODE_MODE_TOOLS.has(calls[i]?.function ?? '')) { + return labels[i]!; + } + } + + return null; +} + +/** + * A live summary for the response's ThinkingBlock: the latest thing the agent did (the current tool + * while streaming), which updates step to step so `ThinkingBlock`'s decode animation replays. Falls + * back to a plain "Thinking" before any tool has run. + */ +export function deriveThinkingTitle(group: Block[]): string { + for (let i = group.length - 1; i >= 0; i--) { + const label = latestBlockActivity(group[i]!); + if (label) { + return label; + } + } + return t('Thinking'); +} + +interface ResponseGroupProps { + blockIndex: number; + group: Block[]; + blocks?: Block[]; + getPageReferrer?: () => string; + interactionPending?: boolean; + pendingInput?: PendingUserInput | null; + readOnly?: boolean; + respondToUserInput?: (inputId: string, responseData?: Record) => void; + runId?: SeerExplorerRunId; + showThinking?: boolean; +} + +/** + * Renders one assistant response as a single top-level `ThinkingBlock` — reasoning, intermediate + * narration, and every tool call interleaved in run order inside it — followed by the final answer + * as a sibling. Replaces the previous one-row-per-block rendering that produced a wall of separate + * "Thinking" and tool-call rows for a single turn. + */ +export function ResponseGroup({ + group, + blockIndex, + blocks, + getPageReferrer, + interactionPending, + pendingInput, + readOnly, + respondToUserInput, + runId, + showThinking, +}: ResponseGroupProps) { + const answer = finalAnswer(group); + const active = group.some(block => block.loading); + + // The reasoning trace is everything except the answer's content: thinking prose (gated on the + // `showThinking` toggle), any intermediate narration, and the tool calls. + const hasTrace = group.some(block => { + const isAnswer = block === answer; + return ( + (showThinking && hasValidContent(block.message.thinking_content)) || + (!isAnswer && hasValidContent(block.message.content)) || + Boolean(block.message.tool_calls?.length) + ); + }); + + const startTime = new Date(group[0]!.timestamp); + const endTime = active ? undefined : new Date(group[group.length - 1]!.timestamp); + + return ( + + + + {hasTrace ? ( + + + {group.map((block, i) => { + const isAnswer = block === answer; + // A block's own tool calls render after its thinking, so they count as "after"; + // "before" is an earlier block's tool calls. Thinking that is flanked on both + // sides gets extra breathing room to set it apart; leading/trailing thinking does + // not, so it stays tight against the answer or the block edge. + const toolCallBefore = group + .slice(0, i) + .some(b => Boolean(b.message.tool_calls?.length)); + const toolCallAtOrAfter = group + .slice(i) + .some(b => Boolean(b.message.tool_calls?.length)); + const thinkingBetweenToolCalls = toolCallBefore && toolCallAtOrAfter; + return ( + + {showThinking && + hasValidContent(block.message.thinking_content) && ( + + + + )} + {!isAnswer && hasValidContent(block.message.content) && ( + + )} + {block.message.tool_calls ? ( + + ) : null} + + ); + })} + + + ) : null} + + {answer ? ( + + ) : null} + + + + ); +} + +// The response's raw reasoning. When it sits between tool calls it is set apart with extra vertical +// space (`data-spaced`); leading or trailing reasoning gets none so it stays tight against the +// answer or the block edge. +const ThinkingProse = styled('div')` + min-width: 0; + font-family: ${p => p.theme.font.family.sans}; + font-size: ${p => p.theme.font.size.sm}; + + &[data-spaced='true'] { + padding-block: ${p => p.theme.space.lg}; + } + + & > :first-child { + margin-top: 0; + } + & > :last-child { + margin-bottom: 0; + } +`; diff --git a/static/app/views/seerExplorer/components/chat/toolUse.spec.tsx b/static/app/views/seerExplorer/components/chat/toolUse.spec.tsx index 7e0bf531bef2..7578c188e3c8 100644 --- a/static/app/views/seerExplorer/components/chat/toolUse.spec.tsx +++ b/static/app/views/seerExplorer/components/chat/toolUse.spec.tsx @@ -991,24 +991,37 @@ describe('ToolUseBlock', () => { ); }); - it('keeps the request detail collapsed until the row is expanded', async () => { - const block = codeModeCallsBlock([issueCall]); + it('decomposes the request query into inline input chips, no disclosure', () => { + const block = codeModeCallsBlock([ + { + ...issueCall, + path: '/api/0/organizations/{organization_id_or_slug}/events/', + resolved_path: + '/api/0/organizations/test-org/events/?dataset=spans&project=ml-service', + title: 'Query spans', + }, + ]); render(); - const detail = screen.getByText('GET /api/0/organizations/test-org/issues/123/'); - expect(detail).not.toBeVisible(); - - await userEvent.click(screen.getByRole('button', {name: /Retrieve an issue/})); - - expect(detail).toBeVisible(); + // A tool call is not a disclosure: the request reads as always-visible input chips rather + // than a raw line hidden behind an expand toggle on the title. + expect(screen.queryByRole('button', {name: /Query spans/})).not.toBeInTheDocument(); + expect(screen.getByText('Input:')).toBeInTheDocument(); + expect(screen.getByText('dataset')).toBeInTheDocument(); + expect(screen.getByText('spans')).toBeInTheDocument(); + expect(screen.getByText('project')).toBeInTheDocument(); + expect(screen.getByText('ml-service')).toBeInTheDocument(); }); - it('surfaces a failed call through a notification', () => { + it('shows the HTTP status code in the trailing chip and the error under Output', () => { const block = codeModeCallsBlock([ {...issueCall, status: 500, title: 'Retrieve an issue'}, ]); render(); + // Status code trails the title; the error prints under Output, mirroring Input. + expect(screen.getByText('500')).toBeInTheDocument(); + expect(screen.getByText('Output:')).toBeInTheDocument(); expect(screen.getByText('Returned HTTP 500')).toBeInTheDocument(); }); }); diff --git a/static/app/views/seerExplorer/components/chat/toolUse.tsx b/static/app/views/seerExplorer/components/chat/toolUse.tsx index 6c10d968cbbd..5929d905113c 100644 --- a/static/app/views/seerExplorer/components/chat/toolUse.tsx +++ b/static/app/views/seerExplorer/components/chat/toolUse.tsx @@ -16,6 +16,7 @@ import {Link} from '@sentry/scraps/link'; import {Text} from '@sentry/scraps/text'; import {Tooltip} from '@sentry/scraps/tooltip'; +import {ProvidedFormattedQuery} from 'sentry/components/searchQueryBuilder/formattedQuery'; import {SeerMarkdown} from 'sentry/components/seer/markdown'; import {AgentWriteApprovalProvider} from 'sentry/components/seer/markdown/embeds/components/agentWriteApproval'; import {IconLink} from 'sentry/icons'; @@ -26,6 +27,7 @@ import {useProjects} from 'sentry/utils/useProjects'; import { callRecordDetail, callRecordFailure, + callRecordInputQuery, callRecordLabel, callRecordStatus, visibleCallRecords, @@ -58,7 +60,7 @@ const LINK_STATUS_PARAMS = new Set(['is_error', 'empty_results']); // Code Mode's tool names cover every action it can take, so "Used sentry_api_execute tool" names // nothing. These rows are built from the calls the execute reported instead; the tool's own label // is never rendered, and a call that produced nothing to show renders no row at all. -const CODE_MODE_TOOLS = new Set(['sentry_api_execute', 'sentry_api_search']); +export const CODE_MODE_TOOLS = new Set(['sentry_api_execute', 'sentry_api_search']); // Identity for deduping a bus link against the positional row link. Params are sorted so the key // does not depend on object key order — today both channels derive params from the same object, but @@ -254,7 +256,7 @@ interface ToolCallListProps { getPageReferrer?: () => string; } -function ToolCallList({block, blocks, getPageReferrer}: ToolCallListProps) { +export function ToolCallList({block, blocks, getPageReferrer}: ToolCallListProps) { const { sortedToolLinks, toolCallToLinkIndexMap, @@ -467,9 +469,7 @@ function ToolCallList({block, blocks, getPageReferrer}: ToolCallListProps) { rows.push(); } return rows.map((row, rowIdx) => ( - - {row} - + {row} )); })} @@ -546,9 +546,10 @@ function CallRow({ * markdown surface stay identical. * * The record's label becomes the title, its outcome the leading glyph, its navigable resource a - * trailing link chip (a real anchor, so middle/cmd-click still work), and any transport failure a - * notification line. The request it ran — and its bounded response body — hangs off the detail slot - * below the title. + * trailing link chip (a real anchor, so middle/cmd-click still work). On failure the HTTP status + * code trails the title and the error prints under `Output:`. The request it ran is decomposed into + * query chips under `Input:` (rendered by the shared `FormattedQuery`), and its bounded response + * body (when there is one) sits inline below. */ function CodeModeCallRow({ record, @@ -565,13 +566,28 @@ function CodeModeCallRow({ url: LocationDescriptor | null; onLinkClick?: (e: React.MouseEvent) => void; }) { - const detail = callRecordDetail(record); + const status = callRecordStatus(record, settled); + const inputQuery = callRecordInputQuery(record); + const body = callRecordDetail(record)?.body; const failure = callRecordFailure(record); + const isFailure = status === 'failure'; return ( : undefined} + // The error prints under `Output:`, mirroring the request under `Input:`. + output={ + isFailure && failure ? ( + + {failure} + + ) : undefined + } reference={ url ? { @@ -582,33 +598,31 @@ function CodeModeCallRow({ } : undefined } - notifications={failure ? [failure] : undefined} > - {detail ? : null} + {body ? {body} : null} ); } -/** - * What the call actually ran, and what came back. - * - * Lives inside the `ToolCall`'s own collapsible panel, so it does not wrap itself in another - * disclosure: the request line summarizes the call and the bounded response body (when there is - * one) sits beneath it. - */ -function RequestDetail({ - detail, -}: { - detail: NonNullable>; -}) { - return ( - - - {detail.request} - - {detail.body ? {detail.body} : null} - - ); +// One entry per link kind buildToolLinkUrl can resolve. A kind absent here is not rendered at all +// (see navLinkLabel): showing the raw kind would leak an internal function name like +// `get_log_attributes` as the visible link text. Keeping this in step with buildToolLinkUrl's cases +// is enforced by a test, so a kind seer starts emitting cannot reach users unlabeled. +const NAV_LINK_LABELS: Record = { + get_issue_details: t('View issue'), + get_trace_waterfall: t('View trace'), + get_replay_details: t('View replay'), + get_profile_flamegraph: t('View profile'), + get_event_details: t('View event'), + get_log_attributes: t('View logs'), + get_metric_attributes: t('View metrics'), + // Dataset-dependent (issues / errors / spans / logs), so the label stays neutral. + telemetry_live_search: t('View results'), +}; + +/** The visible label for a bus link, or undefined when the kind is not renderable. */ +function navLinkLabel(kind: string): string | undefined { + return NAV_LINK_LABELS[kind]; } /** @@ -734,3 +748,15 @@ const ToolCallPlainRow = styled('span')` gap: ${p => p.theme.space.md}; max-width: 100%; `; + +// One nested row inside the response's ThinkingBlock. Unlike a full message turn it carries no +// inline gutter of its own — the ThinkingBlock's panel already provides it, so a second one here +// would push tool calls in and let their chips overrun the panel edge instead of wrapping. Only +// vertical rhythm is kept, and `min-width: 0` lets the row shrink so its chips wrap. +const NestedRow = styled('div')` + display: flex; + align-items: flex-start; + width: 100%; + min-width: 0; + padding-block: ${p => p.theme.space.sm}; +`; diff --git a/static/app/views/seerExplorer/components/seerExplorerContent.tsx b/static/app/views/seerExplorer/components/seerExplorerContent.tsx index 441f670d152b..cf72fddb4139 100644 --- a/static/app/views/seerExplorer/components/seerExplorerContent.tsx +++ b/static/app/views/seerExplorer/components/seerExplorerContent.tsx @@ -27,6 +27,10 @@ import { } from 'sentry/views/navigation/constants'; import {AskUserQuestionBlock} from 'sentry/views/seerExplorer/components/askUserQuestionBlock'; import {BlockComponent} from 'sentry/views/seerExplorer/components/chat'; +import { + groupTranscript, + ResponseGroup, +} from 'sentry/views/seerExplorer/components/chat/responseGroup'; import {EmptyState} from 'sentry/views/seerExplorer/components/emptyState'; import {useExplorerMenu} from 'sentry/views/seerExplorer/components/explorerMenu'; import {FileChangeApprovalBlock} from 'sentry/views/seerExplorer/components/fileChangeApprovalBlock'; @@ -37,7 +41,7 @@ import {SeerExplorerHeader} from 'sentry/views/seerExplorer/components/seerExplo import {UpdateSlackAlert} from 'sentry/views/seerExplorer/components/updateSlackAlert'; import {usePendingUserInput} from 'sentry/views/seerExplorer/hooks/usePendingUserInput'; import {useSeerExplorer} from 'sentry/views/seerExplorer/hooks/useSeerExplorer'; -import type {Block, SeerExplorerSidebarPosition} from 'sentry/views/seerExplorer/types'; +import type {SeerExplorerSidebarPosition} from 'sentry/views/seerExplorer/types'; import { getExplorerFeedbackOptions, getExplorerUrl, @@ -565,27 +569,45 @@ export function SeerExplorerContent({ /> ) : ( - {blocks.map((block: Block, index: number) => { - // For slide-in animation that runs on mount. Avoid running this twice on user blocks when blocks are hydrated. - const key = block.message.role === 'user' ? `user-${index}` : block.id; + {groupTranscript(blocks).map(segment => { + const interactionPending = + isFileApprovalPending || + isAgentWriteApprovalPending || + isQuestionPending || + showReauth; + + if (segment.kind === 'user') { + // For slide-in animation that runs on mount. Avoid running this twice on user + // blocks when blocks are hydrated. + return ( + { + blockRefs.current[segment.index] = el; + }} + block={segment.block} + blockIndex={segment.index} + blocks={blocks} + runId={runId ?? undefined} + getPageReferrer={getPageReferrer} + interactionPending={interactionPending} + pendingInput={pendingInput} + readOnly={readOnly} + respondToUserInput={respondToUserInput} + showThinking={showThinking} + /> + ); + } return ( - { - blockRefs.current[index] = el; - }} - block={block} - blockIndex={index} +