Skip to content

fix(backends): repair missing _type in collection config (#1611) - #1617

Merged
igorls merged 2 commits into
MemPalace:developfrom
mvalentsev:fix/1611-config-json-str-type
May 30, 2026
Merged

fix(backends): repair missing _type in collection config (#1611)#1617
igorls merged 2 commits into
MemPalace:developfrom
mvalentsev:fix/1611-config-json-str-type

Conversation

@mvalentsev

@mvalentsev mvalentsev commented May 25, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

chromadb <= 1.5.8 writes config_json_str = '{}' (empty JSON) when creating
collections. chromadb 1.5.9 introduced a strict _type check in the
collection config deserialization path -- its absence raises
KeyError: '_type' on palace open. Since the pin allows >=1.5.4,<2,
any uv tool upgrade mempalace pulls 1.5.9 and breaks every existing palace.

This adds a fourth pre-open migration step (_fix_missing_collection_type)
that injects "_type": "CollectionConfigurationInternal" into
collections.config_json_str rows that lack it. Same lifecycle and marker-file
pattern as the existing _fix_blob_seq_ids.

  • Backward-compatible: the extra JSON key is ignored by chromadb <= 1.5.8
  • Idempotent: marker file .collection_type_fixed skips sqlite on repeat opens
  • Defensive: skips malformed JSON and non-dict configs without aborting

Closes #1611
Related: #445 (same root error, SQL fix recipe by @nautis)

How to test

  • 9 new unit tests: adds _type, preserves existing, noop without db,
    marker write/skip, noop-marker when valid, multi-collection mixed
    (NULL + empty + valid), non-dict JSON, malformed JSON
  • 3 existing call-sequence tests updated to include the new step
  • E2E: create palace, verify config_json_str = {}, reopen via
    ChromaBackend._client(), confirm fix applied, query returns results
  • Backward compat: palace with injected _type opens on chromadb 1.5.7/1.5.8,
    add + query work
uv run pytest tests/test_backends.py -v -k collection_type
uv run pytest tests/ -v --ignore=tests/benchmarks

Checklist

  • Tests pass (python -m pytest tests/ -v)
  • No hardcoded paths
  • Linter passes (ruff check .)

)

chromadb <= 1.5.8 writes config_json_str = '{}' (empty JSON) when
creating collections. chromadb 1.5.9 introduced a strict _type check
in the collection config deserialization path -- its absence raises
KeyError: '_type' on palace open. Since the pin allows >=1.5.4,<2,
any upgrade pulls 1.5.9 and breaks every existing palace.

Add a fourth pre-open migration step (_fix_missing_collection_type)
that injects "_type": "CollectionConfigurationInternal" into
collections.config_json_str rows that lack it. Same lifecycle and
marker-file pattern as the existing _fix_blob_seq_ids.

Co-Authored-By: nautis <nautis@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a migration utility, _fix_missing_collection_type, to ensure compatibility with chromadb 1.5.9+ by adding a missing _type field to collection configurations within the SQLite database. The migration is integrated into the pre-open safety pass and is accompanied by comprehensive unit tests. Feedback identifies that the SQLite connection in the migration function is not explicitly closed by the context manager, which could lead to resource leaks or file locks; a try...finally block is recommended to ensure the connection is closed before the PersistentClient is instantiated.

Comment thread mempalace/backends/chroma.py Outdated
Address review feedback: `with sqlite3.connect() as conn:` only
manages transactions, it does not close the connection.  An open
connection before PersistentClient instantiation can leave WAL state.
Use explicit `try...finally: conn.close()` matching the read-only
helpers elsewhere in the module.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@igorls
igorls merged commit 9b7cfc9 into MemPalace:develop May 30, 2026
6 checks passed
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.

_fix_blob_seq_ids does not repair collection config_json_str missing _type field (chromadb 1.5.9+ regression)

2 participants