Skip to content

Conversation

@Strift
Copy link
Collaborator

@Strift Strift commented Aug 1, 2025

What

Add conversational search API as released in Meilisearch 1.15.1:

How

Add methods to interact with chat workspaces endpoints:

  • update workspace settings (PATCH /chats/{workspace}/settings)
  • get workspace settings (GET /chats/{workspace}/settings)
  • list workspaces (GET /chats)
  • delete a workspace (DELETE /chats/{workspace_uid}/settings)
  • create a chat completion (POST chats/{workspace}/chat/completions)

Add methods to interact with index chat settings:

  • get an index chat settings (GET /indexes/{index_uid}/settings/chat)
  • update an index chat settings (PUT /indexes/{index_uid}/settings/chat)

Summary by CodeRabbit

  • New Features

    • Chat workspace management: create/update/retrieve/reset workspace settings.
    • Streaming chat completions from workspaces.
    • List all chat workspaces.
    • Index-level chat settings: get and update chat configuration.
  • Bug Fixes

    • Improved request/streaming error handling and timeout management for streaming requests.
  • Tests

    • Added tests covering workspaces, streaming completions, chat settings, and permission behaviors.

@Strift Strift self-assigned this Aug 1, 2025
@coderabbitai
Copy link

coderabbitai bot commented Aug 1, 2025

Walkthrough

Adds chat workspace APIs and types, refactors HTTP request preparation to separate preparation and execution, introduces streaming POST support, extends MeiliSearch and Index clients for chat, and adds tests for chat settings, workspaces, and HttpRequests streaming behavior.

Changes

Cohort / File(s) Change Summary
Chat workspace API
src/chat-workspace.ts
New ChatWorkspace class with methods get(), update(settings), reset(), and streamCompletion(completion) that call workspace-scoped endpoints via the HttpRequests layer.
HTTP request core
src/http-requests.ts
Split request flow: added private #prepareRequest(options) to build URL and RequestInit; refactored #request(...) to use it; added postStream(options) and private #requestStream(...) to support streaming responses, timeout handling, custom httpClient behavior, and enhanced error mapping.
Public client surface
src/meilisearch.ts
Added chat(workspace: string): ChatWorkspace and getChatWorkspaces(): Promise<ResourceResults<{ uid: string }[]>>.
Index chat settings
src/indexes.ts
Added getChat(): Promise<ChatSettings> and updateChat(chatSettings: ChatSettingsPayload): EnqueuedTaskPromise; imported ChatSettings and ChatSettingsPayload.
Types
src/types/types.ts
Added ChatWorkspaceSettings, ChatCompletionRequest, ChatSettings, and ChatSettingsPayload types for chat/workspace configuration and requests.
Tests — chat settings
tests/chat-settings.test.ts
New Vitest suite testing index chat settings retrieval and updates across permission levels and expected authorization failures.
Tests — chat workspaces
tests/chat-workspaces.test.ts
New Vitest suite testing workspace settings CRUD, listing, reset behavior, and streaming completion returning a ReadableStream.
Tests — HTTP requests streaming
tests/http-requests.test.ts
New tests for postStream covering custom httpClient non-stream/stream returns, fetch error responses, empty/null body handling, and error type expectations.
Test utilities
tests/utils/meilisearch-test-utils.ts
getKey extended to handle "Chat" permission, returning the "Default Chat API Key" when present.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant MeiliSearch
    participant ChatWorkspace
    participant HttpRequests
    Client->>MeiliSearch: chat("workspace")
    MeiliSearch->>ChatWorkspace: new ChatWorkspace(HttpRequests, workspace)
    Client->>ChatWorkspace: update(settings)
    ChatWorkspace->>HttpRequests: PATCH /chats/{workspace} (body=settings)
    HttpRequests-->>ChatWorkspace: ChatWorkspaceSettings (response)
    ChatWorkspace-->>Client: ChatWorkspaceSettings

    Client->>ChatWorkspace: streamCompletion(completion with stream=true)
    ChatWorkspace->>HttpRequests: POST /chats/{workspace}/completions (stream=true)
    HttpRequests-->>ChatWorkspace: ReadableStream<Uint8Array>
    ChatWorkspace-->>Client: ReadableStream<Uint8Array>
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Suggested labels

maintenance

Poem

In my burrow I patch and I play,
Workspaces bloom where bytes hop and sway.
Streams trickle in tidy, bright streams,
Settings snugged in soft secret schemes.
A rabbit nods — the chat wakes to day. 🐇

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/add-chat-api

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@Strift Strift added the enhancement New feature or request label Aug 1, 2025
@codecov
Copy link

codecov bot commented Aug 1, 2025

Codecov Report

❌ Patch coverage is 96.42857% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 98.83%. Comparing base (f510917) to head (49bf1df).

Files with missing lines Patch % Lines
src/chat-workspace.ts 94.59% 2 Missing ⚠️
src/http-requests.ts 96.36% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1988      +/-   ##
==========================================
- Coverage   99.02%   98.83%   -0.19%     
==========================================
  Files          18       19       +1     
  Lines        1440     1550     +110     
  Branches      304      334      +30     
==========================================
+ Hits         1426     1532     +106     
- Misses         14       18       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (4)
src/http-requests.ts (1)

295-369: Consider refactoring to reduce code duplication and improve error handling.

This method duplicates significant portions of the existing #request method. Consider the following improvements:

  1. Extract common logic: The URL construction, request initialization, timeout handling, and basic fetch logic are nearly identical between both methods.

  2. Validate custom HTTP client return type: The assumption that custom clients return streams directly should be validated.

  3. Improve error messages: The generic "Response body is null" error could be more descriptive.

Here's a refactored approach:

+  // Common method for request preparation
+  #prepareRequest({ path, method, params, contentType, body, extraRequestInit }: MainRequestOptions) {
+    const url = new URL(path, this.#url);
+    if (params !== undefined) {
+      appendRecordToURLSearchParams(url.searchParams, params);
+    }
+
+    const init: RequestInit = {
+      method,
+      body: contentType === undefined || typeof body !== "string"
+        ? JSON.stringify(body)
+        : body,
+      ...extraRequestInit,
+      ...this.#requestInit,
+      headers: this.#getHeaders(extraRequestInit?.headers, contentType),
+    };
+
+    return { url, init };
+  }

   async #requestStream({
     path,
     method,
     params,
     contentType,
     body,
     extraRequestInit,
   }: MainRequestOptions): Promise<ReadableStream<Uint8Array>> {
-    const url = new URL(path, this.#url);
-    if (params !== undefined) {
-      appendRecordToURLSearchParams(url.searchParams, params);
-    }
-
-    const init: RequestInit = {
-      method,
-      body:
-        contentType === undefined || typeof body !== "string"
-          ? JSON.stringify(body)
-          : body,
-      ...extraRequestInit,
-      ...this.#requestInit,
-      headers: this.#getHeaders(extraRequestInit?.headers, contentType),
-    };
+    const { url, init } = this.#prepareRequest({ path, method, params, contentType, body, extraRequestInit });

     // ... rest of the method
     
     if (this.#customRequestFn !== undefined) {
-      // Custom HTTP clients should return the stream directly
-      return (await this.#customRequestFn(
-        url,
-        init,
-      )) as ReadableStream<Uint8Array>;
+      const result = await this.#customRequestFn(url, init);
+      if (!(result instanceof ReadableStream)) {
+        throw new Error("Custom HTTP client must return a ReadableStream for streaming requests");
+      }
+      return result as ReadableStream<Uint8Array>;
     }

     // ... error handling ...

     if (!response.body) {
-      throw new Error("Response body is null");
+      throw new Error("Response body is null - server did not return a readable stream");
     }
tests/chat.test.ts (2)

18-36: Consider using placeholder API keys in test constants.

The test constants are well-structured, but consider using more obviously fake API keys to avoid any confusion.

- apiKey: "sk-abc...",
+ apiKey: "sk-test-api-key-placeholder",

The satisfies constraint is excellent for ensuring type safety.


59-89: Consider improving error handling in the streaming test.

The streaming test is well-implemented but could benefit from better error handling.

  const reader = stream.getReader();
  const decoder = new TextDecoder();
  try {
    let receivedData = "";
+   let chunkCount = 0;
+   const maxChunks = 100; // Prevent infinite loops in tests
    while (true) {
      const { done, value } = await reader.read();
      if (done) break;
+     
+     if (++chunkCount > maxChunks) {
+       throw new Error("Test exceeded maximum expected chunks");
+     }

      const chunk = decoder.decode(value);
      receivedData += chunk;
    }
    expect(receivedData.length).toBeGreaterThan(0);
  } finally {
    reader.releaseLock();
  }

This adds protection against potential infinite loops and provides better test feedback.

src/chat-workspace.ts (1)

36-46: Fix error message typo and consider validation improvements.

The streaming method has a typo in the error message and could benefit from enhanced validation.

Apply this diff to fix the typo:

-      throw new Error("The SDK only support streaming");
+      throw new Error("The SDK only supports streaming");

Additionally, consider enhancing the validation to be more descriptive:

   async streamCompletion(
     completion: ChatCompletionRequest,
   ): Promise<ReadableStream<Uint8Array>> {
-    if (!completion.stream) {
-      throw new Error("The SDK only supports streaming");
+    if (!completion.stream) {
+      throw new Error("Chat completion requests must have 'stream' set to true. Non-streaming completions are not supported by this SDK.");
     }
     return await this.#httpRequest.postStream({
       path: `chats/${this.#workspace}/chat/completions`,
       body: completion,
     });
   }
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dacd4f3 and cc741e8.

📒 Files selected for processing (6)
  • src/chat-workspace.ts (1 hunks)
  • src/http-requests.ts (1 hunks)
  • src/meilisearch.ts (3 hunks)
  • src/types/types.ts (1 hunks)
  • tests/chat.test.ts (1 hunks)
  • tests/utils/meilisearch-test-utils.ts (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/meilisearch.ts (2)
src/chat-workspace.ts (1)
  • ChatWorkspace (12-47)
src/types/types.ts (1)
  • ResourceResults (129-132)
tests/chat.test.ts (2)
tests/utils/meilisearch-test-utils.ts (2)
  • getClient (369-369)
  • dataset (372-372)
src/types/types.ts (1)
  • ChatWorkspaceSettings (707-718)
src/http-requests.ts (4)
src/types/types.ts (3)
  • RequestOptions (116-116)
  • MainRequestOptions (84-110)
  • MeiliSearchErrorResponse (782-789)
src/errors/meilisearch-request-error.ts (1)
  • MeiliSearchRequestError (3-9)
src/errors/meilisearch-request-timeout-error.ts (1)
  • MeiliSearchRequestTimeOutError (4-13)
src/errors/meilisearch-api-error.ts (1)
  • MeiliSearchApiError (4-20)
🔇 Additional comments (12)
src/http-requests.ts (1)

290-293: LGTM! Clean implementation following established patterns.

The postStream method correctly follows the same pattern as other HTTP method wrappers in the class.

tests/utils/meilisearch-test-utils.ts (1)

47-50: LGTM! Consistent implementation following established patterns.

The addition of the "Chat" permission handling follows the same pattern as existing permission checks and is correctly placed before the "Admin" fallback.

src/types/types.ts (1)

702-728: LGTM! Well-structured type definitions with comprehensive coverage.

The new chat-related types are well-designed with appropriate constraints:

  • ChatWorkspaceSettings properly constrains the source field to supported providers and includes all necessary configuration options
  • ChatCompletionRequest correctly models the chat completion API with proper role constraints for messages
  • Good documentation with reference to official API documentation
src/meilisearch.ts (3)

32-32: LGTM! Clean import addition.

The ResourceResults type import is necessary for the new getChatWorkspaces() method.


42-42: LGTM! Necessary import for chat functionality.

The ChatWorkspace import is required for the new chat workspace factory method.


279-302: LGTM! Clean implementation following established patterns.

The chat workspace methods are well-implemented:

  • chat() follows the factory pattern used by other resource methods like index()
  • getChatWorkspaces() follows the standard GET request pattern used throughout the class
  • Proper JSDoc documentation and consistent method organization
tests/chat.test.ts (2)

1-16: LGTM! Proper test setup with good initialization.

The test setup correctly enables experimental features and initializes the test index with data. The async setup in beforeAll follows best practices.


38-57: LGTM! Comprehensive workspace management tests.

The tests for workspace settings management are well-implemented:

  • Proper separation of update and get operations
  • Correct handling of API key masking in responses
  • Good use of toMatchObject for partial matching
src/chat-workspace.ts (4)

1-5: LGTM - Clean imports with proper type-only imports.

The imports are well-structured using type-only imports where appropriate, which helps with tree-shaking and build optimization.


7-19: LGTM - Well-designed class with proper encapsulation.

The class design follows good practices:

  • Clear JSDoc documentation with reference to official docs
  • Private fields using modern # syntax for true privacy
  • Dependency injection pattern for testability
  • Immutable readonly fields after construction

21-25: LGTM - Simple and correct GET implementation.

The get() method correctly retrieves chat workspace settings using the appropriate HTTP method and path structure.


27-34: LGTM - Proper PATCH implementation for partial updates.

The update() method correctly uses PATCH for partial updates and accepts Partial<ChatWorkspaceSettings> which aligns with REST API conventions.

@Strift Strift marked this pull request as draft August 1, 2025 09:51
@Strift
Copy link
Collaborator Author

Strift commented Aug 1, 2025

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Aug 1, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/http-requests.ts (1)

325-333: Consider stricter type checking for custom HTTP client streaming responses.

While the code checks if the result is a ReadableStream, it then casts it to ReadableStream<Uint8Array>. The custom HTTP client might return a ReadableStream with a different chunk type, which could cause runtime issues.

Consider either:

  1. Documenting that custom HTTP clients must return ReadableStream<Uint8Array> for streaming requests
  2. Adding runtime validation of the stream's chunk type
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cc741e8 and cb83c7a.

📒 Files selected for processing (1)
  • src/http-requests.ts (3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/http-requests.ts (5)
src/types/types.ts (3)
  • MainRequestOptions (84-110)
  • RequestOptions (116-116)
  • MeiliSearchErrorResponse (782-789)
src/errors/meilisearch-error.ts (1)
  • MeiliSearchError (1-3)
src/errors/meilisearch-request-error.ts (1)
  • MeiliSearchRequestError (3-9)
src/errors/meilisearch-request-timeout-error.ts (1)
  • MeiliSearchRequestTimeOutError (4-13)
src/errors/meilisearch-api-error.ts (1)
  • MeiliSearchApiError (4-20)
🔇 Additional comments (3)
src/http-requests.ts (3)

192-222: Good refactoring to extract common request preparation logic.

The extraction of request preparation into #prepareRequest follows the DRY principle and makes the code more maintainable by avoiding duplication between #request and #requestStream.


224-232: Clean refactoring of the request method.

The updated implementation correctly delegates request preparation to #prepareRequest while maintaining the same behavior for request execution and response handling.


301-304: Consistent API design for streaming POST requests.

The new postStream method follows the established pattern of other HTTP methods in the class, providing a clean API for streaming responses.

@Strift Strift marked this pull request as ready for review August 5, 2025 07:30
@Strift Strift requested a review from flevi29 August 6, 2025 05:04
@nicolasvienot
Copy link
Contributor

nicolasvienot commented Aug 6, 2025

Hey!
Should we implement the DELETE /chats/{workspace_uid}/settings?

Also, should we implement the index setting update related to the chat:

GET /indexes/{index_uid}/settings/chat
PUT /indexes/{index_uid}/settings/chat

Also in your PR description, it should be PATCH /chats/{workspace}/settings and GET /chats/{workspace}/settings, but this is what was implemented so all good 👍

Otherwise it seems ok to me :)

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
tests/chat-settings.test.ts (1)

66-66: Consider using a more specific type assertion.

The satisfies operator is good for type checking, but consider if this should be as const to preserve literal types or if the current approach is intentional for flexibility.

src/chat-workspace.ts (1)

66-76: Improve error message grammar and consider validation approach.

The streaming validation and implementation look good, but there are minor improvements to consider:

Apply this diff to fix the grammar:

-      throw new Error("The SDK only support streaming");
+      throw new Error("The SDK only supports streaming");

Consider whether this runtime validation is the best approach, or if the API should be designed to only accept streaming requests at the type level.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cb83c7a and a02f6f7.

⛔ Files ignored due to path filters (1)
  • tests/__snapshots__/chat-settings.test.ts.snap is excluded by !**/*.snap
📒 Files selected for processing (5)
  • src/chat-workspace.ts (1 hunks)
  • src/indexes.ts (2 hunks)
  • src/types/types.ts (1 hunks)
  • tests/chat-settings.test.ts (1 hunks)
  • tests/chat-workspaces.test.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/types/types.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/indexes.ts (2)
src/types/types.ts (2)
  • ChatSettings (729-734)
  • ChatSettingsPayload (736-741)
src/types/task_and_batch.ts (1)
  • EnqueuedTaskPromise (156-162)
tests/chat-workspaces.test.ts (3)
tests/utils/meilisearch-test-utils.ts (1)
  • getClient (369-369)
src/types/types.ts (1)
  • ChatWorkspaceSettings (707-718)
src/task.ts (1)
  • value (36-41)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: integration-tests (Node.js 22)
  • GitHub Check: integration-tests (Node.js 20)
🔇 Additional comments (11)
tests/chat-settings.test.ts (1)

29-74: LGTM! Well-structured test suite with comprehensive permission testing.

The tests properly validate chat settings functionality across different API key permission levels, with appropriate error code validation and type checking. The test structure follows good practices with proper setup/teardown and clear assertions.

src/indexes.ts (2)

53-54: LGTM! Proper type imports added for chat settings.

The import statements correctly include the necessary types for the new chat settings functionality.


1387-1409: LGTM! Chat settings methods follow established patterns.

The implementation is consistent with other settings methods in the class:

  • getChat() properly uses GET request and returns ChatSettings
  • updateChat() correctly uses PUT request with ChatSettingsPayload and returns EnqueuedTaskPromise
  • Endpoint paths match the API specification
  • JSDoc comments provide clear documentation
src/chat-workspace.ts (4)

12-19: LGTM! Clean class structure with proper encapsulation.

The class uses private fields appropriately and has a straightforward constructor. Good use of readonly for immutable fields.


27-31: LGTM! Method follows standard GET pattern.

The get() method correctly implements workspace settings retrieval with proper endpoint construction.


39-46: LGTM! Proper PATCH implementation for partial updates.

The update() method correctly uses PATCH for partial updates and accepts Partial<ChatWorkspaceSettings> as expected.


54-58: LGTM! Clean reset implementation.

The reset() method properly uses DELETE request and returns void as appropriate for a reset operation.

tests/chat-workspaces.test.ts (4)

5-16: LGTM! Proper test setup with experimental features enabled.

The beforeAll setup correctly enables chat completions experimental feature and prepares test data. Good practice to set up the index with documents for realistic testing.


18-36: LGTM! Well-structured test data with API key masking consideration.

The test data setup is comprehensive and correctly handles the API key masking that Meilisearch applies in responses. Good use of satisfies for type checking.


38-65: LGTM! Comprehensive CRUD operation testing.

The tests properly cover all workspace settings operations:

  • Update and verify response
  • Get settings and verify data
  • List workspaces functionality
  • Reset/delete operations

Good test isolation and proper assertions.


67-97: LGTM! Thorough streaming test with proper resource management.

The streaming test properly:

  • Validates the returned ReadableStream type
  • Reads and processes streamed data
  • Uses proper cleanup with try/finally
  • Verifies data was received

Excellent practice with the reader.releaseLock() in the finally block.

@Strift
Copy link
Collaborator Author

Strift commented Aug 7, 2025

Thanks for the review!

I updated the code to implement the following:

  • method to call DELETE /chats/{workspace_uid}/settings
  • method to call GET /indexes/{index_uid}/settings/chat
  • method to call PATCH /indexes/{index_uid}/settings/chat

Copy link
Contributor

@nicolasvienot nicolasvienot left a comment

Choose a reason for hiding this comment

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

PUT /indexes/{index_uid}/settings/chat is now PATCH /indexes/{index_uid}/settings/chat

@Strift Strift requested a review from nicolasvienot August 14, 2025 06:30
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (5)
tests/http-requests.test.ts (5)

21-29: Restore fetch mock in afterEach to avoid cross-test bleed.

Restoring in afterEach is safer if other tests are added later; it keeps each test isolated.

Apply this diff:

-import {
-  afterAll,
-  beforeEach,
-  describe,
-  expect,
-  test,
-  vi,
-  type MockInstance,
-} from "vitest";
+import {
+  afterEach,
+  beforeEach,
+  describe,
+  expect,
+  test,
+  vi,
+  type MockInstance,
+} from "vitest";
@@
-  afterAll(() => {
-    fetchSpy.mockRestore();
-  });
+  afterEach(() => {
+    fetchSpy.mockRestore();
+  });

58-72: Optionally assert response body is parsed into MeiliSearchApiError.cause.

Given MeiliSearchApiError stores the parsed body in cause, you can strengthen the test by checking the message/cause.

You can update the assertion like:

-  await expect(
-    httpRequests.postStream({ path: "chat", body: {} }),
-  ).rejects.toThrow(MeiliSearchApiError);
+  await expect(
+    httpRequests.postStream({ path: "chat", body: {} }),
+  ).rejects.toMatchObject({
+    name: "MeiliSearchApiError",
+    message: "Stream error",
+    cause: { message: "Stream error" },
+  });

90-109: Avoid invoking the failing call twice; assert both type and message from one caught error.

This reduces duplication and clarifies intent.

Apply this diff:

-  await expect(
-    httpRequests.postStream({ path: "chat", body: {} }),
-  ).rejects.toThrow(MeiliSearchError);
-  await expect(
-    httpRequests.postStream({ path: "chat", body: {} }),
-  ).rejects.toThrow(
-    "Response body is null - server did not return a readable stream",
-  );
+  let err: unknown;
+  try {
+    await httpRequests.postStream({ path: "chat", body: {} });
+  } catch (e) {
+    err = e;
+  }
+  expect(err).toBeInstanceOf(MeiliSearchError);
+  expect((err as Error).message).toBe(
+    "Response body is null - server did not return a readable stream",
+  );

45-56: Optional: also assert the httpClient is called once with the prepared URL and POST method.

This ensures correct integration of postStream with custom clients.

Example follow-up assertions you can add:

 const result = await httpRequests.postStream({ path: "chat", body: {} });
 expect(result).toBe(mockStream);
+expect(customHttpClient).toHaveBeenCalledTimes(1);
+const [url, init] = customHttpClient.mock.calls[0]!;
+expect(url).toMatch(/\/chat$/);
+expect(init?.method).toBe("POST");

58-88: Coverage gap: no success-path test for fetch streaming.

Consider adding a test where fetch returns a 200 Response whose body is a ReadableStream to validate the non-httpClient path happy case.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1b7630b and db0d18d.

📒 Files selected for processing (4)
  • src/indexes.ts (2 hunks)
  • src/meilisearch.ts (3 hunks)
  • src/types/types.ts (1 hunks)
  • tests/http-requests.test.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/types/types.ts
  • src/meilisearch.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
tests/http-requests.test.ts (4)
src/types/types.ts (1)
  • Config (47-81)
src/errors/meilisearch-request-error.ts (1)
  • MeiliSearchRequestError (3-9)
src/errors/meilisearch-api-error.ts (1)
  • MeiliSearchApiError (4-20)
src/errors/meilisearch-error.ts (1)
  • MeiliSearchError (1-3)
src/indexes.ts (2)
src/types/types.ts (2)
  • ChatSettings (733-738)
  • ChatSettingsPayload (740-745)
src/types/task_and_batch.ts (1)
  • EnqueuedTaskPromise (156-162)
🔇 Additional comments (2)
src/indexes.ts (2)

53-55: Types import for chat settings looks correct.

Imports of ChatSettings and ChatSettingsPayload from ./types/index.js align with the type definitions in src/types/types.ts and intended usage below.


1392-1396: GET chat settings endpoint/path and return type are correct.

indexes/{uid}/settings/chat and the ChatSettings return type match the documented API and local types.

Copy link
Contributor

@nicolasvienot nicolasvienot left a comment

Choose a reason for hiding this comment

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

LGTM

@Strift Strift added this pull request to the merge queue Aug 15, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Aug 15, 2025
@Strift Strift force-pushed the feat/add-chat-api branch from 909d88c to 49bf1df Compare August 15, 2025 03:03
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/chat-workspace.ts (1)

69-71: Consider supporting non-streaming mode in the future.

While the current limitation to streaming-only is acceptable for an experimental feature, consider tracking this as a future enhancement to support both streaming and non-streaming modes for flexibility.

Would you like me to create an issue to track adding non-streaming support as a future enhancement?

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 909d88c and 49bf1df.

📒 Files selected for processing (9)
  • src/chat-workspace.ts (1 hunks)
  • src/http-requests.ts (3 hunks)
  • src/indexes.ts (2 hunks)
  • src/meilisearch.ts (3 hunks)
  • src/types/types.ts (1 hunks)
  • tests/chat-settings.test.ts (1 hunks)
  • tests/chat-workspaces.test.ts (1 hunks)
  • tests/http-requests.test.ts (1 hunks)
  • tests/utils/meilisearch-test-utils.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
  • tests/http-requests.test.ts
  • tests/utils/meilisearch-test-utils.ts
  • src/meilisearch.ts
  • tests/chat-workspaces.test.ts
  • tests/chat-settings.test.ts
  • src/types/types.ts
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/chat-workspace.ts (2)
src/http-requests.ts (1)
  • HttpRequests (139-366)
src/types/types.ts (2)
  • ChatWorkspaceSettings (711-722)
  • ChatCompletionRequest (724-731)
src/indexes.ts (2)
src/types/types.ts (2)
  • ChatSettings (733-738)
  • ChatSettingsPayload (740-745)
src/types/task_and_batch.ts (1)
  • EnqueuedTaskPromise (156-162)
src/http-requests.ts (5)
src/types/types.ts (3)
  • MainRequestOptions (84-110)
  • RequestOptions (116-116)
  • MeiliSearchErrorResponse (800-807)
src/errors/meilisearch-error.ts (1)
  • MeiliSearchError (1-3)
src/errors/meilisearch-request-error.ts (1)
  • MeiliSearchRequestError (3-9)
src/errors/meilisearch-request-timeout-error.ts (1)
  • MeiliSearchRequestTimeOutError (4-13)
src/errors/meilisearch-api-error.ts (1)
  • MeiliSearchApiError (4-20)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: integration-tests (Node.js 20)
  • GitHub Check: integration-tests (Node.js 22)
🔇 Additional comments (6)
src/http-requests.ts (3)

193-222: LGTM! Clean refactoring that improves code modularity.

The extraction of request preparation logic into #prepareRequest eliminates code duplication between #request and #requestStream, making the code more maintainable.


301-304: LGTM! Well-designed public API for streaming.

The postStream method provides a clean public interface for streaming POST requests, maintaining consistency with other HTTP method wrappers.


306-365: Robust implementation of streaming functionality with proper error handling.

The #requestStream method is well-implemented with:

  • Proper timeout management
  • Clear error handling for custom HTTP clients that don't return ReadableStream
  • Appropriate error details extraction for non-OK responses
  • Null body check with descriptive error message
src/indexes.ts (2)

1387-1396: LGTM! Clear documentation and consistent implementation.

The getChat method follows the established pattern for settings retrieval methods in the class.


1398-1409: LGTM! Correct parameter type and HTTP method.

The updateChat method correctly uses ChatSettingsPayload as the parameter type and PATCH for partial updates, which aligns with the API specification and follows the pattern of other update methods in the class.

src/chat-workspace.ts (1)

1-77: Well-structured chat workspace implementation with clear API boundaries.

The ChatWorkspace class provides a clean interface for managing chat workspaces with:

  • Proper encapsulation using private fields
  • Clear method documentation with experimental tags and API references
  • Consistent error handling for streaming requirements
  • Type-safe operations using the defined TypeScript types

@Strift Strift added this pull request to the merge queue Aug 15, 2025
Merged via the queue into main with commit 714875b Aug 15, 2025
5 of 7 checks passed
@Strift Strift deleted the feat/add-chat-api branch August 15, 2025 03:16
@coderabbitai coderabbitai bot mentioned this pull request Sep 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants