Record observer integration audit finding
This commit is contained in:
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user