You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This illustrates how to use Claude Code for development, allowing it to generate "AI slop" but then periodically using it to rearchitect and clean up the slop.
I directed Claude Code to generate both of these two (2) documents. The file rearchitecture_plan.md was created a while back when it was developing code with bugs that it could not fix so I asked it to review, critique, and identify areas for architectural improvement.
The file fix_plan.md was created after we worked on the project significantly more until it became unmaintainable again, so I pointed it at the refactor_plan.md and asked it to review the code again, and fix_plan.md was born.
And frankly, it got everything pretty much exactly right.
Plan: Unified Module Scope — Single Source of Truth for File/Module Filtering
Context
The codebase has accumulated filtering logic in 8+ locations using 3 different discovery mechanisms (graph-based, filesystem walk, PlanFilesets iteration). Each implements the same core pattern — "include files matching a module prefix, exclude nested submodule files" — differently, causing inconsistencies where fixing one breaks another.
What Prompted This
After implementing the MODSTATE/TREEFILT/FILESCOPE/STATUSFIX/ENGUPD steps (which fixed the specific go-dt tree visibility bug), the user asked for a broader architectural review. The codebase has "become overwhelmed with lots of conflicting special cases." The goal: define filtering rules in one place in gompkg so UIs always work correctly.
Graph-based (getSubmodulePathsToExclude): Uses ModuleGraph.ModulesMapByModulePathByRepoDir — only sees modules the graph discovered
Filesystem walk (FindSubmoduleDirs): Uses filepath.WalkDir looking for go.mod — sees everything on disk
PlanFilesets iteration (FileScopeFilter): Uses PlanFileset.RelPath — only sees modules with filesets in the commit plan
These WILL disagree when: a nested module has no dirty files (graph sees it, PlanFilesets doesn't), or a go.mod exists but isn't in the graph (filesystem sees it, graph doesn't).
Duplicate Code
file_intent_model.go:977 has filterFilesByCommitGroup() operating on []dt.RelFilepath — same algorithm as gompkg/file_scope_filter.go:filterByCommitGroup() operating on []*teatree.File. Both build companion sets, create filters, build nested module exclusions, and apply the same include/exclude logic.
Package Boundary Violations
file_intent_model.go imports gitutils (line 22) and gomcfg (line 23). The gitutils usage is in the duplicate filterFilesByCommitGroup() method (calls gitutils.CreateDirPathFilter) and NewFileStatus() (line 612). 18 total gomtui files import gitutils — the broader cleanup is out of scope for this plan but the duplicate filter removal eliminates one.
Approach: ModuleScope as the Single Filtering Primitive
Core Insight
Every filtering operation answers the same question: "Does this file path belong to module M?" The answer is always: "path starts with M's prefix AND path does NOT start with any nested submodule's prefix." We create ONE type that answers this question, and everything else uses it.
Steps
SCOPE — Create ModuleScope type in gompkg
New file: gommod/gompkg/module_scope.go
// ModuleScope defines which file paths belong to a single Go module,// accounting for nested submodule exclusions. This is the single source// of truth for "does path P belong to module M?"typeModuleScopestruct {
ModulePath dt.RelDirPath// Module dir relative to repo root ("" for root)ExcludePaths []dt.PathSegments// Nested submodule dirs to exclude
}
// ContainsFile returns true if the given file path belongs to this module.func (s*ModuleScope) ContainsFile(fp dt.RelFilepath) bool// FilterFiles returns only files belonging to this module scope.func (s*ModuleScope) FilterFiles(files []dt.RelFilepath) []dt.RelFilepath// FilterTreeFiles returns only teatree.File entries belonging to this module scope.func (s*ModuleScope) FilterTreeFiles(files []*teatree.File) []*teatree.File
ContainsFile implements the core pattern once:
If ModulePath != "": check fp.HasPrefix(ModulePath + "/"); also match fp that equals a file directly in ModulePath
For each ExcludePath: if fp.HasPrefix(ExcludePath + "/") → reject
If ModulePath == "" (root module): accept all paths not excluded
FilterFiles and FilterTreeFiles are convenience wrappers calling ContainsFile per item.
SCOPESET — Create CommitGroupScope composite type
Same file: gommod/gompkg/module_scope.go
// CommitGroupScope combines multiple ModuleScopes for a commit group// (primary module + companion modules). A file belongs to the group// if it belongs to ANY member scope.typeCommitGroupScopestruct {
Members []*ModuleScope
}
// ContainsFile returns true if the file belongs to any member scope.func (g*CommitGroupScope) ContainsFile(fp dt.RelFilepath) bool// FilterFiles returns files belonging to any member scope.func (g*CommitGroupScope) FilterFiles(files []dt.RelFilepath) []dt.RelFilepath// FilterTreeFiles returns teatree.File entries belonging to any member scope.func (g*CommitGroupScope) FilterTreeFiles(files []*teatree.File) []*teatree.File
DISCOVER — Unify submodule discovery into ComputeModuleScope()
Same file: gommod/gompkg/module_scope.go
// ComputeModuleScope creates a ModuleScope for a module, discovering// nested submodules from the available data source.// Preferred: graph-based when ModuleGraph is available.// Fallback: filesystem walk when only a directory path is available.funcComputeModuleScope(modulePath dt.RelDirPath, sourceScopeSource) *ModuleScope// ScopeSource abstracts the two discovery mechanisms.typeScopeSourceinterface {
NestedModulePaths(modulePath dt.RelDirPath) []dt.PathSegments
}
// GraphScopeSource discovers nested modules from a ModuleGraph.typeGraphScopeSourcestruct { ... }
// FSScopeSource discovers nested modules by walking the filesystem.typeFSScopeSourcestruct { ... }
This eliminates the disagreement problem: callers pick the source appropriate to their context, but the filtering logic is always the same.
MIGRATE — Update all consumers to use ModuleScope
Modify: gommod/gompkg/file_scope_filter.go
FileScopeFilter takes []*ModuleScope or *CommitGroupScope instead of raw paths + PlanFilesets
Delete filterByCommitGroup() and filterByModuleScope() — replaced by CommitGroupScope.FilterTreeFiles() and ModuleScope.FilterTreeFiles()
Constructor NewFileScopeFilter() builds the scope from CommitGroupPaths + PlanFilesets (or graph)
Modify: gommod/gompkg/module_ext.go
ComputeModuleState() uses ComputeModuleScope() with GraphScopeSource instead of calling getSubmodulePathsToExclude() directly
This is a document intended to allow us to keep track of the aspects of our architecture that need to become "reactive" so that we don't constantly end up with bugs where things are not updated when they should be and/or that things are inconsistent in so many cases. (I may be misusing the common meaning for "reactive" and if I am help me come up with a better name.)
Other ways to look at it are:
We need to be able to make one code change to affect one requirement change, as much as reasonably possible.
Also, we need our architecture to be D.R.Y.
Issue Category: Parallel Data Structures Without Synchronization
Problem
We maintain the same logical data in multiple places with no automatic synchronization between them. When one copy changes, all others must be manually updated — and we routinely forget some.
Concrete Examples from This Session
1. CommitMessageEditor.groupStates vs. CommitBatch.Groups
The editor caches per-group state (cursor position, unsaved edits, selected type) in CommitMessageEditor.groupStates keyed by group ID. The canonical group list lives in CommitBatch.Groups. When removeCommittedGroup() deleted a group from the batch, nobody cleaned the editor cache. If a group ID was reused, the new group inherited stale cursor/message state from the deleted group.
2. BlockAssignment.GroupID / ChangeBlockInfo.LineGroupIDs vs. CommitBatch.Groups
Block-level group assignments exist in two places:
Persisted: BlockAssignment.GroupID inside FileAssignment.Blocks (stored per-group in CommitGroup.Assignments)
Runtime: ChangeBlockInfo.LineGroupIDs inside FileBlockInfo.Blocks (stored in BatchAssignmentModel.fileBlocks)
When a group was removed, neither location was cleaned. The gutter continued showing "→ 3" for a deleted group because LineGroupIDs still referenced it, and BlockAssignment entries still pointed to the deleted group ID.
3. BatchAssignmentModel.fileBlocks vs. committed file state
fileBlocks maps file paths to their parsed diff/block info. After a commit, the committed files have no diff anymore, but fileBlocks still contained their entries. Other groups that referenced those files (via Assignments) still appeared in the tree as if they had changes.
Pattern to Fix
These are all symptoms of the same root cause: derived/cached state that doesn't automatically invalidate when its source changes. Options:
Single source of truth + computed views: Instead of caching group states by ID in the editor, derive them on-demand from the canonical batch data. Slower but always correct.
Change propagation / observer: When CommitBatch.Groups changes, fire a cleanup hook that all dependent caches subscribe to. More complex but preserves performance.
Transactional mutation: All group mutations go through a single method (e.g., CommitBatch.RemoveGroup(id)) that knows about all the places that need cleanup. This is what we did as a tactical fix — but it's fragile because every new cache site must be added to the cleanup method.
Anti-Pattern: Dead Reconciliation Code
A recurring variant of this problem: reconciliation methods are written anticipating the need (e.g., CommitBatch.AddMissingFiles() existed for exactly the reset-after-commit case) but are never wired into any code path. The method sits unused, the bug ships, and the fix is just "call the method that already exists." This suggests the real problem isn't missing logic — it's missing orchestration. A reactive/observer architecture would eliminate this class of bug because reconciliation would be triggered automatically by data changes rather than requiring explicit calls at every mutation site.
Issue Category: Whole-File Staging vs. Block-Level Assignment
Problem
The UI allows assigning individual change blocks to different groups, giving the user the impression that only those blocks will be committed. But executeCommit() uses git add <filepath>, which stages the entire file. When a file has blocks split across groups, committing one group commits ALL changes to that file. After commit, the file has no diff, but it still appears in other groups as a ghost.
Pattern to Fix
This is a fundamental mismatch between what the UI promises and what git does. Options:
Warn the user (what we did as a tactical fix): Show a warning in the commit confirmation dialog when divergent files exist.
Use git add -p or patch-based staging: Stage only the hunks belonging to the committed group. This is complex because our "change blocks" don't map 1:1 to git hunks.
Prevent divergent assignments: Don't allow a file's blocks to be split across groups — if any block is assigned, the whole file goes to that group. Simpler but less flexible.
Issue Category: Hardcoded Constants Scattered Across Files
Problem
Values like color numbers, layout dimensions, and string literals are hardcoded at every usage site rather than defined once and referenced. When we want to change a color theme or adjust a layout constant, we have to find and update every occurrence — and missing one creates visual inconsistency.
Examples
CommitGroupColor() returns color strings used in the tree, gutter, and editor — but the editor also hardcodes its own color constants ("244", "255", "250", "240", "220") for label/title/body/placeholder/generating styles.
CyanColor, RedColor are shared constants but many other colors are inline literals.
Pattern to Fix
Extract a theme/palette struct that defines all UI colors in one place.
Components receive the palette at construction time rather than hardcoding color values.
Issue Category: God Object — BatchAssignmentModel
Problem
batch_assignment_model.go is enormous (4000+ lines, exceeds the 25K token read limit). It owns too many responsibilities: tree management, split pane, message editing, commit flow state machine, recommendation flow, block assignment, clipboard operations, batch selection, and more. Every new feature adds more state fields and more message handling to this single type.
Why This Matters for Reactivity
The more state a single object manages, the harder it is to keep all of it consistent. removeCommittedGroup() needed a 10-step cleanup because the model holds so many parallel caches. A smaller, focused component would have fewer things to invalidate.
Pattern to Fix
Extract sub-models that own their own state and expose a narrow interface.
The parent model delegates messages to sub-models and coordinates between them.
This is already partially done (CommitMessageEditor, SplitDiffPaneModel, etc.) but the batch assignment model still directly manages too much of their state.
Issue Category: Stale Viewer Cache on View Re-Entry
Problem
When navigating between views (e.g., FileIntentUI → CommitReviewUI → back → CommitReviewUI again), the second entry reuses a cached viewer that doesn't know the world changed while it was offscreen.
Concrete Example: git reset after commit
User commits util.go via CommitReviewUI
removeCommittedGroup() removes util.go from the batch and fileBlocks
User presses [esc] → back to FileIntentUI
User runs git reset externally → util.go is modified again in the working tree
FileIntentUI correctly shows util.go (it rescans on focus)
User presses [r] to go to CommitReviewUI
handleDrillDown() at app_model.go:569 finds the cached viewer → reuses it as-is
The only thing updated on the cached viewer is the verification flag (app_model.go:575-579)
util.go is missing from CommitReviewUI even though it's a modified file
Two Cache Layers, Both Stale
Layer 1 — AppModel.viewerCache (app_model.go:49,569):
Maps "UISelector:repoRoot::modulePath" → the entire UIViewer (BatchAssignmentModel). On cache hit, the viewer is reused without any refresh of its data. Only the verification flag is updated.
Layer 2 — commitPlan.CommitReview (file_intent_model.go:1211):
Even on a cache miss (first creation), HandleDrillDown checks m.commitPlan.CommitReview first. If non-nil (set on the first visit at line 1238), it's used directly — skipping both the config file reload and PruneStaleFiles(). This means even without the viewer cache, a stale in-memory CommitReview is used.
Layer 3 — PruneStaleFiles is one-directional:
Even when PruneStaleFiles runs (only on first load from config), it only removes files that disappeared from the working tree. It never adds files that reappeared (e.g., after git reset). So a file that was committed and then un-committed via git reset is never re-added to the batch.
Tactical fix applied (2026-02-22): Added AddMissingFiles() calls after every PruneStaleFiles() call — in ParseCommitPlan() (centralized loading), and both branches of HandleDrillDown() (runtime reconciliation). This required a new CommitReview.AddMissingFiles() method that delegates to the active batch. The irony: CommitBatch.AddMissingFiles() already existed (written for exactly this case) but was never called from any code path. This is a recurring anti-pattern: reconciliation logic that is written but never wired — the code anticipated the need but nobody connected it. The fix touched 3 files and 3 call sites, demonstrating the "transactional mutation" fragility described above: every new loading path must remember to call both PruneStaleFiles() AND AddMissingFiles(), and forgetting either one produces a silent data inconsistency.
Known follow-on issue: This fix triggers the "New/Reappearing Files Placed in Wrong Group" issue (below) — AddMissingFiles() places reappearing files into the first group's Assignments rather than leaving them unassigned. The user sees the file under a group but with no block-level indicators, which is confusing. The long-term fix (Option C below) would eliminate both problems.
Pattern to Fix
This is fundamentally about re-entry semantics: what should happen when a view becomes active again?
Option A — Invalidate viewer cache on navigation:
When drilling back up (esc), delete the viewer cache entry. Next drill-down creates a fresh viewer. Simple but loses all UI state (expanded nodes, cursor position, unsaved edits).
Option B — Refresh-on-entry hook:
Add a RefreshOnEntry() method to UIViewer. When handleDrillDown reuses a cached viewer, call viewer.RefreshOnEntry() which re-scans the working tree and reconciles:
Add new files that appeared since last visit
Remove files that disappeared
Preserve existing group assignments, messages, and UI state for files that haven't changed
This is the preferred approach — it preserves UX while ensuring data freshness.
Option C — Reactive file list:
CommitReviewUI doesn't own its file list. Instead, it derives it from commitPlan.Files (which FileIntentUI keeps current). On every render or focus, it diffs its current assignments against the canonical file list and reconciles. Most architecturally sound but largest refactor.
Related: commitPlan.CommitReview Bypass
The if review == nil check at file_intent_model.go:1214 means the full loading/pruning path only runs once. After first visit, the runtime CommitReview object is cached in commitPlan.CommitReview and never refreshed. Any external changes (git reset, manual git operations, file edits) are invisible to CommitReviewUI.
This is the same class of problem as "Parallel Data Structures Without Synchronization" above, but at the view/navigation level rather than within a single view.
Issue Category: New/Untracked Files Invisible in Diff Pipeline
Problem
New files (untracked or freshly staged) have no diff output from git diff HEAD. The diff pipeline treats "no diff output" as "no changes" and produces zero rows, resulting in "No diff to display". Consequently, LoadFileBlockInfo creates a FileBlockInfo with zero blocks, making block assignment impossible — the file can't be assigned to any commit group, and Reorg can't move it.
Concrete Examples
1. Untracked file shows "No diff to display"
temp.go is a new untracked file with content (package dt). Selecting it in the tree shows "No diff to display" because git diff HEAD -- temp.go returns empty output for untracked files. GetDiff() (gitutils/diff.go:112-113) jumps to end: when diffOutput == "", creating a blank FileDiff with no OldLines/NewLines even though actualLines was already computed from disk.
2. Can't assign new file to commit group
Even when a new file IS staged (new file: temp.go) and its diff content appears in the split pane, the file may have zero ChangeBlocks (if the diff pipeline didn't parse hunks correctly). With zero blocks, assignBlockToGroup() and tree-level assignment both skip the file because there are no blocks to set LineGroupIDs on. The "→ 6" gutter indicator never appears, and Reorg has no dominant group to compute.
Root Cause Chain
GetDiffRaw() runs git diff HEAD -- <path> — empty for untracked files
GetDiff() line 112: diffOutput == "" → goto end (skips lines 126-127 that set OldLines/NewLines)
end: label creates empty FileDiff via NewFileDiff(relPath) with no lines
extractChangeBlocks() gets empty lines, no hunks → zero ChangeBlocks
buildSplitPaneDiff(): maxLines = 0 → zero rows → "No diff to display"
LoadFileBlockInfo() iterates zero ChangeBlocks → FileBlockInfo.Blocks is empty
Assignment via number keys iterates empty Blocks → nothing assigned
Pattern to Fix
The diff pipeline must handle the "new file, no HEAD version" case. When diffOutput is empty but actualLines (read from disk) has content, synthesize a LinesAddedChangeBlock spanning all actual lines. This restores the full pipeline: rows are created, blocks exist for assignment, gutter indicators work, Reorg can compute dominant groups.
Issue Category: New/Reappearing Files Placed in Wrong Group
Problem
When files reappear in the working tree (e.g., after git reset) or are newly added to the commit plan, AddMissingFiles() places them into the first commit group's Assignments. The user expects new files to appear in the "Not Assigned" pseudo-group, requiring explicit assignment.
Confirmed active (2026-02-22): The Layer 3 tactical fix (above) now calls AddMissingFiles() in 3 places, making this issue reliably reproducible rather than theoretical. After git reset --soft HEAD^, the file reappears in group 1 with no block indicators.
Concrete Example
url.go is loaded into commit #1's tree but its lines aren't assigned to commit #1. It appears in the group but without the "→ 1" gutter indicators. The file should instead appear under "Not Assigned" so the user can explicitly choose where to place it.
Root Cause
AddMissingFiles() (on CommitBatch) adds files to the first group's Assignments list. The tree builder shows these files under that group. But the file has no LineGroupIDs set (unassigned at block level), creating a confusing split: the file appears under a group but none of its lines are actually assigned there.
Pattern to Fix
New/reappearing files should NOT be added to any group's Assignments. Instead, they should only exist in fileBlocks (loaded via LoadFileBlockInfo). The tree builder's getUnassignedFiles() already shows files in fileBlocks that aren't in any group under the "Not Assigned" pseudo-group. The fix is to pass the full commit-intent file list to BatchAssignmentModel for diff loading, without adding them to groups.
Issue Category: Empty Files Show "No diff to display"
Problem
Files with zero content (e.g., EXAMPLE.md that is empty) show "No diff to display" — the same message shown when no file is selected. This is confusing because the user selected a file and expects to see its (empty) content.
Root Cause
SplitDiffPaneModel.View() (split_diff_pane_model.go:558-559) checks len(m.rows) == 0 and returns "No diff to display". Empty files legitimately produce zero rows (no CommitLines, no ActualLines). The view can't distinguish "no content loaded" from "empty file content loaded".
Pattern to Fix
Track whether content has been explicitly loaded (via SetContent). If content was set but produced zero rows, show "(empty file)" instead of "No diff to display".
Issue Category: Bogus "Entire Files Will Be Committed" Warning
Problem
The commit confirmation dialog shows: "WARNING: N files have blocks assigned to other groups. Entire files will be committed (git stages whole files)." This warning was never requested and directly contradicts the design intent of the UI, which is to allow individual block-level commits (not just whole-file commits).
Concrete Example
When committing a group that has files with blocks assigned to other groups, the warning appears implying that the entire file will be committed regardless of block assignments. This is the opposite of the intended behavior.
Root Cause
buildCommitConfirmMessage() (batch_assignment_model.go:4035-4046) checks divergentFileCount > 0 and appends a warning about whole-file staging. The countDivergentFilesInGroup() method counts files with blocks assigned to groups other than the one being committed.
Pattern to Fix
Remove the warning entirely. The commit flow should respect block-level assignments (staging only the changes belonging to the committed group). If whole-file staging is a temporary implementation limitation, it should be fixed in the commit execution path — not presented to the user as an accepted behavior.