Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
222 changes: 142 additions & 80 deletions static/app/views/investigations/detail/cell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {TextArea} from '@sentry/scraps/textarea';

import {addErrorMessage} from 'sentry/actionCreators/indicator';
import {openConfirmModal} from 'sentry/components/confirm';
import {DropdownMenu} from 'sentry/components/dropdownMenu';
import {DropdownMenu, type MenuItemProps} from 'sentry/components/dropdownMenu';
import {Duration} from 'sentry/components/duration';
import {SeerMarkdown} from 'sentry/components/seer/markdown';
import {ChartContent} from 'sentry/components/seer/markdown/embeds/components/chart';
Expand All @@ -21,7 +21,6 @@ import {
IconChevron,
IconClose,
IconEllipsis,
IconRefresh,
IconReturn,
IconSeer,
} from 'sentry/icons';
Expand Down Expand Up @@ -62,9 +61,15 @@ export function InvestigationCell({
investigation,
}: InvestigationCellProps) {
const organizationSlug = useOrganization().slug;
const [panelOpen, setPanelOpen] = useState(false);
const [traceExecutionId, setTraceExecutionId] = useState<string | null>(null);
const [showPrompt, setShowPrompt] = useState(true);
const activeExecutionId = isExecutionActive(block.currentExecution?.status)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MAJOR] — State coordination is becoming a tangled ball

This component now manages panelOpen, traceExecutionId, showPrompt, prompt (4 useState) plus autoOpenedExecutionId (1 useRef) plus a useEffect that synchronizes three of them when activeExecutionId changes.

The same three-setter pattern appears three times in this component:

  • The useEffect at L117: setPanelOpen(true); setTraceExecutionId(activeExecutionId); setShowPrompt(false)
  • openPanel() at L130: setPanelOpen(true); setTraceExecutionId(...); setShowPrompt(false)
  • rerun() at L143: setPanelOpen(true); setTraceExecutionId(execution.id); setShowPrompt(false)

These three state variables aren't independent. They form one concept: the panel's current mode. A discriminated union would collapse all three call sites to one-liners and eliminate the entire class of bugs where the values go out of sync:

type PanelState =
  | { mode: 'closed' }
  | { mode: 'prompt' }
  | { mode: 'tracing'; executionId: string };

Every three-setter call becomes setPanelState({ mode: 'tracing', executionId: x }). The RefinementPanel takes a PanelState instead of three separate prop+setter pairs.

— AI agent review

? (block.currentExecution?.id ?? null)
: null;
const autoOpenedExecutionId = useRef(activeExecutionId);
const [panelOpen, setPanelOpen] = useState(Boolean(activeExecutionId));
const [traceExecutionId, setTraceExecutionId] = useState<string | null>(
activeExecutionId
);
Comment on lines +69 to +71

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this kind of state might need to be unwound when SSE events showup, and other users are driving execution of a cell. might need to watch out.

const [showPrompt, setShowPrompt] = useState(!activeExecutionId);
const [prompt, setPrompt] = useState(() =>
block.outputStatus === 'notRun' ? block.generationPrompt : ''
);
Expand Down Expand Up @@ -108,6 +113,16 @@ export function InvestigationCell({
{onError: () => addErrorMessage(t('Unable to delete this cell.'))}
);

useEffect(() => {
if (!activeExecutionId || autoOpenedExecutionId.current === activeExecutionId) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MAJOR] — RefinementPanel prop surface is a state-management leak

Related to the state-coordination finding above. RefinementPanel takes 11 props, 6 of which are state+setter pairs lifted from the parent (prompt/setPrompt, showPrompt/setShowPrompt, traceExecutionId/setTraceExecutionId). Neither the parent nor the child owns the state — it's a two-headed state machine.

prompt and showPrompt are purely local to the panel's UX. The panel should own them internally. Pass only initialExecutionId (or the discriminated PanelState) and an onStarted(executionId) callback so the parent can track the active execution. Props drop from 11 to ~6.

This compounds with the discriminated-union opportunity: if the parent owns a PanelState, the panel receives it as a controlled value plus onChange, or the panel owns its internal state entirely.

— AI agent review

return;
}
autoOpenedExecutionId.current = activeExecutionId;
setPanelOpen(true);
setTraceExecutionId(activeExecutionId);
setShowPrompt(false);
}, [activeExecutionId]);

function openPanel() {
setPanelOpen(true);
if (block.currentExecution && isExecutionActive(block.currentExecution.status)) {
Expand All @@ -128,38 +143,56 @@ export function InvestigationCell({
setPanelOpen(true);
setTraceExecutionId(execution.id);
setShowPrompt(false);
autoOpenedExecutionId.current = execution.id;
} catch {
// The mutation owns user-facing error handling.
}
}

const refinementButton = (
<Button
size="xs"
variant="transparent"
icon={<IconSeer size="xs" />}
aria-label={t('Ask Seer about %s', displayTitle)}
disabled={waitingForDependencies}
onClick={panelOpen ? () => setPanelOpen(false) : openPanel}
/>
const actionItems: MenuItemProps[] = [];
if (block.kind === 'query') {
actionItems.push({
key: 'rerun',
label: t('Rerun'),
disabled:
!canRun ||
rerunMutation.isPending ||
isExecutionActive(block.currentExecution?.status) ||
!(block.generationPrompt || block.content).trim(),
onAction: () => void rerun(),
});
}
actionItems.push(
{
key: 'refine',
label: t('Refine'),
disabled: waitingForDependencies,
onAction: openPanel,
},
{
key: 'delete',
label: t('Delete'),
priority: 'danger',
disabled:
!canRun ||
deleteMutation.isPending ||
isExecutionActive(block.currentExecution?.status),
onAction: () =>
openConfirmModal({
message: t('Are you sure you want to delete this cell?'),
priority: 'danger',
confirmText: t('Delete'),
onConfirm: () =>
deleteMutation.mutate({
block,
investigationVersion: investigation.version,
}),
}),
}
);

const queryHeaderActions = (
<Flex align="center" gap="xs" flexShrink={0}>
{refinementButton}
<Button
size="xs"
variant="transparent"
icon={<IconRefresh size="xs" />}
aria-label={t('Rerun %s', displayTitle)}
busy={rerunMutation.isPending}
disabled={
!canRun ||
isExecutionActive(block.currentExecution?.status) ||
!(block.generationPrompt || block.content).trim()
}
onClick={() => void rerun()}
/>
const cellActions = (
<CellActions flexShrink={0}>
<DropdownMenu
position="bottom-end"
usePortal
Expand All @@ -170,30 +203,9 @@ export function InvestigationCell({
icon: <IconEllipsis size="xs" />,
'aria-label': t('Cell actions for %s', displayTitle),
}}
items={[
{
key: 'delete',
label: t('Delete'),
priority: 'danger',
disabled:
!canRun ||
deleteMutation.isPending ||
isExecutionActive(block.currentExecution?.status),
onAction: () =>
openConfirmModal({
message: t('Are you sure you want to delete this cell?'),
priority: 'danger',
confirmText: t('Delete'),
onConfirm: () =>
deleteMutation.mutate({
block,
investigationVersion: investigation.version,
}),
}),
},
]}
items={actionItems}
/>
</Flex>
</CellActions>
);

const panel = panelOpen ? (
Expand Down Expand Up @@ -223,7 +235,7 @@ export function InvestigationCell({
{block.kind === 'query' ? (
<Fragment>
<QueryResult
actions={queryHeaderActions}
actions={cellActions}
block={block}
progressState={progressState}
/>
Expand All @@ -233,8 +245,8 @@ export function InvestigationCell({
<Fragment>
<CellResult
block={block}
actions={cellActions}
progressState={progressState}
refinementButton={refinementButton}
streamedMarkdown={
isExecutionActive(block.currentExecution?.status)
? streamedTextQuery.data?.partialMarkdown
Expand All @@ -249,20 +261,20 @@ export function InvestigationCell({
}

function CellResult({
actions,
block,
progressState,
refinementButton,
streamedMarkdown,
}: {
actions: React.ReactNode;
block: InvestigationBlock;
progressState: CellProgressState;
refinementButton: React.ReactNode;
streamedMarkdown?: string | null;
}) {
const markdown =
streamedMarkdown ?? getTextOutput(block.output) ?? (block.content.trim() || null);
return (
<Stack
<CellHoverSurface
position="relative"
flex={1}
minWidth={0}
Expand All @@ -271,15 +283,15 @@ function CellResult({
data-cell-variant="unbordered"
>
<Container position="absolute" top={0} right={0}>
{refinementButton}
{actions}
</Container>
<CellExecutionAlert block={block} />
{markdown ? (
<SeerMarkdown raw={markdown} />
) : (
<CellProgress state={progressState} />
)}
</Stack>
</CellHoverSurface>
);
}

Expand All @@ -292,7 +304,7 @@ function QueryResult({
block: InvestigationBlock;
progressState: CellProgressState;
}) {
const [expanded, setExpanded] = useState(true);
const [expanded, setExpanded] = useState(block.config.autoRun !== true);
Comment thread
cursor[bot] marked this conversation as resolved.
Comment thread
arslnb marked this conversation as resolved.
const output = getQueryOutput(block.output);
const chart =
output?.preferredView === 'chart' ? getRenderableChart(output.chart) : null;
Expand All @@ -304,19 +316,22 @@ function QueryResult({
getChartMetadata(chart);

return (
<Stack width="100%" gap="sm">
<QueryDisclosureButton
size="sm"
variant="transparent"
icon={<IconChevron direction={expanded ? 'down' : 'right'} size="xs" />}
aria-label={t('Toggle %s', title)}
aria-expanded={expanded}
onClick={() => setExpanded(value => !value)}
>
<Text data-test-id="query-cell-title" size="sm" tabular>
{title}
</Text>
</QueryDisclosureButton>
<CellHoverSurface width="100%" gap="sm">
<Flex width="100%" align="center" gap="xs" data-test-id="query-cell-toolbar">
<QueryDisclosureButton
size="sm"
variant="transparent"
icon={<IconChevron direction={expanded ? 'down' : 'right'} size="xs" />}
aria-label={t('Toggle %s', title)}
aria-expanded={expanded}
onClick={() => setExpanded(value => !value)}
>
<Text data-test-id="query-cell-title" size="sm" tabular>
{title}
</Text>
</QueryDisclosureButton>
{actions}
</Flex>
{expanded ? (
<Stack
width="100%"
Expand Down Expand Up @@ -348,7 +363,6 @@ function QueryResult({
</Text>
) : null}
</Stack>
{actions}
</Flex>
<Container width="100%" overflow="hidden" padding={chart ? 'md lg' : '0'}>
<CellExecutionAlert block={block} />
Expand All @@ -364,7 +378,7 @@ function QueryResult({
</Container>
</Stack>
) : null}
</Stack>
</CellHoverSurface>
);
}

Expand Down Expand Up @@ -449,6 +463,15 @@ function getCellProgressState(
return 'waiting';
}

export function shouldDisplayInvestigationBlock(
block: InvestigationBlock,
blocks: InvestigationBlock[]
) {
Comment thread
arslnb marked this conversation as resolved.
// Waiting cells have no useful content yet. Dependency failures and cancellations
// remain visible so users can understand why downstream work stopped.
return getCellProgressState(block, blocks) !== 'waiting';
}

export function shouldPollInvestigationBlocks(blocks: InvestigationBlock[]) {
return blocks.some(
block =>
Expand Down Expand Up @@ -689,7 +712,7 @@ function RefinementPanel({

return (
<RefinementDisclosure defaultExpanded size="sm">
<Disclosure.Title
<AgentActivityDisclosureTitle
leadingItems={<IconSeer size="xs" animation={active ? 'waiting' : undefined} />}
trailingItems={
<Flex align="center" gap="sm">
Expand All @@ -704,8 +727,8 @@ function RefinementPanel({
</Flex>
}
>
<Text monospace>{getExecutionTitle(status)}</Text>
</Disclosure.Title>
<AgentActivityTitle monospace>{getExecutionTitle(status)}</AgentActivityTitle>
</AgentActivityDisclosureTitle>
<RefinementDisclosureContent>
<Stack gap="md">
<Transcript
Expand Down Expand Up @@ -1102,11 +1125,32 @@ function getSeriesName(series: {label: string} | {name: string}) {
}

const QueryDisclosureButton = styled(Button)`
width: 100%;
flex: 1;
justify-content: flex-start;
padding-inline: ${p => p.theme.space.xs};
text-align: left;
`;

const CellActions = styled(Flex)`
opacity: 0;
pointer-events: none;
`;
Comment thread
arslnb marked this conversation as resolved.

const CellHoverSurface = styled(Stack)`
&:hover ${CellActions},
&:focus-within ${CellActions} {
opacity: 1;
pointer-events: auto;
}

@media (hover: none) {
${CellActions} {
opacity: 1;
pointer-events: auto;
}
}
`;
Comment thread
cursor[bot] marked this conversation as resolved.

const QueryTable = styled('table')`
min-width: 100%;
border-collapse: collapse;
Expand All @@ -1115,6 +1159,24 @@ const QueryTable = styled('table')`
const RefinementDisclosure = styled(Disclosure)`
width: 100%;
margin-top: ${p => p.theme.space.lg};

& > div:first-child {
padding-inline: ${p => p.theme.space['2xs']};
}
`;

const AgentActivityDisclosureTitle = styled(Disclosure.Title)`
padding-inline: 0;
`;

const AgentActivityTitle = styled(Text)`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MAJOR] — cell.tsx is 1204 lines and still growing

The file was 1148 lines before this PR. It now sits at 1204 after adding new styled components (CellActions, CellHoverSurface, AgentActivityDisclosureTitle, AgentActivityTitle), a new export (shouldDisplayInvestigationBlock), and more state logic — without extracting anything.

Three natural extraction boundaries exist:

  1. RefinementPanel and children (PendingInvestigationQuestion, Transcript, isRenderableTranscriptBlock, adaptTranscriptBlock) — ~430 lines, self-contained with its own queries, mutations, and styled components.
  2. Output parsing / chart utilities (getTextOutput, getQueryOutput, getRenderableChart, getChartMetadata, getDisplayText, getSeriesName, isRecord) — ~96 lines of pure functions with zero component dependencies.
  3. Progress state logic (getCellProgressState, shouldDisplayInvestigationBlock, shouldPollInvestigationBlocks, hasFailedDependency, hasCancelledDependency, isExecutionActive) — ~80 lines of pure logic already imported by index.tsx.

Extracting RefinementPanel alone drops the file to ~770 lines. All three bring it to ~600.

— AI agent review

font-size: ${p => p.theme.font.size.sm};
font-style: normal;
font-weight: 700;
line-height: ${p => p.theme.font.lineHeight.fixed};
letter-spacing: 0;
vertical-align: middle;
font-variant-numeric: lining-nums tabular-nums;
`;

const RefinementPrompt = styled('div')`
Expand Down
Loading
Loading