Skip to content

[PR-734] Use Method signature for local-runner#718

Merged
luv-bansal merged 5 commits intomasterfrom
signatures-for-local-runners
Aug 7, 2025
Merged

[PR-734] Use Method signature for local-runner#718
luv-bansal merged 5 commits intomasterfrom
signatures-for-local-runners

Conversation

@luv-bansal
Copy link
Copy Markdown
Contributor

@luv-bansal luv-bansal commented Jul 24, 2025

Pull Request Overview

This PR introduces a new patch_version method to the Model class and integrates method signatures into the local runner workflow. The changes enable updating existing model versions with method signatures and configuration updates rather than always creating new versions.

  • Adds a patch_version method to the Model class for updating existing model versions
  • Updates the local runner to include method signatures when creating or updating model versions
  • Refactors ModelBuilder instantiation to occur earlier in the local runner flow

@luv-bansal luv-bansal requested a review from Copilot July 24, 2025 11:55
Copy link
Copy Markdown
Contributor

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 introduces a new patch_version method to the Model class and integrates method signatures into the local runner workflow. The changes enable updating existing model versions with method signatures and configuration updates rather than always creating new versions.

  • Adds a patch_version method to the Model class for updating existing model versions
  • Updates the local runner to include method signatures when creating or updating model versions
  • Refactors ModelBuilder instantiation to occur earlier in the local runner flow

Reviewed Changes

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

File Description
clarifai/client/model.py Adds new patch_version method for updating model versions with kwargs
clarifai/cli/model.py Integrates method signatures into model version creation/updating and moves ModelBuilder instantiation

Comment thread clarifai/client/model.py
Comment thread clarifai/client/model.py Outdated
model_version=dict(id=response.model.model_version.id),
)

def patch_version(self, version_id: str, **kwargs) -> None:
Copy link

Copilot AI Jul 24, 2025

Choose a reason for hiding this comment

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

The return type annotation is 'None' but the method actually returns a Model instance (line 2133-2137). The return type should be 'Model' to match the actual behavior.

Suggested change
def patch_version(self, version_id: str, **kwargs) -> None:
def patch_version(self, version_id: str, **kwargs) -> 'Model':

Copilot uses AI. Check for mistakes.
Comment thread clarifai/client/model.py
Comment thread clarifai/cli/model.py Outdated
Comment on lines 554 to 557
builder = ModelBuilder(model_path, download_validation_only=True)

validate_context(ctx)
logger.info("Checking setup for local runner...")
Copy link

Copilot AI Jul 24, 2025

Choose a reason for hiding this comment

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

[nitpick] The ModelBuilder is instantiated early but only used later in the function. Consider moving this instantiation closer to where it's first used (around line 695) to improve code readability and reduce the scope of the variable.

Suggested change
builder = ModelBuilder(model_path, download_validation_only=True)
validate_context(ctx)
logger.info("Checking setup for local runner...")
validate_context(ctx)
logger.info("Checking setup for local runner...")
builder = ModelBuilder(model_path, download_validation_only=True)

Copilot uses AI. Check for mistakes.
@luv-bansal luv-bansal changed the title Method signature for local-runner [PR-734] Use Method signature for local-runner Jul 24, 2025
@luv-bansal luv-bansal marked this pull request as ready for review August 6, 2025 11:47
@luv-bansal luv-bansal requested a review from sainivedh August 6, 2025 11:47
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Aug 6, 2025

Code Coverage

Package Line Rate Health
clarifai 43%
clarifai.cli 43%
clarifai.cli.templates 28%
clarifai.client 67%
clarifai.client.auth 67%
clarifai.constants 100%
clarifai.datasets 100%
clarifai.datasets.export 80%
clarifai.datasets.upload 75%
clarifai.datasets.upload.loaders 37%
clarifai.models 100%
clarifai.modules 0%
clarifai.rag 72%
clarifai.runners 9%
clarifai.runners.models 59%
clarifai.runners.pipeline_steps 45%
clarifai.runners.pipelines 80%
clarifai.runners.utils 62%
clarifai.runners.utils.data_types 72%
clarifai.schema 100%
clarifai.urls 60%
clarifai.utils 52%
clarifai.utils.evaluation 67%
clarifai.workflows 95%
Summary 61% (7494 / 12227)

Minimum allowed line rate is 50%

Copy link
Copy Markdown
Contributor

@sainivedh sainivedh left a comment

Choose a reason for hiding this comment

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

lgtm

@luv-bansal luv-bansal merged commit 90f7aa6 into master Aug 7, 2025
11 checks passed
@luv-bansal luv-bansal deleted the signatures-for-local-runners branch August 7, 2025 06:40
ackizilkale added a commit that referenced this pull request Aug 8, 2025
### Why

Temperature rather than 1.0 has been deprecated for gpt-5 models
luv-bansal pushed a commit that referenced this pull request Sep 15, 2025
### Why

Temperature rather than 1.0 has been deprecated for gpt-5 models
@srikanthbachala20 srikanthbachala20 mentioned this pull request Sep 24, 2025
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.

3 participants