feat(replay-vision): sample an experiment scanner's variants evenly by ksvat · Pull Request #108868 · PostHog/posthog

ksvat · 2026-09-30T00:58:43Z

Problem

An experiment scanner samples with one rate, so coverage follows rollout: at a 10% rate over 1,000 exposed sessions on a 90/10 split, a team gets ~90 control observations and ~10 test. The small arm — usually the variant under test — starves, and the comparison the scanner exists for has nothing to compare. Layer 4 of stack #108815, per the "Experiment scanner: backend plan" design doc; sits on #108862.

Changes

Balanced sampling for the experiment scanner type, on by default (balance_variants in the scanner config, added in the type's first layer): each sweep counts the exposed persons per watched variant over the experiment window and computes one rate per variant, r / (k · share), capped at 1. Counts rather than the flag's rollout percentages, so a rollout that changed mid-experiment plans against the mix actually being sampled (per review). A capped variant's unspent budget redistributes to the others, so a small variant can never shrink the total; the same example now yields ~50 observations per arm for the same spend.
The sampling decision lives in the candidate query's HAVING, as one salted-hash threshold per attributed variant (multiIf), so decisions stay stable across sweeps and backfills the way plain sampling does.
The exposure join projects the attributed variant when balancing is on (a new opt-in on the recordings list query); the population is unchanged.
Backfills share the query class and salt, so they balance the same buckets. Counts are recomputed each tick (60s budget, metered as the scanner's reads, failing open to plain sampling), so a traffic shift adjusts on the next sweep.
The volume estimate keeps projecting with the plain rate: redistribution spends exactly the configured budget (effective_rate ≡ rate, cap or no cap), so balancing never moves the projection — the invariant has its own test.
Each observation's snapshot records the rates its dispatching tick sampled at, so even per-variant counts don't read as even traffic. Mechanical: the rates thread from the tick's activity output through the apply inputs into the snapshot (all additive dataclass fields with defaults; no Temporal command changes), plus regenerated OpenAPI types.

Balancing cannot create sessions a small variant does not have: a 5% arm on a low-traffic page is sampled whole and stays thin.

How did you test this code?

New test_variant_sampling.py: the doc's 90/10-at-10% example gives even coverage at the same total; a capped small variant frees its budget; watched-subset normalization; zero-share and single-variant edge cases; the multiIf predicate's per-variant thresholds and its no-op when every rate is 1.
New ClickHouse test in test_scanner_candidate_query.py: with rates control=0/test=1 only the test-variant session survives the sweep query, and the unbalanced control run keeps both, so the join projection and the per-variant gate are exercised end to end.
test_temporal.py: the dispatching tick's rates land on the observation's snapshot.
Affected suites (candidate query, sweep, backfills, estimate refresh, temporal): 467 passed. TestScannerCandidateQueryAgainstClickHouse has order-dependent flakes in this sandbox that reproduce on a clean checkout (2–5 failures vary per run, pass in isolation); not a regression from this diff.
Repo-wide mypy clean, ruff clean, hogli build:openapi regenerated.

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 replay-vision flag still gates the whole product, and only experiment scanners (no UI entry point yet) reach this path.

Automatic notifications

Publish to changelog?

Docs update

None: no doc under docs/ covers scanner sampling.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Claude Code (PostHog Desktop cloud task), Claude Fable 5

Layer 4 of stack #108815, from the team lead's "Experiment scanner: backend plan".
Skills invoked: /writing-tests, /writing-dataclasses, /writing-code-comments, /writing-pr-descriptions.
CodeRabbit local pass skipped: cloud task run.
Decision along the way: rates use water-filling (equal budget slices, capped variants freeing theirs) rather than the doc's one-shot formula, so redistribution works for any number of capped variants; the two agree whenever no cap binds.
Public artifact: all test data is invented; no session material is included.

Created with PostHog Desktop

🤖 Generated with Claude Code

github-actions · 2026-09-30T00:58:54Z

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

✅ Complexity (TypeScript) — clean

Cyclomatic complexity above the limit in changed typescript files (10 for production files, 15 for test files). Warn only: worth simplifying when you next touch these functions.

⚠️

Duplication (Python) — 138 new duplicated blocks (worst 278 tokens)

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) — 4 new duplicated blocks (worst 139 tokens)

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.

First copy	Second copy	Lines	Tokens
`products/desktop/packages/ui/src/features/canvas/freeform/BuiltCanvas.tsx:196`	`products/desktop/packages/ui/src/features/canvas/freeform/FreeformCanvas.tsx:162`	20	139
`products/desktop/packages/core/src/sessions/processKilledNotice.ts:9`	`products/desktop/packages/ui/src/features/sessions/components/buildConversationItems.ts:774`	24	130
`products/ai_observability/frontend/redesign/trace/sampleFixtures/anthropicMessagesThinkingToolUse.ts:146`	`products/ai_observability/frontend/redesign/trace/sampleFixtures/langchainCerebrasImageError.ts:122`	20	121
`products/metrics/frontend/components/MetricsSamplesPanel.tsx:55`	`products/metrics/frontend/components/TraceMetricSamples.tsx:77`	21	96

🚨 Comment density — 11% of added code lines are comments (69 of 622)

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/replay_vision/backend/tests/test_variant_sampling.py`	15	128
`products/replay_vision/backend/queries/variant_sampling.py`	13	175
`products/replay_vision/backend/tests/test_scanner_candidate_query.py`	8	115
`products/replay_vision/backend/queries/scanner_candidate_query.py`	5	58
`posthog/session_recordings/queries/session_recording_list_from_query.py`	4	22
`products/replay_vision/backend/queries/scanner_volume_estimate.py`	3	3
`products/replay_vision/backend/temporal/activities/backfill.py`	3	15
`products/replay_vision/backend/temporal/snapshots.py`	3	4

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

⚠️

Bundle size — 🔺 +203.3 KiB (+0.3%)

Uncompressed size of every built .js bundle, compared against the base branch.

Total: 69.22 MiB · 🔺 +203.3 KiB (+0.3%)

File	Size	Δ vs base
`posthog-app/_parent/products/autoresearch/frontend/AutoresearchPipelineScene.js`	53.0 KiB	🔺 +53.0 KiB (new)
`posthog-app/_parent/products/ai_observability/frontend/AIObservabilityTraceScene.js`	179.4 KiB	🔺 +41.1 KiB (+29.7%)
`posthog-app/_parent/products/tasks/frontend/spaces/SpaceScene.js`	21.7 KiB	🔺 +21.7 KiB (new)
`posthog-app/_parent/products/autoresearch/frontend/AutoresearchNewScene.js`	16.1 KiB	🔺 +16.1 KiB (new)
`render-query/src/render-query/render-query.js`	20.19 MiB	🔺 +15.0 KiB (+0.1%)
`posthog-app/_parent/products/posthog_ai/frontend/scenes/TaskTracker/components/EmbeddedTaskComposerImpl.js`	12.9 KiB	🔺 +12.9 KiB (new)
`posthog-app/_parent/products/tasks/frontend/spaces/SpacesScene.js`	9.1 KiB	🔺 +9.1 KiB (new)
`toolbar/src/toolbar/debug/chunk-EventDebugMenu.js`	300.2 KiB	🔺 +7.4 KiB (+2.5%)
`posthog-app/_parent/products/signals/frontend/inbox/InboxScene.js`	457.3 KiB	🔺 +5.3 KiB (+1.2%)
`exporter/_parent/products/posthog_ai/frontend/scenes/TaskTracker/components/EmbeddedTaskComposerImpl.js`	5.1 KiB	🔺 +5.1 KiB (new)
`posthog-app/_parent/products/replay_vision/frontend/replay_scanners/ReplayScannersScene.js`	98.0 KiB	🔺 +4.8 KiB (+5.1%)
`posthog-app/src/scenes/AuthenticatedShell.js`	272.5 KiB	🔺 +3.3 KiB (+1.2%)
`posthog-app/_parent/products/workflows/frontend/Workflows/WorkflowScene.js`	53.8 KiB	🔺 +3.1 KiB (+6.1%)
`posthog-app/_parent/products/autoresearch/frontend/AutoresearchScene.js`	13.9 KiB	🟢 -1.3 KiB (-8.8%)

_{Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report}

✅ Eager graph — within budget

How much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy import() / React.lazy chunks are not counted.

Root	Eager (shipped)	Δ vs base	Budget
entry (logged-out pages, app bootstrap) `src/index.tsx`	1.60 MiB · 22 files	🔺 +3.7 KiB (+0.2%)	`█████████░` 86.9% of 1.84 MiB
logged-out boot: index + App + bootApp (preloaded by every page, including /login) `src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts`	3.54 MiB · 629 files	🔺 +4.5 KiB (+0.1%)	`█████████░` 87.9% of 4.03 MiB
authenticated shell (every logged-in page) `src/scenes/AuthenticatedShell.tsx`	7.45 MiB · 2,385 files	🔺 +44.7 KiB (+0.6%)	`█████████░` 89.3% of 8.34 MiB

🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/layout/navigation-3000/navigationLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/scenes/dashboard/dashboardLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/lemon-ui/LemonMarkdown/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/RichContentEditor/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/CodeSnippet/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/taxonomy/core-filter-definitions-by-group.json stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx

Largest files eagerly shipped from src/index.tsx

Size	File
126.8 KiB	`../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js`
24.6 KiB	`../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js`
6.3 KiB	`../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js`
4.5 KiB	`../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js`
3.9 KiB	`../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js`
1.4 KiB	`../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js`
1.3 KiB	`src/index.tsx`
1.3 KiB	`src/RootErrorBoundary.tsx`
912 B	`../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js`
854 B	`src/scenes/ChunkLoadErrorBoundary.tsx`

Largest files eagerly shipped from src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts

Size	File
301.8 KiB	`../node_modules/.pnpm/posthog-js@1.434.14_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs`
216.9 KiB	`../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js`
126.8 KiB	`../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js`
100.5 KiB	`src/lib/api.ts`
89.6 KiB	`src/products.tsx`
69.4 KiB	`src/lib/lemon-ui/icons/icons.tsx`
40.1 KiB	`src/lib/utils/eventUsageLogic.ts`
38.7 KiB	`../node_modules/.pnpm/@dnd-kit+core@6.0.8_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@dnd-kit/core/dist/core.esm.js`
33.9 KiB	`../node_modules/.pnpm/kea@4.0.0-pre.6_patch_hash=139b8d1f1304f9d9da452a9a1244c94ea679dbcb85687d8999563146879fb6f5_react@18.3.1/node_modules/kea/lib/index.cjs.js`
28.5 KiB	`src/scenes/scenes.ts`

Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx

Size	File
301.8 KiB	`../node_modules/.pnpm/posthog-js@1.434.14_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs`
279.7 KiB	`src/taxonomy/core-filter-definitions-by-group.json`
216.9 KiB	`../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js`
153.7 KiB	`../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js`
126.8 KiB	`../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js`
102.7 KiB	`../packages/quill/packages/quill/dist/index.js`
100.5 KiB	`src/lib/api.ts`
93.3 KiB	`../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js`
90.6 KiB	`../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js`
89.6 KiB	`src/products.tsx`

_{Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479}

✅ Toolbar bundle — eager 2.17 MiB within budget

What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.

Metric	Size	Δ vs base	Budget
Eager (shipped) entry + static imports	2.17 MiB · 19 files	🔺 +4.9 KiB (+0.2%)	`████░░░░░░` 38.0% of 5.72 MiB
Deferred (lazy)	2.10 MiB · 44 files	🔺 +7.4 KiB (+0.3%)	n/a — loads on demand
Loader `dist/toolbar.js`	1.2 KiB	no change	`█░░░░░░░░░` 6.0% of 19.5 KiB

Largest eagerly-shipped chunks

Size	File
813.3 KiB	`dist/toolbar/toolbar-app-BW77XXDE.css`
651.8 KiB	`dist/toolbar/chunk-chunk-XQRTS2G5.js`
259.4 KiB	`dist/toolbar/chunk-chunk-4EKE7GSM.js`
138.3 KiB	`dist/toolbar/chunk-chunk-ZLUZVIUR.js`
131.8 KiB	`dist/toolbar/chunk-chunk-FDH2IBXT.js`
75.2 KiB	`dist/toolbar/toolbar-app-47JXF64O.js`
69.0 KiB	`dist/toolbar/chunk-chunk-TSAL54PB.js`
35.6 KiB	`dist/toolbar/chunk-chunk-S2X4KXBC.js`
21.0 KiB	`dist/toolbar/chunk-chunk-AM7RRUA3.js`
6.8 KiB	`dist/toolbar/chunk-chunk-DV7IWQNF.js`

_{Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile}

✅ Dist folder size — 🔺 +2.37 MiB (+0.2%)

Total size of the built frontend/dist folder (all assets), compared against the base branch.

Total: 951.74 MiB · 🔺 +2.37 MiB (+0.2%)

ℹ️ MCP UI apps size — 33 app(s), 17633.1 KB JS

Built size of each MCP UI app (main.js + styles.css).

App	JS	CSS
debug	597.9 KB	199.2 KB
action	454.1 KB	199.2 KB
action-list	564.2 KB	199.2 KB
cohort	453.1 KB	199.2 KB
cohort-list	563.2 KB	199.2 KB
email-template	452.9 KB	199.2 KB
error-details	469.6 KB	199.2 KB
error-issue	454.5 KB	199.2 KB
error-issue-list	564.8 KB	199.2 KB
experiment	561.3 KB	199.2 KB
experiment-list	564.9 KB	199.2 KB
experiment-results	566.3 KB	199.2 KB
feature-flag	566.8 KB	199.2 KB
feature-flag-list	570.5 KB	199.2 KB
feature-flag-testing	457.3 KB	199.2 KB
inline-scan	453.6 KB	199.2 KB
insight-actors	562.3 KB	199.2 KB
invite-email-preview	452.3 KB	199.2 KB
llm-costs	559.3 KB	199.2 KB
session-recording	455.3 KB	199.2 KB
survey	454.7 KB	199.2 KB
survey-global-stats	561.9 KB	199.2 KB
survey-list	564.9 KB	199.2 KB
survey-stats	561.9 KB	199.2 KB
trace-span	453.5 KB	199.2 KB
trace-span-list	564.1 KB	199.2 KB
vision-observation-list	563.3 KB	199.2 KB
workflow	453.4 KB	199.2 KB
workflow-list	563.5 KB	199.2 KB
loops-review	457.8 KB	199.2 KB
query-results	774.1 KB	199.2 KB
render-ui	858.1 KB	199.2 KB
visual-review-snapshots	457.9 KB	199.2 KB

✅ Playwright — all passed

All tests passed.

View test results →

✅ Django migration risk — migration analysis complete

We've analyzed your migrations for potential risks.

Summary: 0 Safe | 2 Needs Review | 0 Blocked

⚠️ Needs Review

May have performance impact

replay_vision.0102_alter_replayscanner_scanner_type
  └─ #1 ⚠️ AlterField
     Field alteration may cause table locks or data loss (check if changing type or constraints)
     model: replayscanner, field: scanner_type, field_type: CharField

replay_vision.0103_alter_replayobservation_error_reason
  └─ #1 ⚠️ AlterField
     Field alteration may cause table locks or data loss (check if changing type or constraints)
     model: replayobservation, field: error_reason, field_type: TextField

Last updated: 2026-09-30 15:54 UTC (6b49f8c)

trunk-io · 2026-09-30T01:18:14Z

Failed Test	Failure Summary	Logs
`personalAPIKeysLogic leaves the auto-selected feature_flag:write removable`	The test exceeded the maximum allowed time of 5000 ms and did not complete.	Logs ↗︎

_{View Full Report ↗︎ ⋅ Docs}

coderabbitai · 2026-09-30T01:18:33Z

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

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

📝 Walkthrough

Walkthrough

Experiment scanners can derive per-variant sampling rates from exposure counts and apply them in candidate queries. Sweep and backfill workflows pass those rates to child scanner inputs. Observation snapshots and API types include optional sampling rates. Session recording queries can opt into projecting experiment exposure attribution.

Priority: ⬇️ Low

Merge Risk: 🔵 Low · up to f4ffd

Balanced sampling preserves downstream access checks, but exposure counting should also validate the scanner or backfill creator. The remaining risk is bounded to an internal unauthorized read, with no established disclosure to unauthorized users.

Security Architecture Review

Security architecture risk: 🔵 Low · up to f4ffd

A new background exposure-count read does not check the experiment permissions of the person it runs for. Later checks block unauthorized candidate delivery and observation access, limiting the demonstrated exposure to internal reads within one project.

Retained concerns

Low · security · observed: The new balanced-sampling path reads experiment exposure counts with a service identity before checking experiment access. An enabled scanner whose creator retains recording access but loses experiment access can therefore continue triggering internal exposure reads, even though the subsequent candidate query refuses dispatch.

Security review details

Security Blast Radius

inferred — The demonstrated unauthorized operation is an internal exposure-population read for one team's experiment and selected variants per invocation. An active scanner can repeat it across ticks. Linkage constrains the experiment to the supplied team, and the helper returns aggregate counts rather than person identifiers; cross-tenant reachability or external count disclosure is not established.

Security Findings and Attack Paths

observed — The retained authorization finding concerns a newly added service-side read before experiment access is evaluated. A scanner creator's loss of experiment access does not prevent count execution, although the subsequent candidate query refuses dispatch. The separate deferred candidate remains uncertainty, not a second finding.

Trust Boundaries and Controls

observed — Downstream controls materially contain the finding: denied candidate queries stop delivery, and observation reads enforce scanner and recording access, current experiment access, and snapshot-specific experiment access. These checks protect the inspected output paths but do not authorize the preceding count read.

Resilience and Maintainability Implications

observed — Count failure degrades only balancing. Experiment authorization failure returns no sweep candidates without advancing progress, or cancels the backfill, rather than falling back to unauthorized candidate delivery.

Hardening Proposals

proposed — Carry the initiating principal into exposure planning and apply the shared experiment access check before linkage resolution or count execution. Keep authorization rejection distinct from an operational count failure that permits plain-sampling fallback.

🚥 Pre-merge checks | ✅ 1

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description is complete and follows the required template. It explains the problem, user-visible changes, testing, release status, documentation status, agent context, and design decisions. It inc…

✨ Finishing Touches

📝 Generate docstrings

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

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

coderabbitai

Actionable comments posted: 3

Note

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

🟡 Other comments (1)

products/replay_vision/backend/temporal/activities/backfill.py-198-198 (1)
198-198: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Pass the variant sampling plan to the backfill count.

When balanced sampling applies, _enumerate omits variant_sampling_rates, so WindowedCandidateQuery uses the single-rate predicate. Backfill ticks use per-variant rates. The count stored as total_count can therefore differ from the candidates walked, skewing progress and credit estimates. Build the plan from the snapshot and pass its rates to the count query.
🐛 Suggested fix
 from products.replay_vision.backend.queries.scanner_candidate_query import (
     BACKFILL_CANDIDATE_QUERY_TYPE,
     BACKFILL_COUNT_QUERY_TYPE,
     WindowedCandidateQuery,
 )
+from products.replay_vision.backend.queries.variant_sampling import variant_sampling_plan_for_scope
@@
     ) -> int:
         snapshot = BackfillScannerSnapshot.from_scanner(scanner)
+        variant_plan = variant_sampling_plan_for_scope(
+            self.team,
+            scope=snapshot.experiment_scope(),
+            scanner_config=snapshot.scanner_config,
+            sampling_rate=snapshot.sampling_rate,
+        )
         return WindowedCandidateQuery(
@@
             sampling_mode=snapshot.sampling_mode,
             exclude_observed_by_scanner=str(scanner.id) if exclude_observed else None,
             max_execution_time_seconds=ENUMERATION_MAX_EXECUTION_SECONDS,
+            variant_sampling_rates=variant_plan.rates if variant_plan is not None else None,
         ).count(query_type=BACKFILL_COUNT_QUERY_TYPE)

ℹ️ Review info

⚙️ Run configuration

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

Review profile: QUIET

Plan: Enterprise

Run ID: 6b4e0244-3a0e-4ac6-ab70-d9afa29f24aa

📥 Commits

Reviewing files that changed from the base of the PR and between 771afd5 and a3dc70b.

⛔ Files ignored due to path filters (1)

products/replay_vision/frontend/generated/api.schemas.ts is excluded by !**/generated/**

📒 Files selected for processing (20)

posthog/session_recordings/queries/session_recording_list_from_query.py
products/experiments/backend/facade/replay.py
products/replay_vision/backend/api/observations.py
products/replay_vision/backend/queries/scanner_candidate_query.py
products/replay_vision/backend/queries/scanner_volume_estimate.py
products/replay_vision/backend/queries/variant_sampling.py
products/replay_vision/backend/temporal/activities/backfill.py
products/replay_vision/backend/temporal/activities/create_observation.py
products/replay_vision/backend/temporal/activities/find_scanner_candidates.py
products/replay_vision/backend/temporal/backfill_types.py
products/replay_vision/backend/temporal/backfill_workflow.py
products/replay_vision/backend/temporal/snapshots.py
products/replay_vision/backend/temporal/sweep_types.py
products/replay_vision/backend/temporal/sweep_workflow.py
products/replay_vision/backend/temporal/types.py
products/replay_vision/backend/temporal/workflow.py
products/replay_vision/backend/tests/test_scanner_candidate_query.py
products/replay_vision/backend/tests/test_temporal.py
products/replay_vision/backend/tests/test_variant_sampling.py
services/mcp/src/api/generated.ts

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

coderabbitai

🧹 Nitpick comments (1)

products/replay_vision/backend/tests/test_variant_sampling.py (1)
96-97: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Move the test imports to module scope.

Add variant_sampling_plan_for_scope to the existing planner import. Move create_experiment to the import section.

As per coding guidelines: “Always place imports at the top of the file (module level), never inside functions or methods (local imports).”
Proposed change
-        from products.replay_vision.backend.queries.variant_sampling import variant_sampling_plan_for_scope
-        from products.replay_vision.backend.tests.helpers import create_experiment
Update the module-level imports:
from products.replay_vision.backend.queries.variant_sampling import (
    plan_variant_sampling,
    variant_sampling_plan_for_scope,
)
from products.replay_vision.backend.tests.helpers import create_experiment
Source: Coding guidelines

ℹ️ Review info

⚙️ Run configuration

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

Review profile: QUIET

Plan: Enterprise

Run ID: 88ef3214-6f3c-4ded-bf53-08cc8adbca45

📥 Commits

Reviewing files that changed from the base of the PR and between a3dc70b and 44db3f8.

⛔ Files ignored due to path filters (1)

products/replay_vision/frontend/generated/api.schemas.ts is excluded by !**/generated/**

📒 Files selected for processing (5)

products/replay_vision/backend/api/observations.py
products/replay_vision/backend/queries/variant_sampling.py
products/replay_vision/backend/temporal/activities/find_scanner_candidates.py
products/replay_vision/backend/tests/test_variant_sampling.py
services/mcp/src/api/generated.ts

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

github-actions · 2026-09-30T03:34:35Z

🦔 Hogbox preview · ✅ ready

▶ Open the preview


🔑 Login	`test@posthog.com` / `12345678` (demo data)
🧩 Running	this PR's backend and frontend, on the PostHog `:master` base
🔗 Link	stable across rebuilds — a re-push swaps the box underneath, the URL stays
🔒 Access	tailnet only (PostHog VPN)
🛠️ Admin	inspect & debug state in hogland
💤 Idle	sleeps after ~30 min idle (snapshot to S3, zero node cost) and wakes on your next visit in ~30s, behind a brief "waking up" screen

_{commit 6b49f8c · box box-b38936c5812b · ready in 848s (push → usable) · build log · rebuilds on every push, torn down on close}

pr-assigner-resolver-posthog · 2026-09-30T03:35:07Z

👀 Auto-assigned reviewers

These soft owners were skipped because they only have minor changes here. Nothing blocks merge, so self-assign if you'd like a look:

@PostHog/team-experiments (products/experiments/product.yaml)

Soft owners come from each directory's owners.yaml and each product's product.yaml (resolved nearest-file-wins). For a skipped owner, the locator is the file that decided it. Generated files and lockfiles are ignored when deciding ownership.

A new stamphog review started for this PR — the fresh verdict replaces this approval.

TueHaulund · 2026-09-30T09:40:30Z

+    the shares can't be read (the caller then falls back to plain sampling; the scan itself stays
+    the loud path for an unresolvable experiment).
+    """
+    experiment_id = (scope or {}).get("experiment_id")


This turns on balancing for any scanner that watches an experiment, including the old types that target one through the column. Those scanners have no balance_variants key, so they'd switch to balanced sampling on deploy. Should this only apply to scanner_type == EXPERIMENT?

Fixed in f4ffdce: the plan is gated on scanner_type == EXPERIMENT; column-targeted scanners keep plain sampling, with a test.

TueHaulund · 2026-09-30T09:40:32Z

+    from products.experiments.backend.facade.replay import variant_rollout_shares  # noqa: PLC0415
+
+    try:
+        shares = variant_rollout_shares(team, experiment_id=experiment_id)


The plan said to use rollout shares here, but I think that was a mistake on my part. The sessions we sample come from the whole experiment window, so if the rollout changed partway through, the actual mix can be very different. For example, after a change from 90/10 to 10/90, a 10% rate ends up scanning about 46% of sessions, and the estimate never sees it. A variant ramped down to 0% also gets sampled at 100%. Counting exposures per variant from the exposure query would fix both.

Fixed in f4ffdce: shares now come from counting exposed persons per watched variant over the window (an aggregate over exposed_persons_select, metered as the scanner's reads, 60s budget, failing open to plain sampling). A ramped-down variant keeps its real weight, and the rates match the mix actually being sampled. Watched variants with no exposures zero-fill.

TueHaulund · 2026-09-30T09:40:34Z

+    if experiment_id is None:
+        return None
+    config = scanner_config if isinstance(scanner_config, dict) else {}
+    if config.get("balance_variants") is False:


Turning balance_variants on or off doesn't mark the estimate as stale, and the scout cost check misses it too.

With exposure-count shares this turned out to be a non-issue: redistribution spends exactly the configured budget (effective_rate ≡ rate, cap or no cap — new invariant test in f4ffdce), so toggling balance_variants changes how the budget is spread, never the total. The estimate and the scout's cost projection don't move, so there's nothing to invalidate; the estimate now projects with the plain rate again.

TueHaulund · 2026-09-30T09:40:36Z

Could we add a test that the estimate comes out the same with balancing on and off (until a cap kicks in)? Right now only effective_rate is tested.

Added in f4ffdce: test_balancing_never_changes_the_projected_volume asserts the projection is identical with balancing on and off, including when a cap binds — redistribution preserves the total, so the parenthetical never arrives.

A new stamphog review started for this PR — the fresh verdict replaces this approval.

coderabbitai

Note

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

🟡 Other comments (1)

products/replay_vision/backend/queries/variant_sampling.py-154-159 (1)

154-159: 🔒 Security & Privacy | 🛡️ Detected with Advanced Tier | 🟡 Minor | ⚡ Quick win

Authorization Bypass

Reachability: Internal
Exploitability: Difficult
CWE: CWE-863 — Incorrect Authorization

Authorize the exposure-count query with the scanner principal.

_variant_exposure_counts reads experiment exposure counts without a principal or validate_experiment_exposure_access. A scanner creator who loses experiment access can therefore trigger an internal unauthorized count read before candidate dispatch is rejected.

The rates do not reach unauthorized observation readers. Candidate queries run as the scanner or backfill creator, and observation reads enforce both current scanner experiment access and the experiment recorded in each snapshot.

Pass scanner.created_by or backfill.created_by through variant_sampling_plan_for_scope. Validate that principal before resolving the linkage, and return None when the principal is missing or denied.

🧹 Nitpick comments (1)

products/replay_vision/backend/tests/test_variant_sampling.py (1)

85-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Move the local imports to module scope.

ScannerVolumeEstimate, project_monthly_observations, and variant_sampling_plan_for_scope are imported inside test methods. Line 12 already imports from variant_sampling at module level, so an import cycle does not explain these local imports.

As per coding guidelines: "Always place imports at the top of the file (module level), never inside functions or methods (local imports)."

Also applies to: 122-122

Source: Coding guidelines

ℹ️ Review info

⚙️ Run configuration

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

Review profile: QUIET

Plan: Enterprise

Run ID: 78228884-9923-4aa0-9ae9-52a6b9d3ed6c

📥 Commits

Reviewing files that changed from the base of the PR and between 3ff4959 and f4ffdce.

⛔ Files ignored due to path filters (1)

products/replay_vision/frontend/generated/api.schemas.ts is excluded by !**/generated/**

📒 Files selected for processing (7)

products/experiments/backend/facade/replay.py
products/replay_vision/backend/queries/scanner_volume_estimate.py
products/replay_vision/backend/queries/variant_sampling.py
products/replay_vision/backend/temporal/activities/backfill.py
products/replay_vision/backend/temporal/activities/find_scanner_candidates.py
products/replay_vision/backend/tests/test_scanner_candidate_query.py
products/replay_vision/backend/tests/test_variant_sampling.py

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

A new stamphog review started for this PR — the fresh verdict replaces this approval.

stamphog

Approved.

Contained change inside the flag-gated replay-vision product. The access-control concern was fixed, with the exposure-count query now running the experiment access check. The reviewer threads I read have replies showing they were addressed, and new tests cover the sampling logic and the query path. The backfill total-count query may not use the per-variant rates, which could skew progress and credit estimates slightly. That is minor and not a blocker.

Author wrote 0% of the modified lines and has 34 merged PRs in these paths (familiarity MODERATE).
Minor: the backfill total-count query may not use the balanced per-variant rates, so the stored total count could differ slightly from the sessions actually walked (CodeRabbit raised this). Consider a follow-up.

Gate mechanics and policy version

Gate		Result
prerequisites	✓	all clear
deny-list	✓	no deny categories matched
size	✓	417L, 16F substantive, 749L/22F incl. docs/generated/snapshots — within ceiling
tier	✓	T1-agent / T1d-complex (749L, 22F, cross-cutting, feat)
stamphog 2.3.1		`.stamphog/policy.yml` @ `d5f273a` · reviewed head `d5f273a`

Balanced per-variant sampling for experiment scanners, default on: each sweep computes one salted-hash rate per watched variant from the flag's live rollout shares (r / (k · share), capped at 1, with a capped variant's unspent budget redistributed), so an uneven rollout no longer starves the small arm. The exposure join projects the attributed variant when balancing is on, backfills share the same query class and salt, the volume estimate projects with the plan's effective rate, and each observation's snapshot records the rates it was sampled at. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: c2db2a38-d03f-4578-af4a-40f61dc700a4

…variant scope Two review findings: a paused scanner (rate 0) now yields all-zero per-variant rates instead of sampling a zero-share variant whole, and a legacy column scope's singular `variant` counts as the one watched arm, so single-arm targeting gets no balancing plan. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: c2db2a38-d03f-4578-af4a-40f61dc700a4

Review cleanup: the balanced-sampling ClickHouse test imported User, Experiment, and FeatureFlag inside the test body; a test file has no import cycle to break, so they belong in the import section. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: c2db2a38-d03f-4578-af4a-40f61dc700a4

…iment type only Per review: the plan's shares now come from counting the exposed persons per watched variant over the experiment window (an aggregate over the exposure select, metered as the scanner's reads, failing open to plain sampling), because the flag's rollout percentages diverge from the window's real mix whenever the rollout changed mid-experiment. The plan is also gated to the experiment scanner type, so legacy column-targeted scanners keep plain sampling on deploy. The volume estimate goes back to projecting with the plain rate: redistribution spends exactly the configured budget (effective_rate always equals the rate), so balancing never moves the projection — the invariant now has its own test, which also answers why toggling balance_variants needs no estimate or scout cost invalidation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: c2db2a38-d03f-4578-af4a-40f61dc700a4

…uery-kind literal The counts query now runs the experiment's object-level access check as the scanner's creator (backfills: the launcher), the same principal the candidate query authorizes, failing open to plain sampling when denied or missing. Also swaps the malformed-query sweep test's payload off a real product query kind: the activity's module now reaches a HogQL dispatcher through the sampling plan, so the repo's model-crossing guard read the old TrendsQuery literal as this test driving another product's runner and cancelled Backend CI. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: c2db2a38-d03f-4578-af4a-40f61dc700a4

ksvat self-assigned this Sep 30, 2026

ksvat added this pull request to stack #108815 September 30, 2026 00:58

coderabbitai Bot reviewed Sep 30, 2026

View reviewed changes

Comment thread products/replay_vision/backend/queries/variant_sampling.py

Comment thread products/replay_vision/backend/queries/variant_sampling.py Outdated

Comment thread products/replay_vision/backend/queries/variant_sampling.py Outdated

ksvat force-pushed the posthog/rv-experiment-balanced-sampling branch 2 times, most recently from 2682501 to 44db3f8 Compare September 30, 2026 01:35

coderabbitai Bot reviewed Sep 30, 2026

View reviewed changes

ksvat marked this pull request as ready for review September 30, 2026 03:34

ksvat added the stamphog Request AI approval (no full review) label Sep 30, 2026

github-actions Bot requested a deployment to preview-pr-108868 September 30, 2026 03:34 In progress

pr-assigner-resolver-posthog Bot requested review from a team, TueHaulund, arnohillen and fasyy612 and removed request for a team September 30, 2026 03:35

This comment was marked as outdated.

Sign in to view

github-actions Bot deployed to preview-pr-108868 September 30, 2026 03:41 View deployment

This comment was marked as outdated.

Sign in to view

ksvat force-pushed the posthog/rv-experiment-balanced-sampling branch from 7fd0c82 to 5580112 Compare September 30, 2026 03:54

github-actions Bot deployed to preview-pr-108868 September 30, 2026 03:54 View deployment

ksvat force-pushed the posthog/rv-experiment-balanced-sampling branch from 5580112 to 9c36f24 Compare September 30, 2026 05:00

github-actions Bot deployed to preview-pr-108868 September 30, 2026 05:00 View deployment

ksvat force-pushed the posthog/rv-experiment-balanced-sampling branch from 9c36f24 to 527ad5a Compare September 30, 2026 05:16

github-actions Bot deployed to preview-pr-108868 September 30, 2026 05:16 View deployment

ksvat force-pushed the posthog/rv-experiment-balanced-sampling branch from 527ad5a to 3ff4959 Compare September 30, 2026 05:37

github-actions Bot deployed to preview-pr-108868 September 30, 2026 05:37 View deployment

TueHaulund reviewed Sep 30, 2026

View reviewed changes

ksvat force-pushed the posthog/rv-experiment-balanced-sampling branch from 3ff4959 to 9cb9886 Compare September 30, 2026 10:07

github-actions Bot requested a deployment to preview-pr-108868 September 30, 2026 10:07 In progress

github-actions Bot deployed to preview-pr-108868 September 30, 2026 10:07 View deployment

This comment was marked as outdated.

Sign in to view

parameterai Bot reviewed Sep 30, 2026

View reviewed changes

Comment thread products/replay_vision/backend/queries/variant_sampling.py

coderabbitai Bot reviewed Sep 30, 2026

View reviewed changes

github-actions Bot deployed to preview-pr-108868 September 30, 2026 10:27 View deployment

stamphog Bot approved these changes Sep 30, 2026

View reviewed changes

ksvat force-pushed the posthog/rv-experiment-balanced-sampling branch from d5f273a to 39c1904 Compare September 30, 2026 15:31

github-actions Bot deployed to preview-pr-108868 September 30, 2026 15:32 View deployment

ksvat added 5 commits September 30, 2026 08:51

ksvat force-pushed the posthog/rv-experiment-balanced-sampling branch from 39c1904 to 6b49f8c Compare September 30, 2026 15:51

github-actions Bot deployed to preview-pr-108868 September 30, 2026 15:52 View deployment

Conversation

ksvat commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Changes

How did you test this code?

Release status

Automatic notifications

Docs update

🤖 Agent context

Uh oh!

github-actions Bot commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🤖 CI report

⚠️ Needs Review

Uh oh!

trunk-io Bot commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Security Architecture Review

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦔 Hogbox preview · ✅ ready

Uh oh!

pr-assigner-resolver-posthog Bot commented Sep 30, 2026

👀 Auto-assigned reviewers

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

stamphog Bot left a comment

Choose a reason for hiding this comment

Uh oh!

This branch was successfully deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ksvat commented Sep 30, 2026 •

edited

Loading

github-actions Bot commented Sep 30, 2026 •

edited

Loading

trunk-io Bot commented Sep 30, 2026 •

edited

Loading

coderabbitai Bot commented Sep 30, 2026 •

edited

Loading

github-actions Bot commented Sep 30, 2026 •

edited

Loading