From 435dea102f7e92b7ea4af890c11d76ae0d4dc224 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 24 Oct 2026 14:41:56 +0000 Subject: [PATCH] Retire the completed remediation record --- docs/development.md | 4 +- docs/roadmap/complexity.md | 7 +- docs/roadmap/remediation.md | 959 ------------------------------------ 3 files changed, 6 insertions(+), 964 deletions(-) delete mode 100644 docs/roadmap/remediation.md diff --git a/docs/development.md b/docs/development.md index 3a52a65..eb290e7 100644 --- a/docs/development.md +++ b/docs/development.md @@ -179,8 +179,8 @@ Current behavior belongs to the configuration contract's [strict composition](config.md#strict-document-composition), [sibling inheritance](config.md#sibling-inheritance), and [CLI overrides](config.md#explicit-cli-overrides) sections. -The full [remediation record](roadmap/remediation.md) remains available, including -the open algorithm-audit findings. +The completed remediation record, including the algorithm-audit findings, is +preserved at `ea46001:docs/roadmap/remediation.md`. Potential enhancements are tracked in [future features](roadmap/future.md). diff --git a/docs/roadmap/complexity.md b/docs/roadmap/complexity.md index 03843e1..b5f68d7 100644 --- a/docs/roadmap/complexity.md +++ b/docs/roadmap/complexity.md @@ -161,9 +161,10 @@ syntax, dependencies, or complexity-based gates. ## Relationship To Remediation And Other Work -[Remediation](remediation.md) owns the identified correctness and algorithmic -efficiency findings. Several overlap these files, particularly item-scope -application matching and inheritance policy localization. +The completed remediation record at `ea46001:docs/roadmap/remediation.md` +documents the identified correctness and algorithmic efficiency findings. +Several overlap these files, particularly item-scope application matching and +inheritance policy localization. Use the documented contract and completed remediation as the behavioral baseline. Keep corrective changes reviewable under their findings, and retain their diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md deleted file mode 100644 index f2751ea..0000000 --- a/docs/roadmap/remediation.md +++ /dev/null @@ -1,959 +0,0 @@ -# Sequence Implementation Review Remediation - -Status: findings 1–23 are complete, including the six additional algorithm-audit -findings numbered 18–23. This record introduces no new release gate. - -Reviewed commit: `0be7cdb00eff576fda6b30c0600b2134d4e2e086`. -Implementation range: `bc6b977..0be7cdb`. -Authority: the sequence roadmap and implementation plan preserved at -`d0a605a:docs/roadmap/sequences.md` and -`d0a605a:docs/roadmap/implementation.md`, -[ADR 0017](../adr/0017-collection-sequence-policy.md), and -[ADR 0018](../adr/0018-cli-after-structural-resolution.md). - -The public facade, shared identity/composition machinery, scoped policies, -identified-record inheritance, post-decode CLI phase, and final registered -identity validation are present. The findings below concern remaining -correctness, contract-alignment, documentation, and scaling gaps. - -Validation on the reviewed commit: - -- `./scripts/check.sh` passed with Go 1.26.10 on Linux. -- `go test -race ./...` passed. -- Public-API probes outside the repository reproduced findings 1–9. The probes - used labeled byte sources, defaults, and an in-memory filesystem; they did not - change implementation files or use external services. -- Finding 10 follows directly from the maintained text; finding 11 follows from - the prefix-scan loop, not a timing comparison or performance threshold. - -Validation after the initial remediation: `./scripts/check.sh` and -`go test -race ./...` passed with Go 1.26.10 on Linux. The original public-API -probes for findings 1–9 now demonstrate the required corrected behavior. - -Validation after integration remediation: the everyday check, full race suite, -and local documentation/public-boundary checks passed with the same toolchain. -The original and follow-up public-API probes demonstrate corrected behavior for -findings 1–9 and 12–15, including unrelated underlay payload at an old version -index. The accumulated-copy correction is verified by code inspection and -ownership regressions; the contract corrections are verified against the -implemented operations and replay. Each additional finding has its own completion -record and commit. - -Validation after algorithm remediation: `./scripts/check.sh` and -`go test -race ./...` pass with Go 1.26.10 on Linux. Focused regressions cover the -selector scope correction, sparse projection storage, broad CLI metadata, -inheritance policy localization/ownership, effective fallback locations, and -broad collection-policy lookups. Scaling conclusions follow from the revised -algorithms and structural coverage; no timing threshold or public performance -guarantee was added. - -P1 denotes the highest-priority correctness issue here; P2 denotes normal-priority -work needed to complete the selected behavior. Preserve the agreed scope while -fixing these issues. In particular, do not resolve failures by restricting valid -identity fields, requiring repeated declarations, changing merge semantics, or -adding limits. - -## 1. Preserve Retained Explicit Values In Nested CLI Mapping Assignments — P1 - -Status: [x] Complete. - -Completion: override metadata now reads the actual target subtree through one -shared lookup. Ordinary/layered regressions preserve retained values, including -zero and false, their explicit origins, and absence-based resolver behavior. -Verified `go test ./internal/engine -run 'Override|CLI' -count=1` and -`./scripts/check.sh`. - -Owner: [`applyDecodedOverrides`](../../internal/engine/override_decode.go), -especially lines 59–60 and the presence/origin update helpers. - -The helpers receive the complete configuration projection as `existing`, but -interpret it as the subtree at the override path. A partial nested mapping -therefore treats retained input children as absent and drops their presence and -origins. `fillDefaultOrigins` subsequently labels those retained values as defaults. - -Reproduction: load the following into ordinary nested Go structs: - -```yaml -settings: - nested: - a: 1 - b: 2 -``` - -Apply an override at `[settings]` with value -`map[string]any{"nested": map[string]any{"a": 3}}`. The typed value of `b` -initially remains 2, but `Metadata.Lookup([settings,nested,b])` reports -`Explicit: false` and a default origin. Calling `Default` for that field in -application resolution then changes the explicitly supplied 2 to the fallback -value. This violates presence-based defaulting, not merely presentation. - -Required remediation: use the actual existing target subtree when updating -presence/origins, preserving retained descendants and removing genuinely replaced -ones. Keep override batches detached and preserve the selected post-decode order. -Add regressions for nested partial mappings, retained zero/false values, metadata, -and an absence-based resolver default that must not overwrite retained input. - -## 2. Read Upsert Origins At Input Paths And Write Them At Output Paths — P2 - -Status: [x] Complete. - -Completion: sequence origin copying now separates incoming and effective paths -for precedence and inheritance. A regression covers reordered matches, appended -records, composite origins, and actual resource reads from distinct declaring -directories. Verified focused origin/inheritance/overlay/provenance tests, -`go test -race ./internal/engine`, and `./scripts/check.sh`. - -Owners: [`applyOverlaySequenceEvidence`](../../internal/engine/overlay.go), -particularly lines 369 and 381, and the analogous sequence-origin copying in -[`inheritance_collection.go`](../../internal/engine/inheritance_collection.go). - -The overlay helper supplies an output-index path to an origin lookup whose keys -are still incoming-index paths. When those positions differ and another incoming -record occupies the output index, metadata takes that other record's source -location. The same defect was reproduced through mapping-sibling inheritance; -its helper uses the same single-path copying pattern. - -Reproduction: an underlay supplies IDs `[a,b]`; the main source declares `key: id` -and supplies `[b*,a*]`. The result is correctly `[a*,b*]`, but the value origin -for final `a*` is the source line of `b*`, and vice versa. In the probe the two -value lines were 7 and 9; final index 0 incorrectly reported line 7 instead of 9. -With separately authored records this can select a different source/context. - -Required remediation: use the operation's input-to-output projection for origin -subtree reads as well as writes. Preserve composite contributing origins, not -just node-location fallbacks. Test reordered upserts and appended records with -distinct source files/locations, including nested fields, inheritance operations, -and declaring resource contexts. - -## 3. Apply Tier Sequence Operations To Historical Contributions — P2 - -Status: [x] Complete. - -Completion: per-source history now composes strict item projections with the -tier operation, including nested application prefixes and local sequence replay. -Regressions cover all three operations, multiple peers, retained records, removed -fields, source attribution, and a subsequent overlay without a false decoding -event. Verified focused history tests, `go test -race ./internal/engine`, and -`./scripts/check.sh`. - -Owner: [`applyOverlayWithCollectionPolicy`](../../internal/engine/overlay.go), -lines 76–84, and existing sequence-history projection/replay. - -For composed tiers, `applications` is nonempty. This branch appends the tier's -original strict-composition applications without incorporating the newly computed -precedence sequence descriptors. Replay consequently performs a whole-list -replacement or strict append rather than the actual cross-tier upsert/append. -The later decoding reconciliation conceals the mismatch by attributing a -structural combination to typed defaults/decoding. - -Reproduction: lower `[a,b]` and higher `[b*,a*]` correctly yield `[a*,b*]`. -`Explain([items])` instead shows the main contribution producing `[b*,a*]`, then -a `decoding` event reordering it to `[a*,b*]` with the reason "applied retained -defaults and canonical typed decoding". No decoder performed that reordering. - -Required remediation: project each retained per-source application through the -tier's selected operation while keeping peer composition distinct from tier -precedence. Do not collapse authored sources or fake decoding events. Cover -upsert, append, unmatched retained records, reordered matches, removed fields, -multiple higher-tier peers, and a subsequent selected overlay. Replay should -agree with structural assembly before real decoding reconciliation. - -## 4. Reproject Nested Source Destinations After Cross-Tier Item Movement — P2 - -Status: [x] Complete. - -Completion: layered assembly projects incoming source destinations and version -classification evidence with the same item assignments used for values/history. -Earlier contributions retain their historical coordinates, physical identities -and contexts. Main/underlay regressions cover reordered upserts, append, replace, -nested helpers, and source-local version evidence. Verified focused destination, -origin and source-version tests, focused race coverage, and `./scripts/check.sh`. - -Owners: [`projectAcquiredDestinations`](../../internal/engine/imports.go) and the -sequence-operation handoff in [`assembleLayeredInput`](../../internal/engine/load_request.go). - -Source destinations are projected while composing their own tier, but are not -reprojected when that tier's records move to accumulated-list positions. Nested -helper source records can therefore point at a different final record. - -Reproduction: lower `items` contains `[a,b]`. Main imports a single `b` record -with `placement: sequence-item` into `[items]`; that record imports a helper -mapping supplying its value. Upsert retains `a` at index 0 and places the higher -`b` at index 1. `Sources()` nevertheless reports the surviving helper's destination -as `/items/0`, its tier-local position, rather than `/items/1`. - -Required remediation: carry the sequence operation's projection through nested -source-destination/evidence updates at precedence boundaries. Keep physical -source identity and passive resource contexts unchanged, and preserve historical -meaning for displaced contributions rather than relabeling them as survivors. -Test a retained lower record followed by an imported higher record, reordered -matches, and nested helpers. Check version-evidence projection where applicable. - -## 5. Honor Identity Reset On Actual Inheritance Replacement — P2 - -Status: [x] Complete. - -Completion: inheritance resolves identity against actual child-value presence, -discarding parent YAML identity for an explicit replacement while preserving -agreement for retained values and authoritative application constraints. Added -policy-state and public-load regressions for changed/unkeyed/non-sequence -replacements, application agreement/conflict, and mode-only/append/upsert cases. -Verified focused inheritance tests and `./scripts/check.sh`. - -Owners: [`inheritanceCollectionResolution` and `mergeInheritanceCollectionPolicies`](../../internal/engine/inheritance_collection.go), -especially lines 79–80 and 121. - -These paths reconcile the parent's YAML identity before accounting for actual -replacement, and treat the parent's collection as retained even when the child -supplies a replacing value. This is stronger than the selected identity lifetime -and differs from source-tier replacement. - -Reproduction: - -```yaml -configkit: - sequences: - - destination: [base, items] - key: id - - destination: [child, items] - key: name - mode: replace -base: - items: - - id: a - value: parent -child: - extends: base - items: - - name: b - value: child -``` - -Loading fails with `sequence.conflict` at `/child/items`. It should succeed: -actual whole-sequence replacement may discard the parent's YAML identity and -establish `name`. Authoritative application constraints must still apply. - -Required remediation: distinguish actual replacement from a mode-only declaration -before identity agreement and transfer. Test a changed key, replacement with no -new key becoming unkeyed, permitted non-sequence replacement, and an agreeing or -conflicting application registration. Preserve rejection of identity changes -without a replacement value and during append/upsert. - -## 6. Enforce Collection Operations Beneath Newly Added Ancestors — P2 - -Status: [x] Complete. - -Completion: copying a mapping now visits nested collection operations with absent -lower values, preserving ordinary ancestor removal and whole-record semantics. -Incoming record-owned policy transfers through the same operation. Regressions -cover absent/empty/null ancestors, identity and kind failures, empty and valid -initialization, and later inheritance using an imported item's nested identity. -Corrected an older test that deferred the required error until a later tier. -Verified focused merge/collection/inheritance tests, focused ownership race -coverage, and `./scripts/check.sh`. - -Owners: [`mergeRecursiveMappings`](../../internal/engine/recursive_merge.go) and -[`mergeOverlayMappingsWithCollectionPolicy`](../../internal/engine/overlay.go). - -When an incoming ancestor mapping is absent below, the recursive merger clones -the entire subtree and invokes only its ancestor-level added hook. Nested -collection resolutions are not visited. This bypasses their operation rules; -the subsequent declaration-only policy update does not replace those checks. - -Reproduction: combine a lower `{}` with this main tier: - -```yaml -configkit: - sequences: - - destination: [group, items] - mode: append -group: - items: - - id: a -``` - -Loading succeeds despite append having no identity definition. Placing the same -sequence directly at `[items]` correctly fails. The validity of an operation -must not depend on whether an enclosing mapping already existed. - -Required remediation: account for applicable nested collection policies when -adding/copying an ancestor subtree, including owned-policy transfer. Test absent -and present enclosing mappings, append/upsert without identity, explicit empty -lists, invalid non-sequence contributions, and valid identified initialization. -Do not turn this into a general path-existence or shape constraint: ordinary -ancestor replacement/removal retains its documented meaning. - -## 7. Validate Final Typed Identity Fields As Payload, Not Directives — P2 - -Status: [x] Complete. - -Completion: shared identity lookup now distinguishes structural records from -effective payload/reference mappings. Final validation includes literal `extends` -fields while structural source checks still exclude actual directives. Facade -regressions cover defaults and recorded resolution, string/null components, and -duplicate rejection. Verified those regressions and existing structural identity -and inheritance coverage through `./scripts/check.sh`. - -Owners: [`validateFinalSequenceIdentities`](../../internal/engine/final_identity.go) -and [`newSequenceMappingIndex`](../../internal/engine/sequence_identity.go). - -The final validator reuses structural record lookup, which excludes every -`extends` entry. At this phase there is no structural processing: typed defaults -and application resolution can produce ordinary payload with that literal name. -The roadmap expressly distinguishes structural-control exclusion from final -typed-payload validation; this reuse introduces an extra restriction. - -Reproduction: a defaults-only `Catalog` with `Items []map[string]any` containing -`{"extends": "application-data"}`, registered with -`SequenceIdentityAt(Keys("items"), "extends")`, fails with a missing identity -field at `/items/0/extends`, even though the effective field is present. - -Required remediation: distinguish structural-payload lookup from final payload -lookup. Preserve rejection when a source tries to use an actual consumed -directive as identity. Test final fields supplied by defaults and recorded -resolution, including a null component, without adding a broader name ban. - -## 8. Retain Nonfatal Main-Version Diagnostics In Layered Loads — P2 - -Status: [x] Complete. - -Completion: layered assembly returns nonfatal main-version diagnostics on success -and alongside later failures; the shared caller stops only for errors. Regression -coverage verifies ordinary/layered warning parity, exactly one checker call, and -warnings retained with later structural or decoding errors. Verified through -`./scripts/check.sh`, including existing source-version ordering coverage. - -Owner: [`assembleLayeredInput`](../../internal/engine/load_request.go), lines 143–152. - -The moved default-mode version-check call returns its diagnostics only when they -contain errors. Warning-only diagnostics are discarded. Ordinary loading still -retains the same warnings. - -Reproduction: a `VersionRule.Check` that calls -`validation.Warning(Keys("version"), "test warning")` and returns nil yields one -`version.warning` through ordinary `Load`, but zero diagnostics through -`LoadLayered` with the same main input and default source-version mode. - -Required remediation: carry warnings from the single main-tier check into the -shared diagnostic accumulator on successful layered loads. Preserve checker -ownership, invocation count, and the selected acquisition/composition ordering. -Add ordinary/layered parity and warning-plus-later-error regressions. - -## 9. Identify Conflicting Application Registrations In Diagnostics — P2 - -Status: [x] Complete. - -Completion: definition validation retains original one-based registration -positions and names both positions in each overlap diagnostic. No input path or -new public diagnostic structure is invented. A mixed-selector regression verifies -multiple conflicts even after an earlier invalid registration is skipped. -Verified registration/facade coverage through `./scripts/check.sh`. - -Owner: [`compileSequenceIdentityPolicy`](../../internal/engine/sequence_registration.go), -lines 119–128. - -Incompatible dynamic overlaps produce only a generic `options.invalid` message, -an empty configuration path, and no related information. Neither registration -is identified. The roadmap requires definition-time errors to identify the -conflicting registrations without inventing a concrete input destination. - -Reproduction: register `collections -> AnyMappingKey -> items` using `id`, and -`collections -> foo -> items` using `name`. Even with `NoSource`, the error is -only "overlapping sequence registrations must use the same identity fields". -With multiple registrations the caller cannot identify the conflicting pair. - -Required remediation: retain original registration positions and identify both -in safe definition diagnostics, for example through positional message context. -No new public diagnostic structure is required solely for this. Test mixed exact -and dynamic registrations and multiple conflicts, without asserting complete -English wording or exposing identity values. - -## 10. Remove Stale Public Contract Descriptions — P2 - -Status: [x] Complete. - -Completion: the public definition comment now describes final registered-identity -validation and its shape/lifetime boundaries. Selected-overlay prose explicitly -separates ordinary replacement from collection policy and links to the canonical -operation/presence tables. Verified against the implementation, local-link tests, -and `git diff --check`; no runtime behavior changed. - -Owners: [`Definition.SequenceIdentities`](../../lifecycle.go), lines 18–21, and -the [selected-overlay contract](../config.md#selected-overlays). - -The exported definition-field comment still says registrations do not validate -identities introduced by defaults, CLI, or callbacks. Final validation is now -implemented and the registration constructors correctly document it. - -The selected-overlay table and following paragraph also still unconditionally -describe sequences as atomic replacements and `[]` as clearing. A later paragraph -describes collection operations, but does not make the earlier unconditional -claims accurate: identified collections infer upsert, and `[]` then contributes -no records rather than clearing. - -Required remediation: make the symbol comment describe its implemented final -guarantee and boundaries, and qualify the overlay table/paragraph for collection -policy. Link to the canonical collection tables rather than duplicating them. -Keep YAML-only enforcement distinct from registered final validation. - -## 11. Remove The Quadratic Prefix Scan In Item Policy Projection — P2 - -Status: [x] Complete. - -Completion: assigned sequence ranges now supply item offsets directly. Handoffs -without a range lazily accumulate prefix lengths, visiting each preceding -contribution once rather than rescanning every prefix. Regression coverage keeps -inline and policy-less contributions in the correct positions with complete, -partial, or absent ranges. Existing ownership/placement tests remain intact. -Verified focused item-policy tests, `./scripts/check.sh`, `go test -race ./...`, -and inspection of the remaining error-only prefix-scan callers. No new benchmark -fixture or performance threshold was needed for this bounded change. - -Owner: [`projectNestedCollectionPolicies`](../../internal/engine/imports.go), -especially line 968; the scanned helper is `priorSequenceLength` in the same file. - -For every item with local collection policy, the loop scans all preceding -contributions to calculate `itemIndex`, then overwrites that result when the -already-indexed sequence range is available. For N such items this performs -`0 + 1 + ... + (N-1)` contribution visits, excluding path lookup costs, despite -the common case already having the exact assigned index. This is introduced -quadratic work in the feature's independently authored-record path. - -Required remediation: use the existing assigned range first. If a genuine -fallback is necessary, maintain per-destination prefix lengths in one pass -rather than rescanning every prefix. Preserve scope and ordering semantics; -do not add limits, pooling, or a new public cache. Keep ownership/placement -regressions and add a representative many-item/local-policy benchmark only if -needed to exercise the path. A wall-clock threshold is not required. - -## Integration Audit Baseline And Scope - -Reviewed commit: `8d40983271de5239312eb0a2896737be88c5abdc`. - -This follow-up reviews the interaction of collection policy, identified-record -inheritance, dynamic registrations, and post-decode CLI assignment with existing -source placement, source-version checking, typed decoding, inspection, resource -contexts, and ownership boundaries. The accepted ADRs and current configuration -contract remain the authority; the findings require integration corrections, -not additional consumer restrictions or new feature choices. - -Validation: - -- `./scripts/check.sh` and `go test -race ./...` pass with Go 1.26.10 on Linux. -- Isolated public-API probes outside the repository reproduce findings 12–15. - They use labeled byte sources and an in-memory filesystem, with no external - services, real credentials, or repository implementation changes. -- Finding 16 follows from the per-item deep-copy loop; no timing claim or - performance threshold is needed. Finding 17 follows from maintained contract - text that contradicts the implemented collection operations. -- Positive comparisons include sequence-valued identity replacement, ancestor - retirement through source tiers, quoted versions after item movement, and an - append overlay that preserves the main record's version. - -## 12. Complete Identity Retirement Across Replacement Shapes — P2 - -Status: [x] Complete. - -Completion: source composition and inheritance now prepare eligible lower policy -from actual replacement paths before agreement. Removed identity and record state -cannot constrain a new pending key or reappear in later inheritance. Shared mode -selection preserves explicit-mode persistence and authoritative registrations. -Regressions cover null/scalar/mapping/list replacement, ancestor removal with a -new key, reintroduction, and retained/application-constrained conflicts. Verified -focused retirement/inheritance tests, engine ownership race coverage, and -`./scripts/check.sh`. - -Owners: [`resolveStrictCollectionPolicies`](../../internal/engine/compose.go), -especially its `sequencePresent` reset condition; -[`mergeInheritanceCollectionPolicies`](../../internal/engine/inheritance_collection.go); -and ancestor-replacement handling in -[`applyInheritanceScopeTemplates`](../../internal/engine/inheritance_schedule.go). - -Policy agreement and retirement still treat some actual replacements as retained -collections. This has three observable effects under the same selected lifetime -rule: - -1. Source precedence accepts a replacement sequence with a new YAML key, but - rejects the equivalent replacement by null or a scalar. For example, a lower - tier declaring `key: id` for `[items]` with `items: [{id: old}]`, followed by - a main tier declaring `key: name, mode: replace` with `items: null`, fails with - `sequence.conflict` at `/items`. Replacing null with `42` also fails; replacing - it with `[{name: new}]` succeeds. Null/scalar replacement should discard the - lower identity and retain the newly declared key as pending policy, subject - to the application's schema and constraints. -2. Removing an ancestor while declaring a new pending key for its removed - descendant fails before the replacement occurs. A lower - `group: {items: [{id: old}]}` with key `id`, followed by `group: null` and a - root declaration of key `name` for `[group, items]`, fails at `/group/items`. - The analogous `base`/`child` inheritance case fails at `/child/group/items`. - The old collection is being removed rather than retained. -3. Inheritance can resurrect removed identity even without a conflicting new - declaration. This input fails with a missing `id` at - `/grandchild/group/items/0/id`: - - ```yaml - configkit: - sequences: [{destination: [base, group, items], key: id}] - base: {group: {items: [{id: old}]}} - child: - extends: base - group: null - grandchild: - extends: child - group: {items: [{name: new}]} - ``` - - The same remove/reintroduce operations through source tiers succeed. The - inherited descendant identity remains in policy state after its enclosing - mapping is removed, so the grandchild incorrectly receives the old key. - -Required remediation: account for actual value and ancestor replacement before -deciding which lower identity participates in agreement, and retire removed -descendant/record-owned state during inheritance as during source precedence. -Retained inheritance templates must not reestablish discarded identity later. -Preserve authoritative application constraints, independently owned mode -persistence, pending declarations, and rejection of key changes when values are -genuinely retained. Cover direct non-sequence replacement, ancestor removal with -a new pending key, and removal followed by reintroduction across both contexts. -Do not require repeated keys or narrow permitted null/scalar shapes to avoid -these cases. - -## 13. Keep Metadata History In Post-Decode CLI Order — P2 - -Status: [x] Complete. - -Completion: metadata history is assembled from the pre-CLI source view, with CLI -events appended separately; inheritance is projected before CLI and resolution. -Effective fields still describe post-CLI values. Regressions verify chronological -origins and resolver dependencies for ordinary/layered mapping and record -inheritance, including equal-valued assignments. Verified history/override -coverage and `./scripts/check.sh`. - -Owners: [`historyWithInheritance`](../../internal/engine/inheritance_history.go) -and the source/CLI metadata handoff in -[`preparePostDecodeOverrideContext`](../../internal/engine/override_context.go). - -The value explanation follows the new lifecycle, but `Metadata.History` still -inserts inheritance after stored CLI changes. `historyWithInheritance` splits -only at `LayerResolution`, reflecting the former CLI-before-inheritance order. -The reconstructed source view can also append an authored source origin after -the CLI event. - -Reproduction: load `base: {value: parent}` and -`child: {extends: base, value: authored}`, then override `[child, value]` with -`"cli"`. The final value and `Metadata.Lookup` correctly report CLI provenance. -`Explain` reports source → inheritance → CLI. `Metadata.History` instead reports -source → CLI → source → inheritance and ends with a source/inheritance origin. -This violates chronological metadata history and disagrees with the effective -origin and the value explanation. - -Required remediation: assemble metadata history in the implemented source, -inheritance, CLI, and resolution order, preserving the real origins and -dependencies of those events. Cover ordinary/layered loading, mapping and record -inheritance, equal-valued CLI writes, and a later recorded resolver change at the -appropriate existing test owners. Do not impose a new exact event-count or -message-format contract. - -## 14. Carry Main-Version Coordinates Through Sequence Movement — P2 - -Status: [x] Complete. - -Completion: the effective version rule now follows the main declaration through -strict placement, tier combination, and inheritance without changing source-local -checker/evidence paths. Underlay and overlay ownership checks use assigned record -positions, including whole-record removal; integer conversion remains confined -to the actual main header. Regressions cover mounted/indexed paths, appended and -matched records, integer/string parity, unrelated version payload, imported -underlay claims, and inheritance. Corrected an older ownership fixture to target -the actual matching main record. Verified source-version regressions, -`./scripts/check.sh`, and `go test -race ./internal/engine`. - -Owners: [`effectiveSourceVersionRuleForPlacement`](../../internal/engine/source_version.go), -the main-tier projection handoff in -[`assembleLayeredInput`](../../internal/engine/load_request.go), and use of -the effective rule in [`executeLoadRequest`](../../internal/engine/load.go). - -Source destinations and version-evidence sensitivity paths follow item movement, -but the rule used for integer-version decoding retains the original item index. -Consequently an accepted integer header can fail decoding after an otherwise -valid upsert placement. - -Reproduction with the public API: - -- Main input is `id: main` and `version: 4`, placed as one sequence item at - `Keys("items")`. -- A lower input contributes the sequence item `id: lower` at the same destination. -- Register identity `id` for `[items]`, select `SourceVersionModeIfPresent`, and - use `VersionRule.Path = Keys("version")` with an accepting checker. -- Decode into records with `ID string` and `Version string`. - -The checker accepts `4` exactly once. Upsert places the main record at index 1, -but decoding fails with `decode.type` at `/items/1/version`. The integer-to-string -exception is still tied to `/items/0/version`. Quoting the identical version as -`'4'` succeeds. A source-local indexed header path, without item placement, -reproduces the same problem when its record moves during upsert. - -Required remediation: carry the main declaration's effective coordinates through -the existing item projections into decoding and audit the associated ownership -protection handoffs for the same stale-coordinate assumption. Preserve -source-local checker/evidence paths, checker invocation count, main-only -ownership, and the narrow integer-to-string exception. Cover moved existing and -newly appended main records, integer/string parity, and protection of the actual -main header after movement. Do not prohibit supported indexed paths or mounted -main records as a workaround. - -## 15. Exclude Parent Identity From Inherited History Payload — P2 - -Status: [x] Complete. - -Completion: retained record-inheritance operations now exclude parent identity -from their payload and origin projection. Replay preserves the child's own -identity before taking field, record, sequence, or root snapshots, without -inventing inheritance events for retained identity fields. Regressions cover -single and compound identities, explicit child provenance, ordinary inherited -payload, and the parent's original history. Verified inheritance/inspection -coverage, `./scripts/check.sh`, and `go test -race ./internal/engine`. - -Owners: inheritance operation construction in -[`applyInheritanceScopeTemplates`](../../internal/engine/inheritance_schedule.go), -[`appendInheritanceExplanation`](../../internal/engine/inheritance_explain.go), -and transferred-origin projection in -[`inheritance_history.go`](../../internal/engine/inheritance_history.go). - -Identified-record value construction excludes the parent's identity fields, but -the retained history operation still supplies the complete parent record and -its identity origins to the older mapping-inheritance replay machinery. - -Reproduction: - -```yaml -configkit: - sequences: [{destination: [items], key: id}] -items: - - id: base - value: parent - - id: child - extends: {id: base} -``` - -The effective child ID and origin are correct. However, -`Explain(Keys("items").Index(1).Key("id"))` reports `child` → `base` → `child`, -attributing the intermediate value to an inheritance operation and the parent's -ID location. `Metadata.History` likewise claims an inherited parent-ID -contribution. That identity is expressly excluded from inherited payload; it was -never an applied value at the child path. - -Required remediation: make history replay and transferred-origin projection use -the same identity exclusion as value construction. Preserve the child's authored -identity and presence, ordinary inherited payload history, and the parent record's -own source history at its original path. Cover compound identities and identity -fields alongside inherited payload at both record and ancestor queries; keep the -existing inspection API and sensitivity behavior. - -## 16. Avoid Deep-Copying Accumulated Item Policies Per Record — P2 - -Status: [x] Complete. - -Completion: item projection now takes one owned policy copy per destination, -updates that local state incrementally, and stores it once after projection. -Incoming scopes still cross detached ownership boundaries. Inspection confirms -that accumulated record scopes are no longer deep-copied per item; no timing -threshold or benchmark campaign was needed. Existing stable-index, sparse-range, -fallback-offset, and ownership coverage passes, with a new regression for -interleaved destinations, distinct record owners, and sibling/cross-load mutation -independence. Verified focused item-policy tests, `./scripts/check.sh`, and -`go test -race ./...`. - -Owners: [`projectNestedCollectionPolicies`](../../internal/engine/imports.go) -and its use of copying `collectionPolicyScope.at` / `set` helpers in -[`collection_policy_state.go`](../../internal/engine/collection_policy_state.go). - -The prefix-scan correction in finding 11 is present, but a separate quadratic -cost remains on the same independently authored-record path. For every imported -item with local policy, `at` deep-clones the destination's accumulated record -order and every retained record's nested scope. After adding one owner, `set` -deep-clones the growing policy again. With N fixed-size item-local policies at -one destination, this copies approximately `1 + 2 + ... + N` retained scopes, -even when every assigned sequence range is available. - -This follows directly from the loop and `cloneCollectionPolicy`; no busy-host -timing comparison is required. It is an integration cost from reusing detached -read/write helpers within private incremental construction, rather than from -the required final ownership copies. - -Required remediation: construct each destination's owned record-policy state -without repeatedly cloning all earlier records, retaining necessary copies at -ownership boundaries. Preserve record ordering, distinct owners, sparse assigned -positions, fallback offsets, and cross-load detachment. Reuse the existing item -policy and ownership tests; add allocation/scaling evidence only where useful. -Do not introduce public caches, pooling, new caps, or performance thresholds. - -## 17. Reconcile Legacy Sequence-History Contract Text — P2 - -Status: [x] Complete. - -Completion: metadata and explanation prose now describe history through the -selected collection operation, assigned item positions, and displaced payload. -The text distinguishes sequence snapshots and atomic diffs from append/upsert -combination and links to the canonical operation tables. Whole-record upsert, -existing queries, and sensitivity guarantees are unchanged. Verified against -replay/combination code, local documentation-link tests, and `git diff --check`; -the unchanged runtime is covered by the preceding everyday and full race checks. - -Owner: the metadata/provenance and explanation sections of -[`docs/config.md`](../config.md#layered-metadata). - -The metadata section still says that a later tier supplying a sequence records -replacement at the sequence path and that history does not imply identity-based -merging across tiers. That describes the former unconditional replacement model, -not the implemented append/upsert history or the collection operation tables in -the same document. The explanation paragraph about atomic sequences also needs -review so readers can distinguish snapshot/diff granularity from collection -combination semantics. - -Required remediation: describe history for the selected collection operation and -link to the canonical operation tables. Preserve the existing atomic sequence -snapshot/diff behavior and whole-record upsert semantics; correcting the prose -does not authorize recursive record-field merging, a new query API, or broader -history guarantees. - -## Algorithm Audit Baseline And Scope - -Reviewed commit: `c73daf891d0f29c96f7026bdabcccb571f44a751`. -Feature baseline: `4f941f8ba4c9a9ed0afa87b07fe6ba29f131d1c6`. - -This audit examines the sequence identity/operation, scoped-policy, inheritance, -inspection, and post-decode CLI algorithms, including the completed corrections -above. The intervening discovery roadmap commits add no implementation to audit. -The accepted scheduler, deterministic ordering, exact numeric normalization, -selector trie, and deliberately pairwise registration-overlap validation remain -selected design choices; they are not findings here. - -Evidence and validation: - -- Code inspection establishes the complexity of findings 19–23 on bounded-depth - inputs with linear-sized results. Required result/history copies and genuinely - growing inheritance output are excluded from the alleged excess work. Bounds - describe internal operations, not new public performance guarantees. -- An isolated public-API probe reproduces finding 18 using an in-memory - filesystem, without external services or real credentials. -- Temporary tests supplied through a Go overlay, outside the repository, checked - all 768 combinations of append/upsert/replace over the 16 unique-ID lists drawn - from three IDs. Stable ordering, duplicate rejection, and whole-record payload - replacement agreed with an independent reference calculation. -- A structural overlay test confirmed that 4, 8, and 16 one-item contributions - retain 10, 36, and 136 reverse-index cells respectively in `indexSequenceRanges`. - These are representation counts, not timing or real-world benchmark results. -- `go test ./internal/engine -run 'Sequence|Collection|Inheritance|Override|SourceVersion|Metadata|History' -count=1` - passed with Go 1.26.10 on Linux. Passing existing tests does not cover the - incorrect selector scope or establish efficient growth. - -The following are focused corrections within the selected model. This review -changes only the remediation record. - -## 18. Match Application Registrations In Effective Item Coordinates — P2 - -Status: [x] Complete. - -Completion: pending item/helper scopes now compose their local YAML policy without -reinterpreting global mapping-only application selectors below a sequence index. -The enclosing composer still enforces the actual outer collection constraint. -Updated the old unit test to exercise that boundary and added ordinary/layered -facade regressions for exact/dynamic unused selectors, nested helpers, and missing -outer identities. Verified the facade regressions and `./scripts/check.sh`. - -Owner: `composePendingItems` in -[`imports.go`](../../internal/engine/imports.go), especially its local -`composeDocumentsWithCollectionPolicy` call; the current -`TestPendingItemCompositionAppliesApplicationIdentityConstraints` in -[`item_policy_test.go`](../../internal/engine/item_policy_test.go) also needs review. - -Pending item/helper documents are composed in item-local coordinates while the -unchanged global application selector policy is supplied to their composer. -A registration for root `[prompts]` therefore also constrains an unrelated -`[items, 0, prompts]` collection before placement. This contradicts complete, -effective-destination matching and makes an otherwise unused registration reject -valid configuration. Application selectors do not traverse sequence indexes. - -Reproduction: main imports `item.yml` with destination `[items]` and placement -`sequence-item`. The item contains `id: outer` and -`prompts: [{text: payload}]`. Loading succeeds without a registration; adding -only `SequenceIdentityAt(Keys("prompts"), "id")` fails with `compose.invalid` -at `/items/0/prompts/0/id`, although no root `prompts` collection exists. - -Required remediation: separate item-local YAML policy from application matching -in effective coordinates, including nested helper scopes. Preserve applicable -outer collection constraints without reinterpreting global selectors as local -ones. Add ordinary/layered regressions for unused root and dynamic selectors -alongside imported item-local collections. Correct tests that pin the accidental -scope; do not change selector meaning or require consumers to remove valid rules. - -## 19. Avoid Dense Global-Position Storage For Sparse Item Projections — P2 - -Status: [x] Complete. - -Completion: contiguous composition ranges and their origin contributions retain -arithmetic offset lookup. Scattered projections use independently cloned sparse -maps in both directions; unprepared range queries no longer allocate indexes. -Audited the shared projection/history callers for batch preparation. Added -structural coverage for many one-item ranges, widely separated assignments, and -absent coordinates. Existing upsert, history, version, and ownership tests pass, -along with `./scripts/check.sh` and `go test -race ./internal/engine`. - -Owners: `newSequenceItemProjectionIndex` in -[`sequence_projection.go`](../../internal/engine/sequence_projection.go), -`indexSequenceRange` / `projectedCompositionOrigins` in -[`compose.go`](../../internal/engine/compose.go), and contribution indexing in -[`history.go`](../../internal/engine/history.go). - -The reverse projection slice is sized to `maxOutput + 1`, even for one incoming -item assigned to a late assembled position. `indexSequenceRange` materializes -these indexes for ordinary contiguous append ranges too. With N one-item sources -contributing to one sequence, reverse slices retain `1 + 2 + ... + N` cells. -Initialization time and simultaneously retained auxiliary storage are therefore -quadratic despite only N actual mappings. Origin/history projection constructs -the same representation through additional callers. Finding 16's policy-copy -correction does not remove this independent cost. - -Required remediation: retain arithmetic start/end lookup for contiguous ranges -and use storage proportional to actual mappings for scattered positions, such as -a sparse reverse map. Audit shared callers and avoid rebuilding equivalent indexes -inside item queries. Preserve bidirectional lookup, absent positions, upsert -replacement evidence, ordering, and detached results. Verify the representation -on many one-item sources and noncontiguous upserts; add no timing threshold or -collection-size limit. - -## 20. Remove Full Metadata Scans Per CLI Replacement Leaf — P2 - -Status: [x] Complete. - -Completion: presence and origin removal now visits only the known replaced -pre-CLI subtree. Partial mappings still retain omitted fields and their metadata. -Added broad-mapping coverage with escaped keys, explicit zero/false values, -empty-list replacement, null pointers, retained siblings, and deleted descendant -metadata. Existing nested CLI and typed-ownership regressions and -`./scripts/check.sh` pass. - -Owners: `updateOverridePresence`, `deletePresencePathSubtree`, and -`updateOverrideOrigins` in -[`override_decode.go`](../../internal/engine/override_decode.go). - -Every replaced scalar or subtree scans all presence keys, then all origin keys, -to find matching prefixes. A single partial-mapping override updating N scalar -children in an N-field configuration performs quadratic map-entry visits. This -does not require N CLI sources or growing historical output: the same effective -maps remain approximately N entries throughout the update. - -Required remediation: delete affected entries by walking the already known -existing subtree, or use one batch-local prefix index where needed. Retain -necessary copies at ownership boundaries without rescanning unrelated paths for -each leaf. Preserve retained explicit zero/false values, removal of replaced -descendants, null/kind changes, origin attribution, and detached loads. Use the -existing nested CLI and ownership regressions plus focused broad-mapping coverage; -do not change override semantics or impose batch limits. - -## 21. Localize Inheritance Policy Work To The Relevant Scope — P2 - -Status: [x] Complete. - -Completion: inheritance now keeps a per-load semantic path index, including -separate indexes for record-owned scopes. Walker handoff and descendant -retirement touch the changed scope; mapping and identified-record preparation -reuse indexes across sibling queries. Only relevant policies are cloned into -independent templates. Added wide independent YAML groups, pending-policy -replacement, literal-key/index distinctions, and ownership regressions. Existing -nested item/inheritance, mode-lifetime, and application-constraint coverage, -`./scripts/check.sh`, and `go test -race ./internal/engine` pass. - -Owners: `inheritancePolicyScopeAtPath` in -[`inheritance.go`](../../internal/engine/inheritance.go), graph preparation in -[`inheritance_schedule.go`](../../internal/engine/inheritance_schedule.go), -and policy handoff/retirement in -[`inheritance_walk.go`](../../internal/engine/inheritance_walk.go). - -Scope localization scans every entry in the supplied policy table for each -definition. Mapping graph preparation also clones the full supplied table, and -the walker passes its global table to each independent scope and scans that table -again during replacement. For S scopes or siblings and P unrelated collection -policies, these paths perform repeated S-by-P work and copies. - -For example, N independent mapping groups can each contain a constant-sized -`base`/`child extends base` pair and one YAML collection declaration beneath its -base. Input, expanded values, and relationship evidence remain linear in N, but -each group's graph/localization visits or clones the other groups' policies. -This is separate from necessary expansion along a genuinely growing parent chain -and from the selected dependency scheduler. - -Required remediation: partition or index policies by semantic scope once and -carry the relevant state through traversal, including efficient descendant -retirement as inheritance introduces or removes destinations. Clone owned -in-scope state rather than unrelated global state. Preserve record owners, -pending declarations, mode lifetime, destination-local registration matching, -and independent templates. Verify wide independent groups and nested item scopes; -do not replace the agreed scheduling algorithm or add global caches. - -## 22. Index Fallback Location Lookup During Effective Projection — P2 - -Status: [x] Complete. - -Completion: effective location projection lazily builds one read-only fallback -document lookup and shares it across field and entry visits. The ordinary -nearest-location helper retains its existing behavior. Added broad mixed -authored/default mappings, ancestor fallback, literal key/index mismatch, -synthetic CLI coordinates, and detached-projection coverage. Existing secret -and validation diagnostics and `./scripts/check.sh` pass. - -Owner: `documentWithMetadataLocations` in -[`override_context.go`](../../internal/engine/override_context.go), using -`nearestDocumentLocation` in -[`validation.go`](../../internal/engine/validation.go). - -The new effective-projection walk calls the unindexed nearest-location helper -for each field lacking a real origin location, often once for its mapping entry -and again for its value. Each call starts at the fallback root and linearly scans -mapping entries. With N authored root fields and N different default-only root -fields, each default's failed lookup scans all N authored keys: quadratic work -for linear-sized input and output. The shared lifecycle calls this path even -without CLI assignments or a resolver. - -Required remediation: share an indexed fallback lookup or traverse corresponding -containers together while carrying the nearest available location. Existing -`documentLookup` / mapping-index machinery can support bounded-depth lookups -without repeated wide scans. Preserve nearest-ancestor fallback, source IDs, -synthetic locations, secret-reference diagnostics, and publication ownership. -Exercise mixed authored/default-only broad mappings; do not weaken attribution -or skip needed location attachment to avoid the work. - -## 23. Index Repeated Collection-Policy Document Lookups — P2 - -Status: [x] Complete. - -Completion: overlay policy matching shares an immutable incoming-tree lookup; -declaration-only checks lazily index the completed result after merging. -Inheritance policy reconciliation shares one lookup over its stable parent and -child inputs. Added broad overlay/inheritance collections, retained and pending -declarations, escaped keys, ordered append, and origin coverage. Existing -retirement and diagnostic regressions, `./scripts/check.sh`, and the final -`go test -race ./...` pass. - -Owners: `mergeOverlayMappingsWithCollectionPolicy` in -[`overlay.go`](../../internal/engine/overlay.go) and -`mergeInheritanceCollectionPolicies` in -[`inheritance_collection.go`](../../internal/engine/inheritance_collection.go). - -These routines loop over collection policies/declarations and call plain -`documentAtPath` for each destination. That helper linearly searches every -mapping on the path. N independent collections below N sibling mapping keys -therefore cause quadratic mapping-entry searches when checking which lower -policies the incoming tier touches. The inheritance parent/child lookups and -retained-value declaration checks have the same pattern. YAML declarations alone -can establish these policies; the deliberately pairwise application-registration -overlap check is not involved. - -Required remediation: reuse a document lookup built once for each stable input -tree, or use the already collected claims where their scope is sufficient. -Index a mutated result only after its value-changing phase, rather than retaining -a stale lookup. Preserve absent destinations, non-sequence pending declarations, -literal key/index distinctions, retirement, and error locations. Verify broad -independent collections across precedence and inheritance; no new public cache, -shape requirement, or performance gate is needed.