Skip to content

fix(data-modeling): create tier schedule when update finds it gone - #116697

Draft
posthog[bot] wants to merge 1 commit into
masterfrom
posthog-self-driving/fixdata-modeling-create-tier-schedule-8740b5
Draft

posthog[bot] wants to merge 1 commit into
masterfrom
posthog-self-driving/fixdata-modeling-create-tier-schedule-8740b5

Conversation

@posthog

@posthog posthog Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

Problem

  • A user who enables materialization can have it switched back off with no notice. The view then stops refreshing.
  • _apply_reconciliation plans from temporal.list_schedules, which reads Temporal's eventually consistent visibility store.
  • A tier schedule that a teardown or a concurrent reconcile just deleted can still appear in that listing. The plan puts it in to_update, and a_update_schedule raises RPCError NOT_FOUND.
  • The error aborts the reconcile. On the bootstrap path, _bootstrap_dag_best_effort then sets is_materialized=False.
  • The to_create branch already handles the opposite race (ScheduleAlreadyRunningError falls back to update). The to_update branch had no fallback.

Origin

  • Error tracking: issue
  • First signal: 2026-10-14
  • Inbox report: open
  • Likely cause: 0fa379d
  • Task started by: auto-start, after the report was rated P2 and ready to fix

Changes

  • A NOT_FOUND on update now falls back to create. If that create finds the schedule already exists (a concurrent reconcile won), it updates again.
  • A schedule created through this fallback goes into created, so a later failure in the same run rolls it back, the same as for a normal create.
  • Mechanical: the create-then-update logic moves into a local create_or_converge helper that both loops share. Other RPCError statuses still raise as before.

How did you test this code?

  • hogli test products/data_modeling/backend/test/test_schedule_reconcile.py passes locally.
  • The new test fails on master and passes with this change.
  • mypy on schedule_reconcile.py passes.

Test rationale: The new test, test_listed_tier_that_is_already_gone_is_recreated_and_stays_materialized, catches a regression where a listed tier that is already gone aborts the bootstrap and switches materialization off. The closest existing test, test_concurrent_create_converges_to_update_without_rollback, covers only the opposite race on the create path. The test runs through bootstrap_dag_to_tiers, because the user-visible symptom is the is_materialized rollback on that path.

Release status

  • No feature flag controls this change

Docs update

None. No user-facing behavior changes beyond the bug fix.

🤖 Agent context

Autonomy: Fully autonomous

Agent: Claude Code, Claude Opus 5.5 (claude-opus-5-5)

  • Started from an error tracking inbox report. A search found no open PR that fixes this race. #116509 handles Temporal connect failures in nearby files, but it does not touch schedule_reconcile.py, so the two PRs do not conflict.
  • No session material is in the diff. The test data is invented.

🤖 Generated with Claude Code


Created with PostHog Desktop from this inbox report.

The schedule listing reads Temporal's eventually consistent visibility store, so it can name a tier schedule that is already deleted. The update then fails with NOT_FOUND, the reconcile aborts, and a bootstrap marks the saved query not materialized. Fall back to create, and to update again if a concurrent reconcile created it first.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Generated-By: PostHog Desktop
Task-Id: a26fc1c4-c176-4b6f-a499-6349484d24a5
@posthog posthog Bot added the self-driving label Oct 14, 2026
@trunk-io

trunk-io Bot commented Oct 14, 2026

Copy link
Copy Markdown

Merging to master in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

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

@posthog
posthog Bot requested a review from sakce October 14, 2026 01:06
@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

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

✅ Duplication (Python) — clean

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) — clean

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.

🚨 Comment density — 12% of added code lines are comments (6 of 52)

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/data_modeling/backend/logic/schedule_reconcile.py 4 16
products/data_modeling/backend/test/test_schedule_reconcile.py 2 36

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

@stamphog stamphog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved.

Contained scheduling race fix with regression coverage; no model, API, ingestion, or infrastructure contract changes and no open review concerns. The verified Inbox provenance exempts the machine author from the bot-author refusal rule.

Gate mechanics and policy version
Gate Result
prerequisites ✓ all clear
deny-list ✓ no deny categories matched
size ✓ 28L, 1F substantive, 71L/2F incl. docs/generated/snapshots — within ceiling
tier ✓ T1-agent / T1b-small (71L, 2F, single-area, fix)
stamphog 2.5.0 .stamphog/policy.yml @ 2b52f61 · reviewed head 2b52f61

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant