Reimplement field-edge tabby icon indicator as default#123
Merged
Conversation
- Change default indicator mode from caretAnchor to fieldEdgeIcon - Implement FieldEdgeIconView rendering a small pawprint below the caret - Update ActivationIndicatorController.show() to accept mode enum instead of boolean - Update AppDelegate to pass selectedIndicatorMode directly - Toggle shim now maps on → fieldEdgeIcon instead of caretAnchor - Fix SwiftLint line_length violation in SettingsView.swift Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When the screen contains chatbot output with repeated phrases like "Final Answer\nFinal Answer\n...", the model learns the pattern and loops it in the summary output. Two fixes: 1. Deduplicate consecutive identical lines from OCR text before building the prompt, removing the repeating signal at the source. 2. Raise the summary repetition penalty from 1.1 to 1.4 so the sampler actively discourages looping even if non-consecutive repetition slips through. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+57
to
+72
| /// Collapses runs of identical trimmed lines to a single occurrence. | ||
| /// Preserves blank lines and non-duplicate content unchanged. | ||
| private func deduplicateConsecutiveLines(_ text: String) -> String { | ||
| var result: [String] = [] | ||
| var previous: String? | ||
| for line in text.components(separatedBy: "\n") { | ||
| let trimmed = line.trimmingCharacters(in: .whitespaces) | ||
| if trimmed.isEmpty || trimmed != previous { | ||
| result.append(line) | ||
| if !trimmed.isEmpty { | ||
| previous = trimmed | ||
| } | ||
| } | ||
| } | ||
| return result.joined(separator: "\n") | ||
| } |
Contributor
There was a problem hiding this comment.
previous persists across blank lines, deduplicating non-consecutive repetitions
previous is only updated for non-empty trimmed lines, but it is never reset when a blank line is encountered. A sequence like "Section A\n\nSection A\nContent" will silently drop the second "Section A" even though the two occurrences appear in different document regions. The docstring says "Collapses runs of identical trimmed lines", but the implementation deduplicates any identical line that shares its most-recent non-empty predecessor, which is broader than a consecutive run. Resetting previous = nil when a blank line is encountered would align behavior with the documented intent.
Owner
Author
There was a problem hiding this comment.
better to be more safe and remove as much duplicate as possible.
Reverts the indicator simplification from #117 and restores the field-edge icon mode that places tabby's app icon outside the text area's left edge. - Restore ActivationIndicatorController mode-based API with fieldEdgeIcon support - Restore FieldEdgeIconIndicatorView using NSApp.applicationIconImage - Restore fieldEdgeIconOrigin positioning logic (left-edge with screen fallback) - AppDelegate passes selectedIndicatorMode + inputFrameRect - MenuBarView and SettingsView use mode picker instead of boolean toggle - Default migration maps legacy toggle-on to fieldEdgeIcon instead of caretAnchor - Fix pre-existing SwiftLint line_length violation in SettingsView Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
FuJacob
changed the title
Contributor
|
Want your agent to iterate on Greptile's feedback? Try greploops. |
Build the field-edge indicator from SwiftUI elements instead of cropping NSApp.applicationIconImage: dark rounded rect background with a pawprint.fill SF Symbol on top. Adapts to light/dark mode. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
1 participant
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.
Summary
fieldEdgeIconmode: tabby's app icon placed outside the text area's left edgefieldEdgeIconinstead ofcaretAnchorTest plan
🤖 Generated with Claude Code
Greptile Summary
This PR restores the
fieldEdgeIconactivation indicator (tabby's paw-print icon anchored outside the text field's left edge) that was removed in #117, making it the new default mode. BothSettingsViewandMenuBarViewreplace the old on/off toggle with a three-way mode picker (None / Caret / Tabby Icon), and migration logic correctly upgrades legacy users to the new default.ActivationIndicatorControllergains afieldEdgeIconOriginhelper and a newFieldEdgeIconIndicatorView, tracking last-shown mode instead of a boolean to skip redundant frame updates.SuggestionSettingsModelupdates the migration path so fresh installs default to.fieldEdgeIcon; the legacy key is still written for backward compatibility with older builds.LlamaVisualContextSummarizeradds adeduplicateConsecutiveLinespre-pass and bumpsrepetitionPenaltyfrom 1.1 → 1.4 for the summarizer to combat hallucination loops from OCR'd repeated text.Confidence Score: 4/5
Safe to merge with one known outstanding issue: the caretRect.isEmpty guard silently hides the fieldEdgeIcon indicator in apps that report an empty caret rect but a valid input frame rect.
The fieldEdgeIconOrigin function has fallback logic to use inputFrameRect when caretRect is empty, but the guard !caretRect.isEmpty on line 60 exits before that logic can run. This means the fieldEdgeIcon indicator — the new default mode — will not appear in apps like Electron editors or browser text inputs that expose a valid input frame rect but report an empty caret rect. The rest of the changes (migration logic, UI pickers, deduplication, penalty bump) are well-scoped and correct.
tabby/Services/UI/ActivationIndicatorController.swift — the guard !caretRect.isEmpty path needs to be conditioned on mode so fieldEdgeIcon can fall through to its inputFrameRect-based positioning.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A["show(mode, caretRect, inputFrameRect)"] --> B{mode == .hidden?} B -->|Yes| C[hide] B -->|No| D{caretRect.isEmpty?} D -->|Yes| C D -->|No| E[set contentView rootView via view-for-mode] E --> F{switch mode} F -->|.caretAnchor| G[caretAnchorOrigin] F -->|.fieldEdgeIcon| H[fieldEdgeIconOrigin\nprefers inputFrameRect left edge\nfallback to right side] G --> I[compute integral frame] H --> I I --> J{lastMode == mode\nframe unchanged\npanel visible?} J -->|Yes| K[no-op] J -->|No| L[setFrame + orderFrontRegardless\nupdate lastMode]Comments Outside Diff (1)
tabby/Services/UI/ActivationIndicatorController.swift, line 60-63 (link)caretRect.isEmptyguard exits early for all modes, including.fieldEdgeIcon, whose entire purpose is to anchor to the input frame even when caret geometry is unreliable. If an app (e.g., an Electron or browser editor) reports an empty caret rect but a validinputFrameRect, the indicator is hidden despitefieldEdgeIconOriginbeing fully capable of positioning itself frominputFrameRectalone. The guard should pass through when the mode is.fieldEdgeIconand a non-emptyinputFrameRectis available.Reviews (3): Last reviewed commit: "Replace cropped app icon with SwiftUI-bu..." | Re-trigger Greptile