Skip to content

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
nkzw-tech:mainfrom
xortive:codiff-phase-1-rewrite
Open

Refactor: one ReviewSurface UI for all reviews across desktop and codiff-web, with shared review loading infrastructure#167
xortive wants to merge 17 commits into
nkzw-tech:mainfrom
xortive:codiff-phase-1-rewrite

Conversation

@xortive

@xortive xortive commented Sep 4, 2026

Copy link
Copy Markdown

Refactor: one ReviewSurface UI for all reviews across desktop and codiff-web, with shared review loading infrastructure

The Electron app and internal codiff-web don'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:

  • commit/range review
  • per-commit walkthroughs
  • version comparison walkthroughs

such that these features behave consistently across various review types within codiff-web and the desktop Electron app.

As part of this refactor, a few new features were added:

  • Improved comment section in reviews
  • Expansion of collapsed code regions in reviews
  • shift-ctrl-p menu is available always
  • code search filtered by additions, deletions, and unchanged context that is already visible or explicitly expanded
  • model runs for walkthrough generation are run via a scheduler that can execute multiple model invocations and show individual progress

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 unavailable when neither exists; an exact +0 / −0 remains 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 accepted ReviewSource values from loaded ResolvedReviewSource state. Introduce GitSha, provider-neutral Revision and DiffRange, optional DiffSection.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 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.

3. rmxmnnlw — Define immutable review artifacts for ranges, commits, and blobs. 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.

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 from source.type. Let the host select local notes, provider comments, share comments, persisted-comment callbacks, replies, review sessions, navigation, and exact-content resolution. Rename SharedWalkthroughApp.tsx to ReviewSurface.tsx, forward the source-neutral resolver to ReviewCodeView, 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 from core/App.tsx into RepositoryReviewHost, which mounts ReviewSurface for 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 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.

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 under source-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-owned DiffRange content 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. 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.

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. Retain electron/main.cjs, dist/index.html, and generated provider runtimes required by the packaged application.

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
xortive force-pushed the codiff-phase-1-rewrite branch from c1ad068 to 099bb37 Compare September 4, 2026 18:59
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