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.

- [x] Delete the `if !open && !placement.is_bottom() { return div(); }` guard.
- [x] Stop re-applying the box. The chrome is now `.flex().size_full().relative()`,
      the same shape the pinned reference skin (`crates/ui/src/dock/dock.rs`) uses;
      base's `dock_frame` supplies the extent and the overflow clip.
- [x] Keep the closed-bottom strip height override. Note: base's `dock_frame`
      hard-codes `CLOSED_BOTTOM_STRIP` (29px) with `overflow_hidden`, so the
      `TAB_BAR_HEIGHT` (44px) override is clipped and has no visible effect. The
      strip is 29px today; changing it needs an upstream change.
- [ ] 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`

- [x] 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.

- [x] Delete the `REFRESH_DEBOUNCE` constant.
- [x] In `refresh`, drop the spawned timer; call `run_refresh` directly after
      `refresh.request()` returns `Schedule`, matching `RepoStore::refresh`.
- [x] 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`

- [x] The only call sites pass `Left`, `Bottom`, `Right`. The `designated`
      match's `Center` arm now returns early instead of yielding a dead `None`;
      the icon match keeps `Center => return None` for exhaustiveness.

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`

- [x] Confirm the shape. The two `rebuild`s are the same mechanic: one pass over a
      root list, per-status counts, keep matching indices, early-return when
      filter/indices/counts are unchanged, resize the item sizes, notify.
- [x] Extract `crates/workspace/src/views/status_list.rs` with `StatusCounts` and
      `filter_by_status`. Both views now use it; the tuple counts were replaced by
      `StatusCounts`. The notify decision stays local because it would need a trait
      over the two filter enums.

### 3.2 Relay URL normalize/display

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

- [x] Confirm both pairs do prepend-scheme, parse, dedupe, and host-display.
- [x] Extract `normalize_server` and `server_host` into `sidebar/mod.rs`. They live
      in `workspace`, not `signed_ui`: `signed_ui` does not depend on `nostr`, and
      these are used only by the two sidebar modules. Dedupe differs per caller
      (`Vec<RelayUrl>` vs persisted `Vec<String>`) and stays at the call site.

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

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

- [x] Spike: `SignedTabGroupSkin` cannot delegate to the pinned upstream skin.
      - `TabGroupSkin`, `TilesSkin` and `SkinShared` are `pub(crate)` in
        `gpui_component::ui`; only the opaque `DockSkin` renderer is public, and it
        holds that private shared state.
      - `TabGroupRenderer`/`TilesRenderer` are all-or-nothing per method. The
        Signed deltas (window controls, prev/next, plain-sidebar detection, i18n)
        live *inside* `render_tab_bar` and `frame`. There is no hook below the whole
        method, so "delegate and keep only the deltas" has no seam to hang on.
      - Composing `Rc<DockSkin>` would still leave `render_tab_bar` a near-full
        reimplementation while adding a dependency on upstream internals, for no
        line reduction.
- [x] Effort/risk: high effort, high churn, no achievable reduction on this rev.
      `SignedTilesSkin` is the same shape. Recommend keeping the fork as-is. A
      future upstream change (public `DockSkin` with per-part hooks) would be the
      precondition for any delegation.

---

## 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.