Skip to content

feat(investigations): Focus active investigation progress - #122682

Merged
arslnb merged 7 commits into
masterfrom
sentry/investigations-progressive-disclosure
Aug 26, 2026
Merged

feat(investigations): Focus active investigation progress#122682
arslnb merged 7 commits into
masterfrom
sentry/investigations-progressive-disclosure

Conversation

@arslnb

@arslnb arslnb commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Investigation notebooks now focus on work that has started: auto-run cells waiting on dependencies remain hidden and appear once their execution begins. Active cells automatically open their live Seer steps, while generated query evidence starts collapsed.

Manual query cells keep their existing expanded default, and closing an active trace remains respected until a new execution starts.

@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Aug 26, 2026
@arslnb
arslnb marked this pull request as ready for review August 26, 2026 04:27
Comment thread static/app/views/investigations/detail/cell.tsx
arslnb added 2 commits August 26, 2026 10:09
Hide auto-run cells until their dependencies are ready, collapse generated query evidence by default, and automatically expose the live Seer steps for active executions.
@arslnb
arslnb force-pushed the sentry/investigations-progressive-disclosure branch from 602ba5c to e66b2d7 Compare August 26, 2026 17:33
Comment thread static/app/views/investigations/detail/cell.tsx
Comment thread static/app/views/investigations/detail/cell.tsx
Comment thread static/app/views/investigations/detail/index.tsx Outdated
Comment thread static/app/views/investigations/detail/cell.tsx

@billyvg billyvg left a comment

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.

🤖 Review posted by an AI agent on behalf of @billyvg (not the PR author's own review).

Six items below, posted inline.

Comment thread static/app/views/investigations/detail/index.tsx
Comment thread static/app/views/investigations/detail/cell.tsx Outdated
Comment thread static/app/views/investigations/detail/index.tsx
Comment thread static/app/views/investigations/detail/index.tsx Outdated
Comment thread static/app/views/investigations/detail/cell.tsx Outdated
Comment thread static/app/views/investigations/detail/cell.tsx

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit bba5f1f. Configure here.

Comment thread static/app/views/investigations/detail/index.tsx Outdated
Comment on lines +69 to +71
const [traceExecutionId, setTraceExecutionId] = useState<string | null>(
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.

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.

@arslnb
arslnb merged commit 8bf6941 into master Aug 26, 2026
72 checks passed
@arslnb
arslnb deleted the sentry/investigations-progressive-disclosure branch August 26, 2026 21:01

@billyvg billyvg left a comment

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.

🤖 Thermo-nuclear structural review posted by an AI agent on behalf of @billyvg. This is a structural quality review, not a correctness review. Findings inline.


[MAJOR] — cell.tsx is 1204 lines and growing; this PR adds more without extracting

File: cell.tsx (whole file)
Rule: Standard 1 (1000-line threshold), Standard 0 (ambitious simplification)

The file was already 1148 lines before this PR. It now sits at 1204. This PR adds new styled components (CellActions, CellHoverSurface, AgentActivityDisclosureTitle, AgentActivityTitle), a new exported function (shouldDisplayInvestigationBlock), and more state coordination logic to InvestigationCell, none of which pulled anything out.

The file contains three distinct concerns that have natural extraction boundaries:

  1. RefinementPanel and its children (RefinementPanel, PendingInvestigationQuestion, Transcript, isRenderableTranscriptBlock, adaptTranscriptBlock) — ~430 lines, self-contained with its own styled components. This is a full sub-feature with its own API queries, mutation handling, and rendering.
  2. Output parsing and chart utilities (getTextOutput, getQueryOutput, getRenderableChart, getChartMetadata, getDisplayText, getSeriesName, isRecord) — ~96 lines of pure functions with zero component dependencies. These belong in a utils.ts or output.ts file.
  3. Progress state logic (getCellProgressState, shouldDisplayInvestigationBlock, shouldPollInvestigationBlocks, hasFailedDependency, hasCancelledDependency, isInvestigationFailureExecution, isExecutionActive) — ~80 lines of pure logic that both cell.tsx and index.tsx import from.

Extracting any one of these drops the file well below 1000 lines. Extracting all three leaves cell.tsx at ~600 lines — a file whose only job is rendering cell variants.

Code judo move: Extract RefinementPanel and friends into refinementPanel.tsx. Extract the pure output/chart utilities into cellUtils.ts. The progress-state functions could go there too or into their own module since index.tsx already imports from cell.tsx for these.


[MAJOR] — InvestigationCell state coordination is becoming a tangled ball

File: cell.tsx:L63-L128
Rule: Standard 2 (spaghetti growth), Data Clumps smell

The component now juggles 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 (setPanelOpen(true); setTraceExecutionId(id); setShowPrompt(false)) appears in the useEffect, in openPanel, and in rerun. These four pieces of state are not independent — they form one concept: "the panel's current mode" (closed, showing-prompt, tracing-execution).

This is a Data Clump: panelOpen + traceExecutionId + showPrompt always change together. And the repeated three-setter calls are Shotgun Surgery within a single component.

Code judo move: Replace the three state variables with a single discriminated union:

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

Every setPanelOpen(true); setTraceExecutionId(x); setShowPrompt(false) collapses to one setPanelState({ mode: 'tracing', executionId: x }). The useEffect, openPanel, and rerun each become one-liners. The RefinementPanel takes a PanelState instead of three separate prop+setter pairs. This eliminates the entire class of bugs where the three values go out of sync.


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

File: cell.tsx:L544-L555
Rule: Standard 5 (boundary cleanliness), Data Clumps smell

RefinementPanel takes 11 props, 6 of which are state + setter pairs lifted from the parent (prompt/setPrompt, showPrompt/setShowPrompt, traceExecutionId/setTraceExecutionId). The parent creates the state; the child mutates it freely. This is state ownership with no boundary — the panel doesn't own its state and neither does the parent. It's a two-headed state machine.

This smell compounds with the discriminated-union opportunity above. If the parent owns a PanelState, the panel takes it as a controlled value plus an onChange callback, or the panel owns its own state entirely and the parent just passes executionId and onClose.

Code judo move: Let RefinementPanel own prompt and showPrompt internally (they're local to the panel's UX). Pass in only initialExecutionId and an onStarted(executionId) callback so the parent can track the active execution. Props drop from 11 to ~6.


[SUGGESTION] — formatStatus('active') → 'Completed' is a domain lie at the formatting layer

File: index.tsx:L554-L557
Rule: Standard 6 (canonical layer)

A formatting function is the wrong place to remap a domain status. If the backend's active status genuinely means "the investigation finished its initial run," the backend should send completed. If active means "cells can be rerun," the label is wrong. Either way, a formatStatus function that silently changes meaning instead of formatting a string is a layer violation — the next person reading the code will assume active displays as "Active."


[SUGGESTION] — Negative-margin bleed-out on NotebookSummaryCard is fragile

File: index.tsx:L592-L596
Rule: Standard 4 (prefer direct, boring code)

The calc(100% + 2xl) / calc(-1 * lg) / padding-inline: xl math creates a card that visually bleeds past its container. The parent InvestigationCanvas has width: min(100%, 884px) with no padding, so on narrow viewports the negative margins push outside the viewport. This is the kind of "magic layout" that breaks on the next viewport someone tests and is hard for the next developer to modify safely. A simpler approach: widen the canvas itself at this section, or use a full-bleed wrapper, rather than asking one child to break out of its parent's box model.


Verdict: REFINE

No blockers, but two structural issues worth addressing before this grows further. The file-size problem (1204 lines, three extractable concerns) is the easiest win. The state-coordination tangle (panelOpen/traceExecutionId/showPrompt always changing together) is the higher-leverage fix — a discriminated union would delete the entire class of sync bugs and shrink both the component and its child's prop surface. Neither needs to block this PR if there's urgency, but both should happen before the next feature lands in this file.

@billyvg billyvg left a comment

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.

🤖 Structural review findings posted by an AI agent on behalf of @billyvg. Three major items inline.

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

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

);

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants