fix: address PR review feedback from Sourcery AI

- Handle very large integers (beyond i64) by falling back to string
  representation instead of raising OverflowError
- Add compatibility tests for item_wrap=False and list_headers=True
  (marked xfail for known implementation differences)
- Tighten test_numeric_string_key assertion to match actual behavior
- Add test for very large integers beyond i64 range
- Gate benchmark job to only run on push to main/master or manual
  trigger, not on every PR (reduces CI time)
- Add workflow_dispatch trigger for manual benchmark runs

Author: vinitkumar

.github/workflows/rust-ci.yml

@@ -13,6 +13,7 @@ on:
       - 'rust/**'
       - 'tests/test_rust_dicttoxml.py'
       - '.github/workflows/rust-ci.yml'
+  workflow_dispatch:  # Allow manual trigger
 
 env:
   CARGO_TERM_COLOR: always
@@ -87,6 +88,8 @@ jobs:
   benchmark:
     name: Performance Benchmark
     runs-on: ubuntu-latest
+    # Only run benchmarks on push to main/master or manual trigger, not on PRs
+    if: github.event_name == 'push' || github.event_name == 'workflow_dispatch'
     steps:
       - uses: actions/checkout@v4
 

rust/src/lib.rs

@@ -121,10 +121,13 @@ fn convert_value(
         return convert_bool(item_name, val, config);
     }
 
-    // Handle int
+    // Handle int - try i64 first, fall back to string for large integers
     if obj.is_instance_of::<PyInt>() {
-        let val: i64 = obj.extract()?;
-        return convert_number(item_name, &val.to_string(), "int", config);
+        let val_str = match obj.extract::<i64>() {
+            Ok(val) => val.to_string(),
+            Err(_) => obj.str()?.extract::<String>()?, // Fall back for big ints
+        };
+        return convert_number(item_name, &val_str, "int", config);
     }
 
     // Handle float
@@ -277,9 +280,12 @@ fn convert_dict(
             )
             .unwrap();
         }
-        // Handle int
+        // Handle int - try i64 first, fall back to string for large integers
         else if val.is_instance_of::<PyInt>() {
-            let int_val: i64 = val.extract()?;
+            let int_str = match val.extract::<i64>() {
+                Ok(v) => v.to_string(),
+                Err(_) => val.str()?.extract::<String>()?,
+            };
             let mut attrs = Vec::new();
             if let Some((k, v)) = name_attr {
                 attrs.push((k, v));
@@ -291,7 +297,7 @@ fn convert_dict(
             write!(
                 output,
                 "<{}{}>{}</{}>",
-                xml_key, attr_string, int_val, xml_key
+                xml_key, attr_string, int_str, xml_key
             )
             .unwrap();
         }
@@ -456,9 +462,12 @@ fn convert_list(
             )
             .unwrap();
         }
-        // Handle int
+        // Handle int - try i64 first, fall back to string for large integers
         else if item.is_instance_of::<PyInt>() {
-            let int_val: i64 = item.extract()?;
+            let int_str = match item.extract::<i64>() {
+                Ok(v) => v.to_string(),
+                Err(_) => item.str()?.extract::<String>()?,
+            };
             let mut attrs = Vec::new();
             if config.attr_type {
                 attrs.push(("type".to_string(), "int".to_string()));
@@ -467,7 +476,7 @@ fn convert_list(
             write!(
                 output,
                 "<{}{}>{}</{}>",
-                tag_name, attr_string, int_val, tag_name
+                tag_name, attr_string, int_str, tag_name
             )
             .unwrap();
         }

tests/test_rust_dicttoxml.py

@@ -325,6 +325,40 @@ def test_complex_nested_matches(self):
         rust, python = self.compare_outputs(data)
         assert rust == python
 
+    def test_item_wrap_false_matches(self):
+        """Test that item_wrap=False produces matching output."""
+        data = {"colors": ["red", "green", "blue"]}
+        rust, python = self.compare_outputs(data, item_wrap=False)
+        assert rust == python
+
+    @pytest.mark.xfail(reason="Rust list_headers implementation differs from Python - uses different wrapping semantics")
+    def test_list_headers_true_matches(self):
+        """Test that list_headers=True produces matching output."""
+        data = {"items": ["one", "two", "three"]}
+        rust, python = self.compare_outputs(data, list_headers=True)
+        assert rust == python
+
+    @pytest.mark.xfail(reason="Rust item_wrap=False with nested dicts differs from Python - known limitation")
+    def test_item_wrap_false_with_nested_dict_matches(self):
+        """Test item_wrap=False with nested dicts in list."""
+        data = {"users": [{"name": "Alice"}, {"name": "Bob"}]}
+        rust, python = self.compare_outputs(data, item_wrap=False)
+        assert rust == python
+
+    @pytest.mark.xfail(reason="Rust list_headers with nested structures differs from Python - known limitation")
+    def test_list_headers_with_nested_matches(self):
+        """Test list_headers=True with nested structures."""
+        data = {"products": [{"id": 1, "name": "Widget"}, {"id": 2, "name": "Gadget"}]}
+        rust, python = self.compare_outputs(data, list_headers=True)
+        assert rust == python
+
+    def test_very_large_integer_matches(self):
+        """Test that very large integers (beyond i64) produce matching output."""
+        big_int = 10**30  # Way beyond i64 range
+        data = {"huge": big_int}
+        rust, python = self.compare_outputs(data)
+        assert rust == python
+
 
 class TestFastDicttoxmlWrapper:
     """Test the dicttoxml_fast wrapper module."""
@@ -378,11 +412,11 @@ def test_unicode_keys(self):
         assert "México" in result_str
 
     def test_numeric_string_key(self):
-        # Keys that are numbers should be prefixed with 'n'
+        # Keys that are purely numeric should be prefixed with 'n'
         data = {"123": "value"}
         result = rust_dicttoxml(data)
-        # Either the key is modified or wrapped in a name attribute
-        assert b"<n123" in result or b'name="123"' in result
+        # Numeric keys are always prefixed with 'n' per make_valid_xml_name
+        assert b"<n123" in result
 
     def test_key_with_spaces(self):
         data = {"my key": "value"}
@@ -395,6 +429,13 @@ def test_large_integer(self):
         result = rust_dicttoxml(data)
         assert b"9999999999999999" in result
 
+    def test_very_large_integer_beyond_i64(self):
+        """Test integers larger than i64 max (2^63-1) are handled gracefully."""
+        big_int = 10**30  # Way beyond i64 range
+        data = {"huge": big_int}
+        result = rust_dicttoxml(data)
+        assert str(big_int).encode() in result
+
     def test_negative_number(self):
         data = {"negative": -42}
         result = rust_dicttoxml(data)