143 lines
8.0 KiB
Markdown
143 lines
8.0 KiB
Markdown
# Maintainability Audit: Findings and Recommended Changes
|
||
|
||
**Reviewed:** 2026-09-25, source commit `011b021`.
|
||
**Scope:** Recent partial-decision reconciliation, validation context, prompt
|
||
alignment, and request-local reference changes, including related existing code.
|
||
**Status:** R27–R30 open. Previous findings and resolution records remain in Git
|
||
history through `011b021`.
|
||
|
||
## Architecture Assessment
|
||
|
||
The recent changes fit Audita's existing boundaries. Proposal generation,
|
||
validation, prompt assets, and outcome reporting already have suitable owners;
|
||
this audit does not recommend a new common package or a broad refactor.
|
||
|
||
The strongest opportunities are shared prompt instructions and request-scoped
|
||
validation context preparation. Two smaller improvements concern recovery-reason
|
||
classification and partial-result assembly; both predate the latest commits.
|
||
These are bounded maintenance recommendations, not new release gates or changes
|
||
to correction policy, response acceptance, or reporting semantics.
|
||
|
||
## R27 — P2: Consolidate Shared Prompt Instructions
|
||
|
||
**Category:** Repeated behavioral-contract text; risk of future drift.
|
||
|
||
**Evidence:** The four [proposal prompts](../../internal/prompts/assets/prompts/modules.grammar.proposal.yaml)
|
||
repeat literal source-span matching, request-local identity, replacement-text,
|
||
source-based proposal, and confidence instructions. The three active
|
||
[validator prompts](../../internal/prompts/assets/prompts/validators.editorial_review.yaml)
|
||
repeat candidate-reference, supplemental-context, and verdict-confidence guidance.
|
||
Recent contract changes required coordinated edits across these assets. The
|
||
[prompt registry](../../internal/prompts/registry.go) already provides a shared
|
||
hardening fragment supplied through the role-bound adapter.
|
||
|
||
**Recommended change:** Extract genuinely common passages into embedded fragments
|
||
under `internal/prompts/assets/shared`, following the existing fragment-supply
|
||
approach. Keep the fragments owned by `internal/prompts` and use the existing
|
||
PromptKit rendering path. Keep module-specific examples, replacement-policy
|
||
instructions, protected-name exceptions, and semantic criteria in their individual
|
||
prompt definitions. Do not merge similar-looking rules with different meanings.
|
||
|
||
**Acceptance:** Common instructions have one maintained source and appear in all
|
||
intended rendered prompts. The existing module and validator distinctions remain
|
||
intact, including glossary replace-all behavior and the spoken-word protected-name
|
||
exception. Focused rendering checks cover fragment inclusion and representative
|
||
module-specific guidance without requiring exact-prose snapshots or introducing
|
||
a separate templating framework.
|
||
|
||
## R28 — P2: Prepare Validation Context Once Per Request
|
||
|
||
**Category:** Repeated computation and overlapping segment lookup.
|
||
|
||
**Evidence:** [BuildLLMValidationRequest](../../internal/framework/validators/llm_validators.go)
|
||
already indexes transcript segments, but
|
||
[validationContext](../../internal/framework/validators/llm_context.go) scans the
|
||
transcript again for each candidate. It also calls
|
||
[NewProtectedVocabulary](../../internal/framework/validators/protected_terms.go)
|
||
for each candidate, rebuilding the same vocabulary and compiling its matching
|
||
expression repeatedly. This is duplicated preparation rather than copied code.
|
||
|
||
**Recommended change:** Prepare a segment-position lookup and protected vocabulary
|
||
once per validation request, then pass that prepared context into the existing
|
||
context-selection helper. Keep the implementation within `framework/validators`.
|
||
Continue selecting glossary evidence for each candidate individually because its
|
||
proposed replacement can mention different terms. A process-wide cache or new
|
||
context-management package is unnecessary for this cleanup.
|
||
|
||
**Acceptance:** Candidates in one validation request reuse the prepared lookup and
|
||
vocabulary. Context still consists of the available immediate neighbors within the
|
||
current chunk and relevant glossary entries in glossary order. Candidate-specific
|
||
replacement text still participates in glossary selection. Existing optional-context
|
||
trimming, token estimation, and oversized-input handling retain their behavior.
|
||
Verify these outcomes with focused behavioral tests rather than assertions about
|
||
private helper call counts or unmeasured performance guarantees.
|
||
|
||
## R29 — P3: Share Execution-Recovery Classification
|
||
|
||
**Category:** Duplicate reporting rule; predates the latest commits.
|
||
|
||
**Evidence:** [Proposal generation](../../internal/framework/proposal_generation/generate.go)
|
||
and [LLM validation](../../internal/framework/validators/llm_validators.go)
|
||
independently translate structural-repair and transient-retry counts into
|
||
`output_repaired` and `service_recovered` reasons. The conversion is the same in
|
||
both locations; the surrounding success and domain-outcome decisions differ.
|
||
|
||
**Recommended change:** Place a small conversion helper in the existing
|
||
`framework/outcomes` package and use it from both callers. Keep the decision to
|
||
record recoveries at the call sites, along with domain-specific reasons such as
|
||
filtered validator decisions. Do not combine proposal and validation orchestration
|
||
or move domain-success classification into this helper.
|
||
|
||
**Acceptance:** Both callers use the same execution-count conversion. Existing
|
||
reason ordering, successful-operation reporting, and domain-specific additions
|
||
remain intact. A partially unavailable validation batch continues to record its
|
||
failure and execution counts without acquiring recovery records merely because a
|
||
repair or retry occurred. Existing outcome tests should continue to protect these
|
||
distinctions.
|
||
|
||
## R30 — P3: Centralize Partial Validator Result Assembly
|
||
|
||
**Category:** Repeated early-return construction; predates the latest commits.
|
||
|
||
**Evidence:** Several early returns in
|
||
[LLMBackedValidator.Validate](../../internal/framework/validators/llm_validators.go)
|
||
repeat nested slice appends to combine immediate decisions, oversized-input
|
||
decisions, and decisions from completed model batches, together with accumulated
|
||
operation outcomes. This makes it easier for a future failure path to omit facts
|
||
already collected by the validator.
|
||
|
||
**Recommended change:** Use a small local result-building helper to assemble the
|
||
partial result consistently. Keep error handling, batching, success-path ordering,
|
||
and cardinality enforcement explicit. A general-purpose result-aggregation package
|
||
is not needed.
|
||
|
||
**Acceptance:** Cancellation and failure paths retain the same previously collected
|
||
decisions and operations. Result ordering and cardinality behavior remain unchanged;
|
||
the helper does not enforce complete cardinality on interrupted work. Retain the
|
||
existing partial-progress and fatal-failure coverage, adding tests only for a
|
||
material gap exposed by the refactor.
|
||
|
||
## Boundaries to Preserve
|
||
|
||
- Keep proposal and validator reference mapping separate. Proposals can target the
|
||
same segment repeatedly; duplicate validator decisions make that candidate
|
||
unavailable. Similar indexing arithmetic alone does not justify a generic
|
||
reference-management abstraction.
|
||
- Keep distinct protected-term policy checks separate. They already share the
|
||
vocabulary implementation and intentionally apply different policies.
|
||
- Preserve exact source-span matching, request-local references, batching, and
|
||
partial-success behavior. Consolidation is not a reason to strengthen contracts
|
||
or broaden the accepted correction scope.
|
||
|
||
## Review Verification and Limits
|
||
|
||
- This is a source-level maintainability audit, not a finding of demonstrated
|
||
production failures or a measured performance assessment.
|
||
- No application tests were rerun for the read-only audit. This roadmap update
|
||
requires documentation link and whitespace checks only.
|
||
- Future remediation should run focused behavioral tests and the broader suite
|
||
appropriate to the actual change, following the testing policy. No live-provider
|
||
evaluation or new evaluation framework is required by these recommendations.
|
||
- This document records recommendations only; no implementation, prompt, or
|
||
release changes are included in this update.
|