promptkit/docs/roadmap/remediation.md

# Smoke Test Implementation Review Findings

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

Reviewed implementation commits after `8fbd058` through
`2f735aedf9b5dcc98164292db46ae1eab3e15cde`, against the
[feature roadmap](smoke-tests.md), [implementation plan](implementation.md),
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`](../../internal/usecase/smoke_tester.go), lines 355–366,
and [`runOpenRouterKey`](../../internal/usecase/smoke_authentication.go),
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: open.

Locations: [`recordGenerationEvidence`](../../internal/usecase/smoke_tester.go),
lines 240–248, and
[`resolveSmokeTestEndpoint`](../../internal/usecase/smoke_selection.go),
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.

## Finding 3: Initialize Zero-Dispatch Usage Consistently

Priority: P3. Status: open.

Location: [`SmokeTester.Run`](../../internal/usecase/smoke_tester.go),
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](../../observations.go) 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.