Improve context-aware autocomplete behavior#162
Open
Joaov41 wants to merge 7 commits into
Open
Conversation
Owner
|
Thanks for the work! I tested this locally and it seems to have degraded the completions and I'm getting more issues like "```" etc than before. Could you please take a look, thanks!! |
Author
You are right. Funny enough using Apple Intelligence seems to be a little better but with other models is a regression without a doubt. Back to experimentation. |
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.
What changed
This branch ports the local changes from the modified Tabby working tree into a real git branch and documents the differences from the original upstream code.
The main behavior changes are:
Why
The goal is to make suggestions more specific and less chatty, especially in real app text fields where upstream context is sparse and model outputs can otherwise collapse into generic continuations.
Validation
xcodebuild -project tabby.xcodeproj -scheme tabby -destination 'platform=macOS' CODE_SIGNING_ALLOWED=NO -derivedDataPath .codex-derived buildNotes
I did not include local build artifacts, app bundles, or derived data in this PR.
Greptile Summary
This PR ports a working-tree delta into a clean branch, making Tabby's autocomplete more context-aware and less prone to generating generic or model-leaked suggestions. The changes are broadly well-structured and address several issues from the previous review round (pre-buffered suppression registration in
SuggestionInserter, scoped question detection viarecentSentenceFragment, anchored time-unit patterns, removal of debug AX dump scaffolding).LocalSpellCorrectionProvider,LocalWordCompletionProvider) intercept common cases before hitting the generative model, usingNSSpellCheckerwith Levenshtein scoring and safe-pattern guards; the newSuggestionAcceptanceEditenum andreplacePreviousCharacterson the inserter give spell correction a clean, atomic commit path.FocusSnapshotResolvernow collects bounded nearby AX text (maxNodes = 140, maxChars = 1 200) and field-level metadata (placeholder, title, description) and threads these through the request;SuggestionTextNormalizergains anisLikelyAuxiliaryContextCopyguard specifically tuned to reject verbatim copies of that context.schedulePredictionIfFocusedTextChangedWithoutKeyEventdetects AX-only text mutations (automation, some IMEs) and schedules prediction without a CGEvent tap; theisRefreshingFocusForInputEventflag prevents duplicate scheduling whenrefreshNow()fires its synchronous callback during normal key-event handling.Confidence Score: 5/5
Safe to merge; the new local-completion path and normalizer guards are well-bounded and the inserter's atomic suppression registration prevents the partial-failure issue from the previous review.
All core paths are covered by unit tests and the logic is deterministic. The two findings are narrow: a redundant prompt instruction wastes a token budget line, and the suffixText inclusion in the generic-continuation gate can occasionally allow a context-poor generic phrase through when the user has trailing text. Neither affects correctness of insertion, suppression, or session lifecycle.
tabby/Support/SuggestionTextNormalizer.swift (lacksConcreteAuxiliaryContext) and tabby/Support/LlamaPromptRenderer.swift (duplicate instruction)
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A["Key event / AX snapshot change"] --> B{"isRefreshingFocusForInputEvent?"} B -- yes --> C["materializeContext only (no prediction)"] B -- no --> D{"contentSignature changed & no active session?"} D -- no --> E["skip"] D -- yes --> F["schedulePrediction()"] F --> G["FocusedInputContext materialized"] G --> H{"LocalSpellCorrectionProvider.suggestion?"} H -- yes --> I["SuggestionResult .replacePreviousCharacters"] H -- no --> J{"LocalWordCompletionProvider.suggestion?"} J -- yes --> K["SuggestionResult .insert (tail only)"] J -- no --> L["Build SuggestionRequest (prefix+suffix+field+visual)"] L --> M["Prompt renderer"] M --> N["Model output"] N --> O["SuggestionTextNormalizer.normalize"] O --> P["SuggestionResult.text"] I --> Q["startSession(acceptanceEdit:)"] K --> Q P --> Q Q --> R["Ghost text overlay"] R --> S{"Tab pressed?"} S -- insert --> T["SuggestionInserter.insert"] S -- replace --> U["SuggestionInserter.replacePreviousCharacters"]Comments Outside Diff (3)
tabby/Support/SuggestionAvailabilityEvaluator.swift, line 10-56 (link)screenRecordingGrantedparameter is now unusedBoth
disabledReasonandshouldSchedulePredictionstill accept ascreenRecordingGranted: Boolparameter, but the guard that used it was removed as part of this PR. Every call-site in the coordinator still passes the value through, so the parameter is silently ignored.This is harmless in isolation, but it is misleading for callers who may assume the value continues to influence the gating decision. If Screen Recording is now intentionally an optional-only signal (for visual context), the parameter should be removed from these two functions and the call sites updated accordingly, or a comment should document that the parameter is retained only for a future use.
tabby/Services/Focus/FocusSnapshotResolver.swift, line 13-15 (link)dumpAXTreeisfalsein production so it causes no harm, but the comment explicitly marks it as "temporary — remove after caret placement is fixed." If the caret issue has been resolved, this block (along withlastDumpedElementIDand the twoprint-based helpers at the bottom of the file) should be removed before the branch is merged to avoid accumulating debug scaffolding in shared code.tabby/Models/FocusModels.swift, line 218-228 (link)fieldContextTextincontentSignaturetriggers spurious snapshot-driven predictions on external AX changesschedulePredictionIfFocusedTextChangedWithoutKeyEventfires wheneverfocusedContext.contentSignaturediffers from the previous snapshot. By addingfieldContextText ?? ""tocontentSignature, any change in the nearby AX tree — a new chat message arriving, a sidebar label refreshing, a status indicator updating — changes the signature and causes prediction to be scheduled even though the user typed nothing.In a chat app such as Slack or Messages, every incoming message updates the AX subtree around the compose field. Under the new code this would continuously trigger prediction while the user is idle in the compose box, wasting generation cycles and potentially flashing irrelevant suggestions.
The
contentSignatureshould reflect only the editable content (precedingText,trailingText,selection).fieldContextTextis context metadata, not a user-edit signal, and should be excluded from this comparison key.Reviews (7): Last reviewed commit: "Narrow relative-time metadata filtering" | Re-trigger Greptile