⏺ Done. Bumped the variant field limit from 4 to 12. All tests pass.

[navicore]

Changes made:

1. Runtime (crates/runtime/src/variant_ops.rs):
   - Added patch_seq_make_variant_5 through patch_seq_make_variant_12
   - Added re-exports for internal use
2. Runtime exports (crates/runtime/src/lib.rs):
   - Added exports for new make_variant functions
3. Builtin signatures (crates/compiler/src/builtins.rs):
   - Added signatures for variant.make-5 through variant.make-12 (using a loop since the macro only supports up to 5 inputs)
   - Added signatures for wrap-5 through wrap-12
   - Added docs for all new builtins
4. Codegen (crates/compiler/src/codegen/runtime.rs):
   - Added LLVM declarations for new runtime functions
   - Added builtin-to-runtime mappings
5. AST (crates/compiler/src/ast.rs):
   - Changed MAX_VARIANT_FIELDS from 4 to 12
6. Documentation:
   - Updated language-guide.md (comparison table, error example)
   - Updated STDLIB_REFERENCE.md (variant constructors table)
   - Updated ARCHITECTURE.md (comment about field count)

[claude]

Pull Request Review: Bump variant field limit from 4 to 12

Summary

This PR systematically extends the maximum variant field count from 4 to 12 by adding new runtime functions, compiler builtins, and LLVM declarations. The implementation follows the established patterns and maintains consistency across the codebase.

✅ Strengths

1. Comprehensive Coverage

• All layers properly updated: runtime, compiler builtins, codegen, and documentation
• Consistent naming conventions maintained across all new functions
• Proper synchronization between variant.make-N and wrap-N aliases

2. Code Quality

• Functions follow the exact same pattern as existing make_variant_0-4 implementations
• Proper safety annotations and documentation for all unsafe functions
• Clear stack effect documentation in comments
• Error messages are descriptive and consistent

3. Documentation

• Updated all relevant documentation files (ARCHITECTURE.md, STDLIB_REFERENCE.md, language-guide.md)
• Error message example updated to reflect new limit
• Documentation clearly indicates the max field count

4. Type Safety

• Proper type signature generation using loops for builtins 5-12
• Maintains type-level guarantees for stack effects

🔍 Observations & Suggestions

1. Code Duplication (Minor)

The new variant functions (5-12) follow an extremely repetitive pattern with only the field count changing. While this matches the existing style for functions 0-4, consider whether a macro could reduce duplication in the future.

Note: This is purely an observation for future consideration. The current implementation is correct and consistent with the established codebase patterns.

2. Test Coverage (Important)

I do not see integration tests that exercise variants with 5-12 fields. The existing test file tests/integration/src/test-variant-typed.seq only tests up to 4 fields.

Recommendation: Add test cases for the new field counts to ensure they work correctly in practice. At minimum, test one mid-range variant (e.g., 8 fields) and the maximum (12 fields).

3. Performance Considerations (Minor)

Each variant function allocates a new Vec with all fields. For variants with 12 fields, this involves 12 stack pops, creating a Vec with 12 elements, and Arc allocation for VariantData.

This is consistent with the existing implementation and likely acceptable for the use case. However, if variant creation becomes a bottleneck, consider pre-allocating the Vec with capacity.

4. Error Handling (Good)

The panic messages are clear and consistent, matching the existing pattern and will help users debug issues.

🔒 Security Considerations

• No security concerns identified
• All functions properly validate input types before operations
• Panic on type mismatch prevents undefined behavior
• No unsafe memory operations beyond the documented unsafe FFI boundary

🐛 Potential Bugs

No bugs identified. The implementation correctly:

• Pops fields in the right order
• Creates vectors with fields in correct positions
• Maintains Arc reference counting properly
• Exports all new functions in lib.rs

📊 Overall Assessment

Recommendation: ✅ APPROVE with suggestion to add tests

This is a clean, well-executed PR that maintains code quality and consistency. The only substantive improvement would be adding integration tests for the new variant sizes to ensure they work correctly in practice.

The implementation:

• ✅ Follows established patterns
• ✅ Maintains type safety
• ✅ Updates all necessary components
• ✅ Includes documentation updates
• ✅ Has clear commit message stating "All tests pass"
• ⚠️ Could benefit from explicit tests for new field counts

Great work maintaining consistency across the entire codebase!

---

Generated by Claude Code - PR review for #294