Copilot/add readonlymemory support by AlgorithmsAreCool · Pull Request #188 · dameng324/LightProto

AlgorithmsAreCool · 2026-06-14T19:14:45Z

This pr adds built in support for Memory/ReadOnlyMemory/ReadOnlySequence

Copilot using Codex 5.3 was the primary LLM used with human review and critique.

I don't fully understand all of the reader/writer interfaces in this project, but the code seems to be functioning as expected

Agent-Logs-Url: https://github.com/AlgorithmsAreCool/LightProto/sessions/baab16b3-347e-4972-ba9b-bb4ef9467a61 Co-authored-by: AlgorithmsAreCool <10837869+AlgorithmsAreCool@users.noreply.github.com>

Agent-Logs-Url: https://github.com/AlgorithmsAreCool/LightProto/sessions/81c76820-749c-4e93-a4eb-b0d9bb99a31d Co-authored-by: AlgorithmsAreCool <10837869+AlgorithmsAreCool@users.noreply.github.com>

…lySequence<byte> as repeated bytes Agent-Logs-Url: https://github.com/AlgorithmsAreCool/LightProto/sessions/b0e20744-35f3-4e1d-89f5-e161346fdd8c Co-authored-by: AlgorithmsAreCool <10837869+AlgorithmsAreCool@users.noreply.github.com>

Agent-Logs-Url: https://github.com/AlgorithmsAreCool/LightProto/sessions/367968f0-079c-44f8-b8ae-23d7cdad6437 Co-authored-by: AlgorithmsAreCool <10837869+AlgorithmsAreCool@users.noreply.github.com>

codacy-production · 2026-06-14T19:16:48Z

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 101 complexity · 18 duplication

Metric Results

Complexity 101

Duplication 18

View in Codacy

_{NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer}
_{TIP This summary will be updated as you push new changes.}

Copilot

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

This PR adds first-class support for Memory<T>, ReadOnlyMemory<T>, and ReadOnlySequence<T> collection members in LightProto parsing/writing and generator logic, along with new tests and public API updates.

Changes:

Introduces new *ProtoReader / *ProtoWriter implementations for Memory<T>, ReadOnlyMemory<T>, and ReadOnlySequence<T>.
Updates the source generator to treat these types as collections and ensure correct tag behavior for byte-backed memory types.
Adds unit tests validating serialization/deserialization behavior and “single tag” encoding for byte memory members.

Reviewed changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
tests/LightProto.Tests/RuntimeParserTests.cs	Adds runtime (reflection-free) tests ensuring byte memory members encode with a single length-delimited tag.
tests/LightProto.Tests/Parsers/parser.proto	Formatting-only change (closing brace line numbering/whitespace).
tests/LightProto.Tests/Parsers/ReadOnlySequenceTests.cs	Adds coverage for `ReadOnlySequence<int>` parsing/writing behaviors (protobuf-net disabled).
tests/LightProto.Tests/Parsers/ReadOnlySequenceBytesTests.cs	Adds coverage for `ReadOnlySequence<byte>`, including multi-segment sequences.
tests/LightProto.Tests/Parsers/ReadOnlyMemoryTests.cs	Adds coverage for `ReadOnlyMemory<int>` parsing/writing behaviors.
tests/LightProto.Tests/Parsers/ReadOnlyMemoryBytesTests.cs	Adds coverage for `ReadOnlyMemory<byte>` parsing/writing behaviors.
tests/LightProto.Tests/Parsers/MemoryTests.cs	Adds coverage for `Memory<int>` parsing/writing behaviors.
tests/LightProto.Tests/Parsers/MemoryBytesTests.cs	Adds coverage for `Memory<byte>` parsing/writing behaviors.
src/LightProto/PublicAPI/netstandard2.0/PublicAPI.Shipped.txt	Ships public API surface for new memory/sequence readers and writers (netstandard2.0).
src/LightProto/PublicAPI/net/PublicAPI.Shipped.txt	Ships public API surface for new memory/sequence readers and writers (.NET).
src/LightProto/Parser/ReadOnlySequence.cs	Adds `ReadOnlySequenceProtoReader/Writer` implementations with byte special-casing.
src/LightProto/Parser/ReadOnlyMemory.cs	Adds `ReadOnlyMemoryProtoReader/Writer` implementations with byte special-casing.
src/LightProto/Parser/Memory.cs	Adds `MemoryProtoReader/Writer` implementations with byte special-casing.
src/LightProto.Generator/ProtoMember.cs	Updates “not empty” check generation to handle value-type collections (e.g., `Memory<T>`).
src/LightProto.Generator/Helper.cs	Treats memory/sequence types as collections; supports `Length: long`; adjusts tag choice for byte memory types.

+            if (IsByte)
+            {
+                var bytes = (Memory<byte>)(object)value;
+                output.WriteTag(Tag);
+                output.WriteLongLength(bytes.Length);
+                WritingPrimitives.WriteRawBytes(ref output.buffer, ref output.state, bytes.Span);
+                return;
+            }


+            if (IsByte)
+            {
+                var bytes = (ReadOnlyMemory<byte>)(object)value;
+                output.WriteTag(Tag);
+                output.WriteLongLength(bytes.Length);
+                WritingPrimitives.WriteRawBytes(ref output.buffer, ref output.state, bytes.Span);
+                return;
+            }


+                foreach (var segment in value)
+                {
+                    if (segment.IsEmpty)
+                        continue;
+                    WritingPrimitives.WriteRawBytes(ref output.buffer, ref output.state, ((ReadOnlyMemory<byte>)(object)segment).Span);
+                }


+            foreach (var item in value)
+            {
+                foreach (var current in item.Span)
+                {
+                    if (current is null)
+                        throw new Exception("Sequence contained null element");
+                    output.WriteTag(Tag);
+                    ItemWriter.WriteMessageTo(ref output, current);
+                }
+            }


+        public ReadOnlySequenceProtoReader(IProtoReader<TItem> itemReader, uint tag, int itemFixedSize)
+            : this(itemReader, itemFixedSize) { }


dameng324

Thanks for your pull request. Here are some review suggestions.

dameng324 · 2026-06-15T11:57:44Z

+        IProtoWriter<T> ItemWriter { get; }
+        uint Tag { get; set; }
+        int ItemFixedSize { get; }
+        static bool IsByte => typeof(T) == typeof(byte);


It is better that check T is byte or not at compile time rather than runtime, like byte array did before.

Still check it at runtime. you can see how ArrayProtoWriter and ByteArrayProtoWriter is implemented.

Will you try to understand the code and fix it without using AI? I just want to make sure you can understand what I am talking about.

dameng324 · 2026-06-15T12:04:38Z

@@ -0,0 +1,151 @@
+namespace LightProto.Parser
+{
+    public sealed class MemoryProtoWriter<T> : IProtoWriter, IProtoWriter<Memory<T>>, ICollectionWriter


The whole MemoryProtoWriter has too much duplicate codes that ArrayProtoWriter already has. We should figure out how to avoid this.

I'll consolidate the implementations

dameng324 · 2026-06-15T12:06:27Z

@@ -0,0 +1,151 @@
+namespace LightProto.Parser
+{
+    public sealed class ReadOnlyMemoryProtoWriter<T> : IProtoWriter, IProtoWriter<ReadOnlyMemory<T>>, ICollectionWriter


Same issue as MemoryProtoWriter.

dameng324 · 2026-06-15T12:07:09Z

+
+namespace LightProto.Parser
+{
+    public sealed class ReadOnlySequenceProtoWriter<T> : IProtoWriter, IProtoWriter<ReadOnlySequence<T>>, ICollectionWriter


Same issue as MemoryProtoWriter.

dameng324 · 2026-06-15T12:11:16Z

+    public partial record Message
+    {
+        [ProtoMember(1)]
+        [ProtoBuf.ProtoMember(1)]


It is better that adding a packed=true repeated field test case.

AlgorithmsAreCool · 2026-06-15T15:06:51Z

@dameng324
I'll make a 2nd pass and address all feedback.

As a question however, are the writers and readers implementing the correct interfaces?

dameng324 · 2026-06-16T00:10:33Z

As a question however, are the writers and readers implementing the correct interfaces?

Yes, The implementation seems right.

Copilot

Pull request overview

Copilot reviewed 16 out of 17 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

src/LightProto/PublicAPI/netstandard2.0/PublicAPI.Shipped.txt:263

These are new public APIs. With PublicApiAnalyzers, new surface area is typically added to PublicAPI.Unshipped.txt until an official release, then moved to PublicAPI.Shipped.txt. Putting them directly in PublicAPI.Shipped.txt makes the API appear already shipped and can complicate versioning/reviews.

LightProto.Parser.MemoryProtoWriter<T>.IsMessage.get -> bool
LightProto.Parser.MemoryProtoWriter<T>.MemoryProtoWriter(LightProto.IProtoWriter<T>! itemWriter, uint tag, int itemFixedSize) -> void
LightProto.Parser.MemoryProtoWriter<T>.WireType.get -> LightProto.WireFormat.WireType
LightProto.Parser.MemoryProtoWriter<T>.WriteTo(ref LightProto.WriterContext output, System.Memory<T> value) -> void
LightProto.Parser.NullableProtoReader<T>
LightProto.Parser.NullableProtoReader<T>.IsMessage.get -> bool
LightProto.Parser.NullableProtoReader<T>.NullableProtoReader(LightProto.IProtoReader<T>! valueReader) -> void
LightProto.Parser.NullableProtoReader<T>.ParseFrom(ref LightProto.ReaderContext input) -> T?
LightProto.Parser.NullableProtoReader<T>.ValueReader.get -> LightProto.IProtoReader<T>!
LightProto.Parser.NullableProtoReader<T>.WireType.get -> LightProto.WireFormat.WireType
LightProto.Parser.NullableProtoWriter<T>
LightProto.Parser.NullableProtoWriter<T>.CalculateSize(T? value) -> int
LightProto.Parser.NullableProtoWriter<T>.IsMessage.get -> bool
LightProto.Parser.NullableProtoWriter<T>.NullableProtoWriter(LightProto.IProtoWriter<T>! valueWriter) -> void
LightProto.Parser.NullableProtoWriter<T>.ValueWriter.get -> LightProto.IProtoWriter<T>!
LightProto.Parser.NullableProtoWriter<T>.WireType.get -> LightProto.WireFormat.WireType

 LightProto.Parser.LinkedListProtoReader<T>.LinkedListProtoReader(LightProto.IProtoReader<T>! itemReader, uint tag, int itemFixedSize) -> void
 LightProto.Parser.LinkedListProtoWriter<T>
 LightProto.Parser.LinkedListProtoWriter<T>.LinkedListProtoWriter(LightProto.IProtoWriter<T>! itemWriter, uint tag, int itemFixedSize) -> void
 LightProto.Parser.ListProtoReader<T>
 LightProto.Parser.ListProtoReader<T>.ListProtoReader(LightProto.IProtoReader<T>! itemReader, int itemFixedSize) -> void
 LightProto.Parser.ListProtoReader<T>.ListProtoReader(LightProto.IProtoReader<T>! itemReader, uint tag, int itemFixedSize) -> void
 LightProto.Parser.ListProtoWriter<T>
 LightProto.Parser.ListProtoWriter<T>.ListProtoWriter(LightProto.IProtoWriter<T>! itemWriter, uint tag, int itemFixedSize) -> void
 LightProto.Parser.Matrix3x2ProtoParser
 LightProto.Parser.Matrix3x2ProtoParser.Matrix3x2ProtoParser() -> void
 LightProto.Parser.Matrix4x4ProtoParser
 LightProto.Parser.Matrix4x4ProtoParser.Matrix4x4ProtoParser() -> void
+LightProto.Parser.MemoryProtoReader<TItem>
+LightProto.Parser.MemoryProtoReader<TItem>.Empty.get -> System.Memory<TItem>
+LightProto.Parser.MemoryProtoReader<TItem>.IsMessage.get -> bool
+LightProto.Parser.MemoryProtoReader<TItem>.ItemReader.get -> LightProto.IProtoReader<TItem>!


codecov · 2026-06-21T23:46:36Z

Codecov Report

❌ Patch coverage is 68.93617% with 73 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.53%. Comparing base (7a23ce5) to head (b2e35f7).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/LightProto/Parser/ReadOnlySequence.cs	62.19%	23 Missing and 8 partials ⚠️
src/LightProto/Parser/ReadOnlyMemory.cs	63.23%	20 Missing and 5 partials ⚠️
src/LightProto/Parser/Memory.cs	77.94%	11 Missing and 4 partials ⚠️
src/LightProto/Parser/CollectionWriteHelper.cs	88.23%	1 Missing and 1 partial ⚠️

❌ Your patch check has failed because the patch coverage (68.93%) is below the target coverage (95.00%). You can increase the patch coverage or adjust the target coverage.
❌ Your project check has failed because the head coverage (93.53%) is below the target coverage (95.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #188      +/-   ##
==========================================
- Coverage   95.36%   93.53%   -1.83%     
==========================================
  Files         121      127       +6     
  Lines        3061     3328     +267     
  Branches      423      470      +47     
==========================================
+ Hits         2919     3113     +194     
- Misses         76      131      +55     
- Partials       66       84      +18

☔ View full report in Codecov by Harness.
📢 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.

dameng324

I found this issue is too much complex for a new contributor.

If you just want this feature, I can take over this PR.
If you want to become a contributor of this project, you may need more understand of the codebase and more coding by hand instead of AI. I'm glad to help with you.

dameng324 · 2026-06-22T02:04:06Z

    {
        if (elementType.SpecialType == SpecialType.System_Byte && IsArrayType(type))
            return false;
+        if (IsReadOnlyMemoryType(type) || IsMemoryType(type) || IsReadOnlySequenceType(type))


If T is byte, these types are not collection types any more.

dameng324 · 2026-06-22T02:08:52Z

+        IProtoWriter<T> ItemWriter { get; }
+        uint Tag { get; set; }
+        int ItemFixedSize { get; }
+        static bool IsByte => typeof(T) == typeof(byte);


Still check it at runtime. you can see how ArrayProtoWriter and ByteArrayProtoWriter is implemented.

Will you try to understand the code and fix it without using AI? I just want to make sure you can understand what I am talking about.

AlgorithmsAreCool · 2026-06-23T18:27:51Z

I found this issue is too much complex for a new contributor.

If you just want this feature, I can take over this PR. If you want to become a contributor of this project, you may need more understand of the codebase and more coding by hand instead of AI. I'm glad to help with you.

Yeah, I need to study this problem more to understand the mechanics. I'll try to find time at somepoint to understand it better.

That being said, this feature isn't critical, i can just use arrays or my own surrogates

Copilot AI and others added 7 commits May 15, 2026 03:23

Add built-in ReadOnlyMemory collection parser support

234ec0b

Agent-Logs-Url: https://github.com/AlgorithmsAreCool/LightProto/sessions/baab16b3-347e-4972-ba9b-bb4ef9467a61 Co-authored-by: AlgorithmsAreCool <10837869+AlgorithmsAreCool@users.noreply.github.com>

Optimize ReadOnlyMemory writer and improve collection null-check clarity

158a14d

Agent-Logs-Url: https://github.com/AlgorithmsAreCool/LightProto/sessions/baab16b3-347e-4972-ba9b-bb4ef9467a61 Co-authored-by: AlgorithmsAreCool <10837869+AlgorithmsAreCool@users.noreply.github.com>

feat: add Memory and ReadOnlySequence parser support

b1b70bc

Agent-Logs-Url: https://github.com/AlgorithmsAreCool/LightProto/sessions/81c76820-749c-4e93-a4eb-b0d9bb99a31d Co-authored-by: AlgorithmsAreCool <10837869+AlgorithmsAreCool@users.noreply.github.com>

feat: serialize ReadOnlySequence<byte> as bytes

62b89c7

Agent-Logs-Url: https://github.com/AlgorithmsAreCool/LightProto/sessions/367968f0-079c-44f8-b8ae-23d7cdad6437 Co-authored-by: AlgorithmsAreCool <10837869+AlgorithmsAreCool@users.noreply.github.com>

fix: treat MemoryProtoWriter as collection writer in runtime

2c07659

fix: implement ICollectionWriter for readonly memory/sequence writers

6d73d09

Copilot AI review requested due to automatic review settings June 14, 2026 19:14

Copilot AI reviewed Jun 14, 2026

View reviewed changes

dameng324 self-assigned this Jun 15, 2026

dameng324 requested changes Jun 15, 2026

View reviewed changes

Refactor memory parser internals

b2e35f7

AlgorithmsAreCool requested a review from Copilot June 19, 2026 01:59

Copilot started reviewing on behalf of AlgorithmsAreCool June 19, 2026 01:59 View session

Copilot AI reviewed Jun 19, 2026

View reviewed changes

AlgorithmsAreCool requested a review from dameng324 June 20, 2026 14:31

dameng324 requested changes Jun 22, 2026

View reviewed changes

		public ReadOnlySequenceProtoReader(IProtoReader<TItem> itemReader, uint tag, int itemFixedSize)
		: this(itemReader, itemFixedSize) { }

Conversation

AlgorithmsAreCool commented Jun 14, 2026

Uh oh!

codacy-production Bot commented Jun 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Up to standards ✅

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

dameng324 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlgorithmsAreCool commented Jun 15, 2026

Uh oh!

dameng324 commented Jun 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Jun 21, 2026

Codecov Report

Uh oh!

dameng324 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlgorithmsAreCool commented Jun 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codacy-production Bot commented Jun 14, 2026 •

edited

Loading