Skip to content

Cache failing callable defaults too in _get_default_cached#1965

Merged
dmontagu merged 1 commit into
feature/variable-composition-native-handlebarsfrom
feature/variable-composition-cache-failures
May 25, 2026
Merged

Cache failing callable defaults too in _get_default_cached#1965
dmontagu merged 1 commit into
feature/variable-composition-native-handlebarsfrom
feature/variable-composition-cache-failures

Conversation

@dmontagu
Copy link
Copy Markdown
Contributor

Addresses cubic r3296066209 on #1954.

_get_default_cached only memoised successful values, so a callable default that raises escapes the cache — first invocation raises before the assignment, subsequent invocations re-invoke. Worst case: three invocations in one get() (in _get_serialized_default, _resolve_code_default, and the outer-except fallback in _resolve).

Switched the cache to record (ok: bool, value_or_exception: Any). Success caches the value as before; failure caches the exception so re-entry re-raises without re-invoking. Matches cubic's suggested patch.

Test

New test_failing_callable_default_invoked_once_per_get registers an always_raises callable default, calls get(), asserts call_count == 1. Without the fix the count was 3.

513 tests pass; pyright + ruff format + check clean.

Cubic r3296066209: the cache only memoised successful values, so a
callable default that raises escapes the cache — first invocation
raises before the assignment, subsequent invocations re-invoke. In the
worst case a failing callable could fire three times in one `get()`
(in `_get_serialized_default` → `_resolve_code_default` → outer-`except`
fallback in `_resolve`).

Switched the cache to record `(ok: bool, value_or_exception: Any)`. A
successful invocation caches the value as before; a raising invocation
caches the exception so re-entry re-raises without re-running the
callable.

New regression test `test_failing_callable_default_invoked_once_per_get`
registers an `always_raises` callable default, calls `get()`, and
asserts `call_count == 1`. (Without the fix the count was 3.)
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Re-trigger cubic

@dmontagu dmontagu merged commit f3e694c into feature/variable-composition-native-handlebars May 25, 2026
15 checks passed
@dmontagu dmontagu deleted the feature/variable-composition-cache-failures branch May 25, 2026 04:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant