Repository navigation
fix(signals): stop the inbox diff failure toast, keep the real reason - #92006
andrewm4894 wants to merge 1 commit into
Conversation
The report detail's diff panel already renders its own error line, so a failed diff load also raised a toast saying "Load report diff failed: URL not found". The endpoint returns its reason under `error`, which the generic handler does not read, so a 404 fell back to the meaningless "URL not found". Suppress the toast for `loadReportDiff` (as `loadPrChecks` and `loadPrComments` already do), and let the inline error use the endpoint's own message. Generated-By: PostHog Desktop Task-Id: 430dff05-3507-4dea-a501-81891e2cd461
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
|
Hey @andrewm4894! 👋 It looks like your git author email on this PR isn't your
You can fix it for this repo with: git config user.email "you@posthog.com"Or set it globally with |
🤖 CI report✅ Trunk lane — non-backend laneThis PR is assigned to the non-backend lane. It does not run backend Python tests and may merge in parallel with PRs in other lanes.
|
| Root | Eager (shipped) | Δ vs base | Budget |
|---|---|---|---|
entry (logged-out pages, app bootstrap)src/index.tsx |
1.40 MiB · 22 files | no change | ███░░░░░░░ 31.2% of 4.51 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
8.83 MiB · 3,245 files | no change | █████████░ 90.9% of 9.71 MiB |
🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
Largest files eagerly shipped from src/index.tsx
| Size | File |
|---|---|
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 24.6 KiB | ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js |
| 6.3 KiB | ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js |
| 4.5 KiB | ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js |
| 3.9 KiB | ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js |
| 1.4 KiB | ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js |
| 1.3 KiB | src/RootErrorBoundary.tsx |
| 912 B | ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js |
| 789 B | src/scenes/ChunkLoadErrorBoundary.tsx |
| 762 B | src/index.tsx |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 307.0 KiB | ../node_modules/.pnpm/posthog-js@1.422.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/rrweb.js |
| 267.7 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 263.5 KiB | ../node_modules/.pnpm/posthog-js@1.422.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.js |
| 252.9 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 154.2 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 104.7 KiB | src/lib/api.ts |
| 95.2 KiB | ../packages/quill/packages/quill/dist/index.js |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 90.6 KiB | ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js |
Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479
✅ Toolbar bundle — eager 2.26 MiB within budget
What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.
| Metric | Size | Δ vs base | Budget |
|---|---|---|---|
| Eager (shipped) entry + static imports |
2.26 MiB · 18 files | no change | ████░░░░░░ 39.5% of 5.72 MiB |
| Deferred (lazy) | 2.11 MiB · 45 files | no change | n/a — loads on demand |
Loader dist/toolbar.js |
1.1 KiB | no change | █░░░░░░░░░ 5.8% of 19.5 KiB |
Largest eagerly-shipped chunks
| Size | File |
|---|---|
| 750.7 KiB | dist/toolbar/toolbar-app-HC2BLVR5.css |
| 588.3 KiB | dist/toolbar/chunk-chunk-UZOF23P7.js |
| 484.7 KiB | dist/toolbar/chunk-chunk-OQ4CRAT2.js |
| 134.1 KiB | dist/toolbar/chunk-chunk-ESP3L22S.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-FDH2IBXT.js |
| 71.3 KiB | dist/toolbar/toolbar-app-PMPWTAOL.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-TSAL54PB.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-XNAOAA7H.js |
| 21.0 KiB | dist/toolbar/chunk-chunk-U6ERKZIE.js |
| 6.8 KiB | dist/toolbar/chunk-chunk-DV7IWQNF.js |
Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile
✅ Dist folder size — 🔺 +879 B (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1442.90 MiB · 🔺 +879 B (+0.0%)
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
🦔 Hogbox preview · ❌ build failedThe preview didn't come up for commit Previews are optional and never block merging. A failure here is often a hogland or tailnet hiccup rather than anything in your PR, so the check stays green and this comment is the status. |
|
Reviews (1): Last reviewed commit: "fix(signals): stop the inbox diff failur..." | 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: 5b6504a15a
ℹ️ 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".
| loadReportDiffFailure: (_state: string | null, { errorObject }: { errorObject?: any }) => | ||
| typeof errorObject?.data?.error === 'string' && errorObject.data.error | ||
| ? errorObject.data.error | ||
| : "Couldn't load the diff. The branch may have been merged, deleted, or rewritten.", |
There was a problem hiding this comment.
When GitHub rate-limits this endpoint, github_rate_limited_response returns a 429 body with the reason in detail, not error. Because this commit also suppresses the generic loadReportDiff toast, the reducer now discards that reason and shows the misleading fallback that the branch may have been merged, deleted, or rewritten. Check errorObject.data.detail (or the normalized API error message) before using the fallback.
Useful? React with 👍 / 👎.
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Not approved — this change needs a human reviewer.
Re-add the stamphog label to request another review once you have addressed this.
Low-risk frontend fix, but Codex's unresolved inline comment identifies a real regression: suppressing the toast while the reducer only reads the error key means a GitHub rate-limit response (which carries its reason under detail) now silently falls back to the generic guess message instead of surfacing the rate-limit reason anywhere.
- Author wrote 0% of the modified lines and has 162 merged PRs in these paths (familiarity MODERATE).
- chatgpt-codex-connector[bot] reviewed the current head.
- Unresolved P2 inline comment from @chatgpt-codex-connector on inboxReportDetailLogic.ts: the new reducer only checks errorObject.data.error, so a 429 rate-limit response (which carries its reason in
detail) loses that information now that the toast is also suppressed.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 12L, 2F substantive, 48L/3F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1b-small (48L, 3F, two-areas, fix) |
| stamphog 2.0.0b4 | .stamphog/policy.yml @ 5b6504a · reviewed head 5b6504a |
|
To use Codex here, create a Codex account and connect to github. |
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, please remove the |
|
This PR was closed due to lack of activity. Feel free to reopen if it's still relevant. |
Problem
Anyone reading a report in the Self-driving inbox gets a red toast saying
Load report diff failed: URL not foundwhen the "Files changed" diff can't be fetched.Two things are wrong with it:
error, but the generic kea-loaders handler only reads DRF'sdetail, so a 404 falls back to "URL not found" — even though the response said the branch or repository was not found on GitHub.A merged-and-deleted branch is normal, so this fires on healthy reports.
Changes
loadReportDiffjoinsloadPrChecksandloadPrCommentson the toast allowlist, since the same panel already surfaces the failure.How did you test this code?
Added one kea logic case in
inboxReportDetailLogic.test.ts: a 404 whose body carrieserrormust reachreportDiffError. It catches a reducer reading the wrong key — the exact bug here — and was confirmed to fail against the previous reducer before the fix.products/signals/frontend/inbox/logics/inboxReportDetailLogic.test.tspasses locally.Not verified in a running app: the toast suppression is a one-line allowlist entry matching two neighbours that already work this way.
Automatic notifications
Docs update
None.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Reported from a Slack thread as a recurring toast in the inbox, then traced from the toast string back through
initKea's loader failure handler to the signals artefact diff endpoint. Left unassigned because no GitHub handle for the reporter was verified in the session — please assign the reporter on triage.Tools: PostHog Slack app (Claude Code harness) with repo search, file edits, Jest. Repo skills consulted:
writing-tests(gate for the added case).The narrower fix — suppressing the toast only — would have left the inline message guessing at a cause the response already stated, so the reducer now prefers the endpoint's wording.
No customer data, thread contents, or internal material is present in the diff or this description.