Push app-shell presentation logic down into the engine - #86
Merged
Conversation
Seven places where pure logic sat in the AppKit/SwiftUI shell — which has no test target — rather than in BlurtEngine, where `swift test` covers it. Each follows the pattern already set by `OverlayUIState`, `MenuBarStatus`, `RecordingCueGate`, `MeterBarGeometry`, and `RecentDictations.reservedHeight`. Engine gains: - `UpdateAlertContent` — title, body, buttons, and the `downloadURL` the default button opens, for each update-check result. `UpdateCheckModel` kept three hand-built `NSAlert`s whose wording, button order, and "first button downloads" rule were covered by nothing; it now holds only the AppKit half. `appVersionLabel(_:)` is the shared "Blurt 0.1.31" form, so the alerts and the Settings Updates row can't word the version two ways (it was composed in three places). - `APIKeyDisplay` — the account row's masked tail, status/VoiceOver wording, and connect-vs-rotate control titles. The view's fixed `"••••\(key.suffix(4))"` rendered a key shorter than four characters whole; the mask now reveals nothing below `minimumLengthToMask`. - `APIKeySubmission.Outcome.failureReport` — whether a failure is `.inline` (recoverable, beside the field) or `.alert` (a Keychain fault retyping can't fix), messages included. Same move as `PipelinePhase.setupBlocker`: the sheet renders the engine's classification instead of re-deriving severity. - `SetupReadiness` + `PermissionStatus.lostGrant(since:)` — the fully-configured gate, the permission-poll cadence, and the revocation edge that pulls the user back into onboarding, all previously inline in `WizardController`. - `OverlayPlacement.panelSize(pillSize:shadowMargin:)` — the other half of the pill/panel relationship `panelOrigin` already corrects for, so a change to the shadow margin is checked on both sides. - `DictationLog.defaultDisplayPath` — the home-abbreviated log path, next to the URL the writer uses, keeping the `NSString` bridge out of a SwiftUI view. App-side, `@BoundTriggerKey` replaces the `@AppStorage` + `TriggerKey.fromPersisted` pair that three views each spelled out along with a restated right-⌘ default that `fromPersisted` already owns. No user-visible behavior change: every string, button order, and alert style is preserved (`NSAlert` already defaults to `.warning`, so only the caution case sets it, as before). New suites: `UpdateAlertContentTests`, `APIKeyDisplayTests`, `SetupReadinessTests`, plus additions to the submission, placement, and log suites. Docs updated in `AGENTS.md` / `BLURTENGINE.md`. Verified with `scripts/check.sh --portable` (green); the Swift build, tests, and lint run on CI (macos-26), which is the authority on green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016GUfBes8NA1SSNugHQtdb8
There was a problem hiding this comment.
Pull request overview
Moves several pieces of pure presentation / policy logic out of the AppKit/SwiftUI shell (which has no test target) into BlurtEngine so they’re unit-tested and reused consistently across the app UI.
Changes:
- Added engine-owned UI content/projections (
UpdateAlertContent,APIKeyDisplay,SetupReadiness,PermissionStatus.lostGrant(since:),OverlayPlacement.panelSize,DictationLog.defaultDisplayPath). - Updated the app shell to consume those projections (settings/wizard/update alerts/overlay sizing) and added
@BoundTriggerKeyfor live trigger-key display. - Added/expanded engine test coverage for the new projections and updated docs.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Tests/BlurtEngineTests/UpdateAlertContentTests.swift | Adds coverage for update-alert wording, button order, and mapping from update-check results. |
| Tests/BlurtEngineTests/SetupReadinessTests.swift | Tests readiness gating, polling cadence, and permission-revocation detection. |
| Tests/BlurtEngineTests/OverlayPlacementTests.swift | Tests new panelSize arithmetic and invariants with panelOrigin. |
| Tests/BlurtEngineTests/DictationLogTests.swift | Adds tests for the new home-abbreviated display path vs the actual write URL. |
| Tests/BlurtEngineTests/APIKeySubmissionTests.swift | Tests Outcome.failureReport classification and messaging. |
| Tests/BlurtEngineTests/APIKeyDisplayTests.swift | Adds coverage for key masking, labels, and connect/rotate wording. |
| Sources/BlurtEngine/Update/UpdateAlertContent.swift | Introduces engine-owned update alert content (title/message/buttons/default download URL/style). |
| Sources/BlurtEngine/Pipeline/OverlayPlacement.swift | Adds panelSize(pillSize:shadowMargin:) to keep pill/panel geometry arithmetic in-engine. |
| Sources/BlurtEngine/Pipeline/DictationLog.swift | Adds defaultDisplayPath derived next to defaultURL for UI display. |
| Sources/BlurtEngine/Permissions/SetupReadiness.swift | Adds engine-owned “fully configured” rule + permission poll cadence policy. |
| Sources/BlurtEngine/Permissions/PermissionsChecker.swift | Adds PermissionStatus.lostGrant(since:) to detect revocation edges. |
| Sources/BlurtEngine/Config/APIKeySubmission.swift | Adds FailureReport and Outcome.failureReport to centralize severity + messaging. |
| Sources/BlurtEngine/Config/APIKeyDisplay.swift | Adds engine-owned key display projection (masking, wording, accessibility labels, button titles). |
| BLURTENGINE.md | Documents the new engine projections and updated overlay/update/key flows. |
| App/Blurt/Blurt/Wizard/WizardController.swift | Switches readiness/poll cadence/revocation logic to engine (SetupReadiness, lostGrant). |
| App/Blurt/Blurt/Wizard/Steps/PermissionsStepView.swift | Uses @BoundTriggerKey for live trigger-key label instead of duplicating @AppStorage logic. |
| App/Blurt/Blurt/Wizard/Steps/HotkeyStepView.swift | Adds @BoundTriggerKey property wrapper for consistent live trigger-key display. |
| App/Blurt/Blurt/Wizard/Steps/APIKeyStepView.swift | Switches key-row display and failure reporting to APIKeyDisplay / Outcome.failureReport. |
| App/Blurt/Blurt/Wizard/SettingsWindowRoot.swift | Uses engine-provided update-row version label and log display path. |
| App/Blurt/Blurt/Wizard/ReadyView.swift | Uses @BoundTriggerKey for live trigger keycap rendering. |
| App/Blurt/Blurt/Update/UpdateCheckModel.swift | Refactors update checking to resolve UpdateAlertContent and build NSAlert from it. |
| App/Blurt/Blurt/Overlay/OverlayWindowController.swift | Uses OverlayPlacement.panelSize instead of duplicating shadow-margin arithmetic. |
| App/Blurt/Blurt/MenuBar/MenuBarScene.swift | Uses @BoundTriggerKey for live menu-bar reminder text. |
| AGENTS.md | Updates repo guide to reflect new engine-owned projections and usage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+93
to
+97
| // `NSAlert` already defaults to `.warning`, and on current macOS `.warning` | ||
| // and `.informational` render identically (only `.critical` adds the caution | ||
| // badge), so only the caution case is worth stating — same presentation the | ||
| // three hand-built alerts had. | ||
| if content.style == .warning { alert.alertStyle = .warning } |
Three conflicts, all from main's #83 (non-idiomatic Swift cleanup) and #84 (agent-guide restructure) landing on the same lines: - `WizardController.refreshPermissions` — main renamed the local `perms` → `latest`; kept that name with this branch's `latest.lostGrant(since:)` in place of the inlined `allGranted && !allGranted`. - `AGENTS.md` — main rewrote the file's structure (new "Start here" reflexes list, "How the pipeline works", a wrapped ~100-col prose style) and removed or relocated the two paragraphs this branch had amended. Took main's structure and re-applied the additions where the content now lives: `UpdateAlertContent` in the Updates section (now three pieces), the API-key display/failure-report and `SetupReadiness` projections in "Settings, persistence, and cues", the new projections in the "Pure projections over shell logic" convention, and `@BoundTriggerKey` in the Hotkey section — all rewrapped to match. `BLURTENGINE.md`, `OverlayWindowController`, and `DictationLog` auto-merged. Verified nothing this branch depends on was renamed by #83: `SemanticVersion` keeps `CustomStringConvertible` (so `appVersionLabel` still renders "Blurt 0.1.31" and still strips a `v` prefix), and `PipelinePhase.isTerminal` / `BlurtError.isSetupBlocker` / `phase.overlayState` are unchanged. This branch's switches were already exhaustive with spelled-out names, matching the two conventions #83 reinforced. `scripts/check.sh --portable` green on the merge result. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016GUfBes8NA1SSNugHQtdb8
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
App/Blurt/Blurt/Update/UpdateCheckModel.swift:97
UpdateAlertContentexposes astyle(informational vs warning), butpresent(_:)only sets.warningand otherwise leavesNSAlertat its default. This means informational content (e.g. “You’re up to date”) will be presented with the default alert style instead of honoringcontent.style.
Map both Style cases explicitly so the engine’s projection is actually reflected in the AppKit alert.
// `NSAlert` already defaults to `.warning`, and on current macOS `.warning`
// and `.informational` render identically (only `.critical` adds the caution
// badge), so only the caution case is worth stating — same presentation the
// three hand-built alerts had.
if content.style == .warning { alert.alertStyle = .warning }
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
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.
Seven places where pure logic sat in the AppKit/SwiftUI shell — which has no
test target — rather than in BlurtEngine, where
swift testcovers it. Eachfollows the pattern already set by
OverlayUIState,MenuBarStatus,RecordingCueGate,MeterBarGeometry, andRecentDictations.reservedHeight.Engine gains:
UpdateAlertContent— title, body, buttons, and thedownloadURLthe defaultbutton opens, for each update-check result.
UpdateCheckModelkept threehand-built
NSAlerts whose wording, button order, and "first buttondownloads" rule were covered by nothing; it now holds only the AppKit half.
appVersionLabel(_:)is the shared "Blurt 0.1.31" form, so the alerts and theSettings Updates row can't word the version two ways (it was composed in three
places).
APIKeyDisplay— the account row's masked tail, status/VoiceOver wording, andconnect-vs-rotate control titles. The view's fixed
"••••\(key.suffix(4))"rendered a key shorter than four characters whole; the mask now reveals
nothing below
minimumLengthToMask.APIKeySubmission.Outcome.failureReport— whether a failure is.inline(recoverable, beside the field) or
.alert(a Keychain fault retyping can'tfix), messages included. Same move as
PipelinePhase.setupBlocker: the sheetrenders the engine's classification instead of re-deriving severity.
SetupReadiness+PermissionStatus.lostGrant(since:)— the fully-configuredgate, the permission-poll cadence, and the revocation edge that pulls the user
back into onboarding, all previously inline in
WizardController.OverlayPlacement.panelSize(pillSize:shadowMargin:)— the other half of thepill/panel relationship
panelOriginalready corrects for, so a change to theshadow margin is checked on both sides.
DictationLog.defaultDisplayPath— the home-abbreviated log path, next to theURL the writer uses, keeping the
NSStringbridge out of a SwiftUI view.App-side,
@BoundTriggerKeyreplaces the@AppStorage+TriggerKey.fromPersistedpair that three views each spelled out along with a restated right-⌘ default that
fromPersistedalready owns.No user-visible behavior change: every string, button order, and alert style is
preserved (
NSAlertalready defaults to.warning, so only the caution casesets it, as before). New suites:
UpdateAlertContentTests,APIKeyDisplayTests,SetupReadinessTests, plus additions to the submission, placement, and logsuites. Docs updated in
AGENTS.md/BLURTENGINE.md.Verified with
scripts/check.sh --portable(green); the Swift build, tests, andlint run on CI (macos-26), which is the authority on green.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_016GUfBes8NA1SSNugHQtdb8