Skip to content

fix: strip unsupported params for reasoning models & handle UnboundLocalError in Generator#492

Open
YardonYan wants to merge 1 commit into
SylphAI-Inc:mainfrom
YardonYan:fix-issue-481
Open

fix: strip unsupported params for reasoning models & handle UnboundLocalError in Generator#492
YardonYan wants to merge 1 commit into
SylphAI-Inc:mainfrom
YardonYan:fix-issue-481

Conversation

@YardonYan

Copy link
Copy Markdown

Fixes #481

Summary

Two bugs fixed for Issue #481 (Quickstart Colab fails with frequency_penalty error on reasoning models):

Bug 1: Reasoning models reject unsupported Chat Completion params

OpenAI Responses API rejects \ requency_penalty, \presence_penalty, \ emperature, and \ op_p\ for reasoning models (o1, o3-mini, etc.), causing \BadRequestError.

Fix: Added parameter filtering in \OpenAIClient.convert_inputs_to_api_kwargs()\ — when \model_type == ModelType.LLM_REASONING, these unsupported params are stripped from \ inal_model_kwargs\ with a warning log.

Bug 2: \UnboundLocalError\ in \Generator._get_default_mapping()\

When both \output.data\ and \output.raw_response\ were \None\ (e.g. API call failure), the method raised \UnboundLocalError\ because \output_mapping\ and \output_fields\ were never assigned.

Fix: Added an \else\ branch returning empty \output_mapping={}\ and \output_fields=[]\ with a warning log.

Root Cause Analysis (Issue #481)

  1. User runs Quickstart Colab with a reasoning model (e.g. o3-mini)
  2. \compose_model_kwargs\ merges default model_kwargs including \ requency_penalty\
  3. \OpenAIClient\ passes \ requency_penalty\ to OpenAI Responses API → \BadRequestError\
  4. API call fails → \output.data\ and \output.raw_response\ are both None
  5. _get_default_mapping()\ has no else branch → \UnboundLocalError\

Tests Added (6 new, all passing)

  • \ est_convert_inputs_to_api_kwargs_reasoning_model_strips_unsupported_params\
  • \ est_convert_inputs_to_api_kwargs_llm_keeps_all_params\
  • \TestGetDefaultMapping\ class with 4 test cases:
    • \ est_data_with_output_fields\ — normal case
    • \ est_raw_response_only\ — fallback to raw_response
    • \ est_both_data_and_raw_response_none\ — the UnboundLocalError fix
    • \ est_data_none_raw_response_empty_string\ — edge case

Files Changed

  • \�dalflow/adalflow/components/model_client/openai_client.py\ — strip unsupported params for LLM_REASONING
  • \�dalflow/adalflow/core/generator.py\ — add else branch in _get_default_mapping
  • \�dalflow/tests/test_openai_client.py\ — 2 new test methods
  • \�dalflow/tests/test_generator.py\ — new TestGetDefaultMapping class (4 tests)

…calError in Generator

Fixes SylphAI-Inc#481

Bug 1: OpenAI Responses API rejects frequency_penalty, presence_penalty,
temperature, and top_p for reasoning models (o1, o3-mini, etc.).
Added parameter filtering in OpenAIClient.convert_inputs_to_api_kwargs()
when model_type is LLM_REASONING.

Bug 2: Generator._get_default_mapping() raised UnboundLocalError when
both output.data and output.raw_response were None (e.g. API call
failure). Added else branch returning empty mapping and fields.

Tests:
- test_convert_inputs_to_api_kwargs_reasoning_model_strips_unsupported_params
- test_convert_inputs_to_api_kwargs_llm_keeps_all_params
- TestGetDefaultMapping (4 test cases)
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.

Quickstart Colab fails with frequency_penalty error on OpenAI Responses API (o3-mini teacher model)

1 participant