fix(library): surface failed sub-part resolutions in ResolutionResult

[bkfunk]

Summary

resolve_dependencies_multipart (and resolve_dependencies) used to filter the resolver's internal state down to successfully-resolved entries and silently drop everything else, including failed resolutions. The only signal a caller had for a failed sub-part was the on_update callback — and because that callback is Fn, accumulating errors from it required interior mutability (RefCell / Mutex / channel).

In practice, that meant the easiest, most obvious callback (&|_, _| {}) caused resolution failures to vanish entirely. The resulting ResolutionResult would then feed into bake_part_from_* and produce a part with missing geometry, with no error and no panic.

Repro

let resolutions = resolve_dependencies_multipart(
    &doc, cache, &colors, &loader,
    &|_, _| {},                            // <- silent
).await;
let part = bake_part_from_multipart_document(&doc, &resolutions, false);
// `part` may be silently incomplete; nothing flags it.

Change

1. Add failures: HashMap<PartAlias, ResolutionError> to ResolutionResult, populated from previously-discarded Missing states. Exposed via:

   • pub fn failures(&self) -> &HashMap<PartAlias, ResolutionError>
   • pub fn has_failures(&self) -> bool

2. ResolutionState::Missing now carries the original ResolutionError. ResolutionError is not Clone (it contains IoError and ReqwestError), so the error has to live somewhere — and the previously-dropped state map is the natural place.

3. on_update callback signature changes from Fn(PartAlias, Result<(), ResolutionError>) to Fn(PartAlias, Result<(), &ResolutionError>). Small breaking change, but justified: the error now lives in the state map, and callers that want to inspect it via the callback receive a borrow instead of moving ownership. All in-tree callers either passed a no-op closure (source-compatible) or used Display::fmt, which works the same against &ResolutionError. tools/viewer/common::set_document propagates the callback type and is updated accordingly.

4. Regression test exercising the silent-failure path with a no-op callback, using a stub LibraryLoader that always returns FileNotFound.

Incidental fix

cargo test -p ldraw did not build on a stock checkout — parser.rs uses #[tokio::test] but the workspace's tokio dep doesn't enable the macros/rt features. Added the minimal dev-dep needed (tokio = { workspace = true, features = ["macros", "rt"] }) so the existing tests compile alongside the new one. 22 tests pass after this change.

Test plan

• cargo check --workspace passes
• cargo test -p ldraw --lib passes (22 tests, including the new one)
• Stub-loader regression test (resolution_result_surfaces_missing_subpart_with_noop_callback) confirms has_failures() and failures() populate when a sub-part is unresolvable, even with a no-op on_update

[Copilot]

Pull request overview

This PR improves dependency resolution reporting in the ldraw library by preserving failed sub-part resolutions and exposing them to callers via ResolutionResult, preventing silent baking of incomplete geometry when callers use a no-op on_update callback.

Changes:

• Extend ResolutionResult with a failures: HashMap<PartAlias, ResolutionError> plus failures() / has_failures() accessors, populated from resolver Missing states.
• Update resolution tracking so ResolutionState::Missing carries the underlying ResolutionError, and change the on_update callback to pass Result<(), &ResolutionError>.
• Add a regression test for the “no-op callback hides missing sub-part” scenario and add a minimal tokio dev-dependency feature set so existing #[tokio::test] tests compile.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
tools/viewer/common/src/lib.rs	Propagates the updated on_update callback signature (&ResolutionError).
ldraw/src/library.rs	Stores resolution failures in state and surfaces them via ResolutionResult::failures; adds partition helper + regression test.
ldraw/Cargo.toml	Adds non-wasm dev-dependency tokio features needed for #[tokio::test] compilation.

Comments suppressed due to low confidence (1)

ldraw/src/library.rs:144

• ResolutionState is a pub enum, and this change both makes Missing carry data and removes the Clone derive. That’s a breaking public-API change in addition to the on_update signature update. If ResolutionState isn’t intended to be part of the public API, consider reducing its visibility (e.g. pub(crate)), otherwise consider keeping it Clone by storing the error behind Arc/Box (e.g. Missing(Arc<ResolutionError>)) so downstream code can still clone states.

#[derive(Debug)]
pub enum ResolutionState {
    Missing(ResolutionError),
    Pending,
    Subpart,
    Associated(Arc<MultipartDocument>),
}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.