Severity: ⚪ low • Category: untested-path
Location: src/store.rs : 380
What's wrong
On a cache miss the wrapped function is invoked with let result = self.fn_obj.bind(py).call(args, kwargs.as_ref())?.unbind();. If it raises, the ? propagates the error and the slow-path insert is skipped, so the exception must NOT be cached and a subsequent call must re-invoke the function. The entire memory and shared test suites only ever cache functions that succeed. The only error-path coverage is test_async_single_flight_error_recovery, which exercises the async wrapper's coalescing logic, not the underlying Rust miss path. A regression where a raising call poisons the cache (e.g. stores a sentinel, or the slow path partially inserts) would pass all current tests.
Suggested fix
Add tests for memory and shared backends: decorate a fn that raises ValueError on first call (counter-guarded), assert pytest.raises(ValueError), assert cache_info().current_size == 0, then assert a later successful call recomputes (call_count increments) and is then cached. Repeat for an async-decorated fn to confirm the leader's exception isn't stored.
Adversarial verification note
The factual claims in the finding all check out. src/store.rs:380 is let result = self.fn_obj.bind(py).call(args, kwargs.as_ref())?.unbind(); — the ? propagates any exception before the SLOW PATH write-lock/insert block (store.rs:382-409), so a raising call correctly does NOT poison the cache. Across the test suite, no memory-backend (tests/test_basic.py) or shared-backend (tests/test_shared_basic.py) test decorates a function that raises and asserts (a) the exception propagates, (b) current_size stays 0, and (c) a later success recomputes and caches. The only except/raise matches are: tests/test_async.py:289 test_async_single_flight_error_recovery (exercises the AsyncCachedFunction single-flight coalescing path, not the Rust sync miss path), and tests/test_stress.py:143 which is just an error-collection except Exception around a stress test of a non-raising fn(x): return x + 1. So the coverage gap genuinely exists. However: (1) this is a missing-test gap, not a code bug — the underlying behavior is correct, being the natural consequence of ? propagation; (2) accidentally regressing it would require actively writing code to catch+cache exceptions, which is unlikely. The reviewer's claimed severity of 'high' is overstated per the rubric (high = wrong results/deadlock under realistic use); since the code is correct and only the test is absent, this is low.
Verifier correction: The finding is valid as a test-coverage gap, but it is not a 'high' issue: the cited code at store.rs:380 is correct and the exception-not-cached behavior already works via ? propagation skipping the insert block. It should be classified as a low-severity untested-path, not high.
Filed from a multi-agent code review (finder → adversarial verification → synthesis). Confirmed real after a skeptic re-read the code.
Severity: ⚪ low • Category: untested-path
Location:
src/store.rs: 380What's wrong
On a cache miss the wrapped function is invoked with
let result = self.fn_obj.bind(py).call(args, kwargs.as_ref())?.unbind();. If it raises, the?propagates the error and the slow-path insert is skipped, so the exception must NOT be cached and a subsequent call must re-invoke the function. The entire memory and shared test suites only ever cache functions that succeed. The only error-path coverage istest_async_single_flight_error_recovery, which exercises the async wrapper's coalescing logic, not the underlying Rust miss path. A regression where a raising call poisons the cache (e.g. stores a sentinel, or the slow path partially inserts) would pass all current tests.Suggested fix
Add tests for memory and shared backends: decorate a fn that raises ValueError on first call (counter-guarded), assert pytest.raises(ValueError), assert cache_info().current_size == 0, then assert a later successful call recomputes (call_count increments) and is then cached. Repeat for an async-decorated fn to confirm the leader's exception isn't stored.
Adversarial verification note
The factual claims in the finding all check out. src/store.rs:380 is
let result = self.fn_obj.bind(py).call(args, kwargs.as_ref())?.unbind();— the?propagates any exception before the SLOW PATH write-lock/insert block (store.rs:382-409), so a raising call correctly does NOT poison the cache. Across the test suite, no memory-backend (tests/test_basic.py) or shared-backend (tests/test_shared_basic.py) test decorates a function that raises and asserts (a) the exception propagates, (b) current_size stays 0, and (c) a later success recomputes and caches. The onlyexcept/raisematches are: tests/test_async.py:289test_async_single_flight_error_recovery(exercises the AsyncCachedFunction single-flight coalescing path, not the Rust sync miss path), and tests/test_stress.py:143 which is just an error-collectionexcept Exceptionaround a stress test of a non-raisingfn(x): return x + 1. So the coverage gap genuinely exists. However: (1) this is a missing-test gap, not a code bug — the underlying behavior is correct, being the natural consequence of?propagation; (2) accidentally regressing it would require actively writing code to catch+cache exceptions, which is unlikely. The reviewer's claimed severity of 'high' is overstated per the rubric (high = wrong results/deadlock under realistic use); since the code is correct and only the test is absent, this is low.Filed from a multi-agent code review (finder → adversarial verification → synthesis). Confirmed real after a skeptic re-read the code.