Skip to content

♻️ rework polling - #278

Merged
ianardee merged 9 commits into
mainfrom
rework-polling
Nov 5, 2026
Merged

ianardee merged 9 commits into
mainfrom
rework-polling

Conversation

@ianardee

@ianardee ianardee commented Nov 5, 2026 •

Copy link
Copy Markdown
Collaborator

Description

  • Passing polling options in model parameters is not documented and should never be used.
  • Fix data_schema and text_context not passed in hash mode.
  • Split up the enqueue_and_get_result method.
  • Misc fixes.

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.

@ianardee ianardee changed the title Rework polling ♻️ rework polling Nov 5, 2026
@ianardee
ianardee force-pushed the rework-polling branch 2 times, most recently from 93a3195 to 6fca774 Compare November 5, 2026 20:48

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Partial polling-option hashes override defaults with nil and break polling after submission, while the RBS signature incorrectly requires a cancellation token, invalidating existing three-argument calls.

2 open issues
What changed in this PR

This PR separates polling configuration from V2 product parameters in the Mindee Ruby SDK, intentionally removing the legacy way of passing polling options through product parameters.

Changes:

  • Moves polling normalization and validation into PollingOptions and extracts the V2 polling loop.
  • Updates parameter classes, type signatures, and tests to use separate polling options.
  • Includes namespace cleanup, editor configuration updates, and a V1 PDF-processing call adjustment.
File Description
spec/​v2/​product/​split/​split_integration.rb Renames the split parameter variable.
spec/​v2/​product/​ocr/​ocr_integration.rb Renames the OCR parameter variable.
spec/​v2/​product/​crop/​crop_integration.rb Renames the crop parameter variable.
spec/​v2/​product/​classification/​classification_integration.rb Renames the classification parameter variable.
spec/​v2/​file_operations/​split_operation_integration.rb Renames extraction parameters.
spec/​v2/​file_operations/​crop_operation_integration.rb Renames extraction parameters.
spec/​v2/​client_v2_spec.rb Passes polling options separately in the cancellation test.
spec/​v2/​client_v2_integration.rb Uses parameter hashes and removes the legacy polling test.
sig/​mindee/​v2/​product/​split/​params/​split_parameters.rbs Removes the polling constructor option.
sig/​mindee/​v2/​product/​ocr/​params/​ocr_parameters.rbs Removes the polling constructor option.
sig/​mindee/​v2/​product/​extraction/​params/​extraction_parameters.rbs Removes the polling constructor option.
sig/​mindee/​v2/​product/​crop/​params/​crop_parameters.rbs Removes the polling constructor option.
sig/​mindee/​v2/​product/​classification/​params/​classification_parameters.rbs Removes the polling constructor option.
sig/​mindee/​v2/​client.rbs Updates client and private polling signatures.
sig/​mindee/​v2/​client_options/​base_product_parameters.rbs Removes polling-related declarations.
sig/​mindee/​input/​polling_options.rbs Declares hash loading and validation.
lib/​mindee/​v2/​product/​split/​params/​split_parameters.rb Removes the polling constructor option.
lib/​mindee/​v2/​product/​ocr/​params/​ocr_parameters.rb Removes the polling constructor option.
lib/​mindee/​v2/​product/​extraction/​params/​extraction_parameters.rb Removes polling options and forwards context/schema hash values.
lib/​mindee/​v2/​product/​crop/​params/​crop_parameters.rb Removes the polling constructor option.
lib/​mindee/​v2/​product/​classification/​params/​classification_parameters.rb Removes the polling constructor option.
lib/​mindee/​v2/​client.rb Separates polling options and extracts the polling loop.
lib/​mindee/​v2/​client_options/​base_product_parameters.rb Removes polling storage, normalization, and validation.
lib/​mindee/​v1/​client.rb Calls apply_page_options and reformats method signatures.
lib/​mindee/​input/​polling_options.rb Adds hash conversion and constructor validation.
lib/​mindee.rb Reorganizes namespace declarations.
.editorconfig Consolidates whitespace rules and extends Markdown matching.

Review effort: Balanced

Comment thread lib/mindee/input/polling_options.rb Resolved
Comment thread sig/mindee/v2/client.rbs Resolved

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The signature incorrectly requires polling_options:, rejecting typed callers that omit the keyword even though the Ruby implementation supports that form with default settings.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Medium severity Make polling_options optional to match the Ruby default

sig/​mindee/​input/​polling_options.rbs:11

This signature requires polling_options:, but the Ruby implementation defaults it to {}, so PollingOptions.from_hash without arguments is valid and returns the default settings. Typed callers using that supported form will be rejected. Mark the keyword optional to match the implementation.

2 resolved since last review

Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔵 Needs a closer look

Human assessment is needed because existing tests do not verify how custom polling settings affect delays and request counts, leaving the rework’s key behavior insufficiently validated.

No open issues 🎉

Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The new polling validation tests never run in CI because polling_options.rb does not match the unit or integration spec filename patterns.

1 open issue

Review effort: Max

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Critical polling errors can reject jobs completed on the final attempt and create remote jobs before validating options, leaving callers without a job ID.

3 open issues
1 resolved since last review

Review effort: Max

Comment thread lib/mindee/v2/client.rb Resolved
Comment thread lib/mindee/v2/client.rb Resolved
@ianardee
ianardee requested a max review from Copilot November 5, 2026 23:12

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The shipped RBS interface still declares enqueue_form_options, which MindeeApiV2 no longer implements.

1 open issue
3 resolved since last review

Review effort: Max

Comment thread sig/mindee/v2/http/mindee_api_v2.rbs Resolved

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

Regression tests are missing for polling-option normalization and custom settings during polling, and for preserving text_context and data_schema through ExtractionParameters.from_hash.

2 open issues
1 resolved since last review

Review effort: Max

Comment thread lib/mindee/v2/client.rb
Comment on lines +104 to +108
polling_options_instance = case polling_options
when Mindee::Input::PollingOptions
polling_options
when Hash
Mindee::Input::PollingOptions.from_hash(polling_options: polling_options)
Comment on lines +115 to +116
text_context: base_params.fetch(:text_context, nil),
data_schema: base_params.fetch(:data_schema, nil),
@ianardee
ianardee merged commit a06ab8b into main Nov 5, 2026
36 checks passed
@ianardee
ianardee deleted the rework-polling branch November 5, 2026 23:38
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.

2 participants