refactor(architecture): enforce module boundaries - #153
Merged
Conversation
…r/reconciler Give file discovery, status resolution, and rename reconciliation each a single owning class (SyncFileDiscovery, SyncStatusResolver, RenameReconciler) instead of one 661-line service implementing all three algorithms. SyncStatusRefreshService now only orchestrates the three plus the incremental create/modify/delete/rename handlers, per the module boundaries in docs/architecture.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ol composition root Move the sync-domain and Source Control application constructor graph (SyncManager, SyncStatusRefreshService, SyncDiffService, SyncWorkspace, ChangeRepository, SyncSelectionStore, OperationState, RefreshState, SourceControlViewModel, SourceControlActionService, and the ChangeRepository<->SyncStatusService wiring) out of main.ts and into src/runtime/createSyncRuntime.ts. main.ts now only knows Obsidian lifecycle: settings load/save, view/command/ribbon registration, vault event registration -- it no longer needs to know the sync/Source Control constructor graph to add a plugin lifecycle hook. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…rom SourceControlView Pull the "Sync Queue" and "Repository Changes" regions out of the 730-line SourceControlView into standalone render functions, matching the existing FilterMenu/SourceControlHeader/ChangeTree pattern: pure functions taking state + callbacks, never SyncWorkspace/SourceControlActionService/ SourceControlViewModel directly. SourceControlView keeps ownership of view state (collapsed sections, view mode, folder collapse) and now only orchestrates rendering, the diff pane, and scroll-state management. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…sync->UI dependency Add no-restricted-imports rules per docs/architecture.md: src/ui/** and src/logic/source-control/** may not import a concrete Git provider or a push/pull coordinator/executor directly (must go through SyncWorkspace), and src/logic/sync/** may not import src/ui/source-control/** (dependency direction runs UI -> domain, never back). The last rule caught a real pre-existing violation: SyncDiffService and SyncInteractionPort (sync domain) imported computeDiffStat and DiffStatLoadResult from ui/source-control. Move the pure diff-stat computation (computeDiffStat, cheapLocalStat, addedContentStat, deletedContentStat) and the DiffStatLoadResult contract into a new src/logic/sync/DiffStat.ts; ChangePresentation.ts and DiffStatProvider.ts now depend on the domain for these instead of the other way around. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ndaries CLAUDE.md's "Code Architecture" section duplicated (and had drifted from) docs/architecture.md: it said GitLab/GitHub only (no Gitea, though GiteaService already existed) and described sync-manager.ts as a single file. Replace it with a short contract pointing at docs/architecture.md (and docs/bug-fix-guidelines.md for bug fixes) plus the two compatibility gotchas that aren't covered there. docs/architecture.md's module table and "Current hotspots" section now describe the merged code: createSyncRuntime as the composition root, SyncFileDiscovery/SyncStatusResolver/RenameReconciler as SyncStatusRefreshService's three collaborators, DiffStat.ts, and SourceControlView's reduced scope after the SyncQueueSection/ RepositoryChangesSection extraction. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Member
Author
|
🎉 This PR is included in version 1.6.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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.



Summary
1.6.1already carriesdocs/architecture.mdanddocs/bug-fix-guidelines.md(the MUST/MUST NOT module-boundary contract), but the code hadn't fully caught up to it yet, andCLAUDE.mdstill described the pre-refactor shape. This PR makes the code satisfy that contract before more bug fixes land and drift further from it — it is not a general cleanup pass. Scope is capped to what's listed below; see "Explicitly out of scope" for what's deliberately deferred.main.tsas a real composition/lifecycle root — extractedsrc/runtime/createSyncRuntime.tsto own the sync-domain + Source Control application constructor graph (SyncManager,SyncStatusRefreshService,SyncDiffService,SyncWorkspace,ChangeRepository,SyncSelectionStore,OperationState,RefreshState,SourceControlViewModel,SourceControlActionService).main.tsnow only knows Obsidian lifecycle: settings load/save, view/command/ribbon registration, vault event registration.SyncStatusRefreshService(was 661 lines, three algorithms in one class) intoSyncFileDiscovery,SyncStatusResolver, andRenameReconciler, each with a single owning responsibility.SyncStatusRefreshServicenow only orchestrates discovery → resolve → reconcile → publish, plus the incremental create/modify/delete/rename handlers (deliberately not split further — see scope below).SourceControlViewlow-risk extraction — pulled the "Sync Queue" and "Repository Changes" regions intoSyncQueueSection/RepositoryChangesSection, matching the existingFilterMenu/SourceControlHeaderpattern (pure functions: state + callbacks in, neverSyncWorkspace/SourceControlActionService/SourceControlViewModeldirectly).SyncWorkspaceboundary audit —git grepacrosssrc/ui/**andsrc/logic/source-control/**found the boundary already intact (no UI code touchesSyncWorkspace/coordinators directly; the onePushCoordinatorhit is a pre-existing type-only import). No code fix needed for the UI → sync direction.eslint.config.mts) — newno-restricted-importsrules:src/ui/**andsrc/logic/source-control/**may not import a concrete Git provider or a push/pull coordinator/executor directly;src/logic/sync/**may not importsrc/ui/source-control/**.SyncDiffService/SyncInteractionPort(sync domain) importedcomputeDiffStat/DiffStatLoadResultfromui/source-control. Moved the pure diff-stat computation and theDiffStatLoadResultcontract into a newsrc/logic/sync/DiffStat.ts; the UI now depends on the domain for these, not the reverse.SyncFileDiscovery.test.ts,SyncStatusResolver.test.ts,RenameReconciler.test.ts,createSyncRuntime.test.ts,DiffStat.test.ts(net-new coverage — the oldSyncStatusRefreshService.test.tsonly covered incremental handlers, not discovery/resolution/rename-reconciliation directly).CLAUDE.md's "Code Architecture" section replaced with a short contract pointing atdocs/architecture.md/docs/bug-fix-guidelines.md(also fixed stale GitLab/GitHub-only provider list → GitHub/GitLab/Gitea, and the single-filesync-manager.tsdescription).docs/architecture.md's module table and "Current hotspots" section updated to describe the merged code.Explicitly out of scope (per plan)
No
PushCoordinator/PullCoordinatorsplit, noSyncManagerrewrite, no provider/BaseGitServicecleanup, no settings restructure, noChangeItemrewrite, no CSS/UX changes, no sync semantics changes, no new features, no version bump, no DI framework, no command/event bus, no create/modify/delete/rename per-event handler classes.Verification
npx eslint .— 0 errors (verified after every commit, including the new boundary-guard rules actually firing on the real violation above before the fix).npx vitest run— 74 files / 933 tests passed (up from 73 files / 917 tests on1.6.1; net-new coverage, no regressions).npm run build(tsc -noEmit+ Obsidian 1.11.0 compat typecheck + esbuild production) — passed after every commit.git grep -n "SyncWorkspace" -- src/ui→ only doc-comment mentions.git grep -n "PushCoordinator\|PullCoordinator" -- src/ui src/logic/source-control→ only the pre-existing type-only import.DoD checklist
SyncStatusResolver)RenameReconciler)🤖 Generated with Claude Code