fix: apply appendable outside the value codec so append/extend work - #3
Merged
Merged
Conversation
`ChromaDocuments` and `ChromaUris` stacked `appendable` *inside* the
`ValueCodecs.single_nested_value` wrapper. dol's class-wrapping then
re-installs `append`/`extend` as delegated attributes bound to the
un-codec'd leaf store, so appended values skipped the codec and reached
`ChromaCollection.__setitem__` raw -- which unpacks them as `**v`:
TypeError: Collection.upsert() argument after ** must be a mapping, not str
Swapping the two decorators puts `appendable` outermost, so `append` and
`extend` now write the same values `__setitem__` writes. A comment records
the ordering constraint so it doesn't get flipped back.
Adds `AppendableChromaCollection` as the escape hatch for auto-keyed
appends of raw chromadb kwargs (what `ChromaDocuments.append` accidentally
did for mapping arguments before, minus the codec inconsistency).
Verified every other observable surface is byte-identical before/after
(name/MRO/dir, get/set/del/len/contains/iter/items/values, `.collection`,
`.store`). Top-level `chromadol` exports are unchanged.
Closes #2
Claude-Session: https://claude.ai/code/session_01L1aQPB34n7PU7jmbztSjBe
thorwhalen
added a commit
that referenced
this pull request
Sep 22, 2026
base.py defined `ChromaUris` twice; the second (codec field "metadata", which is not a chromadb kwarg) replaced the first, so every ChromaUris write/append raised TypeError. #3's test covered "ChromaUris" only through a fake that accepts any kwarg, so it passed. The second class is now `ChromaMetadatas` on the real "metadatas" field, with no `append` since chromadb refuses an upsert that has metadata but no documents. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Member
Author
|
Post-merge refute review: the |
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
ChromaDocuments/ChromaUrisstacked@appendable(...)inside@ValueCodecs.single_nested_value(...). dol's class-wrap re-installs the innerappend/extendasDelegatedAttributes bound to the unwrapped leaf store, so.append/.extendwrote raw values straight toChromaCollection.__setitem__, which expects a mapping — every call raisedTypeError. The feature was entirely dead (its own demo was commented out in__init__.pyand the old test file). Root cause: i2mint/dol#18, this repo's instance: #2.Fix: apply
appendableoutside the value codec on all three field-scoped classes, soappend/extendgo through the wrapper's__setitem__(and therefore the codec) likestore[k] = valready did. AddedAppendableChromaCollectionfor callers who want to append rawchromadbkwargs mappings instead of a single field's value.No dependents of
chromadolin the fleet manifest. Not a breaking change in practice —.append/.extendraised unconditionally before this fix, so nothing could have been relying on their old behaviour.(Pre-existing, unrelated bug noticed in passing and left untouched: the two decorator stacks for
urisandmetadataboth define a class namedChromaUrisat module scope, so the second silently shadows the first —chromadol.base.ChromaUriscurrently resolves to the metadata-field class, and the uris-field class is unreachable by name. Out of scope for this branch; happy to file a follow-up if wanted.)Test plan
wads ci-local: ruff format + lint, pytest on py3.10/py3.12 (7 passed, incl. 2 new tests pinning append/extend go through the codec andAppendableChromaCollectiontakes raw kwargs), build — all green.Closes #2.
🤖 Generated with Claude Code