Skip to content

fix(alerts): keep platform insight checks out of query single flights - #127919

Merged
trunk-io[bot] merged 3 commits into
masterfrom
claude/insight-single-flight-opt-out-97e1c5
Nov 3, 2026
Merged

trunk-io[bot] merged 3 commits into
masterfrom
claude/insight-single-flight-opt-out-97e1c5

Conversation

@jzhu13

@jzhu13 jzhu13 commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Problem

  • Platform insight alert checks fail with "This query was already running and its result couldn't be reused", and each failure is an errored check that notifies the owner.
  • The production check of the same alert never fails this way, so the errors start when a team moves to the platform.
  • Cause: the platform queries as its own ClickHouse user, and single_flight_variant puts that user in the flight partition. The cache entry has no such partition.
  • Sibling alerts on one insight follow one platform leader. The production check of one of them writes the same entry just after that leader.
  • The followers then find an entry that is not their leader's, and _serve_flight_outcome raises QueryRanConcurrently.

Changes

  • Platform insight checks now run their own query instead of following a flight, so this error no longer reaches alert owners.
  • A new skip_query_single_flight() context manager makes _joins_single_flight return false inside it. The platform enters it around each check.
  • No other caller enters the context, so legacy alert checks, dashboards, the query API and exports keep their single-flight behavior.
  • Rejected: letting any follower serve an entry newer than its leader's. That changes what every caller is served, and an experiment recalculation follower could get another window's result.

How did you test this code?

Test rationale:

  • A new skipped_by_the_caller case in test_runs_that_cannot_share_a_result_never_touch_the_flight puts the run behind a held flight. It catches the skip check being dropped from the runner. Without the fix it raised QueryRanConcurrently.
  • test_a_breach_fires_on_the_platform_and_leaves_the_production_alert_alone now checks the skip during the platform query. It catches the with being dropped from the platform path.
  • CodeRabbit CLI review of the branch: no findings.
  • CI report, comment density warning: the two added comment lines give reasons the code cannot show (why the platform skips, and the cross-user cache race), so they stay.
  • Not checked: a live run against two concurrent writers. The cause comes from production query logs and the single-flight counters.

Release status

  • No feature flag controls this change

Docs update

  • None.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

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

Skills: /writing-tests, /simplify, /code-review, /reviewing-with-coderabbit, /writing-pr-descriptions, /pr-sessions

More context for AI agents
  • Session: Claude Code desktop, read-only production evidence through Metabase and Grafana
  • Decisions: the brief's leading hypothesis (the legacy worker evaluates the single-flight flag as off) is wrong, because the legacy alert worker leads flights too
  • Decisions: a context-scoped opt-out instead of a kwarg, because the extractors are shared with legacy and a kwarg would thread through seven call sites
  • Follow-ups: other callers can still hit the cross-partition overwrite; a fix there needs the cache entry to carry its partition
  • Read closely: posthog/hogql_queries/query_runner.py _joins_single_flight, the only shared-runner change

🤖 Generated with Claude Code

jzhu13 and others added 2 commits November 2, 2026 17:03
A platform insight check and the production check of the same alert run
the same query at the same moment. They sit in different single-flight
partitions, because the partition includes the ClickHouse user, but they
write one cache entry. When production writes after the platform leader,
the platform followers find an entry that is not their leader's and fail
with QueryRanConcurrently.

Add skip_query_single_flight(), a context-scoped opt-out the runner checks
before it joins a flight, and enter it around each platform insight check.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Check the skip before the flag evaluation, name every other writer in the
platform comment, and simplify the test helper's side effect.

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

trunk-io Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

😎 Merged successfully - details.

@github-actions

github-actions Bot commented Nov 3, 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.

⚠️ Comment density — 3% of added code lines are comments (2 of 60)

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/alerts/backend/platform_source_cycle.py 2 12

This check does not block merging. It updates on every push and clears when the share drops.

✅ Playwright — all passed

All tests passed.

View test results →

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jzhu13
jzhu13 marked this pull request as ready for review November 3, 2026 01:31
@parameterai

parameterai Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Risk: No findings

This PR adds a context-scoped skip_query_single_flight() opt-out so the alerts platform's insight checks neither lead nor follow a query single flight, avoiding spurious QueryRanConcurrently errors caused by cross-partition flight/cache-entry interplay. The only shared-runner change is one added condition in _joins_single_flight, and the skip is correctly scoped (token-reset ContextVar, single production caller), so no auth, tenancy, or data-exposure surface is affected.

Sentinel reviewed 718fed3 · Review settings

@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team November 3, 2026 01:32
@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.

Approved.

The shared query-cache coordination change enters risky territory, with owning-team authorship providing assurance and focused tests covering the opt-out. No unresolved review concerns or other refusal grounds were found.

  • Author wrote 62% of the modified lines and has 24 merged PRs in these paths (familiarity MODERATE).
Gate mechanics and policy version
Gate Result
prerequisites ✓ all clear
deny-list ✓ no deny categories matched
size ✓ 48L, 3F substantive, 85L/5F incl. docs/generated/snapshots — within ceiling
tier ✓ T1-agent / T1b-small (85L, 5F, two-areas, fix)
stamphog 2.7.0 .stamphog/policy.yml @ 718fed3 · reviewed head 718fed3

@jzhu13

jzhu13 commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

/trunk merge

@trunk-io
trunk-io Bot merged commit 6c695f3 into master Nov 3, 2026
378 checks passed
@trunk-io
trunk-io Bot deleted the claude/insight-single-flight-opt-out-97e1c5 branch November 3, 2026 02:12
@deployment-status-posthog

deployment-status-posthog Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-11-03 02:37 UTC Run
prod-us ✅ Deployed 2026-11-03 02:47 UTC Run
prod-eu ✅ Deployed 2026-11-03 02:53 UTC Run

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

Labels

stamphog Request AI approval (no full review)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant