eric/promptkit
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Address backend implementation review findings

Browse Source
This commit is contained in:
Eric Rakestraw
2026-07-29 19:08:05 +00:00
parent 359b7313f4
commit f89cb94ed2
17 changed files with 203 additions and 1036 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
internal/backend/registry.go
View File

@@ -11,6 +11,8 @@ import (
"strings"
"gitea.maximumdirect.net/eric/promptkit/internal/domain"
"gitea.maximumdirect.net/eric/promptkit/internal/jsonvalue"
"gitea.maximumdirect.net/eric/promptkit/internal/llm"
)
const (
@@ -75,7 +77,7 @@ func (r *Registry) GetBackend(id string) (domain.Backend, error) {
if !ok {
return domain.Backend{}, fmt.Errorf("%w: %q", ErrBackendNotFound, id)
}
extraParams, err := copyJSONMap(definition.ExtraParams)
extraParams, err := jsonvalue.CopyMap(definition.ExtraParams)
if err != nil {
return domain.Backend{}, fmt.Errorf("copy backend %q: %w", id, err)
}
@@ -83,25 +85,6 @@ func (r *Registry) GetBackend(id string) (domain.Backend, error) {
return definition, nil
}
// IsReservedRequestField reports whether name is owned by the standard
// OpenAI-compatible chat request rather than backend extra parameters.
func IsReservedRequestField(name string) bool {
switch name {
case "model",
"session_id",
"messages",
"temperature",
"max_tokens",
"top_p",
"service_tier",
"reasoning_effort",
"response_format":
return true
default:
return false
}
}
func normalizeBackend(definition domain.Backend) (domain.Backend, error) {
definition.Endpoint = strings.TrimSpace(definition.Endpoint)
if err := validateEndpoint(definition.Endpoint); err != nil {
@@ -126,7 +109,7 @@ func normalizeBackend(definition domain.Backend) (domain.Backend, error) {
if key == "" {
return domain.Backend{}, fmt.Errorf("backend %q extra parameter key must not be empty", definition.ID)
}
if IsReservedRequestField(key) {
if llm.IsReservedOpenAIChatRequestField(key) {
return domain.Backend{}, fmt.Errorf(
"backend %q extra parameter %q collides with a reserved request field",
definition.ID,
@@ -135,7 +118,7 @@ func normalizeBackend(definition domain.Backend) (domain.Backend, error) {
}
}
extraParams, err := copyJSONMap(definition.ExtraParams)
extraParams, err := jsonvalue.CopyMap(definition.ExtraParams)
if err != nil {
return domain.Backend{}, fmt.Errorf("backend %q extra parameters: %w", definition.ID, err)
}

35
internal/backend/registry_test.go
View File

@@ -1,9 +1,7 @@
package backend_test
import (
"encoding/json"
"errors"
"math"
"strings"
"testing"
@@ -188,42 +186,13 @@ func TestNewRegistryValidatesEnvironmentVariableNames(t *testing.T) {
}
}
func TestNewRegistryValidatesExtraParameters(t *testing.T) {
cyclic := map[string]any{}
cyclic["self"] = cyclic
func TestNewRegistryRejectsInvalidAndReservedExtraParameters(t *testing.T) {
tests := []struct {
name string
extraParams map[string]any
}{
{name: "empty top-level key", extraParams: map[string]any{"": true}},
{name: "empty nested key", extraParams: map[string]any{"nested": map[string]int{"": 1}}},
{name: "non-string map key", extraParams: map[string]any{"nested": map[int]string{1: "one"}}},
{name: "unsupported value", extraParams: map[string]any{"value": make(chan int)}},
{name: "cyclic value", extraParams: map[string]any{"value": cyclic}},
{name: "NaN", extraParams: map[string]any{"value": math.NaN()}},
{name: "positive infinity", extraParams: map[string]any{"value": math.Inf(1)}},
{name: "unsafe integer", extraParams: map[string]any{"value": int64(1 << 53)}},
{name: "invalid JSON number", extraParams: map[string]any{"value": json.Number("not-a-number")}},
}
for _, key := range []string{
"model",
"session_id",
"messages",
"temperature",
"max_tokens",
"top_p",
"service_tier",
"reasoning_effort",
"response_format",
} {
tests = append(tests, struct {
name string
extraParams map[string]any
}{
name: "reserved key " + key,
extraParams: map[string]any{key: true},
})
{name: "reserved key", extraParams: map[string]any{"model": "override"}},
}
for _, tc := range tests {

61
internal/backend/json_copy.go → internal/jsonvalue/jsonvalue.go
View File

@@ -1,4 +1,6 @@
package backend
// Package jsonvalue validates and defensively copies JSON-compatible value
// trees used by public configuration and request boundaries.
package jsonvalue
import (
"encoding/json"
@@ -11,16 +13,18 @@ import (
const maxSafeJSONInteger = 1<<53 - 1
type jsonVisit struct {
type visit struct {
typ reflect.Type
ptr uintptr
}
func copyJSONMap(src map[string]any) (map[string]any, error) {
// CopyMap validates and deeply copies an extra-parameter map while preserving
// compatible concrete map, slice, array, scalar, and number types.
func CopyMap(src map[string]any) (map[string]any, error) {
if src == nil {
return nil, nil
}
copied, err := copyJSONValue(reflect.ValueOf(src), "extra_params", make(map[jsonVisit]struct{}))
copied, err := copyValue(reflect.ValueOf(src), "extra_params", make(map[visit]struct{}))
if err != nil {
return nil, err
}
@@ -31,7 +35,7 @@ func copyJSONMap(src map[string]any) (map[string]any, error) {
return out, nil
}
func copyJSONValue(value reflect.Value, path string, seen map[jsonVisit]struct{}) (any, error) {
func copyValue(value reflect.Value, path string, seen map[visit]struct{}) (any, error) {
if !value.IsValid() {
return nil, nil
}
@@ -39,12 +43,15 @@ func copyJSONValue(value reflect.Value, path string, seen map[jsonVisit]struct{}
if value.IsNil() {
return nil, nil
}
return copyJSONValue(value.Elem(), path, seen)
return copyValue(value.Elem(), path, seen)
}
if !value.CanInterface() {
return nil, fmt.Errorf("%s: value cannot be copied", path)
}
if number, ok := value.Interface().(json.Number); ok {
if _, err := json.Marshal(number); err != nil {
return nil, fmt.Errorf("%s: invalid JSON number", path)
}
f, err := strconv.ParseFloat(number.String(), 64)
if err != nil || math.IsNaN(f) || math.IsInf(f, 0) {
return nil, fmt.Errorf("%s: invalid JSON number", path)
@@ -75,28 +82,28 @@ func copyJSONValue(value reflect.Value, path string, seen map[jsonVisit]struct{}
if value.IsNil() {
return nil, nil
}
visit := jsonVisit{typ: value.Type(), ptr: value.Pointer()}
if _, ok := seen[visit]; ok {
current := visit{typ: value.Type(), ptr: value.Pointer()}
if _, ok := seen[current]; ok {
return nil, fmt.Errorf("%s: cyclic value is not supported", path)
}
seen[visit] = struct{}{}
defer delete(seen, visit)
return copyJSONValue(value.Elem(), path, seen)
seen[current] = struct{}{}
defer delete(seen, current)
return copyValue(value.Elem(), path, seen)
case reflect.Map:
return copyJSONMapValue(value, path, seen)
return copyMapValue(value, path, seen)
case reflect.Slice:
if value.IsNil() {
return nil, nil
}
return copyJSONSequenceValue(value, path, seen)
return copySequenceValue(value, path, seen)
case reflect.Array:
return copyJSONSequenceValue(value, path, seen)
return copySequenceValue(value, path, seen)
default:
return nil, fmt.Errorf("%s: unsupported JSON value type %s", path, value.Type())
}
}
func copyJSONMapValue(value reflect.Value, path string, seen map[jsonVisit]struct{}) (any, error) {
func copyMapValue(value reflect.Value, path string, seen map[visit]struct{}) (any, error) {
if value.IsNil() {
return nil, nil
}
@@ -104,12 +111,12 @@ func copyJSONMapValue(value reflect.Value, path string, seen map[jsonVisit]struc
return nil, fmt.Errorf("%s: map key type %s is not supported", path, value.Type().Key())
}
visit := jsonVisit{typ: value.Type(), ptr: value.Pointer()}
if _, ok := seen[visit]; ok {
current := visit{typ: value.Type(), ptr: value.Pointer()}
if _, ok := seen[current]; ok {
return nil, fmt.Errorf("%s: cyclic value is not supported", path)
}
seen[visit] = struct{}{}
defer delete(seen, visit)
seen[current] = struct{}{}
defer delete(seen, current)
keys := value.MapKeys()
sort.Slice(keys, func(i, j int) bool {
@@ -129,7 +136,7 @@ func copyJSONMapValue(value reflect.Value, path string, seen map[jsonVisit]struc
if name == "" {
return nil, fmt.Errorf("%s: map key must not be empty", path)
}
copied, err := copyJSONValue(value.MapIndex(key), path+"."+name, seen)
copied, err := copyValue(value.MapIndex(key), path+"."+name, seen)
if err != nil {
return nil, err
}
@@ -164,22 +171,22 @@ func copyJSONMapValue(value reflect.Value, path string, seen map[jsonVisit]struc
return out, nil
}
func copyJSONSequenceValue(value reflect.Value, path string, seen map[jsonVisit]struct{}) (any, error) {
var visit jsonVisit
func copySequenceValue(value reflect.Value, path string, seen map[visit]struct{}) (any, error) {
var current visit
if value.Kind() == reflect.Slice {
visit = jsonVisit{typ: value.Type(), ptr: value.Pointer()}
if _, ok := seen[visit]; ok {
current = visit{typ: value.Type(), ptr: value.Pointer()}
if _, ok := seen[current]; ok {
return nil, fmt.Errorf("%s: cyclic value is not supported", path)
}
seen[visit] = struct{}{}
defer delete(seen, visit)
seen[current] = struct{}{}
defer delete(seen, current)
}
values := make([]any, value.Len())
preserveType := true
elementType := value.Type().Elem()
for i := 0; i < value.Len(); i++ {
copied, err := copyJSONValue(value.Index(i), fmt.Sprintf("%s[%d]", path, i), seen)
copied, err := copyValue(value.Index(i), fmt.Sprintf("%s[%d]", path, i), seen)
if err != nil {
return nil, err
}

97
internal/jsonvalue/jsonvalue_test.go Normal file
View File

@@ -0,0 +1,97 @@
package jsonvalue_test
import (
"encoding/json"
"math"
"reflect"
"testing"
"gitea.maximumdirect.net/eric/promptkit/internal/jsonvalue"
)
func TestCopyMapPreservesTypesAndIsolatesMutations(t *testing.T) {
nested := map[string]int{"limit": 2}
sequence := []string{"one", "two"}
input := map[string]any{
"count": int64(7),
"number": json.Number("-1.25e+2"),
"nested": nested,
"sequence": sequence,
}
copied, err := jsonvalue.CopyMap(input)
if err != nil {
t.Fatalf("copy map: %v", err)
}
nested["limit"] = 99
sequence[0] = "changed"
input["added"] = true
if got, ok := copied["count"].(int64); !ok || got != 7 {
t.Fatalf("integer type or value changed: %#v", copied["count"])
}
if got, ok := copied["number"].(json.Number); !ok || got != "-1.25e+2" {
t.Fatalf("JSON number type or value changed: %#v", copied["number"])
}
if got := copied["nested"].(map[string]int)["limit"]; got != 2 {
t.Fatalf("nested map was not isolated: %d", got)
}
if got := copied["sequence"].([]string)[0]; got != "one" {
t.Fatalf("sequence was not isolated: %q", got)
}
if _, ok := copied["added"]; ok {
t.Fatalf("top-level map was not isolated: %#v", copied)
}
}
func TestCopyMapRejectsInvalidValues(t *testing.T) {
cyclicMap := map[string]any{}
cyclicMap["self"] = cyclicMap
cyclicSlice := []any{nil}
cyclicSlice[0] = cyclicSlice
tests := []struct {
name string
value any
}{
{name: "empty nested key", value: map[string]int{"": 1}},
{name: "non-string map key", value: map[int]string{1: "one"}},
{name: "unsupported value", value: make(chan int)},
{name: "cyclic map", value: cyclicMap},
{name: "cyclic slice", value: cyclicSlice},
{name: "NaN", value: math.NaN()},
{name: "positive infinity", value: math.Inf(1)},
{name: "unsafe signed integer", value: int64(1 << 53)},
{name: "unsafe unsigned integer", value: uint64(1 << 53)},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
if _, err := jsonvalue.CopyMap(map[string]any{"value": tc.value}); err == nil {
t.Fatal("expected validation error")
}
})
}
}
func TestCopyMapValidatesJSONNumberSyntaxAndRange(t *testing.T) {
for _, number := range []json.Number{"0", "-1", "1.25", "-1.25e+2"} {
t.Run("valid "+number.String(), func(t *testing.T) {
got, err := jsonvalue.CopyMap(map[string]any{"value": number})
if err != nil {
t.Fatalf("copy valid JSON number: %v", err)
}
if !reflect.DeepEqual(got["value"], number) {
t.Fatalf("JSON number changed: got %#v want %#v", got["value"], number)
}
})
}
for _, number := range []json.Number{"", "01", "+1", "1.", ".1", "1e9999", "not-a-number"} {
t.Run("invalid "+number.String(), func(t *testing.T) {
if _, err := jsonvalue.CopyMap(map[string]any{"value": number}); err == nil {
t.Fatal("expected invalid JSON number error")
}
})
}
}

22
internal/llm/openai_compatible_client.go
View File

@@ -14,7 +14,6 @@ import (
"time"
"unicode/utf8"
"gitea.maximumdirect.net/eric/promptkit/internal/backend"
"gitea.maximumdirect.net/eric/promptkit/internal/defaults"
"gitea.maximumdirect.net/eric/promptkit/internal/domain"
)
@@ -263,7 +262,7 @@ func openAIChatRequestPayload(req openAIChatRequest) (map[string]any, error) {
if key == "" {
return nil, errors.New("extra_params key must not be empty")
}
if backend.IsReservedRequestField(key) {
if IsReservedOpenAIChatRequestField(key) {
return nil, fmt.Errorf("extra_params key %q collides with reserved request field", key)
}
if _, err := json.Marshal(value); err != nil {
@@ -275,6 +274,25 @@ func openAIChatRequestPayload(req openAIChatRequest) (map[string]any, error) {
return out, nil
}
// IsReservedOpenAIChatRequestField reports whether name is owned by the
// standard OpenAI-compatible chat request rather than extra parameters.
func IsReservedOpenAIChatRequestField(name string) bool {
switch name {
case "model",
"session_id",
"messages",
"temperature",
"max_tokens",
"top_p",
"service_tier",
"reasoning_effort",
"response_format":
return true
default:
return false
}
}
type openAIChatRequestMessage struct {
Role string `json:"role"`
Content any `json:"content"`

2
internal/usecase/runner.go
View File

@@ -365,7 +365,7 @@ func mergeExecutionTarget(base domain.ExecutionTarget, override domain.Execution
if strings.TrimSpace(override.BackendID) != "" {
out.BackendID = override.BackendID
}
if override.Endpoint != "" {
if strings.TrimSpace(override.Endpoint) != "" {
out.Endpoint = override.Endpoint
}
if override.Model != "" {