feat: switch refinement models from menu bar - #274
blackforestboi wants to merge 36 commits into
Conversation
Add downstream refinement through Apple Intelligence, Gemini, and OpenRouter, with securely stored provider credentials and OpenRouter model selection.\n\nAdd a refined-transcription hotkey, optional spoken instructions, selected-text capture and in-place replacement, a menu-bar selection action, dedicated refinement feedback, and an Include selected text setting.\n\nBuild one shared refinement prompt before provider dispatch so every adapter receives identical base rules, custom instructions, optional spoken instruction, and source material. Preserve existing audio lifecycle, cancellation, persistence, and transcript behavior.\n\nInspired by upstream PR kitlangton#191, "add transcription refinement with Apple Intelligence and Gemini", by Rohit Lal Chandani. Thanks to Rohit for the original implementation and direction.
…efinement action.
Recording used a short wall-clock grace period before closing its capture file, so late Core Audio callbacks could lose the final spoken frames. Finalization now uses the capture timestamp to drain and trim PCM through the stop boundary, while rejecting incomplete captures instead of transcribing a known partial file.
The capture finalizer uses the Core Audio host-time boundary to retain all PCM frames recorded before the stop event. This exposes an explicit post-stop inclusion delay in Hot Key settings for people who want a deliberate tail instead of relying on a hardcoded grace period.\n\nThe setting defaults to 0 ms, preserving the exact stop boundary by default while still allowing an intentional audio tail when configured.
Store the processed source transcript beside its refined result while sharing one recording safely. Keep the recording until the final linked history entry is removed.
Keep the numeric field unlabeled so its placeholder cannot wrap in the trailing value column. Add a timer icon to the setting label while preserving an accessibility label for the value field.
Reset the regular hotkey processor when the refinement shortcut ends a regular recording. This prevents a retained double-tap lock from swallowing subsequent refinement shortcut presses while preserving the refinement hotkey's own double-tap behavior.
Bring back the established modifier-only double-tap-only state machine that was omitted when refinement work was merged into local main. A second press starts the lock and the next press stops it.
Add pause and resume controls beside the timeline, start playback from slider seeks, and keep playback progress state synchronized with the audio player.
Add activation-time display capture, local OCR and image-analysis paths, persistent screen context, replayable history artifacts, and refined hotkey behavior. Screen-aware requests now prefer the selected refinement model, falling back to a vision model only when an uploaded image requires it.
New and migrated settings now start with the voice memo refinement guidelines while remaining editable in Settings.
Clearing the API-key field now deletes its Keychain value instead of allowing the saved credential to repopulate the field.
Add direct OpenAI and Anthropic refinement with secure API-key entry, account-specific model selection, and screen-aware image support. Also improve refinement settings layout and screen-recording permission handling.
Improve the refinement settings header and grouping for provider controls.
Use a checked-in ICNS fallback so GitHub's macOS release runner packages the Hex icon correctly.
Apply pending changesets, synchronize the in-app changelog, and update release metadata for the minor 2026.8.0 release.
Use the project's date-based release number instead of a semantic minor version.
Add a menu-bar History command that opens the window on the History tab, while keeping Settings navigation explicit.
Stop probing the legacy Documents folder during startup. Current settings, history, and the model catalog now resolve directly to Application Support.
Replace the opaque rectangular matte with true alpha-masked light and dark icon assets, and select the inverse palette when macOS appearance changes.
Rebrand the application as Octo while adding CLI refinement providers, durable recording recovery, and a unified hotkey flow for refinement and screen-aware capture. Screen-aware capture now supports deliberate region selection with repositioning, full-display click fallback, and dependable indicator cleanup.
Keep selected text as refinement context while pasting generated output at the active cursor. Update menu bar refinement and race coverage to match the unified paste behavior.
Add a native hover-opened submenu backed by the active refinement provider's model catalog. Persist selections in the existing per-provider settings slots, protect against stale async provider results, and preserve unavailable saved selections without clearing them. Include focused state coverage and a patch changeset. Further build/test execution was skipped at the user's request.
📝 WalkthroughWalkthroughThis PR rebrands the application as Octo and adds release automation, unified hotkey refinement, screen-aware region capture, durable recording recovery, multiple refinement providers, expanded history and settings interfaces, new menu-bar interactions, and extensive migration and integration tests. ChangesOcto application overhaul
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
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)
1156-1221: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winClear temp audio ownership after handing it to
transcriptionResult. When checkpoint creation is enabled but fails, thecatchonly logs and leavesunownedAudioURLpointing atcapturedURL; thedeferthen removes it after the capturing effect returns, beforefinalizeRecordingAndStoreTranscriptcan callpersistHistoryEntry(saveTranscriptionHistory: true). SincepersistHistoryEntryonly succeeds aftertranscriptPersistence.savecompletes, the succeeded transcription can lose its history row. Decouple cleanup from whether a checkpoint was created, or only own/cleanup the original file while usingcheckpoint.audioPathafter a successful checkpoint.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Hex/Features/Transcription/TranscriptionFeature.swift` around lines 1156 - 1221, Update the ownership cleanup around the recording stop flow and the checkpoint persistence catch so a failed checkpoint does not leave unownedAudioURL pointing to capturedURL for defer cleanup. Preserve the original captured file for transcription and history persistence when checkpoint creation fails, and only clear or transfer cleanup ownership after the audio has been handed to the downstream transcription result flow.
🟠 Major comments (26)
Hex/Clients/PasteboardClient.swift-153-165 (1)
153-165: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse the captured replacement contract.
Hex/App/MenuBarRefineSelectedTextButton.swift, Line 42 directly callspasteboard.paste(refinedText), bypassing this capture’s source-app and selection checks. If focus changes while refinement runs, output can be pasted into another app. InvokeselectedText.replace(with:)and handle.clipboardChanged/.pasteFailedinstead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Hex/Clients/PasteboardClient.swift` around lines 153 - 165, The refinement flow should use the captured replacement contract instead of calling pasteboard.paste directly. Update MenuBarRefineSelectedTextButton’s refinement completion to invoke selectedText.replace(with:) and handle the .clipboardChanged and .pasteFailed outcomes, preserving the source-app and selection validation provided by SelectedTextCapture.docs/plans/2026-07-23-003-feat-region-screen-capture-plan.md-71-75 (1)
71-75: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAlign the click assumption with the full-display contract.
R5, KTD4, and the test plan require a click to complete using the full display, but the assumption says “small click-sized capture.” Replace that assumption with an explicit full-display selection to avoid implementing the wrong crop behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/plans/2026-07-23-003-feat-region-screen-capture-plan.md` around lines 71 - 75, Update the “Assumptions” section of the screen-capture plan to replace the small click-sized capture assumption with an explicit full-display selection for click completion. Keep the existing permission-check assumption unchanged and align the wording with the full-display requirements in R5, KTD4, and the test plan.docs/plans/2026-07-23-001-refactor-unified-hotkey-refinement-plan.md-51-56 (1)
51-56: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMake terminal-refinement timing unambiguous.
KTD2 says to delay the stop output until key-up, but the same plan and the processor contract define refinement as triggering when the hold threshold is reached. Update KTD2 so threshold crossing stops/refines immediately; key-up should only classify quick terminal taps.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/plans/2026-07-23-001-refactor-unified-hotkey-refinement-plan.md` around lines 51 - 56, Update KTD2 to state that terminal refinement stops or refines immediately when the hold threshold is reached; reserve key-up handling for classifying quick terminal taps, and keep the wording consistent with the processor contract and KTD5/KTD6 threshold behavior.docs/plans/2026-07-23-002-fix-crash-resilient-recording-recovery-plan.md-75-78 (1)
75-78: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftReconcile durable frame records beyond stale manifest totals.
If the frame record is synchronized and the process exits before the manifest publishes its new totals, recovery can see a complete accepted frame that the manifest does not describe. Unless startup scans and validates that complete tail, the stated zero-frame-loss guarantee is false. Define this reconciliation explicitly or change the acceptance boundary.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/plans/2026-07-23-002-fix-crash-resilient-recording-recovery-plan.md` around lines 75 - 78, Update KTD3’s recovery protocol to reconcile the durable frame-record tail during startup: scan records beyond the manifest’s published totals, validate complete self-validating records, and incorporate any valid tail into recovered sequence/frame/byte totals before finalization. Define handling for incomplete or corrupt trailing records without treating them as accepted frames; otherwise move the acceptance boundary so the zero-frame-loss guarantee only covers frames represented by both synchronized record data and the published manifest.docs/plans/2026-07-23-001-refactor-unified-hotkey-refinement-plan.md-35-37 (1)
35-37: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAlign the double-tap eligibility contract across both documents.
The plan and semantics guide currently describe different availability rules for screenshot-start and terminal-refinement gestures. The processor distinguishes
useDoubleTapOnlyfromdoubleTapLockEnabled, so explicitly identify which setting controls eligibility and use that terminology consistently.
docs/plans/2026-07-23-001-refactor-unified-hotkey-refinement-plan.md#L35-L37: revise R9 to name the precise configuration requirement.docs/hotkey-semantics.md#L3-L7: replace “without double-tap lock” with the matching configuration terminology.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/plans/2026-07-23-001-refactor-unified-hotkey-refinement-plan.md` around lines 35 - 37, Align the double-tap eligibility terminology across both documents: in docs/plans/2026-07-23-001-refactor-unified-hotkey-refinement-plan.md lines 35-37, update R9 to explicitly identify the configuration setting that controls screenshot-start and terminal-refinement gesture eligibility; in docs/hotkey-semantics.md lines 3-7, replace “without double-tap lock” with the same configuration terminology, distinguishing useDoubleTapOnly from doubleTapLockEnabled as required by the processor.Hex/Clients/RecordingRecoveryStore.swift-25-40 (1)
25-40: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winPer-buffer
fsync+ manifest rewrite on the audio path is expensive.Every accepted buffer does
synchronize()(full fsync) plus a JSON encode and atomic manifest write (temp file + rename). At tap cadence that is several fsyncs and file replacements per second on the capture processing queue, which risks dropped buffers on slow disks or under load.Since recovery already relies on the physical frame count (
physicalFrameCount) rather than the manifest, the manifest does not need updating per buffer. Consider syncing/publishing on an interval (e.g. every ~1s) and onseal().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Hex/Clients/RecordingRecoveryStore.swift` around lines 25 - 40, Update append(_:) in RecordingRecoveryStore to avoid synchronizing the file and rewriting the manifest for every buffer; accumulate accepted frames and publish them on a roughly one-second interval, while preserving physical-frame-based recovery. Ensure seal() performs the final synchronize and manifest update so all captured frames are durable and published before sealing.Hex/Features/App/AppFeature.swift-124-151 (1)
124-151: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftRecovery entries are unreachable while history is disabled. Both sites store recovery transcripts inside
transcriptionHistory, butHistoryViewrenders the "History Disabled" placeholder wheneversaveTranscriptionHistoryis false — so recovered audio is retained on disk with no way for the user to view or delete it.
Hex/Features/App/AppFeature.swift#L124-L151: either gate the insert onsaveTranscriptionHistory, or route recovered items to a surface that is visible when history is off.Hex/Features/Settings/SettingsFeature.swift#L604-L621: the retainedrecoverySessionID != nilentries need the same visible surface (or explicit cleanup), otherwise preserving them just orphans the files.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Hex/Features/App/AppFeature.swift` around lines 124 - 151, The recovery flow must not retain entries that are invisible when saveTranscriptionHistory is disabled. In Hex/Features/App/AppFeature.swift lines 124-151, update interruptedRecordingsRecovered handling to gate insertion into transcriptionHistory on saveTranscriptionHistory or route those items to a visible surface. In Hex/Features/Settings/SettingsFeature.swift lines 604-621, apply the same visibility strategy to retained recoverySessionID entries, or explicitly clean up their associated files when history is disabled.Hex/Features/History/HistoryFeature.swift-141-147 (1)
141-147: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winAdd migration for the legacy history file path.
transcriptionHistorynow loads/writes fromStoredFileKey("transcription_history.json"), buttranscriptionHistoryURLdid not exist before this change. Add a startup migration that copies any pre-existing legacy history file to this new path, otherwise existing users start with an empty history after upgrading.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Hex/Features/History/HistoryFeature.swift` around lines 141 - 147, Update startup initialization for transcription history to migrate any existing legacy file to the new StoredFileKey("transcription_history.json") location before history loading occurs. Reuse transcriptionHistoryURL as the destination, copy only when the legacy file exists and the new file is absent, and leave the existing history flow unchanged afterward.Hex/Features/Transcription/TranscriptionFeature.swift-1424-1433 (1)
1424-1433: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winNo escape hatch if screen-context capture never completes. When the transcript arrives before the capture, the reducer parks it in
pendingScreenAwareTranscription, setsisRefining = true, and returns.none— there is no timer or fallback. Combined with the overlay's missing cancellation handling (seeHex/Clients/ScreenCaptureSelectionOverlay.swift), a stuck selection leaves the session pinned in "refining" with the transcript unpasted. Consider a bounded timeout that falls back tobeginRefinement(..., screenContext: nil).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Hex/Features/Transcription/TranscriptionFeature.swift` around lines 1424 - 1433, The screen-aware transcription branch in the reducer can remain indefinitely in isRefining when screen-context capture never completes. Add a bounded timeout for pendingScreenAwareTranscription that invokes beginRefinement with screenContext: nil, clears the pending state as appropriate, and preserves the existing immediate path when screen context arrives normally.CLAUDE.md-215-215 (1)
215-215: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winAlign the release procedure with the GitHub Actions workflow.
This section still directs operators to a local CLI, AWS/S3 credentials, and keychain notarization, while
docs/release-process.mdnow defines tag-triggered releases using repository secrets. Update the surrounding release prerequisites, steps, and troubleshooting or link to the canonical guide.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@CLAUDE.md` at line 215, Update the release procedure surrounding item 12 in CLAUDE.md to match the tag-triggered GitHub Actions workflow and the repository-secret-based process documented in docs/release-process.md. Remove or revise instructions requiring local CLI usage, AWS/S3 credentials, or keychain notarization, and update related prerequisites and troubleshooting; alternatively, replace the outdated procedure with a link to the canonical release guide.HexCore/Sources/HexCore/Settings/HexSettings.swift-181-183 (1)
181-183: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep image-capable providers on their own selected model.
For
.openAIor.anthropic,providerremains unchanged butmodelIDis replaced with an OpenRouter image-model ID. Those requests combine incompatible provider/model identifiers. UseselectedRefinementModelIDwhenever the selected provider supports image input; reserveimageModelID/screenAwareOpenRouterModelIDfor the OpenRouter fallback path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@HexCore/Sources/HexCore/Settings/HexSettings.swift` around lines 181 - 183, Update the modelID selection in the refinement settings construction so providers that support image input, including .openAI and .anthropic, retain selectedRefinementModelID. Only use imageModelID or screenAwareOpenRouterModelID when usesUploadedImage requires the .openRouter fallback selected by the provider expression.Hex/Info.plist-24-27 (1)
24-27: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winConfirm the new
SUPublicEDKeymatches the key used to sign the Octo appcast.A mismatch between this public key and the EdDSA private key used by the release signing step silently rejects every update ("update is improperly signed"), and existing installs signed with the old key cannot be migrated by changing this value alone. Verify the release workflow's signing secret corresponds to this key and that the feed at the new
SUFeedURLis populated before shipping.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Hex/Info.plist` around lines 24 - 27, Verify that the SUPublicEDKey value in the Info.plist matches the EdDSA public key corresponding to the private key used by the release workflow to sign the appcast, and confirm the new SUFeedURL feed is populated before shipping. Do not change the key unless it matches the signing secret; preserve existing-install migration expectations when validating the update configuration.Hex/Info.plist-15-19 (1)
15-19: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
NSAllowsArbitraryLoadsdisables ATS app-wide.All refinement providers (OpenAI/Anthropic/Gemini/OpenRouter) and the Sparkle feed are HTTPS, so a global ATS opt-out isn't needed and weakens transport guarantees. If this exists only for a specific local/dev host, scope it with
NSExceptionDomainsinstead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Hex/Info.plist` around lines 15 - 19, Remove the global NSAllowsArbitraryLoads setting from the NSAppTransportSecurity configuration in Info.plist. Preserve ATS defaults for the HTTPS refinement providers and Sparkle feed; if local development access requires an exception, replace the global opt-out with a narrowly scoped NSExceptionDomains entry for that host.octo.rb-2-5 (1)
2-5: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRelease version metadata is inconsistent across the rebrand.
Hex/Info.plist,package.json, andMARKETING_VERSIONall declare2026.7.24, but two release-facing artifacts disagree.
octo.rb#L2-L5: setversion "2026.7.24"(and a realsha256) so the interpolated release URL resolves to an actual tag.Hex/Resources/changelog.md#L3-L3: correct the## 2026.7.161heading to the shipped version.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@octo.rb` around lines 2 - 5, Update octo.rb lines 2-5 by setting the release version to 2026.7.24 and replacing sha256 :no_check with the actual archive checksum; keep the interpolated URL aligned with that version. Also update Hex/Resources/changelog.md line 3 from 2026.7.161 to 2026.7.24.Hex.xcodeproj/project.pbxproj-400-400 (1)
400-400: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winProject-level
DEVELOPMENT_TEAMchange is shadowed by the target-level value.Both app target configs still set
DEVELOPMENT_TEAM = QC99C9JE59(Lines 500 and 555), which overrides the new project-level5YUPQC9D96. If the rebrand is meant to sign under the new team, the target settings must be updated too; otherwise the project-level change has no effect on the shipped app and only affects targets that inherit it.🔧 Proposed alignment (target configs)
- DEVELOPMENT_TEAM = QC99C9JE59; + DEVELOPMENT_TEAM = 5YUPQC9D96;Also applies to: 466-466
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Hex.xcodeproj/project.pbxproj` at line 400, Update the target-level DEVELOPMENT_TEAM settings in both app target configurations to 5YUPQC9D96 so they no longer override the project-level team change. Preserve the existing signing configuration and update every target entry still using QC99C9JE59.HexCore/Sources/HexCore/StoragePaths.swift-13-21 (1)
13-21: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMigrate persisted Hex settings before switching storage roots.
Existing installs can have
hex_settings.jsonunderApplication Support/com.kitlangton.Hex; these changes now resolve onlyio.github.blackforestboi.Octo/hex_settings.jsonand removed the migration path. Users will silently start with default settings. Add an idempotent legacy-to-Octo migration before reads or writes use the new path.
HexCore/Sources/HexCore/StoragePaths.swift#L13-L21: detect and migrate legacy stored files into the Octo directory.Hex/Models/AppHexSettings.swift#L21-L26: retain compatibility until the legacy settings file has been migrated.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@HexCore/Sources/HexCore/StoragePaths.swift` around lines 13 - 21, In HexCore/Sources/HexCore/StoragePaths.swift:13-21, update the storage-path logic around hexStoredFileURL and hexApplicationSupport to idempotently detect legacy files under com.kitlangton.Hex and move or copy them into the Octo directory before any new-path reads or writes. In Hex/Models/AppHexSettings.swift:21-26, retain the legacy settings-file compatibility path until that migration has completed, while preserving normal Octo-path behavior afterward.Hex/Features/Settings/OpenRouterModelPickerView.swift-5-14 (1)
5-14: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winModel this picker as a TCA feature.
This view holds local model/loading/error state, selection/dismissal behavior, and an unstructured refresh effect. Move that state and lifecycle into a feature and render it through
@Bindable var store: StoreOf<...>, preferably via an optional@Presentschild state in the settings parent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Hex/Features/Settings/OpenRouterModelPickerView.swift` around lines 5 - 14, Convert OpenRouterModelPickerView into a TCA feature: move models, searchText, sortOrder, isRefreshing, errorMessage, selection, dismissal, and refresh lifecycle/effects into the feature’s State and Reducer, and render the view through a bindable StoreOf feature. Integrate presentation with optional `@Presents` child state in the settings parent, preserving the existing model filtering, selection, dismissal, and refresh behavior.Source: Coding guidelines
Hex/Clients/RefinementClient.swift-504-510 (1)
504-510: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
model.hasPrefix("o")matches far more than the o-series.Any OpenAI model id beginning with "o" (e.g. an
openai/-prefixed id routed here, or a futureomni-*) will get areasoningfield it does not accept, producing a 400 that surfaces as a refinement failure. Anchor to the actual o-series naming.🐛 Proposed fix
- return model.hasPrefix("gpt-5") || model.hasPrefix("o") + // o-series ids are always o<digit>, e.g. o1, o3-mini, o4-mini. + return model.hasPrefix("gpt-5") + || model.range(of: #"^o\d"#, options: .regularExpression) != nil🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Hex/Clients/RefinementClient.swift` around lines 504 - 510, The supportsReasoning method’s broad model.hasPrefix("o") check incorrectly enables reasoning for unrelated model IDs. Replace it with a check anchored to the actual o-series naming convention, while preserving the existing gpt-5 handling and .none behavior.Hex/Clients/CLIRefinementClient.swift-406-445 (1)
406-445: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftUnbounded pipe buffers can deadlock the CLI invocation.
standardOutput/standardErrorare only drained interminationHandler(Lines 422-423), and the whole prompt is written synchronously at Line 433. A CLI that emits more than the ~64 KB pipe buffer before exiting blocks on write while this code waits for termination, and a large screen-aware prompt can block the parent'swriteif the child hasn't started reading stdin. Drain both output pipes concurrently and write stdin off the calling thread.🛠️ Sketch of a non-blocking variant
try await withCheckedThrowingContinuation { continuation in + let collector = OutputCollector() + standardOutput.fileHandleForReading.readabilityHandler = { handle in + collector.appendOutput(handle.availableData) + } + standardError.fileHandleForReading.readabilityHandler = { handle in + collector.appendError(handle.availableData) + } process.terminationHandler = { finishedProcess in - let output = standardOutput.fileHandleForReading.readDataToEndOfFile() - let error = standardError.fileHandleForReading.readDataToEndOfFile() + standardOutput.fileHandleForReading.readabilityHandler = nil + standardError.fileHandleForReading.readabilityHandler = nil + let (output, error) = collector.drain( + standardOutput.fileHandleForReading, + standardError.fileHandleForReading + ) continuation.resume(returning: ProcessResult( status: finishedProcess.terminationStatus, standardOutput: String(decoding: output, as: UTF8.self), standardError: String(decoding: error, as: UTF8.self) )) } do { try process.run() - if let input { - standardInput.fileHandleForWriting.write(Data(input.utf8)) - } - try standardInput.fileHandleForWriting.close() + DispatchQueue.global(qos: .userInitiated).async { + if let input { + standardInput.fileHandleForWriting.write(Data(input.utf8)) + } + try? standardInput.fileHandleForWriting.close() + } } catch {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Hex/Clients/CLIRefinementClient.swift` around lines 406 - 445, Update the run(_ command:input:) process lifecycle so standardOutput and standardError are drained concurrently while the CLI runs, rather than only from terminationHandler, and move stdin writing/closing off the calling thread. Preserve ProcessResult output capture, launch-error handling, and cancellation behavior while ensuring large output or input cannot block the invocation.HexCore/Sources/HexCore/Logic/RefinementTextProcessor.swift-53-56 (1)
53-56: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
isRefusalsubstring matching discards valid refinements.Any refined text that merely contains "I apologize" — e.g. a dictated apology email, which is a very plausible refinement target — makes
RefinementClient.validated()return nil and throw.invalidResponse(Hex/Clients/RefinementClient.swiftLines 62-64, 74-80), so the user loses a correct result. Anchor the check to the start of the output instead of scanning the whole body.🐛 Proposed fix
public static func isRefusal(_ text: String) -> Bool { - let lowered = text.lowercased() - return lowered.contains("i can't assist") || lowered.contains("i apologize") || lowered.contains("i'm unable") + // Refusals lead the response; scanning the whole body rejects legitimate + // transcripts that merely mention an apology. + let lowered = text.prefix(120).lowercased() + return lowered.hasPrefix("i can't assist") + || lowered.hasPrefix("i apologize") + || lowered.hasPrefix("i'm unable") }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@HexCore/Sources/HexCore/Logic/RefinementTextProcessor.swift` around lines 53 - 56, Update RefinementTextProcessor.isRefusal to detect refusal phrases only when they occur at the beginning of the lowered text, rather than anywhere in the output. Preserve the existing phrases and ensure valid refinements containing those phrases later in the body are accepted.Hex/Clients/CLIRefinementClient.swift-295-397 (1)
295-397: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winNo timeout on the
codex app-serverhandshake.If the app server starts but never answers
initialize/model/list(and never exits), neitherfinishnorterminationHandlerfires and the continuation is never resumed, so the model-list request hangs until the caller cancels. Given this feeds the settings/menu-bar model picker, add a bounded deadline that callsfinish(.failure(...))and terminates the process.⏱️ Proposed guard
do { try process.run() + DispatchQueue.global().asyncAfter(deadline: .now() + 15) { + finish(.failure(Error.executionFailed(.codex, diagnostic: "Timed out listing models."))) + } writeJSON([🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Hex/Clients/CLIRefinementClient.swift` around lines 295 - 397, Update requestCodexModels to enforce a bounded timeout for the initialize/model-list handshake. Start a deadline after launching the process, and when it expires call finish(.failure(...)) with an appropriate Codex execution or timeout error, then terminate the process; ensure the existing lock/isFinished guard prevents races with output and termination handlers, and cancel the timeout when finish completes.Hex/Features/Settings/RefinementSectionView.swift-97-100 (1)
97-100: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
openRouterModelIDis used as a cross-provider fallback for both display and binding. OpenRouter model IDs are namespaced (e.g.anthropic/claude-sonnet-4) and are not valid for the direct OpenAI/Anthropic APIs, so falling back to that field surfaces — and can persist — a model ID the target provider will reject.
Hex/Features/Settings/RefinementSectionView.swift#L97-L100: drop the?? store.hexSettings.openRouterModelIDfallback and itsforegroundStylecondition; show "Select a model" whenopenAIModelIDis nil.Hex/Features/Settings/RefinementSectionView.swift#L120-L123: apply the same change againstanthropicModelID.Hex/Features/Settings/RefinementSectionView.swift#L368-L378: inmodelBinding(for: .refinement), stop routing.apple, .gemini, .codexCLI, .claudeCLIto$store.hexSettings.openRouterModelID; bind each provider to its own settings key (codexCLIModelID,claudeCLIModelID) and keepopenRouterModelIDfor.openRouteronly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Hex/Features/Settings/RefinementSectionView.swift` around lines 97 - 100, Remove the cross-provider openRouterModelID fallback in RefinementSectionView at lines 97-100 and display “Select a model” based only on openAIModelID, simplifying the foreground style accordingly; apply the same change to the anthropicModelID display at lines 120-123. In modelBinding(for: .refinement) at lines 368-378, bind .apple and .gemini to their own provider settings, bind .codexCLI to codexCLIModelID, bind .claudeCLI to claudeCLIModelID, and reserve openRouterModelID exclusively for .openRouter..changeset/prevent-documents-access.md-2-2 (1)
2-2: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUse the current Octo package name in this changeset.
package.jsondeclares"octo-app", and no other tracked reference useshex-app; this fragment will cause release tooling to complain about an unknown package.🔧 Proposed fix
-"hex-app": patch +"octo-app": patch🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.changeset/prevent-documents-access.md at line 2, Update the changeset package identifier from hex-app to the declared Octo package name octo-app, matching package.json and existing tracked references.Hex/Clients/GeminiAPIKeyStore.swift-43-49 (1)
43-49: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winSet
kSecAttrAccessibleexplicitly, and drop the trailing comma flagged by SwiftLint.Without an explicit accessibility class the item gets the default (
kSecAttrAccessibleWhenUnlocked) and, more importantly, it becomes backup/migration-eligible.kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly(orWhenUnlockedThisDeviceOnly) keeps API keys on this Mac only. NotekSecAttrAccessiblemust not be part of the query dictionary passed toSecItemUpdate, so add it only to the add-path attributes.🔒 Proposed fix
private static func baseQuery(service: String) -> [String: Any] { [ kSecClass as String: kSecClassGenericPassword, kSecAttrService as String: service, - kSecAttrAccount as String: account, + kSecAttrAccount as String: account ] }if status == errSecItemNotFound { var newItem = query newItem[kSecValueData as String] = data + newItem[kSecAttrAccessible as String] = kSecAttrAccessibleAfterFirstUnlockThisDeviceOnly let addStatus = SecItemAdd(newItem as CFDictionary, nil)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Hex/Clients/GeminiAPIKeyStore.swift` around lines 43 - 49, Update the keychain add-path attributes to explicitly set kSecAttrAccessible to a device-only accessibility class such as AfterFirstUnlockThisDeviceOnly, while keeping baseQuery limited to fields valid for SecItemUpdate. Remove the trailing comma from the baseQuery dictionary entry flagged by SwiftLint.Source: Linters/SAST tools
Hex/App/MenuBarRefinementModelPicker.swift-85-100 (1)
85-100: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winPopulate the direct-provider fields during the
openRouterModelIDfallback migration.The fallback preserves an old OpenRouter selection for
.openAIand.anthropic, butapply(_:to:)only writes to the provider-specific fields. For OpenRouter-prepared IDs likeopenai/gpt-4o, the direct catalogs use bare IDs likegpt-4o, so a user can be shown the correct model in the OpenRouter picker and then rendered asUnavailableonce switched to the direct provider. Migrate the value on first selection/change intosettings.openAIModelIDorsettings.anthropicModelIDinstead of reusingopenRouterModelIDfor these providers.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Hex/App/MenuBarRefinementModelPicker.swift` around lines 85 - 100, Update apply(_:to:) so selections for .openAI and .anthropic migrate OpenRouter-formatted IDs into the corresponding provider-specific fields, removing the provider prefix as needed. Store the migrated bare ID in openAIModelID or anthropicModelID instead of relying on openRouterModelID, and ensure selectedModelID reads the direct field after migration.Hex/Features/Settings/SubscriptionModelPickerView.swift-44-48 (1)
44-48: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winLiteral
"default"id isn't normalized tonil, breaking parity with the menu-bar picker.
CLIRefinementClient.models(for: .claude)includes a fixed entry withid: "default". Selecting it here stores the raw string"default", butMenuBarRefinementModelPicker.swiftnormalizes that same sentinel tonil(line 300) andRefinementModelMenuSelection.title/Toggle logic rely onnilmeaning "use default". If this view'sselectedModelIDbinds to the sameclaudeCLIModelIDsetting, choosing "Claude default" here will desync the menu-bar picker's displayed title and selection state.🔧 Suggested fix
Button { - selectedModelID = model.id + selectedModelID = model.id == "default" ? nil : model.id dismiss() } label: {Also update the
selectedModelID == model.idcomparisons on lines 55-56 accordingly ifselectedModelIDcan benil.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Hex/Features/Settings/SubscriptionModelPickerView.swift` around lines 44 - 48, Update the model-selection action in the SubscriptionModelPickerView List so the sentinel model id "default" is stored as nil, matching MenuBarRefinementModelPicker behavior and preserving the shared claudeCLIModelID contract. Adjust the selectedModelID comparison used by the row selection state to treat nil as selected when model.id is "default", while leaving non-default model handling unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c7e4047f-e31a-4670-935f-7fe95cdfd9e1
⛔ Files ignored due to path filters (11)
Hex/AppIcon.icon/Assets/DarkImage.pngis excluded by!**/*.pngHex/AppIcon.icon/Assets/Image.pngis excluded by!**/*.pngHex/Assets.xcassets/OctoMenuBarIcon.imageset/OctoMenuBarIcon.pngis excluded by!**/*.pngHex/Resources/Icons/OctoDarkIcon.pngis excluded by!**/*.pngHex/Resources/Icons/OctoLightIcon.pngis excluded by!**/*.pngtmp/octo-light-edge-aligned.pngis excluded by!**/*.pngtmp/octo-light-opposite-edge.pngis excluded by!**/*.pngtmp/octo-light-ridge-preview.pngis excluded by!**/*.pngtmp/octo-light-ridge-preview.svgis excluded by!**/*.svgtmp/octo-light-tentacles-only.pngis excluded by!**/*.pngtmp/octo-menu-preview.pngis excluded by!**/*.png
📒 Files selected for processing (114)
.changeset/719ddca6.md.changeset/9279574a.md.changeset/cli-refinement-providers.md.changeset/direct-menu-history.md.changeset/durable-recording-recovery.md.changeset/fix-menu-bar-icon-size.md.changeset/fix-paste-last-transcript.md.changeset/fix-sparkle-feed-bootstrap.md.changeset/history-search.md.changeset/interactive-screen-region-capture.md.changeset/menu-bar-refinement-model-picker.md.changeset/octo-dark-app-icon.md.changeset/octo-date-release-readme.md.changeset/prevent-documents-access.md.changeset/rebrand-octo-update-feed.md.changeset/refinement-subtitle-weight.md.changeset/show-ai-refinement-errors.md.changeset/transcription-pill-waveform.md.changeset/unified-hotkey-refinement.md.github/workflows/publish-update-feed.yml.github/workflows/release.ymlCHANGELOG.mdCLAUDE.mdHex.xcodeproj/project.pbxprojHex.xcodeproj/xcshareddata/xcschemes/Octo.xcschemeHex/App/HexApp.swiftHex/App/HexAppDelegate.swiftHex/App/MenuBarCopyLastTranscriptButton.swiftHex/App/MenuBarRefineSelectedTextButton.swiftHex/App/MenuBarRefinementModelPicker.swiftHex/AppIcon.icon/icon.jsonHex/Assets.xcassets/OctoMenuBarIcon.imageset/Contents.jsonHex/Clients/CLIRefinementClient.swiftHex/Clients/DirectProviderModelCatalog.swiftHex/Clients/GeminiAPIKeyStore.swiftHex/Clients/KeyEventMonitorClient.swiftHex/Clients/OpenRouterModelCatalog.swiftHex/Clients/ParakeetClient.swiftHex/Clients/PasteboardClient.swiftHex/Clients/RecordingClient.swiftHex/Clients/RecordingRecoveryStore.swiftHex/Clients/RefinementClient.swiftHex/Clients/ScreenCaptureClient.swiftHex/Clients/ScreenCaptureSelection.swiftHex/Clients/ScreenCaptureSelectionOverlay.swiftHex/Clients/SuperFastCaptureController.swiftHex/Features/App/AppFeature.swiftHex/Features/History/HistoryFeature.swiftHex/Features/Settings/AboutView.swiftHex/Features/Settings/DirectProviderModelPickerView.swiftHex/Features/Settings/HotKeySectionView.swiftHex/Features/Settings/IndicatorSectionView.swiftHex/Features/Settings/OpenRouterModelPickerView.swiftHex/Features/Settings/PermissionsSectionView.swiftHex/Features/Settings/RefinementSectionView.swiftHex/Features/Settings/SettingsFeature.swiftHex/Features/Settings/SettingsView.swiftHex/Features/Settings/SubscriptionModelPickerView.swiftHex/Features/Transcription/TranscriptionFeature.swiftHex/Features/Transcription/TranscriptionIndicatorView.swiftHex/Info.plistHex/LegacyAppIcon.icnsHex/Models/AppHexSettings.swiftHex/Resources/changelog.mdHex/Views/InvisibleWindow.swiftHexCore/Package.swiftHexCore/Sources/HexCore/Logging.swiftHexCore/Sources/HexCore/Logic/HotKeyProcessor.swiftHexCore/Sources/HexCore/Logic/RefinementPromptBuilder.swiftHexCore/Sources/HexCore/Logic/RefinementTextProcessor.swiftHexCore/Sources/HexCore/Logic/ScreenAwarePromptBuilder.swiftHexCore/Sources/HexCore/Models/HotKey.swiftHexCore/Sources/HexCore/Models/OpenRouterModel.swiftHexCore/Sources/HexCore/Models/RefinementMode.swiftHexCore/Sources/HexCore/Models/RefinementProvider.swiftHexCore/Sources/HexCore/Models/RefinementReasoningEffort.swiftHexCore/Sources/HexCore/Models/RefinementRequest.swiftHexCore/Sources/HexCore/Models/ScreenContext.swiftHexCore/Sources/HexCore/Models/TranscriptionHistory.swiftHexCore/Sources/HexCore/PermissionClient/PermissionClient+Live.swiftHexCore/Sources/HexCore/PermissionClient/PermissionClient.swiftHexCore/Sources/HexCore/Settings/HexSettings.swiftHexCore/Sources/HexCore/StoragePaths.swiftHexCore/Sources/HexCore/TranscriptPersistenceClient/TranscriptPersistenceClient.swiftHexCore/Tests/HexCoreTests/HexSettingsMigrationTests.swiftHexCore/Tests/HexCoreTests/HotKeyProcessorTests.swiftHexCore/Tests/HexCoreTests/RefinementTests.swiftHexCore/Tests/HexCoreTests/TranscriptionHistoryTests.swiftHexTests/AppFeatureTests.swiftHexTests/CLIRefinementClientTests.swiftHexTests/HistoryPlaybackTests.swiftHexTests/HistorySearchTests.swiftHexTests/LegacyModelCacheMigratorTests.swiftHexTests/MenuBarRefinementModelPickerTests.swiftHexTests/ModelDownloadFeatureTests.swiftHexTests/RecordingRaceTests.swiftHexTests/ScreenCaptureSelectionTests.swiftLocalizable.xcstringsREADME.mddocs/hotkey-semantics.mddocs/plans/2026-07-23-001-refactor-unified-hotkey-refinement-plan.mddocs/plans/2026-07-23-002-fix-crash-resilient-recording-recovery-plan.mddocs/plans/2026-07-23-003-feat-region-screen-capture-plan.mddocs/plans/2026-07-23-cli-refinement-providers-plan.mddocs/plans/2026-07-23-transcription-pill-waveform-plan.mddocs/plans/2026-07-28-001-feat-menu-bar-refinement-model-picker-plan.mddocs/release-process.mddocs/update-delivery.mddocs/updates/appcast.xmlhex.rbocto.rbpackage.jsontmp/align_suction_cups.pytmp/create_octo_icon_variants.py
💤 Files with no reviewable changes (3)
- .changeset/9279574a.md
- hex.rb
- .changeset/719ddca6.md
| func select() async throws -> CGRect? { | ||
| try await withCheckedThrowingContinuation { continuation in | ||
| self.continuation = continuation | ||
| let view = ScreenCaptureSelectionOverlayView( | ||
| frame: CGRect(origin: .zero, size: window.frame.size), | ||
| minimumDragDistance: 20 / backingScaleFactor, | ||
| onComplete: { [weak self] rectangle in self?.complete(with: rectangle) }, | ||
| onCancel: { [weak self] in self?.cancel() } | ||
| ) | ||
| window.contentView = view | ||
| keyEventMonitor = NSEvent.addLocalMonitorForEvents(matching: [.keyDown, .keyUp]) { [weak view] event in | ||
| if event.type == .keyDown, view?.handleKeyDown(event) == true { | ||
| return nil | ||
| } | ||
| if event.type == .keyUp, view?.handleKeyUp(event) == true { | ||
| return nil | ||
| } | ||
| return event | ||
| } | ||
| NSApp.activate(ignoringOtherApps: true) | ||
| window.makeKeyAndOrderFront(nil) | ||
| window.makeFirstResponder(view) | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Task cancellation leaves the overlay on screen and the continuation never resumed. select() awaits a CheckedContinuation that is only resumed from complete/cancel, which are driven exclusively by user input. TranscriptionFeature cancels CancelID.screenContextCapture on start/cancel/discard paths, but nothing here observes cancellation, so the awaiting task hangs forever and the full-screen .screenSaver-level panel stays up, blocking the desktop until a click/ESC. Wrap the continuation in withTaskCancellationHandler and tear down on cancel.
🐛 Proposed fix
func select() async throws -> CGRect? {
- try await withCheckedThrowingContinuation { continuation in
- self.continuation = continuation
- let view = ScreenCaptureSelectionOverlayView(
+ try await withTaskCancellationHandler {
+ try await withCheckedThrowingContinuation { continuation in
+ self.continuation = continuation
+ let view = ScreenCaptureSelectionOverlayView(
frame: CGRect(origin: .zero, size: window.frame.size),
minimumDragDistance: 20 / backingScaleFactor,
onComplete: { [weak self] rectangle in self?.complete(with: rectangle) },
onCancel: { [weak self] in self?.cancel() }
- )
- window.contentView = view
+ )
+ window.contentView = view
…
+ }
+ } onCancel: {
+ Task { `@MainActor` [weak self] in self?.cancel() }
}
}Also applies to: 105-113
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Hex/Clients/ScreenCaptureSelectionOverlay.swift` around lines 46 - 69, Update
select() to wrap its withCheckedThrowingContinuation in
withTaskCancellationHandler so task cancellation resumes the continuation with
CancellationError and invokes the overlay teardown path. Reuse complete(with:)
or cancel() as appropriate to remove the window and event monitor, ensuring
cancellation cannot leave the overlay visible or the continuation pending.
|
Closing because this change targets the Octo fork's main branch, not upstream Hex. |
1 participant
Summary
Octo users can now see and switch the active refinement model directly from the menu-bar menu. The native submenu opens on hover, stays scoped to the selected refinement provider, and updates the parent label as soon as a model is chosen.
Provider model loads are bound to the provider that initiated them, so late catalog responses cannot write incompatible selections after a provider change. Saved models that disappear from a refreshed catalog remain visible as unavailable instead of being cleared.
Validation
Focused model-selection tests were added but not run, per request. No further build or test commands were run after the request to skip compilation.
Post-Deploy Monitoring & Validation
No additional operational monitoring is required for this local macOS menu and settings change. During the next manual app smoke pass, verify that hovering opens the submenu and that the selected model persists.
Summary by CodeRabbit
New Features
Bug Fixes