fix(agents): round-trip agent message history through pydantic-ai type adapter (#166)#167
Merged
Merged
Conversation
…e adapter (#166) Multi-turn agent chat crashed with a dict-has-no-attribute-conversation_id error: _deserialize_messages returned the raw stored dicts unchanged, but PydanticAI 1.96 requires real ModelMessage objects (it accesses msg.conversation_id on every history item). - _serialize_messages now uses ModelMessagesTypeAdapter.dump_python (mode=json), so stored history can be round-tripped. - _deserialize_messages uses ModelMessagesTypeAdapter.validate_python, degrading to an empty history (with a warning) when stored data predates this format instead of crashing the run. - Replace the serialization tests with a real round-trip test and a legacy-format fallback test.
Reviewer's GuideThis PR fixes agent message history persistence by switching to PydanticAI’s own ModelMessagesTypeAdapter for serialization/deserialization so stored histories can be round-tripped back into real ModelMessage instances without crashes on conversation_id access, while gracefully degrading legacy stored data to an empty history. Sequence diagram for agent message history serialization and deserializationsequenceDiagram
participant AgentService
participant ModelMessagesTypeAdapter
participant Storage
participant Logger
AgentService->>ModelMessagesTypeAdapter: dump_python(messages, mode="json")
ModelMessagesTypeAdapter-->>AgentService: list[dict]
AgentService->>Storage: save(serialized_messages)
Storage->>AgentService: load(serialized_messages)
AgentService->>ModelMessagesTypeAdapter: validate_python(serialized_messages)
alt validation_success
ModelMessagesTypeAdapter-->>AgentService: list[ModelMessage]
AgentService-->>AgentService: use message_history
else ValidationError
ModelMessagesTypeAdapter-->>AgentService: ValidationError
AgentService->>Logger: warning(agents.message_history_deserialize_failed)
AgentService-->>AgentService: message_history = []
end
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
_deserialize_messages, you only catchValidationError— ifModelMessagesTypeAdapter.validate_pythonever starts raising other exception types (e.g.,TypeErroron malformed input), this will still crash; consider broadening or wrapping the exception handling to ensure all deserialization failures degrade to an empty history as intended. - The warning in
_deserialize_messageslogs only the error string andmessage_count; if available in this context, including identifiers like agent ID or task/run ID would make it much easier to correlate deserialization failures with specific stored records.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `_deserialize_messages`, you only catch `ValidationError` — if `ModelMessagesTypeAdapter.validate_python` ever starts raising other exception types (e.g., `TypeError` on malformed input), this will still crash; consider broadening or wrapping the exception handling to ensure all deserialization failures degrade to an empty history as intended.
- The warning in `_deserialize_messages` logs only the error string and `message_count`; if available in this context, including identifiers like agent ID or task/run ID would make it much easier to correlate deserialization failures with specific stored records.
## Individual Comments
### Comment 1
<location path="app/features/agents/service.py" line_range="788-792" />
<code_context>
+ return []
+ try:
+ return ModelMessagesTypeAdapter.validate_python(data)
+ except ValidationError as e:
+ logger.warning(
+ "agents.message_history_deserialize_failed",
+ error=str(e),
+ message_count=len(data),
+ )
+ return []
</code_context>
<issue_to_address>
**suggestion:** Improve logging by including structured exception details instead of only `str(e)`.
`str(e)` drops the traceback and most of the useful context from `ValidationError`. Prefer logging the full exception, e.g. with `exc_info=True`:
```python
except ValidationError:
logger.warning(
"agents.message_history_deserialize_failed",
message_count=len(data),
exc_info=True,
)
return []
```
If `exc_info` isn’t available, log the exception type and `errors()` output instead of only `str(e)`.
Suggested implementation:
```python
except ValidationError:
logger.warning(
"agents.message_history_deserialize_failed",
message_count=len(data),
exc_info=True,
)
return []
```
In the shown snippet, there appears to be a duplicated `try:` and `return ModelMessagesTypeAdapter.validate_python(data)` immediately before the `except ValidationError` block. You may want to clean that up by keeping a single `try/except` block around `ModelMessagesTypeAdapter.validate_python(data)` to avoid unreachable or duplicated code.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…166) Address code-review feedback on the message-history round-trip fix: - _deserialize_messages now catches any Exception, not only ValidationError, so a malformed stored record (wrong shape, type errors) can never crash an otherwise-valid agent run. - The warning logs exc_info (full type, message, traceback) instead of just str(e), and includes session_id so a failure can be correlated with the specific stored record. - Add a regression test that a non-ValidationError adapter failure also degrades to an empty history.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #166.
Running a real task in the agent chat crashed the stream:
Root cause
AgentService._deserialize_messagesreturned the raw stored dicts unchanged (return data # type: ignore), assuming PydanticAI'srun()accepts dict message history. That is false for PydanticAI 1.96 —ModelRequest/ModelResponsenow carry aconversation_idfield and the framework accessesmsg.conversation_idon every history item. A plain dict has no such attribute → crash on any turn with non-empty history._serialize_messageswas the matching half: it hand-rolleddataclasses.asdict()plus astr()fallback, producing lossy dicts that could not round-trip.Changes
_serialize_messages→ModelMessagesTypeAdapter.dump_python(messages, mode="json")— JSON-safe and round-trippable._deserialize_messages→ModelMessagesTypeAdapter.validate_python(data), degrading to an empty history (with aagents.message_history_deserialize_failedwarning) when stored data predates this format, rather than crashing.Tests
test_serialize_deserialize_roundtrip— messages round-trip back into realModelMessageobjects; serialized form is JSON-serializable;conversation_idis accessible.test_deserialize_legacy_format_returns_empty— unparseable legacy history degrades to[].Validation
ruff check+ruff format --checkmypy --strict(25 files, no issues)pyright(0 errors)pytest -m "not integration"— 108 agent unit tests passSummary by Sourcery
Ensure agent message histories are serialized and deserialized via PydanticAI's type adapter so they round-trip as real ModelMessage objects and no longer crash when used as message_history.
Bug Fixes:
Enhancements:
Tests: