From 6a1447c327bb0cf13fa92e1cbb449cbdecad519d Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 26 Oct 2026 17:46:31 +0000 Subject: [PATCH] Reconcile retained history accounting --- docs/roadmap/implementation.md | 8 +++- internal/engine/history.go | 46 +-------------------- internal/engine/history_budget_test.go | 41 +++++++++++++++++- internal/engine/history_cli.go | 2 +- internal/engine/history_cli_test.go | 5 --- internal/engine/history_inheritance.go | 2 +- internal/engine/inheritance.go | 14 ++++++- internal/engine/inheritance_history_test.go | 2 +- 8 files changed, 62 insertions(+), 58 deletions(-) diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index c19f786..a0ed38e 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,6 +1,6 @@ # Interface Contract Remediation Implementation Plan -Status: in progress. Stages 1–12 are complete; Stages 13–18 remain pending. +Status: in progress. Stages 1–15 are complete; Stages 16–18 remain pending. Baseline: `f6d54a094a2b5ada5748686620659399dce7ebf0`. Audience: a Luna coding agent executing the stages sequentially. @@ -663,7 +663,11 @@ mutates it. Focused check: ## Stage 15 — Reconcile Complete Retained-History Accounting Owners: `executeLoadRequest`, `retainedNodeCounter`, model root enumeration, -`history_budget_test.go`, inheritance budget tests. Status: pending. +`history_budget_test.go`, inheritance budget tests. Status: complete. Validation: +`GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./internal/engine -run 'History.*Budget|HistoryNode|Retained|ProjectionBudget' -count=1`, +`GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./...`, +`GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test -race ./internal/engine -run 'History.*Budget|HistoryNode|Retained|ProjectionBudget|CLIHistoryEvidenceKeepsCheckpoints|Detached|ConcurrentReadOnly' -count=1`, +and `git diff --check`. 1. Replace `addInspectionHistory`'s old field/copy enumeration with the actual model and final-inspection root graph. Include all checkpoint, application, diff --git a/internal/engine/history.go b/internal/engine/history.go index 7287b7c..33d492b 100644 --- a/internal/engine/history.go +++ b/internal/engine/history.go @@ -40,50 +40,6 @@ func (c *retainedNodeCounter) add(root *document.Node) bool { return true } -func (c *retainedNodeCounter) canAddClones(roots []*document.Node, copies int) bool { - if c == nil || c.limit < 0 { - return true - } - remaining := c.limit - c.count - if remaining < 0 { - return false - } - for copyIndex := 0; copyIndex < copies; copyIndex++ { - for _, root := range roots { - count, within := documentCloneNodeCount(root, remaining) - if !within { - return false - } - remaining -= count - } - } - return true -} - -func documentCloneNodeCount(root *document.Node, limit int) (int, bool) { - if root == nil { - return 0, true - } - count := 0 - stack := []*document.Node{root} - for len(stack) != 0 { - node := stack[len(stack)-1] - stack = stack[:len(stack)-1] - if node == nil { - continue - } - count++ - if count > limit { - return 0, false - } - for _, entry := range node.Mapping { - stack = append(stack, entry.Value) - } - stack = append(stack, node.Sequence...) - } - return count, true -} - func addInspectionHistory(counter *retainedNodeCounter, inspection *Inspection) bool { if inspection == nil || inspection.history == nil { return false @@ -93,7 +49,7 @@ func addInspectionHistory(counter *retainedNodeCounter, inspection *Inspection) return false } } - return true + return counter.add(inspection.final) } // History applications borrow immutable policy-validated input trees. Metadata diff --git a/internal/engine/history_budget_test.go b/internal/engine/history_budget_test.go index ef2fe01..b82534e 100644 --- a/internal/engine/history_budget_test.go +++ b/internal/engine/history_budget_test.go @@ -23,7 +23,7 @@ type historyBudgetLayeredConfig struct { func retainedInspectionNodeCountForTest(inspection *Inspection) int { seen := make(map[*document.Node]struct{}) model := inspection.history - stack := []*document.Node{model.baseline, model.final} + stack := []*document.Node{model.baseline, model.final, inspection.final} for _, payload := range model.sourcePayloads { if payload.kind == historySourceApplicationPayload { stack = append(stack, payload.application.value) @@ -115,6 +115,25 @@ func TestLoadHistoryBudgetUsesExactRetainedNodeCount(t *testing.T) { } } +func TestHistoryNodeBudgetIncludesFinalInspectionProjection(t *testing.T) { + final := &document.Node{Kind: document.Mapping, Mapping: []document.Entry{{ + Key: "value", Value: &document.Node{Kind: document.String, Text: "final"}, + }}} + builder := newHistoryBuilder() + model := builder.freeze() + inspection := &Inspection{history: model, metadata: Metadata{historyModel: model}, final: final} + + counter := newRetainedNodeCounter(2) + admitted := addInspectionHistory(counter, inspection) + if !admitted || counter.count != 2 { + t.Fatalf("final inspection projection was not counted exactly: admitted=%t count=%d", admitted, counter.count) + } + counter = newRetainedNodeCounter(1) + if addInspectionHistory(counter, inspection) { + t.Fatal("final inspection projection did not consume the node budget") + } +} + func TestLayeredHistoryBudgetRejectsBeforeSecretsAndValidation(t *testing.T) { type callbackCounts struct { lookups int @@ -131,6 +150,7 @@ func TestLayeredHistoryBudgetRejectsBeforeSecretsAndValidation(t *testing.T) { Resolve: func(configuration *historyBudgetLayeredConfig, resolution *Resolution) error { configuration.Child.Value += "-resolved" resolution.Record(Keys("child", "value"), ChangeNormalization, "normalize child") + resolution.Record(Keys("child"), ChangeDerivedDefault, "record enclosing child state") return nil }, Validate: func(*historyBudgetLayeredConfig, *Validation) { counts.validations++ }, @@ -159,6 +179,25 @@ func TestLayeredHistoryBudgetRejectsBeforeSecretsAndValidation(t *testing.T) { if baseline.Inspection.history == nil || baseline.Inspection.metadata.historyModel != baseline.Inspection.history || len(baseline.Inspection.history.inheritancePayloads) == 0 { t.Fatal("layered load did not publish its shared inheritance evidence owner") } + resolution := baseline.Inspection.history.resolutionPayload + if resolution == nil || len(resolution.snapshots) != 2 || len(resolution.replayGroups) == 0 { + t.Fatalf("layered load did not retain overlapping original resolution evidence: %#v", resolution) + } + resolutionPaths := make(map[string]bool, len(resolution.snapshots)) + for _, snapshot := range resolution.snapshots { + resolutionPaths[snapshot.change.Path.String()] = true + } + if !resolutionPaths["/child"] || !resolutionPaths["/child/value"] { + t.Fatalf("layered load resolution snapshots have paths %v, want /child and /child/value", resolutionPaths) + } + group := resolution.replayGroups[0] + groupRecords := make(map[int]bool, len(group.originalRecordIDs)) + for _, id := range group.originalRecordIDs { + groupRecords[id] = true + } + if len(groupRecords) != 2 || !groupRecords[0] || !groupRecords[1] || group.change.Path.String() != "/child" { + t.Fatalf("layered load did not group both overlapping original records: %#v", group) + } exactCounts := &callbackCounts{} loaded, err := loadGroupedWithSecretLookupLimit(makeRequest(exactCounts), testSecretBinding(), lookup(exactCounts), want) if err != nil || loaded == nil { diff --git a/internal/engine/history_cli.go b/internal/engine/history_cli.go index e8b0d28..2721851 100644 --- a/internal/engine/history_cli.go +++ b/internal/engine/history_cli.go @@ -21,7 +21,7 @@ func (builder *historyBuilder) observeTypedCheckpoint(phase historyPhase, root * panic("configkit: typed checkpoint has an invalid history phase") } payload := builder.nextPayload(phase) - checkpoint := historyCheckpointPayload{root: document.Clone(root), origins: cloneOriginIndex(origins)} + checkpoint := historyCheckpointPayload{root: root, origins: cloneOriginIndex(origins)} switch phase { case historyPhasePreCLI: if builder.model.preCLICheckpoint != nil || payload.id != 0 { diff --git a/internal/engine/history_cli_test.go b/internal/engine/history_cli_test.go index a89bfce..f405bfa 100644 --- a/internal/engine/history_cli_test.go +++ b/internal/engine/history_cli_test.go @@ -119,14 +119,9 @@ func TestCLIHistoryEvidenceKeepsCheckpointsAndConditionalReconciliation(t *testi builder.observePreCLICheckpoint(preCLI, preOrigins) builder.observeCLIEvidence(applications, records) builder.observePostCLICheckpoint(postCLI, postOrigins) - preCount, _ := documentAtPath(preCLI, Keys("count")) - postCount, _ := documentAtPath(postCLI, Keys("count")) - preCount.Text = "producer mutation" - postCount.Text = "producer mutation" model := builder.freeze() if model.preCLICheckpoint == nil || model.postCLICheckpoint == nil || - model.preCLICheckpoint.root == preCLI || model.postCLICheckpoint.root == postCLI || len(model.cliPayloads) != 6 { t.Fatalf("checkpoint/CLI payload ownership = pre:%#v post:%#v CLI:%d", model.preCLICheckpoint, model.postCLICheckpoint, len(model.cliPayloads)) } diff --git a/internal/engine/history_inheritance.go b/internal/engine/history_inheritance.go index 9b741e7..70c2001 100644 --- a/internal/engine/history_inheritance.go +++ b/internal/engine/history_inheritance.go @@ -42,7 +42,7 @@ func (builder *historyBuilder) observeInheritanceEvidence(operations []inheritan if payload.id != len(builder.model.inheritancePayloads) { panic("configkit: inheritance payload identifiers are out of order") } - owned := cloneInheritanceOperations(operations) + owned := copyInheritanceOperations(operations, false) builder.model.inheritancePayloads = append(builder.model.inheritancePayloads, historyInheritancePayload{operations: owned}) for operationIndex, operation := range owned { diff --git a/internal/engine/inheritance.go b/internal/engine/inheritance.go index 1dfafb8..7d8ee8f 100644 --- a/internal/engine/inheritance.go +++ b/internal/engine/inheritance.go @@ -85,6 +85,12 @@ func cloneInheritanceTransfer(transfer inheritanceTransfer) inheritanceTransfer } func cloneInheritanceOperations(operations []inheritanceOperation) []inheritanceOperation { + return copyInheritanceOperations(operations, true) +} + +// copyInheritanceOperations detaches mutable operation metadata. History keeps +// the already immutable engine-owned document roots rather than copying them. +func copyInheritanceOperations(operations []inheritanceOperation, cloneDocuments bool) []inheritanceOperation { if operations == nil { return nil } @@ -101,8 +107,12 @@ func cloneInheritanceOperations(operations []inheritanceOperation) []inheritance transfer := cloneInheritanceTransfer(*operation.transfer) cloned[index].transfer = &transfer } - cloned[index].parent = document.Clone(operation.parent) - cloned[index].effective = document.Clone(operation.effective) + cloned[index].parent = operation.parent + cloned[index].effective = operation.effective + if cloneDocuments { + cloned[index].parent = document.Clone(operation.parent) + cloned[index].effective = document.Clone(operation.effective) + } cloned[index].childOrigins = cloneOriginIndex(operation.childOrigins) cloned[index].parentOrigins = cloneOriginIndex(operation.parentOrigins) cloned[index].effectiveOrigins = cloneOriginIndex(operation.effectiveOrigins) diff --git a/internal/engine/inheritance_history_test.go b/internal/engine/inheritance_history_test.go index 2ec5e9f..ed8a150 100644 --- a/internal/engine/inheritance_history_test.go +++ b/internal/engine/inheritance_history_test.go @@ -400,7 +400,7 @@ func TestHistoryNodeBudgetCountsSharedInheritanceRootsOnce(t *testing.T) { model := builder.freeze() inspection := &Inspection{history: model, metadata: Metadata{historyModel: model}} counter := newRetainedNodeCounter(2) - if !addInspectionHistory(counter, inspection) || counter.count != 2 { + if !addInspectionHistory(counter, inspection) || counter.count != 1 { t.Fatal("shared evidence node accounting is incorrect") } operation.effective = extra