355 lines
21 KiB
Markdown
355 lines
21 KiB
Markdown
# Implementation Plan: Consistent Span Deletion
|
||
|
||
**Last updated:** 2026-09-25
|
||
|
||
**Status:** In progress; Stages 1–2 complete.
|
||
**Source baseline:** `a83a518` (feature roadmap); application baseline `0b6cd98`.
|
||
**Audience:** gpt-5.6-terra coding agent.
|
||
|
||
## Scope and Execution Rules
|
||
|
||
Implement [Consistent Span Deletion](span-deletion.md). That document owns the
|
||
product intent, target semantics, and acceptance criteria. This document owns the
|
||
concrete decisions, ordered work, and validation. Execute Stage 1 through Stage 4
|
||
in order. Record actual completion and checks beneath each stage when implemented.
|
||
Do not interpret this ready status as implemented behavior.
|
||
|
||
Read [development guidance](../development.md) and all [agent](../policy/agents.md),
|
||
[architecture](../policy/architecture.md), [documentation](../policy/documentation.md),
|
||
[testing](../policy/testing.md), and [release](../policy/release.md) policies.
|
||
Use graph-first code discovery. Inspect the current workspace before editing;
|
||
retain unrelated user work. Commit coherent completed units with plain English
|
||
messages, following the agent policy. Do not add permission rounds between stages.
|
||
|
||
Aim directly at the final state. Do not add temporary compatibility aliases,
|
||
feature flags, parallel implementations, or scaffolding for intermediate commits.
|
||
Preserve module scope, literal matching, replacement policies, fixed chunks,
|
||
required validation, failure isolation,
|
||
reporting/debug independence, and existing recovery budgets. Do not implement any
|
||
of the [deferred features](future.md), automatic whitespace cleanup, a new deletion
|
||
operation type, segment tombstones, or a second normalization pass.
|
||
|
||
Preserve intended safeguards and outcomes, not incidental implementation choices.
|
||
Validator ordering, internal structure, and payload construction may change where
|
||
needed to satisfy the feature contract. Current code and tests are evidence to
|
||
review, not authority to retain a conflicting restriction. Prefer reusing correct
|
||
mechanisms, but do not freeze them merely because they exist.
|
||
|
||
This replaces the completed resilience implementation plan, which remains in Git
|
||
history at `a83a518:docs/roadmap/implementation.md`. Historical resilience decisions
|
||
and their remediation are not instructions to rebuild or reverse that work.
|
||
During final documentation updates, repair references that describe this filename
|
||
as the resilience plan; use current canonical references or the historical revision.
|
||
No release, version tag, push, upstream change, or new dependency is part of this
|
||
implementation sprint.
|
||
|
||
## Selected Implementation Decisions
|
||
|
||
### Schema and library boundary
|
||
|
||
Remove `minLength: 1` from the `corrected_text` property of
|
||
`internal/framework/responseschema/assets/correction_set.v1.json`. Keep the required
|
||
field, string type, and all unrelated schema constraints. Empty and whitespace-only
|
||
strings are structurally valid; missing, null, and non-string replacements are not.
|
||
Keep `original_text` required, string-valued, and at least one character long.
|
||
Whitespace-only original spans are valid at the structural boundary and must not
|
||
be rejected solely on that basis by domain checks. Replace trimmed-emptiness checks
|
||
for original spans with literal empty-string checks in `CorrectionProposal.Validate`
|
||
and `ProposalShapeValidator.Validate`, and review other consumers for the same
|
||
restriction. Do not trim either span in transit. Resulting-segment checks still use
|
||
whitespace-aware emptiness. Preserve module matching policies: `require_unique`
|
||
for grammar, spoken-word, and homophones; `replace_all` for glossary.
|
||
|
||
Keep `CorrectionSetKey`, the schema asset path, ID/name/version, and existing prompt
|
||
IDs/`v1` selections. This widens an internal model-response constraint without
|
||
changing the correction object shape or creating a second contract to maintain.
|
||
The schema hash and PromptKit's rendered/prepared hashes reflect changed content;
|
||
no hard-coded hash is a compatibility promise. Ordinary report schema versions,
|
||
transcript encoders, configuration, and CLI selections do not change.
|
||
|
||
PromptKit continues to validate the whole response and own structural repairs.
|
||
Do not introduce partial JSON salvage or Audita-side regeneration. Once the schema
|
||
accepts empty replacements, normal per-candidate domain filtering handles proposals
|
||
that would blank their stage-input segment.
|
||
|
||
### Validator identity
|
||
|
||
Rename the existing validator consistently:
|
||
|
||
| Current | Target |
|
||
| --- | --- |
|
||
| `non_empty_corrected_text` key and package directory | `non_empty_resulting_segment` |
|
||
| `KeyNonEmptyCorrectedText` in registry and metadata | `KeyNonEmptyResultingSegment` |
|
||
| `NonEmptyCorrectionValidator` framework type | `NonEmptyResultingSegmentValidator` |
|
||
|
||
Retain the `empty_resulting_segment` rejection/skip reason, deterministic execution
|
||
class, and the resulting-segment safeguard. Reuse preview-based validation where
|
||
correct; neither its exact implementation nor chain position is frozen. Do not
|
||
retain an old-key alias.
|
||
This is an intentional validator identity change in diagnostic/decision records;
|
||
document it, including any affected ordinary filtered-proposal metadata. It is not
|
||
a new operator-selectable setting. Historical release notes remain untouched.
|
||
|
||
### Prompt and payload semantics
|
||
|
||
Review all four proposal definitions and the active validator definitions
|
||
(`editorial_review`, `meaning_reversal_review`, `spoken_form_plausibility`) for
|
||
consistency. Edit only definitions needing clarification. Review the registered
|
||
but inactive `grammar_review` and `spoken_word_review` definitions too; unchanged,
|
||
consistent definitions need no edits. Do not activate additional validators as
|
||
part of this work.
|
||
|
||
Where helpful, explain that an empty replacement deletes the identified span,
|
||
whitespace-only spans can express spacing corrections, and the resulting segment
|
||
must remain nonblank. Do not instruct the model to fabricate padding or content.
|
||
A spoken-word example could delete `"uh "` from `"uh hello"`; a grammar example
|
||
could reduce two spaces to one. Examples and wording are illustrative, not frozen
|
||
requirements or promises of automatic approval. Explain surrounding whitespace
|
||
and punctuation only to the extent needed to produce the intended literal result.
|
||
Qualify uniqueness by module: glossary uses `replace_all`, while the other three
|
||
modules use `require_unique`.
|
||
|
||
Keep module responsibilities intact. Meaning-reversal review remains a narrow
|
||
check for antonyms/reversals, not a new general semantic-equivalence validator.
|
||
Spoken-form plausibility retains its phonetic/mistranscription remit. Empty syntax
|
||
alone must not determine approval or rejection.
|
||
|
||
`BuildLLMValidationRequest` already supplies explicit `corrected_text` and full
|
||
before/after segment text. Start with that representation and verify that empty
|
||
and whitespace-only spans survive it faithfully. Adjust payload construction if
|
||
needed for consistency; do not treat its current structure as an independent
|
||
requirement. No new deletion flag or synthetic sentinel is needed merely to
|
||
represent an empty string.
|
||
|
||
### Debug representation and outcomes
|
||
|
||
`CorrectionLedgerEntry` currently uses strings with `omitempty` for both replacement
|
||
fields, losing explicit empty values. Change `ProposedCorrectedText` and
|
||
`AppliedCorrectedText` to `*string` with their existing JSON names and `omitempty`:
|
||
|
||
- Every candidate-backed entry has a non-nil proposed value, including `""`.
|
||
- Applied entries also have a non-nil applied value, including `""`.
|
||
- Rejected, skipped, and interrupted entries omit the applied value.
|
||
- Module-level failure entries without a candidate omit both values.
|
||
|
||
Construct the pointers directly when mapping the corresponding records; use the
|
||
same simple convention across all dispositions. Do not build custom marshaling
|
||
or a generic optional-value framework. These values remain replacement spans,
|
||
not full resulting-segment text. Preserve redaction and the rest of the ledger.
|
||
|
||
Keep existing operation accounting. A valid approved deletion is an ordinary
|
||
applied correction. A candidate that would blank its stage-input segment is
|
||
filtered with the existing reason and contributes to the proposal operation's
|
||
consolidated filtering recovery. Ordinary editorial/protected-term rejection does
|
||
not become a recovery. Multiple proposals can target the same segment; they are
|
||
validated against stage input and applied sequentially. A proposal applicable to
|
||
that input but made unsafe by an earlier accepted edit is skipped as a conflict,
|
||
with no added model error or filtering recovery. Keep the skipped evidence. The
|
||
runner's `attributeApplyFiltering` already makes this distinction: verify and reuse
|
||
it rather than invent new classification machinery. Structural repairs required
|
||
for unrelated defects still count normally.
|
||
|
||
## Stage 1 — Align Validator Identity and Confirm Domain Semantics
|
||
|
||
**Work**
|
||
|
||
- Rename the validator package/type/key described above. Update `New()` wiring,
|
||
`internal/validators/registry.go`, `chains.go`, metadata, framework validators,
|
||
consumers, and tests in one coherent change. Use graph discovery for symbols and
|
||
string searches for serialized keys. Do not globally rewrite historical docs.
|
||
- Preserve existing `CorrectionProposal.Validate`, `PreviewProposalForSegment`,
|
||
`ApplyProposals`, and deterministic validation behavior wherever already correct.
|
||
Change original-span trimmed-emptiness checks as specified above, and find any
|
||
equivalent downstream restrictions. Keep empty-original rejection and
|
||
whitespace-aware resulting-segment rejection distinct.
|
||
- Extend the existing proposal and validator tests only for missing meaningful
|
||
coverage: empty replacement leaving a nonblank segment, whitespace-only resulting
|
||
text rejection, whitespace-only originals, literal spacing, and apply-time
|
||
reevaluation after an earlier edit. Cover `"hello world"` with `" " -> " "`,
|
||
an empty original, and two separate matching whitespace runs under `require_unique`.
|
||
Assert module-policy ambiguity handling without imposing uniqueness on glossary.
|
||
For apply-time reevaluation, use segment `"a b"` and ordered deletions `"a" -> ""`,
|
||
`"b" -> ""`: each is previewable initially, but after the first edit the second
|
||
must be skipped as `empty_resulting_segment`. The surviving text is `" b"`;
|
||
no automatic trim or segment deletion is expected. At the runner/outcome boundary,
|
||
verify this initially applicable second proposal produces skipped debug evidence
|
||
but no model error or filtering recovery. Reuse/extend existing conflict tests;
|
||
a second full CLI scenario is unnecessary for this attribution check.
|
||
- Ensure protected-term validation still rejects removal of a protected term with
|
||
an empty replacement. Prefer one case in its existing test table. Retain existing
|
||
metadata-preservation tests rather than duplicating all metadata assertions.
|
||
- Update `docs/internal/validators.md` for the renamed implemented validator and
|
||
explain its resulting-segment responsibility; do not yet claim schema alignment.
|
||
|
||
**Validation**
|
||
|
||
Run `go test ./internal/framework/proposals ./internal/framework/validators ./internal/validators/... ./internal/framework/runner ./internal/framework/processreport`.
|
||
Existing registry/chain tests must use the new identity and verify the required
|
||
validation and execution class. Update incidental order assertions if a justified
|
||
ordering change preserves those safeguards.
|
||
Inspect remaining old-name references, retaining only deliberately historical ones.
|
||
|
||
**End state:** The accurately named domain validator and application preserve the
|
||
agreed behavior. No schema or retry workaround has been introduced.
|
||
|
||
**Completed 2026-09-25.** Renamed the validator, admitted nonempty whitespace
|
||
original spans, and verified literal spacing, blank-result protection, protected
|
||
terms, module-policy ambiguity, and apply-time conflict attribution. The specified
|
||
focused package tests passed.
|
||
|
||
## Stage 2 — Align Response Schema, Prompts, and LLM Validation Inputs
|
||
|
||
**Work**
|
||
|
||
- Apply the selected schema relaxation and selectively clarify prompts. Preserve
|
||
intended task scope, hardening, and response contracts while correcting conflicting
|
||
instructions wherever encountered within this feature. Review the
|
||
rendered definitions, not just YAML source, using existing PromptKit inspection
|
||
and prompt tests.
|
||
- In `internal/framework/responseschema/registry_test.go`, retain the check that
|
||
reported hashes equal hashes of the actual embedded bytes. Remove fixed digest
|
||
expectations that merely freeze asset content; do not replace them with a new
|
||
golden digest for this change.
|
||
- Extend the existing real-adapter tests in `internal/framework/llm/adapter_test.go`
|
||
with a bounded table for replacement values: empty string, spaces, ordinary text,
|
||
missing field, null, and a non-string value. Use the embedded schema and local
|
||
HTTP fixture. Give accepted cases a positive repair allowance and assert one
|
||
provider generation, zero repairs, and preservation of the actual string. For
|
||
invalid cases, use zero repair attempts to prove rejection without redundant
|
||
model calls. Include a whitespace-only original in an accepted case and confirm
|
||
an empty original is structurally invalid. Existing tests already own successful
|
||
repair/exhaustion behavior.
|
||
- Extend the validation-request tests in
|
||
`internal/framework/validators/llm_validators_test.go` to marshal a real request
|
||
for deleting `"uh "` from `"uh hello"`. Assert that JSON contains the explicit
|
||
empty replacement plus full before/after text, correct identity, and no immediate
|
||
rejection. Include a whitespace-only-original spacing correction in the request
|
||
coverage and verify literal preservation. A field-presence assertion is required;
|
||
decoding into a plain string alone cannot distinguish omitted from empty.
|
||
- Update canonical prompt/module guidance for the implemented semantics. Do not
|
||
add exact-prose prompt snapshots or assertions for every new sentence; existing
|
||
parsing/rendering/schema-association tests plus editorial review are sufficient.
|
||
|
||
**Validation**
|
||
|
||
Run `go test ./internal/framework/responseschema ./internal/prompts ./internal/framework/llm ./internal/framework/proposal_generation ./internal/framework/validators ./internal/modules/...`.
|
||
Confirm missing/non-string replacements still fail structurally and domain-invalid
|
||
but schema-valid proposals still reach ordinary filtering without an outer retry.
|
||
|
||
**End state:** A literal empty replacement can cross the real library/schema
|
||
boundary, and the model instructions and validator inputs describe its meaning.
|
||
|
||
**Completed 2026-09-25.** Relaxed only the replacement length constraint,
|
||
clarified the relevant proposal and active validation prompts, and verified
|
||
rendering, real-adapter acceptance/rejection, repair counts, and serialized
|
||
before/after validator payloads. The specified focused package tests passed.
|
||
|
||
## Stage 3 — Preserve Empty Replacements in Debug Evidence
|
||
|
||
**Work**
|
||
|
||
- Implement the pointer representation selected above in
|
||
`internal/framework/processreport/correction_ledger.go`, updating every mapping
|
||
branch and affected test consumer. Leave other debug structures unchanged where
|
||
their string fields already serialize empty values explicitly.
|
||
- Extend `correction_ledger_test.go` with a small serialized-disposition table:
|
||
applied deletion, rejected deletion, skipped deletion, interrupted deletion,
|
||
and a module failure without a candidate. Decode to a map or raw JSON fields to
|
||
check presence, `""` values, and absence of the applied value on unapplied entries.
|
||
Preserve existing decision classification/order tests using the renamed key.
|
||
- Update `docs/integrations/output-files.md` to explain the two debug replacement
|
||
fields and the deliberate validator identity change. Keep ordinary schemas and
|
||
report counts unchanged. Update the diagnostics/reporting internal reference
|
||
for the implemented mapping, linking to the public artifact contract.
|
||
|
||
**Validation**
|
||
|
||
Run `go test ./internal/framework/processreport ./internal/core/diagnostics ./internal/cli ./cmd/audita`.
|
||
Tests must inspect serialized output, not just non-nil in-memory values. Retain
|
||
existing configured-secret redaction and debug-off persistence coverage.
|
||
|
||
**End state:** Debug evidence distinguishes a proposed/applied deletion from an
|
||
absent proposal or an edit that was never applied.
|
||
|
||
## Stage 4 — Verify the Integrated Contract and Finish Documentation
|
||
|
||
**Work**
|
||
|
||
Extend `cmd/audita/main_integration_test.go` with representative offline subprocess
|
||
cases using its controlled HTTP provider, real configuration/profiles, embedded
|
||
schemas, built-in module/validator chains, CLI, and output/report/debug writers.
|
||
Keep fixture additions local and small; do not build a new integration framework.
|
||
Use explicit fixture sizing so relevant validator candidates fit one batch, and
|
||
use stable response indices. Do not rely on production chunk/token defaults or
|
||
on concurrent request order to choose responses.
|
||
|
||
Cover these distinct outcomes:
|
||
|
||
- **Clean deletion plus another correction:** A spoken-word stage proposes
|
||
`"uh " -> ""` in `"uh hello"` and an independent repetition collapse. Controlled
|
||
validators approve both. Assert corrected transcript, two applied corrections,
|
||
zero model errors/recoveries, zero structural repairs with a positive repair
|
||
allowance, and explicit empty proposed/applied fields in the debug ledger.
|
||
- **Whitespace-only span:** A grammar stage reduces one uniquely occurring run
|
||
of two spaces to one. Controlled validators approve it; assert the expected
|
||
text and no error/recovery solely due to whitespace. This proves schema, domain
|
||
validation, and application agree through the actual CLI. Use a compact fixture
|
||
without adding a matrix of whitespace characters or lengths.
|
||
- **Partial success with filtering:** Add a proposal to delete all text from a
|
||
separate filler-only segment, preserving that separate segment through fixture
|
||
normalization (use sufficient time separation or explicit normalization settings).
|
||
Assert that this candidate is filtered with `empty_resulting_segment`, the other
|
||
corrections still apply, the blanked segment is instead retained unchanged, and
|
||
the proposal response contributes one consolidated filtering recovery and no
|
||
work-unit error. The renamed deterministic validator must appear in evidence.
|
||
- **Ordinary negative decision:** Return a schema-valid deletion proposal, then a
|
||
valid negative decision from meaning-reversal review after earlier checks approve
|
||
it. Assert unchanged affected text, no applied deletion, no error/recovery solely
|
||
from the rejection, and a rejected debug entry. The controlled decision proves
|
||
the chain respects a negative decision, not that an arbitrary real model will
|
||
make the same editorial judgment.
|
||
|
||
Reuse existing YAML/Markdown/CLI count assertions rather than snapshotting entire
|
||
reports. Verify that the clean case's report totals agree with the transcript and
|
||
ledger. Compare resulting metadata to the normalized input, since initial
|
||
normalization may legitimately merge segments or reassign IDs. Confirm retained
|
||
section membership/order in the existing runner boundary tests; add a targeted
|
||
assertion only if that property is not already covered for the exercised path.
|
||
Do not repeat all cases for all modules, encoders, debug/report combinations, or
|
||
platforms: those unchanged mechanisms already have coverage.
|
||
|
||
Finish the documentation and status work:
|
||
|
||
- Review current module, validator, prompt, diagnostics, and output references for
|
||
consistency. Add a concise note to `docs/migrations/v2.md` describing the renamed
|
||
validator identity and explicit empty debug replacement fields; no alias or
|
||
migration tool is required. No release note/version is selected in this sprint.
|
||
- Review links from other roadmaps to `implementation.md`. References to completed
|
||
resilience stages should point to their historical plan at the recorded revision
|
||
or current canonical behavior, not silently identify this plan as that work.
|
||
Update those references only as needed; do not rewrite historical findings.
|
||
- Mark this plan and `span-deletion.md` implemented only after the criteria pass.
|
||
Keep the baseline description historical and retain future.md's exploratory
|
||
whole-segment-removal status. Record concise evidence under completed stages.
|
||
|
||
**Final validation**
|
||
|
||
Run `go test ./...` once after the integrated changes. Run `git diff --check` and
|
||
validate affected documentation links/examples. Focused stage checks need not be
|
||
repeated solely as ceremony. Additional race/platform/live-provider checks are
|
||
not automatic gates for this feature; run additional checks only for a concrete
|
||
risk introduced by actual changes. This is not a release invocation.
|
||
|
||
**End state:** The feature roadmap's acceptance criteria are evidenced by real
|
||
schema/adapter tests, domain/application tests, serialized debug tests, and
|
||
representative complete CLI workflows. The workspace is committed and the docs
|
||
describe implemented behavior without claiming whole-segment removal.
|
||
|
||
## Open Questions
|
||
|
||
None requiring a product decision remain. Validator names, schema/prompt identity,
|
||
debug representation, module scope, test boundaries, and deferred behavior are
|
||
selected above. Ordinary implementation details may be resolved idiomatically
|
||
within these decisions; a newly discovered conflict with the agreed contract
|
||
should be recorded rather than resolved by silently strengthening restrictions.
|