fix(web-analytics): explain zero sessions in the weekly digest response - #107172
posthog[bot] wants to merge 9 commits into
Conversation
The digest counts only sessions with a $pageview or $screen event from non-test accounts. A project with other session events got a zero that looked valid. The response now carries a metadata block with the period, filters, metric notes and a data_status that flags this case. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: 581f02a9-b1fd-4040-a3f4-2d5237cc9512
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
🤖 CI report
|
| File | Size | Δ vs base |
|---|---|---|
posthog-app/_parent/products/workflows/frontend/Workflows/WorkflowScene.js |
50.7 KiB | 🔺 +11.2 KiB (+28.4%) |
posthog-app/_parent/products/workflows/frontend/TemplateLibrary/MessageTemplate.js |
30.8 KiB | 🔺 +5.6 KiB (+22.2%) |
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.57 MiB · 22 files | 🔺 +636 B (+0.0%) | █████████░ 85.4% 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.58 MiB · 629 files | 🔺 +750 B (+0.0%) | █████████░ 88.9% of 4.03 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
7.39 MiB · 2,330 files | 🔺 +1.4 KiB (+0.0%) | █████████░ 88.6% 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 |
| 267.6 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.4 KiB | src/lib/api.ts |
| 88.0 KiB | src/products.tsx |
| 69.4 KiB | src/lib/lemon-ui/icons/icons.tsx |
| 63.9 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.4 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 |
| 271.7 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 267.6 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 |
| 100.4 KiB | src/lib/api.ts |
| 98.5 KiB | ../packages/quill/packages/quill/dist/index.js |
| 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 |
| 88.0 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.37 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.37 MiB · 19 files | 🔺 +750 B (+0.0%) | ████░░░░░░ 41.5% of 5.72 MiB |
| Deferred (lazy) | 2.10 MiB · 44 files | no change | 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 |
|---|---|
| 797.6 KiB | dist/toolbar/toolbar-app-YQD34LXY.css |
| 650.9 KiB | dist/toolbar/chunk-chunk-ZLZUDQLU.js |
| 483.6 KiB | dist/toolbar/chunk-chunk-6JFSEK3E.js |
| 138.3 KiB | dist/toolbar/chunk-chunk-SL4WBWFJ.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-FDH2IBXT.js |
| 75.2 KiB | dist/toolbar/toolbar-app-BCYDYBUD.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-TSAL54PB.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-4NNH3FVO.js |
| 21.0 KiB | dist/toolbar/chunk-chunk-GRXIOXBW.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 — 🔺 +402.2 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 945.85 MiB · 🔺 +402.2 KiB (+0.0%)
ℹ️ MCP UI apps size — 33 app(s), 17630.1 KB JS
Built size of each MCP UI app (main.js + styles.css).
| App | JS | CSS |
|---|---|---|
| debug | 597.9 KB | 196.2 KB |
| action | 454.1 KB | 196.2 KB |
| action-list | 564.2 KB | 196.2 KB |
| cohort | 453.1 KB | 196.2 KB |
| cohort-list | 563.2 KB | 196.2 KB |
| email-template | 452.9 KB | 196.2 KB |
| error-details | 469.1 KB | 196.2 KB |
| error-issue | 453.8 KB | 196.2 KB |
| error-issue-list | 564.1 KB | 196.2 KB |
| experiment | 561.3 KB | 196.2 KB |
| experiment-list | 564.9 KB | 196.2 KB |
| experiment-results | 566.3 KB | 196.2 KB |
| feature-flag | 566.8 KB | 196.2 KB |
| feature-flag-list | 570.5 KB | 196.2 KB |
| feature-flag-testing | 457.3 KB | 196.2 KB |
| inline-scan | 453.6 KB | 196.2 KB |
| insight-actors | 562.3 KB | 196.2 KB |
| invite-email-preview | 452.3 KB | 196.2 KB |
| llm-costs | 559.3 KB | 196.2 KB |
| session-recording | 455.3 KB | 196.2 KB |
| survey | 454.7 KB | 196.2 KB |
| survey-global-stats | 561.9 KB | 196.2 KB |
| survey-list | 564.9 KB | 196.2 KB |
| survey-stats | 561.9 KB | 196.2 KB |
| trace-span | 453.5 KB | 196.2 KB |
| trace-span-list | 564.1 KB | 196.2 KB |
| vision-observation-list | 563.3 KB | 196.2 KB |
| workflow | 453.4 KB | 196.2 KB |
| workflow-list | 563.5 KB | 196.2 KB |
| loops-review | 457.8 KB | 196.2 KB |
| query-results | 774.1 KB | 196.2 KB |
| render-ui | 857.0 KB | 196.2 KB |
| visual-review-snapshots | 457.9 KB | 196.2 KB |
⚠️ Backend coverage — 98.0% of changed backend lines covered — 1 uncovered
🧪 Backend test coverage
Patch coverage — changed backend lines (products + core): ████████████████████ 98.0% (85 / 86)
| File | Patch | Uncovered changed lines |
|---|---|---|
products/web_analytics/backend/serializers.py |
93.8% | 82 |
🤖 Agents: add a test covering the lines above, or note why under "How did you test this code?". Machine-readable gap list: the patch-coverage artifact on this run (gh run download 36419747797 -n patch-coverage), or the coverage-data block at the end of this comment.
Per-product line coverage (touched products)
| Product | Coverage | Lines |
|---|---|---|
platform_features |
██░░░░░░░░░░░░░░░░░░ 12.1% |
7 / 58 |
warehouse_sources_queue |
██████░░░░░░░░░░░░░░ 29.1% |
92 / 316 |
demo |
████████████░░░░░░░░ 57.8% |
1,545 / 2,673 |
data_tools |
████████████░░░░░░░░ 61.2% |
90 / 147 |
aeo |
██████████████░░░░░░ 70.5% |
467 / 662 |
ai_gateway |
███████████████░░░░░ 75.0% |
9 / 12 |
batch_exports |
████████████████░░░░ 81.2% |
21,459 / 26,431 |
apm |
█████████████████░░░ 84.1% |
1,306 / 1,553 |
ml_inference |
█████████████████░░░ 87.2% |
482 / 553 |
cdp |
██████████████████░░ 88.2% |
4,548 / 5,155 |
mcp_analytics |
██████████████████░░ 88.9% |
4,910 / 5,523 |
product_tours |
██████████████████░░ 89.3% |
1,331 / 1,491 |
dashboards |
██████████████████░░ 89.5% |
6,839 / 7,641 |
signals |
██████████████████░░ 89.9% |
54,653 / 60,807 |
data_warehouse |
██████████████████░░ 89.9% |
13,912 / 15,470 |
notebooks |
██████████████████░░ 90.2% |
15,287 / 16,945 |
cohorts |
██████████████████░░ 90.4% |
8,420 / 9,316 |
streamlit_apps |
██████████████████░░ 90.7% |
2,625 / 2,895 |
managed_warehouse |
██████████████████░░ 90.9% |
10,215 / 11,234 |
tasks |
██████████████████░░ 91.1% |
73,993 / 81,212 |
data_modeling |
██████████████████░░ 91.5% |
10,525 / 11,498 |
business_knowledge |
██████████████████░░ 91.6% |
6,899 / 7,528 |
engineering_analytics |
██████████████████░░ 91.7% |
11,017 / 12,014 |
exports |
██████████████████░░ 91.8% |
9,685 / 10,555 |
ai_training |
██████████████████░░ 92.2% |
356 / 386 |
conversations |
███████████████████░ 92.5% |
28,726 / 31,047 |
early_access_features |
███████████████████░ 92.6% |
1,341 / 1,448 |
managed_migrations |
███████████████████░ 92.7% |
1,581 / 1,705 |
visual_review |
███████████████████░ 92.8% |
9,244 / 9,966 |
canvas |
███████████████████░ 92.8% |
6,877 / 7,409 |
approvals |
███████████████████░ 93.0% |
3,919 / 4,214 |
mcp_registry |
███████████████████░ 93.1% |
1,670 / 1,794 |
error_tracking |
███████████████████░ 93.1% |
15,843 / 17,010 |
notifications |
███████████████████░ 93.2% |
1,145 / 1,229 |
slack_app |
███████████████████░ 93.2% |
13,677 / 14,674 |
stamphog |
███████████████████░ 93.2% |
7,885 / 8,456 |
surveys |
███████████████████░ 93.3% |
6,571 / 7,040 |
context_layer |
███████████████████░ 93.8% |
3,373 / 3,595 |
web_analytics |
███████████████████░ 94.0% |
21,736 / 23,135 |
alerts |
███████████████████░ 94.0% |
8,541 / 9,082 |
billing_alerts |
███████████████████░ 94.1% |
2,094 / 2,226 |
mcp_store |
███████████████████░ 94.4% |
8,940 / 9,472 |
ai_observability |
███████████████████░ 94.4% |
22,534 / 23,870 |
wizard |
███████████████████░ 94.7% |
6,151 / 6,496 |
reminders |
███████████████████░ 94.8% |
760 / 802 |
workflows |
███████████████████░ 94.9% |
14,335 / 15,113 |
review_hog |
███████████████████░ 94.9% |
11,490 / 12,109 |
annotations |
███████████████████░ 95.1% |
817 / 859 |
endpoints |
███████████████████░ 95.1% |
9,211 / 9,681 |
customer_analytics |
███████████████████░ 95.2% |
24,899 / 26,167 |
legal_documents |
███████████████████░ 95.2% |
2,311 / 2,427 |
marketing_analytics |
███████████████████░ 95.3% |
19,214 / 20,161 |
posthog_ai |
███████████████████░ 95.4% |
2,489 / 2,610 |
experiments |
███████████████████░ 95.4% |
32,645 / 34,211 |
growth |
███████████████████░ 95.4% |
9,812 / 10,282 |
logs |
███████████████████░ 95.4% |
15,290 / 16,022 |
actions |
███████████████████░ 95.5% |
756 / 792 |
data_catalog |
███████████████████░ 95.5% |
4,401 / 4,606 |
tracing |
███████████████████░ 95.6% |
3,518 / 3,680 |
autoresearch |
███████████████████░ 95.7% |
8,481 / 8,865 |
messaging |
███████████████████░ 95.8% |
3,798 / 3,963 |
skills |
███████████████████░ 95.8% |
6,972 / 7,274 |
replay_vision |
███████████████████░ 95.9% |
27,154 / 28,310 |
product_analytics |
███████████████████░ 96.2% |
28,495 / 29,617 |
revenue_analytics |
███████████████████░ 96.4% |
1,876 / 1,946 |
access_control |
███████████████████░ 96.4% |
7,122 / 7,386 |
user_interviews |
███████████████████░ 96.5% |
2,859 / 2,963 |
feature_flags |
███████████████████░ 96.5% |
25,499 / 26,416 |
warehouse_sources |
███████████████████░ 97.2% |
452,813 / 465,679 |
data_quality |
████████████████████ 97.7% |
7,592 / 7,774 |
links |
████████████████████ 97.9% |
234 / 239 |
security |
████████████████████ 98.0% |
1,203 / 1,228 |
metrics |
████████████████████ 98.1% |
4,085 / 4,166 |
analytics_platform |
████████████████████ 98.3% |
2,783 / 2,832 |
pulse |
████████████████████ 98.5% |
2,043 / 2,075 |
live_debugger |
████████████████████ 99.2% |
626 / 631 |
field_notes |
████████████████████ 99.4% |
172 / 173 |
Report-only. Patch coverage = changed backend lines covered vs origin/master. Sorted lowest first.
Known gaps: lines covered only by Temporal tests show as uncovered; core line numbers may drift if master changed the same file.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: PostHog/posthog/.coderabbit.yaml Review profile: QUIET Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughThe weekly digest now includes metadata with its data status, reporting-period boundaries, timezone, test-account filter, and metric notes. It determines status from overview metrics and, when those metrics are empty, a session probe. The serializer and generated API types expose the metadata, and tests check the response shape and status cases. Priority: ⬇️ Low Merge Risk: 🟡 Moderate · up to A digest can still leave zero sessions unexplained when it has pageviews, and its period note can misstate the end time. Resolve or explicitly accept those discrepancies before merging. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The new query returns only whether qualifying events exist, and callers still use the existing team-scoped digest endpoint. Its authorization behavior needs confirmation because it uses a different query path from the digest metrics. Retained concerns
Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (4)
products/web_analytics/backend/weekly_digest.py-53-53 (1)
53-53: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCorrect the period-end note.
With the day interval used by
_digest_date_range,QueryDateRange.date_to()returns the end of today, not the time of the request. The note says the period “ends now,” whilemetadata.date_toreports the end of today. Describe the actual calendar-day bound so consumers can interpret the dates consistently. (raw.githubusercontent.com)products/web_analytics/backend/weekly_digest.py-317-317 (1)
317-317: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winApply test-account filtering to goals.
get_goals_for_teamcreatesWebGoalsQuerywithoutfilterTestAccounts=True. Goal conversions can therefore include test accounts while the digest metadata states that metrics exclude them.🐛 Suggested fix
query = WebGoalsQuery( dateRange=DateRange(date_from=f"-{days}d"), compareFilter=CompareFilter(compare=compare), properties=[], + filterTestAccounts=True, )products/web_analytics/backend/serializers.py-84-85 (1)
84-85: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winSerialize period bounds in the project timezone.
WebAnalyticsViewSet.weekly_digestandWebAnalyticsViewSet.recapserialize responses without activatingteam.timezone. Django defaults to UTC withUSE_TZ = True, whileQueryDateRangecomputes both bounds withteam.timezone_info. Default DRFDateTimeFieldoutput can therefore emit non-UTC project bounds with UTC offsets, which conflicts with the help text and thetimezonemetadata field.Serialize these fields in
team.timezone, or update the metadata and help text to state the actual emitted timezone.products/web_analytics/backend/weekly_digest.py-294-299 (1)
294-299: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winDo not map probe failures to
NO_SESSIONS.When the session probe raises, returning
Falsecan reportNO_SESSIONSeven though the query did not establish that no sessions exist. The serializer definesno_sessionsas an actual absence of sessions.Catch the probe failure, record it, and add an explicit unknown status to the API contract. Update generated API types and consumers for the new value.
Suggested fix
class DigestDataStatus(models.TextChoices): OK = "ok", "OK" NO_WEB_SESSIONS = "no_web_sessions", "No web sessions" NO_SESSIONS = "no_sessions", "No sessions" + UNKNOWN = "unknown", "Unknown" -def _has_sessions_in_range(team: Team, date_range: QueryDateRange) -> bool: +def _has_sessions_in_range(team: Team, date_range: QueryDateRange) -> bool | None: tag_queries(product=ProductKey.WEB_ANALYTICS, team_id=team.pk, name="weekly_digest:session_probe") query = parse_select( "SELECT 1 FROM events WHERE timestamp >= {date_from} AND timestamp < {date_to} " "AND notEmpty(events.$session_id) LIMIT 1", @@ - response = execute_hogql_query(query_type="web_analytics_digest_session_probe", query=query, team=team) + try: + response = execute_hogql_query(query_type="web_analytics_digest_session_probe", query=query, team=team) + except Exception as e: + capture_exception(e, {"team_id": team.id}) + return None return bool(response.results) @@ - elif _has_sessions_in_range(team, date_range): - # A plain zero reads as "no traffic", but the project has sessions that the web definition excludes. - data_status = DigestDataStatus.NO_WEB_SESSIONS else: - data_status = DigestDataStatus.NO_SESSIONS + has_sessions = _has_sessions_in_range(team, date_range) + if has_sessions is None: + data_status = DigestDataStatus.UNKNOWN + elif has_sessions: + # A plain zero reads as "no traffic", but the project has sessions that the web definition excludes. + data_status = DigestDataStatus.NO_WEB_SESSIONS + else: + data_status = DigestDataStatus.NO_SESSIONS
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: be20dd19-043f-494a-b334-58d45ce37bb9
⛔ Files ignored due to path filters (1)
products/web_analytics/frontend/generated/api.schemas.tsis excluded by!**/generated/**
📒 Files selected for processing (5)
products/web_analytics/backend/serializers.pyproducts/web_analytics/backend/test/test_api.pyproducts/web_analytics/backend/test/test_weekly_digest.pyproducts/web_analytics/backend/weekly_digest.pyservices/mcp/src/api/generated.ts
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 5 remain after this review.
|
Generated-By: PostHog Desktop Task-Id: 3e8e5e92-7256-4175-a7e5-792e1e8f8b60
A new stamphog review started for this PR — the fresh verdict replaces this approval.
There was a problem hiding this comment.
Not approved — this change needs a human reviewer.
Re-add the stamphog label to request another review once you have addressed this.
CodeRabbit flagged a real, unresolved correctness gap in the new logic itself: a session that started before the period but has a pageview inside it makes the digest report data_status: ok even though the serializer's own definition of ok requires web sessions in the period — exactly the zero/mismatch confusion this PR exists to fix. That's a substantive, unaddressed reviewer concern on the current diff, not a style nit.
- Unresolved CodeRabbit inline comment (Major) on products/web_analytics/backend/weekly_digest.py: the OK-status check (
sessions or pageviews) can misreportokwhen pageviews are nonzero but no session actually falls in the period, contradicting the field's stated definition. - Minor/non-blocking: CodeRabbit also noted the 'ends now' note text doesn't match the actual day-granular date_to boundary — worth a follow-up but not blocking on its own.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 101L, 3F substantive, 208L/8F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (208L, 8F, cross-cutting, fix) |
| stamphog 2.2.0 | .stamphog/policy.yml @ 23470e0 · reviewed head 23470e0 |
🦔 PostHog Review reviewed this pull requestFound 0 must fix, 2 should fix, 3 consider. Published 5 findings (view the review). Resolved comments: 2 fixed, 1 left for you |
…web-analytics-ce717a Generated-By: PostHog Desktop Task-Id: 3e8e5e92-7256-4175-a7e5-792e1e8f8b60
There was a problem hiding this comment.
Not approved — this change needs a human reviewer.
Re-add the stamphog label to request another review once you have addressed this.
CodeRabbit's unresolved inline comment on weekly_digest.py flags a real gap still present in the current diff: the new "ok" status is assigned whenever pageviews>0, even when sessions==0 (a session that started before the period but had a pageview inside it) — exactly the ambiguous-zero case this PR was written to fix, left unfixed.
- Unresolved CodeRabbit inline comment (Major) on products/web_analytics/backend/weekly_digest.py: the data_status='ok' branch doesn't account for pageviews>0 with sessions==0 in the period, so a genuinely ambiguous zero can still be reported as 'ok' — undermining the PR's stated purpose.
- Two additional unresolved CodeRabbit minor comments (period-end note wording, goals query missing filterTestAccounts) are lower priority but also unaddressed.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 101L, 3F substantive, 208L/8F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (208L, 8F, cross-cutting, fix) |
| stamphog 2.2.0 | .stamphog/policy.yml @ 410e24e · reviewed head 410e24e |
|
PostHog Review alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏 |
| def get_digest_metadata(team: Team, overview: dict, days: int = 7) -> dict: | ||
| date_range = _digest_date_range(team, days) | ||
| if overview["sessions"]["current"] or overview["pageviews"]["current"]: | ||
| data_status = DigestDataStatus.OK | ||
| elif _has_sessions_in_range(team, date_range): |
There was a problem hiding this comment.
Use the overview period for its metadata
Issue description
The overview runner calculates its date range before the other digest queries. get_digest_metadata calculates a new range afterward. If the requests cross midnight in the project timezone, the status probe and reported dates use a different day from the headline metrics.
Why we think it's a valid issue
- Checked:
get_overview_for_team(weekly_digest.py:84-130),build_team_digest(weekly_digest.py:322-343),get_digest_metadataand_digest_date_range(weekly_digest.py:278-319), the overview response fields inweb_overview.py, and the cache key and staleness logic inposthog/hogql_queries/query_runner.py,posthog/caching/utils.pyandposthog/interval_specs.py. - Found: The overview response already carries its period:
dateFrom=self.query_date_range.date_from_stranddateTo=self.query_date_range.date_to_str(web_overview.py:325-326).get_overview_for_teamdrops these fields.get_digest_metadatathen builds a newQueryDateRangefromdatetime.now(team.timezone_info)(weekly_digest.py:284,:303). - Found: The midnight crossing inside one request that the reviewer describes needs local midnight to fall between the overview query and the metadata query, which are a few seconds apart. That window is almost impossible to hit.
- Found: Caching makes the mismatch reachable anyway. The digest runs with
ExecutionMode.RECENT_CACHE_CALCULATE_BLOCKING_IF_STALE(weekly_digest.py:46). The cache payload has the relative query (date_from="-7d"), the team and the timezone, but no date (query_router.py:3048-3062), so the key stays the same across days. The query has no interval, so it usesday, withstaleness_default=timedelta(hours=6)(interval_specs.py:104-111,caching/utils.py:93-97). The web analytics runners do not override_is_stale. - Impact: Suppose an overview result is cached shortly before local midnight, and a digest or recap call runs within the next 6 hours. The headline numbers then cover days D-7 to D. But
metadata.date_from/date_to, and the_has_sessions_in_rangeprobe, use days D-6 to D+1. An agent that queries the reported period directly gets different numbers, and a zero-traffic status can come from a different window than the headline. The purpose of this block is to give the exact period behind the numbers. The fix is small: useresponse.dateFrom/dateTofrom the overview for the metadata and the probe. - Priority: Lowered to
consider. The trigger needs a cached overview from before local midnight and a new call within 6 hours. The error is a one-day shift in a 7-day window, in metadata only. The headline metrics stay as correct as before.
Suggested fix
Use the bounds returned by WebOverviewQueryResponse, or pin one timestamp and pass its absolute bounds to the overview and metadata probe.
Prompt to fix with AI (copy-paste)
## Context
@products/web_analytics/backend/weekly_digest.py#L302-306
@products/web_analytics/backend/weekly_digest.py#L341
<issue_description>
The overview runner calculates its date range before the other digest queries. `get_digest_metadata` calculates a new range afterward. If the requests cross midnight in the project timezone, the status probe and reported dates use a different day from the headline metrics.
</issue_description>
<issue_validation>
- **Checked:** `get_overview_for_team` (`weekly_digest.py:84-130`), `build_team_digest` (`weekly_digest.py:322-343`), `get_digest_metadata` and `_digest_date_range` (`weekly_digest.py:278-319`), the overview response fields in `web_overview.py`, and the cache key and staleness logic in `posthog/hogql_queries/query_runner.py`, `posthog/caching/utils.py` and `posthog/interval_specs.py`.
- **Found:** The overview response already carries its period: `dateFrom=self.query_date_range.date_from_str` and `dateTo=self.query_date_range.date_to_str` (`web_overview.py:325-326`). `get_overview_for_team` drops these fields. `get_digest_metadata` then builds a new `QueryDateRange` from `datetime.now(team.timezone_info)` (`weekly_digest.py:284`, `:303`).
- **Found:** The midnight crossing inside one request that the reviewer describes needs local midnight to fall between the overview query and the metadata query, which are a few seconds apart. That window is almost impossible to hit.
- **Found:** Caching makes the mismatch reachable anyway. The digest runs with `ExecutionMode.RECENT_CACHE_CALCULATE_BLOCKING_IF_STALE` (`weekly_digest.py:46`). The cache payload has the relative query (`date_from="-7d"`), the team and the timezone, but no date (`query_router.py:3048-3062`), so the key stays the same across days. The query has no interval, so it uses `day`, with `staleness_default=timedelta(hours=6)` (`interval_specs.py:104-111`, `caching/utils.py:93-97`). The web analytics runners do not override `_is_stale`.
- **Impact:** Suppose an overview result is cached shortly before local midnight, and a digest or recap call runs within the next 6 hours. The headline numbers then cover days D-7 to D. But `metadata.date_from`/`date_to`, and the `_has_sessions_in_range` probe, use days D-6 to D+1. An agent that queries the reported period directly gets different numbers, and a zero-traffic status can come from a different window than the headline. The purpose of this block is to give the exact period behind the numbers. The fix is small: use `response.dateFrom`/`dateTo` from the overview for the metadata and the probe.
- **Priority:** Lowered to `consider`. The trigger needs a cached overview from before local midnight and a new call within 6 hours. The error is a one-day shift in a 7-day window, in metadata only. The headline metrics stay as correct as before.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Use the bounds returned by `WebOverviewQueryResponse`, or pin one timestamp and pass its absolute bounds to the overview and metadata probe.
</potential_solution>
There was a problem hiding this comment.
Escalated. The mismatch is real, but the fix needs a design choice that I should not make unattended.
- The cache key has no absolute date, and day results stay fresh for 6 hours with no cutoff at midnight. So a cached headline from before midnight can sit next to metadata and a probe for the next day.
- Option A: take the metadata and probe bounds from the overview response. This fixes the headline only. Top pages, sources and goals have their own cache entries and can still differ.
- Option A caveat: the response bounds are strings without a timezone, rounded to the second. Their timezone is UTC when the team turns off project-timezone conversion, and the current metadata already mislabels that case.
More detail
- Option B: pin one absolute range for all four digest queries and the probe. This fixes every section, but it changes cache keys and the queries the weekly email uses.
- Option C: keep the code and say in the notes that cached results can trail by up to 6 hours after local midnight. Please pick one.
How this was verified
No code change. I read the cache key, staleness and runner timezone code to confirm the issue. I ran no tests.
DRF converts aware datetimes to the active timezone (UTC) when it serializes them. The digest metadata documents date_from and date_to in the project timezone, so the response now keeps the offset of the value. The OpenAPI schema does not change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: a052359a-f3f4-4a44-b306-2bd516988b8b
There was a problem hiding this comment.
Not approved — this change needs a human reviewer.
Re-add the stamphog label to request another review once you have addressed this.
Reading the diff confirms two of the still-unresolved reviewer concerns are real: the new session probe in get_digest_metadata calls execute_hogql_query with no exception handling, so a probe failure (timeout, ClickHouse error) now aborts the whole digest build even though the core metrics already succeeded — a regression risk for batch weekly-email/notification generation that previously never depended on this query. Separately, the new filter_test_accounts: true metadata field is asserted as applying to every metric, but the unchanged goals query never sets filterTestAccounts, so that claim is false for goal conversions. Both are substantive, unaddressed inline concerns on the current head, not just style nits.
- Unhandled exception risk: _has_sessions_in_range's execute_hogql_query call has no try/except, so a probe failure now breaks the entire digest build (API + batch email/notification paths) even when the core metrics succeeded.
- The new metadata.filter_test_accounts field claims every metric excludes test accounts, but get_goals_for_team's WebGoalsQuery never sets filterTestAccounts, so goal conversions can include test-account activity despite the claim.
- 5 inline review threads (CodeRabbit + posthog[bot]) remain unresolved on the current head, including a Major functional-correctness finding about pageviews-without-in-period-sessions being mislabeled 'ok'.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 112L, 3F substantive, 224L/8F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (224L, 8F, cross-cutting, fix) |
| stamphog 2.2.0 | .stamphog/policy.yml @ cb5b06e · reviewed head cb5b06e |
The goals query does not filter test accounts, but the digest metadata said every metric does. The notes and the filter_test_accounts help_text now name the metrics that exclude test accounts and say that goal conversions include them. Goal counts do not change. Regenerated the OpenAPI types. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: a052359a-f3f4-4a44-b306-2bd516988b8b
|
The two original CI failures are fixed: the recap story and test fixtures now include the required digest metadata. Frontend typechecking passes on the latest head. The new ReviewHog trigger failure is an automation concurrency conflict, unrelated to the diff: the trigger returned HTTP 409 with |
There was a problem hiding this comment.
Stamphog review: error
The review agent could not complete its analysis for this PR (likely too complex for the allocated turn budget). The stamphog label has been kept; a human review is needed.
- Agent could not produce valid structured output after retries
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 116L, 3F substantive, 228L/8F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (228L, 8F, cross-cutting, fix) |
| stamphog 2.2.0 | .stamphog/policy.yml @ 126733c · reviewed head 126733c |
The sessions tables keep only UUIDv7 session IDs. The digest session probe matched any non-empty $session_id, so a project with custom session IDs and no web events got no_web_sessions, but a direct sessions query returned no rows. The probe now requires a UUIDv7 session ID and stays a LIMIT 1 events scan. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: a052359a-f3f4-4a44-b306-2bd516988b8b
There was a problem hiding this comment.
Not approved — this change needs a human reviewer.
Re-add the stamphog label to request another review once you have addressed this.
Four inline review threads (CodeRabbit and the PostHog review bot) flag unresolved functional-correctness bugs in the new probe logic — including one where an unhandled exception from the new probe query can turn an otherwise-successful digest into an API error, which would break the weekly digest email/batch job for a whole project. None of these have been addressed in the diff or discussion; the only follow-up comment addresses unrelated CI flakiness, not these concerns.
- Unresolved CodeRabbit comment: pageviews with a session that started before the period can wrongly report status 'ok' instead of explaining the zero session count.
- Unresolved posthog-review comment: the session probe accepts any non-empty $session_id including invalid IDs, so it can report 'no_web_sessions' when no real session row exists.
- Unresolved posthog-review comment: the digest metadata computes its own date range separately from the overview query, so results can disagree across a midnight boundary in the project timezone.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 117L, 3F substantive, 232L/8F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (232L, 8F, cross-cutting, fix) |
| stamphog 2.2.0 | .stamphog/policy.yml @ 982a086 · reviewed head 982a086 |
The digest serves the overview from a cache that can hold a result from the previous day. The metadata recomputed its period from the current time, so after local midnight the reported dates and the session probe could cover a different day from the headline numbers. The overview now returns the period of its response, and the metadata and the probe use it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: a052359a-f3f4-4a44-b306-2bd516988b8b
There was a problem hiding this comment.
Not approved — this change needs a human reviewer.
Re-add the stamphog label to request another review once you have addressed this.
Reading the diff confirms two substantive, unresolved reviewer concerns: the new status still reports 'ok' when pageviews>0 but sessions=0 (a session starting before the period), which is exactly the ambiguous-zero case this PR exists to explain, and the new session-existence probe query has no exception handling, so a probe failure now aborts a digest build for a team that previously succeeded with an all-zero response — a new production failure mode.
- Unresolved CodeRabbit comment on weekly_digest.py: pageviews>0/sessions=0 (session starting before the period) still yields data_status='ok', mislabeling the exact ambiguous-zero case the PR targets.
- Unaddressed posthog[bot] comment on weekly_digest.py (marked outdated but not fixed in the current diff): _has_sessions_in_range's execute_hogql_query call has no try/except, so a probe failure now raises out of build_team_digest for a team whose metric queries already succeeded, turning a previously-successful zero-value digest into a failed one.
- Related but unfixed minor findings from reviewers: goals query doesn't apply filterTestAccounts despite metadata claiming test accounts are excluded, and the 'ends now' note is inaccurate for the day-granularity date range used.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 123L, 3F substantive, 251L/8F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (251L, 8F, cross-cutting, fix) |
| stamphog 2.2.0 | .stamphog/policy.yml @ 4f40cab · reviewed head 4f40cab |
The session check only explains a zero headline, but its errors aborted a digest whose metric queries had succeeded. The API returned an error, and batch runs marked a zero-traffic project as failed. The check now logs and captures the error and reports a new 'unknown' data status, so a failed check never reads as 'no_sessions'. Regenerated the OpenAPI types for the new status. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: 44eed5d1-e5dc-46c6-bc36-90f1fa33ab50
There was a problem hiding this comment.
Not approved — this change needs a human reviewer.
Re-add the stamphog label to request another review once you have addressed this.
The change is additive to the digest API and the probe-failure and test-account-filter concerns were fixed or are minor, but a substantive, still-unresolved functional-correctness comment from coderabbitai (pageviews with no in-period session can still be labeled data_status "ok") was never addressed by later commits or tests — the diff's status logic still treats pageviews>0 as proof of "ok" exactly as flagged.
- Unresolved coderabbitai inline comment on products/web_analytics/backend/weekly_digest.py ("Handle pageviews without in-period sessions"): a session that starts before the digest period but has an in-period pageview makes get_digest_metadata report data_status='ok' even when sessions=0, contradicting the serializer's own definition of 'ok'. Not fixed by the added parameterized test cases, which cover a different scenario (pageviews entirely outside the period).
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 136L, 3F substantive, 287L/8F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (287L, 8F, cross-cutting, fix) |
| stamphog 2.2.0 | .stamphog/policy.yml @ e0d37de · reviewed head e0d37de |
The 'ok' status said the period has web sessions, but it only checks that pageviews or sessions are above zero. Sessions count by start time, so a period can have pageviews and zero sessions when every pageview belongs to a session that started earlier. The status text now matches the check, and a metric note explains that case. Regenerated the OpenAPI types for the help text. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: 44eed5d1-e5dc-46c6-bc36-90f1fa33ab50
There was a problem hiding this comment.
Not approved — this change needs a human reviewer.
Re-add the stamphog label to request another review once you have addressed this.
CodeRabbit flagged a Major, still-unresolved correctness gap in the new status logic (pageviews-without-in-period-sessions still reports "ok" instead of explaining the zero), confirmed present in the current diff; separately, posthog[bot]'s finding about the metadata date range drifting from the cached headline numbers across midnight was explicitly escalated by the bot itself as needing a human design decision, not fixed. Two substantive, unaddressed concerns on the exact feature this PR adds.
- Unresolved CodeRabbit (Major) comment on weekly_digest.py: when a session starts before the period but has an in-period pageview, sessions=0/pageviews>0 still yields data_status='ok', which the serializer's own docstring says means 'has sessions' — the diff does not add a status for this case.
- posthog[bot]'s inline finding that get_digest_metadata's date range can be computed on a different calendar day than the cached overview response (up to ~6h drift across midnight) was explicitly escalated by the bot as an unresolved design choice (3 options proposed, none picked) — not addressed in this diff.
- Cross-team change (web-analytics + context-mcp) authored by a bot with no owning-team review; author familiarity carries no signal since this is a machine author.
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 138L, 3F substantive, 289L/8F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1d-complex (289L, 8F, cross-cutting, fix) |
| stamphog 2.2.0 | .stamphog/policy.yml @ 3a0fc5b · reviewed head 3a0fc5b |
Problem
web-analytics-weekly-digestMCP tool getsessions: 0for projects that have live session rows. They cannot tell a real zero from a definition mismatch.WebOverviewQuery. It counts only sessions that contain a$pageviewor$screenevent, and it excludes test accounts. A direct count of the sessions table has neither rule.Refs #72105 (a different cause of all-zero digests. This PR does not fix it.)
Origin
Changes
metadatablock:data_statusok,no_web_sessionsorno_sessionsdate_from/date_to/timezonefilter_test_accountstruefor the digestnotesno_web_sessionsmeans: the headline is zero, but the project has events with a$session_idin the period. ALIMIT 1probe query checks this. It runs only when sessions and pageviews are both zero.How did you test this code?
test_weekly_digest.py:test_works_with_no_web_trafficnow has three cases: no events, custom events with a session only, and pageviews outside the period. The custom-events case catches the regression from the report: a project with sessions but no pageviews must returnno_web_sessions, not a plain zero.date_fromandfilter_test_accounts.test_returns_all_expected_keyschecksok. The API shape test includesmetadata.test_weekly_digest.py,test_api.py,test_recap.py,test_weekly_digest_activities.py,test_digest_notification_activities.py.Release status
Automatic notifications
Docs update
None. No doc under
docs/covers the digest response.🤖 Agent context
Autonomy: Fully autonomous
Agent: Claude Code, Claude Opus 5.5 (
claude-opus-5-5)/improving-drf-endpoints,/writing-tests.Created with PostHog Desktop from this inbox report.
🤖 Generated with Claude Code