diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index dc37a0d..bcdbf1b 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -7,11 +7,15 @@ the [selected roadmap](audit-remediation.md), its accepted catalog decisions, and the implementation through `6673284312de23bde86c20eb8de4f3865b157471`. The implementation changes begin after planning commit `6a1a32a`. -The broad architecture and selected simplifications are in place. Four findings -remain open below: two correctness/completeness issues, one error-projection -efficiency issue, and one documentation-status gap. This review implements none -of their remedies. The existing race suite passing does not cover the two new -failing edge cases. +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 remain open and are not duplicated. + +The broad architecture and selected simplifications are in place. Five findings +remain open below: three correctness/completeness issues, one error-projection +efficiency issue, and one documentation-status gap. These reviews implement none +of their remedies. The existing race suite passing does not cover the reproduced +edge cases. The accepted artifact ownership, supported convenience-source types, and self-contained schema changes are intentional compatibility changes. No @@ -167,6 +171,54 @@ 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:** Open. **Related stages:** 3–4, 25–26. + +**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. + ## Stage-By-Stage Review Record Each stage was checked against its requested outcome, changed implementation, @@ -229,3 +281,41 @@ 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. + +## 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.