Skip to content
Open
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
116 changes: 95 additions & 21 deletions apps/code/src/renderer/api/posthogClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,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 @@ -278,13 +289,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 @@ -307,7 +322,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 @@ -338,7 +353,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 @@ -428,7 +443,27 @@ function normalizeSignalReportArtefact(value: unknown): AnyArtefact | null {
return normalizeDismissalArtefact(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 +446 to +464
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.

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 @@ -438,13 +473,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 @@ -458,7 +496,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 497 to 515
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.

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 @@ -481,6 +534,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 @@ -496,6 +550,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 @@ -2040,14 +2118,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
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

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
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ export function ReportDetailPane({
)}

{/* ── 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
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { INBOX_REFETCH_INTERVAL_MS } from "@features/inbox/utils/inboxConstants"
import {
ArrowClockwiseIcon,
DotsThree,
EyeSlashIcon,
GearSixIcon,
MagnifyingGlass,
PauseIcon,
Expand Down Expand Up @@ -531,7 +530,7 @@ export function SignalsToolbar({
<InboxBulkActionButton
color="gray"
loading={isSuppressing}
icon={<EyeSlashIcon size={12} />}
icon={<ThumbsDownIcon size={12} />}
label="Suppress"
tooltipContent="Permanently suppress selected reports"
disabledReason={suppressDisabledReason}
Expand Down Expand Up @@ -603,7 +602,7 @@ export function SignalsToolbar({
<AlertDialog.Content maxWidth="420px">
<AlertDialog.Title>
<Flex align="center" gap="2">
<EyeSlashIcon size={18} />
<ThumbsDownIcon size={18} />
<Text className="font-bold">Suppress reports</Text>
</Flex>
</AlertDialog.Title>
Expand All @@ -629,7 +628,7 @@ export function SignalsToolbar({
{isSuppressing ? (
<Spinner size="1" />
) : (
<EyeSlashIcon size={14} />
<ThumbsDownIcon size={14} />
)}
Suppress
</Button>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@ export function ReportCardContent({
className="min-w-0 flex-1"
>
{prependBadges}
{!isReady && <SignalReportStatusBadge status={report.status} />}
{!(isReady || report.status === "pending_input") && (
<SignalReportStatusBadge status={report.status} />
)}
<SignalReportPriorityBadge priority={report.priority} />
<SignalReportActionabilityBadge
actionability={report.actionability}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,21 +97,21 @@ describe("filterReportsBySearch", () => {
});

describe("buildSignalReportListOrdering", () => {
it("puts status then suggested reviewer then descending field", () => {
it("puts suggested reviewer then status then descending field", () => {
expect(buildSignalReportListOrdering("total_weight", "desc")).toBe(
"status,-is_suggested_reviewer,-total_weight",
"-is_suggested_reviewer,status,-total_weight",
);
});

it("puts status then suggested reviewer then ascending field", () => {
it("puts suggested reviewer then status then ascending field", () => {
expect(buildSignalReportListOrdering("created_at", "asc")).toBe(
"status,-is_suggested_reviewer,created_at",
"-is_suggested_reviewer,status,created_at",
);
});

it("works for signal_count", () => {
expect(buildSignalReportListOrdering("signal_count", "desc")).toBe(
"status,-is_suggested_reviewer,-signal_count",
"-is_suggested_reviewer,status,-signal_count",
);
});
});
Expand Down
6 changes: 3 additions & 3 deletions apps/code/src/renderer/features/inbox/utils/filterReports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,16 +47,16 @@ export function buildStatusFilterParam(statuses: SignalReportStatus[]): string {

/**
* Comma-separated `ordering` for the signal report list API:
* 1. Status rank (ready first — semantic server-side rank, always applied)
* 2. Suggested reviewer (current user's reports first)
* 1. Suggested reviewer (current user's reports first)
* 2. Status rank (ready first — semantic server-side rank, always applied)
* 3. Toolbar-selected field (priority, total_weight, created_at, etc.)
*/
export function buildSignalReportListOrdering(
field: SignalReportOrderingField,
direction: "asc" | "desc",
): string {
const fieldKey = direction === "desc" ? `-${field}` : field;
return `status,-is_suggested_reviewer,${fieldKey}`;
return `-is_suggested_reviewer,status,${fieldKey}`;
}

export function buildSuggestedReviewerFilterParam(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@ export function InboxItem({ isActive, onClick, signalCount }: InboxItemProps) {
<Tooltip
content={
signalCount && signalCount > 0
? `${signalCount} actionable report${signalCount === 1 ? "" : "s"} assigned to you`
: "No actionable reports assigned to you yet"
? `${signalCount} auto pull request${signalCount === 1 ? "" : "s"} assigned to you`
: "No auto pull requests assigned to you yet"
}
shortcut={formatHotkey(SHORTCUTS.INBOX)}
side="right"
Expand All @@ -55,20 +55,17 @@ export function InboxItem({ isActive, onClick, signalCount }: InboxItemProps) {
<EnvelopeSimple size={16} weight={isActive ? "fill" : "regular"} />
}
label={
<>
Inbox
<span className="flex min-w-0 items-center gap-1">
<span className="min-w-0 truncate">Inbox</span>
{signalCount && signalCount > 0 ? (
<span
className="ml-2 inline-flex shrink-0 items-center justify-center rounded-full bg-(--red-9) p-1 font-medium text-[10px] leading-none"
style={{
color: "white",
}}
className="!text-[#fff] inline-flex h-[16px] min-w-[16px] shrink-0 items-center justify-center rounded-full bg-(--red-9) px-1 font-medium text-[10px] leading-none"
title={`${signalCount} actionable reports for you`}
>
{formatSignalCount(signalCount)}
</span>
) : null}
</>
</span>
}
isActive={isActive}
onClick={onClick}
Expand Down
11 changes: 11 additions & 0 deletions apps/code/src/renderer/styles/globals.css
Original file line number Diff line number Diff line change
Expand Up @@ -1069,6 +1069,17 @@ button,
color: var(--gray-12) !important;
}

.rt-BaseMenuItem[data-highlighted][data-variant="destructive"],
[role="menuitem"][data-highlighted][data-variant="destructive"] {
background-color: var(--red-3) !important;
color: var(--red-11) !important;
}

.rt-BaseMenuItem[data-highlighted][data-variant="destructive"] svg,
[role="menuitem"][data-highlighted][data-variant="destructive"] svg {
color: var(--red-11) !important;
}

/* Select/Menu dropdown background matches theme */
.rt-SelectContent,
.rt-BaseMenuContent {
Expand Down
Loading