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](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.