src/Contracts/Http.php (1)

50-54: postStream contract addition looks good; consider documenting all thrown exceptions for parity

The implementation (src/Http/Client.php::postStream) can also throw ClientExceptionInterface and CommunicationException. To keep interface docs aligned and help consumers, consider adding these to the docblock.

Apply this docblock tweak:

     /**
      * @throws ApiException
      * @throws JsonEncodingException
+     * @throws \Psr\Http\Client\ClientExceptionInterface
+     * @throws \Meilisearch\Exceptions\CommunicationException
      */
     public function postStream(string $path, $body = null, array $query = []): \Psr\Http\Message\StreamInterface;

src/Client.php (1)

59-59: Initialize $this->chats endpoint early; confirm typed property exists in the trait

Assuming HandlesChatWorkspaces declares a typed public property for $chats, this avoids dynamic property issues on PHP 8.2+. If it's not declared there, we should add it.

I can open a small follow-up to add a typed property declaration if needed.

tests/Settings/ChatTest.php (1)

16-21: Optional: use heredoc/nowdoc for multi-line documentTemplate for readability

The embedded newline in a single-quoted string is fine but a heredoc/nowdoc improves readability of the template.

Option:

-      'documentTemplate' => '{% for field in fields %}{% if field.is_searchable and field.value != nil %}{{ field.name }}: {{ field.value }}
-{% endif %}{% endfor %}',
+      'documentTemplate' => <<<'TPL'
+{% for field in fields %}{% if field.is_searchable and field.value != nil %}{{ field.name }}: {{ field.value }}
+{% endif %}{% endfor %}
+TPL,

src/Http/Client.php (1)

148-156: Consider setting standard header casing and SSE Accept for streaming

Headers are case-insensitive, but using canonical casing is cleaner. Also, conversational streaming often returns text/event-stream; proactively sending Accept can improve content negotiation. If you prefer to keep postStream generic, we can add Accept at the caller level once the Http interface supports custom headers.

Apply:

-        return $this->executeStream($request, ['Content-type' => 'application/json']);
+        return $this->executeStream($request, [
+            'Content-Type' => 'application/json',
+            'Accept' => 'text/event-stream',
+        ]);

src/Endpoints/Delegates/HandlesChatWorkspaces.php (2)

12-12: Public property may be too permissive; consider tightening mutability

Exposing $chats as a public mutable property invites accidental reassignments. If you need it publicly accessible (to keep $client->chats), consider making it effectively read-only (e.g., via a getter, or documenting/guarding the assignment in the owning class).

---

17-20: Method naming nit: prefer “list” over “get” for collection endpoints

Minor naming nit to align with endpoint semantics: listChatWorkspaces() would mirror the underlying listWorkspaces(). Not a blocker.

src/Endpoints/Delegates/HandlesSettings.php (1)

505-541: Docblock is comprehensive; consider factoring the searchParameters shape into a reusable type

You’ve inlined a large array shape for searchParameters twice (getChat/updateChat). Consider extracting a shared phpdoc typedef (via a dedicated value object/contract or a @phpstan-type alias) to avoid drift and keep it consistent with search settings elsewhere.

src/Contracts/ChatWorkspacesResults.php (2)

13-20: Guard against missing offset/limit to avoid notices

If the API omits offset/limit (e.g., on older servers or different defaults), accessing $params['offset']/$params['limit'] will trigger notices. Safer defaults maintain robustness without changing behavior when fields are present.

Suggested change:

-        $this->offset = $params['offset'];
-        $this->limit = $params['limit'];
+        $this->offset = $params['offset'] ?? 0;
+        $this->limit = $params['limit'] ?? \count($this->data);

If the Chats list endpoint always returns offset/limit/total, feel free to keep strict access. Otherwise, defaulting avoids runtime notices.

---

55-58: Minor: count() duplicates parent::count()

The parent Data already implements Countable. You can drop this override or delegate to parent::count() to avoid duplication.

-    public function count(): int
-    {
-        return \count($this->data);
-    }
+    public function count(): int
+    {
+        return parent::count();
+    }

tests/Endpoints/ChatWorkspaceTest.php (3)

25-31: Enable experimental features once per test class to reduce overhead

Optional: switch to setUpBeforeClass to toggle /experimental-features once, reducing per-test overhead. Not required, just cleaner/faster.

---

7-8: Avoid naming collision with the high-level Client; alias the HTTP Client import

Importing Meilisearch\Http\Client as Client can be confusing alongside $this->client (the high-level client). Alias it to improve readability.

-use Meilisearch\Http\Client;
+use Meilisearch\Http\Client as HttpClient;

And update the instantiation:

-        $http = new Client($this->host, getenv('MEILISEARCH_API_KEY'));
+        $http = new HttpClient($this->host, getenv('MEILISEARCH_API_KEY'));

---

91-131: Streaming test is pragmatic; consider a more defensive guard to reduce flakiness

The maxChunks safety is good. If the backend is slow or verbose, this can still flake. Consider:

• Lowering maxChunks (e.g., 200) and/or
• Skipping the test unless a known-good chat provider/config is available (env flag), or
• Parsing SSE “data:” lines to assert structured progress instead of raw length.

Not a blocker.

If CI occasionally flakes here, I can propose a provider-gated skip mechanism or SSE-aware parser for more deterministic assertions.

src/Contracts/ChatWorkspaceSettings.php (1)

75-87: toArray includes nulls; ensure it’s not used for PATCH bodies as-is

If this toArray() ever feeds PATCH bodies, sending explicit nulls could clear server-side values. Current code paths send raw arrays directly, so this is fine; just noting for future usage.

src/Endpoints/Delegates/HandlesChatWorkspaceSettings.php (7)

16-23: URL-encode the workspace identifier when composing the path

Workspace identifiers can contain characters that should be percent-encoded in URLs (spaces, slashes, unicode, etc.). Encode the path segment to avoid routing errors and subtle bugs.

Apply this diff:

-        $response = $this->http->get('/chats/'.$this->workspaceName.'/settings');
+        $workspace = rawurlencode($this->workspaceName);
+        $response = $this->http->get('/chats/'.$workspace.'/settings');

---

28-37: Verify allowed values for source (string union looks potentially inconsistent with API naming)

Double-check the provider identifiers against the server API. For example, docs often use lower-case openai or hyphenated azure-openai, and vllm (all lower-case). Current values ('openAi', 'azureOpenAi', 'vLlm') may not match server expectations.

If needed, consider centralizing these values in an enum or constants to avoid drift.

I can propose an enum/constant set once the canonical values are confirmed.

---

41-48: Also encode the workspace in update path

Same reasoning as for GET: encode the workspace path segment.

-        $response = $this->http->patch('/chats/'.$this->workspaceName.'/settings', $settings);
+        $workspace = rawurlencode($this->workspaceName);
+        $response = $this->http->patch('/chats/'.$workspace.'/settings', $settings);

---

55-62: Confirm DELETE semantics vs method name, and encode workspace

• The PR summary mentions “DELETE /chats/{workspace_uid}/settings — delete a workspace.” The method here is named resetSettings() and the docblock says “Reset … to default values” while returning ChatWorkspaceSettings. Please verify the server semantics:
  • If it actually deletes the workspace (likely 204/empty body), consider renaming to deleteWorkspace() and return void/array.
  • If it resets settings and returns the resulting settings, then the name and return type are fine.
• Also encode the workspace segment as with GET/PATCH.

-        $response = $this->http->delete('/chats/'.$this->workspaceName.'/settings');
+        $workspace = rawurlencode($this->workspaceName);
+        $response = $this->http->delete('/chats/'.$workspace.'/settings');

If the endpoint deletes the workspace and returns no content, adjust the signature and return accordingly:

-    public function resetSettings(): ChatWorkspaceSettings
+    public function deleteWorkspace(): void
     {
         if (null === $this->workspaceName) {
-            throw new \InvalidArgumentException('Workspace name is required to reset settings');
+            throw new \InvalidArgumentException('Workspace name is required to delete a workspace');
         }
-
-        $response = $this->http->delete('/chats/'.$this->workspaceName.'/settings');
-
-        return new ChatWorkspaceSettings($response);
+        $workspace = rawurlencode($this->workspaceName);
+        $this->http->delete('/chats/'.$workspace.'/settings');
     }

---

67-72: Tighten/clarify the options docblock; the method can enforce stream

• You can restrict role to known values ('system'|'user'|'assistant'), which improves IDE/static analysis.
• Since the method is “streamCompletion”, you can treat stream as optional and document that the method forces stream=true.

-     * @param array{
-     *     model: string,
-     *     messages: array<array{role: string, content: string}>,
-     *     stream: bool
-     * } $options The request body for the chat completion
+     * @param array{
+     *     model: string,
+     *     messages: array<array{role: 'system'|'user'|'assistant', content: string}>,
+     *     stream?: bool
+     * } $options The request body for the chat completion. Note: this method forces stream=true.

---

75-80: Force streaming flag and encode workspace in completion path

Ensure stream is set to true (it’s easy to forget from the caller) and encode the workspace path segment.

     public function streamCompletion(array $options): \Psr\Http\Message\StreamInterface
     {
         if (null === $this->workspaceName) {
             throw new \InvalidArgumentException('Workspace name is required for chat completion');
         }
 
-        return $this->http->postStream('/chats/'.$this->workspaceName.'/chat/completions', $options);
+        $options['stream'] = true;
+        $workspace = rawurlencode($this->workspaceName);
+        return $this->http->postStream('/chats/'.$workspace.'/chat/completions', $options);
     }

---

7-10: Add trait-level phpdoc for host expectations ($http, $workspaceName)

Helps Psalm/PHPStan understand that the trait expects these members on the host class, improving static analysis without changing runtime behavior.

 namespace Meilisearch\Endpoints\Delegates;
 
 use Meilisearch\Contracts\ChatWorkspaceSettings;
 
+/**
+ * @property \Meilisearch\Contracts\Http $http
+ * @property string|null $workspaceName
+ */
 trait HandlesChatWorkspaceSettings

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.

src/Http/Client.php (1)

178-215: Resolved: correct exception is now caught during streaming error parsing

Catching JsonDecodingException here addresses the previous bug where \JsonException was ineffective with the current serializer. This ensures we fall back to the raw body and throw ApiException as intended.

src/Http/Client.php (2)

142-156: Add explicit SSE Accept header (and minor header casing nit) for streaming requests

Great addition. To improve interoperability with proxies and servers that require content negotiation for streaming, set an explicit Accept header for SSE. Also, prefer canonical header casing for readability.

Apply:

-        return $this->executeStream($request, ['Content-type' => 'application/json']);
+        return $this->executeStream($request, [
+            'Content-Type' => 'application/json',
+            'Accept' => 'text/event-stream',
+        ]);

---

178-215: Broaden JSON Content-Type detection to include +json variants (e.g., application/problem+json)

isJSONResponse currently matches only application/json. Many APIs return JSON errors as application/problem+json or other application/*+json types. Broadening detection improves error parsing on both streaming and non-streaming paths without affecting NDJSON.

Outside the selected range, update isJSONResponse:

-    /**
-     * Checks if any of the header values indicate a JSON response.
-     *
-     * @param array $headerValues the array of header values to check
-     *
-     * @return bool true if any header value contains 'application/json', otherwise false
-     */
+    /**
+     * Checks if any of the header values indicate a JSON response.
+     * Matches 'application/json' and any 'application/*+json' (e.g., application/problem+json).
+     *
+     * @param array $headerValues the array of header values to check
+     *
+     * @return bool true if any header value is JSON-based, otherwise false
+     */
     private function isJSONResponse(array $headerValues): bool
     {
-        $filteredHeaders = array_filter($headerValues, static function (string $headerValue) {
-            return false !== strpos($headerValue, 'application/json');
-        });
+        $filteredHeaders = array_filter($headerValues, static function (string $headerValue) {
+            $value = strtolower($headerValue);
+            return false !== strpos($value, 'application/json')
+                || false !== strpos($value, '+json');
+        });
 
         return \count($filteredHeaders) > 0;
     }

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.

src/Http/Client.php (1)

148-149: Fix fatal: StreamInterface resolves to Meilisearch\Http\StreamInterface (missing import) — align with PSR-7 type

Both return types currently point to the local namespace and break phpstan and tests. Either import the PSR interface or fully-qualify the return type. Below diff applies the fully-qualified quick fix to unblock CI immediately.

-    public function postStream(string $path, $body = null, array $query = []): StreamInterface
+    public function postStream(string $path, $body = null, array $query = []): \Psr\Http\Message\StreamInterface
@@
-    private function executeStream(RequestInterface $request, array $headers = []): StreamInterface
+    private function executeStream(RequestInterface $request, array $headers = []): \Psr\Http\Message\StreamInterface

Recommended for readability (outside this hunk): add the import once and keep short typehints, matching the rest of the file.

use Psr\Http\Message\StreamInterface;

This resolves:

• phpstan return type covariance failures
• runtime Fatal error: class Meilisearch\Http\StreamInterface is not available

Also applies to: 185-186

src/Http/Client.php (4)

155-155: Be explicit for SSE and use canonical header casing

Add Accept: text/event-stream (many servers key behavior on this) and use conventional Content-Type casing. No behavior change otherwise.

-        return $this->executeStream($request, ['Content-type' => 'application/json']);
+        return $this->executeStream($request, [
+            'Content-Type' => 'application/json',
+            'Accept' => 'text/event-stream',
+        ]);

---

197-206: Good fallback on JSON parse failures; consider broadening JSON detection and add a test

Catching JsonDecodingException fixes the prior leak. Optional: treat application/problem+json as JSON too (common for error payloads) by relaxing the check to any subtype containing “json”.

If you want, I can open a follow-up PR to:

• extend isJSONResponse to match “json” case-insensitively
• add a test that returns a non-JSON streaming error to assert ApiException includes the raw body

---

178-215: Reduce duplication with parseResponse() by extracting error-body parsing helper

Error handling in executeStream duplicates parseResponse logic. Extract a small helper to keep behavior consistent and make future tweaks (e.g., problem+json) one-liners.

Example (outside this hunk):

/** @return array|string */
private function buildErrorBody(ResponseInterface $response)
{
    $bodyContent = (string) $response->getBody();

    if ($this->isJSONResponse($response->getHeader('content-type'))) {
        try {
            return $this->json->unserialize($bodyContent) ?? $response->getReasonPhrase();
        } catch (JsonDecodingException $e) {
            return '' !== $bodyContent ? $bodyContent : $response->getReasonPhrase();
        }
    }

    return '' !== $bodyContent ? $bodyContent : $response->getReasonPhrase();
}

Then in executeStream():

if ($response->getStatusCode() >= 300) {
    throw new ApiException($response, $this->buildErrorBody($response));
}

---

142-156: Docblock consistency: include return annotation or rely solely on signature

Minor: either add @return \Psr\Http\Message\StreamInterface to the docblock for symmetry with other methods, or keep relying on the typehint only. No functional change.

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.

src/Http/Client.php (2)

24-24: Good catch on importing StreamInterface (aligns with interface signature).

This addresses the prior suggestion to import the PSR StreamInterface for the new streaming API.

---

186-216: Error-path JSON parsing on streams is correct; minor robustness nits.

Catching JsonDecodingException fixes the previously reported leak. Two nits:

• Header name casing: we use 'Content-type' elsewhere—prefer 'Content-Type' consistently.
• JSON detection could be case-insensitive and include application/problem+json.

Outside this hunk, consider refining isJSONResponse to be case-insensitive and to detect problem+json:

private function isJSONResponse(array $headerValues): bool
{
    foreach ($headerValues as $value) {
        if (stripos($value, 'application/json') !== false
            || stripos($value, 'application/problem+json') !== false) {
            return true;
        }
    }
    return false;
}

tests/Endpoints/ChatWorkspaceTest.php (1)

26-32: Guard missing MEILISEARCH_API_KEY in tests to aid local runs.

If the env var is unset or empty, skip with a clear message (previously requested). This avoids confusing failures when running outside CI.

Apply:

-        $http = new Client($this->host, getenv('MEILISEARCH_API_KEY'));
+        $apiKey = getenv('MEILISEARCH_API_KEY');
+        if (!\is_string($apiKey) || '' === $apiKey) {
+            self::markTestSkipped('MEILISEARCH_API_KEY is not configured for chat tests.');
+        }
+        $http = new Client($this->host, $apiKey);

src/Http/Client.php (1)

143-157: Streaming POST: set Accept header and standardize header casing for SSE compatibility.

Most SSE chat-completions endpoints expect Accept: text/event-stream; adding it improves interoperability. Also use canonical header casing.

Apply:

-        return $this->executeStream($request, ['Content-type' => 'application/json']);
+        return $this->executeStream($request, [
+            'Content-Type' => 'application/json',
+            'Accept' => 'text/event-stream',
+        ]);

src/Contracts/ChatWorkspaceSettings.php (1)

100-106: Mirror the phpdoc fix in toArray() for static analysis consistency.

Return-type shape should allow null for prompts.

Apply:

-     *     prompts?: array{
+     *     prompts?: array{
      *         system: string,
      *         searchDescription: string,
      *         searchQParam: string,
      *         searchIndexUidParam: string
-     *     }
+     *     }|null

tests/Endpoints/ChatWorkspaceTest.php (2)

79-82: Make list assertion resilient to unrelated workspaces.

Exact-equality is brittle if the backend contains more than one workspace. Assert presence instead.

Apply:

-        self::assertSame([
-            ['uid' => 'myWorkspace'],
-        ], $response->getResults());
+        self::assertContains(['uid' => 'myWorkspace'], $response->getResults());

---

128-149: Reduce flakiness in streaming test by time-bounding instead of chunk-count failing.

SSE streams may keep connections open; failing on chunk count can be flaky. Prefer a short deadline and exit when some data is received.

Apply:

-            while (!$stream->eof() && $chunkCount < $maxChunks) {
+            $deadline = \microtime(true) + 15.0; // 15s safety deadline
+            while (!$stream->eof() && \microtime(true) < $deadline) {
                 $chunk = $stream->read(8192);
                 if ('' === $chunk) {
                     // Small backoff to avoid tight loop on empty reads
                     usleep(10_000);
                     continue;
                 }
                 $receivedData .= $chunk;
                 ++$chunkCount;
             }
-
-            if ($chunkCount >= $maxChunks) {
-                self::fail('Test exceeded maximum chunk limit of '.$maxChunks);
-            }
+            // If the stream never closed, we still assert we received something within the deadline.

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.

src/Contracts/ChatWorkspacePromptsSettings.php (1)

7-69: DTO looks solid and matches usage (array-access + getters).

Constructor initializes all required fields; toArray mirrors shape used by settings. No blockers.

src/Contracts/ChatWorkspaceSettings.php (1)

1-121: Use Composer’s lint scripts to fix coding style issues

The project defines CS-Fixer commands in composer.json under the lint and lint:fix scripts, so rather than invoking vendor/bin/php-cs-fixer directly (which isn’t present until dependencies are installed), run:

• To preview fixable issues:

  composer lint

• To apply fixes across the codebase (or a specific file):

  composer lint:fix
  # or to target only ChatWorkspaceSettings.php:
  composer lint:fix -- src/Contracts/ChatWorkspaceSettings.php

This will resolve the reported no_unused_imports, yoda_style, and no_extra_blank_lines errors in src/Contracts/ChatWorkspaceSettings.php.

Likely an incorrect or invalid review comment.

src/Contracts/ChatWorkspaceSettings.php (4)

7-16: Make prompts nullable (constructor and property) to allow PATCHes without prompts.

Some PATCH/GET flows won’t always include prompts; forcing a non-null DTO here is brittle and contradicts prior discussion. Make the property nullable with a sensible default.

-    private ChatWorkspacePromptsSettings $prompts;
+    private ?ChatWorkspacePromptsSettings $prompts = null;

---

18-34: Docblock: allow prompts to be omitted or null.

The current phpdoc requires prompts, but the API can accept settings without prompts. Update the shape accordingly.

-     *     apiKey?: string,
-     *     prompts: array{
+     *     apiKey?: string,
+     *     prompts?: array{
      *         system: string,
      *         searchDescription: string,
      *         searchQParam: string,
      *         searchIndexUidParam: string
-     *     }
+     *     }|null

---

35-47: Constructor: guard prompts access; construct DTO only when present and non-null.

Prevents undefined-index notices and aligns with the updated docblock.

-        $this->prompts = new ChatWorkspacePromptsSettings($params['prompts']);
+        $this->prompts = (isset($params['prompts']) && null !== $params['prompts'])
+            ? new ChatWorkspacePromptsSettings($params['prompts'])
+            : null;

---

5-8: Unused same-namespace import is gone — CS issue addressed.

Previous CS failure about importing ChatWorkspacePromptsSettings from the same namespace is resolved here. Thanks for cleaning it up.

src/Contracts/ChatWorkspaceSettings.php (5)

84-87: Getter should return nullable type if prompts is optional.

Keeps public API consistent with constructor and property.

-    public function getPrompts(): ChatWorkspacePromptsSettings
+    public function getPrompts(): ?ChatWorkspacePromptsSettings

---

89-105: toArray() phpdoc: optional keys vs. nulls mismatch.

The docblock marks optional keys with ?:, but implementation currently returns keys with null values. Decide on one behavior; if you filter nulls (recommended), keep ?: in phpdoc. If not filtering, change to key: string|null. Suggest adopting the filtering approach below and keeping ?:.

-     *     apiKey?: string,
-     *     prompts: array{
+     *     apiKey?: string,
+     *     prompts?: array{
      *         system: string,
      *         searchDescription: string,
      *         searchQParam: string,
      *         searchIndexUidParam: string
-     *     }
+     *     }|null

---

106-118: Omit nulls in serialized payload to avoid unintended resets; include prompts only when set.

Sending "apiKey": null (or other nulls) might be interpreted as clearing a field server-side. Prefer omitting nulls entirely.

-        return [
-            'source' => $this->source,
-            'orgId' => $this->orgId,
-            'projectId' => $this->projectId,
-            'apiVersion' => $this->apiVersion,
-            'deploymentId' => $this->deploymentId,
-            'baseUrl' => $this->baseUrl,
-            'apiKey' => $this->apiKey,
-            'prompts' => $this->prompts->toArray(),
-        ];
+        $result = [
+            'source' => $this->source,
+            'orgId' => $this->orgId,
+            'projectId' => $this->projectId,
+            'apiVersion' => $this->apiVersion,
+            'deploymentId' => $this->deploymentId,
+            'baseUrl' => $this->baseUrl,
+            'apiKey' => $this->apiKey,
+        ];
+
+        if (null !== $this->prompts) {
+            $result['prompts'] = $this->prompts->toArray();
+        }
+
+        // Drop nulls to avoid accidental clears on PATCH.
+        return \array_filter($result, static fn ($v) => null !== $v);

---

79-82: Sensitive data: avoid accidental logging of apiKey.

This DTO is often dumped to logs in client apps. Consider documenting that toArray() is a request payload and should not be logged verbatim, or add a dedicated toRequestArray() that includes apiKey while toArray() redacts it (e.g., 'apiKey' => $this->apiKey !== null ? '***' : null).

---

39-46: Optional: lightweight validation for required source.

If empty strings are invalid for source, throw an InvalidArgumentException early; otherwise, add a phpdoc note about accepted values to guide callers.

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.