core: is_unparseable_findings is a single magic-type downcast with no compiler-enforced exhaustiveness

[MaximeGaudin]

Summary

engine::run distinguishes a 'hard failure that should abort the run' from a 'soft failure that becomes a per-skill note' purely by calling llm::is_unparseable_findings(&failure), which does failure.downcast_ref::<UnparseableFindings>().is_some() — this works correctly today (verified: anyhow::Error::downcast_ref does find a wrapped concrete type even after .context(...) is layered on top, confirmed against a synthetic anyhow probe), but nothing in the type system requires every future model-pass failure path to actually construct/propagate UnparseableFindings rather than an ad hoc anyhow!(...)/bail!(...).

Scope

• Severity (reporter's assessment): low
• Area: arch
• Code: packages/core/src/llm/review.rs:217-236, packages/core/src/llm/review.rs:343-382, packages/core/src/engine.rs:265-302
• Reproduced empirically: yes

Minimal reproduction

Empirical check that the downcast survives added context (not slint code, a synthetic probe confirming anyhow's own behavior): built a tiny crate with struct Marker implementing std::error::Error, created let wrapped = Error::new(Marker("x")).context("extra context added later"), then called wrapped.downcast_ref::<Marker>() — result was Some, i.e. downcast survives one layer of added context. Then in slint: review_once (llm/review.rs:343-382) returns Err(UnparseableFindings { detail: ... }.into()) on the specific 'still not JSON after one retry' path, and engine::run (engine.rs:276-278) checks llm::is_unparseable_findings(&failure) on the direct Err from llm::review(one, config) with no intervening .context() call today.

Expected

If the hard-fail/soft-fail distinction is meant to be a permanent, load-bearing part of the contract, it should be expressed as a typed error enum (enum ReviewError { Unparseable(...), Unreachable(...) }) matched exhaustively, so the compiler forces every new failure branch in llm/review.rs or llm/provider.rs to declare which bucket it belongs to, rather than relying on every future contributor remembering to construct exactly this one struct.

Sources

• https://docs.rs/anyhow/latest/anyhow/struct.Error.html#method.downcast_ref — documents downcast_ref's chain-aware behavior, which the probe above corroborates empirically

Notes

Filed as low severity because the mechanism is verified sound as implemented today; the risk is purely forward-looking (a future contributor adding a new error path with anyhow!(...) directly, which would silently degrade a should-be-hard-fail into a soft note with no test catching it, since no test asserts hard-fail behavior for any path other than the two 'still not json' fakes already in review.rs's test module).

---

Filed as part of a systematic pre-release audit. Triage and de-duplication pending.