diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 4dea83e..df3c687 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,13 +1,14 @@ # Interface Remediation Review Findings Reviewed commit: `98c8b7c8423a816acdfceace36165366b13719ee`. +Algorithmic follow-up: `91769e9` (unchanged production code). Comparison baseline: `0dd16aa8a935645ae2847d32edd86461dc4618f0` (the planning commit, before the implementation sprint). Scope: the completed [implementation plan](implementation.md) and [interface remediation roadmap](interface-remediation.md). This report records remaining defects and newly introduced scalability problems; it does not change -the selected architecture or public contracts. All six findings below are open. +the selected architecture or public contracts. All eight findings below are open. ## Architecture And Review Evidence @@ -28,7 +29,7 @@ particular, private builder lifecycle guards are not new consumer obligations. Review combined code/plan inspection, graph-based producer/consumer tracing, existing tests, and disposable public-API probes run against both revisions. -The first three findings were reproduced; the remaining three follow from the +The first three findings were reproduced; the remaining five follow from the new loops and index contents, without performance measurements. Probe programs and the baseline checkout were kept outside the repository. Production code and repository tests were not changed during the review. @@ -46,6 +47,16 @@ git diff --check The passing suite does not detect the reproduced regressions. Suggested coverage below targets those gaps rather than adding a general test matrix or new gates. +The algorithmic follow-up checked normalization, lookup/index construction, +selection, replay dispatch, sensitivity preparation, and retained-root traversal. +It expanded finding 4 to the inheritance/resolution adapters and added findings +7–8. The existing focused engine history, explanation, inheritance, and secret +tests passed with `go test ./internal/engine -run 'History|Explain|Inheritance|Secret' -count=1`. +No benchmark data was collected. The selected shared model, ordered-ID sorting, +net resolution grouping, scratch-state normalization, whole-tree explanation +replay, and fixed-point sensitivity closure are not being reconsidered. The +deferred contributor-trace and branch-local replay experiments remain separate. + ## 1. Final Dynamic Field Tags Can Be Lost From Redaction Priority: P1. Status: open. @@ -189,7 +200,10 @@ Priority: P2. Status: open. Relevant code: `observeTypedCheckpoint` in [history_cli.go](../../internal/engine/history_cli.go), lines 38–49; `sourceContributions` in [history_source.go](../../internal/engine/history_source.go), -lines 185–218; and the lookup implementation in +lines 185–218; `observeInheritanceTransfer` and `documentAtInheritancePath` in +[history_inheritance.go](../../internal/engine/history_inheritance.go), +lines 87–141 and 211–217; `historyConcretePathAt` and its callers in +[history_resolution.go](../../internal/engine/history_resolution.go); and the lookup implementation in [document_walk.go](../../internal/engine/document_walk.go). For every checkpoint path, construction calls `documentAtPath` on both trees. @@ -200,6 +214,15 @@ no CLI overrides. Source-summary construction introduces the same pattern: it calls `sourceContributions` for surviving paths, and that helper searches each raw application again without an index. +The new inheritance adapter similarly enumerates every before/parent/effective +path and then calls `documentAtInheritancePath` for each one. That helper falls +back to the same unindexed search: one inheritance transfer of a broad n-field +mapping incurs Theta(n²) sibling comparisons. Resolution normalization calls +`historyConcretePathAt` before and after for each record; its hand-written +mapping scan costs O(rn) for r records under an n-field mapping, or Theta(n²) +when all n fields are independently recorded. These are the same lookup problem, +not separate findings about the selected inheritance or resolution algorithms. + These are new repeated sibling scans, not the deliberately retained whole-tree explanation replay. Existing document lookup machinery or paired traversal can avoid them. Existing sorting and genuinely repeated producer evidence still @@ -207,7 +230,10 @@ have their own costs; no universal linear-time guarantee is implied. Remediation: build/reuse phase-local lookups or collect corresponding nodes during shared traversal instead of independently searching a broad mapping -for every child. Review the summary membership work as well: the new +for every child or record. Respect scratch-tree mutation boundaries: do not +reuse a stale lookup after replacing or editing its indexed state. Concrete +resolution paths must still reflect the corresponding before/after containers. +Review the summary membership work as well: the new `historySourcePayload.contributions` list is populated and tested but currently has no production reader. Preserve any membership the selected design needs, but avoid computing unused data through repeated searches. Retain behavior @@ -275,3 +301,81 @@ where concrete inventory requires it. Preserve distinct parsed, explicit-key, and explicit-index candidates until classification is unioned by pointer. Extend focused candidate/dependency coverage for repeated parsed paths without asserting timing or promising a new public call-count contract. + +## 7. Explanation Dispatch Repeatedly Searches All Selections And Payloads + +Priority: P2. Status: open. + +Relevant code: `Inspection.Explain`, `historySelectionForPayload`, +`historySelectionForApplication`, and `historySelectionForResolutionGroup` in +[explain.go](../../internal/engine/explain.go), especially lines 64–71, +106–113, and 174–232; and `appendInheritanceExplanation` in +[inheritance_explain.go](../../internal/engine/inheritance_explain.go). + +After the model has selected relevant evidence, each source/CLI application +linearly searches the selection list, scans the entire phase payload list to +find linked replacement records, and searches selections again for each linked +record. Inheritance steps and resolution group members also repeatedly search +the same selection slice. Payload references already provide stable, +comparable keys; these are lookup operations, not semantic comparisons. + +For A applications, P phase payloads, S selections, and R linked replacements, +source/CLI dispatch alone costs O(A(P+S) + RS). With m successive underlay tiers +each setting the same scalar in a constant-size configuration, A, P, and S all +grow with m: this introduces Theta(m²) dispatch work even though the existing +value replay and rendered history for that query have O(m) size/work. It is +independent of finding 5's candidate expansion and remains after that index is +repaired. The inheritance/resolution lookup loops have the corresponding +operation-count times selection-count cost. + +Remediation: index the ordered selections by `historyPayloadRef` once per query +and index or group replacement-to-application links once, rather than rediscover +them for every application. Reuse that lookup for inheritance substeps and +original resolution record IDs. Dispatch can then be linear in the selected +records and traversed relationships, excluding the deliberately selected sorting, +replay, cloning, and rendering costs. Keep replay chronology unchanged and +resolve the attribution precedence correctly with finding 3 rather than +preserving its last-descendant bug. Use query-local or eagerly frozen indexes; +no lazy mutation of published history is needed. Existing explanation owners +should protect output equivalence without a timing assertion. + +## 8. Sensitivity Preparation Expands One Dependency List Per Affected Path + +Priority: P2. Status: open. + +Relevant code: `sensitivityCandidates`, lines 238–247, and +`sensitivityDependencies`, lines 272–286, in +[history_select.go](../../internal/engine/history_select.go); +`observeResolutionEvidence` and `observeResolutionSubtree` in +[history_resolution.go](../../internal/engine/history_resolution.go); and +`classifySecretPathsWithEvidence` in [secret.go](../../internal/engine/secret.go). + +One original resolution record owns one dependency list, and all its before/ +after subtree observations share its attribution reference and reported target. +Nevertheless, `sensitivityDependencies` resolves and clones that attribution +and emits its full dependency list for every observation. For a single recorded +normalization of an n-node subtree with d distinct dependencies, this creates +Theta(nd) ordinary edges although there are only d logical target/dependency +pairs. With n and d growing together, edge materialization alone is quadratic +in the O(n+d) underlying evidence. Those duplicates also become separate +triggers in the unchanged sensitivity-closure implementation. + +The candidate collector repeats the attribution dependency loop for every +affected path and retains a dependency list on each concrete candidate, although +its consumer uses those lists to register dependency paths, not distinct +descendant-target edges. It also calls `sensitivityDependencies` a second time +for inheritance links, rebuilding and discarding the ordinary edge list. These +costs remain even if finding 6's parsed-path comparator is corrected. + +Remediation: enumerate ordinary dependencies once per logical recorded target/ +attribution, or directly from the original resolution payload, and register each +required dependency candidate without broadcasting its list to all descendant +observations. A single record's edge preparation should require O(n+d) traversal +and at most O(d) ordinary edges, apart from path/annotation sizes and unrelated +evidence. Avoid rebuilding ordinary edges merely to obtain inheritance links. +Preserve original records omitted by explanation grouping, concrete/parsed +predicate candidates, dependency order where observable, and existing +pointer-conservative propagation. The fixed-point algorithm and inheritance +link semantics need not change. Reuse transitive/cyclic sensitivity regressions +and add focused coverage for a subtree record with several dependencies; no +performance threshold or new public dependency restriction is warranted.