feat: enhance drift visualization with content drift and orphan detection (go4dot-ov1.5)#124
Conversation
…tion (go4dot-ov1.5) Add two new drift detection capabilities: - ContentDriftFiles: detect conflict files where dest content differs from source - OrphanFiles: detect files in dest managed dirs not tracked by source Enhanced visualization across CLI and dashboard: - Status output shows content drift (≠) and untracked (?) counts - Details panel shows DRIFT STATUS summary section - File tree shows inline drift indicators (≠ for content drift, ? for orphans) - Conflict files annotated with [content differs] when content has drifted Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe PR extends the drift detection system to track content differences and orphaned files. New fields are added to ConfigStatus to record content drift and orphan counts. The drift engine now compares file contents and identifies untracked files in managed directories. Status rendering and UI dashboards are updated to display these new drift indicators. Changes
Sequence DiagramsequenceDiagram
participant DriftEngine as Drift Engine
participant ContentCheck as Content Comparison
participant OrphanDetect as Orphan Detector
participant StatusGather as Status Gatherer
participant StatusRender as Status Renderer
participant UIDisplay as UI Dashboard
DriftEngine->>ContentCheck: hasContentDrift(source, dest)
ContentCheck->>ContentCheck: Compare file contents
ContentCheck-->>DriftEngine: Return drift status
DriftEngine->>OrphanDetect: findOrphanFiles(configPath, home)
OrphanDetect->>OrphanDetect: Walk config & home dirs
OrphanDetect-->>DriftEngine: Return orphan files list
DriftEngine-->>DriftEngine: Populate ContentDriftFiles & OrphanFiles
StatusGather->>DriftEngine: Get DriftResult
DriftEngine-->>StatusGather: Return with drift & orphan data
StatusGather->>StatusGather: Set ContentDrift & Orphans fields
StatusGather-->>StatusRender: Provide ConfigStatus
StatusRender->>StatusRender: Format drift indicators
StatusRender-->>UIDisplay: Return rendered drift details
UIDisplay->>UIDisplay: Render tree with orphan/drift marks
UIDisplay-->>UIDisplay: Display DRIFT STATUS block
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR extends the drift detection pipeline to report two new categories of filesystem divergence — content drift (conflict files whose byte content differs from the dotfiles source) and orphan files (untracked files in home directories managed by a config) — and surfaces both in the TUI details panel and the status text renderer. The feature fits naturally into the existing
Confidence Score: 3/5
|
| Filename | Overview |
|---|---|
| internal/stow/drift.go | Adds ContentDriftFiles and OrphanFiles to DriftResult/DriftSummary, implements hasContentDrift comparison; silently returns false for >10MB same-size files — a potential false negative. |
| internal/stow/symlink.go | Adds findOrphanFiles; symlinks-to-directories are not excluded before orphan classification, leading to possible false orphan reports for directory symlinks placed by other tools. |
| internal/status/gather.go | Adds ContentDrift and Orphans fields to ConfigStatus; orphans correctly set independent of HasDrift; logic is consistent with drift state machine. |
| internal/status/render.go | Extends driftDetails with content drift and orphan rendering; only called for drifted configs, so orphan-only synced configs are not surfaced here — minor inconsistency with the details panel behavior. |
| internal/ui/dashboard/details_panel.go | Adds DRIFT STATUS section, orphan tree augmentation, and content-drift markers; a synced config with orphans shows a "DRIFT STATUS" heading inconsistently with the list view's synced indicator. |
Sequence Diagram
sequenceDiagram
participant G as Gatherer
participant DC as DriftChecker
participant FDC as FullDriftCheckWithHome
participant HCD as hasContentDrift
participant FOF as findOrphanFiles
G->>DC: FullDriftCheck(cfg, dotfilesPath)
DC->>FDC: FullDriftCheckWithHome(cfg, path, home, state)
loop For each config
FDC->>FDC: Walk configPath files
alt target missing
FDC->>FDC: append NewFiles
else target is regular file (conflict)
FDC->>FDC: append ConflictFiles
FDC->>HCD: hasContentDrift(source, dest)
HCD-->>FDC: true/false (skips >10MB same-size)
alt content differs
FDC->>FDC: append ContentDriftFiles
end
else target is symlink to wrong location
FDC->>FDC: append ConflictFiles
FDC->>HCD: hasContentDrift(source, dest)
HCD-->>FDC: true/false
alt content differs
FDC->>FDC: append ContentDriftFiles
end
end
FDC->>FOF: findOrphanFiles(configPath, home)
FOF-->>FDC: OrphanFiles (untracked in managed dirs)
FDC->>FDC: set HasDrift (NewFiles OR ConflictFiles OR MissingFiles)
end
FDC-->>DC: DriftSummary
DC-->>G: DriftSummary
G->>G: Build ConfigStatus per config
note over G: ContentDrift set only when HasDrift=true\nOrphans set regardless of HasDrift
G-->>G: Overview with Configs[]
Last reviewed commit: 39c1924
| // Skip very large files (>10MB) | ||
| if sourceInfo.Size() > 10*1024*1024 { | ||
| return false | ||
| } |
There was a problem hiding this comment.
Silent false negative for large same-size files
When two files both exceed 10 MB and happen to have identical sizes, this block returns false (i.e. "no drift"), even if their contents actually differ. A config file that was edited so that the byte count was preserved — characters replaced without adding or removing bytes — would be silently missed. For a dotfiles manager, that scenario is common (e.g. editing a value in-place in a config).
Because the size equality is already checked on line 233, this skip only applies to the narrow case of equal-size files above 10 MB. It would be safer to return a distinct "unknown" sentinel or at least add a comment that explicitly warns callers that content drift may go undetected for large same-size files.
| // Skip very large files (>10MB) | |
| if sourceInfo.Size() > 10*1024*1024 { | |
| return false | |
| } | |
| // Skip very large files (>10MB): content comparison is skipped for performance. | |
| // NOTE: files with the same size but different content will NOT be detected as | |
| // having content drift. Callers should be aware of this false-negative. | |
| if sourceInfo.Size() > 10*1024*1024 { | |
| return false | |
| } |
| if driftResult != nil && (driftResult.HasDrift || len(driftResult.OrphanFiles) > 0) { | ||
| lines = append(lines, headerStyle.Render("DRIFT STATUS")) | ||
| var driftParts []string | ||
| if len(driftResult.NewFiles) > 0 { | ||
| driftParts = append(driftParts, okStyle.Render(fmt.Sprintf("+%d new", len(driftResult.NewFiles)))) | ||
| } | ||
| if len(driftResult.MissingFiles) > 0 { | ||
| driftParts = append(driftParts, errStyle.Render(fmt.Sprintf("-%d missing", len(driftResult.MissingFiles)))) | ||
| } | ||
| if len(driftResult.ConflictFiles) > 0 { | ||
| conflictText := fmt.Sprintf("!%d conflicts", len(driftResult.ConflictFiles)) | ||
| if len(driftResult.ContentDriftFiles) > 0 { | ||
| conflictText += fmt.Sprintf(" (%d content differs)", len(driftResult.ContentDriftFiles)) | ||
| } | ||
| driftParts = append(driftParts, warnStyle.Render(conflictText)) | ||
| } | ||
| if len(driftResult.OrphanFiles) > 0 { | ||
| driftParts = append(driftParts, subtleStyle.Render(fmt.Sprintf("?%d untracked", len(driftResult.OrphanFiles)))) | ||
| } | ||
| lines = append(lines, " "+strings.Join(driftParts, ", ")) | ||
| lines = append(lines, "") | ||
| } |
There was a problem hiding this comment.
"DRIFT STATUS" section shown for configs reported as synced
A config that has orphan files but no actual drift (NewFiles, MissingFiles, and ConflictFiles are all empty) will have driftResult.HasDrift == false. In gather.go this causes its ConfigStatus.Status to be set to SyncStatusSynced, so the config appears as fully synced in the list view.
However, because this panel checks len(driftResult.OrphanFiles) > 0 independently, a "DRIFT STATUS" heading is rendered in the detail view while the list already shows a green check mark. A user navigating to a config that appears synced would unexpectedly find a drift warning in the details pane.
Consider either:
- Surfacing orphan-only configs with a distinct status (e.g. a
SyncStatusOrphanedor an informational icon), or - Renaming/reframing the section (e.g. "FILESYSTEM INFO") so it doesn't imply drift on a synced config.
| if driftResult != nil && (driftResult.HasDrift || len(driftResult.OrphanFiles) > 0) { | |
| lines = append(lines, headerStyle.Render("DRIFT STATUS")) | |
| var driftParts []string | |
| if len(driftResult.NewFiles) > 0 { | |
| driftParts = append(driftParts, okStyle.Render(fmt.Sprintf("+%d new", len(driftResult.NewFiles)))) | |
| } | |
| if len(driftResult.MissingFiles) > 0 { | |
| driftParts = append(driftParts, errStyle.Render(fmt.Sprintf("-%d missing", len(driftResult.MissingFiles)))) | |
| } | |
| if len(driftResult.ConflictFiles) > 0 { | |
| conflictText := fmt.Sprintf("!%d conflicts", len(driftResult.ConflictFiles)) | |
| if len(driftResult.ContentDriftFiles) > 0 { | |
| conflictText += fmt.Sprintf(" (%d content differs)", len(driftResult.ContentDriftFiles)) | |
| } | |
| driftParts = append(driftParts, warnStyle.Render(conflictText)) | |
| } | |
| if len(driftResult.OrphanFiles) > 0 { | |
| driftParts = append(driftParts, subtleStyle.Render(fmt.Sprintf("?%d untracked", len(driftResult.OrphanFiles)))) | |
| } | |
| lines = append(lines, " "+strings.Join(driftParts, ", ")) | |
| lines = append(lines, "") | |
| } | |
| // Show drift status summary (orphans are informational and shown even on synced configs) | |
| if driftResult != nil && (driftResult.HasDrift || len(driftResult.OrphanFiles) > 0) { | |
| if driftResult.HasDrift { | |
| lines = append(lines, headerStyle.Render("DRIFT STATUS")) | |
| } else { | |
| lines = append(lines, headerStyle.Render("FILESYSTEM INFO")) | |
| } |
| for _, entry := range entries { | ||
| if entry.IsDir() { | ||
| continue |
There was a problem hiding this comment.
Symlinks-to-directories treated as orphan files
entry.IsDir() returns false for symlinks whose target is a directory (because the type bit is ModeSymlink, not ModeDir). Such an entry will fall through the entry.IsDir() guard, reach the symlink-check block, and — if the symlink does not point back into the config path — be appended to orphans as a file-orphan even though it is actually a directory symlink.
This can produce spurious orphan reports for directories that are symlinked into a managed folder by another tool (e.g. a plugin manager placing a symlinked directory beside managed dotfiles).
| for _, entry := range entries { | |
| if entry.IsDir() { | |
| continue | |
| for _, entry := range entries { | |
| // Skip regular directories and symlinks-to-directories | |
| if entry.IsDir() { | |
| continue | |
| } | |
| entryPath := filepath.Join(targetDir, entry.Name()) | |
| if entry.Type()&os.ModeSymlink != 0 { | |
| if fi, err := os.Stat(entryPath); err == nil && fi.IsDir() { | |
| continue | |
| } | |
| } |
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (39.43%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #124 +/- ##
==========================================
- Coverage 49.37% 49.25% -0.12%
==========================================
Files 110 110
Lines 12977 13116 +139
==========================================
+ Hits 6407 6460 +53
- Misses 6570 6656 +86
🚀 New features to boost your workflow:
|
Summary
Merge queue processing by Refinery.
Summary by CodeRabbit
Release Notes