promptkit/docs/roadmap/remediation.md

# Audit Remediation Implementation Review

## Scope And Status

Reviewed all 33 stages of the [implementation plan](implementation.md) against
the [selected roadmap](audit-remediation.md), 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 were still open at that audit baseline and are not duplicated.

The broad architecture and selected simplifications are in place. All five
findings are now addressed, with completion markers below: three correctness/
completeness issues, one error-projection efficiency issue, and one documentation
status gap. Finding descriptions and audit evidence retain the historical
pre-fix behavior; the completion markers describe the delivered corrections.

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](../../internal/llm/openrouter_authentication.go),
`ProbeOpenRouterKey`, lines 61–67;
[outcome mapping](../../internal/usecase/smoke_authentication.go),
`authenticationProbe.run`;
[completion](../../internal/usecase/smoke_tester.go), `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:** Complete. **Stages:** 11–13.

Completion: shared compilation checks both the source base and an unrelated
submission base through the same compiler rules, including unused declarations.
Host and filesystem regressions preserve fragment, explicit-ID, and embedded-ID
references while rejecting unsent source-name dependencies.

**Locations:** [schema compilation](../../internal/validate/standard_validator.go),
`compileSchemaDocument`, lines 426–445, and its host/`fs.FS` callers;
[public wire regression coverage](../../schema_file_contract_test.go).

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:

```json
{"$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:** Complete. **Stages:** 3–4.

Completion: ordinary operation projection retains its mapped cause once, without
joining the original subtree a second time. Multi-engine regression coverage
checks exact cause/owner matching and one occurrence of an independent leaf;
specialized projection and redaction tests remain intact.

**Location:** [public error projection](../../errors.go), `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:

```text
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:** Complete. **Stage:** 33.

Completion: the feature roadmap now records the delivered sprint, links stage
evidence and this follow-up record, and distinguishes historical baseline
measurements from current status without prematurely closing finding 5.

**Location:** [feature roadmap](audit-remediation.md), 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:** Complete. **Related stages:** 3–4, 25–26.

Completion: a private public-boundary wrapper retains operation ownership and a
single mapped cause. Terminal observations classify that owner while retaining
context precedence and generation-specific transport/provider details. Observed
run, prepared-run, repair, and artifact regressions retain child cause identity.

**Locations:** [terminal classifier](../../observations.go), `observedFinish`,
lines 160–192; [public error projection](../../errors.go), `mapPublicError`;
[run boundaries](../../engine.go), `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 findings were open at that audit baseline; their completion
markers above own the current remediation status.

Remediation completion validation: Go 1.27.3 with `GOTOOLCHAIN=local`,
`GOWORK=off`, and `GOPROXY=off` passed the complete
`go test -race -count=1 ./...` suite and `go vet ./...`. New regressions cover
authentication read interruption at the overflow boundary, host/FS schema
portability, relocated native schema submission, nested-cause duplication, and
operation-owned terminal observations. Focused checks passed before each code
unit was committed. Documentation links, formatting, and whitespace were checked.

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.