Repository navigation
refactor(domain): give every aspect one reconciliation mode per scope - #423
Merged
Merged
Conversation
"Unmanaged" meant different things depending on where it was sensed: the differ skipped unmanaged properties entirely, while validation compared every other unmanaged aspect and rejected its drift. The property exception lived in the differ's docstring, and generate depended on it at a distance. TableScope now answers a per-aspect ReconciliationMode — MANAGE, REQUIRE_MATCH, or IGNORE — derived from the existing minimum-scope ladder plus one fallback table that states what each unmanaged aspect does instead. The differ's rule becomes generic (an IGNORE aspect is not compared), UnmanagedAspectDrift becomes the literal REQUIRE_MATCH interpreter, and manages() survives as "mode is MANAGE" for the sites that only need the boolean. No behaviour changes. The DeltaTable scope docstring now states the property exception it previously omitted: a restricted scope carries declared properties without deploying or checking them.
The differ's property gate read
scope.reconciles(TableAspect.PROPERTIES) is ReconciliationMode.IGNORE
which asks the reader to know the whole mode vocabulary just to parse
a guard. TableScope now names the projection, next to manages():
ignores(aspect) — the scope carries the aspect without comparing it.
The differ reads "if the scope ignores properties, nothing to
compare", and the docstrings and architecture doc say "ignore" in
plain English instead of quoting the enum member.
reconciles() stays the single source; both predicates derive from it.
No behaviour changes.
…dicates reconciles() returned a mode, so every call site had to know the enum vocabulary to read a guard — unlike manages() and is_within(), where the verb is the meaning. Validation, the last mode consumer, really asks "must this aspect match?", so TableScope now answers all three questions by name: manages(), requires_match(), and ignores(). With no caller left for the raw mode, ReconciliationMode and the 11-row _UNMANAGED_MODE_BY_ASPECT table collapse into the predicates: ignores() is "properties, when unmanaged"; requires_match() is every other unmanaged aspect. A new aspect still fails loudly — every predicate goes through _MINIMUM_SCOPE_BY_ASPECT, which raises KeyError for an unmapped aspect — and require-match is the safe meaning for an aspect added without a decision, since drift is refused, never converged. No behaviour changes.
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.
What
TableScopenow answers, per aspect, exactly one of three named questions — so every call site reads in plain English instead of re-deriving what "unmanaged" means:manages(aspect)— compare and converge the live table to the declaration.requires_match(aspect)— compare, but drift is refused rather than converged: the declaration mirrors the live state.ignores(aspect)— do not compare at all; declared values are carried but make no assertion.Managed aspects follow the existing
_MINIMUM_SCOPE_BY_ASPECTladder. Below it, every aspect must match except properties, which are ignored — the one aspect the platform co-owns, so a restricted declaration cannot plausibly mirror it. Adding an aspect without a ladder entry still fails loudly (KeyError), and require-match is the safe meaning for an aspect added without a decision, since drift is refused, never converged.The two sites that embedded a scope decision now ask the question by name:
scope.ignores(PROPERTIES), and its docstring drops the "scope-blind except for properties" exception clause for the generic rule: an ignored aspect is not compared.UnmanagedAspectDriftflags drift on every aspect the scoperequires_match— it is now the literal interpreter of that question. Itsrule_name(user-visible in failures) is unchanged.The five boolean sites (creation deferral, FK dependency edges, api column-rule gate,
renamed_fromrejection) keepmanages()— they key only on managed-or-not, and their consequences belong to their layers.Why
Finding 1 of the properties/features/scopes review, and the residue Ousterhout finding 4 named as incomplete: since #335
TableScopewas the one lookup for what a scope manages, but what "unmanaged" means was still decided independently at each site. The concrete costs: the differ carried a documented property exception thatgeneratedepended on at a distance, andDeltaTable's scope docstring claimed "every scope declares the full table shape" — untrue for properties, which are carried but ignored belowfull. A user declaringscope="metadata", properties={...}got value validation and then silence, documented only in the metadata how-to.An earlier iteration introduced a
ReconciliationModeenum returned byscope.reconciles(aspect); review feedback was thatreconciles(...) is ReconciliationMode.IGNOREforces the reader to know the whole mode vocabulary to parse one guard. The three predicates keep the trichotomy (exactly one is true per aspect) without any call site comparing enum members.Pure refactor — no behaviour changes anywhere. The
DeltaTabledocstring now states the property exception,generate's comment points at the ignore rule instead of the differ's behaviour at a distance, and the architecture doc's "Managed aspects" section describes the three questions.Gate: 1403 passed (new predicate tests in
test_table_scope.py), ruff check/format clean, mypy clean (178 files), all 7 import contracts kept.🤖 Generated with Claude Code