642 lines
39 KiB
Markdown
642 lines
39 KiB
Markdown
# Audit Remediation Roadmap
|
||
|
||
## Purpose And Status
|
||
|
||
Prepare PromptKit's audited implementation for a focused round of correctness,
|
||
configuration simplification, resource-use improvements, and internal cleanup.
|
||
This document defines the target and work boundaries. The companion
|
||
[implementation plan](implementation.md) owns staged steps and validation;
|
||
neither document describes completed remediation yet.
|
||
|
||
Baseline: repository commit `bd3dfa9`, with the
|
||
[audit catalog](audit/catalog.md) covering 41 source findings consolidated into
|
||
34 remediation entries. All six catalog decisions have been accepted; their
|
||
implementation remains pending. The planning assessment was completed at
|
||
`3ef2a9e` against the unchanged production baseline, with evidence gathering
|
||
starting at `21fea46`. Its recommendations, measurements, and subsequent review
|
||
refinements are consolidated here; the separate assessment document is retired.
|
||
The catalog and supporting audits own finding evidence and decision rationale.
|
||
This roadmap owns the selected end state, work boundaries, assessment evidence,
|
||
and workstream status. Earlier remediation suggestions in the source audits do
|
||
not override accepted decisions.
|
||
|
||
All workstreams below are pending implementation; their planning assessment is
|
||
complete. No finding is proposed for deferral. Concrete designs below are the
|
||
selected planning recommendations, not claims of tested replacement behavior.
|
||
Private helper names are descriptive unless identifying an existing declaration.
|
||
Catalog numbers are traceability identifiers, not a required implementation
|
||
order or 34 separate delivery stages. The scope
|
||
contains confirmed defects, documentation/test gaps, and maintainability work;
|
||
those categories do not have equal urgency or automatically become release gates.
|
||
|
||
## Targeted End State
|
||
|
||
PromptKit remains an application-neutral Go library with the existing public
|
||
facade and ConfigKit composition boundary. The intended changes are:
|
||
|
||
- Programmatic backend/profile sources hand supported values to ConfigKit
|
||
directly, removing PromptKit's recursive compatibility conversion.
|
||
- Schemas supplied for validation and native structured output are self-contained
|
||
documents, while in-document references remain supported.
|
||
- Artifact-reader ownership, public errors, and smoke-test outcomes accurately
|
||
express the owning operation and collected evidence.
|
||
- Authentication response consumption is bounded by the selected internal
|
||
allowance, without treating overflow as rejected credentials.
|
||
- Identified unnecessary retention, repeated traversal, and temporary allocation
|
||
are reduced with proportionate changes that preserve established behavior.
|
||
- Production execution and its tests use the real generation coordinator;
|
||
obsolete adapters, state, helpers, and duplicate test expectations are removed.
|
||
- Current documentation describes actual behavior, and deliberate compatibility
|
||
changes are explained where consumers encounter them.
|
||
|
||
The [resolved decisions](audit/catalog.md#resolved-decisions) are authoritative
|
||
for the selected changes. They do not authorize stronger numerical precision,
|
||
provider compatibility, scheduling, hash stability, or performance guarantees.
|
||
|
||
## Scope Boundaries
|
||
|
||
Keep the work within the [architecture policy](../policy/architecture.md) and
|
||
[configuration format boundary](../formats.md). ConfigKit owns source composition
|
||
and its raw-source diagnostics; PromptKit validates its final effective values.
|
||
|
||
This round does not include a schema bundler, a new local-only schema mode, an
|
||
upstream ConfigKit extension, a numeric conversion subsystem, new inference
|
||
limits or application deadlines, or a general reflection/validation framework.
|
||
[Schema bundling](future.md#schema-bundling) remains a future idea. Runtime
|
||
provider overrides retain their distinct supported-value and copying behavior.
|
||
|
||
Refactoring does not authorize expanding recognized template expressions,
|
||
introducing additional retries, adding pagination or runtime inventory caches,
|
||
or redesigning the capacity manager. Tests should protect established semantics,
|
||
not turn incidental implementation choices into new requirements. Existing
|
||
privacy and ownership boundaries remain applicable throughout.
|
||
|
||
## Workstreams
|
||
|
||
### A. Maintainer Toolchain Baseline
|
||
|
||
**Catalog:** 1. **Decision:** 5.
|
||
|
||
**Current gap:** The audited local toolchain predates the networking fixes
|
||
identified by the security audit. Toolchain selection and consumer compatibility
|
||
must remain separate concerns.
|
||
|
||
**Assessed baseline:** Default `/snap/bin/go` reports 1.27.0. The assessment
|
||
found an executable Go 1.27.3 installation at
|
||
`/home/eric/go/pkg/mod/golang.org/toolchain@v0.0.1-go1.27.3.linux-amd64/bin/go`;
|
||
other inspected installations were older. No installation is needed for the
|
||
assessed environment. ConfigKit 1.1.0, jsonschema/v6 6.0.2, and ReleaseKit 0.9.0
|
||
remain pinned. The focused version/applicability check found that 1.27.3 excludes
|
||
the affected ranges of GO-2026-6611 and GO-2026-6809 identified in the audit.
|
||
This was not a new full scan or an assessment of consumer binaries.
|
||
|
||
**Selected work:** Put the newest available local Go binary directory first on
|
||
`PATH` and use `GOTOOLCHAIN=local` for ordinary builds, tests, and release
|
||
validation. The existing ReleaseKit launcher already uses local selection.
|
||
Use the assessed 1.27.3 installation unless a newer local version is available
|
||
when implementation begins. Clarify this in contributor guidance. Assessment
|
||
commands also used `GOWORK=off` for isolation; that is not a new restriction on
|
||
consumer workspaces.
|
||
|
||
Leave `go 1.27.0` unchanged for the assessed toolchain. Follow the
|
||
[release procedure](../release.md#prepare-the-candidate) and
|
||
[consumer-minimum policy](../policy/release.md#consumer-minimum-and-release-toolchain)
|
||
for later series changes and existing exceptions. Selecting a newer validation
|
||
patch does not itself raise the consumer minimum. Continuing with default
|
||
1.27.0 is unnecessary given the installed fixed patch.
|
||
|
||
**Completion evidence:** Record the actual validation toolchain and retain the
|
||
focused reassessment above rather than repeating it without changed evidence.
|
||
Local installation work is needed only if the selected installation becomes
|
||
unavailable or new evidence calls for it. No recurring scan gate or consumer
|
||
advisory about generally applicable Go vulnerabilities is introduced.
|
||
|
||
### B. Configuration And Schema Simplification
|
||
|
||
**Catalog:** 3, 5, 6, 15, 21. **Decisions:** 2 and 4.
|
||
|
||
**Current gap:** Convenience-source conversion broadens ConfigKit's accepted
|
||
input forms by recursively rebuilding them, causing silent numeric changes and
|
||
potentially exponential expansion. Schema validation loads external resources
|
||
locally but sends only the root document to the provider. Compilation is
|
||
repeated across direct/prepared and host-file/`fs.FS` paths.
|
||
|
||
**Selected source handoff:** Assign `config.ExtraParams` directly into the
|
||
ordinary fields map in `ProfileSource` and `BackendSource`, then call
|
||
`configkit.Mapping` as now. Optional typed fields and the `[]any` definition
|
||
wrapper already use supported forms. The recursive compatibility adapter in
|
||
`provider_values.go` is retired. ConfigKit's immediate snapshot and deferred
|
||
unsupported/cyclic diagnostics make an extra shallow copy unnecessary. Do not
|
||
add a replacement converter, precision checker, memoization, or raw-source
|
||
validation pass.
|
||
|
||
**Selected schema design:** Retain source-specific root loading, then share
|
||
`compileSchemaDocument(ctx, resourceURL, document)` for compilation. Register
|
||
the decoded root with `Compiler.AddResource`, retain the existing Draft2020
|
||
selection, and use a context-aware loader that rejects requests for external
|
||
resources with an actionable error and no filesystem/network I/O. The compiler
|
||
continues to own URI resolution, embedded IDs, anchors, and bundled metaschemas.
|
||
Send the self-contained root document to the provider unchanged in meaning.
|
||
|
||
The dependency probe found that unused `$defs` entries are not necessarily
|
||
compiled with the root: a denying loader alone misses their external references.
|
||
Enumerate schema locations with a small explicit table of schema-bearing
|
||
keyword positions supported by the pinned dependency, and compile those
|
||
locations through the same compiler/cache. Document the table's dependency
|
||
relationship. Keep this a location traversal, not a second resolver or general
|
||
schema implementation. A lexical ban on non-fragment references would reject
|
||
valid embedded IDs; traversing arbitrary objects would mistake annotation data
|
||
for schemas. Neither is selected.
|
||
|
||
Cover escaped JSON-pointer locations, annotation/example objects, embedded IDs,
|
||
anchors, recursion, and dynamic references. Check embedded-resource dialect
|
||
handling explicitly rather than assuming the root dialect governs every nested
|
||
resource. Preserve existing supported dialect behavior; do not use this walk to
|
||
introduce broader dialect support or new restrictions. The probe recipe and
|
||
observed results are retained under [planning evidence](#planning-evidence).
|
||
|
||
Direct validation invokes shared compilation only after instance decoding;
|
||
prepared validation compiles eagerly as before. Remove external-resource loaders,
|
||
not root containment/read helpers. Change runner fields/constructor parameters
|
||
and the internal engine Options validator slot to `validate.ValidationPreparer`.
|
||
Return concrete validator pointer types from internal constructors so direct
|
||
`Validate` and preparation callers both compile. Preserve nil-validator no-op,
|
||
nil-plan error, cancellation, and frozen-plan behavior. No public facade
|
||
signature change is required.
|
||
|
||
Cover `WithSchemaFile` through public construction and preparation/validation,
|
||
including source replacement/precedence and relevant file errors. Its unused
|
||
private helper result is retired as described in F. External-resource success
|
||
fixtures become early-error cases, retaining independent root-source tests.
|
||
|
||
**Completion evidence:** Focused source-helper cases demonstrate supported-value
|
||
handoff, immediate source capture, and ConfigKit's deferred unsupported/cyclic
|
||
value diagnostics. Include formerly converted input forms to demonstrate the
|
||
migration boundary: nested built-in maps/lists, mutation after source creation,
|
||
and representative `json.Number`, named scalar/typed container, pointer, and
|
||
cyclic inputs for both helpers. Keep optional-field and construction-order
|
||
coverage; do not duplicate ConfigKit's entire limit suite. Schema cases establish external-reference rejection before provider dispatch, retained
|
||
in-document references, unused external-definition rejection, and an outbound
|
||
self-contained constraint usable without other local files. Retain independent source-path and direct/prepared coverage.
|
||
These checks establish packaging completeness, not universal provider support.
|
||
|
||
**Compatibility:** Explain caller conversion of `json.Number`, named scalars,
|
||
typed containers, arrays, and nested pointers into ConfigKit-supported forms.
|
||
For example, replace `[]string` with `[]any` containing strings; callers choose
|
||
and check numeric conversion themselves. Explain removal of external schema
|
||
loading. No arbitrary-precision promise or change to runtime request overrides
|
||
accompanies this simplification.
|
||
|
||
### C. Ownership, Errors, And Diagnostic Evidence
|
||
|
||
**Catalog:** 2, 4, 7, 8, 9, 11, 26, 27. **Decisions:** 1 and 3.
|
||
|
||
**Current gap:** The documented artifact buffer lifetime permits races; nested
|
||
errors can obscure their owning operation; response read failures can be
|
||
misclassified; some pre-dispatch smoke failures produce a completed report;
|
||
and successful authentication responses are read without a byte bound.
|
||
|
||
**Target properties:**
|
||
|
||
- Artifact GoDoc, adapters, and tests align with successful `Read` transferring
|
||
the artifact and mutable body storage to PromptKit. Readers stop mutating
|
||
that storage, including retained aliases. Keep the interface signature and
|
||
existing defensive copy initially. Readers may reuse private buffers when
|
||
they return an independent copy; no release protocol is introduced.
|
||
- Preserve the owning public error category independently of nested causes,
|
||
retaining appropriate `errors.Is`/`errors.As` access. Do not require category
|
||
exclusivity: retained causes can make more than one category match. Preserve
|
||
meaningful direct-not-found versus load-failure distinctions. Use a small
|
||
internal operation error carrying category/cause with `Unwrap`/`Is` behavior.
|
||
Apply it at owning boundaries and select its category in facade translation,
|
||
including before nested capacity/provider/exhaustion projections can obscure
|
||
it. Preserve those specialized public projections and matchable child causes.
|
||
Reordering a whole-chain sentinel switch cannot express operation ownership.
|
||
- Distinguish successful-response body I/O failures from malformed JSON at both
|
||
initial decoding and end verification. Preserve the redacted transport
|
||
boundary, cancellation precedence, and genuine malformed, truncated, or
|
||
oversized-content behavior without adding retries. Record actual non-EOF
|
||
reader errors beneath the existing limited reader. On decode/end-check
|
||
failure, prefer observed context errors, retain size-overflow classification,
|
||
and otherwise propagate recorded I/O failures through `requestFailedError`.
|
||
Distinguish reader-returned errors from parser-generated syntax/type/truncation
|
||
errors, rather than treating every `io.ErrUnexpectedEOF` alike.
|
||
- Return local inference-diagnostic construction failures as `ErrInvalidRequest`
|
||
using explicit preparation-stage information. Preserve completed diagnostics
|
||
for missing credentials, capacity outcomes, and unsupported/inapplicable
|
||
operations; absence of dispatch alone does not imply an invalid request.
|
||
Add internal `GenerationEvidence.PreparationFailed bool`, set only at the
|
||
built-in `PrepareGeneration` failure return. Diagnostic orchestration checks
|
||
it after caller cancellation. Injected-client errors retain their established
|
||
semantics; no new public stage field is needed.
|
||
- Define the authentication response allowance as a named internal constant of
|
||
1 MiB (1,048,576 bytes). Enforce against actual reads regardless of declared
|
||
length, detect overflow, and close without unbounded draining. Report an
|
||
explicit limit diagnostic with credential acceptance unknown and retain
|
||
available HTTP status/timing. Add matching public/domain reason constants
|
||
`SmokeTestReasonAuthenticationResponseLimitExceeded` with value
|
||
`authentication_response_limit_exceeded`. For HTTP 200 recognition overflow,
|
||
use existing `ExecutionFailureOther`, nil `CredentialsAccepted`, and a
|
||
completed report with nil Go error unless caller cancellation applies.
|
||
No new report field, failure enum, or public budget knob is needed.
|
||
Read at most allowance plus one byte using `io.LimitReader`/`io.ReadAll`;
|
||
check overflow before JSON recognition. Preserve 401 rejection without body
|
||
consumption and other non-200 behavior independently of unused bodies.
|
||
- OpenRouter-specific authentication request/response interpretation belongs in
|
||
the outbound adapter. Leave selection, applicability, completion, and neutral
|
||
reporting in the use case. Preserve anonymous probing and credential/body
|
||
privacy. Narrow smoke copying to actual consumers, removing unused request
|
||
cloning and replacing full-report cloning used only to detach a target.
|
||
Use an `internal/llm` authentication operation returning HTTP evidence,
|
||
optional acceptance, overflow/unrecognized indicators, and operational error;
|
||
the use case maps those facts to report outcomes. Move default endpoint
|
||
derivation to an llm helper used by selection, retaining explicit overrides.
|
||
Consume a narrow authentication capability alongside reachability HTTP,
|
||
without reverse dependencies or a plugin registry. Directly detach the
|
||
target/provider map; retain facade and active-backend copying.
|
||
- Exercise `RunRequest`'s privacy formatting and JSON omission boundary with
|
||
synthetic markers in omitted variables, bodies, and execution settings, not
|
||
IDs deliberately present in summaries. Cover `%v`, `%+v`, `%#v`, direct
|
||
String/GoString, value/pointer forms, and direct-key JSON omission.
|
||
|
||
**Completion evidence:** Focused concurrent artifact tests follow the selected
|
||
ownership contract; they do not promise safety for readers that violate it.
|
||
Offline regressions cover composed public causes, both response read-failure
|
||
locations, construction failures with no dispatch, and authentication boundary
|
||
and overflow cases including exact limit/limit-plus-one, missing/inaccurate
|
||
lengths, closure without draining, 401 without consumption, context interruption,
|
||
and redaction. Update reason GoDoc and public conversion with the bound. Assert
|
||
diagnostic facts and privacy rather than complete incidental serialized shapes. Preserve active
|
||
backend copying, facade projection, and provider-map error behavior.
|
||
|
||
The public-boundary outcome matrix for planning is:
|
||
|
||
| Case | Owning outcome | Evidence preserved |
|
||
| --- | --- | --- |
|
||
| Client returns a child missing-prompt error | `ErrLLMGenerate` | Matchable child cause/category. |
|
||
| Reader returns that child error | `ErrArtifactLoad` | Matchable child cause/category. |
|
||
| Direct missing prompt/profile | Existing not-found error | No generic load-error substitution. |
|
||
| 2xx body I/O error at either decode stage | Generation/transport failure | Original read cause with redacted formatting. |
|
||
| Parser-invalid/truncated JSON, extra value, inference size overflow | Existing malformed-response behavior | No additional retry or parser-detail disclosure. |
|
||
| Built-in diagnostic preparation failure | `ErrInvalidRequest`, not a completed report | Zero dispatch and preparation cause. |
|
||
| Missing required credential or capacity outcome | Existing completed diagnostic | No inferred construction failure. |
|
||
| Authentication recognition overflow | Explicit limit reason; acceptance unknown | HTTP/timing facts, not secret-bearing response data. |
|
||
| Caller cancellation | Existing cancellation precedence | No publication of success over cancellation. |
|
||
|
||
### D. Resource Use And Inspection Efficiency
|
||
|
||
**Catalog:** 10, 16, 17, 18, 19, 20.
|
||
|
||
**Current gap:** The audited paths repeatedly copy requirement evidence, retain
|
||
unneeded backing storage or engine ownership, serialize unbounded diagnostic
|
||
text before truncation, reclone overlay descendants, and scan complete catalogs
|
||
for namespace-filtered listings.
|
||
|
||
**Selected remedies and completion evidence:**
|
||
|
||
- **Requirement evidence (10):** Intermediate missing-name sets are owned by
|
||
their evaluation results. Merge the smaller set into the larger, transfer nil
|
||
sets directly, and combine flags as now. Operands subsequently read values and
|
||
known flags, not independent evidence snapshots. This avoids repeated growing
|
||
copies without a global accumulator that could mix branch-local evidence.
|
||
Preserve union/flag outcomes and conservative inspection coverage with
|
||
representative left-heavy cases and independent renderer comparisons.
|
||
- **Prepared handles (16):** Replace `owner *Runner` with a pointer to an immutable,
|
||
non-zero-sized private identity object allocated once per runner constructor.
|
||
Keep the token after claim/discard so foreign-owner checks retain their order
|
||
without retaining the runner graph or adding mutable owner-field races.
|
||
Retain mutex-protected state transitions/payload clearing and detached details.
|
||
Cover foreign owners, namespace views, concurrent claim/discard, one-shot use,
|
||
and details after use/discard. A GC-timing assertion is unnecessary.
|
||
- **Repair feedback (17):** Attempt bounded construction of the complete-list
|
||
result; if it cannot fit, construct the omission form with existing order and
|
||
reservation rules. Normalize only potentially useful prefixes, preserving
|
||
`strings.ToValidUTF8` behavior for invalid-byte runs and rune-safe truncation.
|
||
Use standard `json.Marshal` on bounded normalized strings and existing prefix
|
||
selection rather than a custom escaper. A limited writer around `json.Encoder`
|
||
does not suffice if encoding first allocates the whole original string.
|
||
Preserve the 64 KiB output rule, omission marker, valid JSON, HTML/control
|
||
escaping, Unicode, complete stored diagnostics, and previous candidate.
|
||
Cover fitting/exact-boundary/oversized/many-entry/escaped/invalid inputs.
|
||
- **Session IDs (18):** Apply `strings.Clone` to the validated nonempty normalized
|
||
value before returning from `NormalizeSessionID`. Preserve trimming, Unicode,
|
||
accepted content, and wire/hash values without adding a raw-input limit.
|
||
- **Provider overlays (19):** Clone the lower map once, then recurse into owned
|
||
descendants for overlapping built-in maps and copy only upper replacements.
|
||
Preserve null/array replacement, accepted types, error boundaries, and input
|
||
isolation. This directly removes the repeated descendant cloning measured in
|
||
the assessment without an index or framework.
|
||
- **Namespace listings (20):** Resolve scope in the engine, then pass private
|
||
`configuration.IdentityFilter{All bool, Namespace string}` to captured
|
||
inventories. All copies the sorted slice once; named selection binary-searches
|
||
the exact `namespace + "/"` prefix range and copies only matches. Unqualified
|
||
selection scans immutable state directly, without first copying everything.
|
||
Update the private inventory interface/fakes for backend/profile IDs and
|
||
prompt identities. Preserve final context checks, ordering, exact prefix
|
||
matching, absent results, view/default/All scope, and mutation isolation.
|
||
|
||
Existing sorted inventories supply named ranges without persistent buckets or
|
||
extra construction traversal. Buckets could reduce repeated lookup work but add
|
||
retained state even for consumers that never list resources. Unqualified entries
|
||
need not form a contiguous range; their selection remains a scan. No new public
|
||
signature, cache, pagination, or complexity guarantee accompanies this remedy.
|
||
|
||
The [baseline measurements](#planning-evidence) support implementing 17, 19, and
|
||
20 rather than deferring them. Repeat comparable cases after implementation;
|
||
expected savings and algorithmic estimates are not measured replacement results.
|
||
No exact peak-heap promise, benchmark threshold, or recurring release gate is
|
||
introduced. If new evidence changes the tradeoff, record it and the proposed
|
||
disposition rather than silently dropping selected work. The companion plan
|
||
coordinates the related traversal and trusted-copying changes in E.
|
||
|
||
### E. Internal Coordination And Test Assembly
|
||
|
||
**Catalog:** 22, 23, 24, 25, 34. **Decision:** 6.
|
||
|
||
**Current gap:** Large functions combine distinct responsibilities, JSON copying
|
||
is duplicated across boundaries with different policies, and a test-only legacy
|
||
capacity wrapper remains a production constructor.
|
||
|
||
**Work:**
|
||
|
||
- Extract focused configuration validation/materialization helpers with visible
|
||
inputs and ownership. Preserve final-handoff validation, presence/null rules,
|
||
origins, and meaningful error precedence without pinning incidental ordering
|
||
of independent failures. Keep `validateEffective`'s resource/index traversal
|
||
visible and extract backend/profile/prompt validators with record/path/index
|
||
inputs. Keep typed presence interpretation together in profile construction.
|
||
Split prompt capture into input/metadata projection, message content/template
|
||
capture, and output-contract conversion; retain lazy origins, source-relative
|
||
reads, and one final inspection/hash assembly.
|
||
- Separate requirement recognition/evaluation from reachability and evidence
|
||
bookkeeping, sharing `if`/`with` orchestration where appropriate. Preserve
|
||
scope differences, inactive computed-name behavior, and independent comparison
|
||
with the actual renderer. Share the existing `parse.BranchNode` protocol,
|
||
handling With's changed body dot and unchanged else dot explicitly. Preserve
|
||
known/skipped/unknown classification and limitation bookkeeping; no generic
|
||
visitor or additional template syntax is needed.
|
||
- Consolidate structural copying only where policies genuinely agree. Keep
|
||
facade projection and runtime validation distinct; preserve existing types,
|
||
nils, supported numbers, schema keys, and ownership. Infallible projection
|
||
must not acquire new errors, panics, or silent fallbacks. Retain separate walkers
|
||
if sharing would require a configurable reflection framework. B's removed
|
||
source compatibility adapter must not migrate into this shared machinery.
|
||
Put infallible cloning in `internal/jsonvalue.CloneTrusted(any) any` and route
|
||
facade nested values through it. Runtime `Copy`/`CopyMap` validate, normalize
|
||
indirection/null, and enforce limits/key policy; trusted cloning preserves
|
||
accepted values and existing fast-path/reflection type/nil behavior without
|
||
interpreting numbers. Do not substitute fallible `Copy` and hide errors.
|
||
This centralizes ownership, not necessarily the traversal algorithm: two small
|
||
walkers remain appropriate where their policies differ.
|
||
- Separate generation attempt results and accounting from dispatch/retry control
|
||
flow with small internal helpers. Preserve built-in/injected differences,
|
||
permit release before callbacks or backoff, frozen requests, existing retry
|
||
classification, private evidence, and known/unknown usage distinctions.
|
||
Replace execute's parallel returns with `attemptResult` containing
|
||
`Outcome llm.AttemptOutcome`, `Dispatched`, `CapacityWait`, `RequestDuration`,
|
||
and `StartedAt`. Share usage/evidence/event projections only where facts match;
|
||
built-in retry and injected one-shot control flow remain distinct. C's
|
||
preparation-failure marker is independent of attempt accounting.
|
||
- The initial/repair runner scheduling test uses the production generation
|
||
coordinator via `generation.NewInjected` with the same manager. The production
|
||
`internal/capacity/client.go` wrapper and its retention prose are retired.
|
||
Preserve useful FIFO, cancellation, independent-pool, and panic-release scenarios with their
|
||
owning components. Keep any useful one-shot scaffolding test-only; do not wrap
|
||
the coordinator in another permit-acquisition layer.
|
||
|
||
**Completion evidence:** Existing meaningful behavioral tests and focused cases
|
||
for changed seams continue to exercise real internal collaborators. Tests no
|
||
longer require the retired production wrapper. Review demonstrates simpler
|
||
ownership/control flow; lowering complexity metrics alone is not an acceptance
|
||
criterion. No capacity-manager redesign or public DTO unification is needed.
|
||
|
||
### F. Documentation, Representation, And Test Maintenance
|
||
|
||
**Catalog:** 12, 13, 14, 28, 29, 30, 31, 32, 33.
|
||
|
||
**Current gap:** Several comments contradict implemented behavior, expectations
|
||
are duplicated or over-specified, and internal fields/helpers have no consumer.
|
||
|
||
**Work:**
|
||
|
||
- Correct credential-inspection GoDoc and its example: explicit profile credential
|
||
environment selection can coexist with required credentials; only the
|
||
backend-derived default is suppressed in that case. Correct provider-map wire
|
||
documentation to distinguish configured recursive overlays from runtime
|
||
replacement. Remove stale trimmed-ID and prompt-version comments.
|
||
- Give complete bundled-catalog expectations one independently specified test
|
||
owner. Retain smaller tests for distinct loading, wire, and smoke behavior,
|
||
using controlled fixtures where bundled content is irrelevant. Keep the full
|
||
independent oracle in `internal/configuration/bundled_test.go` and replace its
|
||
formatted-map plugin comparisons with structural assertions. Public tests own
|
||
toggles/order/isolation/overrides; use a controlled model/plugin wire case and
|
||
a configured-default smoke case instead of repeating the complete matrix.
|
||
- Replace literal artifact-hash algorithm assertions with documented opaque
|
||
equality/ownership and byte/size behavior; keep benchmark setup outside timed
|
||
work. Use same-body inline/file equality for size-hint cases and prepare an
|
||
opaque expected hash outside benchmark timing. This does not require changing
|
||
the artifact hash algorithm.
|
||
- Remove unused backend presence records and `QueueCapacitySet`, preserving
|
||
effective queue defaults, explicit zero, provenance, and needed profile/input
|
||
presence tracking. Remove the unused backend field list/map, resolution capture,
|
||
and runtime queue-presence flag, not ConfigKit provenance.
|
||
- Remove duplicated captured prompt output-format and unused message-file fields.
|
||
Preserve actual `content_file` loading, origins, and override-default behavior.
|
||
Account for observable definition-hash changes without promising permanent
|
||
encoding or bit-identical hashes across this change. Remove the unreachable
|
||
fallback with the fields, preserving empty-format request defaulting and
|
||
inspection/preparation equality.
|
||
- Remove unused private reachability endpoint state and the unused schema-helper
|
||
result, preserving actual report evidence and public single-file behavior.
|
||
Remove orphan `filecatalog.RelativePath`, `resourceNamespaceFromID`, and the
|
||
unused runner-test `stringPtr` helper with their obsolete self-tests.
|
||
|
||
**Completion evidence:** Documentation agrees with the owning GoDoc/format/wire
|
||
contracts. Caller checks precede deletion; meaningful discovery, identity,
|
||
namespace, schema, privacy, and bundled behavior remain covered. Tests do not
|
||
merely reproduce the loader's output as their expected answer.
|
||
|
||
## Implementation Plan
|
||
|
||
The [companion plan](implementation.md) defines the ordered, prompt-sized
|
||
implementation units, dependencies, and validation for one end-to-end sprint.
|
||
Workstream letters group target outcomes, not delivery stages. Temporary
|
||
intermediate compilation gaps are acceptable; completion concerns the combined
|
||
end state. No significant product decisions remain open. Equivalent private
|
||
coding details remain flexible; new evidence requiring a scope or contract
|
||
change should be surfaced rather than silently changing the selected target.
|
||
|
||
## Compatibility And Documentation Delivery
|
||
|
||
The selected ownership, schema, and convenience-source changes are deliberate
|
||
pre-1.0 compatibility changes. Deliver them in a minor release under the existing
|
||
[release policy](../policy/release.md#versioning-and-compatibility), with migration
|
||
notes explaining independent artifact storage, self-contained schemas, and
|
||
ConfigKit-supported input types. Mention changed definition hashes so consumers
|
||
can account for comparisons or caches; do not establish a stable encoding.
|
||
Release selection and publication are not part of writing this roadmap.
|
||
|
||
Update exported GoDoc, [consumer guidance](../consumers/pkg-promptkit.md),
|
||
[formats](../formats.md), [wire integration](../integrations/openai-compatible-chat.md),
|
||
and relevant [internal documentation](../internal/overview.md) when their owning
|
||
behavior lands. Clarify ordinary development toolchain selection in
|
||
[development guidance](../development.md), linking to existing release policy.
|
||
Correct standalone documentation defects without waiting for unrelated code.
|
||
|
||
## Planning Evidence
|
||
|
||
These are historical assessment results from unchanged production code at the
|
||
baseline above, using Go 1.27.3 on linux/amd64, Intel i5-7260U. Assessment records
|
||
were completed in commit `3ef2a9e`. Dependency/source inspection supplied the
|
||
ownership, caller, and interface evidence; temporary offline probes supplied the
|
||
schema results and measurements below. No replacement implementation was built
|
||
or benchmarked. Aggregate allocation is not peak live heap, and noisy timings
|
||
are not throughput promises.
|
||
|
||
The temporary harness directory was `/tmp/promptkit-assessment-aqn3B8`. Its
|
||
continued existence is not a planning dependency: the reproduction recipes below
|
||
retain the workloads and invocation settings. Recreate targeted harnesses outside
|
||
the repository or carry equivalent cases into focused implementation benchmarks,
|
||
not a new permanent benchmark framework. Reuse unchanged evidence unless a
|
||
changed dependency, workload, or implementation makes a new measurement useful.
|
||
|
||
### Schema Dependency Probe
|
||
|
||
With jsonschema/v6 6.0.2, decode each document with `json.Decoder.UseNumber`,
|
||
create a fresh compiler with `DefaultDraft(Draft2020)`, register the document at
|
||
`https://example.invalid/retrieval`, and install a loader that counts requests
|
||
and returns an external-resource error. Compile the retrieval URI; for the
|
||
unused-definition case, additionally compile `#/$defs/unused` with the same
|
||
compiler. The harness used `go run -mod=readonly` with
|
||
`GOTOOLCHAIN=local GOWORK=off GOPROXY=off`; no network resources were fetched.
|
||
|
||
| Document case | Root compilation | External loads |
|
||
| --- | --- | --- |
|
||
| `$defs.v` string schema referenced by `#/$defs/v` | Succeeds | 0 |
|
||
| `$anchor: v` in a definition, referenced by `#v` | Succeeds | 0 |
|
||
| Object definition with `next` referring recursively to that definition | Succeeds | 0 |
|
||
| Explicit absolute root ID and reference to its definition through that ID | Succeeds | 0 |
|
||
| Embedded definition with relative `$id: child`, referenced as `child` | Succeeds | 0 |
|
||
| Root `$ref: child.json` with no embedded resource | Fails | 1 |
|
||
| Root string schema with unused `$defs.unused: {$ref: child.json}` | Succeeds; explicitly compiling the definition fails | 0, then 1 |
|
||
| Example instance object containing literal `$ref: child.json` | Succeeds | 0 |
|
||
| Standard 2020-12 `$schema` declaration | Succeeds | 0 |
|
||
|
||
Explicit-ID cases also worked when retrieval URI and document ID differed. A
|
||
review rerun reproduced these results. This supports B's rejecting loader plus
|
||
schema-location enumeration, not a lexical URI ban. Dynamic references, escaped
|
||
pointers, embedded dialect handling, and full location-table coverage remain
|
||
implementation regression cases, not claims established by this small probe.
|
||
|
||
### Overlay Baseline
|
||
|
||
Construct maps outside timing. Start with
|
||
`map[string]any{"leaf": 1, "array": []any{1, 2, "x"}}`; wrap it `depth` times in a map with
|
||
`child` pointing to the previous level and `width` scalar keys named `key0`,
|
||
`key1`, etc., each with its index value. Build lower and upper independently.
|
||
For the disjoint case, wrap upper once more under `new`; otherwise use matching
|
||
keys. Call unchanged `overlayProviderMap`, retaining its result and checking
|
||
errors. These are chains with scalar siblings, not exponentially branching trees.
|
||
|
||
Run a temporary test overlay in `internal/configuration`, substituting its
|
||
absolute JSON mapping path:
|
||
|
||
```sh
|
||
go test -mod=readonly -overlay=/absolute/path/overlay.json ./internal/configuration \
|
||
-run '^$' -bench '^BenchmarkAssessmentOverlay$' \
|
||
-cpu=1 -benchmem -benchtime=50x -count=3
|
||
```
|
||
|
||
The overlay maps an absolute package-local test filename to an external harness
|
||
file declaring that package; it does not edit repository source. The same
|
||
`GOTOOLCHAIN=local GOWORK=off GOPROXY=off` settings apply to all measurements.
|
||
|
||
| 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 |
|
||
|
||
Source inspection shows lower descendants cloned again at each recursive
|
||
overlap. The selected owned-overlay algorithm removes that repeated work;
|
||
post-change measurements must establish actual savings.
|
||
|
||
### Repair Feedback Baseline
|
||
|
||
Call unchanged `formatRepairDiagnostics` with one-element string slices:
|
||
64 ASCII `x` bytes, 1 MiB of `x`, and 1 MiB of repeated `<&>` plus NUL (four bytes
|
||
per repetition). Build inputs outside timing, retain the output in a sink, and
|
||
record input/output lengths after stopping timing. Use the overlay method above
|
||
in `internal/usecase`, benchmark `BenchmarkAssessmentFeedback`, with
|
||
`-cpu=1 -benchmem -benchtime=20x -count=3`.
|
||
|
||
| Input | Observed time per operation | Allocated bytes per operation | Output bytes |
|
||
| --- | --- | --- | --- |
|
||
| 64 ASCII bytes | 780–1,198 ns | 224 | 68 |
|
||
| 1 MiB ASCII | 5.83–7.99 ms | about 6.81 MB | 65,536 |
|
||
| 1 MiB escaped content | 27.7–39.1 ms | about 44.12 MB | 65,531 |
|
||
|
||
Full-string normalization and repeated full marshaling account for the observed
|
||
allocation scale. D's bounded-prefix design must preserve the existing fitting
|
||
result and omission semantics; these timings do not measure the new design.
|
||
|
||
### Listing Baseline
|
||
|
||
For N = 32, 1,024, and 4,096, create sorted IDs `namespace/p%06d` outside timing.
|
||
Sparse mode assigns one ID to `target`; dense assigns half. Other IDs use `other`.
|
||
All mode uses the sparse inventory with the All filter; absent mode selects
|
||
`missing`. The baseline list callback returns `append([]string(nil), ids...)`,
|
||
matching configuration's full-inventory copy. Invoke unchanged
|
||
`filterIdentityList` with background context and empty view namespace. Use an
|
||
`internal/engine` overlay, `BenchmarkAssessmentListing`, and
|
||
`-cpu=1 -benchmem -benchtime=100x -count=3`.
|
||
|
||
| Total IDs | Mode order | ns/op ranges in mode order | 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 |
|
||
|
||
This measures filtering/copying, not full construction or public projection.
|
||
For post-change comparisons, exercise D's selection-before-copy path rather than
|
||
retaining an artificial full-copy callback. Repeated requests repeat the
|
||
baseline full-inventory allocation. Binary search over existing named ranges
|
||
has estimated comparison work proportional to log N plus output and introduces
|
||
no persistent index; no alternative layout was benchmarked.
|
||
|
||
### Requirement Evidence Work Count
|
||
|
||
For a left-heavy recognized condition with N distinct missing names, the current
|
||
merge copies growing sets: `2 + 3 + ... + N` insertions, excluding leaf creation.
|
||
That is 9 at N=4, 135 at N=16, and 8,255 at N=128; the ordinary two-name case
|
||
needs two. These are source-derived counts, not elapsed-time measurements. The
|
||
selected small-to-large owned merge addresses that specific repeated copying;
|
||
it does not establish a universal inspection complexity contract.
|
||
|
||
## Validation And Completion
|
||
|
||
Follow the [testing policy](../policy/testing.md) and existing
|
||
[maintainer validation workflow](../development.md#maintainer-validation).
|
||
Use focused behavioral regressions for demonstrated defects, race testing for
|
||
changed concurrent ownership/scheduling paths, and offline wire tests where
|
||
local success does not prove the emitted request is correct. Reuse successful
|
||
checks for unchanged content; release validation remains the existing workflow.
|
||
No live inference, provider credentials, generalized benchmark suite, or new
|
||
recurring release gate is introduced.
|
||
|
||
The workstream catalog lists partition all 34 entries. During implementation,
|
||
record for each workstream its completed units, evidence, and any explicitly
|
||
justified deferral. A measured deferral is not an implemented fix; optional
|
||
cleanup does not hold unrelated fixes hostage. Full implementation completion
|
||
requires the selected changes, their relevant validation, migration guidance,
|
||
and removal of obsolete documentation/tests. Any remaining work must be named
|
||
and retained in an appropriate roadmap rather than lost during consolidation.
|
||
|
||
When this roadmap's work is complete or otherwise explicitly disposed of, move
|
||
still-useful contract information to its canonical owners and retire temporary
|
||
planning documents under the [documentation lifecycle](../policy/documentation.md#document-lifecycle).
|
||
Keep audit evidence and decision provenance discoverable while they remain
|
||
useful; do not maintain competing current-behavior references.
|