Skip to content

fix: raise ValueError on empty base_state in apply_sparse_delta#67

Open
Grizouforever wants to merge 1 commit into
one-covenant:mainfrom
Grizouforever:fix/empty-base-state-crash
Open

fix: raise ValueError on empty base_state in apply_sparse_delta#67
Grizouforever wants to merge 1 commit into
one-covenant:mainfrom
Grizouforever:fix/empty-base-state-crash

Conversation

@Grizouforever
Copy link
Copy Markdown

@Grizouforever Grizouforever commented Apr 11, 2026

Summary

Root Cause

In grail/infrastructure/delta_checkpoint.py line 137, dtype is inferred by calling next(iter(base_state.values())). When base_state is an empty dict, this raises StopIteration which propagates uncaught to callers in checkpoint_consumer.py (lines 439-444 and 1315-1319) that pass target_dtype=None.

Fix

if target_dtype is None:
    if not base_state:
        raise ValueError("Cannot infer target_dtype: base_state is empty")
    target_dtype = next(iter(base_state.values())).dtype

Test plan

  • All 24 existing test_delta_checkpoint.py tests pass
  • New test test_empty_base_state_raises_value_error confirms ValueError is raised
  • New test test_empty_base_state_with_explicit_dtype confirms empty base_state + explicit dtype returns {}

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes

  • Enhanced error handling for sparse delta operations when base state is empty and no target data type is specified. Users now receive a clear, descriptive error message instead of an obscure inference failure.

…ence

When target_dtype is None and base_state is an empty dict, calling
next(iter(base_state.values())) raises StopIteration which propagates
uncaught to callers in checkpoint_consumer.py. Add an explicit check
that raises a clear ValueError instead.

Also add two regression tests covering the empty base_state path.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 11, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8f0d3e79-6783-4558-b6d3-0c1b6b030255

📥 Commits

Reviewing files that changed from the base of the PR and between 54d6342 and 9b75820.

📒 Files selected for processing (2)
  • grail/infrastructure/delta_checkpoint.py
  • tests/unit/infrastructure/test_delta_checkpoint.py

Walkthrough

A bug fix that adds validation to apply_sparse_delta() to raise a clear ValueError when base_state is empty and target_dtype is None, preventing unhandled StopIteration exceptions.

Changes

Cohort / File(s) Summary
Bug Fix — Empty State Validation
grail/infrastructure/delta_checkpoint.py
Added early validation in apply_sparse_delta() to check if base_state is empty when target_dtype is None. Raises ValueError("Cannot infer target_dtype: base_state is empty") instead of allowing StopIteration to propagate.
Unit Tests
tests/unit/infrastructure/test_delta_checkpoint.py
Added two tests: one verifying that empty base_state with target_dtype=None raises the expected ValueError, and another confirming that explicit target_dtype handles empty delta gracefully.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A bug squashed clean, a StopIteration no more,
When base_state is bare, we know what's in store!
A ValueError whispers, clear as the morning dew,
No more crashing surprises—the fix shines bright and true! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'fix: raise ValueError on empty base_state in apply_sparse_delta' clearly and concisely describes the main change—adding validation to raise ValueError instead of allowing StopIteration on empty base_state.
Linked Issues check ✅ Passed The changes fully address issue #66's requirements: they add explicit validation that raises ValueError('Cannot infer target_dtype: base_state is empty') when base_state is empty and target_dtype is None, with regression tests confirming the fix works and that explicit dtype still returns empty dict.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing issue #66: validation logic in apply_sparse_delta and two regression tests. No unrelated modifications to other components or unnecessary refactoring detected.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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.

Bug: apply_sparse_delta crashes with StopIteration when base_state is empty and target_dtype is None

1 participant