Skip to content
Draft
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
85 changes: 85 additions & 0 deletions packages/core/src/inbox/reportMembership.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ import {
isDismissedReport,
isExcludedFromInbox,
isInboxDetailPath,
isNotActionableReport,
isPullRequestReport,
isReportTabReport,
isStaffOnlyInboxTab,
matchesReviewerScope,
partitionRunsTabReports,
teammateInboxScope,
Expand Down Expand Up @@ -58,10 +60,85 @@ describe("isDismissedReport", () => {
});
});

describe("isNotActionableReport", () => {
it("matches reports the judgment marked not_actionable", () => {
expect(
isNotActionableReport(fakeReport({ actionability: "not_actionable" })),
).toBe(true);
});

it.each(["immediately_actionable", "requires_human_input", null] as const)(
"does not match %s actionability",
(actionability) => {
expect(isNotActionableReport(fakeReport({ actionability }))).toBe(false);
},
);

it.each(["suppressed", "resolved"] as const)(
"excludes terminal %s reports",
(status) => {
expect(
isNotActionableReport(
fakeReport({ actionability: "not_actionable", status }),
),
).toBe(false);
},
);

it("excludes PR-bearing reports", () => {
expect(
isNotActionableReport(
fakeReport({
actionability: "not_actionable",
implementation_pr_url: "https://gh/p/1",
}),
),
).toBe(false);
});

it.each(["potential", "candidate", "in_progress", "pending_input"] as const)(
"excludes in-flight %s runs (they stay in the Runs tab)",
(status) => {
expect(
isNotActionableReport(
fakeReport({ status, actionability: "not_actionable" }),
),
).toBe(false);
},
);

it("excludes failed reports (they stay in the Runs tab)", () => {
expect(
isNotActionableReport(
fakeReport({ status: "failed", actionability: "not_actionable" }),
),
).toBe(false);
});

it("matches a settled (ready) not_actionable report", () => {
expect(
isNotActionableReport(
fakeReport({ status: "ready", actionability: "not_actionable" }),
),
).toBe(true);
});
});

describe("isStaffOnlyInboxTab", () => {
it("gates only the Not actionable tab", () => {
expect(isStaffOnlyInboxTab("not-actionable")).toBe(true);
expect(isStaffOnlyInboxTab("reports")).toBe(false);
expect(isStaffOnlyInboxTab("pulls")).toBe(false);
expect(isStaffOnlyInboxTab("runs")).toBe(false);
expect(isStaffOnlyInboxTab("dismissed")).toBe(false);
});
});

describe("isInboxDetailPath", () => {
it("matches detail paths for each inbox tab", () => {
expect(isInboxDetailPath("/code/inbox/pulls/abc")).toBe(true);
expect(isInboxDetailPath("/code/inbox/reports/abc")).toBe(true);
expect(isInboxDetailPath("/code/inbox/not-actionable/abc")).toBe(true);
expect(isInboxDetailPath("/code/inbox/runs/abc")).toBe(true);
});

Expand Down Expand Up @@ -236,6 +313,14 @@ describe("tabFilters", () => {
),
).toBe(false);
});

it("excludes not-actionable reports (they go to the Not actionable tab)", () => {
expect(
isReportTabReport(
fakeReport({ status: "ready", actionability: "not_actionable" }),
),
).toBe(false);
});
});

describe("matchesReviewerScope", () => {
Expand Down
46 changes: 45 additions & 1 deletion packages/core/src/inbox/reportMembership.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,22 +82,43 @@ export function countInboxScopeReports(
return reports.filter((report) => matchesInboxScope(report, scope)).length;
}

export type InboxTabKey = "pulls" | "reports" | "runs" | "dismissed";
export type InboxTabKey =
| "pulls"
| "reports"
| "not-actionable"
| "runs"
| "dismissed";

export const INBOX_TAB_KEYS: InboxTabKey[] = [
"pulls",
"reports",
"not-actionable",
"runs",
"dismissed",
];

export const INBOX_TAB_LABEL: Record<InboxTabKey, string> = {
pulls: "Pull requests",
reports: "Reports",
"not-actionable": "Not actionable",
runs: "Runs",
dismissed: "Archive",
};

/**
* Tabs only shown to staff (internal) users. Non-staff see Pull requests,
* Reports, Runs and Archive. The Not actionable tab is an internal signal-quality
* audit surface — reports the agent judged not worth acting on — so it stays
* behind the staff flag, matching the PostHog Cloud inbox.
*/
export const INBOX_STAFF_ONLY_TAB_KEYS = new Set<InboxTabKey>([
"not-actionable",
]);

export function isStaffOnlyInboxTab(key: InboxTabKey): boolean {
return INBOX_STAFF_ONLY_TAB_KEYS.has(key);
}

/**
* Canonical inbox tab list routes. Use these constants instead of hard-coding
* `/code/inbox/pulls` etc., so renames stay in one place.
Expand All @@ -112,6 +133,7 @@ export const INBOX_TAB_LIST_ROUTE: Record<
> = {
pulls: "/code/inbox/pulls",
reports: "/code/inbox/reports",
"not-actionable": "/code/inbox/not-actionable",
runs: "/code/inbox/runs",
dismissed: "/code/inbox/dismissed",
};
Expand Down Expand Up @@ -231,9 +253,31 @@ export function isReportTabReport(report: SignalReport): boolean {
// rather than reappearing as a Report.
if (report.implementation_pr_url) return false;
if (isAgentRunReport(report)) return false;
// Reports the agent judged not worth acting on get their own (staff-only)
// tab and are kept out of the main Reports list, matching the Cloud inbox.
if (isNotActionableReport(report)) return false;
return true;
}

/**
* Not-actionable tab membership: reports the agentic actionability judgment
* marked `not_actionable`. A staff-only signal-quality audit surface. These are
* still in-pipeline reports (the judgment is an artefact, not a status), so this
* partitions the same main list the other report tabs read — it just keeps them
* out of the Reports tab via the check in `isReportTabReport`.
*/
export function isNotActionableReport(report: SignalReport): boolean {
if (isExcludedFromInbox(report)) return false;
if (report.status === "failed") return false; // failed runs live in the Runs tab only
if (report.implementation_pr_url) return false;
// In-flight (queued/live) runs belong to the Runs tab until they settle, even
// if a not_actionable judgment is already attached. Mirror the gate order in
// `isReportTabReport` so the two predicates classify a report the same way and
// it can't show in both the Runs and Not actionable tabs at once.
if (isAgentRunReport(report)) return false;
return report.actionability === "not_actionable";
}
Comment on lines +269 to +279

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 In-flight not_actionable reports double-list in Runs and Not actionable tabs

isNotActionableReport does not guard against isAgentRunReport(report). partitionRunsTabReports partitions directly on isQueuedRunReport/isLiveRunReport, bypassing this predicate entirely. A report that has actionability === "not_actionable" set while still in potential, candidate, in_progress, or pending_input status will therefore satisfy both predicates and appear in both the Runs tab and the Not actionable tab simultaneously.

isReportTabReport avoids this only because it calls isAgentRunReport before isNotActionableReport. Adding the same guard here would make the Not actionable predicate self-consistent, matching the existing pattern in isReportTabReport.


export function matchesReviewerScope(
report: SignalReport,
scope: InboxScope,
Expand Down
14 changes: 12 additions & 2 deletions packages/ui/src/features/inbox/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,24 @@ The main objects are:

## Information Architecture

Inbox has four tabs and one reviewer-scope control:
Inbox has five tabs and one reviewer-scope control:

| Tab | Route | Membership |
| --- | --- | --- |
| Pull requests | `/code/inbox/pulls` | Reports with `implementation_pr_url` set |
| Reports | `/code/inbox/reports` | Reports without a PR and not currently running |
| Reports | `/code/inbox/reports` | Reports without a PR, not running, and not judged not-actionable |
| Not actionable | `/code/inbox/not-actionable` | Reports the judgment marked `actionability === "not_actionable"` (**staff-only**) |
| Runs | `/code/inbox/runs` | Reports that are still in progress or waiting on input |
| Archive | `/code/inbox/dismissed` | Terminal reports: archived/suppressed (`status === "suppressed"`) and resolved-by-merged-PR (`status === "resolved"`) |

The **Not actionable** tab is gated to staff (internal) users via
`INBOX_STAFF_ONLY_TAB_KEYS` (see `isStaffOnlyInboxTab`); `InboxTabBar` hides it
for non-staff, keyed off `user.is_staff`. It's an internal signal-quality audit
surface and mirrors the same staff-only tab in `PostHog/posthog`. It reuses the
shared `InboxReportListTab` shell (same as Reports/Pulls); cards link to its own
detail route so back-navigation stays on the tab. `isReportTabReport` excludes
not-actionable reports so they don't double-list in Reports.

Detail pages live under the same tab: `/code/inbox/<tab>/$reportId`.

The Archive tab (route `/code/inbox/dismissed`, user-facing label "Archive") is
Expand Down Expand Up @@ -76,6 +85,7 @@ The tab components are intentionally simple:

- `PullRequestsTab` partitions scoped reports with `isPullRequestReport`.
- `ReportsTab` partitions with `isReportTabReport`.
- `NotActionableTab` (staff-only) partitions with `isNotActionableReport`.
- `RunsTab` partitions with `isAgentRunReport`.
- `DismissedTab` (the "Archive" tab) lists its own `useInboxDismissedReports` query (matching `isDismissedReport`); read-only detail route, restore action per card.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,11 @@ import type { ComponentType, ReactNode } from "react";
interface InboxDetailFrameProps {
report: SignalReport;
/** List route for the back-link (e.g. "/code/inbox/pulls"). */
backTo: "/code/inbox/pulls" | "/code/inbox/reports" | "/code/inbox/dismissed";
backTo:
| "/code/inbox/pulls"
| "/code/inbox/reports"
| "/code/inbox/not-actionable"
| "/code/inbox/dismissed";
backLabel: string;
/**
* Whether to render the Dismiss button + dialog. Off for already-dismissed
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {
isDismissedReport,
isNotActionableReport,
isPullRequestReport,
isReportTabReport,
} from "@posthog/core/inbox/reportMembership";
Expand All @@ -21,6 +22,7 @@ interface InboxReportDetailGateProps {
backTo:
| "/code/inbox/pulls"
| "/code/inbox/reports"
| "/code/inbox/not-actionable"
| "/code/inbox/runs"
| "/code/inbox/dismissed";
backLabel: string;
Expand All @@ -31,6 +33,7 @@ interface InboxReportDetailGateProps {
type InboxDetailRoute =
| "/code/inbox/pulls/$reportId"
| "/code/inbox/reports/$reportId"
| "/code/inbox/not-actionable/$reportId"
| "/code/inbox/runs/$reportId"
| "/code/inbox/dismissed/$reportId";

Expand All @@ -43,6 +46,8 @@ type InboxDetailRoute =
*/
function nonSuppressedDetailRoute(report: SignalReport): InboxDetailRoute {
if (isPullRequestReport(report)) return "/code/inbox/pulls/$reportId";
if (isNotActionableReport(report))
return "/code/inbox/not-actionable/$reportId";
if (isReportTabReport(report)) return "/code/inbox/reports/$reportId";
return "/code/inbox/runs/$reportId";
}
Expand Down Expand Up @@ -165,7 +170,11 @@ function tabFromBackTo(
): InboxDetailTab | null {
if (backTo === "/code/inbox/pulls") return "pulls";
if (backTo === "/code/inbox/runs") return "runs";
// Archive and the staff-only Not actionable tab aren't part of the triage
// funnel, so their detail opens aren't tracked (rank would be measured against
// the wrong list).
if (backTo === "/code/inbox/dismissed") return null;
if (backTo === "/code/inbox/not-actionable") return null;
return "reports";
}

Expand Down
41 changes: 28 additions & 13 deletions packages/ui/src/features/inbox/components/InboxTabBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@ import {
INBOX_TAB_LIST_ROUTE,
type InboxTabCounts,
type InboxTabKey,
isStaffOnlyInboxTab,
} from "@posthog/core/inbox/reportMembership";
import { Tabs, TabsList, TabsTrigger } from "@posthog/quill";
import { useMeQuery } from "@posthog/ui/features/auth/useMeQuery";
import { InboxScopeSelect } from "@posthog/ui/features/inbox/components/InboxScopeSelect";
import { Flex } from "@radix-ui/themes";
import { useNavigate, useRouterState } from "@tanstack/react-router";
Expand All @@ -15,6 +17,11 @@ interface InboxTabBarProps {
}

function activeTabFromPath(pathname: string): InboxTabKey {
// Check "not-actionable" before "reports": the former is not a prefix of the
// latter, but keeping the more specific routes first guards against future
// overlaps.
if (pathname.startsWith(INBOX_TAB_LIST_ROUTE["not-actionable"]))
return "not-actionable";
if (pathname.startsWith(INBOX_TAB_LIST_ROUTE.reports)) return "reports";
if (pathname.startsWith(INBOX_TAB_LIST_ROUTE.runs)) return "runs";
if (pathname.startsWith(INBOX_TAB_LIST_ROUTE.dismissed)) return "dismissed";
Expand All @@ -25,6 +32,11 @@ export function InboxTabBar({ counts }: InboxTabBarProps) {
const navigate = useNavigate();
const pathname = useRouterState({ select: (s) => s.location.pathname });
const activeKey = activeTabFromPath(pathname);
const { data: currentUser } = useMeQuery();
const isStaff = currentUser?.is_staff === true;
const visibleTabKeys = INBOX_TAB_KEYS.filter(
(key) => isStaff || !isStaffOnlyInboxTab(key),
);

return (
<Flex align="center" justify="between" className="min-w-0">
Expand All @@ -39,7 +51,7 @@ export function InboxTabBar({ counts }: InboxTabBarProps) {
variant="line"
className="h-auto gap-0.5 [&_.quill-tabs__indicator]:transition-[transform,width]! [&_.quill-tabs__indicator]:duration-100! [&_.quill-tabs__indicator]:ease-out!"
>
{INBOX_TAB_KEYS.map((key) => {
{visibleTabKeys.map((key) => {
const isActive = key === activeKey;
return (
<TabsTrigger
Expand All @@ -50,18 +62,21 @@ export function InboxTabBar({ counts }: InboxTabBarProps) {
<span className="font-medium text-[13px]">
{INBOX_TAB_LABEL[key]}
</span>
{/* Runs and the open-ended Archive don't get a running total — it adds no signal. */}
{key !== "runs" && key !== "dismissed" && counts[key] > 0 && (
<span
className={
isActive
? "text-[12px] text-gray-11 tabular-nums"
: "text-[12px] text-gray-10 tabular-nums"
}
>
{counts[key]}
</span>
)}
{/* Runs, Not actionable, and the open-ended Archive don't get a running total — it adds no signal. */}
{key !== "runs" &&
key !== "dismissed" &&
key !== "not-actionable" &&
counts[key] > 0 && (
<span
className={
isActive
? "text-[12px] text-gray-11 tabular-nums"
: "text-[12px] text-gray-10 tabular-nums"
}
>
{counts[key]}
</span>
)}
</TabsTrigger>
);
})}
Expand Down
Loading
Loading