Added ollama agent for embeddings #15
Conversation
* added getEmbeddingDimension for getting dimension of the current embedding model
WalkthroughAdds embedding support to the Agents framework: Adapter now declares Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (1)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (5)
ollama.dockerfile (2)
8-14: Consider using a more robust server readiness check.The
sleep 5assumes the Ollama server starts within 5 seconds, which may be unreliable on slower systems or under load. Consider using a polling loop that checks for server readiness.Apply this diff to improve robustness:
# Pre-pull models at build time for Docker layer caching -RUN ollama serve & \ - sleep 5 && \ +RUN ollama serve & SERVER_PID=$! && \ + for i in {1..30}; do curl -s http://localhost:11434/api/tags >/dev/null && break || sleep 1; done && \ for m in $MODELS; do \ echo "Pulling model $m..."; \ ollama pull $m || exit 1; \ done && \ - pkill ollama + kill $SERVER_PIDThis change:
- Waits up to 30 seconds for the server to be ready
- Checks the API endpoint instead of using a fixed sleep
- Kills the specific server process instead of using
pkill
20-20: Improve grep pattern to use word boundary matching.The
grep -q $mpattern is unanchored and performs substring matching, creating a false-positive risk if model names have common prefixes or if the output format changes. Based on ollama list output showing a table with NAME, ID, SIZE, and MODIFIED columns, model names appear at the line start followed by whitespace.Recommended fix: Use
grep -qw "$m"(word boundary) orgrep -q "^$m\\s"(anchored to line start) to ensure only exact model name matches.tests/Agents/AgentTest.php (2)
99-110: Consider extracting the long test text to improve readability.The very long Lorem Ipsum text (5000+ characters) on line 104 significantly impacts code readability. Consider extracting it to a constant or separate test fixture file.
Example refactor:
private const LONG_TEXT = 'Lorem ipsum dolor sit amet...'; // truncated for readability public function testEmbedReturnsArrayWithEmbeddingAdapter(): void { $ollama = new Ollama(); $ollama->setTimeout(10000); $agent = new Agent($ollama); $result = $agent->embed(self::LONG_TEXT); // assertions... }Alternatively, you could use a shorter representative text since
testEmbeddingDimensionsalready covers multiple text lengths including repeated patterns.
112-137: Consider setting timeout consistently across embedding tests.The first embedding test sets
setTimeout(10000)on line 102, but this test doesn't set an explicit timeout. For consistency and to prevent potential timeout issues with long texts (line 122), consider setting the same timeout here.Apply this diff:
public function testEmbeddingDimensions(): void { $ollama = new Ollama(); + $ollama->setTimeout(10000); $agent = new Agent($ollama);src/Agents/Adapters/Deepseek.php (1)
321-334: Code duplication across adapters.The embedding stub implementation is identical across Deepseek, Gemini, and Anthropic adapters. While this is acceptable for "Chill" review mode and the methods are simple, consider whether a trait or abstract base implementation might reduce duplication in future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
composer.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
docker-compose.yml(3 hunks)ollama.dockerfile(1 hunks)src/Agents/Adapter.php(1 hunks)src/Agents/Adapters/Anthropic.php(1 hunks)src/Agents/Adapters/Deepseek.php(1 hunks)src/Agents/Adapters/Gemini.php(1 hunks)src/Agents/Adapters/Ollama.php(1 hunks)src/Agents/Adapters/OpenAI.php(1 hunks)src/Agents/Adapters/Perplexity.php(2 hunks)src/Agents/Adapters/XAI.php(2 hunks)src/Agents/Agent.php(1 hunks)tests/Agents/AgentTest.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (10)
src/Agents/Adapters/Deepseek.php (4)
src/Agents/Adapter.php (3)
getSupportForEmbeddings(101-101)embed(109-109)getEmbeddingDimension(114-114)src/Agents/Adapters/Ollama.php (3)
getSupportForEmbeddings(204-207)embed(64-89)getEmbeddingDimension(114-117)src/Agents/Adapters/OpenAI.php (3)
getSupportForEmbeddings(418-421)embed(423-426)getEmbeddingDimension(428-431)src/Agents/Agent.php (1)
embed(148-156)
src/Agents/Adapters/OpenAI.php (2)
src/Agents/Adapter.php (3)
getSupportForEmbeddings(101-101)embed(109-109)getEmbeddingDimension(114-114)src/Agents/Agent.php (1)
embed(148-156)
src/Agents/Adapters/Perplexity.php (2)
src/Agents/Adapter.php (2)
getSupportForEmbeddings(101-101)embed(109-109)src/Agents/Agent.php (1)
embed(148-156)
src/Agents/Adapters/Gemini.php (3)
src/Agents/Adapter.php (3)
getSupportForEmbeddings(101-101)embed(109-109)getEmbeddingDimension(114-114)src/Agents/Adapters/Ollama.php (3)
getSupportForEmbeddings(204-207)embed(64-89)getEmbeddingDimension(114-117)src/Agents/Agent.php (1)
embed(148-156)
src/Agents/Adapters/XAI.php (3)
src/Agents/Adapter.php (2)
getSupportForEmbeddings(101-101)embed(109-109)src/Agents/Adapters/Ollama.php (2)
getSupportForEmbeddings(204-207)embed(64-89)src/Agents/Agent.php (1)
embed(148-156)
src/Agents/Adapters/Anthropic.php (3)
src/Agents/Adapter.php (3)
getSupportForEmbeddings(101-101)embed(109-109)getEmbeddingDimension(114-114)src/Agents/Adapters/Ollama.php (3)
getSupportForEmbeddings(204-207)embed(64-89)getEmbeddingDimension(114-117)src/Agents/Agent.php (1)
embed(148-156)
src/Agents/Agent.php (8)
src/Agents/Adapter.php (2)
embed(109-109)getSupportForEmbeddings(101-101)src/Agents/Adapters/Anthropic.php (2)
embed(466-469)getSupportForEmbeddings(461-464)src/Agents/Adapters/Deepseek.php (2)
embed(326-329)getSupportForEmbeddings(321-324)src/Agents/Adapters/Gemini.php (2)
embed(357-360)getSupportForEmbeddings(352-355)src/Agents/Adapters/Ollama.php (2)
embed(64-89)getSupportForEmbeddings(204-207)src/Agents/Adapters/OpenAI.php (2)
embed(423-426)getSupportForEmbeddings(418-421)src/Agents/Adapters/Perplexity.php (2)
embed(217-220)getSupportForEmbeddings(110-113)src/Agents/Adapters/XAI.php (2)
embed(167-170)getSupportForEmbeddings(92-95)
src/Agents/Adapter.php (8)
src/Agents/Adapters/Anthropic.php (3)
getSupportForEmbeddings(461-464)embed(466-469)getEmbeddingDimension(471-474)src/Agents/Adapters/Deepseek.php (3)
getSupportForEmbeddings(321-324)embed(326-329)getEmbeddingDimension(331-334)src/Agents/Adapters/Gemini.php (3)
getSupportForEmbeddings(352-355)embed(357-360)getEmbeddingDimension(362-365)src/Agents/Adapters/Ollama.php (3)
getSupportForEmbeddings(204-207)embed(64-89)getEmbeddingDimension(114-117)src/Agents/Adapters/OpenAI.php (3)
getSupportForEmbeddings(418-421)embed(423-426)getEmbeddingDimension(428-431)src/Agents/Adapters/Perplexity.php (2)
getSupportForEmbeddings(110-113)embed(217-220)src/Agents/Adapters/XAI.php (2)
getSupportForEmbeddings(92-95)embed(167-170)src/Agents/Agent.php (1)
embed(148-156)
tests/Agents/AgentTest.php (3)
src/Agents/Adapters/Ollama.php (3)
Ollama(9-208)embed(64-89)getEmbeddingDimension(114-117)src/Agents/Adapter.php (3)
setTimeout(255-260)embed(109-109)getEmbeddingDimension(114-114)src/Agents/Agent.php (2)
Agent(5-157)embed(148-156)
src/Agents/Adapters/Ollama.php (2)
src/Agents/Adapter.php (12)
Adapter(5-271)embed(109-109)setTimeout(255-260)getModels(72-72)getModel(79-79)getEmbeddingDimension(114-114)setModel(87-87)send(65-65)isSchemaSupported(94-94)getName(54-54)formatErrorMessage(122-122)getSupportForEmbeddings(101-101)src/Agents/Agent.php (1)
embed(148-156)
🪛 GitHub Actions: CodeQL
src/Agents/Adapters/Deepseek.php
[error] 326-326: PHPStan: Method Utopia\Agents\Adapters\Deepseek::embed() return type has no value type specified in iterable type array.
src/Agents/Adapters/OpenAI.php
[error] 423-423: PHPStan: Method Utopia\Agents\Adapters\OpenAI::embed() return type has no value type specified in iterable type array.
src/Agents/Adapters/Perplexity.php
[error] 217-217: PHPStan: Method Utopia\Agents\Adapters\Perplexity::embed() return type has no value type specified in iterable type array.
src/Agents/Adapters/Gemini.php
[error] 357-357: PHPStan: Method Utopia\Agents\Adapters\Gemini::embed() return type has no value type specified in iterable type array.
src/Agents/Adapters/XAI.php
[error] 167-167: PHPStan: Method Utopia\Agents\Adapters\XAI::embed() return type has no value type specified in iterable type array.
src/Agents/Adapters/Anthropic.php
[error] 466-466: PHPStan: Method Utopia\Agents\Adapters\Anthropic::embed() return type has no value type specified in iterable type array.
src/Agents/Agent.php
[error] 148-148: PHPStan: Method Utopia\Agents\Agent::embed() return type has no value type specified in iterable type array.
src/Agents/Adapter.php
[error] 109-109: PHPStan: Method Utopia\Agents\Adapter::embed() return type has no value type specified in iterable type array.
src/Agents/Adapters/Ollama.php
[error] 64-64: PHPStan: Method Utopia\Agents\Adapters\Ollama::embed() return type has no value type specified in iterable type array.
[error] 140-140: PHPStan: Method Utopia\Agents\Adapters\Ollama::send() has parameter $messages with no value type specified in iterable type array.
🪛 GitHub Actions: Tests
docker-compose.yml
[warning] 1-1: The attribute version is obsolete, it will be ignored. Remove it to avoid potential confusion.
🪛 PHPMD (2.15.0)
src/Agents/Adapters/Deepseek.php
326-326: Avoid unused parameters such as '$text'. (undefined)
(UnusedFormalParameter)
src/Agents/Adapters/OpenAI.php
423-423: Avoid unused parameters such as '$text'. (undefined)
(UnusedFormalParameter)
src/Agents/Adapters/Perplexity.php
217-217: Avoid unused parameters such as '$text'. (undefined)
(UnusedFormalParameter)
src/Agents/Adapters/Gemini.php
357-357: Avoid unused parameters such as '$text'. (undefined)
(UnusedFormalParameter)
src/Agents/Adapters/XAI.php
167-167: Avoid unused parameters such as '$text'. (undefined)
(UnusedFormalParameter)
src/Agents/Adapters/Anthropic.php
466-466: Avoid unused parameters such as '$text'. (undefined)
(UnusedFormalParameter)
src/Agents/Adapters/Ollama.php
140-140: Avoid unused parameters such as '$messages'. (undefined)
(UnusedFormalParameter)
140-140: Avoid unused parameters such as '$listener'. (undefined)
(UnusedFormalParameter)
🔇 Additional comments (5)
docker-compose.yml (1)
27-41: LGTM! Ollama service configuration is well-designed.The service configuration properly:
- Uses a custom Dockerfile with model preloading
- Exposes the standard Ollama port (11434)
- Persists models across restarts with a named volume
- Sets appropriate restart policy
tests/Agents/AgentTest.php (1)
99-137: LGTM! Comprehensive embedding test coverage.The test suite effectively validates:
- Embedding response structure (embedding array, timing fields)
- Dimension consistency across various input lengths
- Edge cases (short text, long paragraphs, repeated patterns)
The tests properly exercise the Ollama adapter and verify the Agent-to-adapter delegation pattern.
src/Agents/Adapters/Ollama.php (3)
11-35: LGTM! Constants and properties are well-structured.The hardcoded endpoint
http://ollama:11434/api/embeduses a Docker service name, which is appropriate given the Docker-based deployment context shown in the PR.
43-49: LGTM! Constructor is clean and straightforward.
150-207: LGTM! Supporting methods are well-implemented.The remaining methods (
isSchemaSupported,getName,formatErrorMessage, endpoint management, andgetSupportForEmbeddings) are correctly implemented for an embedding-only adapter.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
docker-compose.yml (1)
1-1: Remove obsoleteversionfield.The
versionattribute is obsolete in modern Docker Compose and should be removed to avoid confusion.Apply this diff:
-version: "3.9" - services:src/Agents/Adapter.php (1)
115-118: Fix typo in docblock.Line 116 contains a typo: "dimenion" should be "dimension". Additionally, the method lacks a
@returnannotation.Apply this diff:
/** - * get embedding dimenion of the current model + * Get embedding dimension of the current model + * + * @return int */ abstract public function getEmbeddingDimension(): int;src/Agents/Adapters/Ollama.php (1)
124-129: Add model validation in setModel().The method allows setting any string as the model without validating it exists in the
MODELSconstant or has a dimension mapping inDIMENSIONS. This will cause errors whengetEmbeddingDimension()is called with an invalid model.Apply this diff:
public function setModel(string $model): self { + if (! in_array($model, self::MODELS, true)) { + throw new \InvalidArgumentException("Invalid model: {$model}. Supported models: " . implode(', ', self::MODELS)); + } $this->model = $model; return $this; }
🧹 Nitpick comments (7)
src/Agents/Adapters/OpenAI.php (1)
436-439: Consider adding PHPDoc for consistency.While the return type is clear from the signature, adding a brief PHPDoc comment would improve consistency with other methods and provide context for the exception thrown.
Apply this diff:
+ /** + * Get embedding dimension + * + * @return int + * @throws \Exception + */ public function getEmbeddingDimension(): int { throw new \Exception('Embeddings are not supported for this adapter.'); }src/Agents/Adapters/Anthropic.php (1)
479-482: Consider adding PHPDoc for consistency.While the return type is clear from the signature, adding a brief PHPDoc comment would improve consistency with the
embed()method and document the exception being thrown.Apply this diff:
+ /** + * Get embedding dimension + * + * @return int + * @throws \Exception + */ public function getEmbeddingDimension(): int { throw new \Exception('Embeddings are not supported for this adapter.'); }src/Agents/Adapters/Deepseek.php (1)
339-342: Consider adding PHPDoc for consistency.While the return type is clear from the signature, adding a brief PHPDoc comment would improve consistency with the
embed()method and document the exception being thrown.Apply this diff:
+ /** + * Get embedding dimension + * + * @return int + * @throws \Exception + */ public function getEmbeddingDimension(): int { throw new \Exception('Embeddings are not supported for this adapter.'); }src/Agents/Adapters/Gemini.php (1)
370-373: Consider adding PHPDoc for consistency.While the return type is clear from the signature, adding a brief PHPDoc comment would improve consistency with the
embed()method and document the exception being thrown.Apply this diff:
+ /** + * Get embedding dimension + * + * @return int + * @throws \Exception + */ public function getEmbeddingDimension(): int { throw new \Exception('Embeddings are not supported for this adapter.'); }tests/Agents/AgentTest.php (2)
104-104: Extract long test string to improve readability.The 5000+ word Lorem ipsum text embedded directly in the test makes it difficult to read and maintain. Consider extracting it to a separate fixture file or generating it programmatically.
For example:
$longText = file_get_contents(__DIR__ . '/fixtures/long_text.txt'); // or $longText = str_repeat('Lorem ipsum dolor sit amet. ', 200);
112-137: Consider consistent timeout configuration across tests.This test doesn't set a timeout (using the default 90 seconds), while
testEmbedReturnsArrayWithEmbeddingAdapterexplicitly sets 10000. For consistency and clarity, consider using the same timeout approach in both tests—either set it explicitly in both or rely on the default in both.src/Agents/Adapters/Ollama.php (1)
26-26: Document the endpoint assumption.The hardcoded hostname
ollamaassumes a Docker Compose service name. WhilesetEndpoint()allows overriding this, consider adding a comment or documentation noting this assumption for users deploying in different environments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
docker-compose.yml(2 hunks)src/Agents/Adapter.php(1 hunks)src/Agents/Adapters/Anthropic.php(1 hunks)src/Agents/Adapters/Deepseek.php(1 hunks)src/Agents/Adapters/Gemini.php(1 hunks)src/Agents/Adapters/Ollama.php(1 hunks)src/Agents/Adapters/OpenAI.php(1 hunks)src/Agents/Adapters/Perplexity.php(2 hunks)src/Agents/Adapters/XAI.php(2 hunks)src/Agents/Agent.php(1 hunks)tests/Agents/AgentTest.php(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Agents/Agent.php
🧰 Additional context used
🧬 Code graph analysis (9)
src/Agents/Adapters/Perplexity.php (2)
src/Agents/Adapter.php (2)
getSupportForEmbeddings(101-101)embed(113-113)src/Agents/Agent.php (1)
embed(152-159)
src/Agents/Adapters/XAI.php (3)
src/Agents/Adapter.php (2)
getSupportForEmbeddings(101-101)embed(113-113)src/Agents/Adapters/Ollama.php (2)
getSupportForEmbeddings(203-206)embed(63-88)src/Agents/Agent.php (1)
embed(152-159)
src/Agents/Adapters/Anthropic.php (4)
src/Agents/Adapter.php (3)
getSupportForEmbeddings(101-101)embed(113-113)getEmbeddingDimension(118-118)src/Agents/Adapters/Ollama.php (3)
getSupportForEmbeddings(203-206)embed(63-88)getEmbeddingDimension(113-116)src/Agents/Adapters/OpenAI.php (3)
getSupportForEmbeddings(418-421)embed(431-434)getEmbeddingDimension(436-439)src/Agents/Agent.php (1)
embed(152-159)
tests/Agents/AgentTest.php (3)
src/Agents/Adapters/Ollama.php (3)
Ollama(9-207)embed(63-88)getEmbeddingDimension(113-116)src/Agents/Adapter.php (3)
setTimeout(259-264)embed(113-113)getEmbeddingDimension(118-118)src/Agents/Agent.php (2)
Agent(5-160)embed(152-159)
src/Agents/Adapter.php (8)
src/Agents/Adapters/Anthropic.php (3)
getSupportForEmbeddings(461-464)embed(474-477)getEmbeddingDimension(479-482)src/Agents/Adapters/Deepseek.php (3)
getSupportForEmbeddings(321-324)embed(334-337)getEmbeddingDimension(339-342)src/Agents/Adapters/Gemini.php (3)
getSupportForEmbeddings(352-355)embed(365-368)getEmbeddingDimension(370-373)src/Agents/Adapters/Ollama.php (3)
getSupportForEmbeddings(203-206)embed(63-88)getEmbeddingDimension(113-116)src/Agents/Adapters/OpenAI.php (3)
getSupportForEmbeddings(418-421)embed(431-434)getEmbeddingDimension(436-439)src/Agents/Adapters/Perplexity.php (2)
getSupportForEmbeddings(110-113)embed(225-228)src/Agents/Adapters/XAI.php (2)
getSupportForEmbeddings(92-95)embed(175-178)src/Agents/Agent.php (1)
embed(152-159)
src/Agents/Adapters/Gemini.php (5)
src/Agents/Adapter.php (3)
getSupportForEmbeddings(101-101)embed(113-113)getEmbeddingDimension(118-118)src/Agents/Adapters/Anthropic.php (3)
getSupportForEmbeddings(461-464)embed(474-477)getEmbeddingDimension(479-482)src/Agents/Adapters/Ollama.php (3)
getSupportForEmbeddings(203-206)embed(63-88)getEmbeddingDimension(113-116)src/Agents/Adapters/OpenAI.php (3)
getSupportForEmbeddings(418-421)embed(431-434)getEmbeddingDimension(436-439)src/Agents/Agent.php (1)
embed(152-159)
src/Agents/Adapters/Ollama.php (3)
src/Agents/Adapter.php (12)
Adapter(5-275)embed(113-113)setTimeout(259-264)getModels(72-72)getModel(79-79)getEmbeddingDimension(118-118)setModel(87-87)send(65-65)isSchemaSupported(94-94)getName(54-54)formatErrorMessage(126-126)getSupportForEmbeddings(101-101)src/Agents/Message.php (1)
Message(5-48)src/Agents/Agent.php (2)
__construct(34-41)embed(152-159)
src/Agents/Adapters/OpenAI.php (4)
src/Agents/Adapter.php (3)
getSupportForEmbeddings(101-101)embed(113-113)getEmbeddingDimension(118-118)src/Agents/Adapters/Anthropic.php (3)
getSupportForEmbeddings(461-464)embed(474-477)getEmbeddingDimension(479-482)src/Agents/Adapters/Ollama.php (3)
getSupportForEmbeddings(203-206)embed(63-88)getEmbeddingDimension(113-116)src/Agents/Agent.php (1)
embed(152-159)
src/Agents/Adapters/Deepseek.php (3)
src/Agents/Adapter.php (3)
getSupportForEmbeddings(101-101)embed(113-113)getEmbeddingDimension(118-118)src/Agents/Adapters/Ollama.php (3)
getSupportForEmbeddings(203-206)embed(63-88)getEmbeddingDimension(113-116)src/Agents/Agent.php (1)
embed(152-159)
🪛 PHPMD (2.15.0)
src/Agents/Adapters/Perplexity.php
225-225: Avoid unused parameters such as '$text'. (undefined)
(UnusedFormalParameter)
src/Agents/Adapters/XAI.php
175-175: Avoid unused parameters such as '$text'. (undefined)
(UnusedFormalParameter)
src/Agents/Adapters/Anthropic.php
474-474: Avoid unused parameters such as '$text'. (undefined)
(UnusedFormalParameter)
src/Agents/Adapters/Gemini.php
365-365: Avoid unused parameters such as '$text'. (undefined)
(UnusedFormalParameter)
src/Agents/Adapters/Ollama.php
139-139: Avoid unused parameters such as '$messages'. (undefined)
(UnusedFormalParameter)
139-139: Avoid unused parameters such as '$listener'. (undefined)
(UnusedFormalParameter)
src/Agents/Adapters/OpenAI.php
431-431: Avoid unused parameters such as '$text'. (undefined)
(UnusedFormalParameter)
src/Agents/Adapters/Deepseek.php
334-334: Avoid unused parameters such as '$text'. (undefined)
(UnusedFormalParameter)
⏰ 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). (1)
- GitHub Check: Tests
🔇 Additional comments (16)
src/Agents/Adapters/XAI.php (2)
87-95: LGTM! Embedding support flag correctly implemented.The method correctly indicates that XAI adapter does not support embeddings, aligning with the embedding API surface introduced across adapters.
167-178: LGTM! PHPDoc type hints properly added.The method now includes proper PHPDoc with detailed return type specification, addressing the previous review feedback. The PHPMD warning about the unused
$textparameter is a false positive—the parameter is required by the abstract method contract.docker-compose.yml (2)
21-24: LGTM! Service dependencies properly configured.The test service correctly depends on the Ollama service and is properly networked, ensuring the embedding tests can communicate with Ollama.
26-40: LGTM! Ollama service properly configured.The service configuration includes appropriate persistence for model caching, proper networking, and build-time model specification.
src/Agents/Adapters/Perplexity.php (2)
105-113: LGTM! Embedding support correctly indicated.The method properly signals that Perplexity does not support embeddings, consistent with the embedding API pattern.
217-228: LGTM! PHPDoc type hints properly added.The embed method now includes comprehensive PHPDoc with detailed return type specification, addressing the previous review feedback. The PHPMD warning about the unused
$textparameter is a false positive—the parameter is required by the abstract method contract.src/Agents/Adapters/OpenAI.php (2)
418-421: LGTM! Embedding support flag correctly implemented.The method clearly indicates OpenAI adapter does not support embeddings in this implementation.
423-434: LGTM! PHPDoc type hints properly added.The method now includes comprehensive PHPDoc with detailed return type specification, addressing the previous review feedback. The PHPMD warning about the unused
$textparameter is a false positive—the parameter is required by the abstract method contract.src/Agents/Adapters/Anthropic.php (2)
461-464: LGTM! Embedding support flag correctly implemented.The method properly indicates that Anthropic does not support embeddings, aligning with the framework's embedding API pattern.
466-477: LGTM! PHPDoc type hints properly added.The method now includes comprehensive PHPDoc with detailed return type specification, addressing the previous review feedback. The PHPMD warning about the unused
$textparameter is a false positive—the parameter is required by the abstract method contract.src/Agents/Adapter.php (2)
96-101: LGTM! Embedding support contract properly defined.The abstract method correctly establishes the contract for adapters to indicate embedding support.
103-113: LGTM! PHPDoc with detailed return type specification.The method now includes a comprehensive structured return type, clearly documenting the expected array shape including embedding vector, total_duration, and load_duration fields.
src/Agents/Adapters/Deepseek.php (2)
321-324: LGTM! Embedding support flag correctly implemented.The method properly signals that Deepseek does not support embeddings, consistent with the embedding API pattern.
326-337: LGTM! PHPDoc type hints properly added.The method now includes comprehensive PHPDoc with detailed return type specification, addressing the previous review feedback. The PHPMD warning about the unused
$textparameter is a false positive—the parameter is required by the abstract method contract.src/Agents/Adapters/Gemini.php (2)
352-355: LGTM! Embedding support flag correctly implemented.The method properly indicates that Gemini does not support embeddings in this implementation, aligning with the framework's embedding API pattern.
357-368: LGTM! PHPDoc type hints properly added.The method now includes comprehensive PHPDoc with detailed return type specification, addressing the previous review feedback. The PHPMD warning about the unused
$textparameter is a false positive—the parameter is required by the abstract method contract.
* removed reduant timeout
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/Agents/Adapters/Ollama.php (2)
58-83: Harden embed(): status check + typed JSON + shaped return (fixes PHPStan/CodeQL).Add HTTP error handling, validate/shape the JSON, and return typed values. Also prefer getTimeout(). Response schema uses embeddings: number[][]; supports dimensions. (docs.ollama.com)
public function embed(string $text): array { - $client = new Client(); - $client->setTimeout($this->timeout); + $client = new Client(); + $client->setTimeout($this->getTimeout()); $client->addHeader('Content-Type', 'application/json'); $payload = [ 'model' => $this->model, 'input' => $text, ]; $response = $client->fetch( $this->getEndpoint(), Client::METHOD_POST, $payload ); - $body = $response->getBody(); - $json = is_string($body) ? json_decode($body, true) : null; - if (isset($json['error'])) { - throw new \Exception($json['error']); - } - - return [ - 'embedding' => $json['embeddings'][0], - 'total_duration' => $json['total_duration'] ?? null, - 'load_duration' => $json['load_duration'] ?? null, - ]; + if ($response->getStatusCode() >= 400) { + $raw = $response->getBody(); + $jsonErr = is_string($raw) ? json_decode($raw, true) : null; + throw new \Exception( + 'Ollama API error ('.$response->getStatusCode().'): '.$this->formatErrorMessage($jsonErr), + $response->getStatusCode() + ); + } + + $body = $response->getBody(); + $json = is_string($body) ? json_decode($body, true) : null; + if (!is_array($json)) { + throw new \Exception('Invalid JSON from Ollama embed API'); + } + if (isset($json['error'])) { + throw new \Exception($this->formatErrorMessage($json)); + } + $embeddings = $json['embeddings'] ?? null; + if (!is_array($embeddings) || !isset($embeddings[0]) || !is_array($embeddings[0])) { + throw new \Exception('Invalid response from Ollama embed API: missing embeddings'); + } + $vector = array_map(static fn($v): float => (float) $v, $embeddings[0]); + $totalDuration = array_key_exists('total_duration', $json) ? (int) $json['total_duration'] : null; + $loadDuration = array_key_exists('load_duration', $json) ? (int) $json['load_duration'] : null; + + return [ + 'embedding' => $vector, + 'total_duration' => $totalDuration, + 'load_duration' => $loadDuration, + ]; }As per Adapter::embed() contract. Based on past review comments.
108-111: Guard DIMENSIONS lookup to avoid undefined array key.Validate model exists before indexing; throw a descriptive exception.
- public function getEmbeddingDimension(): int - { - return self::DIMENSIONS[$this->model]; - } + public function getEmbeddingDimension(): int + { + if (!isset(self::DIMENSIONS[$this->model])) { + throw new \DomainException("Embedding dimension not defined for model: {$this->model}"); + } + return self::DIMENSIONS[$this->model]; + }Based on past review comments.
🧹 Nitpick comments (3)
src/Agents/Adapters/Ollama.php (3)
134-137: Silence PHPMD unused parameters in send().Explicitly mark parameters as unused to keep static analysis clean.
public function send(array $messages, ?callable $listener = null): Message { + unset($messages, $listener); throw new \Exception('OllamaAdapter does not support chat or messages. Use embed() instead.'); }
32-44: Optional: support custom output dimensions via API.Ollama supports a dimensions request param; consider exposing it (constructor/setter) and reflecting it in getEmbeddingDimension(). (docs.ollama.com)
- Add property and optional ctor arg:
- protected string $model; + protected string $model; + protected ?int $dimensions = null; @@ - int $timeout = 90 + int $timeout = 90, + ?int $dimensions = null ) { $this->model = $model; $this->setTimeout($timeout); + $this->dimensions = $dimensions; }
- Include in payload when set:
$payload = [ 'model' => $this->model, 'input' => $text, ]; + if ($this->dimensions !== null) { + $payload['dimensions'] = $this->dimensions; + }
- Reflect in dimension getter:
- if (!isset(self::DIMENSIONS[$this->model])) { + if ($this->dimensions !== null) { + return $this->dimensions; + } + if (!isset(self::DIMENSIONS[$this->model])) { throw new \DomainException("Embedding dimension not defined for model: {$this->model}"); } return self::DIMENSIONS[$this->model];Also applies to: 63-66, 108-111
60-62: Tiny: prefer accessor for timeout.Use getTimeout() for consistency with Adapter API (already applied in main diff above).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Agents/Adapters/Ollama.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/Agents/Adapters/Ollama.php (2)
src/Agents/Adapter.php (12)
Adapter(5-275)setTimeout(259-264)embed(113-113)getModels(72-72)getModel(79-79)getEmbeddingDimension(118-118)setModel(87-87)send(65-65)isSchemaSupported(94-94)getName(54-54)formatErrorMessage(126-126)getSupportForEmbeddings(101-101)src/Agents/Agent.php (2)
__construct(34-41)embed(152-159)
🪛 GitHub Actions: CodeQL
src/Agents/Adapters/Ollama.php
[error] 74-75: PHPStan: Cannot access offset 'error' on mixed.
[error] 75-75: PHPStan: Parameter #1 $message of class Exception constructor expects string, mixed given.
[error] 78-79: PHPStan: Method Ollama::embed() should return array{embedding: array<int, float>, total_duration: int|null, load_duration: int|null} but returns array{embedding: mixed, total_duration: mixed, load_duration: mixed}.
[error] 79-81: PHPStan: Cannot access offset 'embeddings' on mixed; Cannot access offset 0 on mixed; Cannot access offset 'total_duration' on mixed; Cannot access offset 'load_duration' on mixed.
🪛 GitHub Actions: Linter
src/Agents/Adapters/Ollama.php
[error] 1-1: Laravel Pint lint error: unary_operator_spaces, not_operator_with_s…
🪛 PHPMD (2.15.0)
src/Agents/Adapters/Ollama.php
134-134: Avoid unused parameters such as '$messages'. (undefined)
(UnusedFormalParameter)
134-134: Avoid unused parameters such as '$listener'. (undefined)
(UnusedFormalParameter)
⏰ 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). (1)
- GitHub Check: Tests
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
src/Agents/Adapters/Ollama.php (3)
174-181: Fix Pint style error (previously flagged).Line 176 has a space after the
!operator which violates thenot_operator_with_successor_spacerule.Apply this diff:
- if (! is_array($json)) { + if (!is_array($json)) { return '(unknown_error) Unknown error'; }
117-120: Add validation for model dimension lookup.This issue was previously flagged: the method accesses
self::DIMENSIONS[$this->model]without checking if the key exists, which will cause an undefined array key error for unknown models.As suggested in the previous review, apply this validation:
public function getEmbeddingDimension(): int { + if (!isset(self::DIMENSIONS[$this->model])) { + throw new \Exception("Embedding dimension not defined for model: {$this->model}"); + } return self::DIMENSIONS[$this->model]; }
58-92: Add validation before accessing response array.Line 88 directly accesses
$json['embeddings'][0]without validating the response structure. This can cause undefined array key errors or null pointer exceptions if the API returns an unexpected response format (e.g., null JSON, missing 'embeddings' key, or empty embeddings array).Apply this diff to add proper validation:
$json = is_string($body) ? json_decode($body, true) : null; if (isset($json['error'])) { throw new \Exception($json['error'], $response->getStatusCode()); } + + if (!is_array($json) || !isset($json['embeddings'][0])) { + throw new \Exception('Invalid response from Ollama embed API'); + } return [ 'embedding' => $json['embeddings'][0], 'total_duration' => $json['total_duration'] ?? null, 'load_duration' => $json['load_duration'] ?? null, ];
🧹 Nitpick comments (1)
src/Agents/Adapters/Ollama.php (1)
128-133: Consider validating the model parameter.The method accepts any string without checking if it's a supported model. This could lead to issues when
getEmbeddingDimension()is called with an unsupported model.Apply this diff to add validation:
public function setModel(string $model): self { + if (!in_array($model, self::MODELS, true)) { + throw new \Exception("Unsupported model: {$model}. Supported models: " . implode(', ', self::MODELS)); + } $this->model = $model; return $this; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Agents/Adapters/Ollama.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/Agents/Adapters/Ollama.php (2)
src/Agents/Adapter.php (12)
Adapter(5-275)setTimeout(259-264)embed(113-113)getModels(72-72)getModel(79-79)getEmbeddingDimension(118-118)setModel(87-87)send(65-65)isSchemaSupported(94-94)getName(54-54)formatErrorMessage(126-126)getSupportForEmbeddings(101-101)src/Agents/Agent.php (1)
embed(152-159)
🪛 PHPMD (2.15.0)
src/Agents/Adapters/Ollama.php
143-143: Avoid unused parameters such as '$messages'. (undefined)
(UnusedFormalParameter)
143-143: Avoid unused parameters such as '$listener'. (undefined)
(UnusedFormalParameter)
🔇 Additional comments (3)
src/Agents/Adapters/Ollama.php (3)
11-30: LGTM! Clean constant and property declarations.The model constants, dimensions mapping, and properties are well-structured and clearly documented.
38-44: LGTM! Constructor properly initialized.The constructor correctly uses the parent's
setTimeout()method and initializes the model.
143-146: LGTM! Correctly indicates unsupported operation.The method appropriately throws an exception since Ollama adapter is embedding-only. The PHPMD warnings about unused parameters are false positives—these parameters are required by the abstract parent method signature.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/Agents/Adapters/Ollama.php (2)
58-92: Fix critical validation issue to resolve PHPStan error.The code accesses
$json['embeddings'][0]without validating that:
$jsonis not null (json_decode could fail)- The 'embeddings' key exists
- The 'embeddings' array has at least one element
This causes the PHPStan pipeline failure and could result in runtime errors with malformed responses.
Apply this diff to add proper validation:
$json = is_string($body) ? json_decode($body, true) : null; if (isset($json['error'])) { throw new \Exception($json['error'], $response->getStatusCode()); } + + if (!is_array($json) || !isset($json['embeddings'][0])) { + throw new \Exception('Invalid response from Ollama embed API'); + } return [ 'embedding' => $json['embeddings'][0], 'total_duration' => $json['total_duration'] ?? null, 'load_duration' => $json['load_duration'] ?? null, ];
174-181: Fix Pint formatting error to unblock pipeline.Line 176 has a space after the unary
!operator, which violates thenot_operator_with_successor_spacerule and causes the Pint pipeline failure.Apply this diff:
- if (!is_array($json)) { + if (!is_array($json)) { return '(unknown_error) Unknown error'; }
🧹 Nitpick comments (1)
src/Agents/Adapters/Ollama.php (1)
117-120: Add validation to prevent undefined array key errors.The method directly indexes
self::DIMENSIONS[$this->model]without checking if the model exists in the map. While currently only one model is defined, this creates a maintainability risk if new models are added or if an invalid model is set.Apply this diff:
public function getEmbeddingDimension(): int { + if (!isset(self::DIMENSIONS[$this->model])) { + throw new \Exception("Embedding dimension not defined for model: {$this->model}"); + } return self::DIMENSIONS[$this->model]; }Based on past review comments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Agents/Adapters/Ollama.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/Agents/Adapters/Ollama.php (2)
src/Agents/Adapter.php (12)
Adapter(5-275)setTimeout(259-264)embed(113-113)getModels(72-72)getModel(79-79)getEmbeddingDimension(118-118)setModel(87-87)send(65-65)isSchemaSupported(94-94)getName(54-54)formatErrorMessage(126-126)getSupportForEmbeddings(101-101)src/Agents/Agent.php (2)
__construct(34-41)embed(152-159)
🪛 GitHub Actions: CodeQL
src/Agents/Adapters/Ollama.php
[error] 88-88: PHPStan analysis error: Offset 'embeddings' does not exist on array {error?: string, embeddings?: array<int, array<int, float>>, total_duration?: int, load_duration?: int} | null. (src/Agents/Adapters/Ollama.php:88)
🪛 GitHub Actions: Linter
src/Agents/Adapters/Ollama.php
[error] 1-1: Pint lint error: not_operator_with_successor_space. [lint rule not_operator_with_successor_space]
🪛 PHPMD (2.15.0)
src/Agents/Adapters/Ollama.php
143-143: Avoid unused parameters such as '$messages'. (undefined)
(UnusedFormalParameter)
143-143: Avoid unused parameters such as '$listener'. (undefined)
(UnusedFormalParameter)
⏰ 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). (1)
- GitHub Check: Tests
🔇 Additional comments (4)
src/Agents/Adapters/Ollama.php (4)
11-30: LGTM! Constants and properties are well-defined.The model constants, properties, and dimension mappings are appropriately structured. The DIMENSIONS constant correctly maps each model to its fixed embedding dimension as specified by the model architecture.
38-44: LGTM! Constructor properly initialized.The constructor correctly uses the parent's
setTimeout()method and appropriately initializes the model property.
99-133: LGTM! Model management methods are correctly implemented.The getter and setter methods for model management are straightforward and properly support method chaining.
153-209: LGTM! Interface and utility methods are correctly implemented.The remaining methods (
isSchemaSupported(),getName(),getEndpoint(),setEndpoint(), andgetSupportForEmbeddings()) are all properly implemented and return appropriate values for an embedding-only adapter.
ArnabChatterjee20k
force-pushed
the
feat-embeddings
branch
from
a70a449 to
3d2c158
Compare
ArnabChatterjee20k
force-pushed
the
feat-embeddings
branch
2 times, most recently
from
2451edf to
688a636
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/Agents/Adapters/Ollama.php (2)
113-116: Add validation for model dimension lookup (previously flagged).Direct array access without validation can cause undefined array key errors if
$this->modelis not in DIMENSIONS. While this can be prevented by constructor validation (see my earlier comment), defensive programming requires checking here as well sincesetModel()doesn't validate.Apply this diff:
public function getEmbeddingDimension(): int { + if (! isset(self::DIMENSIONS[$this->model])) { + throw new \Exception("Embedding dimension not defined for model: {$this->model}"); + } return self::DIMENSIONS[$this->model]; }
58-88: Add HTTP status validation and improve response parsing.Two issues that reduce reliability:
Major: No HTTP status code validation before parsing the response body. If Ollama returns 4xx/5xx errors, the code attempts to parse the error response as if it were successful.
Major: Line 84 silently returns an empty array if
embeddings[0]is missing, which could mask API failures. An empty embedding array would be semantically incorrect and cause downstream issues.Apply this diff to add proper validation:
public function embed(string $text): array { $client = new Client(); $client->setTimeout($this->timeout); $client->addHeader('Content-Type', 'application/json'); $payload = [ 'model' => $this->model, 'input' => $text, ]; $response = $client->fetch( $this->getEndpoint(), Client::METHOD_POST, $payload ); + + if ($response->getStatusCode() >= 400) { + $body = $response->getBody(); + $json = is_string($body) ? json_decode($body, true) : null; + throw new \Exception( + 'Ollama API error: ' . $this->formatErrorMessage($json), + $response->getStatusCode() + ); + } + $body = $response->getBody(); $json = is_string($body) ? json_decode($body, true) : null; if (! is_array($json)) { throw new \Exception('Invalid response format received from the API'); } if (isset($json['error'])) { throw new \Exception($json['error'], $response->getStatusCode()); } + + if (! isset($json['embeddings'][0]) || ! is_array($json['embeddings'][0])) { + throw new \Exception('Invalid or missing embeddings in API response'); + } return [ - 'embedding' => $json['embeddings'][0] ?? [], + 'embedding' => $json['embeddings'][0], 'total_duration' => $json['total_duration'] ?? null, 'load_duration' => $json['load_duration'] ?? null, ]; }
🧹 Nitpick comments (1)
src/Agents/Adapters/Ollama.php (1)
202-205: Add PHPDoc for consistency.While the implementation is correct, adding PHPDoc would improve consistency with other methods in the class.
Apply this diff:
+ /** + * Check if the adapter supports embeddings + * + * @return bool + */ public function getSupportForEmbeddings(): bool { return true; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Agents/Adapters/Ollama.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/Agents/Adapters/Ollama.php (3)
src/Agents/Adapter.php (12)
Adapter(5-275)setTimeout(259-264)embed(113-113)getModels(72-72)getModel(79-79)getEmbeddingDimension(118-118)setModel(87-87)send(65-65)isSchemaSupported(94-94)getName(54-54)formatErrorMessage(126-126)getSupportForEmbeddings(101-101)src/Agents/Message.php (1)
Message(5-48)src/Agents/Agent.php (2)
__construct(34-41)embed(152-159)
🪛 PHPMD (2.15.0)
src/Agents/Adapters/Ollama.php
139-139: Avoid unused parameters such as '$messages'. (undefined)
(UnusedFormalParameter)
139-139: Avoid unused parameters such as '$listener'. (undefined)
(UnusedFormalParameter)
⏰ 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). (1)
- GitHub Check: Tests
🔇 Additional comments (9)
src/Agents/Adapters/Ollama.php (9)
1-8: LGTM!Namespace and imports are correctly defined and all are used within the class.
9-30: LGTM!Class structure and constants are well-defined. The fixed DIMENSIONS mapping appropriately reflects the immutable nature of model embedding dimensions.
90-98: LGTM!Simple getter method is correctly implemented.
100-108: LGTM!Model getter is correctly implemented.
139-142: LGTM!The method correctly throws an exception for embedding-only adapters. The unused parameter warnings from PHPMD are false positives since the signature must match the abstract parent class.
149-152: LGTM!Correctly returns false for embedding adapters.
159-162: LGTM!Adapter name is correctly returned.
170-177: LGTM!Error message formatting is correctly implemented with proper fallbacks.
184-200: LGTM!Endpoint getter and setter are correctly implemented with proper method chaining support.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/Agents/Adapters/Ollama.php (1)
62-92: Fix unsafe array access and add HTTP status validation.Two issues need attention:
Critical: Line 88 accesses
$json['embeddings'][0]without validating thatembeddingsexists and is an array. The fallback to an empty array masks API failures rather than surfacing them.Major: Missing HTTP status code validation before parsing the response body (discussed in previous reviews but not fully implemented).
Apply this diff to fix both issues:
public function embed(string $text): array { $client = new Client(); $client->setTimeout($this->timeout); $client->addHeader('Content-Type', 'application/json'); $payload = [ 'model' => $this->model, 'input' => $text, ]; $response = $client->fetch( $this->getEndpoint(), Client::METHOD_POST, $payload ); + + // Check HTTP status code first + if ($response->getStatusCode() >= 400) { + $body = $response->getBody(); + $json = is_string($body) ? json_decode($body, true) : null; + throw new \Exception( + 'Ollama API error: ' . $this->formatErrorMessage($json), + $response->getStatusCode() + ); + } + $body = $response->getBody(); $json = is_string($body) ? json_decode($body, true) : null; if (! is_array($json)) { throw new \Exception('Invalid response format received from the API'); } - if (isset($json['error'])) { - throw new \Exception($json['error'], $response->getStatusCode()); - } + // Validate embeddings structure + if (! isset($json['embeddings']) || ! is_array($json['embeddings']) || ! isset($json['embeddings'][0])) { + throw new \Exception('Invalid response from Ollama embed API: missing embeddings'); + } return [ - 'embedding' => $json['embeddings'][0] ?? [], + 'embedding' => $json['embeddings'][0], 'total_duration' => $json['total_duration'] ?? null, 'load_duration' => $json['load_duration'] ?? null, ]; }
🧹 Nitpick comments (1)
src/Agents/Adapters/Ollama.php (1)
117-120: Consider adding defensive validation for dimension lookup.While the constructor validates models against
MODELS, adding a check here prevents potential issues ifDIMENSIONSandMODELSdrift out of sync during maintenance.Apply this diff:
public function getEmbeddingDimension(): int { + if (! isset(self::DIMENSIONS[$this->model])) { + throw new \Exception("Embedding dimension not defined for model: {$this->model}"); + } return self::DIMENSIONS[$this->model]; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Agents/Adapters/Ollama.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/Agents/Adapters/Ollama.php (2)
src/Agents/Adapter.php (12)
Adapter(5-275)setTimeout(259-264)embed(113-113)getModels(72-72)getModel(79-79)getEmbeddingDimension(118-118)setModel(87-87)send(65-65)isSchemaSupported(94-94)getName(54-54)formatErrorMessage(126-126)getSupportForEmbeddings(101-101)src/Agents/Agent.php (2)
__construct(34-41)embed(152-159)
🪛 PHPMD (2.15.0)
src/Agents/Adapters/Ollama.php
146-146: Avoid unused parameters such as '$messages'. (undefined)
(UnusedFormalParameter)
146-146: Avoid unused parameters such as '$listener'. (undefined)
(UnusedFormalParameter)
⏰ 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). (1)
- GitHub Check: Tests
🔇 Additional comments (12)
src/Agents/Adapters/Ollama.php (12)
1-30: LGTM! Clean structure and constants.The class structure, imports, and constants are well-defined. The DIMENSIONS mapping and MODELS array provide a clear contract for supported models.
38-48: LGTM! Constructor properly validates model.The model validation added per previous feedback ensures only supported models are accepted. The error message is clear and helpful.
99-102: LGTM!Simple and correct implementation.
109-112: LGTM!Simple and correct implementation.
128-136: LGTM! Model validation is properly implemented.The validation ensures only supported models are set, with a clear error message.
146-149: LGTM! Correct implementation for embedding-only adapter.The method properly throws an exception since Ollama only supports embeddings. The PHPMD warnings about unused parameters are false positives—the parameters are required by the abstract
Adaptersignature.
156-159: LGTM!Correct return value for embedding-only adapter.
166-169: LGTM!Simple and correct implementation.
177-184: LGTM! Minimal but sufficient error formatting.The method provides basic error message extraction as required by the abstract
Adapterclass.
191-194: LGTM!Simple and correct implementation.
202-207: LGTM!Simple and correct implementation with method chaining.
209-212: LGTM!Correctly indicates embedding support.
Summary by CodeRabbit