2026-09-25 03:03:14 +00:00
|
|
|
|
# Span Deletion Review: Required Remediation
|
2026-09-24 03:07:36 +00:00
|
|
|
|
|
2026-09-25 03:03:14 +00:00
|
|
|
|
**Reviewed:** 2026-09-25, source commit `24b8992`.
|
|
|
|
|
|
**Scope:** The four implementation commits after `31979d4`, compared with
|
|
|
|
|
|
[span-deletion.md](span-deletion.md), [implementation.md](implementation.md),
|
|
|
|
|
|
the agreed user intent, and repository policies.
|
2026-09-25 03:06:57 +00:00
|
|
|
|
**Status:** R15 addressed; see the resolution record below. Earlier findings R1–R14 are resolved; their evidence and
|
2026-09-25 03:03:14 +00:00
|
|
|
|
resolution records remain in Git history through `24b8992`.
|
2026-09-24 03:07:36 +00:00
|
|
|
|
|
|
|
|
|
|
## Architecture Assessment
|
|
|
|
|
|
|
2026-09-25 03:03:14 +00:00
|
|
|
|
The intended span-deletion behavior is implemented without introducing another
|
|
|
|
|
|
processing path. PromptKit accepts required string replacements including the
|
|
|
|
|
|
empty string; Audita retains responsibility for literal matching, module-specific
|
|
|
|
|
|
validation, and safe application. Nonempty whitespace-only original spans are
|
|
|
|
|
|
eligible. The renamed resulting-segment validator and application guard prevent
|
|
|
|
|
|
blank segments, while existing module scopes and replacement policies remain.
|
|
|
|
|
|
|
|
|
|
|
|
Validator requests carry the replacement and full before/after segment text.
|
|
|
|
|
|
Approved proposals apply sequentially, with applicability rechecked after earlier
|
|
|
|
|
|
edits. The runner distinguishes original proposal defects from later conflicts.
|
|
|
|
|
|
Debug ledger mapping preserves explicit empty proposed/applied replacements and
|
|
|
|
|
|
keeps absent values distinct. Existing resilience and reporting boundaries remain.
|
|
|
|
|
|
|
|
|
|
|
|
No production defect or unintended strengthening of the agreed requirements was
|
|
|
|
|
|
identified in this review. The finding below concerns a regression test that does
|
|
|
|
|
|
not actually protect one explicitly agreed reporting distinction.
|
|
|
|
|
|
|
|
|
|
|
|
## R15 — P2: Exercise Operation Accounting in the Deletion Conflict Test
|
|
|
|
|
|
|
|
|
|
|
|
**Category:** Ineffective regression coverage for conflict classification.
|
|
|
|
|
|
|
|
|
|
|
|
**Evidence:** `TestRunnerApplyConflictAfterTwoInitiallyValidDeletions` in
|
|
|
|
|
|
[`runner_test.go`](../../internal/framework/runner/runner_test.go) correctly checks
|
|
|
|
|
|
that the first deletion applies and the second is skipped to prevent a blank
|
|
|
|
|
|
segment. However, it uses `fakeModule`, whose `Propose` returns proposals without
|
|
|
|
|
|
operation outcomes. Its two deterministic validators also return no operations.
|
|
|
|
|
|
The final loop asserting absence of errors/recoveries therefore checks an empty
|
|
|
|
|
|
operation list.
|
|
|
|
|
|
|
|
|
|
|
|
In [`runner.go`](../../internal/framework/runner/runner.go),
|
|
|
|
|
|
`mergeFilteredProposals` can attach a recovery only to a successful proposal
|
|
|
|
|
|
operation. Without that operation, even incorrectly attributing the conflict as
|
|
|
|
|
|
a defective proposal cannot produce the recovery the test is meant to detect.
|
|
|
|
|
|
A temporary Go build overlay that removed the initially-applicable guard in
|
|
|
|
|
|
`attributeApplyFiltering` still passed this exact test. The production guard was
|
|
|
|
|
|
not changed and appears correct.
|
|
|
|
|
|
|
|
|
|
|
|
**Required change:** Revise this existing test to supply a successful proposal
|
|
|
|
|
|
operation alongside its two candidates, using the existing `outcomeModule` helper
|
|
|
|
|
|
or equivalent small fixture. Assert that the proposal operation is present before
|
|
|
|
|
|
checking that it has no failure, filtering recovery, or filtered-proposal records.
|
|
|
|
|
|
Retain the existing final-text and skipped-decision assertions. No production
|
|
|
|
|
|
change, additional CLI scenario, or new test framework is needed.
|
|
|
|
|
|
|
|
|
|
|
|
**Acceptance:** The revised test passes with the current production behavior and
|
|
|
|
|
|
fails if an initially valid deletion made unsafe by an earlier edit is incorrectly
|
|
|
|
|
|
recorded as proposal filtering. It continues to prove that only the first deletion
|
|
|
|
|
|
applies and that the second remains visible as a skipped decision. Fix the existing
|
|
|
|
|
|
test rather than adding overlapping coverage.
|
|
|
|
|
|
|
|
|
|
|
|
## Review Verification and Limits
|
|
|
|
|
|
|
|
|
|
|
|
- `go test ./...` passed against the committed implementation.
|
|
|
|
|
|
- Reviewed schema, domain checks, prompt changes, validator payloads, sequential
|
|
|
|
|
|
application/accounting, debug serialization, new tests, and updated contracts.
|
|
|
|
|
|
- The focused overlay experiment passed despite deliberately removing the
|
|
|
|
|
|
conflict-classification guard, confirming R15. The overlay existed only in a
|
|
|
|
|
|
temporary directory and did not modify repository source.
|
|
|
|
|
|
- No live-provider evaluation was performed. Offline fixtures establish pipeline
|
|
|
|
|
|
behavior, not a guarantee that a model will always propose or approve good edits.
|
|
|
|
|
|
- This review records remediation only; it does not implement the test correction
|
|
|
|
|
|
or any deferred feature.
|
2026-09-25 03:06:57 +00:00
|
|
|
|
|
|
|
|
|
|
## Resolution Record
|
|
|
|
|
|
|
|
|
|
|
|
**R15:** Updated the existing conflict test to use `outcomeModule` with a successful
|
|
|
|
|
|
proposal operation. The test requires that operation to be present and checks for
|
|
|
|
|
|
absent failure reasons, recovery reasons, and filtered-proposal records, while
|
|
|
|
|
|
retaining the final-text and skipped-decision assertions. Production code is unchanged.
|
|
|
|
|
|
|
|
|
|
|
|
Validation: `go test ./internal/framework/runner` passed. Repeating the temporary
|
|
|
|
|
|
overlay experiment now fails at the accounting assertion with an erroneous
|
|
|
|
|
|
`proposals_filtered` recovery for the second deletion, confirming that the test
|
|
|
|
|
|
detects the intended regression. The overlay did not modify repository source.
|