narratio/docs/roadmap/remediation.md

Audita v2 and ConfigKit Migration Review Remediation

Status: Complete.

Reviewed 83eaa96 through fae18bf against the approved clean-cutover decisions, feature roadmap, and implementation plan. 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.