Avoid per-element patch-path allocations in C# collection serializers

[Copilot]

Generated collection serializers and ActiveItems() allocate an interpolated string and UTF-8 byte array per element, even when no relevant patch exists.

• Allocation guard: Check for patch entries under the collection’s top-level property once per loop, then short-circuit indexed path construction. Apply the same guard to nested collections and spans.
• Patch semantics: Use the descendant-aware Contains(prefix, property) overload. Keep child-model root-removal checks independent of the parent guard.

bool hasPatch = Patch.Contains("$"u8, "data"u8);
for (int i = 0; i < Items.Count; i++)
{
    if (hasPatch && Patch.IsRemoved(Encoding.UTF8.GetBytes($"$.data[{i}]"))
        || Items[i] != null && Items[i].Patch.IsRemoved("$"u8))
    {
        continue;
    }
    writer.WriteObjectValue(Items[i], options);
}

• Fixes Generated collection serializers allocate per-element when computing indexed patch paths #11934

[azure-pipelines]

Azure Pipelines:
Successfully started running 1 pipeline(s).
1 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@typespec/http-client-csharp@11947

commit: e3db771

[Copilot]

🔵 Needs a closer look

Nested collection and dictionary serializers still construct indexed patch paths without the collection-level guard.

Pull request overview

This PR reduces per-element patch-path allocations in generated C# collection serializers and ActiveItems() helpers.

Changes:

• Adds descendant-aware patch guards for collections, nested arrays, spans, and dictionaries.
• Preserves child-model root-removal checks.
• Updates generated baselines and adds allocation/patch-semantics tests.

File summaries

File	Reviewed changes
packages/http-client-csharp/generator/TestProjects/Local/Sample-TypeSpec/src/Generated/Models/NullableDynamicModel.Serialization.cs	Updates generated collection serializers.
packages/http-client-csharp/generator/TestProjects/Local/Sample-TypeSpec/src/Generated/Models/DynamicModel.Serialization.cs	Updates generated serializers and helpers.
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/DynamicModelSerializationTests/WriteRequiredCollectionDoesNotDuplicatePatchedKey.cs	Updates collection serialization baseline.
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/DynamicModelSerializationTests/WriteReadOnlySpanProperty.cs	Updates span serialization baseline.
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/DynamicModelSerializationTests/WriteNestedArrayPrimitiveProperties.cs	Updates nested-array baseline.
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/DynamicModelSerializationTests/WriteNestedArrayDynamicModelProperties.cs	Updates nested dynamic-model baseline.
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/DynamicModelSerializationTests/WriteNestedArrayDictionaryProperties.cs	Updates nested dictionary baseline.
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/DynamicModelSerializationTests/WriteArrayProperties.cs	Updates array serialization baselines.
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/DynamicModelSerializationTests/PropagateModelListPropertyHelperMethods.cs	Updates active-item helper baseline.
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/ModelReaderWriterValidation/TestProjects/Sample_TypeSpec/DynamicModelTests.cs	Tests allocation reduction and child-removal semantics.
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MrwSerializationTypeDefinition.Dynamic.cs	Generates collection-level guards; nested and dictionary indexed patch-path construction remains ungated.

Review details

Suppressed comments (1)

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MrwSerializationTypeDefinition.Dynamic.cs:167

• This guard only short-circuits patchIsRemovedCondition; nested serializers still unconditionally build indexed paths for writeToPatchStatement after each inner loop (for example, Patch.WriteTo(... Encoding.UTF8.GetBytes($"$.propertyWithNestedArray[{i}]"))). The list-of-dictionaries path is likewise built for each key. Consequently an unpatched nested collection still allocates despite hasPatch being false; gate all indexed Patch.WriteTo/Patch.Contains construction with the prefix guard, including the dictionary path.

                hasPatchDeclaration,

• Files reviewed: 9/11 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Jorge Rangel (jorgerangel-msft)]

🔵 Needs a closer look

Nested collection and dictionary serializers still construct indexed patch paths without the collection-level guard.

Pull request overview
This PR reduces per-element patch-path allocations in generated C# collection serializers and ActiveItems() helpers.

Changes:

• Adds descendant-aware patch guards for collections, nested arrays, spans, and dictionaries.
• Preserves child-model root-removal checks.
• Updates generated baselines and adds allocation/patch-semantics tests.

File summaries
File Reviewed changes
packages/http-client-csharp/generator/TestProjects/Local/Sample-TypeSpec/src/Generated/Models/NullableDynamicModel.Serialization.cs Updates generated collection serializers.
packages/http-client-csharp/generator/TestProjects/Local/Sample-TypeSpec/src/Generated/Models/DynamicModel.Serialization.cs Updates generated serializers and helpers.
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/DynamicModelSerializationTests/WriteRequiredCollectionDoesNotDuplicatePatchedKey.cs Updates collection serialization baseline.
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/DynamicModelSerializationTests/WriteReadOnlySpanProperty.cs Updates span serialization baseline.
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/DynamicModelSerializationTests/WriteNestedArrayPrimitiveProperties.cs Updates nested-array baseline.
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/DynamicModelSerializationTests/WriteNestedArrayDynamicModelProperties.cs Updates nested dynamic-model baseline.
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/DynamicModelSerializationTests/WriteNestedArrayDictionaryProperties.cs Updates nested dictionary baseline.
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/DynamicModelSerializationTests/WriteArrayProperties.cs Updates array serialization baselines.
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/TestData/DynamicModelSerializationTests/PropagateModelListPropertyHelperMethods.cs Updates active-item helper baseline.
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/ModelReaderWriterValidation/TestProjects/Sample_TypeSpec/DynamicModelTests.cs Tests allocation reduction and child-removal semantics.
packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MrwSerializationTypeDefinition.Dynamic.cs Generates collection-level guards; nested and dictionary indexed patch-path construction remains ungated.
Review details

Suppressed comments (1)

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MrwSerializationTypeDefinition.Dynamic.cs:167

• This guard only short-circuits patchIsRemovedCondition; nested serializers still unconditionally build indexed paths for writeToPatchStatement after each inner loop (for example, Patch.WriteTo(... Encoding.UTF8.GetBytes($"$.propertyWithNestedArray[{i}]"))). The list-of-dictionaries path is likewise built for each key. Consequently an unpatched nested collection still allocates despite hasPatch being false; gate all indexed Patch.WriteTo/Patch.Contains construction with the prefix guard, including the dictionary path.

                hasPatchDeclaration,

• Files reviewed: 9/11 changed files
• Comments generated: 0
• Review effort level: Lite

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Copilot validate if we are correctly guarding on nested elements in dictionaries + lists

[Copilot]

🔵 Needs a closer look

Nested collection and dictionary serializers still construct indexed patch paths without the collection-level guard.

...

Not fully before this follow-up. Fixed in fd889537:

• Nested-list Patch.WriteTo now guards indexed path construction.
• Dictionaries guard key encoding, indexed Patch.Contains, and Patch.WriteTo in both framework branches.
• Child-model root-removal checks remain independent of the parent guard.

Regression tests cover list/list, list/dictionary, dictionary/list, and dictionary/dictionary with empty or unrelated patches. Previously these allocated 28–37 KB; they now pass the <1 KB budget. Supported dictionary patch additions/removals and nested child-root removals also pass.

Full regeneration, the normal build, 4,196 .NET tests, and 240 emitter tests passed. Independent review found no significant issues. Validation limits: ESLint was unavailable, Cop was not run, automated review was unavailable, CodeQL skipped the oversized database, and explicit .NET Standard compilation encountered ExperimentalAttribute errors in unchanged files.

Separately, pre-fix probes exposed existing mixed nested-patch merge failures (remove-plus-append and dictionary-of-dictionaries descendant updates). This allocation fix does not change those semantics.

[Jorge Rangel (jorgerangel-msft)]

🔵 Needs a closer look

Nested collection and dictionary serializers still construct indexed patch paths without the collection-level guard.

...

Not fully before this follow-up. Fixed in fd889537:

• Nested-list Patch.WriteTo now guards indexed path construction.
• Dictionaries guard key encoding, indexed Patch.Contains, and Patch.WriteTo in both framework branches.
• Child-model root-removal checks remain independent of the parent guard.

Regression tests cover list/list, list/dictionary, dictionary/list, and dictionary/dictionary with empty or unrelated patches. Previously these allocated 28–37 KB; they now pass the <1 KB budget. Supported dictionary patch additions/removals and nested child-root removals also pass.

Full regeneration, the normal build, 4,196 .NET tests, and 240 emitter tests passed. Independent review found no significant issues. Validation limits: ESLint was unavailable, Cop was not run, automated review was unavailable, CodeQL skipped the oversized database, and explicit .NET Standard compilation encountered ExperimentalAttribute errors in unchanged files.

Separately, pre-fix probes exposed existing mixed nested-patch merge failures (remove-plus-append and dictionary-of-dictionaries descendant updates). This allocation fix does not change those semantics.

Copilot this introduced additional complexity and duplication. Revert the previous changes / commit

[Copilot]

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (3)

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MrwSerializationTypeDefinition.Dynamic.cs:125

• Because every recursive call passes the same root serializedName, this guard becomes true for every nested collection whenever any sibling under the root property has a patch. For example, a patch under $.propertyWithNestedDictionary["patched"] still makes the loop for "unpatched" build an encoded path for every entry, so the allocation guard is not effective for nested collections with sibling patches. Base the guard on the current nested collection prefix (or carry a per-parent result) and add a sibling-patch regression case.

            var hasPatchDeclaration = Declare(
                "hasPatch",
                typeof(bool),
                patchSnippet.Contains(LiteralU8("$"), LiteralU8(serializedName.Split('.')[0])),
                out var hasPatch);

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MrwSerializationTypeDefinition.Dynamic.cs:125

• The new nested-list and read-only-span branches are covered only by generated-source snapshots. The runtime allocation test uses empty inner lists and no span collection, so it would pass even if the hasPatch0 or span path mishandled indexed patches; add a populated nested/span runtime case that asserts patch merging as well as allocations.

            var hasPatchDeclaration = Declare(
                "hasPatch",
                typeof(bool),
                patchSnippet.Contains(LiteralU8("$"), LiteralU8(serializedName.Split('.')[0])),
                out var hasPatch);

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MrwSerializationTypeDefinition.Dynamic.cs:630

• BuildActiveItemsMethod is validated only by TypeProviderWriter text comparison; no runtime test invokes the generated TryResolve.../Active... path after an unrelated parent patch or an indexed removal. Add a populated direct dynamic-list test that exercises both cases, otherwise this new short-circuit can regress while the snapshot still passes.

            var serializedName = GetJsonSerializedName(property.WireInfo!);
            var hasPatchDeclaration = Declare(
                "hasPatch",
                typeof(bool),
                _jsonPatchProperty!.As<JsonPatch>().Contains(LiteralU8("$"), LiteralU8(serializedName.Split('.')[0])),
                out var hasPatch);

• Files reviewed: 12/14 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/test/Providers/MrwSerializationTypeDefinitions/DynamicModelSerializationTests.cs:499

• This test only checks the exact property path and the indexed path text; it never asserts the new descendant-aware Patch.Contains("$"u8, serializedName) guard. A regression that omits the allocation guard or passes the wrong escaped property to that overload would still pass, so assert the generated hasPatch declaration (or compare against a complete baseline) as well.

            MockHelpers.LoadMockGenerator(inputModels: () => [inputModel]);
            var model = ScmCodeModelGenerator.Instance.TypeFactory.CreateModel(inputModel) as ClientModel.Providers.ScmModelProvider;

• Files reviewed: 11/13 changed files
• Comments generated: 2
• Review effort level: Lite

[Jorge Rangel (jorgerangel-msft)]

Copilot why is this duplicated ? Is Patch here the same property from like 296 ?

[Copilot]

Yes. Both access the enclosing model’s same Patch property. hasPatch0 is redundantly emitted by the recursive nested-list serializer; it repeats the collection-level descendant check already performed by hasPatch at line 296.

[Jorge Rangel (jorgerangel-msft)]

Copilot then that's a bug we need to fix. We shouldn't be generating a duplicate check if we already checked the condition before we iterating the elements

[Copilot]

Fixed in 54bf7fc3. Nested collection serialization now reuses the existing outer hasPatch guard instead of generating duplicate inner guard checks, and the affected generated baselines were updated accordingly.

[Copilot]

🔵 Needs a closer look

Two unresolved moderate findings remain involving nested path allocation and interpolated-string brace escaping.

Review details

Suppressed comments (2)

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MrwSerializationTypeDefinition.Dynamic.cs:165

• The indexed Patch.WriteTo emitted for parentIndices.Count > 0 is still unconditional. In generated nested-array code this remains an Encoding.UTF8.GetBytes($"...[{i}]") allocation per outer element (for example, WriteNestedArrayDynamicModelProperties.cs:87 and :90) even when the top-level hasPatch is false, so nested collections do not receive the claimed allocation guard. Guard this nested Patch.WriteTo with hasPatch before constructing the indexed path.

                ? patchSnippet.WriteTo(_utf8JsonWriterSnippet, LiteralU8(jsonPathTemplate)).Terminate()
                : patchSnippet.WriteTo(_utf8JsonWriterSnippet, Utf8Snippets.GetBytes(new FormattableStringExpression(csharpJsonPathTemplate, parentIndices).As<string>())).Terminate();

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MrwSerializationTypeDefinition.Dynamic.cs:706

• This string is inserted verbatim into a FormattableStringExpression, whose formatter treats single {/} as interpolation syntax. A serialized property name such as a{b} therefore produces malformed generated C# even though RequiresJsonPathBracketNotation routes it through this escaping path; escape literal braces for the interpolated-string template separately from JSON-path escaping and add a regression case.

            ValueExpression? optionsVariable = null)
        {
            optionsVariable ??= ModelSerializationExtensionsSnippets.Wire;

• Files reviewed: 11/13 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

The dictionary outer guard still mishandles dotted serialized names and needs correction.

Review details

Suppressed comments (1)

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MrwSerializationTypeDefinition.Dynamic.cs:36

• The new bracket-aware path is used inside CreateDictionarySerializationWithPatch, but the outer collection guard still builds Patch.Contains("$.{jsonSerializedName}") (MrwSerializationTypeDefinition.cs:2003-2005). For a dynamic dictionary whose wire name is foo.bar, an exact patch at $["foo.bar"] is therefore missed and the CLR dictionary serializer runs instead of the patch branch, which can produce duplicate or incorrectly shaped output. Use the same bracket-aware path for the dictionary's outer guard and add a dotted dictionary regression test.

            var jsonPathTemplate = BuildJsonPathForElement(serializedName, parentIndices);
            var csharpJsonPathTemplate = BuildJsonPathForElement(serializedName, parentIndices, escapeForCSharpString: true);
            ValueExpression jsonPath = parentIndices.Count > 0
                ? Utf8Snippets.GetBytes(new FormattableStringExpression(csharpJsonPathTemplate, [.. parentIndices]).As<string>())
                : LiteralU8(jsonPathTemplate);

• Files reviewed: 11/13 changed files
• Comments generated: 0 new
• Review effort level: Lite

[JoshLove-msft]

Reviewed 54bf7fc30. No additional findings beyond the existing dotted-name dictionary guard finding, which still applies to the current code. I have not duplicated that inline comment.

--generated by Copilot

[Copilot]

🟡 Changes recommended

One or more issues must be addressed before approval.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (2)

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MrwSerializationTypeDefinition.Dynamic.cs:720

• When a wire name contains both . and a quote or backslash, this bracket path embeds the raw name. For example, foo\".bar generates an invalid $[\"foo\".bar\"] path, so Patch.Contains/IsRemoved cannot match the property. Escape the name for the JsonPatch quoted segment before applying C# string escaping, and cover quote/backslash names with a regression test.

                ? $"$[\"{propertySerializedName}\"]"
                : $"$.{propertySerializedName}";

            return escapeForCSharpString
                ? jsonPath.Replace("\"", "\\\"")

packages/http-client-csharp/generator/Microsoft.TypeSpec.Generator.ClientModel/src/Providers/MrwSerializationTypeDefinition.Dynamic.cs:180

• The guard only short-circuits the indexed IsRemoved probe; the nested-list Patch.WriteTo below still emits Encoding.UTF8.GetBytes($"...[{i}]...") unconditionally. For example, the generated nested-array baseline still allocates these paths at WriteNestedArrayPrimitiveProperties.cs:90,93 when hasPatch is false, so patch-free nested collections retain the per-element allocation this change is meant to remove. Gate dynamic WriteTo calls with the same descendant guard and add an allocation regression for nested collections.

            var writeToPatchStatement = parentIndices.Count == 0
                ? patchSnippet.WriteTo(_utf8JsonWriterSnippet, LiteralU8(jsonPathTemplate)).Terminate()
                : patchSnippet.WriteTo(_utf8JsonWriterSnippet, Utf8Snippets.GetBytes(new FormattableStringExpression(csharpJsonPathTemplate, parentIndices).As<string>())).Terminate();

• Files reviewed: 11/13 changed files
• Comments generated: 1
• Review effort level: Lite