refactor: more ns.model to BaseModel#30445
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
697f4c3 to 1f9dbe3 Compare | /gemini review |
There was a problem hiding this comment.
Code Review
This pull request continues the refactoring effort to replace flask_restx.Namespace.model with Pydantic's BaseModel for API response serialization. The changes are applied across several controller files in console, service_api, and web contexts, consistently replacing @marshal_with decorators and flask_restx field definitions with Pydantic models and TypeAdapter for validation and serialization. This improves type safety and maintainability. A new unit test has been added to verify the new serialization logic, which is great.
My main feedback is to address a small amount of code duplication. Two helper functions, to_timestamp and format_files_contained, have been duplicated in both api/fields/conversation_fields.py and api/fields/message_fields.py. I've left a comment suggesting to move them to a shared utility module to adhere to the DRY principle.
There was a problem hiding this comment.
Pull request overview
This PR refactors Flask-RESTx namespace models to Pydantic BaseModel for message and conversation-related API endpoints. The refactoring improves type safety and validation while maintaining backward compatibility with existing API responses.
Key changes:
- Migrated message and conversation field definitions from Flask-RESTx to Pydantic BaseModel classes
- Updated controllers across web, service_api, and console endpoints to use TypeAdapter for data validation and serialization
- Added comprehensive unit test for message list mapping to verify the refactoring
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| api/tests/unit_tests/controllers/web/test_message_list.py | New unit test validating message list data mapping with mixed dict/object inputs |
| api/fields/message_fields.py | Complete refactor to Pydantic models for message-related responses (MessageListItem, SavedMessageItem, etc.) |
| api/fields/conversation_fields.py | Complete refactor to Pydantic models for conversation-related responses (SimpleConversation, MessageFile, AgentThought, etc.) |
| api/controllers/web/saved_message.py | Updated to use Pydantic models and TypeAdapter for response serialization |
| api/controllers/web/message.py | Updated message list, feedback, and suggested questions endpoints to use Pydantic models |
| api/controllers/web/conversation.py | Updated conversation list, delete, rename, pin/unpin endpoints to use Pydantic models |
| api/controllers/service_api/app/message.py | Updated service API message endpoints to use Pydantic models |
| api/controllers/service_api/app/conversation.py | Updated service API conversation endpoints to use Pydantic models |
| api/controllers/console/explore/saved_message.py | Updated console explore saved message endpoints to use Pydantic models |
| api/controllers/console/explore/message.py | Updated console explore message endpoints to use Pydantic models |
| api/controllers/console/explore/conversation.py | Updated console explore conversation endpoints to use Pydantic models |
| api/controllers/console/app/conversation.py | Moved MessageTextField class locally since it's only used in this file |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1f9dbe3 to cc2246b Compare cc2246b to 1a51473 Compare
Important
Fixes #<issue number>.Summary
part of #28015 , move api/fields/message_fields.py and api/fields/conversation_fields.py , update all the callsites.
Screenshots
Checklist
dev/reformat(backend) andcd web && npx lint-staged(frontend) to appease the lint gods