feat: transcription refinement via Apple Intelligence / Gemini#191
Open
rlchandani wants to merge 4 commits intokitlangton:mainfrom
Open
feat: transcription refinement via Apple Intelligence / Gemini#191rlchandani wants to merge 4 commits intokitlangton:mainfrom
rlchandani wants to merge 4 commits intokitlangton:mainfrom
Conversation
On-device text refinement and summarization using Apple Intelligence (macOS 26+) or Google Gemini Flash. Supports 5 tone presets with configurable hotkey cycling and menu bar indicator. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughAdds end-to-end transcription refinement: new refinement models/providers/tones, prompt building and text cleaning, pasteboard undo-and-replace, UI for refinement settings and hotkeys, integration into transcription pipeline with placeholder/paste/undo flow, build config updates, and comprehensive unit tests. Changes
Sequence DiagramsequenceDiagram
actor User
participant TxFeature as TranscriptionFeature
participant Pasteboard as PasteboardClient
participant Refiner as RefinementClient
participant Provider as Provider (Apple/Gemini)
participant Processor as RefinementTextProcessor
User->>TxFeature: recording finished / transcript ready
TxFeature->>TxFeature: set isRefining = true
alt refinementMode != .raw
TxFeature->>Pasteboard: paste placeholder (or use undoAndReplace)
TxFeature->>Refiner: refine(text, mode, tone, provider, apiKey)
Refiner->>Provider: dispatch request (concurrent, with timeout)
Provider-->>Refiner: response / error / timeout
Refiner->>Processor: clean(output)
Processor-->>Refiner: cleaned text and flags
alt output acceptable
Refiner-->>TxFeature: refined text
else
Refiner-->>TxFeature: fallback original text
end
TxFeature->>Pasteboard: replace placeholder with final text (or undo)
else
TxFeature->>Pasteboard: paste transcription directly
end
TxFeature->>TxFeature: storeTranscript(...)
TxFeature->>TxFeature: set isRefining = false
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 267a13a939
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Hex.xcodeproj/project.pbxproj
Outdated
| DEAD_CODE_STRIPPING = YES; | ||
| DEVELOPMENT_ASSET_PATHS = "\"Hex/Preview Content\""; | ||
| DEVELOPMENT_TEAM = QC99C9JE59; | ||
| DEVELOPMENT_TEAM = 5D7M37QQLU; |
There was a problem hiding this comment.
This commit switches DEVELOPMENT_TEAM to a different team ID, which will break automatic signing for anyone not in that Apple Developer account and can block local builds/archives in both Debug and Release. Unless this repository is intentionally migrating teams, this should stay on the existing team ID (or inherit project-level settings) so contributors and CI can continue signing.
Useful? React with 👍 / 👎.
Comment on lines
+56
to
+58
| } catch { | ||
| refinementLogger.warning("Refinement failed (\(provider.rawValue)): \(error.localizedDescription); falling back to raw text") | ||
| return text |
There was a problem hiding this comment.
The broad catch in safeRefine converts CancellationError into a raw-text fallback, so cancelling transcription during refinement can still continue the pipeline and paste/store text after the user hit cancel. This is user-visible because .cancel(id: CancelID.transcription) no longer reliably aborts output while refinement is in flight; cancellation should be rethrown (or explicitly checked) before fallback handling.
Useful? React with 👍 / 👎.
| state.isRefining = true | ||
| } | ||
|
|
||
| let supportsUndo = !TerminalAppDetector.isTerminal(sourceAppBundleID) |
There was a problem hiding this comment.
supportsUndo is derived from sourceAppBundleID, which is captured when recording starts, not when text is finally pasted. If the user switches apps during recording/refinement (e.g., starts in Notes, finishes in Terminal), Hex may incorrectly assume undo is supported, paste the Refining… placeholder, and fail to remove it with Cmd+Z in terminal apps.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (8)
Hex/Clients/PasteboardClient.swift (1)
337-357: Consider usingSauce.shared.keyCode(for: .z)for consistency withpostCmdV.The existing
postCmdVmethod (line 310) usesSauce.shared.keyCode(for: .v)to get the V key code dynamically, which handles non-US keyboard layouts correctly. Hardcoding6for Z could cause issues on layouts where virtual keycodes differ.Additionally,
postCmdVexplicitly presses/releases the modifier key (lines 312-321), while this implementation only sets the flag. Consider aligning the approaches for consistent behavior across applications.♻️ Proposed fix using Sauce and explicit modifier handling
`@MainActor` func undoAndReplace(replacement: String) async { // Undo the placeholder paste let source = CGEventSource(stateID: .combinedSessionState) - let zKey: CGKeyCode = 6 // Z key + let zKey = Sauce.shared.keyCode(for: .z) + let cmdKey: CGKeyCode = 55 - let keyDown = CGEvent(keyboardEventSource: source, virtualKey: zKey, keyDown: true) - keyDown?.flags = .maskCommand - keyDown?.post(tap: .cghidEventTap) - - let keyUp = CGEvent(keyboardEventSource: source, virtualKey: zKey, keyDown: false) - keyUp?.flags = .maskCommand - keyUp?.post(tap: .cghidEventTap) + let cmdDown = CGEvent(keyboardEventSource: source, virtualKey: cmdKey, keyDown: true) + let zDown = CGEvent(keyboardEventSource: source, virtualKey: zKey, keyDown: true) + zDown?.flags = .maskCommand + let zUp = CGEvent(keyboardEventSource: source, virtualKey: zKey, keyDown: false) + zUp?.flags = .maskCommand + let cmdUp = CGEvent(keyboardEventSource: source, virtualKey: cmdKey, keyDown: false) + + cmdDown?.post(tap: .cghidEventTap) + zDown?.post(tap: .cghidEventTap) + zUp?.post(tap: .cghidEventTap) + cmdUp?.post(tap: .cghidEventTap) // Let the undo complete try? await Task.sleep(for: .milliseconds(100))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Hex/Clients/PasteboardClient.swift` around lines 337 - 357, The undoAndReplace method hardcodes Z's keycode (zKey = 6) and only sets the command flag on CGEvent, which can break on non‑US layouts and differs from postCmdV; change it to obtain the keycode via Sauce.shared.keyCode(for: .z) and mirror postCmdV's explicit modifier press/release sequence (press command down, send keyDown/keyUp for Z, then release command) before sleeping and calling paste(text:), ensuring behavior matches postCmdV and supports international keyboards.HexCore/Sources/HexCore/Logic/TerminalAppDetector.swift (1)
6-17: Remove trailing comma to satisfy SwiftLint.The static analysis tool flagged a trailing comma violation on line 16.
🔧 Proposed fix
private static let terminalBundleIDs: Set<String> = [ "com.apple.Terminal", "com.googlecode.iterm2", "io.alacritty", "net.kovidgoyal.kitty", "dev.warp.Warp-Stable", "dev.warp.Warp", "co.zeit.hyper", "com.github.wez.wezterm", "com.raggesilver.BlackBox", - "org.tabby", + "org.tabby" ]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@HexCore/Sources/HexCore/Logic/TerminalAppDetector.swift` around lines 6 - 17, The array literal assigned to terminalBundleIDs has a trailing comma after "org.tabby" which triggers SwiftLint; remove the final comma in the static let terminalBundleIDs declaration so the last element is not followed by a comma, leaving the rest of the Set entries unchanged and preserving the Set<String> initialization.HexCore/Tests/HexCoreTests/RefinementSettingsTests.swift (1)
76-76: Force unwrap flagged by linter.While force unwrapping
data(using: .utf8)!is safe for this hardcoded ASCII JSON string in tests, you could usetry XCTUnwrapfor consistency with test conventions.🧹 Alternative using XCTUnwrap
-let data = oldJSON.data(using: .utf8)! +let data = try XCTUnwrap(oldJSON.data(using: .utf8))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@HexCore/Tests/HexCoreTests/RefinementSettingsTests.swift` at line 76, Replace the force unwrap of oldJSON.data(using: .utf8)! with an XCTUnwrap call to satisfy the linter and follow test conventions; specifically, in the test that declares oldJSON and assigns `data`, change the assignment to use `let data = try XCTUnwrap(oldJSON.data(using: .utf8))` (or equivalent) so the test will fail with a clear message if data conversion returns nil while keeping the rest of the test logic intact.Hex/Features/Transcription/TranscriptionIndicatorView.swift (1)
127-132: Remove unnecessary parentheses in while condition.The SwiftLint warning is valid — Swift doesn't require parentheses around conditions in
whilestatements.🧹 Proposed fix
- .task(id: status == .transcribing || status == .refining) { - while (status == .transcribing || status == .refining), !Task.isCancelled { + .task(id: status == .transcribing || status == .refining) { + while status == .transcribing || status == .refining, !Task.isCancelled {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Hex/Features/Transcription/TranscriptionIndicatorView.swift` around lines 127 - 132, In TranscriptionIndicatorView inside the .task block (the async Task with id: status == .transcribing || status == .refining), remove the unnecessary parentheses around the while condition so it reads while status == .transcribing || status == .refining, !Task.isCancelled { ... } — update the while statement in that Task closure accordingly to satisfy SwiftLint and keep behavior unchanged.Hex/Features/Settings/RefinementSectionView.swift (2)
109-126: SecureField dispatches action on every keystroke.The current binding calls
store.send(.setGeminiAPIKey($0.isEmpty ? nil : $0))for each character typed. Consider using a local@Statevariable and committing on.onSubmitor focus loss for better performance and to avoid storing partial keys.💡 Example using local state
`@State` private var apiKeyInput: String = "" // In body: SecureField("Gemini API Key", text: $apiKeyInput) .textFieldStyle(.roundedBorder) .onAppear { apiKeyInput = store.hexSettings.geminiAPIKey ?? "" } .onSubmit { store.send(.setGeminiAPIKey(apiKeyInput.isEmpty ? nil : apiKeyInput)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Hex/Features/Settings/RefinementSectionView.swift` around lines 109 - 126, The SecureField currently calls store.send(.setGeminiAPIKey(...)) on every keystroke (via the Binding used), causing excessive dispatches; change to use a local `@State` var (e.g., apiKeyInput) initialized from store.hexSettings.geminiAPIKey in .onAppear, bind SecureField to that state, and only call store.send(.setGeminiAPIKey(apiKeyInput.isEmpty ? nil : apiKeyInput)) onCommit (.onSubmit) or when focus is lost to commit the final value; update the empty-key warning to read from store.hexSettings.geminiAPIKey or from the committed state as appropriate.
116-116: Simplify the empty key check.The condition can be more concise using optional chaining with nil coalescing.
🧹 Simplified condition
-if store.hexSettings.geminiAPIKey == nil || store.hexSettings.geminiAPIKey?.isEmpty == true { +if store.hexSettings.geminiAPIKey?.isEmpty ?? true {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Hex/Features/Settings/RefinementSectionView.swift` at line 116, The conditional checking for an empty Gemini API key in RefinementSectionView currently uses "store.hexSettings.geminiAPIKey == nil || store.hexSettings.geminiAPIKey?.isEmpty == true"; replace it with a concise optional-aware check such as using optional chaining with nil-coalescing (e.g., evaluate store.hexSettings.geminiAPIKey?.isEmpty ?? true) inside the same if statement to simplify the logic while preserving behavior; update the condition wherever it appears (e.g., in the view code that references store.hexSettings.geminiAPIKey).Hex/Features/App/AppFeature.swift (1)
194-200: Consider adding adisplayNamecomputed property toRefinementTone.This switch duplicates information that could be centralized in the enum itself, improving maintainability if tones are added/renamed.
Example enhancement in RefinementTone
// In RefinementTone.swift public var displayName: String { switch self { case .natural: return "Natural" case .professional: return "Professional" case .casual: return "Casual" case .concise: return "Concise" case .friendly: return "Friendly" } }Then simplify to:
content.body = nextTone.displayName🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Hex/Features/App/AppFeature.swift` around lines 194 - 200, The switch in AppFeature creating label duplicates RefinementTone display strings; add a computed property on the RefinementTone enum (e.g., public var displayName: String) that returns the corresponding string for each case, then replace the switch in AppFeature (where label or content.body is set, e.g., nextTone/label) with nextTone.displayName to centralize the mapping and remove the duplicated switch.Hex/Clients/RefinementClient.swift (1)
148-156: Consider using Codable for response parsing.Manual dictionary access is error-prone and harder to maintain. A
Codablestruct would provide compile-time safety and clearer error messages.💡 Example Codable structures
private struct GeminiResponse: Decodable { let candidates: [Candidate] struct Candidate: Decodable { let content: Content } struct Content: Decodable { let parts: [Part] } struct Part: Decodable { let text: String } } // Usage: let response = try JSONDecoder().decode(GeminiResponse.self, from: data) guard let text = response.candidates.first?.content.parts.first?.text else { throw RefinementError.geminiRequestFailed("No text in response") } return text🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Hex/Clients/RefinementClient.swift` around lines 148 - 156, Replace the manual JSONSerialization/dictionary parsing in RefinementClient with Decodable models: add private Codable structs (e.g., GeminiResponse, Candidate, Content, Part) and use JSONDecoder().decode(GeminiResponse.self, from: data) to parse the response inside the method currently extracting resultText; then read response.candidates.first?.content.parts.first?.text and throw RefinementError.geminiRequestFailed("No text in response") (or a similarly clear message) if missing so parsing is type-safe and provides clearer errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Hex.xcodeproj/project.pbxproj`:
- Line 444: The commit changes the shared Xcode project setting DEVELOPMENT_TEAM
to a user-specific Team ID; revert the modifications to DEVELOPMENT_TEAM in the
shared project.pbxproj (restore the repository’s canonical or previous value)
and move any user/CI-specific team overrides into an untracked, local config
(e.g., an .xcconfig, the project.pbxproj.user, or CI environment variables) so
the shared file no longer contains a personal Team ID; search for the symbol
DEVELOPMENT_TEAM in the project to locate and remove/revert the two edits and
add guidance in the README or CI config for contributors to set their own
signing/team locally.
In `@Hex/Clients/RefinementClient.swift`:
- Line 119: The URL currently embeds the API key in the query string (the let
url = ... line in RefinementClient.swift) and force-unwraps the URL; change this
to build a URL without the key, guard-let-unwrap the URL safely, and move the
API key into the request header using the x-goog-api-key header when creating
the URLRequest (where you currently use url/request construction in the same
class/function). Ensure the code never interpolates apiKey into the URL and uses
guard let url = URL(string: ...) else { /* handle error */ } to avoid
force-unwraps.
In `@Hex/Features/App/AppFeature.swift`:
- Around line 192-213: showToneNotification currently posts a
UNNotificationRequest via UNUserNotificationCenter.add(_:), which will silently
fail if the user hasn't granted notification permission; update the startup flow
to request notification authorization and check the authorization status before
adding notifications. Specifically, add a one-time authorization request
(UNUserNotificationCenter.current().requestAuthorization...) during app
initialization (e.g., AppFeature startup) and modify showToneNotification to
query UNUserNotificationCenter.current().getNotificationSettings() or check the
authorization result before calling add(_:); if not authorized, fall back to a
non-permission-dependent UX (e.g., transient HUD/menu bar flash) so
notifications don't silently fail. Ensure references: showToneNotification,
UNUserNotificationCenter.add(_:), requestAuthorization/getNotificationSettings.
In `@Hex/Features/Settings/SettingsFeature.swift`:
- Around line 34-36: The global hotkey monitor in TranscriptionFeature currently
only checks SharedReaderKey.isSettingHotKey and misses the new
SettingsFeature.isSettingCycleToneHotkey, allowing recording to start during
cycle-tone remapping; fix by introducing a single shared "capture in progress"
flag (e.g., SharedReaderKey.isCapturingHotkey) or by expanding the gate to
include SettingsFeature.isSettingCycleToneHotkey, then update
TranscriptionFeature (the global monitor short-circuit logic) and any other
capture checks to use that shared flag (or both flags) so all hotkey-capture
states (isSettingHotKey, isSettingCycleToneHotkey, etc.) disable the main
recording hotkey consistently.
In `@Hex/Features/Transcription/TranscriptionFeature.swift`:
- Around line 464-510: When you paste the inline placeholder ("Refining…" /
"Summarizing…") in the run closure before calling refinement.refine, ensure that
any failure or cancellation paths also remove that placeholder: call await
pasteboard.undoAndReplace("") when supportsUndo is true (and fall back
appropriately if not) from the catch path in this closure and from the
cancellation handler function handleCancel(_:) and from handleTranscriptionError
so the temporary text is always cleaned up; locate the pasteboard.paste(...) and
pasteboard.undoAndReplace(...) usages around refinement.refine(...) and mirror
the undo logic into the catch block and the handleCancel(_:) /
handleTranscriptionError implementations for the same refinementMode flow.
- Around line 24-25: The record hotkey handler (hotKeyPressed) only checks
state.isTranscribing and therefore allows starting a second session during
refinement; update the busy check to treat refinement (and prewarming if
intended) as busy by including state.isRefining (and optionally
state.isPrewarming) in the condition used in hotKeyPressed so the hotkey is
ignored when isRefining is true (e.g., change checks from state.isTranscribing
to state.isTranscribing || state.isRefining).
In `@HexCore/Sources/HexCore/Logic/RefinementTextProcessor.swift`:
- Around line 58-61: isOffScript currently only flags outputs that are too long;
add short-output signals so terse off-script answers are caught: in
isOffScript(output:input:mode:) keep the lengthRatio/maxRatio logic but also
compute a token-overlap ratio between output and input (e.g., tokenize by
whitespace/punctuation, form sets, compute intersection/union) and for mode ==
.refined return true if overlapRatio is below a small threshold (e.g., < 0.2)
indicating the output lacks content from the input; for mode == .summarized
require expected bullet/list structure (check for "-" or "•" or multiple
newline-prefixed items and a minimum number of bullets) and flag as off-script
if missing; combine these checks with the existing lengthRatio and return true
if any failure condition is met.
- Around line 18-46: The preamble/trailing filter is too broad because the
hasPrefix checks (in the loops that inspect lines via while let first =
lines.first / while let last = lines.last) remove legitimate content; change
those checks to only remove exact canned responses or clearly punctuated
variants (e.g., compare trimmed.lowercased() == "sure" or
trimmed.lowercased().hasPrefix("sure,") and similarly for "here's"/"here is" as
"here's" or "here is," and for "i hope" require "i hope that" or punctuation),
or replace the current hasPrefix calls with a stricter full-line match or a
narrow regex anchored to start and end, so only true canned lines are removed
while leaving real dictation intact.
- Around line 7-12: The current stripLeakedTags(_:) uses
replacingOccurrences(of: "Text:", ...) which removes every instance; change it
to only strip a leading leaked wrapper by matching and removing a leading
pattern (e.g. optional opening quote, optional whitespace, then "Text:" then
optional whitespace/quote) at the start of the string so internal "Text:" tokens
remain untouched; update the implementation of stripLeakedTags(_:) to perform a
prefix-only removal (using a regex anchored at ^ or by checking hasPrefix after
trimming leading quotes/whitespace) before the subsequent trimming calls.
In `@HexCore/Sources/HexCore/Settings/HexSettings.swift`:
- Line 54: The geminiAPIKey is currently stored as a plaintext property on
HexSettings (geminiAPIKey) and serialized into hex_settings.json; refactor to
store it securely in the macOS Keychain instead: remove it from the
JSON-serializable state and replace the public stored property with computed
accessors on HexSettings that read/write the key using
SecItemCopyMatching/SecItemAdd/SecItemUpdate/SecItemDelete (or a Keychain
wrapper), provide a stable service/account identifier constant for the item, and
implement migration logic in your settings load path to detect an existing
plaintext geminiAPIKey in the JSON, write it into the Keychain, then clear it
from the saved JSON and persist settings; also update any encode/decode or
save/load methods and all call sites that reference geminiAPIKey so they use the
new computed getter/setter.
In `@HexCore/Tests/HexCoreTests/RefinementModelTests.swift`:
- Around line 63-73: The test testSummarizedValidTones is vacuous because it
only asserts against locally defined arrays and RefinementTone.allCases, not the
actual summarized-mode validation in AppFeature; extract the summarized-mode
valid-tone logic into a model-level API (e.g., add a static method
RefinementMode.validTones(for: .summarized) or RefinementTone.validFor(mode:))
that encapsulates the allowed tones, update the test to call that new method
(instead of checking local arrays or allCases), and assert the expected
included/excluded tones against its return value; alternatively remove the test
if you prefer not to promote the AppFeature logic into the model.
---
Nitpick comments:
In `@Hex/Clients/PasteboardClient.swift`:
- Around line 337-357: The undoAndReplace method hardcodes Z's keycode (zKey =
6) and only sets the command flag on CGEvent, which can break on non‑US layouts
and differs from postCmdV; change it to obtain the keycode via
Sauce.shared.keyCode(for: .z) and mirror postCmdV's explicit modifier
press/release sequence (press command down, send keyDown/keyUp for Z, then
release command) before sleeping and calling paste(text:), ensuring behavior
matches postCmdV and supports international keyboards.
In `@Hex/Clients/RefinementClient.swift`:
- Around line 148-156: Replace the manual JSONSerialization/dictionary parsing
in RefinementClient with Decodable models: add private Codable structs (e.g.,
GeminiResponse, Candidate, Content, Part) and use
JSONDecoder().decode(GeminiResponse.self, from: data) to parse the response
inside the method currently extracting resultText; then read
response.candidates.first?.content.parts.first?.text and throw
RefinementError.geminiRequestFailed("No text in response") (or a similarly clear
message) if missing so parsing is type-safe and provides clearer errors.
In `@Hex/Features/App/AppFeature.swift`:
- Around line 194-200: The switch in AppFeature creating label duplicates
RefinementTone display strings; add a computed property on the RefinementTone
enum (e.g., public var displayName: String) that returns the corresponding
string for each case, then replace the switch in AppFeature (where label or
content.body is set, e.g., nextTone/label) with nextTone.displayName to
centralize the mapping and remove the duplicated switch.
In `@Hex/Features/Settings/RefinementSectionView.swift`:
- Around line 109-126: The SecureField currently calls
store.send(.setGeminiAPIKey(...)) on every keystroke (via the Binding used),
causing excessive dispatches; change to use a local `@State` var (e.g.,
apiKeyInput) initialized from store.hexSettings.geminiAPIKey in .onAppear, bind
SecureField to that state, and only call
store.send(.setGeminiAPIKey(apiKeyInput.isEmpty ? nil : apiKeyInput)) onCommit
(.onSubmit) or when focus is lost to commit the final value; update the
empty-key warning to read from store.hexSettings.geminiAPIKey or from the
committed state as appropriate.
- Line 116: The conditional checking for an empty Gemini API key in
RefinementSectionView currently uses "store.hexSettings.geminiAPIKey == nil ||
store.hexSettings.geminiAPIKey?.isEmpty == true"; replace it with a concise
optional-aware check such as using optional chaining with nil-coalescing (e.g.,
evaluate store.hexSettings.geminiAPIKey?.isEmpty ?? true) inside the same if
statement to simplify the logic while preserving behavior; update the condition
wherever it appears (e.g., in the view code that references
store.hexSettings.geminiAPIKey).
In `@Hex/Features/Transcription/TranscriptionIndicatorView.swift`:
- Around line 127-132: In TranscriptionIndicatorView inside the .task block (the
async Task with id: status == .transcribing || status == .refining), remove the
unnecessary parentheses around the while condition so it reads while status ==
.transcribing || status == .refining, !Task.isCancelled { ... } — update the
while statement in that Task closure accordingly to satisfy SwiftLint and keep
behavior unchanged.
In `@HexCore/Sources/HexCore/Logic/TerminalAppDetector.swift`:
- Around line 6-17: The array literal assigned to terminalBundleIDs has a
trailing comma after "org.tabby" which triggers SwiftLint; remove the final
comma in the static let terminalBundleIDs declaration so the last element is not
followed by a comma, leaving the rest of the Set entries unchanged and
preserving the Set<String> initialization.
In `@HexCore/Tests/HexCoreTests/RefinementSettingsTests.swift`:
- Line 76: Replace the force unwrap of oldJSON.data(using: .utf8)! with an
XCTUnwrap call to satisfy the linter and follow test conventions; specifically,
in the test that declares oldJSON and assigns `data`, change the assignment to
use `let data = try XCTUnwrap(oldJSON.data(using: .utf8))` (or equivalent) so
the test will fail with a clear message if data conversion returns nil while
keeping the rest of the test logic intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 53527cb1-9dc2-44c0-b2d7-505f927d5eb7
📒 Files selected for processing (22)
Hex.xcodeproj/project.pbxprojHex/App/HexApp.swiftHex/Clients/PasteboardClient.swiftHex/Clients/RefinementClient.swiftHex/Features/App/AppFeature.swiftHex/Features/Settings/RefinementSectionView.swiftHex/Features/Settings/SettingsFeature.swiftHex/Features/Settings/SettingsView.swiftHex/Features/Transcription/TranscriptionFeature.swiftHex/Features/Transcription/TranscriptionIndicatorView.swiftHexCore/Sources/HexCore/Logic/RefinementPromptBuilder.swiftHexCore/Sources/HexCore/Logic/RefinementTextProcessor.swiftHexCore/Sources/HexCore/Logic/TerminalAppDetector.swiftHexCore/Sources/HexCore/Models/RefinementMode.swiftHexCore/Sources/HexCore/Models/RefinementProvider.swiftHexCore/Sources/HexCore/Models/RefinementTone.swiftHexCore/Sources/HexCore/Settings/HexSettings.swiftHexCore/Tests/HexCoreTests/RefinementModelTests.swiftHexCore/Tests/HexCoreTests/RefinementPromptBuilderTests.swiftHexCore/Tests/HexCoreTests/RefinementSettingsTests.swiftHexCore/Tests/HexCoreTests/RefinementTextProcessorTests.swiftHexCore/Tests/HexCoreTests/TerminalAppDetectorTests.swift
Hex.xcodeproj/project.pbxproj
Outdated
| DEAD_CODE_STRIPPING = YES; | ||
| DEVELOPMENT_ASSET_PATHS = "\"Hex/Preview Content\""; | ||
| DEVELOPMENT_TEAM = QC99C9JE59; | ||
| DEVELOPMENT_TEAM = 5D7M37QQLU; |
Contributor
There was a problem hiding this comment.
Avoid committing team-specific signing identity changes to shared project config.
Line 444 and Line 499 switch DEVELOPMENT_TEAM to a different Team ID, which can break signing/archiving for other contributors/CI and introduces config drift with existing project-level values. Please keep the repository’s canonical team ID (or move this to local/unshared config).
Suggested fix
- DEVELOPMENT_TEAM = 5D7M37QQLU;
+ DEVELOPMENT_TEAM = QC99C9JE59;Also applies to: 499-499
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Hex.xcodeproj/project.pbxproj` at line 444, The commit changes the shared
Xcode project setting DEVELOPMENT_TEAM to a user-specific Team ID; revert the
modifications to DEVELOPMENT_TEAM in the shared project.pbxproj (restore the
repository’s canonical or previous value) and move any user/CI-specific team
overrides into an untracked, local config (e.g., an .xcconfig, the
project.pbxproj.user, or CI environment variables) so the shared file no longer
contains a personal Team ID; search for the symbol DEVELOPMENT_TEAM in the
project to locate and remove/revert the two edits and add guidance in the README
or CI config for contributors to set their own signing/team locally.
Hex/Clients/RefinementClient.swift
Outdated
| let label = mode == .summarized ? "Summarizing" : "Refining" | ||
| refinementLogger.notice("\(label) via Gemini (\(text.count) chars)") | ||
|
|
||
| let url = URL(string: "https://generativelanguage.googleapis.com/v1beta/models/gemini-3.1-flash-lite-preview:generateContent?key=\(apiKey)")! |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Google Gemini API support x-goog-api-key header for authentication?
💡 Result:
Yes, the Google Gemini API supports the x-goog-api-key header for authentication with an API key.
Citations:
- 1: https://ai.google.dev/gemini-api/docs/api-key
- 2: https://ai.google.dev/api
- 3: https://gemini-api.apidog.io/doc-965852
- 4: https://ai.google.dev/gemini-api/docs/quickstart
🏁 Script executed:
head -n 160 Hex/Clients/RefinementClient.swift | tail -n 50Repository: kitlangton/Hex
Length of output: 2023
Security: API key exposed in URL query string.
Passing the API key as a URL query parameter exposes it in server access logs, proxy logs, and network monitoring tools. Google's Gemini API supports the x-goog-api-key header, which is the recommended approach. Additionally, the force unwrap can be safely replaced with a guard statement.
🔒 Proposed fix using header authentication
-let url = URL(string: "https://generativelanguage.googleapis.com/v1beta/models/gemini-3.1-flash-lite-preview:generateContent?key=\(apiKey)")!
+guard let url = URL(string: "https://generativelanguage.googleapis.com/v1beta/models/gemini-3.1-flash-lite-preview:generateContent") else {
+ throw RefinementError.geminiRequestFailed("Invalid URL")
+}
var request = URLRequest(url: url)
request.httpMethod = "POST"
request.setValue("application/json", forHTTPHeaderField: "Content-Type")
+request.setValue(apiKey, forHTTPHeaderField: "x-goog-api-key")
request.httpBody = try JSONSerialization.data(withJSONObject: body)🧰 Tools
🪛 SwiftLint (0.63.2)
[Warning] 119-119: Force unwrapping should be avoided
(force_unwrapping)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Hex/Clients/RefinementClient.swift` at line 119, The URL currently embeds the
API key in the query string (the let url = ... line in RefinementClient.swift)
and force-unwraps the URL; change this to build a URL without the key,
guard-let-unwrap the URL safely, and move the API key into the request header
using the x-goog-api-key header when creating the URLRequest (where you
currently use url/request construction in the same class/function). Ensure the
code never interpolates apiKey into the URL and uses guard let url = URL(string:
...) else { /* handle error */ } to avoid force-unwraps.
Comment on lines
+464
to
510
| return .run { [refinement, pasteboard] send in | ||
| do { | ||
| try await finalizeRecordingAndStoreTranscript( | ||
| result: modifiedResult, | ||
| let finalText: String | ||
| if refinementMode != .raw { | ||
| // Paste placeholder at cursor while refining (skip for terminal apps) | ||
| let placeholder = refinementMode == .refined ? "Refining\u{2026}" : "Summarizing\u{2026}" | ||
| if supportsUndo { | ||
| await pasteboard.paste(placeholder) | ||
| } | ||
|
|
||
| finalText = try await refinement.refine(modifiedResult, refinementMode, refinementTone, refinementProvider, geminiAPIKey) | ||
|
|
||
| guard !finalText.isEmpty else { | ||
| if supportsUndo { | ||
| await pasteboard.undoAndReplace("") | ||
| } | ||
| await send(.transcriptionCompleted) | ||
| return | ||
| } | ||
|
|
||
| if supportsUndo { | ||
| await pasteboard.undoAndReplace(finalText) | ||
| } else { | ||
| await pasteboard.paste(finalText) | ||
| } | ||
| } else { | ||
| finalText = modifiedResult | ||
| guard !finalText.isEmpty else { | ||
| await send(.transcriptionCompleted) | ||
| return | ||
| } | ||
| await pasteboard.paste(finalText) | ||
| } | ||
| soundEffect.play(.pasteTranscript) | ||
|
|
||
| try await storeTranscript( | ||
| result: finalText, | ||
| duration: duration, | ||
| sourceAppBundleID: sourceAppBundleID, | ||
| sourceAppName: sourceAppName, | ||
| audioURL: audioURL, | ||
| transcriptionHistory: transcriptionHistory | ||
| ) | ||
| await send(.transcriptionCompleted) | ||
| } catch { | ||
| await send(.transcriptionError(error, audioURL)) | ||
| } |
Contributor
There was a problem hiding this comment.
Remove the inline placeholder on failure paths too.
After Line 471 pastes Refining…/Summarizing…, only the happy path calls undoAndReplace. If refinement throws or the transcription effect is cancelled, handleTranscriptionError and handleCancel never clear that placeholder, so the target app can be left with stray status text inserted.
💡 Suggested fix
} catch {
+ if refinementMode != .raw && supportsUndo {
+ await pasteboard.undoAndReplace("")
+ }
await send(.transcriptionError(error, audioURL))
}Mirror the same cleanup in handleCancel(_:) so explicit cancellation does not leave the placeholder behind either.
Also applies to: 515-522, 574-590
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Hex/Features/Transcription/TranscriptionFeature.swift` around lines 464 -
510, When you paste the inline placeholder ("Refining…" / "Summarizing…") in the
run closure before calling refinement.refine, ensure that any failure or
cancellation paths also remove that placeholder: call await
pasteboard.undoAndReplace("") when supportsUndo is true (and fall back
appropriately if not) from the catch path in this closure and from the
cancellation handler function handleCancel(_:) and from handleTranscriptionError
so the temporary text is always cleaned up; locate the pasteboard.paste(...) and
pasteboard.undoAndReplace(...) usages around refinement.refine(...) and mirror
the undo logic into the catch block and the handleCancel(_:) /
handleTranscriptionError implementations for the same refinementMode flow.
Comment on lines
+58
to
+61
| public static func isOffScript(output: String, input: String, mode: RefinementMode) -> Bool { | ||
| let lengthRatio = Double(output.count) / max(Double(input.count), 1.0) | ||
| let maxRatio: Double = mode == .summarized ? 1.2 : 1.8 | ||
| return lengthRatio > maxRatio |
Contributor
There was a problem hiding this comment.
isOffScript only catches overlong failures.
A short off-script answer still passes this check. For example, input What time is the meeting tomorrow? and output 3 PM. yields a small ratio, so the model's answer would be accepted as a “refinement.” Add another signal here—token overlap for .refined, required bullet structure for .summarized, etc.—so terse failures fall back to the original text too.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@HexCore/Sources/HexCore/Logic/RefinementTextProcessor.swift` around lines 58
- 61, isOffScript currently only flags outputs that are too long; add
short-output signals so terse off-script answers are caught: in
isOffScript(output:input:mode:) keep the lengthRatio/maxRatio logic but also
compute a token-overlap ratio between output and input (e.g., tokenize by
whitespace/punctuation, form sets, compute intersection/union) and for mode ==
.refined return true if overlapRatio is below a small threshold (e.g., < 0.2)
indicating the output lacks content from the input; for mode == .summarized
require expected bullet/list structure (check for "-" or "•" or multiple
newline-prefixed items and a minimum number of bullets) and flag as off-script
if missing; combine these checks with the existing lengthRatio and return true
if any failure condition is met.
| public var refinementProvider: RefinementProvider | ||
| public var refinementTone: RefinementTone | ||
| public var cycleToneHotkey: HotKey? | ||
| public var geminiAPIKey: String? |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for existing Keychain usage patterns in the codebase
rg -n "SecItem|Keychain|kSecClass" --type swift
# Check how settings are persisted
rg -n "UserDefaults|AppStorage|fileSystemPersistence" --type swift -A 2Repository: kitlangton/Hex
Length of output: 40
🏁 Script executed:
# First, examine the HexSettings.swift file to understand its structure and how it's used
head -150 HexCore/Sources/HexCore/Settings/HexSettings.swiftRepository: kitlangton/Hex
Length of output: 5517
🏁 Script executed:
# Search for how HexSettings is instantiated, saved, or loaded
rg "HexSettings\(\)|HexSettings\s*=" --type swift -B 2 -A 2Repository: kitlangton/Hex
Length of output: 3351
🏁 Script executed:
# Look for persistence mechanisms related to HexSettings
rg "HexSettings" --type swift -B 1 -A 1 | head -60Repository: kitlangton/Hex
Length of output: 4537
🏁 Script executed:
# Check if there's any `@AppStorage`, UserDefaults, or custom persistence
rg "@AppStorage|\.userDefaults|Codable|Encodable" HexCore/Sources/HexCore/Settings/ --type swift -A 1Repository: kitlangton/Hex
Length of output: 715
🏁 Script executed:
# Find where HexSettings is saved/loaded
rg "HexSettings" Hex/Models/AppHexSettings.swift -A 10 -B 2Repository: kitlangton/Hex
Length of output: 456
🏁 Script executed:
# Search for SharedDefaults or similar persistence mechanism
rg "SharedDefaults|FileStorage|UserDefaults" --type swift | head -40Repository: kitlangton/Hex
Length of output: 242
🏁 Script executed:
# Look for any encryption or Keychain wrappers
rg -n "encrypt|Keychain|SecItem|kSec" --type swift | head -20Repository: kitlangton/Hex
Length of output: 342
🏁 Script executed:
# Find FileStorageKey and .fileStorage definition
rg "FileStorageKey|fileStorage" --type swift -B 2 -A 3 | head -60Repository: kitlangton/Hex
Length of output: 1189
🏁 Script executed:
# Find .hexSettingsURL definition
rg "hexSettingsURL" --type swift -B 2 -A 2Repository: kitlangton/Hex
Length of output: 591
🏁 Script executed:
# Check what FileStorage does with HexSettings
rg "FileStorage" --type swift --context 5 | head -80Repository: kitlangton/Hex
Length of output: 1456
🏁 Script executed:
# Find the hexMigratedFileURL function to understand storage location
rg "hexMigratedFileURL" --type swift -B 2 -A 5Repository: kitlangton/Hex
Length of output: 1584
🏁 Script executed:
# Search for any encryption wrappers around file storage
rg "encrypt|cipher|secure|protection" --type swift -i | head -20Repository: kitlangton/Hex
Length of output: 157
🏁 Script executed:
# Check if there's any file protection/encryption applied to the JSON storage
rg "fileProtection|setAttributes|protectedDataUntilFirstUserAuthentication" --type swiftRepository: kitlangton/Hex
Length of output: 40
Store API key in macOS Keychain instead of plaintext settings file.
The geminiAPIKey is serialized as plaintext JSON in hex_settings.json. API keys are sensitive credentials that should be stored in the macOS Keychain to prevent exposure through:
- Unencrypted preference files
- System backups
- Other processes with file access
Refactor to use Keychain storage via SecItem APIs or a wrapper library for this credential.
Also applies to: 91-91, 121-121, 174-174, 312-312
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@HexCore/Sources/HexCore/Settings/HexSettings.swift` at line 54, The
geminiAPIKey is currently stored as a plaintext property on HexSettings
(geminiAPIKey) and serialized into hex_settings.json; refactor to store it
securely in the macOS Keychain instead: remove it from the JSON-serializable
state and replace the public stored property with computed accessors on
HexSettings that read/write the key using
SecItemCopyMatching/SecItemAdd/SecItemUpdate/SecItemDelete (or a Keychain
wrapper), provide a stable service/account identifier constant for the item, and
implement migration logic in your settings load path to detect an existing
plaintext geminiAPIKey in the JSON, write it into the Keychain, then clear it
from the saved JSON and persist settings; also update any encode/decode or
save/load methods and all call sites that reference geminiAPIKey so they use the
new computed getter/setter.
Comment on lines
+63
to
+73
| func testSummarizedValidTones() { | ||
| let validForSummary: [RefinementTone] = [.natural, .professional, .concise] | ||
| let invalidForSummary: [RefinementTone] = [.casual, .friendly] | ||
|
|
||
| for tone in validForSummary { | ||
| XCTAssertTrue(RefinementTone.allCases.contains(tone), "\(tone) should exist") | ||
| } | ||
| for tone in invalidForSummary { | ||
| XCTAssertFalse(validForSummary.contains(tone), "\(tone) should not be in summary tones") | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if there's actual validation logic for summarized mode valid tones
ast-grep --pattern $'func $_(mode: RefinementMode) -> $_ {
$$$
}'
rg -n "validTones|summarized.*tone|tone.*summarized" --type swiftRepository: kitlangton/Hex
Length of output: 1218
🏁 Script executed:
# Get context around the validTones logic in AppFeature.swift
sed -n '100,115p' Hex/Features/App/AppFeature.swift
# Get the full RefinementModelTests.swift file to understand test intent
wc -l HexCore/Tests/HexCoreTests/RefinementModelTests.swift
cat -n HexCore/Tests/HexCoreTests/RefinementModelTests.swiftRepository: kitlangton/Hex
Length of output: 3714
🏁 Script executed:
# Search for RefinementMode and RefinementTone definitions
fd -e swift -x grep -l "enum RefinementMode\|enum RefinementTone" {}Repository: kitlangton/Hex
Length of output: 165
🏁 Script executed:
# Check RefinementMode and RefinementTone definitions
cat -n HexCore/Sources/HexCore/Models/RefinementMode.swift
cat -n HexCore/Sources/HexCore/Models/RefinementTone.swiftRepository: kitlangton/Hex
Length of output: 885
Test assertions don't verify actual model behavior.
This test only validates local arrays defined within the test itself. The assertion RefinementTone.allCases.contains(tone) on line 68 is always true for any RefinementTone value, making it vacuous.
The actual validation logic for summarized mode valid tones exists in AppFeature.swift (line 104), not in the model layer. Consider either extracting this validation to a model method (e.g., RefinementMode.validTones()) and testing that, or remove this test if it's not intended to guard against regressions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@HexCore/Tests/HexCoreTests/RefinementModelTests.swift` around lines 63 - 73,
The test testSummarizedValidTones is vacuous because it only asserts against
locally defined arrays and RefinementTone.allCases, not the actual
summarized-mode validation in AppFeature; extract the summarized-mode valid-tone
logic into a model-level API (e.g., add a static method
RefinementMode.validTones(for: .summarized) or RefinementTone.validFor(mode:))
that encapsulates the allowed tones, update the test to call that new method
(instead of checking local arrays or allCases), and assert the expected
included/excluded tones against its return value; alternatively remove the test
if you prefer not to promote the AppFeature logic into the model.
1. Revert DEVELOPMENT_TEAM to original (QC99C9JE59) so other contributors can build without signing errors. 2. Propagate CancellationError in safeRefine instead of swallowing it. ESC during refinement now correctly aborts without pasting text. 3. Re-check frontmost app at paste time instead of using the bundle ID captured at recording start. Prevents placeholder in terminal apps when user switches apps during refinement. Adds 7 new tests covering cancellation detection and app-switch scenarios (159 total, up from 152). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (4)
Hex/Features/Transcription/TranscriptionFeature.swift (3)
462-516:⚠️ Potential issue | 🟠 MajorRemove the inline placeholder on failure paths too.
This issue was previously flagged. After line 475 pastes the placeholder (
Refining…/Summarizing…), only the happy path callsundoAndReplace. If refinement throws, the catch block at lines 512-514 never clears the placeholder, leaving stray status text in the target app.💡 Suggested fix
The catch block needs access to
supportsUndoandrefinementModeto clean up properly. Consider restructuring:} catch { + // Clean up placeholder if we pasted one + if refinementMode != .raw && supportsUndo { + await pasteboard.undoAndReplace("") + } await send(.transcriptionError(error, audioURL)) }Note:
supportsUndois currently scoped inside theif refinementMode != .rawblock. You'll need to hoist it to be accessible in the catch block, or restructure the flow so cleanup is guaranteed.,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Hex/Features/Transcription/TranscriptionFeature.swift` around lines 462 - 516, Hoist the variables needed for cleanup so the catch path can remove the inline placeholder: declare a local supportsUndo (and keep refinementMode in scope) outside the if refinementMode != .raw branch before calling pasteboard.paste, then after any paste of the placeholder ensure the catch block checks supportsUndo and refinementMode and calls pasteboard.undoAndReplace("") when supported (or otherwise clears the placeholder appropriately) before sending .transcriptionError; update references to pasteboard.paste, pasteboard.undoAndReplace, refinement.refine, and the catch block to use the hoisted supportsUndo variable so the placeholder is always cleared on failure.
577-595:⚠️ Potential issue | 🟠 MajorhandleCancel should also clean up the placeholder.
When cancellation occurs during refinement (after the placeholder has been pasted),
handleCancelclears state but doesn't undo the placeholder text that may have been inserted into the target app. This is related to the error path issue—explicit cancellation via ESC or hotkey re-press should also remove the "Refining…" text.This would require the cancel handler to know whether a placeholder was pasted and to which app, which may need state tracking or a different approach (e.g., storing a "placeholderPasted" flag).
,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Hex/Features/Transcription/TranscriptionFeature.swift` around lines 577 - 595, handleCancel currently clears flags but doesn't remove a pasted "Refining…" placeholder; add tracking in State (e.g., placeholderPasted: Bool and placeholderTarget: some identifier or placeholderContext) and set that when the code path that pastes the placeholder performs the paste. In handleCancel, check state.placeholderPasted and if true call a new helper (e.g., undoPlaceholder(for:)) or perform the reverse paste/delete into the recorded placeholderTarget to remove the "Refining…" text, then reset state.placeholderPasted and placeholderTarget; keep the existing sleepManagement.allowSleep(), recording.stopRecording(), and soundEffect.play(.cancel) behavior. Ensure the helper runs inside the same Effect returned by handleCancel so placeholder cleanup is performed atomically with the other cancellation work.
101-104:⚠️ Potential issue | 🔴 CriticalTreat refinement as a busy state for the record hotkey.
This issue was previously flagged. Once
isRefining = trueis set (line 459), the record hotkey should behave like it does during transcription. Currently.hotKeyPressedonly checksstate.isTranscribingat line 104, so pressing the hotkey during refinement could start a second session while the first one can still paste stale text into the target app.💡 Suggested fix
case .hotKeyPressed: - // If we're transcribing, send a cancel first. Otherwise start recording immediately. + // If we're transcribing/refining, send a cancel first. Otherwise start recording immediately. // We'll decide later (on release) whether to keep or discard the recording. - return handleHotKeyPressed(isTranscribing: state.isTranscribing) + return handleHotKeyPressed(isProcessing: state.isTranscribing || state.isRefining)And update the helper:
- func handleHotKeyPressed(isTranscribing: Bool) -> Effect<Action> { - // If already transcribing, cancel first. Otherwise start recording immediately. - let maybeCancel = isTranscribing ? Effect.send(Action.cancel) : .none + func handleHotKeyPressed(isProcessing: Bool) -> Effect<Action> { + // If already transcribing/refining, cancel first. Otherwise start recording immediately. + let maybeCancel = isProcessing ? Effect.send(Action.cancel) : .none let startRecording = Effect.send(Action.startRecording) return .merge(maybeCancel, startRecording) },
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Hex/Features/Transcription/TranscriptionFeature.swift` around lines 101 - 104, handleHotKeyPressed currently only checks state.isTranscribing when deciding to cancel/start, so pressing the hotkey during refinement can start a new session; change the logic to treat refinement as a busy state by using a combined condition (e.g., pass state.isTranscribing || state.isRefining instead of state.isTranscribing) when calling handleHotKeyPressed, and update the helper signature/parameter name (or its internal check) in handleHotKeyPressed to reflect/expect "isBusy" (or check both isTranscribing and isRefining) so the hotkey behaves the same during refinement as during active transcription.Hex/Clients/RefinementClient.swift (1)
125-140:⚠️ Potential issue | 🟠 MajorSecurity: API key exposed in URL query string.
This issue was previously flagged. The API key is passed as a URL query parameter, which exposes it in server access logs, proxy logs, and network monitoring tools. Google's Gemini API supports the
x-goog-api-keyheader for authentication, which is the recommended approach. The force unwrap can also be safely replaced with a guard statement.🔒 Proposed fix using header authentication
-let url = URL(string: "https://generativelanguage.googleapis.com/v1beta/models/gemini-3.1-flash-lite-preview:generateContent?key=\(apiKey)")! +guard let url = URL(string: "https://generativelanguage.googleapis.com/v1beta/models/gemini-3.1-flash-lite-preview:generateContent") else { + throw RefinementError.geminiRequestFailed("Invalid URL") +} var request = URLRequest(url: url) request.httpMethod = "POST" request.setValue("application/json", forHTTPHeaderField: "Content-Type") +request.setValue(apiKey, forHTTPHeaderField: "x-goog-api-key") request.httpBody = try JSONSerialization.data(withJSONObject: body),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Hex/Clients/RefinementClient.swift` around lines 125 - 140, The URL currently embeds the apiKey and force-unwraps URL; change to build the endpoint without the key, guard-let the URL (avoid force-unwrap) and move authentication into the request headers by setting "x-goog-api-key" to apiKey on the URLRequest (the existing variable request) instead of including it in the URL string; update the code that constructs url, request.httpMethod, request.setValue("application/json", forHTTPHeaderField: "Content-Type"), and request.httpBody to add request.setValue(apiKey, forHTTPHeaderField: "x-goog-api-key") and remove the key from the URL construction (refer to the URL instantiation, apiKey variable, and request object in RefinementClient.swift).
🧹 Nitpick comments (1)
Hex/Clients/RefinementClient.swift (1)
154-162: Consider using Codable structs for type-safe JSON parsing.The manual dictionary-based JSON parsing is functional but fragile. If the API response structure changes subtly, errors may be unclear. For improved maintainability and better error messages, consider defining Codable structs for the expected response shape.
♻️ Example Codable approach
private struct GeminiResponse: Codable { struct Candidate: Codable { struct Content: Codable { struct Part: Codable { let text: String } let parts: [Part] } let content: Content } let candidates: [Candidate] } // Usage: let response = try JSONDecoder().decode(GeminiResponse.self, from: data) guard let resultText = response.candidates.first?.content.parts.first?.text else { throw RefinementError.geminiRequestFailed("No text in response") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Hex/Clients/RefinementClient.swift` around lines 154 - 162, Replace the fragile dictionary-based parsing in the block that throws RefinementError.geminiRequestFailed with a Codable-based decoder: define Codable structs (e.g., GeminiResponse, Candidate, Content, Part) matching the response shape (candidates → content → parts → text), use JSONDecoder().decode(GeminiResponse.self, from: data) where the current guard is, then extract resultText via response.candidates.first?.content.parts.first?.text and throw a clear RefinementError.geminiRequestFailed("No text in response") if missing; keep the same error paths and return semantics as the original method in RefinementClient.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@Hex/Clients/RefinementClient.swift`:
- Around line 125-140: The URL currently embeds the apiKey and force-unwraps
URL; change to build the endpoint without the key, guard-let the URL (avoid
force-unwrap) and move authentication into the request headers by setting
"x-goog-api-key" to apiKey on the URLRequest (the existing variable request)
instead of including it in the URL string; update the code that constructs url,
request.httpMethod, request.setValue("application/json", forHTTPHeaderField:
"Content-Type"), and request.httpBody to add request.setValue(apiKey,
forHTTPHeaderField: "x-goog-api-key") and remove the key from the URL
construction (refer to the URL instantiation, apiKey variable, and request
object in RefinementClient.swift).
In `@Hex/Features/Transcription/TranscriptionFeature.swift`:
- Around line 462-516: Hoist the variables needed for cleanup so the catch path
can remove the inline placeholder: declare a local supportsUndo (and keep
refinementMode in scope) outside the if refinementMode != .raw branch before
calling pasteboard.paste, then after any paste of the placeholder ensure the
catch block checks supportsUndo and refinementMode and calls
pasteboard.undoAndReplace("") when supported (or otherwise clears the
placeholder appropriately) before sending .transcriptionError; update references
to pasteboard.paste, pasteboard.undoAndReplace, refinement.refine, and the catch
block to use the hoisted supportsUndo variable so the placeholder is always
cleared on failure.
- Around line 577-595: handleCancel currently clears flags but doesn't remove a
pasted "Refining…" placeholder; add tracking in State (e.g., placeholderPasted:
Bool and placeholderTarget: some identifier or placeholderContext) and set that
when the code path that pastes the placeholder performs the paste. In
handleCancel, check state.placeholderPasted and if true call a new helper (e.g.,
undoPlaceholder(for:)) or perform the reverse paste/delete into the recorded
placeholderTarget to remove the "Refining…" text, then reset
state.placeholderPasted and placeholderTarget; keep the existing
sleepManagement.allowSleep(), recording.stopRecording(), and
soundEffect.play(.cancel) behavior. Ensure the helper runs inside the same
Effect returned by handleCancel so placeholder cleanup is performed atomically
with the other cancellation work.
- Around line 101-104: handleHotKeyPressed currently only checks
state.isTranscribing when deciding to cancel/start, so pressing the hotkey
during refinement can start a new session; change the logic to treat refinement
as a busy state by using a combined condition (e.g., pass state.isTranscribing
|| state.isRefining instead of state.isTranscribing) when calling
handleHotKeyPressed, and update the helper signature/parameter name (or its
internal check) in handleHotKeyPressed to reflect/expect "isBusy" (or check both
isTranscribing and isRefining) so the hotkey behaves the same during refinement
as during active transcription.
---
Nitpick comments:
In `@Hex/Clients/RefinementClient.swift`:
- Around line 154-162: Replace the fragile dictionary-based parsing in the block
that throws RefinementError.geminiRequestFailed with a Codable-based decoder:
define Codable structs (e.g., GeminiResponse, Candidate, Content, Part) matching
the response shape (candidates → content → parts → text), use
JSONDecoder().decode(GeminiResponse.self, from: data) where the current guard
is, then extract resultText via
response.candidates.first?.content.parts.first?.text and throw a clear
RefinementError.geminiRequestFailed("No text in response") if missing; keep the
same error paths and return semantics as the original method in
RefinementClient.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cbf4d1ca-d415-42d0-b475-a2b5bb4dea98
📒 Files selected for processing (6)
Hex.xcodeproj/project.pbxprojHex/Clients/RefinementClient.swiftHex/Features/Transcription/TranscriptionFeature.swiftHexCore/Sources/HexCore/Logic/RefinementTextProcessor.swiftHexCore/Tests/HexCoreTests/RefinementTextProcessorTests.swiftHexCore/Tests/HexCoreTests/TerminalAppDetectorTests.swift
✅ Files skipped from review due to trivial changes (1)
- HexCore/Tests/HexCoreTests/RefinementTextProcessorTests.swift
🚧 Files skipped from review as they are similar to previous changes (3)
- Hex.xcodeproj/project.pbxproj
- HexCore/Tests/HexCoreTests/TerminalAppDetectorTests.swift
- HexCore/Sources/HexCore/Logic/RefinementTextProcessor.swift
…cleanup 1. Move Gemini API key from URL query param to x-goog-api-key header to prevent key leaking in logs and referer headers. 2. Request notification permission before posting tone cycle notifications. Handles notDetermined and denied states. 3. Disable recording hotkey during cycle-tone hotkey capture by checking isSettingCycleToneHotkey in the hotkey monitor. 4. Treat isRefining as a busy state for hotkey press. Pressing the hotkey during refinement now cancels first (same as during transcription). 5. Clean up inline placeholder on error and cancel paths. Tracks didPastePlaceholder locally in the effect and undoes it in the catch block. 6. Tighten stripLeakedTags to only strip "Text:" at the start of output (not globally). Tighten stripPreamble to only strip short throwaway lines ending with colon/exclamation — preserves legitimate content starting with "Sure", "Here's", etc. 166 total tests (104 XCTest + 62 Swift Testing), all passing. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Select text in any app and press a configurable hotkey to refine or summarize it using the current mode, tone, and provider. The selection is copied, processed via Apple Intelligence or Gemini, and pasted back replacing the original selection. - New refineSelectionHotkey in HexSettings with hotkey capture UI - New refineSelection action in AppFeature with Cmd+C → refine → paste flow - Recording hotkey disabled during refine-selection hotkey capture - Settings test coverage for new field and migration Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (3)
HexCore/Sources/HexCore/Logic/RefinementTextProcessor.swift (2)
37-42:⚠️ Potential issue | 🟠 MajorThe pre/postamble filters can still delete real dictated text.
Short lines like
Of course we should ship this today.orLet me know by Friday.still match the broadhasPrefixchecks here and get dropped even though they are valid content. Match exact canned wrappers, or much narrower punctuated variants, instead of generic prefixes.Also applies to: 60-64
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@HexCore/Sources/HexCore/Logic/RefinementTextProcessor.swift` around lines 37 - 42, The pre/postamble detection (isPreamblePhrase and isIntroLine used in the if that checks trimmed.count < 80) is too broad and removes legitimate short lines because it uses hasPrefix for generic words; change these checks to match only exact canned wrappers or tightly punctuated variants (e.g., full phrase equality or prefix + punctuation like "of course," "of course:" or regex anchored patterns) rather than plain hasPrefix for "certainly", "of course", "sure", "here's", "here is"; update the isPreamblePhrase and isIntroLine logic to use those exact-match or punctuation-aware matches so valid dictated sentences (e.g., "Of course we should ship this today.") are not dropped.
83-86:⚠️ Potential issue | 🟠 MajorShort off-script model answers still pass this guard.
Lines 84-86 only reject outputs that are too long, so a terse answer like
3 PM.for dictated inputWhat time is the meeting tomorrow?is still accepted as a refinement. Add a second signal here—token overlap with the input or other mode-specific validation—before pasting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@HexCore/Sources/HexCore/Logic/RefinementTextProcessor.swift` around lines 83 - 86, isOffScript currently only checks lengthRatio (output/input) and misses terse off-topic answers; update isOffScript to require a second signal before accepting a refinement by computing token overlap or a mode-specific validation in addition to lengthRatio. Concretely, inside isOffScript add a tokenization-based overlap check (e.g., split on whitespace or use the project tokenizer) to compute overlapRatio = sharedTokens / max(inputTokens.count, 1), then combine with lengthRatio: return true if lengthRatio > maxRatio OR overlapRatio < minOverlap (use stricter minOverlap for .summarized vs other RefinementMode values) so very short or non-overlapping outputs (like "3 PM.") are rejected; reference the isOffScript function, lengthRatio, maxRatio, and RefinementMode when implementing thresholds and logic.Hex/Features/App/AppFeature.swift (1)
197-202:⚠️ Potential issue | 🟡 MinorRe-check notification authorization after prompting.
settingsis captured beforerequestAuthorization, so on a first-run deny this code still falls through tocenter.add(_:)using the stale.notDeterminedvalue. Use the returnedBoolor fetch updated settings after the prompt before scheduling the notification.🛠️ Minimal fix
let settings = await center.notificationSettings() if settings.authorizationStatus == .notDetermined { - _ = try? await center.requestAuthorization(options: [.alert]) + let granted = (try? await center.requestAuthorization(options: [.alert])) ?? false + guard granted else { return } } - guard settings.authorizationStatus != .denied else { return } + let updatedSettings = await center.notificationSettings() + guard updatedSettings.authorizationStatus != .denied else { return }Also applies to: 222-222
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Hex/Features/App/AppFeature.swift` around lines 197 - 202, The notification permission check captures settings before requestAuthorization, so if the prompt occurs the stale `settings` value (e.g., `.notDetermined`) can be used later; update the logic in AppFeature where `center.notificationSettings()` and `center.requestAuthorization(options:)` are called (lines around the `settings` variable and `center.add(_:)`) to either use the Bool returned by `requestAuthorization` or call `await center.notificationSettings()` again after `requestAuthorization` and re-evaluate `authorizationStatus` before proceeding to `center.add(_:)`; ensure any early return uses the refreshed status to avoid scheduling notifications with stale permission state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Hex/Clients/RefinementClient.swift`:
- Around line 149-152: The guard in RefinementClient that currently builds
errorBody and calls refinementLogger.error with the raw response body should be
changed to avoid logging sensitive content; log only the HTTP status code (e.g.,
"Gemini API error HTTP <statusCode>") and, if you must record the response body
for diagnostics, send it through HexLog/HexLog helper with a privacy annotation
(privacy: .private) instead of refinementLogger.error; update the thrown
RefinementError.geminiRequestFailed call (and the guard block around it) to not
include the raw body and ensure refinementLogger/HexLog usage follows the
codebase's unified logging pattern.
- Around line 6-8: The file imports FoundationModels conditionally but leaves
the appleProcess function (which calls LanguageModelSession()) unguarded; wrap
the entire appleProcess function definition (the appleProcess(...) method that
constructs/uses LanguageModelSession()) inside the same conditional compilation
block (`#if` canImport(FoundationModels) ... `#endif`) so the function is only
compiled when FoundationModels is available, preserving the existing `@available`
attribute on appleProcess.
In `@Hex/Features/App/AppFeature.swift`:
- Around line 266-283: The key event handler currently only pauses when
isSettingCycleToneHotkey is true, so add the other hotkey-recording flags (e.g.
isSettingDictationHotkey and isSettingPasteLastHotkey) to the initial gate so
the monitor returns false while any hotkey is being recorded; update the check
in keyEventMonitor.handleKeyEvent (or the initial if) to short-circuit if
isSettingCycleToneHotkey || isSettingDictationHotkey ||
isSettingPasteLastHotkey, keeping the rest of the guard logic that matches
hexSettings.cycleToneHotkey and the send(.cycleTone) call unchanged.
In `@Hex/Features/Transcription/TranscriptionFeature.swift`:
- Around line 516-521: The catch-all currently forwards any thrown error
(including CancellationError from refinement.refine(...)) into
send(.transcriptionError(error, audioURL)); change the catch logic so that after
performing the placeholder cleanup via pasteboard.undoAndReplace("") when
didPastePlaceholder is true, you detect CancellationError (or Task.isCancelled)
and return early without calling send(.transcriptionError...), otherwise
continue to send the transcription error; reference the symbols
refinement.refine, didPastePlaceholder, pasteboard.undoAndReplace(_:) and
send(.transcriptionError(error, audioURL)) when making the change.
- Around line 468-496: Capture the frontmost app bundle ID before inserting the
placeholder (already done into currentAppBundleID), then before any
undoAndReplace or placeholder-replacement logic (the calls to
pasteboard.undoAndReplace and pasteboard.paste after refinement.refine and in
the empty-result branch) re-check
NSWorkspace.shared.frontmostApplication?.bundleIdentifier on the MainActor and
compare it to the saved currentAppBundleID; only perform undoAndReplace to
remove the placeholder if the bundle IDs still match, otherwise treat the
placeholder as lost — do not send a global undo (skip undoAndReplace), clear
didPastePlaceholder, and directly paste finalText (or skip pasting empty
replacement) so we never undo unrelated user work; apply this check for both the
success and empty-result branches that reference didPastePlaceholder.
In `@HexCore/Sources/HexCore/Logic/RefinementTextProcessor.swift`:
- Around line 8-16: The function stripLeakedTags currently always strips
surrounding quotes even when no "Text:" wrapper was present; change it so quote
removal only happens when a leaked wrapper is detected: in stripLeakedTags(_:)
compute a boolean (e.g., hadWrapper) by checking result.hasPrefix("Text:") or
result.hasPrefix("\"Text:") (or by peeking past an initial quote) and only
perform the trimming of surrounding quotes on result if hadWrapper is true; keep
the existing logic that drops the leading "Text:" when detected and use the same
result variable for these conditional trims.
---
Duplicate comments:
In `@Hex/Features/App/AppFeature.swift`:
- Around line 197-202: The notification permission check captures settings
before requestAuthorization, so if the prompt occurs the stale `settings` value
(e.g., `.notDetermined`) can be used later; update the logic in AppFeature where
`center.notificationSettings()` and `center.requestAuthorization(options:)` are
called (lines around the `settings` variable and `center.add(_:)`) to either use
the Bool returned by `requestAuthorization` or call `await
center.notificationSettings()` again after `requestAuthorization` and
re-evaluate `authorizationStatus` before proceeding to `center.add(_:)`; ensure
any early return uses the refreshed status to avoid scheduling notifications
with stale permission state.
In `@HexCore/Sources/HexCore/Logic/RefinementTextProcessor.swift`:
- Around line 37-42: The pre/postamble detection (isPreamblePhrase and
isIntroLine used in the if that checks trimmed.count < 80) is too broad and
removes legitimate short lines because it uses hasPrefix for generic words;
change these checks to match only exact canned wrappers or tightly punctuated
variants (e.g., full phrase equality or prefix + punctuation like "of course,"
"of course:" or regex anchored patterns) rather than plain hasPrefix for
"certainly", "of course", "sure", "here's", "here is"; update the
isPreamblePhrase and isIntroLine logic to use those exact-match or
punctuation-aware matches so valid dictated sentences (e.g., "Of course we
should ship this today.") are not dropped.
- Around line 83-86: isOffScript currently only checks lengthRatio
(output/input) and misses terse off-topic answers; update isOffScript to require
a second signal before accepting a refinement by computing token overlap or a
mode-specific validation in addition to lengthRatio. Concretely, inside
isOffScript add a tokenization-based overlap check (e.g., split on whitespace or
use the project tokenizer) to compute overlapRatio = sharedTokens /
max(inputTokens.count, 1), then combine with lengthRatio: return true if
lengthRatio > maxRatio OR overlapRatio < minOverlap (use stricter minOverlap for
.summarized vs other RefinementMode values) so very short or non-overlapping
outputs (like "3 PM.") are rejected; reference the isOffScript function,
lengthRatio, maxRatio, and RefinementMode when implementing thresholds and
logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8e8181a7-3868-4401-b29e-74271b8d3bb3
📒 Files selected for processing (5)
Hex/Clients/RefinementClient.swiftHex/Features/App/AppFeature.swiftHex/Features/Transcription/TranscriptionFeature.swiftHexCore/Sources/HexCore/Logic/RefinementTextProcessor.swiftHexCore/Tests/HexCoreTests/RefinementTextProcessorTests.swift
✅ Files skipped from review due to trivial changes (1)
- HexCore/Tests/HexCoreTests/RefinementTextProcessorTests.swift
Comment on lines
+6
to
+8
| #if canImport(FoundationModels) | ||
| import FoundationModels | ||
| #endif |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "RefinementClient.swift" | head -5Repository: kitlangton/Hex
Length of output: 94
🏁 Script executed:
cat -n ./Hex/Clients/RefinementClient.swiftRepository: kitlangton/Hex
Length of output: 7800
Wrap the appleProcess function definition in #if canImport(FoundationModels).
The conditional import guards the module import (lines 6–8), but the appleProcess function (lines 108–117) uses LanguageModelSession() from FoundationModels without compile-time guards. Although the function is marked @available(macOS 26.0, *), that attribute only affects runtime availability checking—it does not prevent compilation errors when the module is unavailable. On a toolchain/SDK where FoundationModels is not available, the file will fail to compile at line 110. Wrap the function definition in #if canImport(FoundationModels) to ensure safe compilation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Hex/Clients/RefinementClient.swift` around lines 6 - 8, The file imports
FoundationModels conditionally but leaves the appleProcess function (which calls
LanguageModelSession()) unguarded; wrap the entire appleProcess function
definition (the appleProcess(...) method that constructs/uses
LanguageModelSession()) inside the same conditional compilation block (`#if`
canImport(FoundationModels) ... `#endif`) so the function is only compiled when
FoundationModels is available, preserving the existing `@available` attribute on
appleProcess.
Comment on lines
+149
to
+152
| guard httpResponse.statusCode == 200 else { | ||
| let errorBody = String(data: data, encoding: .utf8) ?? "unknown" | ||
| refinementLogger.error("Gemini API error \(httpResponse.statusCode): \(errorBody)") | ||
| throw RefinementError.geminiRequestFailed("HTTP \(httpResponse.statusCode)") |
Contributor
There was a problem hiding this comment.
Avoid logging the raw Gemini error body here.
The response body on API failures can echo prompt/request content, and Lines 150-151 currently emit it verbatim. Log the status code only, or mark the body private before sending it to HexLog. As per coding guidelines "Use the unified logging helper HexLog for all diagnostics ... and use privacy annotations (, privacy: .private) for sensitive data like transcript text or file paths."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Hex/Clients/RefinementClient.swift` around lines 149 - 152, The guard in
RefinementClient that currently builds errorBody and calls
refinementLogger.error with the raw response body should be changed to avoid
logging sensitive content; log only the HTTP status code (e.g., "Gemini API
error HTTP <statusCode>") and, if you must record the response body for
diagnostics, send it through HexLog/HexLog helper with a privacy annotation
(privacy: .private) instead of refinementLogger.error; update the thrown
RefinementError.geminiRequestFailed call (and the guard block around it) to not
include the raw body and ensure refinementLogger/HexLog usage follows the
codebase's unified logging pattern.
Comment on lines
+266
to
+283
| @Shared(.isSettingCycleToneHotkey) var isSettingCycleToneHotkey: Bool | ||
| @Shared(.hexSettings) var hexSettings: HexSettings | ||
|
|
||
| let token = keyEventMonitor.handleKeyEvent { keyEvent in | ||
| if isSettingCycleToneHotkey { return false } | ||
|
|
||
| guard let toneHotkey = hexSettings.cycleToneHotkey, | ||
| let key = keyEvent.key, | ||
| key == toneHotkey.key, | ||
| keyEvent.modifiers.matchesExactly(toneHotkey.modifiers) else { | ||
| return false | ||
| } | ||
|
|
||
| MainActor.assumeIsolated { | ||
| send(.cycleTone) | ||
| } | ||
| return true | ||
| } |
Contributor
There was a problem hiding this comment.
Pause this monitor while other hotkeys are being recorded.
Lines 266-283 only check isSettingCycleToneHotkey. If the current cycle-tone chord is pressed while Settings is capturing the main dictation hotkey or the paste-last hotkey, this effect can intercept the event and fire .cycleTone instead of letting Settings record it. Gate this effect on the other hotkey-recording flags too.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Hex/Features/App/AppFeature.swift` around lines 266 - 283, The key event
handler currently only pauses when isSettingCycleToneHotkey is true, so add the
other hotkey-recording flags (e.g. isSettingDictationHotkey and
isSettingPasteLastHotkey) to the initial gate so the monitor returns false while
any hotkey is being recorded; update the check in keyEventMonitor.handleKeyEvent
(or the initial if) to short-circuit if isSettingCycleToneHotkey ||
isSettingDictationHotkey || isSettingPasteLastHotkey, keeping the rest of the
guard logic that matches hexSettings.cycleToneHotkey and the send(.cycleTone)
call unchanged.
Comment on lines
+468
to
+496
| let currentAppBundleID = await MainActor.run { | ||
| NSWorkspace.shared.frontmostApplication?.bundleIdentifier | ||
| } | ||
| let supportsUndo = !TerminalAppDetector.isTerminal(currentAppBundleID) | ||
|
|
||
| // Paste placeholder at cursor while refining (skip for terminal apps) | ||
| let placeholder = refinementMode == .refined ? "Refining\u{2026}" : "Summarizing\u{2026}" | ||
| if supportsUndo { | ||
| await pasteboard.paste(placeholder) | ||
| didPastePlaceholder = true | ||
| } | ||
|
|
||
| finalText = try await refinement.refine(modifiedResult, refinementMode, refinementTone, refinementProvider, geminiAPIKey) | ||
|
|
||
| guard !finalText.isEmpty else { | ||
| if didPastePlaceholder { | ||
| await pasteboard.undoAndReplace("") | ||
| didPastePlaceholder = false | ||
| } | ||
| await send(.transcriptionCompleted) | ||
| return | ||
| } | ||
|
|
||
| if didPastePlaceholder { | ||
| await pasteboard.undoAndReplace(finalText) | ||
| didPastePlaceholder = false | ||
| } else { | ||
| await pasteboard.paste(finalText) | ||
| } |
Contributor
There was a problem hiding this comment.
undoAndReplace can undo the wrong thing after the refinement delay.
Once Lines 476-477 paste Refining…/Summarizing…, this effect can sit for up to 15 seconds before Lines 492 or 519 call undoAndReplace. Hex/Clients/PasteboardClient.swift Lines 338-357 always send a global Cmd+Z, so if the user switches apps or types anything before refinement finishes, the success/error path can undo unrelated work and paste into the wrong target. Re-check the frontmost app before every undo/replace and skip the inline placeholder whenever you can no longer guarantee you are undoing the placeholder paste.
Also applies to: 517-520
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Hex/Features/Transcription/TranscriptionFeature.swift` around lines 468 -
496, Capture the frontmost app bundle ID before inserting the placeholder
(already done into currentAppBundleID), then before any undoAndReplace or
placeholder-replacement logic (the calls to pasteboard.undoAndReplace and
pasteboard.paste after refinement.refine and in the empty-result branch)
re-check NSWorkspace.shared.frontmostApplication?.bundleIdentifier on the
MainActor and compare it to the saved currentAppBundleID; only perform
undoAndReplace to remove the placeholder if the bundle IDs still match,
otherwise treat the placeholder as lost — do not send a global undo (skip
undoAndReplace), clear didPastePlaceholder, and directly paste finalText (or
skip pasting empty replacement) so we never undo unrelated user work; apply this
check for both the success and empty-result branches that reference
didPastePlaceholder.
Comment on lines
516
to
521
| } catch { | ||
| // Clean up placeholder if it was pasted before the error/cancellation | ||
| if didPastePlaceholder { | ||
| await pasteboard.undoAndReplace("") | ||
| } | ||
| await send(.transcriptionError(error, audioURL)) |
Contributor
There was a problem hiding this comment.
Explicit cancellation still falls into the error path here.
refinement.refine(...) is designed to rethrow CancellationError, but the catch-all at Lines 516-521 immediately forwards that into .transcriptionError. That turns an intentional cancel into an error state instead of a silent abort. Handle CancellationError separately after any placeholder cleanup and then return.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Hex/Features/Transcription/TranscriptionFeature.swift` around lines 516 -
521, The catch-all currently forwards any thrown error (including
CancellationError from refinement.refine(...)) into
send(.transcriptionError(error, audioURL)); change the catch logic so that after
performing the placeholder cleanup via pasteboard.undoAndReplace("") when
didPastePlaceholder is true, you detect CancellationError (or Task.isCancelled)
and return early without calling send(.transcriptionError...), otherwise
continue to send the transcription error; reference the symbols
refinement.refine, didPastePlaceholder, pasteboard.undoAndReplace(_:) and
send(.transcriptionError(error, audioURL)) when making the change.
Comment on lines
+8
to
+16
| public static func stripLeakedTags(_ text: String) -> String { | ||
| var result = text.trimmingCharacters(in: .whitespacesAndNewlines) | ||
| if result.hasPrefix("Text:") { | ||
| result = String(result.dropFirst(5)) | ||
| } | ||
| return result | ||
| .trimmingCharacters(in: .whitespacesAndNewlines) | ||
| .trimmingCharacters(in: .init(charactersIn: "\"")) | ||
| .trimmingCharacters(in: .whitespacesAndNewlines) |
Contributor
There was a problem hiding this comment.
Only trim surrounding quotes when a leaked Text: wrapper was actually detected.
"Text: hello" still keeps the leaked prefix because the opening quote blocks the hasPrefix("Text:") check, but any legitimate quoted output like "Hello" loses its quotes because Lines 14-15 always trim them. Make the quote stripping conditional on a detected leading wrapper.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@HexCore/Sources/HexCore/Logic/RefinementTextProcessor.swift` around lines 8 -
16, The function stripLeakedTags currently always strips surrounding quotes even
when no "Text:" wrapper was present; change it so quote removal only happens
when a leaked wrapper is detected: in stripLeakedTags(_:) compute a boolean
(e.g., hadWrapper) by checking result.hasPrefix("Text:") or
result.hasPrefix("\"Text:") (or by peeking past an initial quote) and only
perform the trimming of surrounding quotes on result if hadWrapper is true; keep
the existing logic that drops the leading "Text:" when detected and use the same
result variable for these conditional trims.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Hex/Features/Transcription/TranscriptionFeature.swift (1)
463-525:⚠️ Potential issue | 🟡 MinorConsider handling
CancellationErrorseparately in the catch block.The catch block at Lines 517-522 treats all errors the same, including
CancellationErrorfrom task cancellation. An explicit cancel (via.cancelaction) will now show as a transcription error instead of a silent abort. Consider returning early for cancellation:🐛 Suggested handling for CancellationError
} catch { // Clean up placeholder if it was pasted before the error/cancellation if didPastePlaceholder { await pasteboard.undoAndReplace("") } + // Explicit cancellation should not be reported as an error + if error is CancellationError || Task.isCancelled { + return + } await send(.transcriptionError(error, audioURL)) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Hex/Features/Transcription/TranscriptionFeature.swift` around lines 463 - 525, The catch block in the transcription run currently treats all errors (including CancellationError) as real errors and sends .transcriptionError; modify the catch so it first checks for Task.isCancelled or that the caught error is a CancellationError and if so clean up any pasted placeholder (using didPastePlaceholder and pasteboard.undoAndReplace("")) and then return/exit without calling send(.transcriptionError); otherwise keep the existing placeholder cleanup and send(.transcriptionError(error, audioURL)). Ensure you update the catch near the run closure that uses didPastePlaceholder, pasteboard, and send(.transcriptionError(...)).
♻️ Duplicate comments (3)
Hex/Features/App/AppFeature.swift (2)
304-336:⚠️ Potential issue | 🟡 MinorPause tone-cycle monitor while other hotkeys are being recorded.
This monitoring effect only checks
isSettingCycleToneHotkeyat Line 311. If the user presses their cycle-tone chord while Settings is capturing the main dictation hotkey, paste-last hotkey, or refine-selection hotkey, this monitor will intercept the event and fire.cycleToneinstead of letting Settings record it.🐛 Proposed fix to gate on all capture states
private func startToneCycleMonitoring() -> Effect<Action> { .run { send in `@Shared`(.isSettingCycleToneHotkey) var isSettingCycleToneHotkey: Bool + `@Shared`(.isSettingHotKey) var isSettingHotKey: Bool + `@Shared`(.isSettingPasteLastTranscriptHotkey) var isSettingPasteLastTranscriptHotkey: Bool + `@Shared`(.isSettingRefineSelectionHotkey) var isSettingRefineSelectionHotkey: Bool `@Shared`(.hexSettings) var hexSettings: HexSettings let token = keyEventMonitor.handleKeyEvent { keyEvent in - if isSettingCycleToneHotkey { return false } + if isSettingCycleToneHotkey || isSettingHotKey || isSettingPasteLastTranscriptHotkey || isSettingRefineSelectionHotkey { + return false + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Hex/Features/App/AppFeature.swift` around lines 304 - 336, The tone-cycle monitor currently only checks `@Shared`(.isSettingCycleToneHotkey) before handling key events, so it can intercept the cycle-tone chord while other hotkey captures are active; update the keyEventMonitor.handler in startToneCycleMonitoring to also read the other capture flags (e.g. `@Shared`(.isSettingDictationHotkey), `@Shared`(.isSettingPasteLastHotkey), `@Shared`(.isSettingRefineSelectionHotkey) or whatever the actual Shared keys are for those settings) and return false if any of those flags are true; ensure you still use hexSettings.cycleToneHotkey and send(.cycleTone) as before but only when no capture flags are set, and keep the existing token cancellation/withTaskCancellationHandler behavior.
338-370:⚠️ Potential issue | 🟡 MinorPause refine-selection monitor while other hotkeys are being recorded.
Same issue as tone-cycle monitoring — only checks
isSettingRefineSelectionHotkeyat Line 345. Gate on all capture states.🐛 Proposed fix
private func startRefineSelectionMonitoring() -> Effect<Action> { .run { send in `@Shared`(.isSettingRefineSelectionHotkey) var isSettingRefineSelectionHotkey: Bool + `@Shared`(.isSettingHotKey) var isSettingHotKey: Bool + `@Shared`(.isSettingPasteLastTranscriptHotkey) var isSettingPasteLastTranscriptHotkey: Bool + `@Shared`(.isSettingCycleToneHotkey) var isSettingCycleToneHotkey: Bool `@Shared`(.hexSettings) var hexSettings: HexSettings let token = keyEventMonitor.handleKeyEvent { keyEvent in - if isSettingRefineSelectionHotkey { return false } + if isSettingRefineSelectionHotkey || isSettingHotKey || isSettingPasteLastTranscriptHotkey || isSettingCycleToneHotkey { + return false + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Hex/Features/App/AppFeature.swift` around lines 338 - 370, The key handler in startRefineSelectionMonitoring currently only checks isSettingRefineSelectionHotkey before matching the refine hotkey; update the closure passed to keyEventMonitor.handleKeyEvent to gate on all hotkey-capture state flags (not just isSettingRefineSelectionHotkey) so the refine-selection monitor is paused while any other hotkey is being recorded—read the other `@Shared` boolean flags (the same capture-state flags used in the tone-cycle monitor) alongside isSettingRefineSelectionHotkey before processing hexSettings.refineSelectionHotkey and sending .refineSelection.Hex/Features/Transcription/TranscriptionFeature.swift (1)
468-496:⚠️ Potential issue | 🟠 Major
undoAndReplacemay undo unrelated work if the user switches apps during refinement.Refinement can take up to 15 seconds (per PR description). If the user switches apps or types between placeholder paste (Lines 476-479) and
undoAndReplace(Lines 493 or 520), the global Cmd+Z will undo work in the wrong app. Consider re-checking the frontmost app before callingundoAndReplaceand skipping the undo if it changed.💡 Suggested mitigation
if didPastePlaceholder { + // Re-check frontmost app to avoid undoing in wrong context + let newAppBundleID = await MainActor.run { + NSWorkspace.shared.frontmostApplication?.bundleIdentifier + } + guard newAppBundleID == currentAppBundleID else { + // App changed - just paste without undo to avoid corrupting other app + await pasteboard.paste(finalText) + didPastePlaceholder = false + // Continue to sound and storage + soundEffect.play(.pasteTranscript) + try await storeTranscript(...) + await send(.transcriptionCompleted) + return + } await pasteboard.undoAndReplace(finalText) didPastePlaceholder = false } else {Apply similar logic to the empty-result branch at Lines 484-490.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Hex/Features/Transcription/TranscriptionFeature.swift` around lines 468 - 496, The pasted placeholder may be undone in the wrong app if the frontmost application changed during the await; before calling pasteboard.undoAndReplace (both in the empty-result branch after refinement.refine and in the success branch where finalText is applied), re-check the current frontmost application using MainActor.run to get NSWorkspace.shared.frontmostApplication?.bundleIdentifier and compare it to the saved currentAppBundleID (or re-evaluate TerminalAppDetector.isTerminal if needed); only perform undoAndReplace when the bundle ID matches (otherwise skip the undo and directly paste finalText or do nothing), and ensure didPastePlaceholder is reset appropriately in both branches.
🧹 Nitpick comments (2)
Hex/Features/Settings/SettingsFeature.swift (1)
231-233: Remove unnecessary parentheses around the condition.SwiftLint flags this as a
control_statementviolation.🧹 Suggested fix
- if (target == .pasteLastTranscript || target == .cycleTone || target == .refineSelection), keyEvent.key != nil, updatedModifiers.isEmpty { + if target == .pasteLastTranscript || target == .cycleTone || target == .refineSelection, keyEvent.key != nil, updatedModifiers.isEmpty { return .none }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Hex/Features/Settings/SettingsFeature.swift` around lines 231 - 233, The if-statement in SettingsFeature.swift uses unnecessary parentheses around the compound condition; remove the outer parentheses so the conditional reads directly as if target == .pasteLastTranscript || target == .cycleTone || target == .refineSelection, keyEvent.key != nil, updatedModifiers.isEmpty to satisfy SwiftLint's control_statement rule—update the conditional inside the method that evaluates target/keyEvent.key/updatedModifiers (the existing if that currently wraps the whole condition in parentheses) to the parenthesis-free form.HexCore/Tests/HexCoreTests/RefinementSettingsTests.swift (1)
78-91: Avoid force unwrapping in test code.While this is a test file and the force unwrap is safe for a hardcoded valid JSON string, using
try XCTUnwrapis more idiomatic and produces better failure messages.🧹 Suggested improvement
func testRefinementFieldsDecodeDefaultsWhenMissing() throws { // Simulate settings JSON from before refinement feature existed let oldJSON = "{\"soundEffectsEnabled\":true}" - let data = oldJSON.data(using: .utf8)! + let data = try XCTUnwrap(oldJSON.data(using: .utf8)) let decoded = try JSONDecoder().decode(HexSettings.self, from: data)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@HexCore/Tests/HexCoreTests/RefinementSettingsTests.swift` around lines 78 - 91, In testRefinementFieldsDecodeDefaultsWhenMissing replace the force unwrap of the JSON Data (the expression oldJSON.data(using: .utf8)!) with an idiomatic test unwrap using try XCTUnwrap so failures produce useful diagnostics; specifically, change the creation of the data variable to use try XCTUnwrap(oldJSON.data(using: .utf8)) and keep the rest of the test (JSONDecoder().decode(HexSettings.self, from: data) and the XCTAssert* checks) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Hex/Features/App/AppFeature.swift`:
- Around line 234-264: In showToneNotification(_:) the code uses the original
settings after calling requestAuthorization, so if the user just granted
permission we still treat it as denied; after calling
center.requestAuthorization(...) (when settings.authorizationStatus ==
.notDetermined) re-fetch the authorization state by awaiting
center.notificationSettings() again (or use the boolean granted returned by
requestAuthorization) and then use the refreshed settings.authorizationStatus in
the guard before creating the UNNotificationRequest.
- Around line 118-154: The code reads the clipboard directly in AppFeature.swift
(inside the .refineSelection handler) using NSPasteboard.general.string(forType:
.string); add a read operation to the PasteboardClient and use it instead: add
var read: `@Sendable` () async -> String? to the PasteboardClient protocol/type,
implement that read in PasteboardClientLive to return the NSPasteboard string,
and replace the direct NSPasteboard.general.string(...) call in the
.refineSelection block with await pasteboard.read(); ensure the rest of the flow
(guarding nil/empty and tests) remains unchanged.
---
Outside diff comments:
In `@Hex/Features/Transcription/TranscriptionFeature.swift`:
- Around line 463-525: The catch block in the transcription run currently treats
all errors (including CancellationError) as real errors and sends
.transcriptionError; modify the catch so it first checks for Task.isCancelled or
that the caught error is a CancellationError and if so clean up any pasted
placeholder (using didPastePlaceholder and pasteboard.undoAndReplace("")) and
then return/exit without calling send(.transcriptionError); otherwise keep the
existing placeholder cleanup and send(.transcriptionError(error, audioURL)).
Ensure you update the catch near the run closure that uses didPastePlaceholder,
pasteboard, and send(.transcriptionError(...)).
---
Duplicate comments:
In `@Hex/Features/App/AppFeature.swift`:
- Around line 304-336: The tone-cycle monitor currently only checks
`@Shared`(.isSettingCycleToneHotkey) before handling key events, so it can
intercept the cycle-tone chord while other hotkey captures are active; update
the keyEventMonitor.handler in startToneCycleMonitoring to also read the other
capture flags (e.g. `@Shared`(.isSettingDictationHotkey),
`@Shared`(.isSettingPasteLastHotkey), `@Shared`(.isSettingRefineSelectionHotkey) or
whatever the actual Shared keys are for those settings) and return false if any
of those flags are true; ensure you still use hexSettings.cycleToneHotkey and
send(.cycleTone) as before but only when no capture flags are set, and keep the
existing token cancellation/withTaskCancellationHandler behavior.
- Around line 338-370: The key handler in startRefineSelectionMonitoring
currently only checks isSettingRefineSelectionHotkey before matching the refine
hotkey; update the closure passed to keyEventMonitor.handleKeyEvent to gate on
all hotkey-capture state flags (not just isSettingRefineSelectionHotkey) so the
refine-selection monitor is paused while any other hotkey is being recorded—read
the other `@Shared` boolean flags (the same capture-state flags used in the
tone-cycle monitor) alongside isSettingRefineSelectionHotkey before processing
hexSettings.refineSelectionHotkey and sending .refineSelection.
In `@Hex/Features/Transcription/TranscriptionFeature.swift`:
- Around line 468-496: The pasted placeholder may be undone in the wrong app if
the frontmost application changed during the await; before calling
pasteboard.undoAndReplace (both in the empty-result branch after
refinement.refine and in the success branch where finalText is applied),
re-check the current frontmost application using MainActor.run to get
NSWorkspace.shared.frontmostApplication?.bundleIdentifier and compare it to the
saved currentAppBundleID (or re-evaluate TerminalAppDetector.isTerminal if
needed); only perform undoAndReplace when the bundle ID matches (otherwise skip
the undo and directly paste finalText or do nothing), and ensure
didPastePlaceholder is reset appropriately in both branches.
---
Nitpick comments:
In `@Hex/Features/Settings/SettingsFeature.swift`:
- Around line 231-233: The if-statement in SettingsFeature.swift uses
unnecessary parentheses around the compound condition; remove the outer
parentheses so the conditional reads directly as if target ==
.pasteLastTranscript || target == .cycleTone || target == .refineSelection,
keyEvent.key != nil, updatedModifiers.isEmpty to satisfy SwiftLint's
control_statement rule—update the conditional inside the method that evaluates
target/keyEvent.key/updatedModifiers (the existing if that currently wraps the
whole condition in parentheses) to the parenthesis-free form.
In `@HexCore/Tests/HexCoreTests/RefinementSettingsTests.swift`:
- Around line 78-91: In testRefinementFieldsDecodeDefaultsWhenMissing replace
the force unwrap of the JSON Data (the expression oldJSON.data(using: .utf8)!)
with an idiomatic test unwrap using try XCTUnwrap so failures produce useful
diagnostics; specifically, change the creation of the data variable to use try
XCTUnwrap(oldJSON.data(using: .utf8)) and keep the rest of the test
(JSONDecoder().decode(HexSettings.self, from: data) and the XCTAssert* checks)
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8f4a172c-f85f-408b-b09c-f3c58a769a42
📒 Files selected for processing (6)
Hex/Features/App/AppFeature.swiftHex/Features/Settings/RefinementSectionView.swiftHex/Features/Settings/SettingsFeature.swiftHex/Features/Transcription/TranscriptionFeature.swiftHexCore/Sources/HexCore/Settings/HexSettings.swiftHexCore/Tests/HexCoreTests/RefinementSettingsTests.swift
🚧 Files skipped from review as they are similar to previous changes (2)
- Hex/Features/Settings/RefinementSectionView.swift
- HexCore/Sources/HexCore/Settings/HexSettings.swift
Comment on lines
+118
to
+154
| case .refineSelection: | ||
| let mode = state.settings.hexSettings.refinementMode | ||
| let tone = state.settings.hexSettings.refinementTone | ||
| let provider = state.settings.hexSettings.refinementProvider | ||
| let apiKey = state.settings.hexSettings.geminiAPIKey | ||
|
|
||
| guard mode != .raw else { return .none } | ||
|
|
||
| return .run { [refinement, pasteboard] _ in | ||
| // Copy current selection to clipboard via Cmd+C | ||
| await MainActor.run { | ||
| let source = CGEventSource(stateID: .combinedSessionState) | ||
| let cKey: CGKeyCode = 8 | ||
| let keyDown = CGEvent(keyboardEventSource: source, virtualKey: cKey, keyDown: true) | ||
| keyDown?.flags = .maskCommand | ||
| keyDown?.post(tap: .cghidEventTap) | ||
| let keyUp = CGEvent(keyboardEventSource: source, virtualKey: cKey, keyDown: false) | ||
| keyUp?.flags = .maskCommand | ||
| keyUp?.post(tap: .cghidEventTap) | ||
| } | ||
|
|
||
| // Wait for clipboard to update | ||
| try? await Task.sleep(for: .milliseconds(150)) | ||
|
|
||
| // Read the selected text from clipboard | ||
| let selectedText = await MainActor.run { | ||
| NSPasteboard.general.string(forType: .string) | ||
| } | ||
|
|
||
| guard let selectedText, !selectedText.isEmpty else { return } | ||
|
|
||
| // Refine the selected text | ||
| let refined = try await refinement.refine(selectedText, mode, tone, provider, apiKey) | ||
|
|
||
| // Paste the refined text back (replaces the selection) | ||
| await pasteboard.paste(refined) | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if PasteboardClient has a read/get method
ast-grep --pattern $'struct PasteboardClient {
$$$
}'
rg -n "PasteboardClient" --type swift -A 10 | head -60Repository: kitlangton/Hex
Length of output: 3875
🏁 Script executed:
#!/bin/bash
# Check if there are other direct NSPasteboard.general accesses
rg "NSPasteboard\.general" --type swift -n
# Get full PasteboardClient struct definition
cat -n Hex/Clients/PasteboardClient.swift | head -30Repository: kitlangton/Hex
Length of output: 1481
🏁 Script executed:
#!/bin/bash
# Get more context around the other NSPasteboard.general uses
cat -n Hex/Clients/PasteboardClient.swift | sed -n '90,110p'
cat -n Hex/Clients/PasteboardClient.swift | sed -n '200,215p'
cat -n Hex/Clients/PasteboardClient.swift | sed -n '255,270p'
# Check the full PasteboardClient struct definition and methods
rg -A 50 "^struct PasteboardClient" Hex/Clients/PasteboardClient.swift | head -60Repository: kitlangton/Hex
Length of output: 4201
🏁 Script executed:
#!/bin/bash
# Check how AppFeature.swift uses the pasteboard dependency elsewhere
rg -B 5 -A 5 "pasteboard" Hex/Features/App/AppFeature.swift | grep -E "(pasteboard|await)" | head -20
# Check if there's a testValue or preview value for PasteboardClient
rg -A 15 "extension PasteboardClient" Hex/Clients/PasteboardClient.swift | grep -E "testValue|previewValue" -A 10Repository: kitlangton/Hex
Length of output: 371
🏁 Script executed:
#!/bin/bash
# Check if there are other cases in AppFeature where clipboard is read
rg -n "NSPasteboard\|Pasteboard" Hex/Features/App/AppFeature.swift
# Check what other dependencies exist in AppFeature for reference
rg "@Dependency" Hex/Features/App/AppFeature.swift | head -10Repository: kitlangton/Hex
Length of output: 290
Add a read method to PasteboardClient and use it instead of directly accessing NSPasteboard.general.
AppFeature.swift line 144 reads from the clipboard directly (NSPasteboard.general.string(forType: .string)) instead of using the injected pasteboard dependency. PasteboardClient currently lacks a read method, creating an asymmetry where write operations (paste, copy) are dependency-injected but reads bypass the dependency. Add var read: @sendable () async -> String? to PasteboardClient and implement it in PasteboardClientLive to make clipboard reads testable and consistent with the dependency injection pattern.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Hex/Features/App/AppFeature.swift` around lines 118 - 154, The code reads the
clipboard directly in AppFeature.swift (inside the .refineSelection handler)
using NSPasteboard.general.string(forType: .string); add a read operation to the
PasteboardClient and use it instead: add var read: `@Sendable` () async -> String?
to the PasteboardClient protocol/type, implement that read in
PasteboardClientLive to return the NSPasteboard string, and replace the direct
NSPasteboard.general.string(...) call in the .refineSelection block with await
pasteboard.read(); ensure the rest of the flow (guarding nil/empty and tests)
remains unchanged.
Comment on lines
+234
to
+264
| @MainActor | ||
| private static func showToneNotification(_ tone: RefinementTone) async { | ||
| let center = UNUserNotificationCenter.current() | ||
|
|
||
| // Ensure notification permission is granted | ||
| let settings = await center.notificationSettings() | ||
| if settings.authorizationStatus == .notDetermined { | ||
| _ = try? await center.requestAuthorization(options: [.alert]) | ||
| } | ||
| guard settings.authorizationStatus != .denied else { return } | ||
|
|
||
| let label: String = switch tone { | ||
| case .natural: "Natural" | ||
| case .professional: "Professional" | ||
| case .casual: "Casual" | ||
| case .concise: "Concise" | ||
| case .friendly: "Friendly" | ||
| } | ||
|
|
||
| let content = UNMutableNotificationContent() | ||
| content.title = "Hex Tone" | ||
| content.body = label | ||
| content.sound = nil | ||
|
|
||
| let request = UNNotificationRequest( | ||
| identifier: "hex-tone-cycle", | ||
| content: content, | ||
| trigger: nil | ||
| ) | ||
| try? await center.add(request) | ||
| } |
Contributor
There was a problem hiding this comment.
Authorization check uses stale settings after requesting permission.
The code fetches notificationSettings() at Line 239, then requests authorization if notDetermined at Line 241, but the guard at Line 243 still uses the original settings object which hasn't been refreshed. If permission was just granted, notifications will still be skipped until next invocation.
🐛 Proposed fix
`@MainActor`
private static func showToneNotification(_ tone: RefinementTone) async {
let center = UNUserNotificationCenter.current()
- // Ensure notification permission is granted
- let settings = await center.notificationSettings()
- if settings.authorizationStatus == .notDetermined {
+ // Ensure notification permission is granted
+ var settings = await center.notificationSettings()
+ if settings.authorizationStatus == .notDetermined {
_ = try? await center.requestAuthorization(options: [.alert])
+ settings = await center.notificationSettings()
}
guard settings.authorizationStatus != .denied else { return }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Hex/Features/App/AppFeature.swift` around lines 234 - 264, In
showToneNotification(_:) the code uses the original settings after calling
requestAuthorization, so if the user just granted permission we still treat it
as denied; after calling center.requestAuthorization(...) (when
settings.authorizationStatus == .notDetermined) re-fetch the authorization state
by awaiting center.notificationSettings() again (or use the boolean granted
returned by requestAuthorization) and then use the refreshed
settings.authorizationStatus in the guard before creating the
UNNotificationRequest.
|
I think this is cool. What will be cooler is if we can somehow run a transform script, any arbitrary script, after every Speech to text invocation. That way I can use any tool rather than sticking to Apple intelligence or Gemini |
|
This is exactly what I was hoping someone would build for Hex, really clean implementation. |
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
3 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.
Problem
Hex produces great raw transcriptions, but dictated speech often contains filler words, grammar issues, and loose phrasing. Users currently have to manually clean up transcriptions before sending them in professional contexts.
Proposed Solution
Transcription refinement that post-processes dictated text using on-device Apple Intelligence (macOS 26+) or Google Gemini Flash before pasting. Follows Hex's existing TCA architecture and patterns.
What it does
Robustness
Architecture
RefinementClient— TCA dependency, same pattern asTranscriptionClientandRecordingClientRefinementTextProcessor— pure text-processing logic in HexCore (fully testable)RefinementPromptBuilder— prompt generation in HexCore (fully testable)TerminalAppDetector— identifies apps that don't support undo (in HexCore)Files changed
12 new files, 10 modified — 1,306 lines added, 29 removed.
Test plan
Screenshots
Settings: Transcription Refinement (Apple Intelligence)
Mode, tone, hotkey configuration, and Apple Intelligence provider selected.
Settings: Transcription Refinement (Gemini Flash)
Same section with Gemini Flash selected, showing the API key field.
Menu Bar Indicator
The "P" suffix indicates Refined mode with Professional tone.
Questions for maintainer
Happy to adjust the implementation based on your feedback.
Summary by CodeRabbit
New Features
Tests
Chores