Retire the completed remediation record
This commit is contained in:
@@ -179,8 +179,8 @@ Current behavior belongs to the configuration contract's
|
|||||||
[strict composition](config.md#strict-document-composition),
|
[strict composition](config.md#strict-document-composition),
|
||||||
[sibling inheritance](config.md#sibling-inheritance), and
|
[sibling inheritance](config.md#sibling-inheritance), and
|
||||||
[CLI overrides](config.md#explicit-cli-overrides) sections.
|
[CLI overrides](config.md#explicit-cli-overrides) sections.
|
||||||
The full [remediation record](roadmap/remediation.md) remains available, including
|
The completed remediation record, including the algorithm-audit findings, is
|
||||||
the open algorithm-audit findings.
|
preserved at `ea46001:docs/roadmap/remediation.md`.
|
||||||
|
|
||||||
Potential enhancements are tracked in [future features](roadmap/future.md).
|
Potential enhancements are tracked in [future features](roadmap/future.md).
|
||||||
|
|
||||||
|
|||||||
@@ -161,9 +161,10 @@ syntax, dependencies, or complexity-based gates.
|
|||||||
|
|
||||||
## Relationship To Remediation And Other Work
|
## Relationship To Remediation And Other Work
|
||||||
|
|
||||||
[Remediation](remediation.md) owns the identified correctness and algorithmic
|
The completed remediation record at `ea46001:docs/roadmap/remediation.md`
|
||||||
efficiency findings. Several overlap these files, particularly item-scope
|
documents the identified correctness and algorithmic efficiency findings.
|
||||||
application matching and inheritance policy localization.
|
Several overlap these files, particularly item-scope application matching and
|
||||||
|
inheritance policy localization.
|
||||||
|
|
||||||
Use the documented contract and completed remediation as the behavioral baseline.
|
Use the documented contract and completed remediation as the behavioral baseline.
|
||||||
Keep corrective changes reviewable under their findings, and retain their
|
Keep corrective changes reviewable under their findings, and retain their
|
||||||
|
|||||||
@@ -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.
|
|
||||||
Reference in New Issue
Block a user