Record correction repair review findings
This commit is contained in:
138
docs/roadmap/remediation.md
Normal file
138
docs/roadmap/remediation.md
Normal file
@@ -0,0 +1,138 @@
|
||||
# Correction Repair Review Remediation
|
||||
|
||||
**Updated:** 2026-11-11
|
||||
|
||||
**Status:** Two open findings from the stage-by-stage review of the
|
||||
[implementation plan](implementation.md), against committed revision `aa8073c`.
|
||||
This document records required follow-up work; it does not implement the fixes.
|
||||
|
||||
## Review Result
|
||||
|
||||
The intended processing architecture is present: module-owned repair prompts,
|
||||
validator-owned findings, full original exchanges with independent proposal
|
||||
branches, ConfigKit-resolved budgets and profiles, and PromptKit-owned rendering,
|
||||
execution, retries, structural repair, and backend admission. Revisions undergo
|
||||
fresh validation and enter deterministic application under the existing conflict
|
||||
checks and stage barrier. Ordinary reports summarize branches; debug artifacts
|
||||
retain detailed evidence.
|
||||
|
||||
The review found two gaps in outcome attribution and retained diagnostic evidence.
|
||||
Neither reproduced case applies an invalid correction or changes the transcript
|
||||
incorrectly. Both should be addressed before treating reporting and diagnostics
|
||||
as complete.
|
||||
|
||||
No additional strengthening of invariants, contracts, prohibitions, or guarantees
|
||||
was identified in the reviewed implementation. In particular, the new semantic
|
||||
budget has no additional maximum, zero disables extra feedback and repair,
|
||||
revisions may change their target within the original section, empty replacements
|
||||
and whitespace spans remain supported, and provider request limits are not
|
||||
replaced by a new local context-size restriction.
|
||||
|
||||
## Findings
|
||||
|
||||
### R1 — Preserve Defect Attribution for Revised Proposals (P2)
|
||||
|
||||
**Affected plan:** Stage 6, item 5; Stage 7, items 1–2.
|
||||
|
||||
**Evidence:** In [repairCandidates](../../internal/framework/runner/repair.go),
|
||||
lines 178–191 retain revalidation decisions, rejections, and operations, but drop
|
||||
`validated.filtered`. `RepairAttempt` has no corresponding defect data, and
|
||||
`repairOperations` (lines 230–249) consequently emits no defect attribution for
|
||||
that revision. `mergeFilteredProposals` in
|
||||
[runner.go](../../internal/framework/runner/runner.go) attributes original
|
||||
proposal defects only; it cannot recover the discarded revision evidence.
|
||||
|
||||
**Reproduced behavior:** Begin with one segment, `cat`, and an initially
|
||||
low-confidence proposal to replace it with `Cat`. Supply a structurally valid
|
||||
repair with `proposal_ref: 1`, `action: revise`, and correction
|
||||
`{id: 99, original_text: cat, corrected_text: Cat, confidence: 0.99}`.
|
||||
Revalidation correctly rejects the out-of-section target with
|
||||
`invalid_target_segment_id`, and the branch exhausts its one-attempt budget.
|
||||
However, its sole `correction_repair` operation is recorded as successful with
|
||||
no failure reason, filtered defect, or recovery reason. The defect is visible
|
||||
only in retained validation/debug evidence; ordinary operation reporting cannot
|
||||
attribute it to the repair response. This also loses the defect's operation
|
||||
attribution when a later attempt succeeds.
|
||||
|
||||
**Required change:** Carry the defective revision's evidence into its originating
|
||||
repair attempt and operation, following the plan's distinction between original
|
||||
proposal filtering and defects introduced by a repair response. Preserve the
|
||||
existing treatment of ordinary editorial rejection, low confidence, withdrawal,
|
||||
and application conflicts; those are not automatically operational errors.
|
||||
Keep original proposal-response counts separate and avoid attributing the later
|
||||
defect to the original response or counting it twice.
|
||||
|
||||
**Acceptance:** Add a regression covering a schema-valid revision with an invalid
|
||||
target or missing source span, both at budget exhaustion and before a later
|
||||
successful repair. Verify that the specific defect remains attributable to its
|
||||
repair operation in ordinary reporting, while proposal-response totals, semantic
|
||||
attempt counts, and the final branch disposition remain correct. Include an
|
||||
ordinary rejected revision to protect the existing distinction from response
|
||||
defects.
|
||||
|
||||
### R2 — Retain Application Skip Reasons in Repaired Ledger Entries (P2)
|
||||
|
||||
**Affected plan:** Stage 6, item 5; Stage 8, item 3.
|
||||
|
||||
**Evidence:** [BuildCorrectionLedger](../../internal/framework/processreport/correction_ledger.go)
|
||||
skips every repaired proposal in the `module.SkippedChanges` loop (lines 131–134).
|
||||
Its replacement repair-row construction (lines 194–209) copies the branch
|
||||
disposition and attempt history, but never joins the actual skipped application
|
||||
result. `DispositionReasonCode` and `DispositionMessage` are therefore empty for
|
||||
`approved_skipped` repair rows, although they are retained for unrepaired skipped
|
||||
proposals.
|
||||
|
||||
**Reproduced behavior:** An earlier accepted proposal changes `cat` to `dog`.
|
||||
A later repair validates a revision of that same original span to `fox`, then
|
||||
correctly skips at application because the earlier edit removed its source span.
|
||||
The runner retains the actual `SkippedChange`, but the repaired proposal's debug
|
||||
ledger row retains only `approved_skipped` and its pre-application decisions.
|
||||
A focused ledger probe confirms that a supplied skip reason and message disappear
|
||||
from the serialized entry. Repair interaction artifacts are written before
|
||||
application, so they cannot supply the missing application result.
|
||||
|
||||
**Required change:** Join the final skipped change to the repaired ledger entry
|
||||
using the existing stage-instance/proposal identity, and populate the existing
|
||||
disposition reason and message fields. Preserve one final row per repaired
|
||||
proposal, its original proposal data, and its revision history. The compact
|
||||
ordinary report can retain its existing fields.
|
||||
|
||||
**Acceptance:** Extend the existing overlapping-edit runner/ledger coverage so a
|
||||
validated but skipped revision retains the actual application reason and message
|
||||
in its single final debug row. Verify that it remains `approved_skipped`, has no
|
||||
applied-edit fields, and does not create a duplicate ledger row or an additional
|
||||
operational error.
|
||||
|
||||
## Stage Review Coverage
|
||||
|
||||
| Stage | Evidence reviewed | Result |
|
||||
| --- | --- | --- |
|
||||
| 1 — Response and native composition | Repair schema, correspondence decoder, native multi-turn wire/schema test with a separate repair profile | No additional finding |
|
||||
| 2 — Module prompts | Four registered assets, shared contract, rendering tests, operator replacement and message-file loading | No additional finding |
|
||||
| 3 — Exchange and adapter | Capture through proposal contracts, frozen variables, prefix comparison, independent suffixes, failure classification, native structural-repair accounting | No additional finding |
|
||||
| 4 — Configuration and readiness | Root/stage resolution, null fallback, zero and positive budgets, recorded resolution, disabled-profile readiness, CLI role wiring | No additional finding |
|
||||
| 5 — Findings | Applicability selection, independent deterministic/semantic findings, unavailable assessments, no-effect handling, zero-budget shortlist, cardinality and protection checks | No additional finding |
|
||||
| 6 — Branch execution and application | Per-proposal budget, retargeting, deletion, withdrawal, exhaustion, cancellation, full revalidation, deterministic application and stage commit | R1; application evidence feeds R2 |
|
||||
| 7 — Ordinary outcomes | Operation ordering, logical/native counts, compact branch rows, report-disabled summaries, error/recovery mapping | R1 |
|
||||
| 8 — Debug and ledger | Artifact coordinates, redaction, optional-write failures, original/revised identities, interrupted evidence, application-result join | R2 |
|
||||
| 9 — Assembled command and docs | Controlled-provider subprocess flows, prompt override, cancellation, output selections, canonical documentation and preview example | Existing checks pass; add regressions for R1 and R2 |
|
||||
|
||||
## Validation Performed
|
||||
|
||||
- `go test ./...` passed.
|
||||
- `go test -race ./internal/framework/runner ./internal/framework/llm ./cmd/audita`
|
||||
passed.
|
||||
- `go build -o /tmp/audita-review ./cmd/audita` passed.
|
||||
- The repair-preview command in [the example guide](../../examples/README.md)
|
||||
successfully rendered the grammar repair instruction and schema without a
|
||||
provider call. Maintained-example integration tests also passed in the full
|
||||
suite.
|
||||
- All 67 local links and anchors in the 13 documentation files changed by the
|
||||
implementation resolved.
|
||||
- Two temporary Go-overlay probes reproduced R1 and R2 against the committed
|
||||
code. Their assertions failed as described above. They were not added to the
|
||||
repository; the existing suite does not detect these gaps.
|
||||
|
||||
Resolve both findings with focused regressions, then repeat the affected report,
|
||||
ledger, runner, and assembled-command checks. Retain the current architecture and
|
||||
accepted operator choices while closing these evidence gaps.
|
||||
Reference in New Issue
Block a user