Skip to content

Conversation

@Avendor7
Copy link

@Avendor7 Avendor7 commented Jul 1, 2025

Adds $client->models()->load() and $client->models()->unload() functions for loading and unloading models into and out of memory

Code follows existing style and adds tests

@ArdaGnsrn ArdaGnsrn requested a review from Copilot July 26, 2025 05:39
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for loading and unloading models into and out of memory by implementing new load() and unload() methods in the Models resource. The implementation includes proper response classes, contract updates, tests, and documentation.

  • Adds load() and unload() methods to the Models resource
  • Creates dedicated response classes for both operations
  • Updates the ModelsContract interface to include the new methods

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/Resources/Models.php Implements load() and unload() methods using the generate endpoint
src/Contracts/ModelsContract.php Adds method signatures for load() and unload() operations
src/Responses/Models/LoadModelResponse.php Response class for model loading operations
src/Responses/Models/UnloadModelResponse.php Response class for model unloading operations
tests/OllamaTest.php Test cases for both load and unload functionality
README.md Documentation for the new load() and unload() methods
Comments suppressed due to low confidence (1)

src/Responses/Models/UnloadModelResponse.php:23

  • There's an unnecessary trailing comma after the last parameter in the constructor.
    )

Comment on lines +182 to +185
{
$response = $this->ollamaClient->post('generate', [
'model' => $modelName,
]);
Copy link

Copilot AI Jul 26, 2025

Choose a reason for hiding this comment

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

The $verbose parameter is defined but never used in the method implementation. Either remove the unused parameter or implement its functionality.

Suggested change
{
$response = $this->ollamaClient->post('generate', [
'model' => $modelName,
]);
{
if ($verbose) {
error_log("Loading model: " . $modelName);
}
$response = $this->ollamaClient->post('generate', [
'model' => $modelName,
]);
if ($verbose) {
error_log("Model loaded successfully: " . $modelName);
}

Copilot uses AI. Check for mistakes.
*/
public function load(string $modelName, bool $verbose = false): LoadModelResponse
{
$response = $this->ollamaClient->post('generate', [
Copy link

Copilot AI Jul 26, 2025

Choose a reason for hiding this comment

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

Using the 'generate' endpoint for loading models is unconventional. Consider using a dedicated '/api/load' endpoint if available, or document why the generate endpoint is used for this purpose.

Suggested change
$response = $this->ollamaClient->post('generate', [
// Use the dedicated 'load' endpoint if available, otherwise document why 'generate' is used.
$response = $this->ollamaClient->post('load', [

Copilot uses AI. Check for mistakes.
Comment on lines +195 to +197
$response = $this->ollamaClient->post('generate', [
'model' => $modelName,
'keep_alive' => 0,
Copy link

Copilot AI Jul 26, 2025

Choose a reason for hiding this comment

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

Using the 'generate' endpoint for unloading models is unconventional. Consider using a dedicated '/api/unload' endpoint if available, or document why the generate endpoint is used for this purpose.

Suggested change
$response = $this->ollamaClient->post('generate', [
'model' => $modelName,
'keep_alive' => 0,
// Use the dedicated '/api/unload' endpoint for unloading models.
$response = $this->ollamaClient->post('unload', [
'model' => $modelName,

Copilot uses AI. Check for mistakes.
Comment on lines +89 to +91
* @return LoadModelResponse
*/
public function load(string $modelName): LoadModelResponse;
Copy link

Copilot AI Jul 26, 2025

Choose a reason for hiding this comment

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

The contract method signature doesn't include the $verbose parameter that's present in the implementation, creating an inconsistency between the contract and implementation.

Suggested change
* @return LoadModelResponse
*/
public function load(string $modelName): LoadModelResponse;
* @param bool $verbose
* @return LoadModelResponse
*/
public function load(string $modelName, bool $verbose = false): LoadModelResponse;

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant