Keep session IDs on repair requests and retire completed plans
This commit is contained in:
@@ -77,15 +77,16 @@ an inability to perform validation is an operational error.
|
|||||||
|
|
||||||
When an internal repairer is present, a JSON or JSON Schema content failure can
|
When an internal repairer is present, a JSON or JSON Schema content failure can
|
||||||
trigger bounded repair attempts. Repair receives the effective execution
|
trigger bounded repair attempts. Repair receives the effective execution
|
||||||
target, validation errors, prior output, and structured-output specification.
|
target and session ID, validation errors, prior output, and structured-output
|
||||||
This capability remains internal and is not a public option.
|
specification. This capability remains internal and is not a public option.
|
||||||
|
|
||||||
A successful result includes the output artifact and raw output, validation
|
A successful result includes the output artifact and raw output, validation
|
||||||
state, effective session ID, prompt and rendered-prompt hashes, selected
|
state, effective session ID, prompt and rendered-prompt hashes, selected
|
||||||
profile and backend, effective settings, input hashes, token usage, a generated
|
profile and backend, effective settings, input hashes, token usage, a generated
|
||||||
run identifier, and UTC timing. The same effective session reaches generation
|
run identifier, and UTC timing. The same effective session reaches initial
|
||||||
through the rendered prompt. The same effective target, including backend
|
generation and any repair attempt through the rendered prompt. The same
|
||||||
identity, reaches generation and any repair attempt.
|
effective target, including backend identity, reaches generation and any
|
||||||
|
repair attempt.
|
||||||
|
|
||||||
## Failure Categories
|
## Failure Categories
|
||||||
|
|
||||||
|
|||||||
@@ -1,556 +0,0 @@
|
|||||||
# Explicit Per-Run Profile Overrides Implementation Plan
|
|
||||||
|
|
||||||
**Status:** Complete.
|
|
||||||
|
|
||||||
## Purpose
|
|
||||||
|
|
||||||
This document is the decision-complete implementation plan for the
|
|
||||||
[explicit per-run profile overrides](profile-overrides.md). It is written for
|
|
||||||
a coding agent that will implement each stage in order.
|
|
||||||
|
|
||||||
The feature roadmap owns the intended capability, user intent, policy choices,
|
|
||||||
compatibility decision, and target end state. This document owns the concrete
|
|
||||||
API changes, internal representation, implementation sequence, test ownership,
|
|
||||||
documentation updates, and completion gates.
|
|
||||||
|
|
||||||
## Implementation Rules
|
|
||||||
|
|
||||||
- Complete the stages in order. Keep the repository compiling and the focused
|
|
||||||
tests passing at every stage boundary.
|
|
||||||
- Preserve unrelated working-tree changes. The selected feature roadmap and
|
|
||||||
the removal of its source idea from `future.md` may already be uncommitted
|
|
||||||
when implementation begins; retain both.
|
|
||||||
- Follow every policy under `docs/policy/`, the task-specific reading guide in
|
|
||||||
`docs/development.md`, and the target behavior in
|
|
||||||
`profile-overrides.md`.
|
|
||||||
- Keep the public API in the root `promptkit` package and implementation
|
|
||||||
details under `internal/`. Do not create another public package.
|
|
||||||
- Do not broaden the feature into conversation storage, automatic session
|
|
||||||
generation, provider capability discovery, backend-specific headers,
|
|
||||||
concurrency control, retries, queues, or file-format changes.
|
|
||||||
- Preserve synchronous `Prepare` and `Run`, engine-local invocation state,
|
|
||||||
endpoint-only profiles, backend-selected profiles, and injected
|
|
||||||
`LLMClient` behavior.
|
|
||||||
- Keep all tests deterministic and offline. Use existing fakes or local
|
|
||||||
`httptest` servers; never contact OpenRouter or another live provider.
|
|
||||||
- Update exact GoDoc with each exported declaration change. Update durable
|
|
||||||
current-state documents only after the corresponding behavior is
|
|
||||||
implemented.
|
|
||||||
- Add tests at the narrowest stable owner described below. Do not reproduce
|
|
||||||
the same precedence table at the domain, runner, facade, and transport
|
|
||||||
layers.
|
|
||||||
- Do not create a release, change a module version, or tag a commit. The final
|
|
||||||
implementation handoff must call out the planned pre-`v1` source
|
|
||||||
compatibility change so it can be included in the next minor release note.
|
|
||||||
|
|
||||||
## Fixed Design
|
|
||||||
|
|
||||||
### Public API
|
|
||||||
|
|
||||||
Add a direct session field to `RunRequest`:
|
|
||||||
|
|
||||||
```go
|
|
||||||
type RunRequest struct {
|
|
||||||
// Insert after ProfileID and before APIKey.
|
|
||||||
SessionID string
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
Its exact GoDoc must state that:
|
|
||||||
|
|
||||||
- a nonblank value is trimmed and overrides the prompt definition's
|
|
||||||
`session_id` template;
|
|
||||||
- a blank value supplies no direct override;
|
|
||||||
- the maximum is 256 Unicode code points after trimming;
|
|
||||||
- a direct value is opaque consumer metadata, is not a credential, and may be
|
|
||||||
exposed in prepared values, results, collaborator requests, provider
|
|
||||||
requests, and provider observability; and
|
|
||||||
- callers should use stable, non-sensitive identifiers.
|
|
||||||
|
|
||||||
Change the existing reasoning override field:
|
|
||||||
|
|
||||||
```go
|
|
||||||
type ExecutionTargetOverride struct {
|
|
||||||
// Existing fields remain otherwise unchanged.
|
|
||||||
ReasoningEffort *string
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
The field has these exact states:
|
|
||||||
|
|
||||||
| Public value | Effective behavior |
|
|
||||||
| --- | --- |
|
|
||||||
| `nil` | Inherit the profile's reasoning effort. |
|
|
||||||
| Pointer to a nonblank string | Trim and replace the profile value. |
|
|
||||||
| Pointer to an empty or whitespace-only string | Clear the inherited value and disable reasoning for this run. |
|
|
||||||
|
|
||||||
Do not add `DisableReasoning`, a parallel reasoning override, a public optional
|
|
||||||
string wrapper, or a fixed reasoning enum. The pointer is the one supported
|
|
||||||
runtime reasoning surface.
|
|
||||||
|
|
||||||
Add completed-run session metadata:
|
|
||||||
|
|
||||||
```go
|
|
||||||
type RunResult struct {
|
|
||||||
// Insert after PromptHash and before RenderedPromptHash.
|
|
||||||
SessionID string `json:"session_id,omitempty"`
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
`PreparedRun.SessionID` remains the preparation metadata field. Update its
|
|
||||||
GoDoc from "rendered" to "effective direct or rendered" session identity.
|
|
||||||
Likewise, update `RenderedPrompt.SessionID` GoDoc because injected clients may
|
|
||||||
receive a direct request value rather than one produced by the prompt
|
|
||||||
renderer.
|
|
||||||
|
|
||||||
Do not add session identity to `ExecutionTarget`,
|
|
||||||
`ExecutionTargetOverride`, `Profile`, or the profile file format. Do not add a
|
|
||||||
second result field that records whether the session came from the request or
|
|
||||||
prompt.
|
|
||||||
|
|
||||||
### Internal Values And Copy Ownership
|
|
||||||
|
|
||||||
Mirror the public request changes in `internal/domain`:
|
|
||||||
|
|
||||||
```go
|
|
||||||
type RunRequest struct {
|
|
||||||
// Insert after ProfileID and before APIKey.
|
|
||||||
SessionID string
|
|
||||||
}
|
|
||||||
|
|
||||||
type ExecutionTargetOverride struct {
|
|
||||||
// Existing fields...
|
|
||||||
ReasoningEffort *string
|
|
||||||
}
|
|
||||||
|
|
||||||
type RunResult struct {
|
|
||||||
// Insert after PromptHash and before RenderedPromptHash.
|
|
||||||
SessionID string
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
`toDomainRunRequest` copies `RunRequest.SessionID`. The public-to-domain
|
|
||||||
execution override conversion must allocate a new string for a non-nil
|
|
||||||
reasoning pointer rather than retain the caller's pointer. Add a small
|
|
||||||
`copyStringPtr` helper beside the existing numeric pointer-copy helpers in
|
|
||||||
`convert.go`.
|
|
||||||
|
|
||||||
The internal and public profile and effective-target fields remain ordinary
|
|
||||||
strings:
|
|
||||||
|
|
||||||
- `domain.ExecutionProfile.ReasoningEffort`
|
|
||||||
- `Profile.ReasoningEffort`
|
|
||||||
- `domain.ExecutionTarget.ReasoningEffort`
|
|
||||||
- `ExecutionTarget.ReasoningEffort`
|
|
||||||
|
|
||||||
Do not extend `ExecutionTargetPresence`. That value exists to preserve
|
|
||||||
explicit numeric zero serialization. After reasoning resolution, an empty
|
|
||||||
effective reasoning string is sufficient to instruct built-in and injected
|
|
||||||
clients to omit reasoning.
|
|
||||||
|
|
||||||
### Shared Session Normalization
|
|
||||||
|
|
||||||
Add `internal/domain/session.go` with one shared internal domain rule:
|
|
||||||
|
|
||||||
```go
|
|
||||||
func NormalizeSessionID(raw string) (string, error)
|
|
||||||
```
|
|
||||||
|
|
||||||
The function:
|
|
||||||
|
|
||||||
- trims surrounding Unicode whitespace with `strings.TrimSpace`;
|
|
||||||
- returns `""`, `nil` for a blank value;
|
|
||||||
- counts Unicode code points with `utf8.RuneCountInString`;
|
|
||||||
- rejects a normalized value longer than `SessionIDMaxLength`; and
|
|
||||||
- returns an ordinary internal error containing the actual and maximum
|
|
||||||
lengths, without attaching a prompt, request, or transport error sentinel.
|
|
||||||
|
|
||||||
Keep `SessionIDMaxLength` in `internal/domain` at 256. The renderer, runner,
|
|
||||||
and model client wrap normalization failures into their own error boundaries:
|
|
||||||
|
|
||||||
- prompt session template: prompt-render failure;
|
|
||||||
- direct request session: invalid request; and
|
|
||||||
- defensive model-client validation: invalid generate request.
|
|
||||||
|
|
||||||
Refactor `internal/prompt.renderSessionID` and
|
|
||||||
`internal/llm.openAIChatRequestFromGenerateRequest` to use this helper. Remove
|
|
||||||
their duplicated trimming and rune-count logic and any now-unused imports.
|
|
||||||
|
|
||||||
### Direct Session Resolution
|
|
||||||
|
|
||||||
In `Runner.Prepare`, preserve required prompt-ID validation as the first
|
|
||||||
request check. Immediately afterward, normalize `req.SessionID`. A
|
|
||||||
normalization failure must return an error matching use-case
|
|
||||||
`ErrInvalidRequest` before prompt or profile loading begins.
|
|
||||||
|
|
||||||
Continue loading and hashing the original prompt definition. When the
|
|
||||||
normalized direct session is nonempty:
|
|
||||||
|
|
||||||
1. make a value copy of the loaded prompt definition;
|
|
||||||
2. clear only the copy's `SessionID` template;
|
|
||||||
3. pass the copied definition to the existing renderer; and
|
|
||||||
4. set the renderer's returned `RenderedPrompt.SessionID` to the normalized
|
|
||||||
direct value before hashing or constructing `PreparedRun`.
|
|
||||||
|
|
||||||
Do not mutate the repository-owned prompt definition. Do not change the
|
|
||||||
renderer interface, inject the direct value into `Vars`, or expose it to
|
|
||||||
message templates. Message rendering continues to use the request's existing
|
|
||||||
variables and inputs.
|
|
||||||
|
|
||||||
When the direct session is blank, pass the original definition to the renderer
|
|
||||||
and retain current prompt-template behavior. This includes current missing
|
|
||||||
variable, invalid template, blank result, and maximum-length behavior.
|
|
||||||
|
|
||||||
The prompt-definition hash must always be computed from the original
|
|
||||||
definition. The rendered-prompt hash must use the effective session ID after
|
|
||||||
the direct override is applied.
|
|
||||||
|
|
||||||
`Runner.Run` must copy `PreparedRun.SessionID` into the internal `RunResult`.
|
|
||||||
The existing generation request must continue receiving the same effective
|
|
||||||
value through `RenderedPrompt.SessionID`.
|
|
||||||
|
|
||||||
### Reasoning Resolution
|
|
||||||
|
|
||||||
Leave `mergeExecutionTarget`, which overlays framework/backend/profile
|
|
||||||
`ExecutionTarget` values, unchanged: blank profile-level reasoning strings
|
|
||||||
continue to mean "no replacement."
|
|
||||||
|
|
||||||
Change only `mergeExecutionTargetOverride`, which applies the request's
|
|
||||||
pointer-based override:
|
|
||||||
|
|
||||||
```go
|
|
||||||
if override.ReasoningEffort != nil {
|
|
||||||
out.ReasoningEffort = strings.TrimSpace(*override.ReasoningEffort)
|
|
||||||
}
|
|
||||||
```
|
|
||||||
|
|
||||||
Assign the trimmed value even when it is empty. This is the operation that
|
|
||||||
clears inherited reasoning. Do not validate against a fixed vocabulary and do
|
|
||||||
not translate an empty value to `none`, `null`, or any other provider value.
|
|
||||||
|
|
||||||
The built-in client already omits an empty effective `ReasoningEffort` and
|
|
||||||
serializes a nonempty value. Preserve that transport behavior. A consumer may
|
|
||||||
still explicitly provide a provider-supported nonblank value such as `none`;
|
|
||||||
Promptkit treats it as an opaque override rather than its own disable
|
|
||||||
mechanism.
|
|
||||||
|
|
||||||
### Stable JSON And Result Conversion
|
|
||||||
|
|
||||||
Add `session_id,omitempty` to the private `runResultJSON` representation in
|
|
||||||
`json.go`. Carry it in both `RunResult.MarshalJSON` and
|
|
||||||
`RunResult.UnmarshalJSON`.
|
|
||||||
|
|
||||||
The stable JSON contract is:
|
|
||||||
|
|
||||||
- a nonempty `RunResult.SessionID` serializes as top-level `session_id`;
|
|
||||||
- an empty value is omitted; and
|
|
||||||
- JSON produced before this field existed remains valid and decodes with an
|
|
||||||
empty session ID.
|
|
||||||
|
|
||||||
Carry the internal result value through `fromDomainRunResult`. No custom JSON
|
|
||||||
work is needed for `PreparedRun`, whose existing `SessionID` field already has
|
|
||||||
the required name and omission behavior.
|
|
||||||
|
|
||||||
### OpenAI-Compatible Wire Contract
|
|
||||||
|
|
||||||
Do not add `x-session-id`. The built-in client continues sending the effective
|
|
||||||
nonempty value only as the top-level `session_id` body field. Keep
|
|
||||||
`session_id` in the reserved extra-parameter field set.
|
|
||||||
|
|
||||||
The existing transport tests that assert body serialization, blank omission,
|
|
||||||
length rejection, and absence of the session header remain the canonical wire
|
|
||||||
tests. Refactoring normalization must not weaken those assertions.
|
|
||||||
|
|
||||||
### Error Identities
|
|
||||||
|
|
||||||
Do not add public or internal error sentinels.
|
|
||||||
|
|
||||||
| Failure | Required identity |
|
|
||||||
| --- | --- |
|
|
||||||
| Direct session exceeds the maximum | Public `ErrInvalidRequest` through use-case `ErrInvalidRequest` |
|
|
||||||
| Prompt session template is invalid or cannot render | Existing `ErrPromptRender` behavior |
|
|
||||||
| Prompt-rendered session exceeds the maximum | Existing `ErrPromptRender` behavior |
|
|
||||||
| Model client defensively receives an overlong session | Existing internal LLM `ErrInvalidRequest` |
|
|
||||||
| Blank reasoning pointer | No error; clears inherited reasoning |
|
|
||||||
| Nonblank unfamiliar reasoning value | No Promptkit validation error; pass through after trimming |
|
|
||||||
|
|
||||||
Do not make exact diagnostic prose a public contract. Tests should use
|
|
||||||
`errors.Is` and inspect only a useful semantic fragment when necessary.
|
|
||||||
|
|
||||||
## Test Ownership
|
|
||||||
|
|
||||||
Use the following ownership split:
|
|
||||||
|
|
||||||
- `internal/domain/session_test.go` owns shared trimming, blank handling, and
|
|
||||||
the 256/257 Unicode-code-point boundary.
|
|
||||||
- `internal/prompt/renderer_test.go` retains ownership of session-template
|
|
||||||
parsing, rendering, and prompt-render error mapping. Update existing tests
|
|
||||||
only as required by the shared normalizer refactor.
|
|
||||||
- `internal/usecase/runner_test.go` owns request/profile reasoning precedence,
|
|
||||||
direct-session bypass of the template, invalid direct-session
|
|
||||||
classification, and application of the effective session before hashing.
|
|
||||||
- Root external-package tests own public conversion, an assembled
|
|
||||||
direct-session workflow, completed-result propagation, and stable public
|
|
||||||
JSON.
|
|
||||||
- `internal/llm/openai_compatible_client_test.go` remains the sole owner of
|
|
||||||
body-versus-header delivery and wire omission. Do not duplicate those exact
|
|
||||||
HTTP assertions in a root workflow test.
|
|
||||||
|
|
||||||
Prefer extending coherent existing table tests over creating parallel
|
|
||||||
end-to-end suites. Keep literal-limit testing centralized around
|
|
||||||
`domain.SessionIDMaxLength`; use relative boundary cases rather than copying
|
|
||||||
the number into multiple tests.
|
|
||||||
|
|
||||||
## Stage 1 — Tri-State Reasoning Override
|
|
||||||
|
|
||||||
**Status:** Complete.
|
|
||||||
|
|
||||||
### Goal
|
|
||||||
|
|
||||||
Replace the ambiguous string request override with an owned pointer and
|
|
||||||
implement inherit, replace, and clear semantics without changing profiles or
|
|
||||||
the provider payload model.
|
|
||||||
|
|
||||||
### Work
|
|
||||||
|
|
||||||
1. In `types.go`, change
|
|
||||||
`ExecutionTargetOverride.ReasoningEffort` to `*string` and write exact
|
|
||||||
tri-state GoDoc. Update the surrounding override summary so it no longer
|
|
||||||
says every empty string inherits.
|
|
||||||
2. In `internal/domain/domain.go`, make the corresponding override field a
|
|
||||||
`*string`. Leave profile and effective-target fields unchanged.
|
|
||||||
3. In `convert.go`, add `copyStringPtr` beside `copyFloat64Ptr` and
|
|
||||||
`copyIntPtr`, and use it when converting the override. Never retain the
|
|
||||||
caller's pointer.
|
|
||||||
4. In `internal/usecase/runner.go`, change only the request-override merge to
|
|
||||||
assign `strings.TrimSpace(*override.ReasoningEffort)` whenever the pointer
|
|
||||||
is non-nil. Preserve profile-level blank-string behavior.
|
|
||||||
5. Update all in-repository public and internal
|
|
||||||
`ExecutionTargetOverride` composite literals to use string pointers where
|
|
||||||
they intend an override. Do not change `Profile`, `ExecutionProfile`, or
|
|
||||||
effective `ExecutionTarget` literals.
|
|
||||||
6. Keep `ExecutionTargetPresence`, the model-client request type, and the
|
|
||||||
OpenAI-compatible wire structure unchanged.
|
|
||||||
|
|
||||||
### Tests
|
|
||||||
|
|
||||||
1. Update the existing root execution-precedence test so its ordinary
|
|
||||||
nonblank reasoning override uses a pointer.
|
|
||||||
2. Extend that coherent public precedence coverage with a profile that has a
|
|
||||||
nonempty reasoning default and a pointer to a blank request value; assert
|
|
||||||
that `PreparedRun.EffectiveModelParams.ReasoningEffort` is empty.
|
|
||||||
3. In the runner resolution tests, cover all three states:
|
|
||||||
- nil inherits the profile value;
|
|
||||||
- a pointer to `" high "` produces `"high"`; and
|
|
||||||
- a pointer to whitespace produces `""`.
|
|
||||||
4. Retain the existing model-client tests for serializing nonempty reasoning
|
|
||||||
and omitting empty reasoning. Do not add another HTTP test for the same
|
|
||||||
transport branches.
|
|
||||||
|
|
||||||
### Focused Validation
|
|
||||||
|
|
||||||
Run:
|
|
||||||
|
|
||||||
```sh
|
|
||||||
gofmt -w types.go convert.go internal/domain/domain.go \
|
|
||||||
internal/usecase/runner.go engine_test.go \
|
|
||||||
internal/usecase/runner_test.go
|
|
||||||
go test . ./internal/usecase ./internal/llm
|
|
||||||
go vet . ./internal/usecase ./internal/llm
|
|
||||||
git diff --check
|
|
||||||
```
|
|
||||||
|
|
||||||
### Completion Gate
|
|
||||||
|
|
||||||
This stage is complete when the repository builds with the pointer-based
|
|
||||||
public API, all three reasoning states resolve correctly, caller pointer
|
|
||||||
ownership is not retained, and the provider client still receives only the
|
|
||||||
resolved string.
|
|
||||||
|
|
||||||
## Stage 2 — Direct Session Resolution And Result Metadata
|
|
||||||
|
|
||||||
**Status:** Complete.
|
|
||||||
|
|
||||||
### Goal
|
|
||||||
|
|
||||||
Add direct per-run session identity, bypass the prompt session template when
|
|
||||||
selected, and carry the effective value consistently through hashes,
|
|
||||||
generation, results, and stable JSON.
|
|
||||||
|
|
||||||
### Work
|
|
||||||
|
|
||||||
1. Add `internal/domain/session.go` with the shared normalization function and
|
|
||||||
refactor the prompt renderer and model client to consume it.
|
|
||||||
2. Add `SessionID` to public and internal `RunRequest` and copy it in
|
|
||||||
`toDomainRunRequest`.
|
|
||||||
3. Implement direct normalization at the start of `Runner.Prepare` after
|
|
||||||
required prompt-ID validation. Wrap an overlong value with use-case
|
|
||||||
`ErrInvalidRequest`.
|
|
||||||
4. Preserve the original definition for its hash. For a nonempty direct
|
|
||||||
session, render with a copied definition whose session template is cleared,
|
|
||||||
then apply the direct value before computing the rendered-prompt hash.
|
|
||||||
5. Leave message variables and templates unchanged. Confirm that a direct
|
|
||||||
session succeeds even if the unused prompt session template would fail
|
|
||||||
because of a missing variable.
|
|
||||||
6. Add `SessionID` to internal and public `RunResult`, populate it from
|
|
||||||
`PreparedRun` in `Runner.Run`, and carry it through `fromDomainRunResult`.
|
|
||||||
7. Add the field to `runResultJSON`, `MarshalJSON`, and `UnmarshalJSON` with
|
|
||||||
`omitempty`.
|
|
||||||
8. Update GoDoc for `RunRequest.SessionID`, `PreparedRun.SessionID`,
|
|
||||||
`RunResult.SessionID`, and `RenderedPrompt.SessionID`. Ensure none describes
|
|
||||||
the value as necessarily renderer-produced.
|
|
||||||
9. Keep the built-in HTTP request body and reserved-field behavior unchanged;
|
|
||||||
no session header is added.
|
|
||||||
|
|
||||||
### Tests
|
|
||||||
|
|
||||||
1. Add focused domain normalization cases for:
|
|
||||||
- surrounding whitespace;
|
|
||||||
- blank input;
|
|
||||||
- exactly `SessionIDMaxLength` Unicode code points; and
|
|
||||||
- one code point over the limit.
|
|
||||||
2. Add runner tests showing:
|
|
||||||
- a direct value is trimmed and wins over a prompt session template;
|
|
||||||
- a direct value bypasses an otherwise failing session template while
|
|
||||||
message templates still render normally;
|
|
||||||
- blank direct input retains prompt-template behavior;
|
|
||||||
- changing only the direct session changes the rendered-prompt hash but not
|
|
||||||
the prompt-definition hash; and
|
|
||||||
- an overlong direct value matches `ErrInvalidRequest` and does not invoke
|
|
||||||
generation.
|
|
||||||
3. Add one assembled root-package workflow using an injected fake model
|
|
||||||
client. Assert that the same direct session appears in `PreparedRun`,
|
|
||||||
`GenerateRequest.Prompt`, and `RunResult`. This test owns the public adapter
|
|
||||||
path; it must not repeat body/header assertions.
|
|
||||||
4. Extend the public `RunResult` JSON round-trip test to cover a nonempty
|
|
||||||
session, and add or extend omission coverage for an empty session.
|
|
||||||
5. Keep the existing prompt-renderer and model-client boundary tests passing
|
|
||||||
after normalization is centralized.
|
|
||||||
|
|
||||||
### Focused Validation
|
|
||||||
|
|
||||||
Run:
|
|
||||||
|
|
||||||
```sh
|
|
||||||
gofmt -w types.go convert.go json.go internal/domain/domain.go \
|
|
||||||
internal/domain/session.go internal/domain/session_test.go \
|
|
||||||
internal/prompt/go_renderer.go internal/prompt/renderer_test.go \
|
|
||||||
internal/usecase/runner.go internal/usecase/runner_test.go \
|
|
||||||
internal/llm/openai_compatible_client.go \
|
|
||||||
internal/llm/openai_compatible_client_test.go \
|
|
||||||
engine_test.go public_contract_test.go
|
|
||||||
go test . ./internal/domain ./internal/prompt ./internal/usecase ./internal/llm
|
|
||||||
go vet . ./internal/domain ./internal/prompt ./internal/usecase ./internal/llm
|
|
||||||
git diff --check
|
|
||||||
```
|
|
||||||
|
|
||||||
If an existing test file did not require an edit, omit it from the `gofmt`
|
|
||||||
arguments rather than touching it mechanically.
|
|
||||||
|
|
||||||
### Completion Gate
|
|
||||||
|
|
||||||
This stage is complete when one normalized effective session flows from the
|
|
||||||
public request through preparation, hashing, generation, completed results,
|
|
||||||
and stable JSON; direct values bypass the prompt session template; and the
|
|
||||||
existing body-only transport contract remains intact.
|
|
||||||
|
|
||||||
## Stage 3 — Durable Documentation And Final Validation
|
|
||||||
|
|
||||||
**Status:** Complete.
|
|
||||||
|
|
||||||
### Goal
|
|
||||||
|
|
||||||
Make current-state contracts accurately describe the implemented feature,
|
|
||||||
record the compatibility impact for release handoff, and validate the complete
|
|
||||||
repository.
|
|
||||||
|
|
||||||
### Work
|
|
||||||
|
|
||||||
1. Review all changed exported declarations in `types.go` and `doc.go`.
|
|
||||||
Ensure GoDoc is the canonical owner of exact field types, pointer states,
|
|
||||||
copying, normalization, privacy exposure, JSON, and error behavior.
|
|
||||||
2. Update `docs/consumers/pkg-promptkit.md` with concise task-oriented guidance
|
|
||||||
for:
|
|
||||||
- supplying a direct per-run session;
|
|
||||||
- inheriting, replacing, and disabling profile reasoning with a string
|
|
||||||
pointer; and
|
|
||||||
- treating session IDs as stable, non-secret correlation values.
|
|
||||||
Keep snippets illustrative and link to GoDoc for the exact contract.
|
|
||||||
3. Update `docs/formats.md` only where the file format interacts with runtime
|
|
||||||
precedence:
|
|
||||||
- a direct request session bypasses the prompt `session_id` template; and
|
|
||||||
- reasoning is the exception to the general nonempty request-string rule
|
|
||||||
because its pointer can explicitly clear the profile value.
|
|
||||||
Do not add or change YAML fields.
|
|
||||||
4. Update `docs/integrations/openai-compatible-chat.md` to describe the
|
|
||||||
effective direct-or-rendered session, body-only delivery, the 256-code-point
|
|
||||||
check, and omission of an explicitly disabled effective reasoning setting.
|
|
||||||
Do not duplicate public Go field declarations.
|
|
||||||
5. Update `docs/internal/runner.md` to describe direct-session normalization
|
|
||||||
and template bypass, effective hashing, tri-state reasoning precedence, and
|
|
||||||
propagation into completed results.
|
|
||||||
6. Do not update `docs/internal/llm.md` or the internal component inventory.
|
|
||||||
Shared normalization is an incidental mechanism and does not change the
|
|
||||||
model client's documented flow or any package responsibility.
|
|
||||||
7. Do not change architecture, documentation, or testing policy unless
|
|
||||||
implementation reveals a genuine policy change. This feature is expected
|
|
||||||
to fit the current policies.
|
|
||||||
8. After every implementation and documentation check passes, set the feature
|
|
||||||
roadmap and this implementation plan status to `Complete`. Do not remove
|
|
||||||
either roadmap in the implementation change; lifecycle retirement can
|
|
||||||
follow after review.
|
|
||||||
9. In the implementation handoff, explicitly identify
|
|
||||||
`ExecutionTargetOverride.ReasoningEffort` changing from `string` to
|
|
||||||
`*string` as a pre-`v1` minor-release source compatibility change. Do not
|
|
||||||
edit release procedure or create a tag.
|
|
||||||
|
|
||||||
### Full Validation
|
|
||||||
|
|
||||||
Run the complete sequence from `docs/development.md`:
|
|
||||||
|
|
||||||
```sh
|
|
||||||
go test ./...
|
|
||||||
go test -race ./...
|
|
||||||
go vet ./...
|
|
||||||
go build ./...
|
|
||||||
go run ./examples/go-library/prepare
|
|
||||||
gofmt -l $(git ls-files '*.go')
|
|
||||||
git diff --check
|
|
||||||
```
|
|
||||||
|
|
||||||
The formatting command must produce no paths. Follow every added or changed
|
|
||||||
Markdown link and confirm its target and heading exist. Also inspect:
|
|
||||||
|
|
||||||
```sh
|
|
||||||
git status --short
|
|
||||||
git diff --stat
|
|
||||||
git diff
|
|
||||||
```
|
|
||||||
|
|
||||||
Confirm that:
|
|
||||||
|
|
||||||
- only intended feature, test, documentation, and roadmap files changed;
|
|
||||||
- no `go.work`, `go.work.sum`, local module replacement, credential, generated
|
|
||||||
binary, or unrelated change was introduced;
|
|
||||||
- all public and internal override literals use the correct pointer or string
|
|
||||||
type for their layer;
|
|
||||||
- `session_id` remains reserved in extra parameters;
|
|
||||||
- the built-in client does not set `x-session-id`;
|
|
||||||
- current-state documents describe implemented behavior rather than referring
|
|
||||||
readers to the roadmaps; and
|
|
||||||
- the feature and implementation roadmaps contain no unresolved work marked
|
|
||||||
complete.
|
|
||||||
|
|
||||||
### Completion Gate
|
|
||||||
|
|
||||||
The implementation is complete only when every target-end-state item in
|
|
||||||
`profile-overrides.md` is implemented, the full validation sequence passes,
|
|
||||||
durable contracts no longer depend on roadmap prose, and the compatibility
|
|
||||||
change is clearly reported for the next minor release.
|
|
||||||
|
|
||||||
## Open Questions
|
|
||||||
|
|
||||||
None. The feature roadmap and this plan fix the public representation,
|
|
||||||
precedence, normalization, error identities, body-versus-header choice,
|
|
||||||
metadata, hashing, test ownership, compatibility treatment, and non-goals
|
|
||||||
needed for implementation.
|
|
||||||
@@ -1,176 +0,0 @@
|
|||||||
# Explicit Per-Run Profile Overrides
|
|
||||||
|
|
||||||
**Status:** Complete.
|
|
||||||
|
|
||||||
## Purpose
|
|
||||||
|
|
||||||
This roadmap defines the scope and target end state for explicit per-run
|
|
||||||
session and reasoning controls. It establishes the behavioral boundary and
|
|
||||||
policy choices for the work.
|
|
||||||
|
|
||||||
This document is planning material, not a description of current behavior.
|
|
||||||
Current exported contracts remain owned by Go declarations and GoDoc, prompt
|
|
||||||
and profile files by the [format reference](../formats.md), and outbound HTTP
|
|
||||||
behavior by the
|
|
||||||
[OpenAI-compatible integration contract](../integrations/openai-compatible-chat.md).
|
|
||||||
|
|
||||||
## Motivation
|
|
||||||
|
|
||||||
Prompt definitions can currently render a session ID from request variables,
|
|
||||||
and execution profiles can provide a default reasoning effort. A nonblank
|
|
||||||
per-run reasoning value can replace that profile default, but consumers cannot
|
|
||||||
directly supply a session ID or explicitly clear inherited reasoning.
|
|
||||||
|
|
||||||
Consumers should be able to reuse one prompt and profile across conversations,
|
|
||||||
agent workflows, and reasoning levels. Choosing a session or reasoning level
|
|
||||||
for one run should not require duplicate prompt definitions, duplicate
|
|
||||||
profiles, or provider-specific values that mean "disabled."
|
|
||||||
|
|
||||||
## Scope
|
|
||||||
|
|
||||||
The work will add two independent per-run controls:
|
|
||||||
|
|
||||||
- a direct session ID associated with the run; and
|
|
||||||
- a tri-state reasoning override that can inherit, replace, or disable the
|
|
||||||
selected profile's reasoning effort.
|
|
||||||
|
|
||||||
These controls belong to the existing request and execution-target surfaces.
|
|
||||||
They will not introduce another prompt or profile selection mechanism.
|
|
||||||
|
|
||||||
## Session ID Behavior
|
|
||||||
|
|
||||||
`RunRequest` will accept an optional direct session ID. Promptkit will treat
|
|
||||||
session IDs as opaque, consumer-supplied correlation values rather than
|
|
||||||
credentials or conversation storage keys managed by the library.
|
|
||||||
|
|
||||||
The effective session ID will resolve as follows:
|
|
||||||
|
|
||||||
1. a nonblank direct request session ID;
|
|
||||||
2. the rendered prompt-definition session ID template; or
|
|
||||||
3. no session ID.
|
|
||||||
|
|
||||||
A direct value will be trimmed and will take complete precedence over the
|
|
||||||
prompt template. When a direct value is present, Promptkit will not parse or
|
|
||||||
render the prompt's session ID template. An unused template therefore cannot
|
|
||||||
fail the run because of a missing variable or another template error. A blank
|
|
||||||
direct value means that no direct override was supplied and retains the prompt
|
|
||||||
template behavior.
|
|
||||||
|
|
||||||
The direct value will use the existing session validation rule: at most 256
|
|
||||||
Unicode code points after trimming. An invalid direct value will fail at the
|
|
||||||
per-run request boundary. Prompt-rendered session IDs will retain their
|
|
||||||
existing prompt-render failure boundary.
|
|
||||||
|
|
||||||
For the built-in OpenAI-compatible client, a nonempty effective session ID
|
|
||||||
will continue to be sent as the top-level `session_id` request-body field. It
|
|
||||||
will not also be sent through the `x-session-id` header. This matches
|
|
||||||
OpenRouter's documented request schema and avoids two competing wire values.
|
|
||||||
Promptkit will not claim that every OpenAI-compatible backend implements
|
|
||||||
OpenRouter's sticky-routing or observability semantics.
|
|
||||||
|
|
||||||
Consumers are responsible for choosing stable identifiers for related calls.
|
|
||||||
Session IDs may appear in prepared values, results, provider requests, and
|
|
||||||
provider observability systems, so they must not contain credentials or
|
|
||||||
unnecessarily sensitive data.
|
|
||||||
|
|
||||||
## Reasoning Override Behavior
|
|
||||||
|
|
||||||
The profile's reasoning effort will remain a reusable default. The per-run
|
|
||||||
execution override will distinguish three states:
|
|
||||||
|
|
||||||
- omitted: inherit the selected profile's reasoning effort;
|
|
||||||
- nonblank: replace the profile value with the trimmed request value; and
|
|
||||||
- explicitly blank: disable inherited reasoning for the run.
|
|
||||||
|
|
||||||
The public `ExecutionTargetOverride.ReasoningEffort` field will become a string
|
|
||||||
pointer so `nil`, a pointer to a nonblank string, and a pointer to a blank
|
|
||||||
string represent those three states directly. `Profile.ReasoningEffort` and
|
|
||||||
the effective `ExecutionTarget.ReasoningEffort` will remain strings.
|
|
||||||
|
|
||||||
Promptkit will not define a closed set of reasoning effort names because
|
|
||||||
supported values may vary across OpenAI-compatible backends. Explicit disable
|
|
||||||
will produce an empty effective reasoning setting, and the built-in client
|
|
||||||
will omit `reasoning_effort` from the provider request. Promptkit will not
|
|
||||||
translate disable into a provider-specific magic value such as `none`.
|
|
||||||
|
|
||||||
## Metadata And Generation Boundary
|
|
||||||
|
|
||||||
The effective direct or rendered session ID will:
|
|
||||||
|
|
||||||
- remain visible in `PreparedRun`;
|
|
||||||
- be added to `RunResult`;
|
|
||||||
- be included in the rendered-prompt hash; and
|
|
||||||
- be supplied to injected model clients through the rendered prompt.
|
|
||||||
|
|
||||||
The prompt-definition hash will continue to describe the selected definition,
|
|
||||||
including its configured session template, even when a direct value bypasses
|
|
||||||
that template. The rendered-prompt hash will describe the effective session ID
|
|
||||||
and rendered messages used for the run.
|
|
||||||
|
|
||||||
The effective reasoning effort will remain visible through
|
|
||||||
`EffectiveModelParams` in prepared and completed run metadata and through the
|
|
||||||
execution target supplied to injected model clients. An empty effective value
|
|
||||||
means that the client should omit a reasoning setting.
|
|
||||||
|
|
||||||
## Compatibility
|
|
||||||
|
|
||||||
Adding a direct session field and completed-run session metadata is additive.
|
|
||||||
Changing `ExecutionTargetOverride.ReasoningEffort` from `string` to `*string`
|
|
||||||
is a source compatibility change for consumers that initialize that field.
|
|
||||||
|
|
||||||
Promptkit's [pre-`v1` release policy](../release.md#release-model) permits
|
|
||||||
public API changes in a minor release. This work will use that release boundary
|
|
||||||
in favor of a single idiomatic tri-state field rather than permanently adding
|
|
||||||
a second disable flag or parallel override field. Release notes for the
|
|
||||||
version that publishes the change will identify the required consumer update.
|
|
||||||
|
|
||||||
Existing behavior will otherwise remain compatible:
|
|
||||||
|
|
||||||
- prompts without a direct session ID continue rendering their session
|
|
||||||
template;
|
|
||||||
- prompts without either session source continue without a session ID;
|
|
||||||
- profiles continue supplying reasoning defaults;
|
|
||||||
- omitted per-run reasoning continues inheriting the profile;
|
|
||||||
- endpoint-only and backend-selected profiles behave identically;
|
|
||||||
- injected model clients continue receiving the effective rendered prompt and
|
|
||||||
execution target; and
|
|
||||||
- synchronous `Prepare` and `Run` behavior remains unchanged.
|
|
||||||
|
|
||||||
## Non-Goals
|
|
||||||
|
|
||||||
This scope does not include:
|
|
||||||
|
|
||||||
- conversation history, memory, or message persistence;
|
|
||||||
- durable session storage or session lifecycle management;
|
|
||||||
- automatic session ID generation;
|
|
||||||
- user identity, authorization, or tenancy policy;
|
|
||||||
- provider-specific reasoning vocabularies or capability discovery;
|
|
||||||
- translating reasoning settings between provider protocols;
|
|
||||||
- backend-specific header configuration or other transport capabilities;
|
|
||||||
- changes to prompt or profile file formats;
|
|
||||||
- concurrency limits, admission queues, asynchronous jobs, or retries; or
|
|
||||||
- changes to model, backend, prompt, or profile selection.
|
|
||||||
|
|
||||||
## Target End State
|
|
||||||
|
|
||||||
This roadmap reaches its target end state when:
|
|
||||||
|
|
||||||
- consumers can provide a direct per-run session ID without modifying prompt
|
|
||||||
variables or definitions;
|
|
||||||
- a direct session ID bypasses and takes precedence over the prompt session
|
|
||||||
template;
|
|
||||||
- direct and rendered session IDs share the established normalization and
|
|
||||||
maximum-length contract;
|
|
||||||
- the effective session ID is carried consistently through preparation,
|
|
||||||
hashing, execution, completed-run metadata, and injected clients;
|
|
||||||
- the built-in client sends the effective session ID only as the documented
|
|
||||||
top-level request-body field;
|
|
||||||
- consumers can inherit, replace, or explicitly disable a profile's reasoning
|
|
||||||
effort through one tri-state override;
|
|
||||||
- explicit reasoning disable results in omission from the provider request;
|
|
||||||
- effective reasoning remains visible in preparation, results, and the model
|
|
||||||
client boundary;
|
|
||||||
- the compatibility impact is identified for the next release according to
|
|
||||||
the pre-`v1` minor-release policy; and
|
|
||||||
- current-state GoDoc, consumer, integration, and internal documentation
|
|
||||||
describe the implemented behavior without relying on this roadmap.
|
|
||||||
@@ -17,6 +17,7 @@ type OutputRepairer interface {
|
|||||||
type RepairRequest struct {
|
type RepairRequest struct {
|
||||||
PreviousOutput string
|
PreviousOutput string
|
||||||
ValidationErrors []string
|
ValidationErrors []string
|
||||||
|
SessionID string
|
||||||
Target domain.ExecutionTarget
|
Target domain.ExecutionTarget
|
||||||
StructuredOutput *domain.StructuredOutputSpec
|
StructuredOutput *domain.StructuredOutputSpec
|
||||||
Attempt int
|
Attempt int
|
||||||
@@ -42,23 +43,26 @@ func (r *defaultOutputRepairer) Repair(ctx context.Context, req RepairRequest) (
|
|||||||
errs = strings.Join(req.ValidationErrors, "\n")
|
errs = strings.Join(req.ValidationErrors, "\n")
|
||||||
}
|
}
|
||||||
|
|
||||||
prompt := domain.RenderedPrompt{Messages: []domain.RenderedMessage{
|
prompt := domain.RenderedPrompt{
|
||||||
{
|
SessionID: req.SessionID,
|
||||||
Role: "system",
|
Messages: []domain.RenderedMessage{
|
||||||
Content: "You repair invalid JSON output. Return only corrected JSON. Do not include explanations or markdown code fences.",
|
{
|
||||||
|
Role: "system",
|
||||||
|
Content: "You repair invalid JSON output. Return only corrected JSON. Do not include explanations or markdown code fences.",
|
||||||
|
},
|
||||||
|
{
|
||||||
|
Role: "user",
|
||||||
|
Content: fmt.Sprintf(
|
||||||
|
"Repair attempt %d of %d for validation mode %s.\n\nValidation errors:\n%s\n\nPrevious output:\n%s\n\nReturn only corrected JSON.",
|
||||||
|
req.Attempt,
|
||||||
|
req.MaxAttempts,
|
||||||
|
req.Mode,
|
||||||
|
errs,
|
||||||
|
req.PreviousOutput,
|
||||||
|
),
|
||||||
|
},
|
||||||
},
|
},
|
||||||
{
|
}
|
||||||
Role: "user",
|
|
||||||
Content: fmt.Sprintf(
|
|
||||||
"Repair attempt %d of %d for validation mode %s.\n\nValidation errors:\n%s\n\nPrevious output:\n%s\n\nReturn only corrected JSON.",
|
|
||||||
req.Attempt,
|
|
||||||
req.MaxAttempts,
|
|
||||||
req.Mode,
|
|
||||||
errs,
|
|
||||||
req.PreviousOutput,
|
|
||||||
),
|
|
||||||
},
|
|
||||||
}}
|
|
||||||
|
|
||||||
resp, err := r.llm.Generate(ctx, domain.GenerateRequest{
|
resp, err := r.llm.Generate(ctx, domain.GenerateRequest{
|
||||||
Prompt: prompt,
|
Prompt: prompt,
|
||||||
|
|||||||
@@ -127,6 +127,7 @@ func (r *Runner) Run(ctx context.Context, req domain.RunRequest) (*domain.RunRes
|
|||||||
repairResp, repairErr := r.repairer.Repair(ctx, RepairRequest{
|
repairResp, repairErr := r.repairer.Repair(ctx, RepairRequest{
|
||||||
PreviousOutput: genResp.Content,
|
PreviousOutput: genResp.Content,
|
||||||
ValidationErrors: validationResult.Errors,
|
ValidationErrors: validationResult.Errors,
|
||||||
|
SessionID: prepared.SessionID,
|
||||||
Target: prepared.EffectiveModelParams,
|
Target: prepared.EffectiveModelParams,
|
||||||
StructuredOutput: prepared.StructuredOutput,
|
StructuredOutput: prepared.StructuredOutput,
|
||||||
Attempt: attemptsUsed,
|
Attempt: attemptsUsed,
|
||||||
|
|||||||
@@ -1690,6 +1690,41 @@ func TestRunnerRunStructuredRepairRemainsBoundedAndUsesEffectiveModelSettings(t
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestRunnerRunRepairCarriesEffectiveSessionID(t *testing.T) {
|
||||||
|
llmClient := &fakeLLM{resp: &domain.GenerateResponse{Content: `{"broken":`}}
|
||||||
|
runner := NewRunnerWithRepairer(
|
||||||
|
&fakePromptRepo{def: promptDef(domain.FormatJSON, domain.ValidationJSON, 1)},
|
||||||
|
&fakeExecutionProfileRepo{profiles: map[string]*domain.ExecutionProfile{
|
||||||
|
"exec": {ID: "exec", Endpoint: "http://example.test/v1", Model: "model"},
|
||||||
|
}},
|
||||||
|
nil,
|
||||||
|
defaultArtifactReader(),
|
||||||
|
defaultRenderer(),
|
||||||
|
llmClient,
|
||||||
|
validate.NewStandardValidator("."),
|
||||||
|
NewDefaultOutputRepairer(llmClient),
|
||||||
|
)
|
||||||
|
|
||||||
|
result, err := runner.Run(context.Background(), domain.RunRequest{
|
||||||
|
PromptID: "p",
|
||||||
|
ProfileID: "exec",
|
||||||
|
SessionID: " repair-session ",
|
||||||
|
Inputs: singleInputRef(),
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("expected no error, got %v", err)
|
||||||
|
}
|
||||||
|
if llmClient.calls != 2 {
|
||||||
|
t.Fatalf("expected initial generation and one repair, got %d calls", llmClient.calls)
|
||||||
|
}
|
||||||
|
if llmClient.lastReq.Prompt.SessionID != "repair-session" {
|
||||||
|
t.Fatalf("expected repair generation to retain effective session, got %q", llmClient.lastReq.Prompt.SessionID)
|
||||||
|
}
|
||||||
|
if result.SessionID != "repair-session" {
|
||||||
|
t.Fatalf("expected result to retain effective session, got %q", result.SessionID)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestRunnerRunJSONSchemaRepairCarriesStructuredOutputSpec(t *testing.T) {
|
func TestRunnerRunJSONSchemaRepairCarriesStructuredOutputSpec(t *testing.T) {
|
||||||
def := promptDef(domain.FormatJSON, domain.ValidationJSONSchema, 1)
|
def := promptDef(domain.FormatJSON, domain.ValidationJSONSchema, 1)
|
||||||
def.Validation.SchemaPath = "events.schema.json"
|
def.Validation.SchemaPath = "events.schema.json"
|
||||||
|
|||||||
Reference in New Issue
Block a user