Files
signed/docs/REFACTOR_PLAN.md
T
2026-10-03 10:21:08 +07:00

265 lines
23 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Backend refactor plan
Branch: `audit` @ `6c4cc9f`. Baseline: `cargo check --workspace` clean (~10s incremental), `cargo test --workspace` 111/111 passing.
Scope: `signed_core`, `signed_git`, `signed_nostr`, `signed_state`, `utils`, `paths`, `settings` (~13,000 lines). `workspace`, `signed_ui`, `dock`, `desktop` are touched only as call sites. `.delta/` is ignored throughout.
## Goals
1. Behavior lives in `impl` blocks on a named domain type. No free functions in domain crates.
2. Free functions are allowed only in `utils` (and `paths`, which is the same kind of crate) — pure, stateless helpers.
3. Comments only where they carry non-obvious "why": protocol rules, safety invariants, races, ordering guarantees. No summaries, no field-name restatements.
4. Tests only for protocol interop and user-visible contracts. Delete tests that lock internal structure.
Everything below is behavior-preserving except the explicit deletions in Phase 0.
---
## 1. Audit summary
| Crate | Lines | Free fns (non-test) | Impl blocks | Comment lines | Tests |
|---|---|---|---|---|---|
| `signed_core` | 2,226 | 47 | 6 | ~216 | 29 |
| `signed_git` | 3,743 | 52 | 3 | ~276 | 38 |
| `signed_nostr` | 257 | 3 | 1 | ~6 | 0 |
| `signed_state` | 6,206 | 53 | ~20 | ~460 | 25 |
| `utils` | 98 | 4 | 0 | ~3 | 3 |
| `paths` | 105 | 8 | 0 | ~26 | 0 |
| `settings` | 369 | 1 | 4 | ~44 | 5 |
### Structural findings
- **`signed_git` is 52 free functions and 3 impl blocks.** `gix::open` is called 29×; nearly every entry point re-opens the repository. Eight near-duplicate `worktree_*`/`repo_*` pairs exist only to bridge `&Path` ↔ `&gix::Repository` (`worktree_all_commits`/`all_commits`, `worktree_branches`/`repo_branches`, `worktree_ref_state`/`repo_ref_state`, `worktree_current_branch`/`current_branch`, `head_commit_id`/`head_commit`, …). The `worktree_` prefix means two different things (path-based vs `&gix::Repository`-based). `GitCache` is the one type done right — it is the template for the refactor.
- **`signed_state/backend.rs` (2,020 lines) is a god object plus 24 free helpers.** `create_repository` (L370–546) and `publish_local_repo` (L549–722) duplicate ~120 lines of validation/announce/push/retract/fan-out; `push_repo_from` (L752) is already the factored third variant. The grasp push loop (`push_staged_to_grasps` + 6 helpers) and the grasp URL family float freely. `Backend::connect_repo_relays` (method) calls a free fn of the same name (L1375).
- **`signed_core` operates on foreign types.** 47 free functions take `&Event`/`&RepoAddr`-alias first args or are module-bound constructors; inherent impls are impossible without introducing types. `RepoAddr` is a bare alias for the SDK's `Coordinate`, so it can carry no methods.
- **`signed_nostr::new_backend` returns a tuple** `(Client, UniversalSigner)` whose halves are always created and consumed together.
- Duplication across crates: three middle-truncation helpers (`utils` private `truncate_middle`, `signed_ui::middle_truncate`, `workspace::truncate_label`), two hand-rolled mbox envelope parsers, three git-CLI spawn patterns, and `workspace/src/views/repo/mod.rs:1955` re-assembling by hand what `worktree_snapshot` should provide.
### Dead code (grep-verified, zero callers outside own crate)
| Item | Location | Note |
|---|---|---|
| `login`, `login_with_new_identity`, `login_with_nsec`, `login_with_bunker`, `logout`, `with_master_key` | backend.rs L917–1025, L1451 | No UI path calls these; `import_dialog::open` is an empty stub (TODO.md confirms) |
| `BackendEvent::SyncProgress` pipeline (variant, `sync_progress` field + accessor, progress task) | backend.rs L43, L64, L1104, L1191–1263 | No subscriber; TODO.md retains it for an unbuilt indicator |
| `BackendEvent::Error` | backend.rs L47 | Emitted 9×, subscribed 0× — errors currently vanish |
| `RepoStore::merge_pull_request` + `publish_applied_status` | repo.rs L1195, L1499 | Zero callers |
| `Inbox::mark_read` / `mark_archived` | inbox.rs L27, L43 | Zero callers |
| `InboxItem::kind` | signed_core/inbox.rs L45 | View reads `root_event.kind` directly |
| `filters::notification_comments`, `filters::NOTIFICATION_KINDS`, `inbox::notification_root` re-exports | signed_core | Internal-only despite `pub` + root re-export |
| `paths::{home_dir, config_dir, data_dir}` pub-ness | paths/lib.rs | Internal-only |
| `local_repo_addr` re-export | signed_state/lib.rs L19 | Internal-only |
| wasm32 code paths (`new_backend` wasm variant, `init` wasm variant, `cfg!` guards, `nostr-memory` dep) | signed_nostr/backend.rs L25, signed_state/lib.rs L65, backend.rs L174, checkouts.rs L103/136/162 | No wasm target exists in the workspace |
| Unused `signed_core/lib.rs` root re-exports | lib.rs L11–14 | Callers use `filters::`/`inbox::` module paths |
### Speculative generality
- `U: AsRef<str>` generics on `clone_repo`, `fetch_repo_refs`, `GitCache::ensure_clone` — every call site passes `&str`/`String`.
- `GraspSignals` exposes 10 bools; consumers read 4.
- `RepoStore::set_status` and `reply` are pub but only used internally (draft-PR flow / `comment`).
---
## 2. Target types
New or promoted types that absorb the free functions. Each row is a home, not a new abstraction layer — they are mostly newtypes over data the code already passes around.
| Crate | Type | Absorbs |
|---|---|---|
| `signed_core` | `RepoAddr` (newtype over `Coordinate`, replacing the alias) | `repo_addr` → `RepoAddr::new`; `identifier_from_name` → `RepoAddr::identifier_from_name` (NIP-34 identifier policy stays here, not utils); enables `addr.announcement_filter()` etc. |
| `signed_core` | `PullRequest<'a>(&'a Event)` | `pull_request_patches`, `pull_request_patch`, `latest_update`, private `forward_series`, `patch_produces_commit` |
| `signed_core` | `trait Nip34Event` (extension on `Event`) | `current_commit_of`, `merge_base_of`, `clone_urls_of`, `branch_name_of`, `activity_subject`, `references_root`, `is_git_activity` — needed because the same accessors apply to PR roots *and* updates, where `PullRequest` can't go |
| `signed_core` | `RepoState { refs, head }` | `build_state` → `RepoState::builder(id)`, `parse_state` → `RepoState::from_event`; kills the anonymous tuple return |
| `signed_core` | `ThreadResolver<'a>` (wraps the event lookup closure) | `resolve_thread_root`, `parent_id`, `nip10_root_id`, `first_e_id`, `first_uppercase_e_id`, `first_tag_id`, `e_tag_with_marker`, `notification_root` |
| `signed_core` | `Filters` (unit struct) | the 12 filter constructors (`announcement`, `state`, `activity`, `statuses_for`, `grasp_list`, `comments_for`, `notifications`, `authored_activity`, `all_announcements`, `all_states`, `deletions`, `deletions_for_repo`) — repo-scoped ones become `RepoAddr` methods |
| `signed_core` | `Announcement::forks_in` (assoc fn) | `fork_candidates` |
| `signed_nostr` | `NostrBackend { client, signer }` | `new_backend` → `NostrBackend::open(path)`; private `with_database` assoc fn; kills the tuple |
| `signed_git` | `Repo` (wrapper over `gix::Repository`) | ~46 of the 52 free functions — see Appendix A. `Repo::open`, `Repo::open_cached` (object cache for history walks), `Repo::init`, `Repo::clone`. One `gix::open` per operation instead of 29 scattered calls. The `worktree_*`/`repo_*` pairs collapse |
| `signed_git` | `GitCache` (existing) | unchanged except `open`/`ensure_clone` return `Repo` |
| `signed_state` | `GraspServer<'a>(&'a RelayUrl)` | `grasp_base_url`, `grasp_clone_url`, `grasp06_prs_url`, `pr_clone_urls`, `grasp_list_servers`, `latest_grasp_list_servers`, `user_grasp_list_servers` |
| `signed_state` | `GraspPush { client, signer, executor }` | `push_staged_to_grasps`, `sign_state_event`, `stage_event_on_relay`, `keep_newest`, `is_transient_grasp_denial`, `is_stale_advertisement_race` (→ `PushRejection::classify`), `PushOutcome` — moved to a new `push.rs` |
| `signed_state` | `BunkerCredential { uri, session_key }` | `extract_master_key`, `with_master_key` (if the bunker login path survives Phase 0 — otherwise deleted) |
| `signed_state` | `StatusIndex(HashMap<EventId, RepoStatus>)` | `resolve_statuses`, `status_of`; `RepoStore::status_of` reads through it |
| `signed_state` | `PatchSeries` | `patch_current_commit`, `split_patch_series` consumers in repo.rs |
| `signed_state` | `Mirrors` (unit struct over the `OnceLock<GitCache>`) | the 6 free fns in `git_store.rs` |
| `signed_state` | methods on existing types | `query_inbox`/`load_state`/`save_state`/`fetch_notifications`/`event_references` → `Inbox`; `pr_proposes_checkout`, `resolve_associations`, `checkout_status`, `checkout_push_status`, `compute_statuses` → `CheckoutsStore` assoc fns; `resolve_local_repos` → `LocalReposStore`; `next_update` → `impl UpdateEvent`; `retract_event`, `publish_best_effort`, `ensure_bootstrap_relays`, `subscribe_bootstrap_only`, `sync_bootstrap_only` → `Backend`; `publish_patch_series`, `comment_builder` → `RepoStore`; `url_identity`, `same_repo_url` → `utils` |
| `utils` | consolidation | one `middle_truncate` (absorbing 3 copies — `signed_ui` gains a `utils` dep), `flatten_whitespace`, `url_identity`/`same_repo_url`, `latest`, `sort_newest_first`/`sort_oldest_first` |
`settings` needs nothing: it is already plain data + one store. `default_scan_paths` folds into the `Default` impl.
---
## 3. Phases
Each phase ends with `cargo check --workspace && cargo test --workspace` green and its own commit, so review and bisection stay cheap. Call-site updates in `workspace`/`signed_ui`/`desktop` happen in the same commit as the API change.
### Phase 0 — Delete dead code and non-critical tests
The cheapest phase; shrinks every later diff.
1. Delete the `login`/`logout` family + `with_master_key` (backend.rs). Either delete the empty `import_dialog` stub and its menu entry, or leave the stub — **decision D1**.
2. Delete the `SyncProgress` pipeline (variant, field, accessor, progress task). TODO.md's planned indicator can re-add from git history when it is actually built — **decision D2, default delete**.
3. `BackendEvent::Error`: 9 emit sites, 0 subscribers. Either wire one subscriber in `workspace` (a banner/toast, ~15 lines, makes existing error emissions user-visible) or delete variant + emissions — **decision D3, default: wire the subscriber** (AGENTS.md requires errors reach the UI; the emissions already exist).
4. Delete `merge_pull_request` + `publish_applied_status` (recoverable from history if the merge UI is built) — **decision D4, default delete**.
5. Delete `Inbox::mark_read`/`mark_archived`, `InboxItem::kind`.
6. Demote to private: `RepoStore::set_status`, `RepoStore::reply`, `Backend::restore_session`, `Backend::set_signer`.
7. Privatize `filters::notification_comments`, `NOTIFICATION_KINDS`, `inbox::notification_root`, `paths::{home_dir, config_dir, data_dir}`; drop unused `signed_core`/`signed_state` root re-exports (`local_repo_addr`, `GraspSignals` path, etc.).
8. Remove wasm32 paths + `nostr-memory` target dep — **decision D5, default remove** (no wasm target exists). Revert is trivial if a web build materializes.
9. Replace `U: AsRef<str>` generics with concrete `&[String]`/`&str` signatures.
10. **Delete all NON-CRITICAL tests** (list in §5). Doing this before the structural phases means the structure changes never touch them.
### Phase 1 — `utils` consolidation
- Add `middle_truncate`; delete `signed_ui`'s private copy and `workspace::truncate_label`; `shorten_pubkey` calls it.
- Move in: `flatten_whitespace`, `url_identity`, `same_repo_url`, `latest`, `sort_newest_first`, `sort_oldest_first`.
- These are drop-in; domain crates already depend on `utils`.
### Phase 2 — `signed_core`
Dependency bottom, consumed by everything else.
1. `RepoAddr`: alias → newtype with `new`, `identifier_from_name`, `Display`/`From`/`FromStr` delegation. Mechanical fallout across all crates (~30 sites).
2. `Nip34Event` extension trait for the tag accessors; `PullRequest<'a>` newtype; `Announcement::forks_in`.
3. `RepoState` (replaces the tuple return; one consumer in `signed_state/repo.rs` refresh).
4. `ThreadResolver<'a>` absorbing the seven inbox/threading helpers.
5. `Filters` unit struct + repo-scoped filters as `RepoAddr` methods; `RepoStatus::resolve` assoc fn.
### Phase 3 — `signed_nostr`
`NostrBackend::open` / `in_memory`; `signed_state::init` constructs it. ~30 lines.
### Phase 4 — `signed_git` → `Repo`
The biggest mechanical phase.
1. Introduce `Repo` wrapping `gix::Repository` with `open`, `open_cached`, `init`, `clone`.
2. Move every function from Appendix A into methods; collapse the 8 duplicate pairs; the `worktree_` prefix disappears (`repo.dirty()`, `repo.snapshot()`, `repo.branches()` …).
3. Fold remote operations into methods (`push_ref`, `push_all`, `fetch_refs`, `origin_url`, `ensure_origin`, `set_origin`, `remote_has_refs`); one git-CLI spawn helper instead of three patterns.
4. One mbox envelope parser shared by `split_patch_series`/`patch_commits` (stays in `signed_git` as an impl on the patch parser, not utils — it is not generic).
5. `GitCache::open`/`ensure_clone` return `Repo`.
6. Extend `Repo::snapshot()` with branches/tags/current_branch so `workspace/src/views/repo/mod.rs:1955` (`load_repo_data`) collapses into it.
7. Keep pure string parsing (`sanitize_path_component`, `fork_namespace`, `split_patch_series`, `patch_diffs`, `patch_commits`) as assoc fns on the owning type (`Repo::sanitize_path_component`, `PatchParser::…`) or move to `utils` if they carry no git semantics — Appendix A marks each.
### Phase 5 — `signed_state`
1. New `push.rs`: `GraspServer` newtype + `GraspPush` + `PushOutcome`/`PushRejection`, moving ~400 lines out of `backend.rs`.
2. Factor the shared announce→authorize-push→retract-on-failure→fan-out orchestration out of `create_repository`/`publish_local_repo`, using `push_repo_from` as the template (~120 lines saved). Add `publish_one` (sign → send → `require_relay_accepted`, returning the `Event`) and collapse its 3 open-coded copies in repo.rs; merge the shadowed `connect_repo_relays` pair.
3. `StatusIndex`, `PatchSeries`, `Mirrors`; `Inbox`/`CheckoutsStore`/`LocalReposStore`/`Backend`/`RepoStore` method moves per §2.
4. Fold `push_repository`/`push_checkout` wrapper bodies into one helper.
5. Consider whether `backend.rs` (~2,020 → ~1,200 lines) still warrants a file split after the push extraction; stop there — the remaining methods are genuinely `Backend`'s job.
### Phase 6 — Comment stripping
Apply §4 across all backend crates. Mechanical; do it last so it never masks a behavioral diff.
### Phase 7 — Final sweep
- `cargo fmt`, `cargo clippy --workspace`, full test run.
- Invariant grep: `grep -rn '^\(pub \)\?\(async \)\?fn ' crates/{signed_core,signed_git,signed_nostr,signed_state}/src` returns only `lib.rs` `init` and test helpers.
- Update `docs/TODO.md` (both entries are consumed by this plan) and this document's status.
---
## 4. Comment policy
Delete by default. Keep only (one line each, unless truly needed):
- **Protocol/format rules**: NIP-34 `u`-tag parsing workaround (model.rs L311 — the SDK's `Nip34Tag` doesn't model the `u` tag); `subject`-tag fallback; `refs/heads` publication form in state events; NIP-10 `e` vs NIP-22 uppercase `E` semantics; kind→root mapping table in inbox.rs L115–124; `comments_for` returning two filters because `#E`+`#e` would AND (filters.rs L109–112); ngit URL parity notes (`grasp_base_url` scheme mapping, `/prs/` namespace, `clone` tag ordering).
- **Safety invariants**: `sanitize_path_component` untrusted-input contract; fast-forward-only branch updates; `checkout_push_status` never fetching checked-out refs.
- **Races / retry taxonomy**: grasp denial classification (backend.rs L1620–1638 — the single most valuable comment in the codebase); stale-advertisement race; same-second resend dedup on `sign_state_event`; convergence probe rationale.
- **Ordering guarantees**: `commits_since` oldest-first (`git am` order); connect-relays-before-send; state-event-authorizes-push; retract-on-failed-push; fan-out-after-landing.
- **Non-obvious `None`/error contracts** where callers depend on them (merge-base `Ok(None)` = unrelated history, unborn HEAD → `None`).
- **Perf "why"**: object-cache rationale in history walks; commit cap for virtual lists.
Delete: every field doc that restates the field name, method summary one-liners, `/// Get the profile.`-style docs, organizational comments, test-helper docs, per-constant restatements, duplicated platform tables in `paths`.
Expected reduction: ~1,030 comment lines across backend crates → roughly 200.
---
## 5. Test disposition
Rule: a test earns its place if breaking the behavior would silently break a user (protocol wire formats, interop with `git`/`ngit`/`nak`, state machines users hit). Delete tests of internal ordering, trivial helpers, and coalescing machinery.
### Keep (critical interop, ~73 tests)
- `signed_core/model.rs`: announcement parsing, malformed-value dropping, `u`-tag fork semantics (3), maintainer authority (2), patch-set chain assembly (3), `latest_update` author scoping (2).
- `signed_core/inbox.rs`: threading/root resolution (5) + `group` thread-merging.
- `signed_core/filters.rs`: both (NIP-22 `K` vs NIP-34 `k`).
- `signed_core/status.rs`, `state.rs`: all (kind-30618 wire format, latest-wins, authority, round-trip).
- `signed_git/tests.rs`: `split_patch_series…`, `patch_commits…`, `parses_real_format_patch_output`, `push_all_mirrors…`, `remote_has_refs…`, `fetch_repo_refs_imports_heads_under_a_prefix`, `repo_ref_state_lists…`, `root_commit_reports…`, `blocks_parent_components`, `merge_base_finds…`, `working_copy_cloned_from_the_mirror…`, `head_commit_and_commits_since…` (pins `git am` order), plus `fast_forward_branches_moves_the_mirror_and_keeps_local_work` — kept as a safety invariant, not structure.
- `signed_git/nip34.rs`: all 10 (on-disk marker interop with `nak`/`ngit`).
- `signed_state/backend.rs`: the 6 URL-format/denial-classification tests.
- `signed_state/repo.rs`: all 3 (format-patch header, NIP-22 comment builder, maintainer filters).
- `signed_state/checkouts.rs`: `same_repo_url…`, `checkout_status_reports_ahead_branches_only`, `checkout_push_status_counts_unpushed_commits_only`.
- `signed_state/local_repos.rs`: the 2 sidebar-contract tests (own announcement dropped / other owner's linked).
- `settings`: `save_and_load_roundtrip`, `partial_json_merges_with_defaults` (breaking these silently resets user settings).
### Delete (~38 tests, ~1,100 lines)
- `signed_git/tests.rs` (16): scan walker, `init_repository` contents, `set_origin`, `worktree_last_commits` (2), `all_commits_lists_every_commit`, `find_readme_prefers_markdown`, `current_branch_tracks_checkout`, `worktree_snapshot_reflects…`, diff display (4), `worktree_dirty…`, `worktree_commits_ahead…`.
- `signed_state` (10): `push_outcome_reports_partial_failures`, `resolve_orders_remembered_freshest_first`, `resolve_deduplicates_paths…`, the 4 `local_repos` trivial pass-through/name tests, all 4 `refresh.rs` coalescing tests.
- `signed_core` (9): the 4 `InboxReadState` policy tests, 2 `fork_candidates` ordering/policy tests, 3 `utils` formatting tests.
- `settings` (3): keep only the 2 named above.
Consolidate retained helpers while there: three git-env helpers → one, five bare-server `git init --bare` blocks → one fixture, delete the inline `run` closure at tests.rs L634.
Out of scope but flagged: `signed_ui/pixel_avatar` (identicon determinism — borderline, visual), `dock/tests/render_smoke.rs` (UI smoke).
---
## 6. Risks
- **`PullRequest<'a>` lifetimes** at `pull_requests/detail.rs` call sites — mitigated by keeping it a borrow newtype, not an owning copy.
- **`Repo` wrapper vs background tasks**: `RepoStore` spawns git work via `cx.background_spawn`. The wrapper must stay `Send` when moved into tasks exactly as `gix::Repository` is today; no interior mutation planned, so this is mechanical.
- **Phase 2 `RepoAddr` newtype** touches ~30 call sites in one commit — small each, many files; consider `impl From<Coordinate>` bridges to keep the diff mechanical.
- **Backend push orchestration dedup** (Phase 5.2) is the only phase that reorders logic; land it alone in its own commit behind the existing tests (URL-format + denial-classification + `push_all`/`fetch_repo_refs` interop tests are the net).
## 7. Open decisions
| # | Question | Default |
|---|---|---|
| D1 | Delete the empty `import_dialog` stub + menu entry, or keep the shell? | Keep shell (UI territory, 0 cost) |
| D2 | `SyncProgress` pipeline | Delete; re-add with the feature |
| D3 | `BackendEvent::Error` (9 emitters, 0 subscribers) | Wire one workspace subscriber |
| D4 | `merge_pull_request` (unwired feature) | Delete; recover from history when merge UI is built |
| D5 | wasm32 code paths + `nostr-memory` dep | Remove (no wasm target in workspace) |
| D6 | `paths`: keep as a second function-crate, or fold into `utils`? | Keep separate (no `nostr` dep for path users) |
---
## Appendix A — `signed_git` function → `Repo` method mapping
| Current free fn | Becomes |
|---|---|
| `merge_base`, `head_commit_id`, `commits_since`, `root_commit`, `refs_with_prefix`, `delete_refs_with_prefix` | `Repo::merge_base`, `head`, `commits_since`, `root_commit`, `refs_with_prefix`, `delete_refs_with_prefix` |
| `init_repository` | `Repo::init` |
| `worktree_current_branch`/`current_branch` | `Repo::current_branch` |
| `worktree_ref_exists` | `Repo::ref_exists` |
| `worktree_branches`/`repo_branches` | `Repo::branches` |
| `worktree_ref_state`/`repo_ref_state` | `Repo::ref_state` |
| `repo_tags` | `Repo::tags` |
| `fast_forward_branches` | `Repo::fast_forward_branches` |
| `worktree_checkout_branch`/`worktree_checkout_tag` | `Repo::checkout_branch` / `checkout_tag` |
| `worktree_snapshot` | `Repo::snapshot` (+ branches/tags/current_branch, absorbing `load_repo_data`) |
| `worktree_dirty` | `Repo::is_dirty` |
| `worktree_commits_ahead` | `Repo::commits_ahead` |
| `worktree_all_commits`/`all_commits` | `Repo::all_commits` |
| `worktree_last_commits`/`last_commits` | `Repo::last_commits` |
| `worktree_commit_range_commits` | `Repo::commit_range` |
| `worktree_commit` | `Repo::commit` |
| `worktree_commit_diff`/`commit_diff` | `Repo::commit_diff` |
| `worktree_commit_range_diff` | `Repo::range_diff` |
| `worktree_entries`, `worktree_read`, `find_readme` | `Repo::entries`, `read`, `find_readme` |
| `clone_repo` | `Repo::clone` |
| `push_commit_ref`, `push_main`, `push_all` | `Repo::push_ref`, `push_main`, `push_all` |
| `remote_has_refs` | `Repo::remote_has_refs` |
| `ensure_origin`, `set_origin`, `origin_url` | `Repo::ensure_origin`, `set_origin`, `origin_url` |
| `fetch_repo_refs`, `fetch_all` | `Repo::fetch_refs`, `fetch` |
| `apply_patch`, `format_patch_between` | `Repo::apply_patch`, `format_patch_between` |
| `detect_nip34`, `set_nostr_repo` | `Repo::nip34_binding`, `set_nostr_repo` |
| `find_git_repos` | stays free in `scan.rs` (directory-tree scan, no repo opened) — acceptable exception, or `utils` |
| `sanitize_path_component`, `fork_namespace` | assoc fns on `GitCache` (they build its layout) |
| `split_patch_series`, `patch_diffs`, `patch_commits` | assoc fns on a small `PatchParser` impl (pure parsing, no `Repo`) |
| `is_grasp_url` + private nip34 url helpers | private assoc fns of the NIP-34 detection impl |
| `repository_signature`, `open_with_cache`, `resolve_commit`, `move_head`, `force_checkout`, `edit_local_config` | private methods/assoc fns on `Repo` |