From 9748897751c745264bd78622aa8a275b60d3652e Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Tue, 11 Aug 2026 16:09:50 +0000 Subject: [PATCH] Document inspection and target resolution audit findings --- audit.md | 250 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 250 insertions(+) diff --git a/audit.md b/audit.md index ae1b191..ecde8f8 100644 --- a/audit.md +++ b/audit.md @@ -2606,3 +2606,253 @@ Temporary probes, removed before this artifact was edited, confirmed: - Stage 17 owns cross-cutting efficiency consolidation. S10-F05 provides the measured JSON-only allocation candidate, while source caching across operations remains intentionally out of scope. + +## Stage 11: Inspection And Execution-Target Resolution + +### Scope Reviewed + +The review covered `internal/usecase/profile_inspection.go`, +`internal/usecase/prompt_inspection.go`, and the selection, target-resolution, +and preparation portions of `internal/usecase/runner.go`, together with their +focused use-case and public contract tests. The preparation entry point in +`internal/usecase/prepared_execution.go` was consulted only through creation +of its frozen execution snapshot. Model invocation, repair coordination, +capacity scheduling, and prepared-handle lifecycle were not audited. + +The code graph was used first to establish the exact shared paths. Prompt +inspection and all preparation workflows converge on +`resolvePromptDefinition`; profile inspection and preparation converge on +`resolveProfileSelection`, `resolveExecutionTarget`, and +`validateResolvedExecutionTarget`; and `Prepare`, `Run`, and +`PrepareExecution` converge on `resolvePreparation`. The review followed those +helpers through default, backend, profile, request, credential, session, +output-contract, schema-metadata, rendering, hashing, and prepared-snapshot +consumers only as far as needed to decide preparation fidelity. + +### Accepted Findings + +#### S11-F01: Runtime NaN overrides bypass the documented numeric ranges + +- **Category:** correctness +- **Severity:** high +- **Confidence:** confirmed +- **Status:** accepted +- **Affected code:** `internal/usecase/runner.go` + (`mergeExecutionTargetOverride` and `resolvePreparation`), + `internal/usecase/runner_test.go` + (`TestRunnerPrepareInvalidRequestNumericOverridesFail` and the target-merge + tables), and root execution-setting tests +- **Contract at issue:** A request temperature must be from `0` through `2` + and top-p must be from `0` through `1`. Preparation is the boundary that + validates explicit numeric overrides and publishes stable, executable target + metadata. +- **Evidence:** `mergeExecutionTargetOverride` rejects each float only with + less-than and greater-than comparisons. IEEE NaN makes all those comparisons + false, so both `Temperature: &nan` and `TopP: &nan` are copied into the + effective target and marked explicitly present. A temporary public probe + passed NaN for both fields to `Engine.Prepare`; it returned a nil error and a + `PreparedRun` containing NaN in both effective settings. The invalid-override + table covers ordinary finite values below and above the ranges but not + non-finite values. S08-F01 records the analogous profile-source defect; this + finding is the distinct per-request path. +- **Failure mode:** A request can produce a supposedly prepared value that + cannot be represented by its stable JSON contract and cannot be serialized + as a valid provider number. Failure is deferred from request validation to a + later serializer or injected client, and the accepted target contradicts + the public numeric range. +- **Recommended direction:** Apply an explicit finite-number check before the + range checks for both pointer fields, using the same domain-level numeric + acceptance rule ultimately used to resolve S08-F01. Return the existing + invalid-request category before artifact, rendering, or model work. +- **Required verification:** Exercise NaN, positive and negative infinity, + both finite out-of-range sides, both exact boundaries, and representative + interior values for temperature and top-p. Cover `Prepare`, + `PrepareExecution`, and ordinary request resolution, require + `ErrInvalidRequest` with no partial prepared value for invalid cases, and + retain explicit-zero presence assertions. Confirm stable JSON never receives + a non-finite effective setting. + +#### S11-F02: Request output-contract replacement accepts unsupported enum values + +- **Category:** correctness +- **Severity:** medium +- **Confidence:** confirmed +- **Status:** accepted +- **Affected code:** `internal/usecase/runner.go` + (`resolveOutputContract`, `resolvePreparation`, and + `resolveStructuredOutput`), `internal/usecase/prepared_execution.go` + (`PrepareExecution` and `prepareValidation`), and output-contract preparation + tests in `internal/usecase/runner_test.go` and the root package +- **Contract at issue:** A request override replaces the complete prompt + output contract, with empty format normalized to `text`; every other + effective format and validation mode must be one of the declared supported + constants. `PrepareExecution` promises a completely prepared execution + snapshot rather than a handle retaining an already-invalid contract. +- **Evidence:** `resolveOutputContract` copies a non-nil request value and + defaults only an empty format. It never checks an unknown nonempty format or + validation mode. `resolveStructuredOutput` and both validation-plan + preparers special-case only `json_schema`; a different unknown mode is + retained without validation. A temporary public probe supplied format + `binary` and validation mode `unknown`. Both `Engine.Prepare` and + `Engine.PrepareExecution` returned nil errors, and their details preserved + the unsupported values. Existing tests cover valid replacement and the + empty-format default, but no unsupported enum. +- **Failure mode:** Offline preparation publishes metadata outside the stable + output vocabulary, while executable preparation returns a handle whose + selected content-check mode was never accepted. The preparation APIs cease + to be reliable preflight boundaries, and different later consumers can + silently interpret the unknown format as text or reject the mode only after + work that preparation should have prevented. +- **Recommended direction:** Normalize and validate the complete effective + output contract once in the shared resolution phase. Preserve the documented + empty-format fallback, require one supported validation mode, and keep + JSON-Schema path checks and schema loading in their existing preparation + owners. Request-invalid enum values should retain the invalid-request + identity rather than masquerading as source or generated-content failures. +- **Required verification:** At both preparation APIs, table the empty and + three supported formats, every supported validation mode, an empty mode, + unknown nonempty format and mode values, and JSON-Schema path requirements. + Invalid request values must fail before artifact reads, rendering, + validation-plan construction, admission, or generation and return no + partial value or handle. Retain a parity assertion that both preparation + workflows publish the same normalized contract for every valid case. + +### Unresolved Observations + +None. Concerns about reserved provider parameters, timeout-to-duration +conversion, model calls, repairs, capacity leases, and handle claiming were +assigned to their owning later stages rather than inferred from preparation +traces. + +### Coverage Ledger + +- **Prompt selection and inspection:** `InspectPrompt`, `Prepare`, `Run`, and + `PrepareExecution` use the same exact ID/version repository lookup and + definition hash. Nonblank identifiers are passed unchanged, ambiguous and + absent selections preserve source identities for public mapping, and + inspection copies declared input metadata without resolving a default + profile, schema, artifact, template, or credential. Focused internal and + root tests protect unchanged lookup values, call counts, hash parity, + ownership, cancellation classification, and public not-found isolation. +- **Profile and backend selection:** Inspection trims its required explicit + profile ID; request preparation selects a trimmed explicit ID before the + prompt's trimmed default. Both paths use the same repository precedence, + make a value copy before normalizing the selected backend ID, resolve that + backend once, and preserve profile and backend error identities. Endpoint- + only profiles retain an empty backend identity, while profile or request + endpoint overrides retain a selected backend's identity and capacity key. +- **Target merging:** `resolveExecutionTarget` applies the framework timeout + baseline, backend defaults, profile values, and request override in order. + Profile numeric zero inherits the lower layer, request pointer zero is + explicit and recorded in `ExecutionTargetPresence`, nonblank strings replace + their lower layer, reasoning implements inherit/set/clear tri-state + semantics, and each nonempty extra-parameter map replaces rather than merges + its predecessor. Target helper and engine integration tables cover every + field and backend/profile/request precedence. S11-F01 is the untested NaN + escape from the otherwise complete float range checks. +- **Credentials:** Backend and profile environment-variable names follow the + documented precedence; a profile requiring a direct key clears an inherited + backend name, and an explicit request environment name or direct key + satisfies preparation. Inspection reports the environment name or separate + direct-key requirement without reading the environment. Preparation checks + availability, retains a direct value only in private execution state, and + clears it from prepared metadata. Focused tests cover absent environments, + direct-key precedence, request-name precedence, mutual exclusivity during + inspection, and redaction. +- **Session precedence:** Direct session IDs are normalized before source + loading. A nonblank direct value clears the session template only on a + definition copy, is installed after message rendering, leaves the original + definition hash unchanged, and participates in the rendered-prompt hash. A + blank direct value retains template behavior. The renderer's source-owned + normalization and limits were established in Stage 9 and were not reopened. +- **Output contracts and schemas:** Prompt inspection reports the normalized + declared contract without loading a schema. A request value replaces the + whole prompt contract and an empty effective format becomes text. Schema + metadata is loaded only for JSON-Schema mode; the compiled-plan versus + document-only distinction remains S10-F03. Unsupported request enums bypass + the shared preparation boundary as S11-F02 records. +- **Preparation and freezing:** `Prepare` and `Run` use one resolution and + completion pipeline; `PrepareExecution` uses the same resolution and render + completion around a retained validation plan. The prompt definition is + consumed into hashes and rendered messages during preparation, target maps + and public results are copied, and the opaque execution snapshot is cloned + before it is exposed. Maintained public tests mutate prompt, profile, schema, + artifact, request, and returned-detail sources after preparation and protect + later execution independence. Prepared-handle synchronization and credential + revalidation remain Stage 13 scope. +- **Errors and cancellation:** Blank required selections fail before source + work; canceled inspection fails before repository calls and preserves both + operation and context identities; absent sources remain distinct public + not-found errors; unknown backends remain profile-load failures; invalid + numeric request values and missing credentials remain invalid requests; and + schema preparation remains a validation operation. No new error-identity + defect was found apart from the invalid values accepted by S11-F01 and + S11-F02. +- **Duplication and test ownership:** Selection, hashing, profile/backend + resolution, target merging, and structural target validation each have one + use-case owner shared by inspection and preparation. Root tests own public + mapping, source assembly, and mutation independence; focused use-case tests + own collaborator call counts, merge rules, and operation categories. No + duplicate finding was warranted. S02-F02 and S08-F01 already establish that + profile acceptance itself needs a shared domain owner; Stage 17 can decide + whether runtime numeric and output-contract validation should join the same + source-neutral validation boundary. + +### Verification Performed + +The code knowledge graph inventoried every function in the three scoped files, +traced the shared helpers inbound from inspection and all preparation entry +points, and traced their default, backend, profile, request, credential, +output-contract, renderer, schema, and snapshot consumers. Source was then read +for every scoped helper and focused test, together with public GoDoc, the +framework format reference, the package consumer guide, internal runner and +architecture documents, and prior audit handoffs. + +The following focused commands passed: + +```sh +go test ./internal/usecase -run 'Test(RunnerInspectPrompt|RunnerInspectProfile|RunnerPrepare|ResolveExecutionTarget|MergeExecutionTarget|ExecutionProfileToTarget)' -count=1 +go test . -run 'Test(InspectPrompt|InspectProfile|EngineExecutionSettingPrecedence|CustomBackendFlowsThroughProfilesOverridesAndInjectedClient|PreparedExecutionFreezesSourcesAndReturnsIndependentDetails)' -count=1 +resolution_audit_cover=$(mktemp) +go test -coverprofile="$resolution_audit_cover" ./internal/usecase +go tool cover -func="$resolution_audit_cover" +rm "$resolution_audit_cover" +go vet ./internal/usecase . +``` + +The repository-wide `go test ./...`, `go test -race ./...`, `go vet ./...`, +and `go run ./examples/go-library/prepare` checks also passed. The coverage +diagnostic reported 90.3% statement coverage for `internal/usecase`; coverage +was used only to locate unexercised resolution branches, not as evidence by +itself. + +A temporary public-package probe, removed before this artifact was edited, +confirmed that: + +- `Prepare` accepted NaN temperature and top-p pointers and returned both NaN + values in `EffectiveModelParams`; and +- `Prepare` and `PrepareExecution` accepted format `binary` and validation mode + `unknown` and preserved both unsupported values in their returned details. + +### Handoff + +- The Stage 0 baseline remains absent and was not backfilled during this + inspection and preparation review. +- S08-F01 owns non-finite values entering from profile sources; S11-F01 owns + the distinct request-override path. A later remediation should use one + finite-range rule rather than fixing these paths independently. +- Stage 12 should use the resolved prompt, target, contract, session, and + prepared values recorded here as established inputs. It should not re-audit + merge precedence; S11-F02 supplies the preflight defect when considering + whether invalid modes can reach generation or validation. +- Stage 13 owns lifecycle, concurrent claim/discard, execution-time credential + revalidation, and the retained snapshot after `PrepareExecution` returns. + The source-to-snapshot freezing boundary recorded here is its starting + invariant. +- Stage 14 owns endpoint construction, environment lookup inside the default + client, timeout conversion, reserved extra-parameter enforcement, and wire + serialization of the already resolved target. Those mechanics were not + reviewed here. +- Stage 17 should consolidate the source-neutral acceptance rules already + identified by S02-F02, S08-F01, S11-F01, and S11-F02 without moving use-case + orchestration into a lower-level source package.