Add opt-in screen & clipboard reference context for LLM cleanup (Feature 03) - #43
Open
csmashe wants to merge 2 commits into
Open
Add opt-in screen & clipboard reference context for LLM cleanup (Feature 03)#43csmashe wants to merge 2 commits into
csmashe wants to merge 2 commits into
Conversation
…ure 03) Feed a bounded, local-first snippet of the focused element's on-screen text (+ nearby labels) and/or the clipboard into Medium/High LLM cleanup as read-only spelling reference, so dictated proper nouns, identifiers, ticket numbers, etc. get the on-screen spelling. Two independent global toggles (screen, clipboard), each with a per-profile override; both default off. - AtSpiUrlExtractor.TryHarvestFocusedContext: minimal-call focused-element harvest via Collection.GetMatches(STATE_FOCUSED) with a bounded BFS fallback; caches the a11y bus address for process lifetime; skips password fields (focused + siblings); caps ~1s wall-clock, ~40 node visits, ~2500 chars, and the GetText read; scopes to the recorded window (title-relate check) and bails when the window is unknown. - ActiveWindowService.GetFocusedScreenContext(process, title) + interface method; TextInsertionService.TryGetClipboardTextAsync gains an opt-in bounded/cancelable read. - DictationOrchestrator: capture inside the background snapshot task, gated on the effective per-profile toggle AND "a Medium/High cleanup with an available provider will run"; non-browser windows capture concurrently, browser windows defer until after the URL rematch so URL-profile opt-outs prevent the read. Threads a labelled, budget-split reference string through the cleanup pipeline; records coarse ScreenContextApplied/ClipboardContextApplied history flags. - PromptProcessingService.AppendReferenceContext: injection-safe framing (treat as inert data, defang the closing delimiter, hard length cap) before the provider call. - Settings UI: two toggles in the Advanced section + two per-profile override combos; new strings translated across en/de/es/ru. - Tests for the pure helpers (framing/defang/cap, source labelling/budget split, whitespace collapse, password-role skip, title relate).
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
📝 WalkthroughWalkthroughIntroduces opt-in reference context capture from focused screen text and clipboard text for LLM cleanup, with new settings and per-profile overrides, AT-SPI harvesting, bounded clipboard reads, prompt framing, UI wiring, localization, and tests. ChangesReference Context Feature
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant DictationOrchestrator
participant ActiveWindowService
participant TextInsertionService
participant LlmCleanupService
participant PromptProcessingService
DictationOrchestrator->>ActiveWindowService: GetFocusedScreenContext(processName, title)
ActiveWindowService-->>DictationOrchestrator: focused screen context or null
DictationOrchestrator->>TextInsertionService: TryGetClipboardTextAsync(maxChars, ct)
TextInsertionService-->>DictationOrchestrator: clipboard context or null
DictationOrchestrator->>DictationOrchestrator: BuildReferenceContext(screen, clipboard)
DictationOrchestrator->>LlmCleanupService: CleanAsync(text, level, statusCallback, referenceContext)
LlmCleanupService->>PromptProcessingService: ProcessSystemPromptAsync(systemPrompt, inputText, ct, referenceContext)
PromptProcessingService->>PromptProcessingService: AppendReferenceContext(systemPrompt, referenceContext)
PromptProcessingService-->>LlmCleanupService: framed prompt
LlmCleanupService-->>DictationOrchestrator: cleaned text
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
tests/TypeWhisper.Linux.Tests/TextInsertionServiceTests.cs (1)
1152-1157: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMirror the bounded/cancellable clipboard contract in the fake.
The fake ignores
maxCharsandct, so tests throughTextInsertionService.TryGetClipboardTextAsync(maxChars, ct)won’t catch contract regressions.Proposed test fake update
public Task<string?> TryGetClipboardTextAsync( int maxChars = int.MaxValue, CancellationToken ct = default ) { - return Task.FromResult(Clipboard); + ct.ThrowIfCancellationRequested(); + if (Clipboard is null || maxChars == int.MaxValue) + { + return Task.FromResult(Clipboard); + } + + var limit = Math.Min(Clipboard.Length, Math.Max(0, maxChars)); + return Task.FromResult<string?>(Clipboard[..limit]); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/TypeWhisper.Linux.Tests/TextInsertionServiceTests.cs` around lines 1152 - 1157, The test fake for TryGetClipboardTextAsync currently ignores the maxChars and ct parameters, so update the fake implementation to mirror the real clipboard contract used by TextInsertionService by honoring the cancellation token and applying the maxChars bound to the returned Clipboard value. Make the change in the TryGetClipboardTextAsync method so tests exercise the same behavior as the production path and can catch regressions in bounded or cancellable clipboard reads.src/TypeWhisper.Linux/Views/Sections/ProfilesSection.axaml (1)
295-295: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTrailing unused row in
RowDefinitions.The row list has 27 entries (14
Auto+ 131separators), but the highestGrid.Rowactually used in this grid is24, which only needs 25 entries. One extraAuto,1pair is dead weight.🧹 Proposed trim
- <Grid RowDefinitions="Auto,1,Auto,1,Auto,1,Auto,1,Auto,1,Auto,1,Auto,1,Auto,1,Auto,1,Auto,1,Auto,1,Auto,1,Auto,1,Auto"> + <Grid RowDefinitions="Auto,1,Auto,1,Auto,1,Auto,1,Auto,1,Auto,1,Auto,1,Auto,1,Auto,1,Auto,1,Auto,1,Auto">🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/TypeWhisper.Linux/Views/Sections/ProfilesSection.axaml` at line 295, The Grid in ProfilesSection.axaml has an extra unused trailing row definition in its RowDefinitions list. Trim the last dead `Auto,1` pair from the Grid setup so the row count matches the highest `Grid.Row` used by the section, keeping the layout identifiers and separator pattern in sync.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/TypeWhisper.Linux/Services/ActiveWindow/AtSpiUrlExtractor.cs`:
- Around line 320-322: The screen-context matching in AtSpiUrlExtractor is too
permissive because `IsMatchingApp` allows browser-family aliases, which can map
`msedge` to `Google Chrome` and harvest the wrong window. Tighten the logic in
the `processHint`/`appName` path to require exact app identity or a
focused-context-specific alias table for this extractor, and avoid reusing the
broader `IsMatchingApp` behavior here.
In `@src/TypeWhisper.Linux/Services/DictationOrchestrator.cs`:
- Line 2860: The StopAsync flow in DictationOrchestrator is restoring
ducking/media too late relative to the new snapshotTask.WaitAsync timeout, which
can leave audio ducked and media paused after recording stops. Move the
audio/media restoration logic to run before the snapshot wait in StopAsync, and
keep the existing snapshot wait/capture handling afterward so the system is
restored promptly even if browser URL/context capture takes up to 5 seconds.
In `@src/TypeWhisper.Linux/Services/PromptProcessingService.cs`:
- Around line 139-170: AppendReferenceContext only neutralizes the exact
lowercase closing tag, so the sanitization can be bypassed with casing variants.
Update the closing-tag replacement in AppendReferenceContext to use a
case-insensitive replace on the untrusted referenceContext so any variant of the
reference_context terminator is defanged before being inserted into the prompt.
Keep the rest of the framing logic in PromptProcessingService unchanged.
---
Nitpick comments:
In `@src/TypeWhisper.Linux/Views/Sections/ProfilesSection.axaml`:
- Line 295: The Grid in ProfilesSection.axaml has an extra unused trailing row
definition in its RowDefinitions list. Trim the last dead `Auto,1` pair from the
Grid setup so the row count matches the highest `Grid.Row` used by the section,
keeping the layout identifiers and separator pattern in sync.
In `@tests/TypeWhisper.Linux.Tests/TextInsertionServiceTests.cs`:
- Around line 1152-1157: The test fake for TryGetClipboardTextAsync currently
ignores the maxChars and ct parameters, so update the fake implementation to
mirror the real clipboard contract used by TextInsertionService by honoring the
cancellation token and applying the maxChars bound to the returned Clipboard
value. Make the change in the TryGetClipboardTextAsync method so tests exercise
the same behavior as the production path and can catch regressions in bounded or
cancellable clipboard reads.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4b987f0a-994e-44c3-98c7-b8c9922800dc
📒 Files selected for processing (21)
src/TypeWhisper.Core/Interfaces/IActiveWindowService.cssrc/TypeWhisper.Core/Models/AppSettings.cssrc/TypeWhisper.Core/Models/Profile.cssrc/TypeWhisper.Core/Models/TranscriptionRecord.cssrc/TypeWhisper.Linux/Resources/Localization/de.jsonsrc/TypeWhisper.Linux/Resources/Localization/en.jsonsrc/TypeWhisper.Linux/Resources/Localization/es.jsonsrc/TypeWhisper.Linux/Resources/Localization/ru.jsonsrc/TypeWhisper.Linux/Services/ActiveWindow/AtSpiUrlExtractor.cssrc/TypeWhisper.Linux/Services/ActiveWindowService.cssrc/TypeWhisper.Linux/Services/DictationOrchestrator.cssrc/TypeWhisper.Linux/Services/LlmCleanupService.cssrc/TypeWhisper.Linux/Services/PromptProcessingService.cssrc/TypeWhisper.Linux/Services/TextInsertionService.cssrc/TypeWhisper.Linux/ViewModels/Sections/AdvancedSectionViewModel.cssrc/TypeWhisper.Linux/ViewModels/Sections/ProfilesSectionViewModel.cssrc/TypeWhisper.Linux/Views/Sections/AdvancedSection.axamlsrc/TypeWhisper.Linux/Views/Sections/ProfilesSection.axamltests/TypeWhisper.Linux.Tests/AtSpiFocusedContextHelperTests.cstests/TypeWhisper.Linux.Tests/ReferenceContextFramingTests.cstests/TypeWhisper.Linux.Tests/TextInsertionServiceTests.cs
… using - AtSpiUrlExtractor.IsFocusTargetApp: drop IsMatchingApp browser-family aliasing (Edge↔Chrome) for the harvest — exact process match or title-trailing match only, so a capture scoped to the recorded window can't harvest another browser's screen. - DictationOrchestrator.StopAsync: restore audio ducking / media before awaiting the background snapshot (now up to 5 s) so the user's audio isn't left ducked after stop. - PromptProcessingService.AppendReferenceContext: defang the closing delimiter case-insensitively (attacker-controlled text; the LLM reads pseudo-XML loosely). - TextInsertionService.TryGetClipboardTextAsync: restore `using` for the process (CodeQL) while keeping kill-on-timeout via an inner finally.
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Feeds the content the user is looking at into Medium/High LLM cleanup as read-only spelling reference, so dictated proper nouns, function identifiers, ticket numbers, and unusual names get the on-screen/clipboard spelling. Two independent global toggles (screen, clipboard), each with a per-profile override; both default off with zero cost when disabled.
What it does
AtSpiUrlExtractor.TryHarvestFocusedContext): targets the focused element via a singleCollection.GetMatches(STATE_FOCUSED)round trip with a tightly-bounded BFS fallback; reads the focused node + up to 8 nearby sibling labels. Caches the a11y bus address for process lifetime; skips password fields (focused and siblings); hard caps ~1 s wall-clock, ~40 node visits, ~2500-char output, and the per-nodeGetTextread.wl-paste/xclipread, exposed with an opt-in bounded + cancelable variant (a huge/hung clipboard can't stall or over-read).DictationOrchestrator): capture runs in the existing background snapshot task, only when a toggle is effective for the matched profile and a Medium/High cleanup with an available provider will actually run. Non-browser windows capture concurrently with the (instant) URL early-return; browser windows defer until after the URL rematch so a URL-specific profile can turn a source off before it's read. The harvest is scoped to the recorded window and bails when the window can't be identified.PromptProcessingService.AppendReferenceContext): untrusted screen/clipboard text is wrapped as inert<reference_context>data (defanged closing delimiter, hard length cap, budget-split between sources), used only to fix spelling of words already dictated — never followed as instructions.ScreenContextApplied/ClipboardContextAppliedhistory flags.Testing
BuildReferenceContext(source labelling, empty handling, budget split so both sources survive), whitespace collapse, snippet combine/cap, password-role skip, window title relate.STATE_FOCUSABLEvsSTATE_FOCUSEDconstant, provider-availability gating, window scoping, read-budget enforcement, and the URL opt-out ordering).🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests