Files
promptkit/docs/roadmap/implementation.md

379 lines
17 KiB
Markdown

# Optional Request-Parameter Omission Implementation Plan
**Status:** Ready for implementation.
## Purpose
This document is the decision-complete implementation plan for
[omitting unset optional request parameters](optional-request-parameters.md).
It is written for a `gpt-5.6-terra` coding agent that will implement each stage
in order.
The feature roadmap owns the motivation, policy choices, compatibility
boundary, non-goals, and target end state. This document owns the concrete
design, file-level work, test ownership, documentation updates, validation,
and completion gates.
The separate [application fallback profiles](fallback-profiles.md) roadmap is
not part of this implementation plan. Preserve it unchanged for its own later
planning and implementation cycle.
## Implementation Rules
- Complete the stages in order. Stage 1 must leave code, tests, GoDoc, and
current-state documentation mutually accurate; Stage 2 performs the final
audit and repository-wide acceptance.
- Preserve unrelated working-tree changes. In particular, do not edit,
implement, retire, or reclassify `fallback-profiles.md`.
- Follow every policy under `docs/policy/`, the task-specific reading guide in
`docs/development.md`, and the accepted behavior in
`optional-request-parameters.md`.
- Keep the existing package boundaries. Framework defaults remain in
`internal/defaults`, resolution remains in `internal/usecase`, and outbound
OpenAI-compatible serialization remains in `internal/llm`.
- Do not add an exported type, field, option, method, error, or public package.
This feature changes default and wire semantics within existing contracts.
- Do not replace numeric profile fields with pointers or add profile presence
tracking. File and in-memory profile zero values retain their existing
inheritance semantics; runtime pointer overrides remain the only supported
way to select an explicit numeric zero.
- Preserve required request fields, session IDs, structured output,
credentials, extra-parameter validation, reasoning clearing, deadlines,
capacity management, and all existing precedence rules.
- Do not query a provider for defaults or capabilities and do not add
backend- or model-specific serialization branches.
- Keep tests lean and behavioral. Use the existing root precedence test to own
resolved public/injected-client metadata and the existing model-client tests
to own wire inclusion and omission. Do not duplicate those matrices in a
new end-to-end fixture.
- Update exact exported semantics in GoDoc, profile/default semantics in
`docs/formats.md`, and provider request-body semantics in
`docs/integrations/openai-compatible-chat.md` in the same stage as the code.
- Do not add release notes, change a module version, commit, tag, push, or
publish a release as part of this plan.
## Fixed Design
### Framework Defaults
In `internal/defaults/defaults.go`, remove these constants:
```go
ExecutionDefaultTemperature
ExecutionDefaultMaxTokens
ExecutionDefaultTopP
```
They currently encode zero for `temperature` and `max_tokens` and one for
`top_p`. Optional provider controls are no longer framework defaults, so
retaining zero-valued constants under default-oriented names would obscure the
new contract.
Keep `ExecutionDefaultTimeoutSeconds` at its current positive value. Timeout is
a Promptkit-owned generation deadline and is not an OpenAI-compatible request
body field.
Keep `ExecutionTargetDefault` as the common resolution baseline, but have it
initialize only `TimeoutSeconds`. The zero Go values for `Temperature`,
`MaxTokens`, and `TopP` then represent unspecified provider controls. Do not
rename this internal function or add a second defaults constructor.
### Resolution And Public Metadata
Do not change the merge functions or precedence in
`internal/usecase/runner.go`:
1. the baseline target supplies only the Promptkit timeout;
2. nonzero profile numeric fields replace the baseline;
3. non-nil runtime numeric overrides replace profile values; and
4. `ExecutionTargetPresence` records runtime overrides, including explicit
zero values.
Consequently, when a profile omits the optional provider controls, resolved
`ExecutionTarget` values contain zero for `Temperature`, `MaxTokens`, and
`TopP`. That zero is stable metadata for “unspecified” unless the accompanying
`GenerateRequest.TargetPresence` bit reports an explicit runtime zero.
Do not expose target presence in `PreparedRun`, `RunResult`, or
`ProfileInspection`, and do not change their stable JSON shapes. As already
true for `max_tokens`, those metadata values report the resolved numeric value
rather than provenance. A prepared result containing `top_p: 0` therefore does
not distinguish an unspecified value from an explicit runtime zero; injected
clients receive the separate presence value when the distinction affects
execution.
Update root GoDoc in `types.go` so it no longer calls an unspecified optional
provider control an effective provider value:
- `ExecutionTarget.Temperature`, `MaxTokens`, and `TopP` must each state that
zero leaves the field unspecified to compatible providers unless the
corresponding `ExecutionTargetPresence` bit is true;
- `ExecutionTarget.TimeoutSeconds` retains its existing deadline semantics;
- `Profile` and `ExecutionTargetOverride` documentation must describe zero or
nil as inheriting a lower-precedence value and otherwise leaving the provider
control unspecified, rather than implying that every field receives a
concrete framework value; and
- `PreparedRun`, `ProfileInspection`, and other effective-target summaries may
continue to describe precedence, but must not imply that Promptkit knows a
provider's omitted default.
Do not change field types, field order, JSON tags, conversion functions, string
formatting, or copying behavior.
### Outbound Request Semantics
The current built-in client already has the required mechanism:
`openAIChatRequestFromGenerateRequest` includes `temperature`, `max_tokens`, or
`top_p` when the resolved value is nonzero or the corresponding target-presence
bit is true, and `openAIChatRequestPayload` omits nil fields. Preserve that
logic.
No production change should be needed in
`internal/llm/openai_compatible_client.go`. Change it only if a focused failing
test demonstrates that the existing implementation does not meet this plan;
do not special-case `top_p`, inspect profile provenance, or move framework
default policy into the transport.
The resulting behavior is:
- an omitted profile `top_p` resolves to zero and is absent from the body;
- a nonzero profile or runtime `top_p` is included;
- an explicit runtime `top_p` of zero is included because presence is true;
- the same rules continue to apply to `temperature` and `max_tokens`;
- empty `service_tier` and effective `reasoning_effort` remain absent;
- configured `extra_params` remain present after validation; and
- `model`, `messages`, conditional `session_id`, and conditional
`response_format` remain unchanged.
### Profile Formats And Built-In Profiles
Do not change YAML or public `Profile` field shapes. Numeric zero in a file or
in-memory profile continues to mean “do not replace the lower layer.” With no
lower provider value, zero therefore resolves to unspecified. An explicit
profile-level numeric zero remains unsupported; consumers use a runtime
pointer override when zero itself must be sent.
Do not edit files under `internal/profile/builtin/assets/`. Values declared in
those files are explicit profile policy and remain effective. Existing
nonzero-profile tests are sufficient to protect explicit inclusion; do not add
one test per built-in asset or parameter.
### Test Ownership
Use these existing boundaries:
- In `engine_test.go`, update the “framework defaults” row of
`TestEngineExecutionSettingPrecedence` so the zero-valued profile expects
`TopP: 0` while retaining `Temperature: 0`, `MaxTokens: 0`, and the positive
timeout. Rename that row to describe unspecified provider controls plus the
framework timeout. Keep the rows proving nonzero profile precedence and
explicit runtime-zero presence unchanged.
- Remove
`TestRunnerRunBuiltInDefaultsUsedWhenProfileOmitsOptionalFields` from
`internal/usecase/runner_test.go`. Its literal-default assertions duplicate
the stronger assembled root precedence test and depend on the internal
constants being removed. Do not replace it with another internal
default-value test.
- Keep
`TestOpenAICompatibleClientOmitsImplicitZeroNumericFields` and
`TestOpenAICompatibleClientSerializesExplicitZeroNumericOverrides` in
`internal/llm/openai_compatible_client_test.go`. Together they own the wire
distinction and should pass without weakening their assertions.
- Keep the existing nonzero request serialization and profile-precedence tests
passing. They prove that explicitly configured values continue to be sent
and selected.
Do not add snapshots, golden files, provider calls, or a broad duplicate
integration test. Add a new test only if the implementation exposes a distinct
contract risk not covered by the tests above, and record that reason in the
test name or nearby test structure rather than in a new planning document.
### Canonical Documentation
Update current-state documentation in Stage 1:
- In `docs/formats.md`, replace the optional provider-control entries in the
framework-default table with clear unspecified/omitted semantics, while
retaining the positive `timeout_seconds` framework default. Explain that
profile numeric zero inherits a lower layer and otherwise remains
unspecified; an explicit runtime pointer zero is retained.
- In `docs/integrations/openai-compatible-chat.md`, state that
`temperature`, `max_tokens`, and `top_p` are included only when selected by a
profile or runtime override, including explicit runtime zero, and are absent
when unspecified. Keep the existing ownership of required fields,
`session_id`, structured output, extra parameters, and timeout behavior.
- In `types.go`, apply the GoDoc changes described above; these declarations
own the exact public value semantics.
Do not add a README or release-document note. The consumer guide already
routes exact field behavior to GoDoc and profile/default behavior to the format
reference, so do not duplicate the new contract there. The internal LLM
document describes flow rather than exact field omission and does not require
a change unless its current text is found to contradict the implementation.
## Stage 1: Implement Omission Semantics And Canonical Contracts
### Objective
Remove optional provider controls from the framework baseline, preserve
explicit profile and runtime values, update the canonical contracts, and prove
the behavior at the existing resolution and wire boundaries.
### Implementation Prompt
Implement only Stage 1 of `docs/roadmap/implementation.md`. Read the complete
feature roadmap, implementation rules, and fixed design above before editing.
1. In `internal/defaults/defaults.go`, remove the three provider-control
constants and make `ExecutionTargetDefault` initialize only
`TimeoutSeconds`.
2. In `engine_test.go`, update and rename the default-precedence table row
exactly as described under Test Ownership.
3. Remove the redundant literal-default test from
`internal/usecase/runner_test.go`; do not weaken other precedence,
profile-value, or runtime-zero tests.
4. Update the affected exported GoDoc in `types.go` without changing any
declaration, JSON tag, or serialization shape.
5. Update `docs/formats.md` and
`docs/integrations/openai-compatible-chat.md` according to Canonical
Documentation.
6. Run the focused validation below. Repair regressions in scope, but do not
broaden the feature or change the established serializer merely to make a
mistaken expectation pass.
Do not edit built-in profile assets, fallback-profile work, backend
registration, profile parsing, target merge logic, public value shapes,
prepared-execution lifecycle, capacity management, or release material.
### Focused Validation
Run from the repository root:
```sh
gofmt -w internal/defaults/defaults.go types.go engine_test.go \
internal/usecase/runner_test.go
go test . -run 'TestEngineExecutionSettingPrecedence'
go test ./internal/llm -run \
'TestOpenAICompatibleClient(GenerateSuccess|OmitsImplicitZeroNumericFields|SerializesExplicitZeroNumericOverrides)'
go test ./internal/usecase -run \
'Test(ResolveExecutionTarget|RunnerPrepareRequestNumericOverridePresence|RunnerPrepareSelectedProfileBeatsBuiltInDefault|RunnerRunSelectedProfileBeatsBuiltInDefault)'
go test . ./internal/defaults ./internal/usecase ./internal/llm
go vet . ./internal/defaults ./internal/usecase ./internal/llm
git diff --check
```
If a focused regular expression does not match an existing test name, inspect
the current names and run the narrowest equivalent set; do not silently skip
the intended resolution, explicit-profile, explicit-zero, and wire-omission
coverage.
### Completion Gate
Stage 1 is complete only when:
- the resolution baseline contains no provider tuning value and retains the
Promptkit timeout;
- an omitted `top_p` resolves to zero and the built-in client omits it;
- nonzero profile and runtime values remain effective and serialized;
- explicit runtime zero values remain distinguishable and serialized through
`ExecutionTargetPresence`;
- no public type or stable JSON shape changed;
- required fields, structured output, session IDs, extra parameters,
reasoning, credentials, and deadlines retain their existing behavior;
- GoDoc, format documentation, and the integration contract describe the
implemented behavior without conflicting ownership; and
- all focused tests, vet, formatting, and whitespace checks pass.
## Stage 2: Audit Compatibility And Validate The Repository
### Objective
Confirm that the narrow semantic change is complete across all public,
injected-client, built-in-profile, documentation, and repository surfaces,
then mark the temporary planning documents complete.
### Implementation Prompt
Implement only Stage 2 of `docs/roadmap/implementation.md` after Stage 1
satisfies its completion gate.
1. Search tracked Go and Markdown files for the removed constant names,
framework `top_p` defaults, claims that all effective provider controls have
concrete framework values, and request-body inclusion rules. Correct only
stale statements or tests owned by this feature.
2. Confirm that `internal/profile/builtin/assets/` has no feature-related diff
and that its explicit nonzero optional controls still pass ordinary profile
validation and resolution tests.
3. Confirm that `internal/llm/openai_compatible_client.go` either has no diff or
contains only a change required by a focused failing contract test. The
default policy must remain outside the transport.
4. Follow every changed Markdown link and confirm its target exists. Verify
that current-state documents describe implemented behavior and that exact
contracts remain with GoDoc, the format reference, and the integration
contract.
5. Run the complete validation sequence below and repair only in-scope
failures.
6. After all checks pass, change the status of
`optional-request-parameters.md` and this document to `Complete`. Do not
change the status or contents of `fallback-profiles.md`.
7. Re-run `git diff --check` and inspect the final working tree and diff.
Do not delete temporary roadmaps in this stage; retirement is a separate
maintainer action. Do not add release notes, change versions, or create a
commit, tag, push, or release.
### Full Validation
Run from the repository root:
```sh
gofmt -w internal/defaults/defaults.go types.go engine_test.go \
internal/usecase/runner_test.go
gofmt -l $(git ls-files '*.go')
go test ./...
go test -race ./...
go vet ./...
go build ./...
go run ./examples/go-library/prepare
git diff --check
git status --short
```
The `gofmt -l` command must print no paths. The maintained example must remain
offline and require no real credential or provider.
Inspect the final diff and confirm:
- only this feature's files and pre-existing user changes are present;
- no built-in profile asset, public declaration shape, stable JSON tag,
credential rule, workspace file, local module replacement, generated binary,
or unrelated formatting changed;
- the removed provider-default constants have no remaining references;
- the provider omission policy is implemented by resolution plus the existing
generic serializer, not by a `top_p` transport special case;
- the optional-parameter roadmap and this plan are complete while the fallback
roadmap remains selected; and
- no commit, tag, push, or release was created.
### Completion Gate
The implementation is complete only when:
- every Stage 1 gate remains satisfied;
- the ordinary and race-enabled suites pass;
- vet, build, formatting, the maintained offline example, Markdown links, and
whitespace checks pass;
- public metadata, injected-client presence, profile inheritance, and outbound
omission semantics are mutually consistent;
- explicit built-in and consumer profile values retain their behavior;
- both feature-specific roadmap statuses are `Complete`;
- `fallback-profiles.md` remains unchanged and selected for later work; and
- the repository is ready for maintainer review without a commit or release
having been created by this plan.
## Open Questions
None. The feature roadmap and fixed design above fully specify the behavior,
compatibility boundary, implementation, documentation ownership, and test
strategy.