feat(inbox): add Dismissed tab with restore#2704
Conversation
|
React Doctor found no issues in the changed files. 🎉 Reviewed by React Doctor for commit |
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Reviews (2): Last reviewed commit: "feat(inbox): add read-only detail view f..." | Re-trigger Greptile |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e17a9f273d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
andrewm4894
force-pushed
the
inbox/dismissed-tab
branch
2 times, most recently
from
79333c6 to
4de74ab
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 95690b4824
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Added: the dismissed report cards now show why each report was suppressed — the dismissal reason as a chip (with the note as a tooltip). The reason/note are denormalised onto the list |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c06b7d57e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f391b98053
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b870732fd7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const onDismissedRoute = backTo === "/code/inbox/dismissed"; | ||
| const isSuppressed = resolvedReport?.status === "suppressed"; | ||
| let redirectTo: InboxDetailRoute | null = null; | ||
| if (resolvedReport && !isFetching) { |
There was a problem hiding this comment.
When a report is already cached as a normal inbox report but has been suppressed in another session, useInboxReportById returns that cached row while the forced fresh fetch is still pending. This branch only postpones the redirect while isFetching, but the component then falls through and renders children(resolvedReport), so /reports, /pulls, or /runs can briefly show full triage controls (including Create PR/Discuss) for a now-suppressed report until the fetch settles. Consider rendering the spinner or disabling actions while the route-boundary status is being confirmed.
Useful? React with 👍 / 👎.
|
🤖 Pushed 6c59a46 (via PostHog Code from a Slack thread) doing the rename + one review fix: Dismiss/Dismissed → Archive (user-facing copy), aligning with the Cloud "Archive" wording:
Internal vocabulary is deliberately unchanged — the route segment ( Addressed the open Codex comment ("Hide stale detail actions during status refetch"): on a triage route, Typecheck ( |
| /** | ||
| * Dismissed tab membership: reports the user suppressed from the inbox. The | ||
| * dismiss action sets `suppressed`; `deleted` is terminal and never listed. | ||
| * These reports are fetched by a dedicated query (the main pipeline query | ||
| * excludes them), so this predicate is applied to that separate list. | ||
| */ | ||
| export function isDismissedReport(report: SignalReport): boolean { | ||
| return report.status === "suppressed"; | ||
| } |
There was a problem hiding this comment.
Hmm, I don't quite get this - I would think dimissed (or archived), should include both dismissed, resolved, snoozed AND suppressed
There was a problem hiding this comment.
Good question — the short answer is there's only one status that fits. The SignalReportStatus enum is potential | candidate | in_progress | ready | failed | pending_input | suppressed | deleted — there is no separate dismissed or resolved status, the archive action just sets suppressed. The other not-in-inbox states don't belong here: deleted is permanent (gone, never restorable), and snooze isn't a status at all — it's a temporary snoozed_until timestamp on an otherwise-active report that auto-returns to the inbox when it elapses, so surfacing it in a manual restore list would be wrong. I've expanded the doc comment in e5ae02c to spell this out.
| /** | ||
| * Card for the Dismissed tab. Links into the read-only dismissed detail view; | ||
| * the Restore button (right column) stops propagation so it doesn't navigate. | ||
| */ |
There was a problem hiding this comment.
This feels too custom, like it should just be a variant of the report card, fully handled by the report card component
There was a problem hiding this comment.
Done in e5ae02c — DismissedReportCard is gone, the Archive tab now renders <ReportCard variant="archived">. The variant shares all the card chrome and just swaps the triage actions for a single Restore button, shows the archive date + dismissal reason in the metadata row, links to the read-only archived detail view, and skips the artefact fetch it never renders. Net ~150 lines of duplication removed.
| /** | ||
| * Status filter for the Dismissed tab. Suppressed reports are excluded from the | ||
| * main pipeline query, so the Dismissed tab fetches them explicitly. `deleted` | ||
| * is terminal and stripped server-side, so it is never listed here. | ||
| */ | ||
| export const INBOX_DISMISSED_STATUS_FILTER = "suppressed"; |
There was a problem hiding this comment.
Mentioned in the other comment, feels fishy only suppressed state is special here
There was a problem hiding this comment.
Same reasoning as the isDismissedReport thread: suppressed is the only status that represents a user-archived report. deleted is stripped server-side and snooze is a timestamp rather than a status, so neither is a candidate here. Expanded the comment in e5ae02c to make the rationale explicit.
Address Michael's review on #2704: - Replace the standalone DismissedReportCard with an `archived` variant of ReportCard, so the archive list reuses the same card chrome (priority monogram, title, headline, source meta, findings count) instead of duplicating ~150 lines. The variant swaps triage affordances for a single Restore action, shows the archive date + dismissal reason in the metadata row, links to the read-only archived detail view, and skips the artefact fetch it doesn't render. - Clarify why `suppressed` is the only status the Archive tab lists: there is no separate dismissed/resolved status in the enum, `deleted` is permanent, and snooze is a temporary timestamp that auto-returns — none belong in a manual restore list. Generated-By: PostHog Code Task-Id: c0aa88e1-2b85-49c1-8f0e-d6b98f20b251
Adds a fourth inbox tab listing dismissed (suppressed) reports, newest first, with a per-card Restore action that reopens a report back into the pipeline (reuses the state action's 'potential' transition). - Dedicated useInboxDismissedReports query (status=suppressed); the main pipeline query excludes suppressed, so the tab fetches them separately. - useInboxRestoreReport invalidates reportKeys.all so a restored report moves out of Dismissed and back into the pipeline tabs. - Cards are read-only (no detail link): the report detail endpoint currently 404s for suppressed reports. Clicking-through depends on PostHog/posthog#64019 shipping; until then Restore is the only action. - No count badge on the tab (an open-ended archive total adds no signal), so no extra always-on query in the inbox shell. - Reviewer-scope selector hidden on the tab (the dismissed list isn't scoped). Updates the feature CLAUDE.md IA map to four tabs.
Dismissed cards now link into a detail view (summary + evidence) with a single Restore action; no triage affordances since the report is out of the pipeline. - DismissedReportDetail reuses InboxReportDetailGate + InboxDetailFrame. - InboxDetailFrame gains a showDismiss opt-out (a dismissed report shouldn't show 'Dismiss'); the gate skips OPENED/CLOSED engagement tracking for the dismissed tab (its rank would be measured against the wrong list). - Routes restructured: dismissed.tsx is now an Outlet layout, with dismissed.index (list) and dismissed.$reportId (detail). - The route loader resolves the cached report from the dismissed list cache, so navigation renders instantly; the detail's own fetch depends on the backend serving suppressed reports on retrieve/signals (PostHog/posthog#64019).
- DismissedReportDetail gains a copy-link button beside Restore (same deep-link util the other detail screens use), so a dismissed report can be shared. - useOpenInboxReport now routes a suppressed report to the Dismissed detail (was falling through to the Reports detail, which showed a stray Dismiss action on an already-dismissed report). Adds navigateToInboxDismissedDetail.
Surface why each report was suppressed: render the dismissal reason (with the note as a tooltip) as a chip in the DismissedReportCard meta row. The reason and note are denormalised onto the list SignalReport by the backend serializer, mirroring how priority/actionability/already_addressed are lifted from their artefacts, so cards avoid an N+1 per-card fetch. Reason codes are client-owned, so an unknown value falls back to the raw code via dismissalReasonLabel rather than being dropped.
A /code/inbox/dismissed/$reportId URL can go stale (history, bookmark, or copied deep link) after the report was restored and moved on. The detail view rendered Restore unconditionally, and READY/RESOLVED -> POTENTIAL is an allowed server-side transition, so restoring re-queued an active report. Redirect non-suppressed reports to their current home (pulls when a PR exists, else reports), mirroring useOpenInboxReport's routing, instead of offering Restore.
Follow-ups on the dismissed-detail guard: - Don't redirect on a stale cache snapshot. After a dismissal the suppress mutation invalidates but doesn't rewrite reportKeys.detail, so the gate can briefly surface the pre-dismissal (e.g. ready) record via initialData. The redirect now waits for the detail query to settle (!isFetching) — which the query forces fresh on mount via initialDataUpdatedAt: 0 — so a just-dismissed report opened from the Dismissed list no longer bounces straight out. - Route non-suppressed reports by the same membership predicates the tabs use: Pulls when a PR exists, Runs for in-flight runs (potential/candidate/ in_progress/pending_input), else Reports — instead of sending every non-PR report to Reports, which left run-state reports on the wrong tab.
…direct isReportTabReport excludes failed (failed runs live only in the Runs tab), so the previous isAgentRunReport check sent a restored-then-failed report to /reports, which doesn't list it. Route by membership precedence instead: PR -> pulls, isReportTabReport -> reports, else -> runs.
#54: a suppressed report reached via a stale /pulls, /reports, or /runs URL rendered that tab's full triage actions (dismiss, discuss, create PR) on an out-of-pipeline report, now that the backend serves suppressed reports on retrieve. Centralize the status<->route guard in InboxReportDetailGate: redirect across the dismissed<->pipeline boundary in both directions (suppressed off the triage routes -> dismissed; non-suppressed off /dismissed -> its current home), gated on a settled fetch. This subsumes the bespoke guard that lived in DismissedReportDetail, so that component is back to just rendering. #59: harden useInboxRestoreReport to re-read the report and no-op (with a toast + list refresh) if it's no longer suppressed, covering the card-level Restore on a row left stale by a change in another session — not just the detail path.
Aligns the inbox archive terminology with the PostHog Cloud "Archive" wording: - "Dismissed" tab → "Archive"; tab/card/detail copy and empty state now say archived. - "Dismiss" action → "Archive" (single-report button + dialog title/confirm, card and detail tooltips/aria-labels, bulk-action toast). - The suppress-permanently option tooltip and the suppress success toast now read "archive(d)". Internal identifiers keep the existing vocabulary on purpose: the route segment (/code/inbox/dismissed), query keys, component/hook/file names, and the backend status (suppressed) are unchanged, so analytics and deep links stay stable. The distinct "Snooze" and "Suppress" bulk labels are left as-is. Also addresses the gate review comment: on a triage route, InboxReportDetailGate now holds the spinner while the forced post-mount fetch confirms the status, so a report suppressed in another session can't briefly show triage actions (create PR / discuss / archive) from a stale cached snapshot before the redirect fires. The Archive route keeps its instant render-from-cache. Generated-By: PostHog Code Task-Id: c0aa88e1-2b85-49c1-8f0e-d6b98f20b251
Address Michael's review on #2704: - Replace the standalone DismissedReportCard with an `archived` variant of ReportCard, so the archive list reuses the same card chrome (priority monogram, title, headline, source meta, findings count) instead of duplicating ~150 lines. The variant swaps triage affordances for a single Restore action, shows the archive date + dismissal reason in the metadata row, links to the read-only archived detail view, and skips the artefact fetch it doesn't render. - Clarify why `suppressed` is the only status the Archive tab lists: there is no separate dismissed/resolved status in the enum, `deleted` is permanent, and snooze is a temporary timestamp that auto-returns — none belong in a manual restore list. Generated-By: PostHog Code Task-Id: c0aa88e1-2b85-49c1-8f0e-d6b98f20b251
Twixes
force-pushed
the
inbox/dismissed-tab
branch
from
e5ae02c to
4f3f02b
Compare
2 participants
Adds a Dismissed tab to the inbox: lists dismissed (suppressed) reports, lets you re-read one, restore it, or grab a shareable link.
Backend dependency (PostHog/posthog#64019) is merged + deployed, and the whole feature has been verified end-to-end against the live app (including a real restore round-trip — status confirmed flipping
suppressed → potentialserver-side).What's in it
Tab + list
useInboxDismissedReportsfetchesstatus=suppressed(the main pipeline query excludes suppressed reports), no polling — relies onreportKeys.allinvalidation, which dismiss/restore both trigger.reportMembership.ts(isDismissedReport), unit-tested.Restore
useInboxRestoreReportreuses thestateaction'spotential("reopen") transition — the only reopen path the backend exposes. InvalidatesreportKeys.allso the report leaves Dismissed.potential, which is a queued run status, so a restored report lands in the Runs tab (Queued), not Reports — it re-enters the pipeline rather than popping back to its prior state (that prior status isn't persisted, and thestateendpoint only acceptspotential/suppressed). It cycles back to Reports on its own once it re-reachesready. It stays re-dismissible throughout (via the run → report detail, or from Reports once ready).Read-only detail
DismissedReportDetailreusesInboxReportDetailGate+InboxDetailFrame— summary + evidence + Restore + copy-link. No triage affordances (dismiss / discuss / create PR / reviewers): the report is out of the pipeline until restored.InboxDetailFramegains ashowDismissopt-out; the gate skips OPENED/CLOSED engagement tracking for the dismissed tab (not a trackedInboxDetailTab— its rank would be measured against the wrong list).dismissed.tsxis anOutletlayout, withdismissed.index(list) anddismissed.$reportId(detail). The loader resolves the cached report from the dismissed list cache, so navigation renders instantly.Shareable link
copyInboxReportLinkdeep-link util the other detail screens use).useOpenInboxReportnow routes a suppressed report to the Dismissed detail (it previously had no suppressed branch and fell through to the Reports detail, which showed a stray Dismiss action on an already-dismissed report). AddsnavigateToInboxDismissedDetail.Verification
@posthog/core+@posthog/uitypecheck ✅ · core tests 1550 passed ✅ · biome lint ✅ (pre-commit runs biome + full typecheck)019eced2…wentsuppressed → potential.Reviewer notes
suppressed → potentialrestore → Runs behaviour above is intentional, not a bug — it's the only reopen transition the backend supports. Flagged here so it doesn't read as one.