De-cardify UI surfaces and harden GitOps diffs#268
Conversation
Copy as promptReviewed by reactreview for commit c36f40b. Configure here. |
|
@codex please review this PR and give me 10-20 issues if any. Categorize findings as required, recommended, or optional. |
3eb6275 to
952c2fe
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3eb6275a51
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return queryClient.getQueryCache().subscribe((event) => { | ||
| const queryKey = event.query.queryKey | ||
| if (!isCurrentProjectDiffQuery(queryKey, projectId)) return | ||
| if (event.type === 'removed' || event.query.state.isInvalidated) { | ||
| setVersion((current) => current + 1) |
There was a problem hiding this comment.
[required] Refresh diff stream when projectDiff cache is absent
useProjectDiffStream relies on useProjectDiffInvalidationVersion to restart streaming after worktree-invalidating actions, but the version bump only happens from QueryCache events for existing projectDiff queries. In streaming mode the projectDiff query is disabled (queryEnabled is false), so that cache entry often does not exist; invalidation calls then produce no matching event and the stream never restarts. This leaves the GitOps panel showing stale diff content after commits/staging changes until the user manually changes baseline/view.
Useful? React with 👍 / 👎.
| includeUntracked, | ||
| streamRefreshVersion, | ||
| ) | ||
| const queryEnabled = enabled && Boolean(projectId) && !stream.canStream |
There was a problem hiding this comment.
[recommended] Fallback to non-stream diff when stream startup fails
The hook disables the legacy query path whenever the streaming API method exists, even if starting the stream fails at runtime (IPC mismatch, transient backend error, or bridge issue). In that case users only see an error/null diff although getProjectDiff could still return usable data. Adding a fallback to the non-stream query on stream start failure would prevent complete diff outage in partial-failure environments.
Useful? React with 👍 / 👎.
|
Addressed the required streamed-diff refresh finding. Streaming diffs now use an explicit project diff invalidation signal instead of relying on a disabled React Query cache entry existing. Also removed the old non-stream project diff request/query path rather than adding fallback behavior; streamed diffs are the only renderer path now. |
|
@codex please review this PR and give me 10-20 issues if any. Categorize findings as required, recommended, or optional. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b4d6e50aa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| baselineRev, | ||
| env, | ||
| hasHead, | ||
| includeUntracked: true, |
There was a problem hiding this comment.
Keep tracked-only stats when untracked files are excluded
In loadTrackedWorktreeSnapshot, the patch is generated from git add -u (tracked files only), but stats are computed with includeUntracked: true, so fileCount/insertions/deletions can include untracked files that are not present in the emitted diff. This produces inconsistent metadata (e.g., header counts disagree with rendered patch) whenever untracked files exist and includeUntracked is disabled.
Useful? React with 👍 / 👎.
| diffBaseline, | ||
| showDiffInMainView && (projectGitState?.isGitRepo ?? false), | ||
| ) | ||
| const diffLoadError: string | null = null |
There was a problem hiding this comment.
Restore diff error propagation to the GitOps footer
This sets diffLoadError to a constant null, removing the previous wiring from useDesktopDiff and preventing diff-load failures from surfacing in the composer GitOps message field. When diff streaming/parsing fails, users no longer get the footer-level error signal that was previously shown, which makes comment/commit flows appear healthy even though diff data is unavailable.
Useful? React with 👍 / 👎.
| blurOnLeave = false, | ||
| tolerancePx = DEFAULT_HOVER_TOLERANCE_PX, | ||
| isFocused, | ||
| extraBoundaryRefs = [], |
There was a problem hiding this comment.
Stabilize default extra boundary refs in hover hook
Using extraBoundaryRefs = [] in the parameter list creates a new array on every render, and that array is included in the effect dependency list, so the pointermove listener is torn down and re-registered every render when callers omit this prop. This adds avoidable churn to a global event listener and can degrade hover/focus responsiveness in frequently re-rendering views.
Useful? React with 👍 / 👎.
| "check": "bun run lint && bun run typecheck && bun run test && bun run build", | ||
| "ai:check": "bun run lint && bun run typecheck && bun run test", | ||
| "check": "bun run ai:check && bun run build", | ||
| "ai:check": "biome check --write --error-on-warnings . && tsgo --noEmit && tsgo -p tsconfig.desktop.json --noEmit && tsgo -p tsconfig.electron.json --noEmit && tsgo -p tsconfig.scripts.json --noEmit && bun ./scripts/check-vite-import-resolution.ts && vitest run", |
There was a problem hiding this comment.
Avoid auto-writing files inside ai:check
ai:check now runs biome check --write, and .husky/pre-push executes ai:check, so the pre-push gate mutates tracked files after commits are created. That means push validation can run against locally rewritten code that is not in the commit being pushed, leaving dirty trees and allowing unformatted/unchecked commit contents to be pushed despite a passing hook.
Useful? React with 👍 / 👎.
|
Addressed this batch too:\n\n- fixed tracked-only diff stats so hidden untracked files no longer affect visible diff counts\n- restored diff load error propagation from the streamed diff panel up to the GitOps footer\n- stabilized the hover-to-focus default boundary refs\n- made |
Summary
This is the big de-cardification / UI hardening branch.
@howcode/*module boundaries and moves native-ish capability surfaces into dedicated foldersNotes
This deliberately keeps behavior mostly the same while cleaning up the surface grammar and making future pluginisation less painful. Sidebar work is mostly parked and should be handled separately.
Validation
bun run ai:checkChangelog
Updated
docs/changelog.mdunder0.1.66.