Conversation
Replay vision singleton schedules now pass an offset. The hourly read meter and search suggestions start two minutes after the hour with ten minutes of jitter, and the sub-hourly ones take a stable offset from the shared helper, which compute_schedule_offset now uses too. The surfacing scoring sweep takes a stable offset as well. Replay count metrics keep minute zero because the query reads a rolling hour with no cursor. The schedule carries a nosemgrep reason. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
🤖 CI report
|
|
[Medium risk] Adjusts timing of scheduled replay and vision workflows. The PR appears safe to merge, with a non-blocking gap in tests for its schedule timing. Reviews (1) · Last reviewed commit: "fix(replay): move replay vision and scor..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 📝 WalkthroughWalkthroughThe replay-count metrics schedule keeps its hourly interval and now documents why it runs at minute zero. Replay Vision schedule offsets now use deterministic values, and the interval schedule helper requires an offset and supports optional jitter. The read-meter and search-suggestions schedules configure a two-minute offset and ten-minute jitter. Priority: ⬇️ Low Merge Risk: 🔵 Low · up to Newly created scanner schedules may start together despite their staggered timing. This is limited to initial runs and has a localized correction; the remaining merge risk is low. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The affected jobs retain their identities and existing update behavior, and the reviewed callers supply the required offsets. Production behavior when jitter is applied to existing schedules remains unverified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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/schedule.py-146-152 (1)
146-152: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick winDo not trigger new schedules immediately.
trigger_immediately=Truestarts one workflow whenClient.create_schedulecreates the schedule. This bypasses the configuredoffsetandjitter. The reconciler can create multiple scanner schedules concurrently, so their first workflows can read the same in-flight counts before staggering applies.Set
trigger_immediately=Falsein both schedule-creation branches and update the test expectation.Suggested fix
- client, schedule_id, schedule, trigger_immediately=True, search_attributes=search_attributes + client, schedule_id, schedule, trigger_immediately=False, search_attributes=search_attributesApply the same change in
a_upsert_scanner_schedule, which has a separate creation branch.- assert create.call_args.kwargs["trigger_immediately"] is True + assert create.call_args.kwargs["trigger_immediately"] is False
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 59c08e91-98c4-4143-b1e9-e369adc488d5
📒 Files selected for processing (8)
posthog/temporal/schedule.pyposthog/temporal/session_replay/surfacing_scoring_sweep/schedule.pyproducts/replay_vision/backend/temporal/estimates.pyproducts/replay_vision/backend/temporal/gemini_cleanup_sweep/schedule.pyproducts/replay_vision/backend/temporal/read_meter.pyproducts/replay_vision/backend/temporal/reconciler.pyproducts/replay_vision/backend/temporal/schedule.pyproducts/replay_vision/backend/temporal/search_suggestions.py
Included review availability: Your plan provides up to 12 included reviews per hour; 6 remain after this review.
This branch has not been deployed
1 participant
Problem
Replay vision's hourly read meter and search suggestion refresher start at minute zero, and its sub-hourly singletons fire on their interval boundaries with the rest of the fleet. Split from #106458; merge after the helper in #106984.
Changes
The hourly read meter and search suggestions now start two minutes after the hour with up to ten minutes of jitter. The estimates refresh, reconciler and Gemini cleanup sweep take a stable offset keyed by schedule id, and so does the surfacing scoring sweep.
upsert_interval_schedulenow requires an offset, so a new singleton cannot forget one.Per-scanner offsets now come from the shared helper. They stay evenly spread, but map to different minutes. Existing scanner schedules keep their current offset until the scanner is next updated.
Replay count metrics keep minute zero on purpose. Their query reads a rolling hour with no cursor, so a later start would skip data. The schedule now says so, with an exemption for the minute-zero lint in #106997.
How did you test this code?
The replay vision schedule, read meter, reconciler, search suggestion and estimate tests pass locally, except the cases that need Postgres, which was down locally. The surfacing sweep schedule test passes. CI runs the full suites, and they passed with the same code on #106458.
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Release status
Automatic notifications
Docs update
None.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Agent: Claude Code, Claude Opus 5.5; Codex, GPT-6 (review fixes on #106458).
Split from #106458 by owning team: the owners.yaml resolver decided ownership, and a team gets its own PR when its owned files clear the reviewer assigner's bar of 10 lines or 3 files. The code is unchanged from the reviewed head of #106458, except that its signals test-fixture fix dropped because #106460 fixed the same bug on master. CodeRabbit CLI ran once with
--deepover all the split PRs combined on master 8475a79 and reported no findings. Skills for the split: stacking-prs, establishing-code-ownership, reviewing-with-coderabbit, writing-pr-descriptions. The original change also used qa-team, announcing-behavior-changes, writing-tests, writing-code-comments, writing-ui-components, writing-user-facing-copy, running-ci-preflight and debugging-ci-failures.