Add custom cleanup instructions and semantic retractions - #52
Conversation
![tasuku-app[bot]](https://avatars.githubusercontent.com/in/4436451?s=60&v=4){kind=small}
There was a problem hiding this comment.
Tasuku review
High risk: a model-controlled retraction flag bypasses protections against silent mutation or loss of dictated URLs, email addresses, numbers, and vocabulary.
Findings
All 1 finding(s) are posted as inline review comments.
Review confidence
98% · HIGH risk
Review coverage
| Category | Status |
|---|---|
| Correctness | Pass |
| Production safety | Pass |
| Architecture conventions | Pass |
| Maintainability | Pass |
| Security | Findings |
| Reliability | Pass |
| Performance | Pass |
| Testing | Pass |
| Compatibility | Pass |
| Dependencies | Not applicable |
| Ux accessibility | Not applicable |
| Operability | Not applicable |
| Documentation | Not applicable |
Important files
- Sources/DictatorCore/CleanupSafetyValidator.swift
- Sources/DictatorCore/LLM/CleanupResponseDecoder.swift
- Sources/DictatorCore/ProviderProtocols.swift
- Tests/DictatorCoreTests/CleanupProcessingTests.swift
Validation
| Validation | Evidence |
|---|---|
| Passed Source inspection |
git diff --name-status; git diff --stat; path-scoped git diff and targeted rg/git show inspectionInspected all four changed files plus every production decoder caller and relevant existing tests; no generated or minified artifacts were involved. |
| Not run Test |
Not runBuilds, tests, linters, formatters, type checkers, and package commands were prohibited by the review scope. |
Context used
| throw ProviderError.cleanupRejected("unexpected length change") | ||
| } | ||
|
|
||
| guard !retractionApplied else { return } |
There was a problem hiding this comment.
The validator returns before protected-token and vocabulary checks whenever the model reports any transcription retraction. A legitimate unrelated retraction—or an erroneous or malicious provider response—can therefore delete an email or number or substitute a URL while still being accepted. Replace the global bypass with validated withdrawn spans or another deterministic check that exempts only values proven to belong to withdrawn speech.
Evidence
At head line 26, guard !retractionApplied else { return } executes before both the protected-pattern and vocabulary loops; CleanupResponseDecoder derives this flag solely from the same untrusted model payload.
Validation
| Validation | Evidence |
|---|---|
| Passed Source inspection |
git diff --unified=60 62ea218e6944f4c0cefefe584ba66c6561761f1f ae5d6f16335dd0d93d7b2087f7cb5476c5119083 -- Sources/DictatorCore/CleanupSafetyValidator.swift Sources/DictatorCore/LLM/CleanupResponseDecoder.swift Tests/DictatorCoreTests/CleanupProcessingTests.swiftConfirmed that any true model-supplied retraction flag skips every protected-token and vocabulary check, including tokens unrelated to a retraction. |
| Not run Test |
Not runThe review scope explicitly prohibited executing tests. |
There was a problem hiding this comment.
Plan consolidated here: view plan
There was a problem hiding this comment.
Fixed in 1008291: Replaced the model-controlled global bypass with deterministically verified withdrawn-source ranges; unrelated protected tokens and vocabulary remain mandatory.
Agent-reported verification
- PASS:
git diff --checkcompleted successfully. - PASS: Confirmed production code contains no
retractionAppliedbypass and changes are limited to four approved cleanup files. - BLOCKED:
xcodebuild -project Dictator.xcodeproj -scheme Dictator -configuration Debug -destination 'platform=macOS' -only-testing:DictatorCoreTests/CleanupProcessingTests test—/bin/bash: xcodebuild: command not foundin this Linux container. - BLOCKED:
xcodebuild -project Dictator.xcodeproj -scheme Dictator -configuration Debug -destination 'platform=macOS' test—/bin/bash: xcodebuild: command not foundin this Linux container.
There was a problem hiding this comment.
Still open on 1008291: The new commits scope exemptions to source-matched spans, but line 23 accepts cue phrases through only a word boundary; ordinary dictated text such as Scratch that disk and reboot therefore authorizes removal of the preceding span and its protected tokens. Require the cue to end at punctuation or end-of-input, or validate the complete following clause before granting the exemption.
Validation
| Validation | Evidence |
|---|---|
| Passed Source inspection |
git diff --name-status ae5d6f16335dd0d93d7b2087f7cb5476c5119083 10082916c796acbc95669dc67f8f4d4c8aebf18dInspected all four changed files through path-scoped diffs, then traced validator callers, prompt behavior, models, and fallback handling with targeted searches and reads. |
| Not run Test |
Not runBuilds, tests, linters, formatters, type checkers, and other executable validation were prohibited by the review scope. |
Plan · Scope cleanup validation exemptions to verified retractionsReplace the provider-controlled global validation bypass with exact withdrawn-source spans that are deterministically validated. Protected tokens and vocabulary outside those spans will remain mandatory. Approach
Risks
Verification
Reply with |
|
@tasuku-app implement |
Implementation completeReplaced the global retraction bypass with exact UTF-16 withdrawn spans. Claims now require matching source text, valid non-overlapping ranges, source boundaries, and explicit adjacent withdrawal language. Length, protected-token occurrences, and enabled vocabulary are validated against only the retained transcript. Added regressions for malformed claims, repeated values, legacy bypass attempts, and unrelated emails, URLs, numbers, code, and vocabulary. Agent-reported verification
Result |
There was a problem hiding this comment.
Tasuku review
High risk: verified retractions remain bypassable through partial cue matches, and valid cleanup can fall back to the raw transcript in two newly introduced cases.
Findings
All 2 finding(s) are posted as inline review comments.
Review confidence
97% · HIGH risk
Review coverage
| Category | Status |
|---|---|
| Correctness | Findings |
| Production safety | Pass |
| Architecture conventions | Pass |
| Maintainability | Pass |
| Security | Pass |
| Reliability | Pass |
| Performance | Pass |
| Testing | Pass |
| Compatibility | Pass |
| Dependencies | Not applicable |
| Ux accessibility | Not applicable |
| Operability | Not applicable |
| Documentation | Not applicable |
Important files
- Sources/DictatorCore/CleanupSafetyValidator.swift
- Sources/DictatorCore/LLM/CleanupResponseDecoder.swift
- Sources/DictatorCore/ProviderProtocols.swift
- Tests/DictatorCoreTests/CleanupProcessingTests.swift
Validation
| Validation | Evidence |
|---|---|
| Passed Source inspection |
git diff --name-status ae5d6f16335dd0d93d7b2087f7cb5476c5119083 10082916c796acbc95669dc67f8f4d4c8aebf18dInspected all four changed files through path-scoped diffs, then traced validator callers, prompt behavior, models, and fallback handling with targeted searches and reads. |
| Not run Test |
Not runBuilds, tests, linters, formatters, type checkers, and other executable validation were prohibited by the review scope. |
Context used
| let ratio = Double(trimmed.count) / Double(max(raw.count, 1)) | ||
| let baseline = try retainedBaseline(raw: raw, withdrawnSpans: withdrawnSpans) | ||
| let ratio = Double(trimmed.count) / Double(max(baseline.count, 1)) | ||
| guard (0.45...1.65).contains(ratio) else { |
There was a problem hiding this comment.
The ratio includes the withdrawal cue, so withdrawing the first sentence from Email old@example.com. Scratch that. Hi. incorrectly rejects the valid output Hi.. The coordinator then falls back to the full raw transcript, re-inserting explicitly withdrawn text. Exclude verified cue text from the ratio baseline, or waive only the lower bound after span verification while retaining the protected-token checks.
Evidence
Line 39 applies the 0.45 lower bound even when baseline still contains Scratch that; CleanupCoordinator.cleanOrFallback converts this rejection into the original transcription fallback.
Validation
| Validation | Evidence |
|---|---|
| Passed Source inspection |
git diff --unified=40 ae5d6f16335dd0d93d7b2087f7cb5476c5119083 10082916c796acbc95669dc67f8f4d4c8aebf18d -- Sources/DictatorCore/CleanupSafetyValidator.swiftConfirmed that withdrawn source spans are removed from the baseline but their immediately following withdrawal cues remain subject to the lower length ratio. |
| Not run Test |
Not runThe review scope explicitly prohibited executing tests. |
| for term in vocabulary.filter(\.isEnabled).map(\.value) { | ||
| let baselineCount = caseInsensitiveOccurrenceCount(of: term, in: baseline) | ||
| let cleanedCount = caseInsensitiveOccurrenceCount(of: term, in: trimmed) | ||
| guard cleanedCount >= baselineCount else { |
There was a problem hiding this comment.
Requiring every vocabulary occurrence to remain rejects Dictator Dictator becoming Dictator, despite the prompt directing the model to remove accidental repetition. The rejection falls back to the uncleaned repeated transcript. Restore existence-based vocabulary preservation or distinguish accidental repetition from separate intended uses before comparing counts.
Evidence
The changed guard at line 54 compares cleaned and baseline occurrence counts, whereas the base implementation required only that a vocabulary term present in the source remain present in the output.
Validation
| Validation | Evidence |
|---|---|
| Passed Source inspection |
git show ae5d6f16335dd0d93d7b2087f7cb5476c5119083:Sources/DictatorCore/CleanupSafetyValidator.swiftConfirmed that the base required at least one retained occurrence, while the new head requires the complete baseline count. |
| Not run Test |
Not runThe review scope explicitly prohibited executing tests. |
2 participants
Summary
Why
Dictation can serve as a scratchpad. A speaker may explore alternatives, withdraw earlier thoughts, and end with one intended sentence. Cleanup should preserve ordinary ideation while removing only content the speaker explicitly retracted.
Impact
retractionAppliedValidation
xcodebuild -project Dictator.xcodeproj -scheme Dictator -configuration Debug -destination platform=macOS -derivedDataPath /tmp/dictator-retraction-derived CODE_SIGNING_ALLOWED=NO -only-testing:DictatorCoreTests/CleanupProcessingTests test— 17 tests passedxcodebuild -project Dictator.xcodeproj -scheme Dictator -configuration Debug -destination platform=macOS -derivedDataPath /tmp/dictator-retraction-derived CODE_SIGNING_ALLOWED=NO test— succeeded with one intentional live-audio skipgit diff --check