fix(experiments): update error classification to prevent unnecessary retries - #70915
Merged
rodrigoi merged 3 commits intoJul 15, 2026
Merged
Conversation
This was referenced Jul 14, 2026
Member
Author
rodrigoi
force-pushed
the
experiments/recalculation-workflow-org-fairness
branch
from
July 15, 2026 03:46
4e8c12d to
360827d
Compare
rodrigoi
force-pushed
the
experiments/recalculation-workflow-error-classification
branch
from
July 15, 2026 03:46
a8dbe62 to
9c71075
Compare
rodrigoi
marked this pull request as ready for review
July 15, 2026 03:48
Contributor
|
Reviews (1): Last reviewed commit: "fix(experiments): in general, value erro..." | Re-trigger Greptile |
Contributor
🤖 CI report
|
| File | Patch | Uncovered changed lines |
|---|---|---|
products/experiments/backend/temporal/recalculation_metrics.py |
62.5% | 118, 142–143 |
🤖 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 29453971439 -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 |
batch_exports |
████████░░░░░░░░░░░░ 39.7% |
8,414 / 21,220 |
demo |
███████████░░░░░░░░░ 56.2% |
1,497 / 2,663 |
warehouse_sources_queue |
████████████░░░░░░░░ 59.2% |
148 / 250 |
tasks |
█████████████░░░░░░░ 67.4% |
25,560 / 37,895 |
data_tools |
██████████████░░░░░░ 70.0% |
63 / 90 |
ai_gateway |
███████████████░░░░░ 75.0% |
9 / 12 |
signals |
████████████████░░░░ 79.1% |
19,035 / 24,073 |
data_modeling |
████████████████░░░░ 80.0% |
4,834 / 6,045 |
cdp |
████████████████░░░░ 80.7% |
3,118 / 3,864 |
wizard |
████████████████░░░░ 82.5% |
772 / 936 |
notebooks |
█████████████████░░░ 84.3% |
6,343 / 7,520 |
agent_platform |
█████████████████░░░ 84.7% |
3,273 / 3,862 |
cohorts |
█████████████████░░░ 86.1% |
4,022 / 4,671 |
actions |
█████████████████░░░ 86.6% |
717 / 828 |
product_tours |
█████████████████░░░ 87.5% |
1,266 / 1,447 |
exports |
██████████████████░░ 88.3% |
6,891 / 7,800 |
visual_review |
██████████████████░░ 88.5% |
5,565 / 6,287 |
business_knowledge |
██████████████████░░ 88.5% |
4,400 / 4,969 |
conversations |
██████████████████░░ 88.9% |
16,129 / 18,133 |
dashboards |
██████████████████░░ 89.1% |
5,719 / 6,418 |
mcp_analytics |
██████████████████░░ 89.1% |
2,502 / 2,807 |
error_tracking |
██████████████████░░ 89.6% |
9,718 / 10,852 |
alerts |
██████████████████░░ 89.9% |
3,638 / 4,046 |
engineering_analytics |
██████████████████░░ 90.1% |
5,105 / 5,665 |
streamlit_apps |
██████████████████░░ 90.4% |
2,499 / 2,764 |
slack_app |
██████████████████░░ 90.6% |
9,511 / 10,503 |
links |
██████████████████░░ 90.6% |
183 / 202 |
marketing_analytics |
██████████████████░░ 90.8% |
11,514 / 12,684 |
product_analytics |
██████████████████░░ 91.1% |
5,599 / 6,143 |
mcp_store |
██████████████████░░ 91.8% |
3,668 / 3,995 |
managed_migrations |
██████████████████░░ 91.9% |
908 / 988 |
data_warehouse |
██████████████████░░ 92.1% |
18,133 / 19,683 |
workflows |
██████████████████░░ 92.4% |
5,148 / 5,574 |
notifications |
███████████████████░ 92.7% |
1,026 / 1,107 |
web_analytics |
███████████████████░ 92.7% |
13,624 / 14,691 |
ai_observability |
███████████████████░ 92.8% |
14,868 / 16,019 |
surveys |
███████████████████░ 92.9% |
5,660 / 6,094 |
posthog_ai |
███████████████████░ 93.2% |
1,322 / 1,418 |
tracing |
███████████████████░ 93.3% |
2,435 / 2,611 |
approvals |
███████████████████░ 93.3% |
3,395 / 3,640 |
reminders |
███████████████████░ 93.4% |
468 / 501 |
early_access_features |
███████████████████░ 93.8% |
848 / 904 |
legal_documents |
███████████████████░ 94.1% |
1,568 / 1,667 |
endpoints |
███████████████████░ 94.1% |
8,606 / 9,143 |
skills |
███████████████████░ 94.4% |
2,827 / 2,995 |
revenue_analytics |
███████████████████░ 94.5% |
3,598 / 3,809 |
messaging |
███████████████████░ 94.5% |
2,530 / 2,677 |
review_hog |
███████████████████░ 94.6% |
6,529 / 6,902 |
logs |
███████████████████░ 95.3% |
9,528 / 9,994 |
experiments |
███████████████████░ 95.6% |
24,171 / 25,288 |
replay_vision |
███████████████████░ 95.7% |
13,337 / 13,935 |
growth |
███████████████████░ 95.8% |
2,837 / 2,960 |
annotations |
███████████████████░ 96.2% |
732 / 761 |
warehouse_sources |
███████████████████░ 96.2% |
225,590 / 234,493 |
feature_flags |
███████████████████░ 96.3% |
16,002 / 16,625 |
user_interviews |
███████████████████░ 96.4% |
2,242 / 2,325 |
access_control |
███████████████████░ 96.8% |
849 / 877 |
data_catalog |
███████████████████░ 97.1% |
2,035 / 2,095 |
customer_analytics |
███████████████████░ 97.2% |
7,480 / 7,698 |
analytics_platform |
████████████████████ 98.0% |
2,102 / 2,145 |
metrics |
████████████████████ 98.2% |
2,404 / 2,448 |
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.
jurajmajerik
approved these changes
Jul 15, 2026
rodrigoi
force-pushed
the
experiments/recalculation-workflow-org-fairness
branch
from
July 15, 2026 17:44
360827d to
485ac0a
Compare
rodrigoi
force-pushed
the
experiments/recalculation-workflow-error-classification
branch
2 times, most recently
from
July 15, 2026 18:40
7668cab to
ff1a0f2
Compare
rodrigoi
force-pushed
the
experiments/recalculation-workflow-org-fairness
branch
2 times, most recently
from
July 15, 2026 19:09
271dd94 to
d9ccf1f
Compare
rodrigoi
force-pushed
the
experiments/recalculation-workflow-error-classification
branch
from
July 15, 2026 19:09
ff1a0f2 to
605d105
Compare
rodrigoi
force-pushed
the
experiments/recalculation-workflow-org-fairness
branch
from
July 15, 2026 20:54
d9ccf1f to
c6c755e
Compare
rodrigoi
force-pushed
the
experiments/recalculation-workflow-error-classification
branch
from
July 15, 2026 20:54
605d105 to
222218d
Compare
Base automatically changed from
experiments/recalculation-workflow-org-fairness
to
master
July 15, 2026 21:58
rodrigoi
force-pushed
the
experiments/recalculation-workflow-error-classification
branch
from
July 15, 2026 22:01
222218d to
5de6182
Compare
rodrigoi
deleted the
experiments/recalculation-workflow-error-classification
branch
July 15, 2026 22:52
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Problem
Stacked on #70807. With Temporal owning retries (#70793) on an 8-attempt budget (#70801), every raising error gets the same treatment, but the failure classes aren't equally retryable. Production data says so directly: the terminal errors the recalc path actually produces are ClickHouse at-capacity rate limiting and out-of-memory, and the attempt histogram shows successes are overwhelmingly first-try with a small tail rescued on attempts 2-3. An out-of-memory query is deterministic for a fixed query shape and window, so retrying it 8 times burns 8 identical ClickHouse queries to reach the same FAILED row, while the metric sits in a loading state the whole time.
The redesign also left two observability distortions: the schedule-to-start histogram measures from the original schedule time, so under server-side retries it folds backoff (including the intentional 60s quota waits) into the queue-pressure signal the autoscaler story depends on; and the failures counter counts every intentional backpressure bounce as a failure, skewing failure-rate dashboards.
Changes
Deterministic failures fail fast
New
NON_RETRYABLE_ERROR_TYPES(out_of_memory,byte_limit,validation_error), keyed on the existingclassify_experiment_query_errortaxonomy. When the calc activity hits one, it persists the FAILED row immediately, even on a non-final attempt, and raisesApplicationError(non_retryable=True). One ClickHouse query instead of eight, and the user sees the error right away.Bare
ValueErroris also non-retryable, by exception class: everyValueErrorreachable from the modern calc path is a deterministic guard (baseline variant misconfiguration like "No control variant found", session property validation, step builder invariants) that needs experiment-level intervention to fix, never a transient condition. Retyping those raises at the source is a larger refactor other parts of the system depend on, so retryability keys on the class here while the taxonomy continues to report them asserver_error.timeoutis deliberately not in the set: the class conflatesTIMEOUT_EXCEEDED(deterministic) withSOCKET_TIMEOUT(a transient network blip that retry genuinely fixes), and recalc timeouts are effectively nonexistent in production.products/experiments/backend/temporal/models.pyproducts/experiments/backend/temporal/recalculation_logic.pyAt-capacity joins the backpressure path
ClickHouseAtCapacityis now handled likeConcurrencyLimitExceeded(#70801): 60snext_retry_delay, no exception capture, persisted only on the final attempt. Cluster-level saturation and org-level quota have identical semantics, and the at-capacity message is what the recalc path's real rate-limited terminal errors say.products/experiments/backend/temporal/recalculation_logic.pyObservability fixes
The schedule-to-start histogram now measures from
current_attempt_scheduled_time, so retry backoff no longer reads as queue pressure. The failures counter gains anerror_typeattribute (ApplicationError.typewhen set, so backpressure bounces and permanent classes are named; exception class name otherwise), letting dashboards exclude intentional waits from failure rates.products/experiments/backend/temporal/recalculation_metrics.pyHow did you test this code?
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
Automatic notifications
Docs update
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Model: Fable 5
Manually refactored: yes
Skills used:
Relevant decisions:
classify_experiment_query_errortaxonomy rather than fresh exception matching, so the activity's retry policy and the error-event dashboards share one source of truth and can't drift.ValueErrors are made non-retryable by exception class inside the activity instead of retyping the raises toValidationErrorat the source; the retype was surveyed and rejected for this PR because other parts of the system depend on the current types.timeoutstays retryable, reversing an earlier lean, based on production evidence: zero recalc timeouts in 90 days, and the class includes socket timeouts that a retry genuinely fixes.error_typeusesApplicationError.typewith a class-name fallback, keeping label cardinality bounded to our own taxonomy strings plus the handful of exception classes this code raises.