diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 03dc81c..5dad20f 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -1,6 +1,6 @@ # Smoke Test Implementation Review Findings -Status: two findings completed; one open. Review completed 2026-10-13. +Status: all three findings completed 2026-10-13. Review completed 2026-10-13. Reviewed implementation commits after `8fbd058` through `2f735aedf9b5dcc98164292db46ae1eab3e15cde`, against the @@ -101,7 +101,7 @@ passed for the facade, model client, generation, use case, and engine packages. ## Finding 3: Initialize Zero-Dispatch Usage Consistently -Priority: P3. Status: open. +Priority: P3. Status: completed 2026-10-13. Location: [`SmokeTester.Run`](../../internal/usecase/smoke_tester.go), lines 52–87, and its early return paths. @@ -130,6 +130,14 @@ inference and an inapplicable/unsupported operation, reusing accumulator coverag for arithmetic. Existing coordinator tests already cover correct zero-dispatch accounting after the generation boundary is entered. +Completion: report finalization now applies the shared zero-attempt accumulator +whenever no attempt was dispatched, replacing scattered early-return assignments. +Unsupported/inapplicable operations and early cancellation return known, complete +zero usage; actual dispatches retain their reported or unknown usage unchanged. +The strengthened orchestration assertions failed before the fix. Final validation +passed with `GOWORK=off`: `go test -race ./...`, `go vet ./...`, the three offline +consumer examples, and `git diff --check`. + ## Review Validation Passed with `GOWORK=off`: @@ -141,7 +149,8 @@ Passed with `GOWORK=off`: 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. +suite at the reviewed revision did not cover these cases; the completed +remediations add the corresponding regression coverage. Both catalogs remain selected at locally validated `v3.20261013.0` candidates. Their publication and any publication-date re-pinning remain separate release diff --git a/internal/usecase/smoke_tester.go b/internal/usecase/smoke_tester.go index 95ac751..6c1248c 100644 --- a/internal/usecase/smoke_tester.go +++ b/internal/usecase/smoke_tester.go @@ -57,6 +57,12 @@ func (t *SmokeTester) Run(ctx context.Context, request domain.SmokeTestRequest) } finish := func(err error) (*domain.SmokeTestReport, error) { report.Timing.Duration = nonnegativeDuration(now().UTC().Sub(startedAt)) + for _, attempt := range report.Attempts { + if attempt.Dispatched { + return report, err + } + } + report.Usage = noDispatchUsage() return report, err } @@ -163,7 +169,6 @@ func (t *SmokeTester) runInference( if errors.Is(err, ErrAPIKeyRequired) || errors.Is(err, ErrAPIKeyEnvMissing) { report.Reason = domain.SmokeTestReasonRequiredCredentialMissing report.Failure = domain.ExecutionFailureInvalidRequest - report.Usage = noDispatchUsage() report.Completed = true return finish(nil) } @@ -187,13 +192,11 @@ func (t *SmokeTester) runInference( if err != nil { if contextErr := ctx.Err(); contextErr != nil { report.Failure = generation.ClassifyFailure(contextErr) - report.Usage = noDispatchUsage() return finish(contextErr) } var capacityErr *CapacityError if errors.As(err, &capacityErr) { report.Failure = domain.ExecutionFailureCapacity - report.Usage = noDispatchUsage() report.Completed = true return finish(nil) } diff --git a/internal/usecase/smoke_tester_test.go b/internal/usecase/smoke_tester_test.go index 87afd2c..634eb06 100644 --- a/internal/usecase/smoke_tester_test.go +++ b/internal/usecase/smoke_tester_test.go @@ -184,7 +184,8 @@ func TestSmokeTesterReportsUnsupportedAndInapplicableAuthentication(t *testing.T t.Fatal(err) } if report.Supported != tc.supported || report.Reason != tc.reason || !report.Completed || - !sameBool(report.Applicable, tc.applicable) || len(report.Attempts) != 0 || report.Facts != (domain.SmokeTestFacts{}) { + !sameBool(report.Applicable, tc.applicable) || len(report.Attempts) != 0 || report.Facts != (domain.SmokeTestFacts{}) || + report.Usage != noDispatchUsage() { t.Fatalf("report = %+v", report) } }) @@ -201,7 +202,7 @@ func TestSmokeTesterReportsReachabilityUnavailableWithoutHTTPClient(t *testing.T } if report.Supported || report.Applicable != nil || !report.Completed || report.Reason != domain.SmokeTestReasonHTTPDiagnosticsUnavailable || - len(report.Attempts) != 0 || report.Facts != (domain.SmokeTestFacts{}) { + len(report.Attempts) != 0 || report.Facts != (domain.SmokeTestFacts{}) || report.Usage != noDispatchUsage() { t.Fatalf("unsupported reachability report = %+v", report) } } @@ -226,14 +227,16 @@ func TestSmokeTesterSelectionAndCancellationDoNotDispatch(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) cancel() - report, err := tester.Run(ctx, domain.SmokeTestRequest{ - Kind: domain.SmokeTestReachability, BackendID: "alpha", - }) - if !errors.Is(err, context.Canceled) || report == nil || report.Completed || report.Failure != domain.ExecutionFailureCanceled { - t.Fatalf("canceled report=%+v error=%v", report, err) - } - if report.Facts != (domain.SmokeTestFacts{}) || len(report.Attempts) != 0 || calls != 0 { - t.Fatalf("cancellation dispatched or invented facts: %+v calls=%d", report, calls) + for _, kind := range []domain.SmokeTestKind{domain.SmokeTestReachability, domain.SmokeTestTextInference} { + report, err := tester.Run(ctx, domain.SmokeTestRequest{ + Kind: kind, BackendID: "alpha", + }) + if !errors.Is(err, context.Canceled) || report == nil || report.Completed || report.Failure != domain.ExecutionFailureCanceled { + t.Fatalf("canceled report=%+v error=%v", report, err) + } + if report.Facts != (domain.SmokeTestFacts{}) || len(report.Attempts) != 0 || calls != 0 || report.Usage != noDispatchUsage() { + t.Fatalf("cancellation dispatched, invented facts, or left usage unknown: %+v calls=%d", report, calls) + } } }