-
Notifications
You must be signed in to change notification settings - Fork 16
feat: implement with_structured_output method with dual mode support #15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: implement with_structured_output method with dual mode support #15
Conversation
- Add with_structured_output method supporting both Pydantic schema and raw JSON modes - Support both HumanMessage and list of messages as input - Add comprehensive error handling with clear error messages - Add response_format parameter support for API compatibility - Include comprehensive test suite with 20+ test cases - Add detailed documentation and examples - Fix type safety issues and improve code quality Features: - Mode 1: With Pydantic schema (returns validated Pydantic object) - Mode 2: Without schema (returns raw JSON dict) - Robust error handling for JSON parsing and validation failures - Support for complex Pydantic models with optional fields - Unicode and special character support - Custom model parameters support
|
@bnarasimha21 Can you review my PR and let me know if you have any feedback? Please can you add Hacktoberfest label to the issue? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements a with_structured_output method for ChatGradient that enables structured, validated output in two modes: with Pydantic schema validation (returning typed objects) or without schema (returning raw JSON dictionaries). The implementation includes comprehensive error handling, support for multiple input formats (single HumanMessage or list of messages), and extensive test coverage.
Key Changes:
- Dual-mode structured output support with automatic validation
- Robust error handling with detailed context in error messages
- Flexible input processing for different message formats
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| langchain_gradient/chat_models.py | Adds with_structured_output, process_human_message, and parser factory methods with full validation logic |
| tests/integration_tests/test_with_structured_output.py | Comprehensive test suite covering both modes, error cases, edge cases, and end-to-end workflows |
| WITH_STRUCTURED_OUTPUT_DOCUMENTATION.md | Complete documentation with usage examples, error handling patterns, and best practices |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from langchain_core.callbacks import ( | ||
| CallbackManagerForLLMRun, | ||
| ) | ||
| from langchain_core.runnables import RunnablePassthrough,Runnable |
Copilot
AI
Oct 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing space after comma in import statement. Should be RunnablePassthrough, Runnable for consistency with Python style guidelines.
| from langchain_core.runnables import RunnablePassthrough,Runnable | |
| from langchain_core.runnables import RunnablePassthrough, Runnable |
| if hasattr(first_message, 'content'): | ||
| return {"input": first_message.content} | ||
| else: | ||
| raise ValueError("First message in list must have 'content' attribute") | ||
| elif hasattr(input_data, 'content'): | ||
| return {"input": input_data.content} | ||
| else: | ||
| raise ValueError( | ||
| "Input must be a HumanMessage or list of messages. " |
Copilot
AI
Oct 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The validation logic doesn't verify that list items are actually HumanMessage instances. Line 483 only checks for the 'content' attribute via hasattr, which could pass for any object with a 'content' attribute. Consider adding isinstance(first_message, HumanMessage) check before accessing content.
| if hasattr(first_message, 'content'): | |
| return {"input": first_message.content} | |
| else: | |
| raise ValueError("First message in list must have 'content' attribute") | |
| elif hasattr(input_data, 'content'): | |
| return {"input": input_data.content} | |
| else: | |
| raise ValueError( | |
| "Input must be a HumanMessage or list of messages. " | |
| if isinstance(first_message, HumanMessage): | |
| return {"input": first_message.content} | |
| else: | |
| raise ValueError("First message in list must be a HumanMessage instance") | |
| elif isinstance(input_data, HumanMessage): | |
| return {"input": input_data.content} | |
| else: | |
| raise ValueError( | |
| "Input must be a HumanMessage or list of HumanMessage instances. " |
| if hasattr(first_message, 'content'): | ||
| return {"input": first_message.content} | ||
| else: | ||
| raise ValueError("First message in list must have 'content' attribute") | ||
| elif hasattr(input_data, 'content'): | ||
| return {"input": input_data.content} | ||
| else: | ||
| raise ValueError( | ||
| "Input must be a HumanMessage or list of messages. " |
Copilot
AI
Oct 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using hasattr check instead of isinstance validation allows any object with a 'content' attribute to pass validation, which doesn't match the type hint Union[HumanMessage, List[HumanMessage]]. Should verify isinstance(input_data, HumanMessage) for type safety.
| if hasattr(first_message, 'content'): | |
| return {"input": first_message.content} | |
| else: | |
| raise ValueError("First message in list must have 'content' attribute") | |
| elif hasattr(input_data, 'content'): | |
| return {"input": input_data.content} | |
| else: | |
| raise ValueError( | |
| "Input must be a HumanMessage or list of messages. " | |
| if isinstance(first_message, HumanMessage): | |
| return {"input": first_message.content} | |
| else: | |
| raise ValueError("First message in list must be a HumanMessage instance") | |
| elif isinstance(input_data, HumanMessage): | |
| return {"input": input_data.content} | |
| else: | |
| raise ValueError( | |
| "Input must be a HumanMessage or list of HumanMessage instances. " |
| assert isinstance(json_result, dict) | ||
|
|
||
| # Verify data consistency (rough check) | ||
| assert person_result.name.lower() in json_result.get("name", "").lower() |
Copilot
AI
Oct 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This assertion will fail if json_result doesn't contain a 'name' key, as get() returns empty string and the check becomes unreliable. The test should verify the key exists first or handle the case where field names might differ between modes.
| assert person_result.name.lower() in json_result.get("name", "").lower() | |
| # Find name-related fields in json_result | |
| name_fields = [k for k in json_result.keys() if 'name' in k.lower()] | |
| assert len(name_fields) > 0, f"No name-related fields found in {json_result.keys()}" | |
| # Check that person_result.name appears in at least one of the name fields | |
| assert any(person_result.name.lower() in str(json_result[k]).lower() for k in name_fields), ( | |
| f"{person_result.name} not found in any name-related field: {[json_result[k] for k in name_fields]}" | |
| ) |
Features: