crabidy/quality/node-editing.md

4.8 KiB

Quality gates — editable and deletable nodes

Checklist for the implement stage. Automatic tests live in crabidy-core/src/lib.rs, tidaldy/src/lib.rs, and cbd-tui/src/app/{bindings,mod}.rs test modules; run with devenv shell -- cargo test --workspace (network-dependent tidaldy tests stay #[ignore]). 10 tests fail at gate-writing time — they define the target.

Contract & wire

  • Proto changes are additive only: existing field numbers untouched; LibraryNodeChild.is_editable = 5, is_deletable = 6; RenameLibraryNode and DeleteLibraryNode rpcs present. Old clients keep working against the new server.
  • rpc.rs (server) maps errors per the documented contract for both new rpcs: NotSupportedfailed_precondition, InvalidInputinvalid_argument, everything else → internal. No color-eyre report or debug formatting of internals leaks into Status messages.
  • ProviderOrchestrator::{rename,delete}_lib_node route /tidal-prefixed paths to the Tidal client and answer anything else (including the synthetic root /) with NotSupported — same prefix discipline as create_lib_node.
  • Both new commands follow the existing bounded(1)-reply rendezvous pattern; no new unbounded channels anywhere in the feature.

Provider semantics (tidaldy)

  • rename_lib_node trims the new title; empty/whitespace → InvalidInput; any path that is not a SearchTerm (including /tidal/search itself) → NotSupported; validation happens before any network call.
  • Rename replaces the term in place (keeps its list position); renaming onto an existing term merges (the old slot is removed, no duplicates ever); renaming an unknown term registers the new one (stale-client forgiveness). Returns get_lib_node(new_path) — the node at its new percent-encoded path.
  • delete_lib_node accepts only SearchTerm paths (NotSupported otherwise), removes the term idempotently (unknown term → success), and returns the refreshed /tidal/search parent node.
  • rename_search_term / remove_search_term follow the established lock discipline: poison-tolerant, the search_terms lock is never held across an await.
  • Term children returned by the Search arm set is_editable: true, is_deletable: true; nothing else in the provider sets either flag.
  • Queued search tracks keep playing after a rename/delete of their term: get_urls_for_track / get_metadata_for_track resolve from the track id embedded in the path, independent of term registration.

No panics on user input (hard rule)

  • All todo!() stubs from api-design are gone (grep the workspace).
  • Rename/delete failures (network, auth, malformed paths from stale clients) surface as ProviderError/Status, never a panic; the TUI's rename and delete paths handle an error reply without crashing the orchestrate task (log + stay put).

TUI behavior

  • e opens the overlay only when the selected item is_editable, prefilled with the current title; d sends DeleteNode only when the selected item is_deletable; both are silent no-ops otherwise (including when nothing is selected or the list is empty).
  • The overlay carries its purpose: submit sends CreateNode for InputPurpose::Create and RenameNode for InputPurpose::Rename; the rendered label distinguishes them (new node: vs rename:). While the overlay is open the bindings table stays unreachable (existing input-mode bypass, unchanged).
  • Delete is deliberately unconfirmed (architecture/node-editing.md D4); the open question about confirmation for higher-value nodes is preserved in the architecture doc, not silently dropped.
  • Modifiable children are visibly marked in the library list ([e], [d] or [ed] suffix, COLOR_SECONDARY), coexisting with the [%] marker for creatable ones.
  • RpcClient::rename_library_node evicts the old path and the parent entry, then caches the node under its new path; delete_library_node evicts the deleted path and the parent, then caches the returned parent. No stale /tidal/search listing can resurrect an old term.
  • On successful rename the library navigates into the renamed node; on successful delete it shows the refreshed parent; on failure the library stays where it was.

Code quality

  • Public items added in all five crates have doc comments matching final behavior; the e/d bindings appear in the help modal automatically.
  • No new dependencies.
  • devenv shell -- cargo fmt --check, cargo clippy --workspace (no new warnings), cargo test --workspace all pass.