Record additional history scaling findings
This commit is contained in:
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user