Files
promptkit/docs/roadmap/planning-assessment.md

881 lines
55 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Audit Remediation Planning Assessment
## Purpose And Authority
Work through the unresolved design and evidence-dependent choices in the
[audit remediation roadmap](audit-remediation.md) before writing its staged
implementation plan. Each numbered topic below is an assessment unit: gather
the stated evidence, compare viable approaches, and record a recommendation
before proceeding to the next topic.
The [catalog's six accepted decisions](audit/catalog.md#resolved-decisions)
remain authoritative. This assessment selects ways to implement those decisions;
it does not reopen them or repeat the repository-wide audits. Catalog numbers
below identify the relevant findings, not additional implementation stages.
Assessment started on 2026-11-05 at clean commit
`21fea46fb3b0ce9483db2399c9587f4a2323d687`. Assessment records below distinguish
completed analysis from still-pending implementation.
## Assessment Method And Outputs
Read the [contributor guide](../development.md) and applicable policies before
each topic. Reuse existing audit evidence, source inspection, and successful
checks where sufficient. Prefer graph discovery for callers and dependencies,
then inspect the relevant code, contracts, and tests. Follow the
[configuration-boundary ADR](../adr/0001-configuration-boundary.md).
This is analysis, not remediation. Do not change production code, dependencies,
installed toolchains, public contracts, or repository tests during the
assessment. No prototype or alternative implementation is needed. Use existing
offline benchmarks and tools where helpful; temporary measurement harnesses
outside the repository are appropriate only when needed to answer a specific
question. Record their setup sufficiently to interpret or repeat the results.
Do not use live inference, credentials, or private production data.
Append an assessment record beneath each completed topic containing:
- **Evidence:** repository commit, concrete declarations/callers/tests, relevant
dependency version, and measurements or commands actually used.
- **Alternatives:** plausible approaches and their semantic, maintenance, and
resource tradeoffs. Do not manufacture alternatives to an obvious correction.
- **Recommendation:** the selected design or a justified proposed deferral,
affected boundaries, and reasoning. Distinguish observed results from inference.
- **Plan handoff:** consequential signatures, fields, algorithms, or ownership
transitions to specify; dependencies; migration/documentation impact; and
implementation-time validation needed.
- **Status:** assessed, or a specific remaining uncertainty and the next bounded
investigation needed. Assessment completion does not mean remediation complete.
For measurements, record the selected Go version, workload size/shape, and the
relevant time, allocations, bytes, or retention evidence. Use small ordinary
cases and larger valid cases to distinguish fixed overhead from scaling.
Repeat noisy measurements only as needed; do not introduce benchmark thresholds,
a generalized benchmark project, or a recurring release gate. Code-based work
counts can establish avoidable traversal without timing every helper. Do not
present estimated improvements as measured results from unimplemented code.
Recommendations should settle consequential implementation choices, not private
helper names or equivalent coding details. Proceed between topics without new
human-review or publication gates. If evidence requires changing an accepted
decision or broadening scope, identify that question for the maintainer rather
than silently selecting a new contract.
## 1. Establish The Evidence And Toolchain Baseline
**Catalog:** 1; applies to subsequent measurements.
Collect the current commit and worktree state, dependency versions, available
Go installations, and the effective toolchain used by existing development and
release commands. Compare the selected local toolchain against the specific
networking fixes identified in the security audit, using authoritative upstream
information where needed. Distinguish library/client applicability from server
or platform-only findings and uncertainty about downstream builds.
Recommend the toolchain to use and any environment update needed before
remediation validation. Record an unavailable patched toolchain as environment
work to schedule, not permission to install one during assessment or to stop
independent source analysis. Retain the accepted separation between maintainer
selection and consumer minimum; do not add consumer advisories or scan gates.
### Assessment Record
**Status:** Assessed; no environment installation is needed.
**Evidence:** Default `/snap/bin/go` reports Go 1.27.0, but the existing module
cache contains executable Go 1.27.3 at
`/home/eric/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.27.3.linux-amd64/bin/go`;
invoking it reports `go1.27.3 linux/amd64`. Other inspected SDK/cache installs
are older. `go.mod` declares 1.27.0, ConfigKit 1.1.0, jsonschema/v6 6.0.2, and
ReleaseKit 0.9.0. `scripts/releasekit.sh` sets `GOTOOLCHAIN=local`, so its
toolchain is selected by `PATH`, not automatically upgraded.
**Recommendation and alternatives:** Put the existing 1.27.3 binary directory
first on `PATH`, with `GOTOOLCHAIN=local` and `GOWORK=off`, for subsequent
assessment commands and implementation validation. The authoritative affected
ranges for [GO-2026-6611](https://pkg.go.dev/vuln/GO-2026-6611) and
[GO-2026-6809](https://pkg.go.dev/vuln/GO-2026-6809) exclude 1.27.3: the two
specific audited exposures are addressed by that selection. This is a focused
version/applicability reassessment, not a new full vulnerability scan or a claim
about consumer binaries. Continuing with default 1.27.0 retains the identified
exposure and is not recommended; installation is unnecessary.
**Plan handoff:** Clarify ordinary development toolchain selection in
`docs/development.md`; leave `go 1.27.0` unchanged. Record the actually selected
patch when validating remediation. No consumer advisory or recurring scan gate.
## 2. Specify Direct ConfigKit Source Handoff
**Catalog:** 3, 5; informs 24.
Inspect the pinned ConfigKit `Mapping` contract and implementation at the
snapshot/error boundary, plus both PromptKit convenience helpers and their
callers. Establish which ordinary field projections remain, when values are
captured, and how unsupported values, cycles, and shared graphs reach deferred
diagnostics. Inventory existing fixtures using formerly converted types.
Recommend the exact removal boundary for the compatibility converter, supported
handoff cases, and migration examples. Identify which current tests remain
meaningful and which assert intentionally removed accommodation. Do not design
a replacement converter, pre-validation pass, or upstream feature. Existing
upstream ownership evidence is sufficient; exhaustive ConfigKit limit testing
is not an assessment deliverable.
### Assessment Record
**Status:** Assessed; remove the compatibility adapter without replacement.
**Evidence:** `ProfileSource` (`profiles.go:29`) and `BackendSource`
(`backends.go:82`) already project optional typed fields into built-in values
and wrap definitions in `[]any`. Their only recursive compatibility dependency
is `providerMapForMapping(config.ExtraParams)`. The pinned ConfigKit 1.1.0
`source.go:Mapping` promises immediate capture and deferred unsupported/cyclic
diagnostics. Its `internal/engine/source.go:Mapping` builds an owned document
and retains `mapIssue`; `readSource` reports it when consumed. PromptKit's
`provider_values.go` instead expands maps/slices before that bounded handoff
and parses `json.Number` through a lossy float fallback. Existing
`sources_contract_test.go` mainly protects flat-map ownership, not this broader
input boundary.
**Recommendation and alternatives:** Assign `config.ExtraParams` directly into
the ordinary fields map in both helpers, then call `configkit.Mapping` as now.
Delete `provider_values.go` after its two callers are replaced. A shallow map
copy adds no capture protection beyond Mapping; another recursive converter or
raw validation would retain the responsibility being removed. Runtime overrides
continue using their separate JSON-value boundary.
**Plan handoff:** Cover both helpers with nested built-in maps/`[]any`, mutation
after source creation, and deferred failures for representative `json.Number`,
named scalar/typed container, pointer, and cyclic inputs. Keep construction
order/optional-field coverage. Explain caller conversion to supported forms:
for example, `[]string` becomes `[]any` of strings; callers choose and check
numeric conversion themselves. Do not assert preserved numeric spelling or
duplicate ConfigKit's entire resource-limit suite. Source-helper GoDoc and
migration notes own the changed accepted-value boundary.
## 3. Specify Self-Contained Schema Resolution And Compilation
**Catalog:** 6, 15, 21; coordinates with 32.
Inspect the pinned schema compiler's resource registration/loading facilities,
the four direct/prepared and host-file/`fs.FS` paths, native-schema projection,
and runner validation interfaces. Establish where root loading, reference
resolution, compilation, cancellation, and malformed-output short-circuiting
occur.
Use a compact semantic case inventory to distinguish references resolved inside
the supplied document from references needing another document. Include local
fragments, anchors, recursion, and supported identifiers/base-URI behavior;
preserve existing dialect support rather than adding it. Do not equate an
external-looking reference string with external resource loading without
checking resolution semantics.
Recommend the shared compilation boundary, needed source adapters and internal
interface signatures, and external-resource rejection/error path. Identify
obsolete loaders/tests and retained containment, single-file, freezing, and
direct-validation cases. Specify how the provider receives the self-contained
document unchanged in meaning. No bundler, schema rewriting prototype, or
live provider compatibility experiment is needed.
### Assessment Record
**Status:** Assessed; use explicit root registration and a no-external-resource
loader, with coverage for unused schema definitions.
**Evidence:** `internal/validate/standard_validator.go` has four compilation
paths; its root loaders already preserve JSON numbers, validate the 2020-12
dialect, and check source containment/context. `Runner.prepareValidation` uses
only `ValidationPreparer`, despite storing `Validator`. In pinned jsonschema
6.0.2, `Compiler.AddResource` registers the decoded document, `roots.go` collects
embedded IDs/anchors, and `loader.go` consults registered resources and bundled
metaschemas before the custom loader.
An offline dependency-contract probe in
`/tmp/promptkit-assessment-aqn3B8/schema.go`, run with Go 1.27.3 using
`go run -mod=readonly` and `GOPROXY=off`, registered one document and installed a
loader that only counts/rejects external loads. Fragment, anchor, recursion,
absolute self-ID, embedded child-ID, and standard metaschema cases compiled
with zero external loads; a real external reference failed with one load.
Crucially, an unused `$defs` entry with an external reference did not trigger a
load until compiled explicitly. An example payload containing a literal `$ref`
did not trigger loading. Repeating with a retrieval URI different from the
document's explicit ID gave the same results. A denying loader alone does not establish
that every declared schema is self-contained.
**Recommendation and alternatives:** Retain source-specific root loading, then
share `compileSchemaDocument(ctx, resourceURL, document)` returning the compiled
executor. Register the root, retain Draft2020, and install a context-aware loader
returning an actionable external-resource error without filesystem/network I/O.
Keep compiler resolution authoritative for IDs and anchors. To cover otherwise
unused declarations, enumerate schema locations using a small, explicit table
of the pinned 2020-12 schema-bearing keyword positions (including its supported
legacy positions) and compile those locations with the same compiler/cache.
Do not traverse annotation/example objects as schemas. This bounded policy walk
is not a URI resolver, bundler, or general schema framework. A lexical ban on
non-fragment references rejects embedded IDs; root-only compilation misses the
demonstrated unused-definition case. Neither is recommended.
**Plan handoff:** Specify escaped JSON-pointer location enumeration and retained
embedded-ID/dynamic-reference semantics in focused cases; preserve dialect and
number handling. Both direct schema callbacks call the shared compilation only
after instance decoding, while prepared paths call it eagerly as before. Remove
external loaders, not root path containment/read helpers. Change runner fields
and constructor parameters to `validate.ValidationPreparer`, retaining the nil
no-op and nil-plan error behavior. Validator constructors can return their
concrete pointer types so existing direct `Validate` callers and preparation
both compile; update the internal engine Options validator slot to the
preparation interface too. No public facade signature changes are needed.
Replace external resource success fixtures with early-error cases, retain independent root-source
tests, and add public `WithSchemaFile` option/replacement/error coverage.
## 4. Specify Ownership And Retention Remedies
**Catalog:** 4, 16, 18.
Trace public artifact adaptation and prepared-handle creation, claim, discard,
details, and namespace-view operations. Inspect which references keep an engine
alive and which synchronization owns state changes. Trace session normalization
to its retained copies. Use reference/lifetime analysis first; use an existing
allocation profile only if the retained graph is unclear.
Recommend the artifact contract/documentation changes under accepted ownership
transfer, retaining the defensive copy initially. Specify the prepared-handle
ownership identity and state transitions without relying on distinct zero-sized
allocations comparing unequal. Identify the session-string detachment point.
List relevant concurrent and mutation regressions for implementation, without
building a new ownership framework or changing lifetimes during assessment.
### Assessment Record
**Status:** Assessed; narrow references rather than adding lifecycle machinery.
**Evidence:** `artifact_reader.go:publicArtifactReaderAdapter.Read` copies the
returned body only after the consumer call returns. `PreparedExecution` in
`internal/usecase/prepared_execution.go` retains `owner *Runner` even after its
mutex-protected claim/discard drops the payload; `Details` uses separately
captured data. `Runner` owns repositories, clients, and admission. Namespace
views delegate to the same internal engine/runner. `NormalizeSessionID` in
`internal/domain/session.go` returns `strings.TrimSpace(raw)` directly.
**Recommendation and alternatives:** Document successful artifact return as
ownership transfer, as accepted, and keep adapter copying. Replace the prepared
handle's runner pointer with a pointer to a non-zero-sized private identity
object allocated once by each runner constructor and copied into its handles.
Keep that immutable token after claim/discard; it retains no runner graph and
preserves foreign-owner validation order without a new owner-field race.
Clearing a mutable runner pointer is viable but complicates synchronized checks
and error ordering unnecessarily. Clone the validated nonempty normalized
session string with `strings.Clone` before returning it; this detaches padded
input without imposing a new limit.
**Plan handoff:** Retain ready-to-claimed/discarded transitions under the current
mutex, private payload clearing, one-shot execution, and detached details.
Protect cross-engine rejection, same-engine namespace views, concurrent
claim/discard, and details after discard/use. Artifact regressions must use
independently returned buffers and avoid claiming safety for contract-violating
readers. Existing mutation, trimming, Unicode, hash, and wire assertions remain
the semantic oracle; a GC timing assertion is unnecessary.
## 5. Specify Error Ownership And Preparation-Stage Information
**Catalog:** 7, 8, 9; informs 25.
Trace representative collaborator failures through internal wrapping and public
translation, including direct lookup failures and nested public sentinels.
Inspect body-reader errors at initial decoding and end verification, current
redaction, context precedence, and retry classification. Trace diagnostic
preparation versus dispatch, including legitimate completed no-dispatch cases.
Recommend how owning-operation information and preparation-stage failures are
represented and propagated. Specify the distinction between stream I/O failure,
malformed/truncated content, size overflow, and cancellation using existing
boundaries where possible. Give a small expected-outcome matrix of public
category, retained cause, diagnostic completion, and dispatch behavior. Retained
causes may match multiple sentinels; category exclusivity is not the target.
### Assessment Record
**Status:** Assessed; carry ownership and preparation provenance explicitly.
**Evidence:** `errors.go:publicErrorFor` uses whole-chain `errors.Is` in a fixed
order; `mapPublicError` also searches nested capacity/exhaustion/provider types
before normal mapping. `wrapGenerationError` and artifact loading add operation
sentinels but no separately readable owning category. In
`internal/llm/openai_compatible_client.go:decodeOpenAIChatResponse`, both decoder
calls replace non-context errors with malformed-response messages. In
`generation.Coordinator.generate`, `PrepareGeneration` fails before any capacity
attempt, while `SmokeTester.runInference` concludes every generation error as
report data. `GenerationEvidence` currently lacks preparation-stage information.
**Recommendation and alternatives:** Add a small internal use-case operation
error carrying category and cause, with `Unwrap`/`Is` behavior. Apply it at owning
boundaries and let facade mapping select that category independently of nested
causes. Preserve legitimate direct not-found mappings when assigning the
category. Keep specialized public capacity/provider/exhaustion projections, but
do not let their presence inside a collaborator cause erase the outer owning
category. A globally reordered sentinel switch cannot express this distinction.
Add `PreparationFailed bool` to internal `GenerationEvidence`, set only at the
built-in `PrepareGeneration` failure return. Diagnostic orchestration checks it
after caller cancellation and returns `ErrInvalidRequest` with the cause.
Do not infer this stage from error identity or attempt count; injected failures
and intentional missing-key/capacity reports keep their established semantics.
This needs no new public field or retry policy.
Wrap the successful-response reader with a small read-error recorder beneath
the existing limited reader. At decoding/EOF verification failure, prefer an
observed context error; preserve size overflow as malformed; otherwise propagate
an actual non-EOF reader failure through `requestFailedError`. Parser-generated
syntax/type/truncation failures, EOF-truncated JSON, and actual extra values
remain malformed. Distinguish errors returned by the reader from those created
by `encoding/json`, rather than categorizing every `io.ErrUnexpectedEOF` alike.
Retain redacted default formatting and exact underlying read causes.
**Plan handoff:** Protect these expected outcomes at the public boundary:
| Case | Owning category / result | Retained evidence |
| --- | --- | --- |
| Client returns child missing-prompt error | `ErrLLMGenerate`; error | Child cause remains matchable, including its category. |
| Reader returns the same child error | `ErrArtifactLoad`; error | Child cause remains matchable. |
| Direct missing prompt/profile | Existing not-found category; error | No conversion into a generic load failure. |
| 2xx body read fails during decode or final EOF check | Generation plus transport; diagnostic transport failure | Original reader cause, no raw body/detail formatting. |
| Invalid/truncated JSON, extra value, size overflow | Existing malformed-response behavior | No new retry or parser-detail disclosure. |
| Built-in diagnostic preparation fails | `ErrInvalidRequest`; not a completed diagnostic | Zero dispatch and original preparation cause. |
| Missing required credential/capacity report | Existing completed diagnostic outcome | No inferred construction failure. |
| Caller cancellation | Existing cancellation/error handling | Cancellation takes precedence over publishing success. |
## 6. Specify Authentication Overflow Reporting And Adapter Ownership
**Catalog:** 2, 26, 27; uses topic 5's error/stage conclusions.
Inspect public/domain diagnostic DTOs, outcome labels, body ownership, HTTP
evidence, authentication recognition, and facade projection. Determine whether
the selected overflow outcome fits existing fields or needs an explicit
additive representation. Inventory actual smoke-copy consumers rather than
assuming full reports need cloning.
Recommend concrete field/type or outcome changes and their public/internal
mapping. State the facts reported for overflow: acceptance unknown, explicit
limit evidence, and independently available HTTP status/timing. Explain the
interaction with non-success statuses and cancellation without discarding
established independently known evidence. Preserve neutral reporting and privacy.
Specify the bounded read/overflow detection operation around the accepted 1 MiB
allowance, including exact-boundary, missing/inaccurate length, and close behavior.
Recommend the outbound protocol-adapter seam and narrow target-copy operation.
Keep the body-bound fix independently implementable before adapter relocation;
do not add a public budget option or general diagnostic plugin framework.
### Assessment Record
**Status:** Assessed; one reason constant suffices, without a new report field.
**Evidence:** Public/domain smoke reports already have `Reason`, `Failure`,
nullable `CredentialsAccepted`, HTTP status on attempts, and timing. Current
`smoke_authentication.go` reads a body only to recognize HTTP 200; 401 establishes
rejection without reading it. Other statuses retain their existing outcome.
`smoke_tester.go` copies these facts into attempts and `finishHTTP` handles
caller cancellation. The outbound client already owns authenticated GET and
HTTP evidence. The broad domain request/report cloners are used only by their
self-tests and a target-only copy respectively, as recorded in audit 11.
**Recommendation and alternatives:** Add matching public/domain
`SmokeTestReasonAuthenticationResponseLimitExceeded` constants with value
`authentication_response_limit_exceeded`. On a 200 recognition overflow set that
reason, existing `ExecutionFailureOther`, acceptance nil, and retain HTTP/timing
facts; conclude with a nil Go error unless caller cancellation applies. A new
general failure enum or report Boolean is unnecessary: the existing reason
already provides an explicit machine-readable diagnostic. Keep 401 rejection
and non-200 behavior independent of unused response bodies.
Read at most allowance plus one byte with `io.LimitReader`/`io.ReadAll`, detect
length greater than 1 MiB before JSON recognition, and close without draining.
Do not trust `ContentLength`. Existing bounded bytes plus one-byte lookahead are
sufficient; streaming the small account parser adds complexity without need.
Cancellation takes its existing precedence; overflow is not malformed content.
Move account-response recognition and bounded response ownership into an
`internal/llm` OpenRouter authentication operation, returning HTTP evidence,
optional acceptance, overflow/unrecognized indicators, and operational error.
Move default endpoint derivation into a small llm adapter helper used by
selection; explicit overrides remain unchanged. Use-case orchestration maps
those factual results to report reasons/completion and consumes a narrow
authentication capability alongside reachability HTTP capability. No reverse
dependency or plugin registry is needed. Replace target-only full-report cloning
with direct target/provider-map detachment; retain facade projection and active
backend cloners.
**Plan handoff:** Fix and test the byte bound before relocation. Include exact
limit versus limit-plus-one, escaping/invalid bodies, inaccurate declared length,
closure, 401 without body consumption, context interruption, redaction, and
detached report values. Update reason GoDoc/public conversion and focused
diagnostic documentation in the same implementation unit. No secret-bearing
body, header, or provider detail is added to report data.
## 7. Assess JSON Copying Compatibility And Provider Overlay Cost
**Catalog:** 19, 24; uses topic 2's converter-removal boundary.
Inventory callers of facade copying, runtime JSON validation/copying, prepared
snapshots, and configuration overlay. Build a compact compatibility matrix for
accepted concrete types, nil/indirection handling, exact numbers, empty keys,
error behavior, bounds, and mutable ownership. Separate trusted-value copying
from fallible input acceptance and source encoding.
Collect existing overlay benchmark/allocation evidence, supplemented only where
needed with representative depth, breadth, overlapping/disjoint maps, and array
replacement cases within current limits. Identify repeated visits/copies and
their construction-time cost; respect the existing bounded-depth qualification.
Recommend which traversal can actually be removed, the shared operation's
contract, and the overlay algorithm/ownership model. Prefer retaining a small
distinct walker over a configurable framework when policies differ. Recommend
implementing or explicitly deferring the optional overlay optimization based on
expected benefit and complexity; identify estimates as such. Do not add errors,
panics, or silent fallbacks to an infallible facade projection.
### Assessment Record
**Status:** Assessed; implement the overlay optimization and centralize trusted
copy ownership, retaining distinct acceptance policies.
**Evidence:** `targets.go:overlayProviderMap` clones the complete lower subtree
at every recursive overlap, then clones replacement values. `convert.go:copyAny`
has fast paths plus `copyReflectValue`; graph callers are prepared details,
client requests, execution targets, and structured schemas. These values have
already crossed an acceptance/copy boundary. `internal/jsonvalue` instead owns
fallible input acceptance and bounded copying. The concrete differences are:
| Property | Runtime `Copy` / `CopyMap` | Existing facade projection |
| --- | --- | --- |
| Numbers and typed collections | Validates; preserves compatible concrete types and exact `json.Number`. | Copies accepted values without numeric interpretation. |
| Indirection / null | Dereferences and normalizes null, sometimes changing container type. | Reflection preserves pointers/typed nils; fast paths have their existing nil behavior. |
| Keys and errors | General schema keys allowed; provider keys cannot be empty; fallible limits/cycle checks. | Infallible; trusts earlier acceptance; no new checks. |
| Mutation | Returns independent accepted state. | Returns independent client/inspection state. |
**Measurements:** An external Go test overlay calls the unchanged private
overlay function with nested chains containing two scalar keys per level and
an array leaf; inputs are built outside timing. Go 1.27.3, linux/amd64,
Intel i5-7260U, `-cpu=1 -benchmem -benchtime=50x -count=3`:
| Depth / width / overlap | Observed ns/op range | B/op | Allocs/op |
| --- | --- | --- | --- |
| 2 / 2 / overlapping | 20,104–34,618 | 9,496 | 236 |
| 12 / 2 / overlapping | 263,387–287,814 | 117,869 | 2,609 |
| 40 / 2 / overlapping | 2,355,659–2,580,220 | about 1,241,620 | about 22,672 |
| 4 / 16 / overlapping | 168,483–178,889 | 65,434 | 1,591 |
| 12 / 2 / disjoint upper root | 49,437–53,966 | 32,496 | 686 |
Harness: `/tmp/promptkit-assessment-aqn3B8/config_test.go`, mapped only in the
Go invocation by `overlay.json`; command was `go test -mod=readonly -overlay=…
./internal/configuration -run '^$' -bench '^BenchmarkAssessmentOverlay$'` with
the flags above, `GOTOOLCHAIN=local GOWORK=off GOPROXY=off`. Repository sources
were not edited. These are baseline costs, not measured optimized speedups.
**Recommendation and alternatives:** Clone lower once, then recurse into that
owned result for overlapping built-in maps, copying only upper replacements.
This removes repeated descendant clones without another index or framework;
the measured allocations justify this small construction-time improvement.
Preserve null/array replacement, accepted types, error boundaries, and isolation.
Put infallible trusted cloning in `internal/jsonvalue.CloneTrusted(any) any` and
route facade nested values through it. Preserve current fast-path/null/type
behavior explicitly. Do not substitute fallible `Copy` and hide its error.
Keep validation traversal separate where normalization differs; share only
structural helpers that actually match. Moving trusted cloning to its existing
owner removes the facade's independent implementation responsibility, but does
not by itself eliminate both traversal algorithms. Forcing one configurable
reflection walker is not justified by this evidence.
**Plan handoff:** Remove source conversion first; preserve independently
specified mutation/type/nil/schema-key cases. Measure the changed overlay with
these same shapes after implementation; do not promise the timing ranges or
global linear-time validation. No optional optimization is deferred here.
## 8. Assess Requirement Evidence Accumulation And Traversal
**Catalog:** 10, 23.
Trace ownership and merging of condition-evidence sets and the shared versus
node-specific responsibilities of `if`/`with`. Inspect real-renderer comparison
tests and scope/reachability cases. Collect operation counts or targeted
baseline measurements across increasing left-heavy recognized conditions;
include a small ordinary case so pathological scaling is not the only input.
Recommend an accumulation/merge algorithm and a narrow branch-orchestration
boundary, explaining expected work without promising a new universal complexity
contract. Keep the algorithmic fix distinct from traversal cleanup. Specify
preserved dot/root scopes, inactive computed names, and conservative incomplete
analysis. No interpreter, expanded grammar, or expression limit is proposed.
### Assessment Record
**Status:** Assessed; use owned-set merging and one explicit branch protocol.
**Evidence:** `requirements.go:evaluateValue` creates fresh missing-name sets;
`eq`/`ne` combine recursive evaluation results using
`mergeConditionEvaluations`, which copies every member into fresh sets on each
merge. Later evaluation reads operand values/known flags, not independent
snapshots of these sets. For a left-heavy tree with n distinct missing names,
the source performs `2 + 3 + … + n` membership insertions in merges, excluding
leaf creation: 9 at n=4, 135 at n=16, and 8,255 at n=128. This is an exact
source-derived work count for the described case, not measured elapsed time.
The ordinary two-name case needs only two such insertions.
`requirementWalker.visit` repeats branch selection, dependency-delta tracking,
inactive traversal, and limitation recording for IfNode/WithNode. The latter
changes body dot but keeps else dot. Existing known-condition/scope tests compare
against actual Go rendering and remain independent semantic evidence.
**Recommendation and alternatives:** Treat intermediate evidence sets as owned
by their evaluation result. Merge into the larger existing set, adding only
the smaller set's members (nil sets transfer directly), while combining flags
as now. This removes the repeated copying in the demonstrated left-heavy case
and bounds repeated movement better on balanced trees. A single global mutable
accumulator would complicate branch-local evidence and is not needed.
Share IfNode/WithNode orchestration through their existing `parse.BranchNode`
representation, handling the With node's body-dot calculation explicitly inside
the narrow helper. Keep recognition/evaluation separately readable; do not
merge the inspector with template execution. A generic visitor/callback
framework or more recognized syntax would not help this defect.
**Plan handoff:** Protect set union/flag outcomes and representative left-heavy
inspection without asserting helper allocation counts as a durable contract.
Retain known/skipped/unknown branch classification, changed-dot/root access,
inactive computed-name behavior, and meaningful limitation bookkeeping. The
algorithmic fix may land before branch extraction; both remain selected work.
## 9. Assess Bounded Repair-Feedback Encoding
**Catalog:** 17.
Inspect feedback framing, JSON escaping, truncation, and the locations retaining
full diagnostics and prior output. Measure current formatting with ordinary,
large, and heavily escaped synthetic diagnostic strings, separating final output
size from temporary allocations. Identify which complete serializations account
for the avoidable memory cost.
Compare a bounded incremental approach with the current implementation using
source-level algorithm and complexity reasoning; no alternative encoder needs
to be built. Recommend an encoding/truncation approach and its preservation
cases, or a justified proposed deferral if benefit does not warrant complexity.
Any deferral must acknowledge the outstanding allocation issue. Do not cap raw
diagnostics, discard stored errors, or assert an exact peak-heap guarantee.
### Assessment Record
**Status:** Assessed; implement bounded normalization/encoding, not a deferral.
**Evidence:** `repairer.go:formatRepairDiagnostics` normalizes every complete
diagnostic, marshals the whole list, then marshals complete entries again when
over budget. Truncation also marshals candidate prefixes repeatedly. Full errors
and previous candidate are retained separately from this feedback string.
An external test overlay calls only the unchanged formatter. With Go 1.27.3,
the topic 7 CPU/environment, `-cpu=1 -benchmem -benchtime=20x -count=3`, one
64-byte ASCII diagnostic costs about 780–1,198 ns and 224 B/op. A 1 MiB ASCII
diagnostic costs 5.83–7.99 ms and about 6.81 MB/op; a 1 MiB string repeating
`<&>` plus NUL costs 27.7–39.1 ms and 44.12 MB/op. Observed output lengths are
68, 65,536, and 65,531 bytes respectively. These are baseline aggregate
allocations, not live-heap peaks or measured savings from a replacement.
Harness: `/tmp/promptkit-assessment-aqn3B8/usecase_test.go` via
`feedback-overlay.json`; `go test -mod=readonly -overlay=… ./internal/usecase
-run '^$' -bench '^BenchmarkAssessmentFeedback$'` with the flags above. Input
strings are built outside timing; input/output lengths are recorded after
timing to avoid Go's ResetTimer clearing custom metrics. A preliminary run
without these metrics found similar allocation magnitudes; the final timings
above are noisy and are not claimed as stable throughput.
**Recommendation and alternatives:** Preserve the complete-list fast result
when it fits by first attempting bounded construction. If it cannot fit, build
the bounded omission form with the existing reservation/order rules. Process
only each candidate's potentially useful normalized prefix, retaining the
current `strings.ToValidUTF8` behavior for runs of invalid bytes and rune-safe
truncation. Use standard `json.Marshal` on bounded normalized strings and the
existing bounded-prefix selection logic rather than implementing a general
JSON escaper. Escaping may expand bounded scratch space, but must not cause
serialization proportional to an unbounded original diagnostic. A pure
`json.Encoder` into a limited writer is insufficient if it constructs the whole
encoded string before writing. The measured large-string allocation justifies
the focused change; retaining current allocation is not recommended.
**Plan handoff:** Keep the 64 KiB output rule, ordering, omission marker, valid
JSON, HTML/control escaping, Unicode/invalid-byte normalization, and full stored
errors/candidate. Include fit, exact boundary, oversized single entry, many
entries, and heavily escaped/invalid strings. Measure the same baseline shapes
after implementation; do not make peak-memory or timing values a public promise.
## 10. Assess Namespace Listing Index Tradeoffs
**Catalog:** 20.
Inspect inventory capture, namespace identity, ordering, filtered retrieval,
cancellation, and result detachment. Gather baseline time/allocation evidence for
unfiltered listings and sparse, dense, and absent namespace matches at ordinary
and larger valid inventory sizes. Consider both a single request and repeated
namespace inspection, without assuming every consumer repeatedly lists entries.
Compare captured buckets/ranges with the existing scan. Account for construction
work, retained index memory, and query output copying; do not optimize query time
while ignoring its construction/retention tradeoff. Recommend a specific layout
and query algorithm, or a justified proposed deferral. No pagination, runtime
cache, or new ordering/cancellation guarantee is needed.
### Assessment Record
**Status:** Assessed; use ranges in existing sorted inventories, not a new
persistent namespace index.
**Evidence:** Configuration captures lexically sorted ID slices in `load.go`.
Its `listing.go` copies the whole slice before engine `filterIdentityList`
scans and appends matches, even for All mode. Qualified IDs for an exact
`namespace + "/"` prefix form a contiguous sorted range; unqualified IDs need
not form one range. The current private inventory interface has no filter input.
An external overlay benchmark of the unchanged engine filter uses a list
callback matching configuration's whole-slice copy. Synthetic sorted valid IDs
are built outside timing: sparse selects one ID, dense selects half, absent
selects none. Go 1.27.3, same CPU/environment as topic 7,
`-cpu=1 -benchmem -benchtime=100x -count=3`:
| Total IDs | Mode | ns/op range | B/op | Allocs/op |
| --- | --- | --- | --- | --- |
| 32 | All / sparse / dense / absent | 1,176–1,332 / 759–937 / 1,053–1,875 / 797–819 | 1,472 / 528 / 960 / 512 | 5 / 2 / 4 / 1 |
| 1,024 | All / sparse / dense / absent | 41,851–48,965 / 13,860–19,334 / 38,416–45,031 / 15,258–19,282 | 78,144 / 18,448 / 37,184 / 18,432 | 11 / 2 / 9 / 1 |
| 4,096 | All / sparse / dense / absent | 194,479–202,439 / 92,265–95,215 / 148,209–151,180 / 93,157–94,526 | 305,472 / 65,552 / 166,208 / 65,536 | 14 / 2 / 12 / 1 |
Harness: `/tmp/promptkit-assessment-aqn3B8/listing_test.go` via
`listing-overlay.json`; `go test -mod=readonly -overlay=… ./internal/engine
-run '^$' -bench '^BenchmarkAssessmentListing$'` with the flags above. This
isolates filtering/copy cost, not full engine construction or public projection.
Repeated namespace requests repeat that measured whole-inventory allocation.
**Recommendation and alternatives:** Resolve the filter in the engine first.
Pass an internal `configuration.IdentityFilter{All bool, Namespace string}` to
the captured inventory: All copies the complete slice once; named selection
uses binary search to find the prefix's start/end and copies only that range;
empty Namespace with All false scans for unqualified entries directly from
immutable state, without first copying everything. Apply the same selection
to backend/profile IDs and prompt identities, preserving ordering and final
context checks. No public signature changes are needed.
This selects the audit's sorted-range alternative: named selection has expected
comparison work proportional to log N plus its output, while unqualified
selection remains a scan. Retaining buckets/range maps could reduce repeated
named lookup to a map operation and index unqualified results, but adds per-kind
retained state and construction work even when a consumer never lists resources.
Existing sorting already supplies named ranges with no new persistent index or
additional construction traversal. The measured wasted copying justifies the
simple range remedy, not extra permanent buckets. These cost comparisons are
design estimates; no alternative implementation was benchmarked.
**Plan handoff:** Update the private inventory interface and its test fakes;
test exact namespace prefixes, prefix-sharing names, mixed unqualified entries,
All/default/view scope, absent results, mutation isolation, and cancellation.
Repeat the baseline measurements after implementation. Do not introduce a
cache, pagination, or a new unqualified-query complexity guarantee. This is a
selected remedy, not a proposed deferral of catalog 20.
## 11. Select Remaining Refactoring, Test, And Cleanup Boundaries
**Catalog:** 11–14, 22, 25, 28–34; consolidate related conclusions above.
Reuse existing audit evidence and perform targeted caller checks rather than
reauditing these findings. Identify the configuration helper boundaries,
generation attempt-result fields, and accounting seam consistent with topics
2–8. Locate the real coordinator assembly to use in scheduling tests and the
behavioral coverage that must survive wrapper retirement.
Select one independent bundled-catalog test oracle and the smaller distinct
risks owned elsewhere. Identify privacy-formatting cases, public schema-file
coverage from topic 3, and replacement assertions for opaque hashes. Map obsolete
fields/helpers to actual callers and tests, including serialized definition-hash
effects. Record the owning documentation for each correction/migration.
Recommend a concrete removal/extraction inventory, preservation tests, and
ordering dependencies. Do not infer that self-tests justify production helpers,
or that lack of example coverage makes supported APIs obsolete. This is not a
new deletion sweep, exact helper-design exercise, or complexity-metric target.
### Assessment Record
**Status:** Assessed; retain focused package boundaries and remove the recorded
residue, not a broader subsystem.
**Evidence:** Current `validateEffective`, `materializePrompt`, and
`buildExecutionProfiles` match the responsibility mixtures in audit 03. Current
`Coordinator.execute` returns outcome, dispatched, two durations, and start time;
the retry loop independently projects these into evidence and observation.
Graph caller checks reconfirm eight test callers for `capacity.NewClient`, only
the respective self-test for `RelativePath`/`resourceNamespaceFromID`, and no
callers for `stringPtr`. `loaded.backends` is only populated during resolution;
`QueueCapacitySet` is only assigned/asserted. Domain `OutputFormat` duplicates
the validated output contract; runtime `ContentFile` is not populated by prompt
capture. `HashDefinition` marshals these fields, so their deletion changes hashes.
Private reachability `initialEndpoint` and the third `fileSource` result have
the unused roles recorded in audit 12.
`internal/configuration/bundled_test.go` already has the complete independent
17-profile/model inventory, but uses formatted-map plugin checks. Public bundle,
wire, and smoke tests repeat expected models/defaults. `formatting.go` implements
RunRequest formatting; `types.go` omits its direct API key from JSON. Existing
artifact size-hint tests calculate SHA-256 despite opaque public hash semantics.
**Recommendation and alternatives:** Use these concrete extraction/removal units:
- Keep `validateEffective`'s final-resource/index traversal; extract backend,
profile, and prompt record validators with explicit record/path/index inputs.
Keep typed presence interpretation together in execution-profile construction.
- Split prompt materialization into input/metadata projection, message
content/template capture, and output-contract conversion. Retain lazy origin
lookup, source-relative reads, and a single final inspection/hash assembly.
Do not add a second configuration validation pass or generic field schema.
- Replace execute's parallel returns with an `attemptResult` carrying
`Outcome llm.AttemptOutcome`, `Dispatched`, `CapacityWait`, `RequestDuration`,
and `StartedAt`. Share usage/evidence/event projection helpers where the facts
match. Keep built-in retry and injected one-shot control flow separate; release
permits before accounting callbacks/backoff. Topic 5's stage marker remains
independent of attempt accounting.
- Change the initial/repair scheduling test to `generation.NewInjected` with
the same capacity manager, then retire production `capacity/client.go` and
its retention prose. Test capacity primitives directly or with a small
test-only wrapper; preserve FIFO/cancel/race/panic/independent-pool scenarios.
- Keep the complete bundled asset oracle in `internal/configuration/bundled_test.go`
and use structural plugin expectations there. Public option tests own toggles,
order, isolation, and override behavior. Keep a small wire test of a controlled
model/plugin override and a smoke test of configured default selection; do not
repeat the complete maintained model matrix outside its owner.
- Add marker-based formatting tests for RunRequest values/pointers with `%v`,
`%+v`, `%#v`, and direct String/GoString, plus direct-key JSON omission.
Put markers in omitted contents, such as variables, bodies, and execution
settings, not IDs deliberately present in summaries. Do not pin the entire
formatted or serialized shape.
- Replace size-hint literal digests with same-body inline/file hash equality;
retain body/actual-size/read-boundary assertions. Prepare an opaque benchmark
expected hash outside timing instead of independently selecting its algorithm.
- Delete only unused backend presence capture/field list/map and the runtime
queue-presence flag; keep profile/input presence and ConfigKit provenance.
Remove duplicate captured output format/runtime message-file fields and their
unreachable fallback, preserving actual YAML loading and override defaults.
Remove the private probe endpoint echo, discarded helper return, orphan
wrappers/self-tests, and unused pointer helper identified above.
Keeping existing broad functions is viable but leaves the audited review burden;
a general configuration/attempt framework would obscure the explicit rules.
No evidence justifies either alternative over these small extractions.
**Plan handoff:** Correct credential, provider-map, and identity comments in their
canonical owners. Update internal assembly/capacity/validation descriptions when
code lands, and consumer/format/wire guidance plus release migration notes for
ownership, schema, accepted source values, and definition hashes. Include topic
3's public schema-file wiring cases. Preserve request override empty-format
defaulting and inspection/preparation hash equality, not literal old hashes.
Independent caller/behavior checks, not graph reachability alone, justify deletion.
## 12. Consolidate Recommendations For Implementation Planning
Review the assessment records in order and reconcile cross-topic choices.
Produce a compact handoff covering all 34 catalog entries: selected remedy or
explicit proposed deferral, supporting assessment topic, consequential design
choice, dependencies, and implementation-time validation. Findings whose remedy
was already settled need only a brief mapping, not another investigation.
Ensure source conversion disappears rather than migrating into shared copying;
schema simplification and compilation agree; errors/stages precede accounting
refactoring; the authentication fix can precede relocation; and actual
coordinator tests precede wrapper removal. Separate optional cleanup from fixes
so a deferral does not obstruct unrelated remediation.
Record any scope/contract question that still needs maintainer direction with a
recommendation and viable alternative. A deferral is a recommendation, not
silent authorization to drop accepted work. Unavailable measurements or tooling
must be identified, together with whether they could change the recommendation;
do not present a conditional choice as a settled implementation instruction.
Once consequential choices are settled, the subsequent implementation plan can
define focused ordered stages, selected designs, migration/documentation work,
and proportionate validation. Implementation correctness, race regressions,
post-change measurements, and release checks remain work for implementation and
release preparation. No claim of remediation completion follows from this
assessment, and no new recurring release or human-review gate is introduced.
### Assessment Record
**Status:** Assessed; all twelve topics have recommendations. No remediation is
implemented, and no catalog entry is proposed for deferral.
**Evidence and recommendation:** The source review, dependency probe, baseline
measurements, and caller checks recorded above settle the consequential choices.
Keep the roadmap's accepted product scope. Use the following mapping as the
handoff inventory, not as 34 mandatory implementation stages:
| Catalog | Assessment topic | Recommended remedy | Implementation evidence to retain/add |
| --- | --- | --- | --- |
| 1 | 1 | Select existing Go 1.27.3; document ordinary-development selection. | Actual toolchain version and focused advisory reassessment; no consumer-minimum bump. |
| 2 | 6 | Bounded authentication read and explicit limit reason. | Exact/overflow and inaccurate-length cases, closure, neutral acceptance facts. |
| 3 | 2 | Remove pre-Mapping recursive source expansion. | Direct nested handoff/capture and ConfigKit-deferred cycle diagnostics. |
| 4 | 4 | Artifact result ownership transfer; keep defensive adapter copy. | Public GoDoc, independently owned concurrent returns, adapter isolation. |
| 5 | 2 | Remove lossy number conversion with the same adapter. | Unsupported-number deferred error and supported built-in numeric handoff, not new precision promises. |
| 6 | 3 | Self-contained root plus compiler-enforced reference policy. | Internal/embedded IDs and anchors work; external references, including unused definitions, fail before dispatch. |
| 7 | 5 | Explicit owning-operation category across public mapping. | Nested client/reader causes and specialized public error preservation. |
| 8 | 5 | Record actual response-reader failures separately. | Both decoder stages, retained cause/redaction, malformed versus transport distinction. |
| 9 | 5 | Internal built-in preparation-failure evidence flag. | Invalid diagnostic construction returns an ordinary Go error; intentional reports remain reports. |
| 10 | 8 | Merge owned temporary sets small-to-large. | Representative deep condition outcomes and preserved independent renderer comparisons. |
| 11 | 11 | Test request formatting privacy at its public boundary. | Synthetic-marker assertions for value/pointer formats and direct-key JSON omission. |
| 12 | 11 | Correct credential-inspection GoDoc/example. | Explicit profile environment selection versus backend-derived default. |
| 13 | 11 | Correct provider-map integration summary. | Link to format-owned recursive overlay versus runtime replacement semantics. |
| 14 | 11 | Correct identity/listing comments. | Literal IDs and current collection release-version ownership. |
| 15 | 3 | Public single-schema-file wiring tests. | Real temporary schema, source replacement/precedence, relevant path errors. |
| 16 | 4 | Standalone immutable runner ownership token. | Foreign owner, namespace view, one-shot claim/discard, retained details. |
| 17 | 9 | Bounded normalized-prefix construction and JSON encoding. | Existing output semantics and same-shape post-change allocation measurements. |
| 18 | 4 | Clone accepted normalized session strings. | Existing Unicode/trim/content/hash/wire behavior. |
| 19 | 7 | Clone lower once; overlay into owned descendants. | Null/array/map semantics, input isolation, repeat baseline measurements. |
| 20 | 10 | Select sorted named ranges before copying. | Sparse/absent/mixed/All/view cases, detachment/cancellation, repeat measurements. |
| 21 | 3 | Shared compilation and explicit preparation interface. | Source-specific containment, direct malformed-output ordering, frozen validation. |
| 22 | 11 | Record-specific configuration and capture helpers. | Final-handoff/presence/origin behavior and meaningful error paths. |
| 23 | 8 | Share branch orchestration; separate evaluation/evidence responsibilities. | If/With scope, skipped paths, partial coverage, independent rendering oracle. |
| 24 | 7 | Centralize trusted clone ownership; preserve distinct validation policies. | Concrete types, nils, numbers, schema keys, caller-owned snapshots; no new projection failure. |
| 25 | 11 | Attempt-result value plus shared accounting projections. | Permit lifetime, status-only retry, injected single dispatch, evidence and usage presence. |
| 26 | 6 | OpenRouter authentication protocol in outbound adapter. | Default/override destination, anonymous probe, factual reporting/privacy. |
| 27 | 6 | Direct target detachment; retire unused smoke cloners. | Provider-map errors, facade ownership, live backend metadata copying. |
| 28 | 11 | One independent bundled catalog oracle. | Smaller option, controlled wire, and diagnostic-selection tests. |
| 29 | 11 | Opaque relational artifact hash assertions. | Complete body/actual size and source parity; benchmark setup outside timing. |
| 30 | 11 | Remove unused backend presence bookkeeping. | Explicit zero/default queue behavior and retained operator provenance. |
| 31 | 11 | Remove duplicate captured fields/fallback. | Real content-file capture, override default, hash equality relationships and migration note. |
| 32 | 11 | Remove private endpoint echo and unused directory return. | Actual sanitized HTTP evidence and public schema-file behavior. |
| 33 | 11 | Remove the three orphan helpers and obsolete self-tests. | Caller check plus retained real discovery/identity/listing behavior. |
| 34 | 11 | Move scheduling test to coordinator; remove production wrapper. | FIFO/cancellation/panic/independent-backend assertions remain with owning components. |
**Dependencies and plan boundaries:** Establish toolchain selection; remove the
source converter before trusted-copy work. Deliver schema policy/shared compiler
and its typed dependency in focused units, with source/wire regressions. Correct
owning errors, stream failures, and diagnostic preparation provenance before
accounting extraction. Authentication bounding/reporting can land before adapter
relocation. Apply the evidence-set fix before branch cleanup where it simplifies
review. Migrate the scheduling test before deleting the wrapper; adjust opaque
hash assertions before removing serialized definition fields. Documentation
corrections and independent cleanup can proceed without waiting on unrelated
performance work.
Split these into prompt-sized implementation stages rather than one stage per
workstream. Keep consequential defects and optional refactoring separately
reviewable, and update the affected canonical documentation with each behavior
change. Stage-level tests should protect the risks in the table rather than
reproduce every helper branch. The final remediation validation can use the
existing offline workflow without creating additional recurring release gates.
**Remaining uncertainty and alternatives:** None requires a new product decision
or external publication before planning. Alternatives and rejected complexity
are recorded in the owning topic. Benchmarks of unimplemented alternatives,
post-change peak heap, provider acceptance, and downstream deployments were not
measured and are not claimed. Baseline results establish the avoidable work;
implementation must verify actual improvement and preservation. The schema probe
exposes a concrete unused-definition case that the plan must include, rather
than silently declaring a rejecting loader sufficient.
**Scope verification:** Only this documentation file is changed in PromptKit.
The temporary harnesses are outside the repository, exercise unchanged code or
the pinned dependency, and make no provider calls. No production implementation,
repository test, module metadata, installed toolchain, public contract, or
release has been changed by the assessment.