fix(llm): accept list payloads for single-list schemas - #363
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughStructured-output parsing now wraps top-level JSON arrays for Pydantic models containing exactly one list field. Primary, sanitized, and repaired parsing paths share this validation behavior, with tests covering accepted single-list and rejected multi-field schemas. ChangesStructured output normalization
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant LLMParser
participant _validate_structured_payload
participant PydanticResponseModel
LLMParser->>_validate_structured_payload: pass parsed JSON or repaired payload
_validate_structured_payload->>_validate_structured_payload: detect single-list schema and wrap top-level array
_validate_structured_payload->>PydanticResponseModel: call model_validate
PydanticResponseModel-->>LLMParser: return validated response
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@reflexio/server/llm/_litellm_structured_output.py`:
- Line 51: Update the list-detection condition in the structured-output schema
handling to recognize both a raw unsubscripted list annotation and parameterized
lists via get_origin(field.annotation). Preserve the existing wrapping behavior
for list[str]-style annotations while ensuring fields annotated simply as list
are also treated as top-level arrays.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b7c6c9fc-9d37-4530-906d-728637f5325c
📒 Files selected for processing (2)
reflexio/server/llm/_litellm_structured_output.pytests/server/llm/test_litellm_client_unit.py
2fd0bdc to
541db60
Compare
|
Fixed in |
Summary
Finding
test-backend-pipelinesurfaced a playbook extraction parse failure from MiniMax where the model returned a semantically valid list of playbook entries while the schema expected the canonical object wrapper ({"playbooks": [...]}). Layer: codebase under test. Root cause: the structured-output parser was too brittle for single-list wrapper schemas, causing one extraction batch to fail and leaving too few user playbooks for aggregation.Fix
Normalize only the safe case: when the target Pydantic response model has exactly one
list[...]field and the parsed JSON root is a list, validate{"<field>": parsed_list}. All other schemas keep the existing strict validation behavior.Verification
uv run --no-sync ruff check open_source/reflexio/reflexio/server/llm/_litellm_structured_output.py open_source/reflexio/tests/server/llm/test_litellm_client_unit.pyuv run --no-sync pytest open_source/reflexio/tests/server/llm/test_litellm_client_unit.py -q -o 'addopts='(220 passed)Summary by CodeRabbit