Repository navigation
Conversation
32bb25f to
6731332
Compare
Signed-off-by: mday-io <mdaytn@gmail.com>
6731332 to
aeb4097
Compare
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Preserve deployed hashes sqlmesh/core/selector.py |
select_models records serialized definitions and deployed hashes for unselected external models before updating schemas. It restores those hashes only when a model remains present and its serialized definition is unchanged. |
Selector test coverage tests/core/test_selector_native.py |
Tests cover preserved fingerprints for an unselected external coparent and direct or indirect modification behavior when a selected external model's schema changes. |
Estimated code review effort: 2 (Simple) | ~15 minutes
Change: Bug fix
Merge Risk: 
aeb40
-
No merge-blocking issue is established from the supplied evidence.
Security architecture risk: 
aeb40
-
The change narrowly preserves deployed identities during selected planning. No new access or authority was identified, but interrupted or overlapping planning remains less well established.
Retained concerns
No architecture-level concerns identified.Security review details
-
Security Blast Radius
- inferred — The demonstrated effect is on planning identity within the resolved target or existing fallback environment's model graph, including unselected external co-parents and descendant fingerprints. The changed path does not introduce another environment-resolution mechanism.
Trust Boundaries and Controls
- observed — Unselected model definitions continue to come from the configured state reader, while selected definitions come from the local model collection. Preserved hashes are captured from the environment-sourced objects, not from an unselected local edit.
Resilience and Maintainability Implications
- inferred — Refresh is not transactionally isolated from cached environment objects: cached snapshots return their model nodes directly, and schema processing mutates hashes before all work completes. An exception skips the new restoration loop. This ownership and partial-failure exposure predates the PR; the available comparison does not establish a worsened security outcome.
-
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Title check The title clearly and concisely describes the primary change: preventing selected plans from rehashing unselected external models. Description check The description includes the required Description, Test Plan, and Checklist sections. It explains the cause, implementation, scope, regression coverage, and reported test results. The unchecked CI ite… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Description check
-
Explanation
The description includes the required Description, Test Plan, and Checklist sections. It explains the cause, implementation, scope, regression coverage, and reported test results. The unchecked CI item is a non-critical remaining status item and does not make the description incomplete.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 4 functions across 2 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Description
Why
Schema refresh during a selected plan can recompute cached fingerprints for an unselected external co-parent sourced from deployed state. When the persisted fingerprint differs from a fresh calculation, the plan can report external and downstream changes outside the selection.
How
Preserve the deployed data and metadata hashes for unselected external models in the schema-update DAG when their serialized definitions remain unchanged. Selected external edits, selected dependency schema changes and non-external models keep the existing behavior.
The regression deliberately injects inconsistent persisted data and metadata hashes. It proves this preservation invariant; it does not claim that a normal SQLMesh migration creates the mismatches.
Test Plan
make stylepassed all four hooks.make fast-testpassed all six phases: 2,947 passed and 13 skipped, with four workers.Checklist
make styleand fixed any issuesmake fast-test)git commit -s) per the DCOSummary by CodeRabbit