promptkit/docs/roadmap/remediation.md

Audit Remediation Implementation Review

Scope And Status

Reviewed all 33 stages of the implementation plan against the selected roadmap, its accepted catalog decisions, and the implementation through 6673284312de23bde86c20eb8de4f3865b157471. The implementation changes begin after planning commit 6a1a32a.

An integration-focused follow-up reviewed the same implementation at 356563a, the documentation-only commit recording the original review. It adds finding 5; the earlier findings remain open and are not duplicated.

The broad architecture and selected simplifications are in place. Five findings remain open below: three correctness/completeness issues, one error-projection efficiency issue, and one documentation-status gap. These reviews implement none of their remedies. The existing race suite passing does not cover the reproduced edge cases.

The accepted artifact ownership, supported convenience-source types, and self-contained schema changes are intentional compatibility changes. No inadvertently strengthened contract was identified beyond those deliberately selected changes. Historical measurements are observations, not performance contracts.

Findings

1. Preserve Caller Cancellation When Authentication Also Overflows [P2]

Status: Complete. Stages: 7, 21–22.

Completion: interrupted body reads preserve caller cancellation before overflow classification. Public below-limit and overflow-boundary regressions retain HTTP facts, unknown acceptance, incomplete cancellation reports, and body closure.

Locations: authentication adapter, ProbeOpenRouterKey, lines 61–67; outcome mapping, authenticationProbe.run; completion, finishHTTP.

The adapter checks the number of returned bytes before the read error. A legal io.Reader can return the allowance-plus-one byte and context.Canceled in the same read. The adapter sets Overflow and returns nil, discarding cancellation. The use case then selects ExecutionFailureOther; finishHTTP does not apply caller cancellation because it only does so for context-classified failures.

Verified reproduction: An offline public SmokeTest uses an HTTP 200 body that cancels the caller and returns context.Canceled with its final bytes. With 1,024 bytes it returns the cancellation and an incomplete report. With 1,048,577 bytes it instead returns:

• nil Go error despite ctx.Err() == context.Canceled;
• Completed == true;
• failure other and reason authentication_response_limit_exceeded.

The body is closed and the read allowance is honored in both cases; those parts of the remedy work. The completed overflow result violates Stage 7's explicit exception for caller cancellation.

Required correction: Preserve context interruption across the adapter and outcome mapping when overflow and interruption coincide. Retain independently observed HTTP facts and unknown credential acceptance. Ordinary overflow must still be a completed diagnostic with nil Go error. Do not change the established treatment of a known, concluded authentication result merely because cancellation arrives later.

Regression: Exercise bytes-plus-cancellation at the overflow boundary through the public diagnostic path, alongside ordinary overflow and interrupted below-limit controls. Keep caller deadline handling consistent with the same existing cancellation policy.

2. Check Schema Self-Containment Independently Of Its Source Filename [P2]

Status: Open. Stages: 11–13.

Locations: schema compilation, compileSchemaDocument, lines 426–445, and its host/fs.FS callers; public wire regression coverage.

The compiler registers the document at its physical file or source-relative retrieval URI. Its denying loader rejects unknown resources, but references back to that registered retrieval URI do not invoke the loader. Consequently, successful local compilation does not establish that the outbound root is self-contained without PromptKit's source context.

Verified reproduction: Load a file named schema.json containing:

{"$defs":{"value":{"type":"integer"}},"$ref":"schema.json#/$defs/value"}

Public PrepareExecution succeeds. An offline HTTP client captures the actual native structured-output payload, returns 3, and RunPrepared passes local validation. However, compiling the captured schema at a neutral submission URI, https://provider.invalid/submitted-schema, with external loading disabled attempts to load https://provider.invalid/schema.json and fails. The source retrieval URI was never sent to the provider. Changing the reference to #/$defs/value succeeds both locally and at the neutral URI.

This is a packaging gap in the selected self-contained-schema remedy, not a claim about any particular provider's schema support or enforcement.

Required correction: Establish self-containment without relying on the source retrieval URI being available upstream. Detect references that depend on that unsent filename/path and provide an actionable error. Preserve valid fragment references, explicit/embedded IDs, anchors, recursion, dialect handling, and checks of otherwise unused schema declarations. Keep source location for root loading and diagnostics; do not add external loading, a lexical ban on all non-fragment references, a bundler, or a provider-acceptance guarantee.

Regression: Cover host-file and fs.FS source-name dependencies, retain explicit-ID and fragment controls, and evaluate the actual outbound schema without the original source base. Preparation must fail before dispatch when the selected self-contained contract is not satisfied.

3. Avoid Duplicating An Already-Retained Error Cause [P3]

Status: Open. Stages: 3–4.

Location: public error projection, mapPublicError, particularly lines 26–38.

The ordinary operation branch joins the owner, the mapped cause, and the original cause. For ordinary errors, the mapped cause already retains the original through wrapping or is that same error. This introduces two paths to the same cause. When an injected client returns an error from another PromptKit engine, each additional enclosing engine repeats the previous cause tree in both branches. Formatting duplicates diagnostics; unsuccessful error-tree searches can also revisit the repeated branches.

Verified reproduction: Begin with a public missing-prompt error, then run a valid parent whose injected client returns that error; repeat with each parent's result. Owner and exact-cause matching remain correct, but formatted byte counts at depths zero through eight are:

158, 361, 767, 1579, 3203, 6451, 12947, 25939, 51923

The input is a linear chain of consumer operations; the projection creates exponential repeated text/traversal. Even one ordinary error repeats its cause unnecessarily. This is introduced by the new join, not required to preserve the owning category.

Algorithmic analysis: Let d be the number of enclosing operations, with a fixed-size leaf diagnostic and fixed-size per-operation context. The ordinary mapped cause already wraps the previous error, yet the join also stores that previous error as another child. Its formatted length therefore follows L(d) = 2*L(d-1) + O(1), giving Theta(2^d) output bytes. The cause graph needs only O(d) distinct error objects, but standard errors.Is/errors.As traverse both paths without remembering previously visited objects. An unsuccessful search consequently follows W(d) = 2*W(d-1) + O(1) as well. The mapper itself performs such unsuccessful category/type searches, so this is not limited to callers that log the final diagnostic.

Retaining one already-mapped cause path removes the avoidable branching: formatted length becomes L(d) = L(d-1) + O(1), and an individual formatting or unsuccessful search over that corrected ordinary chain can be linear in d. This is a structural alternative, not a request for error-graph memoization, new nesting limits, or exact timing/allocation guarantees. It does not assert that a sequence of d separate public operations, each processing its own growing error, has linear total cost. Arbitrary consumer error implementations also retain their own costs.

Required correction: Retain the owning category and each necessary cause without adding an original cause that the projection already retains. Preserve errors.Is/errors.As, specialized capacity/provider/retry projections, and redacted transport formatting. Do not require category exclusivity, discard consumer causes, or impose a new nesting limit.

Regression: Extend composed-error coverage across several enclosing operations. Verify owner and exact-cause matching and that an independent leaf diagnostic is not duplicated at every boundary. Protect this relationship rather than exact complete error wording or a timing threshold.

4. Reconcile The Feature Roadmap's Implementation Status [P3]

Status: Open. Stage: 33.

Location: feature roadmap, Purpose And Status and workstream completion reporting.

The feature roadmap still says neither document describes completed remediation and all workstreams are pending. The implementation plan now records all 33 completed stages and post-change measurements. Stage 33 specifically required actual completion/evidence to be recorded in the roadmap. Its current status contradicts the delivered work and makes it difficult to distinguish original findings from remaining review work.

Required correction: Record that the implementation landed, point to the stage evidence and outstanding findings here, and distinguish historical assessment-only statements from current overall status. Preserve the original baseline measurements and decision provenance as historical evidence. Do not claim all issues are resolved until the findings are addressed; normal roadmap retirement can follow when appropriate.

5. Integrate Operation Ownership With Terminal Observation Classification [P2]

Status: Open. Related stages: 3–4, 25–26.

Locations: terminal classifier, observedFinish, lines 160–192; public error projection, mapPublicError; run boundaries, Run and RunPrepared.

The new operation errors and public projection preserve the enclosing operation's category while retaining child causes. The existing terminal observer classifier still uses a whole-chain sentinel-priority switch. A retained child ErrPromptNotFound wins over the enclosing generation or artifact failure. The generation coordinator, which classifies the actual injected call, reports generation instead. Thus the owning-operation remedy is incomplete at the existing observation boundary even though the returned error now retains the correct outer category.

Verified reproduction: Obtain a child engine's missing-prompt error, then return it from a valid parent engine's injected client or artifact reader. With an observer, the offline race-enabled probe produces:

Parent path	Last attempt failure	Terminal failure	Parent error category retained
Run, initial generation	generation	prompt_not_found	ErrLLMGenerate
RunPrepared, initial generation	generation	prompt_not_found	ErrLLMGenerate
Run, repair generation	generation	prompt_not_found	ErrLLMGenerate
Run, artifact reading	No model attempt	prompt_not_found	ErrArtifactLoad

The exact child error remains matchable in all four cases. The parent's prompt exists; the terminal event nevertheless describes a missing parent prompt. This can misdirect downstream monitoring and recovery. Existing composed-cause tests check returned errors, while existing attempt/terminal consistency tests use simple injected errors and miss this combination.

Required correction: Preserve and consume the owning-operation information when classifying terminal events instead of selecting a nested operation by global sentinel priority. Keep legitimate context/deadline precedence, built-in provider/transport/malformed classifications, retry metadata, and public errors.Is/errors.As access. This does not require a new exported API or category exclusivity. Simply moving generation ahead of other sentinels would create the same ambiguity for other owners.

Regression: Cover observed ordinary/prepared generation, repair failure, and artifact failures carrying another engine's error. Preserve direct missing prompt/profile classification and existing cancellation/provider cases. Do not require universal attempt/terminal equality: later validation or cancellation can legitimately change an operation's final outcome.

Stage-By-Stage Review Record

Each stage was checked against its requested outcome, changed implementation, relevant tests, and documentation. The entries below identify the review focus; “no additional finding” is a review result, not a guarantee of defect-free code.

Stage	Current evidence and result
1	Development guidance selects the latest available local toolchain independently of the module minimum; validation used Go 1.27.3 and go.mod remains at 1.27.0. No additional finding.
2	Both source helpers hand provider maps directly to ConfigKit; the converter is deleted. Source tests cover capture, unsupported values, cycles, ordering, and inheritance. No additional finding.
3	Internal operation errors carry explicit owner/cause with Is/Unwrap; lookup and execution boundaries use them. See finding 3 for facade composition.
4	Public nested-error and specialized-capacity tests retain owner/cause. Repeated cause retention remains inefficient: finding 3.
5	Response-reader errors are recorded below the limited reader and distinguished from parser/framing failures at both decode points; cause/redaction tests cover bytes with errors. No additional finding.
6	Built-in preparation has explicit typed error/evidence markers; inference diagnostics check cancellation first and report construction errors without dispatch. Injected clients retain their separate semantics. No additional finding.
7	Authentication reads at most allowance plus one, closes bodies, and reports explicit overflow. Simultaneous cancellation remains incorrect: finding 1.
8	Artifact GoDoc, defensive adapter copying, migration guidance, and concurrent tests follow the accepted storage transfer. No additional finding.
9	Prepared handles retain an immutable non-zero-sized identity token, not the runner; claim/discard locking remains. Normalized session IDs clone accepted storage. No additional finding.
10	Runner and engine declare ValidationPreparer; concrete validator constructors preserve direct validation and nil-plan behavior. No additional finding.
11	Host and fs.FS paths share compilation, retain root loading, and deny external loader requests. Source-base dependence remains: finding 2.
12	Declared schema traversal uses compiler-resolved dialects, escaped pointers, and schema-bearing positions, excluding annotations. It does not resolve finding 2. No separate traversal finding.
13	Public schema-file precedence/errors, freezing, and native submission are tested; the unused helper result is gone. Outbound relocation coverage misses finding 2.
14	Overlay clones the lower map once, merges owned descendants, and copies replacements. Isolation/null/array cases and comparable allocation results are recorded. No additional finding.
15	Trusted projection copying moved to internal/jsonvalue; fallible runtime input copying remains distinct. Type/nil/number/schema-key behavior is covered. No additional finding.
16	Owned evidence sets merge smaller into larger; callers consume values/flags rather than independent map snapshots. Work-count evidence and requirements tests support the change. No additional finding.
17	Shared branch orchestration preserves with body-dot versus else-dot behavior, inactive paths, and conservative inspection. Independent rendering comparisons remain. No additional finding.
18	Bounded normalization preserves invalid-byte-run replacement and rune boundaries; encoding delegates to standard JSON marshaling. No additional finding.
19	Complete/omission feedback paths use bounded prefixes and retain complete stored diagnostics/candidate behavior. Boundary/escaping tests and comparable allocation results are recorded. No additional finding.
20	Scope resolves before inventory selection; named ranges use binary search, All copies once, and unqualified selection scans without a full prior copy. Ordering, ownership, cancellation, and allocation evidence remain. No additional finding.
21	OpenRouter endpoint derivation and account-response interpretation live in the outbound adapter behind a narrow capability. Finding 1 remains in its read/error ordering.
22	Use cases retain selection/applicability/reporting; target-only detachment replaces unused broad cloners. Overflow/context outcome composition contributes to finding 1.
23	Extracted record validators retain final-handoff traversal, resource indexes, field paths, and existing checks. No raw-source validation phase was added. No additional finding.
24	Input metadata, message/template capture, output conversion, and effective targets are separated while preserving presence, source-relative reads, lazy origins, and final hash assembly. No additional finding.
25	Attempt results replace parallel returns without moving acquisition/release outside attempt scope. Built-in and injected dispatch remain distinct. No additional finding.
26	Shared accounting runs after permit release and preserves distinct HTTP/usage facts; observation and scheduling tests exercise the real coordinator. No additional finding.
27	Synthetic markers exercise value/pointer formatting and direct-key JSON omission; credential, identity, listing, and provider-map documentation is corrected. No additional finding.
28	One internal test owns the complete bundled asset oracle; smaller public option/wire/smoke tests retain distinct assembled behavior. No additional finding.
29	Artifact tests compare same-body inline/file opaque hashes and actual byte/size behavior; benchmark correctness setup remains outside timing. No additional finding.
30	Unconsumed backend presence/queue state, endpoint echo, helper result, and named orphan helpers are removed; effective policy/provenance and public features remain. No additional finding.
31	Runtime definitions use the output contract and captured message content without duplicate fields. Configuration content_file, override defaulting, hash relations, and migration guidance remain. No additional finding.
32	Runner scheduling uses generation.NewInjected; the production wrapper is deleted. Useful manager scenarios use test-only one-shot scaffolding. No additional finding.
33	Current GoDoc/consumer/format/integration/internal guidance largely matches the code; validation passes. Feature-roadmap completion reporting remains stale: finding 4.

Validation And Limits

At the reviewed commit, using Go 1.27.3 with GOTOOLCHAIN=local, GOWORK=off, and GOPROXY=off:

• go test -race -count=1 ./... passed, including the maintained examples.
• go vet ./... passed.
• Tracked Go files produced no gofmt -l output.
• git diff --check 6a1a32a..HEAD passed.
• Additional temporary Go test overlays outside the repository reproduced findings 1–3. The cancellation and relocated-wire-schema assertions fail as described; the nested-error probe measured the duplicated text while retaining correct category/cause matching.

The probes use controlled readers, HTTP transports, and the pinned schema compiler. No credentials, provider calls, external schema retrieval, dependency changes, or publication were needed. They establish local failure mechanisms, not universal provider behavior. Optimization review checked source changes and the recorded comparable measurements; it did not add benchmark thresholds or new release gates.

Integration Follow-Up Coverage

The follow-up focused on interactions between the changed components and the existing consumer workflows, rather than repeating the stage-local review. Production code is unchanged between 6673284 and the follow-up baseline 356563a.

Integration seam	Review and validation result
Convenience sources, ConfigKit inheritance, and optimized provider overlays	An assembled probe captured backend/profile source maps before caller mutation, inherited a profile, and verified recursive backend-to-profile overlay plus detached inspection results. Passed.
Namespace-filtered inventories, inspection, and execution selection	The assembled probe listed a scoped prompt, inspected requirements, validated its request, and prepared it through the same view. An unscoped owning engine then ran the handle with its captured prompt/profile/backend identities. Passed.
Trusted copying, frozen validation, runtime overrides, and repair	The assembled probe combined direct session and reasoning/numeric overrides, appended messages, a schema with internal definitions, and bounded repair. It mutated prepared details, client-owned nested provider/schema values, and the source schema after preparation. Repair and frozen validation remained correct, and inspection/preparation hashes agreed. Passed.
Generation accounting, retries/repair, and capacity	Existing assembled tests for shared/independent pools, backoff permit release, logical admission, repair, frozen credentials, usage, and observer events passed under the race detector. No additional scheduling/accounting finding.
Prepared-owner token, namespace views, and observation isolation	Existing lifecycle, foreign-owner, concurrent claim/discard, delegated-operation, and observer-isolation tests passed under the race detector. No additional lifecycle finding.
Operation-owned errors and existing terminal observers	New offline observed-call probes fail in all four combinations documented in finding 5. Existing plain injected-error tests still pass.
Shared schema compiler and provider-native output	Source-to-native-payload review retains finding 2; the new assembled frozen-schema/repair probe passed for a genuinely self-contained fragment-reference schema. No duplicate finding.
HTTP read classification and diagnostic adapter/reporting	Adapter/coordinator/use-case review and the focused internal suites retain the existing read-failure classifications. The overflow/cancellation conflict remains finding 1. No duplicate finding.
Requirement-evidence ownership and renderer/preflight behavior	Evaluation/branch ownership was reviewed alongside actual rendering and the assembled requirements/preflight workflow. Existing prompt requirements and use-case suites passed; no additional recognition scope or execution-readiness guarantee was introduced.

Validation used Go 1.27.3 with GOTOOLCHAIN=local, GOWORK=off, and GOPROXY=off:

• The focused race-enabled public integration selection passed, covering retry/repair/accounting, backend capacity, frozen prepared execution, delegation, namespace views, schema-file wiring, programmatic sources, and existing injected-error observation classifications.
• Race-enabled configuration, prompt, validation, generation, use-case, and model-client suites passed.
• The additional composed-snapshot/repair probe passed under the race detector.
• The new operation-ownership observation probe failed as documented in finding 5, independently of the already recorded findings.

Temporary probes were supplied using Go test overlays outside the repository; no implementation or permanent test changes were made. All provider behavior was controlled offline. The prior full-suite/vet evidence above remains applicable to the unchanged implementation; this follow-up adds focused integration evidence rather than a new release gate.

Algorithmic Follow-Up Coverage

A further source-based review at 7203e5d focused on algorithmic correctness, efficiency, and scalability of the implemented stages. It confirms the exponential behavior in finding 3 and adds the recurrence and refactoring opportunity there, rather than creating a duplicate finding. No additional distinct algorithmic or ordinary-correctness finding was identified in this pass. All five existing findings remain open.

The review used the plan and roadmap to exclude deliberate algorithm choices: the small-to-large evidence merge, owned provider overlay, bounded feedback encoding and prefix selection, namespace binary searches, declared-schema location compilation, trusted-copy traversal policies, and selected ownership copies were not reopened. In particular, the chosen unqualified-list scan and schema-location strategy are not reported as new optimization requests.

For the remaining implementation glue:

• Source helpers map records once before ConfigKit's handoff, without the old recursive compatibility expansion. ConfigKit's own processing is outside this review's implementation scope.
• Extracted record validators use a backend-ID index rather than scanning every backend for each profile. Prompt input duplicate detection uses a set; input and message capture process their records once, with one final definition hash and one lazily shared metadata snapshot. Existing sorted inventory construction and dependency/template work are separate costs, not disguised linear-time guarantees.
• The response-error recorder adds constant metadata work per read. The authentication recognizer parses a fixed number of document levels/fields, so its processing is linear in the consumed body size, not multiplied by arbitrary input depth through a custom recursive recognizer.
• Generation accounting updates a fixed number of usage/event fields per attempt and appends captured attempt records with amortized constant append cost. Storing A requested attempt records requires O(A) space. Error classification additionally pays for the supplied cause graph, including finding 3; arbitrary collaborator/callback work is not constant-time.
• The repair request uses the original message prefix plus a bounded-count suffix, not an accumulating conversation from every prior repair. Repeated prefix processing follows the amount of message data actually resubmitted; it is not an unintended growing-history algorithm.

The complexity finding follows from current source and the standard library's error-tree traversal. No new benchmarks, real-world performance collection, provider calls, implementation changes, or public complexity guarantees were introduced. Existing validation evidence above applies to the unchanged code.