Skip to content

Fix seed PR backfill interval overrides#5764

Draft
eakmanrq wants to merge 2 commits intomainfrom
eakmanrq/seed-pr-fix-review
Draft

Fix seed PR backfill interval overrides#5764
eakmanrq wants to merge 2 commits intomainfrom
eakmanrq/seed-pr-fix-review

Conversation

@eakmanrq
Copy link
Copy Markdown
Collaborator

Description

Prevent PR plans with seed changes from reusing stale prod interval ends for new snapshot versions. This falls back to execution time when a carried-over default end would be older than the requested start, and filters stale end overrides after refreshing snapshot intervals so downstream snapshots are still backfilled and their physical tables get created. It also adds a regression test covering a seed change with start="2 months ago".

Test Plan

  • pytest tests/core/test_context.py -k 'test_plan_seed_model_excluded_from_default_end or test_seed_model_pr_plan_with_stale_prod_intervals'\n\n## Checklist\n\n- [ ] I have run make style and fixed any issues\n- [x] I have added tests for my changes (if applicable)\n- [ ] All existing tests pass (make fast-test)\n- [x] My commits are signed off (git commit -s) per the DCO\n

Signed-off-by: eakmanrq <6326532+eakmanrq@users.noreply.github.com>
@eakmanrq eakmanrq marked this pull request as draft April 13, 2026 21:52
@eakmanrq eakmanrq requested a review from Copilot April 14, 2026 01:42
Signed-off-by: eakmanrq <6326532+eakmanrq@users.noreply.github.com>
@eakmanrq eakmanrq force-pushed the eakmanrq/seed-pr-fix-review branch from 596e55e to 60086e2 Compare April 14, 2026 01:43
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes planning behavior for PR/dev plans involving seed changes by preventing stale prod interval-end overrides from being reused for new snapshot versions (which could otherwise cause missing intervals to be skipped and downstream physical tables not to be created).

Changes:

  • Compute a single plan_execution_time and reuse it across default start/end and start-override calculations.
  • Refresh snapshot intervals and then filter out stale per-model end overrides for snapshots whose new versions have no intervals yet.
  • Add a regression test reproducing the stale end-override scenario with start="2 months ago" and a seed change.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
sqlmesh/core/context.py Adds plan_execution_time reuse and filters stale end_override_per_model entries after interval refresh.
tests/core/test_context.py Adds a slow regression test covering stale prod interval-end overrides during a PR plan with a seed change.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread sqlmesh/core/context.py
# This ensures that no models outside the impacted sub-DAG(s) will be backfilled unexpectedly.
backfill_models = modified_model_names or None

plan_execution_time = execution_time or now()
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

plan_execution_time = execution_time or now() will ignore valid falsy execution_time values (e.g., epoch millis 0) and silently replace them with now(). Use an explicit None check (e.g., execution_time if execution_time is not None else now()) to preserve caller-provided values.

Suggested change
plan_execution_time = execution_time or now()
plan_execution_time = execution_time if execution_time is not None else now()

Copilot uses AI. Check for mistakes.
Comment on lines +1226 to +1230
@pytest.mark.slow
def test_seed_model_pr_plan_filters_stale_end_override(
copy_to_temp_path: t.Callable, mocker: MockerFixture
):
path = copy_to_temp_path("examples/sushi")
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

PR description/test plan references test_seed_model_pr_plan_with_stale_prod_intervals, but the added regression test is named test_seed_model_pr_plan_filters_stale_end_override. Consider updating the PR description/test plan (or renaming the test) so the suggested -k selector matches.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants