Skip to content

feat(desktop): git decoration in v2 file tree#3320

Merged
saddlepaddle merged 8 commits intomainfrom
saddlepaddle/unexpected-slipper
Apr 10, 2026
Merged

feat(desktop): git decoration in v2 file tree#3320
saddlepaddle merged 8 commits intomainfrom
saddlepaddle/unexpected-slipper

Conversation

@saddlepaddle
Copy link
Copy Markdown
Collaborator

@saddlepaddle saddlepaddle commented Apr 10, 2026

Summary

  • Adds VS Code-style git decoration to the v2 workspace "All files" tree: colored filenames, single-letter status badges (M/A/U/D/R/C), and roll-up dots on ancestor folders
  • Gitignored paths (node_modules, dist, .turbo, etc.) render muted via text-muted-foreground
  • Backend: git.getStatus gains an ignoredPaths field (via git ls-files --others --ignored --exclude-standard --directory, run inside the existing Promise.all for zero added latency)
  • New useGitStatusMap hook is a pure derivation over the shared git.getStatus query cache — zero extra subscriptions, zero extra event handlers
  • Unifies server-side dirty signal: GitWatcher now watches both .git/ and the worktree (via multiplexed @superset/workspace-fs watcher), coalescing both into one debounced git:changed per workspace. useChangesTab drops its fs:events subscription and client-side 300ms debounce
  • File tree polish: reduced indent (10px), fixed sticky z-index overflow at deep nesting, right padding for badges, folder dots via LuCircle icon

Test plan

  • Open a v2 workspace, modify a file → gold M badge + colored filename, ancestor folders show gold dot
  • Add an untracked file → green U badge
  • Delete a tracked file → no dot propagated to ancestor folders (deleted files are excluded from roll-up since they don't appear in the tree)
  • Verify node_modules, .turbo, dist render muted when visible
  • Switch to Changes tab and back — decorations persist, no extra network requests
  • Edit a file from terminal → decoration updates within ~300ms
  • Expand a deeply nested folder (depth > 10) → folders render correctly (z-index fix)

Summary by cubic

Adds VS Code–style git decorations to the v2 “All files” tree and mutes gitignored paths. Consolidates git-change signals across .git/ and the worktree for fewer, faster renders.

  • New Features

    • Colored filenames and single-letter badges (M/A/U/D/R/C) for changed files.
    • Ancestor folders show a roll-up dot using the worst descendant status; deleted files don’t roll up.
    • Gitignored paths render muted; git.getStatus now returns ignoredPaths (via git ls-files --others --ignored --exclude-standard --directory).
  • Refactors

    • Introduced useGitStatus as the single owner of the git.getStatus query and git:changed subscription; useChangesTab consumes injected status; useGitStatusMap derives fileStatusByPath and folderStatusByPath.
    • Server GitWatcher watches both .git/ and the worktree via @superset/workspace-fs, coalescing into one debounced git:changed per workspace; client drops fs:events and the 300ms debounce, and stops refetching listCommits on git:changed.
    • File tree polish: 10px indent, right padding for badges, stable sticky z-index at deep nesting, folder dots via LuCircle; switched from custom --diff-* tokens to Tailwind colors (modified uses yellow); “No files found” only shows when the root is loaded.

Written for commit fafcee6. Summary will update on new commits.

Summary by CodeRabbit

  • New Features

    • File tree shows git-status decorations for files and folders (status badges/indicators).
    • Git status is fetched and provided to workspace UI for consistent status display.
  • Improvements

    • Ignored paths are respected and muted entries are visually dimmed.
    • Updated status color palette, tighter tree indentation/spacing, and “No files found” shown when empty.
    • Improved repository change detection so workspace status updates more reliably.

Adds VS Code-style decoration to the v2 workspace file tree:
- Modified files get a colored name + single-letter badge (M/A/U/D/R/C)
- Ancestor folders of any change get a colored dot roll-up
- Gitignored paths (node_modules, dist, .turbo, etc.) render muted

Backend: git.getStatus gains an ignoredPaths field populated by
`git ls-files --others --ignored --exclude-standard --directory`, run
inside the existing Promise.all so it adds zero wall-clock latency.

Client: new useGitStatusMap hook is a pure derivation over the shared
git.getStatus query cache (same key as useChangesTab → React Query
dedupes → zero extra subscriptions). TreeNode resolves decoration +
isMuted once per node and passes primitives down so WorkspaceFilesTreeItem
can React.memo cleanly — after a refresh, only rows whose state actually
flipped re-render.

Also unifies the server-side dirty signal: GitWatcher now watches both
.git/ and the worktree (via @superset/workspace-fs's multiplexed watcher
manager, so zero duplicate native watchers) and coalesces both sources
into one debounced git:changed per workspace. useChangesTab drops its
fs:events subscription and client-side 300ms debounce — git:changed is
now the single refetch trigger.
… tweaks

- Bump row right padding to pr-4 for breathing room on status badges
- Reduce TREE_INDENT from 16px to 10px so deep paths stay on screen
- Shift paddingLeft to (depth-1)*indent so root items aren't over-indented
- Fix sticky folder z-index going negative at depth > 10 (was 10-depth,
  now 50-depth) — folders at depth 11+ were rendering behind the
  scroll container background
- Use LuCircle icon at opacity-50 for folder status dots instead of
  the tiny bullet character
- Skip deleted files when rolling up status to ancestor folders —
  expanding the folder wouldn't show the deleted file anyway
- Remove "Loading files..." placeholder
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 10, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds end-to-end Git status support: server returns ignored paths and watches worktrees; desktop fetches status via a new hook, maps file/folder statuses and ignored paths, and propagates decorations and muted state into the file tree UI.

Changes

Cohort / File(s) Summary
Git status hooks
apps/desktop/src/renderer/hooks/host-service/useGitStatus/index.ts, apps/desktop/src/renderer/hooks/host-service/useGitStatus/useGitStatus.ts, apps/desktop/src/renderer/hooks/host-service/useGitStatusMap/index.ts, apps/desktop/src/renderer/hooks/host-service/useGitStatusMap/useGitStatusMap.ts
Added useGitStatus (TRPC query + server-event wiring) and useGitStatusMap (normalize paths; build fileStatusByPath, propagate highest-severity to folderStatusByPath, produce ignoredPaths).
Files tree UI
apps/desktop/src/renderer/routes/.../FilesTab/FilesTab.tsx, .../WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx, .../hooks/useChangesTab/useChangesTab.tsx, apps/desktop/src/renderer/screens/.../FilesView/constants.ts
Wired gitStatus through WorkspaceSidebar → useChangesTab → FilesTab; propagate decoration and isMuted/ignored state into tree nodes; changed tree padding/indent/sticky math; useChangesTab signature updated to accept external gitStatus; TREE_INDENT 16→10.
Backend: event watching
packages/host-service/src/events/event-bus.ts, packages/host-service/src/events/git-watcher.ts
EventBus supplies filesystem to GitWatcher; GitWatcher now accepts WorkspaceFilesystemManager, starts an additional worktree-root async-iterator watch per workspace, manages disposers, and updates constructor signature.
Backend: git router
packages/host-service/src/trpc/router/git/git.ts
git.getStatus runs git ls-files --others --ignored --exclude-standard --directory in parallel, normalizes output, and returns ignoredPaths: string[] (failures swallowed).
Styling / status colors
apps/desktop/src/renderer/globals.css, apps/desktop/src/renderer/routes/.../useChangesTab/components/ChangesFileList/ChangesFileList.tsx
Removed legacy --diff-* CSS variables; updated STATUS_COLORS to explicit Tailwind text color classes per FileStatus.
Small UI tweaks
apps/desktop/src/renderer/routes/.../usePaneRegistry/components/FilePane/FilePane.tsx
Adjusted fallback container classes to use full-width (w-full) for several FilePane states.

Sequence Diagram(s)

sequenceDiagram
    participant UI as Desktop UI (WorkspaceSidebar / FilesTab)
    participant StatusHook as useGitStatus
    participant MapHook as useGitStatusMap
    participant TRPC as TRPC Client/Cache
    participant Router as HostService Git Router
    participant GitWatcher as HostService GitWatcher
    participant FS as Workspace Filesystem

    FS->>GitWatcher: worktree/fs change events
    GitWatcher->>TRPC: emit server event
    UI->>StatusHook: useGitStatus(workspaceId) subscribes
    StatusHook->>TRPC: query git.getStatus
    alt cache miss or refetch
        TRPC->>Router: git.getStatus request
        Router->>FS: run git.status() and git ls-files (--ignored)
        Router-->>TRPC: {againstBase, staged, unstaged, ignoredPaths}
    end
    TRPC-->>StatusHook: status payload
    StatusHook-->>MapHook: provide status.data
    MapHook->>MapHook: compute fileStatusByPath, folderStatusByPath, ignoredPaths
    MapHook-->>UI: maps & ignoredPaths
    UI->>UI: render TreeNodes with decoration / isMuted
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Poem

🐰 I hopped through branches, mapped each trail,

Marked muted burrows where ignored files pale,
Badges on leaves and folders softly glow,
Worktrees whisper changes down below,
A rabbit cheers — the tree now tells the show!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 18.18% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main feature: adding git decoration to the v2 file tree in the desktop app.
Description check ✅ Passed Pull request description is comprehensive and well-structured, covering objectives, implementation details, test plan, and technical refactoring scope.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch saddlepaddle/unexpected-slipper

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Apr 10, 2026

Greptile Summary

This PR adds VS Code-style git decorations to the v2 workspace file tree: colored filenames, single-letter status badges (M/A/U/D/R/C), roll-up dots on ancestor folders, and muted rendering for gitignored paths. It also refactors the event plumbing so that the server-side GitWatcher becomes the single, debounced source of truth for git-state changes — removing a redundant client-side debounce and fs:events subscription from useChangesTab.

Key changes:

  • New useGitStatusMap hook — pure derivation over the existing git.getStatus React Query cache; zero extra subscriptions or refetches
  • git.getStatus extended — adds ignoredPaths field via git ls-files --others --ignored --exclude-standard --directory, batched inside the existing Promise.all for zero latency cost
  • GitWatcher expanded — now watches both .git/ (metadata) and the worktree root (file edits) via the shared @superset/workspace-fs watcher, coalescing both into one debounced git:changed per workspace
  • useChangesTab simplified — drops client-side 300ms debounce and fs:events listener; relies entirely on server-side debounce
  • WorkspaceFilesTreeItem polished — status badge rendering, React.memo wrapping, reduced indent (10 px), fixed sticky z-index at deep nesting (50 − depth instead of 10 − depth)
  • Loading-state regression — the ternary refactor in FilesTab accidentally dropped the "Loading files…" branch; users see "No files found" while the initial tree fetch is in flight

Confidence Score: 4/5

Safe to merge after fixing the loading-state regression; all other issues are minor quality concerns.

The architecture is well thought-out (zero extra subscriptions, server-side debounce, React.memo on tree items) and the feature scope is well-contained. One concrete user-facing regression exists — the 'Loading files…' indicator was silently dropped during the ternary refactor — which should be fixed before shipping. The remaining issues (mutable EMPTY_RESULT, race window in error handler, unused changedAncestors) are code-quality concerns that don't affect the primary user path.

FilesTab.tsx (loading-state regression) and git-watcher.ts (error-handler race window)

Important Files Changed

Filename Overview
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx Integrates git decoration and muted-path rendering into the file tree; loading-state branch was accidentally dropped during the ternary refactor, causing "No files found" to flash while the tree loads.
apps/desktop/src/renderer/hooks/host-service/useGitStatusMap/useGitStatusMap.ts New hook that derives git status maps from the shared React Query cache. Logic is sound; mutable shared EMPTY_RESULT constant and dead changedAncestors computation are minor issues.
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx Adds VS Code-style status badges, muted-path dimming, z-index/indent polish, and wraps component in React.memo. Implementation is clean and correct.
packages/host-service/src/events/git-watcher.ts Adds worktree fs-watch leg to the existing .git/ watcher so both sources coalesce into one debounced git:changed signal. Logic is solid; narrow race window in the error handler could leave a worktree watcher iterator un-terminated.
packages/host-service/src/trpc/router/git/git.ts Adds ignoredPaths field to getStatus by running git ls-files --directory inside the existing Promise.all — zero latency impact. Implementation is correct.
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsx Drops client-side debounce and fs:events subscription in favour of the new server-side GitWatcher debounce — simplification is correct and safe.
packages/host-service/src/events/event-bus.ts Minimal change: passes filesystem manager to GitWatcher constructor. No issues.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/constants.ts TREE_INDENT reduced from 16px to 10px to match the new padding formula — cosmetic change only.

Sequence Diagram

sequenceDiagram
    participant WT as Worktree / .git dir
    participant GW as GitWatcher (server)
    participant EB as EventBus (server)
    participant WS as WebSocket
    participant UCT as useChangesTab (client)
    participant UGSM as useGitStatusMap (client)
    participant RQ as React Query Cache

    WT->>GW: fs change (worktree watch or .git/ watch)
    GW->>GW: debouncedEmit (300ms)
    GW->>EB: git:changed(workspaceId)
    EB->>WS: broadcast {type:"git:changed"}
    WS->>UCT: useWorkspaceEvent("git:changed")
    UCT->>RQ: invalidate git.getStatus + git.listCommits
    RQ->>RQ: refetch git.getStatus
    RQ-->>UGSM: useMemo re-derives (same cache key)
    UGSM-->>FilesTab: updated statusByPath / worstStatusByFolder / ignoredPaths
Loading

Comments Outside Diff (1)

  1. apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx, line 309-313 (link)

    P1 "Loading files..." indicator removed

    The refactor flattened the ternary but dropped the loading-state branch. When fileTree.isLoadingRoot === true and rootEntries is still empty, users now see "No files found" instead of "Loading files...", which is actively misleading.

    The original code had three branches:

    1. isLoadingRoot && rootEntries.length === 0 → "Loading files…"
    2. rootEntries.length === 0 && !isCreatingAtRoot → "No files found"
    3. Otherwise → render tree

    After this change only branches 2 & 3 remain, so branch 1 silently falls into branch 2.

Reviews (1): Last reviewed commit: "fix(desktop): file tree polish — padding..." | Re-trigger Greptile

Comment on lines +36 to +41
const EMPTY_RESULT: UseGitStatusMapResult = {
statusByPath: new Map(),
changedAncestors: new Set(),
worstStatusByFolder: new Map(),
ignoredPaths: new Set(),
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Mutable Map/Set in shared module-level constant

EMPTY_RESULT holds live Map and Set objects at module scope. Because they're mutable, any consumer that accidentally calls .set() / .add() on them (e.g., during a future refactor) will corrupt the shared singleton for all workspaces — with no error and no indication of where the mutation came from.

Prefer a factory function that constructs fresh instances:

function emptyResult(): UseGitStatusMapResult {
  return {
    statusByPath: new Map(),
    changedAncestors: new Set(),
    worstStatusByFolder: new Map(),
    ignoredPaths: new Set(),
  };
}

Then replace return EMPTY_RESULT; with return emptyResult();. The allocation cost is negligible — this path is only hit when status.data is falsy.

Comment on lines +173 to +182
watcher.on("error", () => {
// Watcher died — remove it so rescan can re-add
this.watched.delete(workspaceId);
const entry = this.watched.get(workspaceId);
if (entry) {
entry.disposeWorktreeWatch();
this.watched.delete(workspaceId);
}
watcher.close();
});
} catch {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Race window: disposeWorktreeWatch may never be called on early error

The error handler looks up this.watched.get(workspaceId) to call entry.disposeWorktreeWatch(). However, this.watched.set(...) is called after this handler is registered (and after startWorktreeWatch() returns). If the error event fires in the narrow window between handler registration and this.watched.set(...), the lookup returns undefined, and the worktree watcher's async loop is never terminated — leaking the iterator until close() is eventually called.

Capture the disposer in the closure before registering the error handler:

const disposeWorktreeWatch = this.startWorktreeWatch(workspaceId, worktreePath);

let watcher: FSWatcher;
try {
    watcher = watch(gitDir, { recursive: true }, () => {
        this.debouncedEmit(workspaceId);
    });
    watcher.on("error", () => {
        disposeWorktreeWatch(); // always available via closure
        this.watched.delete(workspaceId);
        watcher.close();
    });
} catch {
    disposeWorktreeWatch();
    return;
}

this.watched.set(workspaceId, { workspaceId, worktreePath, gitDir, watcher, disposeWorktreeWatch });

Comment on lines +85 to +104
const changedAncestors = new Set<string>();
const worstStatusByFolder = new Map<string, FileStatus>();
for (const [path, fileStatus] of merged) {
// Deleted files don't appear in the tree, so propagating a
// dot to ancestor folders is misleading — users expand the
// folder expecting to find something but there's nothing there.
if (fileStatus === "deleted") continue;

const segments = path.split("/");
for (let i = 1; i < segments.length; i++) {
const ancestor = segments.slice(0, i).join("/");
changedAncestors.add(ancestor);
const existing = worstStatusByFolder.get(ancestor);
if (
!existing ||
STATUS_SEVERITY[fileStatus] > STATUS_SEVERITY[existing]
) {
worstStatusByFolder.set(ancestor, fileStatus);
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 changedAncestors computed but never consumed

changedAncestors is built, included in the return value, and exported in the interface, but FilesTab.tsx only destructures statusByPath, worstStatusByFolder, and ignoredPaths. The set is computed on every status.data change for no current benefit.

If there are no planned near-term consumers, consider removing it from the return type to keep the interface minimal. If it's intentionally reserved for future use, a // TODO: comment would help future readers.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 10, 2026

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

  • ✅ Neon database branch
  • ✅ Electric Fly.io app

Thank you for your contribution! 🎉

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
apps/desktop/src/renderer/hooks/host-service/useGitStatusMap/useGitStatusMap.ts (1)

48-54: Update the hook comment to match the new invalidation flow.

This still says useChangesTab owns the debounced invalidate, but the debounce moved server-side in GitWatcher. Rewording it now avoids sending the next reader down the wrong path.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/desktop/src/renderer/hooks/host-service/useGitStatusMap/useGitStatusMap.ts`
around lines 48 - 54, Update the block comment in useGitStatusMap to reflect
that the debounce logic was moved server-side into GitWatcher: state that
useGitStatusMap deliberately does NOT subscribe to `git:changed` or `fs:events`
because `useChangesTab` is mounted unconditionally and only triggers
invalidation (the debouncing of events now happens in `GitWatcher`), and warn
that adding a local subscription would cause duplicate handlers/refetches;
reference the hook names `useGitStatusMap`, `useChangesTab`, and `GitWatcher` to
make the new invalidation flow explicit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx:
- Around line 103-105: The sticky item's zIndex currently uses a hard cutoff
"zIndex: 50 - depth" in WorkspaceFilesTreeItem which underflows for deep
nesting; change it to compute zIndex with a safe floor or much larger headroom
(e.g. use Math.max(50 - depth, MIN_Z_INDEX) or use a large base like
BASE_Z_INDEX - depth) so zIndex never becomes negative—update the zIndex
expression where position: "sticky" and top: (depth - 1) * rowHeight are set to
use a clamped value (refer to WorkspaceFilesTreeItem and the zIndex calculation)
and pick a sensible MIN_Z_INDEX/BASE_Z_INDEX constant to avoid overlap for very
deep trees.

In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx:
- Around line 531-534: The empty-state message is shown too early because
fileTree.rootEntries.length === 0 can be true while the root is still loading;
update the conditional in FilesTab.tsx (the FilesTab component) to also check
fileTree.isLoadingRoot (or equivalent) so the "No files found" message only
renders when root loading is finished and there truly are no entries;
specifically, change the existing conditional that uses fileTree.rootEntries and
isCreatingAtRoot to require !fileTree.isLoadingRoot before rendering the empty
state so the area remains blank during initial root resolution.

---

Nitpick comments:
In
`@apps/desktop/src/renderer/hooks/host-service/useGitStatusMap/useGitStatusMap.ts`:
- Around line 48-54: Update the block comment in useGitStatusMap to reflect that
the debounce logic was moved server-side into GitWatcher: state that
useGitStatusMap deliberately does NOT subscribe to `git:changed` or `fs:events`
because `useChangesTab` is mounted unconditionally and only triggers
invalidation (the debouncing of events now happens in `GitWatcher`), and warn
that adding a local subscription would cause duplicate handlers/refetches;
reference the hook names `useGitStatusMap`, `useChangesTab`, and `GitWatcher` to
make the new invalidation flow explicit.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a4aa5dfb-c566-4ce0-b3a2-33e21f4102b7

📥 Commits

Reviewing files that changed from the base of the PR and between 5b14771 and 0d07a5f.

📒 Files selected for processing (9)
  • apps/desktop/src/renderer/hooks/host-service/useGitStatusMap/index.ts
  • apps/desktop/src/renderer/hooks/host-service/useGitStatusMap/useGitStatusMap.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsx
  • apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/constants.ts
  • packages/host-service/src/events/event-bus.ts
  • packages/host-service/src/events/git-watcher.ts
  • packages/host-service/src/trpc/router/git/git.ts

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 issues found across 9 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/host-service/src/trpc/router/git/git.ts">

<violation number="1" location="packages/host-service/src/trpc/router/git/git.ts:104">
P2: Avoid silently swallowing errors from `git.raw(...)`; log contextual warning before falling back.

(Based on your team's feedback about handling async errors explicitly and avoiding silent catch blocks.) [FEEDBACK_USED]</violation>
</file>

<file name="packages/host-service/src/events/git-watcher.ts">

<violation number="1" location="packages/host-service/src/events/git-watcher.ts:187">
P1: A failed worktree watcher startup is treated as success, so the workspace can get stuck without worktree git-change updates.</violation>

<violation number="2" location="packages/host-service/src/events/git-watcher.ts:250">
P2: Avoid silently swallowing errors during iterator cleanup; log a warning with workspace context.</violation>
</file>

<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx">

<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx:531">
P2: The new empty-state condition ignores `isLoadingRoot`, so the UI can show “No files found” while files are still loading.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread packages/host-service/src/events/git-watcher.ts Outdated
"--exclude-standard",
"--directory",
])
.catch(() => ""),
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Avoid silently swallowing errors from git.raw(...); log contextual warning before falling back.

(Based on your team's feedback about handling async errors explicitly and avoiding silent catch blocks.)

View Feedback

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/host-service/src/trpc/router/git/git.ts, line 104:

<comment>Avoid silently swallowing errors from `git.raw(...)`; log contextual warning before falling back.

(Based on your team's feedback about handling async errors explicitly and avoiding silent catch blocks.) </comment>

<file context>
@@ -86,13 +86,31 @@ export const gitRouter = router({
+							"--exclude-standard",
+							"--directory",
+						])
+						.catch(() => ""),
+				]);
+
</file context>
Suggested change
.catch(() => ""),
.catch((error) => {
console.warn("[git.getStatus] Failed to list ignored paths:", error);
return "";
}),
Fix with Cubic


return () => {
disposed = true;
void iterator?.return?.().catch(() => {});
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Apr 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Avoid silently swallowing errors during iterator cleanup; log a warning with workspace context.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/host-service/src/events/git-watcher.ts, line 250:

<comment>Avoid silently swallowing errors during iterator cleanup; log a warning with workspace context.</comment>

<file context>
@@ -141,25 +164,91 @@ export class GitWatcher {
+
+		return () => {
+			disposed = true;
+			void iterator?.return?.().catch(() => {});
+			iterator = null;
+		};
</file context>
Suggested change
void iterator?.return?.().catch(() => {});
void iterator?.return?.().catch((error) => {
console.warn("[git-watcher] failed to close worktree watch stream:", {
workspaceId,
error,
});
});
Fix with Cubic

…s tab UI

Three single-purpose hooks instead of one tangled one:

- useGitStatus(workspaceId) — fetches + subscribes + invalidates. Single
  owner of the git.getStatus query and git:changed subscription. Called
  once at the WorkspaceSidebar level.
- useGitStatusMap(statusData) — pure useMemo over the data, returns
  { fileStatusByPath, folderStatusByPath, ignoredPaths }. No fetching,
  no subscription, no dependency on CollectionsProvider.
- useChangesTab({ workspaceId, gitStatus, ... }) — now just the Changes
  tab UI. Accepts git status as a param instead of fetching internally.
  Still owns its Changes-tab-specific queries (listCommits, listBranches,
  getCommitFiles) and filter state.

WorkspaceSidebar calls useGitStatus once and passes the query result to
useChangesTab and the query data to FilesTab. Single query, single
subscription, explicit data flow — no more React Query dedup tricks or
"two hooks with the same key happen to share a cache entry."

Also:
- Renamed `statusByPath` → `fileStatusByPath` and `worstStatusByFolder`
  → `folderStatusByPath` for symmetry.
- Dropped the unused `changedAncestors` set (value-equivalent to
  folderStatusByPath.keys()).
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx (1)

536-539: ⚠️ Potential issue | 🟡 Minor

Keep the empty state behind root-load completion.

rootEntries.length === 0 is also true during the initial root fetch, so this still flashes "No files found" for non-empty workspaces. Gate it on !fileTree.isLoadingRoot.

Suggested fix
-				{fileTree.rootEntries.length === 0 && !isCreatingAtRoot ? (
+				{fileTree.rootEntries.length === 0 &&
+				!fileTree.isLoadingRoot &&
+				!isCreatingAtRoot ? (
 					<div className="px-2 py-3 text-sm text-muted-foreground">
 						No files found
 					</div>
 				) : (
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx
around lines 536 - 539, The empty-state "No files found" is shown during the
initial root fetch; in FilesTab change the render condition that currently uses
fileTree.rootEntries.length === 0 && !isCreatingAtRoot to also require the root
load to be finished by checking fileTree.isLoadingRoot (i.e., only show the
message when rootEntries is empty, not creating at root, and
fileTree.isLoadingRoot is false). Update the conditional that renders the "No
files found" block (referencing fileTree.rootEntries, isCreatingAtRoot, and
fileTree.isLoadingRoot) so the message is gated behind root-load completion.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/hooks/host-service/useGitStatusMap/useGitStatusMap.ts (1)

99-100: Extract path normalization into a shared helper.

normalizePath() is now duplicated here and in FilesTab.tsx as toPosix(). If those rules drift, ignored-path matching and status lookups will diverge silently. Move this into one shared utility and reuse it from both call sites.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/desktop/src/renderer/hooks/host-service/useGitStatusMap/useGitStatusMap.ts`
around lines 99 - 100, normalizePath() is duplicated as toPosix() elsewhere
causing drift; extract the path normalization logic into a single shared utility
(e.g., export a normalizePath/toPosix function from a new or existing utils
module) and replace local definitions: remove the normalizePath implementation
from useGitStatusMap.ts and update callers (including FilesTab.tsx which uses
toPosix) to import and use the shared function; ensure the exported function
preserves current behavior (replace backslashes with "/") and update any
tests/imports accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/desktop/src/renderer/hooks/host-service/useGitStatus/useGitStatus.ts`:
- Around line 30-33: The invalidate callback in the useGitStatus hook is
overbroad: it calls utils.git.listCommits.invalidate on every git:changed event
causing commit history to refetch on ordinary file edits; remove (or gate) the
utils.git.listCommits.invalidate({ workspaceId }) call from the invalidate
function in useGitStatus so it only invalidates utils.git.getStatus.invalidate({
workspaceId }), or alternatively only call listCommits.invalidate when the event
indicates a ref/HEAD change (e.g. add a condition on the event payload before
calling utils.git.listCommits.invalidate).

---

Duplicate comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx:
- Around line 536-539: The empty-state "No files found" is shown during the
initial root fetch; in FilesTab change the render condition that currently uses
fileTree.rootEntries.length === 0 && !isCreatingAtRoot to also require the root
load to be finished by checking fileTree.isLoadingRoot (i.e., only show the
message when rootEntries is empty, not creating at root, and
fileTree.isLoadingRoot is false). Update the conditional that renders the "No
files found" block (referencing fileTree.rootEntries, isCreatingAtRoot, and
fileTree.isLoadingRoot) so the message is gated behind root-load completion.

---

Nitpick comments:
In
`@apps/desktop/src/renderer/hooks/host-service/useGitStatusMap/useGitStatusMap.ts`:
- Around line 99-100: normalizePath() is duplicated as toPosix() elsewhere
causing drift; extract the path normalization logic into a single shared utility
(e.g., export a normalizePath/toPosix function from a new or existing utils
module) and replace local definitions: remove the normalizePath implementation
from useGitStatusMap.ts and update callers (including FilesTab.tsx which uses
toPosix) to import and use the shared function; ensure the exported function
preserves current behavior (replace backslashes with "/") and update any
tests/imports accordingly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a1bd3713-499b-4a8c-a80e-e679322ddd9c

📥 Commits

Reviewing files that changed from the base of the PR and between 0d07a5f and e7c4ff7.

📒 Files selected for processing (7)
  • apps/desktop/src/renderer/hooks/host-service/useGitStatus/index.ts
  • apps/desktop/src/renderer/hooks/host-service/useGitStatus/useGitStatus.ts
  • apps/desktop/src/renderer/hooks/host-service/useGitStatusMap/index.ts
  • apps/desktop/src/renderer/hooks/host-service/useGitStatusMap/useGitStatusMap.ts
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/WorkspaceSidebar.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsx
✅ Files skipped from review due to trivial changes (2)
  • apps/desktop/src/renderer/hooks/host-service/useGitStatus/index.ts
  • apps/desktop/src/renderer/hooks/host-service/useGitStatusMap/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/useChangesTab.tsx

…colors

Removed 15 lines of custom CSS variables (--diff-added, --diff-modified,
etc.) from globals.css. These were only used in two places — ChangesFileList
and WorkspaceFilesTreeItem — and encouraged a pattern of adding bespoke
tokens to globals.css for every semantic color.

Replaced with standard Tailwind colors + dark: variants, leveraging the
existing @custom-variant dark (&:is(.dark *)) in packages/ui/src/globals.css
and the .dark class the theme store sets on the root element.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx (1)

101-104: ⚠️ Potential issue | 🟡 Minor

Clamp sticky z-index to avoid deep nesting underflow.

Line 103 can go non-positive (50 - depth) for deep trees, causing sticky overlap regressions again.

Suggested fix
 		...(isFolder
 			? {
 					position: "sticky" as const,
 					top: (depth - 1) * rowHeight,
-					zIndex: 50 - depth,
+					zIndex: Math.max(1, 1000 - depth),
 				}
 			: {}),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx
around lines 101 - 104, The zIndex calculation in WorkspaceFilesTreeItem (where
the inline style sets position: "sticky", top: (depth - 1) * rowHeight, zIndex:
50 - depth) can become non-positive for deep trees; update that zIndex
assignment to clamp to a positive minimum (e.g., use Math.max with a floor of 1)
so zIndex = Math.max(1, 50 - depth) to avoid underflow and sticky overlap
regressions, keeping the change localized to the style object in
WorkspaceFilesTreeItem.tsx.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx (1)

12-31: Consider centralizing git status presentation constants.

STATUS_TEXT_CLASS / STATUS_LETTER here now mirror logic in the changes list component. A shared constant module would reduce drift risk.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx
around lines 12 - 31, Extract STATUS_TEXT_CLASS and STATUS_LETTER into a shared
constants module (e.g., gitStatusConstants) and replace the local definitions in
WorkspaceFilesTreeItem (the STATUS_TEXT_CLASS and STATUS_LETTER symbols) with
imports from that new module; update the other component that currently
duplicates this logic (the changes list component) to import the same constants
instead of keeping its own copies, then remove the duplicate definitions from
both files so all git-status-to-presentation mapping is centralized and reused.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx:
- Around line 101-104: The zIndex calculation in WorkspaceFilesTreeItem (where
the inline style sets position: "sticky", top: (depth - 1) * rowHeight, zIndex:
50 - depth) can become non-positive for deep trees; update that zIndex
assignment to clamp to a positive minimum (e.g., use Math.max with a floor of 1)
so zIndex = Math.max(1, 50 - depth) to avoid underflow and sticky overlap
regressions, keeping the change localized to the style object in
WorkspaceFilesTreeItem.tsx.

---

Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx:
- Around line 12-31: Extract STATUS_TEXT_CLASS and STATUS_LETTER into a shared
constants module (e.g., gitStatusConstants) and replace the local definitions in
WorkspaceFilesTreeItem (the STATUS_TEXT_CLASS and STATUS_LETTER symbols) with
imports from that new module; update the other component that currently
duplicates this logic (the changes list component) to import the same constants
instead of keeping its own copies, then remove the duplicate definitions from
both files so all git-status-to-presentation mapping is centralized and reused.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 022792a4-ac16-409f-8a5f-e2405752cf34

📥 Commits

Reviewing files that changed from the base of the PR and between 94bfaef and 30f0ff1.

📒 Files selected for processing (3)
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/ChangesFileList.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/FilePane.tsx
✅ Files skipped from review due to trivial changes (1)
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/hooks/usePaneRegistry/components/FilePane/FilePane.tsx

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx (1)

169-172: memo() optimization is ineffective—parent passes unstable callback references.

The comment claims callbacks are "stable across renders in practice," but the parent FilesTab.tsx passes onSelectFile, onToggleDirectory, onNewFile, onNewFolder, onRename, and onDelete as inline or non-memoized functions. Every parent render creates new function references, causing this memoized component to re-render anyway.

Either wrap the callbacks in useCallback in the parent, or remove memo() to avoid misleading future maintainers.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx
around lines 169 - 172, WorkspaceFilesTreeItem is memoized but parent callbacks
in FilesTab.tsx (onSelectFile, onToggleDirectory, onNewFile, onNewFolder,
onRename, onDelete) are created inline each render, so memo() is ineffective;
either remove memo() from WorkspaceFilesTreeItem export or (preferable) make
those callbacks stable by wrapping each handler in FilesTab.tsx with useCallback
(preserve their dependency arrays so they update only when truly needed) so
WorkspaceFilesTreeItem can actually benefit from memoization.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx:
- Around line 12-31: Remove the invalid "changed" entries from the
STATUS_TEXT_CLASS and STATUS_LETTER constants to match the FileStatus union
(which uses "modified" instead); update the mappings in STATUS_TEXT_CLASS and
STATUS_LETTER so they only include added, copied, modified, deleted, renamed,
and untracked (keep the "modified" mappings as the yellow text and "M" badge).
- Line 98: The padding formula in WorkspaceFilesTreeItem is inconsistent with
NewItemInput causing a 6px shift; update WorkspaceFilesTreeItem's paddingLeft
calculation (currently "8 + (depth - 1) * indent") to use the same formula as
NewItemInput (use TREE_INDENT) — e.g., compute paddingLeft as "4 + depth *
TREE_INDENT" or otherwise ensure "indent" equals TREE_INDENT so both use the
identical base and per-depth values; change the expression in
WorkspaceFilesTreeItem to reference TREE_INDENT (or align indent) so display and
inline-edit modes align exactly.

---

Nitpick comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx:
- Around line 169-172: WorkspaceFilesTreeItem is memoized but parent callbacks
in FilesTab.tsx (onSelectFile, onToggleDirectory, onNewFile, onNewFolder,
onRename, onDelete) are created inline each render, so memo() is ineffective;
either remove memo() from WorkspaceFilesTreeItem export or (preferable) make
those callbacks stable by wrapping each handler in FilesTab.tsx with useCallback
(preserve their dependency arrays so they update only when truly needed) so
WorkspaceFilesTreeItem can actually benefit from memoization.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: faf751cb-22b7-4793-8ab6-a913210e91df

📥 Commits

Reviewing files that changed from the base of the PR and between 30f0ff1 and 53ac98d.

📒 Files selected for processing (2)
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/ChangesFileList.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/$workspaceId/components/WorkspaceSidebar/hooks/useChangesTab/components/ChangesFileList/ChangesFileList.tsx

Comment on lines +12 to +31
const STATUS_TEXT_CLASS: Record<FileStatus, string> = {
added: "text-green-700 dark:text-green-400",
copied: "text-purple-700 dark:text-purple-400",
changed: "text-yellow-600 dark:text-yellow-400",
deleted: "text-red-700 dark:text-red-500",
modified: "text-yellow-600 dark:text-yellow-400",
renamed: "text-blue-600 dark:text-blue-400",
untracked: "text-green-700 dark:text-green-400",
};

// Single-letter badge shown on the right of changed file rows, VS Code style.
const STATUS_LETTER: Record<FileStatus, string> = {
added: "A",
copied: "C",
changed: "M",
deleted: "D",
modified: "M",
renamed: "R",
untracked: "U",
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the FileStatus type definition in useGitStatusMap
fd -t f 'useGitStatusMap' --exec cat {}

# Also check the shared changes-types.ts for comparison
cat apps/desktop/src/shared/changes-types.ts 2>/dev/null || echo "File not found"

Repository: superset-sh/superset

Length of output: 5737


Remove the "changed" key from both STATUS_TEXT_CLASS and STATUS_LETTER constants.

The FileStatus type does not include "changed" (it has: "added", "modified", "deleted", "renamed", "copied", "untracked"). Including "changed" in the Record<FileStatus, string> mappings creates a type error. Since "changed" and "modified" map to identical values (yellow text and "M" badge), remove the "changed" entries:

Changes needed:
const STATUS_TEXT_CLASS: Record<FileStatus, string> = {
	added: "text-green-700 dark:text-green-400",
	copied: "text-purple-700 dark:text-purple-400",
	deleted: "text-red-700 dark:text-red-500",
	modified: "text-yellow-600 dark:text-yellow-400",
	renamed: "text-blue-600 dark:text-blue-400",
	untracked: "text-green-700 dark:text-green-400",
};

const STATUS_LETTER: Record<FileStatus, string> = {
	added: "A",
	copied: "C",
	deleted: "D",
	modified: "M",
	renamed: "R",
	untracked: "U",
};
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx
around lines 12 - 31, Remove the invalid "changed" entries from the
STATUS_TEXT_CLASS and STATUS_LETTER constants to match the FileStatus union
(which uses "modified" instead); update the mappings in STATUS_TEXT_CLASS and
STATUS_LETTER so they only include added, copied, modified, deleted, renamed,
and untracked (keep the "modified" mappings as the yellow text and "M" badge).

style={{
height: rowHeight,
paddingLeft: 4 + depth * indent,
paddingLeft: 8 + (depth - 1) * indent,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check NewItemInput indentation formula
rg -n "paddingLeft" apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/\$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/NewItemInput/

Repository: superset-sh/superset

Length of output: 282


🏁 Script executed:

# Check the actual code in WorkspaceFilesTreeItem.tsx around line 98
sed -n '90,110p' apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/\$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx

Repository: superset-sh/superset

Length of output: 571


🏁 Script executed:

# Find TREE_INDENT constant definition
rg -n "TREE_INDENT" apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/ --type ts --type tsx

Repository: superset-sh/superset

Length of output: 91


🏁 Script executed:

# Check the context of indentation in NewItemInput.tsx
sed -n '55,75p' apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/\$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/NewItemInput/NewItemInput.tsx

Repository: superset-sh/superset

Length of output: 596


🏁 Script executed:

# Find TREE_INDENT constant definition (without type filter)
rg -n "TREE_INDENT" apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/

Repository: superset-sh/superset

Length of output: 816


🏁 Script executed:

# Find where WorkspaceFilesTreeItem is used and what indent value is passed
rg -n "WorkspaceFilesTreeItem" apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/ -A 3 -B 3

Repository: superset-sh/superset

Length of output: 8532


🏁 Script executed:

# Check the component signature to see indent parameter definition
sed -n '1,50p' apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/\$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx

Repository: superset-sh/superset

Length of output: 1914


🏁 Script executed:

# Find TREE_INDENT constant definition
rg -n "const TREE_INDENT\|export.*TREE_INDENT" apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/constants --type-list | head -5

Repository: superset-sh/superset

Length of output: 453


🏁 Script executed:

# Read the constants file to find TREE_INDENT value
cat apps/desktop/src/renderer/screens/main/components/WorkspaceView/RightSidebar/FilesView/constants.ts

Repository: superset-sh/superset

Length of output: 807


🏁 Script executed:

# Check if there are any comments in NewItemInput or WorkspaceFilesTreeItem explaining the indentation
grep -n "paddingLeft\|indent" apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/\$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/NewItemInput/NewItemInput.tsx -B 2 -A 2

Repository: superset-sh/superset

Length of output: 166


🏁 Script executed:

# Check the broader context in FilesTab to see if this indentation difference is documented
sed -n '550,570p' apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/\$workspaceId/components/WorkspaceSidebar/components/FilesTab/FilesTab.tsx

Repository: superset-sh/superset

Length of output: 850


Fix indentation formula mismatch between display and inline edit modes.

WorkspaceFilesTreeItem (line 98) uses 8 + (depth - 1) * indent while NewItemInput uses 4 + depth * TREE_INDENT. With TREE_INDENT=10, this produces a consistent 6px misalignment at all depths (e.g., depth=1: 8px vs 14px), causing visual shift when toggling between viewing a file item and editing it inline.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/`$workspaceId/components/WorkspaceSidebar/components/FilesTab/components/WorkspaceFilesTreeItem/WorkspaceFilesTreeItem.tsx
at line 98, The padding formula in WorkspaceFilesTreeItem is inconsistent with
NewItemInput causing a 6px shift; update WorkspaceFilesTreeItem's paddingLeft
calculation (currently "8 + (depth - 1) * indent") to use the same formula as
NewItemInput (use TREE_INDENT) — e.g., compute paddingLeft as "4 + depth *
TREE_INDENT" or otherwise ensure "indent" equals TREE_INDENT so both use the
identical base and per-depth values; change the expression in
WorkspaceFilesTreeItem to reference TREE_INDENT (or align indent) so display and
inline-edit modes align exactly.

- useGitStatus: stop invalidating listCommits on git:changed. With the
  unified watcher, git:changed now fires on ordinary file edits, so
  refetching commit history every save was wasteful. listCommits now
  refreshes on window focus / mutations only.
- FilesTab: gate the "No files found" empty state on !isLoadingRoot so
  it doesn't flash during the initial root fetch.
- git-watcher: reorder initialization so disposeWorktreeWatch is captured
  in the .git watcher's error handler closure — fixes a race where the
  error could fire before this.watched.set ran, leaking the worktree
  watch.
- useGitStatusMap: swap EMPTY_RESULT singleton for an emptyResult()
  factory so consumers can't accidentally mutate a module-level Map/Set.
- WorkspaceFilesTreeItem: clamp sticky folder z-index with
  Math.max(1, ...) so depths past 50 don't underflow into the scroll
  container.
@saddlepaddle saddlepaddle merged commit 23b53ed into main Apr 10, 2026
15 checks passed
MocA-Love pushed a commit to MocA-Love/superset that referenced this pull request Apr 11, 2026
* feat(desktop): git decoration in v2 "All files" tree

Adds VS Code-style decoration to the v2 workspace file tree:
- Modified files get a colored name + single-letter badge (M/A/U/D/R/C)
- Ancestor folders of any change get a colored dot roll-up
- Gitignored paths (node_modules, dist, .turbo, etc.) render muted

Backend: git.getStatus gains an ignoredPaths field populated by
`git ls-files --others --ignored --exclude-standard --directory`, run
inside the existing Promise.all so it adds zero wall-clock latency.

Client: new useGitStatusMap hook is a pure derivation over the shared
git.getStatus query cache (same key as useChangesTab → React Query
dedupes → zero extra subscriptions). TreeNode resolves decoration +
isMuted once per node and passes primitives down so WorkspaceFilesTreeItem
can React.memo cleanly — after a refresh, only rows whose state actually
flipped re-render.

Also unifies the server-side dirty signal: GitWatcher now watches both
.git/ and the worktree (via @superset/workspace-fs's multiplexed watcher
manager, so zero duplicate native watchers) and coalesces both sources
into one debounced git:changed per workspace. useChangesTab drops its
fs:events subscription and client-side 300ms debounce — git:changed is
now the single refetch trigger.

* fix(desktop): add right padding to file tree status badge

* fix(desktop): file tree polish — padding, indent, z-index, decoration tweaks

- Bump row right padding to pr-4 for breathing room on status badges
- Reduce TREE_INDENT from 16px to 10px so deep paths stay on screen
- Shift paddingLeft to (depth-1)*indent so root items aren't over-indented
- Fix sticky folder z-index going negative at depth > 10 (was 10-depth,
  now 50-depth) — folders at depth 11+ were rendering behind the
  scroll container background
- Use LuCircle icon at opacity-50 for folder status dots instead of
  the tiny bullet character
- Skip deleted files when rolling up status to ancestor folders —
  expanding the folder wouldn't show the deleted file anyway
- Remove "Loading files..." placeholder

* refactor(desktop): split git status fetching from derivation + Changes tab UI

Three single-purpose hooks instead of one tangled one:

- useGitStatus(workspaceId) — fetches + subscribes + invalidates. Single
  owner of the git.getStatus query and git:changed subscription. Called
  once at the WorkspaceSidebar level.
- useGitStatusMap(statusData) — pure useMemo over the data, returns
  { fileStatusByPath, folderStatusByPath, ignoredPaths }. No fetching,
  no subscription, no dependency on CollectionsProvider.
- useChangesTab({ workspaceId, gitStatus, ... }) — now just the Changes
  tab UI. Accepts git status as a param instead of fetching internally.
  Still owns its Changes-tab-specific queries (listCommits, listBranches,
  getCommitFiles) and filter state.

WorkspaceSidebar calls useGitStatus once and passes the query result to
useChangesTab and the query data to FilesTab. Single query, single
subscription, explicit data flow — no more React Query dedup tricks or
"two hooks with the same key happen to share a cache entry."

Also:
- Renamed `statusByPath` → `fileStatusByPath` and `worstStatusByFolder`
  → `folderStatusByPath` for symmetry.
- Dropped the unused `changedAncestors` set (value-equivalent to
  folderStatusByPath.keys()).

* refactor(desktop): drop custom --diff-* tokens for standard Tailwind colors

Removed 15 lines of custom CSS variables (--diff-added, --diff-modified,
etc.) from globals.css. These were only used in two places — ChangesFileList
and WorkspaceFilesTreeItem — and encouraged a pattern of adding bespoke
tokens to globals.css for every semantic color.

Replaced with standard Tailwind colors + dark: variants, leveraging the
existing @custom-variant dark (&:is(.dark *)) in packages/ui/src/globals.css
and the .dark class the theme store sets on the root element.

* fix(desktop): tighten diff status colors to match old OKLCH tokens

* refactor(desktop): use yellow instead of amber for modified status

* fix(desktop): address PR review feedback

- useGitStatus: stop invalidating listCommits on git:changed. With the
  unified watcher, git:changed now fires on ordinary file edits, so
  refetching commit history every save was wasteful. listCommits now
  refreshes on window focus / mutations only.
- FilesTab: gate the "No files found" empty state on !isLoadingRoot so
  it doesn't flash during the initial root fetch.
- git-watcher: reorder initialization so disposeWorktreeWatch is captured
  in the .git watcher's error handler closure — fixes a race where the
  error could fire before this.watched.set ran, leaking the worktree
  watch.
- useGitStatusMap: swap EMPTY_RESULT singleton for an emptyResult()
  factory so consumers can't accidentally mutate a module-level Map/Set.
- WorkspaceFilesTreeItem: clamp sticky folder z-index with
  Math.max(1, ...) so depths past 50 don't underflow into the scroll
  container.
@coderabbitai coderabbitai Bot mentioned this pull request Apr 12, 2026
10 tasks
@Kitenite Kitenite deleted the saddlepaddle/unexpected-slipper branch May 6, 2026 04:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant