diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 2547267..11489b6 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -212,7 +212,15 @@ in one focused overlay regression. ## 4. History Construction Repeatedly Performs Unindexed Mapping Searches -Priority: P2. Status: open. +Priority: P2. Status: complete. + +Completion: checkpoints, inheritance sides, and resolution before/final states +use one immutable document lookup per stable state, rather than scanning broad +mappings for every path. Lookups are rebuilt after scratch replay changes. +Removed the unused summary contribution list and its searches; the existing +test still verifies composite metadata and individual explanation attribution. +The full ordinary suite and `git diff --check` passed, including collection, +inheritance, resolution shape, and typed-checkpoint cases. Relevant code: `observeTypedCheckpoint` in [history_cli.go](../../internal/engine/history_cli.go), lines 38–49; diff --git a/internal/engine/history_cli.go b/internal/engine/history_cli.go index 2721851..90a6a4a 100644 --- a/internal/engine/history_cli.go +++ b/internal/engine/history_cli.go @@ -37,9 +37,10 @@ func (builder *historyBuilder) observeTypedCheckpoint(phase historyPhase, root * beforeRoot := builder.scratch paths := unionCLIHistoryPaths(beforeRoot, root) + lookup := buildDocumentLookup(beforeRoot, root) for _, path := range paths { - beforeNode, before := documentAtPath(beforeRoot, path) - afterNode, after := documentAtPath(root, path) + beforeNode, before := documentAtPathWithLookup(beforeRoot, path, lookup) + afterNode, after := documentAtPathWithLookup(root, path, lookup) sensitive := beforeNode != nil && beforeNode.Sensitive || afterNode != nil && afterNode.Sensitive builder.addObservation(historyObservation{ path: path, phase: phase, payload: payload, diff --git a/internal/engine/history_inheritance.go b/internal/engine/history_inheritance.go index db1ac55..d12ea56 100644 --- a/internal/engine/history_inheritance.go +++ b/internal/engine/history_inheritance.go @@ -95,12 +95,13 @@ func (builder *historyBuilder) observeInheritanceTransfer(payload historyPayload parentSide := unionInheritancePaths(beforePaths, parentPaths) parentRef := payload parentRef.substep = operationIndex * 2 + lookup := buildDocumentLookup(beforeChild, operation.parent) for _, path := range parentSide { if inheritanceOperationIdentity(operation, path) { continue } - beforeNode, before := documentAtInheritancePath(beforeChild, transfer.to, path) - parentNode, after := documentAtInheritancePath(operation.parent, transfer.to, path) + beforeNode, before := documentAtInheritancePath(beforeChild, transfer.to, path, lookup) + parentNode, after := documentAtInheritancePath(operation.parent, transfer.to, path, lookup) selection := selectInheritanceEvidence(resolveInheritanceAttribution(operation, path, inheritanceShapeFacts{ child: before, parent: after, }), inheritanceEvidenceParent, inheritanceEvidenceDisplaced) @@ -123,12 +124,13 @@ func (builder *historyBuilder) observeInheritanceTransfer(payload historyPayload childSide := unionInheritancePaths(parentStatePaths, effectivePaths) childRef := payload childRef.substep = operationIndex*2 + 1 + lookup = buildDocumentLookup(parentState, operation.effective) for _, path := range childSide { if inheritanceOperationIdentity(operation, path) { continue } - parentNode, before := documentAtInheritancePath(parentState, transfer.to, path) - effectiveNode, after := documentAtInheritancePath(operation.effective, transfer.to, path) + parentNode, before := documentAtInheritancePath(parentState, transfer.to, path, lookup) + effectiveNode, after := documentAtInheritancePath(operation.effective, transfer.to, path, lookup) selection := selectInheritanceEvidence(resolveInheritanceAttribution(operation, path, inheritanceShapeFacts{ parent: before, effective: after, }), inheritanceEvidenceChild, inheritanceEvidenceRemoval, inheritanceEvidenceDisplaced) @@ -203,12 +205,12 @@ func unionInheritancePaths(groups ...[]Path) []Path { return paths } -func documentAtInheritancePath(root *document.Node, rootPath, path Path) (*document.Node, bool) { +func documentAtInheritancePath(root *document.Node, rootPath, path Path, lookup *documentLookup) (*document.Node, bool) { relative, within := relativeHistoryPath(rootPath, path) if !within { return nil, false } - return documentAtPath(root, relative) + return documentAtPathWithLookup(root, relative, lookup) } func resolveInheritanceAttribution(operation inheritanceOperation, destination Path, shapes ...inheritanceShapeFacts) []inheritanceAttributionSelection { diff --git a/internal/engine/history_resolution.go b/internal/engine/history_resolution.go index 6c4a16b..d4e12a1 100644 --- a/internal/engine/history_resolution.go +++ b/internal/engine/history_resolution.go @@ -37,6 +37,7 @@ func (builder *historyBuilder) observeResolutionEvidence(snapshots []resolutionS replayGroups: coalescedResolutionSnapshots(snapshots), } attributions := make([]historyAttributionRef, len(snapshots)) + lookup := buildDocumentLookup(builder.scratch) for recordID, snapshot := range snapshots { snapshot.change = cloneChange(snapshot.change) payload.snapshots[recordID] = snapshot @@ -47,7 +48,7 @@ func (builder *historyBuilder) observeResolutionEvidence(snapshots []resolutionS } attribution := builder.addAttribution(snapshot.change.Origin, snapshot.change.Reasons, snapshot.change.Dependencies) attributions[recordID] = attribution - beforePath, beforeFound := historyConcretePathAt(builder.scratch, snapshot.change.Path) + beforePath, beforeFound := historyConcretePathAt(builder.scratch, snapshot.change.Path, lookup) if beforeFound { observeResolutionSubtree(builder, snapshot.before, beforePath, snapshot.change.Path, ref, attribution, true, false) } @@ -59,9 +60,10 @@ func (builder *historyBuilder) observeResolutionEvidence(snapshots []resolutionS covering := payload.snapshots[group.coveringRecordID] replaceHistoryPath(&builder.scratch, group.change.Path, covering.after) } + lookup = buildDocumentLookup(builder.scratch) for recordID, snapshot := range payload.snapshots { ref := historyPayloadRef{phase: historyPhaseResolution, id: recordID} - afterPath, afterFound := historyConcretePathAt(builder.scratch, snapshot.change.Path) + afterPath, afterFound := historyConcretePathAt(builder.scratch, snapshot.change.Path, lookup) if afterFound { observeResolutionSubtree(builder, snapshot.after, afterPath, snapshot.change.Path, ref, attributions[recordID], false, true) } @@ -85,7 +87,7 @@ func observeResolutionSubtree(builder *historyBuilder, root *document.Node, path }) } -func historyConcretePathAt(root *document.Node, path Path) (Path, bool) { +func historyConcretePathAt(root *document.Node, path Path, lookup *documentLookup) (Path, bool) { if root == nil || !path.valid() { return Path{}, false } @@ -97,17 +99,11 @@ func historyConcretePathAt(root *document.Node, path Path) (Path, bool) { if segment.kind == pathSegmentIndex { return Path{}, false } - found := false - for _, entry := range current.Mapping { - if entry.Key == segment.key { - current = entry.Value - found = true - break - } - } - if !found { + position := lookup.find(current, segment.key) + if position < 0 { return Path{}, false } + current = current.Mapping[position].Value concrete = concrete.Key(segment.key) case document.Sequence: index, ok := schemaIndex(segment) diff --git a/internal/engine/history_source.go b/internal/engine/history_source.go index 1a9478f..b0f7870 100644 --- a/internal/engine/history_source.go +++ b/internal/engine/history_source.go @@ -26,7 +26,6 @@ type historySourcePayload struct { hasApplicationRef bool path Path origin Origin - contributions []historyPayloadRef } func (builder *historyBuilder) observeBaseline(root *document.Node) { @@ -128,8 +127,7 @@ func (builder *historyBuilder) observeSourceEvidence(root *document.Node, origin if !found || origin.Layer == "" || sourceOriginRepresentedByOperation(builder.model, path, origin) { return } - contributions := builder.sourceContributions(path, origin) - payload := builder.addSourceSummary(path, origin, contributions) + payload := builder.addSourceSummary(path, origin) attribution := builder.addAttribution(origin, nil, nil) builder.addObservation(historyObservation{ path: path, phase: historyPhaseSources, payload: payload, @@ -173,69 +171,17 @@ func (builder *historyBuilder) addSourcePayload(application *historyApplication, return payload } -func (builder *historyBuilder) addSourceSummary(path Path, origin Origin, contributions []historyPayloadRef) historyPayloadRef { +func (builder *historyBuilder) addSourceSummary(path Path, origin Origin) historyPayloadRef { payload := builder.nextPayload(historyPhaseSources) if payload.id != len(builder.model.sourcePayloads) { panic("configkit: source payload identifiers are out of order") } builder.model.sourcePayloads = append(builder.model.sourcePayloads, historySourcePayload{ kind: historySourceSummaryPayload, path: path.clone(), origin: cloneOrigin(origin), - contributions: append([]historyPayloadRef(nil), contributions...), }) return payload } -func (builder *historyBuilder) sourceContributions(path Path, summary Origin) []historyPayloadRef { - var contributions []historyPayloadRef - seen := make(map[historyPayloadRef]struct{}) - for _, observationID := range builder.model.observationsByPointer[path.String()] { - observation := builder.model.observations[observationID] - if observation.phase != historyPhaseSources || observation.role != historyRoleContribution || !sameConcreteHistoryPath(path, observation.path) { - continue - } - attribution, ok := builder.model.resolveAttribution(observation.attributionRef) - if !ok || !sourceLocationsContainedBy(attribution.origin, summary) { - continue - } - if observation.payload.id < 0 || observation.payload.id >= len(builder.model.sourcePayloads) { - continue - } - payload := builder.model.sourcePayloads[observation.payload.id] - if payload.kind != historySourceApplicationPayload { - continue - } - local, ok := historyApplicationLocalPath(payload.application, path) - if !ok { - continue - } - if _, exists := documentAtPath(payload.application.value, local); !exists { - continue - } - if _, exists := seen[observation.payload]; exists { - continue - } - seen[observation.payload] = struct{}{} - contributions = append(contributions, observation.payload) - } - return contributions -} - -func sourceLocationsContainedBy(candidate, summary Origin) bool { - if len(candidate.Locations) == 0 { - return false - } - available := make(map[Location]struct{}, len(summary.Locations)) - for _, location := range summary.Locations { - available[location] = struct{}{} - } - for _, location := range candidate.Locations { - if _, found := available[location]; !found { - return false - } - } - return true -} - func sourceApplicationOrigin(application historyApplication, node *document.Node) Origin { origin := cloneOrigin(application.origin) origin.Layer = application.layer diff --git a/internal/engine/history_source_test.go b/internal/engine/history_source_test.go index 28d3a21..4ca03c0 100644 --- a/internal/engine/history_source_test.go +++ b/internal/engine/history_source_test.go @@ -77,18 +77,6 @@ func TestHistorySourceObservationsPreserveRawAndCompositeDefinitions(t *testing. if composite == nil || !reflect.DeepEqual(locationSourceIDs(composite.origin.Locations), []string{"base", "overlay"}) { t.Fatalf("empty mapping composite attribution = %#v; selections %#v", composite, emptySelections) } - var emptySummary *historySourcePayload - for index := range model.sourcePayloads { - payload := &model.sourcePayloads[index] - if payload.kind == historySourceSummaryPayload && sameConcreteHistoryPath(payload.path, Keys("section", "empty")) { - emptySummary = payload - break - } - } - if emptySummary == nil || len(emptySummary.contributions) != 2 || emptySummary.contributions[0].id != 0 || emptySummary.contributions[1].id != 1 { - t.Fatalf("empty mapping summary contribution references = %#v", emptySummary) - } - metadata := Metadata{historyModel: model} metadataHistory := metadata.History(Keys("section", "empty")) var compositeSources []string