Added an implementation roadmap for the issues identified in the code audit
This commit is contained in:
329
docs/roadmap/implementation.md
Normal file
329
docs/roadmap/implementation.md
Normal file
@@ -0,0 +1,329 @@
|
||||
# Pre-1.0 Deduplication Implementation Plan
|
||||
|
||||
This plan turns `docs/roadmap/audit.md` into staged, prompt-sized cleanup work for an LLM coding agent. Each stage should be implemented in order and kept small enough to review as an independent commit.
|
||||
|
||||
## Operating rules
|
||||
|
||||
- Read `docs/roadmap/audit.md` before starting any stage.
|
||||
- Preserve public CLI, report, diagnostics, config precedence, prompt metadata, and output-schema behavior unless a stage explicitly calls out an intended behavior change.
|
||||
- Keep the four production module packages separate: `glossary`, `homophones`, `spoken_word`, and `grammar`.
|
||||
- Do not introduce plugin systems, generic workflow engines, broad CLI framework rewrites, reflection-heavy config mappers, or merged module packages.
|
||||
- Prefer narrow helpers, catalogs, constants, and pure mapping functions over broad abstractions.
|
||||
- Run the targeted tests listed in each stage before moving to the next stage.
|
||||
- Run `go test ./...` before declaring the full sequence complete.
|
||||
- Ignore unrelated worktree changes, including the existing deletion of `docs/roadmap/publish.md`, unless the user explicitly asks to handle them.
|
||||
- Do not reduce parity, release-fixture, subprocess, or module-specific behavior coverage while consolidating helpers.
|
||||
|
||||
## Stages
|
||||
|
||||
### Stage 1: Diagnostics artifact constants and metadata paths
|
||||
|
||||
Goal:
|
||||
|
||||
- Centralize diagnostics artifact names and report diagnostics metadata path construction without changing any filenames or report fields.
|
||||
|
||||
Key edits:
|
||||
|
||||
- Define constants in `internal/core/diagnostics` for:
|
||||
- `source-transcript.json`
|
||||
- `source-transcript-parsed.json`
|
||||
- `normalized-transcript.json`
|
||||
- `normalization-summary.json`
|
||||
- `chunking-summary.json`
|
||||
- `utilization-diagnostics.json`
|
||||
- `correction-ledger.json`
|
||||
- `invocation.json`
|
||||
- `effective-config.json`
|
||||
- `report.json`
|
||||
- `error.log`
|
||||
- Add a diagnostics helper that builds `reporting.DiagnosticsMetadata` from a run directory path and failure/success status.
|
||||
- Update `RunDirectory` methods to use the constants.
|
||||
- Update CLI report assembly and utilization/correction-ledger writes to use the constants/helper instead of raw strings.
|
||||
|
||||
Behavior changes:
|
||||
|
||||
- None. All artifact names, report JSON keys, and path values must remain byte-for-byte compatible except for normal timestamp/order differences in existing outputs.
|
||||
|
||||
Tests:
|
||||
|
||||
- Add or update `internal/core/diagnostics` tests proving metadata helper paths match the artifact constants.
|
||||
- Run `go test ./internal/core/diagnostics ./internal/core/reporting ./internal/cli`.
|
||||
- Run any existing CLI report/diagnostics tests touched by this stage.
|
||||
|
||||
Acceptance criteria:
|
||||
|
||||
- No raw core diagnostics artifact filename strings remain in CLI report metadata assembly.
|
||||
- Existing success and failure reports still point to files that are actually written.
|
||||
- Retention behavior is unchanged.
|
||||
|
||||
### Stage 2: Output schema validation and module catalog
|
||||
|
||||
Goal:
|
||||
|
||||
- Move public key validation to small canonical catalogs so config validation, runtime resolution, and factory behavior cannot drift.
|
||||
|
||||
Key edits:
|
||||
|
||||
- Add `SupportedKeys`, `IsSupported`, or an equivalent validation helper to `internal/core/outputschema`.
|
||||
- Update `config.Validate` to use `internal/core/outputschema` for output schema validation.
|
||||
- Add a small canonical module key catalog that is importable by:
|
||||
- `internal/core/config`
|
||||
- `internal/framework/modules`
|
||||
- `internal/validators`
|
||||
- `internal/framework/validators`
|
||||
- Use the module catalog for default module key constants, known-key checks, validator chain keys, and confidence-threshold lookup.
|
||||
- Keep module construction in `internal/framework/modules`; the catalog must not construct modules.
|
||||
|
||||
Behavior changes:
|
||||
|
||||
- Intended behavior change: unsupported configured module keys should fail during config validation, including `audita config validate`.
|
||||
- Repeated supported module keys remain valid.
|
||||
- Output schema behavior remains unchanged for `bare-segments`, `audita-v1`, and unsupported names.
|
||||
|
||||
Tests:
|
||||
|
||||
- Add `internal/core/config` tests for unsupported module keys and repeated supported module keys.
|
||||
- Add config validation tests that every supported output schema validates.
|
||||
- Add or update output schema registry tests for supported and unsupported schemas.
|
||||
- Update module registry and validator chain tests to use the shared catalog where appropriate.
|
||||
- Run `go test ./internal/core/config ./internal/core/outputschema ./internal/framework/modules ./internal/framework/validators ./internal/validators/... ./internal/cli`.
|
||||
|
||||
Acceptance criteria:
|
||||
|
||||
- Unknown modules fail before runner setup in config validation paths.
|
||||
- No duplicated hardcoded output schema support list remains in config validation.
|
||||
- No import cycle is introduced.
|
||||
|
||||
### Stage 3: Effective config loading context
|
||||
|
||||
Goal:
|
||||
|
||||
- Centralize config path resolution and defaults+file+env loading while keeping command-specific CLI overrides explicit.
|
||||
|
||||
Key edits:
|
||||
|
||||
- Move config path resolution from `internal/cli` into `internal/core/config` or add an equivalent exported helper there.
|
||||
- Add an effective config loader that returns:
|
||||
- effective `config.Config`
|
||||
- config path
|
||||
- config source (`flag`, `env`, `default`, or empty)
|
||||
- config version pointer when a file was loaded
|
||||
- Use the shared loader in `audita process` before applying CLI overrides.
|
||||
- Use the shared loader in `audita config print-effective`.
|
||||
- Keep `audita config validate` as file-only: load file, apply to defaults, validate, and do not apply environment overrides.
|
||||
|
||||
Behavior changes:
|
||||
|
||||
- None. Preserve existing precedence:
|
||||
- `process`: defaults, file config, environment, CLI flags
|
||||
- `config print-effective`: defaults, file config, environment
|
||||
- `config validate`: file config applied to defaults only
|
||||
- Preserve explicit config path failure behavior and missing default path non-fatal behavior.
|
||||
|
||||
Tests:
|
||||
|
||||
- Add table-driven config loader tests for:
|
||||
- explicit `--config`
|
||||
- `AUDITA_CONFIG`
|
||||
- default search paths
|
||||
- missing explicit path
|
||||
- missing env path
|
||||
- missing default paths
|
||||
- Add or update CLI tests proving `process` and `config print-effective` share file+env behavior.
|
||||
- Add or update CLI tests proving `config validate` ignores environment overrides.
|
||||
- Run `go test ./internal/core/config ./internal/cli ./cmd/audita`.
|
||||
|
||||
Acceptance criteria:
|
||||
|
||||
- Config precedence is unchanged.
|
||||
- Config source/path/version metadata in invocation and reports is unchanged.
|
||||
- Config command stdout/stderr and exit-code behavior is unchanged except for the intended unknown-module validation from Stage 2.
|
||||
|
||||
### Stage 4: Prompt/schema metadata and stage-name helpers
|
||||
|
||||
Goal:
|
||||
|
||||
- Centralize diagnostics-visible metadata and stage-name construction without changing production diagnostics names.
|
||||
|
||||
Key edits:
|
||||
|
||||
- Add a helper or method in `internal/prompts` that returns the stable prompt metadata diagnostics shape currently expanded by call sites.
|
||||
- Add a helper or method in `internal/framework/responseschema` that returns the stable response schema metadata diagnostics shape currently expanded by call sites.
|
||||
- Add shared proposal and validator stage-name helpers in the lowest package that avoids import cycles.
|
||||
- Use the helpers in proposal generation, LLM validators, and production modules.
|
||||
|
||||
Behavior changes:
|
||||
|
||||
- None. Preserve current production stage names:
|
||||
- module proposal stages keep their existing `proposal` naming form;
|
||||
- validator batch stages keep their existing validator/batch naming form.
|
||||
- Preserve all prompt metadata and response schema metadata field names and values.
|
||||
|
||||
Tests:
|
||||
|
||||
- Add prompt metadata helper tests covering every registered prompt.
|
||||
- Add response schema metadata helper tests covering every registered response schema.
|
||||
- Add stage-name helper tests for no-section, section, and validator batch cases.
|
||||
- Run `go test ./internal/prompts ./internal/framework/responseschema ./internal/framework/proposal_generation ./internal/framework/validators ./internal/modules/...`.
|
||||
|
||||
Acceptance criteria:
|
||||
|
||||
- No manual prompt metadata map expansion remains in production module proposal plumbing.
|
||||
- No duplicated response schema metadata map construction remains in proposal generation and LLM validators.
|
||||
- Existing diagnostics fixture/path assertions still pass.
|
||||
|
||||
### Stage 5: Shared module proposal and prompt payload plumbing
|
||||
|
||||
Goal:
|
||||
|
||||
- Remove duplicated proposal execution and transcript-section prompt payload construction while preserving module-specific domain behavior.
|
||||
|
||||
Key edits:
|
||||
|
||||
- Add a narrow shared proposal execution helper, preferably in `internal/framework/proposal_generation` unless import cycles require a small module helper package.
|
||||
- The helper should own:
|
||||
- transcript description extraction from config;
|
||||
- `GenerateCandidates` request construction;
|
||||
- prompt metadata attachment;
|
||||
- stage-name selection;
|
||||
- conversion from generated corrections/warnings to `contracts.ProposalResult`.
|
||||
- Add shared transcript-section prompt payload construction in `internal/framework/promptcontext`.
|
||||
- Update each production module to provide only:
|
||||
- module key;
|
||||
- replacement policy;
|
||||
- validator chain;
|
||||
- prompt ID;
|
||||
- domain-specific `BuildProposalMessages` call or message builder.
|
||||
- Remove each module's redundant section transcript filtering if the runner already passes section-limited transcripts.
|
||||
|
||||
Behavior changes:
|
||||
|
||||
- None. Preserve module keys, replacement policies, validator chains, prompt IDs, diagnostics directories, proposal indexes, warning behavior, and correction mapping.
|
||||
|
||||
Tests:
|
||||
|
||||
- Add promptcontext tests for transcript section payload shape, empty transcript handling, section index, and category copying.
|
||||
- Keep one module-specific prompt test per production module for domain wording and constraints.
|
||||
- Add or update module proposal tests proving diagnostics are still written under the same module instance directory.
|
||||
- Run `go test ./internal/framework/promptcontext ./internal/framework/proposal_generation ./internal/modules/... ./internal/cli`.
|
||||
|
||||
Acceptance criteria:
|
||||
|
||||
- Four production modules share proposal execution plumbing.
|
||||
- Module packages remain separate and readable.
|
||||
- CLI parity and release fixture behavior is unchanged.
|
||||
|
||||
### Stage 6: Validator classification and malformed LLM output policy
|
||||
|
||||
Goal:
|
||||
|
||||
- Use one source of truth for validator execution class and one shared classifier for malformed structured-output errors.
|
||||
|
||||
Key edits:
|
||||
|
||||
- Make validator execution class resolvable by stable validator key and by validator instance.
|
||||
- Replace the correction-ledger hardcoded LLM-backed validator map with the canonical metadata source.
|
||||
- Remove redundant validator metadata fields only after all call sites use the canonical source.
|
||||
- Add a shared malformed structured-output classifier in `internal/framework/llm` or another low-level framework package.
|
||||
- Update proposal generation and LLM validators to use the shared classifier while preserving their different handling outcomes.
|
||||
|
||||
Behavior changes:
|
||||
|
||||
- None. Proposal-generation malformed payloads still downgrade to warnings with zero proposals for affected sections.
|
||||
- Validator malformed payloads still reject affected batches with warnings.
|
||||
- Correction-ledger deterministic vs LLM validator sections should be unchanged for current validators.
|
||||
|
||||
Tests:
|
||||
|
||||
- Add validator metadata tests proving every registered validator has the expected execution class by key and instance.
|
||||
- Add correction-ledger tests proving deterministic and LLM-backed decisions are classified through canonical metadata.
|
||||
- Add shared malformed-output classifier tests covering current adapter malformed-output messages.
|
||||
- Update proposal-generation and validator tests to assert representative malformed adapter errors are still downgraded.
|
||||
- Run `go test ./internal/validators/... ./internal/framework/validators ./internal/framework/proposal_generation ./internal/framework/llm ./internal/cli`.
|
||||
|
||||
Acceptance criteria:
|
||||
|
||||
- No local hardcoded LLM-backed validator map remains in correction-ledger construction.
|
||||
- Proposal-generation and validator malformed-output classifier lists cannot drift.
|
||||
- Existing runner validator ordering is unchanged.
|
||||
|
||||
### Stage 7: Redaction and adapter workflow cleanup
|
||||
|
||||
Goal:
|
||||
|
||||
- Reduce duplicated secret extraction/redaction setup while preserving all no-secret-leak guarantees.
|
||||
|
||||
Key edits:
|
||||
|
||||
- Add a shared helper that extracts all configured LLM secret values from `config.Config`.
|
||||
- Use the helper in proposal-generation diagnostics and validator diagnostics setup.
|
||||
- Keep config structural redaction (`Config.Redacted`) separate from byte/string payload redaction.
|
||||
- Keep adapter error redaction behavior compatible with current surfaced errors.
|
||||
- Move runner adapter shims only if Stage 6 or this stage makes them materially larger; otherwise leave them in runner.
|
||||
|
||||
Behavior changes:
|
||||
|
||||
- None. Redaction token and no-secret-leak behavior remain unchanged.
|
||||
|
||||
Tests:
|
||||
|
||||
- Add or update tests proving proposal diagnostics, validator diagnostics, effective config artifacts, and surfaced adapter errors redact the same configured secrets.
|
||||
- Keep existing subprocess no-secret-leak tests.
|
||||
- Run `go test ./internal/core/config ./internal/framework/llm ./internal/framework/proposal_generation ./internal/framework/validators ./internal/cli ./cmd/audita`.
|
||||
|
||||
Acceptance criteria:
|
||||
|
||||
- Secret-list assembly is no longer duplicated between proposal and validator paths.
|
||||
- No plaintext configured API key appears in diagnostics, reports, stdout, or stderr in existing redaction tests.
|
||||
- No unrelated adapter behavior changes.
|
||||
|
||||
### Stage 8: Test helper cleanup and dead-code sweep
|
||||
|
||||
Goal:
|
||||
|
||||
- Consolidate test-only duplication and remove dead/redundant code left by prior stages.
|
||||
|
||||
Key edits:
|
||||
|
||||
- Consolidate package-local fake LLM clients, fixture readers, diagnostics glob helpers, and run-directory helpers where duplication is clear.
|
||||
- Use cross-package test support only if it does not obscure test intent or introduce awkward imports.
|
||||
- Remove redundant metadata fields, constants, or helper functions made obsolete by earlier stages.
|
||||
- Keep module-specific prompt and behavior assertions local to each module package.
|
||||
|
||||
Behavior changes:
|
||||
|
||||
- None.
|
||||
|
||||
Tests:
|
||||
|
||||
- Run all package tests touched by helper cleanup.
|
||||
- Run `go test ./internal/modules/... ./internal/framework/... ./internal/cli ./cmd/audita`.
|
||||
- Run `go test ./...` before completing the full sequence.
|
||||
|
||||
Acceptance criteria:
|
||||
|
||||
- Test helpers are simpler without reducing coverage.
|
||||
- No parity or release fixture assertions are removed unless replaced by equivalent or stronger assertions.
|
||||
- No production behavior changes.
|
||||
|
||||
## Final verification
|
||||
|
||||
Before declaring the staged cleanup complete:
|
||||
|
||||
- Run:
|
||||
- `go test ./internal/core/config ./internal/core/outputschema`
|
||||
- `go test ./internal/core/diagnostics ./internal/core/reporting`
|
||||
- `go test ./internal/framework/proposal_generation ./internal/framework/validators ./internal/framework/runner`
|
||||
- `go test ./internal/validators/...`
|
||||
- `go test ./internal/modules/...`
|
||||
- `go test ./internal/cli ./cmd/audita`
|
||||
- `go test ./...`
|
||||
- Inspect `git diff` for accidental public CLI, config, report, diagnostics, prompt metadata, stage-name, or output-schema changes.
|
||||
- Update docs only when behavior intentionally changes, especially the intended Stage 2 unknown-module validation change.
|
||||
- Keep commits stage-sized and mention behavior-preservation tests in each commit message or PR description.
|
||||
|
||||
## Assumptions
|
||||
|
||||
- Unknown configured module keys should become config-validation failures before 1.0.
|
||||
- Diagnostics filenames and stage names are public enough to preserve unless a stage explicitly says otherwise.
|
||||
- Each stage should be implemented and reviewed separately.
|
||||
@@ -1,72 +0,0 @@
|
||||
# Hard-Cutover Roadmap for Module-Stage LLM Resilience
|
||||
|
||||
## Summary
|
||||
|
||||
This roadmap captures the module-stage resilience work for Audita:
|
||||
|
||||
- fail fast on initialization, configuration, schema, and other pre-module setup errors;
|
||||
- remain resilient during module execution when LLM payloads are malformed or individual proposed corrections are invalid;
|
||||
- reject bad corrections through validator/reporting paths instead of aborting the module or process;
|
||||
- keep success stderr quiet; surface warnings only through report and diagnostics artifacts;
|
||||
- use a hard cutover only, with no compatibility aliases or transitional code.
|
||||
|
||||
Locked decisions:
|
||||
|
||||
- keep the stable public validator key `non_empty_corrected_text`;
|
||||
- change that validator’s behavior to mean “the resulting segment text must not be empty/whitespace-only after applying the proposal preview”;
|
||||
- malformed LLM-validator batch payloads reject the entire affected batch under that validator and continue;
|
||||
- proposal/validator transport failures, timeouts, and provider/runtime call failures remain fatal;
|
||||
- malformed structured payloads are downgraded; non-malformed runtime call failures are not.
|
||||
|
||||
## Stage 1: Proposal Intake Hardening
|
||||
|
||||
- Stop treating invalid individual structured corrections as fatal during proposal generation.
|
||||
- Preserve returned correction ordering and proposal-index assignment even when individual corrections are malformed.
|
||||
- Allow `corrected_text == ""` when the resulting segment remains non-empty after previewed application.
|
||||
- Downgrade malformed proposal-generation structured payloads into section-scoped warnings with zero proposals for that section.
|
||||
- Keep proposal-generation transport/provider/runtime call failures fatal.
|
||||
|
||||
Deterministic validation changes:
|
||||
|
||||
- Add `proposal_shape` as a built-in deterministic validator and run it first in every built-in module chain.
|
||||
- Reject malformed proposal fields with stable reason codes:
|
||||
- `invalid_target_segment_id`
|
||||
- `empty_original_text`
|
||||
- `invalid_confidence`
|
||||
- Keep `non_empty_corrected_text` as the stable validator key, but change its semantics to reject only `empty_resulting_segment`.
|
||||
- Keep validator rejection and apply-time skip as distinct outcomes.
|
||||
|
||||
## Stage 2: LLM Validator Resilience
|
||||
|
||||
- Keep validator transport/provider/runtime call failures fatal.
|
||||
- Downgrade malformed validator structured payloads into batch-scoped validator rejections plus module warnings.
|
||||
- Downgrade oversized single-proposal validator inputs into per-proposal validator rejections plus module warnings.
|
||||
- Preserve decision-cardinality enforcement as an internal invariant after malformed-payload degradation has synthesized complete decision sets.
|
||||
|
||||
Stable reason codes introduced or relied upon by this cutover:
|
||||
|
||||
- `empty_resulting_segment`
|
||||
- `invalid_target_segment_id`
|
||||
- `empty_original_text`
|
||||
- `invalid_confidence`
|
||||
- `validator_response_malformed`
|
||||
- `validator_input_too_large`
|
||||
- `proposal_response_malformed`
|
||||
|
||||
## Stage 3: Reporting, Diagnostics, and Docs
|
||||
|
||||
- Add module warning records to runner results and process reports.
|
||||
- Keep correction-ledger entries per-correction only; do not add standalone warning rows.
|
||||
- Record malformed proposal-generation and validator-batch warnings through report and diagnostics artifacts only.
|
||||
- Keep successful runs quiet on stderr even when warnings are present.
|
||||
- Update README and architecture/public-contract/diagnostics/validators/release-checklist docs to reflect the new behavior.
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
- Empty `corrected_text` may delete words, but proposals that would blank the whole segment are rejected or skipped safely.
|
||||
- Invalid proposal shape is rejected by validators, not by proposal generation.
|
||||
- Malformed proposal-generation payloads succeed with warnings and zero proposals for the affected section.
|
||||
- Malformed validator payloads reject only the affected validator batch and do not fail the module.
|
||||
- Oversized single validator inputs reject only the affected proposal.
|
||||
- Transport/provider/runtime LLM failures still fail the module and process.
|
||||
- Successful runs with warnings still exit `0`, emit transcript output normally, keep stderr empty, and expose warnings in report/diagnostics artifacts.
|
||||
Reference in New Issue
Block a user