refactor: decompose large source and test files#798
Conversation
a35450d to
ee74ba4
Compare
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @State var pendingDeletionModel: RuntimeModelOption? | ||
| @State var endpointAPIKeyDraft = "" | ||
| @State var endpointCredentialError: String? | ||
| /// The LM Studio models directory if it exists, probed once in `onAppear` so the filesystem | ||
| /// `fileExists` check never runs on the SwiftUI render path. Nil disables the LM Studio toggle. | ||
| @State private var lmStudioModelsURL: URL? | ||
| @State var lmStudioModelsURL: URL? | ||
| /// Whether to also scan the user's LM Studio library. Persisted via the same key the locator | ||
| /// reads, so the toggle and the model scan stay in sync. LM Studio models are an additive, | ||
| /// read-only source; Cotabby's own folder is always scanned and is always the download target. | ||
| @AppStorage(BundledRuntimeLocator.lmStudioSourceEnabledKey) private var lmStudioSourceEnabled = false | ||
| @AppStorage(BundledRuntimeLocator.lmStudioSourceEnabledKey) var lmStudioSourceEnabled = false |
There was a problem hiding this comment.
@State/@AppStorage widened to internal
The PR description explains this is forced by Swift's rule that private declarations in one file are not visible to extension members in another file. That's correct — but it means pendingDeletionModel, endpointAPIKeyDraft, endpointCredentialError, lmStudioModelsURL, and lmStudioSourceEnabled are now settable by any other type in the Cotabby module. For @AppStorage, a write from outside the view bypasses SwiftUI's change-tracking and can leave the UI stale until the next render cycle. There is no evidence this happens today, but the exposure is permanent once the access level is relaxed. An internal(set) modifier (@State internal(set) var …) would allow external reads while restricting writes to the same-file (extension) surface that actually needs them.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| static func color(for colorScheme: ColorScheme) -> Color { | ||
| colorScheme == .dark | ||
| ? Color(red: 0.45, green: 0.85, blue: 0.45) | ||
| : Color(red: 0.15, green: 0.60, blue: 0.20) | ||
| } | ||
| } | ||
|
|
||
| /// Small SwiftUI view hosted inside the floating AppKit panel. | ||
| /// Keeping the rendered content separate from the window controller makes styling easier to evolve | ||
| /// without touching the AppKit positioning code. | ||
| struct GhostSuggestionView: View { | ||
| @Environment(\.colorScheme) var colorScheme | ||
| let layout: GhostSuggestionLayout |
There was a problem hiding this comment.
SuggestionCorrectionStyle and GhostSuggestionView promoted to internal
Both types were private when they lived inside OverlayController.swift. Moving them to a separate file was the right call, but it silently makes them instantiable by any code in the Cotabby module — in particular, GhostSuggestionView takes a GhostSuggestionLayout and raw NSFont parameters that callers outside OverlayController have no business constructing. A // swiftlint:disable:next private_type_acl suppression would be self-defeating, but marking them internal explicitly (already the default) with a file-level note, or moving them into the OverlayController file as fileprivate, keeps the intent clear. Not a blocking issue since this is a single-target app, but the door is now open.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Summary
Decomposes the largest mixed-responsibility files into cohesive suggestion-domain models, engine-pane sections, overlay view content, and focused test suites. The change is intentionally behavior-preserving and makes the next settings/coordinator refactors reviewable at their actual ownership boundaries.
Validation
swiftlint lint --quiet— exit 0xcodegen generate— project regenerated successfullyxcodebuild -project Cotabby.xcodeproj -scheme Cotabby -destination 'platform=macOS' build-for-testing -derivedDataPath build/DerivedData— TEST BUILD SUCCEEDEDtest-without-buildingwas attempted; the test host could not load the ad-hoc-signed bundle because the app and test bundle have different Team IDs. This is the repository's known local hosted-test signing limitation; compilation and linking succeeded.Linked issues
None.
Risk / rollout notes
This PR is stacked on #797. Cross-file
EngineAndModelPaneViewextension members are module-internal because Swift cannot share lexicalprivateaccess across files; the view type itself remains internal and no public API is introduced. No persistence keys, snapshots, runtime behavior, or UI copy change.Greptile Summary
This PR decomposes two large monolithic files —
SuggestionModels.swift(766 lines) andSuggestionSessionReconcilerTests.swift(1177 lines) — plusEngineAndModelPaneView.swiftandOverlayController.swiftinto cohesive single-responsibility files. The change is behavior-preserving: no logic, UI copy, persistence keys, or model contracts change.SuggestionModels.swiftis split into 7 focused model files (ActiveSuggestionSession,FocusedInputContext,SuggestionConfiguration,SuggestionRequest,SuggestionResult,SuggestionPresentationModels,SuggestionClientError).EngineAndModelPaneView.swiftis decomposed into extension files per section (+Actions,+Power,+Endpoint,+OpenSource,+AppleIntelligence);@State/@AppStorageproperties widen fromprivatetointernalas a required Swift constraint for cross-file extensions.Confidence Score: 4/5
Safe to merge — no logic, persistence keys, or UI copy changed; the decomposition is structural only.
The split is well-executed: test method counts were manually verified (109 + 41 methods fully preserved), the Xcode project was regenerated, and the build succeeded. The one genuine side-effect is that @State and @AppStorage properties on EngineAndModelPaneView lost their private modifier — a Swift cross-file extension constraint documented in the PR description — and GhostSuggestionView/SuggestionCorrectionStyle similarly moved from private to internal. Neither change affects current runtime behavior in this single-target app, but the expanded access surface is a permanent trade-off worth being aware of on future refactors.
EngineAndModelPaneView.swift and SuggestionOverlayViews.swift are where the access-level widening lands; everything else is a straight move with no behavioural delta.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD subgraph Before["Before (monolithic files)"] SM["SuggestionModels.swift\n766 lines"] EV["EngineAndModelPaneView.swift\n~730 lines"] OC["OverlayController.swift\n+GhostSuggestionView\n+SuggestionCorrectionStyle"] RT["SuggestionSessionReconcilerTests.swift\n1177 lines"] AE["SuggestionAvailabilityEvaluatorTests.swift\n783 lines"] end subgraph After["After (decomposed)"] SC["SuggestionConfiguration.swift"] AS["ActiveSuggestionSession.swift"] FI["FocusedInputContext.swift"] SR["SuggestionRequest.swift"] SRes["SuggestionResult.swift"] SP["SuggestionPresentationModels.swift"] SCE["SuggestionClientError.swift"] EV2["EngineAndModelPaneView.swift\n(struct + body only)"] EA["EngineAndModelPaneView+Actions.swift"] EP["EngineAndModelPaneView+Power.swift"] EEnd["EngineAndModelPaneView+Endpoint.swift"] EOS["EngineAndModelPaneView+OpenSource.swift"] EAI["EngineAndModelPaneView+AppleIntelligence.swift"] SOV["SuggestionOverlayViews.swift"] T1["SuggestionSessionReconciliationTests"] T2["SuggestionSessionTypingTests"] T3["SuggestionInsertionChunkTests"] T4["SuggestionPhraseAcceptanceTests"] T5["SuggestionWordAcceptanceTests"] T6["SuggestionOverlayAcceptanceTests"] T7["SuggestionStaleAcceptanceEchoTests"] T8["FocusSnapshotExternalApplicationIdentityTests"] T9["SuggestionSettingsModelDisabledAppsTests"] end SM -->|split| SC & AS & FI & SR & SRes & SP & SCE EV -->|decomposed| EV2 & EA & EP & EEnd & EOS & EAI OC -->|extracted| SOV RT -->|split 109 tests| T1 & T2 & T3 & T4 & T5 & T6 & T7 AE -->|split 41 tests| T8 & T9 & AE%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD subgraph Before["Before (monolithic files)"] SM["SuggestionModels.swift\n766 lines"] EV["EngineAndModelPaneView.swift\n~730 lines"] OC["OverlayController.swift\n+GhostSuggestionView\n+SuggestionCorrectionStyle"] RT["SuggestionSessionReconcilerTests.swift\n1177 lines"] AE["SuggestionAvailabilityEvaluatorTests.swift\n783 lines"] end subgraph After["After (decomposed)"] SC["SuggestionConfiguration.swift"] AS["ActiveSuggestionSession.swift"] FI["FocusedInputContext.swift"] SR["SuggestionRequest.swift"] SRes["SuggestionResult.swift"] SP["SuggestionPresentationModels.swift"] SCE["SuggestionClientError.swift"] EV2["EngineAndModelPaneView.swift\n(struct + body only)"] EA["EngineAndModelPaneView+Actions.swift"] EP["EngineAndModelPaneView+Power.swift"] EEnd["EngineAndModelPaneView+Endpoint.swift"] EOS["EngineAndModelPaneView+OpenSource.swift"] EAI["EngineAndModelPaneView+AppleIntelligence.swift"] SOV["SuggestionOverlayViews.swift"] T1["SuggestionSessionReconciliationTests"] T2["SuggestionSessionTypingTests"] T3["SuggestionInsertionChunkTests"] T4["SuggestionPhraseAcceptanceTests"] T5["SuggestionWordAcceptanceTests"] T6["SuggestionOverlayAcceptanceTests"] T7["SuggestionStaleAcceptanceEchoTests"] T8["FocusSnapshotExternalApplicationIdentityTests"] T9["SuggestionSettingsModelDisabledAppsTests"] end SM -->|split| SC & AS & FI & SR & SRes & SP & SCE EV -->|decomposed| EV2 & EA & EP & EEnd & EOS & EAI OC -->|extracted| SOV RT -->|split 109 tests| T1 & T2 & T3 & T4 & T5 & T6 & T7 AE -->|split 41 tests| T8 & T9 & AEReviews (1): Last reviewed commit: "refactor: decompose large source and tes..." | Re-trigger Greptile