Files
ClaudeDo/docs/superpowers/specs/2026-06-19-feature-unification-design.md
Mika Kuns 0993eb0e75 docs(unification): spec + phased plan for one-component-per-feature
Maps duplication into three buckets (parallel impls, entry-point sprawl, dead/leftover) and defines six phased unification slices: groundwork, DialogService, MergeCoordinator, WorktreeActions, AgentConfigeditor, unified DiffViewer.
2026-06-26 16:11:47 +02:00

7.9 KiB
Raw Blame History

Feature unification — one component per feature

Date: 2026-06-19

Goal

ClaudeDo grew organically; several features now exist as parallel implementations or are reachable through many hand-wired entry points. This design maps the duplication and defines a target where each feature is one component, reached through one path, with dead code removed.

Method

Mapped via five parallel exploration agents (merge/conflict, review→merge, diff+worktree, task create/edit, UI entry-point inventory), then verified the load-bearing claims by grep/read before writing this. Every file:line below was confirmed against the working tree on 2026-06-19.

Key finding: it is NOT three merge engines

There is one merge engine (TaskMergeService), wrapped once for multi-child units (PlanningMergeOrchestrator), with one conflict resolver (the Rider 3-pane). Worker/CLAUDE.md already records "there is no separate 'Merge all' entry — approve is the single review+merge action." What looks like 23 merge features is entry-point sprawl in the UI plus one dead hunks-API left over from the Layer-C rework. So unification is mostly UI plumbing + deletion, not re-architecting the engine.

Findings — three buckets

Bucket A — genuine duplication (parallel implementations of one job)

# Feature Duplicated components Shared already
A1 Diff viewing DiffModalViewModel (worktree + commit-range), WorktreeModalViewModel (file-tree + per-file), PlanningDiffViewModel (per-subtask + integration) UnifiedDiffParser, DiffLinesView (good)
A2 Agent-config editing ListSettingsModalViewModel (list scope), AgentSettingsSectionViewModel (task scope); global lives in SettingsModalViewModel InheritanceResolver, InheritedBadge (good)
A3 Worktree actions WorktreesOverviewModalViewModel per-row cmds (Merge/Discard/Keep/ForceRemove/ShowDiff/Jump) vs MergeSectionViewModel (Merge/OpenDiff) same IWorkerClient calls
A4 Merge display AgentStripView re-displays MergeSectionViewModel state

Bucket B — entry-point sprawl (one backend, many hand-wired doors)

# Feature Doors Evidence
B1 Conflict-resolution seam 5 copies of Func<string,string,Task>? RequestConflictResolution WorktreesOverviewModalViewModel.cs:83, DiffModalViewModel.cs:75, MergeModalViewModel.cs:33, MergeSectionViewModel.cs:51, DetailsIslandViewModel.cs:347 (delegates). Threaded through MainWindow.axaml.cs:81, IslandsShellViewModel.cs:49/202, DiffModalViewModel.cs:103, MergeSectionViewModel.cs:159
B2 Diff (open) 34 MergeSection "Open Diff", TaskHeaderBar "Review Merged Diff", WorktreesOverview "Show Diff", Planning "Review Combined"
B3 List Settings dialog 3 Lists context menu, Tasks header button, shell bridge IslandsShellViewModel.cs:190-194
B4 Worktrees Overview 23 Repos menu (global), Lists context menu (per-list)
B5 Repo Import 2 Repos menu, Lists footer button

The conflict-resolution target is already single-point (IslandsShellViewModel.RequestConflictResolutionAsync, line 49). What is duplicated is the seam plumbing: five VMs each own the Func and it is threaded by hand.

Bucket C — dead / leftover

# Item Evidence
C1 Dead hunks conflict API TaskMergeService.GetConflictsAsync (Lifecycle/TaskMergeService.cs:250) ← WorkerHub.GetMergeConflicts (Hub/WorkerHub.cs:378) ← WorkerClient "GetMergeConflicts" (Services/WorkerClient.cs:276) ← IWorkerClient. Live resolver uses GetMergeConflictDocuments (WorkerHub.cs:389). Only TaskMergeServiceTests.cs:672 still references the old one.
C2 Two task-creation paths UI quick-add TasksIslandViewModel.AddAsync writes EF directly (db.Tasks.Add); MCP ExternalMcpService.AddTask is the service path. They can drift.
C3 Stale worktrees .claude/worktrees/feat+planning-sessions-ui/… carries old copies of DiffModalViewModel/ListSettingsModalViewModel/WorktreeModalViewModel; layer-c resolver leftovers. Worktree hygiene, not main code.
C4 Naming drift (deferred) Hub StartConflictMerge/ContinueConflictMerge/AbortConflictMerge (WorkerHub.cs:367/405/414) vs service MergeAsync/ContinueMergeAsync/AbortMergeAsync. Documented as intentional at Worker/CLAUDE.md:153.

Targets — one component per feature

  1. MergeCoordinator (B1). Replace the five RequestConflictResolution Func seams with one injected coordinator exposing MergeAsync(taskId, targetBranch) that owns the "merge → on-conflict open resolver" sequence. Every door (review Approve, Diff Merge button, WorktreesOverview single + batch, Details merge section) calls it. The single resolution point (IslandsShellViewModel.RequestConflictResolutionAsync) becomes the coordinator's body.
  2. DiffViewer (A1 + B2). One DiffViewerViewModel + view with a DiffSource abstraction (DirtyWorktree | BranchVsBase | CommitRange | PlanningAggregate | IntegrationBranch) and an optional file-tree pane. Replaces DiffModal + WorktreeModal + PlanningDiff shells; keeps UnifiedDiffParser/DiffLinesView. All B2 doors open it with a different source.
  3. WorktreeActions (A3). One WorktreeActionsViewModel for a single task's worktree (merge/diff/discard/keep/force-remove), reused by both the overview rows and the Details merge section instead of each owning copies.
  4. AgentConfigEditor (A2). One editor component parameterized by scope (Global | List | Task) over InheritanceResolver, embedded in Settings, List Settings, and the Details panel. Collapses the duplicated property set + reset commands + badges.
  5. DialogService (B3B5). Consolidate the per-modal Show* Func seams (IslandsShellViewModel.cs:59-71) into one IDialogService with typed open methods (OpenListSettings(list), OpenRepoImport(), OpenWorktreesOverview(listId?)…). Menu, context menu, and footer all call the same method; duplicate command definitions across ListsIsland/shell collapse to one.
  6. Single task-creation path (C2). Route UI quick-add through the same creation path MCP AddTask uses (repository/service), so both honor the same invariants.

Plus C1 (delete dead hunks API + its test) and C3 (prune stale worktrees) as groundwork. C4 naming alignment is deferred — it is documented-intentional and would churn the hub + WorkerClient + every IWorkerClient fake (see the "fakes to sync" hazard) for cosmetic gain.

Decisions

  • Phased, each phase ships green. Six independently buildable/committable slices; cheapest and lowest-risk first (see the plan). No big-bang.
  • One plan file per slice. Matching the 2026-06-05 layer-A/B/C convention, each slice gets its own docs/superpowers/plans/2026-06-19-unify-<slice>.md authored when it is picked up. This umbrella plan sequences them and details Phase 01.
  • DiffViewer (A1) is last. Highest effort and most UX-sensitive (file-tree vs whole-unified are different layouts); deferring it lets the cheaper wins land first and de-risks the big one.
  • Keep the merge engine and the resolver seam contract. TaskMergeService, PlanningMergeOrchestrator, ConflictResolverViewModel ctor/OpenAsync/OpenForPlanningAsync/CloseRequested are unchanged — unification is above them.
  • Naming alignment deferred, not done (rationale above).

Out of scope / deferred

  • Hub/service merge-method renaming (C4).
  • Subtask deletion in the UI (a missing feature surfaced during mapping, not a duplicate).
  • Any DB migration, worker engine change, or push.

Acceptance (per phase)

Each phase: dotnet build -c Release clean for touched projects; the relevant test project green; locales in parity (Localization.Tests) where keys change; the feature reachable through its single new path with the old doors removed or delegating. UI phases (25) flag a visual-verification gap for Mika to confirm in the running app.