promptkit/docs/roadmap/remediation.md

Smoke Test Implementation Review Findings

Status: two findings completed; one open. Review completed 2026-10-13.

Reviewed implementation commits after 8fbd058 through 2f735aedf9b5dcc98164292db46ae1eab3e15cde, against the feature roadmap, implementation plan, public GoDoc, and applicable repository policies. No implementation changes were made during this review.

The facade/internal boundary, final-value configuration ownership, catalog-owned diagnostic metadata, shared generation/capacity machinery, private inference cases, and neutral factual reporting are in place. No additional strengthened contracts or prohibitions requiring remediation were identified. The findings below are reporting-correctness gaps, not reasons to redesign the feature.

Finding 1: Preserve Caller Cancellation Independently Of Body Error Type

Priority: P2. Status: completed 2026-10-13.

Locations: finishHTTP, lines 355–366, and runOpenRouterKey, lines 107–110.

finishHTTP returns a caller-context error only when report.Failure has already been classified as canceled/deadline. Authentication body reads can fail with an ordinary I/O error while the caller context is canceled. Their failure is then classified as transport, so the operation incorrectly returns Completed=true and nil Go error despite interruption before authentication recognition finished.

An offline public-API reproduction supplies an HTTP 200 response whose body Read cancels the caller context and returns io.ErrUnexpectedEOF. The current result is Completed=true, Failure="transport", nil error, and an already canceled caller context. This conflicts with the accepted partial-report/error boundary; callers cannot recognize the cancellation with errors.Is.

Required outcome: recognize caller interruption during unfinished HTTP probe work independently of the underlying I/O error's identity. Return the retained report with Completed=false, the corresponding canceled/deadline failure, and an error preserving ctx.Err() identity. Retain response/status/destination facts already observed, leaving acceptance unknown when recognition did not complete. Keep configured request-timeout failures with an active caller as concluded reports, and do not revoke a diagnostic already completed before later cancellation.

Regression coverage: extend the owning use-case cancellation test with a body that cancels the caller and returns a non-context I/O error. Its existing fake returns context.Canceled, which masks this defect. Check partial-report and context identity rather than exact error text; reuse existing timeout coverage.

Completion: HTTP probe failures now check the caller context at the failed-I/O boundary before classification, preserving partial response evidence even for opaque transport/body errors. Completed authentication is not revoked when body closure subsequently cancels the caller. The new regressions failed before the fix; GOWORK=off go test -race ./internal/usecase ., focused use-case vet, and git diff --check passed. Existing configured-timeout coverage remains green.

Finding 2: Report The Actual Initial Inference Request Destination

Priority: P2. Status: completed 2026-10-13.

Locations: recordGenerationEvidence, lines 240–248, and resolveSmokeTestEndpoint, lines 171–188.

Inference selection leaves the diagnostic endpoint at the profile's base URL, and every attempt copies that value into InitialEndpoint. The built-in client actually dispatches to that base plus /chat/completions. Thus a request with no redirect reports different initial and final destinations even though no redirect occurred. InitialEndpoint is documented as the destination before redirects, not the base used to construct the request.

An offline public-API reproduction observes an actual request to https://openrouter.ai/api/v1/chat/completions, while the attempt reports InitialEndpoint="https://openrouter.ai/api/v1" and Endpoint="https://openrouter.ai/api/v1/chat/completions". Target.Endpoint also reports the base as its initially resolved diagnostic destination. This makes destination attribution misleading to consumers.

Required outcome: retain the actual pre-redirect request URL through the narrow internal evidence path, using the existing prepared-request URL composition rather than duplicating inference serialization/routing. Populate initial/final endpoint fields from their appropriate sources, and align Target.Endpoint with its stated meaning. Preserve the configured base in Target.EffectiveSettings.Endpoint. Continue excluding URL user information; do not infer an injected client's actual HTTP destinations from configuration.

Regression coverage: one built-in inference request without redirects must report its observed URL as both initial and final. Add a focused redirect case that distinguishes the actual completion URL from the last destination. Reuse existing HTTP redirect tests for credential/response attribution mechanics and keep injected-client HTTP destination evidence unknown.

Completion: built-in generation now passes the frozen prepared request URL through internal generation evidence. Diagnostic target/initial-attempt URLs use that value, while the configured base stays in effective settings and injected inference leaves HTTP destinations unknown. Public GoDoc and the internal generation document describe the corrected behavior. No-redirect, redirect, and injected regressions failed before the fix; race-enabled tests passed for the facade, model client, generation, use case, and engine packages.

Finding 3: Initialize Zero-Dispatch Usage Consistently

Priority: P3. Status: open.

Location: SmokeTester.Run, lines 52–87, and its early return paths.

Reports begin with a zero-valued ObservedTokenUsage. Unsupported/inapplicable operations and cancellation before generation return it without applying the shared zero-attempt accounting. The public ObservedTokenCount contract defines aggregate zero as known and complete when nothing was dispatched. Missing-required-credential and capacity-exhaustion returns already use the correct accumulator result, making the behavior inconsistent across otherwise equivalent no-dispatch paths.

Offline reproductions of inapplicable RakestrawHome authentication and inference with an already-canceled context both produce no attempts but return TotalTokens={Value:0, Known:false, Complete:false}. The same applies to the other token fields. There is no unobserved generation to justify unknown usage.

Required outcome: use the existing zero-attempt usage accumulator consistently when no attempt was dispatched, including early cancellation and unsupported or inapplicable outcomes. Preserve request facts as absent. Do not mark usage known for an actual dispatch whose token fields were not reported or overwrite usage retained from earlier dispatched attempts.

Regression coverage: focused orchestration assertions for early canceled inference and an inapplicable/unsupported operation, reusing accumulator coverage for arithmetic. Existing coordinator tests already cover correct zero-dispatch accounting after the generation boundary is entered.

Review Validation

Passed with GOWORK=off:

• go test -race ./...;
• go vet ./...;
• python3 scripts/check-examples.py.

The three findings were reproduced through the public API using a temporary external Go program and a controlled HTTP RoundTripper; no live provider was contacted and no reproduction source was added to the repository. The passing suite does not currently cover these cases.

Both catalogs remain selected at locally validated v3.20261013.0 candidates. Their publication and any publication-date re-pinning remain separate release preparation, as deliberately agreed, rather than an implementation defect or a new review gate.