Skip to content

fix(workflow): use input config variants in MultiRankingWorkflow#686

Merged
LinoGiger merged 1 commit into
mainfrom
fix(workflow)/ranking-pair-maker-config-variant
Jul 18, 2026
Merged

fix(workflow): use input config variants in MultiRankingWorkflow#686
LinoGiger merged 1 commit into
mainfrom
fix(workflow)/ranking-pair-maker-config-variant

Conversation

@RapidPoseidon

Copy link
Copy Markdown
Contributor

Problem

The nightly Run SDK Orders test (failing run) fails on the ranking order step:

pydantic_core.ValidationError: 2 validation errors for IPairMakerConfig
  Input should be a valid instance of IPairMakerConfigFullPermutationPairMakerConfig
  Input should be a valid instance of IPairMakerConfigOnlinePairMakerConfig
  input_type=IPairMakerConfigModelOnlinePairMakerConfigModel

Root cause

The generated client carries two parallel one-of unions:

Union Accepts
IPairMakerConfig (request) IPairMakerConfigFullPermutationPairMakerConfig, IPairMakerConfigOnlinePairMakerConfig
IPairMakerConfigModel (read) ...ModelFullPermutation...Model, ...ModelOnline...Model

IOrderWorkflowInputGroupedRankingWorkflowInput.pairMakerConfig / .rankingConfig expect the non-Model request unions, but MultiRankingWorkflow.__init__ instantiated the ...Model... read-side classes, which the request union rejects at construction. The OpenAPI client itself is up to date — only this hand-written wrapper was stale, and nothing type-checks the union membership, so only the integration run caught it.

Fix

Swap to the matching request-side variants in _multi_ranking_workflow.py:

  • IPairMakerConfigModelFullPermutationPairMakerConfigModelIPairMakerConfigFullPermutationPairMakerConfig
  • IPairMakerConfigModelOnlinePairMakerConfigModelIPairMakerConfigOnlinePairMakerConfig
  • IRankingConfigModelBradleyTerryRankingConfigModelIRankingConfigBradleyTerryRankingConfig

The variants have identical fields, so the serialized payload is byte-for-byte unchanged:

max_group_size=5:  {"_t":"GroupedRankingWorkflow","pairMakerConfig":{"_t":"FullPermutationPairMaker"},"rankingConfig":{"_t":"BradleyTerryRankingConfig","startingScore":1200}}
max_group_size=25: {"_t":"GroupedRankingWorkflow","pairMakerConfig":{"_t":"OnlinePairMaker","randomMatchesRatio":0.5,"totalComparisonBudget":10},"rankingConfig":{"_t":"BradleyTerryRankingConfig","startingScore":1200}}

Verification

  • Reproduced the crash and confirmed both pair-maker paths (full-permutation ≤10, online >10) now construct and serialize.
  • black clean, pyright src/rapidata/rapidata_client → 0 errors.

🔗 Session: https://session-0deec1d3.poseidon.rapidata.internal/

MultiRankingWorkflow built the request-side IPairMakerConfig / IRankingConfig
unions from the read-side ...Model... variant classes, which those unions do
not accept, so create_ranking_order raised a pydantic ValidationError at
construction time.

Swap to the matching non-Model variants (IPairMakerConfigOnlinePairMakerConfig,
IPairMakerConfigFullPermutationPairMakerConfig,
IRankingConfigBradleyTerryRankingConfig). The variants have identical fields,
so the serialized payload is unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: lino <68745352+LinoGiger@users.noreply.github.com>
@claude

claude Bot commented Jul 18, 2026

Copy link
Copy Markdown

Review

Overview

Fixes a nightly integration test failure in MultiRankingWorkflow.__init__ (src/rapidata/rapidata_client/workflow/_multi_ranking_workflow.py): the constructor was populating the request-side IPairMakerConfig / IRankingConfig one-of unions with the read-side (...Model...) variant classes, which those unions don't accept, causing a pydantic_core.ValidationError at construction time.

Correctness

  • Verified against the generated client (src/rapidata/api_client/models/i_pair_maker_config.py, i_ranking_config.py): both one-of unions' actual_instance type only accepts the non-Model variants (IPairMakerConfigFullPermutationPairMakerConfig, IPairMakerConfigOnlinePairMakerConfig, IRankingConfigBradleyTerryRankingConfig). The swap in this PR aligns the wrapper with those unions — the fix is correct.
  • Confirmed the replacement classes exist under src/rapidata/api_client/models/ and expose the same fields (_t, totalComparisonBudget, randomMatchesRatio, startingScore) as the ones being replaced, so behavior/serialized payload is unchanged as described in the PR body.
  • No other file in rapidata_client/workflow/ (or elsewhere under rapidata_client/) uses a ...Model variant to construct a request-side one-of union, so this looks like an isolated one-off rather than a systemic pattern — good to confirm, no follow-up sweep needed.

Code quality / conventions

  • Minimal, surgical diff — only import paths and constructor calls changed, no unrelated churn. Matches the CLAUDE.md guidance against scope creep.
  • Follows the existing local-import convention inside __init__ used throughout this file.

Test coverage

  • No unit test accompanies the fix, and there's no existing test file for MultiRankingWorkflow in the repo. Given the union-membership mismatch wasn't caught by pyright (both classes structurally satisfy IPairMakerConfig's constructor at the type level via pydantic's flexible validation, apparently), a small regression test — e.g., instantiating MultiRankingWorkflow for both the full-permutation and online branches and asserting no ValidationError — would be cheap insurance against this recurring, and would replace reliance on the nightly integration suite alone for this specific bug class.

Risk

  • Low risk: additive/corrective change to a single constructor, no public API signature change, no behavior change to serialized payloads.

Overall: correct, well-scoped fix with a clear root-cause writeup. Only suggestion is adding a lightweight unit test for MultiRankingWorkflow construction to guard against this regressing again without needing the nightly integration run to catch it.

@LinoGiger
LinoGiger marked this pull request as ready for review July 18, 2026 11:44
@LinoGiger
LinoGiger merged commit 4092dcb into main Jul 18, 2026
3 checks passed
@LinoGiger
LinoGiger deleted the fix(workflow)/ranking-pair-maker-config-variant branch July 18, 2026 11:44
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