base-layer/promptkit
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 4 Wiki Activity

Remove completed audit and implementation roadmaps

Browse Source
This commit is contained in:
Eric Rakestraw
2026-11-06 01:19:33 +00:00
parent c9663ec63c
commit 4faf43cc29
18 changed files with 3 additions and 5353 deletions
Download Patch File Download Diff File Expand all files Collapse all files

564
docs/roadmap/remediation.md
View File

@@ -1,564 +0,0 @@
# Implementation Review Findings
## Responsibility Simplification Review
Reviewed all 17 stages of the current [implementation plan](implementation.md)
against [responsibility-simplification.md](responsibility-simplification.md)
and its accepted audit decisions. The implementation spans `448e64d` through
`5ee602cc4b5a3afe4e7d361eb77181ef0940c7cd`, after planning commit `de46935`.
The targeted architecture is implemented, and the ownership regression below
is now addressed. Its evidence and review table describe the pre-fix baseline;
the completion marker records the correction. No additional unintended
restriction or public guarantee was identified. Removing copier-derived schema
limits remains the selected acceptance change, while runtime provider-value
support remains in place. The earlier
33-stage review and its five completed findings are retained separately below.
### R1. Isolate Custom JSON Encoders From Retained Metadata [P2]
**Status:** Complete. **Stages:** 2, 13–15; particularly the shared prepared snapshot
and removal of private smoke-target detachment.
Completion: built-in generation preparation uses trusted detachment before
payload construction, so whole-payload encoding and per-value encoding-failure
diagnostics cannot mutate retained provider values through their receivers.
The single prepared snapshot and named/typed custom encoding remain intact.
Public regressions failed before the fix and now protect prepared details,
result settings, smoke settings, caller input, custom wire output, and failure
classification without dispatch. Go 1.27.3 `go test -race ./...` and
`go vet ./...` passed, including the existing retry, repair, and concurrent
prepared-lifecycle coverage. Internal model-client documentation was updated.
**Locations:** [provider payload construction](../../internal/llm/openai_compatible_client.go),
`openAIChatRequestPayload`, lines 468–475, and `PrepareGeneration`, lines 133–139;
[prepared metadata sharing](../../internal/usecase/prepared_execution.go),
`PrepareExecution`, lines 52–57;
[smoke report target](../../internal/usecase/smoke_tester.go), `smokeReportTarget`.
Supported named map/slice values can implement `json.Marshaler`. The acceptance
copier preserves compatible concrete types, including their methods. The built-in
payload builder places provider values into the wire map by reference, and
`json.Marshal` can invoke those methods on retained values. For example:
```go
type mutatingMap map[string]string
func (m mutatingMap) MarshalJSON() ([]byte, error) {
m["value"] = "changed by encoder"
return json.Marshal(map[string]string(m))
}
```
This is an accepted finite, acyclic string-keyed map. Its custom encoding method
mutates only its receiver, not external state. No concurrent caller mutation or
invalid injected collaborator is involved.
**Verified reproduction:** An offline public-API probe constructs an ordinary
engine with the built-in LLM client and an in-memory HTTP transport returning a
valid response. It supplies `ExtraParams["options"] = mutatingMap{"value":
"original"}`. Preparation succeeds. It compares details before/after
`RunPrepared`, then separately inspects a text-inference smoke report using the
same kind of runtime override. Both operations return nil Go errors:
| Observation | Current implementation | With pre-sprint detachment restored via Go overlay |
| --- | --- | --- |
| Prepared details before execution | `original` | `original` |
| Prepared details after execution | `changed by encoder` | `original` |
| Smoke report's effective provider setting | `changed by encoder` | `original` |
| Caller-owned input map | `original` | `original` |
The comparison overlays only `internal/usecase/prepared_execution.go` and
`internal/usecase/smoke_tester.go` from `de46935` onto the current code. It
isolates the lost metadata ownership separation; it is not a test of the entire
old revision. The probe and overlays live outside the repository.
**Cause and impact:** Removing the second prepared snapshot is correct only if
all consumers treat the shared snapshot as immutable. Injected clients receive
independent public requests, but built-in encoding is another callback boundary:
custom encoding code can mutate its input. The new sharing exposes that mutation
through retained details and diagnostic settings. It also creates an unsynchronized
mutation path alongside otherwise supported concurrent details reads. The
existing mutating-client regression exercises an injected client, so it does not
cover this built-in path. Encoder failure diagnostics are relevant too:
`PrepareGeneration` may separately marshal original extra-parameter values when
whole-payload encoding fails.
**Required correction:** Keep the single immutable metadata snapshot, but detach
mutable accepted provider values at the built-in encoding boundary before any
custom encoder runs. Ensure both payload encoding and per-value encoding-failure
diagnostics operate on detached values. Reuse trusted copying where appropriate;
do not reinstate the duplicate details snapshot or reintroduce input validation
merely to copy accepted data.
Retain current named/typed input and custom wire-encoding behavior, native retry
payload freezing, error classification, and the outward public ownership copies.
Rejecting these types, stripping their methods through JSON-native normalization,
or requiring nonmutating custom encoders would narrow the selected runtime
contract instead of repairing ownership.
**Regression coverage:** Exercise a supported named provider map with a
receiver-mutating `MarshalJSON` through the built-in offline transport. Check
unchanged prepared details and smoke effective settings, independent caller
input, and the expected custom wire representation. Include the encoding-failure
path so its per-value diagnostic cannot mutate retained state. Retain existing
injected initial/repair mutation and concurrent prepared lifecycle protection.
### Stage-By-Stage Review
This is the pre-remediation review record; R1's completion above addresses its
identified gap without changing the other selected boundaries.
| Stage | Current evidence and result |
| --- | --- |
| 1 | Constructor routing defaults and their only error path are removed. Resolved endpoint/model checks and HTTP-client timeout ownership remain. No additional finding. |
| 2 | One root payload builder preserves field omission/presence, reserved extensions, session/native schema handling, and encoded-body freezing. Its borrowed provider values participate in R1. |
| 3 | Private repositories explicitly require captured definitions, metadata, requirements, and profile identity; capability/hash fallbacks are removed. No additional finding. |
| 4 | Repository fixtures implement captured operations; metadata-only and selection/error coverage remain. No additional finding. |
| 5 | Rendering uses captured templates only, with per-call clones/functions and retained session/input/message behavior. No additional finding. |
| 6 | Renderer fixtures/benchmarks supply captured parses; syntax, isolation, cancellation, artifact-text, and requirements comparisons remain covered. No additional finding. |
| 7 | Smoke syntax and schema checks use separate prepared plans; malformed syntax leaves schema matching unevaluated, including scalar/nonmatching controls. No additional finding. |
| 8 | Direct validator interface/methods and on-demand callbacks are deleted; shared instance validation and eager source-specific preparation remain. No additional finding. |
| 9 | Tests use prepared plans and retain source containment, portability, references, precision, freezing, and synchronous cancellation coverage. No additional finding. |
| 10 | Trusted schema detachment removes provider-copy budgets. Deep schema isolation and a real schema with over 100,000 annotation values are covered. No additional finding. |
| 11 | Registry/profile lookup uses trusted detachment after final acceptance, while configuration validation/overlay and runtime acceptance checks remain. No additional finding. |
| 12 | Private target resolution transfers owned maps, preserving empty-map inheritance and nil projections. Outward/injected projections still detach values. No additional finding. |
| 13 | One redacted metadata snapshot is shared, and cleanup no longer writes it; owner/claim/discard state and outward copying remain. Built-in custom encoding can still mutate the snapshot: R1. |
| 14 | Smoke projection is infallible and outward values are detached; credentials/endpoints remain sanitized. Built-in encoding can mutate effective settings before final report projection: R1. |
| 15 | Added tests cover injected initial/repair mutation, returned details/results, named/numeric runtime values, and concurrent lifecycle reads. The built-in custom-encoder path is missing: R1. |
| 16 | Canonical format, integration, model-client, runner, source, and component guidance reflects the selected changes without new resource/provider promises. The immutability description is not fully realized until R1 is fixed. |
| 17 | Fresh full race tests and vet pass, but those checks do not cover R1. Documentation/history links and delivered scope were reviewed; completion still requires the ownership correction. |
### Review Validation And Limits
At the reviewed pre-fix baseline, using Go 1.27.3 with `GOTOOLCHAIN=local`,
`GOWORK=off`, and `GOPROXY=off`:
- `go test -race ./...` passed, including the maintained offline examples.
- `go vet ./...` passed.
- The public ownership probe reproduced both metadata mutations, with nil Go
errors and unchanged caller values; the two-file historical overlay prevented
both mutations.
The review changed no production code or repository tests. Its source analysis
and targeted probe established R1 despite the passing existing suite; the
completion marker above records the subsequent fix and validation. The review
does not claim universal callback safety, provider compatibility, or defect-free code.
## Earlier Audit Remediation Review: Scope And Status
Reviewed all 33 stages of the [implementation plan](implementation.md) against
the [selected roadmap](audit-remediation.md), its accepted catalog decisions,
and the implementation through `6673284312de23bde86c20eb8de4f3865b157471`.
The implementation changes begin after planning commit `6a1a32a`.
An integration-focused follow-up reviewed the same implementation at `356563a`,
the documentation-only commit recording the original review. It adds finding 5;
the earlier findings were still open at that audit baseline and are not duplicated.
The broad architecture and selected simplifications are in place. All five
findings are now addressed, with completion markers below: three correctness/
completeness issues, one error-projection efficiency issue, and one documentation
status gap. Finding descriptions and audit evidence retain the historical
pre-fix behavior; the completion markers describe the delivered corrections.
The accepted artifact ownership, supported convenience-source types, and
self-contained schema changes are intentional compatibility changes. No
inadvertently strengthened contract was identified beyond those deliberately
selected changes. Historical measurements are observations, not performance
contracts.
## Earlier Audit Findings
### 1. Preserve Caller Cancellation When Authentication Also Overflows [P2]
**Status:** Complete. **Stages:** 7, 21–22.
Completion: interrupted body reads preserve caller cancellation before overflow
classification. Public below-limit and overflow-boundary regressions retain HTTP
facts, unknown acceptance, incomplete cancellation reports, and body closure.
**Locations:** [authentication adapter](../../internal/llm/openrouter_authentication.go),
`ProbeOpenRouterKey`, lines 61–67;
[outcome mapping](../../internal/usecase/smoke_authentication.go),
`authenticationProbe.run`;
[completion](../../internal/usecase/smoke_tester.go), `finishHTTP`.
The adapter checks the number of returned bytes before the read error. A legal
`io.Reader` can return the allowance-plus-one byte and `context.Canceled` in the
same read. The adapter sets `Overflow` and returns nil, discarding cancellation.
The use case then selects `ExecutionFailureOther`; `finishHTTP` does not apply
caller cancellation because it only does so for context-classified failures.
**Verified reproduction:** An offline public `SmokeTest` uses an HTTP 200 body
that cancels the caller and returns `context.Canceled` with its final bytes.
With 1,024 bytes it returns the cancellation and an incomplete report. With
1,048,577 bytes it instead returns:
- nil Go error despite `ctx.Err() == context.Canceled`;
- `Completed == true`;
- failure `other` and reason `authentication_response_limit_exceeded`.
The body is closed and the read allowance is honored in both cases; those parts
of the remedy work. The completed overflow result violates Stage 7's explicit
exception for caller cancellation.
**Required correction:** Preserve context interruption across the adapter and
outcome mapping when overflow and interruption coincide. Retain independently
observed HTTP facts and unknown credential acceptance. Ordinary overflow must
still be a completed diagnostic with nil Go error. Do not change the established
treatment of a known, concluded authentication result merely because cancellation
arrives later.
**Regression:** Exercise bytes-plus-cancellation at the overflow boundary through
the public diagnostic path, alongside ordinary overflow and interrupted
below-limit controls. Keep caller deadline handling consistent with the same
existing cancellation policy.
### 2. Check Schema Self-Containment Independently Of Its Source Filename [P2]
**Status:** Complete. **Stages:** 11–13.
Completion: shared compilation checks both the source base and an unrelated
submission base through the same compiler rules, including unused declarations.
Host and filesystem regressions preserve fragment, explicit-ID, and embedded-ID
references while rejecting unsent source-name dependencies.
**Locations:** [schema compilation](../../internal/validate/standard_validator.go),
`compileSchemaDocument`, lines 426–445, and its host/`fs.FS` callers;
[public wire regression coverage](../../schema_file_contract_test.go).
The compiler registers the document at its physical file or source-relative
retrieval URI. Its denying loader rejects unknown resources, but references
back to that registered retrieval URI do not invoke the loader. Consequently,
successful local compilation does not establish that the outbound root is
self-contained without PromptKit's source context.
**Verified reproduction:** Load a file named `schema.json` containing:
```json
{"$defs":{"value":{"type":"integer"}},"$ref":"schema.json#/$defs/value"}
```
Public `PrepareExecution` succeeds. An offline HTTP client captures the actual
native structured-output payload, returns `3`, and `RunPrepared` passes local
validation. However, compiling the captured schema at a neutral submission URI,
`https://provider.invalid/submitted-schema`, with external loading disabled
attempts to load `https://provider.invalid/schema.json` and fails. The source
retrieval URI was never sent to the provider. Changing the reference to
`#/$defs/value` succeeds both locally and at the neutral URI.
This is a packaging gap in the selected self-contained-schema remedy, not a
claim about any particular provider's schema support or enforcement.
**Required correction:** Establish self-containment without relying on the
source retrieval URI being available upstream. Detect references that depend
on that unsent filename/path and provide an actionable error. Preserve valid
fragment references, explicit/embedded IDs, anchors, recursion, dialect handling,
and checks of otherwise unused schema declarations. Keep source location for
root loading and diagnostics; do not add external loading, a lexical ban on all
non-fragment references, a bundler, or a provider-acceptance guarantee.
**Regression:** Cover host-file and `fs.FS` source-name dependencies, retain
explicit-ID and fragment controls, and evaluate the actual outbound schema
without the original source base. Preparation must fail before dispatch when
the selected self-contained contract is not satisfied.
### 3. Avoid Duplicating An Already-Retained Error Cause [P3]
**Status:** Complete. **Stages:** 3–4.
Completion: ordinary operation projection retains its mapped cause once, without
joining the original subtree a second time. Multi-engine regression coverage
checks exact cause/owner matching and one occurrence of an independent leaf;
specialized projection and redaction tests remain intact.
**Location:** [public error projection](../../errors.go), `mapPublicError`,
particularly lines 26–38.
The ordinary operation branch joins the owner, the mapped cause, and the
original cause. For ordinary errors, the mapped cause already retains the
original through wrapping or is that same error. This introduces two paths to
the same cause. When an injected client returns an error from another PromptKit
engine, each additional enclosing engine repeats the previous cause tree in
both branches. Formatting duplicates diagnostics; unsuccessful error-tree
searches can also revisit the repeated branches.
**Verified reproduction:** Begin with a public missing-prompt error, then run a
valid parent whose injected client returns that error; repeat with each parent's
result. Owner and exact-cause matching remain correct, but formatted byte counts
at depths zero through eight are:
```text
158, 361, 767, 1579, 3203, 6451, 12947, 25939, 51923
```
The input is a linear chain of consumer operations; the projection creates
exponential repeated text/traversal. Even one ordinary error repeats its cause
unnecessarily. This is introduced by the new join, not required to preserve the
owning category.
**Algorithmic analysis:** Let `d` be the number of enclosing operations, with a
fixed-size leaf diagnostic and fixed-size per-operation context. The ordinary
mapped cause already wraps the previous error, yet the join also stores that
previous error as another child. Its formatted length therefore follows
`L(d) = 2*L(d-1) + O(1)`, giving `Theta(2^d)` output bytes. The cause graph needs
only `O(d)` distinct error objects, but standard `errors.Is`/`errors.As` traverse
both paths without remembering previously visited objects. An unsuccessful
search consequently follows `W(d) = 2*W(d-1) + O(1)` as well. The mapper itself
performs such unsuccessful category/type searches, so this is not limited to
callers that log the final diagnostic.
Retaining one already-mapped cause path removes the avoidable branching:
formatted length becomes `L(d) = L(d-1) + O(1)`, and an individual formatting or
unsuccessful search over that corrected ordinary chain can be linear in `d`.
This is a structural alternative, not a request for error-graph memoization,
new nesting limits, or exact timing/allocation guarantees. It does not assert
that a sequence of `d` separate public operations, each processing its own
growing error, has linear total cost. Arbitrary consumer error implementations
also retain their own costs.
**Required correction:** Retain the owning category and each necessary cause
without adding an original cause that the projection already retains. Preserve
`errors.Is`/`errors.As`, specialized capacity/provider/retry projections, and
redacted transport formatting. Do not require category exclusivity, discard
consumer causes, or impose a new nesting limit.
**Regression:** Extend composed-error coverage across several enclosing
operations. Verify owner and exact-cause matching and that an independent leaf
diagnostic is not duplicated at every boundary. Protect this relationship rather
than exact complete error wording or a timing threshold.
### 4. Reconcile The Feature Roadmap's Implementation Status [P3]
**Status:** Complete. **Stage:** 33.
Completion: the feature roadmap now records the delivered sprint, links stage
evidence and this follow-up record, and distinguishes historical baseline
measurements from current status without prematurely closing finding 5.
**Location:** [feature roadmap](audit-remediation.md), Purpose And Status and
workstream completion reporting.
The feature roadmap still says neither document describes completed remediation
and all workstreams are pending. The implementation plan now records all 33
completed stages and post-change
measurements. Stage 33 specifically required actual completion/evidence to be
recorded in the roadmap. Its current status contradicts the delivered work and
makes it difficult to distinguish original findings from remaining review work.
**Required correction:** Record that the implementation landed, point to the
stage evidence and outstanding findings here, and distinguish historical
assessment-only statements from current overall status. Preserve the original
baseline measurements and decision provenance as historical evidence. Do not
claim all issues are resolved until the findings are addressed; normal roadmap
retirement can follow when appropriate.
### 5. Integrate Operation Ownership With Terminal Observation Classification [P2]
**Status:** Complete. **Related stages:** 3–4, 25–26.
Completion: a private public-boundary wrapper retains operation ownership and a
single mapped cause. Terminal observations classify that owner while retaining
context precedence and generation-specific transport/provider details. Observed
run, prepared-run, repair, and artifact regressions retain child cause identity.
**Locations:** [terminal classifier](../../observations.go), `observedFinish`,
lines 160–192; [public error projection](../../errors.go), `mapPublicError`;
[run boundaries](../../engine.go), `Run` and `RunPrepared`.
The new operation errors and public projection preserve the enclosing operation's
category while retaining child causes. The existing terminal observer classifier
still uses a whole-chain sentinel-priority switch. A retained child
`ErrPromptNotFound` wins over the enclosing generation or artifact failure.
The generation coordinator, which classifies the actual injected call, reports
`generation` instead. Thus the owning-operation remedy is incomplete at the
existing observation boundary even though the returned error now retains the
correct outer category.
**Verified reproduction:** Obtain a child engine's missing-prompt error, then
return it from a valid parent engine's injected client or artifact reader. With
an observer, the offline race-enabled probe produces:
| Parent path | Last attempt failure | Terminal failure | Parent error category retained |
| --- | --- | --- | --- |
| `Run`, initial generation | `generation` | `prompt_not_found` | `ErrLLMGenerate` |
| `RunPrepared`, initial generation | `generation` | `prompt_not_found` | `ErrLLMGenerate` |
| `Run`, repair generation | `generation` | `prompt_not_found` | `ErrLLMGenerate` |
| `Run`, artifact reading | No model attempt | `prompt_not_found` | `ErrArtifactLoad` |
The exact child error remains matchable in all four cases. The parent's prompt
exists; the terminal event nevertheless describes a missing parent prompt. This
can misdirect downstream monitoring and recovery. Existing composed-cause tests
check returned errors, while existing attempt/terminal consistency tests use
simple injected errors and miss this combination.
**Required correction:** Preserve and consume the owning-operation information
when classifying terminal events instead of selecting a nested operation by
global sentinel priority. Keep legitimate context/deadline precedence, built-in
provider/transport/malformed classifications, retry metadata, and public
`errors.Is`/`errors.As` access. This does not require a new exported API or
category exclusivity. Simply moving generation ahead of other sentinels would
create the same ambiguity for other owners.
**Regression:** Cover observed ordinary/prepared generation, repair failure,
and artifact failures carrying another engine's error. Preserve direct missing
prompt/profile classification and existing cancellation/provider cases. Do not
require universal attempt/terminal equality: later validation or cancellation
can legitimately change an operation's final outcome.
## Earlier Stage-By-Stage Review Record
Each stage was checked against its requested outcome, changed implementation,
relevant tests, and documentation. The entries below identify the review focus;
“no additional finding” is a review result, not a guarantee of defect-free code.
| Stage | Current evidence and result |
| --- | --- |
| 1 | Development guidance selects the latest available local toolchain independently of the module minimum; validation used Go 1.27.3 and `go.mod` remains at 1.27.0. No additional finding. |
| 2 | Both source helpers hand provider maps directly to ConfigKit; the converter is deleted. Source tests cover capture, unsupported values, cycles, ordering, and inheritance. No additional finding. |
| 3 | Internal operation errors carry explicit owner/cause with `Is`/`Unwrap`; lookup and execution boundaries use them. See finding 3 for facade composition. |
| 4 | Public nested-error and specialized-capacity tests retain owner/cause. Repeated cause retention remains inefficient: finding 3. |
| 5 | Response-reader errors are recorded below the limited reader and distinguished from parser/framing failures at both decode points; cause/redaction tests cover bytes with errors. No additional finding. |
| 6 | Built-in preparation has explicit typed error/evidence markers; inference diagnostics check cancellation first and report construction errors without dispatch. Injected clients retain their separate semantics. No additional finding. |
| 7 | Authentication reads at most allowance plus one, closes bodies, and reports explicit overflow. Simultaneous cancellation remains incorrect: finding 1. |
| 8 | Artifact GoDoc, defensive adapter copying, migration guidance, and concurrent tests follow the accepted storage transfer. No additional finding. |
| 9 | Prepared handles retain an immutable non-zero-sized identity token, not the runner; claim/discard locking remains. Normalized session IDs clone accepted storage. No additional finding. |
| 10 | Runner and engine declare `ValidationPreparer`; concrete validator constructors preserve direct validation and nil-plan behavior. No additional finding. |
| 11 | Host and `fs.FS` paths share compilation, retain root loading, and deny external loader requests. Source-base dependence remains: finding 2. |
| 12 | Declared schema traversal uses compiler-resolved dialects, escaped pointers, and schema-bearing positions, excluding annotations. It does not resolve finding 2. No separate traversal finding. |
| 13 | Public schema-file precedence/errors, freezing, and native submission are tested; the unused helper result is gone. Outbound relocation coverage misses finding 2. |
| 14 | Overlay clones the lower map once, merges owned descendants, and copies replacements. Isolation/null/array cases and comparable allocation results are recorded. No additional finding. |
| 15 | Trusted projection copying moved to `internal/jsonvalue`; fallible runtime input copying remains distinct. Type/nil/number/schema-key behavior is covered. No additional finding. |
| 16 | Owned evidence sets merge smaller into larger; callers consume values/flags rather than independent map snapshots. Work-count evidence and requirements tests support the change. No additional finding. |
| 17 | Shared branch orchestration preserves `with` body-dot versus else-dot behavior, inactive paths, and conservative inspection. Independent rendering comparisons remain. No additional finding. |
| 18 | Bounded normalization preserves invalid-byte-run replacement and rune boundaries; encoding delegates to standard JSON marshaling. No additional finding. |
| 19 | Complete/omission feedback paths use bounded prefixes and retain complete stored diagnostics/candidate behavior. Boundary/escaping tests and comparable allocation results are recorded. No additional finding. |
| 20 | Scope resolves before inventory selection; named ranges use binary search, All copies once, and unqualified selection scans without a full prior copy. Ordering, ownership, cancellation, and allocation evidence remain. No additional finding. |
| 21 | OpenRouter endpoint derivation and account-response interpretation live in the outbound adapter behind a narrow capability. Finding 1 remains in its read/error ordering. |
| 22 | Use cases retain selection/applicability/reporting; target-only detachment replaces unused broad cloners. Overflow/context outcome composition contributes to finding 1. |
| 23 | Extracted record validators retain final-handoff traversal, resource indexes, field paths, and existing checks. No raw-source validation phase was added. No additional finding. |
| 24 | Input metadata, message/template capture, output conversion, and effective targets are separated while preserving presence, source-relative reads, lazy origins, and final hash assembly. No additional finding. |
| 25 | Attempt results replace parallel returns without moving acquisition/release outside attempt scope. Built-in and injected dispatch remain distinct. No additional finding. |
| 26 | Shared accounting runs after permit release and preserves distinct HTTP/usage facts; observation and scheduling tests exercise the real coordinator. No additional finding. |
| 27 | Synthetic markers exercise value/pointer formatting and direct-key JSON omission; credential, identity, listing, and provider-map documentation is corrected. No additional finding. |
| 28 | One internal test owns the complete bundled asset oracle; smaller public option/wire/smoke tests retain distinct assembled behavior. No additional finding. |
| 29 | Artifact tests compare same-body inline/file opaque hashes and actual byte/size behavior; benchmark correctness setup remains outside timing. No additional finding. |
| 30 | Unconsumed backend presence/queue state, endpoint echo, helper result, and named orphan helpers are removed; effective policy/provenance and public features remain. No additional finding. |
| 31 | Runtime definitions use the output contract and captured message content without duplicate fields. Configuration `content_file`, override defaulting, hash relations, and migration guidance remain. No additional finding. |
| 32 | Runner scheduling uses `generation.NewInjected`; the production wrapper is deleted. Useful manager scenarios use test-only one-shot scaffolding. No additional finding. |
| 33 | Current GoDoc/consumer/format/integration/internal guidance largely matches the code; validation passes. Feature-roadmap completion reporting remains stale: finding 4. |
## Earlier Validation And Limits
At the reviewed commit, using Go 1.27.3 with `GOTOOLCHAIN=local`, `GOWORK=off`,
and `GOPROXY=off`:
- `go test -race -count=1 ./...` passed, including the maintained examples.
- `go vet ./...` passed.
- Tracked Go files produced no `gofmt -l` output.
- `git diff --check 6a1a32a..HEAD` passed.
- Additional temporary Go test overlays outside the repository reproduced
findings 1–3. The cancellation and relocated-wire-schema assertions fail as
described; the nested-error probe measured the duplicated text while retaining
correct category/cause matching.
The probes use controlled readers, HTTP transports, and the pinned schema
compiler. No credentials, provider calls, external schema retrieval, dependency
changes, or publication were needed. They establish local failure mechanisms,
not universal provider behavior. Optimization review checked source changes and
the recorded comparable measurements; it did not add benchmark thresholds or
new release gates.
## Earlier Integration Follow-Up Coverage
The follow-up focused on interactions between the changed components and the
existing consumer workflows, rather than repeating the stage-local review.
Production code is unchanged between `6673284` and the follow-up baseline
`356563a`.
| Integration seam | Review and validation result |
| --- | --- |
| Convenience sources, ConfigKit inheritance, and optimized provider overlays | An assembled probe captured backend/profile source maps before caller mutation, inherited a profile, and verified recursive backend-to-profile overlay plus detached inspection results. Passed. |
| Namespace-filtered inventories, inspection, and execution selection | The assembled probe listed a scoped prompt, inspected requirements, validated its request, and prepared it through the same view. An unscoped owning engine then ran the handle with its captured prompt/profile/backend identities. Passed. |
| Trusted copying, frozen validation, runtime overrides, and repair | The assembled probe combined direct session and reasoning/numeric overrides, appended messages, a schema with internal definitions, and bounded repair. It mutated prepared details, client-owned nested provider/schema values, and the source schema after preparation. Repair and frozen validation remained correct, and inspection/preparation hashes agreed. Passed. |
| Generation accounting, retries/repair, and capacity | Existing assembled tests for shared/independent pools, backoff permit release, logical admission, repair, frozen credentials, usage, and observer events passed under the race detector. No additional scheduling/accounting finding. |
| Prepared-owner token, namespace views, and observation isolation | Existing lifecycle, foreign-owner, concurrent claim/discard, delegated-operation, and observer-isolation tests passed under the race detector. No additional lifecycle finding. |
| Operation-owned errors and existing terminal observers | New offline observed-call probes fail in all four combinations documented in finding 5. Existing plain injected-error tests still pass. |
| Shared schema compiler and provider-native output | Source-to-native-payload review retains finding 2; the new assembled frozen-schema/repair probe passed for a genuinely self-contained fragment-reference schema. No duplicate finding. |
| HTTP read classification and diagnostic adapter/reporting | Adapter/coordinator/use-case review and the focused internal suites retain the existing read-failure classifications. The overflow/cancellation conflict remains finding 1. No duplicate finding. |
| Requirement-evidence ownership and renderer/preflight behavior | Evaluation/branch ownership was reviewed alongside actual rendering and the assembled requirements/preflight workflow. Existing prompt requirements and use-case suites passed; no additional recognition scope or execution-readiness guarantee was introduced. |
Validation used Go 1.27.3 with `GOTOOLCHAIN=local`, `GOWORK=off`, and
`GOPROXY=off`:
- The focused race-enabled public integration selection passed, covering
retry/repair/accounting, backend capacity, frozen prepared execution,
delegation, namespace views, schema-file wiring, programmatic sources, and
existing injected-error observation classifications.
- Race-enabled configuration, prompt, validation, generation, use-case, and
model-client suites passed.
- The additional composed-snapshot/repair probe passed under the race detector.
- The new operation-ownership observation probe failed as documented in
finding 5, independently of the already recorded findings.
Temporary probes were supplied using Go test overlays outside the repository;
no implementation or permanent test changes were made. All provider behavior
was controlled offline. The prior full-suite/vet evidence above remains
applicable to the unchanged implementation; this follow-up adds focused
integration evidence rather than a new release gate.
## Earlier Algorithmic Follow-Up Coverage
A further source-based review at `7203e5d` focused on algorithmic correctness,
efficiency, and scalability of the implemented stages. It confirms the
exponential behavior in finding 3 and adds the recurrence and refactoring
opportunity there, rather than creating a duplicate finding. No additional
distinct algorithmic or ordinary-correctness finding was identified in this
pass. All five findings were open at that audit baseline; their completion
markers above own the current remediation status.
Remediation completion validation: Go 1.27.3 with `GOTOOLCHAIN=local`,
`GOWORK=off`, and `GOPROXY=off` passed the complete
`go test -race -count=1 ./...` suite and `go vet ./...`. New regressions cover
authentication read interruption at the overflow boundary, host/FS schema
portability, relocated native schema submission, nested-cause duplication, and
operation-owned terminal observations. Focused checks passed before each code
unit was committed. Documentation links, formatting, and whitespace were checked.
The review used the plan and roadmap to exclude deliberate algorithm choices:
the small-to-large evidence merge, owned provider overlay, bounded feedback
encoding and prefix selection, namespace binary searches, declared-schema
location compilation, trusted-copy traversal policies, and selected ownership
copies were not reopened. In particular, the chosen unqualified-list scan and
schema-location strategy are not reported as new optimization requests.
For the remaining implementation glue:
- Source helpers map records once before ConfigKit's handoff, without the old
recursive compatibility expansion. ConfigKit's own processing is outside
this review's implementation scope.
- Extracted record validators use a backend-ID index rather than scanning every
backend for each profile. Prompt input duplicate detection uses a set; input
and message capture process their records once, with one final definition
hash and one lazily shared metadata snapshot. Existing sorted inventory
construction and dependency/template work are separate costs, not disguised
linear-time guarantees.
- The response-error recorder adds constant metadata work per read. The
authentication recognizer parses a fixed number of document levels/fields,
so its processing is linear in the consumed body size, not multiplied by
arbitrary input depth through a custom recursive recognizer.
- Generation accounting updates a fixed number of usage/event fields per
attempt and appends captured attempt records with amortized constant append
cost. Storing `A` requested attempt records requires `O(A)` space. Error
classification additionally pays for the supplied cause graph, including
finding 3; arbitrary collaborator/callback work is not constant-time.
- The repair request uses the original message prefix plus a bounded-count
suffix, not an accumulating conversation from every prior repair. Repeated
prefix processing follows the amount of message data actually resubmitted;
it is not an unintended growing-history algorithm.
The complexity finding follows from current source and the standard library's
error-tree traversal. No new benchmarks, real-world performance collection,
provider calls, implementation changes, or public complexity guarantees were
introduced. Existing validation evidence above applies to the unchanged code.