Skip to content

[ENH] Eventual consistency in clients #5947

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
HammadB merged 1 commit into from
Jan 9, 2026
Merged

[ENH] Eventual consistency in clients #5947

HammadB merged 1 commit into from
Jan 9, 2026
+311 −10

Conversation

@HammadB
Copy link
Collaborator

@HammadB HammadB commented Dec 4, 2025 •
edited
Loading

Description of changes

Summarize the changes made by this PR.

  • Improvements & Bug fixes
    • /
  • New functionality
    • Adds read level to the clients.

Test plan

How are these changes tested?
E2E Tests that check searches succeed were added.

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

Migration plan

None required, the server handles both cases fwd/back.

Observability plan

Client changes don't entail this for now

Documentation Changes

Will update docs in a seperate PR

@github-actions
Copy link

github-actions bot commented Dec 4, 2025

Reviewer Checklist

Please leverage this checklist to ensure your code review is thorough before approving

Testing, Bugs, Errors, Logs, Documentation

  • Can you think of any use case in which the code does not behave as intended? Have they been tested?
  • Can you think of any inputs or external events that could break the code? Is user input validated and safe? Have they been tested?
  • If appropriate, are there adequate property based tests?
  • If appropriate, are there adequate unit tests?
  • Should any logging, debugging, tracing information be added or removed?
  • Are error messages user-friendly?
  • Have all documentation changes needed been made?
  • Have all non-obvious changes been commented?

System Compatibility

  • Are there any potential impacts on other parts of the system or backward compatibility?
  • Does this change intersect with any items on our roadmap, and if so, is there a plan for fitting them together?

Quality

  • Is this code of a unexpectedly high quality (Readability, Modularity, Intuitiveness)

@HammadB Graphite App
Copy link
Collaborator Author

HammadB commented Dec 4, 2025 •
edited
Loading

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

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

This was referenced Dec 4, 2025
Merged
Open
HammadB
HammadB commented Dec 4, 2025
View reviewed changes
clients/new-js/packages/chromadb/src/api/types.gen.ts
user_id: string;
};

export type HashMap = {
Copy link
Collaborator Author

@HammadB HammadB Dec 4, 2025

Choose a reason for hiding this comment

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

Will revert these gen types after talking to @itaismith

Copy link
Contributor

@itaismith itaismith Jan 9, 2026

Choose a reason for hiding this comment

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

Did you run into any issues with the API gen here?

HammadB
HammadB commented Dec 4, 2025
View reviewed changes
clients/new-js/packages/ai-embeddings/common/package-lock.json Outdated
@@ -0,0 +1,1867 @@
{
Copy link
Collaborator Author

@HammadB HammadB Dec 4, 2025

Choose a reason for hiding this comment

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

Will revert these - they shouldn't be needed

@jeffchuber
Copy link
Contributor

jeffchuber commented Dec 9, 2025

My only API naming question is should we bias towards telling you what it does "skip_wal" or the benefit "fast_search" or the technical team "eventual_consistency" ... I lean towards eventual_consistency in this case.

@HammadB HammadB force-pushed the hammad/eventual_consistency_frontend branch from b46d7a5 to bf3841d Compare January 9, 2026 16:22
@HammadB HammadB force-pushed the hammad/eventual_consistency_python_client branch 10 times, most recently from 43e5741 to 765cdbf Compare January 9, 2026 17:38
@HammadB HammadB marked this pull request as ready for review January 9, 2026 17:39
@HammadB HammadB requested a review from rescrv as a code owner January 9, 2026 17:39
@propel-code-bot
Copy link
Contributor

propel-code-bot bot commented Jan 9, 2026 •
edited
Loading

Clients support configurable read-level for search consistency

Extends the Python (sync and async), JavaScript, and Rust client search APIs to accept an optional read_level parameter that controls whether search requests include the write-ahead log. A new shared ReadLevel enum/type is introduced for all clients, the generated HTTP payloads and type definitions now carry the optional read_level field, and tests cover the default and explicit behaviors.

Key Changes

• Added ReadLevel enum/type across Python, TypeScript, and Rust clients and threaded it through Collection.search/AsyncCollection.search and the underlying HTTP calls.
• Updated client payload builders (collection.ts, types.gen.ts, async_fastapi.py, fastapi.py) to serialize the optional read_level to the /search endpoint and adjusted the Rust client via search_with_options to maintain backward compatibility.
• Introduced new tests in Python (chromadb/test/api/test_search_api.py) and TypeScript (search.expression.test.ts) validating default and explicit read-level behavior.

Affected Areas

• Python collection search APIs
• TypeScript client collection implementation and generated API types
• Rust collection client
• FastAPI/AsyncFastAPI search request handling
• Client-side search expression tests

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

@HammadB HammadB requested a review from itaismith January 9, 2026 17:52
rescrv
rescrv approved these changes Jan 9, 2026
View reviewed changes
Copy link
Contributor

@rescrv rescrv left a comment

Choose a reason for hiding this comment

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

Question about design is for you. I'd prefer options, but we can always do an = None on a type in the future to make it optional.

rust/chroma/src/collection.rs
pub async fn search_with_options(
&self,
searches: Vec<SearchPayload>,
read_level: ReadLevel,
Copy link
Contributor

@rescrv rescrv Jan 9, 2026

Choose a reason for hiding this comment

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

Thinking forward: It's search_with_options, not, search-with-read-level. Do we want to wrap ReadLevel in an options for non-breaking API extensions in the future?

Copy link
Collaborator Author

@HammadB HammadB Jan 9, 2026

Choose a reason for hiding this comment

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

I am personally ok with adding that in the future, I am always a bit weary of single-item options. Its an easy change for any user due to the compiler IMO.

Copy link
Collaborator Author

@HammadB HammadB Jan 9, 2026

Choose a reason for hiding this comment

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

Wdyt?

@HammadB HammadB force-pushed the hammad/eventual_consistency_python_client branch from 765cdbf to 73942af Compare January 9, 2026 18:17
@HammadB HammadB force-pushed the hammad/eventual_consistency_frontend branch from bf3841d to b8dae44 Compare January 9, 2026 18:17
propel-code-bot[bot]
propel-code-bot bot reviewed Jan 9, 2026
View reviewed changes
propel-code-bot[bot]
propel-code-bot bot reviewed Jan 9, 2026
View reviewed changes
itaismith
itaismith approved these changes Jan 9, 2026
View reviewed changes
Copy link
Contributor

@itaismith itaismith left a comment

Choose a reason for hiding this comment

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

Reviewed Python and JS clients

clients/new-js/packages/chromadb/src/api/types.gen.ts
user_id: string;
};

export type HashMap = {
Copy link
Contributor

@itaismith itaismith Jan 9, 2026

Choose a reason for hiding this comment

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

Did you run into any issues with the API gen here?

@HammadB
Copy link
Collaborator Author

HammadB commented Jan 9, 2026

Reviewed Python and JS clients
Did you run into any issues with the API gen here?

Nope - just worked

@HammadB HammadB merged commit a26d674 into hammad/eventual_consistency_frontend Jan 9, 2026
71 of 79 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@propel-code-bot propel-code-bot[bot] propel-code-bot[bot] left review comments

@rescrv rescrv rescrv approved these changes

@itaismith itaismith itaismith approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@HammadB @jeffchuber @rescrv @itaismith