Repository navigation
get_size() resolves the leaf key when wrapped with a dol key codec - #3
Merged
Merged
Conversation
HfMapping.get_size() forwarded its key argument straight to the Hugging Face API. __getitem__ and the other dunders correctly see the leaf key when a dol key codec (wrap_kvs, KeyCodecs.prefixed, ...) wraps the instance -- dol delegates dunders to the wrapped store -- but get_size, a non-dunder method, received the outer, unmapped key. Latent today (hfdol has no built-in wrapping), but a correctness trap the moment a caller wraps one of these Mappings, which for a package of dict-like classes is a natural thing to do. Adds `_leaf_key`, which resolves through dol.inner_most_key / wrapped_self when dol is installed and the instance is wrapped, and is a no-op otherwise (import is lazy; dol stays an optional, undeclared dependency, matching current pyproject.toml). Verified against all four shapes from the issue's own repro: unwrapped, class-wrap (KeyCodecs.prefixed), instance-wrap (wrap_kvs), and the plain unwrapped case (regression check) -- all route the correct repo id to the (stubbed) Hugging Face API. Closes #2 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Covers the 3 scenarios from #2's own repro (class-wrap via KeyCodecs.prefixed, instance-wrap via wrap_kvs, and the unwrapped case as a regression guard), with the HF API call stubbed so it runs offline. Adds dol to the `testing` extra since the test needs it (dol itself stays a lazy, undeclared runtime dependency of the package -- see _leaf_key's docstring). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
HfMapping.get_size()forwarded itskeyargument straight to the module-levelget_size(), with no leaf-key resolution.__getitem__/__contains__/etc. correctly see the leaf key when the instance is wrapped with adolkey codec (dunders are delegated to the wrapped store by construction);get_size, a non-dunder method, received the outer, unmapped key instead.This was latent, not live — hfdol ships no key-transform wrapping itself — but it becomes a real bug the moment a caller wraps one of these
Mappings, which is a natural thing to do for a package whose classes arecollections.abc.Mappings.Fix
Adds
_leaf_key(self, key), which:dol.wrapped_self/dol.inner_most_key(dol stays undeclared/optional — nothing else in hfdol needs it, matching currentpyproject.toml);keyunchanged on any import/resolution failure — including the unwrapped case, which is the common path and must not regress.Uses
default=Noneoninner_most_keybecause i2mint/dol#84 changed unresolvable lookups from returningNoneto raisingAttributeError— this repo doesn't pin a dol version, so the code has to be correct against both behaviors.Verification
Ran the exact 4-shape repro from #2 (network calls stubbed) against this fix:
s['music_genre'](dunder, was already correct)KeyCodecs.prefixed)get_size— now sends the correct leaf keywrap_kvs)get_size— now sends the correct leaf keyget_size— unaffected (regression check)All four pass. Also ran
pytest --doctest-modules— 1 passed.Breaking-change check
Checked
_tmp/fleet_dependents.jsonper the AGENT_BRIEF:hfdolhas no dependents on this box.Closes #2
🤖 Generated with Claude Code
https://claude.ai/code/session_011HSBVhDjRU4apSLcRkavv9