diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 0e8ba1c..c725a3d 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -343,7 +343,14 @@ asserting timing or promising a new public call-count contract. ## 7. Explanation Dispatch Repeatedly Searches All Selections And Payloads -Priority: P2. Status: open. +Priority: P2. Status: complete. + +Completion: a query-local payload-reference index and application-fallback map +are built in one pass over ordered selections. Source/CLI, inheritance, and +resolution replay use direct lookups while retaining model chronology and the +query-scoped attribution correction. The tier-history regression now also +checks explanation contribution order/origins. The full ordinary suite and +`git diff --check` passed; no replay algorithm or shared-state lifetime changed. Relevant code: `Inspection.Explain`, `historySelectionForPayload`, `historySelectionForApplication`, and `historySelectionForResolutionGroup` in diff --git a/facade_tier_history_test.go b/facade_tier_history_test.go index a790bb2..000adc2 100644 --- a/facade_tier_history_test.go +++ b/facade_tier_history_test.go @@ -24,11 +24,18 @@ func TestTierHistoryRecordsEachContributionOnce(t *testing.T) { if len(history) != 3 { t.Fatalf("history has %d records; want the three independent contributions", len(history)) } + explanation, found := result.Explain(configkit.Keys("x")) + if !found || len(explanation.History) != len(history) { + t.Fatal("explanation lost ordered tier contributions") + } for index, sourceID := range []string{"source-2", "source-3", "source-1"} { change := history[index] if len(change.Origin.Locations) != 1 || change.Origin.Locations[0].SourceID != sourceID { t.Fatalf("contribution %d has origin %#v", index, change.Origin) } + if len(explanation.History[index].Origin.Locations) != 1 || explanation.History[index].Origin.Locations[0] != change.Origin.Locations[0] { + t.Fatalf("explanation contribution %d has a different source origin", index) + } if index == 0 { if len(change.Previous) != 0 { t.Fatal("first source contribution has a previous origin") diff --git a/internal/engine/explain.go b/internal/engine/explain.go index e779944..12178e9 100644 --- a/internal/engine/explain.go +++ b/internal/engine/explain.go @@ -44,17 +44,18 @@ func (r *Inspection) Explain(path Path) (Explanation, bool) { if model == nil { return Explanation{}, false } - selections := model.selectHistory(path) - if len(selections) == 0 && !model.hasObservedShape(path) { + selected := model.selectHistory(path) + if len(selected) == 0 && !model.hasObservedShape(path) { return Explanation{}, false } + selections := model.indexReplaySelections(selected) explanation := Explanation{Path: path.clone(), Effective: valueSnapshotWithLookup(r.final, path, r.secrets, model.documentLookup)} if info, ok := r.metadata.Lookup(path); ok { explanation.Origin = cloneOrigin(info.Origin) } working := document.Clone(model.baseline) - if baseline, found := historySelectionForPayload(selections, historyPayloadRef{phase: historyPhaseBaseline, id: 0}); found && baseline.after { + if baseline, found := selections.byPayload[historyPayloadRef{phase: historyPhaseBaseline, id: 0}]; found && baseline.after { explanation.History = append(explanation.History, ValueChange{ After: valueSnapshot(working, path, r.secrets), Origin: cloneOrigin(baseline.attribution.origin), @@ -68,7 +69,7 @@ func (r *Inspection) Explain(path Path) (Explanation, bool) { application := payload.application before := valueSnapshot(working, path, r.secrets) applyHistoryApplication(&working, application, replayIndex) - selected, relevant := historySelectionForApplication(model, selections, historyPhaseSources, historyPayloadRef{phase: historyPhaseSources, id: payloadID}) + selected, relevant := selections.application(historyPayloadRef{phase: historyPhaseSources, id: payloadID}) if !relevant { continue } @@ -88,7 +89,7 @@ func (r *Inspection) Explain(path Path) (Explanation, bool) { // decoding applies them to the final input tree. Show that reconciliation // explicitly before typed CLI contributions and domain resolution. if checkpoint := model.preCLICheckpoint; checkpoint != nil { - selection, selected := historySelectionForPayload(selections, historyPayloadRef{phase: historyPhasePreCLI, id: 0}) + selection, selected := selections.byPayload[historyPayloadRef{phase: historyPhasePreCLI, id: 0}] replayed, _ := documentAtPath(working, path) decoded, _ := documentAtPathWithLookup(checkpoint.root, path, model.documentLookup) if selected && !configurationValuesEqual(replayed, decoded) { @@ -110,7 +111,7 @@ func (r *Inspection) Explain(path Path) (Explanation, bool) { applicationRef := historyPayloadRef{phase: historyPhaseCLI, id: payloadID} before := valueSnapshot(working, path, r.secrets) applyHistoryApplication(&working, payload.application, replayIndex) - selected, relevant := historySelectionForApplication(model, selections, historyPhaseCLI, applicationRef) + selected, relevant := selections.application(applicationRef) if !relevant { continue } @@ -125,7 +126,7 @@ func (r *Inspection) Explain(path Path) (Explanation, bool) { }) } if checkpoint := model.postCLICheckpoint; checkpoint != nil { - selection, selected := historySelectionForPayload(selections, historyPayloadRef{phase: historyPhasePostCLI, id: 0}) + selection, selected := selections.byPayload[historyPayloadRef{phase: historyPhasePostCLI, id: 0}] replayed, _ := documentAtPath(working, path) effectiveInput, _ := documentAtPathWithLookup(checkpoint.root, path, model.documentLookup) if selected && !configurationValuesEqual(replayed, effectiveInput) { @@ -141,7 +142,7 @@ func (r *Inspection) Explain(path Path) (Explanation, bool) { } if model.resolutionPayload != nil { for _, group := range model.resolutionPayload.replayGroups { - selection, selected := historySelectionForResolutionGroup(selections, group) + selection, selected := selections.resolutionGroup(group) if !selected || group.coveringRecordID < 0 || group.coveringRecordID >= len(model.resolutionPayload.snapshots) { continue } @@ -171,41 +172,51 @@ func (r *Inspection) Explain(path Path) (Explanation, bool) { return explanation, true } -func historySelectionForPayload(selections []historySelection, ref historyPayloadRef) (historySelection, bool) { - for _, selection := range selections { - if selection.payload == ref { - return selection, true - } - } - return historySelection{}, false +// historyReplaySelections is query-local dispatch state, not another evidence +// or attribution owner. Replay still follows the model's ordered payloads. +type historyReplaySelections struct { + byPayload map[historyPayloadRef]historySelection + applicationFallbacks map[historyPayloadRef]historySelection } -func historySelectionForApplication(model *historyModel, selections []historySelection, phase historyPhase, applicationRef historyPayloadRef) (historySelection, bool) { - if direct, ok := historySelectionForPayload(selections, applicationRef); ok { - return direct, true +func (model *historyModel) indexReplaySelections(ordered []historySelection) historyReplaySelections { + indexed := historyReplaySelections{ + byPayload: make(map[historyPayloadRef]historySelection, len(ordered)), + applicationFallbacks: make(map[historyPayloadRef]historySelection), } - // Only removed/replaced paths lacking a direct contribution need an - // operation fallback. A descendant's location cannot describe its container. - var selected historySelection - found := false - var payloads []historySourcePayload - switch phase { - case historyPhaseSources: - payloads = model.sourcePayloads - case historyPhaseCLI: - payloads = model.cliPayloads - default: - return selected, found - } - for payloadID, candidate := range payloads { - if candidate.kind != historySourceOverlayPayload || !candidate.hasApplicationRef || candidate.applicationRef != applicationRef { + for _, selection := range ordered { + ref := selection.payload + indexed.byPayload[ref] = selection + if selection.role != historyRoleOperation || ref.substep != 0 { continue } - if operation, ok := historySelectionForPayload(selections, historyPayloadRef{phase: phase, id: payloadID}); ok { - selected, found = operation, true + var payloads []historySourcePayload + switch ref.phase { + case historyPhaseSources: + payloads = model.sourcePayloads + case historyPhaseCLI: + payloads = model.cliPayloads + default: + continue + } + if ref.id < 0 || ref.id >= len(payloads) { + continue + } + payload := payloads[ref.id] + if payload.kind == historySourceOverlayPayload && payload.hasApplicationRef { + indexed.applicationFallbacks[payload.applicationRef] = selection } } - return selected, found + return indexed +} + +func (selections historyReplaySelections) application(ref historyPayloadRef) (historySelection, bool) { + if direct, found := selections.byPayload[ref]; found { + return direct, true + } + // Only removed/replaced paths lacking a direct contribution need fallback. + selection, found := selections.applicationFallbacks[ref] + return selection, found } func checkpointExplanationAttribution(selection historySelection) (Origin, []string) { @@ -215,11 +226,11 @@ func checkpointExplanationAttribution(selection historySelection) (Origin, []str return Origin{Operation: ChangeDecoding}, nil } -func historySelectionForResolutionGroup(selections []historySelection, group resolutionReplayGroup) (historySelection, bool) { +func (selections historyReplaySelections) resolutionGroup(group resolutionReplayGroup) (historySelection, bool) { var fallback historySelection found := false for _, recordID := range group.originalRecordIDs { - selection, ok := historySelectionForPayload(selections, historyPayloadRef{phase: historyPhaseResolution, id: recordID}) + selection, ok := selections.byPayload[historyPayloadRef{phase: historyPhaseResolution, id: recordID}] if !ok { continue } diff --git a/internal/engine/inheritance_explain.go b/internal/engine/inheritance_explain.go index dcc3a45..bea0403 100644 --- a/internal/engine/inheritance_explain.go +++ b/internal/engine/inheritance_explain.go @@ -2,7 +2,7 @@ package engine import "gitea.maximumdirect.net/eric/ConfigKit/internal/document" -func appendInheritanceExplanation(explanation *Explanation, working **document.Node, query Path, model *historyModel, selections []historySelection, secrets map[string]bool) { +func appendInheritanceExplanation(explanation *Explanation, working **document.Node, query Path, model *historyModel, selections historyReplaySelections, secrets map[string]bool) { if model == nil { return } @@ -10,7 +10,7 @@ func appendInheritanceExplanation(explanation *Explanation, working **document.N for operationIndex, operation := range payload.operations { if operation.control != nil { ref := historyPayloadRef{phase: historyPhaseInheritance, id: 0, substep: operationIndex * 2} - selection, selected := historySelectionForPayload(selections, ref) + selection, selected := selections.byPayload[ref] if !selected { continue } @@ -25,8 +25,8 @@ func appendInheritanceExplanation(explanation *Explanation, working **document.N } parentRef := historyPayloadRef{phase: historyPhaseInheritance, id: 0, substep: operationIndex * 2} childRef := historyPayloadRef{phase: historyPhaseInheritance, id: 0, substep: operationIndex*2 + 1} - parentSelection, hasParent := historySelectionForPayload(selections, parentRef) - childSelection, hasChild := historySelectionForPayload(selections, childRef) + parentSelection, hasParent := selections.byPayload[parentRef] + childSelection, hasChild := selections.byPayload[childRef] before := valueSnapshot(*working, query, secrets) replaceHistoryPath(working, operation.transfer.to, operation.parent) // A record keeps its own identity throughout inheritance. Restore it