diff --git a/packages/core/src/inbox/reportFiltering.ts b/packages/core/src/inbox/reportFiltering.ts index 170b65db17..42471ba0de 100644 --- a/packages/core/src/inbox/reportFiltering.ts +++ b/packages/core/src/inbox/reportFiltering.ts @@ -13,14 +13,15 @@ export const INBOX_PIPELINE_STATUS_FILTER = "potential,candidate,in_progress,ready,pending_input,failed"; /** - * Status filter for the Archive tab. `suppressed` is the only archived status: - * it is the single state the archive action sets, and the only not-in-inbox - * state worth restoring. `deleted` is permanent and stripped server-side; snooze - * is a temporary `snoozed_until` timestamp, not a status, and auto-returns. See - * `isDismissedReport` for the full rationale. Suppressed reports are excluded - * from the main pipeline query, so the Archive tab fetches them explicitly. + * Status filter for the Archive tab. Two terminal states land here: `suppressed` + * (the user archived the report — restorable) and `resolved` (its implementation + * PR merged — terminal, shown for reference only). `deleted` is permanent and + * stripped server-side; snooze is a temporary `snoozed_until` timestamp, not a + * status, and auto-returns. See `isDismissedReport` for the full rationale. Both + * states are excluded from the main pipeline query, so the Archive tab fetches + * them explicitly. */ -export const INBOX_DISMISSED_STATUS_FILTER = "suppressed"; +export const INBOX_DISMISSED_STATUS_FILTER = "suppressed,resolved"; /** * Status filter for the Pull requests tab's list and count. Only `ready` PRs — diff --git a/packages/core/src/inbox/reportMembership.test.ts b/packages/core/src/inbox/reportMembership.test.ts index d7c1b4e894..bb9f0e4e71 100644 --- a/packages/core/src/inbox/reportMembership.test.ts +++ b/packages/core/src/inbox/reportMembership.test.ts @@ -10,8 +10,11 @@ import { isDismissedReport, isExcludedFromInbox, isInboxDetailPath, + isNotActionableReport, isPullRequestReport, isReportTabReport, + isResolvedReport, + isStaffOnlyInboxTab, matchesReviewerScope, partitionRunsTabReports, teammateInboxScope, @@ -38,8 +41,9 @@ function fakeReport(overrides: Partial = {}): SignalReport { } describe("isDismissedReport", () => { - it("matches only suppressed reports", () => { + it("matches suppressed and resolved reports", () => { expect(isDismissedReport(fakeReport({ status: "suppressed" }))).toBe(true); + expect(isDismissedReport(fakeReport({ status: "resolved" }))).toBe(true); }); it.each([ @@ -55,10 +59,95 @@ describe("isDismissedReport", () => { }); }); +describe("isResolvedReport", () => { + it("matches only resolved reports", () => { + expect(isResolvedReport(fakeReport({ status: "resolved" }))).toBe(true); + expect(isResolvedReport(fakeReport({ status: "suppressed" }))).toBe(false); + expect(isResolvedReport(fakeReport({ status: "ready" }))).toBe(false); + }); +}); + +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("excludes terminal (suppressed/resolved/deleted) reports", () => { + expect( + isNotActionableReport( + fakeReport({ actionability: "not_actionable", status: "suppressed" }), + ), + ).toBe(false); + expect( + isNotActionableReport( + fakeReport({ actionability: "not_actionable", status: "resolved" }), + ), + ).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); }); @@ -158,7 +247,10 @@ describe("tabFilters", () => { }); describe("isExcludedFromInbox", () => { - it("returns true for suppressed and deleted", () => { + it("returns true for resolved, suppressed and deleted", () => { + expect(isExcludedFromInbox(fakeReport({ status: "resolved" }))).toBe( + true, + ); expect(isExcludedFromInbox(fakeReport({ status: "suppressed" }))).toBe( true, ); @@ -230,6 +322,18 @@ 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); + }); + + it("excludes resolved reports (they go to the Archive tab)", () => { + expect(isReportTabReport(fakeReport({ status: "resolved" }))).toBe(false); + }); }); describe("matchesReviewerScope", () => { diff --git a/packages/core/src/inbox/reportMembership.ts b/packages/core/src/inbox/reportMembership.ts index 0ff36c718d..78d7f72284 100644 --- a/packages/core/src/inbox/reportMembership.ts +++ b/packages/core/src/inbox/reportMembership.ts @@ -7,6 +7,7 @@ import type { SignalReport } from "@posthog/shared/types"; * them out via their own predicates. */ export const INBOX_EXCLUDED_STATUSES = new Set([ + "resolved", "suppressed", "deleted", ]); @@ -16,10 +17,10 @@ export function isExcludedFromInbox(report: SignalReport): boolean { } /** - * Archive tab membership. `suppressed` is the only status that represents "the - * user archived this out of the inbox" — there is no separate `dismissed` / - * `resolved` status in the enum (see `SignalReportStatus`), the archive action - * sets `suppressed`. The other not-in-inbox states are deliberately excluded: + * Archive tab membership. Two terminal states live here: `suppressed` (the user + * archived the report out of the inbox — restorable) and `resolved` (the + * report's implementation PR was merged — terminal, shown for reference only, + * not restorable). The other not-in-inbox state is deliberately excluded: * `deleted` is permanent (gone, never restorable, stripped server-side), and * snooze is not a status at all — it is a temporary `snoozed_until` timestamp * on an otherwise-active report that auto-returns to the inbox when it elapses, @@ -28,7 +29,16 @@ export function isExcludedFromInbox(report: SignalReport): boolean { * predicate is applied to that separate list. */ export function isDismissedReport(report: SignalReport): boolean { - return report.status === "suppressed"; + return report.status === "suppressed" || report.status === "resolved"; +} + +/** + * A report whose implementation PR has been merged. It lands in the Archive tab + * for reference but, unlike a user-suppressed report, has no Restore action — + * the work is finished, not paused. + */ +export function isResolvedReport(report: SignalReport): boolean { + return report.status === "resolved"; } export type InboxScope = "for-you" | "entire-project" | `teammate:${string}`; @@ -78,11 +88,17 @@ 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", ]; @@ -90,10 +106,25 @@ export const INBOX_TAB_KEYS: InboxTabKey[] = [ export const INBOX_TAB_LABEL: Record = { 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([ + "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. @@ -108,6 +139,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", }; @@ -227,9 +259,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"; +} + export function matchesReviewerScope( report: SignalReport, scope: InboxScope, diff --git a/packages/core/src/inbox/reportPresentation.ts b/packages/core/src/inbox/reportPresentation.ts index b2718396ba..26094eebc9 100644 --- a/packages/core/src/inbox/reportPresentation.ts +++ b/packages/core/src/inbox/reportPresentation.ts @@ -60,6 +60,8 @@ export function inboxStatusLabel(status: SignalReportStatus): string { return "Gathering"; case "failed": return "Failed"; + case "resolved": + return "Resolved"; case "suppressed": return "Suppressed"; case "deleted": @@ -83,6 +85,8 @@ export function inboxStatusAccentCss(status: SignalReportStatus): string { return "var(--gray-9)"; case "failed": return "var(--red-9)"; + case "resolved": + return "var(--green-9)"; default: return "var(--gray-8)"; } diff --git a/packages/core/src/inbox/statusLabels.ts b/packages/core/src/inbox/statusLabels.ts index c787db3665..51b4f51937 100644 --- a/packages/core/src/inbox/statusLabels.ts +++ b/packages/core/src/inbox/statusLabels.ts @@ -14,6 +14,8 @@ export function inboxStatusLabel(status: SignalReportStatus): string { return "Gathering"; case "failed": return "Failed"; + case "resolved": + return "Resolved"; case "suppressed": return "Suppressed"; case "deleted": diff --git a/packages/shared/src/signal-types.ts b/packages/shared/src/signal-types.ts index b7cb8e38d6..526a7f044f 100644 --- a/packages/shared/src/signal-types.ts +++ b/packages/shared/src/signal-types.ts @@ -5,6 +5,7 @@ export type SignalReportStatus = | "ready" | "failed" | "pending_input" + | "resolved" | "suppressed" | "deleted"; diff --git a/packages/ui/src/features/inbox/CLAUDE.md b/packages/ui/src/features/inbox/CLAUDE.md index 9f01dc0cd1..a8495254af 100644 --- a/packages/ui/src/features/inbox/CLAUDE.md +++ b/packages/ui/src/features/inbox/CLAUDE.md @@ -15,29 +15,43 @@ 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` | Reports the user archived/suppressed (`status === "suppressed"`) | +| Archive | `/code/inbox/dismissed` | Terminal reports: user-archived (`suppressed`) + merged-PR (`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//$reportId`. The Archive tab (route `/code/inbox/dismissed`, user-facing label "Archive") is -the exception: suppressed reports are excluded from the -main pipeline query, so the tab fetches them with a dedicated `status=suppressed` -query (`useInboxDismissedReports`). Its detail view (`DismissedReportDetail`) is -read-only — summary + evidence + a single Restore action, no triage affordances — -and depends on the backend serving suppressed reports on the `retrieve`/`signals` -read paths (PostHog/posthog#64019). Restore uses `useInboxRestoreReport`, which -reuses the `state` action's `potential` ("reopen") transition — the only reopen -path the backend exposes. The reviewer scope control is hidden on this tab since -the archive list is not scoped, and the tab carries no count badge. The -Archive detail is **not** a tracked `InboxDetailTab` (no OPENED/CLOSED -engagement events), since its rank would be measured against the wrong list. +the exception: it holds two terminal states — `suppressed` (the user archived +the report) and `resolved` (its implementation PR merged). Both are excluded from +the main pipeline query, so the tab fetches them with a dedicated +`status=suppressed,resolved` query (`useInboxDismissedReports`). Suppressed +reports carry a Restore action (`useInboxRestoreReport`, which reuses the `state` +action's `potential` "reopen" transition — the only reopen path the backend +exposes); resolved reports are terminal and shown for reference only, with no +Restore (`isResolvedReport` gates the row action in `ReportCard`). This is the +home for a report once its PR is merged — it used to fall off every tab. Its +detail view (`DismissedReportDetail`) is read-only — summary + evidence, no +triage affordances — and depends on the backend serving these reports on the +`retrieve`/`signals` read paths (PostHog/posthog#64019). The reviewer scope +control is hidden on this tab since the archive list is not scoped, and the tab +carries no count badge. The Archive detail is **not** a tracked `InboxDetailTab` +(no OPENED/CLOSED engagement events), since its rank would be measured against +the wrong list. The internal route segment, query key, and component/hook names keep the `dismissed`/`suppressed` vocabulary (the backend status is `suppressed`); only diff --git a/packages/ui/src/features/inbox/components/DismissedReportDetail.tsx b/packages/ui/src/features/inbox/components/DismissedReportDetail.tsx index a5c9d2aa78..eedf0478f3 100644 --- a/packages/ui/src/features/inbox/components/DismissedReportDetail.tsx +++ b/packages/ui/src/features/inbox/components/DismissedReportDetail.tsx @@ -4,6 +4,7 @@ import { FileTextIcon, MagnifyingGlassIcon, } from "@phosphor-icons/react"; +import { isResolvedReport } from "@posthog/core/inbox/reportMembership"; import { Button } from "@posthog/quill"; import type { SignalReport } from "@posthog/shared/types"; import { InboxDetailFrame } from "@posthog/ui/features/inbox/components/InboxDetailFrame"; @@ -19,14 +20,15 @@ interface DismissedReportDetailProps { } /** - * Detail view for an archived (suppressed) report. Read-only re-read of what the - * report was — summary + evidence — with a single Restore action. No triage - * affordances (archive, discuss, create PR, reviewers): the report is out of the - * pipeline until it's restored. + * Detail view for an archived report. Read-only re-read of what the report was — + * summary + evidence. Suppressed reports get a single Restore action; resolved + * reports (their PR merged) are terminal and shown for reference only, with no + * Restore. No triage affordances (archive, discuss, create PR, reviewers): the + * report is out of the pipeline. * * The gate keeps reports on the route that matches their status: a no-longer- - * suppressed report opened here (stale URL or restored elsewhere) is redirected - * to its current home, so this content only ever renders a suppressed report. + * archived report opened here (stale URL or restored elsewhere) is redirected to + * its current home, so this content only ever renders an archived report. */ export function DismissedReportDetail({ reportId, @@ -55,7 +57,8 @@ function DismissedReportDetailContent({ report }: { report: SignalReport }) { showDismiss={false} primaryAction={ <> - + {/* Resolved reports are terminal (PR merged) — no Restore. */} + {!isResolvedReport(report) && }