Record sequence algorithm audit findings

docs/roadmap/remediation.md

@@ -1,8 +1,8 @@
 # Sequence Implementation Review Remediation
 
-Status: all findings 1–17 are complete. The subsequent integration audit at
-`8d40983271de5239312eb0a2896737be88c5abdc` identified findings 12–17, now
-remediated below. This record introduces no new release gate.
+Status: findings 1–17 are complete. The algorithm audit below identifies six
+additional open findings, numbered 18–23. This record introduces no new release
+gate.
 
 Reviewed commit: `0be7cdb00eff576fda6b30c0600b2134d4e2e086`.
 Implementation range: `bc6b977..0be7cdb`.
@@ -708,3 +708,198 @@ 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: [ ] Open.
+
+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: [ ] Open.
+
+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: [ ] Open.
+
+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: [ ] Open.
+
+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: [ ] Open.
+
+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: [ ] Open.
+
+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.