Parse all the datetimes before sending to webhooks#3657
Conversation
mdmohsin7
commented
Dec 8, 2025
mdmohsin7
commented
mdmohsin7
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request introduces a much-needed improvement by centralizing datetime serialization to ISO format. The new recursive approach in as_dict_cleaned_dates is robust and handles nested structures effectively. However, there are significant code duplication issues and a redundant serialization call that need to be addressed to maintain code quality and efficiency.
| def convert_datetime_to_iso(obj): | ||
| """Recursively convert datetime objects to ISO format strings""" | ||
| if isinstance(obj, datetime): | ||
| return obj.isoformat() | ||
| elif isinstance(obj, dict): | ||
| return {key: convert_datetime_to_iso(value) for key, value in obj.items()} | ||
| elif isinstance(obj, list): | ||
| return [convert_datetime_to_iso(item) for item in obj] | ||
| else: | ||
| return obj |
There was a problem hiding this comment.
The convert_datetime_to_iso function introduced here implements a recursive datetime serialization logic. This exact same logic is duplicated in backend/utils/app_integrations.py and backend/utils/webhooks.py. To improve maintainability and adhere to the DRY principle, please extract this common utility into a shared module (e.g., backend/utils/serialization_utils.py) and import it where needed.
| def _json_serialize_datetime(obj: Any) -> Any: | ||
| """Helper function to recursively convert datetime objects to ISO format strings for JSON serialization""" | ||
| if isinstance(obj, datetime): | ||
| return obj.isoformat() | ||
| elif isinstance(obj, dict): | ||
| return {key: _json_serialize_datetime(value) for key, value in obj.items()} | ||
| elif isinstance(obj, list): | ||
| return [_json_serialize_datetime(item) for item in obj] | ||
| else: | ||
| return obj |
There was a problem hiding this comment.
This _json_serialize_datetime helper function is a duplicate of the convert_datetime_to_iso function in backend/models/conversation.py and the _json_serialize_datetime function in backend/utils/webhooks.py. Please refactor this into a single, shared utility function in a common module to avoid code duplication and improve maintainability.
| def _json_serialize_datetime(obj: Any) -> Any: | ||
| """Helper function to recursively convert datetime objects to ISO format strings for JSON serialization""" | ||
| if isinstance(obj, datetime): | ||
| return obj.isoformat() | ||
| elif isinstance(obj, dict): | ||
| return {key: _json_serialize_datetime(value) for key, value in obj.items()} | ||
| elif isinstance(obj, list): | ||
| return [_json_serialize_datetime(item) for item in obj] | ||
| else: | ||
| return obj |
There was a problem hiding this comment.
This _json_serialize_datetime helper function is a duplicate of the convert_datetime_to_iso function in backend/models/conversation.py and the _json_serialize_datetime function in backend/utils/app_integrations.py. Please refactor this into a single, shared utility function in a common module to avoid code duplication and improve maintainability.
| webhook_url += f'?uid={uid}' | ||
| try: | ||
| payload = memory.as_dict_cleaned_dates() | ||
| payload = _json_serialize_datetime(payload) |
There was a problem hiding this comment.
The memory.as_dict_cleaned_dates() method already recursively converts all datetime objects within the conversation dictionary to ISO format strings. Therefore, calling _json_serialize_datetime(payload) on its output is redundant and adds unnecessary processing. You can remove this line.
| payload = _json_serialize_datetime(payload) | |
| payload = memory.as_dict_cleaned_dates() |
<img width="1195" height="402" alt="image" src="https://github.com/user-attachments/assets/ac24971c-d826-4d39-b395-73e53da212b3" />
1 participant
