196 lines
12 KiB
Markdown
196 lines
12 KiB
Markdown
# Correction Repair Review Remediation
|
||
|
||
**Updated:** 2026-11-11
|
||
|
||
**Status:** Four open findings from the stage-by-stage review of the
|
||
[implementation plan](implementation.md), against implementation revision
|
||
`aa8073c`. R1 and R2 were confirmed in the follow-up audit, which added R3 and
|
||
R4.
|
||
|
||
## 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 gaps in outcome attribution, retained diagnostic evidence,
|
||
and replay of configured cache settings. The default validation path also
|
||
repeats replacement previews across validators. The reproduced cases preserve
|
||
correction acceptance and application.
|
||
|
||
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.
|
||
|
||
### R3 — Preserve Cache Settings When Replaying Repair Turns (P3)
|
||
|
||
**Affected plan:** Stage 3, items 3–5.
|
||
|
||
**Evidence:** [The adapter](../../internal/framework/llm/adapter.go#L117) appends
|
||
the current repair instruction as native PromptKit messages. It retains that
|
||
instruction for the next attempt through `convertMessages`, which copies only
|
||
role and content. On the next attempt the adapter reconstructs those prior
|
||
turns without their `CacheControl`. Operator prompt replacements already
|
||
support PromptKit's per-message `cache_control` field.
|
||
|
||
**Reproduced behavior:** Override the grammar repair prompt with a user message
|
||
using `cache_control: {type: ephemeral, ttl: 1h}`, then execute two repair
|
||
turns on the same branch. The first call sends the instruction as a cached text
|
||
block. The second call sends the same earlier instruction as a plain string,
|
||
while the new instruction still carries the cache settings. The transport
|
||
confirmed this difference with a local fixture.
|
||
|
||
**Required change:** Preserve native cache metadata for rendered instructions
|
||
carried into later turns. Keep its conversion in the adapter boundary and leave
|
||
wire encoding to PromptKit.
|
||
|
||
**Acceptance:** Extend the two-turn adapter test with a cache-controlled
|
||
operator repair prompt. Verify that the previous instruction retains its
|
||
cache settings, text, and role when replayed. The test should check the
|
||
outbound request without depending on provider caching behavior.
|
||
|
||
### R4 — Reuse Replacement Previews Across Feedback Checks (P3)
|
||
|
||
**Affected plan:** Stage 5, items 1–2.
|
||
|
||
**Evidence:** [feedbackCandidates](../../internal/framework/runner/feedback.go#L30)
|
||
calls `feedbackPreview` for every candidate at every validator, including
|
||
shape and confidence checks that never use it. Each call scans the section for
|
||
the target, then builds the replacement preview. The
|
||
[runner](../../internal/framework/runner/runner.go#L744) previews the candidate
|
||
again for ambiguity, and [validator request construction](../../internal/framework/validators/llm_validators.go#L269)
|
||
rebuilds the preview for the LLM payload. The default positive repair budget
|
||
adds this work even when all proposals pass the initial chain.
|
||
|
||
**Measured behavior:** An offline benchmark with 100 segments and 100 valid
|
||
proposals exercised the same five deterministic validators at budgets zero
|
||
and one, with identical approvals and no repairs. On the review machine,
|
||
validation rose from about 0.28 to 0.50 ms and allocated bytes from about 0.40 to
|
||
0.68 MB per section. With 1,000 segments and proposals, the same probe rose from
|
||
about 4.2 to 8.0 ms and from 5.4 to 7.8 MB. These measurements isolate local
|
||
validation work.
|
||
|
||
**Required change:** Reuse the existing replacement preview across applicability,
|
||
ambiguity, and validation payload construction for a given candidate version
|
||
where the rules use the same before/after text. Reuse a section target index and
|
||
compute previews only for checks that need them. Recompute the preview for a
|
||
revision, and preserve the separate application check against the current text.
|
||
|
||
**Acceptance:** Keep the existing independent-feedback and zero-budget coverage
|
||
passing, including ambiguous spans, blank results, invalid confidence, and
|
||
protection exceptions. Re-run a focused offline benchmark to verify reduced
|
||
scanning and allocation in the feedback path.
|
||
|
||
## 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, cache control, failure classification, native structural-repair accounting | R3 |
|
||
| 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, preview reuse, independent deterministic/semantic findings, unavailable assessments, no-effect handling, zero-budget shortlist, cardinality and protection checks | R4 |
|
||
| 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 |
|
||
|
||
## Validation Performed
|
||
|
||
- The fresh `go test -count=1 ./...` run passed.
|
||
- Fresh race runs passed for the runner, LLM adapter, process reporting, and CLI.
|
||
- The CLI build 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.
|
||
- The implementation documentation passed its link review. The follow-up
|
||
checked the links in this roadmap and ran `git diff --check`.
|
||
- Temporary Go overlays reproduced the defects in R1–R3 and measured the
|
||
validation overhead in R4 against the committed implementation.
|
||
|
||
Resolve the findings with focused checks at their owning boundaries. Retain the
|
||
current architecture and accepted operator choices.
|