21 KiB
Correction Repair Review Remediation
Updated: 2026-11-11
Status: All nine findings resolved. Findings refer to implementation revision
aa8073c. See the implementation plan.
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, replay of configured cache settings, and protected-alias feedback. The algorithmic audit found repeated section scans, quadratic proposal bookkeeping, and debug-only history retained during ordinary runs. These are local implementation costs. The plan deliberately calls for full conversation replay and fresh validation of each revision.
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)
Completed. Revision defects now appear on the originating repair recovery. Subprocess tests cover exhaustion, later approval, and ordinary rejection.
Affected plan: Stage 6, item 5; Stage 7, items 1–2.
Evidence: In repairCandidates,
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 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)
Completed. The debug ledger joins the application skip reason and message by stage and proposal. Ledger and overlap tests pass.
Affected plan: Stage 6, item 5; Stage 8, item 3.
Evidence: BuildCorrectionLedger
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)
Completed. Replay retains each rendered instruction cache setting. The two-turn adapter test verifies the outbound role, text, and cache metadata.
Affected plan: Stage 3, items 3–5.
Evidence: The adapter 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)
Completed. Each source section shares one target index across validation passes. Preview-dependent checks reuse one lazy preview per version. Targeted tests passed, and the offline benchmark showed lower allocation counts at both sizes.
Affected plan: Stage 5, items 1–2.
Evidence: feedbackCandidates
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 previews the candidate
again for ambiguity, and validator request construction
rebuilds the preview for the LLM payload. The default positive repair budget
adds this work even when all proposals pass the initial chain.
Complexity: For a section with S segments and M candidate versions
(initial proposals and revisions), the feedback selector alone can perform
O(V × M × S) target lookup work across V validators. Each singleton
revalidation also rebuilds section-wide indexes inside presence, nonempty-result,
and semantic validators. With a shared section index and one preview per
version that needs it, lookup and candidate traversal take expected
O(S + V × M) time, plus the actual text and payload work. That removes the
quadratic lookup term when proposals grow with section size.
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. Pass the same read-only section lookup into singleton revalidation so validators can use the original target and its neighbors without rebuilding their indexes.
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.
R5 — Index Initial Findings and Compact Filtering Once (P3)
Completed. Initial findings are grouped by proposal in validator order. Superseded filtering records are removed once, including on partial returns. Runner and report tests pass.
Affected plan: Stage 6, items 1 and 5.
Evidence: repairCandidates
calls actionable with the full initial rejection list for every proposal.
Each eligible proposal then calls repairFindings over the same full list.
Every successful revision also walks and compacts the original filtered-defect
slice. Findings extraction
already appends in validator order.
Complexity: With P original proposals and D initial rejection
records, branch selection and initial feedback can take Θ(P × D) time.
If a validation outage leaves one unavailable decision per proposal, this
already takes quadratic time before the runner determines no repairs can start.
With Q approved repairs and F original filtering records, repeated
compaction can add Θ(Q × F) work in the worst case. Both products can be
quadratic in proposal count even with a fixed validator chain.
Refactoring: Group initial rejections by proposal index in a single pass.
Walk the proposals in their existing order and read only each proposal's
findings. Record the approved replacements in a set and filter the original
defect list once when returning the section result, including partial results
on error. The combined bookkeeping then takes expected O(P + D + F) time
apart from the individual repair calls and revalidations.
Acceptance: Preserve validator order within findings, proposal order across branches, no-effect and unavailable-only handling, and original filtering evidence for candidates that remain excluded.
R6 — Index Application Results for Repair Outcomes (P3)
Completed. Application outcomes use a stage-local index built once. The runner test covers an original approval, a skipped revision, and an applied revision in the same stage.
Affected plan: Stage 6, item 5, and Stage 7, item 3.
Evidence: After applying the stage, Runner.Run scans the stage's entire applied-change slice for each approved repair. A repair that was skipped scans the whole slice.
Complexity: With B repair branches, Q approved repairs, and A
applied changes in the stage, outcome classification costs
O(B + Q × A) beyond walking the work units. It can be quadratic even when
every approved repair is applied and the matches occur at successive positions.
Refactoring: Index the application results once by the existing stage-local
proposal index, then look up each approved repair. This takes expected
O(A + B) time and O(A) auxiliary space for an applied-index set. An
index carrying skip details could also support the evidence join in R2.
Acceptance: Keep disposition based on actual application. Cover a stage with an ordinary approval, an applied revision, and an approved revision skipped by the existing conflict checks.
R7 — Keep Detailed Repair History in Debug Runs (P3)
Completed. Ordinary runs retain a compact attempt count and disposition. Debug runs retain the revision history. Runner, reporting, and CLI tests pass with debug on and off.
Affected plan: Stage 7, item 3, and Stage 8, items 2–3.
Evidence: repairCandidates stores the revision, decisions, rejections, and operation slice on every attempt, even when debug capture is disabled. The run retains those attempts in its work units. The ordinary report needs a branch's proposal index, attempt count, and disposition. It counts executions from the work unit's separate operation list. The debug ledger reads the revision, decisions, and artifact references. Rejections are read during the interaction write, and the saved attempt-level operation slice has no consumer.
Complexity: For K attempts, the runner retains O(K + H) additional
history across the run, where H is the size of the retained revision text and
validator records. With debug off, that history can be reduced to
O(B) branch summaries for B branches after ordinary operation facts
have been emitted. Those operation facts still scale with the number of calls.
The conversation and findings needed for the next attempt remain branch-local
working state.
Refactoring: Keep compact outcome fields separate from optional ledger history. Retain detailed records for the ledger when debug capture is active. Use rejections locally for feedback and artifact writes and keep one copy of operation facts for ordinary accounting. Pass the final revision through the existing approved-candidate path for application.
Acceptance: Preserve counts and compact outcomes with debug off, including when ordinary reporting is disabled. Debug runs retain per-attempt decisions and available artifact paths on success and interruption.
R8 — Select a Protected-Spelling Finding in One Pass (P3)
Completed. Protection checks select a deterministic spelling in one pass without sorting. Validator and runner tests pass.
Affected plan: Stage 5, item 4.
Evidence: The protection checks collect every violating spelling into a slice, sort it, and read only the first value. This code runs during validation and through the application-time protection guard.
Complexity: For K reported violations, selection uses up to
O(K log K) string comparisons and O(K) extra storage. A running
lexical minimum needs O(K) comparisons and constant auxiliary space.
The matching and identity checks still do their existing work.
Refactoring: Select the same deterministic representative as violations are detected, using the evidence corrected in R9. Preserve the precedence of identity loss over capitalization and the existing rejection rules.
R9 — Name the Affected Protected Alias (P3)
Completed. Protection feedback identifies a spelling whose occurrence count fell. The alias regression and existing spoken-word protection tests pass.
Affected plan: Stage 5, item 4.
Evidence: When an identity loses a mention, the identity-preservation check uses the first prior occurrence's spelling for its finding. An identity can have multiple protected aliases. The first spelling can still be present after the edit.
Reproduced behavior: Protect both Mira and Captain Mira under one
identity. In a glossary stage, replace Captain Mira with the captain in
Mira spoke to Captain Mira. The validator correctly rejects the edit but
says it may alter Mira, the spelling that stayed. The same misleading
message becomes repair feedback.
Refactoring: Derive a specific spelling from before/after occurrence counts for the identity that lost a mention, or describe the identity loss when a spelling cannot be identified reliably. Counting spelling deficits can be done in expected linear time in matched occurrences. Preserve the existing identity-level protection decision and spoken-word repetition exception.
Acceptance: Exercise two aliases of one identity. The finding identifies the removed alias or the affected identity without attributing the edit to the unchanged spelling.
Stage Review Coverage
| Stage | Evidence reviewed | Result |
|---|---|---|
| 1 — Response and native composition | Repair schema, typed decode, correspondence, native multi-turn test | Response processing scales with response size. |
| 2 — Module prompts | Four registered definitions, shared contract, rendering and operator replacements | Per-request feedback serialization scales with supplied content. |
| 3 — Exchange and adapter | Shared section exchange, prepared prefix, branch history, native failure handling | R3. Cost grows with the sum of the full requests sent, as specified in the plan. |
| 4 — Configuration and readiness | Root/stage resolution, zero and positive budgets, profile readiness, role creation | A fixed number of role selections per stage. Readiness deduplicates profile inspections. |
| 5 — Findings | Applicability, previews, cardinality, protection, semantic input and batching | R4, R8, R9. Decision cardinality uses indexed checks. |
| 6 — Branch execution and application | Eligibility, versioned revalidation, defect attribution, application outcomes and cancellation | R1, R4–R6. Application evidence feeds R2. |
| 7 — Ordinary outcomes | Operation aggregation and sorting, report-disabled summaries, compact branch rows | R1. New presentation work scans and sorts the recorded facts. |
| 8 — Debug and ledger | Attempt retention, optional artifacts, redaction, indexed lineage and interrupted joins | R2, R7. The ledger joins decisions and branches by identity. |
| 9 — Assembled command and docs | Controlled-provider subprocess flows, cancellation, output selection, documentation and preview | Existing tests cover the principal execution paths. The recorded findings identify the gaps. |
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 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.
- The algorithmic audit derived scaling from the current source and its callers. A temporary Go overlay reproduced the wrong-alias finding in R9.
- Fresh tests passed for runner, validators, process reporting, LLM adapter, preflight, response schemas, proposal generation, configuration, and prompts during the algorithmic audit.
- After remediation, the fresh full test suite, targeted race checks for the runner, validators, process reporting, LLM adapter, and CLI, and the CLI build passed.