refactor: replace settings notifications with @Observable and direct calls

[datlechin]

Summary

• Remove 7 dead NotificationCenter notification names (appearanceSettingsDidChange, generalSettingsDidChange, tabSettingsDidChange, keyboardSettingsDidChange, aiSettingsDidChange, settingsDidChange, historySettingsDidChange) and the SettingsChangeInfo infrastructure
• Replace historySettingsDidChange Combine subscriber in QueryHistoryManager with a direct applySettingsChange() call from AppSettingsManager
• Convert 2 SwiftUI .onReceive notification subscribers to @Observable observation (.onChange(of:) in SQLEditorView, direct property read in QueryEditorView)
• Remove 11 dead notification observers and their notification definitions from Phase 1
• 3 notifications remain: dataGridSettingsDidChange, editorSettingsDidChange (AppKit bridges), accessibilityTextSizeDidChange (system event)

Net: -259 lines across both commits. Zero Combine imports remain in settings code.

Test plan

• Build succeeds with no warnings in changed files
• Change editor settings (font size, vim mode, word wrap) — editor updates live
• Change data grid settings (row height, date format) — grid updates live
• Change history settings (auto-cleanup, retention) — cleanup runs immediately
• Toggle vim mode on/off — indicator in QueryEditorView toolbar appears/disappears
• Grep for removed notification names returns zero hits in TablePro/

Summary by CodeRabbit

• Refactor

  • Streamlined settings change propagation system, consolidating multiple notification types into a unified mechanism for improved efficiency and reliability.
  • Simplified editor settings synchronization to enable more direct, responsive UI updates for features like Vim mode indication.

• Documentation

  • Updated plugin settings tracking documentation with current line references and driver plugin count.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR refactors the settings notification architecture by removing seven domain-specific notification names and a SettingsChangeInfo payload struct from the notification layer. AppSettingsManager now posts notifications differently, QueryHistoryManager replaces NotificationCenter subscription with an explicit applySettingsChange() method, and UI views switch from notification observers to direct setting observation or onChange triggers.

Changes

Cohort / File(s)	Summary
Settings Notification Architecture Cleanup TablePro/Core/Services/Infrastructure/SettingsNotifications.swift	Removed domain-specific notification names (history, appearance, general, tabs, keyboard, ai, generic settingsDidChange) and deleted SettingsChangeInfo struct with userInfo handling extension. Retained only dataGridSettingsDidChange, editorSettingsDidChange, and accessibilityTextSizeDidChange.
Settings Manager Notification Refactor TablePro/Core/Storage/AppSettingsManager.swift, TablePro/Core/Storage/QueryHistoryManager.swift	Simplified notification posting to single call per domain instead of dual-post pattern. QueryHistoryManager removes NotificationCenter subscription and adds explicit @MainActor applySettingsChange() method to handle settings updates and conditional cleanup.
UI Observer Pattern Updates TablePro/Views/Editor/QueryEditorView.swift, TablePro/Views/Editor/SQLEditorView.swift	Removed local state tracking for vim mode and editor settings changes. Views now read directly from AppSettingsManager.shared or use onChange triggers instead of onReceive notification observers.
Documentation & Localization TablePro/Resources/Localizable.xcstrings, docs/development/plugin-settings-tracking.md	Updated localization entries as stale; corrected line number references in plugin settings documentation and updated driver plugin count from 8 to 9.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Notifications trimmed with careful paws,
Domain-specific signals now withdrawn without a pause,
Direct observation blooms where broadcast leaves did fall,
Settings flow more clearly—simpler, cleaner, less enthrall!
🎉

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: removing NotificationCenter-based settings infrastructure and replacing it with @Observable and direct method calls, which is the primary objective across all modified files.
Docstring Coverage	✅ Passed	Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch refactor/remove-dead-notifications

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

TablePro/Core/Storage/AppSettingsManager.swift (1)

82-85: ⚠️ Potential issue | 🟠 Major

Apply history settings inline instead of hopping through Task.

After this refactor, applyHistorySettingsImmediately() is just a synchronous @MainActor call into QueryHistoryManager.shared.applySettingsChange(). Wrapping it in Task delays updateSettingsCache(), so QueryHistoryStorage can briefly keep the old retention limits after history changes. Any history write in that window still uses stale settings. Calling it inline closes that gap, and storage.cleanup() already moves its work off the main actor anyway.

🛠️ Proposed fix

-            Task { await applyHistorySettingsImmediately() }
+            applyHistorySettingsImmediately()
@@
-    private func applyHistorySettingsImmediately() async {
+    private func applyHistorySettingsImmediately() {
         QueryHistoryManager.shared.applySettingsChange()
     }

Also applies to: 178-180

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@TablePro/Core/Storage/AppSettingsManager.swift` around lines 82 - 85, Replace
the asynchronous fire-and-forget Task call with an inline await call so settings
are applied immediately: after storage.saveHistory(validated) and the other
occurrence (lines around the second save), call await
applyHistorySettingsImmediately() directly instead of wrapping it in Task {
await applyHistorySettingsImmediately() }; this ensures updateSettingsCache()
and QueryHistoryStorage see the new retention limits
immediately—applySettingsChange() on QueryHistoryManager.shared and
storage.cleanup() can remain unchanged.

🧹 Nitpick comments (1)

docs/development/plugin-settings-tracking.md (1)

21-29: Avoid hard-coded source line numbers in this doc.

These references already drifted in this PR. Prefer symbol names or file-only references so routine refactors in PluginManager.swift do not force doc churn.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@docs/development/plugin-settings-tracking.md` around lines 21 - 29, Replace
the hard-coded source line numbers with stable identifiers: use file-only
references (e.g., Core/Plugins/PluginManager.swift) and, where relevant, symbol
or method names (e.g., PluginManager.register/unregister,
PluginManager.verifyCodeSignature, InstalledPluginsView, BrowsePluginsView,
RegistryPluginRow, PluginInstallModifier) instead of "PluginManager.swift:382"
or ":586"; update each table row to reference the file or symbol name and a
short context note (e.g., "PluginManager.register/unregister — immediate
capability register/unregister") so routine refactors won't require doc changes.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@TablePro/Core/Storage/AppSettingsManager.swift`:
- Around line 82-85: Replace the asynchronous fire-and-forget Task call with an
inline await call so settings are applied immediately: after
storage.saveHistory(validated) and the other occurrence (lines around the second
save), call await applyHistorySettingsImmediately() directly instead of wrapping
it in Task { await applyHistorySettingsImmediately() }; this ensures
updateSettingsCache() and QueryHistoryStorage see the new retention limits
immediately—applySettingsChange() on QueryHistoryManager.shared and
storage.cleanup() can remain unchanged.

---

Nitpick comments:
In `@docs/development/plugin-settings-tracking.md`:
- Around line 21-29: Replace the hard-coded source line numbers with stable
identifiers: use file-only references (e.g., Core/Plugins/PluginManager.swift)
and, where relevant, symbol or method names (e.g.,
PluginManager.register/unregister, PluginManager.verifyCodeSignature,
InstalledPluginsView, BrowsePluginsView, RegistryPluginRow,
PluginInstallModifier) instead of "PluginManager.swift:382" or ":586"; update
each table row to reference the file or symbol name and a short context note
(e.g., "PluginManager.register/unregister — immediate capability
register/unregister") so routine refactors won't require doc changes.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3a121119-8ecd-45d4-91e5-a6dc38d41a85

📥 Commits

Reviewing files that changed from the base of the PR and between 585ab90 and da970ff.

📒 Files selected for processing (7)

• TablePro/Core/Services/Infrastructure/SettingsNotifications.swift
• TablePro/Core/Storage/AppSettingsManager.swift
• TablePro/Core/Storage/QueryHistoryManager.swift
• TablePro/Resources/Localizable.xcstrings
• TablePro/Views/Editor/QueryEditorView.swift
• TablePro/Views/Editor/SQLEditorView.swift
• docs/development/plugin-settings-tracking.md