Record algorithmic efficiency audit findings

docs/roadmap/efficiency.md

@@ -0,0 +1,245 @@
+# Algorithmic Efficiency Audit
+
+Reviewed revision: `5098b56655bd383e22a57d6cf09d99db3e92090f`.
+Audit date: 2026-10-25.
+
+## Scope And Assessment
+
+This source-level audit reviewed the public facade, acquisition and discovery,
+strict composition, precedence assembly, collection policy, inheritance, schema
+and typed-value operations, callbacks, secrets, inspection, and comparison.
+Knowledge-graph searches and caller traces guided source inspection; the bounds
+below follow from the actual loops and handoffs, not timing measurements.
+
+The implementation already uses indexed mapping merges, tuple-indexed sequence
+matching, sparse item projections, scoped import adjacency, inheritance policy
+localization, and worklist sensitivity propagation. The findings are remaining
+batch-processing costs, not a recommendation to replace those mechanisms.
+
+The review excludes expressly selected algorithms, including deterministic
+sorting, stable Kahn scheduling, exact numeric identity normalization, and the
+pairwise registration-overlap check expressly selected in the completed sequence
+plan at `d0a605a:docs/roadmap/implementation.md`. It also leaves the
+[deferred performance experiments](future.md#3-performance-experiments) deferred.
+Required detached copies and genuinely expanding configuration/history output
+are not treated as avoidable overhead.
+
+Complexities describe the identified internal work, not an end-to-end runtime
+bound or a new consumer guarantee. Examples use bounded-depth paths and
+constant-sized records unless noted; hash-index alternatives use expected lookup
+costs, with string processing and actual copied payload accounted for separately.
+Existing resource limits bound realizable inputs but do not remove these growth
+patterns. No new limits, public caches, dependencies, or release gates are proposed.
+
+No additional functional defect was established during this pass. The four open
+findings in the separate [functional correctness audit](correctness.md) remain
+applicable and are not duplicated here.
+
+## 1. Policy Retirement Scans Every Collection For Every Replacement
+
+Status: open. Priority: medium.
+
+**Relevant code.** `collectionPolicyScope.retireCollectionPoliciesAtOrBelow` in
+[collection_policy_state.go](../../internal/engine/collection_policy_state.go),
+lines 163–174, scans the complete policy table. The replacement hook in
+`mergeOverlayMappingsWithCollectionPolicy`
+([overlay.go](../../internal/engine/overlay.go), lines 182–185) calls it for each
+ordinary replaced value. `applyOverlaySequenceEvidence` also calls it for
+whole-collection replacement.
+
+**Scaling case.** A lower tier has P independently declared collection policies
+and Q ordinary scalar settings elsewhere. A higher tier changes those Q scalar
+settings without touching a collection. Each scalar replacement scans all P
+policies even though none is affected: O(P × Q) policy visits. Taking P = Q = N
+gives quadratic work for linear-sized inputs, effective values, and policy state.
+This occurs within one precedence merge; it does not depend on many tiers,
+registration-overlap checking, or expanding inheritance output.
+
+**Potential refactoring.** Prepare an operation-local semantic path index over
+the policy table and use descendant queries for retirement, maintaining it as
+policies change. Alternatively, batch retirement against the old policy state
+where its timing can be preserved. For the disjoint example, preparation plus
+queries can be O(P + Q); generally charge additional work to affected entries
+and path lengths. Preserve explicit-mode persistence, record ownership, and the
+point at which old identity is retired. A delayed blanket retirement must not
+clear newly established higher-tier policy. Existing inheritance policy indexing
+provides a related mechanism to evaluate, not a requirement to unify its interface.
+
+## 2. Small Tiers Recopy Unrelated Accumulated Collection Policy
+
+Status: open. Priority: medium.
+
+**Relevant code.** `resolveStrictCollectionPolicies` in
+[compose.go](../../internal/engine/compose.go), lines 330–334, first obtains
+`effectiveLower` through `lowerCollectionPoliciesForClaims`, then clones the
+result. The latter helper
+([collection_retirement.go](../../internal/engine/collection_retirement.go),
+lines 65–71) already deep-clones and scans the entire lower scope.
+`cloneCollectionPolicyScope` / `cloneCollectionPolicy` in
+[collection_policy_state.go](../../internal/engine/collection_policy_state.go)
+also copy record-order and nested record-policy state.
+
+`assembleLayeredInput` in [load_request.go](../../internal/engine/load_request.go)
+supplies the accumulated lower policy when composing each underlay group and
+when applying that group to the accumulator. The overlay resolver repeats this
+whole-table preparation and additionally checks all retained policy destinations
+against the incoming tree (`overlay.go`, lines 145–150).
+
+**Scaling case.** An initial underlay establishes P constant-sized collection
+policies. Each of T later underlays contributes only one unrelated scalar, with
+no declarations or collection changes. Despite having no relevant collection
+work, each tier copies/scans all P policies: O(T × P) work and cumulative
+allocation. P = T = N yields quadratic bookkeeping for O(P + T) input and final
+policy/value sizes. These are transient copies of unchanged private policy, not
+T separately published snapshots required by the public ownership contract.
+
+**Potential refactoring.** Separate read-only lower-policy lookup from the owned
+policy changes produced by the current tier. Resolve only policies intersecting
+incoming claims/declarations, and apply those changes to the owned assembly
+state. A path index or localized view can avoid scanning unrelated entries;
+retain copies where state actually crosses a mutable ownership boundary. At
+minimum, avoid the immediate duplicate deep copy, but removing only that copy
+does not change the O(T × P) growth. Aim for work proportional to initial state,
+incoming material, and affected policy rather than the complete accumulated
+table per tier. Preserve pending declarations, replacement eligibility, and
+independent nested record-policy ownership.
+
+This is distinct from finding 1: one tier can avoid repeated retirement scans
+and still recopy the full table at every subsequent tier. It is also distinct
+from the deferred metadata-copy/public-record handoff experiment.
+
+## 3. Tier-History Projection Repeatedly Searches Wide Mappings
+
+Status: open. Priority: medium.
+
+**Relevant code.** `projectTierHistoryApplications` in
+[sequence_history_projection.go](../../internal/engine/sequence_history_projection.go),
+lines 17–30, calls unindexed `documentAtPath` for each mapped sequence in a source
+application. That helper delegates to `documentAtPathWithLookup` with no lookup;
+[document_walk.go](../../internal/engine/document_walk.go), lines 51–58, then
+linearly searches each mapping on the path.
+
+**Scaling case.** A source has N sibling collections under `collections`, each
+with a constant number of records. A single wildcard registration can identify
+all of them. When this tier upserts/appends over lower collections, the source
+application contains N sequence contributions requiring projection. Locating
+each `/collections/<key>` searches the same N-entry mapping, costing
+1 + 2 + ... + N entry comparisons: O(N²), despite linear-sized source material
+and projection output. This does not require N application registrations.
+
+**Potential refactoring.** Lazily build one read-only `documentLookup` for each
+original application value with active mapped contributions and reuse it, or
+collect the relevant sequence nodes in one traversal. With V input nodes, S
+contributions, maximum depth D, and I projected items, the lookup/projection
+portion can be O(V + S × D + I), rather than repeated wide scans. Preserve
+authored coordinates, noncontiguous item projections, replacement evidence,
+and independent output storage. This concerns load-time coordinate projection,
+not the deferred branch-local explanation-replay experiment.
+
+## 4. Compound Identity Restoration Rescans Both Records Per Field
+
+Status: open. Priority: medium.
+
+**Relevant code.** `preserveInheritanceIdentityFields` in
+[inheritance_scope.go](../../internal/engine/inheritance_scope.go), lines 143–161,
+searches the child's mapping for each identity field, then searches the merged
+mapping again. `combineInheritanceTemplate` invokes it after merging the parent
+payload with the child in
+[inheritance_combine.go](../../internal/engine/inheritance_combine.go), line 104.
+
+**Scaling case.** For K identity fields, C child entries, and M merged entries,
+the searches cost O(K × (C + M)), plus the required identity-value clones.
+A single parent/child relationship with K = C = M = N direct scalar identity
+fields performs quadratic comparisons for linear-sized records and identity
+metadata. There is no declared small bound on compound-identity width. The
+shared identity validator and parent-identity exclusion already use mapping or
+membership indexes; this restoration step does not.
+
+**Potential refactoring.** Index merged entry positions once and walk child
+entries against an identity-field set, or use equivalent operation-local mapping
+indexes. This can be O(K + C + M), plus required copies. Preserve child-owned
+identity values and locations, existing mapping order, and detached nodes; do
+not change parent lookup or identity equality. If the step proves redundant
+after the established merge, removal needs a separate equivalence review rather
+than an assumption that identity preservation is unnecessary.
+
+## 5. Secret Failure Reporting Repeats Unindexed Location Searches
+
+Status: open. Priority: low; affects batches of failures.
+
+**Relevant code.** `resolveSecretValue` reports each failed reference through
+`secretDiagnostic` in
+[secret_reference.go](../../internal/engine/secret_reference.go), lines 53–78 and
+112–117. That helper calls unindexed `nearestDocumentLocation`; the mapping
+branch of `nearestDocumentLocationWithLookup` in
+[validation.go](../../internal/engine/validation.go), lines 144–155, falls back
+to a linear search when no lookup is supplied.
+
+**Scaling case.** An N-entry mapping of secret references whose environment
+values are all absent produces N diagnostics. Each location lookup starts at
+the root and scans the mapping for its key, yielding O(N²) comparisons for
+O(N) references and diagnostics. Reusing the same environment name still
+reproduces the lookup pattern even though environment lookup itself is cached.
+Deterministic traversal and diagnostic sorting add their ordinary O(N log N)
+costs; they do not require the quadratic location work.
+
+**Potential refactoring.** Share a lazily initialized read-only `documentLookup`
+for the duration of secret resolution, using `nearestDocumentLocationWithLookup`
+for failures, or carry matching document/location state through traversal. For
+V input nodes and F failed references at depth D, attribution can be
+O(V + F × D), before existing diagnostic sorting. Keep the success path free
+of an eager index if it is unnecessary. Preserve all diagnostics, nearest-known
+ancestor fallback, source coordinates, and safe messages; failing fast instead
+would change behavior rather than optimize it.
+
+## 6. Dynamic Values Repeat A Type-Graph Eligibility Walk
+
+Status: open. Priority: low; most relevant to repeated dynamic container types.
+
+**Relevant code.** `valueOperation.dynamicDescriptor` in
+[value_operation.go](../../internal/engine/value_operation.go), lines 36–48,
+uses the schema descriptor cache but still calls `descriptorContainsSecret`
+with a fresh visited map on every value. That helper (lines 51–68) traverses
+the reachable descriptor graph. Dynamic branches in projection and cloning
+invoke it for every non-nil interface value, before handling the concrete value.
+
+**Scaling case.** Let N dynamic values have the same concrete pointer type whose
+descriptor graph has size S (nodes plus member/element edges). Each call rechecks
+the same graph, costing O(N × S), even after descriptor construction is cached.
+For example, typed defaults can supply N interface entries holding nil pointers
+to one wide, supported, secret-free struct type. Actual values and projected
+output contain only N nulls, while the struct has S fields. Taking N = S makes
+the repeated eligibility walk quadratic in the combined type/value description,
+without requiring quadratic value expansion.
+
+**Potential refactoring.** Remember successfully approved dynamic descriptors
+by concrete type within `valueOperation`, after complete construction and the
+secret-semantics check. The repeated-type case becomes O(S + N) eligibility
+work, with one entry per approved concrete type. Do not treat a traversal-local
+visited marker as a completed negative result for a cyclic descriptor graph.
+Do not cache partial descriptors or path-bearing failures; coordinate with
+finding 2 of the [correctness audit](correctness.md), which identifies failed
+construction cache poisoning. Preserve rejection of dynamic secret-reference
+types, including nil pointers to them. This is operation-local reuse, not the
+deferred cross-load schema cache or a redesign of typed cloning.
+
+## Validation And Recommended Order
+
+Existing focused tests passed on Go 1.26.10, Linux amd64:
+
+```sh
+go test ./internal/engine -run 'Collection|Sequence|Inheritance|Secret|Dynamic' -count=1
+go test . -run '^TestLocalDocumentationLinks$' -count=1
+git diff --check
+```
+
+No benchmarks, timing comparisons, or new tests were needed to establish the
+loop bounds. Passing tests do not establish efficient growth. Production code
+and existing correctness findings are unchanged by this audit.
+
+Consider findings 1 and 2 together when selecting a policy-state refactoring,
+while keeping their acceptance cases distinct. Findings 3 and 4 are narrower
+independent changes; findings 5 and 6 are lower-priority follow-ups. These are
+opportunities for consideration, not an implementation sequence or new release
+blockers. Any later refactoring should preserve the existing contracts and use
+focused behavioral coverage without adding performance thresholds.