# Audita v2 and ConfigKit Migration Review Remediation Status: Complete. Reviewed `83eaa96` through `fae18bf` against the approved clean-cutover decisions, [feature roadmap](audita.md), and [implementation plan](implementation.md). Review date: 2026-09-25. No production changes were made. The previous completed transcription review is retained below as historical context. ## 1. Preserve omitted modules in direct YAML serialization Priority: P2. Status: Complete. The safe YAML projection now omits nil modules while retaining explicit empty and ordered/repeated selections. A round-trip regression failed before the fix and passes afterward. Config package tests, including credential-safety coverage, and its race tests pass. Location: `internal/config/credential_projection.go:25` (`AuditaConfig.MarshalYAML`), especially the plain slice field at line 30 and assignment at line 40. The direct safe YAML projection passes a nil module slice to yaml.v3, which serializes it as `modules: []`. Loading that output produces a non-nil empty slice. Under the new Audita contract these are different instructions: nil delegates to Audita's configuration/default sequence; empty passes an explicit empty module argument and disables correction modules. Serialization therefore silently changes behavior for a configuration that is subsequently reloaded. Evidence: a temporary package test marshaled `AuditaConfig{Binary: "audita", Timeout: "3h"}`, unmarshaled it into another `AuditaConfig`, and required `Modules == nil`. It failed with output containing `modules: []`. The test was removed after reproduction. This concerns direct `yaml.Marshal` of the typed configuration; ordinary CLI `config show` uses `MarshalEffectivePipeline` through a separate JSON/value-tree projection and does not use this faulty path. Required remediation: - Make the direct safe YAML projection preserve nil as null or omission and explicit empty as `[]`. Do not simply add `omitempty` to the slice: that would instead erase the explicit empty selection. - Keep credential serialization name-only and preserve module order/repetitions. - Add a small round-trip regression covering nil, explicit empty, and an ordered repeated selection; retain effective-output inspection coverage without duplicating its whole test matrix. ## 2. Remove the unapproved manifest compatibility decoder Priority: P2. Status: Complete. Removed the permissive custom decoder; ordinary strict typed decoding now applies to provenance as well. Replaced historical-acceptance coverage with retired/unknown-field rejection tests, all of which failed before removal. Current provenance round-trip coverage remains intact. Corrected the roadmap, plan, internal contract, and operator guidance for narrow backed-up migration without discarding execution/checkpoint state or changing resume authority. Location: `internal/manifest/manifest.go:79-89`, `internal/manifest/store_test.go:78`, and `docs/internal/manifest.md`'s decoding contract. Also correct the conflicting migration language in the plan/roadmap. The new `EffectiveConfigProvenance.UnmarshalJSON` bypasses the manifest store's `DisallowUnknownFields` by using permissive `json.Unmarshal` for the entire provenance object. It accepts retired `selected_profile` data and silently discards it. It also silently accepts unknown current provenance fields, including unknown nested selection fields, rather than limiting behavior to the renamed label. The user expressly selected a clean cutover without backward-compatibility shims. This adds precisely such a shim and a new historical-input acceptance guarantee. The regression test explicitly requires the old selection to load, and the internal guide now declares permissive informational-field decoding. The plan contributed to this divergence: it said existing generic decoding would handle historical optional provenance and required old manifests to load. That assumption was false; the baseline store already strictly decoded these fields. Repetition in the plan, tests, and documentation does not authorize overriding the user's clean-cutover decision. Required remediation: - Remove the custom compatibility unmarshaler and retain ordinary strict typed decoding for the new `selected_overlay` representation. Do not add dual fields, special retired-field handling, automatic manifest rewrites, or a format-version bump just for this rename. - Replace the historical-acceptance test with coverage for current provenance round trips and unknown/retired provenance rejection through the real store. - Correct the plan and current documentation to acknowledge the upgrade effect: manifests with retired provenance require deliberate operator migration before loading. Document narrow removal/renaming of retired informational provenance on a backed-up manifest, preserving execution/checkpoint state; do not advise deleting all session progress or rerunning everything. - Keep provenance informational: this correction does not authorize adding it to semantic resume keys or invalidating work based on overlay identity alone. If transparent historical-manifest acceptance is desired instead, obtain an explicit exception to the clean-cutover decision before retaining a compatibility reader. Do not treat that exception as already granted. ## Review verification The following records the original review, before remediation. - Existing `go test ./...` passed. - Race tests passed for subprocess, Audita, Notarius, config, stage, app, and manifest packages after removing the temporary reproduction. - `go vet ./...` passed. - The temporary direct-YAML regression failed as described above. - Source inspection and the new historical-provenance test establish finding 2; `LocalStore.LoadReader` still uses strict decoding outside the custom hook. The primary architecture is otherwise in place: ConfigKit v0.4.0 overlays, shared named credential forwarding/redaction, a v2-only Audita subprocess adapter, ordinary reports as diagnostics, and versioned ordered-module polish evidence. No live model/provider runs were performed during this review. ## Migration remediation verification Completed on 2026-09-25. Both new regressions failed before their respective fixes. After remediation, `go test ./...`, `go test -race ./...`, `go vet ./...`, `go build ./...`, documentation checks, maintained configuration example checks, and `git diff --check` passed. No live provider runs were needed. ## Historical completed transcription review Status: Complete. Reviewed commits `7b0bf99` through `96b4e73` against the speaker-checkpoint and upload-replay roadmap and the original user request. No production changes were made during review. The two failures below were reproduced with temporary tests using the existing stage fixtures; those temporary tests were removed afterward. ### 1. Reconcile Aggregate State Even When No Speaker Needs HTTP Work Priority: P2. Status: Complete. Implemented read-only aggregate identity/evidence checks before resume skips. Regression tests cover removed inputs, missing/duplicate/mismatched outputs, repair without HTTP requests, downstream invalidation, and exclusion of removed speakers from merge without deleting their files. Equivalent path forms and output ordering remain reusable. Focused app/stage tests and documentation checks pass. Location: `internal/stage/transcribe_resume.go:21` and the reconciliation result consumed there. `ValidateResume` returns reusable whenever `reconciliation.work` is empty. That only establishes that the remaining speakers need no transcription; it does not establish that the stored speaker collection and aggregate outputs match the current input set. Removing a speaker while the remaining inputs are unchanged produces an empty work list, so a succeeded record can be skipped without pruning the removed speaker. Its old aggregate output can then remain available to merge. Likewise, missing/inconsistent aggregate output references are not repaired when all individual checkpoints remain valid. A prepare rerun may already stale transcribe in some workflows, but the resume validator cannot rely on that to satisfy its own current-input/output checks. Reproduction: create complete alice/bob checkpoint records and aggregate outputs with the existing fixture, mark the stage succeeded, remove bob from the manifest's audio inputs, and call `ValidateResume`. It returns reusable despite still retaining bob in the stored collection and aggregate outputs. Required change: - Distinguish no HTTP work from no reconciliation work. Validate that the committed speaker set and aggregate transcript references represent the reconciled current set, including relevant identity/evidence, before skipping. - When only collection/output repair is needed, return non-resumable and let normal execution checkpoint the pruned collection and rebuild aggregate outputs without retranscribing reusable speakers. Preserve existing dependent invalidation; do not force otherwise-current speaker jobs. - Keep resume inspection read-only. Do not fix this by deleting incidental transcript files or changing stage selection rules. Verification: add removed-only input coverage (not just removal combined with an added speaker), plus incomplete/inconsistent aggregate output coverage. Exercise the runner skip path and assert zero additional WhisperX requests, correct rebuilt outputs, and no removed speaker reaching merge. ### 2. Remove the Unrequested Positive Audio-Size Requirement Priority: P2. Status: Complete. Audio evidence now permits zero size while still rejecting negative values. Regression tests verify zero-size manifest round trips, successful service results being checkpointed and reused, and service rejection being propagated without creating a completion record. Nonempty valid-JSON output validation is unchanged. Both new success regressions failed before the fix. Location: `internal/manifest/transcribe_state.go:95`. The new record validator requires `AudioSize > 0`. Previously Narratio accepted a regular `.flac` file and delegated transcription acceptance to WhisperX; neither the user request nor the roadmap introduced minimum audio size or a new audio-content validation policy. Zero-byte content still has a valid checksum and can be represented by the checkpoint mechanism. The current check runs after a successful response and output materialization, causing the entire stage to fail instead of retaining the completed work. Reproduction: prepare one speaker, replace its prepared audio with an empty regular file, and use the existing successful fake WhisperX client. Stage execution fails at checkpoint validation with `audio_size must be positive`. This demonstrates the extra Narratio restriction; it does not assert that a particular production WhisperX deployment accepts empty audio. Required change: - Permit zero audio size and reject negative sizes. Keep byte hashing and size equality checks. Let the downstream service decide whether the input can be transcribed, as before. - Do not remove the nonempty valid-JSON output requirement, which has a separate existing justification. Do not add a new early empty-audio rejection instead. - Update any new tests/documentation that promote strictly positive input size to a contract. Verification: cover a zero-size record round-trip and successful adapter result being checkpointed/reused for that input; retain negative-size rejection and ordinary downstream-failure tests. ### Verification After Remediation Completed on 2026-09-22 with Go 1.26.8: focused stage/application tests, changed-package race tests, `go test ./...`, `go test -race ./...`, `go vet ./...`, `go build ./...`, `go test ./internal/doccheck`, `go test ./internal/config -run '^TestExamplesLoadAndValidate$'`, and `git diff --check` all passed. Existing replay and cancellation regression tests remain intact; the HTTP adapter required no changes.