-
Notifications
You must be signed in to change notification settings - Fork 2.3k
fix: Add backward compatibility for conversationCompacted message type #5819
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
fix: Add backward compatibility for conversationCompacted message type #5819
Conversation
08d5e34 to
4036477
Compare
katzdave
left a comment
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.
Nice thanks for the quick fix!
|
|
||
| let mut raw: Vec<serde_json::Value> = Vec::deserialize(deserializer)?; | ||
|
|
||
| // Convert old "conversationCompacted" to "systemNotification" for backward compatibility |
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.
Can we add to this comment pre 14.0
| for item in &mut raw { | ||
| if let Some(obj) = item.as_object_mut() { | ||
| if obj.get("type").and_then(|v| v.as_str()) == Some("conversationCompacted") { | ||
| obj.insert("type".to_string(), serde_json::json!("systemNotification")); |
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.
we can just get rid of inserting the new message. Let's just not write the old conversationCompacted.
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.
modified it to cleaner code,
raw.retain(|item|
item.get("type").and_then(|v| v.as_str()) != Some("conversationCompacted")
);
| } | ||
|
|
||
| #[test] | ||
| fn test_backward_compatibility_conversation_compacted() { |
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.
No need for test if we simplify the logic.
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.
removed the test.
8e7a516 to
3bfffde
Compare
| let mut content: Vec<MessageContent> = serde_json::from_value(serde_json::Value::Array(raw)) | ||
| .map_err(|e| Error::custom(format!("Failed to deserialize MessageContent: {}", e)))?; | ||
|
|
||
| // Sanitize text content |
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.
we can drop little comments like this (the obe above is ok tho)
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.
I will be more careful with AI slop comments from now on.
|
nice one @vaibhavgeek |
Remove conversationCompacted messages instead of converting them to maintain compatibility with pre 14.0 versions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> Signed-off-by: Vaibhav Maheshwari <[email protected]>
Signed-off-by: Vaibhav Maheshwari <[email protected]>
f95e8c5 to
550156a
Compare
* main: (48 commits) [fix] generic check for gemini compat (#5842) Add scheduler to diagnostics (#5849) Cors and token (#5850) fix sessions coming back with empty messages (#5841) markdown export from URL (#5830) Next camp refactor live (#5706) Add out of context compaction test via error proxy (#5805) fix: Add backward compatibility for conversationCompacted message type (#5819) Add /agent/stop endpoint, make max active agents configurable (#5826) Handle 404s (#5791) Persist provider name and model config in the session (#5419) Comment out the flaky mcp callers (#5827) Slash commands (#5718) fix: remove setx calls to not permanently edit the windows shell PATH (#5821) fix: Parse maas models for gcp vertex provider (#5816) fix: support Gemini 3's thought signatures (#5806) chore: Add Adrian Cole to Maintainers (#5815) [MCP-UI] Proxy and Better Message Handling (#5487) Release 1.15.0 Document New Window menu in macOS dock (#5811) ...
block#5819) Signed-off-by: Vaibhav Maheshwari <[email protected]> Co-authored-by: Claude <[email protected]>
block#5819) Signed-off-by: Vaibhav Maheshwari <[email protected]> Co-authored-by: Claude <[email protected]> Signed-off-by: Sai Karthik <[email protected]>
block#5819) Signed-off-by: Vaibhav Maheshwari <[email protected]> Co-authored-by: Claude <[email protected]> Signed-off-by: Blair Allan <[email protected]>
Automatically converts legacy "conversationCompacted" messages to the new "systemNotification" format with "thinkingMessage" type during deserialization. This ensures older conversation history can be loaded without breaking changes.
Added test coverage for the backward compatibility conversion.
🤖 Generated with Claude Code
Summary
Type of Change
AI Assistance
Testing
Related Issues
Relates to #5804
Discussion: #5804 (comment)
Screenshots/Demos (for UX changes)
Before:
After:
Submitting a Recipe?
Email: