Repository navigation
Conversation
|
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
|
| Root | Eager (shipped) | Δ vs base | Budget |
|---|---|---|---|
entry (logged-out pages, app bootstrap)src/index.tsx |
1.43 MiB · 22 files | no change | ███░░░░░░░ 31.6% of 4.51 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
8.84 MiB · 3,249 files | 🔺 +168 B (+0.0%) | █████████░ 91.1% of 9.71 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/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers 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/RootErrorBoundary.tsx |
| 912 B | ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js |
| 789 B | src/scenes/ChunkLoadErrorBoundary.tsx |
| 762 B | src/index.tsx |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 307.0 KiB | ../node_modules/.pnpm/posthog-js@1.422.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/rrweb.js |
| 267.7 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 |
| 263.5 KiB | ../node_modules/.pnpm/posthog-js@1.422.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.js |
| 256.1 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 154.2 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 |
| 104.5 KiB | src/lib/api.ts |
| 95.2 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 |
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.26 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.26 MiB · 18 files | 🔺 +16 B (+0.0%) | ████░░░░░░ 39.5% of 5.72 MiB |
| Deferred (lazy) | 2.11 MiB · 45 files | no change | n/a — loads on demand |
Loader dist/toolbar.js |
1.1 KiB | no change | █░░░░░░░░░ 5.8% of 19.5 KiB |
Largest eagerly-shipped chunks
| Size | File |
|---|---|
| 751.7 KiB | dist/toolbar/toolbar-app-JNIXJPZH.css |
| 590.1 KiB | dist/toolbar/chunk-chunk-R5O367IJ.js |
| 484.7 KiB | dist/toolbar/chunk-chunk-OTPWDCQE.js |
| 134.1 KiB | dist/toolbar/chunk-chunk-MAULHS7N.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-FDH2IBXT.js |
| 71.3 KiB | dist/toolbar/toolbar-app-HYCWRENX.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-TSAL54PB.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-SLIUW5HB.js |
| 21.0 KiB | dist/toolbar/chunk-chunk-NLCPKWQX.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 — 🔺 +56.3 KiB (+0.0%)
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 1448.77 MiB · 🔺 +56.3 KiB (+0.0%)
ℹ️ MCP UI apps size — 33 app(s), 17662.9 KB JS
Built size of each MCP UI app (main.js + styles.css).
| App | JS | CSS |
|---|---|---|
| debug | 600.0 KB | 195.2 KB |
| action | 458.2 KB | 195.2 KB |
| action-list | 564.9 KB | 195.2 KB |
| cohort | 457.2 KB | 195.2 KB |
| cohort-list | 563.8 KB | 195.2 KB |
| email-template | 457.0 KB | 195.2 KB |
| error-details | 472.9 KB | 195.2 KB |
| error-issue | 457.9 KB | 195.2 KB |
| error-issue-list | 564.7 KB | 195.2 KB |
| experiment | 562.0 KB | 195.2 KB |
| experiment-list | 565.6 KB | 195.2 KB |
| experiment-results | 567.1 KB | 195.2 KB |
| feature-flag | 567.6 KB | 195.2 KB |
| feature-flag-list | 571.4 KB | 195.2 KB |
| feature-flag-testing | 461.4 KB | 195.2 KB |
| inline-scan | 457.7 KB | 195.2 KB |
| insight-actors | 563.0 KB | 195.2 KB |
| invite-email-preview | 456.4 KB | 195.2 KB |
| llm-costs | 560.0 KB | 195.2 KB |
| session-recording | 459.0 KB | 195.2 KB |
| survey | 458.8 KB | 195.2 KB |
| survey-global-stats | 562.7 KB | 195.2 KB |
| survey-list | 565.5 KB | 195.2 KB |
| survey-stats | 562.7 KB | 195.2 KB |
| trace-span | 457.6 KB | 195.2 KB |
| trace-span-list | 564.7 KB | 195.2 KB |
| vision-observation-list | 563.9 KB | 195.2 KB |
| workflow | 457.5 KB | 195.2 KB |
| workflow-list | 564.2 KB | 195.2 KB |
| loops-review | 461.9 KB | 195.2 KB |
| query-results | 755.2 KB | 195.2 KB |
| render-ui | 838.3 KB | 195.2 KB |
| visual-review-snapshots | 462.0 KB | 195.2 KB |
⚠️ Backend coverage — 90.0% of changed backend lines covered — 32 uncovered
🧪 Backend test coverage
Patch coverage — changed backend lines (products + core): ██████████████████░░ 90.0% (313 / 345)
| File | Patch | Uncovered changed lines |
|---|---|---|
products/experiments/backend/temporal/metric_resolution.py |
44.4% | 70–74 |
products/experiments/backend/running_time_estimate_service.py |
68.6% | 46–47, 49–52, 54–56, 58–61, 96–97, 102 |
products/experiments/backend/presentation/views.py |
85.2% | 169, 172–173, 175 |
products/experiments/backend/running_time_calculator.py |
94.4% | 294, 316, 383, 411, 415, 419 |
products/experiments/backend/test/test_running_time_calculator.py |
99.1% | 298 |
🤖 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 33560772125 -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.4% |
91 / 309 |
demo |
███████████░░░░░░░░░ 56.9% |
1,518 / 2,666 |
autoresearch |
████████████░░░░░░░░ 62.5% |
5 / 8 |
data_tools |
█████████████░░░░░░░ 66.2% |
90 / 136 |
ai_gateway |
███████████████░░░░░ 75.0% |
9 / 12 |
batch_exports |
████████████████░░░░ 79.9% |
19,710 / 24,669 |
apm |
█████████████████░░░ 83.3% |
1,234 / 1,481 |
signals |
█████████████████░░░ 84.5% |
33,009 / 39,077 |
cdp |
█████████████████░░░ 85.2% |
4,310 / 5,056 |
wizard |
█████████████████░░░ 86.7% |
1,090 / 1,257 |
data_modeling |
█████████████████░░░ 86.7% |
9,654 / 11,133 |
notebooks |
██████████████████░░ 87.7% |
11,202 / 12,770 |
product_tours |
██████████████████░░ 87.9% |
1,303 / 1,482 |
data_warehouse |
██████████████████░░ 88.6% |
12,782 / 14,425 |
engineering_analytics |
██████████████████░░ 89.0% |
7,707 / 8,656 |
cohorts |
██████████████████░░ 89.1% |
7,370 / 8,273 |
business_knowledge |
██████████████████░░ 89.1% |
4,440 / 4,984 |
dashboards |
██████████████████░░ 89.6% |
6,807 / 7,598 |
tasks |
██████████████████░░ 90.0% |
63,198 / 70,232 |
streamlit_apps |
██████████████████░░ 90.2% |
2,500 / 2,773 |
exports |
██████████████████░░ 90.5% |
8,717 / 9,637 |
links |
██████████████████░░ 90.6% |
183 / 202 |
managed_warehouse |
██████████████████░░ 90.7% |
8,563 / 9,436 |
alerts |
██████████████████░░ 90.9% |
5,182 / 5,700 |
visual_review |
██████████████████░░ 90.9% |
6,880 / 7,567 |
conversations |
██████████████████░░ 91.0% |
20,910 / 22,973 |
canvas |
██████████████████░░ 91.0% |
4,913 / 5,397 |
mcp_analytics |
██████████████████░░ 91.2% |
4,374 / 4,795 |
error_tracking |
██████████████████░░ 91.5% |
12,672 / 13,853 |
notifications |
██████████████████░░ 92.2% |
1,101 / 1,194 |
early_access_features |
███████████████████░ 92.5% |
1,323 / 1,430 |
managed_migrations |
███████████████████░ 92.6% |
1,556 / 1,681 |
slack_app |
███████████████████░ 92.8% |
12,354 / 13,315 |
stamphog |
███████████████████░ 92.9% |
5,775 / 6,216 |
web_analytics |
███████████████████░ 93.1% |
18,597 / 19,966 |
context_layer |
███████████████████░ 93.3% |
2,573 / 2,758 |
surveys |
███████████████████░ 93.4% |
6,051 / 6,482 |
reminders |
███████████████████░ 93.4% |
468 / 501 |
mcp_store |
███████████████████░ 93.4% |
7,609 / 8,144 |
posthog_ai |
███████████████████░ 93.5% |
1,372 / 1,468 |
marketing_analytics |
███████████████████░ 93.5% |
17,322 / 18,526 |
ai_observability |
███████████████████░ 93.7% |
17,988 / 19,194 |
approvals |
███████████████████░ 93.7% |
3,492 / 3,725 |
billing_alerts |
███████████████████░ 93.9% |
2,078 / 2,212 |
workflows |
███████████████████░ 94.1% |
11,042 / 11,739 |
legal_documents |
███████████████████░ 94.1% |
1,970 / 2,093 |
review_hog |
███████████████████░ 94.3% |
9,982 / 10,583 |
endpoints |
███████████████████░ 94.4% |
8,866 / 9,396 |
tracing |
███████████████████░ 94.4% |
2,678 / 2,837 |
access_control |
███████████████████░ 94.9% |
5,828 / 6,139 |
customer_analytics |
███████████████████░ 95.0% |
17,702 / 18,639 |
annotations |
███████████████████░ 95.2% |
795 / 835 |
product_analytics |
███████████████████░ 95.3% |
19,292 / 20,252 |
replay_vision |
███████████████████░ 95.4% |
26,086 / 27,355 |
experiments |
███████████████████░ 95.4% |
29,583 / 31,004 |
actions |
███████████████████░ 95.5% |
706 / 739 |
skills |
███████████████████░ 95.6% |
5,403 / 5,651 |
messaging |
███████████████████░ 95.8% |
3,636 / 3,795 |
logs |
███████████████████░ 95.9% |
13,495 / 14,077 |
growth |
███████████████████░ 95.9% |
7,644 / 7,969 |
feature_flags |
███████████████████░ 96.1% |
19,585 / 20,382 |
revenue_analytics |
███████████████████░ 96.3% |
1,841 / 1,911 |
user_interviews |
███████████████████░ 96.5% |
2,639 / 2,735 |
warehouse_sources |
███████████████████░ 97.2% |
406,037 / 417,835 |
data_quality |
███████████████████░ 97.5% |
4,103 / 4,209 |
analytics_platform |
████████████████████ 98.1% |
2,286 / 2,330 |
data_catalog |
████████████████████ 98.3% |
3,234 / 3,289 |
metrics |
████████████████████ 98.3% |
3,505 / 3,564 |
pulse |
████████████████████ 98.4% |
2,017 / 2,049 |
live_debugger |
████████████████████ 99.2% |
613 / 618 |
field_notes |
████████████████████ 99.4% |
158 / 159 |
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.
14dc07a to
08e9edb
Compare
c296a05 to
e1dd7f0
Compare
5cc9701 to
b932551
Compare
🦔 Hogbox preview · ❌ build failedThe preview didn't come up for commit Previews are optional and never block merging. A failure here is often a hogland or tailnet hiccup rather than anything in your PR, so the check stays green and this comment is the status. |
PR overviewThis pull request adds an endpoint that estimates experiment running times using experiment metrics and result data. One issue has been addressed, but the new endpoint still allows an authenticated member to trigger amplified PostgreSQL workloads by repeatedly requesting batches of accessible experiments. The remaining concern is limited to resource consumption but could degrade database or service performance until metric selection or endpoint throttling is tightened. Open issues (1)
Fixed/addressed: 1 · PR risk: 5/10 |
Prompt To Fix All With AI### Issue 1
products/experiments/backend/running_time_estimate_service.py:91
**Result refreshes miss cache invalidation**
When a fresh metric recalculation completes, this reads `query_to` from the nested result blob even though the value belongs to the `ExperimentMetricResult` row. The cache key therefore remains unchanged and the endpoint returns stale exposure and running-time estimates for up to 15 minutes.
### Issue 2
products/experiments/backend/running_time_estimate_service.py:86
**Saved primary metrics are omitted**
When a saved metric is the first ordered primary metric, this selects exclusively from `experiment.metrics` and instead chooses an inline metric or no metric. The endpoint then calculates a different target and remaining duration from the detail page, which merges saved and inline primary metrics before selecting.
### Issue 3
products/experiments/backend/running_time_calculator.py:227-229
**Result blobs lack concrete types**
The new production calculation API represents metric and result structures with `dict[str, Any]`, and the service also introduces unparameterized dictionaries. This bypasses the repository's strict typing requirement across the main inputs to the estimate calculation; define concrete typed structures for these result and metric shapes instead.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "chore: update OpenAPI generated types" | Re-trigger Greptile |
| return None | ||
|
|
||
| return BaselineStats( |
There was a problem hiding this comment.
Result blobs lack concrete types
The new production calculation API represents metric and result structures with dict[str, Any], and the service also introduces unparameterized dictionaries. This bypasses the repository's strict typing requirement across the main inputs to the estimate calculation; define concrete typed structures for these result and metric shapes instead.
File Used: AGENTS.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: products/experiments/backend/running_time_calculator.py
Line: 227-229
Comment:
**Result blobs lack concrete types**
The new production calculation API represents metric and result structures with `dict[str, Any]`, and the service also introduces unparameterized dictionaries. This bypasses the repository's strict typing requirement across the main inputs to the estimate calculation; define concrete typed structures for these result and metric shapes instead.
**File Used:** `AGENTS.md` ([source](https://github.com/posthog/posthog/blob/b9325515dbe9cba62c84d02ac7bfebbf17acdba5/AGENTS.md))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Leaving as-is intentionally. The result blob is a model-dumped ExperimentQueryResponse (dynamic JSON that already has a pydantic type upstream), and the calculator deliberately mirrors the frontend's loose baseline shape. Those raw values immediately land in the typed BaselineStats dataclass at the boundary, which is where the typing guarantee matters. A parallel typed DTO for the raw blob would duplicate the schema type without adding safety past that boundary.
There was a problem hiding this comment.
That context is fair. Since this intentionally consumes a model-dumped dynamic JSON blob and immediately normalizes it into the existing BaselineStats dataclass, adding a parallel DTO would duplicate the upstream schema without improving this path. I’m withdrawing the typing comment; leaving dict[str, Any] here is reasonable.
| results = get_run_results(recalc) | ||
| query_to = recalc.query_to | ||
| else: | ||
| payload = build_timeseries_cold_start_payload(experiment) |
There was a problem hiding this comment.
Low: Unthrottled database amplification
On the cold-start path, build_timeseries_cold_start_payload enumerates every metric and performs a result query for each one, even though this endpoint needs only the selected sizing metric. This runs before the cache lookup and is repeated for up to 100 experiments; session-authenticated requests also bypass the default API-key throttles. An authenticated member can repeatedly submit batches of accessible experiment IDs to generate disproportionate PostgreSQL load. Resolve only the selected metric's latest result and/or add an endpoint throttle that covers session users.
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, please remove the |
|
This PR was closed due to lack of activity. Feel free to reopen if it's still relevant. |
Problem
The experiments list shows each running experiment's remaining time from a value the frontend persists back onto the experiment on every exposure refresh. That auto-save churns the experiment version and pollutes activity history. This PR adds the read-only endpoint the list needs, so remaining time can be computed on demand instead of stored.
Changes
This PR adds a backend endpoint that returns running-time estimates for a batch of experiments, keyed by id. It is the first half of moving remaining-time off persisted experiment state; the frontend list migration and the auto-save removal are follow-ups.
The endpoint
GET /api/projects/{id}/experiments/running_time_estimates/?ids=12,45returns{"results": {"12": {...}, ...}}. It is read-only and batch-first, so the list can fetch every visible running row in one call. It is team-scoped through the viewset queryset, caps the id count, and omits ids with no matching experiment. The existingcalculate_running_timePOST is untouched; it stays the pure calculator behind the running-time modal.products/experiments/backend/presentation/views.pyproducts/experiments/backend/presentation/serializers.pyproducts/experiments/backend/facade/api.pyEstimate math
Each estimate mirrors the frontend
runningTimeLogicso the list and the detail page agree. Automatic mode reads the sizing metric's latest completedExperimentMetricResultand derives the baseline and exposure rate; manual mode reads the savedexposure_estimate_config. The sizing metric is the first primary metric, classified the same way as the frontendgetCalculatorMetricType, with no extra exclusion so the two surfaces stay consistent.products/experiments/backend/running_time_calculator.pyproducts/experiments/backend/running_time_estimate_service.pyCaching
The per-experiment estimate reads through a 15 minute Django cache. The key folds in the experiment and feature-flag
updated_atand the result window, so an edit or a fresher result misses the cache instead of serving stale numbers. A cache miss computes from Postgres only; no ClickHouse query runs on this path.products/experiments/backend/running_time_estimate_service.pyGenerated types
Regenerated from the new serializer and action with
hogli build:openapi.products/experiments/frontend/generated/api.ts(generated)products/experiments/frontend/generated/api.schemas.ts(generated)services/mcp/src/api/generated.ts(generated)How did you test this code?
Most of the logic is pure and covered by fast unit tests: the result-blob mapping, sizing-metric selection, and the manual and automatic estimate branches, including the too-little-data fallback. One DB-backed test class guards the endpoint wiring: the keyed response shape, the team boundary, and the empty-ids case.
Automatic notifications
Docs update
None. No user-facing behavior changes yet; this endpoint has no consumer until the list migration lands.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Model: Opus 4.8 (1M context)
Manually refactored: no
Skills used:
Relevant decisions:
calculate_running_time, so the pure stateless calculator stays pure and the experiment-reading logic lives on its own read path.GET ?ids=returning a dict keyed by id, following thesurveysresponses_countprecedent, since the value is read-only and the list needs every visible row at once.facade/api.py, not the service module directly, so the product-lint import surface stays clean; the compute logic sits inrunning_time_estimate_service.py.