signed/docs/repo-view-refactor-plan.md

# Repo view refactor plan

## Goal

Make `crates/workspace/src/views/repo/` easy to navigate and change:

- Each concern owns its own state (its own struct fields), instead of all concerns sharing one 38-field struct.
- Shared UI moves to the module that consumes it, so sibling views stop importing from `views::repo`.
- No behavior change. No new global state. No new store. The GPUI patterns already used in the repo (`Entity` + observe, `TreeState`, `ComboboxState`, `VirtualListScrollHandle`) stay the only patterns used.

## Constraints

- Follow `.rules`: no `unwrap` in production, no silently discarded errors, full-word names, comments explain "why" only.
- Do not over-engineer. Files and History become entities because they already render and run async independently. Refs and Banners stay plain field groups on the shell.
- Keep the shell as the single load/reconcile point. The clone/worktree and `ref_generation` belong to the shell, not to child views.
- `signed_ui` does **not** depend on `signed_git` (verified in `crates/signed_ui/Cargo.toml`). Anything that takes a `signed_git` type cannot move there.

## Current state (verified)

| File | Lines | Content |
|---|---|---|
| `mod.rs` | ~376 | `RepoDetailView` struct (38 fields), constructors, `render`, panel impls, `display_name` |
| `store.rs` | ~98 | `attach_store`, `apply_announcement`, `refresh_ready_statuses`, `refresh_statuses` |
| `actions.rs` | ~208 | action methods + `open_repo_panel` / `open_repo_item` free functions |
| `loading.rs` | ~435 | `load_repo`, `apply_repo_data`, `sync_ref_selector`, `clone_to_folder`, `load_repo_data` |
| `refs.rs` | ~279 | `switch_ref`, `restore_selection`, `reload_worktree`, `catch_up_worktree` |
| `files.rs` | ~480 | file tree, previews, markdown/code state, eviction |
| `history.rs` | ~228 | commits tab render + per-file and full commit walks |
| `header.rs` | ~715 | header render, maintainers, fork row, clone URL |
| `banners.rs` | ~315 | ready/push suggestion banners |
| `about.rs` | ~224 | about dialog |
| `init_dialog.rs` | ~202 | publish-to-NIP-34 dialog |
| `helpers.rs` | ~722 | `pub(crate)` grab bag: tree building, diff rendering, discussion UI, share targets, commit rows |

### Problems

1. **`RepoDetailView` is a god object.** 38 fields across six concerns. All 12 files are `impl RepoDetailView`, so any file can read/write any field. The file split added navigation cost without encapsulation.
2. **`helpers.rs` is an inverted dependency hub.** `views/issues/detail.rs` and `views/pull_requests/*.rs` import from `views::repo::helpers` for discussion UI, diff rows and commit rows. Sibling views reaching into `repo` is backwards.
3. **Two regions have independent async + render lifecycles** (file browser, commit history) but live as shell fields, sharing `worktree` and `ref_generation` by hand.

### Existing good pattern

`IssuesView` (`views/issues/mod.rs`): own struct (~13 fields), `cx.observe(&store, ..)`, `rebuild()` into local state, `Render`, no shell fields. The refactor brings `RepoDetailView` in line with this.

## Target structure

```mermaid
graph TD
    Shell["RepoDetailView shell\nstore, dock_area, tabs, header,\nload orchestration, worktree, generation"] --> Files["Entity<RepoFilesView>\nfiles.rs"]
    Shell --> History["Entity<RepoHistoryView>\nhistory.rs"]
    Shell --> Refs["RefSwitcher (plain)\nrefs.rs"]
    Shell --> Banners["Banners (plain)\nbanners.rs"]
    Files --> Store["Entity<RepoStore>"]
    History --> Store
```

Field ownership after the refactor:

| Concern | Fields | Owner |
|---|---|---|
| Files | `tree_state, worktree_paths, md, code, readme_name, selected_file, files, file_order, preview_bytes, loading_files, commits, pending_commits, loading_commits` | `RepoFilesView` |
| History | `all_commits, loading_all_commits, item_sizes, scroll_handle` | `RepoHistoryView` |
| Refs | `branch_select, tag_select, ref_branches, ref_tags, switching_ref` | `RefSwitcher` |
| Banners | `banner_dismissed, ready_requested, ready_head, ready_statuses, push_statuses` | `Banners` |
| Shell | `focus_handle, dock_area, store, repo_started, active_tab, loading, error, head_commit, worktree, ref_generation, _subscriptions` | `RepoDetailView` (11 fields) |

Shared modules after Phase 1:

| New / changed module | Contents | Consumers |
|---|---|---|
| `views/tree.rs` | `TreeItemSeed`, `tree_items`, `build_tree_items`, `sorted_worktree_paths` + the 3 tree tests | repo files/loading, commit_diff |
| `views/commit_diff/mod.rs` | adds `DiffRow`, `diff_rows`, `render_diff_row`, `render_diff_line`, `find_item`, `GUTTER_WIDTH`, `DIFF_ROW_HEIGHT`, `commit_row`, `COMMIT_ROW_HEIGHT` | commit_diff, PR new, repo history |
| `views/discussion.rs` | `sidebar_title`, `sidebar_section`, `comments_section`, `comment_form`, `issue_roots`, `pr_roots` | issues detail, PR detail |
| `signed_ui/src/ref_selector.rs` | `ref_selector_trigger` | repo header, PR new |
| `repo/files.rs` | `code_language`, `is_markdown_path` (only used there) | repo files |
| `repo/header.rs` | `ShareTargets`, `truncate_naddr_link` (only used there) | repo header |

`views/repo/helpers.rs` is deleted at the end of Phase 1.

---

## Phase 0 - baseline

No code. Record the current state so each later phase can be compared.

1. `cargo fmt --all -- --check`
2. `cargo check --offline --workspace --all-targets`
3. `cargo test --offline -p workspace`
4. `cargo clippy --offline -p workspace --all-targets`

Do not run plain `cargo` without `--offline`; the sandbox fails the git fetch and it looks like a dependency error.

---

## Phase 1 - extract shared modules (dissolve `helpers.rs`)

Low risk, no state moves. Land it as one commit.

### 1.1 Create `crates/workspace/src/views/tree.rs`

Move from `repo/helpers.rs`: `TreeItemSeed`, `tree_items`, `build_tree_items`, `sorted_worktree_paths`, and the three tests (`builds_nested_tree_from_flat_entries`, `tree_builder_handles_deep_nesting`, `tree_builder_merges_shared_prefixes`).

- Add `pub(crate) mod tree;` to `views/mod.rs`.
- Update imports in `repo/loading.rs`, `repo/refs.rs`, `commit_diff/mod.rs` to `crate::views::tree::...`.

### 1.2 Move diff and commit-row rendering into `views/commit_diff/mod.rs`

Move from `repo/helpers.rs`: `GUTTER_WIDTH`, `DIFF_ROW_HEIGHT`, `DiffRow`, `diff_rows`, `render_diff_row`, `render_diff_line`, `find_item`, `COMMIT_ROW_HEIGHT`, `commit_row`.

- `commit_diff/mod.rs` already owns `DiffPane` and depends on `signed_git`, so this is its natural home and keeps `signed_ui` free of a `signed_git` dependency.
- Update imports in `views/pull_requests/new.rs` and `repo/history.rs`.

### 1.3 Create `crates/workspace/src/views/discussion.rs`

Move from `repo/helpers.rs`: `sidebar_title`, `sidebar_section`, `comments_section`, `comment_form`, `issue_roots`, `pr_roots`.

- Add `pub(crate) mod discussion;` to `views/mod.rs`.
- Update imports in `views/issues/detail.rs` and `views/pull_requests/detail.rs`. After this, neither imports from `views::repo`.

### 1.4 Move `ref_selector_trigger` into `signed_ui`

It takes `CustomIconName` (from `assets`) and `ComboboxTriggerContext` (from `gpui_component`); both are already `signed_ui` dependencies, so no dependency changes.

- Add `crates/signed_ui/src/ref_selector.rs`, export it from `lib.rs`.
- Update imports in `repo/header.rs` and `views/pull_requests/new.rs`.

### 1.5 Move `code_language` and `is_markdown_path` into `repo/files.rs`

Only `repo/files.rs` uses them. Keep them private there.

### 1.6 Move `ShareTargets` and `truncate_naddr_link` into `repo/header.rs`

Only `repo/header.rs` uses them. Keep them private there.

### 1.7 Delete `repo/helpers.rs`

Remove `pub(super) mod helpers;` from `repo/mod.rs`. Confirm no `use ...repo::helpers` remains anywhere:

```
grep -rn "repo::helpers" crates/workspace/src
```

### Phase 1 validation

`cargo fmt --all`, `cargo check --offline -p workspace --all-targets`, `cargo test --offline -p workspace`, `cargo clippy --offline -p workspace --all-targets`.

---

## Phase 2 - extract `Entity<RepoFilesView>`

Largest win: removes 14 fields and most of the preview logic from the shell.

### 2.1 Define the view

In `repo/files.rs`, replace `impl RepoDetailView` with `pub(super) struct RepoFilesView` holding: `tree_state`, `worktree`, `worktree_paths`, `md`, `code`, `readme_name`, `selected_file`, `files`, `file_order`, `preview_bytes`, `loading_files`, `commits`, `pending_commits`, `loading_commits`.

Move the supporting types and helpers from the current `files.rs` into the view: `FileContent`, `MarkdownView`, `CodeView`, `MAX_PREVIEW_BYTES`, `MAX_PREVIEWED_FILES`, `MAX_PREVIEW_CACHE_BYTES`, `source_hash`, `preview_spinner`, `render_tree_item`, `render_tree_column`, `render_content_column`, `set_markdown`, `markdown_element`, `set_code`, `code_element`, `open_file`, `drop_preview_of`, `evict_previews`.

Move from `repo/history.rs`: `load_commit`, `load_commits` (the per-file commit map).

### 2.2 Define the view's interface

- `pub(super) fn new(window: &mut Window, cx: &mut Context<Self>) -> Self` - creates the `TreeState`.
- `pub(super) fn set_worktree(&mut self, path: PathBuf)`.
- `pub(super) fn apply_entries(&mut self, tree: Vec<TreeItemSeed>, paths: Vec<String>, window, cx)` - used by `load_repo` / `reload_worktree` / `catch_up_worktree`.
- `pub(super) fn set_readme(&mut self, path: Option<PathBuf>, bytes: Option<Vec<u8>>, cx)`.
- `pub(super) fn clear_previews(&mut self)` - branch switch.
- `pub(super) fn catch_up(&mut self, snapshot, window, cx) -> bool` - rebuild tree, drop removed previews, re-render README; returns whether anything changed.
- `impl Render for RepoFilesView`.
- `pub(super) fn pane_title(&self) -> SharedString` - `selected_file` or `readme_name` or `"Overview"`.

### 2.3 Move the clone loading/error display out of the file view

`render_content_column` currently shows "Cloning repository..." / a load error from `self.loading` and `self.error`, which are shell state. Move that decision to the shell's `render`: while `self.loading`, render a spinner in the tab body; when `self.error` is set, the existing `Alert` already covers it. `render_content_column` then handles only file previews and the README.

### 2.4 Wire the shell

- Add `files: Entity<RepoFilesView>` to `RepoDetailView`.
- In `new_common`, `let files = cx.new(|cx| RepoFilesView::new(window, cx));`.
- In `render`, the Files tab body becomes `self.files.clone()`.
- In `load_repo` (`loading.rs`) and `reload_worktree` / `catch_up_worktree` (`refs.rs`), replace direct field writes with calls on `self.files`.
- Remove the now-unused `files.rs` imports from `mod.rs` and the moved fields from the struct and constructor.

### Phase 2 validation

Same commands. Manual: open explore repo, click files in the tree, open the README, switch branch (previews clear), switch back, confirm no spinner sticks.

---

## Phase 3 - extract `Entity<RepoHistoryView>`

### 3.1 Define the view

In `repo/history.rs`, replace the commits-tab methods with `pub(super) struct RepoHistoryView` holding: `store: Entity<RepoStore>`, `dock_area: WeakEntity<DockArea>`, `worktree: Option<PathBuf>`, `all_commits`, `loading_all_commits`, `item_sizes`, `scroll_handle`.

Move: `render_commits_tab` (becomes `impl Render`), `load_all_commits`, `open_commit_diff`.

### 3.2 Display name

`open_commit_diff` uses the shell's `display_name`. Extract the `display_name` logic from `RepoDetailView` into a free function in `repo/mod.rs`:

```rust
pub(super) fn repo_display_name(store: &RepoStore) -> SharedString
```

It keeps the local-path fallback that `RepoStore::name()` does not have. Use it in the shell's `Panel::title`, in the header, and in `RepoHistoryView::open_commit_diff`.

### 3.3 Interface

- `pub(super) fn new(store, dock_area, window, cx) -> Self`.
- `pub(super) fn set_worktree(&mut self, path: Option<PathBuf>)`.
- `pub(super) fn reload(&mut self, cx)` - clears `all_commits` and starts the walk (called when HEAD changes or the branch switches).
- `impl Render for RepoHistoryView`.

### 3.4 Wire the shell

- Add `history: Entity<RepoHistoryView>` to `RepoDetailView`; create it in `new_common`.
- In `render`, tab 1 becomes `self.history.clone()`.
- Replace `self.all_commits` / `self.loading_all_commits` / `self.item_sizes` writes in `load_repo`, `reload_worktree`, `catch_up_worktree`, and the header-commit pill path with `self.history.update(..)` calls.
- Remove the moved fields from the struct and constructor.

### Phase 3 validation

Same commands. Manual: open the Commits tab, scroll a long history, click a commit (diff panel opens), switch branch and confirm the list reloads.

---

## Phase 4 - group `RefSwitcher` and `Banners`

Plain structs on the shell. No entity, no observer changes.

### 4.1 `RefSwitcher`

Move into a `struct RefSwitcher { branch_select, tag_select, ref_branches, ref_tags, switching_ref }` field on the shell. Update `refs.rs` and `loading.rs` methods to read/write `self.refs.*`. `switch_ref` stays on the shell because it fans out to files, history and `head_commit`.

`ref_generation` stays on the shell: it is shared with the files and history loads.

### 4.2 `Banners`

Move into a `struct Banners { dismissed, ready_requested, ready_head, ready_statuses, push_statuses }` field. `banners.rs` and `store.rs` methods keep their `impl RepoDetailView` shape but read/write `self.banners.*`.

### Phase 4 validation

Same commands. Manual: the ready-to-contribute banner appears and dismisses, the push banner appears for an owned repo, dismissing survives a store refresh.

---

## Phase 5 - fold `store.rs` and tidy

1. Move `attach_store`, `apply_announcement`, `refresh_ready_statuses`, `refresh_statuses` into `mod.rs` and delete `repo/store.rs`.
2. Remove `mod store;` from `repo/mod.rs`.
3. Confirm `mod.rs` reads as a shell: struct, constructors, load coordination, `render`, panel impls.
4. Final validation:

```
cargo fmt --all
cargo check --offline --workspace --all-targets
cargo test --offline --workspace
cargo clippy --offline --workspace --all-targets
```

## Validation (manual smoke, after each phase)

- Open a repo from the explore list, then open an issue and a PR.
- Deep-link straight to an issue / PR without visiting the repo panel.
- Open a local repository (never announced).
- Initialize a local repo to NIP-34, confirm it leaves the sidebar's local section.
- Clone to folder; clone again before the first clone completes.
- Switch a branch and a tag; confirm previews and the commit list reset.
- Owned repo with unpushed commits: push banner, push, republish banner.

## Boundary test for "done"

- No file can touch fields it does not own.
- `repo/mod.rs` is a shell, roughly 200 lines.
- `grep -rn "views::repo::helpers" crates/workspace/src` returns nothing.
- `views/issues` and `views/pull_requests` have no `use ...views::repo`.

## Non-goals

- No behavior change; no UI redesign.
- No new global state, no new store, no changes to `signed_state` or `dock`.
- No more `impl RepoDetailView` chapters. New files own structs, not fragments of one struct.
- Do not move `commit_row` into `signed_ui`: it takes `signed_git::FileCommit` and `signed_ui` does not depend on `signed_git`.

## Risks and open questions

- **Async generation.** `ref_generation` discards stale loads. It stays on the shell; when the shell pushes a snapshot into a child view, the child must not start a new load that outlives the generation. Simplest rule: only the shell starts loads, child views only render and own per-file preview fetches keyed to the current worktree.
- **Files owns the per-file commit walk.** `load_commit`/`load_commits` move with the preview state, so the shell no longer coordinates them. Confirm the README commit lookup still works after the move.
- **History is small.** After moving `load_commit`/`load_commits` to Files, `history.rs` is ~150 lines. If an entity feels heavy for that, a plain `struct History` field is an acceptable fallback; the field ownership still improves.
- **`RepoStore::name()` vs `display_name`.** `RepoStore::name()` returns `Unknown` for local repos. The extracted `repo_display_name` must keep the local-path fallback so titles are unchanged.