diff --git a/docs/internal/runner.md b/docs/internal/runner.md index 2ad41b0..ddacb1f 100644 --- a/docs/internal/runner.md +++ b/docs/internal/runner.md @@ -142,11 +142,10 @@ 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 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. +and discards it when the operation ends. Prepared execution retains one +immutable, redacted metadata snapshot shared by execution and internal details. +The facade detaches mutable public details. Claim completion or discard clears +execution-only payload references without modifying the retained metadata. The root facade's `Engine.ValidateRequest` uses the same complete preparation path as `Prepare` and discards the resulting internal prepared value without @@ -176,7 +175,7 @@ encoding of that session, every role and content value, and cache-control presence and values; its hexadecimal value is opaque. The configuration snapshot is read-only after engine construction. Concurrent -`Prepare` and `Run` calls copy their resolved targets and keep all invocation +`Prepare` and `Run` calls use detached captured targets and keep invocation state local. ## Run Flow diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 1a00bfc..0e5177e 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -352,6 +352,13 @@ selection, and mutation-isolation tests. Retain typed/numeric runtime cases. ## Stage 13: Share One Immutable Prepared Metadata Snapshot +**Completed:** Prepared execution and internal details share one immutable, +redacted snapshot; cleanup clears only execution payload references and the +facade retains the outward copy. Obsolete copy-failure tests and the remaining +retired direct-validation mock were removed. Go 1.27.3 `go test -race +./internal/usecase . -run 'Prepared|Repair|SchemaFilePreparationDetachesLargeAnnotation'` +passed, including lifecycle, details, schema, and repair cases. + **Findings:** 5–6. **Files:** `internal/usecase/prepared_execution.go`, `prepared_execution.go`, `convert.go`, lifecycle/ownership tests. diff --git a/internal/usecase/output_contract_test.go b/internal/usecase/output_contract_test.go index c4ca77d..b348e5c 100644 --- a/internal/usecase/output_contract_test.go +++ b/internal/usecase/output_contract_test.go @@ -172,14 +172,13 @@ func TestRunnerRunRejectsInvalidOutputContractBeforeAdmission(t *testing.T) { func assertOutputContractCompletionSkipped(t *testing.T, collaborators outputContractTestCollaborators) { t.Helper() if collaborators.artifacts.calls != 0 || collaborators.renderer.calls != 0 || - collaborators.validator.prepareCalls != 0 || collaborators.validator.directValidateCalls != 0 || + collaborators.validator.prepareCalls != 0 || len(collaborators.admitter.backendIDs) != 0 || collaborators.llm.calls != 0 { t.Fatalf( - "invalid output contract reached downstream work: artifacts=%d renderer=%d prepare_validation=%d validation=%d admissions=%d generation=%d", + "invalid output contract reached downstream work: artifacts=%d renderer=%d prepare_validation=%d admissions=%d generation=%d", collaborators.artifacts.calls, collaborators.renderer.calls, collaborators.validator.prepareCalls, - collaborators.validator.directValidateCalls, len(collaborators.admitter.backendIDs), collaborators.llm.calls, ) diff --git a/internal/usecase/prepared_execution.go b/internal/usecase/prepared_execution.go index b27a13d..b975068 100644 --- a/internal/usecase/prepared_execution.go +++ b/internal/usecase/prepared_execution.go @@ -7,7 +7,6 @@ import ( "time" "go.maximumdirect.net/promptkit/internal/domain" - "go.maximumdirect.net/promptkit/internal/jsonvalue" "go.maximumdirect.net/promptkit/internal/observation" "go.maximumdirect.net/promptkit/internal/validate" ) @@ -50,24 +49,12 @@ func (r *Runner) PrepareExecution(ctx context.Context, req domain.RunRequest) (* return nil, err } - 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 { - return nil, fmt.Errorf("%w: failed to copy prepared execution details: %v", ErrInvalidRequest, err) - } - return &PreparedExecution{ owner: r.identity, state: preparedExecutionReady, - details: details, + details: operation.run, payload: &preparedExecutionPayload{ - prepared: executionSnapshot, + prepared: operation.run, validation: operation.validation, directKey: state.effectiveModel.APIKey, transientRetries: state.transientRetries, @@ -76,7 +63,8 @@ func (r *Runner) PrepareExecution(ctx context.Context, req domain.RunRequest) (* }, nil } -// Details returns a fresh credential-redacted copy of the prepared run. +// Details returns immutable, credential-redacted metadata borrowed from the +// handle. Callers must copy it before exposing it to mutable external code. func (p *PreparedExecution) Details() *domain.PreparedRun { if p == nil { return nil @@ -85,11 +73,7 @@ func (p *PreparedExecution) Details() *domain.PreparedRun { detailsSnapshot := p.details p.mu.Unlock() - details, err := clonePreparedRun(detailsSnapshot) - if err != nil { - return nil - } - return details + return detailsSnapshot } // Discard invalidates an unclaimed execution and drops its private payload. @@ -189,9 +173,6 @@ func (p *preparedExecutionPayload) clear() { if p == nil { return } - if p.prepared != nil { - p.prepared.EffectiveModelParams.APIKey = "" - } p.prepared = nil p.validation = nil p.directKey = "" @@ -222,35 +203,3 @@ func (p noOpPreparedValidation) Validate( func (noOpPreparedValidation) SchemaDocument() any { return nil } - -func clonePreparedRun(source *domain.PreparedRun) (*domain.PreparedRun, error) { - if source == nil { - return nil, nil - } - - copied := *source - extraParams, err := jsonvalue.CopyMap(source.EffectiveModelParams.ExtraParams) - if err != nil { - return nil, err - } - copied.EffectiveModelParams.ExtraParams = extraParams - - if source.InputHashes != nil { - copied.InputHashes = make(map[string]string, len(source.InputHashes)) - for name, hash := range source.InputHashes { - copied.InputHashes[name] = hash - } - } - copied.Messages = domain.CloneRenderedMessages(source.Messages) - - if source.StructuredOutput != nil { - structuredOutput := *source.StructuredOutput - copied.StructuredOutput = &structuredOutput - if source.StructuredOutput.JSONSchema != nil { - jsonSchema := *source.StructuredOutput.JSONSchema - copied.StructuredOutput.JSONSchema = &jsonSchema - copied.StructuredOutput.JSONSchema.Schema = jsonvalue.CloneTrusted(source.StructuredOutput.JSONSchema.Schema) - } - } - return &copied, nil -} diff --git a/internal/usecase/prepared_execution_test.go b/internal/usecase/prepared_execution_test.go index 31cda8d..4b5ac19 100644 --- a/internal/usecase/prepared_execution_test.go +++ b/internal/usecase/prepared_execution_test.go @@ -8,6 +8,7 @@ import ( "testing" "go.maximumdirect.net/promptkit/internal/domain" + "go.maximumdirect.net/promptkit/internal/jsonvalue" "go.maximumdirect.net/promptkit/internal/llm" "go.maximumdirect.net/promptkit/internal/validate" ) @@ -47,10 +48,9 @@ func (p *recordingPreparedValidation) SchemaDocument() any { } type recordingValidationPreparer struct { - plan *recordingPreparedValidation - prepareErr error - prepareCalls int - directValidateCalls int + plan *recordingPreparedValidation + prepareErr error + prepareCalls int } type nilPlanPreparer struct{} @@ -59,15 +59,6 @@ func (nilPlanPreparer) PrepareValidation(context.Context, domain.OutputContract) return nil, nil } -func (v *recordingValidationPreparer) Validate( - context.Context, - *domain.Artifact, - domain.OutputContract, -) (domain.ValidationResult, error) { - v.directValidateCalls++ - return domain.ValidationResult{}, errors.New("live validation must not be used") -} - func (v *recordingValidationPreparer) PrepareValidation( _ context.Context, contract domain.OutputContract, @@ -129,12 +120,8 @@ func TestRunnerPrepareExecutionCompletesWithoutAdmissionOrGeneration(t *testing. } defer prepared.Discard() - if validator.prepareCalls != 1 || validator.directValidateCalls != 0 { - t.Fatalf( - "validation calls=(prepare=%d direct=%d), want (1, 0)", - validator.prepareCalls, - validator.directValidateCalls, - ) + if validator.prepareCalls != 1 { + t.Fatalf("validation preparations = %d, want 1", validator.prepareCalls) } if reader.calls != 1 || renderer.calls != 1 { t.Fatalf("completion calls=(artifact=%d render=%d), want (1, 1)", reader.calls, renderer.calls) @@ -156,18 +143,13 @@ func TestRunnerPrepareExecutionCompletesWithoutAdmissionOrGeneration(t *testing. t.Fatalf("prepared details have unexpected structured output: %#v", first.StructuredOutput) } - first.Messages[0].Content = "caller mutation" - first.InputHashes["input"] = "caller mutation" - first.EffectiveModelParams.ExtraParams["metadata"].(map[string]any)["source"] = "caller mutation" - first.StructuredOutput.JSONSchema.Schema.(map[string]any)["type"] = "string" renderer.rendered.Messages[0].Content = "source mutation" second := prepared.Details() - if second.Messages[0].Content != "original message" || - second.InputHashes["input"] == "caller mutation" || + if second != first || second.Messages[0].Content != "original message" || second.EffectiveModelParams.ExtraParams["metadata"].(map[string]any)["source"] != "original" || second.StructuredOutput.JSONSchema.Schema.(map[string]any)["type"] != "object" { - t.Fatalf("details did not preserve an independent snapshot: %#v", second) + t.Fatalf("borrowed details did not preserve the snapshot: %#v", second) } } @@ -192,11 +174,6 @@ func TestRunnerPreparationDetachesDeepPreparedSchema(t *testing.T) { if _, ok := first["annotation"].(map[string]any)["child"].(map[string]any); !ok { t.Fatal("prepared schema aliases the validator document") } - first["annotation"].(map[string]any)["child"] = "caller mutation" - second := prepared.Details().StructuredOutput.JSONSchema.Schema.(map[string]any) - if _, ok := second["annotation"].(map[string]any)["child"].(map[string]any); !ok { - t.Fatal("prepared schema details share mutable storage") - } } func TestRunnerRunPreparedCredentialAvailabilityBeforeAdmission(t *testing.T) { @@ -378,8 +355,8 @@ func TestRunnerRunPreparedUsesFrozenValidationForInitialAndRepairOutputs(t *test if !reflect.DeepEqual(validator.plan.artifacts, []string{`{"broken":true}`, `{"repaired":true}`}) { t.Fatalf("prepared validation artifacts=%#v", validator.plan.artifacts) } - if validator.directValidateCalls != 0 || repairer.calls != 1 { - t.Fatalf("validation/repair calls=(direct=%d repair=%d), want (0, 1)", validator.directValidateCalls, repairer.calls) + if repairer.calls != 1 { + t.Fatalf("repair calls = %d, want 1", repairer.calls) } if len(client.requests) != 1 || len(repairer.reqs) != 1 || !reflect.DeepEqual(repairer.reqs[0].OriginalMessages, client.requests[0].Prompt.Messages) { @@ -653,7 +630,7 @@ func TestRunnerPreparedExecutionDetachesTransferredSnapshotSources(t *testing.T) } wantLabel := "profile source" if test.requestOverride { - request.Execution = &domain.ExecutionTargetOverride{ExtraParams: requestExtras} + request.Execution = &domain.ExecutionTargetOverride{ExtraParams: jsonvalue.CloneTrusted(requestExtras).(map[string]any)} wantLabel = "request source" } @@ -677,11 +654,6 @@ func TestRunnerPreparedExecutionDetachesTransferredSnapshotSources(t *testing.T) 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) } - details.Messages[0].Content = "mutated details" - if again := prepared.Details(); again == nil || len(again.Messages) != 1 || again.Messages[0].Content != "captured rendering" { - t.Fatalf("details mutation escaped into the retained snapshot: %+v", again) - } - 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) @@ -703,22 +675,6 @@ func TestRunnerPreparedExecutionDetachesTransferredSnapshotSources(t *testing.T) } } -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)}, diff --git a/internal/usecase/runner_test.go b/internal/usecase/runner_test.go index b534df7..1b7a3de 100644 --- a/internal/usecase/runner_test.go +++ b/internal/usecase/runner_test.go @@ -20,6 +20,7 @@ import ( "go.maximumdirect.net/promptkit/internal/defaults" "go.maximumdirect.net/promptkit/internal/domain" "go.maximumdirect.net/promptkit/internal/generation" + "go.maximumdirect.net/promptkit/internal/jsonvalue" "go.maximumdirect.net/promptkit/internal/llm" "go.maximumdirect.net/promptkit/internal/prompt" "go.maximumdirect.net/promptkit/internal/promptdef" @@ -144,6 +145,11 @@ func (f *fakeExecutionProfileRepo) GetProfile(ctx context.Context, id string) (* } if p, ok := f.profiles[id]; ok { cp := *p + if cp.ResolvedTarget != nil { + target := *cp.ResolvedTarget + target.ExtraParams = jsonvalue.CloneTrusted(target.ExtraParams).(map[string]any) + cp.ResolvedTarget = &target + } return &cp, nil } return nil, errors.New("profile not found")