Skip to content

Update TryGet to combine arrays only when an Append exists #54289

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.

Sign up for GitHub

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

Merged
m-nash merged 2 commits into from
Dec 5, 2025
Merged

Update TryGet to combine arrays only when an Append exists #54289

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
69a9813
Update TryGet to combine arrays only when an Append exists
m-nash Dec 3, 2025
7f45f47
add additional asserts
m-nash Dec 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions sdk/core/System.ClientModel/src/ModelReaderWriter/JsonPatch.Implementation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ private bool TryGetEncodedValueInternal(ReadOnlySpan<byte> jsonPath, out Encoded

if (TryGetParentMatch(jsonPath, true, out var parentPath, out encodedValue))
{
if (parentPath.IsArrayIndex() && encodedValue.Kind == ValueKind.Removed)
{
return false;
}

// normalize jsonPath once to avoid multiple Normalize calls in GetMaxSibling
Span<byte> normalizedJsonPathBuffer = stackalloc byte[jsonPath.Length];
JsonPathComparer.Default.Normalize(jsonPath, normalizedJsonPathBuffer, out var bytesWritten);
Expand All @@ -81,17 +86,6 @@ private bool TryGetEncodedValueInternal(ReadOnlySpan<byte> jsonPath, out Encoded
JsonPathReader reader = new(normalizedJsonPath);
reader.Advance(parentPath);
ReadOnlySpan<byte> normalizedArrayPath = reader.GetNextArray();
if (normalizedArrayPath.IsEmpty)
{
// no array in sub path
GetSubPath(parentPath, normalizedJsonPath, ref childPath);
if (!encodedValue.Value.TryGetJson(childPath, out var childJson))
{
return false;
}
value = new(encodedValue.Kind, childJson);
return true;
}

// see if the requested index exist in root first
// collect and adjust indexes in a new path as I go
Expand All @@ -113,6 +107,12 @@ private bool TryGetEncodedValueInternal(ReadOnlySpan<byte> jsonPath, out Encoded
return true;
}

if (parentPath.IsRoot() && (!_properties.TryGetValue(normalizedArrayPath.GetParent(), out parentValue) || !parentValue.Kind.HasFlag(ValueKind.ArrayItemAppend)))
{
// if ancestor array doesn't exist in properties or is not an append we should stop adjusting index path
break;
}

AdjustJsonPath(
indexRequested - Math.Max(length, GetMaxSibling(normalizedArrayPath) + 1),
reader.Current.ValueSpan.Length,
Expand Down
257 changes: 257 additions & 0 deletions sdk/core/System.ClientModel/tests/ModelReaderWriterTests/JsonPatchTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -559,5 +559,262 @@ public void NonExistentPathShouldNotThrow()

Assert.DoesNotThrow(() => patch.TryGetValue("$.reasoning"u8, out string? value));
}

[Test]
public void IterateOnArray_RemoveItemMatch()
{
var json = """
{
"output": [
{
"id": "rs_01dfc61333fbdf3600692dffc53f4c8196b55eae21c8727d1a",
"type": "reasoning",
"summary": []
},
{
"id": "ci_01dfc61333fbdf3600692dfff0a80881968003a7f1010139ba",
"type": "code_interpreter_call",
"status": "completed",
"code": "import numpy as np\r\nimport matplotlib.pyplot as plt\r\n\r\n# Generate data\r\nx = np.linspace(0, 2*np.pi, 1000)\r\ny = np.sin(x)\r\n\r\n# Create plot\r\nplt.figure(figsize=(6, 4))\r\nplt.plot(x, y, label='sin(x)', color='royalblue')\r\nplt.title('Sine Wave (0 to 2\u03c0)')\r\nplt.xlabel('x')\r\nplt.ylabel('sin(x)')\r\nplt.grid(True, alpha=0.3)\r\nplt.legend()\r\n\r\n# Save as PNG\r\noutput_path = '/mnt/data/sine_wave.png'\r\nplt.savefig(output_path, dpi=150, bbox_inches='tight')\r\n\r\n# Show the plot inline\r\nplt.show()\r\n\r\noutput_path",
"container_id": "cntr_692dffc4e7708190bcd6e0e7c35b27ce000abd71d9736540",
"outputs": [
{
"type": "image",
"url": "data:image/png;base64,iVBORw0KGgoAAAANSUhEUg"
}
]
},
{
"id": "msg_01dfc61333fbdf3600692dfffcbcfc81969f92318d9d3c41ca",
"type": "message",
"status": "completed",
"content": [
{
"type": "output_text",
"annotations": [],
"logprobs": [],
"text": "I've generated the sine wave plot and embedded it in the tool outputs as a base64 PNG."
}
],
"role": "assistant"
}
]
}
""";

var expected = """
{
"output": [
{
"id": "rs_01dfc61333fbdf3600692dffc53f4c8196b55eae21c8727d1a",
"type": "reasoning",
"summary": []
},
{
"id": "ci_01dfc61333fbdf3600692dfff0a80881968003a7f1010139ba",
"type": "code_interpreter_call",
"status": "completed",
"code": "import numpy as np\r\nimport matplotlib.pyplot as plt\r\n\r\n# Generate data\r\nx = np.linspace(0, 2*np.pi, 1000)\r\ny = np.sin(x)\r\n\r\n# Create plot\r\nplt.figure(figsize=(6, 4))\r\nplt.plot(x, y, label='sin(x)', color='royalblue')\r\nplt.title('Sine Wave (0 to 2\u03c0)')\r\nplt.xlabel('x')\r\nplt.ylabel('sin(x)')\r\nplt.grid(True, alpha=0.3)\r\nplt.legend()\r\n\r\n# Save as PNG\r\noutput_path = '/mnt/data/sine_wave.png'\r\nplt.savefig(output_path, dpi=150, bbox_inches='tight')\r\n\r\n# Show the plot inline\r\nplt.show()\r\n\r\noutput_path",
"container_id": "cntr_692dffc4e7708190bcd6e0e7c35b27ce000abd71d9736540",
"outputs": [
{
"type": "image"}
]
},
{
"id": "msg_01dfc61333fbdf3600692dfffcbcfc81969f92318d9d3c41ca",
"type": "message",
"status": "completed",
"content": [
{
"type": "output_text",
"annotations": [],
"logprobs": [],
"text": "I've generated the sine wave plot and embedded it in the tool outputs as a base64 PNG."
}
],
"role": "assistant"
}
]
}
""";

var patch = new JsonPatch(BinaryData.FromString(json));
var index = 0;
var imageUrl = "";

while (patch.TryGetValue(Encoding.UTF8.GetBytes($"$.output[{index}].type"), out string? type))
{
if (type == "code_interpreter_call")
{
var path = Encoding.UTF8.GetBytes($"$.output[{index}].outputs[0].url");

imageUrl = patch.GetString(path);

patch.Remove(path);
}

++index;
}

Assert.AreEqual(expected, patch.ToString("J"));
}

[Test]
public void IterateOverArrayItems()
{
var json = """
{
"items": [
{ "id": 1, "name": "Item1" },
{ "id": 2, "name": "Item2" },
{ "id": 3, "name": "Item3" }
]
}
""";
var patch = new JsonPatch(BinaryData.FromString(json));
var index = 0;
while (patch.TryGetValue(Encoding.UTF8.GetBytes($"$.items[{index}].id"), out int id))
{
var namePath = Encoding.UTF8.GetBytes($"$.items[{index}].name");
Assert.AreEqual($"Item{id}", patch.GetString(namePath));
++index;
}

Assert.AreEqual(3, index);
}

[Test]
public void IterateOverArrayItems_WithModify()
{
var json = """
{
"items": [
{ "id": 1, "name": "Item1" },
{ "id": 2, "name": "Item2" },
{ "id": 3, "name": "Item3" }
]
}
""";
var patch = new JsonPatch(BinaryData.FromString(json));
patch.Set("$.items[1].name"u8, "UpdatedItem2");
var index = 0;
while (patch.TryGetValue(Encoding.UTF8.GetBytes($"$.items[{index}].id"), out int id))
{
var namePath = Encoding.UTF8.GetBytes($"$.items[{index}].name");
if (index == 1)
{
Assert.AreEqual("UpdatedItem2", patch.GetString(namePath));
}
else
{
Assert.AreEqual($"Item{id}", patch.GetString(namePath));
}
++index;
}

Assert.AreEqual(3, index);
}

[Test]
public void IterateOverArrayItems_WithRemoveItemProperty()
{
var json = """
{
"items": [
{ "id": 1, "name": "Item1" },
{ "id": 2, "name": "Item2" },
{ "id": 3, "name": "Item3" }
]
}
""";
var patch = new JsonPatch(BinaryData.FromString(json));
patch.Remove("$.items[1].id"u8);
var index = 0;
while (patch.TryGetJson(Encoding.UTF8.GetBytes($"$.items[{index}]"), out var itemJson))
{
var namePath = Encoding.UTF8.GetBytes($"$.items[{index}].name");
Assert.AreEqual($"Item{index + 1}", patch.GetString(namePath));
++index;
}

Assert.AreEqual(3, index);

// if we look for id it will stop after 1 item because the 2nd item id was removed
index = 0;
while (patch.TryGetValue(Encoding.UTF8.GetBytes($"$.items[{index}].id"), out int id))
{
var namePath = Encoding.UTF8.GetBytes($"$.items[{index}].name");
Assert.AreEqual($"Item{id}", patch.GetString(namePath));
++index;
}

Assert.AreEqual(1, index);

// if we get by json it will iterate over all 3 since the id property removal can still be returned as empty json
index = 0;
while (patch.TryGetJson(Encoding.UTF8.GetBytes($"$.items[{index}].id"), out var idJson))
{
var namePath = Encoding.UTF8.GetBytes($"$.items[{index}].name");
Assert.AreEqual($"Item{index + 1}", patch.GetString(namePath));
++index;
}

Assert.AreEqual(3, index);
}

[Test]
public void IterateOverArrayItems_WithRemoveItem()
{
var json = """
{
"items": [
{ "id": 1, "name": "Item1" },
{ "id": 2, "name": "Item2" },
{ "id": 3, "name": "Item3" }
]
}
""";
var patch = new JsonPatch(BinaryData.FromString(json));
patch.Remove("$.items[1]"u8);
var index = 0;
while (patch.TryGetValue(Encoding.UTF8.GetBytes($"$.items[{index}].id"), out int id))
{
var namePath = Encoding.UTF8.GetBytes($"$.items[{index}].name");
Assert.AreEqual($"Item{id}", patch.GetString(namePath));
++index;
}

// iteration will stop after 1 item because item 2 was removed and therefore it won't have an id
Assert.AreEqual(1, index);
Assert.AreEqual("{ \"id\": 3, \"name\": \"Item3\" }", patch.GetJson("$.items[2]"u8).ToString());
Assert.AreEqual(3, patch.GetInt32("$.items[2].id"u8));
Assert.AreEqual("Item3", patch.GetString("$.items[2].name"u8));
}

[Test]
public void IterateOverArrayItems_WithAppend()
{
var json = """
{
"items": [
{ "id": 1, "name": "Item1" },
{ "id": 2, "name": "Item2" },
{ "id": 3, "name": "Item3" }
]
}
""";
var patch = new JsonPatch(BinaryData.FromString(json));
patch.Append("$.items"u8, "{\"id\":4,\"name\":\"Item4\"}"u8);
var index = 0;
while (patch.TryGetValue(Encoding.UTF8.GetBytes($"$.items[{index}].id"), out int id))
{
var namePath = Encoding.UTF8.GetBytes($"$.items[{index}].name");
Assert.AreEqual($"Item{id}", patch.GetString(namePath));
++index;
}

Assert.AreEqual(4, index);
}
}
}
Loading