[DMS-1124] Slice 2: Root-Table-Only Profile Merge#927
Open
simpat-adam wants to merge 30 commits into
Open
Conversation
…olver context slim
…acceptance scenarios
The post-classifier gate rejected supported Slice 2 shapes (multi-table plans whose profiled runtime behavior stays on the root table) as UnknownFailure after the slice-fence classifier had already approved them. Slice 2 is defined by runtime shape, not by compile-time table count; the classifier is the single source of truth. Removed the gate in DefaultRelationalWriteExecutor and the duplicate single-table check in the RelationalWriteProfileMergeRequest constructor. Rewrote the executor unit test and PostgreSQL/SQL Server Fixture 9 from shape-gate-failure assertions to multi-table-root-only success assertions, and refreshed the stale comment in the profile routing test support. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…rence hidden Per profiles.md:782, a hidden member path naming a sub-member of a document reference (e.g. schoolReference.schoolId) must preserve the whole reference- derived storage family — the FK column and every propagated/derived identity binding — because they are governed by the same owning reference root. The prior matcher used per-binding exact-or-descendant matching against the binding''"'"''s own path, which under-preserved: hiding schoolReference.schoolId left the FK at schoolReference and sibling schoolReference.localEducationAgencyId writable. Renamed HiddenPathMatchKind.AncestorOrExact to ReferenceRooted. Reference- sourced bindings (DocumentReference, ReferenceDerived, ReferenceDerivedMember) are now matched against their owning reference root, read directly from DocumentReferenceBinding.ReferenceObjectPath and ReferenceDerivedValueSourceMetadata.ReferenceObjectPath rather than derived by longest-prefix. The classifier and post-pass drift check now share a single (memberPath, governingPath, matchKind) inventory so runtime matching and metadata validation stay on the same semantics. Tests cover the sibling-hidden case at every level: governance-rules unit tests, classifier unit tests (flipped FK + sibling to HiddenPreserved), resolver unit test for ReferenceDerivedMember, and PostgreSQL + SQL Server integration Fixture C.1 asserting FK and propagated identity both preserve when studentReference.studentUniqueId is hidden. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ifier entry Surfaced in post-merge review: - ProfileRootKeyUnificationContext.CurrentState carried but never read. - GovernedBindingEntry.MemberPath written but never consumed by the drift check (only GoverningPath + MatchKind are used). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds an ArgumentException guardrail to RelationalWriteProfileMergeRequest so any RootExtensionRows or CollectionCandidates arriving from the flattener surface as a deterministic contract-drift failure instead of being silently dropped by the root-only Slice 2 synthesizer. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…omplete scope metadata - Split stale Fixture 9 into 9a (POST) and 9b (PUT) conservative slice-fence fixtures; both assert no DML reaches the extension row (9b verifies the stored column value is preserved, not just row count). - Promote the two-non-collection-scopes stored-state projection invoker into Backend.Tests.Common so MSSQL Task 2 can reuse it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…r complete scope metadata - MSSQL parity of PG 9a (POST) and 9b (PUT) conservative slice-fence fixtures against the School resource. 9b reuses TwoNonCollectionScopesStoredStateProjectionInvoker from Backend.Tests.Common and verifies the stored SchoolExtension column value is preserved, not just the row count. - Update the stale "Fixture 9 (shape-gate)" top-header comment in both the PG and MSSQL files to reflect the new conservative-fence framing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…teProfileMergeInvariantException Only the flattener-produced-non-root-buffer precondition in RelationalWriteProfileMergeRequest maps to the new exception type; the other two constructor preconditions (root-table mismatch, request-instance / current-state-vs-context pairing) stay as ArgumentException so caller-wiring bugs remain visible rather than being silently reclassified by the upcoming executor catch in Task 5. New type is public sealed : Exception to match project precedent (RelationalWriteRequestValidationException) and satisfy SonarAnalyzer S3871. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…iled catalog Slice 2's contract has no completeness marker for collection scopes, so VisibleRequestCollectionItems / VisibleStoredCollection Rows can be silently empty when Core omits real collection data. Add ProfileSliceFenceClassifier.ClassifyFromCatalog which returns the deepest collection family when any ScopeKind.Collection descriptor appears in the compiled catalog, and have the executor take the max of the existing request/context classification and this new catalog-derived signal. No new contract markers, no Core changes - minimal merge-safe fix. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…istic slice-fence failure When RelationalWriteProfileMergeInvariantException escapes the Slice 2 profile merge synthesizer (flattener produced a non-root buffer despite the upstream fence classifying as root-table-only), the executor now rolls back and returns a deterministic SeparateTableNonCollection slice-fence UnknownFailure instead of letting the exception propagate as an unhandled 500. Pairs with Task 3's narrowed invariant exception type; other caller-wiring ArgumentExceptions still surface normally as programmer errors. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…sifier Three new fixtures covering ProfileSliceFenceClassifier.ClassifyFromCatalog's outcomes: RootTableOnly when the catalog has only non-collection scopes, TopLevelCollection when a top-level collection scope is present, and NestedAndExtensionCollections when a nested collection scope is present. Fixtures reuse the existing ProfileSliceFenceClassifierTestHelpers and follow the Given_/It_ pattern already in the file. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…fence and invariant exception mapping Two new tests cover behaviors added in Tasks 4 and 5: - It_fences_profiled_create_new_when_catalog_has_top_level_collection_scope: proves the catalog-derived fence escalates to TopLevelCollection when the compiled catalog contains a Collection scope even though the request-side classifier alone would return RootTableOnly. - It_maps_profile_merge_invariant_exception_to_slice_fence_failure: proves a RelationalWriteProfileMergeInvariantException thrown from the synthesizer is caught, rolled back, and mapped to an UnknownFailure whose message names SeparateTableNonCollection. Also extends the existing RecordingRelationalWriteProfileMergeSynthesizer stub with an ExceptionToThrow property, mirroring RecordingRelationalWritePersister. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…n coverage in contract validator tests Three new fixtures pin down the behavior of ValidateWriteContext's required-scope- coverage check against a catalog that mirrors the School shape (root + RootExtension non-collection + a Collection scope): (A) positive complete-metadata on both sides returns no failures, (B) stored-side missing $._ext.sample returns a single StoredScopeStates mismatch, (C) request-side missing $._ext.sample returns a single RequestScopeStates mismatch. The Collection scope $.addresses[*] is never required by the coverage rule - it is only there to match the realistic shape. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ed helper CompiledScopeAdapterFactory and ScopeTopologyIndex each carried an identical private NormalizeAdditionalScopes / CountScopeDepth pair. Promote them to a shared InlinedScopeNormalization helper in Backend.External.Profile so both callers (one in Backend.External, one in Backend) resolve to the same implementation. Behavior unchanged; 576/576 backend unit tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…uting fixtures ProfileWriteContractValidator.GetRequiredRootedNonCollectionScopes was treating CollectionExtensionScope-backed scopes (e.g. $._ext.sample.addresses[*]._ext.sample in the School catalog) as root-rooted required scopes. Their CollectionAncestorsInOrder came back empty because the owning base collection is stored under a sibling scope path ($.addresses[*], shared via BaseCollectionItemId) that the walk-up ancestor resolver cannot see. The [*] literal in the JSON-scope path is a structural marker of collection-membership, so exclude any scope containing [*] from the required set regardless of the ancestor walk result. Update PostgresqlProfileExecutorRoutingTests and MssqlProfileExecutorRoutingTests helpers to emit complete non-collection scope metadata on BOTH request and stored sides (using the promoted TwoNonCollectionScopesStoredStateProjectionInvoker and adding the RootExtension scope to the inlined-descendant helper), and update the family-name assertions on the three pre-existing "reaching slice fence" fixtures to expect NestedAndExtensionCollections - the family Task 4 conservative catalog fence returns for the School catalog (which has nested collections). Fixes five pre-existing CI failures on the PG Integration job and the MSSQL parity failures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bradbanister
approved these changes
Apr 22, 2026
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.
Summary
Implements Slice 2 of DMS-1124 per
reference/design/backend-redesign/epics/07-relational-write-path/03b-profile-aware-persist-executor/02-root-table-only-profile-merge.md. Removes the Slice 1 fence for profiled writes whose persisted behavior is confined to the root table, on resources whose compiled write plan is a single table. Everything outside that strict gate stays fenced for later slices.Follow-up cleanup items surfaced during this slice are captured in
reference/design/backend-redesign/epics/07-relational-write-path/dms-1124-slice2-followups.md(not merge blockers).What lands
DefaultRelationalWriteExecutor: after Slice 1's classifier returnsRootTableOnlyANDWritePlan.TablePlansInDependencyOrder.Length == 1, the executor falls through to a new profile merge synthesizer.RootTableOnly+ multi-table returns a distinct Slice-2 shape-gate failure. Everything else remains fenced via the existingBuildSliceFenceResult.RelationalWriteMergeResult/RelationalWriteMergedTableRow/RelationalWriteMergedTableState, with aSupportsGuardedNoOpflag. No-profile synthesizer setstrue; Slice 2 profile synthesizer setsfalse. Guarded-no-op gate in the executor now checks the flag before callingIsNoOpCandidate.FlattenerMemberEvaluation,ProfileKeyUnificationGuardrails,RelationalWriteMergeSupport) so the post-overlay resolver reuses the flattener's scalar/descriptor/reference-derived evaluators + the guardrail typing (InvalidOperationExceptionpreserved).ProfileMemberGovernanceRules,ProfileRootTableBindingClassifier): single source of truth for visibility lookup + hidden-path ancestor/exact matching. Classifier emitsRootBindingDispositionper binding (VisibleWritable/HiddenPreserved/ClearOnVisibleAbsent/StorageManaged) plusResolverOwnedBindingIndices. Fail-closed drift check on stored-scope / hidden-member-path metadata.ProfileRootKeyUnificationResolver): replays eachKeyUnificationWritePlanagainst merged logical member state (visible-request / hidden-stored / visible-absent), writes canonical + synthetic-presence bindings, applies guardrails, throwsRelationalWriteRequestValidationExceptionon disagreement (matches flattener message format).RelationalWriteProfileMergeSynthesizer): composes classifier + overlay + resolver. 4 constructor invariants enforced.SupportsGuardedNoOp: falsehardcoded.NamingStressItem, 3 scenarios) and a new synthetic DDL fixture (profile-root-only-merge, 6 scenarios) — all Slice 2 acceptance criteria proven end-to-end on both dialects.Classifier bug fix
During integration testing, the drift-check in
ProfileRootTableBindingClassifier.ValidateStoredScopeMetadatawas found to reject hidden paths targeting key-unification members — the drift inventory was built only from ordinary bindings, skipping resolver-owned k-u members. Fix: extended the inventory-building pass to register k-u member paths with their correctMatchKindForafter the main binding loop. AddedProfileMemberGovernanceRules.MatchKindFor(KeyUnificationMemberWritePlan)overload (replaces a duplicated local helper in the resolver).Test counts
All green. Full pgsql + mssql integration suites rerun after final rebase onto main.
Reviewer focus (per spec Section 8)
ProfileRootTableBindingClassifier.cs+ProfileRootTableBindingClassifierTests.cs)RelationalWriteProfileMerge.cs+ its tests)ProfileRootKeyUnificationResolver.cs+ tests + integration)DefaultRelationalWriteExecutor.csStep 4 + executor unit tests + integration fixture)Explicit ignore (deferred ownership)
Commits
Test plan
profile-root-only-mergefixture)MssqlGeneratedDdlBaselineDatabase.csconflict with#921 [DMS-1004]resolved (kept the Windows path-separator helperBuildSqlServerSiblingPath; main'ssnapshotFileNameextraction preserved)