Repository navigation
Conversation
|
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 |
|
Important Review skippedWe couldn't safely recover the incremental review. No full review was started, and the last reviewed checkpoint was preserved. Retry later, or explicitly request a full review by commenting You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe change adds Priority: ⬇️ Low Merge Risk: 🔵 Low · up to The gallery can recommend a template that users cannot activate, and its onboarding and schedule guidance is inaccurate. Other templates remain available, so the impact is bounded; fix these issues before expanding the feature. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The gallery reuses the existing draft editor rather than executing workflows directly. Risk is bounded, but an email template’s audience changes outside the gallery flag, so disabling the gallery does not fully roll back that change. 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 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.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (2)
products/workflows/backend/templates/onboarding_started_but_not_completed_template.json-4-8 (1)
4-8: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
starts_on.detailsays/onboarding, but the trigger filters onmessaging/campaigns.The card shows "on /onboarding". The trigger at lines 37-40 and lines 131-134 matches
$current_url icontains "messaging/campaigns". If a user picks this template, the workflow starts on a different page than the card says. Make the two agree. Either change the trigger value to/onboarding, or changedetailto describe the real filter.products/workflows/backend/templates/trial-ending-reminder.json-4-8 (1)
4-8: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRemove the unsupported three-day detail.
The first-run card displays “ends in 3 days” beside
$pageview. This workflow waits 12 days after its trigger, then sends an email described as a two-day reminder. The card’s timing conflicts with the configured sequence.Suggested fix
- "detail": "ends in 3 days" + "detail": ""
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
- Review profile: QUIET
- Plan: Enterprise
- Run ID:
d2252ca5-4264-402c-952f-5581cf12cf94
⛔ Files ignored due to path filters (1)
products/workflows/frontend/generated/api.schemas.tsis excluded by!**/generated/**
📒 Files selected for processing (31)
frontend/src/lib/constants.tsxproducts/workflows/backend/presentation/views/hog_flow_template.pyproducts/workflows/backend/templates/announce_a_new_feature_template.jsonproducts/workflows/backend/templates/celebrate-milestone.jsonproducts/workflows/backend/templates/feature-adoption-tips.jsonproducts/workflows/backend/templates/heavy-usage-detected.jsonproducts/workflows/backend/templates/high-intent-notification.jsonproducts/workflows/backend/templates/negative-survey-response.jsonproducts/workflows/backend/templates/onboarding_started_but_not_completed_template.jsonproducts/workflows/backend/templates/re-engagement-workflow.jsonproducts/workflows/backend/templates/repeated-failure-alert.jsonproducts/workflows/backend/templates/support-ticket-notification.jsonproducts/workflows/backend/templates/ticket-unresolved-alert.jsonproducts/workflows/backend/templates/trial-ending-reminder.jsonproducts/workflows/backend/templates/trial_started_upgrade_nudge_template.jsonproducts/workflows/backend/templates/unlocking_advanced_features.jsonproducts/workflows/backend/templates/unused-features-education.jsonproducts/workflows/backend/templates/user-stuck-onboarding.jsonproducts/workflows/backend/templates/user-upgrade-webhook.jsonproducts/workflows/backend/templates/welcome_email_sequence_template.jsonproducts/workflows/backend/tests/api/test_hog_flow_template.pyproducts/workflows/frontend/Workflows/templates/WorkflowTemplateCard.tsxproducts/workflows/frontend/WorkflowsScene.tsxproducts/workflows/frontend/emptyState/workflowsEmptyState.tsxproducts/workflows/frontend/firstRun/GalleryTemplateCard.tsxproducts/workflows/frontend/firstRun/WorkflowsFirstRun.stories.tsxproducts/workflows/frontend/firstRun/WorkflowsFirstRunGallery.tsxproducts/workflows/frontend/firstRun/WorkflowsFirstRunTab.tsxproducts/workflows/frontend/firstRun/firstRunGalleryLogic.test.tsproducts/workflows/frontend/firstRun/firstRunGalleryLogic.tsservices/mcp/src/api/generated.ts
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 3 remain after this review.
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (1)
products/workflows/frontend/firstRun/TemplateStartsOnLine.tsx-25-25 (1)
25-25: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGuard against an empty
eventslist on non-schedule templates.
startsOn.events[0]isundefinedwhen aneventorno_eventtemplate has an emptyeventsarray and nomatchedEvent.EventChipthen receivesundefined, andPropertyKeyInforenders an empty chip. The API type does not forbid this shape. Onlyscheduleis documented as empty. Render the chip only when an event name exists.Proposed fix
- <EventChip event={matchedEvent ?? startsOn.events[0]} /> + {(matchedEvent ?? startsOn.events[0]) && <EventChip event={(matchedEvent ?? startsOn.events[0])!} />}
🧹 Nitpick comments (1)
products/workflows/frontend/firstRun/firstRunGalleryLogic.ts (1)
257-266: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winGallery-shown analytics can fire on a failed or repeated load.
loadSeenEventsSuccessfiresworkflows first run gallery showneach time it runs. A retry throughloadEmailTemplatesre-triggers the chain, so the event is captured again for the same visit. Also,loadSeenEventsruns with an empty list when there are no templates, and the event still fires withready_count: 0. Both cases skew funnel counts.Capture the event once per mount, for example with a flag reducer or a guard on the first success.
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
- Review profile: QUIET
- Plan: Enterprise
- Run ID:
89c80bef-3bb9-40ce-9ed8-2fda13cae006
📒 Files selected for processing (7)
products/workflows/frontend/firstRun/GalleryTemplateCard.tsxproducts/workflows/frontend/firstRun/TemplateStartsOnLine.tsxproducts/workflows/frontend/firstRun/WorkflowsFirstRun.stories.tsxproducts/workflows/frontend/firstRun/WorkflowsFirstRunGallery.tsxproducts/workflows/frontend/firstRun/WorkflowsFirstRunTab.test.tsxproducts/workflows/frontend/firstRun/firstRunGalleryLogic.test.tsproducts/workflows/frontend/firstRun/firstRunGalleryLogic.ts
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 5 remain after this review.
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (1)
products/workflows/frontend/firstRun/WorkflowsFirstRunGallery.tsx-96-96 (1)
96-96: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDescribe schedule templates accurately.
When no project events have arrived, schedule templates are not ready either. They do not start on the events named in this subtitle. Use wording that covers both event-based and schedule templates. As per coding guidelines, “Use the names users see, and stay consistent across every surface.”
Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
- Review profile: QUIET
- Plan: Enterprise
- Run ID:
b4b35d06-82c1-4e8e-973e-66c9ec1ee312
📒 Files selected for processing (5)
products/workflows/frontend/firstRun/WorkflowsFirstRun.stories.tsxproducts/workflows/frontend/firstRun/WorkflowsFirstRunGallery.tsxproducts/workflows/frontend/firstRun/WorkflowsFirstRunTab.tsxproducts/workflows/frontend/firstRun/firstRunGalleryLogic.test.tsproducts/workflows/frontend/firstRun/firstRunGalleryLogic.ts
💤 Files with no reviewable changes (1)
- products/workflows/frontend/firstRun/WorkflowsFirstRun.stories.tsx
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 7 remain after this review.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
🟡 Minor · Keep this template out of the ready gallery until its workflow… · re-engagement-workflow.json:4-8
products/workflows/backend/templates/re-engagement-workflow.json:4-8
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep this template out of the ready gallery until its workflow is runnable.
When
$pageviewhas been observed, this new metadata puts the template in “Picked for your data.” Picking it opens the template editor, but activating the saved draft fails because its event trigger has empty filters. If the trigger alone is fixed, the 14-day condition still matchesTRUEand routes toexit_node, so the email is skipped. Remove this metadata until the trigger and activity check are valid.Suggested fix
- "starts_on": { - "kind": "no_event", - "events": ["$pageview"], - "detail": "for 14 days" - },
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
- Review profile: QUIET
- Plan: Enterprise
- Run ID:
d559fd78-0b5c-4903-9f8b-fec3f92144b1
📒 Files selected for processing (3)
products/workflows/frontend/firstRun/GalleryTemplateCard.tsxproducts/workflows/frontend/firstRun/WorkflowsFirstRunGallery.tsxproducts/workflows/frontend/firstRun/firstRunGalleryLogic.test.ts
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 3 remain after this review.
6fefaff to
40f19ba
Compare
|
Note 🤖 GPT-6.1 Sol responding on behalf of Michael Replaced by the draft fork PR #329 in its native fork stack. @meikelmosby will start migrating the stack from the fork into PostHog/posthog for upstream review and CI. Closing this master-targeted copy to keep the handoff clear. |
Note
Stacked on #111726, which adds
starts_onto the global templates. This PR carries that commit. Review the commits after it.Problem
Note
🔵 Merge risk: low (merge-risk rubric)
workflows-first-runon.WorkflowsScenerendersWorkflowsTableand the gate decides as before. Logic and render tests cover the switch and the fit rules.Changes
workflows-first-runflag, a project with no workflows sees an email template gallery on the Workflows tab instead of the empty state.createWorkflowFromTemplatepath, until the "Make it yours" screen lands.event_definitionsrequest over everystarts_onevent. Only definitions with alast_seen_atcount, so a planned event does not make a template ready.bypassFeatureFlaghands the tab back to the scene under the flag, so the scene tabs stay reachable.?empty_state=1still shows the old screen.workflows first run gallery shown(ready_count,recommended_template_id) andworkflows first run template picked(template_id,recommended,ready).highlightedprop onWorkflowTemplateCard, and the flag constant.signed_upand pageviewsHow did you test this code?
Test seams:
firstRunGalleryLogic.test.tsdrives the gallery logic with templates and event definitions mocked at the HTTP layer.ingested_eventalone.starts_onevent in a single request.WorkflowsFirstRunTab.test.tsxrenders the tab and checks that a project without workflows gets the gallery and one with a workflow gets the table.WorkflowsFirstRun.stories.tsxrenders the whole app at/workflowsfor the three projects and a 520px scene. The screenshots above come from those stories in a headless browser.Release status
Automatic notifications
Docs update
None. The flag is off.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Agent: Claude Code, Claude Opus 5.5. Review panel: GPT-6.1 Sol and Claude Opus 5.5.
tdd,qa-swarm,merge-risk,/placing-product-frontend-code,/writing-kea-logics,/writing-ui-components,/writing-tests,/writing-user-facing-copy,/writing-code-comments,/writing-pr-descriptions,/reviewing-with-coderabbit.workflowTemplatesLogic.gh pr list --search "workflows first run gallery"found nothing related.🤖 Generated with Claude Code