Skip to content

feat(alerts): start a team's first insight alert on the alerts platform - #127840

Open
jzhu13 wants to merge 3 commits into
masterfrom
jimmy/insight-new-alerts-on-platform
Open

jzhu13 wants to merge 3 commits into
masterfrom
jimmy/insight-new-alerts-on-platform

Conversation

@jzhu13

@jzhu13 jzhu13 commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Warning

Hold the merge. It moves new insight alerts onto the alerts platform, so it waits for the customer email about check-timing changes, and for #127773 (the logs version) to merge.

Problem

Changes

  • A new insight alert now starts on the alerts platform unless its team has other insight alerts and none of them is on the platform.
  • A team with no other alert, and every new team, starts on the platform.
  • A team on the platform keeps getting new alerts there, as before.
  • A team whose alerts all stay on insight, such as one rolled back, keeps new alerts on insight.
  • Alerts the platform cannot run (real-time, investigation agent, child environments, no matching delivery path) still stay on insight.
  • Mechanical: has_begun_cutover becomes takes_new_alert, and tests that assumed a new team's alert stays on insight now create an insight-owned alert first.

How did you test this code?

Test rationale: extended test_a_new_alert_follows_its_team_and_starts_on_the_platform_in_a_team_without_alerts (renamed from test_a_new_alert_is_handed_over_only_in_a_team_on_the_platform) with a team that has no other alert. The old rule left that alert on insight.

  • Ran products/alerts/backend/, posthog/tasks/alerts/test/, the alert interval API tests, products/metrics/backend/tests/test_alerts.py, and repo-wide mypy locally.
  • Not checked: the dashboard and insight API tests that create alerts; CI covers them.

Release status

  • No feature flag controls this change

Docs update

  • None.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Claude Code, Opus 5.5 (claude-opus-5-5)

Skills: /writing-tests, /writing-code-comments, /writing-pr-descriptions

More context for AI agents
  • Decisions: the rule copies takes_new_alert from feat(logs): start a team's first logs alert on the alerts platform #127773 unchanged, so both sources answer "who owns a new alert" the same way.
  • Follow-ups: the shadow backfill gap for new teams' first alerts closes once this merges, because those alerts start on the platform.
  • Read closely: products/alerts/backend/platform_cutover.py takes_new_alert.

🤖 Generated with Claude Code

A new insight alert went to the platform only when its team already had a
platform-owned alert, so a team with no alerts, including every new team,
stayed on insight until someone cut it over.

The platform now owns a new insight alert unless the team has other insight
alerts and none of them is on the platform. A rolled-back team keeps new
alerts on insight, and alerts the platform cannot run still stay there.
This mirrors the logs rule.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jzhu13 jzhu13 self-assigned this Nov 2, 2026
@trunk-io

trunk-io Bot commented Nov 2, 2026

Copy link
Copy Markdown

Merging to master in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

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

@github-actions

github-actions Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

🤖 CI report

⚠️ Trunk lane — backend Python lane

This PR is assigned to the backend Python lane. It runs backend Python tests and may merge in parallel with PRs in other lanes.

✅ 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.

⚠️ Playwright — 1 flaky

🎭 Playwright report · View test results →

⚠️ 1 flaky test:

  • Inline editing insight title via compact card popover (chromium)

These issues are not necessarily caused by your changes.
Annoyed by this section? Help fix flakies and failures and it will go green!

@jzhu13
jzhu13 marked this pull request as ready for review November 3, 2026 01:05
@parameterai

parameterai Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Risk: No findings

This increment backdates a newly created platform-owned insight alert's shadow start (owner_changed_at) to the team's earliest existing platform cutover time, so the insight stack stops its redundant comparison checks for that alert when the team's week ends instead of starting a new week per alert. The platform copy still takes the real handover time, and delivery was already fully platform-owned, so the change only shortens a telemetry-only shadow window; I found no security impact in the new team_cutover_at query, the shadow_from plumbing, or their interaction with the shadow/rollback paths.

Sentinel reviewed 40c1776 · Review settings

@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested review from a team November 3, 2026 01:06
@jzhu13 jzhu13 added the stamphog Request AI approval (no full review) label Nov 3, 2026

@stamphog stamphog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not approved — this change needs a human reviewer.

Re-add the stamphog label to request another review once you have addressed this.

The risky creation-path change has owning-team authorship, and no substantive review concerns are present; the trusted context also reports a clean current-head agent assessment. The explicit release hold remains outstanding: ask @jzhu13 to confirm its prerequisites and lift it before re-requesting.

  • Author wrote 67% of the modified lines and has 24 merged PRs in these paths (familiarity MODERATE).
  • PostHog Review (Standard) reviewed the current head: no findings.
  • Outstanding maintainer hold: @jzhu13, an owning-team member, explicitly marks this PR as waiting for customer communication and PR #127773; no withdrawal is present. Have @jzhu13 confirm those prerequisites are complete and explicitly lift the hold before re-requesting.
Gate mechanics and policy version
Gate Result
prerequisites ✓ all clear
deny-list ✓ no deny categories matched
size ✓ 26L, 2F substantive, 86L/10F incl. docs/generated/snapshots — within ceiling
tier ✓ T1-agent / T1c-medium (86L, 10F, two-areas, feat)
stamphog 2.7.0 .stamphog/policy.yml @ 5c12e51 · reviewed head 5c12e51

@stamphog stamphog Bot removed the stamphog Request AI approval (no full review) label Nov 3, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@veria-ai

veria-ai Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

Reviews

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@posthog posthog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Agent review by Daniel's review agent - this is not Daniel's review.
Daniel has not read this PR yet. Treat these comments as automated findings, not as his opinion.

Result: no findings
Skills used: rs-review-pr (teammate mode), rs-adversarial-review, review-hog-perspective-logic-correctness, review-hog-perspective-performance-reliability
Head reviewed: 40c1776

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants