Refactor: one ReviewSurface UI for all reviews across desktop and codiff-web, with shared review loading infrastructure - #167
Open
xortive wants to merge 17 commits into
Open
Conversation
Separate accepted review inputs from loaded sources and make commit, index, and working-copy revisions explicit, including nullable absent sides and conflict stages. Share source parsing, provider URL canonicalization, and logical/exact identity formatting across Core and Electron, while selecting generated-file attributes from the effective revision.
Move unchanged definitions from `core/types.ts` into `review-identity`, `review-comments`, `review-history`, `walkthrough`, and `generation`. Keep `core/types.ts` as the compatibility re-export, publish `./types`, and make no runtime behavior change.
Add provider-neutral `ReviewArtifactSource`, `RangeArtifact`, `StackSnapshot`, `CommitArtifact`, and `BlobArtifact` types with provenance and explicit complete, opaque, or truncated coverage. Separate requested range endpoints from the effective range returned by a provider; key each commit request and result by commit SHA plus selected parent, using `commitSha:root` for a root commit; validate stack order and endpoint consistency; and deduplicate or cancel reads within one artifact run.
Implement provider-neutral range, stack, selected-parent commit, and blob artifact reads in the GitHub and GitLab packages, with Electron retained as the CLI transport. Convert provider ranges into exact DiffRanges and adapt commit content to one native-first contract that falls back only for missing objects, rejects moved heads, and preserves provider runtime packaging.
Stop deriving host behavior from source type and let each host select local, provider, share, reply, session, navigation, and exact-content capabilities. Rename the common surface, forward one source-neutral content resolver through the code view, and render the changed-file filter only in Tree.
Extract desktop review orchestration into RepositoryReviewHost and mount the common surface for local and provider sources. Keep comment capabilities, drafts, History sessions, request cancellation, source generations, and current-state guards at the host boundary. Refresh against canonical reviewed-code identity so metadata, revision-only, ordering, and hydration changes preserve active content requests and walkthrough work. When code changes, retain the visible walkthrough, reject any old-code result without cancelling its generation, and require an explicit restart before replacing it.
Hand the first `git status` result that painted the working tree to the repository watcher. Parse porcelain-v2 `u` records as `conflicted` with their stage information instead of applying single-letter status normalization, and cover the unmerged combinations directly.
Paint provider files first, then defer History, comments, walkthrough work, Git identity, and automatic exact-content hydration. Run hydration as one bounded batch with one state merge and terminal retry behavior, and include the observed provider head in exact identity so force-pushed results cannot land.
Queue generation units, run up to three at once by default, report progress for each unit, preserve successful units when another unit fails, and reuse a unit only when its reviewed diff, prompt, and model settings still match. Phase 1 schedules the current narrative as one unit; later phases can schedule per-commit and comparison units. Switching History sources cancels work for the previous source, and Core exports the scheduler and progress APIs for other hosts.
Attach commit-to-commit, commit-to-index, and index-to-working-copy ranges to local sections, including nullable absent sides and conflict-stage index coordinates. Separate local, provider, and share submissions; require two commit endpoints for provider writes; persist share positions; and keep outdated provider threads at their original locations.
…t range Resolve provider targets against the exact range before writing: reject working-copy/index markers, missing files or lines, and a moved GitHub head. Create and then publish GitLab draft notes, block retry when an accepted draft lacks a discussion id, wait for that id before replying, and expose approve/comment/request-changes through the provider session.
Build the commit or PR/MR title, body, author, and editability once for Tree, Walkthrough, and Comments. Store collapse state under `source-description:${provider}:${url}:${headSha}` so mode switches preserve it and a new PR head starts expanded.
Search additions, deletions, and visible unchanged context independently, with additions and deletions enabled by default. Restrict unchanged search to context already displayed in the hunk rather than hidden full-file contents loaded for rendering.
Dispatch commit, index, and working-copy bytes through one host-owned DiffRange content run. Eagerly hydrate loadable provider text before the first review render, and share the native-first, provider-fallback reader and cache with explicit loads, images, comment navigation, and context expansion while preserving cancellation, stale-head rejection, rename semantics, and exact added or deleted reconstruction.
Load overview and inline provider threads together, normalize every reply from its root, and preserve immutable old/new, ranged, file, rename, and outdated coordinates. Render resolvable threads against their exact Pierre code regions, including historical revision content, while retaining full comment bodies, authors, permalinks, and actions in the missing-region fallback when content or coordinates are unavailable.
Record `git`, `gh`, and `glab` child processes against the window action that started them, use `initial-load` for startup, redact tokens from argv, and preserve cancellation for shared processes. Have `scripts/check-startup-trace.mjs` verify first-usable and deferred-completion ordering and budget.
Ignore `.jj/**` in Vite and exclude repository metadata, evals, test scenarios, Electron tests, service/web sources, scripts, and selected docs from Forge packages. Retain `electron/main.cjs`, `dist/index.html`, and generated provider runtimes required by the packaged application.
xortive
force-pushed
the
codiff-phase-1-rewrite
branch
from
September 4, 2026 18:59
c1ad068 to
099bb37
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refactor: one
ReviewSurfaceUI for all reviews across desktop andcodiff-web, with shared review loading infrastructureThe Electron app and internal
codiff-webdon't share MR/PR loading, commenting, and walkthrough generation paths. Within the electron app,MR/PR reviews have different UI compared to local git diff reviews.
This MR consolidates all reviews into a single UI, with abstracted interfaces for providing review data, such that review providers can indicate what
capabilities they offer, like commenting, versions, etc.
This is in preparation for a further MR to add:
such that these features behave consistently across various review types within
codiff-weband the desktop Electron app.As part of this refactor, a few new features were added:
Provider reviews prioritize correctness over initial paint time. Codiff loads exact base and head text for every loadable text file before exposing the first usable review, using local Git first and provider blob endpoints when an object is missing. Provider patches remain the fallback for binary or unavailable files. Walkthrough diffstats use materialized content first, provider counts second, and display
Diff unavailablewhen neither exists; an exact+0 / −0remains valid.This MR was written almost entirely with AI assistance, primarily with the GPT-5.6 model family.
I have separated the changes into logical commits, and gone through several iterations of review using other models and different review techniques (prioritizing bugfixes vs fixing flawed abstractions), and iteratively testing using the Desktop app to see if functionality is maintained.
Commit summaries below are AI-authored. My fork has the full stacked diff of MRs open, if you want to clone and play with the later MRs that add the mentioned features above. These have not been as carefully reviewed as the Phase 1 stack, but Phase 4 can be cloned and successfully
runs commit/range tree reviews, per commit walkthroughs, and version comparison walkthroughs. They still have poor AI-written MR descriptions, but commit messages have more detail.
Commit stack
1.
xslnvoqt— Store resolved commit SHAs separately from branch names and PR numbers. Separate acceptedReviewSourcevalues from loadedResolvedReviewSourcestate. IntroduceGitSha, provider-neutralRevisionandDiffRange, optionalDiffSection.range, nullable absent-side endpoints, and conflict-stage index revisions. Share one runtime codec for source parsing, provider URL canonicalization, reload selection, Electron window identity, and logical/exact source keys. Select generated-file attributes from the effective revision: pinned commits, cached index, or live working copy.2.
pmplmnqo— Move existing Core types into domain files. Move unchanged definitions fromcore/types.tsintoreview-identity,review-comments,review-history,walkthrough, andgeneration. Keepcore/types.tsas the compatibility re-export, publish./types, and make no runtime behavior change.3.
rmxmnnlw— Define immutable review artifacts for ranges, commits, and blobs. Add provider-neutralReviewArtifactSource,RangeArtifact,StackSnapshot,CommitArtifact, andBlobArtifacttypes with provenance and explicit complete, opaque, or truncated coverage. Separate requested range endpoints from the effective range returned by a provider; key each commit request and result by commit SHA plus selected parent, usingcommitSha:rootfor a root commit; validate stack order and endpoint consistency; and deduplicate or cancel reads within one artifact run.4.
tslprqku— Resolve current GitHub and GitLab reviews into shared artifacts. Implement provider-neutral range, stack, selected-parent commit, and blob reads, with Electron retained as the CLI transport. Convert textual provider patches into ready sections, preserve GitHub additions/deletions when a patch is omitted, and keep known GitHub patchless or GitLab collapsed/large text files recoverable through their exact revision coordinates. Adapt exact commit content to one native-first contract that falls back to the selected provider only for missing objects and rejects reads after the observed provider head moves.5.
ykvruzzk— Pass comment and note permissions into ReviewSurface. Stop deriving host behavior fromsource.type. Let the host select local notes, provider comments, share comments, persisted-comment callbacks, replies, review sessions, navigation, and exact-content resolution. RenameSharedWalkthroughApp.tsxtoReviewSurface.tsx, forward the source-neutral resolver toReviewCodeView, and keep static snapshots usable without one. Render the file filter only in Tree.6.
tyomvtmr— Move desktop reviews onto ReviewSurface. Extract desktop review orchestration fromcore/App.tsxintoRepositoryReviewHost, which mountsReviewSurfacefor local and provider reviews. Create one content run per root, exact source identity, and file-fingerprint generation; route section, image, search, and context requests through it; abort replaced runs; and guard late results against the active state. Preserve consumer-specific React replacement and Pierre in-place hydration at the host/surface boundary.7.
uxrlmrmo— Seed repository watchers from initial Git status. Hand the firstgit statusresult that painted the working tree to the repository watcher. Parse porcelain-v2urecords asconflictedwith their stage information instead of applying single-letter status normalization, and cover the unmerged combinations directly.8.
opruovkv— Show the first repository review before History and comments finish. Separate provider-file startup from History, comments, walkthrough work, and Git identity, and establish one bounded exact-content hydration batch with stable source identity and stale-result cancellation. The later shared-content revision moves completion of that batch ahead of the first usable render while retaining the deferred work split.9.
rmspzktl— Introduce a walkthrough generation scheduler for batches of model work. Queue generation units, run up to three at once by default, report progress for each unit, preserve successful units when another unit fails, and reuse a unit only when its reviewed diff, prompt, and model settings still match. Phase 1 schedules the current narrative as one unit; later phases can schedule per-commit and comparison units. Switching History sources cancels work for the previous source, and Core exports the scheduler and progress APIs for other hosts.10.
zvkzsssq— Carry exact review ranges through local diffs and comment positions. Attach commit-to-commit ranges to local commits/ranges, commit-to-index ranges to staged sections, and index-to-working-copy ranges to unstaged sections. Represent an absent side with a null endpoint, including staged additions in an unborn repository, and retain conflict-stage index coordinates. Use those ranges for content and generated-attribute reads while requiring two commit endpoints for provider submissions. Keep local notes, provider drafts, share drafts, and scroll-only section ids distinct.11.
qwlvyrvz— Reject GitHub and GitLab comments that do not match the current commit range. Resolve provider targets against the exact range before writing: reject working-copy/index markers, missing files or lines, and a moved GitHub head. Create and then publish GitLab draft notes, block retry when an accepted draft lacks a discussion id, wait for that id before replying, and expose approve/comment/request-changes through the provider session.12.
oszslyov— Share one PR description header across Tree, Walkthrough, and Comments. Build the commit or PR/MR title, body, author, and editability once for Tree, Walkthrough, and Comments. Store collapse state undersource-description:${provider}:${url}:${headSha}so mode switches preserve it and a new PR head starts expanded.13.
nukzvwmz— Filter diff search by line type. Search additions, deletions, and visible unchanged context independently, with additions and deletions enabled by default. Restrict unchanged search to context already displayed in the hunk rather than hidden full-file contents loaded for rendering.14.
lptlvvpu— Load exact review content by revision. Dispatch commit, index, and working-copy bytes through one host-ownedDiffRangecontent run. Eagerly hydrate every loadable provider text section before the first usable review render, using native Git first and the selected provider as a missing-object fallback. Reuse the same cache for explicit loads, images, comment navigation, and context expansion; preserve cancellation, stale-head rejection, binary and rename semantics, and exact added/deleted reconstruction with an empty absent side. Keep search limited to provider hunks and context the user has already expanded. Resolve walkthrough rows from exact content first, provider counts second, and otherwise mark the diff unavailable.15.
mtzzltrp— Present review comments in their anchored code regions. Load overview and inline provider threads together, normalize replies from their roots, and preserve immutable old/new, ranged, file, rename, and outdated coordinates. Render resolvable threads against their exact code regions, loading only the selected comment’s file and recorded revision range when historical context is required. Keep full bodies, authors, permalinks, and actions in the missing-region fallback when content or coordinates are unavailable.16.
vstxrvlm— Attribute startup Git and gh/glab calls to the window that started them. Recordgit,gh, andglabchild processes against the window action that started them, useinitial-loadfor startup, redact tokens from argv, and preserve cancellation for shared processes. Havescripts/check-startup-trace.mjsverify first-usable and deferred-completion ordering and budget.17.
nxmrkyrk— Keep .jj, evals, and tests out of packaged apps. Ignore.jj/**in Vite and exclude repository metadata, evals, test scenarios, Electron tests, service/web sources, scripts, and selected docs from Forge packages. Retainelectron/main.cjs,dist/index.html, and generated provider runtimes required by the packaged application.