diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index c0e2779..b280d17 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -1,473 +1,523 @@ -# Effective Prompt Requirement Inspection Implementation Plan +# Computational Efficiency Implementation Plan -Implement the [feature roadmap](inspection.md) in order. Each numbered stage is -scoped for one Luna coding-agent prompt. This plan replaces the earlier -interpreter-style analysis scope with simple enumeration and conservative -classification. The completed ReleaseKit plan remains available in Git history. +Implement the remaining local work in the [performance roadmap](performance.md) +in order. Each sequential stage is scoped for one Luna coding-agent prompt. +All stages below are pending. Update their status and evidence as work lands. +The completed inspection plan is preserved at +`481e2d4:docs/roadmap/implementation.md` in Git history. -## Baseline And Working Rules +## Baseline And Scope -- Planning baseline: `e459c7d`. The inspection API is not implemented. - `InspectPrompt` reports declarations, `Prepare` needs artifacts, and the - configured engine already captures compiled templates. -- Start with [development](../development.md), then the - [agent](../policy/agents.md), [architecture](../policy/architecture.md), - [testing](../policy/testing.md), and [documentation](../policy/documentation.md) - policies. Reuse unchanged policies already read. -- Follow [ADR 0001](../adr/0001-configuration-boundary.md): interpret only final - effective configuration at the ConfigKit handoff. Operator provenance is - separate; inspection does not examine raw sources or intermediate values. -- Preserve unrelated work and commit completed, verified units under the agent - policy. Keep implemented internal documentation accurate as stages land. -- Preserve existing construction, inspection, preparation, rendering, and - execution contracts. An inspection limitation is not a definition error. -- Use captured parsed syntax, direct traversal, and maps keyed by exact name. - No general evaluator, assignment-value propagation, branch-environment joins, - possible-name sets, branch combinations, analysis budget, or global cache is - needed. Do not add caller limits, defaults, input schemas, or authoring bans. -- The root package owns public declarations/conversion. `internal/prompt` - owns discovery, `internal/usecase` coordinates selection, and - `internal/configuration` supplies captured syntax/metadata. Runtime/domain - values remain independent of ConfigKit. -- No new dependency, upstream ConfigKit change, provider access, CLI/HTTP - feature, application wire format, or publication is part of these stages. +- Planning baseline: `481e2d494c5a4e8b7f3a2a6f1cceaeb3f7c71d05`. +- Roadmap item 1 is complete. ConfigKit received the performance request in its + `docs/roadmap/promptkit-performance-request.md`, commit `d53021e`. Do not + resubmit it, implement upstream changes, wait for a release, or upgrade the + dependency as part of this plan. A later ordinary dependency update will + pick up available upstream improvements. +- Roadmap items 2–5 are localized runtime improvements. Item 7 concerns + identity/metadata inspection; item 6 is conditional on ownership evidence + and measured benefit. The latter condition is part of the accepted scope, + not permission to leave the review unfinished. +- No public declarations, configuration semantics, dependencies, release + tooling, or version tags need changing. This is an internal optimization + effort, not a new consumer performance contract. -## Concrete Public Values +## Working Rules -Add root `requirements.go` in Stage 5: +Start with [development](../development.md) and select applicable policies. +Read the [agent](../policy/agents.md), +[architecture](../policy/architecture.md), +[testing](../policy/testing.md), and +[documentation](../policy/documentation.md) policies; reuse unchanged policies +already read. Consult the [component overview](../internal/overview.md) and the +focused documents named below. -```go -func (e *Engine) InspectRequirements( - ctx context.Context, req RequirementInspectionRequest, -) (*RequirementInspection, error) +Preserve the [configuration handoff ADR](../adr/0001-configuration-boundary.md). +Use captured final configuration; operator provenance stays out of ordinary +execution. Keep request/result ownership, error identities, exact selection, +prepared-handle lifecycle, cancellation, and generation/repair behavior intact. +Retain indexed lookup, captured parses, per-render text reuse, operation-local +schema reuse, frozen transport retries, and constant-time queue removal. -type RequirementInspectionRequest struct { - PromptID string - PromptVersion string - ProfileID string - Vars map[string]string - SessionID string -} +Use standard-library mechanisms. Add no global cache, writer pool, borrowed +public result, new execution/input limit, or performance release gate. These +are scope boundaries, not new permanent product prohibitions. Internal helper +names and allocation choices below are implementation decisions, not public +invariants. Tests must not freeze a particular capacity, clone count, internal +call sequence, or exact allocation/timing number. -type RequirementKind string // values: required, conditional, optional +For each stage, inspect the current tree, preserve unrelated work, implement +only that stage, format changed Go files, run its focused checks, review the +diff, and commit the completed unit with a plain-English message. Update owning +internal documentation when its mechanics change. Do not widen scope to +unrelated cleanup or retire other roadmaps. Publication is a separate task. -type InputRequirement struct { - Name string - Kind RequirementKind - Declared bool - ContentType *string - Description *string - Explanation string -} +## Measurement And Evidence Method -type VariableRequirement struct { - Name string - Kind RequirementKind - Supplied bool - Explanation string -} +Add maintained benchmarks before changing the behavior they measure. Record +the benchmark-only baseline commit so the same fixture can be rerun later. +Keep setup, source acquisition, template compilation, fixture creation, and +engine construction outside timing unless the benchmark explicitly measures +that operation. Use `ReportAllocs`, retain a result in a sink where necessary, +and check representative output outside timing. Run benchmark comparisons +without the race detector. -type RequirementIssueCode string +The default comparison command is: -type RequirementIssue struct { - Code RequirementIssueCode - InputName string - VariableName string - Description string -} - -type RequirementInspection struct { - PromptID string - PromptVersion string - ProfileID string - Inputs []InputRequirement - Variables []VariableRequirement - Complete bool - Issues []RequirementIssue -} +```sh +GOTOOLCHAIN=local GOWORK=off GOPROXY=off \ + go test -mod=readonly -run '^$' -bench 'BENCHMARK_PATTERN' \ + -benchmem -benchtime=500ms -count=5 PACKAGE ``` -Use root-owned values and internal counterparts in -`internal/domain/requirements.go`; do not expose internal aliases. -Kind constants are `RequirementRequired`, `RequirementConditional`, and -`RequirementOptional`. Retain four useful issue categories: +Replace the pattern and package with each stage's values. Dependencies and the +selected Go toolchain must already be installed for this offline invocation. +Use the same environment and unchanged fixture for before/after comparisons. +Record revisions, Go/module versions, processor, parallelism, command, repeated +results, and a short interpretation in this plan's execution evidence. Compare +ranges and medians; no benchmark-analysis dependency is required. Keep raw +profiles and generated binaries outside the repository. -| Constant | Value | Discovery limitation | -| --- | --- | --- | -| `RequirementIssueMissingVariable` | `missing_variable` | An absent value prevents determining a dependency decision/name. | -| `RequirementIssueInputContent` | `input_content` | Unavailable artifact content affects dependencies. | -| `RequirementIssueDynamicInputName` | `dynamic_input_name` | A referenced input name cannot be determined. | -| `RequirementIssueUnsupportedConstruct` | `unsupported_construct` | Analysis does not cover a dependency-relevant expression or scope. | +For changes affecting buffer/string/handle lifetimes, also compare retained +heap with a documented, fixed synthetic workload: hold a small batch of +results or handles, force GC, compare live heap against an equivalent setup-only +baseline, and use `runtime.KeepAlive` for the retained objects. An opt-in +benchmark is sufficient; do not add heap/timing assertions to ordinary tests. +Distinguish immutable shared strings from mutable deep copies. Report noisy +retained-heap evidence honestly rather than deriving it from `B/op`. -An empty associated name means that no particular entry was identified. -Descriptions and explanations are safe human-readable summaries, not strings -callers must parse. There is no expression tree, source offset, JSON tag/schema, -or stable diagnostic wording contract. +Probes from the original review are preliminary evidence. Maintained fixtures +and recorded comparisons own the implementation evidence. Retain improvements +when measurements support the intended reduction without a material correctness, +retention, or maintenance trade-off; the original percentages are not targets. -### Classification And Ownership +## Stage 1 — Establish Validation And Captured-Rendering Benchmarks -- Seed declared inputs from final effective definitions. Required declarations - stay required independently of syntax. Others start optional. -- Collect occurrences with definite, possible, or inactive reachability. - Definite helper/direct required-variable accesses are required; possible ones - are conditional; recognizable inactive references are optional. A lookup - that execution permits to be absent remains optional unless another access - establishes a stronger requirement. -- Aggregate exact names with required > conditional > optional. No proof that - a name in both unresolved branches is required is necessary: conditional is - acceptable until an independent definite access/declaration establishes it. -- Include recognizable inactive inputs and caller variables in both inventories. - Give inactive-only entries an `Explanation`, independent of declared input - `Description`. Any applicable occurrence, including an optional lookup, - removes the inactive-only explanation; required/conditional uses also - override optional classification. Inactivity is not an uncertainty issue. -- Input metadata pointers distinguish absence/null from an explicit string, - including an empty string. `Declared` distinguishes metadata ownership from - a helper-only reference. Do not infer new input formats. -- `Supplied` means presence of the exact caller key, including empty strings. - It is independent of classification and completeness; do not echo values. -- Sort input and variable entries lexically by exact name. Deduplicate issues - by code/input-name/variable-name and sort by that tuple. Do not promise a - nil-versus-empty slice distinction. -- Return independent slices and metadata pointers, copy request variables at - the facade as for `RunRequest`, and keep captured syntax immutable. Retain - callers' existing responsibility for concurrently mutating requests. +**Outcome:** Reproducible baselines for roadmap items 2 and 3; no production +change in this stage. -### Deliberately Small Recognition Scope - -Use the standard template parse tree and `template.IsTrue` for known values. -Only determine a value when needed for a recognized name or simple condition. - -| Syntax | Initial handling | -| --- | --- | -| Literal `input "name"` | Collect the exact name without reading its body. Parenthesized literal wrappers and a literal piped directly to `input` are equivalent. | -| Direct root `.name` or `$.name` | Collect a required caller key when active; supplied strings are known, absence is unresolved rather than an inspection error. | -| `index` of the root map with one literal string key | Collect an optional lookup; its value is supplied text or the string zero value if absent, following execution. | -| `input .name`, `input $.name`, or `input (index . "name")` | Resolve a directly available string as an input identifier on applicable paths. | -| One simple value piped directly to `input`, or parenthesized simple expressions | Recognize the same bounded forms without evaluating arbitrary pipelines. | -| String/Boolean/nil literals and simple direct values | Use known values for conditions; no coercion of caller strings. | -| `not` of a simple predicate; two-string `eq`/`ne` | Recognize negation and ordinary equality/inequality without adding the full comparison family. | -| `if`/`else`, `with` | Classify branch occurrences using a recognized condition; unresolved branches are possible, not guesses. | -| Locals, changed dot, and root binding | Retain scope identity sufficient to distinguish root-map accesses from local/string/unknown values; no local-value propagation is required. | -| Other pipelines/functions, `and`/`or`, assignments, ranges, invoked named templates, or complex names | Collect safely identifiable references conservatively and record dependency-relevant limitations; do not interpret their execution. | - -An unsupported expression does not automatically make every argument a definite -access: short-circuiting or unknown scopes can make references merely possible. -Keep complex forms outside complete analysis instead of emulating their rules. -Do not interpret `$local` as a caller key or `.field` on a string/unknown dot as -a root lookup. Explicit root accesses remain recognizable only while the root -binding's identity is known. Unsupported changes to a binding make its identity -unknown where affected; no assignment-value or cross-branch reconstruction is -needed. - -Walk known inactive subtrees for syntactically identifiable literal input names -and caller keys. Do not evaluate skipped expressions, resolve skipped dynamic -input names, or produce uncertainty solely from skipped constructs. Names -hidden behind unsupported scopes need not be invented. Exclude bypassed session -templates and never-invoked definitions entirely. - -### Simple Algorithm And Completeness - -1. Seed declaration entries and visit each selected template. -2. Maintain only reachability and the scope identity needed to recognize caller - keys; collect names and minimal classification evidence in indexed summaries. -3. For simple conditions, visit both branches with definite/possible/inactive - reachability as appropriate. A possible parent never becomes definite just - because an inner predicate is known true; a known inactive parent stays - inactive. No branch environment or assignment-value joins are needed. -4. Aggregate/finalize names, metadata, presence, explanations, and limitations. - -Use shared traversal helpers for active/possible/inactive syntax rather than -separate interpreters. Do not enumerate execution combinations. Prefer a direct -pass through syntax and final aggregation; no special machinery is warranted -by ordinary prompt sizes. - -`Complete` means that the supported analysis established the requirement -inventory/classifications. Conservative false is acceptable for difficult cases. -Missing direct text-substitution values alone do not make discovery incomplete. -Unresolved branches with dependency-bearing references return conditional -entries and incomplete status; no proof of identical branches is required. -Unknown pure text without a dependency effect need not change completeness. - -An unsupported call that may hide dependencies or an unresolved active input -name makes discovery incomplete. Preserve recognized names; do not create a -wildcard entry. Maintain simple flags for possible unknown input/caller-key -effects, promoting affected optional entries to conditional at finalization, -rather than repeatedly rescanning the inventory. Known required entries remain -required. Clear inactive-only explanations where those unknown effects undermine -the conclusion. - -A complete empty or all-optional input inventory needs no artifact bindings. -An incomplete inventory does not prove that no others are needed. A complete -result is not a rendering/readiness/provider guarantee. - -## Stage 1 — Capture Effective Metadata And Template Snapshots - -**Status: Complete (2026-10-08).** Effective input metadata presence and -construction-time templates are available through an internal snapshot without -changing prompt hashes or the existing inspection shape. - -**Outcome:** The analyzer can obtain existing compiled templates and declaration -metadata without artifact reads or changing old inspection/execution behavior. - -**Read:** `internal/configuration/load.go`, `prompts.go`, -`internal/prompt/template.go`, `internal/domain/domain.go`, -`internal/promptdef/hash.go`, and the internal source document. +**Read:** `internal/validate/standard_validator.go`, its tests, +`internal/prompt/go_renderer.go`, `template.go`, renderer tests, and +[internal sources](../internal/sources.md). **Work:** -1. Retain content-type/description presence alongside configuration - `promptEntry`, rather than changing the hashed prompt definition or old - public inspection shape. -2. Capture effective per-input presence from `Resolution.InputInfo` during the - existing Resolve callback before prompt normalization. Use final - prompt-ID/version/input-index paths, following the existing presence-capture - pattern. Materialize values from the normalized final declaration. -3. Add `prompt.RequirementSnapshot` with captured templates and metadata, and - narrow internal `GetRequirementSnapshot(id, version string) - (RequirementSnapshot, bool)` access on the captured configuration adapter. - Use selected exact keys, without reparsing or reopening resources. -4. Preserve renderer/constructor paths and update internal source documentation. +1. Add `BenchmarkBasicValidation` with short text, 1 MiB nonempty text, + whitespace-only text, and Unicode whitespace. Construct bodies/contracts + outside timing and exercise the real basic validator. +2. Add `BenchmarkCompiledRendering` using `CompileTemplate` and a small + `TemplateLookup` fixture built before timing. Exercise + `NewCompiledRenderer`, not only `NewGoRenderer`'s reparsing fallback. +3. Cover a small message, an artifact-backed message around 256 KiB, and + repeated references across session/messages. Keep session output within the + existing session contract, e.g. render an input length rather than its body. + Retain the existing fallback benchmark where useful; label the paths clearly. +4. Add opt-in `BenchmarkRetainedRenderedOutput` using the evidence method + above. Capture baselines and record the benchmark commit for later comparison. -**Verify:** Focused final-value metadata tests for absent/null, empty, and -nonempty values, plus equivalent-definition hashes and old inspection behavior. -Use actual configuration capture rather than another source-loading matrix. -Run `go test ./internal/configuration ./internal/promptdef ./internal/prompt`; -commit. +**Verify:** `go test ./internal/validate ./internal/prompt`; compare the two new +benchmark families with the standard command. Benchmarks have no provider +calls, allocation assertions, or source parsing hidden in the compiled path. -## Stage 2 — Enumerate Literal Inputs And Caller Keys +## Stage 2 — Check Basic Output Emptiness Without A String Copy -**Status: Complete (2026-10-08).** The internal walker enumerates declarations, -literal input references, and supported caller keys with conservative branch -and unsupported-scope classifications. - -**Outcome:** An internal syntax walker produces deduplicated inventories and -does not confuse caller variables with template locals. +**Outcome:** Basic validation avoids a response-sized temporary string. **Work:** -1. Add internal result counterparts in `internal/domain/requirements.go` and - an analyzer entry in `internal/prompt/requirements.go`, taking context, - selected definition/snapshot, supplied variables, and session bypass. -2. Seed declarations, visit executable message roots and the applicable session - root, and collect literal input references and direct/literal-index caller - keys. Exclude uncalled definitions and a bypassed session template. -3. Implement aggregation, exact names, supplied-key presence, independent - metadata, sorting, and safe issues/explanations. Add ordinary traversal - cancellation checks without stronger interruption promises or workers. -4. Track scope identity, not local values: root-map versus nonroot/unknown dot - and local bindings. For unsupported scope changes, keep safely recognizable - references and report incomplete analysis rather than inventing caller keys. -5. Until Stage 3, classify branch references conservatively as possible and - unsupported dependency-bearing expressions as limitations. -6. Update the implemented internal component/source descriptions. +1. Replace `strings.TrimSpace(string(artifact.Body)) == ""` in the basic branch + with `len(bytes.TrimSpace(artifact.Body)) == 0`. Remove imports only if unused. +2. Preserve statuses, `IsValid`, repair input, error handling, and the existing + before/after cancellation checks. Do not change the other validation modes. +3. Reuse existing behavioral tests. Add only missing meaningful cases for empty + bytes, ASCII/Unicode whitespace, and nonempty bytes, preserving behavior for + arbitrary bytes rather than adding a UTF-8 requirement. +4. Compare `BenchmarkBasicValidation` against Stage 1 and record the result. -**Verify:** Literal declared/undeclared input names, repeated references, -required unused declarations, optional declarations/lookups, missing versus -empty direct variables, root versus local references, and empty input-free -templates. Test unsupported-scope uncertainty, not assignment interpretation. -Run `go test ./internal/prompt ./internal/domain`; commit. +**Verify:** `go test ./internal/validate ./internal/usecase`; run the benchmark +family. Existing empty-output repair behavior remains unchanged. -## Stage 3 — Classify Simple Branches And Inactive References +## Stage 3 — Render Messages And Sessions With Local String Builders -**Status: Complete (2026-10-08).** Supported `if`/`with` conditions classify -selected paths, while recognizable skipped-path references remain optional -inventory entries without introducing uncertainty. - -**Outcome:** Ordinary supplied-context branches give useful classifications; -inactive inputs and variables remain optional inventory. +**Outcome:** Template output becomes a string without a final buffer-to-string +copy. **Work:** -1. Add only the simple condition recognition in the scope table: direct values, - literal-key lookups, literals, negation, and two-string equality/inequality. - Use Go truthiness; a supplied `"false"` string is truthy. -2. For `if`/`with`, collect the predicate's references under the parent's - reachability. Visit selected and skipped bodies with the appropriate - reachability and dot identity. In unresolved branches, mark accesses possible. -3. Collect syntactically recognizable inactive input/caller names without - evaluating skipped expressions. Add inactive-only explanations; required - declarations or other definite/possible uses take precedence. -4. Unknown/content-dependent conditions preserve known entries and limitations. - An unresolved text-only branch need not be incomplete; dependency-bearing - unresolved branches can remain incomplete without cross-branch proofs. -5. Do not add assignment environments, alias-value propagation, branch joins, - more comparison families, or short-circuit evaluation for complex conditions. +1. Replace the local message `bytes.Buffer` in `Render` and the session buffer + in `executeSessionTemplate` with local `strings.Builder` values. Execute + templates into their addresses and obtain `String()` afterward. +2. Keep template cloning/binding, error checks, cache metadata, ordering, and + cancellation placement. Leave artifact-text conversion/cache unchanged. + Do not introduce eager growth based on all artifacts or shared writers. +3. Run existing exact-content, session, cancellation, and compiled-template + concurrency coverage. Add a behavioral case only if the changed path lacks + protection; do not test builder internals. +4. Compare Stage 1's compiled-rendering and retained-output measurements. + Check both allocated bytes and live output storage; explain any retention + trade-off before retaining the change. Update internal sources if needed. -**Verify:** Known true/false branches, nested reachability, string truthiness, -simple comparisons, `with` dot versus explicit root, missing decision values, -and content-dependent conditions. Include inactive literal input and caller -references, promotion by active reuse, and declared required inputs. -Confirm skipped unsupported/dynamic expressions do not introduce uncertainty -and an all-optional complete inventory needs no artifacts. -Compare a few supported cases with the real renderer. -Run `go test ./internal/prompt`; commit. +**Verify:** `go test -race ./internal/prompt`; run compiled-rendering benchmarks +without `-race`. Different request inputs must remain independent. -## Stage 4 — Resolve Direct Names And Finish Limitation Reporting +## Stage 4 — Reserve A Useful Initial File-Read Capacity -**Status: Complete (2026-10-08).** Supported caller-derived input names resolve -exactly; unresolved names preserve recognizable dependencies and report -structured limitations, with affected optional entries conservatively marked -conditional. +**Outcome:** Ordinary file reads avoid early geometric buffer growth without +trusting metadata as the amount of content to return. -**Outcome:** Simple supplied names resolve exactly; complicated names and -constructs return useful partial inventories without speculative analysis. +**Read:** `internal/artifact/reader.go`, `reader_test.go`, and internal sources. **Work:** -1. Recognize the direct variable/lookup-derived helper names, simple - parentheses, and direct-to-input pipelines in the scope table. - Use a known string as an identifier without trimming or coercion. -2. Missing decision/name values, input-body dependence, unresolved helper - names, and dependency-relevant unsupported syntax produce the four issue - categories. Collect recognized references conservatively from unsupported - subtrees without executing functions, loops, or named calls. -3. Keep skipped computed input names unresolved without inventing an entry or - marking incompleteness from inactivity alone. The syntactically visible - caller key can still appear as an optional variable. -4. Finalize unknown-name/scope effects on affected optional entries using the - simple flags described above. Required declarations/definite accesses stay - required; uncertainty overrides misleading inactive-only explanations. -5. Finish independent result values and safe diagnostics. No possible-name - sets, underlying expression/value traces, placeholder names, or payload reads. +1. Add `BenchmarkArtifactFileRead` using real temporary regular files of small, + 1 MiB, and larger representative sizes. Create files outside timing, warm + content-type lookup, then measure the real reader. Record a baseline before + production edits. +2. Reserve initial capacity from the opened file's size. Use zero for a + nonpositive hint and `min(size, 1 MiB)` for a positive hint before conversion + to `int`. The 1 MiB reservation ceiling is an internal allocation heuristic, + not a file/content limit; data continues to grow normally beyond it. This + avoids an enormous eager allocation from stale or unusually large metadata + and keeps integer conversion safe on supported architectures. +3. Keep the read-until-EOF loop and existing regular-file, error, hash, metadata, + closure, and cancellation behavior. Do not size the result from `Stat`, use + `ReadFull` to enforce its size, impose an input limit, or add background I/O. +4. Extend the existing opened-file seam to test an overreported and an + underreported size, including a very large reported hint with small actual + content. Assert returned bytes/size/hash and failures, not backing capacity. + Add opt-in `BenchmarkRetainedArtifactFileRead` for representative live-buffer + retention, including overreported size. Compare both allocation and retention + and update internal sources. -**Verify:** Exact direct names, optional literal-key lookups, empty versus -absent supplied names, and equivalent simple parenthesized/piped forms. -Use a few complex assignment/pipeline/range/named-call examples to prove -incomplete-success behavior, not broad interpretation coverage. -Check unresolved-name effects on optional entries and absent unrelated values -in results/explanations. Run `go test ./internal/prompt`; commit. +**Verify:** `go test ./internal/artifact`; run `BenchmarkArtifactFileRead`. +Existing progress-cancellation and nonregular-file tests remain authoritative. -## Stage 5 — Wire Selection And The Public Facade +## Stage 5 — Establish Encoding Benchmarks And Error-Boundary Coverage -**Status: Complete (2026-10-08).** The root API now selects captured prompt and -profile identities, analyzes the exact captured snapshot, and returns -caller-owned requirement values without entering preparation or execution. +**Outcome:** A repeatable request-preparation baseline and focused protection +for the classifications that Stage 6 must preserve. No production change. -**Outcome:** Consumers can inspect a selected captured prompt before artifacts -exist, without execution collaborators or readiness checks. +**Read:** `internal/llm/openai_compatible_client.go`, client/prepared-generation +tests, [integration contract](../integrations/openai-compatible-chat.md), and +[internal model client](../internal/llm.md). **Work:** -1. Add `internal/usecase/requirements_inspection.go` and an internal request. - Check the nonblank prompt ID, use existing session normalization, honor - context cancellation, and select via `resolvePromptDefinition`. -2. Reuse request-profile/default-profile selection and - `resolveProfileSelection` for identity only. Share a small private selector - helper with preparation if needed, preserving its error identities/order. -3. Obtain the captured snapshot through the narrow repository capability and - invoke the analyzer. Missing required internal capability/snapshot is - operational `ErrPromptLoad`, not a successful partial result or source fallback. -4. Do not call preparation, target-completeness/credential checks, environment - lookups, artifact/schema readers, renderer, validator, admission, generation, - observations, or run-ID machinery. -5. Forward through `internal/engine` and add root `requirements.go` with the - planned values/method, conversion/copy helpers, GoDoc, and `mapPublicError`. - Nil engine follows `ErrInvalidConfig`. Operational errors return no result; - discovery limitations return an incomplete result and nil error. -6. Preserve public selection/session/cancellation error categories. Do not add - an error sentinel for incomplete discovery. Update internal runner/component - documents for the implemented method. +1. Add `BenchmarkPrepareGeneration`, measuring actual `PrepareGeneration` + without HTTP dispatch: no extras, small extras, a 1 MiB string extra, and + nested JSON-compatible extras. Build inputs and the client outside timing; + use a direct synthetic credential or no credential so environment changes + do not affect results. Include a representative structured-output request. +2. Extend existing invalid-extra tests only as needed for a nested unsupported + value or cycle, checking `ErrInvalidRequest`, useful key context, and zero + transport calls. Do not freeze full diagnostic wording or all invalid-key + combinations. +3. Cover a serializable-extra request whose schema cannot be encoded: the + client currently classifies full-request encoding failure as + `ErrRequestFailed`. Also cover invalid extras plus an unencodable schema: + invalid extras retain `ErrInvalidRequest`. This protects classification + when validation moves to a failure-only path. +4. Capture the baseline. Existing body omission, explicit-zero, cache-control, + structured-output, and frozen-attempt tests own their established behavior. -**Verify:** Focused selection tests cover explicit/default/partial profiles, -exact versions/ambiguity, direct-session bypass, cancellation, nil engine, -operational errors, and incomplete success. Use failing/counting execution -collaborators only to prove absence of the relevant effects. -Run affected use-case/engine tests and `go test ./...` for assembled facade -wiring; commit. +**Verify:** `go test ./internal/llm`; run `BenchmarkPrepareGeneration`. -## Stage 6 — Protect Public Inventory And Ownership +## Stage 6 — Encode Valid Extra Parameters Only In The Complete Request -**Status: Complete (2026-10-08).** External-package tests cover the assembled -inspection contract, captured-state ownership, and artifact-free use with real -configured engines. - -**Outcome:** Public contract coverage protects the consumer need without -duplicating the syntax walker's detailed cases. +**Outcome:** Successful request preparation does not serialize extras twice. **Work:** -1. Add external-package `requirements_inspection_contract_test.go` with real - configured engines and minimal synthetic definitions. -2. Cover exact required `transcript`, undeclared literals, declaration metadata, - simple context-selected names, and inactive input/variable entries with safe - explanations. Missing required variables and complete discovery are separate. -3. Cover conditional/incomplete results and empty/all-optional complete - inventories. A direct session ID excludes its template-only inventory - entries; declarations/message references remain. -4. Prove inspection before artifacts exist and without credentials/schema - preparation/provider access. Use a valid partial profile and, where useful, - an explicit direct-key-required profile without its key. -5. Compare a small set of supported definite requirements with later preparation - using synthetic artifacts. Optional inactive inventory is not an executed - access; incomplete cases need not satisfy complete-analysis parity. -6. Verify captured file-backed content survives later file change/removal. - Mutate returned slices/metadata and repeat inspection; exercise simultaneous - inspections alongside preparation to protect captured-state ownership. -7. Check disclosure with marker values: unrelated variable values, direct - session values, template text, and credentials stay absent. Binding names, - caller keys, declared descriptions, and safe inactive explanations are allowed. -8. Root tests own public values/errors/assembled behavior; analyzer tests own - recognition/classification. Avoid duplicated interpretation matrices. +1. Keep `openAIChatRequestPayload` responsible for body assembly and empty/ + reserved-key checks. Remove its successful-path per-value `json.Marshal` + calls; place accepted values directly into the payload map. +2. Marshal the complete assembled map once in `PrepareGeneration`. On success, + continue authentication resolution and construction of the frozen payload + exactly as before. Do not change body shapes or omission/presence semantics. +3. On complete-request marshal failure only, examine the extras with the + existing per-value marshal check to identify an invalid extra. Return the + existing `ErrInvalidRequest` classification and `extra_params.` context + for that failure. If extras are serializable, retain `ErrRequestFailed` for + the original full-request encoding failure. Cold-path repeated work is + acceptable; successful requests get the optimization. No new reflective + validator or stricter accepted-value rules are needed. +4. Adjust any helper-only tests whose incidental assembly boundary changed, + retaining protection at `PrepareGeneration`/`Generate`. Preserve frozen + request bytes across attempts and record Stage 5 comparisons. Update internal + model-client mechanics; change integration prose only enough to describe + validation before dispatch without prescribing two encodings. -**Verify:** Run focused contract/affected package tests and `-race` for -ownership cases. Reuse unchanged prior checks rather than rerunning the full -suite solely for this unit; commit. +**Verify:** `go test ./internal/llm ./internal/generation`; run encoding +benchmarks. Invalid requests still dispatch nothing, and retry tests still +exercise one frozen body per generation. -## Stage 7 — Document And Verify The Assembled Feature +## Stage 7 — Establish Inspection Benchmarks -**Status: Complete (2026-10-08).** GoDoc, consumer guidance, framework-format -cross-references, and the maintained preparation example now describe the -implemented discovery boundary. Final offline verification passed: -`go test -race -vet=off -mod=readonly -count=1 ./...`, -`go vet -mod=readonly ./...`, and `python3 scripts/check-examples.py`, with -`GOTOOLCHAIN=local GOWORK=off GOPROXY=off GONOPROXY=none GOSUMDB=off`. - -**Outcome:** Implemented semantics and analysis limits are documented in their -canonical owners, with final verification ready for user review. +**Outcome:** Owned lookup and assembled requirement-inspection baselines, with +no production change. This separates fixture work from the selection refactor. **Work:** -1. Finish GoDoc for classifications, presence, metadata, explanations, issues, - completeness, errors/ownership, and the deliberately small supported scope. - Complex valid templates may yield partial results; no authoring ban is added. -2. Update the consumer guide to compare the inspection APIs and preparation. - Show required input discovery, optional inactive input/variable inventory, - missing required variables, and explicit incomplete discovery. -3. Update relevant format cross-references and internal source/runner/component - documents. Keep exact implemented API contracts in GoDoc rather than copying - them into every guide. -4. Add a small inspection use to the maintained preparation example, preserving - its existing assertions. Verify it locally without adding another release gate. -5. Review roadmap acceptance against implementation/tests. Confirm no general - interpreter, stronger caller restriction, raw/intermediate configuration - validation, execution-readiness check, or speculative completeness promise - was introduced. -6. On final changed code, run one assembled offline verification using the local - Go installation and prepared module caches: - `go test -race -vet=off -mod=readonly -count=1 ./...`, - `go vet -mod=readonly ./...`, and - `python3 scripts/check-examples.py`. Set `GOTOOLCHAIN=local`, `GOWORK=off`, - `GOPROXY=off`, `GONOPROXY=none`, and `GOSUMDB=off`. - Check changed Go formatting, local documentation links, and `git diff --check`. - Release-tool fixtures need not rerun when tooling is unchanged. -7. Record implemented status and verification in these temporary review - documents, commit, and report remaining limitations honestly. Release - preparation later owns version selection, notes, retirement, tags, and pushes. +1. Add `BenchmarkInspectionSelection` in configuration tests using a captured + snapshot constructed outside timing. Measure existing prompt/profile lookups + with small and representative mutable nested-extra payloads. +2. Add root `BenchmarkRequirementInspection` using a real engine with a simple + prompt and the same small/large-extra distinction. Build the engine and + request outside timing, supply needed caller variables, and exercise only + the public inspection call. No artifacts, schemas, or providers are accessed. +3. Check selected identities and inventories outside timing. Record the + benchmark-only baseline commit and repeated measurements before routing + changes. Do not treat long immutable strings as deep-copy workload. -## Completion Criteria +**Verify:** `go test ./internal/configuration .`; run the two new benchmark +families in their owning packages. -- The separate public operation returns input and caller-variable inventories - classified required, conditional, or optional for the selected context. -- Recognizable inactive references remain optional entries in both inventories, - with safe explanations and aggregation that preserves stronger requirements. -- Simple branches/direct names agree with execution; complex cases preserve - recognized names and explicit limitations without assignment/path solving. -- Effective metadata/captured templates come from final configuration and remain - immutable; existing inspection, hashes, and execution behavior remain intact. -- Selection, session bypass, missing versus empty caller keys, metadata presence, - classification, and completeness have distinct documented meanings. -- No artifacts, credentials, provider access, schemas, or execution state are - needed for inspection. Results preserve disclosure and public error boundaries. -- Focused tests, the maintained example, canonical docs, and final assembled - checks demonstrate the feature. No significant design questions remain open. +## Stage 8 — Add Shared Lightweight Configuration Lookups + +**Outcome:** Captured configuration can provide identity/inspection metadata +without copying execution payloads. Existing runtime behavior stays unchanged +until Stages 9 and 10. + +**Read:** `internal/configuration/prompts.go`, `targets.go`, their tests, +`internal/usecase/prompt_inspection.go`, `profile_inspection.go`, and public +inspection GoDoc. + +**Work:** + +1. Extract existing prompt ID/version selection into a private + `lookupPromptEntry` helper. Preserve literal keys, blank checks, omitted + version ambiguity, not-found mapping, and construction-time hashes. Use it + from existing owned-definition/inspection construction; do not duplicate + selection rules. +2. Add internal `GetPromptInspection(context.Context, string, string) + (*domain.PromptInspection, error)` returning owned cached inspection data + with copied inputs, + without copying template structures. Keep `GetPromptSnapshot` independently + owned, but stop copying an unused inspection-input slice just to obtain its + cached hash. Retain the private `prompt` helper where callers need both + values, composing it from shared lookup/copy helpers. +3. Extract a shared private indexed-profile lookup used by `GetProfile` and + new `GetProfileIdentity(ctx, id) (string, error)`. The new method returns the + selected lookup ID after successful lookup, with no `ExtraParams` copy or + readiness/credential validation. `GetProfile` retains its owned deep copy. + Use existing context/not-found handling and the captured target invariant. +4. Test selection parity, owned returned prompt inputs, missing IDs, and partial + profiles through configuration APIs. Extend Stage 7's configuration benchmark + with lightweight lookup cases so old/new paths can be compared with the same + payload. Root requirement-inspection routing remains unchanged in this stage. + +**Verify:** `go test ./internal/configuration ./internal/usecase .`; run the two +benchmark families in their owning packages. No new public methods or ConfigKit +queries occur during lookup. + +## Stage 9 — Use Profile Identity Lookup During Requirement Inspection + +**Outcome:** Requirement inspection no longer deep-copies a target it does not +use. + +**Work:** + +1. Add a narrow optional repository capability in `internal/usecase` matching + `GetProfileIdentity`. Add `resolveProfileIdentity` beside the existing + profile selection helper, sharing basic ID/repository checks where useful. +2. Use the capability for the assembled configuration adapter. Wrap repository + failures with existing `ErrProfileLoad` and keep exact requested identities. + For internal repositories lacking the capability, fall back to + `resolveProfileSelection`, preserving its nil-profile/nil-target checks. + Do not change existing repository interfaces or public extension contracts. +3. Route only the identity requirement in `InspectRequirements` through this + helper. Keep default selection, partial-profile support, cancellation/error + order, direct-session handling, and analyzer behavior. `InspectProfile` and + preparation still need complete owned targets and keep their existing path. +4. Reuse existing selection tests, add focused capability/fallback parity + coverage, and compare `BenchmarkRequirementInspection` with Stage 7, + especially the mutable nested-extra case. Update internal runner mechanics. + +**Verify:** `go test ./internal/configuration ./internal/usecase .`; run the +requirement-inspection benchmark. No model, credential, admission, schema, or +provenance work is introduced. + +## Stage 10 — Select Prompt Inspection Metadata Without Execution-Definition Copies + +**Outcome:** Prompt and requirement inspection avoid copying template execution +structures they do not consume. + +**Work:** + +1. Add an optional `GetPromptInspection` repository capability in the runner. + Factor a shared inspection-selection helper from `InspectPrompt`: use the + captured metadata capability when available, otherwise derive the same + result from `resolvePromptDefinition` as today. Preserve guards, not-found/ + ambiguity categories, and cached-hash versus fallback-hash behavior. +2. Let `InspectPrompt` use that helper. Let `InspectRequirements` select the + same prompt metadata, use its default profile when needed, and request the + exact requirement snapshot by selected ID/version. +3. The analyzer currently reads only `definition.ID`/`Version`; pass a local + identity-only `domain.PromptDefinition` constructed from that metadata rather + than loading a full execution definition. Do not change analysis semantics + or enlarge its scope. Keep snapshot metadata/template ownership as today. +4. Protect exact selectors, default selection, session bypass, caller-owned + results, and hash equality with preparation through existing tests. Cover + both optional-capability and fallback paths without pinning helper call + counts. Compare inspection benchmarks and update runner/source documentation. + +**Verify:** `go test -race ./internal/configuration ./internal/usecase .`; +run inspection benchmarks without `-race`. Requirement inventories and public +prompt metadata remain identical for the same captured context. + +## Stage 11 — Establish Assembled Workflow Benchmarks + +**Outcome:** Reproducible end-to-end local benchmarks, including the baseline +for conditional snapshot optimization. No production change. + +**Read:** `internal/usecase/runner.go`, `prepared_execution.go`, root +`prepared_execution.go`, `convert.go`, client adapters, and +`prepared_execution_contract_test.go`. Locate the actual adapter file rather +than assuming an adapter filename. + +**Work:** + +1. Add root `BenchmarkEnginePreparation`, `BenchmarkEngineRun`, and + `BenchmarkPreparedExecutionDetails` using a real assembled engine and an + injected deterministic client. Use no live HTTP, environment credentials, + or source mutation in timing. Keep engine setup outside timing; discard + unused prepared handles so the benchmark does not retain every iteration. +2. Cover a small case and separate large-message and mutable-payload cases. + Use a few hundred optional schema properties and nested extra-map entries + within existing JSON-value limits, not merely long immutable strings. + Use the real schema validator for the assembled structured-output case. + Separate creation cost from repeated `Details()` cost. +3. Capture the benchmark-only baseline commit and repeated results. Verify + that the large-message case measures rendering/output storage while the + mutable-payload case exercises deep copying. Record the specific schemas, + input sizes, retained sinks, and handle lifetimes used by the fixtures. + +**Verify:** `go test .`; run the three new families. Check representative +content/validation outside timing. Keep preparation and `Run` measurements +separate from engine construction and provider latency. + +## Stage 12 — Establish Snapshot Ownership And Live-Heap Evidence + +**Outcome:** A measured ownership inventory for Stage 13's conditional change. +No production change; keep this analysis separate from creating the assembled +workflow benchmarks. + +**Read:** The preparation, handle, conversion, and adapter files listed in +Stage 11, plus `internal/domain/message.go` and `internal/jsonvalue/jsonvalue.go`. + +**Work:** + +1. Add a focused internal `BenchmarkPreparedSnapshot` using existing fast + runner collaborators to expose snapshot cost without schema-compilation + noise. Add opt-in `BenchmarkRetainedPreparedHandles` with ready, completed, + and discarded subcases, holding a fixed batch under the common live-heap + evidence method. These measurements are not lifecycle assertions. +2. Record the ownership inventory and baseline in execution evidence: message + slices/cache pointers are copied by concatenation; hashes are newly allocated; + schema is copied from the validation plan; nested extras can still alias an + internal repository/request after shallow target merging; details must remain + separate from execution and survive completion/discard; public conversion + and injected-client adapters create caller-owned values. Confirm these facts + against current code, including fake/internal collaborators, before Stage 13. +3. Record readiness/run/discard lifetime assumptions and repeated observations. + Check whether the candidate below can save mutable schema/message metadata + copies without transferring an externally accessible map. Do not remove a + production copy during this analysis stage. + +**Verify:** `go test ./internal/usecase .`; run the new families. Benchmarks must +check representative content/validation outside timing, avoid counting immutable +message length as deep-copied bytes, and document lifetime assumptions. + +## Stage 13 — Conditionally Consolidate Prepared Creation's First Snapshot Copy + +**Outcome:** Either one justified creation-copy simplification with preserved +ownership, or a completed, evidence-backed decision to retain the current +implementation. This does not authorize skipping measurement or ownership work. + +**Selected candidate:** Transfer preparation's already-owned messages, hashes, +and schema into its private execution payload instead of deep-copying them +again in creation. Still detach nested extras and still make a separate, +complete details snapshot. + +**Work:** + +1. Use Stage 12's inventory to confirm that `operation.run`'s message/cache + structures, input-hash map, and copied schema have no mutable external owner. + Immutable string sharing is safe; no read-only promise should be imposed on + public or injected values to justify removing a copy. +2. If the inventory supports the candidate, replace the first general + `clonePreparedRun(operation.run)` with transfer of `operation.run`, after + deeply copying its `EffectiveModelParams.ExtraParams` using existing + `jsonvalue.CopyMap` and preserving the invalid-request error boundary. + The schema already passed through `jsonvalue.Copy` during preparation; + keep that validation. Leave the subsequent full clone for `details` intact. +3. Keep `Details()`'s defensive clone and facade conversion, claim/discard + synchronization, payload cleanup, observers, and generated-client copies. + Further conversion or borrowed-snapshot redesigns are not this stage's + candidate. Preserve public availability of details after execution and + discard, and the separation of mutable details from execution state. +4. Verify independence from retained internal source-profile, renderer, and + validation-plan data using existing tests plus missing meaningful nested- + mutation coverage. Compare Stages 11–12 creation and live-heap evidence. Retain + the candidate only if ownership remains correct and savings justify it + without a material retention/complexity trade-off. Otherwise restore the + candidate edit and record why the current design is retained; keep useful + benchmarks. Do not substitute a more elaborate refactor merely to claim a + reduction. Update runner mechanics if the change is retained. + +**Verify:** `go test -race ./internal/usecase .`; run snapshot/assembled/retained +benchmarks without `-race`. Existing single-claim, discard races, frozen-source, +redaction, repair, and details-after-completion behavior must continue to pass. + +## Stage 14 — Consolidate Evidence And Verify The Complete Local Scope + +**Outcome:** The local roadmap scope is complete with clear measurements and +unchanged consumer contracts. + +**Work:** + +1. Record a concise before/after summary and revision/command references for + each changed path in execution evidence. Keep any Stage 13 retain decision + explicit. Include cumulative-allocation versus retained-heap distinctions + and small-input trade-offs; do not imply provider-latency improvements. +2. Review canonical internal source/runner/model-client descriptions against + the implemented flow. Consumer GoDoc and format semantics should remain + unchanged; remove incidental duplicated encoding mechanics where Stage 6 + required it. Update the inventory only if responsibilities actually changed. +3. Verify roadmap coverage: item 1 remains complete without dependency work; + items 2–5 have implemented local improvements; item 7 uses lightweight + inspection; item 6 has the selected conditional outcome and evidence; + production compiled rendering and assembled offline workflows have maintained + benchmarks. Do not leave a requested item silently unexamined. +4. Run assembled verification below once on final code, review the complete + diff and tree for artifacts, and mark stages/roadmap progress accurately. + Keep this plan as the implementation record until ordinary roadmap + retirement; do not add release gates or publish a release. + +**Verify:** + +```sh +go test -race -vet=off -mod=readonly ./... +go vet -mod=readonly ./... +python3 scripts/check-examples.py +git diff --check +``` + +Check local Markdown links and changed-file formatting using existing tooling +or an appropriate focused check. Reuse successful checks for unchanged content +rather than rerunning merely to create the final commit. + +## Execution Evidence + +Implementation has not started. Record benchmark-only baseline revisions, +comparisons, ownership analysis, conditional decisions, and final verification +here as the stages are completed. Keep the feature roadmap focused on target +state and progress rather than duplicating the staged procedure. + +## Open Questions + +No consumer-policy decisions remain open. File reservation and encoding error +handling have concrete approaches above. The prepared-copy candidate is +deliberately conditional on accepted evidence/ownership criteria, with a +defined retain-current-design outcome; it needs technical verification, not a +new consumer contract or a stronger ownership guarantee. diff --git a/docs/roadmap/performance.md b/docs/roadmap/performance.md index eff5cae..719c22c 100644 --- a/docs/roadmap/performance.md +++ b/docs/roadmap/performance.md @@ -10,9 +10,10 @@ preparation, rendering, and execution paths. Large payloads and concurrent library use make redundant full-payload copies especially worth addressing. Prefer small, idiomatic improvements over additional framework machinery. -This roadmap captures the intended outcomes, recommended changes, and suggested -order of work. It is not a detailed implementation plan or a new public -performance guarantee. Existing contracts remain owned by GoDoc, the +This roadmap captures the intended outcomes and recommended changes. The +[implementation plan](implementation.md) owns staged work and verification. +This roadmap introduces no new public performance guarantee. Existing contracts +remain owned by GoDoc, the [format reference](../formats.md), and the [integration contract](../integrations/openai-compatible-chat.md). Preserve the boundaries in the [architecture policy](../policy/architecture.md) and @@ -145,19 +146,6 @@ errors. The target is inspection cost driven by the information being inspected, not unrelated provider payloads. This requires no new public lookup API and does not move provenance analysis into ordinary prompt execution. -## Suggested Order Of Work - -Start with basic validation and template output (items 2 and 3), then file-read -capacity (item 5). These are localized changes with directly identifiable -allocation costs. Follow with request encoding and identity-only inspection -(items 4 and 7), preserving their error and selection semantics. - -Keep prepared-state consolidation (item 6) conditional on measured savings and -a clear ownership analysis; it has the greatest risk of weakening isolation or -concurrency behavior. The ConfigKit request (item 1) is complete and does not -block local improvements. This order is a recommendation, not a -dependency between otherwise independent changes. - ## Benchmarking And Verification Use focused, offline before/after benchmarks to establish useful savings in