refactor: wave 3 architecture refactoring#191
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
- Fix orphaned escapeLikeWildcards docstring placement in SQLEscaping - Scope toUTF8Data() String extension as internal - Keep isCancelledLock and _isCancelled private in ExportService - Remove redundant init(forTesting:) from SchemaProviderRegistry
There was a problem hiding this comment.
Pull request overview
Wave 3 refactor that reduces architectural coupling and file-level complexity by extracting shared services, deduplicating SQL logic, and splitting “god files” into focused extensions—while preserving behavior.
Changes:
- Centralize SQL temporal-function detection in
SQLEscapingand reuse it from statement generation + sidebar save. - Extract per-connection schema-provider caching into
SchemaProviderRegistry(removingDatabaseManager→Viewsdependency). - Split large view/service implementations into per-concern/per-format extension files (
DataGridViewcoordinator pieces,ExportServiceexporters).
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| TableProTests/Core/Services/SchemaProviderRegistryTests.swift | Adds unit tests for new schema-provider registry behavior (creation, refcounting, purge/clear). |
| TableProTests/Core/Database/SQLEscapingTests.swift | Adds coverage for SQLEscaping.isTemporalFunction behavior (case/whitespace/known functions). |
| TablePro/Views/Results/Extensions/DataGridView+Sort.swift | Moves native sorting + header context-menu logic out of DataGridView.swift. |
| TablePro/Views/Results/Extensions/DataGridView+Selection.swift | Moves column resize/move + selection-sync logic into a dedicated extension file. |
| TablePro/Views/Results/Extensions/DataGridView+Editing.swift | Moves inline editing + overlay editor + text delegate handling into a dedicated extension file. |
| TablePro/Views/Results/Extensions/DataGridView+Columns.swift | Moves view construction and row-view provisioning into a dedicated extension file. |
| TablePro/Views/Results/Extensions/DataGridView+Click.swift | Moves click/double-click and FK navigation handlers into a dedicated extension file. |
| TablePro/Views/Results/DataGridView.swift | Adjusts access control to support cross-file extensions; removes moved coordinator logic. |
| TablePro/Views/Main/MainContentCoordinator.swift | Switches schema-provider caching from static storage to SchemaProviderRegistry. |
| TablePro/Views/Main/Extensions/MainContentCoordinator+SidebarSave.swift | Replaces local temporal-function list with SQLEscaping.isTemporalFunction. |
| TablePro/Core/Services/SchemaProviderRegistry.swift | New @MainActor registry to share/refcount SQLSchemaProvider instances with deferred cleanup. |
| TablePro/Core/Services/ExportService.swift | Keeps shared export infrastructure; loosens access where needed for per-format extensions. |
| TablePro/Core/Services/ExportService+XLSX.swift | Extracts XLSX export implementation into its own file. |
| TablePro/Core/Services/ExportService+SQL.swift | Extracts SQL export implementation into its own file. |
| TablePro/Core/Services/ExportService+MQL.swift | Extracts MongoDB/MQL export implementation into its own file. |
| TablePro/Core/Services/ExportService+JSON.swift | Extracts JSON export implementation into its own file. |
| TablePro/Core/Services/ExportService+CSV.swift | Extracts CSV export implementation into its own file. |
| TablePro/Core/Database/SQLEscaping.swift | Adds canonical temporal-function set + isTemporalFunction helper. |
| TablePro/Core/Database/DatabaseManager.swift | Updates disconnect cleanup to use SchemaProviderRegistry instead of MainContentCoordinator. |
| TablePro/Core/ChangeTracking/SQLStatementGenerator.swift | Reuses SQLEscaping.isTemporalFunction instead of maintaining its own set. |
| TablePro/ContentView.swift | Resolves schema provider via SchemaProviderRegistry instead of MainContentCoordinator. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Wave 3: Architecture Refactoring
Wave 1 fixed safety issues (#188). Wave 2 consolidated high-impact duplications (#189). Wave 3 tackles architectural debt: god files, extracted services, and remaining duplication.
Changes
DUP-4: Deduplicate SQL temporal function expressions
temporalFunctionExpressionsset +isTemporalFunction()toSQLEscapingSQLStatementGeneratorandMainContentCoordinator+SidebarSaveSQLEscapingTestsAP-6: Extract SchemaProviderRegistry from MainContentCoordinator
SchemaProviderRegistrysingletonDatabaseManager(Core) no longer importsMainContentCoordinator(Views)ContentViewnow usesSchemaProviderRegistry.shareddirectlySchemaProviderRegistryTestsGOD-2: Split DataGridView TableViewCoordinator into extensions
TableViewCoordinatorinto 5 extension files:DataGridView+Selection.swift— column resize/move, selection changeDataGridView+Editing.swift— cell editing, overlay editor, text field delegatesDataGridView+Columns.swift— viewFor, rowViewForRow delegatesDataGridView+Click.swift— click, double-click, FK arrow handlersDataGridView+Sort.swift— sort descriptors, header context menuDataGridView.swift: 1,344 → ~800 linesGOD-3: Split ExportService into per-format extensions
ExportServiceinto 5 per-format extension files:ExportService+CSV.swift,ExportService+JSON.swift,ExportService+SQL.swiftExportService+XLSX.swift,ExportService+MQL.swiftExportService.swift: 1,339 → ~410 lines (shared infrastructure only)Testing