Record prompt audit findings and recommendations
This commit is contained in:
@@ -1,85 +1,246 @@
|
||||
# Span Deletion Review: Required Remediation
|
||||
# Prompt Audit: Findings and Recommended Changes
|
||||
|
||||
**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.
|
||||
**Status:** R15 addressed; see the resolution record below. Earlier findings R1–R14 are resolved; their evidence and
|
||||
resolution records remain in Git history through `24b8992`.
|
||||
**Reviewed:** 2026-09-25, source commit `b8089f4`.
|
||||
**Scope:** All four proposal prompts, three active LLM-validator prompts, two
|
||||
inactive validator definitions, shared hardening text, response schemas, supplied
|
||||
context, and downstream validation/application behavior.
|
||||
**Status:** R16–R26 open. R1–R15 are resolved; their findings and resolution records
|
||||
remain in Git history through `b8089f4`.
|
||||
|
||||
## Architecture Assessment
|
||||
|
||||
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.
|
||||
PromptKit owns structural response validation and repair. Audita owns domain
|
||||
validation, decision reconciliation, and application. The recent span-deletion
|
||||
and partial-decision changes are aligned across these boundaries. This audit found
|
||||
prompt contradictions, incomplete explanations of application semantics, and
|
||||
opportunities to improve instructions and supplied context.
|
||||
|
||||
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.
|
||||
R16–R19 and R22–R23/R25–R26 are focused wording improvements suitable for
|
||||
consideration before v2.0.0. R20–R21 require separate design or policy decisions;
|
||||
R24 is optional cleanup after checking consumers. Their inclusion here does not
|
||||
make every item a release gate or authorize an unreviewed behavioral expansion.
|
||||
The shared editorial validator's broad remit is intentional and needs no new
|
||||
module-scope enforcement as part of this audit.
|
||||
|
||||
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.
|
||||
## R16 — P2: Make the Grammar Article Exception Consistent
|
||||
|
||||
## R15 — P2: Exercise Operation Accounting in the Deletion Conflict Test
|
||||
**Category:** Conflicting instructions.
|
||||
|
||||
**Category:** Ineffective regression coverage for conflict classification.
|
||||
**Evidence:** The [grammar prompt](../../internal/prompts/assets/prompts/modules.grammar.proposal.yaml)
|
||||
allows whole-word `a`/`an` substitution, while its system instruction prohibits word
|
||||
substitution and a later rule permits only capitalization changes to written words.
|
||||
A model can follow those prohibitions and omit the expressly permitted correction.
|
||||
|
||||
**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.
|
||||
**Recommended change:** State the existing article exception consistently in the
|
||||
system message and word-preservation rule. Consolidate overlapping prohibitions
|
||||
where helpful without expanding grammar into spelling, homophone, or content edits.
|
||||
|
||||
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.
|
||||
**Acceptance:** The prompt unambiguously permits `a`/`an` correction under the
|
||||
existing conditions while retaining the module's conservative formatting remit.
|
||||
|
||||
**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.
|
||||
## R17 — P2: Clarify the Glossary Mishearing Exception
|
||||
|
||||
**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.
|
||||
**Category:** Prompt conflict with intended glossary correction behavior.
|
||||
|
||||
**Evidence:** The [glossary prompt](../../internal/prompts/assets/prompts/modules.glossary.proposal.yaml)
|
||||
permits replacing an existing glossary term with another for a plausible mishearing,
|
||||
then treats present glossary names as protected and prohibits altering them.
|
||||
The glossary-specific [protected-term validation](../../internal/framework/validators/protected_terms.go)
|
||||
deliberately allows protected-term swaps, subject to subsequent validation.
|
||||
|
||||
**Recommended change:** Distinguish preserving correct glossary spellings from
|
||||
correcting a contextually supported misidentification. Make the plausible-mishearing
|
||||
exception explicit while retaining acoustic plausibility and context requirements.
|
||||
|
||||
**Acceptance:** Recognizing a source word as a glossary term does not itself imply
|
||||
that the prompt forbids a supported correction to another term. Unrelated name
|
||||
substitution and gratuitous spelling/capitalization normalization remain outside
|
||||
scope; no validator relaxation is implied by this wording change.
|
||||
|
||||
## R18 — P2: Explain Literal Replacement and Context Spans
|
||||
|
||||
**Category:** Incomplete application contract in proposal instructions.
|
||||
|
||||
**Evidence:** [Glossary](../../internal/prompts/assets/prompts/modules.glossary.proposal.yaml)
|
||||
uses `replace_all` without explaining that every matching occurrence in the target
|
||||
segment changes. Grammar, homophones, and spoken-word require unique matches, but
|
||||
all proposal prompts ask for only the exact span needing replacement, which can
|
||||
discourage including unchanged context. [Preview/application](../../internal/framework/proposals/preview.go)
|
||||
uses literal, case-sensitive matching.
|
||||
|
||||
**Recommended change:** Explain glossary's replace-all behavior explicitly. Allow
|
||||
unchanged surrounding text in a source span when needed to identify the intended
|
||||
edit, preserving that context in the replacement. Explain literal matching without
|
||||
imposing uniqueness on glossary or adding automatic whitespace cleanup.
|
||||
|
||||
**Acceptance:** Guidance explains how to target one intended occurrence when a
|
||||
shorter span repeats, and that a glossary proposal using a repeated span affects
|
||||
all matches. Existing module replacement policies and eligible whitespace spans
|
||||
remain unchanged.
|
||||
|
||||
## R19 — P2: Align Output Instructions with Correction and Decision Identity
|
||||
|
||||
**Category:** Ambiguous response shape and identity guidance.
|
||||
|
||||
**Evidence:** Proposal prompts request only changed “segments,” although their
|
||||
[schema](../../internal/framework/responseschema/assets/correction_set.v1.json)
|
||||
contains correction proposals and multiple proposals can target one segment.
|
||||
Validator payloads contain both segment `id` and `correction_index`; the validator
|
||||
prompts request a decision for each correction index without expressly warning
|
||||
against substituting segment IDs or renumbering. Both schemas require a top-level
|
||||
object containing a named list, which prose currently leaves largely to the schema.
|
||||
|
||||
**Recommended change:** Request proposed corrections rather than changed segments,
|
||||
allow multiple proposals per segment, and explain that their source spans refer to
|
||||
the supplied text rather than imagined results of other proposals. Tell validators
|
||||
to copy each supplied `correction_index` exactly, including negative decisions,
|
||||
without renumbering or substituting segment IDs. Show the top-level
|
||||
`{"corrections":[...]}` and `{"validations":[...]}` shapes concisely.
|
||||
|
||||
**Acceptance:** Instructions match the supplied schemas and identity model without
|
||||
requiring one proposal per segment. Validators are still asked for complete decision
|
||||
sets; tolerant reconciliation remains a fallback, not the preferred output.
|
||||
|
||||
## R20 — P2: Evaluate Context Parity for LLM Validation
|
||||
|
||||
**Category:** Input-context limitation; separate design work.
|
||||
|
||||
**Evidence:** Proposal generation supplies section text and glossary data. The
|
||||
[validator variable builder](../../internal/framework/validators/llm_prompt_variables.go)
|
||||
supplies candidate before/after segments, categories, and transcript description,
|
||||
but not neighboring transcript segments or glossary definitions. Spoken-form and
|
||||
editorial prompts ask for contextual judgments; evidence supporting the proposal
|
||||
may be absent during validation. Other batched candidates are not reliable context.
|
||||
|
||||
**Recommended change:** Evaluate supplying relevant glossary information and
|
||||
neighboring transcript context to validators, including its effect on token
|
||||
estimation and batching. Choose the context scope in a separate design pass.
|
||||
Avoid asking models to rely on evidence they do not receive; do not silently weaken
|
||||
validation or imply access to audio.
|
||||
|
||||
**Acceptance:** A selected design states which evidence each validator receives
|
||||
and how it fits existing budgets. Representative examples distinguish cases needing
|
||||
additional context from cases already decidable from the segment. This is not a
|
||||
requirement to copy the entire section or glossary into every request.
|
||||
|
||||
## R21 — P2: Decide How Protected-Term Repetition Should Be Handled
|
||||
|
||||
**Category:** Prompt/validator policy tension; separate decision required.
|
||||
|
||||
**Evidence:** The [spoken-word prompt](../../internal/prompts/assets/prompts/modules.spoken_word.proposal.yaml)
|
||||
permits accidental repetition cleanup while preserving named entities. The
|
||||
[protected-term check](../../internal/framework/validators/protected_terms.go)
|
||||
requires each protected identity's occurrence count to be preserved. Thus
|
||||
`Jesters Jesters arrived` to `Jesters arrived` can be rejected even when repetition
|
||||
is accidental and the named entity remains.
|
||||
|
||||
**Recommended change:** Explicitly decide whether preservation of identity should
|
||||
permit removing a redundant occurrence. Evaluate a narrow allowance for supported
|
||||
repetition cleanup, retaining ordinary editorial and meaning checks. If current
|
||||
count preservation is retained, document that as a deliberate policy choice rather
|
||||
than silently making the prompt more restrictive to fit existing code.
|
||||
|
||||
**Acceptance:** Prompt and validation expectations agree after the policy decision.
|
||||
Any allowance preserves substantive references and intentional emphasis; any
|
||||
restriction is explicitly selected. This audit does not authorize a general
|
||||
relaxation of protected-term validation.
|
||||
|
||||
## R22 — P3: Clarify Full-Segment Spoken-Form Assessment
|
||||
|
||||
**Category:** Validator wording improvement.
|
||||
|
||||
**Evidence:** The [spoken-form validator](../../internal/prompts/assets/prompts/validators.spoken_form_plausibility.yaml)
|
||||
explicitly directs full before/after comparison for deletions, while its general
|
||||
criteria discuss source and replacement spans and surrounding segment context.
|
||||
Full segment text is available for every candidate.
|
||||
|
||||
**Recommended change:** Explicitly direct full before/after assessment for all
|
||||
corrections, retaining the narrow acoustic/mistranscription remit and the distinction
|
||||
from style or readability review.
|
||||
|
||||
**Acceptance:** Guidance consistently uses the available segment evidence without
|
||||
turning this validator into a general editorial or semantic-equivalence check.
|
||||
|
||||
## R23 — P3: Add a Deletion Example to Meaning-Reversal Guidance
|
||||
|
||||
**Category:** Optional illustrative improvement.
|
||||
|
||||
**Evidence:** The [meaning-reversal prompt](../../internal/prompts/assets/prompts/validators.meaning_reversal_review.yaml)
|
||||
correctly explains empty replacement and full-segment comparison, but its concrete
|
||||
examples focus on word substitutions such as visible/invisible and up/down.
|
||||
|
||||
**Recommended change:** Add a concise example where deleting a negation reverses
|
||||
meaning. Preserve context-sensitive judgment and the validator's narrow scope.
|
||||
|
||||
**Acceptance:** The example illustrates why a deletion can reverse meaning without
|
||||
prohibiting empty replacements or making every word deletion a reversal. It is not
|
||||
a new literal-token rule or a guarantee of model accuracy.
|
||||
|
||||
## R24 — P3: Review the Two Inactive Validator Definitions
|
||||
|
||||
**Category:** Optional maintenance cleanup.
|
||||
|
||||
**Evidence:** Registered [grammar_review](../../internal/prompts/assets/prompts/validators.grammar_review.yaml)
|
||||
and [spoken_word_review](../../internal/prompts/assets/prompts/validators.spoken_word_review.yaml)
|
||||
definitions largely duplicate editorial review and have already diverged from its
|
||||
updated deletion guidance. Neither is used by the [built-in chains](../../internal/validators/chains.go).
|
||||
|
||||
**Recommended change:** Check consumers and retained API uses, then retire the
|
||||
unused definitions and associated registrations if nothing needs them. If retained,
|
||||
document their purpose and keep relevant guidance consistent. Do not activate them
|
||||
as extra validation stages merely because they exist.
|
||||
|
||||
**Acceptance:** Each retained definition has a clear purpose; removal, if selected,
|
||||
updates actual consumers, tests, and prompt inventory, with any compatibility
|
||||
implications considered explicitly.
|
||||
|
||||
## R25 — P3: Define What Confidence Means
|
||||
|
||||
**Category:** Underspecified output semantics.
|
||||
|
||||
**Evidence:** Proposal and validator prompts specify the confidence range but not
|
||||
its interpretation. Proposal confidence drives threshold filtering. Validator
|
||||
confidence is range-checked but does not determine the verdict, which comes from
|
||||
`approved`.
|
||||
|
||||
**Recommended change:** Describe proposal confidence as confidence that the proposed
|
||||
correction is warranted, and validator confidence as confidence in the supplied
|
||||
verdict, including a negative verdict. Do not introduce a validator confidence
|
||||
threshold or request inflated scores to pass existing thresholds.
|
||||
|
||||
**Acceptance:** A confident rejection can legitimately have high confidence;
|
||||
`approved` remains authoritative for the validator verdict. No promise of numerical
|
||||
calibration or new runtime filtering behavior is added.
|
||||
|
||||
## R26 — P3: Clarify Hardening and Remove Assumed Pipeline Ordering
|
||||
|
||||
**Category:** Shared wording conflicts with the correction task or configuration.
|
||||
|
||||
**Evidence:** The [hardening fragment](../../internal/prompts/assets/shared/prompt_hardening.md)
|
||||
calls the transcript the “source of truth for what was said,” although correcting
|
||||
transcription errors is the task. The grammar prompt claims other corrections occur
|
||||
in a later review stage, although pipeline selection/order is configurable.
|
||||
|
||||
**Recommended change:** Describe the transcript as source material and require
|
||||
only evidence-supported corrections while retaining the data-versus-instructions
|
||||
boundary. Replace the grammar scheduling claim with a statement that homophone and
|
||||
mistranscription corrections are outside this module's scope.
|
||||
|
||||
**Acceptance:** Hardening discourages invention without implying the transcript
|
||||
cannot contain errors. Module guidance remains true regardless of configured stage
|
||||
order or whether another module is enabled. No pipeline ordering restriction is
|
||||
introduced.
|
||||
|
||||
## 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.
|
||||
|
||||
## 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.
|
||||
- This is a source/contract audit, not evidence of the frequency of any particular
|
||||
model failure in production.
|
||||
- `go test ./internal/prompts ./internal/framework/validators ./internal/framework/responseschema`
|
||||
passed during the audit. Those checks verify existing deterministic behavior,
|
||||
not the effectiveness of proposed language changes.
|
||||
- No live-provider evaluation was performed and none is made a release gate here.
|
||||
- Remediation should use focused tests for changed contracts or payloads and retain
|
||||
prompt parsing/rendering checks. Avoid exact-prose snapshots and a new evaluation
|
||||
framework solely to implement wording improvements.
|
||||
- This document catalogs observations and recommendations only; no prompts,
|
||||
application behavior, or release artifacts were changed by this update.
|
||||
|
||||
Reference in New Issue
Block a user