Skip to content

fix(test): pin consolidator fallback test to the config model - #374

Merged
yyiilluu merged 2 commits into
mainfrom
fix/consolidator-fallback-test-model
Jul 21, 2026
Merged

fix(test): pin consolidator fallback test to the config model#374
yyiilluu merged 2 commits into
mainfrom
fix/consolidator-fallback-test-model

Conversation

@yyiilluu

@yyiilluu yyiilluu commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Why

main is red. test_consolidator_calls_litellm_with_fallback_configured has been
failing since #372 landed:

E   ValueError: Structured-output fallback models must use the same transport
    strategy as 'gpt-test' (pydantic_passthrough); incompatible fallbacks: ['gpt-5.4-mini']

Root cause

_build_real_client_consolidator patched the generation site var to a fake
"gpt-test", so the LiteLLMConfig(model=...) each test passed never reached the
request — PlaybookConsolidator reads its request model from the site var
(deduplication_utils.py:191), not from the client config, and passes it as
model=self.model_name (components/consolidator.py:743).

#372 added _validate_structured_fallback_strategies, which resolves a transport
strategy per model and rejects a ladder whose fallbacks disagree with the primary.
Measured strategies:

model provider strategy
gpt-test (unresolvable) pydantic_passthrough
gpt-5.4-mini openai native_json_schema
minimax/MiniMax-M3 minimax native_json_schema

The fake model made the pairing artificially incompatible. The guard itself is
correct — the test was asserting against a model it never intended to use.

Fix

Pin the patched site var to config.model. The fallback-configured case now
exercises a realistic production pairing (MiniMax primary + OpenAI fallback, both
native_json_schema). The second test (claude-code/..., no fallbacks) never
reaches the guard and is unaffected.

Also included: a style: commit applying ruff format to client/client.py,
the only file in the package ruff format --check flagged. No behavior change.

Verification

  • pytest tests/server/services/playbook/test_playbook_consolidator_integration.py — 14 passed (1 failed before)
  • ruff format --check reflexio tests — 821 files already formatted
  • ruff check + pyright clean on both touched files

Summary by CodeRabbit

  • Bug Fixes

    • Improved filtering of outdated operation status updates, helping prevent stale results from being processed.
  • Tests

    • Updated integration coverage to validate structured-output fallback behavior using the configured primary model.

yyiilluu added 2 commits July 21, 2026 15:15
`_build_real_client_consolidator` patched the generation site var to a
fake `"gpt-test"` model, so the `LiteLLMConfig(model=...)` each test
passed never reached the request — `PlaybookConsolidator` resolves its
request model from the site var, not from the client config.

The structured-output fallback guard added in #372 resolves a transport
strategy per model. `"gpt-test"` is unknown to LiteLLM, so it resolved to
`pydantic_passthrough` while the `gpt-5.4-mini` fallback resolved to
`native_json_schema`, and the guard rejected the ladder:

    ValueError: Structured-output fallback models must use the same
    transport strategy as 'gpt-test' (pydantic_passthrough);
    incompatible fallbacks: ['gpt-5.4-mini']

`test_consolidator_calls_litellm_with_fallback_configured` has been
failing on main since #372.

Pin the site var to `config.model` so the test exercises the model it
declares. The fallback-configured case now runs a realistic production
pairing (`minimax/MiniMax-M3` primary + `gpt-5.4-mini` fallback, both
`native_json_schema`) instead of an artificially incompatible one.
`_wait_for_operation`'s skew condition was committed hand-wrapped and is
the only file in the package that `ruff format --check` flags. No
behavior change.
@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 0693db7f-fc5f-41f9-be89-727123704123

📥 Commits

Reviewing files that changed from the base of the PR and between 5227fb1 and 834c213.

📒 Files selected for processing (2)
  • reflexio/client/client.py
  • tests/server/services/playbook/test_playbook_consolidator_integration.py

📝 Walkthrough

Walkthrough

The client’s stale-operation condition is reformatted without behavioral change. The playbook consolidator integration helper now derives its default generation model from the supplied LiteLLMConfig.

Changes

Polling and fallback alignment

Layer / File(s) Summary
Stale-operation filter expression
reflexio/client/client.py
The stale-operation boolean guard is reformatted while preserving its existing condition.
Fallback integration model configuration
tests/server/services/playbook/test_playbook_consolidator_integration.py
The patched site-variable lookup uses config.model as the default generation model and updates the helper documentation.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

Suggested reviewers: yilu331

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: pinning the consolidator fallback test to the configured model.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/consolidator-fallback-test-model

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@yyiilluu
yyiilluu merged commit 5f78279 into main Jul 21, 2026
1 check passed
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