refactor: clean up diff UI arguments and test fixtures

CONTRIBUTING.md

@@ -62,7 +62,7 @@ Summary:
 - **Swift unit tests** — cover ViewModel-level behavior in `shell/mac/Tests/JayJayTests/`. Run with `just test-app`.
 - **XCUITest scenes** — cover user-visible flows in `shell/mac/Tests/JayJayUITests/`. Run with `just test-ui`.
 
-UI tests launch the app against deterministic fixtures at `/tmp/jayjay-test-fixtures/{simple,conflict}` built by `just shell::ui-test-setup`. Each scene subclasses `SceneBase` and asserts against accessibility identifiers declared in `shell/mac/Sources/JayJay/Shared/AccessibilityIdentifiers.swift`. When adding a new user-visible view or interaction:
+UI tests launch the app against deterministic fixtures under `/tmp/jayjay-test-fixtures` built by `just shell::ui-test-setup`. Canonical fixtures cover simple, complex, structured-format, review-note, bookmark-diff, and conflict workflows. Repository-mutating scenes use workflow-named copies generated from those canonical fixtures so test order cannot leak state. Each scene subclasses `SceneBase` and asserts against accessibility identifiers declared in `shell/mac/Sources/JayJay/Shared/AccessibilityIdentifiers.swift`. When adding a new user-visible view or interaction:
 
 1. Attach a stable `.accessibilityIdentifier(...)` to the view, keyed by the data that makes it unique (change-id prefix, file path, etc.). Add a constant/function to `AID` so tests and views share the same string.
 2. Write a scene test under `Tests/JayJayUITests/Scenes/` that exercises the flow end-to-end.

agents/testing.md

@@ -22,11 +22,13 @@ Load this file before adding fixtures, reorganizing tests, or changing UI test b
 
 UI tests live in `shell/mac/Tests/JayJayUITests/`.
 
-Each `SceneBase` subclass launches the app against a named fixture. The default fixture is `simple`; override `fixtureName` when a scene needs a different repo state.
+Each `SceneBase` subclass launches a named fixture. The default fixture is `simple`; override `fixtureName` when a scene needs `complex`, `formats`, `review-notes`, `bookmark-diff`, or `conflict`.
+
+Use `complex` when the workflow depends on scale or mixed diff shapes. It has more than 30 changed paths and 1,000 changed lines across additions, rewrites, deletions, renames, binary content, deep paths, and a path containing spaces. Keep narrow scenes on purpose-built fixtures so their assertions stay legible.
 
 Use accessibility identifiers from `shell/mac/Sources/JayJay/Shared/AccessibilityIdentifiers.swift`. Add identifiers at the view body and key them by stable data such as change-id prefix or file path.
 
-If a scene mutates repo state, give it its own fixture. Tests share a filesystem and run alphabetically, so mutations on `simple` leak into later tests. `ui-test-setup` already creates `simple-newchange` for `NewChangeScene`; add sibling copies for new mutating scenes.
+The sandboxed XCUITest runner cannot create repositories where the launched app can open them. Mutating scenes therefore use dedicated copies generated from a canonical fixture by `ui-test-fixtures.sh`; name those copies for the workflow, not for their source fixture. Each scene gets an isolated review store.
 
 ## GPUI Tests
 

scripts/ui-test-fixtures.sh

@@ -21,15 +21,13 @@ setup_defaults() {
   defaults write "$bundle_id" jayjay.sideBySideDiff -bool NO
   defaults write "$bundle_id" jayjay.ignoreWhitespace -bool NO
   defaults write "$bundle_id" jayjay.treeFileList -bool NO
-  defaults write "$bundle_id" jayjay.recentRepos -array "$fixtures/simple-formats"
+  defaults write "$bundle_id" jayjay.recentRepos -array "$fixtures/formats"
   defaults delete "$bundle_id" jayjay.lastOpenedRepo 2>/dev/null || true
   # Start each run with the command palette at its default (centered) position.
   defaults delete "$bundle_id" commandPalette.frameOrigin 2>/dev/null || true
 }
 
 # Simple: three commits + an active working copy with two new files.
-# Clone once, then copy per-class for tests that mutate repo state so leaks
-# don't bleed across scenes (NewChangeScene creates a new @).
 fixture_simple() {
   jj git init --colocate "$fixtures/simple"
   (
@@ -45,24 +43,35 @@ fixture_simple() {
     echo "wip 1" > wip1.txt
     echo "wip 2" > wip2.txt
   )
-  cp -R "$fixtures/simple" "$fixtures/simple-newchange"
-  cp -R "$fixtures/simple" "$fixtures/simple-commit"
-  cp -R "$fixtures/simple" "$fixtures/simple-editdesc"
-  cp -R "$fixtures/simple" "$fixtures/simple-save-description"
-  cp -R "$fixtures/simple" "$fixtures/simple-diffstats"
-  cp -R "$fixtures/simple" "$fixtures/simple-bookmark-diff"
-  cp -R "$fixtures/simple" "$fixtures/simple-review-notes"
-  cp -R "$fixtures/simple" "$fixtures/simple-formats"
+}
+
+copy_fixture() {
+  local source="$1"
+  local destination="$2"
+  cp -R "$fixtures/$source" "$fixtures/$destination"
+}
+
+fixture_mutating_scenes() {
+  copy_fixture simple commit
+  copy_fixture simple edit-description
+  copy_fixture simple save-description
+  copy_fixture simple diff-stats
+  copy_fixture simple new-change
+}
+
+fixture_bookmark_diff() {
+  copy_fixture simple bookmark-diff
   (
-    cd "$fixtures/simple-bookmark-diff"
+    cd "$fixtures/bookmark-diff"
     jj bookmark create bookmark-diff -r @
   )
 }
 
 # Simple plus structured files for projection/rendering checks.
 fixture_formats() {
+  copy_fixture simple formats
   (
-    cd "$fixtures/simple-formats"
+    cd "$fixtures/formats"
     cp "$format_fixtures/analysis.ipynb" analysis.ipynb
     cp "$format_fixtures/notes.md" notes.md
     cp "$format_fixtures/release.html" release.html
@@ -76,8 +85,9 @@ fixture_formats() {
 
 # A long function so ReviewNotesScene can verify the diff expands around embedded note rows.
 fixture_review_notes() {
+  copy_fixture simple review-notes
   (
-    cd "$fixtures/simple-review-notes"
+    cd "$fixtures/review-notes"
     printf '%s\n' \
       'func fibonacciReport(limit: Int) -> String {' \
       '    var values: [Int] = []' \
@@ -107,6 +117,93 @@ fixture_review_notes() {
   )
 }
 
+# Complex: a broad working-copy diff with enough files and changed lines to exercise Diff Edit's large-repository policy.
+fixture_complex() {
+  jj git init --colocate "$fixtures/complex"
+  (
+    cd "$fixtures/complex"
+    mkdir -p assets config/environments "docs/product specs" docs/guides src/modules
+
+    for module_number in {1..12}; do
+      printf -v module '%02d' "$module_number"
+      {
+        printf 'public enum Module%s {\n' "$module"
+        for line in {1..36}; do
+          printf '    public static let value%s = "module-%s-base-%s"\n' "$line" "$module" "$line"
+        done
+        printf '}\n'
+      } > "src/modules/module-$module.swift"
+    done
+
+    for guide_number in {1..10}; do
+      printf -v guide '%02d' "$guide_number"
+      {
+        printf '# Guide %s\n\n' "$guide"
+        for line in {1..24}; do
+          printf 'Baseline guide %s paragraph %s with stable documentation context.\n' "$guide" "$line"
+        done
+      } > "docs/guides/guide-$guide.md"
+    done
+
+    for environment in development staging production test; do
+      printf '{\n  "environment": "%s",\n  "featureFlags": ["search", "sync", "review"]\n}\n' "$environment" > "config/environments/$environment.json"
+    done
+
+    printf '# API overview\n\nBaseline API contract.\n' > "docs/product specs/api overview.md"
+    printf '\x00\x01jayjay-baseline-binary\x00' > assets/logo.bin
+
+    jj describe -m "complex baseline"
+    jj bookmark create main -r @
+    jj new
+
+    for module_number in {1..8}; do
+      printf -v module '%02d' "$module_number"
+      {
+        printf 'public enum Module%s {\n' "$module"
+        for line in {1..48}; do
+          printf '    public static let revisedValue%s = "module-%s-working-copy-%s"\n' "$line" "$module" "$line"
+        done
+        printf '}\n'
+      } > "src/modules/module-$module.swift"
+    done
+
+    printf '{\n  "environment": "development",\n  "featureFlags": ["search", "sync", "review", "debug-menu"],\n  "apiBaseURL": "https://dev.example.test"\n}\n' > config/environments/development.json
+    printf '{\n  "environment": "staging",\n  "featureFlags": ["search", "sync", "review", "audit-log"],\n  "apiBaseURL": "https://staging.example.test"\n}\n' > config/environments/staging.json
+
+    for guide_number in {1..4}; do
+      printf -v guide '%02d' "$guide_number"
+      rm "docs/guides/guide-$guide.md"
+    done
+    mkdir -p docs/reference
+    mv docs/guides/guide-05.md docs/reference/guide-05-renamed.md
+    mv docs/guides/guide-06.md docs/reference/guide-06-renamed.md
+
+    for feature_number in {1..24}; do
+      printf -v feature '%02d' "$feature_number"
+      mkdir -p "src/features/area-$feature"
+      {
+        printf 'public struct Feature%sComponent {\n' "$feature"
+        for line in {1..34}; do
+          printf '    public let field%s = "feature-%s-value-%s"\n' "$line" "$feature" "$line"
+        done
+        printf '}\n'
+      } > "src/features/area-$feature/component.swift"
+    done
+
+    mkdir -p src/generated/tables
+    {
+      printf 'public let generatedRows = [\n'
+      for ((line = 1; line <= 420; line++)); do
+        printf '    "generated-row-%03d",\n' "$line"
+      done
+      printf ']\n'
+    } > src/generated/tables/large_table.swift
+
+    printf '# API overview\n\nWorking-copy API contract with authentication, pagination, retries, and structured errors.\n' > "docs/product specs/api overview.md"
+    printf '\x00\x02jayjay-working-copy-binary\x00' > assets/logo.bin
+  )
+}
+
 # Conflict: @ is a rebased change with one file containing multiple conflicts.
 fixture_conflict() {
   jj git init --colocate "$fixtures/conflict"
@@ -122,12 +219,12 @@ fixture_conflict() {
     write_conflict_file feature
     jj rebase -r @ -d main
   )
-  cp -R "$fixtures/conflict" "$fixtures/conflict-use-ours"
+  copy_fixture conflict conflict-use-ours
 }
 
 fixture_repository_stores() {
   printf '{"repositories":[]}\n' > "$fixtures/repositories-empty.json"
-  printf '{"repositories":["%s"]}\n' "$fixtures/simple-formats" > "$fixtures/repositories-pinned.json"
+  printf '{"repositories":["%s"]}\n' "$fixtures/formats" > "$fixtures/repositories-pinned.json"
 }
 
 # Three sections editing the same keys so every side of the rebase collides.
@@ -154,7 +251,10 @@ setup_defaults
 rm -rf "$fixtures"
 mkdir -p "$fixtures"
 fixture_simple
+fixture_mutating_scenes
+fixture_bookmark_diff
 fixture_formats
 fixture_review_notes
+fixture_complex
 fixture_conflict
 fixture_repository_stores

shell/gpui/src/diff/diff_view/render.rs

@@ -10,10 +10,10 @@ use super::header::{
 };
 use super::note_banner::stale_notes_banner;
 use super::placeholders::{placeholder, placeholder_card, placeholder_inner};
-use super::sbs_body::side_by_side_body;
+use super::sbs_body::{SideBySideBodyState, side_by_side_body};
 use super::sbs_note_banner::with_sbs_note_banner;
 use super::state::{DetailMode, DiffViewMode, DiffViewState, FindState};
-use super::unified_body::unified_body;
+use super::unified_body::{UnifiedBodyState, unified_body};
 use crate::app::theme::{Theme, theme, with_alpha};
 use crate::diff::image_diff::{hunk_is_image, image_diff_view};
 use crate::diff::markdown_diff::markdown_diff_view;
@@ -153,24 +153,28 @@ pub fn diff_view(
                     .into_any_element()
             }
             (Some(fd), DiffViewMode::Unified) => unified_body(
-                fd,
-                t.clone(),
-                query.clone(),
-                scroll.clone(),
-                state.unified_bounds.clone(),
-                &state.wrap_cache,
-                state.notes,
+                UnifiedBodyState {
+                    file_diff: fd,
+                    theme: t.clone(),
+                    query: query.clone(),
+                    scroll: scroll.clone(),
+                    bounds: state.unified_bounds.clone(),
+                    wrap_cache: &state.wrap_cache,
+                    notes: state.notes,
+                },
                 cx,
             ),
             (Some(fd), DiffViewMode::SideBySide) => {
                 let sbs = side_by_side_body(
-                    fd,
-                    t.clone(),
-                    query.clone(),
-                    scroll.clone(),
-                    state.sbs_old_bounds.clone(),
-                    state.sbs_new_bounds.clone(),
-                    &state.wrap_cache,
+                    SideBySideBodyState {
+                        file_diff: fd,
+                        theme: t.clone(),
+                        query: query.clone(),
+                        scroll: scroll.clone(),
+                        old_bounds: state.sbs_old_bounds.clone(),
+                        new_bounds: state.sbs_new_bounds.clone(),
+                        wrap_cache: &state.wrap_cache,
+                    },
                     cx,
                 );
                 with_sbs_note_banner(sbs, state.notes, &t, cx)

shell/gpui/src/diff/diff_view/sbs_body.rs

@@ -19,23 +19,37 @@ use crate::repo::window::{DiffWrapCacheSlot, PanelBoundsSlot, RepoWindow};
 use crate::ui::primitives::no_scrollbar_gutter;
 use crate::ui::scrollbar::vertical_uniform_scrollbar;
 
-#[allow(clippy::too_many_arguments)]
+pub(super) struct SideBySideBodyState<'a> {
+    pub(super) file_diff: &'a FileDiff,
+    pub(super) theme: Theme,
+    pub(super) query: Option<String>,
+    pub(super) scroll: UniformListScrollHandle,
+    pub(super) old_bounds: PanelBoundsSlot,
+    pub(super) new_bounds: PanelBoundsSlot,
+    pub(super) wrap_cache: &'a DiffWrapCacheSlot,
+}
+
 pub(super) fn side_by_side_body(
-    fd: &FileDiff,
-    theme: Theme,
-    query: Option<String>,
-    scroll: UniformListScrollHandle,
-    old_bounds: PanelBoundsSlot,
-    new_bounds: PanelBoundsSlot,
-    wrap_cache: &DiffWrapCacheSlot,
+    state: SideBySideBodyState<'_>,
     cx: &mut Context<RepoWindow>,
 ) -> AnyElement {
+    let SideBySideBodyState {
+        file_diff,
+        theme,
+        query,
+        scroll,
+        old_bounds,
+        new_bounds,
+        wrap_cache,
+    } = state;
     let theme = Arc::new(theme);
     let query = Arc::new(query);
     let advance = fonts::mono_advance(cx, px(12.));
     let old_cols = wrap_cols_from_bounds(old_bounds.get(), advance);
     let new_cols = wrap_cols_from_bounds(new_bounds.get(), advance);
-    let rows = wrap_cache.borrow_mut().side_by_side(fd, old_cols, new_cols);
+    let rows = wrap_cache
+        .borrow_mut()
+        .side_by_side(file_diff, old_cols, new_cols);
     let count = rows.len();
 
     let old_gutter = {
@@ -68,7 +82,7 @@ pub(super) fn side_by_side_body(
     };
 
     let old_content = sbs_content_list(
-        SbsContentArgs {
+        SbsContentState {
             id: "sbs-old-content",
             count,
             rows: rows.clone(),
@@ -82,7 +96,7 @@ pub(super) fn side_by_side_body(
         cx,
     );
     let new_content = sbs_content_list(
-        SbsContentArgs {
+        SbsContentState {
             id: "sbs-new-content",
             count,
             rows,
@@ -165,7 +179,7 @@ pub(super) fn side_by_side_body(
         .into_any_element()
 }
 
-struct SbsContentArgs {
+struct SbsContentState {
     id: &'static str,
     count: usize,
     rows: Arc<Vec<WrappedSbsRow>>,
@@ -177,8 +191,8 @@ struct SbsContentArgs {
     advance: Pixels,
 }
 
-fn sbs_content_list(args: SbsContentArgs, cx: &mut Context<RepoWindow>) -> UniformList {
-    let SbsContentArgs {
+fn sbs_content_list(state: SbsContentState, cx: &mut Context<RepoWindow>) -> UniformList {
+    let SbsContentState {
         id,
         count,
         rows,
@@ -188,7 +202,7 @@ fn sbs_content_list(args: SbsContentArgs, cx: &mut Context<RepoWindow>) -> Unifo
         side,
         bounds,
         advance,
-    } = args;
+    } = state;
     uniform_list(
         id,
         count,