Skip to content

refactor: wave 2 high-impact consolidation#189

Merged
datlechin merged 2 commits intomainfrom
wave2-consolidation
Mar 7, 2026
Merged

refactor: wave 2 high-impact consolidation#189
datlechin merged 2 commits intomainfrom
wave2-consolidation

Conversation

@datlechin
Copy link
Owner

Summary

Wave 2 of the refactoring plan from REFACTOR_TRACKER.md. Consolidates the highest-impact code duplication across 4 independent items, each touching different files with no overlap.

DUP-1: Extract SQLStatementScanner

  • New SQLStatementScanner enum consolidates 3 duplicate SQL statement-boundary FSMs into one
  • Deleted ~280 lines from MainContentCoordinator, MainContentCoordinator+MultiStatement, and SQLContextAnalyzer
  • Bug fix: SQLContextAnalyzer's version was missing block comment (/* */), double-quote, and backtick handling
  • 22 new tests covering all edge cases (strings, comments, escapes, cursor positioning)

DUP-2: Move AI provider resolution to AIProviderFactory

  • Added resolveProvider(for:settings:) and resolveModel(for:config:settings:) to AIProviderFactory
  • Deleted identical private methods from InlineSuggestionManager and AIChatViewModel (~60 lines)

DUP-3: Add DatabaseType.beginTransactionSQL

  • New computed property replaces 5 inline switches + 1 helper method across 7 files
  • Bug fix: MainContentCoordinator+TableOperations and MainContentCoordinator+SQLPreview gave MySQL/MariaDB "BEGIN" instead of "START TRANSACTION"

NOTIF-1: Centralize notification names

  • Moved 12 scattered Notification.Name extensions from 6 files into new AppNotifications.swift
  • Zero call-site changes needed — same extensions remain in scope

Test fixes

  • Fixed flaky QueryHistoryStorage/HistoryDataProvider tests caused by clearAllHistory() cross-suite interference
  • Added QueryHistoryStorage(isolatedForTesting:) and QueryHistoryManager(isolatedStorage:) for test isolation

Stats

  • 27 files changed, ~480 lines removed, ~140 lines added (net -340)
  • 3 new files, 24 modified files

Test plan

  • All 22 new SQLStatementScannerTests pass
  • All existing SQLContextAnalyzerTests pass (43 tests)
  • Full test suite passes (2 consecutive runs, 0 failures)
  • xcodebuild build succeeds
  • Manual: multi-statement query with block comments in editor
  • Manual: autocomplete inside block comment returns no completions

- Extract SQLStatementScanner from 3 duplicate FSM implementations
- Move AI provider resolution (resolveProvider/resolveModel) to AIProviderFactory
- Add DatabaseType.beginTransactionSQL computed property, replacing 5 switches + 1 helper
- Centralize 12 scattered notification names into AppNotifications.swift
- Fix MySQL/MariaDB getting BEGIN instead of START TRANSACTION in table operations and SQL preview
- Fix SQLContextAnalyzer missing block comment, double-quote, and backtick handling
- Fix flaky history tests caused by clearAllHistory cross-suite interference
@chatgpt-codex-connector
Copy link

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@datlechin datlechin self-assigned this Mar 7, 2026
@datlechin datlechin merged commit 9dd0815 into main Mar 7, 2026
1 check passed
@datlechin datlechin deleted the wave2-consolidation branch March 7, 2026 05:01
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.

1 participant