Skip to content

feat(code): inbox toolbar, artefact parsing, and pending_input UX #2073

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.

Sign up for GitHub

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

Open
Twixes wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(code): inbox toolbar, artefact parsing, and pending_input UX #2073

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
116 changes: 95 additions & 21 deletions apps/code/src/renderer/api/posthogClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,17 @@ function optionalString(value: unknown): string | null {
return typeof value === "string" ? value : null;
}

/** Accepts string ids; some serializers may emit other primitives in edge cases. */
function optionalArtefactId(value: unknown): string | null {
if (typeof value === "string" && value.length > 0) {
return value;
}
if (typeof value === "number" && Number.isFinite(value)) {
return String(value);
}
return null;
}

type AnyArtefact =
| SignalReportArtefact
| PriorityJudgmentArtefact
Expand All @@ -269,13 +280,17 @@ const PRIORITY_VALUES = new Set(["P0", "P1", "P2", "P3", "P4"]);
function normalizePriorityJudgmentArtefact(
value: Record<string, unknown>,
): PriorityJudgmentArtefact | null {
const id = optionalString(value.id);
const id = optionalArtefactId(value.id);
if (!id) return null;

const contentValue = isObjectRecord(value.content) ? value.content : null;
if (!contentValue) return null;

const priority = optionalString(contentValue.priority);
const rawPriority = optionalString(contentValue.priority);
const priority =
rawPriority && /^p[0-4]$/i.test(rawPriority)
? rawPriority.toUpperCase()
: rawPriority;
if (!priority || !PRIORITY_VALUES.has(priority)) return null;

return {
Expand All @@ -298,7 +313,7 @@ const ACTIONABILITY_VALUES = new Set([
function normalizeActionabilityJudgmentArtefact(
value: Record<string, unknown>,
): ActionabilityJudgmentArtefact | null {
const id = optionalString(value.id);
const id = optionalArtefactId(value.id);
if (!id) return null;

const contentValue = isObjectRecord(value.content) ? value.content : null;
Expand Down Expand Up @@ -329,7 +344,7 @@ function normalizeActionabilityJudgmentArtefact(
function normalizeSignalFindingArtefact(
value: Record<string, unknown>,
): SignalFindingArtefact | null {
const id = optionalString(value.id);
const id = optionalArtefactId(value.id);
if (!id) return null;

const contentValue = isObjectRecord(value.content) ? value.content : null;
Expand Down Expand Up @@ -383,7 +398,27 @@ function normalizeSignalReportArtefact(value: unknown): AnyArtefact | null {
return normalizePriorityJudgmentArtefact(value);
}

const id = optionalString(value.id);
// Infer structured artefacts when `type` is missing or does not match the API
// (shape matches; enums are still validated inside each normalizer).
const contentForInfer = isObjectRecord(value.content) ? value.content : null;
if (contentForInfer) {
if (optionalString(contentForInfer.signal_id)) {
const inferredFinding = normalizeSignalFindingArtefact(value);
if (inferredFinding) {
return inferredFinding;
}
}
const inferredPriority = normalizePriorityJudgmentArtefact(value);
if (inferredPriority) {
return inferredPriority;
}
const inferredActionability = normalizeActionabilityJudgmentArtefact(value);
if (inferredActionability) {
return inferredActionability;
}
}
Comment on lines +401 to +419
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot May 6, 2026

Choose a reason for hiding this comment

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

P2 Inference runs for known non-structured types

The inference block is entered whenever dispatchType is not one of the three explicitly dispatched values, which includes "video_segment" and "suggested_reviewers". If a video_segment artefact's content object happens to contain a priority field matching P0–P4 (or an actionability field), it will be silently reclassified as a priority_judgment / actionability_judgment before the video_segment-specific path at line 443 is ever reached. The comment says "type is missing or does not match the API", so the guard should scope the inference to dispatchType === null (or an explicit allowlist of drifted-type values) rather than letting it run for all unmatched strings.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: apps/code/src/renderer/api/posthogClient.ts
Line: 401-419

Comment:
**Inference runs for known non-structured types**

The inference block is entered whenever `dispatchType` is not one of the three explicitly dispatched values, which includes `"video_segment"` and `"suggested_reviewers"`. If a `video_segment` artefact's content object happens to contain a `priority` field matching `P0–P4` (or an `actionability` field), it will be silently reclassified as a `priority_judgment` / `actionability_judgment` before the `video_segment`-specific path at line 443 is ever reached. The comment says "type is missing or does not match the API", so the guard should scope the inference to `dispatchType === null` (or an explicit allowlist of drifted-type values) rather than letting it run for all unmatched strings.

How can I resolve this? If you propose a fix, please make it concise.


const id = optionalArtefactId(value.id);
if (!id) {
return null;
}
Expand All @@ -393,13 +428,16 @@ function normalizeSignalReportArtefact(value: unknown): AnyArtefact | null {
optionalString(value.created_at) ?? new Date(0).toISOString();

// suggested_reviewers: content is an array of reviewer objects
if (type === "suggested_reviewers" && Array.isArray(value.content)) {
return {
id,
type: "suggested_reviewers" as const,
created_at,
content: value.content as SuggestedReviewersArtefact["content"],
};
if (type === "suggested_reviewers") {
if (Array.isArray(value.content)) {
return {
id,
type: "suggested_reviewers" as const,
created_at,
content: value.content as SuggestedReviewersArtefact["content"],
};
}
return null;
}

// video_segment and other artefacts with object content
Expand All @@ -413,7 +451,22 @@ function normalizeSignalReportArtefact(value: unknown): AnyArtefact | null {

// The backend may return empty content objects when binary decode fails.
if (!content && !sessionId) {
return null;
return {
id,
type,
created_at,
content: {
session_id: "",
start_time: optionalString(contentValue.start_time) ?? "",
end_time: optionalString(contentValue.end_time) ?? "",
distinct_id: optionalString(contentValue.distinct_id) ?? "",
content: "",
distance_to_centroid:
typeof contentValue.distance_to_centroid === "number"
? contentValue.distance_to_centroid
: null,
},
};
}
Comment on lines 452 to 470
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot May 6, 2026

Choose a reason for hiding this comment

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

P2 Placeholder type may not be video_segment

When the binary-decode fallback fires, the returned object uses type set to whatever dispatchType ?? "unknown" resolved to. Any consumer that switches on type to choose a renderer (e.g. a video_segment card) will not match if type is "unknown" or an unexpected string, so the placeholder content shape (session_id, start_time, …) would be rendered by a catch-all/unknown branch instead of the video segment UI, potentially blanking the section the PR description says it should preserve. Hardcoding type: "video_segment" as const in the placeholder (or only entering this branch when dispatchType === "video_segment") would make the intent explicit.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: apps/code/src/renderer/api/posthogClient.ts
Line: 452-470

Comment:
**Placeholder type may not be `video_segment`**

When the binary-decode fallback fires, the returned object uses `type` set to whatever `dispatchType ?? "unknown"` resolved to. Any consumer that switches on `type` to choose a renderer (e.g. a `video_segment` card) will not match if `type` is `"unknown"` or an unexpected string, so the placeholder content shape (`session_id`, `start_time`, …) would be rendered by a catch-all/unknown branch instead of the video segment UI, potentially blanking the section the PR description says it should preserve. Hardcoding `type: "video_segment" as const` in the placeholder (or only entering this branch when `dispatchType === "video_segment"`) would make the intent explicit.

How can I resolve this? If you propose a fix, please make it concise.


return {
Expand All @@ -436,6 +489,7 @@ function normalizeSignalReportArtefact(value: unknown): AnyArtefact | null {

function parseSignalReportArtefactsPayload(
value: unknown,
debugContext?: { teamId: number; reportId: string },
): SignalReportArtefactsResponse {
const payload = isObjectRecord(value) ? value : null;
const rawResults = Array.isArray(payload?.results)
Expand All @@ -451,6 +505,30 @@ function parseSignalReportArtefactsPayload(
typeof payload?.count === "number" ? payload.count : results.length;

if (rawResults.length > 0 && results.length === 0) {
if (debugContext) {
const sample = rawResults.slice(0, 5).map((item) => {
if (!isObjectRecord(item)) {
return { shape: typeof item };
}
const t = optionalString(item.type);
const content = item.content;
return {
type: t ?? "(missing)",
idKind: typeof item.id,
contentKind: Array.isArray(content)
? "array"
: isObjectRecord(content)
? "object"
: typeof content,
};
});
log.warn("Signal report artefacts payload did not match schema", {
teamId: debugContext.teamId,
reportId: debugContext.reportId,
rawCount: rawResults.length,
sample: sample,
});
}
return {
results: [],
count: 0,
Expand Down Expand Up @@ -1961,14 +2039,10 @@ export class PostHogAPIClient {
}

const data = (await response.json()) as unknown;
const parsed = parseSignalReportArtefactsPayload(data);

if (parsed.unavailableReason) {
log.warn("Signal report artefacts payload did not match schema", {
teamId,
reportId,
});
}
const parsed = parseSignalReportArtefactsPayload(data, {
teamId,
reportId,
});

return parsed;
} catch (error) {
Expand Down
4 changes: 2 additions & 2 deletions apps/code/src/renderer/components/ui/Tooltip.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ export function Tooltip({
side={side}
align={align}
sideOffset={sideOffset}
className="dark flex items-center gap-[8px] rounded-[6px] border border-(--gray-4) bg-(--gray-2) px-[10px] py-[6px] text-(--gray-12) text-xs leading-[1.4]"
className="dark z-[200000] flex items-center gap-[8px] rounded-[6px] border border-(--gray-4) bg-(--gray-2) px-[10px] py-[6px] text-(--gray-12) text-xs leading-[1.4]"
style={{
whiteSpace: isSimpleContent ? "nowrap" : "normal",
boxShadow: "0 4px 12px rgba(0, 0, 0, 0.25)",
zIndex: 9999,
zIndex: 200000,
Comment on lines +46 to +50
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot May 6, 2026

Choose a reason for hiding this comment

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

P2 The z-index is now specified twice with the same value — z-[200000] in the className Tailwind string and zIndex: 200000 in the style prop. Inline styles always win over utility classes, so the Tailwind token has no effect and will mislead future readers about what actually controls the stacking order.

Suggested change
className="dark z-[200000] flex items-center gap-[8px] rounded-[6px] border border-(--gray-4) bg-(--gray-2) px-[10px] py-[6px] text-(--gray-12) text-xs leading-[1.4]"
style={{
whiteSpace: isSimpleContent ? "nowrap" : "normal",
boxShadow: "0 4px 12px rgba(0, 0, 0, 0.25)",
zIndex: 9999,
zIndex: 200000,
className="dark flex items-center gap-[8px] rounded-[6px] border border-(--gray-4) bg-(--gray-2) px-[10px] py-[6px] text-(--gray-12) text-xs leading-[1.4]"
style={{
whiteSpace: isSimpleContent ? "nowrap" : "normal",
boxShadow: "0 4px 12px rgba(0, 0, 0, 0.25)",
zIndex: 200000,
Prompt To Fix With AI 
This is a comment left during a code review.
Path: apps/code/src/renderer/components/ui/Tooltip.tsx
Line: 46-50

Comment:
The z-index is now specified twice with the same value — `z-[200000]` in the `className` Tailwind string and `zIndex: 200000` in the `style` prop. Inline styles always win over utility classes, so the Tailwind token has no effect and will mislead future readers about what actually controls the stacking order.

```suggestion
            className="dark flex items-center gap-[8px] rounded-[6px] border border-(--gray-4) bg-(--gray-2) px-[10px] py-[6px] text-(--gray-12) text-xs leading-[1.4]"
            style={{
              whiteSpace: isSimpleContent ? "nowrap" : "normal",
              boxShadow: "0 4px 12px rgba(0, 0, 0, 0.25)",
              zIndex: 200000,
```

How can I resolve this? If you propose a fix, please make it concise.

animationDuration: "150ms",
animationTimingFunction: "ease-out",
willChange: "transform, opacity",
Expand Down
2 changes: 1 addition & 1 deletion apps/code/src/renderer/features/inbox/components/detail/ReportDetailPane.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ export function ReportDetailPane({ report, onClose }: ReportDetailPaneProps) {
)}

{/* ── Description ─────────────────────────────────────── */}
{report.status !== "ready" ? (
{report.status !== "ready" && report.status !== "pending_input" ? (
<Tooltip content="This is a preliminary description. A full researched summary will replace it when the research agent completes its work.">
<div className="cursor-help">
<SignalReportSummaryMarkdown
Expand Down
Loading
Loading