From a22c1a7f593ee12a2c3412a95d9e944425fccf93 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Sat, 8 Aug 2026 21:59:06 +0000 Subject: [PATCH] Audit generic and Seriatim modules --- docs/roadmap/audit.md | 184 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 178 insertions(+), 6 deletions(-) diff --git a/docs/roadmap/audit.md b/docs/roadmap/audit.md index 35eb962..b90f307 100644 --- a/docs/roadmap/audit.md +++ b/docs/roadmap/audit.md @@ -19,11 +19,12 @@ change only roadmap audit documents do not change that production target. ## Executive Summary Pending final synthesis. The initial baseline is healthy. The architecture, -configuration/CLI, pipeline composition, reference/handoff, runtime, and state -reviews have found three High findings, four Medium findings, and thirteen Low -findings, with no production dependency inversion, unbounded framework worker -pool, completion-order-dependent result assembly, debug-to-cache coupling, or -model-visible credential material in the embedded LLM assets. +configuration/CLI, pipeline composition, reference/handoff, runtime, state, +LLM, and generic/Seriatim reviews have found three High findings, four Medium +findings, and fifteen Low findings, with no production dependency inversion, +unbounded framework worker pool, completion-order-dependent result assembly, +debug-to-cache coupling, model-visible credential material in the embedded LLM +assets, or domain leakage across the Seriatim and generic module boundaries. ## Finding Index @@ -50,6 +51,9 @@ Final cross-area ordering is pending synthesis. | LLM-002 | Low | Correctness | Recheck cancellation after scheduler admission | | LLM-003 | Low | Correctness | Reject duplicate virtual prompt names | | LLM-004 | Low | Duplication | Share the read-only in-memory filesystem mechanics | +| MOD-001 | Low | Simplicity | Narrow generic integer option decoding | +| MOD-002 | Low | Efficiency | Reuse compiled response schemas within a prepared validator | +| MOD-003 | Low | Simplicity | Remove unreachable JSON metadata clone helpers | ## Findings @@ -713,6 +717,105 @@ Final cross-area ordering is pending synthesis. - **Grouping:** Independent; implement after or alongside LLM-003 without moving manifest collision policy into the generic filesystem. +### Generic And Seriatim Modules + +### MOD-001 — Narrow generic integer option decoding + +- **Severity:** Low +- **Category:** Simplicity +- **Evidence:** `internal/modules/generic/chunk/units/chunker.go:138`–`220` + routes only `max_units` and `overlap_units` through a 61-line numeric + conversion family that accepts every signed and unsigned Go integer type, + integral `float64` values, and `json.Number`. Its only production callers are + the two local positive/non-negative wrappers. In contrast, the other integer + option in this family, JSON output's `window_units`, accepts the + configuration-native `int` directly + (`internal/modules/generic/output/json/encoder.go:178`–`188`). The focused + chunker tests reject a fractional float and malformed `json.Number`, but do + not establish a caller or contract for any accepted non-`int` + representation. +- **Impact:** The generic chunker's private option surface is substantially + larger than its two-option configuration contract and silently treats a + floating-point value such as `2.0` as an integer while the adjacent generic + output decoder rejects that representation. Additional numeric branches and + architecture-dependent range logic can drift without providing behavior + used by the maintained configuration path. +- **Recommendation:** Define the supported module-option scalar type at the + configuration boundary and reduce these two options to that representation, + currently `int`, with their existing positive/non-negative and overlap + checks. If a second real option source requires `json.Number` or another + representation, normalize it once at that source rather than retaining a + speculative all-Go-numeric converter in this leaf module. +- **Preserve:** Keep defaults, strict unknown-option rejection, precise option + names in diagnostics, `overlap_units < max_units`, platform-safe values, and + validation/build decoding parity. +- **Validation:** Add focused rejection cases for integral floats and other + unsupported numeric representations, retain boundary/overlap tests, and run + `go test ./internal/modules/generic/chunk/units ./internal/core/config`. +- **Grouping:** Independent. + +### MOD-002 — Reuse compiled response schemas within a prepared validator + +- **Severity:** Low +- **Category:** Efficiency +- **Evidence:** Every call to + `internal/modules/generic/validate/valid_json_schema.Validator.Validate` + delegates to `validate`, which parses the candidate, reparses the complete + response schema, creates a new `jsonschema.Compiler`, adds the schema + resource, and compiles it before validation + (`validator.go:29`–`56`). The framework constructs one validator instance per + prepared chain position (`internal/framework/pipeline/prepare.go:258`–`299`) + and invokes that same prepared instance for each chunk/artifact candidate + and retry (`runner.go:430`–`489` and `runner_typed.go:458`–`542`). Within a + chain position the active artifact schema is stable, while candidate content + changes. +- **Impact:** A run with many chunks or retries repeatedly pays schema parse, + compiler construction, resource loading, and compilation for identical + schema bytes. The cost is deterministic local CPU/allocation work on every + validation attempt; current LLM-backed workloads limit its overall severity. +- **Recommendation:** Cache the compiled schema on the prepared validator, + keyed by the complete schema identity including its JSON bytes or digest, + and make concurrent first use safe because extract validators can run in + parallel. Continue parsing each candidate independently and preserve support + for a validator instance receiving a different schema rather than assuming + one globally. +- **Preserve:** Retain operational errors for missing/malformed schemas, + ordinary `invalid_json` and `json_schema_invalid` rejections, support for + both chunk and artifact targets, exact candidate ownership, and standalone + use without a preceding `valid_json` validator. +- **Validation:** Add repeated-schema and changed-schema cases, exercise one + validator concurrently, verify malformed-schema errors are stable, and use a + focused benchmark or compiler hook to demonstrate one compilation per + distinct schema; run + `go test -race ./internal/modules/generic/validate/valid_json_schema ./internal/framework/pipeline`. +- **Grouping:** Independent. + +### MOD-003 — Remove unreachable JSON metadata clone helpers + +- **Severity:** Low +- **Category:** Simplicity +- **Evidence:** `cloneMetadata` and `cloneJSONMetadataValue` at + `internal/modules/generic/output/json/encoder.go:497`–`527` form a mutually + recursive deep-clone implementation. Knowledge-graph inbound-call inspection + and a scoped source search find only the calls between those two functions; + no encoder path, registration path, or test reaches either helper. Actual + output ownership is handled by `cloneNormalizeOutputs`, serialized-artifact + cloning, and immediate JSON serialization. +- **Impact:** Thirty-one lines of recursive, type-specific ownership code imply + a boundary that does not exist, add an untested maintenance surface, and can + mislead later changes into choosing the dead helper instead of the encoder's + active clone/serialization paths. +- **Recommendation:** Delete the two unreachable helpers. Do not replace them + with a shared abstraction unless a live metadata owner later demonstrates a + caller. +- **Preserve:** Keep request non-mutation, owned output bytes, exact preservation + of chunk-map annotation numbers, cloned normalized artifacts, and caller + mutation isolation after `Encode` returns. +- **Validation:** Retain the output encoder's request-mutation and annotation + number tests, run `go test ./internal/modules/generic/output/json`, and + confirm no production caller is removed with graph and compiler checks. +- **Grouping:** Independent. +