Skip to content

♻️ harmonize V2 CLI with other SDKs, add tests#432

Merged
sebastianMindee merged 4 commits into
mainfrom
update-v2-cli
Jun 18, 2026
Merged

♻️ harmonize V2 CLI with other SDKs, add tests#432
sebastianMindee merged 4 commits into
mainfrom
update-v2-cli

Conversation

@sebastianMindee

Copy link
Copy Markdown
Collaborator

Description

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Requires a change to the official Guide documentation.

@sebastianMindee sebastianMindee requested a review from ianardee June 18, 2026 12:50
Comment thread .github/workflows/_test-cli.yml Fixed
Comment thread .github/workflows/cron.yml Fixed
Comment thread mindee/v2/commands/inference_command.py Outdated
polygon: bool = False
"""Whether to expose ``--polygon/-p``."""
text_context: bool = False
"""Whether to expose ``--text-context/-t``."""

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same as PHP: this is not a good idea, most of the options are not shared.

Comment thread mindee/v2/commands/inference_command.py Outdated
Comment on lines +61 to +65
PRODUCTS: dict[str, _ProductTypes] = {
"classification": _ProductTypes(
response_class=ClassificationResponse,
params_class=ClassificationParameters,
),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Makes more sense to define the options (rag, raw_text, etc) at the product level, since the options are integral part of the product.

Perhaps there is a way to get them from the actual XxxParameters class?

Comment thread tests/v2/test_cli.py

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this is OK, but better to test the CLI directly rather than with synthetic testing.

@sebastianMindee sebastianMindee requested a review from ianardee June 18, 2026 15:07
@sebastianMindee sebastianMindee merged commit f281a09 into main Jun 18, 2026
42 checks passed
@sebastianMindee sebastianMindee deleted the update-v2-cli branch June 18, 2026 19:54
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