Compare commits

20 Commits

Author SHA1 Message Date
5ccfa4a345 Plan the codebase audit remediation 2026-08-11 20:10:14 +00:00
14e03f19d0 Consolidate and close the codebase audit 2026-08-11 17:21:29 +00:00
7c562a9374 Document cross-cutting architecture audit findings 2026-08-11 17:10:59 +00:00
ef97d85ac9 Document repository-wide test strategy audit 2026-08-11 17:00:59 +00:00
805e48c873 Document capacity scheduling audit results 2026-08-11 16:51:28 +00:00
e9e126dcba Document OpenAI transport audit findings 2026-08-11 16:42:55 +00:00
32e7a3557c Document prepared execution lifecycle audit 2026-08-11 16:31:05 +00:00
1d1b04e2e0 Record ordinary execution and repair audit findings 2026-08-11 16:22:09 +00:00
9748897751 Document inspection and target resolution audit findings 2026-08-11 16:09:50 +00:00
9d020039d5 Record output validation audit findings 2026-08-11 15:59:46 +00:00
5247ce0b73 Record artifact loading and prompt rendering audit findings 2026-08-11 15:41:57 +00:00
c434aa1dae Record profile source audit findings 2026-08-11 15:29:23 +00:00
ac9b3f3d80 Record prompt source audit findings 2026-08-11 15:18:17 +00:00
4f12a89a1b Record backend registry and defaults audit findings 2026-08-11 15:05:15 +00:00
df31e7f58e Record domain and JSON value audit findings 2026-08-11 14:54:56 +00:00
0678d242b9 Record engine operation audit findings 2026-08-11 14:43:20 +00:00
3b4ea21208 Record engine construction audit findings 2026-08-11 14:36:33 +00:00
1430e85147 Record configuration and adapter audit findings 2026-08-11 14:28:07 +00:00
34d7a19da5 Record public value and error audit findings 2026-08-11 14:17:55 +00:00
ebf1602635 Prepare the codebase audit plan 2026-08-11 14:05:30 +00:00
6 changed files with 6179 additions and 22 deletions

View File

@@ -0,0 +1,635 @@
# Codebase Audit Sequence
## Purpose
This document defines the staged sequence for auditing Promptkit before further
feature development. The audit is intended to identify high-confidence
opportunities to improve correctness, efficiency, duplication, implementation
clarity, and test-suite quality without changing production behavior during the
review itself.
The audit findings belong in `audit.md`. A later, separate planning pass will
translate accepted findings into a staged remediation plan in
`implementation.md`. Neither this sequence nor the findings log owns current
behavior; the canonical sources identified by the
[documentation policy](../policy/documentation.md) remain authoritative.
Each stage below is deliberately scoped for one LLM coding-agent prompt. Run
the stages in order and do not combine them. A stage may discover a concern
outside its scope, but it should record that concern for the owning later stage
rather than expanding its own review.
## Governing Policies And Boundaries
Every stage must follow:
- the [development guide](../development.md), including its task-specific
reading guide;
- the [architecture policy](../policy/architecture.md), especially the public
facade, internal-package, dependency-direction, and consumer boundaries;
- the [testing policy](../policy/testing.md), including its risk-based,
behavior-oriented standard; and
- the [documentation policy](../policy/documentation.md), including canonical
ownership and the temporary nature of roadmap documents.
This is an audit, not an implementation pass:
- Do not change production code, tests, examples, fixtures, public contracts,
or current-state documentation.
- Limit repository edits to the audit artifacts explicitly authorized for the
stage.
- Do not silently repair an issue while investigating it.
- Do not treat coverage, complexity, similarity, lint, or graph output as a
finding without confirming the underlying behavior in source and tests.
- Do not recommend centralization merely because code looks similar. The code
must implement the same semantic rule, and consolidation must improve
ownership or reduce a credible drift risk.
- Do not recommend performance work without identifying a relevant execution
path and establishing a defensible cost model, measurement, or complexity
problem.
- Preserve unrelated working-tree changes. Record the audit baseline rather
than requiring an otherwise unrelated dirty tree to be cleaned.
## Finding Standard
Record each actionable finding in `audit.md` with:
- a stable ID in the form `SNN-FNN`, where the first number is the stage;
- category: correctness, efficiency, duplication, clarity, testing, or
contract-documentation consistency;
- severity: critical, high, medium, or low;
- confidence: confirmed, high, medium, or low;
- affected packages, files, symbols, and tests;
- the contract, invariant, policy, or maintenance concern at issue;
- concrete evidence and a concise explanation of the failure mode or cost;
- the recommended direction, without implementation-level sequencing;
- the verification or regression protection that remediation would require;
and
- status: accepted, deferred, rejected, superseded, or resolved.
Use **confirmed** confidence when the problem is reproduced or follows
unavoidably from a complete trace. Use **high** confidence when direct source
and test evidence establishes the problem but a safe reproduction is not
practical. Medium- and low-confidence concerns belong in a separate
observations section until a later stage confirms or rejects them; they must
not enter the remediation plan as if they were findings.
Severity describes impact, not implementation effort:
- **Critical:** credible data disclosure, data corruption, deadlock, unbounded
resource consumption, or a broadly unusable public contract.
- **High:** violation of an important public contract or invariant, a likely
concurrency or resource-lifecycle defect, or a failure with substantial
downstream impact.
- **Medium:** a real but narrower behavioral defect, meaningful avoidable cost,
duplicated policy with credible drift risk, or a material testing gap.
- **Low:** a bounded clarity, maintainability, or testing-friction problem with
a concrete improvement and little behavioral risk.
When a reviewed area yields no finding, record the important behavior or risk
that was inspected and found adequately implemented or tested. This coverage
ledger prevents later reviewers from mistaking silence for omission.
## Per-Stage Procedure
Unless a stage says otherwise, its single agent prompt should:
1. Read the required policies, focused internal documentation, production
files, and tests for that stage.
2. Use the code knowledge graph for symbol discovery, callers, callees, and
cross-package traces; confirm important conclusions against source.
3. Trace normal, boundary, and failure paths through the narrowest relevant
public or package contract.
4. Review correctness, meaningful runtime cost, semantic duplication,
responsibility clarity, and the value and ownership of tests in scope.
5. Run the narrowest existing tests needed to validate conclusions. Use
race-enabled or repeated focused tests when concurrency or nondeterminism is
in scope. Do not add permanent tests during the audit.
6. Add the stage result to `audit.md`: accepted findings, unresolved
observations, areas verified, commands run, and any handoff to a later
stage.
7. Recheck the working tree and confirm that only the authorized audit artifact
changed.
## Stage 0: Initialize The Audit And Establish The Baseline
Create `audit.md` and establish a reproducible starting point before reviewing
individual components.
Record:
- the audited commit, branch, Go version, module identity, and working-tree
state;
- unrelated pre-existing changes that all later stages must preserve;
- the implemented package and public-facade inventory;
- the baseline validation results; and
- the finding template, status vocabulary, and coverage ledger used by later
stages.
Refresh the code knowledge graph for the recorded commit. Run the repository's
ordinary tests, race tests, vet, build, maintained offline preparation example,
Go formatting check, Markdown link check, and repository-hygiene checks. Run
package coverage once as a diagnostic and record the result without defining a
coverage target or committing generated output. Measure coarse package test
duration only if it can be done without adding tooling or changing tests.
Compare the validation requirements stated by the testing policy, development
guide, and release procedure. Record a finding if their ownership or command
sets are materially inconsistent; do not edit those documents in this stage.
**Exit condition:** `audit.md` contains the baseline, ledger structure, and
validation result, and no component-level audit has begun.
## Stage 1: Public Values, Conversion, Errors, And Formatting
Review the root facade's public request, result, inspection, prepared-run, and
error values together with public-to-internal and internal-to-public
conversion. Scope the review to `doc.go`, `types.go`, `convert.go`, `errors.go`,
`capacity_error.go`, `formatting.go`, and `prepared_execution.go`, plus the
directly relevant portions of root tests.
Focus on:
- zero-value and nil behavior;
- defensive copying, aliasing, and immutable snapshots;
- lossless conversion and field precedence;
- error identity through `errors.Is` and `errors.As`;
- containment of internal representations;
- safe `String`, `GoString`, and diagnostic formatting;
- accidental disclosure of credentials, prompt content, generated content, or
other private state; and
- conversion or copying logic that represents the same rule in multiple
places.
Review only tests that own these value and boundary contracts. Defer engine
assembly, execution coordination, and transport behavior to their later
stages.
**Exit condition:** all root value-conversion and error-formatting paths have a
recorded audit result without evaluating engine orchestration.
## Stage 2: Public Configuration And Extension Adapters
Review the smaller public construction and extension surfaces in
`backends.go`, `profiles.go`, `artifact_reader.go`, `json.go`, and
`llm_adapter.go`, together with their directly relevant root and internal
adapter tests.
Focus on:
- validation performed at the public boundary;
- ownership and copying of caller-supplied maps, slices, filesystems, readers,
and clients;
- adapter error propagation and cancellation;
- consistency between convenience constructors and general configuration;
- whether extension interfaces are as narrow as their consumers require;
- whether public helpers duplicate internal policy or merely translate it;
and
- whether tests protect consumer-visible behavior rather than private adapter
choreography.
Do not review how `NewEngine` combines these values; that belongs to Stage 3.
**Exit condition:** every non-engine public configuration helper and adapter
has a recorded result and any assembly questions are handed to Stage 3.
## Stage 3: Engine Construction, Options, And Source Assembly
Review the construction and configuration portions of `engine.go` and the
corresponding tests in `engine_test.go`. Limit the scope to `NewEngine`, option
application, dependency defaults, backend registration, profile and prompt
source composition, fallback-profile placement, validator and client
selection, capacity-manager construction, and construction-time validation.
Focus on:
- deterministic option precedence;
- required versus optional dependencies;
- isolation between engine instances;
- freezing or copying consumer configuration at the correct boundary;
- correct dependency direction and absence of process-global mutable state;
- failure atomicity and useful public errors;
- consistency between configured backends and capacity policies; and
- assembly logic that is repeated or split across unclear owners.
Do not audit the runtime behavior of `Run`, `Prepare`, or inspection methods;
that belongs to Stage 4 and the internal use-case stages.
**Exit condition:** engine construction and source assembly are fully accounted
for, including tests, without expanding into runtime orchestration.
## Stage 4: Engine Operations And Root Contract Coverage
Review the remaining public methods in `engine.go` and their directly relevant
root tests, including the external-package contracts in
`public_contract_test.go` and `prepared_execution_contract_test.go` only where
they exercise the engine boundary under review.
Focus on:
- request translation and context propagation;
- ordinary run, preparation, inspection, and prepared-execution entry points;
- public error mapping and preservation of injected dependency errors;
- result and prepared-state ownership;
- consistency between method and package-level convenience functions;
- public behavior that is asserted redundantly in root internal tests and
external-package contract tests; and
- important public behavior that is tested only through internal packages.
Treat internal runner, transport, validation, and capacity mechanics as black
boxes in this stage. Hand questions about their implementation to their owning
later stages.
**Exit condition:** the public execution boundary and its contract-test
ownership are recorded without duplicating internal component audits.
## Stage 5: Internal Domain And JSON-Compatible Values
Review `internal/domain` and `internal/jsonvalue`, including all of their tests.
Focus on:
- domain invariants and invalid states;
- session normalization;
- prepared-run and schema immutability;
- deep-copy correctness for every supported JSON-compatible shape;
- numeric-type preservation and rejection policy;
- cycles, excessive nesting, unsupported values, and nil distinctions;
- avoidable repeated copying on execution paths; and
- whether generic value machinery has a single clear owner.
Trace important callers to confirm that these packages enforce the invariants
their consumers assume, but do not audit the callers' broader behavior.
**Exit condition:** shared value semantics and their test ownership are fully
recorded.
## Stage 6: Backend Registry, Defaults, And Built-In Profiles
Review `internal/backend`, `internal/defaults`, and
`internal/profile/builtin`, including their focused tests and the relevant
backend-policy traces into engine assembly and the LLM reserved-field rule.
Focus on:
- immutable registry construction and lookup;
- built-in versus consumer ID collision rules;
- endpoint, credential-environment, header, parameter, and concurrency
validation;
- defensive copies at registry boundaries;
- application-neutral default ownership;
- built-in profile/backend consistency;
- reserved request-field ownership without dependency inversion; and
- duplicated validation or default policy across public and internal layers.
Defer scheduling mechanics to Stage 15 and actual HTTP request construction to
Stage 14.
**Exit condition:** registry and default-policy correctness are recorded, with
transport and scheduling questions handed to their owning stages.
## Stage 7: File Discovery And Prompt Definitions
Review `internal/filecatalog` and `internal/promptdef`, including their tests
and fixtures. Read the framework format reference and internal source document
before evaluating behavior.
Focus on:
- deterministic discovery and duplicate handling;
- filesystem and `fs.FS` parity;
- root and relative-path normalization;
- strict YAML decoding and version selection;
- prompt ID, message, input, cache-control, and validation declarations;
- inline versus file-backed content rules;
- containment of referenced files where promised;
- malformed input and contextual error behavior;
- unnecessary repeated directory scans or file reads; and
- fixture and case duplication that does not protect distinct parser risks.
Do not audit rendering, artifact loading, profile loading, or schema validation
in this stage.
**Exit condition:** discovery and prompt-definition parsing have complete
findings and coverage-ledger entries.
## Stage 8: Profile Sources And Repository Composition
Review `internal/profile` excluding its built-in subpackage, including all
repository tests and profile fixtures. Read the profile format contract first.
Focus on:
- strict decoding and profile validation;
- filesystem and `fs.FS` parity;
- repository overlay and fallback precedence;
- distinction between absence and a malformed authoritative source;
- preservation of useful error identity and context;
- conversion to immutable execution profiles;
- duplicate IDs and deterministic selection;
- repeated parsing, validation, or copying; and
- whether tests at repository, engine, and public-contract layers have clear,
nonduplicative ownership.
Defer resolution of a profile with runtime overrides and backend definitions to
Stage 11.
**Exit condition:** profile-source and repository-composition behavior are
fully recorded.
## Stage 9: Artifact Loading And Prompt Rendering
Review `internal/artifact` and `internal/prompt`, including all focused tests.
Read the internal source document and format reference first.
Focus on:
- inline and file artifact ownership, metadata, hashing, and error behavior;
- copied versus shared byte storage;
- caller-selected path semantics and architecture-policy boundaries;
- template parsing and execution;
- artifact, variable, session, and cache-control rendering;
- missing, extra, nil, and malformed input behavior;
- deterministic output and safe diagnostics;
- unnecessary repeated reads, hashes, parses, or allocations on common paths;
and
- tests coupled to incidental template or struct implementation.
Do not audit the runner's decision about when rendering occurs.
**Exit condition:** input materialization and rendering are accounted for
through their package boundaries.
## Stage 10: Output Validation And Frozen Validation Plans
Review `internal/validate`, including all tests, schema fixtures used by the
root contract suite, and traces from preparation into frozen validation plans.
Read the format and internal source documents first.
Focus on:
- basic, JSON, and JSON Schema mode semantics;
- schema-path resolution and filesystem/`fs.FS` parity;
- schema compilation, transitive references, and source-lifetime independence;
- output normalization and preservation;
- malformed schema and malformed model-output errors;
- thread safety of reusable validators and prepared plans;
- expensive recompilation or copying on repeated execution; and
- whether parser, validator, runner, and public tests each own distinct risks.
Do not audit repair decisions or provider request construction.
**Exit condition:** validation behavior, plan lifetime, and focused test value
are fully recorded.
## Stage 11: Inspection And Execution-Target Resolution
Review `internal/usecase/profile_inspection.go`,
`internal/usecase/prompt_inspection.go`, and the preparation and target-
resolution portions of `internal/usecase/runner.go`, together with their
focused tests. Use graph traces to define the exact helper and call-path scope
before reviewing.
Focus on:
- prompt and profile selection;
- backend lookup and endpoint overrides;
- reasoning, session, and other runtime precedence;
- merge semantics for default, profile, backend, and per-run values;
- inspection fidelity versus actual execution;
- credential-name versus credential-value handling;
- prompt-definition and schema freezing during preparation;
- stable error identity and context; and
- duplicated resolution rules across inspection, preparation, and execution.
Do not review model invocation, repair execution, or prepared-handle lifecycle;
those belong to Stages 12 and 13.
**Exit condition:** all selection, merge, inspection, and preparation rules are
traced and recorded once.
## Stage 12: Ordinary Execution, Validation, And Repair Coordination
Review `internal/usecase/runner.go`, `internal/usecase/repairer.go`, and
`internal/usecase/capacity_error.go` only for the ordinary execution path after
preparation, together with the corresponding sections of `runner_test.go`.
Use the Stage 11 resolution result as an established input rather than
reauditing it.
Focus on:
- rendering, generation, validation, and optional repair transitions;
- context cancellation and dependency-error propagation;
- partial result and usage accounting;
- exact attempt count and repair eligibility;
- avoidance of unintended retries;
- capacity-error translation;
- cleanup and failure behavior on every exit path;
- repeated orchestration or request construction; and
- oversized tests, helpers, or case matrices that obscure distinct behavior.
Treat LLM transport and capacity scheduling as injected package contracts;
their mechanics belong to Stages 14 and 15.
**Exit condition:** the ordinary execution state machine and its test ownership
are fully recorded.
## Stage 13: Prepared Execution Lifecycle
Review `internal/usecase/prepared_execution.go`, its focused tests, and the
prepared-execution portions of the root facade and external contract tests.
Do not repeat the public value review from Stages 1 and 4 or the resolution
review from Stage 11.
Focus on:
- single-attempt or other lifecycle guarantees;
- concurrent use and synchronization;
- discard behavior and resource release;
- frozen source, target, credential, capacity, timing, and schema semantics;
- independence of returned details and results;
- context and error behavior;
- consistency between ordinary and prepared execution where promised;
- private-state containment in formatting; and
- redundant assertions across internal, root, and external-package tests.
Run focused race tests and repeated tests for lifecycle behavior where useful.
**Exit condition:** prepared execution has one complete lifecycle analysis and
a clear map of which test layer owns each guarantee.
## Stage 14: OpenAI-Compatible Transport
Review `internal/llm`, including all transport tests. Read the
OpenAI-compatible integration contract and internal LLM document first.
Focus on:
- request endpoint, headers, authentication, and JSON body construction;
- omission versus explicit zero-value behavior;
- reserved-field enforcement and extra-parameter collision handling;
- session ID and reasoning encoding;
- structured-output and cache-control translation;
- client and per-generation deadlines;
- cancellation, body closure, bounded response reads, and decode failures;
- non-success HTTP response behavior;
- response choices, usage, and malformed-success handling;
- wire-visible compatibility and safe error disclosure;
- unnecessary marshaling, copying, or buffering; and
- whether the large transport test file can be simplified without losing
protocol-risk coverage.
Use `httptest`-based existing tests; do not contact a live provider.
**Exit condition:** every outbound and inbound wire path has a recorded result,
including focused test ownership.
## Stage 15: Capacity, Admission, And Concurrency
Review `internal/capacity`, its tests, `capacity_contract_test.go`, and the
integration points already identified in engine and use-case stages. Read the
internal capacity document first.
Focus on:
- bounded run admission and queue-capacity enforcement;
- per-backend limited and unlimited scheduling;
- FIFO behavior and cancellation-safe waiter removal;
- permit release on success, error, panic-relevant boundaries, and
cancellation;
- goroutine, timer, and waiter lifecycle;
- starvation, deadlock, race, and engine-isolation risks;
- lock scope and meaningful contention or allocation costs;
- preservation of injected-client concurrency where promised;
- relational testing of configured limits rather than duplicated defaults;
and
- duplication between internal concurrency tests and public contract tests.
Run focused ordinary, race-enabled, and repeated tests. Repetition must remain
bounded and diagnostic; a test that passes many times is not proof of
correctness without a source-level synchronization analysis.
**Exit condition:** concurrency invariants have both a source trace and a
test-ownership assessment.
## Stage 16: Repository-Wide Test Strategy And Maintained Examples
Perform a suite-level review after every component has been audited. Review
the testing policy, test inventory, fixtures, external-package root tests,
`architecture_test.go`, and both maintained examples. Use the component-stage
coverage ledger instead of repeating every individual test assertion.
Construct a risk-to-owner matrix for:
- public compatibility and error identity;
- parsing, validation, and serialization;
- immutability and data integrity;
- external wire behavior;
- cancellation, failure propagation, and recovery;
- concurrency and resource lifecycle; and
- representative assembled consumer workflows.
Identify only evidence-backed cases of:
- consequential behavior with no credible test owner;
- the same semantic rule asserted redundantly at several layers;
- tests coupled to private helpers, internal constants, exact noncontractual
wording, or collaborator choreography;
- low-value or obsolete cases whose lifetime cost exceeds their protection;
- missing failure, cancellation, race, or boundary coverage;
- nondeterminism, shared state, environment dependence, fixed ports, or test
ordering assumptions;
- helpers and fixtures whose complexity is not justified; and
- maintained examples that duplicate one another without protecting distinct
workflows.
Use coverage and timing only to direct attention. Do not propose tests solely
to raise percentages or remove tests solely to shorten the suite.
**Exit condition:** every important risk has a named test owner or an accepted
finding, and every proposed test deletion or consolidation states what
protection remains.
## Stage 17: Cross-Cutting Duplication, Efficiency, And Architecture Review
Review the codebase as a whole using the completed component findings, graph
traces, complexity signals, similarity signals, and package dependency map.
Do not reopen settled package behavior without new cross-cutting evidence.
Focus on:
- one semantic policy implemented by multiple packages;
- repeated public/internal transformations with credible drift risk;
- interfaces broader than their actual consumers;
- responsibilities split across packages or concentrated in the facade
contrary to the architecture policy;
- repeated parsing, copying, schema compilation, request construction, or
source traversal on important paths;
- avoidable lock contention or serial work supported by the concurrency audit;
- abstractions that add indirection without enforcing a boundary; and
- discrepancies between implemented package responsibilities and their
canonical architecture or internal documentation.
For each possible consolidation, state why the code represents one rule, which
package should own it, and why the resulting dependency direction remains
valid. For each efficiency finding, state the path frequency, input scale,
complexity or measurement evidence, and the benchmark or invariant needed to
verify a remediation.
**Exit condition:** all cross-cutting opportunities are either accepted with
high confidence, retained as explicitly lower-confidence observations, or
rejected with a short rationale.
## Stage 18: Consolidate And Close The Audit
Perform a findings-only synthesis. Do not change code and do not write the
remediation plan yet.
- Recheck every accepted finding against the final audited tree.
- Merge duplicates and mark superseded IDs without erasing their history.
- Separate shared root causes from downstream symptoms.
- Confirm that every accepted item is confirmed or high confidence.
- Confirm that severity describes impact rather than effort.
- Reject speculative cleanup, coverage-driven test work, and centralization
without a clear owner or drift risk.
- Record dependencies and a recommended remediation order.
- Distinguish behavioral fixes, safe refactors, performance work, test gaps,
test consolidation, and documentation synchronization.
- Add an audit summary stating what was reviewed, what validation ran, the
accepted finding counts by category and severity, and any residual
uncertainty.
- Re-run baseline validation if audit-only investigation could have affected
repository state, and confirm that only authorized roadmap files differ from
the recorded baseline.
The recommended ordering should place correctness, data-integrity,
resource-lifecycle, and concurrency defects first; policy duplication and
missing protection for consequential behavior next; then clarity, test
consolidation, and demonstrated efficiency improvements. Actual implementation
stages must be decided in the later `implementation.md` planning pass, where
files, dependencies, acceptance criteria, and validation can be made
decision-complete.
**Exit condition:** `audit.md` is a complete, internally consistent input to a
separate remediation-planning prompt, with no code or test changes mixed into
the audit.
## Completion Criteria
The audit is complete only when:
- every production component and public boundary appears in the coverage
ledger;
- every test file and maintained example has been reviewed at its owning stage
or in the suite-wide stage;
- important cross-package paths have been traced end to end;
- concurrency-sensitive behavior has received source and race-test review;
- every accepted finding meets the evidence and confidence standard;
- lower-confidence observations are visibly separated from remediation
candidates;
- proposed test additions, deletions, and consolidations are justified against
the testing policy;
- proposed simplifications identify a durable responsibility owner;
- proposed efficiency work has a relevant cost model or measurement plan; and
- the repository remains unchanged except for the authorized audit roadmap
artifacts.

4606
docs/roadmap/audit.md Normal file

File diff suppressed because it is too large Load Diff

View File

@@ -59,24 +59,3 @@ if supported at all, should be an explicit option and must not expose secrets.
Reconsider this idea after downstream use of `InspectPrompt`,
`InspectProfile`, and fixture-based preparation demonstrates a concrete gap.
### Structured Generation Errors
**Reason for deferral:** Existing `ErrLLMGenerate` classification, preserved
injected-client errors, and prepared execution details currently provide the
necessary failure boundary. A typed error should wait for stronger downstream
demand and a transport-neutral field design.
Promptkit could expose safe structured generation context through
`errors.As` while preserving `errors.Is(err, ErrLLMGenerate)`. Potential
fields include the selected backend ID and model plus an optional HTTP status
when the built-in OpenAI-compatible transport supplies one.
The design must not expose provider response bodies, endpoints, credential
environment names or values, request content, or generated content. It should
not duplicate prompt and profile provenance already available from a prepared
execution, and it must preserve the identity of errors returned by injected
clients. Retry and backoff policy remains a consumer responsibility.
Reconsider this idea when consumers need structured generation diagnostics
beyond the existing sentinel, wrapped client error, and preparation record.

View File

@@ -38,7 +38,33 @@ consumers.
## Ideas
No ideas currently await selection.
### Public bounded output repair
After the codebase-audit remediations are complete, Promptkit should make its
bounded output-repair capability available through the public engine. A
consumer should be able to request a limited number of corrective generation
attempts when JSON or JSON Schema output fails content validation, without
having to reproduce Promptkit's generation, validation, capacity, and result-
accounting orchestration.
- Repair is validation recovery, not a general provider retry, failover, or
backoff policy. Transport failures, cancellation, and operational schema or
validation errors must retain their ordinary error behavior.
- Repair must stop after the first valid result or the configured attempt
bound. Exhausting the bound should preserve the final invalid result and its
validation diagnostics rather than inventing success.
- Initial generation and every repair attempt must use the same resolved
backend, effective execution settings and presence semantics, session,
credential boundary, structured-output contract, and backend-capacity
policy.
- Results should report the number of repair attempts and cumulative usage for
every model call made by the run.
- Ordinary and prepared execution should expose coherent behavior, including
cancellation, frozen prepared state, error identity, and capacity lifetime.
Select this work only after the accepted audit findings affecting shared
execution invariants, validation, orchestration, transport, and repair
internals have been remediated.
## Entry Format

View File

@@ -0,0 +1,840 @@
# Audit Remediation Implementation Plan
## Purpose
This document is the decision-complete implementation plan for the accepted
findings in the [codebase audit](audit.md). It is written for a
`gpt-5.6-terra` coding agent that will implement one numbered stage per prompt,
in order.
The audit remains the evidence and rationale for each finding. This plan owns
implementation order, selected policy decisions, required code and test work,
and stage gates. It does not activate the future public output-repair feature
described in the [future feature catalog](future.md); it only corrects and
protects the retained internal repair machinery on which that later feature
may build.
## Implementation Policies
Every stage must follow the [development guide](../development.md),
[architecture policy](../policy/architecture.md),
[testing policy](../policy/testing.md), and
[documentation policy](../policy/documentation.md). Before changing a
subsystem, read the focused current-state documents identified by the
development guide and inspect the exact implementation and tests named by the
stage.
Apply these rules throughout:
- Implement exactly one stage per agent prompt. Do not combine stages or begin
a later stage early.
- Inspect the working tree before editing and preserve unrelated changes.
- Use the code knowledge graph to locate symbols, callers, and dependency
paths; confirm important conclusions against source.
- Keep the root package as the public facade and implementation under
`internal/`. Do not expose internal representations or add a public package.
- Put source-neutral invariants in their assigned internal owner while
preserving source-specific normalization, error classification, and public
translation at existing boundaries.
- Add regression protection at the narrowest stable owner in the same stage
as a behavioral fix. Retain only representative integration coverage at
higher layers.
- Do not add tests to raise coverage percentages. Do not preserve tests that
assert an incidental algorithm, private constant, dormant serialization
shape, or duplicated lower-layer truth table.
- Keep all tests deterministic, offline, race-safe, credential-free, and free
of fixed-port or mutable-service assumptions.
- Do not add engine-wide caches, generic facade abstractions, scheduler
changes, provider retry policy, or new consumer configuration unless a stage
explicitly requires it.
- Update canonical GoDoc and current-state documents in the same stage as the
behavior they describe. Do not describe a later stage as already
implemented.
- Format changed Go files. Run the stage's focused commands, then at least
`go test ./...` and `go vet ./...`. Run focused race tests wherever the
stage changes ownership, cancellation, shared state, or lifecycle behavior.
- Do not commit, push, tag, or publish unless separately instructed.
## Decisions Fixed By This Plan
The implementing agent must not reopen these choices:
1. **JSON-compatible numbers:** accept every value Go can faithfully encode as
a JSON number: every signed and unsigned integer width, finite `float32` and
`float64` values, and a `json.Number` whose text is valid JSON-number syntax.
Do not impose the current IEEE-754 safe-integer restriction. Reject NaN,
infinities, and malformed `json.Number` text. Preserve supported concrete
numeric types when copying.
2. **JSON-shaped traversal bounds:** allow at most 100 JSON container levels
and 100,000 produced JSON value nodes per `Copy` or `CopyMap` operation.
Count the root, each map/slice/array container, and every produced child
value; map keys are not separate nodes. Pointer and interface indirection
do not add JSON depth or an extra node. Repeated appearances of an acyclic
shared value count each produced occurrence. Continue rejecting active-path
cycles and return deterministic, path-aware validation errors on either
bound.
3. **Execution timeout bound:** a positive `TimeoutSeconds` must fit in
`time.Duration` after multiplication by `time.Second`. Derive the maximum
from `math.MaxInt64` and `time.Second`; do not duplicate its numeric literal
in tests or documentation.
4. **Output contracts:** the only valid formats are `text`, `markdown`, and
`json`; the only valid validation modes are `none`, `basic`, `json`, and
`json_schema`; repair attempts are non-negative; and `json_schema` requires
a nonblank schema path. A non-nil request replacement defaults an empty
format to `text` before shared validation. It does not default an empty
validation mode.
5. **Prompt content paths:** every `content_file` is an exact, relative path
resolved from its prompt file and contained by the configured prompt source
root. Directory, `fs.FS`, and single-file sources all reject absolute and
escaping paths. A single-file source's root is the containing directory of
that selected prompt file. Trimming determines only whether a value is
blank; it must not change the path opened. OS containment must account for
symlinks; containment inside an injected `fs.FS` remains expressed in that
filesystem's namespace.
6. **Profile IDs:** normalize file-backed IDs with `strings.TrimSpace` once,
just as in-memory IDs are normalized. Use the normalized value for
selection, duplicate detection, results, and diagnostics. A whitespace-only
ID is invalid, and IDs that become equal after normalization are
duplicates.
7. **Ordinary artifact files:** the built-in `File` reader supports regular
files, including symlinks whose targets are regular files. It rejects
directories, FIFOs, devices, sockets, and other non-regular targets before
consuming them. It remains unrestricted by an application root and does
not introduce an application-specific byte limit.
8. **Validation cancellation:** do not return early by abandoning goroutines
around `fs.FS` or the JSON Schema dependency. Promptkit must check
cancellation before, between, and after work it controls; read opened files
in context-checked chunks; and let a canceled context win before publishing
a result after synchronous decode, compile, or validation calls. Go's
`fs.FS` and the current JSON Schema library expose no general mechanism to
preempt a blocked `Open`, `Read`, compile, or validation method, so canonical
documentation must describe this synchronous limitation rather than claim
impossible asynchronous interruption.
9. **Successful provider-response limit:** the built-in OpenAI-compatible
client accepts at most 16 MiB (`16 << 20` bytes) for the complete successful
HTTP response body, including surrounding whitespace. The limit is fixed,
internal, and application-neutral. Exactly the limit is allowed; the first
byte beyond it fails as `internal/llm.ErrMalformedResponse`. Do not add a
public setting. Non-success response parsing remains outside this audit
remediation and belongs to the separate structured-generation-error
roadmap.
10. **Repair machinery:** retain and fix the internal repairer, cumulative
usage, and bounded repair state machine. The public engine must continue to
install no repairer and remain single-pass. Do not activate public repair
in this plan.
## Stage 1: Centralize Execution-Setting And Session Invariants
**Findings:** S05-F01, S17-F01, S14-F02. This stage also resolves the
source-specific evidence in S02-F02, S08-F01, and S11-F01.
Add a source-neutral execution-setting validator to `internal/domain`. It must
validate temperature, maximum tokens, top-p, and timeout on a domain execution
target: temperature and top-p must be finite and within their closed ranges,
maximum tokens must be non-negative, and timeout must be non-negative and no
greater than the derived duration-safe maximum. Keep optional-pointer presence,
profile required fields, normalization, and error wrapping outside this
validator.
Use that owner from:
- in-memory profile validation in the root package;
- OS and `fs.FS` profile validation;
- resolved request/target validation in `internal/usecase`; and
- the built-in model client as a defensive final boundary.
Remove the duplicated scalar comparisons from those callers. Preserve
`ErrInvalidConfig` for in-memory construction, profile-load identities for file
profiles, `ErrInvalidRequest` for runtime overrides, and the LLM package's
defensive invalid-request identity. Explicit numeric zero must retain its
presence semantics.
Update `internal/domain.NormalizeSessionID` to reject invalid UTF-8 before
trimming or rune counting. Preserve the existing blank and 256-code-point
rules. Direct requests must still map failures to `ErrInvalidRequest`, while
session-template failures remain renderer failures.
Add one domain-owned table for every exact setting boundary, finite neighbors,
NaN, both infinities, negative values, and the timeout representability edge.
Retain small boundary-integration cases for in-memory profiles, both file
source forms, request overrides through `Prepare` and `PrepareExecution`, and
the model-client defense. Add malformed UTF-8 session cases before, within,
and after otherwise valid content.
Update the architecture policy and internal component overview so
`internal/domain` explicitly owns source-neutral invariants for its shared
execution values, without claiming ownership of source-specific policy.
Run focused domain, profile, use-case, root, and LLM tests, including the
affected race-enabled request and profile cases, followed by the repository
test and vet gates.
## Stage 2: Centralize Output-Contract Legality
**Finding:** S17-F02, including the request-boundary symptom S11-F02.
Add one pure `internal/domain` validator for `OutputContract`. It must enforce
the format, validation-mode, non-negative repair-attempt, and JSON-Schema path
rules fixed above. It must not load schemas or apply source/request defaults.
Make prompt-definition normalization call the shared validator after its file-
specific normalization. Keep prompt-required fields and contextual
`ErrInvalidPromptDefinition` ownership in `internal/promptdef`. Make request
resolution default an empty replacement format to `text`, then call the same
validator and translate failure to `ErrInvalidRequest` before artifact,
rendering, validation, admission, or generation work. Keep schema loading and
compilation in `internal/validate`.
Add a domain table covering every supported and unsupported enum, empty values,
negative and non-negative repair counts, and schema-path relationships. Retain
small prompt-source and use-case integration tables that prove correct error
categories and parity between `Prepare` and `PrepareExecution`; do not repeat
the entire domain table at those layers.
Update the architecture and internal overview language added in Stage 1 to
include source-neutral output-contract invariants. Run focused domain,
prompt-definition, use-case, and root tests, then repository test and vet
gates.
## Stage 3: Make JSON-Compatible Value Handling Coherent And Bounded
**Findings:** S05-F02, S05-F03, S05-F04.
Refactor `internal/jsonvalue` around the numeric and traversal decisions fixed
by this plan. Remove the safe-integer restriction and apply one numeric rule to
all supported representations. Preserve concrete named and unnamed scalar,
map, slice, and array types where the existing contract promises preservation;
keep nil versus empty container distinctions and `Copy` versus `CopyMap` empty-
key behavior.
Extend the traversal state to track JSON container depth and produced-node
work. Enforce the 100-level and 100,000-node limits before allocation or
descent would cross them. Continue using active-path identity for cycle
detection; do not use alias memoization that would make distinct JSON paths
share mutable output. Errors must identify the structural path and whether the
depth or work budget was exceeded.
Expand the focused package tables by behavior branch: signed and unsigned
integer widths, ordinary and named finite floats, `json.Number`, pointers and
interfaces, named maps/slices/arrays, nil and empty values, mixed nested trees,
arrays, mutation isolation, active cycles, alternating just-below/at/over
depth, and shared acyclic subgraphs just below and over the work budget. Tests
must derive their edges from package constants or relationships instead of
copying unexplained literals.
Retain only representative public/backend/profile/prepared integration cases
that prove error translation and ownership. Update public GoDoc only if it
currently states the narrower safe-integer behavior; otherwise the existing
finite JSON-compatible-number contract remains canonical. Update the relevant
public value GoDoc and format/internal documentation to state that excessively
deep or large JSON-shaped values are rejected for safety; keep the exact
numeric limits owned by the internal constants rather than duplicating them
throughout consumer documentation. Run focused package and caller tests,
focused race tests, repository tests, and vet.
## Stage 4: Consolidate Stable Public JSON And Remove Dormant Internal JSON
**Findings:** S02-F01, S02-F05, S05-F05.
Refactor `json.go` so each public value has one ordinary field mapping. Use
private aliases or embedded wire representations for ordinary fields and keep
only timestamp, millisecond-duration, and intentional omission exceptions
explicit. Preserve every existing JSON name and omission rule.
Before converting `duration_ms`, reject values outside the millisecond range
that can be multiplied by `time.Millisecond` without overflow. Derive both
edges from `time.Duration` bounds. Return a contextual decode error and do not
partially update the receiver on failure.
Add fully populated `PreparedRun` and `RunResult` contract cases. Verify all
ordinary fields, intentional omissions, zero and nonzero timing, complete
round trips, the largest safe positive and negative millisecond values, and
their first unsafe neighbors.
Remove unused JSON tags and serialization tests from
`internal/domain.PreparedRun` after confirming production never marshals that
type. Keep credential absence protected at preparation/clone producers and
move any useful cache-control JSON assertion to the public `PreparedRun`
contract. Do not retain a parallel internal wire format.
Run focused domain and root JSON tests, repository tests, and vet.
## Stage 5: Harden Public Ownership And Diagnostic Contracts
**Findings:** S01-F01, S02-F03, S02-F04, S13-F01.
Extend the existing run-request formatting test with distinct input URI,
input-body, variable, and API-key sentinels. Require their absence from
`String`, `GoString`, `%v`, `%+v`, and `%#v` while retaining positive structural
summary assertions.
Add one focused public-LLM-adapter ownership test. Have the injected client
mutate and retain prompt messages, cache-control pointers, nested target extra
parameters, and structured-output schema values; prove the domain/prepared
source remains unchanged and later details or execution cannot race with those
mutations.
Add one direct all-field mapping test for `OpenAICompatibleProfile`. Populate
every field distinctly and compare the complete returned `Profile`. Keep only
the existing higher-level cases that prove normal validation and nested-value
ownership.
Make copied `PreparedExecution` values format opaquely by using value-receiver
formatting behavior shared by non-nil pointers and values. A nil pointer may
use Go's normal `<nil>` formatting, but formatting must never panic or expose
internal types, field names, addresses, credentials, or content. Cover original
pointers, copied values, zero values, and nil pointers under string, Go-string,
and ordinary fmt verbs, and prove formatting does not claim or discard a
handle.
Run focused root tests and the affected prepared/adapter race tests, followed
by repository tests and vet.
## Stage 6: Correct Engine Construction Edges And Immutable Defaults
**Findings:** S03-F01, S03-F02, S06-F01.
Change the shared single-file option helper so trimming is used only for the
blank-input check. Perform `Stat`, path decomposition, storage, diagnostics,
and later access with the exact caller path for prompt, profile, and schema
files. Add one compact table covering existing leading- and trailing-whitespace
names through all three options.
Strengthen engine construction tests with three discriminating cases:
- reverse the argument order of in-memory, ordinary, fallback, and built-in
profile categories while retaining fixed category precedence;
- collide `Config.ProfileDir` with an ordinary profile option and prove the
option replaces the configuration source; and
- place a valid same-category replacement after an invalid option and prove
construction still fails at the earlier invalid option.
Convert `internal/defaults.LLMRequestTimeoutDefault` from a variable to a
constant without changing its value or adding a setter. Do not add a test that
mutates or pins a noncontractual default; existing client deadline behavior is
the verification owner.
Run focused engine construction, default-client construction, and race tests,
then repository tests and vet.
## Stage 7: Contain And Preserve Prompt Content Paths
**Finding:** S07-F01.
Refactor prompt content resolution so both repository forms receive an
explicit source-root abstraction. Enforce the path decision fixed by this plan
before any content read. Use exact parsed path text after a separate blank
check. For OS sources, canonicalize the root and resolved target sufficiently
to reject symlink escape; for injected `fs.FS`, use its clean relative path
namespace. A parent component that remains inside the root is valid. Absolute,
escaping, and symlink-escaping targets are invalid.
Apply the same behavioral table to an OS directory, `WithPromptFS`, and a
single-file source: ordinary sibling, nested parent still within root, parent
escape, absolute path, symlink escape where supported, and existing names with
leading or trailing whitespace. Prove rejected targets cause no outside read
and public operations preserve `ErrPromptLoad`.
Update the framework format reference and internal source document to make the
single-file root and absolute-path rule explicit. Run focused prompt-definition
and public source tests, including race tests, then repository tests and vet.
## Stage 8: Correct Prompt Selection, Strictness, Coverage, And Lookup Cost
**Findings:** S07-F02, S07-F03, S07-F04, S07-F06.
Correct both existing prompt repository paths before consolidating them in the
next stage:
- Recover selector metadata from YAML `id` and `version`; never use a filename
stem as an identity.
- Apply normalized ID and requested-version selection before semantic
normalization or `content_file` reads.
- Associate strict YAML, semantic, and content errors only with a reliably
matching selected definition. An unidentifiable malformed file is unrelated
to point lookup; a reliably selected malformed file remains authoritative.
- Require exactly one YAML document. Comments and trailing whitespace are
allowed; a second empty or populated document and malformed trailing YAML
are `ErrInvalidYAML`.
- Continue scanning the YAML metadata required for duplicate detection, but
open content only for selected candidates. A selected content file is opened
once; unrelated and different-version bodies are never opened.
Add paired OS and `fs.FS` regressions for same-stem/different-ID malformed
files, same-ID/different-version invalid files, selected malformed definitions,
additional YAML documents, duplicates, and counting filesystem behavior.
Add a compact normalization table for the previously uncovered missing
version, blank input name, blank message role, invalid output format, negative
repair attempts, and explicit blank default profile. Output-contract rows
should exercise the shared Stage 2 owner rather than recreate its full table.
Run focused prompt-definition, use-case inspection, and root source tests,
focused race tests, repository tests, and vet.
## Stage 9: Unify Prompt Repository Semantics
**Finding:** S07-F05.
After Stage 8 establishes correct behavior in both paths, replace their
duplicated discovery-to-selection algorithms with one source-neutral prompt
selection and normalization flow. Introduce only the small internal source
adapter needed for YAML discovery, bytes, exact content opening, display paths,
and root containment. Keep genuine OS and `fs.FS` mechanics at the adapter
edge.
Move exact selection, version filtering, strict one-document decoding,
selected-error classification, normalization, duplicate handling, and
not-found behavior into the shared flow. Preserve point-in-time source access;
do not cache catalogs or definitions across operations.
Turn the Stage 8 behavior matrix into a shared suite over both adapters and
retain source-specific tests only for distinct path and I/O failures. Delete
superseded duplicate helpers and tests only after the shared suite protects
their meaningful behavior. Use a counting filesystem and a before/after
benchmark over small and large prompt catalogs to confirm unrelated content is
not read and the refactor adds no second scan; do not enforce wall-clock
thresholds.
Update the internal source document to describe the unified semantic owner.
Run focused package, integration, race, repository test, and vet gates.
## Stage 10: Correct Profile Source Validation And Identity
**Findings:** S08-F02, S08-F03, S08-F04, S08-F05. The non-finite scalar
symptom S08-F01 is already resolved by Stage 1.
Make file profile normalization pass `extra_params` through
`internal/jsonvalue.CopyMap` before publishing a domain profile. Preserve
`ErrInvalidProfile` and source path context for empty keys, non-finite values,
nested invalid data, or traversal-budget failures. Do not move reserved
OpenAI-compatible field policy into the profile package.
Use YAML metadata ID as the only selector; never infer authority from a
filename. Normalize the decoded ID once according to this plan. Require exactly
one YAML document in both metadata and strict selected decoding, so trailing
raw credentials, unknown fields, empty documents, and malformed YAML cannot be
ignored. Reliably selected malformed definitions must stop overlay fallback;
unrelated malformed files must not.
Add shared OS and `fs.FS` tables for invalid/valid extra parameters,
same-stem/different-ID malformed files beside a valid profile, fallback
behavior, additional documents, leading/trailing/blank IDs, normalized
duplicates, and exact inspection/preparation of the normalized ID. Retain only
representative public error-translation cases.
Update the framework format and internal source documents if needed to state
ID normalization and one-document behavior. Run focused profile, use-case,
root, and race tests, followed by repository tests and vet.
## Stage 11: Eliminate Duplicate Profile Decoding
**Finding:** S08-F06.
Refactor point lookup so each file receives one metadata pass and only
canonical ID matches receive strict full decoding and normalization. Reuse
bytes already read for metadata; do not decode every unrelated full profile or
turn the repository into a cache. Preserve deterministic duplicate detection,
strict selected errors, overlay fallthrough only on not-found, and fresh
point-in-time reads on every operation.
Add counting/parser-observation tests where stable behavior can be observed,
plus benchmarks for small and large catalogs reporting time and allocations.
Exercise valid selection, unrelated malformed files, selected malformed files,
duplicates, overlay fallthrough, and repeated lookup. Do not add brittle exact
allocation thresholds to ordinary tests.
Run focused profile and root integration tests, benchmarks for diagnostic
comparison, race tests, repository tests, and vet.
## Stage 12: Correct Artifact Semantics, Cancellation, And Hash Tests
**Findings:** S09-F01, S09-F02, S09-F05.
Treat an explicitly typed empty inline reference as a valid zero-byte artifact,
including `InlineWithURI`. Keep absence at the input map/reference boundary and
compute the same metadata and opaque equality value used for other bodies.
For ordinary file references, inspect the target before opening and again
after opening; reject anything that is not a regular file under the decision
above. Replace unbounded `io.ReadAll` with a normal synchronous chunked read
that checks `ctx.Err()` before open, before and after each read, and before
publishing the artifact. Do not return partial artifacts, add a hidden size
limit, or launch an abandoned reader goroutine.
Add source-parity cases for empty and nonempty inline, inline-with-URI, and file
content. Add a platform-appropriate FIFO regression proving the known FIFO is
rejected without requiring an external writer, and cancellation cases for a
pre-canceled file and a progressing regular-file read. Run them repeatedly and
under the race detector.
Replace exact SHA-256 literals with relational assertions: nonempty and stable
for repeat reads, equal for equal inline/file bodies, unequal for changed
bodies, and propagated opaquely through preparation. Do not document or test a
specific algorithm.
Update public GoDoc and the internal source document to describe regular-file
support and cancellation checkpoints. Run focused package, use-case, root,
race, repository test, and vet gates.
## Stage 13: Make Rendering Cancellation-Aware And Reuse Artifact Text
**Findings:** S09-F03, S09-F04.
Check context before session work, before and after each template parse and
execution, before and after every message, and before returning the completed
prompt. Make the `input` helper return an error when cancellation is observed.
Do not run template execution in a detached goroutine.
Within one `Render` call, lazily convert each named artifact body to text once
and memoize that string for the session and all messages. Build the cached
string in 64 KiB chunks with one pre-grown `strings.Builder`, checking the
context between chunks. Preserve bytes exactly, including invalid UTF-8; do not
cache across render calls or mutate artifacts. Unknown and nil inputs retain
their current errors, and a canceled conversion must not publish or cache a
partial string.
Add deterministic tests for pre-cancellation, cancellation during the chunked
input conversion, and cancellation observed after final-message execution.
Require the context identity and no partial prompt while preserving active-
context template errors. Add benchmarks for one and repeated references across
session and messages; report allocations without hard-coded timing limits.
Run focused renderer/use-case/root tests, benchmarks, repeated race tests,
repository tests, and vet.
## Stage 14: Preserve Exact JSON Validation Semantics
**Findings:** S10-F01, S10-F05.
Create one helper for decoding exactly one JSON value with
`json.Decoder.UseNumber` and required EOF after trailing whitespace. Use it for
schema documents and JSON Schema instance values so large integers, precise
decimals, and exponents retain exact `json.Number` semantics through
compilation, prepared metadata, copying, and validation.
For plain `ValidationJSON`, use a non-materializing complete-document syntax
check such as `json.Valid`; do not build a generic tree. Preserve the current
result distinction: malformed generated JSON is a completed failed validation,
not an operational error, and original output bytes remain unchanged.
Add focused OS and `fs.FS` cases around `2^53`, `1e400`, precise decimals,
ordinary numbers, malformed syntax, and trailing values. Exercise schema
`const`, minimum/maximum, and `multipleOf`, and verify the public structured
schema retains exact numeric values. Add benchmarks for scalar, object, and
large-array JSON validation with allocation reporting but no wall-clock
contract.
Update format/internal validation documentation only where it currently
implies float64-limited semantics. Run focused validator/use-case/root tests,
benchmarks, race tests, repository tests, and vet.
## Stage 15: Escape Schema Resources And Compile Once Per Operation
**Findings:** S10-F02, S10-F03.
Represent schema compiler resources with `url.URL` rather than string
concatenation. Use canonical escaped file URLs for OS paths and a private
scheme URL whose path segments are escaped for `fs.FS`. Preserve separators,
decode resource paths exactly once at the loader boundary, and continue
rejecting remote and escaping references. Legal filenames containing percent,
space, `#`, `?`, or Unicode must compile, including contained relative
references.
Unify JSON Schema preparation around `validate.PreparedValidation`:
- the shared preparation pipeline must create one operation-local compiled
plan and derive provider-facing root schema metadata from that plan;
- `Prepare` may discard the plan after returning metadata;
- `Run` must retain and use the plan for its one operation so the schema graph
is not loaded or compiled again during validation; and
- `PrepareExecution` must retain the same plan in its frozen payload.
Remove the document-only `SchemaDocumentLoader` capability if it has no
remaining production caller. Do not add an engine-wide or cross-operation
schema cache. Keep a clear private preparation carrier in `internal/usecase`
if needed so public `domain.PreparedRun` remains free of validator interfaces.
Replace the existing legal-filename expected failure with valid behavior and
retain a genuine compiler-registration failure only if reachable through a
valid source. Add public parity tests for invalid keywords, malformed and
missing direct/second-level references, unsupported dialects, escapes, remote
references, and valid multi-document graphs. A counting source must show each
document read once per operation and fresh reads across separate operations.
Update internal source, validator, and runner documentation for the unified
plan lifetime. Run focused validator/use-case/root tests, race tests,
repository tests, and vet.
## Stage 16: Make Validation Cancellation Authoritative
**Finding:** S10-F04.
Apply the cancellation decision fixed above. Thread context through schema
resource loaders and all Promptkit-controlled read/decode helpers. Read opened
schema files in context-checked chunks. Check the context immediately before
and after JSON decoding, schema compilation, and schema execution; if
cancellation occurred during a synchronous dependency call, return the context
error instead of a schema or successful validation result. Do not publish a
partial plan or validation result.
Do not place arbitrary `fs.FS` calls or JSON Schema work in goroutines merely
to race them against `ctx.Done()`. Tests must therefore distinguish:
- prompt cancellation before work;
- cancellation between controlled read chunks;
- cancellation that becomes authoritative immediately after a synchronous
compile or validation call returns; and
- the documented limitation that Promptkit cannot preempt a dependency method
that never returns.
Use deterministic controlled readers/contexts rather than sleeps. Assert no
goroutine growth or leaked work and preserve operational validation and public
context identities. Update validator GoDoc and internal source/runner documents
to state the synchronous cancellation boundary accurately.
Run focused cancellation tests normally, repeatedly, and under the race
detector, followed by repository tests and vet.
## Stage 17: Repair And Protect The Retained Internal Repair Path
**Findings:** S12-F01, S12-F02, S12-F03.
Retain the internal repair architecture. Extend `RepairRequest` with
`ExecutionTargetPresence` and carry the resolved presence bits unchanged into
the default repairer's `GenerateRequest`. Factor one use-case-local constructor
for common initial/repair generation fields—effective target, presence,
credential, backend identity, session, and structured output—while keeping the
initial and repair prompts intentionally separate.
Accumulate every completed generation response's five token-usage fields into
run-level usage. The final content/raw output/artifact continues to come from
the last candidate, while usage includes initial generation and every completed
repair exactly once. A repair call that returns an error still returns no
partial public result under current error semantics.
Replace the one-attempt-only repair coverage with a compact state-machine
table for:
- initial success with no repair;
- ineligible basic validation despite a positive budget;
- explicit zero and inherited-zero presence across initial and repair calls;
- success before a larger budget is exhausted;
- exact exhaustion of a larger budget; and
- advancement of attempt number, maximum, prior output, diagnostics, final
status, cumulative usage, and collaborator call count.
Retain the distinct capacity integration proving initial and repair generation
use the same backend pool and one whole-run admission lease, but simplify it if
the new focused table makes repair-state assertions redundant.
Update the internal runner and capacity documents for presence fidelity and
cumulative usage. Do not alter `NewRunner` to install a repairer, public GoDoc
that says the engine is single-pass, or the future public repair roadmap.
Run focused use-case, prepared, capacity, and race tests, followed by repository
tests and vet.
## Stage 18: Preserve Transport Error Identities And Test Deterministically
**Findings:** S14-F01, S04-F01, S16-F02. Timeout overflow S14-F02 is already
resolved through Stage 1's shared bound.
Preserve the underlying `http.Client.Do` error in the chain while retaining
`internal/llm.ErrRequestFailed` and the public generation category. Do not add
headers, request content, endpoints, or provider bodies to error text.
Cancellation and deadline identities must survive caller cancellation, caller
deadline, generation deadline, and whole-request client timeout.
Replace the port-9999 test with a controlled round tripper or `httptest`
endpoint that records the selected URL and returns a deliberate result. It must
make no host-dependent connection and must separately prove empty configured
base acceptance and request-endpoint precedence.
Extend the existing external ordinary-run cancellation test to require both
`ErrLLMGenerate` and `context.Canceled`; retain lower-layer tests only for their
distinct error owners.
Update the OpenAI-compatible integration and internal LLM documents for error
identity behavior. Run focused LLM and root tests with repetition and the race
detector, followed by repository tests and vet.
## Stage 19: Validate And Compose Effective Provider Endpoints
**Finding:** S14-F03.
Add one source-neutral OpenAI-compatible base-endpoint validator in
`internal/domain`, alongside the effective execution target invariant. It must
trim surrounding configuration whitespace, require absolute HTTP or HTTPS with
a host, and reject user information, query, and fragment. Use it from backend
registration, in-memory and file profiles, resolved request overrides, and the
built-in client defense while preserving each boundary's existing config,
profile-load, invalid-request, or LLM error category.
An empty configured base URL remains valid for a built-in client because a
resolved request endpoint may supply it later. Validate only a nonempty
configured base at construction, and always validate the final selected
endpoint before transport. Backends and endpoint-only profiles retain their
existing nonempty endpoint requirements.
Compose the completion URL through parsed URL operations (prefer
`url.JoinPath`) so nested paths and trailing slashes reach exactly one
`/chat/completions` suffix. Never append to a raw string.
Add endpoint tables for HTTP and HTTPS, hosts, nested paths, repeated trailing
slashes, queries, fragments, user information, relative paths, missing hosts,
unsupported schemes, and request/profile/config error mapping. Require every
invalid selected endpoint to fail before transport.
Update the architecture/internal overview for domain endpoint invariants and
the OpenAI-compatible integration and internal LLM documents for URL behavior.
Run focused domain/backend/profile/use-case/LLM/root tests, repetition and race
tests where ownership crosses packages, followed by repository tests and vet.
## Stage 20: Bound And Strictly Frame Successful Provider Responses
**Findings:** S14-F04, S14-F05.
Enforce the 16 MiB successful-response decision without first copying the
entire body. Reject an over-limit `Content-Length` immediately, but also wrap
the body in a counting/limited reader that reads at most one byte beyond the
limit so chunked or dishonest responses cannot bypass it. Exactly-limit bodies
remain valid. Always close the body; do not drain an unbounded oversized
stream.
Decode exactly one response object. After the first decode, require only
trailing JSON whitespace and EOF. A second value, non-whitespace suffix,
truncated body, malformed JSON, or size overflow returns
`ErrMalformedResponse` with no partial response and no provider content in the
error.
Add streaming tests just below, at, and one byte over the limit with and
without `Content-Length`, plus a continuing oversized stream. Assert bounded
bytes read, timely return, no partial result, and closure. Add trailing
whitespace success and trailing garbage/second-value failures. Retain ordinary
response mapping and redaction cases.
Document the fixed successful-response boundary and strict one-document rule
in the integration and internal LLM documents. Explicitly leave bounded
non-success error-envelope parsing to the structured-generation-error roadmap.
Run focused transport tests normally, repeatedly, and under race, followed by
repository tests and vet.
## Stage 21: Consolidate Transport Test Scaffolding
**Finding:** S14-F06.
After transport behavior is stable, introduce one small recording-provider
fixture for common request capture and successful/error response setup.
Organize focused tables around request mapping, authentication, endpoint
composition, timeout/error identity, and response framing. Keep specialized
round trippers/readers for cancellation, deadlines, byte counts, continuing
streams, and body closure.
Retain every existing durable assertion for method, path, headers,
authentication, omission and explicit presence, reserved fields, cache
control, structured output, response mapping, usage, error redaction, and
timeout precedence. Retain all Stage 18 through 20 regressions. Delete repeated
servers, generic-map decoding, and response literals only where the fixture
makes the owning behavior clearer; do not replace wire assertions with a broad
snapshot.
Run the LLM suite normally, with shuffle/repetition, and under the race
detector. Deliberately inspect the resulting test inventory against the audit's
transport matrix before running repository tests and vet.
## Stage 22: Restore One Canonical Maintainer Validation Workflow
**Finding:** S16-F01.
Make `docs/development.md` the canonical owner of the complete local maintainer
workflow, as assigned by the documentation policy. Its validation section must
include, from the repository root:
```sh
go test ./...
go test -race ./...
go vet ./...
go build ./...
go run ./examples/go-library/prepare
go run ./examples/go-library/run
```
It must also own the Go formatting, local Markdown link, `git diff --check`,
workspace/vendor/replacement, generated-output, credential, and working-tree
hygiene checks used before accepting changes.
Change the testing policy to state the semantic requirements and link to that
canonical workflow instead of maintaining a partial competing command list.
Change the release procedure to invoke the development-guide validation as a
release prerequisite rather than presenting a separately maintained copy;
retain release-specific metadata, candidate, tag, and publication commands in
the release document.
Run both examples offline and confirm that a missing or invalid Run example
fixture makes its command fail. Validate all changed Markdown links and ensure
current-state documentation describes only the implemented workflow.
## Stage 23: Complete Traceability And Final Validation
This final stage introduces no new behavior. Review the final tree against the
finding-to-stage table below and the evidence in `audit.md`. Confirm every
canonical group is implemented and every source-specific symptom retains its
required regression and error boundary. Do not mark a finding resolved merely
because a nearby refactor landed.
Run the complete development-guide workflow, including both examples, all
formatting and link checks, and repository hygiene. Also run shuffled ordinary
tests and repeated race-enabled tests for the changed concurrency,
cancellation, prepared, validation, repair, and transport packages. Run the
accepted performance benchmarks for prompt lookup, profile lookup, rendering,
and JSON validation and record only qualitative before/after conclusions; do
not establish release timing promises.
Inspect canonical GoDoc, formats, integration, architecture, and internal
documents against the final implementation. Confirm the public engine still
performs no output repair and the future repair entry remains future work.
Confirm the structured-generation-error feature was not implemented as part of
transport remediation.
Leave `audit-sequence.md`, `audit.md`, and this plan in place for maintainer
review. Retire them only in a separately authorized roadmap-cleanup pass after
the remediation has been reviewed and accepted.
## Finding-To-Stage Traceability
| Stage | Canonical findings | Historical or source-specific records handled with the canonical owner |
| ---: | --- | --- |
| 1 | S05-F01, S17-F01, S14-F02 | S02-F02, S08-F01, S11-F01 |
| 2 | S17-F02 | S11-F02 |
| 3 | S05-F02, S05-F03, S05-F04 | None |
| 4 | S02-F01, S02-F05, S05-F05 | None |
| 5 | S01-F01, S02-F03, S02-F04, S13-F01 | None |
| 6 | S03-F01, S03-F02, S06-F01 | None |
| 7 | S07-F01 | None |
| 8 | S07-F02, S07-F03, S07-F04, S07-F06 | None |
| 9 | S07-F05 | None |
| 10 | S08-F02, S08-F03, S08-F04, S08-F05 | S08-F01 was handled in Stage 1 |
| 11 | S08-F06 | None |
| 12 | S09-F01, S09-F02, S09-F05 | None |
| 13 | S09-F03, S09-F04 | None |
| 14 | S10-F01, S10-F05 | None |
| 15 | S10-F02, S10-F03 | None |
| 16 | S10-F04 | None |
| 17 | S12-F01, S12-F02, S12-F03 | None |
| 18 | S14-F01, S04-F01, S16-F02 | S14-F02 was handled in Stage 1 |
| 19 | S14-F03 | None |
| 20 | S14-F04, S14-F05 | None |
| 21 | S14-F06 | None |
| 22 | S16-F01 | None |
The table maps all 49 canonical remediation groups exactly once. S02-F02 is
the one superseded historical finding retained as evidence under S17-F01;
S08-F01, S11-F01, and S11-F02 retain their source-specific regression
responsibilities without being double-counted as canonical groups.
## Open Questions
None. The numeric contract, resource bounds, path and identity rules,
validation-cancellation limitation, repair retention, transport response
limit, and documentation ownership required to implement these stages are
fixed above.

View File

@@ -0,0 +1,71 @@
# Structured Generation Errors
## Purpose
Promptkit should give downstream applications actionable, machine-readable
details when the built-in OpenAI-compatible client receives a non-success HTTP
response. Today the client reports only the status code and discards the
provider response body. This makes ordinary configuration failures—such as an
unsupported strict JSON Schema keyword—unnecessarily difficult to diagnose.
## Target End State
Failures from the built-in transport are available through a public typed error
that works with `errors.As` while continuing to match `ErrLLMGenerate` through
`errors.Is`. The error should expose:
- the HTTP status code;
- a normalized provider error code or type when supplied; and
- a bounded provider message extracted from a recognized OpenAI-compatible
JSON error envelope.
The ordinary `Error()` string should remain safe and concise: it should include
the status and provider code or type, but not automatically include the
provider message. Consumers that deliberately want the provider's diagnostic
text can retrieve it from the typed error and apply their own disclosure and
logging policy.
This contract should be available for both ordinary and prepared execution.
Errors returned by injected model clients must continue to preserve their own
identity and should not be converted into fabricated HTTP details.
## Safety And Compatibility Boundaries
- Never expose the raw response body, response headers, endpoint, credentials,
request messages, schema document, or generated content through this API.
- Read only a small fixed maximum response body, reject malformed or
unrecognized envelopes, normalize invalid UTF-8 and control characters, and
cap every retained diagnostic field independently.
- Treat the extracted provider message as untrusted and potentially sensitive:
its GoDoc must tell consumers not to log or display it without applying their
own policy.
- Preserve the existing generic behavior when a response is empty, non-JSON,
oversized, or does not match a recognized error envelope.
- Do not assign retryability from an HTTP status. Promptkit supplies facts;
downstream applications retain retry and presentation policy.
## Recommended API Direction
Prefer one immutable public `GenerationError` value, constructed internally and
carrying accessors for HTTP status, provider code or type, and provider message.
This keeps the exact representation evolvable while giving consumers an
idiomatic `errors.As` contract. Public Go declarations and GoDoc should own the
final exact names and semantics.
The internal OpenAI-compatible client should parse only the conventional
top-level `error` envelope and pass normalized details through the use-case and
public error-mapping layers. The integration documentation should continue to
own wire behavior; the public declarations should own the consumer contract.
## Acceptance Criteria
- A downstream consumer can distinguish a provider HTTP 400 from other
generation failures and obtain a bounded provider explanation when present.
- The typed error still satisfies `errors.Is(err, ErrLLMGenerate)`.
- Existing cancellation, capacity, validation, and injected-client error
identities remain unchanged.
- Tests cover recognized string and numeric provider codes, absent and malformed
envelopes, oversized bodies and fields, control characters, and error-chain
behavior without making live provider requests.
- Current-state GoDoc and the OpenAI-compatible integration and internal-client
documents are updated only when the implementation lands.