[BUG] Raise Error when can't deserialize configuration json from server, lazily load ef on CollectionModel, warn on api_key #4471

jairad26 · 2025-05-07T01:13:50Z

Description of changes

This PR fixes a bug where it warns users if the configuration is not deserializable rather than raising an error, giving the users an empty config. Now, it raises an error if the configuration is not deserializable. This will only happen when the client is ahead of the server, which we do not need to support.

EF's from the config are now lazily loaded. only when ._embed is invoked from the model is it tried to be built, and will take precedent over the provided ef.

This also fixes a bug with list_collections, where if the user tries to load collections without the proper embedding function parameters set up, like api key env var, etc it would throw an hnsw and spann not configured error, which is not the root cause. instead, it will now load only the configuration_json, and at embed time try to load the ef, so errors will only propogate at that point, and with much clearer error messages.

Now, using api_key directly in embedding_functions will give a deprecation warning saying it is not persisted.

Test plan

How are these changes tested?

Tests pass locally with pytest for python, yarn test for js, cargo test for rust

Documentation Changes

Are all docstrings for user-facing APIs updated if required? Do we need to make documentation changes in the docs section?

github-actions · 2025-05-07T01:13:59Z

jairad26 · 2025-05-07T01:14:04Z

[ENH] add Mistral embedding function, fix some naming mismatches across clients #4653
[ENH] Add validation when multiple embedding functions set on client #4507
[BUG] Raise Error when can't deserialize configuration json from server, lazily load ef on CollectionModel, warn on api_key #4471 👈 (View in Graphite)
main

This stack of pull requests is managed by Graphite. Learn more about stacking.

propel-code-bot · 2025-05-07T16:34:15Z

Stricter Error Handling and Lazy Embedding Function Loading for Collection Configuration

This PR refactors the configuration loading and embedding function behavior throughout ChromaDB. It enforces that configuration deserialization errors now raise exceptions rather than being silently handled or replaced with default/empty values, preventing silent misconfigurations and client/server version mismatches. Embedding functions specified in configuration are now lazily constructed at embed time (e.g., add/query), allowing basic collection operations like list_collections and get_collection to work even if EF setups (e.g., API keys) are not present, and surfacing more precise errors only when embedding is triggered. Additionally, a deprecation warning is added when users provide API keys directly to embedding function constructors, guiding them to use environment variables for persistence.

Key Changes:
• CollectionModel now stores configuration as configuration_json and strictly requires valid deserialization, raising ValueError on failure
• Lazy loading of embedding functions at embed-time (Collection._embed), with improved error reporting on misconfiguration or missing parameters
• Passing API keys directly to embedding functions triggers DeprecationWarning to discourage non-persistent configurations
• Expanded and improved tests for handling invalid configurations, ensuring correct error propagation and lazy loading behavior
• Numerous codebase-wide changes to pass and use configuration_json consistently (including db, segment, proto, etc.), and legacy configuration upgrades remain supported

Affected Areas:
• chromadb/types.py
• chromadb/api/collection_configuration.py
• chromadb/api/models/CollectionCommon.py
• chromadb/utils/embedding_functions/*
• chromadb/db/mixins/sysdb.py
• chromadb/db/impl/grpc/server.py
• chromadb/api/rust.py
• chromadb/api/segment.py
• chromadb/proto/convert.py
• test suite (test_collection_configuration.py, property/invariants.py)

Potential Impact:

Functionality: Invalid or out-of-sync configuration now causes explicit errors rather than fallback; downstream systems must be ready for exceptions. Basic collection operations succeed even when embedding backends are misconfigured, but embedding-related collection operations will fail predictably.

Performance: No major performance impact expected; lazy EF loading may reduce unnecessary object construction.

Security: Deprecated use of direct API keys improves security by encouraging environment variable usage.

Scalability: Improved error granularity benefits large installations with many collections having diverse embedding setups.

Review Focus:
• Are all usages of configuration_json correctly and safely handled?
• Could any places calling configuration loading experience unhandled exceptions under server/client mismatch or storage/outdated data?
• Are all EF lazy-loading error paths clear and actionable for end-users?
• Check if new error messages are sufficiently descriptive and actionable

Testing Needed

• Verify that collections with malformed config fail at the correct point (add, query) with clear error messages
• Test list_collections and get_collection on databases with intentionally broken/missing EF config
• Check deprecation warnings on direct API key usage in all embedding functions
• Ensure all existing and new tests covering erroneous configs pass

Code Quality Assessment

chromadb/api/models/CollectionCommon.py: Constructor simplifies EF handling, embed() refactored for lazy loading and clarity; removes silent mismatches

chromadb/utils/embedding_functions/*: Adds deprecation warnings for API key direct usage in all external EF providers; consistent pattern applied

chromadb/test/configurations/test_collection_configuration.py: Significant new tests for invalid/edge cases; test logic is comprehensive and systematic

chromadb/types.py: Systematically updated to enforce strict config handling, but removal of warnings import (fixed) and 'json' naming nit; otherwise, clean

chromadb/api/collection_configuration.py: Rewritten EF builder logic for better error granularity; follows modern error-handling patterns

Best Practices

Error Handling:
• Explicit exceptions for config load failures
• Lazy error surfacing for dependent resources

Configuration Management:
• Consistent use of configuration_json
• Precedence for EF from config over constructor arg

Test Coverage:
• Comprehensive edge and negative case tests for configuration handling

Security:
• Discouragement of direct API key persistence in code

Potential Issues

• Downstream consumer code that relied on warning+empty config fallback may now break if not ready for exceptions
• Lazy EF loading means some config or EF misconfigurations may go undetected until runtime (e.g., user thinks a collection is valid until embedding APIs are used)
• Proto and DB layer changes strictly require config JSON; data corruptions may now cause harder errors
• Environment-variable dependence for EF may still confuse some users who expect persistence via direct API key

This summary was automatically generated by @propel-code-bot

propel-code-bot · 2025-05-07T19:56:39Z

chromadb/types.py

+            configuration = CollectionConfiguration(
+                hnsw=None,
+                spann=None,
+                embedding_function=None,
+            )


[DataTypeCheck]

Now the fallback value for configuration in from_json() constructs CollectionConfiguration(hnsw=None, spann=None, embedding_function=None). Confirm that this matches the expected constructor signature and field types. Add appropriate type annotations to CollectionConfiguration to clarify that None is valid for these fields, or provide documentation about acceptable values.

propel-code-bot · 2025-05-07T21:55:51Z

chromadb/types.py

@@ -8,16 +8,13 @@
 from uuid import UUID
 from enum import Enum
 from pydantic import BaseModel
-import warnings



[CriticalError]

The warnings module is used in this file now (see usage in chromadb/api/collection_configuration.py), but it is not imported. Add the import at the top of the file to prevent runtime errors.

propel-code-bot · 2025-05-07T21:55:52Z

chromadb/api/collection_configuration.py

+                warnings.warn(
+                    f"Embedding function {ef_config['name']} not found. Add @register_embedding_function decorator to the class definition.",
+                    stacklevel=2,
                )
+                ef = None


[BestPractice]

Changing from ValueError to warnings.warn and ef = None when an embedding function is not found might mask critical configuration errors. If an embedding function is specified in the configuration but cannot be located, it typically indicates a misconfiguration that should prevent the system from proceeding as if everything is normal. This could lead to a collection being created or used without the intended embedding capabilities, with the issue only becoming apparent later. Raising an error ensures such issues are addressed immediately. This approach is also more consistent with the stricter error handling for configuration loading adopted elsewhere in these changes (e.g., in chromadb/types.py).

Consider restoring the behavior of raising an error:

Suggested change

warnings.warn(

f"Embedding function {ef_config['name']} not found. Add @register_embedding_function decorator to the class definition.",

stacklevel=2,

)

ef = None

raise ValueError(

f"Embedding function {ef_config['name']} not found. Add @register_embedding_function decorator to the class definition."

)

⚡ Committable suggestion

Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

propel-code-bot · 2025-05-07T22:04:22Z

chromadb/types.py

        try:
            return load_collection_configuration_from_json(self.configuration_json)
        except Exception as e:
-            warnings.warn(
-                f"Server does not respond with configuration_json. Please update server: {e}",
-                DeprecationWarning,
-                stacklevel=2,
-            )
-            return CollectionConfiguration(
-                hnsw=HNSWConfiguration(),
-                spann=SpannConfiguration(),
-                embedding_function=None,
+            raise ValueError(


[CriticalError]

Instead of returning a default configuration on deserialization error, now code raises ValueError. Ensure that all consumers of this method are prepared to handle this exception appropriately. If unhandled, it may lead to crashes during deserialization.

propel-code-bot · 2025-05-07T22:04:23Z

chromadb/types.py

        try:
            configuration_json = json_map.get("configuration_json", None)


[DataTypeCheck]

Calls to load_collection_configuration_from_json may now receive None for configuration_json. Before calling the loader, consider explicitly validating the presence of configuration_json to provide a clearer error message.

jairad26 · 2025-05-19T22:15:41Z

chromadb/api/collection_configuration.py

+                    UserWarning,
+                    stacklevel=2,
+                )
+                ef = MalformedEmbeddingFunction(  # type: ignore


this also gives us leeway for future embedding functions to make breaking changes. The embedding function being "malformed" (i.e. the config could not build an ef in the case where let's say a model name gets deprecated, an argument name is changed etc.) should not stop the user from the using the ef. Instead it should give them a warning, and now give them the ability to update the ef if necessary. see line 669.
I was also able to get into the sqlite, modify the config json str manually and trigger this path, and this gives users a way out, basically if they ever hit a "broken" state.

this also help with the discussion about what happens if the user has thousands of collections, all configured with diff api key env vars. this will allow them to not need to set them all to build their efs. Instead they will get the malformed ef, and not error for list_collections.

Could we not instead defer ef construction to use, making it lazy, removing this misc type? This feels very messy

we can, the downside with that is on collection construction, if theres no ef it auto gives default ef

chroma/chromadb/api/models/CollectionCommon.py

Line 120 in d596825

] = ef.DefaultEmbeddingFunction(), # type: ignore

so if a user does collection._embed, checks collection._embedding_function they'll see the default ef.

added this, ef's are now lazily loaded during _embed

users will see the configuration_json as expected, but it will not fail even if the ef is "misconfigured" until embedding time. this includes if the ef somehow gets out of sync, or if they dont set the env var at list_collections/get_collection time. thanks for the suggestion it feels a lot cleaner now

HammadB · 2025-05-30T01:31:13Z

chromadb/api/models/CollectionCommon.py

-            if embedding_function is not None and not isinstance(
-                embedding_function, ef.DefaultEmbeddingFunction
-            ):
-                if embedding_function.name() is not config_ef.name():


Don't we want this validation?

HammadB · 2025-05-30T01:36:55Z

chromadb/types.py

@@ -92,7 +89,7 @@ def __init__(
        self,
        id: UUID,
        name: str,
-        configuration: CollectionConfiguration,
+        configuration_json: Dict[str, Any],


naming nit- its not json!

HammadB · 2025-05-30T01:38:21Z

chromadb/proto/convert.py

-        configuration=load_collection_configuration_from_json_str(
-            collection.configuration_json_str
-        ),
+        configuration_json=json.loads(collection.configuration_json_str),


Do you care if this errors?

HammadB · 2025-05-30T01:39:49Z

chromadb/db/impl/grpc/server.py

-        configuration = load_collection_configuration_from_json_str(
-            request.configuration_json_str
-        )
+        configuration_json = json.loads(request.configuration_json_str)


do you care that this can error? Should it be made nice?

HammadB

test more.

jairad26 marked this pull request as ready for review May 7, 2025 01:14

jairad26 force-pushed the jai/fix-list-collections branch from c11b7ad to cd63d71 Compare May 7, 2025 16:33

jairad26 force-pushed the jai/fix-list-collections branch 3 times, most recently from b40998d to af51006 Compare May 7, 2025 19:54

propel-code-bot bot reviewed May 7, 2025

View reviewed changes

jairad26 force-pushed the jai/fix-list-collections branch from af51006 to 318d249 Compare May 7, 2025 21:54

propel-code-bot bot reviewed May 7, 2025

View reviewed changes

jairad26 force-pushed the jai/fix-list-collections branch from 318d249 to 7bb55b9 Compare May 7, 2025 22:02

propel-code-bot bot reviewed May 7, 2025

View reviewed changes

jairad26 changed the title ~~[BUG] Replace empty config objects with None during deserializing~~ [BUG] Raise Error when can't deserialize configuration json from server May 12, 2025

jairad26 force-pushed the jai/fix-list-collections branch 2 times, most recently from 0ccccea to 0c840c7 Compare May 19, 2025 17:56

jairad26 changed the title ~~[BUG] Raise Error when can't deserialize configuration json from server~~ [BUG] Raise Error when can't deserialize configuration json from server, use malformed ef when can't build from config May 19, 2025

jairad26 force-pushed the jai/fix-list-collections branch 2 times, most recently from 2d2dbde to 6e26c32 Compare May 19, 2025 18:30

jairad26 commented May 19, 2025

View reviewed changes

jairad26 force-pushed the jai/fix-list-collections branch from 6e26c32 to a76cbc4 Compare May 20, 2025 20:33

jairad26 changed the title ~~[BUG] Raise Error when can't deserialize configuration json from server, use malformed ef when can't build from config~~ [BUG] Raise Error when can't deserialize configuration json from server, use malformed ef when can't build from config, warn on api_key May 20, 2025

jairad26 force-pushed the jai/fix-list-collections branch from a76cbc4 to 4a22684 Compare May 20, 2025 21:28

codetheweb approved these changes May 21, 2025

View reviewed changes

jairad26 force-pushed the jai/fix-list-collections branch 2 times, most recently from 0976e2c to fe4811f Compare May 27, 2025 19:48

jairad26 force-pushed the jai/fix-list-collections branch from fe4811f to 874e3d9 Compare May 28, 2025 19:09

jairad26 force-pushed the jai/fix-list-collections branch 2 times, most recently from 9be5a1b to 2c18cb2 Compare May 28, 2025 23:20

HammadB reviewed May 30, 2025

View reviewed changes

HammadB approved these changes May 30, 2025

View reviewed changes

jairad26 force-pushed the jai/fix-list-collections branch 4 times, most recently from a862a92 to e8ffaa9 Compare May 30, 2025 02:37

jairad26 mentioned this pull request May 30, 2025

[ENH] Add validation when multiple embedding functions set on client #4507

Merged

1 task

jairad26 force-pushed the jai/fix-list-collections branch from e8ffaa9 to 1aff970 Compare May 30, 2025 02:53

[BUG] Replace empty config objects with None during deserializing

457a780

jairad26 force-pushed the jai/fix-list-collections branch from 1aff970 to 457a780 Compare May 30, 2025 03:03

jairad26 merged commit 5e58b94 into main May 30, 2025
72 checks passed

jairad26 mentioned this pull request May 30, 2025

[ENH] add Mistral embedding function, fix some naming mismatches across clients #4653

Merged

1 task

		try:
		configuration_json = json_map.get("configuration_json", None)

[BUG] Raise Error when can't deserialize configuration json from server, lazily load ef on CollectionModel, warn on api_key #4471

[BUG] Raise Error when can't deserialize configuration json from server, lazily load ef on CollectionModel, warn on api_key #4471

Uh oh!

Conversation

jairad26 commented May 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of changes

Test plan

Documentation Changes

Uh oh!

github-actions bot commented May 7, 2025

Reviewer Checklist

Testing, Bugs, Errors, Logs, Documentation

System Compatibility

Quality

Uh oh!

jairad26 commented May 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

propel-code-bot bot commented May 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

propel-code-bot bot May 7, 2025

Choose a reason for hiding this comment

Uh oh!

propel-code-bot bot May 7, 2025

Choose a reason for hiding this comment

Uh oh!

propel-code-bot bot May 7, 2025

Choose a reason for hiding this comment

Uh oh!

propel-code-bot bot May 7, 2025

Choose a reason for hiding this comment

Uh oh!

propel-code-bot bot May 7, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jairad26 May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HammadB left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jairad26 commented May 7, 2025 •

edited

Loading

jairad26 commented May 7, 2025 •

edited

Loading

propel-code-bot bot commented May 7, 2025 •

edited

Loading

jairad26 May 27, 2025 •

edited

Loading