From 78bff9139533fcdf599c4ae1ca7d68d86515d6d7 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Fri, 9 Oct 2026 00:27:24 +0000 Subject: [PATCH] Encode generation payloads once on success --- docs/integrations/openai-compatible-chat.md | 5 ++- docs/internal/llm.md | 8 +++-- docs/roadmap/implementation.md | 35 ++++++++++++++++++++- internal/llm/openai_compatible_client.go | 8 +++-- 4 files changed, 47 insertions(+), 9 deletions(-) diff --git a/docs/integrations/openai-compatible-chat.md b/docs/integrations/openai-compatible-chat.md index 90106fa..d6777df 100644 --- a/docs/integrations/openai-compatible-chat.md +++ b/docs/integrations/openai-compatible-chat.md @@ -78,9 +78,8 @@ The client conditionally includes: The engine resolves backend, profile, and request extra-parameter maps by whole-map replacement rather than key merging. The resulting effective map is -then merged directly into the top-level body after JSON serialization is -verified. Empty keys and collisions with these reserved fields are rejected -before any provider call: +merged into the top-level body. Empty keys, values that cannot be encoded, and +collisions with these reserved fields are rejected before any provider call: - `model` - `session_id` diff --git a/docs/internal/llm.md b/docs/internal/llm.md index 2277873..d53c0fc 100644 --- a/docs/internal/llm.md +++ b/docs/internal/llm.md @@ -34,9 +34,13 @@ resolved request target may supply the endpoint. Generation preparation then: 1. validates shared execution-setting invariants and the final selected base endpoint; 2. maps the internal request into the OpenAI-compatible chat payload; -3. validates and merges extra parameters; +3. checks extra-parameter keys against empty and reserved names, then merges + their values into the payload; 4. composes `/chat/completions` through parsed URL path operations; -5. resolves authentication and freezes the URL, body, and optional credential. +5. encodes the complete body once on success. If encoding fails, extra values + are checked individually to retain invalid-request classification; otherwise + the complete-request failure remains a request failure; +6. resolves authentication and freezes the URL, body, and optional credential. Each execution of that prepared request creates a fresh HTTP request, body reader, and attempt timeout context. It closes the response body and returns diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index d06ec62..a398d0b 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -2,7 +2,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. -Stages 1–5 are complete; Stages 6–14 remain pending. Update status and evidence +Stages 1–6 are complete; Stages 7–14 remain pending. Update status and evidence as each work unit lands. The completed inspection plan is preserved at `481e2d4:docs/roadmap/implementation.md` in Git history. @@ -252,6 +252,11 @@ tests, [integration contract](../integrations/openai-compatible-chat.md), and ## Stage 6 — Encode Valid Extra Parameters Only In The Complete Request +**Status: Complete (2026-10-09).** Successful request preparation now encodes +the assembled payload once. Extra-parameter classification remains covered on +the failure path, including precedence when the schema also cannot be encoded. +The Stage 5 benchmark comparison is recorded below. + **Outcome:** Successful request preparation does not serialize extras twice. **Work:** @@ -786,6 +791,34 @@ these measurements are the comparison reference for the following request encoding change, not a provider-latency estimate. Focused verification passed: `go test -mod=readonly ./internal/llm`. +### Request-Preparation Copy Comparison + +The before results above use the Stage 5 benchmark fixture committed at +`3f15715`; the after results below use the unchanged fixture after the client +change in the working tree based on `c63ca53`. Both measurements use the same +offline environment and command recorded in the baseline subsection. `ns/op` +values list all five samples, followed by median and range. Allocation figures +are `B/op; allocs/op`. + +| Preparation case | Before `ns/op` (median; range) | Before allocations | After `ns/op` (median; range) | After allocations | +| --- | --- | --- | --- | --- | +| No extras | 2771, 2873, 2872, 2907, 2885 (2873; 2771–2907) | 1632; 23 | 2849, 2841, 2872, 2746, 2902 (2849; 2746–2902) | 1632; 23 | +| Small extras | 4216, 4255, 4440, 4090, 4245 (4245; 4090–4440) | 1928; 32 | 3772, 3865, 3888, 3804, 3931 (3865; 3772–3931) | 1904; 29 | +| 1 MiB string extra | 3108655, 3046197, 3165484, 3115831, 3074278 (3108655; 3046197–3165484) | 2147898–2213942; 26–27 | 1524191, 1516315, 1537340, 1565157, 1538894 (1537340; 1516315–1565157) | 1140425–1197988; 25 | +| Nested JSON extras | 5933, 5803, 5675, 5490, 5611 (5675; 5490–5933) | 2304; 40 | 4440, 4446, 4389, 4302, 4481 (4440; 4302–4481) | 2016; 32 | +| Structured output | 6421, 6018, 6479, 6153, 6712 (6421; 6018–6712) | 2632–2633; 47 | 6253, 6137, 6267, 6315, 6232 (6253; 6137–6315) | 2632–2633; 47 | + +The 1 MiB extra reduced median preparation time by about half and cumulative +allocation by approximately 1 MB. Small and nested extras also used fewer +bytes and allocations; no-extras and structured-output allocations were +unchanged. Timing ranges are machine-local evidence, not a consumer guarantee. +Invalid nested extras retain `ErrInvalidRequest` and key context before +transport. With an unencodable schema, serializable extras retain +`ErrRequestFailed`; invalid extras retain `ErrInvalidRequest`. Prepared retry +attempts continue to use the frozen body. Verification passed: +`go test -mod=readonly ./internal/llm ./internal/generation`, and the benchmark +family above was rerun without the race detector. + ## Open Questions No consumer-policy decisions remain open. File reservation and encoding error diff --git a/internal/llm/openai_compatible_client.go b/internal/llm/openai_compatible_client.go index 7d25493..e32aba3 100644 --- a/internal/llm/openai_compatible_client.go +++ b/internal/llm/openai_compatible_client.go @@ -152,6 +152,11 @@ func (c *OpenAICompatibleClient) PrepareGeneration(req domain.GenerateRequest) ( payload, err := json.Marshal(wirePayload) if err != nil { + for key, value := range req.Target.ExtraParams { + if _, extraErr := json.Marshal(value); extraErr != nil { + return nil, fmt.Errorf("%w: extra_params.%s must be JSON-serializable: %v", ErrInvalidRequest, key, extraErr) + } + } return nil, fmt.Errorf("%w: failed to encode request: %v", ErrRequestFailed, err) } apiKey := strings.TrimSpace(req.Target.APIKey) @@ -446,9 +451,6 @@ func openAIChatRequestPayload(req openAIChatRequest) (map[string]any, error) { if IsReservedOpenAIChatRequestField(key) { return nil, fmt.Errorf("extra_params key %q collides with reserved request field", key) } - if _, err := json.Marshal(value); err != nil { - return nil, fmt.Errorf("extra_params.%s must be JSON-serializable: %w", key, err) - } out[key] = value }