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

• Remove field version: u64 and its doc comment (the claim that views key caches to it is false).
• Remove initializers version: 0 in both constructors.
• Remove the bump this.version = this.version.wrapping_add(1);.
• 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

• Delete clone_url.rs.
• 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

• Verify each of labels, subject_override, labels_and_subject, cover_note, COVER_NOTE_KIND, annotations_for for references outside this crate.
• Delete the ones with no production caller and drop them from the lib.rs re-exports.
• 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

• Delete the method. No callers.

1.5 Backend::pushing_repos() accessor

File: crates/signed_state/src/backend.rs

• 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

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

• Drop the len parameter; its only call site passes 4.
• 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

• crates/signed_state/src/repo.rs - delete the comment that describes querying cover notes and labels per root; no such query exists.
• 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 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.
• 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

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

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