[Eric Del Orefice (ericdelorefice)]

Motivation and Context

ChromaCollection has two handlers for the same condition, and the error type they were written against changed underneath them.

pyproject.toml allows chromadb >= 0.5, < 1.6. Across that range the error for a missing collection is not the same:

chromadb 0.5.0   errors.py defines NotFoundError: False
chromadb 1.5.9   client.get_collection("no_such")   -> chromadb.errors.NotFoundError
                 isinstance(e, ValueError)          -> False

Both symptoms follow from that one fact, and they point in opposite directions.

collection_exists catches too much. except Exception: return False, so an unreachable server, a rejected request or a timeout all report that the collection is not there. Three places in this repo already say that is not the rule. _get_collection, nine lines above, raises RuntimeError from the identical client.get_collection(...) call. ensure_collection_deleted, in the same class, raises VectorStoreOperationException for anything that is not a missing collection. And VectorStore.collection_exists in data/vector.py wraps this method and narrows to except VectorStoreOperationException, so the layer above already assumes only a recognised failure becomes False.

ensure_collection_deleted catches too little. Its except ValueError is the 0.5-era signal. On chromadb 1.x a genuinely absent collection raises NotFoundError, which is not a ValueError, so it falls through to except Exception and is re-raised as VectorStoreOperationException. The "could not be deleted because it doesn't exist" path is unreachable on any chromadb 1.x, which is most of the supported range.

Description

One module-level tuple resolves the version difference once:

try:  # chromadb >= 1.0 raises NotFoundError for a missing collection
    from chromadb.errors import NotFoundError as _ChromaNotFoundError

    COLLECTION_NOT_FOUND: tuple[type[Exception], ...] = (ValueError, _ChromaNotFoundError)
except ImportError:  # chromadb < 1.0 raised ValueError
    COLLECTION_NOT_FOUND = (ValueError,)

collection_exists narrows from Exception to that tuple. ensure_collection_deleted widens from ValueError to the same tuple. Nothing else changes, and the guarded import keeps chromadb 0.5 working, where NotFoundError does not exist.

Contribution Checklist

Four unit tests, no Chroma server. They resolve the not-found error themselves rather than importing the new constant, so the same file runs against the code before and after this change.

Against unmodified main, two fail - one per symptom - and two pass:

FAILED test_collection_exists_raises_when_the_server_cannot_be_reached
FAILED test_ensure_collection_deleted_tolerates_an_absent_collection

The two that pass on main pin what must not move: a not-found still returns False, and a real failure during delete still raises.

17 passed                   # tests/unit/connectors/memory/test_chroma.py
All checks passed!          # ruff check
2 files already formatted   # ruff format --check

Note

This is the connector I named as the correct one in #14473, and it is correct in ensure_collection_deleted, which was the method that issue was about. collection_exists underneath it is not, which I only saw when I went back to check the claim.

RedisCollection.collection_exists has the broad-catch half of this and is already covered by #14474. CosmosNoSqlCollection.collection_exists catches CosmosHttpResponseError, which includes 401, 403 and 5xx, so it may want the same narrowing - left out here because I have not read it closely enough to say.

[Copilot]

Copilot review overview

🟡 Changes recommended

Modern Chroma still treats unrelated ValueError exceptions as missing collections.

Review effort: Balanced
Findings: 1

Open (1)

• Avoid catching ValueError when dedicated NotFoundError is available · New

What changed in this PR

Updates Chroma missing-collection handling across supported versions.

Changes:

• Adds version-compatible not-found exception handling.
• Adds tests for missing collections and connection failures.

File	Description
python/​semantic_kernel/​connectors/​chroma.py	Narrows exception handling for lookup and deletion.
python/​tests/​unit/​connectors/​memory/​test_chroma.py	Tests not-found and operational failure behavior.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[PRABHU KIRAN VANDRANKI (VANDRANKI)]

This is a community review and does not clear the merge gate. I ran this on Windows against upstream main at 64005c7, with real chromadb 1.5.9 for the behavior checks.

The fix does what it says on chromadb 1.x. I have one behavior change to flag and one piece of code that cannot run.

What I ran with a real in-process Chroma client (chromadb.EphemeralClient()):

• On main, ensure_collection_deleted() on a collection that does not exist raises VectorStoreOperationException: Failed to delete collection nope .... On the branch it is tolerated. get_collection on a missing name raises chromadb.errors.NotFoundError, which is not a ValueError (checked), so main's except ValueError could never match on 1.x.
• collection_exists() on a missing collection returns False on both main and the branch.

1. Behavior change at the store level. VectorStore.collection_exists in data/vector.py only catches VectorStoreOperationException, and this PR re-raises the raw exception. With a mocked client whose get_collection raises ConnectionError("connection refused"), ChromaStore(client=client).collection_exists("c") returns False on main and raises ConnectionError on the branch. That may be what you want, and the PR text says so. But it is a different contract from the Cosmos change in #14515, which wraps in VectorStoreOperationException (and so still reads as absent at the store level). It would be good to pick one on purpose. If you want a typed error, wrap it. If you want it raw, a short comment would help.
2. The except ImportError fallback cannot run. The connector's own line 11 imports chromadb.api.collection_configuration, and I installed chromadb 0.6.3 and checked that this module does not exist there: importing semantic_kernel.connectors.chroma fails with ModuleNotFoundError. So the connector does not import on 0.6.3. I did not check each version between 0.6.3 and 1.0, so I cannot say exactly where the import starts working. Because of that, on the versions I checked the (ValueError,) branch, COLLECTION_NOT_FOUND_IS_VALUE_ERROR and the two skipif tests never execute. So the chromadb >= 0.5 pin in pyproject.toml that the PR text relies on does not hold for this connector, at least on 0.6.3. I would drop the fallback, or leave it and say it is defensive.

I did not run the new unit tests myself, only the probes above, and I did not test against an HTTP Chroma server, so the exception type for a server-side 404 is unverified.

[Eric Del Orefice (ericdelorefice)]

Thank you for testing this against a real Chroma client, and for checking 0.6.3. Both points are right.

The fallback could never run. Line 11 imports chromadb.api.collection_configuration, which does not exist in 0.6.3 (I checked the 0.6.3 and 1.0.0 tags as well). In 805a786 the connector catches NotFoundError directly, and the two tests that were skipped on old versions now always run.

On the contract, I agree it should be one choice, and I've taken the one from #14515: any other error in collection_exists is now wrapped in VectorStoreOperationException, which is also what ensure_collection_deleted here already raised. The trade-off you pointed out stays: at the store level, VectorStore.collection_exists turns that exception into False, so a failure still reads as absent there for both connectors. That wrapper is shared, so I'd rather raise it with the maintainers than change it in this PR.

Your review also surfaced that the chromadb >= 0.5 pin in pyproject.toml is stale for this connector, since it only imports on 1.x. I've left the pin alone because it covers the whole package, but I'm happy to bump it here if the maintainers want.

The Chroma tests pass on the branch, the two changed ones fail against the previous commit, and ruff 0.9 is clean.

🤖 Addressed by Claude Code