Record functional correctness audit findings
This commit is contained in:
131
docs/roadmap/audit/01-correctness.md
Normal file
131
docs/roadmap/audit/01-correctness.md
Normal file
@@ -0,0 +1,131 @@
|
||||
# Functional Correctness Audit
|
||||
|
||||
Reviewed 2026-11-04 against commit `d9f6aad`.
|
||||
|
||||
## Scope And Evidence
|
||||
|
||||
Reviewed the public facade and conversions; ConfigKit handoff, normalization,
|
||||
presence-aware target assembly, collections and namespace selection; artifact
|
||||
loading and template rendering/requirement discovery; ordinary and single-use
|
||||
prepared execution; output validation and repair; backend admission and active
|
||||
permits; HTTP request/response handling and transient retries; execution
|
||||
observations; and smoke-test selection and reporting. Contracts were checked
|
||||
against GoDoc, the format and integration references, internal documentation,
|
||||
and relevant existing tests. Source loading before the ConfigKit handoff remains
|
||||
ConfigKit's responsibility.
|
||||
|
||||
Validation:
|
||||
|
||||
- `go test -race -vet=off ./...` passed, including the maintained examples.
|
||||
- `go vet ./...` passed.
|
||||
- An offline, temporary public-API reproduction outside the repository confirmed
|
||||
the number conversion and captured schema payload described below. It used
|
||||
`ProfileSource`, `Prepare`, a runtime override, and `WithSchemaFS` with
|
||||
`fstest.MapFS`; no provider was contacted.
|
||||
|
||||
No production code or tests were changed. This audit does not establish provider
|
||||
availability or acceptance of arbitrary JSON Schema features.
|
||||
|
||||
## 1. Provider Source Helpers Silently Change Exact JSON Numbers
|
||||
|
||||
**Priority:** P2. Confirmed data-conversion defect.
|
||||
|
||||
**Code:** [providerValueForMapping](../../../provider_values.go:51), used by
|
||||
[ProfileSource](../../../profiles.go:33) and
|
||||
[BackendSource](../../../backends.go:100).
|
||||
|
||||
**Circumstances:** A caller supplies a valid `json.Number` in provider
|
||||
`ExtraParams` that cannot be represented exactly by the helper's chosen native
|
||||
number type. After signed and unsigned integer parsing fail, the helper converts
|
||||
the value to `float64` without checking whether that conversion loses precision.
|
||||
The same problem affects sufficiently precise fractional values.
|
||||
|
||||
**Expected:** A source helper preserves the meaning of a supported provider
|
||||
value, or reports that the value cannot be represented. It must not silently
|
||||
substitute a different number. This concerns PromptKit's own convenience-source
|
||||
conversion, not additional validation of ConfigKit's raw sources.
|
||||
|
||||
**Actual / reproduction:** With
|
||||
`ExtraParams: map[string]any{"number": json.Number("18446744073709551617")}`
|
||||
on a profile, `NewEngine` succeeds. JSON encoding of
|
||||
`Prepare(...).EffectiveModelParams.ExtraParams` produces:
|
||||
|
||||
```json
|
||||
{"number":18446744073709552000}
|
||||
```
|
||||
|
||||
Supplying the identical value through `RunRequest.Execution.ExtraParams`
|
||||
instead produces:
|
||||
|
||||
```json
|
||||
{"number":18446744073709551617}
|
||||
```
|
||||
|
||||
The built-in client serializes those effective values directly into the request,
|
||||
so configuration-source selection changes the actual provider parameter.
|
||||
`BackendSource` shares the same converter. No high-precision numeric preservation
|
||||
claim needs to be added for arbitrary source formats to correct this discrepancy.
|
||||
|
||||
**Potential remediation:** Preserve exact numbers through a numeric
|
||||
representation supported by ConfigKit, if available. If that contract cannot
|
||||
represent a particular `json.Number` exactly, surface a deferred source error
|
||||
rather than returning a rounded float or a numeric-looking string. Check numeric
|
||||
value equivalence rather than textual spelling: alternate exponent spellings
|
||||
need not be preserved. Add focused regression coverage for both source helpers
|
||||
and contrast with the already-preserving runtime path.
|
||||
|
||||
## 2. External Schema Resources Do Not Reach The Provider Constraint
|
||||
|
||||
**Priority:** P2. Confirmed schema-packaging gap; remediation requires choosing
|
||||
how provider-facing schemas accommodate the existing local-reference feature.
|
||||
|
||||
**Code:** [preparedValidation.SchemaDocument](../../../internal/validate/standard_validator.go:63),
|
||||
[StandardValidator.PrepareValidation](../../../internal/validate/standard_validator.go:74),
|
||||
[FSValidator.PrepareValidation](../../../internal/validate/standard_validator.go:126),
|
||||
and [structuredOutputFromValidationPlan](../../../internal/usecase/runner.go:419).
|
||||
|
||||
**Circumstances:** A JSON Schema output contract uses a root schema referring to
|
||||
another file within the configured schema source. The
|
||||
[schema format contract](../../formats.md#schemas) supports such references.
|
||||
Preparation compiles the transitive resources locally, but `SchemaDocument`
|
||||
exposes only the unchanged root document. The runner always uses that document
|
||||
for the native provider constraint in JSON Schema mode.
|
||||
|
||||
**Expected:** The provider-facing constraint should carry the schema resources
|
||||
needed to interpret the supported local references, or there should be an
|
||||
explicitly chosen way to retain local validation without submitting an
|
||||
unresolvable native constraint. This is not a promise that every provider accepts
|
||||
every schema dialect feature.
|
||||
|
||||
**Actual / reproduction:** With these files under `WithSchemaFS`:
|
||||
|
||||
```text
|
||||
root.json: {"type":"object","properties":{"x":{"$ref":"child.json"}},"required":["x"]}
|
||||
child.json: {"type":"integer"}
|
||||
```
|
||||
|
||||
`Prepare` succeeds, but `PreparedRun.StructuredOutput.JSONSchema.Schema` contains
|
||||
only the root object, including the literal `$ref: "child.json"`. No child
|
||||
document, bundled definition, or reference rewrite is present. The HTTP adapter
|
||||
passes that object unchanged to `response_format.json_schema.schema`. An upstream
|
||||
provider has no access to the application filesystem or private `fs.FS`, so it
|
||||
cannot resolve that reference from the request. A provider requiring resolvable
|
||||
native schemas can reject generation before local validation or repair is
|
||||
reached. No live-provider rejection was tested.
|
||||
|
||||
The internal source document accurately describes the existing root-only
|
||||
projection; the gap is between the supported local schema-resource graph and
|
||||
its mandatory projection into an outbound constraint, rather than a failure of
|
||||
local compilation or prepared-resource freezing. Self-contained schemas are
|
||||
unaffected.
|
||||
|
||||
**Potential remediation:** Produce a self-contained provider schema from the
|
||||
operation's captured resources, preserving reference semantics, identifiers,
|
||||
anchors, and recursion. Avoid naive recursive textual substitution. An
|
||||
alternative is an explicit choice to use local-only schema validation when
|
||||
native submission cannot represent the resource graph. Any alternative that
|
||||
restricts existing schema sources or silently disables native structured output
|
||||
requires a deliberate compatibility/policy decision. Protect the selected
|
||||
behavior with an offline test combining transitive schema preparation and the
|
||||
actual outgoing request, while retaining frozen local validation for
|
||||
`RunPrepared` and repair.
|
||||
Reference in New Issue
Block a user