Isolate parsed provider parameters
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
# Consolidated audit catalog
|
||||
|
||||
Status: **26 distinct open findings: four P2 and 22 P3**, consolidated from 30
|
||||
Status: **25 distinct open findings: three P2 and 22 P3**, consolidated from 30
|
||||
numbered entries in seven audit documents. Reviewed audit state:
|
||||
`db732e3a1de55c844f77870e1c6db683c2685d5d`.
|
||||
|
||||
@@ -58,6 +58,11 @@ Also noted in the [documentation cross-references](13-documentation.md#previousl
|
||||
|
||||
## F02 — P2: CLI provider parameters share mutable state across requests
|
||||
|
||||
**Complete.** CLI JSON is copied at captured binding application and each
|
||||
aggregate seed boundary. Both regressions failed before the fix and focused
|
||||
aligned-workspace tests passed, including nested objects and arrays, nil and
|
||||
empty values, and numeric types.
|
||||
|
||||
ckprompt assigns parsed `ExtraParams` maps directly to request destinations, and
|
||||
smoke seeds share those nested maps. Mutation by one callback can change later
|
||||
smoke requests or another destination receiving the same captured bindings.
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
# Audit remediation implementation plan
|
||||
|
||||
Status: in progress; Stage 1 complete.
|
||||
Status: in progress; Stages 1–2 complete.
|
||||
|
||||
Baseline: `d492c7747175c01a53261206c2b4356e53453621`. Before this sprint,
|
||||
intervening changes recorded decisions and retired the completed older
|
||||
@@ -135,6 +135,13 @@ successful incomplete aggregate or lose cancellation beside another cause.
|
||||
|
||||
## Stage 2 — Isolate parsed provider-parameter JSON
|
||||
|
||||
**Complete.** Both captured binding replay and aggregate request seed creation
|
||||
copy nested JSON objects and arrays. The two new boundary regressions failed
|
||||
against the original implementation; focused request-flag and aggregate tests
|
||||
passed in the aligned workspace with Go 1.27.0, `GOTOOLCHAIN=local`,
|
||||
`GOPROXY=off`, and `-mod=readonly`. Nil, empty, integer and decimal values are
|
||||
covered.
|
||||
|
||||
Findings: F02. Sources: ckprompt `request_flags.go` and `smoke_command.go`.
|
||||
|
||||
1. Add one private recursive copier for the known CLI JSON object/array
|
||||
|
||||
@@ -85,6 +85,7 @@ func (f ExecutionFlags[I]) bindings() []requestBinding[I] {
|
||||
}
|
||||
|
||||
// NewExecutionFlags binds execution controls to a field of the caller's input.
|
||||
// Parsed provider parameters are copied when the binding is applied.
|
||||
func NewExecutionFlags[I any](destination func(*I) *ExecutionInput) ExecutionFlags[I] {
|
||||
target := func(input *I) *promptkit.ExecutionTargetOverride {
|
||||
value := destination(input)
|
||||
@@ -104,7 +105,36 @@ func NewExecutionFlags[I any](destination func(*I) *ExecutionInput) ExecutionFla
|
||||
ReasoningEffort: executionFlag(requestFlags.ReasoningEffort.meta, parseDefinitionString, func(i *I, v string) { target(i).ReasoningEffort = &v }),
|
||||
APIKeyEnv: executionFlag(requestFlags.APIKeyEnv.meta, parseDefinitionString, func(i *I, v string) { target(i).APIKeyEnv = v }),
|
||||
TransientRetries: executionFlag(requestFlags.TransientRetries.meta, strconv.Atoi, func(i *I, v int) { destination(i).TransientRetries = &v }),
|
||||
ExtraParams: executionFlag(requestFlags.ExtraParams.meta, parseExtraParams, func(i *I, v map[string]any) { target(i).ExtraParams = v }),
|
||||
ExtraParams: executionFlag(requestFlags.ExtraParams.meta, parseExtraParams, func(i *I, v map[string]any) { target(i).ExtraParams = copyProviderParams(v) }),
|
||||
}
|
||||
}
|
||||
|
||||
func copyProviderParams(params map[string]any) map[string]any {
|
||||
if params == nil {
|
||||
return nil
|
||||
}
|
||||
result := make(map[string]any, len(params))
|
||||
for key, value := range params {
|
||||
result[key] = copyProviderValue(value)
|
||||
}
|
||||
return result
|
||||
}
|
||||
|
||||
func copyProviderValue(value any) any {
|
||||
switch value := value.(type) {
|
||||
case map[string]any:
|
||||
return copyProviderParams(value)
|
||||
case []any:
|
||||
if value == nil {
|
||||
return []any(nil)
|
||||
}
|
||||
result := make([]any, len(value))
|
||||
for i, element := range value {
|
||||
result[i] = copyProviderValue(element)
|
||||
}
|
||||
return result
|
||||
default:
|
||||
return value
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -76,3 +76,33 @@ func TestValidationForwardsExecutionControls(t *testing.T) {
|
||||
t.Fatalf("result/request = %#v %#v", result, received)
|
||||
}
|
||||
}
|
||||
|
||||
func TestExecutionExtraParamsCapturedApplicationsAreIndependent(t *testing.T) {
|
||||
type target struct{ execution ckprompt.ExecutionInput }
|
||||
type outer struct{ captured commandkit.FlagValues[target] }
|
||||
flag := ckprompt.NewExecutionFlags(func(i *target) *ckprompt.ExecutionInput { return &i.execution }).ExtraParams
|
||||
captured := commandkit.CaptureFlags([]commandkit.Flag[target]{flag}, func(i *outer) *commandkit.FlagValues[target] { return &i.captured })
|
||||
command := commandkit.NewCommand(commandkit.CommandSpec[outer]{Name: "run", Flags: captured, Run: func(_ commandkit.Invocation, input outer) commandkit.Outcome {
|
||||
var first, second target
|
||||
if err := input.captured.ApplyTo(&first, flag); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
first.execution.Execution.ExtraParams["nested"].(map[string]any)["array"].([]any)[0].(map[string]any)["number"] = int64(9)
|
||||
if err := input.captured.ApplySuppliedTo(&second, flag); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
value := second.execution.Execution.ExtraParams["nested"].(map[string]any)["array"].([]any)[0].(map[string]any)["number"]
|
||||
if value != int64(1) {
|
||||
t.Errorf("second value = %#v", value)
|
||||
}
|
||||
return commandkit.Success()
|
||||
}})
|
||||
app, err := commandkit.New(commandkit.Identity{Name: "app"}, command)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
result := app.Execute(context.Background(), []string{"run", `--llm-extra-params={"nested":{"array":[{"number":1}]}}`}, commandkit.Streams{Out: io.Discard, Err: io.Discard})
|
||||
if result.Kind != commandkit.OutcomeSuccess {
|
||||
t.Fatal(result)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -219,3 +219,33 @@ func TestSmokeAggregateCancellationRetainsPartialResults(t *testing.T) {
|
||||
t.Fatalf("partial aggregate entries = %#v", document.Tests)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSmokeAggregateIsolatesProviderParameters(t *testing.T) {
|
||||
calls := 0
|
||||
app := newPromptTestApp(t, Command(Suite[struct{}, struct{}]{
|
||||
SmokeTest: func(_ context.Context, request SmokeRequest, _, _ struct{}) (commandkit.Acquired[*promptkit.SmokeTestReport], error) {
|
||||
calls++
|
||||
params := request.Request.Execution.ExtraParams
|
||||
value := params["nested"].(map[string]any)["array"].([]any)[0].(map[string]any)
|
||||
if value["number"] != int64(1) {
|
||||
t.Errorf("call %d: %#v", calls, value)
|
||||
}
|
||||
value["number"] = int64(9)
|
||||
return commandkit.Acquired[*promptkit.SmokeTestReport]{Value: &promptkit.SmokeTestReport{Kind: request.Request.Kind}}, nil
|
||||
},
|
||||
}))
|
||||
outcome := app.Execute(context.Background(), []string{"promptkit", "test", "all", `--llm-extra-params={"nested":{"array":[{"number":1}]}}`}, commandkit.Streams{Out: io.Discard, Err: io.Discard})
|
||||
if outcome.Kind != commandkit.OutcomeSuccess || calls != 4 {
|
||||
t.Fatalf("outcome=%#v calls=%d", outcome, calls)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCopyProviderParamsPreservesEmptyAndNilValues(t *testing.T) {
|
||||
if copyProviderParams(nil) != nil {
|
||||
t.Fatal("nil object became non-nil")
|
||||
}
|
||||
copied := copyProviderParams(map[string]any{"emptyObject": map[string]any{}, "nilObject": map[string]any(nil), "emptyArray": []any{}, "nilArray": []any(nil), "null": nil, "decimal": float64(1.5)})
|
||||
if copied["emptyObject"].(map[string]any) == nil || copied["nilObject"].(map[string]any) != nil || copied["emptyArray"].([]any) == nil || copied["nilArray"].([]any) != nil || copied["null"] != nil || copied["decimal"] != float64(1.5) {
|
||||
t.Fatalf("copied = %#v", copied)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -232,6 +232,7 @@ func smokeRequestForKind(seed promptkit.SmokeTestRequest, kind promptkit.SmokeTe
|
||||
request.Kind = kind
|
||||
if seed.Execution != nil {
|
||||
execution := *seed.Execution
|
||||
execution.ExtraParams = copyProviderParams(seed.Execution.ExtraParams)
|
||||
execution.Temperature = copyFloat64(seed.Execution.Temperature)
|
||||
execution.MaxTokens = copyInt(seed.Execution.MaxTokens)
|
||||
execution.TopP = copyFloat64(seed.Execution.TopP)
|
||||
|
||||
Reference in New Issue
Block a user