Repository navigation
docs(flags): add the flag_evaluations parity spot check runbook - #92959
Conversation
Generated-By: PostHog Desktop Task-Id: 18fa3ea7-a7e2-4fa4-b891-c30a92e4719d
|
😎 Merged successfully - details. |
🤖 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. |
|
Reviews (1): Last reviewed commit: "docs(flags): add the flag_evaluations pa..." | Re-trigger Greptile |
There was a problem hiding this comment.
🟡 Changes recommended
The runbook includes an agent-style /metabase-prod-query instruction that is likely unclear for a human “run by hand” workflow and should be replaced with direct Metabase guidance.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds an internal runbook documenting how to manually spot-check parity between $feature_flag_called events in events and rows in flag_evaluations before widening the ingestion fork allowlist.
Changes:
- Introduces a step-by-step parity spot check guide (inputs, query, interpretation of deficit/excess).
- Captures known correctness pitfalls (e.g.,
$feature_flagJSON typing, grouping by(team_id, uuid), and ingestion-lane gating). - Links the runbook to the table family/schema definition (
sql.py) for context.
File summaries
| File | Description |
|---|---|
| posthog/models/flag_evaluations/PARITY_CHECK.md | New Markdown runbook describing how to run and interpret a flag_evaluations parity spot check query. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Approved.
Pure documentation addition (a runbook markdown file) with no code, config, or dependency changes; content matches its .md extension and carries no executable payload.
- 👍 on the PR from greptile-apps[bot].
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 0L, 0F substantive, 105L/1F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1c-medium (105L, 1F, single-area, docs) |
| stamphog 2.0.0b4 | .stamphog/policy.yml @ e9c06f1 · reviewed head e9c06f1 |
Generated-By: PostHog Desktop Task-Id: 18fa3ea7-a7e2-4fa4-b891-c30a92e4719d
A new stamphog review started for this PR — the fresh verdict replaces this approval.
There was a problem hiding this comment.
Approved.
Pure documentation addition (new runbook markdown file, no code changes), well outside risky territory; the lone reviewer comment is a minor clarity nit on an outdated commit, not a blocking concern.
- 👍 on the PR from greptile-apps[bot].
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 0L, 0F substantive, 117L/1F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1c-medium (117L, 1F, single-area, docs) |
| stamphog 2.0.0b4 | .stamphog/policy.yml @ ac9cbbd · reviewed head ac9cbbd |
Problem
Widening the flag_evaluations fork to more teams is gated on a parity check: does every row the fork writes also reach the events table, and vice versa. Nothing records how to run that check, so the next person widening the allowlist has no way to find it.
The fork's own counters increment on the Kafka ack, so they see nothing past it. The Kafka engine table, the materialized view, the writable Distributed table, and the shard are all unobserved, and
kafka_flag_evaluationssetskafka_skip_broken_messages, so a malformed row is discarded after a successful produce. Only comparing the two tables catches that.Changes
posthog/models/flag_evaluations/PARITY_CHECK.md, a runbook for running the check by hand through Metabase.JSONTyperejecting a numeric$feature_flag, grouping by(team_id, uuid)before counting, the memory ceilings needed because this runs on the cluster serving customer queries, and delayed lanes never forking.How did you test this code?
Not applicable: no code changed. Verified the runbook's claims against the source it describes, in particular the lane gate in
nodejs/src/ingestion/common/flag-evaluations/flag-evaluations-service.ts, which returnsundefinedfor any lane outsideREALTIME_INGESTION_LANESand is why delayed lanes never fork.Automatic notifications
Docs update
None. This PR is the docs update.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Written by Claude Code in PostHog Desktop. The user supplied the query's shape, the correctness gotchas, and pointers to PR #92749 and a private note; the note's prod row counts, query costs, and team ids were kept out of this public repo. Skills invoked:
/writing-user-facing-copy,/writing-pr-descriptions.During review the user flagged that an earlier draft's "delayed lanes never fork" claim was carried from the note without checking the code. Tracing
createFlagEvaluationsServiceshowed the lane restriction is enforced inflag-evaluations-service.ts, not left to deployment config, so the runbook was rewritten to cite that gate and its stated reason: a backfill owns this table's history, so a delayed lane forking too would double-write rows the backfill already covers.Nothing in this PR carries material from the session that is not already public.
Created with PostHog Desktop