fix(workflows): move a new broadcast onto its own URL once its draft is saved - #107557
Conversation
|
😎 Merged successfully - details. |
🤖 CI report✅ Trunk lane — non-backend lane (
|
| Function | Location | Complexity | Limit |
|---|---|---|---|
launchBroadcast |
products/workflows/frontend/Broadcasts/broadcastWizardLogic.ts:1190 |
22 | 10 |
<anonymous> |
products/workflows/frontend/Broadcasts/broadcastWizardLogic.ts:823 |
17 | 10 |
setEmail |
products/workflows/frontend/Broadcasts/broadcastWizardLogic.ts:1021 |
12 | 10 |
moveToDraft |
products/workflows/frontend/Broadcasts/broadcastWizardLogic.ts:1324 |
11 | 10 |
✅ Duplication (Python) — clean
New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.
✅ Duplication (TypeScript) — clean
New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.
⚠️ Comment density — 5% of added code lines are comments (12 of 234)
This section warns when comments are more than 3% of the code lines a PR adds, and alerts above 6%. Before agent-assisted PRs, the typical share was about 2%. Only full-line comments count. Docstrings, generated files, snapshots, migrations, and workflow files are left out.
Comments that restate the code, record how the change came about, or narrate the next line add noise for the next reader. Keep the comments that explain a reason the code cannot show, and remove the rest. See .agents/skills/writing-code-comments/SKILL.md for the house rules.
Files with the most added comment lines:
| File | Comment lines | Added lines |
|---|---|---|
products/workflows/frontend/Broadcasts/broadcastWizardLogic.ts |
12 | 65 |
This check does not block merging. It updates on every push and clears when the share drops.
⚠️ Bundle size — 🔺 +36.6 KiB (+0.1%)
Uncompressed size of every built .js bundle, compared against the base branch.
Total: 68.92 MiB · 🔺 +36.6 KiB (+0.1%)
| File | Size | Δ vs base |
|---|---|---|
posthog-app/_parent/products/business_knowledge/frontend/scenes/BusinessKnowledgePlaygroundScene.js |
26.2 KiB | 🔺 +26.2 KiB (new) |
render-query/src/render-query/render-query.js |
20.25 MiB | 🟢 -19.2 KiB (-0.1%) |
posthog-app/_parent/products/workflows/frontend/Workflows/WorkflowScene.js |
50.7 KiB | 🔺 +11.2 KiB (+28.4%) |
posthog-app/_parent/products/workflows/frontend/TemplateLibrary/MessageTemplate.js |
30.8 KiB | 🔺 +5.6 KiB (+22.2%) |
posthog-app/_parent/products/signals/frontend/inbox/InboxScene.js |
467.4 KiB | 🔺 +1016 B (+0.2%) |
Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report
✅ Eager graph — within budget
How much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy import() / React.lazy chunks are not counted.
| Root | Eager (shipped) | Δ vs base | Budget |
|---|---|---|---|
entry (logged-out pages, app bootstrap)src/index.tsx |
1.57 MiB · 22 files | 🔺 +1.3 KiB (+0.1%) | █████████░ 85.4% of 1.84 MiB |
logged-out boot: index + App + bootApp (preloaded by every page, including /login)src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts |
3.56 MiB · 629 files | 🟢 -21.8 KiB (-0.6%) | █████████░ 88.4% of 4.03 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
7.37 MiB · 2,330 files | 🟢 -19.0 KiB (-0.3%) | █████████░ 88.3% of 8.34 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/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/layout/navigation-3000/navigationLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/scenes/dashboard/dashboardLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/lemon-ui/LemonMarkdown/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/RichContentEditor/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/CodeSnippet/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/taxonomy/core-filter-definitions-by-group.json stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 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
🟢 src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts 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
🟢 [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/index.tsx |
| 1.3 KiB | src/RootErrorBoundary.tsx |
| 912 B | ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js |
| 854 B | src/scenes/ChunkLoadErrorBoundary.tsx |
Largest files eagerly shipped from src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
| Size | File |
|---|---|
| 301.8 KiB | ../node_modules/.pnpm/posthog-js@1.434.14_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 267.6 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 |
| 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 |
| 100.4 KiB | src/lib/api.ts |
| 88.4 KiB | src/products.tsx |
| 69.4 KiB | src/lib/lemon-ui/icons/icons.tsx |
| 40.1 KiB | src/lib/utils/eventUsageLogic.ts |
| 38.7 KiB | ../node_modules/.pnpm/@dnd-kit+core@6.0.8_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@dnd-kit/core/dist/core.esm.js |
| 33.9 KiB | ../node_modules/.pnpm/kea@4.0.0-pre.6_patch_hash=139b8d1f1304f9d9da452a9a1244c94ea679dbcb85687d8999563146879fb6f5_react@18.3.1/node_modules/kea/lib/index.cjs.js |
| 28.4 KiB | src/scenes/scenes.ts |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 301.8 KiB | ../node_modules/.pnpm/posthog-js@1.434.14_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 271.7 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 267.6 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 |
| 153.7 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 |
| 100.4 KiB | src/lib/api.ts |
| 98.5 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 |
| 88.4 KiB | src/products.tsx |
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.38 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.38 MiB · 19 files | 🔺 +1.4 KiB (+0.1%) | ████░░░░░░ 41.5% of 5.72 MiB |
| Deferred (lazy) | 2.10 MiB · 44 files | no change | n/a — loads on demand |
Loader dist/toolbar.js |
1.2 KiB | no change | █░░░░░░░░░ 6.0% of 19.5 KiB |
Largest eagerly-shipped chunks
| Size | File |
|---|---|
| 798.3 KiB | dist/toolbar/toolbar-app-QRKUYVPZ.css |
| 650.9 KiB | dist/toolbar/chunk-chunk-ZLZUDQLU.js |
| 483.6 KiB | dist/toolbar/chunk-chunk-6JFSEK3E.js |
| 138.3 KiB | dist/toolbar/chunk-chunk-SL4WBWFJ.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-FDH2IBXT.js |
| 75.2 KiB | dist/toolbar/toolbar-app-BCYDYBUD.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-TSAL54PB.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-4NNH3FVO.js |
| 21.0 KiB | dist/toolbar/chunk-chunk-GRXIOXBW.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 — 🔺 +289.4 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 945.70 MiB · 🔺 +289.4 KiB (+0.0%)
|
[Medium risk] Adds URL navigation when a new broadcast draft is saved. The PR is not yet safe to merge because a failed email autosave can leave a newly created draft on Reviews (2) · Last reviewed commit: "fix(workflows): keep an email edit made ..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAfter a new broadcast draft is saved, the wizard replaces the URL with the saved broadcast URL and current step when no email edit is pending. Email autosaves and Continue also trigger this URL update after saving. The wizard tracks email-edit generations so an older save does not clear the pending state of a newer edit. Launch error and audience-limit paths also trigger the URL update. During draft hydration, the wizard resumes at a valid step from the URL and removes that parameter while preserving other search and hash parameters. If the URL has no valid step, the wizard resumes at the first incomplete step or review. Priority: ⬇️ Low Merge Risk: 🟡 Moderate · up to After a launch conflict, reloading can open a new broadcast wizard instead of the saved draft. Update the URL on that path before merging. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change is confined to how the broadcast wizard moves to and restores an already saved draft. No new server endpoint or permission is evident. The main remaining uncertainty is how repeated creation requests are handled. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟠 Major · Update the URL after launch creates a draft. · broadcastWizardLogic.ts:1198
products/workflows/frontend/Broadcasts/broadcastWizardLogic.ts:1198
🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUpdate the URL after launch creates a draft.
If
launchBroadcastcreates a draft and a later preview, schedule, or activation request fails, the page remains at/broadcasts/new. A reload then starts a new wizard instead of reopening the saved draft. CallshowSavedDraftUrlafter storing the draft, before the later requests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 5f8c70d1-66c9-45ad-95bf-bdb165720435
📒 Files selected for processing (1)
products/workflows/frontend/Broadcasts/broadcastWizardLogic.ts
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 480f8f49-dbb6-45b1-9f3f-5aafeffe26d6
📒 Files selected for processing (2)
products/workflows/frontend/Broadcasts/broadcastWizardLogic.test.tsproducts/workflows/frontend/Broadcasts/broadcastWizardLogic.ts
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 8 remain after this review.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: dade2698-a918-42c5-b00c-0e29584f6681
📒 Files selected for processing (2)
products/workflows/frontend/Broadcasts/broadcastWizardLogic.test.tsproducts/workflows/frontend/Broadcasts/broadcastWizardLogic.ts
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 8 remain after this review.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
products/workflows/frontend/Broadcasts/broadcastWizardLogic.test.ts (1)
199-199: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTest the Continue race on the route-mounted wizard.
Arrange the test so an email edit is pending before
continueStep()while the route is still/broadcasts/new. Do not reset the route after the saved-draft transition. Assert that the route remains/broadcasts/newuntil the later edit saves.The current test keeps using the original logic after manually pushing
/broadcasts/new. Its final route assertion can pass even if navigation occurs before the later save.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 1cfe8ad5-e69b-4ce8-b703-f05d8299cc45
📒 Files selected for processing (2)
products/workflows/frontend/Broadcasts/broadcastWizardLogic.test.tsproducts/workflows/frontend/Broadcasts/broadcastWizardLogic.ts
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 7 remain after this review.
🦔 Hogbox preview · ✅ ready▶ Open the preview
commit |
Comments Outside DiffThese findings sit on lines the diff does not cover, so they could not be posted inline. Each one leaves this list once its file changes.
|
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Not approved — escalated to a human reviewer.
Re-add the stamphog label to request another review once you have addressed this.
The fix correctly moves the URL off /broadcasts/new once a draft saves via ensureDraft, setEmail, and Continue, with tests covering the associated races. But CodeRabbit flagged (Major) that launchBroadcast's early-failure paths — e.g. the audience-over-limit return, which sends the user back to the recipients step to fix filters — still leave the page on /broadcasts/new after the draft is created, reproducing the exact orphaned-draft-on-reload bug this PR is meant to fix. That comment is unaddressed in the current diff (launchBroadcast never calls the new showSavedDraftUrl action), and an unaddressed valid bot concern should not be auto-approved.
- Author wrote 100% of the modified lines and has 3 merged PRs in these paths (familiarity STRONG).
- coderabbitai[bot] reviewed the current head.
- CodeRabbit's Major comment on broadcastWizardLogic.ts (near launchBroadcast) — call showSavedDraftUrl after the draft is created in launchBroadcast, before the later blast-radius/schedule requests — is not addressed in the current diff, leaving a code path where a mid-launch failure (e.g. audience over the batch limit) orphans the draft on reload, the same bug class this PR fixes elsewhere.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 68L, 1F substantive, 190L/2F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1c-medium (190L, 2F, single-area, fix) |
| stamphog 2.2.0 | .stamphog/policy.yml @ da78dd0 · reviewed head da78dd0 |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: b7cf2d1c-96e5-4267-a7e7-a7c93516866f
📒 Files selected for processing (2)
products/workflows/frontend/Broadcasts/broadcastWizardLogic.test.tsproducts/workflows/frontend/Broadcasts/broadcastWizardLogic.ts
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 6 remain after this review.
There was a problem hiding this comment.
Not approved — escalated to a human reviewer.
Re-add the stamphog label to request another review once you have addressed this.
Greptile flagged a still-unaddressed P1 concern (autosave failure leaves emailEditPending true, so the wizard never moves off /broadcasts/new and the draft is orphaned on reload) — I traced the current code and this failure path still doesn't clear the pending flag or retry the URL move, so the bot's concern reads as valid and unresolved in the current diff.
- Author wrote 100% of the modified lines and has 3 merged PRs in these paths (familiarity STRONG).
- 👍 on the PR from greptile-apps[bot].
- Greptile (independent bot reviewer) raised a P1 'Autosave failure blocks draft navigation' concern that appears unresolved in the current head: the setEmail autosave's catch block only clears cache.emailEditPending for EditedElsewhereError, not for generic/network failures, so a transient save error can still leave a newly created draft stuck on /broadcasts/new and orphaned on reload — the exact bug class this PR is meant to fix, just narrowed to one edge case.
- This is not a regression versus master (master orphans drafts in more cases), but the unresolved bot concern should be addressed or explicitly rebutted before merge.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 72L, 1F substantive, 238L/2F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1c-medium (238L, 2F, single-area, fix) |
| stamphog 2.2.0 | .stamphog/policy.yml @ 8b547a8 · reviewed head 8b547a8 |
There was a problem hiding this comment.
Approved.
Frontend state/routing fix for the broadcast wizard, not risky territory (no auth, billing, migration, API contract, dependency, or CI surface); author is on the owning team with STRONG familiarity (100% of touched lines, 3 days ago) and added extensive new tests covering the race conditions. The diff shows the prior CodeRabbit "update URL after launch" concern and the Greptile autosave-blocking concern were both addressed with new showSavedDraftUrl calls and a retry mechanism; no active human reviewer, hold comment, or in-progress (👀) review is present.
- Author wrote 100% of the modified lines and has 3 merged PRs in these paths (familiarity STRONG).
- 👍 on the PR from greptile-apps[bot].
- If the email autosave keeps failing past the 3 retries, emailEditPending is never cleared and the wizard stays on /broadcasts/new indefinitely — same as pre-existing behavior, not a regression, and explicitly called out as untested-in-browser in the PR description.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 85L, 1F substantive, 274L/2F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1c-medium (274L, 2F, single-area, fix) |
| stamphog 2.2.0 | .stamphog/policy.yml @ 8802378 · reviewed head 8802378 |
This branch was successfully deployed
1 participant
Problem
/broadcasts/newuntil launch.Changes
/broadcasts/newwith/broadcasts/<id>?step=<current step>.stepfrom the URL. Without the parameter, a draft resumes at its first incomplete step, as before.How did you test this code?
Playwright against a local stack:
/broadcasts/new/broadcasts/<id>, still on the goal step/broadcasts/new/broadcasts/<id>on the content stepPOST201), on master and on this branch.POSTfor 6 seconds while the subject was typed on the content step./broadcasts/newthat stops at the audience limit or on an edit conflict, and for an autosave that fails once and then saves.Release status
Automatic notifications
Docs update
None.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Agent: Claude Code, Opus 5.5 (claude-opus-5-5)