350 lines
21 KiB
Markdown
350 lines
21 KiB
Markdown
# Implementation Plan: Consistent Span Deletion
|
|
|
|
**Last updated:** 2026-09-25
|
|
|
|
**Status:** In progress; Stage 1 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.
|
|
|
|
## 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.
|