Audit generic and Seriatim modules
This commit is contained in:
@@ -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.
|
||||
|
||||
<!--
|
||||
Finding template for later audit stages:
|
||||
|
||||
@@ -737,6 +840,19 @@ Finding template for later audit stages:
|
||||
registrar shape while retaining family-local registration policy and
|
||||
diagnostics. Combining them would move extension ownership out of the domain
|
||||
registrars and weaken the composition boundary established by ADR-0004.
|
||||
- Seriatim's `decodeOptionalSegmentID`, `decodeOptionalNumber`, and timestamp
|
||||
checks deliberately keep separate external number handling at the input
|
||||
adapter. Segment IDs accept only positive canonical decimal JSON numbers or
|
||||
strings, while optional timestamps accept finite non-negative decimals and
|
||||
preserve their lexical `json.Number` form in generic metadata. Replacing
|
||||
these with a permissive generic number coercer would weaken the documented
|
||||
source contract and leak an external-format decision into `core/source`.
|
||||
- `generic/valid_json` and `generic/valid_json_schema` both detect malformed
|
||||
JSON, but remain useful separate serialized-validator capabilities. The
|
||||
former is a cheap format-only boundary; the latter must parse an instance to
|
||||
apply an independently supplied schema and can be selected without the
|
||||
former. MOD-002 concerns repeated compilation within one schema validator,
|
||||
not merging the two module identities or requiring a particular chain.
|
||||
- `internal/modules/dnd/register.registerModules`, `registerEvidence`,
|
||||
`registerValidators`, and `registerDefaultChains` use explicit typed
|
||||
registration lists. At this architectural pass, that repetition preserves
|
||||
@@ -1303,6 +1419,56 @@ Finding template for later audit stages:
|
||||
provenance, and scheduled concurrency; scheduler and promptfs tests cover
|
||||
limits, queued cancellation, release, path scoping, reads, and ownership.
|
||||
|
||||
### Generic And Seriatim Modules
|
||||
|
||||
- **Seriatim source boundary:** The transcript adapter strictly decodes one
|
||||
top-level JSON object, requires metadata and a non-empty segment array,
|
||||
validates unique positive canonical IDs, non-empty speaker/text values, and
|
||||
finite ordered timestamps, then chooses a deterministic document ID from the
|
||||
request, metadata, or raw-byte digest. It constructs generic units with
|
||||
exact self-references, computes the canonical source digest, and runs the
|
||||
framework document validator. Transcript-only speaker/start/end helpers and
|
||||
external metadata remain in the adapter package; the resulting document
|
||||
exposes only generic source fields and metadata.
|
||||
- **Generic chunk planning:** The unit chunker validates the complete source
|
||||
document, computes source-addressed ranges by unit position with bounded
|
||||
overlap, returns only the source digest and unit-ID ranges, and leaves
|
||||
materialization, chunk identity, and plan validation to the framework. Tests
|
||||
cover defaults, exact boundaries, overlap, invalid/unknown options, empty
|
||||
sources, and non-retention of source units. MOD-001 records the only
|
||||
avoidable option-decoder surface found.
|
||||
- **Merge, normalize, and validation ownership:** Typed append-order combines
|
||||
values in framework order through a family-supplied typed combiner, while
|
||||
typed no-op normalization preserves the exact artifact type; D&D registrars
|
||||
instantiate those domain-neutral mechanics without introducing D&D imports
|
||||
into the generic packages. Always-accept/reject, JSON, and JSON-Schema
|
||||
validators retain distinct selectable targets and strict empty option sets.
|
||||
Schema mismatch remains an ordinary rejection and malformed schema remains
|
||||
an operational error. MOD-002 records only repeated schema compilation.
|
||||
- **JSON output and evidence:** Output decoding rejects unknown and
|
||||
context-incompatible fields, normalizes/deduplicates/sorts evidence lane
|
||||
allowlists, and validates configured lanes before preparation. Preparation
|
||||
retains the full configured policy while selecting active exact-typed
|
||||
evidence projectors; runtime evidence is built only from compatible accepted
|
||||
normalized outputs. The encoder sorts lanes and logical files, rejects
|
||||
unsafe or colliding lane filenames, emits no physical paths, validates and
|
||||
republishes opt-in chunk-map and prepared evidence-context artifacts, and
|
||||
preserves annotation number bytes. MOD-003 records only unreachable clone
|
||||
code outside those live ownership paths.
|
||||
- **Registration and dependency direction:** Generic registration checks all
|
||||
required registries before mutation and registers the unit chunker, four
|
||||
validators, and JSON output; Seriatim independently registers only its input
|
||||
adapter. Direct import inspection found no D&D package in either family and
|
||||
no Seriatim package below the input registrar. Typed append/no-op libraries
|
||||
are instantiated by the D&D family rather than acquiring domain concepts.
|
||||
- **Composed contracts:** Focused module tests cover strict registration,
|
||||
source parsing/identity/metadata, chunk planning, typed combination,
|
||||
validator outcomes, JSON bundle determinism, chunk-map/evidence validation,
|
||||
and mutation isolation. Framework evidence-preparation/output tests prove
|
||||
invocation filtering and exact codec/projector compatibility; maintained CLI
|
||||
example contracts prove Seriatim input through generic JSON publication and
|
||||
opt-in chunk-map provenance.
|
||||
|
||||
## Validation Record
|
||||
|
||||
| Date | Scope | Command or check | Result |
|
||||
@@ -1340,6 +1506,12 @@ Finding template for later audit stages:
|
||||
| 2026-08-08 | LLM and asset graph/content review | Exact symbol reads and call traces across scheduled completion, PromptKit adaptation, profiles/preflight/fingerprints, sessions, debug, asset flattening/hashing, prompt filesystems, all fourteen manifests, every prompt YAML, shared/model-visible content, private schemas, and the root asset leaf | LLM-001 through LLM-004 recorded; profile parity, exact prompt order/cache points, asset ownership, and secret-free model-visible content otherwise confirmed |
|
||||
| 2026-08-08 | LLM and prompt filesystem race tests | `go test -race ./internal/framework/llm ./internal/framework/promptfs` | Pass |
|
||||
| 2026-08-08 | Composed CLI and D&D prompt-cache tests | `go test ./internal/cli ./internal/modules/dnd/register` | Pass |
|
||||
| 2026-08-08 | Audit target integrity before generic/Seriatim review | `git diff --quiet 92e89076a268089e703978fb9d7176200e93344c..HEAD -- . ':(exclude)docs/roadmap/**'` | Pass; production target unchanged |
|
||||
| 2026-08-08 | Generic and Seriatim graph/code review | Architecture clusters, exact symbol reads, call traces, scoped caller/search checks, and complete implementation/test inspection across input parsing, chunk planning, typed merge/normalize helpers, validators, JSON output, registration, evidence preparation, and output publication | MOD-001 through MOD-003 recorded; source/domain boundaries and live ownership paths otherwise confirmed |
|
||||
| 2026-08-08 | Generic and Seriatim module tests | `go test ./internal/modules/generic/... ./internal/modules/seriatim/...` | Pass |
|
||||
| 2026-08-08 | Published source-artifact codec tests | `go test ./internal/framework/chunkmap ./internal/framework/evidencecontext` | Pass |
|
||||
| 2026-08-08 | Composed evidence/output contracts | Focused `go test` runs in `./internal/framework/pipeline` and `./internal/cli` for evidence preparation/publication, maintained minimal JSON output, and production chunk-map provenance | Pass |
|
||||
| 2026-08-08 | Generic and Seriatim import boundaries | Direct `go list` import-edge audit plus scoped transcript-adapter import search | Pass; no D&D dependency in generic/Seriatim packages and no production import of the transcript adapter outside Seriatim registration |
|
||||
|
||||
## Coverage Matrix
|
||||
|
||||
@@ -1352,7 +1524,7 @@ Finding template for later audit stages:
|
||||
| Execution, validation, retry, and concurrency | Reviewed | Internal pipeline runtime contract; runner, chunk planning/validation, concurrent lane engine, typed execution/validation, retry/normalize, synchronization, output suppression, and focused concurrency, retry, rejection, session, typed-checkpoint, debug, manifest, candidate-encoding, and handoff tests | Target-integrity check, graph state-machine/call/complexity review, race tests, fresh tests, shuffled tests | RUN-001, RUN-002, RUN-003, RUN-004 |
|
||||
| State, checkpoints, debugging, and file safety | Reviewed | State and operations docs plus architecture state/security policy; CLI output, cache-root, checkpoint identity, recomputation, debug allocation, and terminal owners; core file I/O, debug bundle, and source digest/clone helpers; framework checkpoint, chunk-plan, chunk-map, debug, evidence-context implementations and focused tests | Target-integrity check, graph architecture/complexity/call traces, state collaborator race tests, CLI tests | STATE-001, STATE-002, STATE-003 |
|
||||
| LLM runtime, prompt filesystems, and assets | Reviewed | LLM internal/integration/configuration/operations docs and related ADRs; scheduled client, scheduler, PromptKit adapter/profile inspector, profile/source fingerprints, redaction, debug, asset registry, schema loader, promptfs implementations and focused tests; root assets, fallback profile, all fourteen module manifests and prompt YAML files, shared/model-visible fragments, private response schemas, and representative composed loaders | Target-integrity check, graph architecture/symbol/call review, content scan and exact prompt-order comparison, focused race tests, composed CLI and D&D prompt-cache tests | LLM-001, LLM-002, LLM-003, LLM-004 |
|
||||
| Generic and Seriatim modules | Pending | — | — | — |
|
||||
| Generic and Seriatim modules | Reviewed | Internal module guide; Seriatim, JSON output, chunk-map, and evidence-context integration contracts; all implementation/tests under `internal/modules/generic/` and `internal/modules/seriatim/`; framework evidence preparation/output paths and focused tests; maintained CLI example and production output contracts | Target-integrity check, graph architecture/symbol/call/caller review, direct import map, required module and codec tests, focused composed evidence/output tests | MOD-001, MOD-002, MOD-003 |
|
||||
| Shared D&D types, codecs, and family mechanics | Pending | — | — | — |
|
||||
| NPC, item, and location registries | Pending | — | — | — |
|
||||
| NPC, item, and location occurrences | Pending | — | — | — |
|
||||
|
||||
Reference in New Issue
Block a user