Reviewing files that changed from the base of the PR and between c80b2bb and 1fcdbb0.

• protographic/src/sdl-to-proto-visitor.ts (1 hunks)
• protographic/tests/sdl-to-proto/01-basic-types.test.ts (4 hunks)
• protographic/tests/sdl-to-proto/02-complex-types.test.ts (3 hunks)
• protographic/tests/sdl-to-proto/05-edge-cases.test.ts (1 hunks)
• protographic/tests/sdl-to-proto/06-field-ordering.test.ts (6 hunks)
• protographic/tests/sdl-to-proto/11-lists.test.ts (16 hunks)

• GitHub Check: Analyze (go)

protographic/src/sdl-to-proto-visitor.ts (1)

596-612: LGTM! Excellent refactoring to standardize wrapper message structure.

This refactoring simplifies and unifies the wrapper message generation logic by always creating a consistent nested structure with an inner List message. The benefits include:

• Eliminates conditional logic complexity for different nesting levels
• Provides a standardized, predictable message structure
• Improves code maintainability and readability

The new uniform structure where all wrapper messages contain a nested List message with repeated items and an outer list field is much cleaner than the previous conditional approach.

protographic/tests/sdl-to-proto/01-basic-types.test.ts (1)

145-148: LGTM! Test snapshots correctly updated for new wrapper structure.

The test snapshots have been properly updated to reflect the refactored wrapper message structure. All ListOf* messages now consistently use the nested List message pattern with:

• Inner List message containing repeated items
• Outer wrapper message containing list field

This provides comprehensive test coverage validating that the implementation change produces the expected protobuf output format across various data types (String, User, Float, Int, Point, TreeNode, etc.).

Also applies to: 252-255, 485-488, 492-495, 520-523, 527-530, 715-718, 722-725

protographic/tests/sdl-to-proto/05-edge-cases.test.ts (1)

369-372: LGTM! Edge case test snapshots properly updated.

The edge case test snapshots have been consistently updated to use the new nested List message structure for ListOfComment, ListOfPost, and ListOfString wrapper messages. This ensures the refactoring works correctly across complex scenarios including:

• Federation entity types with @key directives
• Union and interface types
• Complex schema structures with various GraphQL constructs

The consistent application of the nested structure pattern across all test scenarios validates the comprehensive nature of the implementation change.

Also applies to: 376-379, 383-386

protographic/tests/sdl-to-proto/11-lists.test.ts (16)

69-107: LGTM! Consistent nested list wrapper structure implemented.

The test snapshots correctly reflect the new standardized list wrapper message structure. The nested List message pattern provides consistent nullability handling across all list types.

---

150-162: LGTM! Proper nested structure for basic list wrappers.

The ListOfString and ListOfUser wrapper messages now consistently use the nested List message pattern, which standardizes nullability handling.

---

220-231: LGTM! Nested structure maintained for complex list scenarios.

The deeply nested list scenarios correctly implement the standardized wrapper message structure with the inner List message containing the repeated items.

---

292-303: LGTM! Consistent pattern applied to all list wrapper types.

The changes maintain consistency across different list wrapper scenarios, ensuring uniform nullability handling regardless of the contained type (String, User, etc.).

---

365-369: LGTM! Enum list wrappers follow the same pattern.

The ListOfStatus wrapper correctly implements the nested structure, demonstrating that the standardization applies to all types including enums.

---

444-448: LGTM! Interface list wrappers maintain consistency.

The ListOfNode wrapper for interface types follows the same nested structure pattern, ensuring uniform behavior across different GraphQL type categories.

---

530-534: LGTM! Union list wrappers use standardized structure.

The ListOfSearchResult wrapper for union types correctly implements the nested List message pattern, maintaining consistency across all GraphQL type kinds.

---

608-648: LGTM! Scalar and custom scalar list wrappers are consistent.

All scalar type list wrappers (ListOfDateTime, ListOfFloat, ListOfInt, ListOfJSON, ListOfString) properly implement the nested structure, ensuring uniform nullability handling for all scalar types.

---

756-768: LGTM! Deep nesting maintains the standardized pattern.

Even with deeply nested list structures, the wrapper messages consistently use the nested List message pattern, proving the robustness of the new approach.

---

850-862: LGTM! Input type list wrappers follow the pattern.

The ListOfString and ListOfUpdateUserInput wrappers for input types correctly implement the nested structure, ensuring consistency across output and input type scenarios.

---

1001-1076: LGTM! Complex mixed scenarios maintain consistency.

The comprehensive test with mixed types (enums, interfaces, unions, nested lists) shows that all wrapper messages consistently use the nested List structure, demonstrating the thoroughness of the refactoring.

---

1204-1216: LGTM! Edge cases properly handle the new structure.

The edge case scenarios with various nullable combinations correctly implement the nested wrapper pattern, ensuring robust nullability handling in all scenarios.

---

1304-1343: LGTM! Recursive types work correctly with nested wrappers.

The recursive type scenarios (ListOfCategory, ListOfComment, ListOfUser) properly implement the nested structure, showing that the pattern works well with self-referencing types.

---

1473-1533: LGTM! Complex input scenarios maintain standardization.

All input type list wrappers in complex scenarios consistently use the nested List message structure, ensuring uniform behavior across simple and complex input type scenarios.

---

1690-1977: LGTM! Recursive input types handle nesting correctly.

The recursive input type scenarios properly implement the nested wrapper structure, demonstrating that the pattern works effectively with complex recursive input types and deep nesting.

---

1944-1977: LGTM! Most complex scenarios maintain consistency.

The most complex mixed recursive and non-recursive scenarios with deep nesting all use the standardized nested List message pattern, proving the robustness and consistency of the refactoring approach.

protographic/tests/sdl-to-proto/02-complex-types.test.ts (3)

40-45: LGTM! Enum-related list wrapper uses standardized structure.

The ListOfUser wrapper message correctly implements the nested List message pattern, ensuring consistent nullability handling for enum-related scenarios.

---

235-240: LGTM! Circular reference list wrapper maintains consistency.

The ListOfTreeNode wrapper for circular/recursive types properly implements the nested structure, demonstrating that the standardization works well with self-referencing types.

---

311-323: LGTM! Input type list wrappers follow the standardized pattern.

Both ListOfAddressInput and ListOfUser wrapper messages correctly implement the nested List message structure, ensuring consistent nullability handling for input type scenarios.

protographic/tests/sdl-to-proto/06-field-ordering.test.ts (3)

1157-1189: LGTM! Correctly implements nested List structure verification.

The test now properly verifies both the outer wrapper message (with list field) and the nested List message (with items field), aligning with the refactored structure where all list wrappers contain a nested List message.

---

1228-1256: LGTM! Consistent field preservation verification.

The field number preservation logic correctly verifies both existing wrapper types maintain their field numbers and new wrapper types receive the expected field numbers. The implementation is consistent with the nested structure pattern.

---

1286-1514: LGTM! Comprehensive nested and mixed wrapper type verification.

This segment excellently handles complex scenarios including:

• Nested wrapper types (ListOfListOfString, ListOfListOfInt, etc.)
• Mixed simple and nested wrapper combinations
• Field number preservation across schema modifications
• Proper verification of wrapper type removal

The implementation consistently follows the nested structure pattern and provides thorough test coverage for the refactored list wrapper message generation logic.

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch ludwig/eng-7679-protographic-use-nested-list-element-for-single-list