Document inspection and target resolution audit findings
This commit is contained in:
250
audit.md
250
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.
|
||||
|
||||
Reference in New Issue
Block a user