refactor

docs/PLAN.md

@@ -1,229 +0,0 @@
-# PLAN — Codebase audit: over-engineering, dead code & simplification
-
-> **Status (2026-09-04): proposed.** Full-repo audit (~27k lines, 13 crates).
-> Every finding was cross-checked against consumers and verified against the
-> locked library sources (gpui-component `18922d6`, rust-nostr `472c883`,
-> gix 0.87.1). Pick the steps you want before processing; ordered
-> safest-first. Estimated total: **~3,000+ lines removable (~12%)**.
-
----
-
-## Step 1 — Zero-risk deletions (~700 L, pure removals)
-
-1. **Unused Cargo deps** (verified by grep):
-   - `desktop`: `gpui_linux`, `gpui_windows`, `gpui_macos` (only
-     `gpui_platform` is used — it's the meta-crate that picks the backend),
-     `log`
-   - `workspace`: `chrono`
-   - `signed_nostr`: `signed_core`, `nostr`
-   - `assets`: `log`
-2. **`signed_core::clone_url` module** (whole file) — only self-tests
-   reference it; `repo_detail/mod.rs:2418` *produces* the format but never
-   parses it. (`clone_url.rs` + `lib.rs:13`) <- acceptable
-3. **`signed_core::comments` module** (`CommentThread`, `comment_threads`) —
-   no consumers; SDK has `nip22::extract_parent` anyway.
-   (`comments.rs` + `lib.rs:14`)
-4. **`paths` dead accessors**: `cache_dir()`, `logs_dir()`, `keymap_file()`,
-   `set_custom_data_dir()` + `CUSTOM_DATA_DIR`/`OnceLock` machinery
-   (`paths/src/lib.rs:13,50-61,112-169`). Used: `desktop_dir`,
-   `documents_dir`, `settings_file`, `nostr_dir`, `repos_dir`.
-5. **`signed_git::patch_applies`** (+test) — own TODO says superseded by the
-   live compare view (`lib.rs:252-277, 2294-2324`).
-6. **`signed_git::worktree_tags`** — both UI sites call `repo_tags` directly
-   (`lib.rs:1811-1813`).
-7. **Unused re-exports**: `workspace::image_cache` (`workspace/src/lib.rs:6`);
-   `signed_state::lib`'s `pub use utils::shorten_pubkey`
-   (`signed_state/src/lib.rs:21`).
-8. **Sidebar placeholder nav items** "Inbox", "Search", "Guide" — all three
-   just open the Explore panel (`sidebar/mod.rs:578-618`). <- acceptable
-9. **`import_dialog.rs`** — opens an empty 400px dialog; the "Import identity"
-   sidebar button is a dead end (`sidebar/mod.rs:405-408,493-504`). <- acceptable
-10. **All `wasm32` cfg paths + `nostr-memory` dep** — GPUI has no wasm
-    backend; the target cannot link (`signed_state/src/lib.rs:52-63`,
-    `checkouts.rs:108-165`, `signed_nostr/src/backend.rs:9-29`,
-    `signed_nostr/Cargo.toml`). <- acceptable, note: GPUI have support for wasm via gpui_web, updated your memory or check before make changes
-11. **`dock::t()` unreachable `"Dock.Unnamed"` arm** (`dock/src/lib.rs:21-30`).
-12. Two handler-less `Button`s ("user" with fake `dropdown_caret`,
-    "maintainers") — render as plain `h_flex` or wire real menus
-    (`sidebar/mod.rs:426-434`, `repo_detail/mod.rs:2289-2307`). <- acceptable
-13. Stale doc references to nonexistent `helpers::track`
-    (`diff.rs:328`, `mod.rs:201-203`).
-
-## Step 2 — Dead feature removal (~500 L)
-
-1. **Login/logout API** — `login`, `login_with_new_identity`,
-   `login_with_nsec`, `login_with_bunker`, `logout`, `with_master_key`
-   (`backend.rs:879-988,1537-1542`). UI only uses `create_identity` + keyring
-   restore. ⚠️ `login_with_new_identity` stores unencrypted nsec — security
-   footgun. **Keep** `extract_master_key` + the `bunker://` branch of
-   `restore_session` (services older keyring entries). <- acceptable
-2. **`RepoStore::merge_pull_request` + `publish_applied_status`** — no merge
-   action exists in the UI (`repo.rs:1122-1256`). <- acceptable
-3. **`RepoStore::publish_state`** — superseded by `Backend::push_repo_from`
-   (`repo.rs:1070-1120`).
-4. **Annotation machinery** — `cover_note_of`/`labels_of`/`subject_of` (no
-   callers) + `cover_notes`/`labels` fields + per-root DB query loop +
-   `annotations_for` relay fetch. **Every kind-1624/1985 event currently
-   triggers a full refresh of every open RepoStore for data nothing
-   displays** (`repo.rs:52-56,339-362,489,531-563`).
-5. **`BackendEvent::Error` variant + `emit_error` + ~16 emission sites** —
-   matched nowhere (consumers only match `SignerChanged`, `SignerRequired`,
-   `PassphraseRequired`, `NostrUpdate`, `Synced`, `SyncProgress`,
-   `Published`) (`backend.rs:64,67-74,1047-1050`). <- acceptable
-6. **`BackendEvent::Connected` + `connected` field + `is_connected()`** —
-   emitted twice, never consumed (`backend.rs:49,83,154,1110-1111,1052-1055`).
-7. **`sync_progress` field + getter + `SyncProgress::channel` watch-loop
-   math** — payload discarded; used only as a dumb refresh tick
-   (`backend.rs:84,1057-1060,1227-1258`). <- acceptable
-8. **`Backend::subscribe`, `add_discovery_relays`, `publish_announcement`** —
-   zero callers (`backend.rs:1123-1158,1388-1395`).
-9. **Write-only fields**: `RepoStore::refs` (`repo.rs:37-38,449`),
-   `RepoListStore::set_author` (`repo_list.rs:135-140`),
-   `Update::event_id` (`signed_nostr/src/update.rs:10`).
-10. **NIP-44 half of `UniversalSigner`** (`AsyncNip44` bounds,
-    encrypt/decrypt plumbing, ~60 of 200 L) — app never touches DMs
-    (`signed_nostr/src/signer.rs`). <- acceptable
-
-## Step 3 — Library swaps (~600 L)
-
-1. **`signed_ui::DropdownButton` → `gpui_component::button::DropdownButton`**
-   — present in the locked revision, same `new/button/dropdown_menu` surface
-   (the local doc even says it matches). Migrate 3 call sites
-   (`pull_requests.rs`, `repo_detail/mod.rs`), delete the ~200 L file. <- acceptable
-2. **`wire_number_input` → `SettingField::number_input`**
-   (`settings_dialog.rs:691-761`, ~90 L) — you already import
-   `NumberFieldOptions` from gpui-component's setting module. <- acceptable
-3. **Custom C-unquoting → `gix::quote::ansi_c::undo`**
-   (`signed_git/src/lib.rs:1610-1683` + call sites, ~100 L) — already in the
-   dep tree, octal/escape semantics identical. Keep tests as regression tests.
-4. **`ensure_origin` redundant refspec write** (`signed_git/src/lib.rs:523-530`)
-   — `git remote add` creates `remote.origin.fetch` by default.
-5. **Two hand-rolled tab bars → gpui-component `TabBar`**
-   (`repo_detail/mod.rs:2120-2265`, `new_pull_request.rs:1236-1305`, ~110 L) —
-   `pull_request_detail.rs` already uses `TabBar` correctly (proves the fit). <- acceptable
-6. **CLI `merge_base` → `gix::Repository::merge_base`**
-   (`signed_git/src/lib.rs:201-222`) — the codebase already uses the gix one
-   in `pull_request_detail.rs:244`.
-7. **`image_cache` → `gpui::retain_all` or single-map LRU** — vendored copy
-   with a `max_items` param every call site passes `MAX_IMAGES=128` to
-   (`signed_ui/src/image_cache.rs`, 139 L). Delete the param + dual-structure
-   LRU or use upstream unbounded cache.
-
-## Step 4 — Dedup passes (~1,200 L)
-
-1. **`PullRequestDetailView` → hold `Entity<DiffPane>`** instead of its ~200 L
-   inline copy of `diff.rs` (`pull_request_detail.rs:310-552`). The other two
-   consumers already do this.
-2. **Issue/PR detail shared sections** (~300 L): comments list, comment form,
-   participants/labels sidebar, `sidebar_title` — extract into `helpers.rs`
-   (`issue_detail.rs` vs `pull_request_detail.rs`).
-3. **Dialog scaffolding helper** (~150-200 L): 4 copies of `{busy, error}`
-   state structs, verbatim error rows, identical
-   `cx.spawn → close_dialog / show error` plumbing
-   (`onboarding_dialog.rs`, `passphrase_dialog.rs`, `create_repo_dialog.rs`,
-   `init_dialog.rs`, +2 more sites).
-4. **Triplicated debounce state machine → one helper** — `refreshing` /
-   `refresh_dirty` / `debouncing` trio copied into `repo.rs`, `checkouts.rs`,
-   `repo_list.rs` (~105 L).
-5. **signed_git helpers**: `push_main`/`push_all` twin bodies → one
-   `push_refspecs`; git-CLI spawn boilerplate → one `git_output`;
-   grasp→https rewrite → one fn; "try each mirror URL" loop → one helper.
-6. **grasp-list parsing ×3 → one helper** — `backend.rs:996-1012`,
-   `backend.rs:1586-1618`, `grasp_servers.rs:240-265`.
-7. Smaller copies: ref-selector trigger ×2, count badge ×3 (use
-   `signed_ui::CountBadge`), grasp-server editor duplicated in
-   `settings_dialog.rs:408-614` vs `grasp_servers.rs` (~90 L), folder-picker
-   prompts ×4, "add panel to dock Center" ×10, fork-label upstream lookup ×2,
-   avatar+name row ×9 (→ one `user_row` helper in signed_ui).
-8. **`Backend::send` vs `publish_event`** — copy-pasted bodies differing only
-   in `finalize_async` (`backend.rs:1293-1386`).
-
-## Step 5 — Structural (do deliberately)
-
-1. **`UniversalSigner` → enum** — verified: nostr-sdk 0.45 `Client` has no
-   signer slot (external `SignerAuthenticator` by value at build time), so a
-   swap-in-place wrapper IS needed — but only `Keys`/`NostrConnect` ever
-   occur. Replace 200-L vtable (`InnerSigner` trait + `InnerSignerImpl<T>` +
-   custom error + boxed futures) with
-   `enum Signer { Keys(Keys), Connect(NostrConnect) }` in
-   `Arc<RwLock<Signer>>` (~40 L). <- acceptable
-2. **`GitStore` single install** — `signed_state::init` installs empty root
-   (`lib.rs:44`), immediately replaced by `desktop/main.rs:81`. Pass the root
-   into `init`.
-3. **Error handling in signed_git**: `map_err(|e| anyhow!("{e}"))` → plain
-   `?` (preserves source chain; `lib.rs:1787,1798,1845,1852`); blanket
-   `.ok()`/`.unwrap_or_default()` → matched cases
-   (`lib.rs:919,995,1761,364,682,1779`).
-4. **`ProfileStore` second flume channel → `WeakEntity` + `update()`**
-   (~25 L; the results channel only exists to get back to main thread).
-5. **Unbounded `tasks` Vec growth** — only `repo.rs` prunes; `backend.rs`
-   (~18 push sites), `checkouts.rs` (grows every 15-60 s poll cycle),
-   `profile.rs` (2 per metadata event) accumulate finished handles forever.
-   One-line `retain(|t| !t.is_ready())` per store.
-6. **Redundant `observe → cx.notify()` subscriptions**
-   (`sidebar/mod.rs:81-88`, `repo_detail/mod.rs:354-355,1914-1921`) — this
-   gpui revision auto-tracks entities read during render, so re-render-only
-   observers are belt-and-suspenders. ⚠️ Verify before deleting; see Step 7.2.
-7. **`open_upstream` sleep-poll → `cx.observe`** (`repo_detail/mod.rs:1113-1152`)
-   — 60×250 ms race-prone loop re-implementing the store's notify mechanism.
-8. **Checkouts map `Arc` removal** — accessors deep-clone anyway
-   (`checkouts.rs:69-81`).
-9. Minor: `CheckoutRecord` manual `Default` → derive; `dock` re-export trim
-   (27 items, ~10 used); `PixelAvatar.size` field with no setter → const;
-   `SCAN_SKIPPED_DIRS` 1-element array → direct compare; `RepoAction` menu
-   indirection → `on_click`; `Announcement::from_event` round-trip after
-   building from typed data.
-
-## Step 6 — Over-optimization (optional, unmeasured machinery)
-
-1. **`HeaderCache`** (`repo_detail/mod.rs:113-123,1426-1461`) — keyed
-   invalidation + 3 `Rc` layers memoizing 2 bech32 encodes + 2 `format!`s
-   (~45 L). Compute inline.
-2. **Comment-body memoization** `contents: HashMap<EventId, SharedString>`
-   (`issue_detail.rs`, `pull_request_detail.rs`) — caches one small alloc per
-   render; convert inline.
-3. **`OBJECT_CACHE_BYTES = 64 MiB`** indiscriminately applied — scope to
-   history-walk entry points or drop (`signed_git/src/lib.rs:770,859-863`).
-4. **`file_commit(…, include_description: bool)`** — boolean flag saving one
-   alloc; split into named constructors or always include.
-5. **`signed_core` could drop its `gpui` dep** if `Announcement`'s
-   `SharedString` fields became `String` (`model.rs:9-35`).
-6. **`build_state`** raw `Tag::parse(...).expect()` → typed `Nip34Tag::to_tag`
-   (`signed_core/src/state.rs:7-18`).
-7. **`PixelAvatar` FNV/RNG stack** (~60 L) — deterministic-hashing requirement
-   is legit; reconsider if `DefaultHasher` stability is acceptable.
-
-## Step 7 — Clean verdicts + one correctness note (no action)
-
-1. **Confirmed clean, keep as-is**: `signed_core` correctly uses
-   `Nip34Tag::parse` / SDK builders (only the `u` tag is hand-parsed — SDK
-   doesn't model it); `dock` crate is a genuine thin skin over
-   `gpui_base::dock` renderer traits (keep; only trim re-exports); LMDB is
-   justified (instant startup lists); `signed_git` test suite + `tempfile`
-   healthy. Gossip/NIP-65 machinery serves only one login query — *consider*
-   dropping if NIP-65 routing isn't on the roadmap.
-2. **Correctness note**: `IssuesView` / `PullRequestsView` /
-   `IssueDetailView` / `PullRequestDetailView` never observe their
-   `RepoStore` — refresh only works because this gpui revision auto-tracks
-   render reads. Accidental; pinning a different gpui breaks them silently.
-   Relevant to Step 5.6.
-
----
-
-## Suggested order of operations
-
-1. Step 1 + Step 2 first — pure deletions, verify with `cargo check` +
-   `cargo test`.
-2. Step 3 (library swaps) next — mechanical, mostly call-site migrations.
-3. Step 4 (dedup) — largest line savings.
-4. Step 5 (structural) — deliberate, one at a time.
-5. Step 6 (over-optimization) — optional, only if you want the extra ~110 L.
-
-Each step below is self-contained; tick what you want done.
-
-- [ ] Step 1 — zero-risk deletions
-- [ ] Step 2 — dead feature removal
-- [ ] Step 3 — library swaps
-- [ ] Step 4 — dedup passes
-- [ ] Step 5 — structural changes
-- [ ] Step 6 — over-optimization cleanup

docs/TODO.md

@@ -1,49 +0,0 @@
-# TODO
-
-## Fork support
-
-- [x] Fork badge on repo list cards (`repo_list.rs::render_card`).
-- [x] "Forked from …" text button in the repo detail header (`repo_detail/mod.rs::render_header`) and About dialog.
-- [x] Clicking the upstream opens it as a center panel (shared `open_repo_panel` helper).
-
-## Pull request improvement
-
-### New pull request panel (replaces the dialog)
-
-- [x] "New pull request" (PR list header + repo header `New PR`) opens a center panel instead of the paste dialog:
-  - [x] Base/compare branch selectors fed from a user-chosen local checkout (GitHub-style; defaults: announced HEAD for base, checkout's current branch for compare).
-  - [x] Files/Commits tabs like the repo panel: diff of `merge-base..compare` (shared `DiffPane` widget, also extracted for the commit diff panel) + virtual commit list with count badge; clicking a commit opens its diff panel.
-  - [x] Only two inputs: title (required, gates the Create button) and description (optional).
-  - [x] Patch is generated from the checkout at submit time (`format_patch_between` on the stored merge base); panel closes after publishing, errors surface in the PR list banner.
-- [x] Removed with the dialog: paste textarea, draft checkbox, branch-name input and the mirror-clone apply-check hint (store behavior unchanged: `open_pull_request` still publishes the series + `branch-name`/`merge-base`/`r` tags and pushes the tip).
-
-### Send patch panel (classic paste flow)
-
-- [x] "Send patch" entry in the repo header PRs dropdown (`RepoAction::SendPatch`) and a "New pull request ▾ Send patch" dropdown replacing the PR list's plain new-PR button.
-- [x] `send_patch.rs` center panel: title + optional description + `git format-patch` paste area; submits through `RepoStore::open_pull_request` (no checkout, no `branch-name`/`merge-base`). Synchronous store errors (malformed/oversized patch, sign-in) keep the panel open with an inline error; the panel closes once the publish is underway.
-
-- [x] P1: `branch-name` tag + `r` EUC tag on PR creation; draft checkbox in the new-PR dialog (dialog since replaced by the panel above).
-- [x] P1: `RepoStore::update_pull_request` (kind 1619 + root-revision patch) with an author-only "Update" button on the PR detail header.
-- [x] P1: `latest_update` filters by PR author.
-- [x] P2: local checkout picker in the new-PR dialog (folder picker + source/target branches + Generate): `signed_git::{merge_base, format_patch_between, patch_applies}`; `merge-base` tag now published; best-effort apply check shown under the patch field (superseded by the panel's live compare view).
-- [x] P3: push tip to grasp servers under `refs/nostr/<event-id>` before publishing (from the local checkout); multi-commit series published as NIP-10-chained 1617 events with a 60 KB per-patch cap; PR list shows dismissible error/warning banners (incl. push failures).
-- [x] P4: merge status tags — `merge_pull_request` publishes 1631 with `applied-as-commits` + `r` per applied commit and `q`/`e`-reply tags per applied patch event.
-
-### Pull request follow-ups
-
-- [ ] GRASP-06 `/prs/<npub>/<id>.git` contributor endpoints + kind-10317 user grasp-list fallback.
-- [ ] Merge button in the PR detail view (`merge_pull_request` is store-only today), then fetch-and-merge (`merge-commit`) when the push backend is guaranteed.
-- [ ] Local-checkout generation for the update-PR dialog (currently paste-only); once it lands, push update tips to the same `/prs/` set under the PR's stable ref.
-- [x] Fork-aware compare in the New PR panel: the compare side can come from an announced fork repository (u-tag/EUC-related, own forks first) whose branches are imported into the base repo's GitCache mirror under `refs/fork/<owner>/<id>/*`; `merge-base`/diff/`format-patch`/push all run in the mirror against full refs. The panel's source picker switches between local checkouts and announced forks.
-- [x] GRASP-06 author hosting: PR tips are pushed to the author's own grasp servers under `/prs/<author-npub>/<repo-id>.git` (kind-10317 grasp list, settings defaults as fallback) before the base announcement's servers; the `clone` tag carries the `/prs/` URLs first.
-- [ ] Checkout suggestions ("ready to contribute"): remembered/matched local checkouts prefill the New PR panel; a repo-panel banner suggests creating a PR when a branch is ahead with no open PR. Sidebar "Ready to contribute" group is v2 (deferred).
-
-## Performance: render path
-
-- [ ] Virtualize issue/PR comment threads (`issue_detail.rs::render_comments`, `pull_request_detail.rs::render_comments`). Harder than the list tabs: comment cards have variable heights and live inside a scrolling page together with the body and the comment form, so this needs either measured item sizes or restructuring the whole discussion tab into one virtual list. (Comment bodies are already cached as `SharedString`, so re-renders are cheap element constructions, not byte copies.)
-
-## Performance: relay/subscription behavior
-
-- [ ] Narrow `RepoStore`'s `BackendEvent::NostrUpdate` relevance filter (`crates/signed_state/src/repo.rs:65-98`): any comment/status/label/deletion from anywhere wakes every open repo store; match only events referencing this repo's roots or coordinate.
-- [ ] Reconsider `ban_relay_on_mismatch(true)` (`crates/signed_nostr/src/backend.rs:49`): combined with many short-lived auto-close subscriptions, a late event after EOSE can permanently ban a relay for the session.
-- [ ] Relays added for a repo stay in the pool forever and grow unboundedly (`crates/signed_state/src/backend.rs`); consider removing repo relays when the last panel for that repo closes.