git-diff: Lazy#5
Conversation
Remap the light and dark CSS token blocks in style.css to the exact GitHub Primer palettes (light: #ffffff canvas, #d1d9e0 borders, #1f2328 text, #0969da accent, #e6ffec/#ffebe9 diff fills; dark: #0d1117 canvas, #30363d borders, #e6edf3 text, #2f81f7 accent, translucent green/red diff overlays). Every component derives from these tokens, so the whole app re-skins at once. Route the JS accent override (lib/accent.ts → applyAccent) through a new per-theme --primer-accent token so the inline value resolves to the correct Primer blue in each mode. Declutter the header: drop the CPU/MEM/LOAD system-stats bar from TitleBar.vue and flatten its background to a clean Primer bar. Sidebar keeps only Submit review (Comment/Request removed).
GitHub's primary review action is green. Switch the sidebar Submit review button from the blue accent to the success token (#1f883d light / #238636 dark), matching the design and GitHub's Primer convention.
…reachable - style.css: in the Primer dark block, --primary-foreground and --success-foreground were still near-black (carried over from the old Pierre dark accent). With Primer's blue/green emphasis backgrounds this rendered unreadable dark text on the primary/success buttons. Set both to #ffffff so dark-mode CTAs have white text. - App.vue: the sidebar 'Submit review' button now opens the ReviewPanel instead of submitting immediately. The decluttered sidebar dropped the Comment/Request buttons that previously opened the panel, leaving the review-summary composer hard to reach; routing Submit review through the panel restores discoverable access (and matches GitHub's 'finish your review' flow). The panel's own Start review still performs the submit.
Revert the sidebar Submit review button to submit immediately (startReview) rather than opening the ReviewPanel first. Keeps the decluttered single-button sidebar per the design owner's preference; the ReviewPanel remains reachable via the line-comment-without-active-review path (App.vue). The dark-mode CTA contrast fix is unaffected.
…lease pipeline Three features matching nkzw-tech/codiff that git-diff lacked: 1. Lazy loading of large diff files (codiff's "6–8x faster startup"): viewport-deferred "mount once" rendering via IntersectionObserver. Files over 400 changed lines show a height-estimated placeholder until scrolled near, then mount and stay mounted. A shared registry lets in-diff search and hunk navigation force-render deferred files on demand so neither silently skips un-rendered content. 2. Resolved / outdated comment states + "hide resolved & outdated" tweak. `resolved` is persisted (new migration + PATCH .../resolve endpoint wired through the full stack); `outdated` is derived client-side (anchor line no longer present in the diff). Resolved/outdated comments dim and show a pill. 3. Operational Homebrew cask + automated GitHub releases. New .github/workflows/release.yml builds the unsigned arm64 DMG on a v* tag, publishes a Release with checksums, and opens a cask bump PR to oullin/homebrew-tap. distribution.md updated. Tests: Go storage/handler/migrator tests for resolve + the new column; Vitest for the lazy registry, estimatedDiffHeight, and useLazyDiffFile.
Apply the GitHub Primer light theme from the design handoff exactly: - Add Primer status fill tokens (--success/-attention/-danger/-done -subtle/-fg/-emphasis) for light and dark, matching the design's #dafbe1 / #fff8c5 / #ffebe9 / #fbefff palette. - StatusBadge: drive A/M/D/R chips from those tokens instead of approximate translucent rgb fills. - FileRow: replace the lime-green selected-row glow with the Primer blue (--gd-accent-soft bg + 3px --gd-accent bar); turn the viewed check Primer green (--success-emphasis) instead of accent blue. - FileHeader: Unstaged tag -> exact attention tokens; viewed toggle becomes a neutral pill that dims with a green check, per the design.
Bring the design bundle's diffstat-mini into the main file header: five 8px squares proportioned add/del (green --success-emphasis, red --danger-fg, neutral --gd-panel-3), matching git-diff-review's header layout. Sidebar rows keep the +N -N text variant.
There was a problem hiding this comment.
Code Review
This pull request introduces the ability to resolve and unresolve review comments, updating the database schema, Go backend handlers, IPC bridge, and Vue frontend components accordingly. It also implements viewport-deferred lazy rendering for large diff files to optimize performance, alongside a visual refresh using GitHub Primer design tokens. The review feedback highlights two key improvements: adding { immediate: true } to the rendered watcher in useLazyDiffFile.ts to prevent intersection observers from running indefinitely on already-rendered small files, and refactoring SetReviewCommentResolved in comments.go to fetch the comment first, avoiding redundant database operations and duplicate event emissions.
Add the Claude Design handoff bundle for the Git Diff Review UI as a standalone prototype reference under git-diff-review/. - index-dark.html: GitHub Primer dark theme tokens + component CSS - index.html: light sibling sharing the same JS - app.jsx / data.js / icons.js: React prototype, mock diff data, Octicons Copied byte-for-byte from the design export (md5-verified).
04166a1 to
86a646b
Compare
… components Decompose the DiffBody.vue god component (Phase A, partial) by extracting presentational leaf components, deduplicating markup that was copied across the split and unified render branches: - DiffGutter.vue: line-number cell with optional accent bar - DiffCodeCell.vue: sign prefix + highlighted code cell - DiffHunkHeader.vue: @@ hunk header + expand up/down controls (owns header parsing) Applied to both hunk headers, the unified line row, and the split context row. Public props/emits unchanged; DiffList.vue and App.vue untouched. DiffBody.vue shrinks ~433 lines. vue-tsc clean; all pre-existing-passing tests still pass.
Phase B of the DiffBody decomposition. Extract the remaining domain logic out of the DiffBody.vue component (per the 'components are presentational, composables own logic' rule) into pure, unit-testable composables: - useContextExpansionControls: expand up/down affordance math + dispatch (dedupes gap math that mirrored useContextExpansion internals) - useLineComments: line<->comment matching + lineSideAndNumber side resolution - useSplitCellRender: split-row render model (renderPair + CellRender) Also removes dead code (isAnchored, unused getAnchor). DiffBody's script now reads as wiring. Adds 21 unit tests covering the extracted logic with no component mount. vue-tsc/oxlint clean; full suite green (132 pass).
Completes the DiffBody leaf decomposition. The split-view pair row previously inlined the two-column markup and called renderPair(row) 13 times per row. Extract DiffSplitRow.vue (built from DiffGutter/DiffCodeCell) so the render model is computed once per row and the markup lives in one place. DiffBody.vue is now 450 lines (from 1024). vue-tsc/oxlint clean; suite green.
…package roots The lone non-TS source file (forge.config.cjs) becomes forge.config.ts using ESM (import/export default, import.meta.dirname). Extend check-typescript-only to scan workspace/package roots non-recursively so a new .js/.jsx/.mjs/.cjs config file at a root is caught going forward (previously only nested dirs were guarded, which is why forge.config.cjs slipped through).
…sertions) App.test.ts was failing on baseline for two reasons: - it mounted App without an active Pinia, so useAuthStore() threw - its content assertions checked for the full repo path and a 'Reviews' label that the current UI no longer renders Install a fresh Pinia in the mount and assert on what the UI actually shows (the changed file, the changed-file summary, and the Submit review control), preserving the test's intent. Full UI suite now green (133 passed).
Phase C of the App.vue decomposition. Move the pure derived view-state (changed-file maps, repoPaths, selectedFile, reviewComments, threadsByPath, userInitials, changedIndex, threadsForFile) into a side-effect-free composable that takes the repo/review/auth refs as input. Adds 8 unit tests covering the selectors with no component mount. App.vue shrinks ~45 lines; suite green.
Move the command-palette registration block into a dedicated composable that delegates each command to an injected app action. App.vue no longer owns the registration boilerplate (and no longer imports useCommandRegistry directly).
Move the review-session domain out of App.vue into a composable: start review, viewed-state, comment CRUD (review + pending drafts), and markdown export. It owns the comment-draft, pending-comment, and markdown-copy composables; repo/ review/auth state is injected so the Pinia stores remain the source of truth. App.vue drops ~190 lines and a batch of now-unused imports.
Move repository navigation (open repo/commit/PR, refresh, branch switch/create, repo add/remove, file selection, search-result open) into a composable. Owns creatingBranch/branchCreateError and fileElementID; store refs and the review/ selected-file/pull-request collaborators are injected. App.vue drops ~225 lines plus several now-unused imports; openRepo path stays covered by App.test.
Move the session lifecycle out of App.vue into a composable: auth bootstrap, enter-app, CLI launch-intent application, logout teardown, and the mount-time wiring of keyboard shortcuts + launch-intent subscription (owns onMounted/ onUnmounted). Collaborators are injected; the composable holds no state. App.vue is now 466 lines (from 1021) and reads as store wiring + composable composition + template. Phase C+D done: selectors, commands, review session, repo browsing, and session lifecycle are all dedicated composables. vue-tsc/ oxlint clean; full suite green (141).
…omain workspace Establish a single domain workspace (packages/domain, was packages/contracts) that holds both the shared contract DTOs (root export) and the framework- agnostic diff engine under @git-diff/domain/diff: patch parsing, context expansion, and word-level highlight ranges (moved out of ui's lib). - Renames @git-diff/contracts -> @git-diff/domain across ui, bridge, electron (single root specifier; no subpath imports existed). - Adds @git-diff/domain/diff subpath; ui imports of @lib/patch* and @lib/wordHi now resolve to the domain package. - diffSearch.ts stays in ui (it is a DOM utility, not pure domain logic). - Updates package deps, root turbo filters, Makefile format paths, README. domain + bridge build clean; ui vue-tsc/electron-tsc/oxlint clean; 141 tests green.
…postate The review package mixed git I/O with the domain model (RepositoryState, ChangedFile, DiffSection, CommitSummary, RepositoryFile, ...). Move those pure data types + status/mode constants into a dedicated internal/domain/repostate package with no I/O. The review package keeps git-adapter internals (WorkingTreeDirtyError, status/diff parsing structs) and re-exports the domain types as aliases so all existing review.* references keep compiling (Phase 1, zero call-site churn). Converts ChangedFile.pathSectionID to a free function (methods can't be defined on an aliased type). go build/vet clean; all tests green.
…state domain Consumers of the repo-state domain types now import internal/domain/repostate directly instead of the review git-adapter package. walks/* (9 files) and service/walkthrough no longer import review at all — the walkthrough generation domain is fully decoupled from git execution. httpx/walkthrough keeps review only for its git reader functions (ReadRepositoryState/ReadCommitState) and uses repostate for types. go build/vet clean; all tests green.
…tration
Move the inline 8-service Server{} composition out of bootstrap.Serve into a
newServer() factory in server.go — the composition root for the HTTP service
graph (storage repos + AI providers + user config -> application services).
Serve() now only parses flags, opens the store/socket, builds the server, and
runs it. go build/vet clean; all 13 packages' tests green.
…olve write)
- useLazyDiffFile: add { immediate: true } to the rendered watcher so the
IntersectionObserver is stopped right away for files already rendered on init
(small/non-deferred files), fixing an observer leak.
- storage.SetReviewCommentResolved: fetch the comment first and no-op when it is
already in the desired state, avoiding a redundant write + duplicate
resolved/unresolved event; update the struct in memory instead of re-reading.
go build/vet clean; all Go + UI tests green.
dist/ is tsc build output and shouldn't be tracked. ui and api already ignore it; bridge and domain had no .gitignore so their compiled output leaked into git, creating diff noise on every refactor. Resolution of these packages is 100% via dist (exports default -> dist, no src condition, no vite alias), so the committed output was load-bearing for two entry points that don't go through turbo's ^build: - pnpm dev: add dependsOn ["^build"] to the turbo dev task so the libraries build before the UI dev stack starts. - release CI: add a "Build shared TS libraries" step before the renderer build, which runs pnpm -C packages/ui run build directly (bypassing turbo ^build). pnpm build/test already build the libraries in topological order. - add packages/bridge/.gitignore and packages/domain/.gitignore (dist/) - git rm --cached the 84 previously-tracked dist files
- Replace go-fmt.compose.yaml long-lived container with ephemeral 'docker run --rm' against ghcr.io/oullin/go-fmt:v0.2.7 (full image), mirroring the ~/Sites/sasu pattern. - make format now delegates Go + TS/Vue formatting to the container (fmt-all): 'go format' for the workspace, git-selected 'ts' for changed .ts/.vue; format-all formats the whole repo. - Retire local TS formatting: delete scripts/blank-lines.ts, .oxfmtrc.json, and per-package format/format:check scripts; drop oxfmt + oxc-parser deps. oxlint/pnpm lint kept (go-fmt does not lint). - bridge/domain build scripts no longer run 'pnpm run format' before tsc. - Adopt go-fmt's canonical oxfmt default style (2-space, no local config).
Whole-repo reformat produced by 'make format-all' with go-fmt v0.2.7: oxfmt default 2-space indentation + blank-line rules for .ts/.vue, and gofmt/goimports for Go. No logic changes.
…forcement Convert domain's relative imports (./, ../) to the #domain/* Node subpath alias, mirroring bridge's #bridge/* pattern. Declare the alias in domain/package.json imports (resolvePackageJsonImports is already on). Consolidate the no-relative-imports check into one generic, argv-driven script at the repo root and wire it into each TS package's build so every package self-enforces: domain and bridge now run check:imports before tsc, and ui points its existing check at the shared script.
Add .oxfmtrc.json copied verbatim from go-fmt v0.2.7 (useTabs, tabWidth 4, singleQuote, trailingComma all, printWidth 200, semi, arrowParens always) so the containerized oxfmt formats with go-fmt's canonical style instead of oxfmt's built-in defaults. Reflow all TS/Vue to tabs accordingly.
…files Route/endpoint strings were duplicated as raw literals with no single source of truth. Extract one routes file per layer, living next to each router: - packages/ui/electron/ipc/routes.ts — IpcChannels, nested by domain. Handlers (*.ipc.ts) and the renderer bridge (preload.ts) now reference the constants instead of re-typing channel strings. - packages/bridge/src/routes.ts — HttpRoutes for the /v1/* paths, consumed by the 11 bridge clients (static strings + builders for parameterized paths). - packages/api/internal/app/httpx/routes.go — path constants combined with the HTTP method inline in router.go. Pure extraction: every literal preserved byte-for-byte, no route renamed. The three files are independent (no cross-language sharing).
No description provided.