Skip to content

core: one context struct for the (conn, root, idgen, cfg, embedder) bundle threaded through 15 functions in three orders #134

Description

@samkeen

Found during the pure-refactor pass on claude/rust-codebase-refactor-7ot3v1; deferred there because it's a large, test-touching diff that deserves its own PR — this issue is the map.

The smell

Fifteen function definitions in b2-core thread the same 4–5 values, in three incompatible parameter orders:

order functions
conn, vault_root, path, idgen, cfg[, embedder] ingest::ingest_file, ingest::project_file
conn, vault_root, idgen, cfg[, embedder] ingest::project_vault, ingest::ingest_vault_with_progress, rm::reproject_dangled
conn, idgen, cfg[, embedder], vault_root mv::move_note / move_resource / move_dir, add::add_note / create_note, rm::delete_note / delete_resource / delete_dir

Roughly 59 call sites carry the bundle (13 in vault.rs, ~30 in crates/b2-core/tests/), and four #[allow(clippy::too_many_arguments)] (add.rs ×2, mv.rs ×2) exist purely to silence it. rm::reproject_dangled even uses a different order from the three public fns in its own file.

The shape of the fix

Two context structs, split along the model-free vs. embedding posture the module docs already document (rm/create_note/write are model-free; mv/add/link embed):

struct ProjectionCtx<'a> {
    conn: &'a Connection,
    root: &'a Path,
    idgen: &'a dyn IdGen,
    cfg: &'a ChunkConfig,
}
struct EmbedCtx<'a> {
    proj: ProjectionCtx<'a>,
    embedder: &'a dyn Embedder,
}

Vault builds them via fn ctx(&self) / fn embed_ctx(&self), so a façade call collapses to e.g. mv::move_note(self.embed_ctx(), &b2id, &old_rel, to). The type system then encodes the posture — a model-free op cannot receive an embedder — which today is doc-enforced only. Deletes all four clippy allows and the ordering hazard.

Short-lived, Copy-able borrow-only view structs passed into calls and never stored are the sanctioned exception to the "prefer owned fields" rule (CLAUDE.md, Rust data modeling — the NoteRow precedent).

Riders worth taking in the same PR (same files, same review)

From the same survey of mv.rs:

  • the move "preflight dance" (normalize → same-path check → occupied-target check → create_dir_all + rename) is written three times (move_note/move_resource/move_dir);
  • the "rewrite inbound files" loop is written three times, with move_dir inventing an let empty = BTreeMap::new() sentinel;
  • the wikilink .md-convention replacement and the fragment-preserving resource replacement are each computed in two places — as pure functions they'd finally get direct unit tests (the #fragment logic currently has none).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions