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
64 changes: 18 additions & 46 deletions static/app/views/investigations/detail/cell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -104,11 +104,6 @@ export function InvestigationCell({
block.title ||
chartTitle ||
(block.kind === 'query' ? t('Untitled query') : t('Untitled cell'));
const rerunMutation = useRunInvestigationBlockMutation(
organizationSlug,
investigation.id,
{onError: () => addErrorMessage(t('Unable to rerun this cell.'))}
);
const deleteMutation = useDeleteInvestigationBlockMutation(
organizationSlug,
investigation.id,
Expand Down Expand Up @@ -136,36 +131,7 @@ export function InvestigationCell({
setPrompt(block.outputStatus === 'notRun' ? block.generationPrompt : '');
}

async function rerun() {
try {
const execution = await rerunMutation.mutateAsync({
block,
investigationVersion: investigation.version,
});
setPanelOpen(true);
setTraceExecutionId(execution.id);
setShowPrompt(false);
autoOpenedExecutionId.current = execution.id;
} catch {
// The mutation owns user-facing error handling.
}
}

const actionItems: MenuItemProps[] = [];
if (block.kind === 'query') {
// oxlint-disable-next-line react/refs
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(
const actionItems: MenuItemProps[] = [
{
key: 'refine',
label: t('Refine'),
Expand All @@ -191,8 +157,8 @@ export function InvestigationCell({
investigationVersion: investigation.version,
}),
}),
}
);
},
];

const cellActions = (
<CellActions flexShrink={0}>
Expand Down Expand Up @@ -307,7 +273,7 @@ function QueryResult({
block: InvestigationBlock;
progressState: CellProgressState;
}) {
const [expanded, setExpanded] = useState(block.config.autoRun !== true);
const [expanded, setExpanded] = useState(true);
const output = getQueryOutput(block.output);
const chart =
output?.preferredView === 'chart' ? getRenderableChart(output.chart) : null;
Expand Down Expand Up @@ -466,13 +432,18 @@ function getCellProgressState(
return 'waiting';
}

export function shouldDisplayInvestigationBlock(
block: InvestigationBlock,
blocks: InvestigationBlock[]
) {
// 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 shouldDisplayInvestigationBlock(block: InvestigationBlock) {
if (block.kind === 'text') {
return Boolean((getTextOutput(block.output) ?? block.content).trim());
}
Comment on lines +435 to +438

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.

Bug: In shouldDisplayInvestigationBlock, an empty markdown output ('') incorrectly hides a text block, even if it has user-written content, due to improper use of the nullish coalescing operator.
Severity: MEDIUM

Suggested Fix

Modify the logic in shouldDisplayInvestigationBlock to correctly handle an empty string from getTextOutput. Change the condition to use a logical OR (||) instead of nullish coalescing (??), like (getTextOutput(block.output) || block.content).trim(). This will ensure that if getTextOutput returns a falsy empty string, it correctly falls back to block.content.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: static/app/views/investigations/detail/cell.tsx#L435-L438

Potential issue: The `shouldDisplayInvestigationBlock` function determines if a text
block should be visible. It uses the expression `(getTextOutput(block.output) ??
block.content).trim()`. The `getTextOutput` function can return an empty string (`''`)
if the API response contains `markdown: ''`. Because the nullish coalescing operator
(`??`) only provides a fallback for `null` or `undefined`, the expression evaluates to
`''` instead of falling back to `block.content`. Consequently, `Boolean(''.trim())`
becomes `false`, and the block is hidden from view, even if `block.content` contains
user-written text that should be displayed.

Also affects:

  • static/app/views/investigations/detail/cell.tsx:273~279

Did we get this right? 👍 / 👎 to inform future reviews.

const output = getQueryOutput(block.output);
if (!output || output.isEmpty) {
return false;
}
return Boolean(
output.tableMarkdown.trim() ||
(output.preferredView === 'chart' && getRenderableChart(output.chart))
);

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.

Awaiting-input cells stay hidden

Medium Severity

shouldDisplayInvestigationBlock only reveals cells that already have persisted text, a table, or a chart. A first-run cell in awaiting_input has none of those, so InvestigationCell never mounts and the pending-question UI cannot appear. The run stays blocked with no way to answer, while polling continues.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d379dcb. Configure here.

}

export function shouldPollInvestigationBlocks(blocks: InvestigationBlock[]) {
Expand Down Expand Up @@ -1090,7 +1061,7 @@ function getTextOutput(output: unknown): string | null {

type RenderableQueryOutput = Pick<
InvestigationQueryOutput,
'chart' | 'preferredView' | 'tableMarkdown'
'chart' | 'preferredView' | 'tableMarkdown' | 'isEmpty'
>;

function getQueryOutput(output: unknown): RenderableQueryOutput | null {
Expand All @@ -1110,6 +1081,7 @@ function getQueryOutput(output: unknown): RenderableQueryOutput | null {
: null;
return {
chart,
isEmpty: 'isEmpty' in output && output.isEmpty === true,
preferredView: output.preferredView,
tableMarkdown: output.tableMarkdown,
};
Expand Down
Loading
Loading