signed/docs/over-engineering-cleanup-plan.md

# Over-engineering cleanup plan

## Goal

Remove code that exists but cannot be reached, and machinery that guards states no
caller can produce. Findings came from a four-way read-only audit of
`signed_state`, `workspace/views`, `signed_git`/`signed_core`, and
`dock`/`signed_ui`/misc, cross-checked against the pinned dependency sources in
`~/.cargo/git/checkouts/`.

The main claim in each task below was verified by grepping callers, not by
reading the definition alone. Items that were only reported by the audit and not
independently reproduced are in Phase 4 and must be verified before deletion.

## Rules for every step

- Line numbers are from the current working tree and will drift. Re-grep before
  editing; do not trust a number from this file after other tasks land.
- Delete code, do not comment it out, do not add `#[allow(dead_code)]`.
- If a symbol looks dead but is part of a public API or a feature that is only
  not wired yet, stop and ask.
- Do not reintroduce the `views/repo` generation counters that were removed in
  this working tree.
- Keep comments out. Remove any comment that describes the code being deleted.

## Verification commands

Always pass `--offline`; a plain `cargo` invocation re-resolves and fails in the
sandbox.

```
cargo fmt -p <crate> 2>/dev/null
cargo check --offline --workspace --all-targets
cargo clippy --offline --workspace --all-targets
cargo test --offline --workspace
```

`cargo fmt -- --check` prints unrelated "unstable features" noise on stable.
Filter with `grep -E "^Diff in"`. Never hand-reformat; rustfmt is authoritative.

---

## Phase 1 - delete dead code

No behavior change. Each task is independent; commit per crate.

### 1.1 `RepoStore::version`

File: `crates/signed_state/src/repo.rs`

- [x] Remove field `version: u64` and its doc comment (the claim that views key
      caches to it is false).
- [x] Remove initializers `version: 0` in both constructors.
- [x] Remove the bump `this.version = this.version.wrapping_add(1);`.
- [x] Remove `pub fn version(&self) -> u64`.

Evidence: `grep -rn "\.version()" crates` has no call sites; the only read is the
accessor itself.

Acceptance: `grep -rn "version" crates/signed_state/src/repo.rs` shows only
unrelated uses (none of the four removed sites).

### 1.2 `clone_url` module

Files: `crates/signed_core/src/clone_url.rs`, `crates/signed_core/src/lib.rs`

- [x] Delete `clone_url.rs`.
- [x] Remove `mod clone_url;` and the `pub use clone_url::{CloneTarget, parse_clone_url};`
      re-export.

Evidence: only definition, its own test, and the re-export reference these. It
also reimplements percent-decoding.

### 1.3 NIP-32 labels / cover-note helpers

Files: `crates/signed_core/src/annotations.rs`, `crates/signed_core/src/filters.rs`,
`crates/signed_core/src/lib.rs`

- [x] Verify each of `labels`, `subject_override`, `labels_and_subject`,
      `cover_note`, `COVER_NOTE_KIND`, `annotations_for` for references outside
      this crate.
- [x] Delete the ones with no production caller and drop them from the `lib.rs`
      re-exports.
- [x] Keep anything still needed (for example `cover_note` /
      `COVER_NOTE_KIND` may be used by the inbox view).

Evidence: the UI uses `tags.hashtags()` directly in `views/discussion.rs`, not
these helpers.

### 1.4 `Backend::emit_error`

File: `crates/signed_state/src/backend.rs`

- [x] Delete the method. No callers.

### 1.5 `Backend::pushing_repos()` accessor

File: `crates/signed_state/src/backend.rs`

- [x] Delete the getter. The `Entity<HashSet<RepoAddr>>` is used internally; only
      the accessor is unused.
- [ ] Optional follow-up (separate task): nothing observes that entity, so it
      could be a plain `HashSet` field. Defer; it is a refactor, not a deletion.

### 1.6 `DropdownButton` speculative knobs

File: `crates/signed_ui/src/dropdown_button.rs`

- [x] Remove the `caret: Option<CaretBuilder>` field and the `CaretBuilder` type
      alias; it is never set, so the `unwrap_or_else` default always runs.
      Inline the default caret.
- [x] Remove the `anchor()` builder method (never called; it is already marked
      `#[allow(dead_code)]`). Keep the `anchor` field, which is set in the
      constructor and used when rendering.

### 1.7 `utils::shorten_pubkey`

File: `crates/utils/src/pubkey.rs`

- [x] Drop the `len` parameter; its only call site passes `4`.
- [x] Rename to a fixed-width helper if that reads better, or leave the name.

It duplicates `signed_ui::middle_truncate` conceptually, but `utils` has no gpui
dependency, so do not move `middle_truncate`; just remove the speculative
parameter.

### 1.8 Comment artifacts

- [x] `crates/signed_state/src/repo.rs` - delete the comment that describes
      querying cover notes and labels per root; no such query exists.
- [x] Remove any comment left dangling by the tasks above.

---

## Phase 2 - remove guards that cannot fire

Each changes behavior on paper but not in practice. Smoke test after each.

### 2.1 `SignedDockSkin::render_dock` early return and frame duplication

File: `crates/dock/src/dock_area.rs`

Base (`gpui_base::dock`) computes `dock_extent`, returns before calling the
renderer when the extent is `px(0.)`, and wraps the renderer's output in
`dock_frame`. `dock_extent` is `px(0.)` exactly when `!open && !is_bottom`, which
is precisely the condition of the early return here.

- [ ] Delete the `if !open && !placement.is_bottom() { return div(); }` guard.
- [ ] Stop re-applying `.flex().flex_none().relative().overflow_hidden()` and the
      per-placement width/height; base already applies them.
- [ ] Keep the closed-bottom strip height override, but confirm against base's
      `CLOSED_BOTTOM_STRIP` that the intended height is `TAB_BAR_HEIGHT`.
- [ ] Smoke test: open and close left, bottom, and right docks; check widths,
      the bottom strip height, and resize handles.

### 2.2 `push_staged_to_grasps` empty-refs guard

File: `crates/signed_state/src/backend.rs`

- [ ] Delete the `if refs.is_empty() { return outcome; }` guard.

All three call sites pass a non-empty `refs`: one passes a literal one-element
vec, one is inside `if !refs.is_empty()`, one is the `else` of that check.

### 2.3 `InboxView` per-view debounce

File: `crates/workspace/src/views/inbox.rs`

The backend pump already coalesces relay bursts into one `NostrUpdate`, and
`query_inbox` reads only the local database.

- [ ] Delete the `REFRESH_DEBOUNCE` constant.
- [ ] In `refresh`, drop the spawned timer; call `run_refresh` directly after
      `refresh.request()` returns `Schedule`, matching `RepoStore::refresh`.
- [ ] Keep `RefreshGate` for fold/overlap.
- [ ] Smoke test: inbox updates live as relay events land, with no added delay.

### 2.4 `DockPlacement::Center` arms in `dock_toggle_button` (optional)

File: `crates/dock/src/tab_panel.rs`

- [ ] The only call sites pass `Left`, `Bottom`, `Right`. Collapse the `Center`
      arms to `unreachable!()` or restructure so the match is exhaustive without
      a dead branch.

Low value; skip if it makes the match less readable.

---

## Phase 3 - consolidate duplication (needs a decision)

Verify the duplication before extracting; each could be intentional.

### 3.1 `PullRequestsView` and `IssuesView`

Files: `crates/workspace/src/views/pull_requests/mod.rs`,
`crates/workspace/src/views/issues/mod.rs`

- [ ] Confirm the filter enum, visible-index rebuild, counts tuple, and
      virtual-list resize are the same shape.
- [ ] If so, extract one small helper for the filtered index + counts + notify
      decision and use it in both.

### 3.2 Relay URL normalize/display

Files: `crates/workspace/src/views/sidebar/settings_dialog.rs`,
`crates/workspace/src/views/sidebar/grasp_servers.rs`

- [ ] Confirm both pairs do prepend-scheme, parse, dedupe, and host-display.
- [ ] Extract one normalize helper and one display helper. Decide the crate
      (check whether `signed_ui` may depend on `nostr`).

### 3.3 `crates/dock` vs the pinned `gpui_component` dock renderer

Files: `crates/dock/src/*` vs the pinned rev's `crates/ui/src/dock/*`

- [ ] Spike only: pick one part (`SignedTabGroupSkin` or `SignedTilesSkin`) and
      determine whether it can delegate to the upstream `DockSkin` trait
      implementation and keep only the Signed deltas (window controls in the tab
      bar, plain-sidebar detection, prev/next, i18n).
- [ ] Report effort and risk before doing any replacement. Do not start a
      rewrite of this crate in this cleanup.

---

## Phase 4 - triage flagged items

Verify each, then delete or dismiss. These were reported by the audit but not
independently reproduced.

- [ ] `signed_git/src/worktree.rs` - manual `refs/heads/{rev}` fallback; check
      whether gix's ref DWIM already covers it.
- [ ] `signed_git/src/repo.rs` - `refs_with_prefix`; check
      `repo.references()?.prefixed(prefix)`.
- [ ] `signed_git/src/patch.rs` - the mbox envelope is scanned twice; check
      whether `patch_commits` can consume `split_patch_series` output, and
      whether the hard-coded 40-hex checks should use `gix::ObjectId::from_hex`.
- [ ] `assets/src/lib.rs` - `themes` handles a `Cow::Owned` case the build
      features cannot produce.
- [ ] `signed_state/src/backend.rs` - `GraspServerResult::git_url` populated but
      never read.
- [ ] `signed_nostr/src/signer.rs` - `UniversalSignerError` vs
      `nostr::Error::other`. Keep the `InnerSigner` erasure shim; only the error
      wrapper is replaceable.
- [ ] `signed_git/src/history.rs` - `last_commit` referenced only from tests.
- [ ] `signed_git/src/repo.rs` - `init_repository` / `root_commit` 40-length
      guards on an `ObjectId` string.
- [ ] `signed_core/src/model.rs` - `Upstream.relay_hint` parsed but unused in
      production.
- [ ] `signed_core/src/inbox.rs` - `root_kind` duplicates `root_event`; confirm
      before removing, it is read by the inbox view.

---

## Do not touch

- The two `pull_requests` generation counters (`load_generation` in
  `detail.rs`, `compare_generation` in `new.rs`). Both were verified reachable.
- Tasks stored in a `Vec<Task<..>>` for lifetime cancellation. This is
  intentional.
- `RefreshGate` on `CheckoutsStore`; its timer-driven debounce is load-bearing.
- The `dev`-time `init_dialog.rs` `.detach()`; the task owns a window-scoped
  dialog and has no owning struct.

## Acceptance criteria

- Every removed symbol returns empty for a repo-wide grep.
- No new `#[allow(dead_code)]`.
- `cargo fmt -- --check` diff-free, `cargo check --offline --workspace
  --all-targets` clean, `cargo clippy --offline --workspace --all-targets`
  clean, `cargo test --offline --workspace` green.
- Manual smoke: open and close docks, watch the inbox update live, open a repo
  and switch branches, publish a repo.

## Suggested commit sequence

1. `signed_state`: 1.1, 1.4, 1.5 (dead code), plus 1.8 comments in the same files.
2. `signed_core`: 1.2, 1.3 (dead modules).
3. `signed_ui` + `utils`: 1.6, 1.7.
4. `dock`: 2.1, 2.4.
5. `signed_state`: 2.2.
6. `workspace`: 2.3.
7. Phases 3 and 4 as separate, individually reviewed changes.