Skip to content

Add AcceptanceGranularity setting (Word / Phrase / Full) for primary accept key#388

Merged
FuJacob merged 2 commits into
FuJacob:mainfrom
rpickmans:feat/acceptance-granularity-phrase
May 30, 2026
Merged

Add AcceptanceGranularity setting (Word / Phrase / Full) for primary accept key#388
FuJacob merged 2 commits into
FuJacob:mainfrom
rpickmans:feat/acceptance-granularity-phrase

Conversation

@rpickmans
Copy link
Copy Markdown
Contributor

@rpickmans rpickmans commented May 28, 2026

Summary

Adds a user-selectable acceptance granularity to the primary accept key (default Tab): Word (current behavior, default), Phrase (stops at .!?\n or their quoted/bracketed equivalents — Cursor-style), or Full Suggestion (same outcome as the dedicated full-accept key). The dedicated full-accept key (default `) remains an unconditional per-press override regardless of the chosen granularity.

The new nextAcceptancePhrase chunker composes over the existing nextAcceptanceChunk, so word-boundary, internal-punctuation, and leading-whitespace policy stay identical across multi-word seams. Two extra rules make sentence boundaries robust:

  • Newlines are scanned inside each composed chunk — nextAcceptanceChunk returns leading whitespace including \n, so a tail like hello\nworld would otherwise carry the newline forward unnoticed.
  • Terminator detection walks back past closing quotes and brackets, so "done." Next stops at the closing quote rather than over-accepting into the next sentence. Handles ", ', ), ], }, and curly quotes , .

Known limitation: a tail ending in U.S.A. is treated as a sentence end (early break). Rule-based scanners can't disambiguate this without NLP; Cursor and Copilot behave the same way. Covered by a dedicated test that documents the behavior.

Validation

xcodebuild -project Cotabby.xcodeproj -scheme Cotabby \
  -destination 'platform=macOS' CODE_SIGNING_ALLOWED=NO test
# ** TEST SUCCEEDED **  Executed 449 tests, with 1 test skipped and 0 failures
swiftlint lint --quiet
# exit 0

18 new nextAcceptancePhrase tests cover empty tail, no-terminator, .!? boundaries, newline-between-tokens, leading-newline, multiple newlines, leading whitespace, abbreviation false-break (documented as known limitation), autoAcceptTrailingPunctuation flag invariance, interior punctuation preservation, straight/curly quoted sentence ends, parenthesized sentence ends, nested closers, and bare-closing-quote (no false break).

Linked issues

Fixes #10

Risk / rollout notes

  • Default unchanged. New installs and existing installs without the persisted UserDefaults key get .word, preserving current Tab behavior. The setting is read once per accept call from the value-typed snapshot, so a mid-press toggle in Settings can't strand a partially-handled press.
  • Settings exposed in both UIs. Picker added to both the legacy SettingsView.shortcutsSection and the redesigned ShortcutsPaneView so the setting is reachable regardless of which Settings shell is active.
  • UI naming question for maintainer: the "Accept Word" row label was kept as-is to stay symmetric with the WelcomeView onboarding (WelcomeView.swift:303) and the parallel label in the other settings pane. The new "Acceptance Mode" Picker disambiguates what the key actually does. If you'd prefer a dynamic row label ("Accept Word" / "Accept Phrase" / "Accept Full") to match the picker selection, happy to make that change in this PR or a follow-up.
  • Per-app and per-profile granularity stay out of scope per the design discussion on the issue — those touch the rules system and warrant a separate PR.
  • prepareAcceptance signature change (added required granularity: parameter, no default) — only 2 in-tree callers (coordinator + state-helper test); both updated. .full is precondition-rejected in prepareAcceptance because the coordinator routes full accepts to prepareFullAcceptance for the distinct invalid-reason message; defensive handling could be substituted if you'd prefer.

Greptile Summary

This PR adds a user-selectable AcceptanceGranularity setting (Word / Phrase / Full) for the primary accept key, while keeping the dedicated full-accept key as an unconditional per-press override. The new nextAcceptancePhrase chunker composes over nextAcceptanceChunk, inheriting existing word-boundary and leading-whitespace policy, with extra rules for newlines (which appear as leading whitespace in the composed chunks) and closing-punctuation walk-back for quoted/parenthesised sentence ends.

  • AcceptanceGranularity is persisted in UserDefaults with a .word fallback, wired into the settings snapshot publisher via an outer CombineLatest, and exposed in both the legacy SettingsView and the redesigned ShortcutsPaneView through a new shared AcceptanceModePickerView component.
  • The prepareAcceptance signature gains a required granularity: parameter; .full is guarded with assertionFailure + graceful fallback since the coordinator already routes full accepts to prepareFullAcceptance. 18 new unit tests cover the chunker in isolation; a new integration test in SuggestionStateHelperTests confirms the .phrase branch propagates correctly through prepareAcceptance end-to-end.

Confidence Score: 5/5

Safe to merge — the new phrase chunker composes cleanly over the existing word chunker, default behavior is unchanged for existing installs, and all previously flagged issues have been addressed.

The core logic in nextAcceptancePhrase is well-reasoned: newline detection correctly captures
from leading-whitespace chunks, the closing-punctuation walk-back handles nested quoted/parenthesised sentence ends, and autoAcceptTrailingPunctuation invariance holds. The .full guard now uses assertionFailure + graceful .invalid return instead of preconditionFailure, matching every other invalid branch. The integration test for .phrase in prepareAcceptance closes the only gap from the prior review round. No new behavioral regressions are introduced; the .word default preserves all existing acceptance behaviour for users who never touch the new setting.

No files require special attention.

Important Files Changed

Filename Overview
Cotabby/Support/SuggestionSessionReconciler.swift Adds nextAcceptancePhrase and endsInSentenceTerminator; logic is sound — newline scan correctly captures \n from leading-whitespace chunks, closing-punc walk-back handles nested/quoted sentence ends, and autoAcceptTrailingPunctuation invariance is verified by tests.
Cotabby/Services/Suggestion/SuggestionInteractionState.swift prepareAcceptance gains a required granularity: parameter; .full now uses assertionFailure + .invalid return instead of preconditionFailure, matching the graceful degradation pattern of every other invalid branch.
Cotabby/App/Coordinators/SuggestionCoordinator+Acceptance.swift Correctly reads granularity from the settings snapshot and routes .full (and the fullText flag) to prepareFullAcceptance so .full never reaches prepareAcceptance.
Cotabby/Models/SuggestionSettingsModel.swift Persists acceptanceGranularity via UserDefaults rawValue with a safe .word fallback; adds setAcceptanceGranularity with early-exit guard; layers $acceptanceGranularity into the snapshot publisher via a second CombineLatest around the existing CombineLatest4.
Cotabby/UI/Settings/Components/AcceptanceModePickerView.swift New shared picker component that eliminates the verbatim duplication between SettingsView and ShortcutsPaneView; clean Binding wrapping via setAcceptanceGranularity.
CotabbyTests/SuggestionSessionReconcilerTests.swift 18 new tests cover the phrase chunker thoroughly: empty input, no terminator, .!? stops, newline variants, leading whitespace, abbreviation false-break (documented known limitation), flag invariance, interior punctuation, straight/curly quotes, parentheses, nested closers, and bare closing quote non-break.
CotabbyTests/SuggestionStateHelperTests.swift Adds integration test for .phrase granularity through prepareAcceptance, confirming the accepted chunk spans to the sentence terminator and that session.remainingText is unmodified before commit.
Cotabby/Models/SuggestionEngineModels.swift AcceptanceGranularity enum added as String/Codable/CaseIterable/Sendable; acceptanceGranularity field added to SuggestionSettingsSnapshot.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Key press detected] --> B{fullText flag\nor granularity == .full?}
    B -- Yes --> C[prepareFullAcceptance\nreturns entire remainingText]
    B -- No --> D{granularity}
    D -- .word --> E[nextAcceptanceChunk\none word + optional trailing punct]
    D -- .phrase --> F[nextAcceptancePhrase\ncompose chunks until .!? or newline]
    F --> G{chunk contains newline?}
    G -- Yes --> H[Accumulate up to newline inclusive\nreturn early]
    G -- No --> I[Accumulate chunk\nadvance working]
    I --> J{endsInSentenceTerminator?\nwalk back past closing punct}
    J -- Yes --> K[Return phrase]
    J -- No --> F
    E --> L[guard chunk not empty]
    K --> L
    H --> L
    L -- empty --> M[.invalid — key passes through]
    L -- non-empty --> N[.ready liveContext / session / acceptedChunk]
    N --> O[Coordinator calls commitAcceptedChunk]
Loading

Comments Outside Diff (1)

  1. Cotabby/UI/Settings/Panes/ShortcutsPaneView.swift, line 381-386 (link)

    P2 Verbatim Picker duplication across both settings UIs

    The five-line Picker block and its acceptanceGranularityBinding helper are replicated character-for-character in SettingsView.swift. If the labels or cases change (e.g., renaming "Full Suggestion" or adding a fourth mode), both files need identical edits. Extracting this into a small private AcceptanceModePickerView or a @ViewBuilder helper shared by both would remove the drift risk entirely.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

    Fix in Codex Fix in Claude Code

Reviews (2): Last reviewed commit: "Harden AcceptanceGranularity: graceful ...." | Re-trigger Greptile

…y accept

Fixes FuJacob#10. The primary accept key (default Tab) now honors a user-selected
granularity: Word (current behavior), Phrase (Cursor-style — stop at .!?\n or
their quoted/bracketed equivalents), or Full Suggestion (same outcome as the
dedicated full-accept key). The dedicated full-accept key remains an unconditional
per-press override.

The new nextAcceptancePhrase chunker composes over the existing
nextAcceptanceChunk so word-boundary, internal-punctuation, and leading-whitespace
policy stay identical across multi-word seams. Two extra rules make sentence
boundaries work robustly:
- newlines are scanned inside each composed chunk (the chunker returns leading
  whitespace including \n, so a tail like "hello\nworld" would otherwise carry
  the newline forward unnoticed)
- terminator detection walks back past closing quotes and brackets so
  "done." Next stops at the closing quote rather than over-accepting

Known limitation: a tail ending in U.S.A. is treated as a sentence end (early
break). Rule-based scanners can't disambiguate without NLP; Cursor and Copilot
behave the same way.

Per-app and per-profile granularity stay out of scope for this PR (touches the
rules system — follow-up issue).
Comment thread Cotabby/Services/Suggestion/SuggestionInteractionState.swift Outdated
Comment thread CotabbyTests/SuggestionStateHelperTests.swift
…ase test

Addresses review feedback on FuJacob#388:
- prepareAcceptance: replace preconditionFailure on the .full arm with
  assertionFailure + an .invalid return so a future mis-route degrades
  gracefully in release instead of crashing, matching the other .invalid
  branches.
- Extract the duplicated Acceptance Mode picker and binding from SettingsView
  and ShortcutsPaneView into a shared AcceptanceModePickerView.
- Add an integration test driving prepareAcceptance(granularity: .phrase)
  through nextAcceptancePhrase.
@rpickmans
Copy link
Copy Markdown
Contributor Author

Pushed 24233cf addressing the three points:

  • The .full arm of prepareAcceptance now uses assertionFailure plus an .invalid return, so a stray .full degrades gracefully in release.
  • Pulled the duplicated Acceptance Mode picker and its binding out of SettingsView and ShortcutsPaneView into a shared AcceptanceModePickerView, so the labels and cases live in one place.
  • Added the .phrase integration test through prepareAcceptance.

Build and the full test suite pass (450 tests), SwiftLint is clean, and the project file was regenerated with xcodegen.

@FuJacob
Copy link
Copy Markdown
Owner

FuJacob commented May 29, 2026

Thanks @rpickmans will take a look tonight! :)

Copy link
Copy Markdown
Owner

@FuJacob FuJacob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thank you for your hard work!! :)

@FuJacob FuJacob merged commit 0ab34d0 into FuJacob:main May 30, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add suggestion acceptance behavior settings

2 participants