Skip to content

update_message_from_xml method#178

Merged
sadrasabouri merged 11 commits intodevfrom
update_message_from_xml
Nov 13, 2025
Merged

update_message_from_xml method#178
sadrasabouri merged 11 commits intodevfrom
update_message_from_xml

Conversation

@sepandhaghighi
Copy link
Copy Markdown
Member

@sepandhaghighi sepandhaghighi commented Nov 12, 2025

Reference Issues/PRs

#142

What does this implement/fix? Explain your changes.

  • README.md updated
  • Response class update_message_from_xml method added
  • Prompt class update_message_from_xml method added

Any other comments?

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Nov 12, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.65%. Comparing base (ac169b7) to head (917cafa).

Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #178      +/-   ##
==========================================
+ Coverage   98.64%   98.65%   +0.02%     
==========================================
  Files          11       11              
  Lines        1754     1776      +22     
  Branches      168      173       +5     
==========================================
+ Hits         1730     1752      +22     
  Misses          5        5              
  Partials       19       19              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sepandhaghighi sepandhaghighi self-assigned this Nov 12, 2025
@sepandhaghighi sepandhaghighi added the enhancement New feature or request label Nov 12, 2025
@sepandhaghighi sepandhaghighi added this to the memor v1.0 milestone Nov 12, 2025
@sepandhaghighi sepandhaghighi marked this pull request as ready for review November 12, 2025 21:14
Copy link
Copy Markdown
Member

@sadrasabouri sadrasabouri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just have two minor questions.

Comment thread memor/message.py
"""
for tag, nodes in structure.items():
for node in nodes:
element = ElementTree.SubElement(parent, tag)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't we use the SafeElementTree here instead, like other places?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the defusedxml library does not include the SubElement and Element classes, we used SafeElementTree for parsing and ElementTree for constructing the XML tree. The XML vulnerability mentioned is only a concern during parsing, not during tree construction.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the defusedxml library does not include the SubElement and Element classes, we used SafeElementTree for parsing and ElementTree for constructing the XML tree. The XML vulnerability mentioned is only a concern during parsing, not during tree construction.

Comment thread memor/message.py

root = ElementTree.Element("root")
_build_xml_element(root, xml_tree)
return "".join(ElementTree.tostring(element, encoding="unicode") for element in root)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and here

Copy link
Copy Markdown
Member

@sadrasabouri sadrasabouri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, sounds good.

@sadrasabouri sadrasabouri merged commit 026fb8f into dev Nov 13, 2025
25 checks passed
@sadrasabouri sadrasabouri deleted the update_message_from_xml branch November 13, 2025 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants