Skip to content

Commit ae17f84

Browse files
datlechindatlechin
authored
fix: refactor autocomplete system for reliability (#384)
* fix: refactor autocomplete system for reliability and correct behavior - Strip .capsLock and .function from modifier flags so Escape/Ctrl+Space work on all keyboards - Remove weak var on SourceEditor struct delegates to prevent nil reference issues - Cancel in-flight completion tasks instead of blocking new triggers - Replace NSWindow with NSPanel subclass to fix canBecomeKeyWindow warnings - Add isManualTrigger flag so Ctrl+Space shows completions even with empty prefix - Wire up updateSchemaProvider on connection change (was dead code) - Add schema load retry with 30s cooldown for failed initial loads - Add diagnostic logging at key completion pipeline failure points * fix: address PR review feedback - Add setupFavoritesObserver() on connectionId change to rebind favorites - Add explicit access control to SuggestionPanel (internal) - Use package-appropriate logger subsystem in CodeEditSourceEditor
1 parent fe882ff commit ae17f84

12 files changed

Lines changed: 105 additions & 26 deletions

File tree

‎LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/CodeSuggestion/Model/CodeSuggestionDelegate.swift‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ public protocol CodeSuggestionDelegate: AnyObject {
1111

1212
func completionSuggestionsRequested(
1313
textView: TextViewController,
14-
cursorPosition: CursorPosition
14+
cursorPosition: CursorPosition,
15+
isManualTrigger: Bool
1516
) async -> (windowPosition: CursorPosition, items: [CodeSuggestionEntry])?
1617

1718
// This can't be async, we need it to be snappy. At most, it should just be filtering completion items
@@ -36,4 +37,19 @@ public extension CodeSuggestionDelegate {
3637
func completionTriggerCharacters() -> Set<String> { [] }
3738
func completionWindowDidClose() { }
3839
func completionWindowDidSelect(item: CodeSuggestionEntry) { }
40+
41+
func completionSuggestionsRequested(
42+
textView: TextViewController,
43+
cursorPosition: CursorPosition,
44+
isManualTrigger: Bool
45+
) async -> (windowPosition: CursorPosition, items: [CodeSuggestionEntry])? {
46+
await completionSuggestionsRequested(textView: textView, cursorPosition: cursorPosition)
47+
}
48+
49+
func completionSuggestionsRequested(
50+
textView: TextViewController,
51+
cursorPosition: CursorPosition
52+
) async -> (windowPosition: CursorPosition, items: [CodeSuggestionEntry])? {
53+
nil
54+
}
3955
}

‎LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/CodeSuggestion/Model/SuggestionTriggerCharacterModel.swift‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
import AppKit
99
import CodeEditTextView
10+
import os
1011
import TextStory
1112

1213
/// Triggers the suggestion window when trigger characters are typed.
@@ -17,11 +18,14 @@ import TextStory
1718
/// essentially a textview delegate ensures both of those promises are upheld.
1819
@MainActor
1920
final class SuggestionTriggerCharacterModel {
21+
private static let logger = Logger(subsystem: "com.CodeEditSourceEditor", category: "CompletionTrigger")
22+
2023
weak var controller: TextViewController?
2124
private var lastPosition: NSRange?
2225

2326
func textView(_ textView: TextView, didReplaceContentsIn range: NSRange, with string: String) {
2427
guard let controller, let completionDelegate = controller.completionDelegate else {
28+
Self.logger.debug("Typing trigger skipped: controller=\(self.controller == nil ? "nil" : "set"), completionDelegate=\(self.controller?.completionDelegate == nil ? "nil" : "set")")
2529
return
2630
}
2731

‎LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/CodeSuggestion/Model/SuggestionViewModel.swift‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@
66
//
77

88
import AppKit
9+
import os
910

1011
@MainActor
1112
final class SuggestionViewModel: ObservableObject {
13+
private static let logger = Logger(subsystem: "com.CodeEditSourceEditor", category: "SuggestionVM")
1214
/// The items to be displayed in the window
1315
@Published var items: [CodeSuggestionEntry] = []
1416
@Published var selectedIndex: Int = 0
@@ -72,13 +74,17 @@ final class SuggestionViewModel: ObservableObject {
7274
textView: TextViewController,
7375
delegate: CodeSuggestionDelegate,
7476
cursorPosition: CursorPosition,
77+
isManualTrigger: Bool = false,
7578
showWindowOnParent: @escaping @MainActor (NSWindow, NSRect) -> Void
7679
) {
7780
self.activeTextView = nil
7881
self.delegate = nil
7982
itemsRequestTask?.cancel()
8083

81-
guard let targetParentWindow = textView.view.window else { return }
84+
guard let targetParentWindow = textView.view.window else {
85+
Self.logger.warning("showCompletions: textView.view.window is nil")
86+
return
87+
}
8288

8389
self.activeTextView = textView
8490
self.delegate = delegate
@@ -88,11 +94,15 @@ final class SuggestionViewModel: ObservableObject {
8894
do {
8995
guard let completionItems = await delegate.completionSuggestionsRequested(
9096
textView: textView,
91-
cursorPosition: cursorPosition
97+
cursorPosition: cursorPosition,
98+
isManualTrigger: isManualTrigger
9299
) else {
100+
Self.logger.debug("showCompletions: delegate returned nil items")
93101
return
94102
}
95103

104+
Self.logger.debug("showCompletions: got \(completionItems.items.count) items")
105+
96106
try Task.checkCancellation()
97107
try await MainActor.run {
98108
try Task.checkCancellation()
@@ -104,6 +114,7 @@ final class SuggestionViewModel: ObservableObject {
104114
let cursorRect = textView.view.window?.convertToScreen(
105115
textView.textView.convert(cursorRect, to: nil)
106116
) else {
117+
Self.logger.warning("showCompletions: cursor rect resolution failed")
107118
return
108119
}
109120

@@ -125,7 +136,10 @@ final class SuggestionViewModel: ObservableObject {
125136
position: CursorPosition,
126137
close: () -> Void
127138
) {
128-
guard itemsRequestTask == nil else { return }
139+
if itemsRequestTask != nil {
140+
itemsRequestTask?.cancel()
141+
itemsRequestTask = nil
142+
}
129143

130144
if activeTextView !== textView {
131145
close()

‎LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/CodeSuggestion/Window/SuggestionController+Window.swift‎

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,11 @@
88
import AppKit
99
import SwiftUI
1010

11+
internal final class SuggestionPanel: NSPanel {
12+
override var canBecomeKey: Bool { false }
13+
override var canBecomeMain: Bool { false }
14+
}
15+
1116
extension SuggestionController {
1217
/// Will constrain the window's frame to be within the visible screen
1318
public func constrainWindowToScreenEdges(cursorRect: NSRect, font: NSFont) {
@@ -90,25 +95,26 @@ extension SuggestionController {
9095

9196
// MARK: - Private Methods
9297

93-
static func makeWindow() -> NSWindow {
94-
let window = NSWindow(
98+
static func makeWindow() -> NSPanel {
99+
let panel = SuggestionPanel(
95100
contentRect: .zero,
96101
styleMask: [.resizable, .fullSizeContentView, .nonactivatingPanel, .utilityWindow],
97102
backing: .buffered,
98103
defer: false
99104
)
100105

101-
window.titleVisibility = .hidden
102-
window.titlebarAppearsTransparent = true
103-
window.isExcludedFromWindowsMenu = true
104-
window.isReleasedWhenClosed = false
105-
window.level = .popUpMenu
106-
window.hasShadow = true
107-
window.isOpaque = false
108-
window.tabbingMode = .disallowed
109-
window.hidesOnDeactivate = true
110-
window.backgroundColor = .clear
111-
112-
return window
106+
panel.becomesKeyOnlyIfNeeded = true
107+
panel.titleVisibility = .hidden
108+
panel.titlebarAppearsTransparent = true
109+
panel.isExcludedFromWindowsMenu = true
110+
panel.isReleasedWhenClosed = false
111+
panel.level = .popUpMenu
112+
panel.hasShadow = true
113+
panel.isOpaque = false
114+
panel.tabbingMode = .disallowed
115+
panel.hidesOnDeactivate = true
116+
panel.backgroundColor = .clear
117+
118+
return panel
113119
}
114120
}

‎LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/CodeSuggestion/Window/SuggestionController.swift‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,12 +89,14 @@ public final class SuggestionController: NSWindowController {
8989
textView: TextViewController,
9090
delegate: CodeSuggestionDelegate,
9191
cursorPosition: CursorPosition,
92+
isManualTrigger: Bool = false,
9293
asPopover: Bool = false
9394
) {
9495
model.showCompletions(
9596
textView: textView,
9697
delegate: delegate,
97-
cursorPosition: cursorPosition
98+
cursorPosition: cursorPosition,
99+
isManualTrigger: isManualTrigger
98100
) { parentWindow, cursorRect in
99101
self.model.updateTheme(from: textView)
100102

‎LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/Controller/TextViewController+Lifecycle.swift‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,7 @@ extension TextViewController {
206206

207207
func handleEvent(event: NSEvent) -> NSEvent? {
208208
let modifierFlags = event.modifierFlags.intersection(.deviceIndependentFlagsMask)
209+
.subtracting([.capsLock, .function])
209210
switch event.type {
210211
case .keyDown:
211212
let tabKey: UInt16 = 0x30
@@ -309,7 +310,8 @@ extension TextViewController {
309310
SuggestionController.shared.showCompletions(
310311
textView: self,
311312
delegate: completionDelegate,
312-
cursorPosition: cursorPosition
313+
cursorPosition: cursorPosition,
314+
isManualTrigger: true
313315
)
314316
return nil
315317
}

‎LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/SourceEditor/SourceEditor.swift‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,8 @@ public struct SourceEditor: NSViewControllerRepresentable {
9090
var highlightProviders: [any HighlightProviding]?
9191
var undoManager: CEUndoManager?
9292
var coordinators: [any TextViewCoordinator]
93-
weak var completionDelegate: CodeSuggestionDelegate?
94-
weak var jumpToDefinitionDelegate: JumpToDefinitionDelegate?
93+
var completionDelegate: CodeSuggestionDelegate?
94+
var jumpToDefinitionDelegate: JumpToDefinitionDelegate?
9595

9696
public typealias NSViewControllerType = TextViewController
9797

‎TablePro/Core/Autocomplete/CompletionEngine.swift‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,10 @@ final class CompletionEngine {
5151
provider.updateFavoriteKeywords(keywords)
5252
}
5353

54+
func retrySchemaIfNeeded() async {
55+
await provider.retrySchemaIfNeeded()
56+
}
57+
5458
/// Get completions for the given text and cursor position
5559
/// This is a pure function - no side effects
5660
func getCompletions(

‎TablePro/Core/Autocomplete/SQLCompletionProvider.swift‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,10 @@ final class SQLCompletionProvider {
5959
self.favoriteKeywords = keywords
6060
}
6161

62+
func retrySchemaIfNeeded() async {
63+
await schemaProvider.retryLoadSchemaIfNeeded()
64+
}
65+
6266
// MARK: - Public API
6367

6468
/// Get completion suggestions for the current cursor position

‎TablePro/Core/Autocomplete/SQLSchemaProvider.swift‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ actor SQLSchemaProvider {
1818
private let maxCachedTables = 50
1919
private var isLoading = false
2020
private var lastLoadError: Error?
21+
private var lastRetryAttempt: Date?
22+
private let retryCooldown: TimeInterval = 30
2123

2224
// Store a weak driver reference to avoid retaining it after disconnect (MEM-9)
2325
private weak var cachedDriver: (any DatabaseDriver)?
@@ -83,6 +85,15 @@ actor SQLSchemaProvider {
8385
}
8486
}
8587

88+
func retryLoadSchemaIfNeeded() async {
89+
guard lastLoadError != nil, tables.isEmpty, !isLoading else { return }
90+
guard let driver = cachedDriver else { return }
91+
if let last = lastRetryAttempt, Date().timeIntervalSince(last) < retryCooldown { return }
92+
lastRetryAttempt = Date()
93+
lastLoadError = nil
94+
await loadSchema(using: driver, connection: connectionInfo)
95+
}
96+
8697
/// Check if schema is loaded
8798
func isSchemaLoaded() -> Bool {
8899
!tables.isEmpty

0 commit comments

Comments
 (0)