Skip to content

feat(flags): count projects tab evaluations from flag_evaluations - #110148

Merged
trunk-io[bot] merged 2 commits into
masterfrom
haacked/projects-tab-flag-evaluations
Oct 1, 2026
Merged

trunk-io[bot] merged 2 commits into
masterfrom
haacked/projects-tab-flag-evaluations

Conversation

@haacked

@haacked haacked commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Problem

  • Organizations that store flag calls only in the flag evaluations table would see "0 evals · 7d" for every project on a flag's Projects tab.
  • The tab counts $feature_flag_called rows in events.
  • New organizations are about to move to FLAG_EVALUATIONS_ONLY, where ingestion stops writing those rows to events.
  • Ingestion still writes both tables in that mode today, so this change lands before anything breaks.

Changes

  • The Projects tab reads its 7-day per-project counts from flag_evaluations when the organization is on FLAG_EVALUATIONS_ONLY.
  • Every other mode keeps reading events, so organizations on EVENTS see no change. That covers self-hosted and hobby installs.
  • The new query filters on team_id, flag_key and timestamp, which are the first three columns of the flag_evaluations sort key.
  • READ_FLAG_EVALUATIONS stays on events, unlike the Usage tab. The rollout keeps existing organizations unchanged, so only organizations whose events writes will stop switch tables.
  • The staff help text for the mode still says mode 2 acts as mode 1. A follow-up updates that wording once the other FLAG_EVALUATIONS_ONLY reader changes land.
  • The cache key names the source table. A mode change cannot serve a cached count from the other table, and EVENTS and READ_FLAG_EVALUATIONS share entries.
  • get_cached_evaluations_7d_by_team takes the organization id. One call never spans organizations, because the view passes only that organization's teams.
  • Each request adds one Postgres primary-key read for the mode, which is the new entry in the query snapshot.
  • Mechanical: a shared _create_flag_evaluations test helper in posthog/test/base.py.

How did you test this code?

Test rationale:

  • test_counts_flag_calls_by_team_from_the_selected_table extends test_counts_events_by_team. It fails if the flag_evaluations query counts other flag keys or rows older than 7 days, or reads the wrong table.
  • test_evaluation_counts_come_from_the_table_the_mode_selects extends test_evaluation_counts_match_events over all three modes. It fails if the view stops passing the organization, or if READ_FLAG_EVALUATIONS moves off events.
  • test_mode_change_does_not_serve_the_result_cached_under_the_previous_mode is new. It fails if the cache key stops naming the source table, which no other test catches.
  • Ran locally: posthog/test/test_feature_flag_analytics.py and products/feature_flags/backend/api/test/test_organization_feature_flag.py.
  • Not checked: the query plan against production-sized flag_evaluations data. No EXPLAIN was run.
  • New events schema: not needed. The query reads the events table with raw SQL, not through HogQL, and that path is unchanged.

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

Release status

  • No feature flag controls this change
  • This change is behind a feature flag and is not available to users
  • This change makes a previously flagged feature available to everyone

The organization's stored flag evaluations mode selects the table. No organization reaches FLAG_EVALUATIONS_ONLY without an explicit mode change.

Automatic notifications

  • Publish to changelog?

Docs update

None. No doc under docs/ covers this tab.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

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

  • Session: https://claude.ai/code/session_01Hqgf9Fx7M77d5BMEKbxQBG
  • Built from a written plan through the /go pipeline. The plan asked for decisions to be recorded here rather than raised as questions.
  • Skills invoked: /writing-tests, /writing-code-comments, /improving-drf-endpoints (no serializer or schema change was needed), /simplify, /reviewing-with-coderabbit, /writing-pr-descriptions.
  • CodeRabbit CLI: ran with --deep, because the diff adds raw ClickHouse SQL that filters by team. It returned 0 findings.
  • A spec-only subagent wrote the first tests before it saw the implementation. The simplify pass then moved the table decision from the SQL builder into the cached function, so the cache key could name the table.
  • No duplicate: an open-PR search for "flag_evaluations projects tab" and "evaluations_7d" found nothing.
  • Public artifact: the diff carries no material from outside this repository. All test data is invented.

https://claude.ai/code/session_01Hqgf9Fx7M77d5BMEKbxQBG

Organizations on FLAG_EVALUATIONS_ONLY read the 7-day per-project eval
counts from flag_evaluations. Every other mode keeps reading events. The
cache key names the source table, so a mode change cannot serve a count
read from the other table.

Claude-Session: https://claude.ai/code/session_01Hqgf9Fx7M77d5BMEKbxQBG
@haacked haacked added the skip-inkeep-docs Use this label to skip an Inkeep docs PR in posthog.com label Oct 1, 2026
@haacked haacked self-assigned this Oct 1, 2026
@trunk-io

trunk-io Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

😎 Merged successfully - details.

@posthog

posthog Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

🦔 PostHog Review reviewed this pull request

Nothing worth raising this time. Enjoy the moment:

Someone relaxing in a sunny garden

@github-actions

github-actions Bot commented Oct 1, 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 — 6% of added code lines are comments (7 of 122)

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/feature_flags/backend/facade/enums.py 4 4
products/feature_flags/backend/flag_analytics.py 2 35
posthog/test/base.py 1 26

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 →

@greptile-apps

greptile-apps Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Retrigger

[Medium risk] Changes how the projects tab counts flag evaluations.

The PR appears safe to merge; no outstanding finding or new actionable issue remains.

Reviews (2) · Last reviewed commit: "chore(flags): explain why read_flag_eval..."

Copilot AI 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.

Copilot review overview

🟢 Approval recommended

Source selection, cache isolation, tenant scoping, and regression coverage are consistent and complete.

Review effort: Balanced
Findings: None

What changed in this PR

Updates cross-project feature flag evaluation counts for FLAG_EVALUATIONS_ONLY organizations.

Changes:

  • Selects the appropriate ClickHouse source by organization mode.
  • Separates caches by source table.
  • Adds query, endpoint, and cache-transition coverage.
File Description
products/​feature_flags/​backend/​flag_analytics.py Selects and caches counts by source table.
products/​feature_flags/​backend/​api/​organization_feature_flag.py Passes organization context to count selection.
products/​feature_flags/​backend/​api/​test/​test_organization_feature_flag.py Tests mode-dependent endpoint counts.
products/​feature_flags/​backend/​api/​test/​__snapshots__/​test_organization_feature_flag.ambr Records the added mode lookup.
posthog/​test/​test_feature_flag_analytics.py Tests both sources and cache transitions.
posthog/​test/​base.py Adds a flag-evaluation fixture helper.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

📝 Walkthrough

Walkthrough

Seven-day cross-project evaluation counts can now come from either events or the flag-evaluations table. The organization’s evaluation mode selects the source, and the selected source is part of the cache key. The organization feature-flag view passes its organization ID to the cached lookup. Tests cover source-specific counts, mode changes, and cached lookups.

Priority: ➖ Normal

Merge Risk: 🔵 Low · up to 027db

The source selection and cache behavior are covered, but switching an organization before every team has ingestion coverage can undercount Projects tab evaluations. Merge with explicit ownership of that prerequisite and keep uncovered organizations on EVENTS.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 027db

Access remains limited to projects the user may view, and the inspected changes show no access-control bypass. Remaining uncertainty concerns rollout sequencing and the completeness of evaluation data when organizations switch modes.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The inspected changed read path remains bounded to authorized flag-team counts within the requested organization. It introduces access to another count source, not a demonstrated expansion to other tenants or privileged operations.

Security Findings and Attack Paths

  • inferred — No introduced boundary bypass was established in the inspected path. The request-derived flag key is passed as a query parameter, team scope comes from authorization-filtered flags, and the storage source comes from organization configuration.

Trust Boundaries and Controls

  • observed — The inherited endpoint controls include authentication and organization membership. The handler additionally restricts teams by user visibility and filters flags by per-team RBAC before querying counts.

Resilience and Maintainability Implications

  • inferred — An in-flight request can finish using the mode it already read during a concurrent change. Its result remains keyed by that source, so it cannot overwrite or satisfy the other source’s cache entry. Expiry and rollback may affect freshness, but do not establish cross-tenant disclosure in the inspected caller.
🚥 Pre-merge checks | ✅ 1
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description is complete and follows the repository template. It explains the problem, user-visible changes, mode-specific behavior, cache handling, tests and rationale, release status, documentati…
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Comment @coderabbitai help to get the list of available commands.

@posthog posthog Bot removed the reviewhog label Oct 1, 2026

@coderabbitai coderabbitai 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.

Note

Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.

🟡 Other comments (1)
products/feature_flags/backend/flag_analytics.py-360-362 (1)

360-362: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Gate FLAG_EVALUATIONS_ONLY on ingestion coverage.

FLAG_EVALUATIONS_ONLY makes this query read flag_evaluations for every team in the organization. Ingestion uses separate team, lane, topic, and exclusion settings. If any team is not covered by the active dual_write fork, recent evaluations can be missing and the zero-filled result can undercount that team. Keep the organization on EVENTS until ingestion covers every team.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: PostHog/posthog/.coderabbit.yaml

Review profile: QUIET

Plan: Enterprise

Run ID: e06e9400-ce0e-47b1-8f61-f9fa4cdb42f1

📥 Commits

Reviewing files that changed from the base of the PR and between 64b9d3f and 027dbce.

📒 Files selected for processing (2)
  • products/feature_flags/backend/facade/enums.py
  • products/feature_flags/backend/flag_analytics.py

Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 7 remain after this review.

@posthog posthog Bot removed the reviewhog label Oct 1, 2026
@haacked
haacked marked this pull request as ready for review October 1, 2026 22:38
@haacked haacked added the stamphog Request AI approval (no full review) label Oct 1, 2026
@parameterai

parameterai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Risk: No findings

This change makes the organization flag Projects tab count evaluations from the flag_evaluations ClickHouse table when the organization is on FLAG_EVALUATIONS_ONLY, while every other mode keeps reading events. The table choice is centralized in the cached reader and the cache key names the source table, which correctly prevents a mode change from serving stale counts. I found no security issues: the team ids passed to the query stay scoped to the requesting organization's accessible teams, SQL values are parameterized, and the mode lookup fails safe to events.

Sentinel reviewed 027dbce · Review settings

@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team October 1, 2026 22:39
@posthog-project-board-bot posthog-project-board-bot Bot moved this to In Review in Feature Flags Oct 1, 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.

Contained read-path change for the Projects tab. It only affects organizations explicitly set to the flag-evaluations-only mode, and the owning-team author included tests for each mode and for cache isolation. The CodeRabbit ingestion-coverage note is a rollout concern, because no organization gets this mode without an explicit change.

  • Author wrote 0% of the modified lines and has 42 merged PRs in these paths (familiarity MODERATE).
  • coderabbitai[bot] reviewed the current head.
Gate mechanics and policy version
Gate Result
prerequisites ✓ all clear
deny-list ✓ no deny categories matched
size ✓ 53L, 3F substantive, 167L/7F incl. docs/generated/snapshots — within ceiling
tier ✓ T1-agent / T1c-medium (167L, 7F, two-areas, feat)
stamphog 2.3.1 .stamphog/policy.yml @ 027dbce · reviewed head 027dbce

@trunk-io

trunk-io Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Static Badge   Static Badge   Static Badge

View Full Report ↗︎ ⋅ Docs

@trunk-io
trunk-io Bot merged commit 0a728c9 into master Oct 1, 2026
252 checks passed
@trunk-io
trunk-io Bot deleted the haacked/projects-tab-flag-evaluations branch October 1, 2026 23:19
@github-project-automation github-project-automation Bot moved this from Approved to Done in Feature Flags Oct 1, 2026
@deployment-status-posthog

deployment-status-posthog Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-10-01 23:38 UTC Run
prod-us ✅ Deployed 2026-10-01 23:52 UTC Run
prod-eu ✅ Deployed 2026-10-01 23:52 UTC Run

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

Labels

feature/feature-flags Feature Tag: Feature flags skip-inkeep-docs Use this label to skip an Inkeep docs PR in posthog.com stamphog Request AI approval (no full review) team/feature-flags

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants