diff --git a/docs/internal/runner.md b/docs/internal/runner.md index 324871b..ee4532d 100644 --- a/docs/internal/runner.md +++ b/docs/internal/runner.md @@ -105,8 +105,11 @@ one execution-precedence and error-ordering implementation while allowing a full backend pool to reject work before expensive schema, artifact, and rendering operations. `Prepare` discards the plan after returning its public metadata. `Run` retains the plan through initial and repaired-output validation -and discards it when the operation ends. Prepared execution stores the same -kind of plan only in its private payload. +and discards it when the operation ends. Prepared execution transfers the +completed preparation's owned message, input-hash, and copied-schema values +into its private payload, while deep-copying mutable provider extras and +creating an independent details snapshot. Claim completion or discard clears +the execution payload without removing the details snapshot. Pointer-based numeric overrides preserve an explicit zero. Invalid negative or out-of-range values fail as invalid requests. Endpoint overrides do not change diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index b136f41..507f00d 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -3,7 +3,7 @@ Implement the remaining local work in the [performance roadmap](performance.md) in order. Each sequential stage is scoped for one Luna coding-agent prompt. Update status and evidence as each work unit lands. -Stages 1–12 are complete; Stages 13–14 remain pending. The completed +Stages 1–13 are complete; Stage 14 remains pending. The completed inspection plan is preserved at `481e2d4:docs/roadmap/implementation.md` in Git history. @@ -496,9 +496,17 @@ message length as deep-copied bytes, and document lifetime assumptions. ## Stage 13 — Conditionally Consolidate Prepared Creation's First Snapshot Copy -**Outcome:** Either one justified creation-copy simplification with preserved -ownership, or a completed, evidence-backed decision to retain the current -implementation. This does not authorize skipping measurement or ownership work. +**Status: Complete (2026-10-09).** The selected transfer was retained. Prepared +execution now reuses preparation-owned messages, hashes, and schema, while +deep-copying nested provider extras and keeping an independent details +snapshot. Source-mutation and invalid-copy coverage pass; benchmark evidence +shows meaningful reductions for mutable payloads without a material increase +in retained heap. + +**Outcome:** The ownership inventory supports transferring preparation-owned +values. The simplified creation path is retained with its nested-extra copy, +independent details snapshot, and existing cleanup and caller-ownership +boundaries. **Selected candidate:** Transfer preparation's already-owned messages, hashes, and schema into its private execution payload instead of deep-copying them @@ -892,6 +900,68 @@ separate details clone. Focused verification passed: `go test -mod=readonly ./internal/usecase .`; representative content, copied schema, fake validation status, and post-completion details were checked outside timing. +### Prepared Creation Snapshot Comparison + +The before measurements use the Stage 11 assembled benchmark fixture committed +at `743e78d` and the Stage 12 ownership fixture committed at `11ecb92`. The +after measurements use the Stage 13 candidate in this completion. Both runs +used the same fixtures and environment: Linux/amd64, Intel Core i5-7260U @ +2.20 GHz, benchmark parallelism 4, Go 1.26.8, ConfigKit `v0.5.0`, JSON Schema +`v6.0.2`, and backend modules `v3.20260930.0`. Benchmarks ran without the race +detector. The commands are those in the Assembled Workflow Benchmark Baseline +and Prepared Snapshot Ownership And Live-Heap Baseline subsections above. + +The assembled creation benchmark measures `PrepareExecution` followed by +discarding its unused handle. Times list five `ns/op` samples, then median and +range; allocations are `B/op; allocs/op`. + +| Case | Before `ns/op` (median; range) | Before allocations | After `ns/op` (median; range) | After allocations | +| --- | --- | --- | --- | --- | +| Small | 8194, 8016, 8095, 8073, 7939 (8073; 7939–8194) | 5210; 56 | 11028, 11059, 11756, 12279, 11089 (11089; 11028–12279) | 4665; 53 | +| Large message | 937857, 955672, 919810, 1058503, 1094114 (955672; 919810–1094114) | 873679–873681; 56 | 1751124, 1647596, 1761392, 1556071, 1584325 (1647596; 1556071–1761392) | 873135–873144; 53 | +| Mutable payload | 8114785, 8394320, 10387083, 8418604, 8528766 (8418604; 8114785–10387083) | 3711282–3711329; 60727 | 12157300, 12363416, 12185745, 11868982, 21225784 (12185745; 11868982–21225784) | 3483089–3483252; 56324–56326 | + +Small and large cases each save about 0.5 KB and three allocations, while the +mutable case saves about 228 KB and 4,401–4,403 allocations. The after timing +samples in this run are higher than the recorded before samples, while other +repetitions on the same machine varied substantially; the timing evidence +does not establish a reliable latency improvement or regression. The ordinary +`Prepare`, `Run`, and public `Details()` allocation counts are unchanged within +their recorded baseline ranges; this candidate only changes prepared-execution +creation. + +The focused internal snapshot benchmark isolates the same creation path using +the Stage 12 fake-collaborator fixture. Times again list five samples, median, +and range. + +| Snapshot case | Before `ns/op` (median; range) | Before allocations | After `ns/op` (median; range) | After allocations | +| --- | --- | --- | --- | --- | +| Small | 45776, 51524, 46933, 46834, 44663 (46834; 44663–51524) | 13014–13016; 216 | 50907, 79812, 62186, 65569, 62599 (62599; 50907–79812) | 9405–9406; 156 | +| Mutable payload | 5343223, 5234718, 4213550, 4419520, 5046181 (5046181; 4213550–5343223) | 1211931–1212104; 25243–25244 | 5991662, 5508812, 6244981, 5176459, 4365122 (5508812; 4365122–6244981) | 983033–983309; 20837–20839 | + +The internal fixture saves about 3.6 KB/60 allocations for the small case and +about 229 KB/4,404–4,407 allocations for the mutable case. Timing samples vary +between repeated runs and do not establish a consistent speedup or regression. + +The retained-heap comparison uses the documented fixed batch of four mutable- +payload handles. These are five independently repeated `retained-B` samples +after the candidate; before samples are recorded in the baseline subsection. + +| Handle state | Before samples (median; range) | After samples (median; range) | +| --- | --- | --- | +| Ready | 1458352, 1458256, 1459320, 1459048, 1459160 (1459048; 1458256–1459320) | 1458936, 1459048, 1460592, 1460464, 1460192 (1460192; 1458936–1460592) | +| Completed | 731040, 730768, 729352, 729448, 729624 (729624; 729352–731040) | 731312, 730496, 730768, 730544, 731040 (730768; 730496–731312) | +| Discarded | 728560, 730928, 730544, 730496, 730496 (730496; 728560–730928) | 731040, 731088, 730720, 730768, 730608 (730768; 730608–731088) | + +All before/after retained ranges overlap. The median shifts are about 1.1 KB +for ready and completed handles and 272 B for discarded handles, all small +relative to their live sets and within observed run-to-run variation. This is +not evidence of a material retention increase, nor an exact object-size +guarantee. Together with the measured allocation reduction and mutation tests, +it supports retaining the simple ownership transfer while keeping the nested +extra-map copy, details clone, lifecycle cleanup, and caller-owned conversions. +Focused validation passed: `go test -race -mod=readonly ./internal/usecase .`. + ### Validation And Compiled-Rendering Baseline The benchmark-only baseline is commit diff --git a/internal/usecase/prepared_execution.go b/internal/usecase/prepared_execution.go index bdef990..4a2b499 100644 --- a/internal/usecase/prepared_execution.go +++ b/internal/usecase/prepared_execution.go @@ -50,10 +50,12 @@ func (r *Runner) PrepareExecution(ctx context.Context, req domain.RunRequest) (* return nil, err } - executionSnapshot, err := clonePreparedRun(operation.run) + executionSnapshot := operation.run + extraParams, err := jsonvalue.CopyMap(executionSnapshot.EffectiveModelParams.ExtraParams) if err != nil { return nil, fmt.Errorf("%w: failed to copy prepared execution: %v", ErrInvalidRequest, err) } + executionSnapshot.EffectiveModelParams.ExtraParams = extraParams details, err := clonePreparedRun(executionSnapshot) if err != nil { diff --git a/internal/usecase/prepared_execution_test.go b/internal/usecase/prepared_execution_test.go index ab968be..1a8199d 100644 --- a/internal/usecase/prepared_execution_test.go +++ b/internal/usecase/prepared_execution_test.go @@ -631,6 +631,124 @@ func TestRunnerPreparedExecutionOwnershipUseAndDiscard(t *testing.T) { } } +func TestRunnerPreparedExecutionDetachesTransferredSnapshotSources(t *testing.T) { + newExtras := func(value string) map[string]any { + return map[string]any{"provider_options": map[string]any{ + "provider": map[string]any{"settings": map[string]any{"labels": []any{value}}}, + }} + } + extraLabel := func(extraParams map[string]any) string { + return extraParams["provider_options"].(map[string]any)["provider"].(map[string]any)["settings"].(map[string]any)["labels"].([]any)[0].(string) + } + mutateExtra := func(extraParams map[string]any, value string) { + extraParams["provider_options"].(map[string]any)["provider"].(map[string]any)["settings"].(map[string]any)["labels"].([]any)[0] = value + } + + for _, test := range []struct { + name string + requestOverride bool + }{ + {name: "profile target"}, + {name: "request override", requestOverride: true}, + } { + t.Run(test.name, func(t *testing.T) { + profileExtras := newExtras("profile source") + requestExtras := newExtras("request source") + profile := defaultExecutionProfile() + profile.ResolvedTarget.ExtraParams = profileExtras + + schemaDocument := map[string]any{ + "type": "object", + "properties": map[string]any{"value": map[string]any{"type": "string"}}, + } + definition := promptDef(domain.FormatJSON, domain.ValidationJSONSchema, 0) + definition.Validation.SchemaPath = "schema.json" + control := &domain.CacheControl{Type: domain.CacheControlEphemeral, TTL: "1h"} + rendered := &domain.RenderedPrompt{Messages: []domain.RenderedMessage{{ + Role: domain.RoleUser, Content: "captured rendering", CacheControl: control, + }}} + renderer := &fakeRenderer{rendered: rendered} + sourceArtifact := &domain.Artifact{Body: []byte("captured input"), Hash: "captured-input-hash"} + reader := &fakeArtifactReader{artifactsByURI: map[string]*domain.Artifact{"source": sourceArtifact}} + llmClient := &fakeLLM{resp: &domain.GenerateResponse{Content: `{"value":"ok"}`}} + validator := &recordingValidationPreparer{ + plan: &recordingPreparedValidation{schemaDocument: schemaDocument}, + } + runner := NewRunner( + &fakePromptRepo{def: definition}, + &fakeExecutionProfileRepo{profiles: map[string]*domain.ExecutionProfile{"exec": profile}}, + reader, renderer, llmClient, validator, nil, + ) + request := domain.RunRequest{ + PromptID: "p", PromptVersion: "1", ProfileID: "exec", + Inputs: map[string]domain.ArtifactRef{ + "transcript": {Type: domain.ArtifactRefFile, URI: "source"}, + }, + } + wantLabel := "profile source" + if test.requestOverride { + request.Execution = &domain.ExecutionTargetOverride{ExtraParams: requestExtras} + wantLabel = "request source" + } + + prepared, err := runner.PrepareExecution(context.Background(), request) + if err != nil { + t.Fatalf("prepare execution: %v", err) + } + mutateExtra(profileExtras, "mutated profile source") + mutateExtra(requestExtras, "mutated request source") + rendered.Messages[0].Content = "mutated renderer source" + control.TTL = "mutated renderer cache metadata" + schemaDocument["properties"].(map[string]any)["value"].(map[string]any)["type"] = "integer" + sourceArtifact.Hash = "mutated source hash" + + details := prepared.Details() + if details == nil || len(details.Messages) != 1 || details.Messages[0].Content != "captured rendering" || + details.Messages[0].CacheControl == nil || details.Messages[0].CacheControl.TTL != "1h" || + details.InputHashes["transcript"] != "captured-input-hash" || + extraLabel(details.EffectiveModelParams.ExtraParams) != wantLabel || + details.StructuredOutput == nil || details.StructuredOutput.JSONSchema == nil || + details.StructuredOutput.JSONSchema.Schema.(map[string]any)["properties"].(map[string]any)["value"].(map[string]any)["type"] != "string" { + t.Fatalf("prepared details retained a source alias: %#v", details) + } + + result, err := runner.RunPrepared(context.Background(), prepared) + if err != nil || result == nil || !result.Validation.IsValid || result.InputHashes["transcript"] != "captured-input-hash" { + t.Fatalf("run prepared snapshot=(%+v, %v)", result, err) + } + if len(llmClient.lastReq.Prompt.Messages) != 1 || llmClient.lastReq.StructuredOutput == nil || + llmClient.lastReq.StructuredOutput.JSONSchema == nil || + extraLabel(llmClient.lastReq.Target.ExtraParams) != wantLabel || + llmClient.lastReq.Prompt.Messages[0].Content != "captured rendering" || + llmClient.lastReq.Prompt.Messages[0].CacheControl == nil || llmClient.lastReq.Prompt.Messages[0].CacheControl.TTL != "1h" || + llmClient.lastReq.StructuredOutput.JSONSchema.Schema.(map[string]any)["properties"].(map[string]any)["value"].(map[string]any)["type"] != "string" { + t.Fatalf("execution snapshot retained a source alias: %#v", llmClient.lastReq) + } + if completedDetails := prepared.Details(); completedDetails == nil || + len(completedDetails.Messages) != 1 || completedDetails.Messages[0].Content != "captured rendering" || + extraLabel(completedDetails.EffectiveModelParams.ExtraParams) != wantLabel { + t.Fatalf("completed details changed after source mutation: %#v", completedDetails) + } + }) + } +} + +func TestRunnerPrepareExecutionClassifiesExtraCopyFailureAsInvalidRequest(t *testing.T) { + profile := defaultExecutionProfile() + profile.ResolvedTarget.ExtraParams = map[string]any{"unsupported": func() {}} + runner := NewRunner( + &fakePromptRepo{def: promptDef(domain.FormatText, domain.ValidationNone, 0)}, + &fakeExecutionProfileRepo{profiles: map[string]*domain.ExecutionProfile{"exec": profile}}, + defaultArtifactReader(), defaultRenderer(), &fakeLLM{forbid: true}, nil, nil, + ) + prepared, err := runner.PrepareExecution(context.Background(), domain.RunRequest{ + PromptID: "p", ProfileID: "exec", + }) + if prepared != nil || !errors.Is(err, ErrInvalidRequest) { + t.Fatalf("prepared execution copy failure=(%v, %v), want ErrInvalidRequest", prepared, err) + } +} + func TestRunnerPreparedExecutionWithoutValidatorSkipsValidation(t *testing.T) { runner := NewRunner( &fakePromptRepo{def: promptDef(domain.FormatJSON, domain.ValidationJSON, 0)},