feat(code): Inbox 2.0#2547
Conversation
|
React Doctor found 20 issues in 13 files · 20 warnings. 20 warnings
Reviewed by React Doctor for commit |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
This reverts commit 0757815.
joshsny
left a comment
joshsny
left a comment
There was a problem hiding this comment.
LGTM, your change to the remote debugging port looks unintentional
Debug port for screenshot/Electron attach lives on add-screenshot-dev-app-skill.
…not merge) Adds a parent /sync-inbox-to-cloud skill plus five sub-skills that orchestrate porting features and UI polish from the desktop Inbox to PostHog Cloud: - /inspect-inbox-surfaces - /plan-inbox-sync - /implement-inbox-sync (with translation.md reference) - /finalize-inbox-sync - /reflect-on-inbox-sync Plus references/parallelization.md describing how to slice work across parallel sub-agents and the upcoming-direction mock screenshots used as supplementary context. Not for merge — this branch exists as a long-lived reference / checkout target while the cloud Inbox port is in flight. See PR description.
Internal-shape changes from a multi-reviewer pass on the Inbox 2.0 surface. Behavior visible to users: dismiss buttons on the PR and Report detail screens are now wired up (previously disabled placeholders), the "Stop run" placeholder on agent-run detail is gone (no backend endpoint exists), deep-linked reports route directly to the correct tab/detail based on whether the report has an implementation PR, and PR list cards no longer fire one GitHub REST call each for diff stats. Behavior + correctness: - `isInboxDetailPath` matches the runs detail route too, so the global Inbox header no longer stacks on top of run detail pages. - `findReportInInboxListCache` runtime-narrows each prefix-matched cache entry; seeded detail entries and scope-count numbers no longer cause a TypeError when resolving an uncached detail. - Bulk-action partial failures keep failed ids in the selection so the user can retry; only succeeded ids are dropped. - `isReportTabReport` guards against terminal-status reports. - `isAgentRunReport` accepts `pending_input` to match the doc. - Deep links navigate to `/code/inbox/pulls/$id` or `/code/inbox/reports/$id` based on `implementation_pr_url`. - "Open in GitHub" re-validates `implementation_pr_url` via `parsePrUrl` at click time so only `https://github.com/<owner>/<repo>/pull/<n>` is ever opened. Performance: - New `git.getPrDiffStatsBatch(prUrls[])` runs one GitHub GraphQL request (alias-batched) for the entire visible PR list. `PrDiffStats` reads from a `PrDiffStatsBatchProvider` mounted by `PullRequestsTab`; falls back to the per-PR REST query on detail pages only. - `useInboxScopeOptionCounts` no longer fans out one list-count request per teammate when the scope menu opens — counts are limited to For you and Entire project. - Inbox card detail prefetch drops the redundant `onPointerEnter` handler; TanStack `<Link preload="intent">` already covers it. - Single O(N) `buildSuppressDisabledReasonMap` lookup per render replaces the inline O(N²) helper that fired per card on every search keystroke. Architecture: - `InboxReportListTab` shell owns the dismiss state machine, skeleton, search bar, and scope-aware empty state. `PullRequestsTab` / `ReportsTab` are thin configuration wrappers; `RunsTab` keeps its own layout. - `InboxReportDetailGate` shares the loading / missing-report fallback for both detail screens. - `InboxDetailFrame` owns the entire header + summary/evidence column for PR and Report detail; variants supply breadcrumb, meta prefix, extra badges, primary action, and section titles via slots. - `useInboxReportDismissAction` packages the per-report dismiss flow. - `useInboxCloudTaskRunner` extracts the shared cloud-task driver (guards, model resolution, `TaskService.createTask`, toast/track/openTask wiring). `useCreatePrReport` / `useDiscussReport` are thin wrappers. - `useSignalSourceManager` is split into `useSignalSourceToggles`, `useSignalEvaluations`, `useSignalTeamConfigMutations`, and `useSignalUserAutonomyMutations`. The existing entry point is preserved as a thin aggregator so call sites keep their flat destructured shape. - `inboxReportKeys` lives in a new leaf module (`utils/inboxQueryKeys`) shared by hooks, detail cache, and bulk actions. Cleanups: - Type-safe `SOURCE_PRODUCT_META` (`Partial<Record<SourceProduct, …>>`) with a `getSourceProductMeta` helper. - Renderer `trpcClient.*` direct calls migrated to `useQuery` / `useMutation`. - `useReportTasks` drops the `as unknown as Task` double-cast. - Inline `style={}` for static values converted to Tailwind classes. - Path-alias violations under `features/inbox/components` and the `../../task-detail/service/service` reach-up fixed. - Dead state slots in `inboxSignalsFilterStore` deleted, with a `persist` `migrate` that drops the old localStorage keys. - Dead `pendingInboxOpenMethod` module, unused `INBOX_BADGE_RADIUS_CLASS` export, unused `ReviewerScope` alias, and unused `suggestedReviewerSeed` removed. - Inbox tab list routes centralized as `INBOX_TAB_LIST_ROUTE`; the detail-path regex derives from `INBOX_TAB_KEYS`. - Date label on `ReportCard` falls back to `created_at`. - Dismiss tooltip no longer promises GitHub PR closure (closing the upstream PR remains a known gap; TODO unchanged). Tests: 128 inbox tests pass; +4 new (`inboxRoutes.test.ts`, mixed-shape case for `inboxReportDetailCache`, `pending_input` case for `isAgentRunReport`, migrate-from-v1 test for `inboxSignalsFilterStore`).
oliverb123
left a comment
oliverb123
left a comment
There was a problem hiding this comment.
Did the usual and just dump3e my ui feedback somewhere vaguely sensible, and let you route it properly to the relevant code.
Seems like the major things are the fetching tasks associated with a particular report is broken, and you deleted functionality around changing the suggested reviewers list. Also I think we could go 2 column in the detail view
| import { ConventionalCommitScopeTag } from "@features/inbox/components/ConventionalCommitScopeTag"; | ||
| import { InboxCardSourceMeta } from "@features/inbox/components/InboxCardSourceMeta"; | ||
| import { InboxCardTitle } from "@features/inbox/components/InboxCardTitle"; | ||
| import { SuggestedReviewerAvatarStack } from "@features/inbox/components/SuggestedReviewerAvatarStack"; |
There was a problem hiding this comment.
I dislike the the variance in corner radius here, particularly this close together and not nested, and generally I like the idea of using icons here but making them look a bit like buttons is ugly imo
| import type { SignalReport } from "@shared/types"; | ||
| import { Link, useNavigate } from "@tanstack/react-router"; | ||
| import { PriorityMonogram } from "./PriorityMonogram"; | ||
|
|
There was a problem hiding this comment.
I really want a link to the relevant commit here, or at least it's short hash so I can go look at it.
| } | ||
|
|
||
| export function ReportCard({ | ||
| report, |
There was a problem hiding this comment.
I see the open in GitHub button but no way to view the associated tasks? Seems like either the pulling of associated tasks is broken, or this is missing?
| report, | ||
| detailRoute, | ||
| ); | ||
| const navigate = useNavigate(); |
There was a problem hiding this comment.
Feels wider than it needs to be, and puts the evidence and artifacts at the bottom. Particularly once we/I move towards "artifacts as log", we should make this two-column (dynamic to single column when the screen is too narrow) imo, otherwise your burying all the rich data (diffs, tasks, artefacts) below this wall of text
| const { data: artefactsResp } = useInboxReportArtefacts(report.id, { | ||
| staleTime: 5 * 60 * 1000, | ||
| refetchOnWindowFocus: false, | ||
| }); |
There was a problem hiding this comment.
My impression is "runs" is only meant to show ongoing tasks? Seems like a bit of an arbitrary limitation imo, but given it, I'd call it "running" or "live" instead
| }); | ||
| const repoSlug = extractRepoSelectionRepository(artefactsResp?.results); | ||
| const firstSource = report.source_products?.[0]; | ||
| const sourceMeta = firstSource ? SOURCE_PRODUCT_META[firstSource] : null; |
There was a problem hiding this comment.
UI seems to be lying about there being only 100 items (what items? reports? runs?) "for me"
Also this feels like a weird place to put this, I thought it would be a tab of curated reports or something, not a filtering UI
| undefined, | ||
| { month: "short", day: "numeric" }, | ||
| ); | ||
| const isReady = report.status === "ready"; |
There was a problem hiding this comment.
Spacing between the "findings" header and the first finding is bad, count is in a pill but the rest of the ui uses round rects, count pill has a BG color that matches the rest of the UI but icon doesn't
Also feels weird to call them "findings" here but "evidence" in the PR version, unless I'm confused about what this is displaying (it's just a single, right?)
| const conventionalTitle = parseConventionalCommitTitle(report.title); | ||
| const cardTitle = displayConventionalCommitTitle( | ||
| report.title, | ||
| "Untitled report", |
There was a problem hiding this comment.
This header in general could be removed imo, but failing that, "this is your inbox" is really odd and should be removed
| ); | ||
|
|
||
| const openDetail = () => { | ||
| prefetch(); |
There was a problem hiding this comment.
No way to view suggested reviewers inside a report, even though we display them in the list view, and no way to modify suggested reviewers on a report OR a PR detail page (serious regression I'd actually block merging on)
Tighten the review-feedback commit before it lands: - AgentRunDetail uses InboxReportDetailGate (drops its own loading + missing-report fallback to match PR and Report detail). - PullRequestDetail's "Open in GitHub" handler stops reconstructing the URL from `parsePrUrl` parts; the parser already proves the source URL is a canonical GitHub PR URL. - useInboxReportDismissAction memoizes the closed-dialog "no reports" array to a stable identity so useInboxBulkActions' eligibility memo doesn't bust on every parent render of the detail screen. - useDiscussReport drops `pendingQuestion` state in favor of a ref; the question now travels through buildInput without retriggering the runner. - git.getPrDiffStatsBatch chunks into groups of 25 PRs and parallelizes the chunks with Promise.all so a single GraphQL request never exceeds GitHub's complexity budget on large list views. - usePrDiffStatsBatch sets explicit gcTime so distinct filter/search snapshots don't accumulate stat blobs in the React Query cache indefinitely. - PrDiffStatsBatchProvider is inlined: the context object and usePrDiffStatsFromBatch consumer hook live in utils/prDiffStatsBatchContext.ts, and the provider mounting collapses into a small PullRequestsBatchProvider colocated in PullRequestsTab. - Centralize team-config / user-autonomy / source-configs query keys as signalsConfigKeys in inboxQueryKeys.ts; mutation hooks import from there instead of inlining string-literal keys. - Flatten the 3-level empty-title ternary in InboxReportListTab into a resolveEmptyTitle helper with early returns. - Drop the unused lookupPrDiffStats export. - Trim narrate-the-refactor JSDoc on useInboxReports, useSignalSourceManager, useInboxCloudTaskRunner, useInboxReportDismissAction, and usePrDiffStatsBatch. Tests: 128 inbox + 27 git service pass; 0 typecheck errors.
…nce, do not merge)" This reverts commit 00e2eb9.
5 participants
Summary
Replaces the legacy Inbox list with the v2 Self-driving: Pull requests, Reports, Agents, plus scoped For you filtering.
Polished list/detail UI for PRs, reports, and agent runs, including repo/source metadata, reviewer affordances, and task-log-backed run details.
Promotes agent configuration into a first-class view.