feat: implement combine operations (merge, concat, zip) for Phase 3

[andreasronge]

Summary

Implements the three combine operations needed to complete Phase 3 of the DSL:

• merge: Combines multiple objects with later values overriding earlier ones
• concat: Concatenates multiple lists into a single list
• zip: Zips multiple lists together, stopping at the shortest

Changes

Implementation

• Added eval/merge, eval/concat, and eval/zip to operations.ex
• Each operation properly evaluates sub-expressions and handles type errors
• Module documentation updated to list all Phase 3 operations

Validation

• Added validation rules for all three operations in validator.ex
• Validates that required fields exist and contain lists of valid expressions
• Follows existing patterns used for and/or operations

Tests

Added comprehensive test coverage including:

• Basic operation tests for all three operations
• Edge cases (empty lists/objects, single element, unequal lengths for zip)
• Variable/expression evaluation
• Type error handling
• Validation error handling
• E2E test demonstrating operations working together with let bindings

All 226 tests pass with no failures.

Test Plan

• Run mix test to execute full test suite
• All existing tests continue to pass
• New tests specifically cover the three new operations

Fixes #36

[github-actions]

PR Review: feat: implement combine operations (merge, concat, zip) for Phase 3

Summary

Well-implemented PR that adds three combine operations (merge, concat, zip) to complete Phase 3 of the DSL. The code follows established patterns from existing operations and includes comprehensive test coverage.

What's Good

1. Consistent patterns: The implementation follows the exact patterns from and/or operations for validating list-based inputs (lines 117-153 in validator.ex) and uses Interpreter.eval/2 for recursive sub-expression evaluation, matching let/if patterns.

2. Proper error handling: Each operation correctly validates input types at runtime and returns descriptive error messages (e.g., "merge requires map values, got [1, 2, 3]").

3. Complete validation: All three operations have proper validation rules checking for required fields and list types.

4. Comprehensive tests: 45 new tests covering:

   • Basic functionality for all three operations
   • Edge cases (empty inputs, single element, unequal lengths for zip)
   • Variable/expression evaluation
   • Type error handling
   • Validation error handling
   • E2E test combining operations with let bindings

5. Module doc updated: The module documentation in operations.ex:5-8 correctly lists all Phase 3 operations.

6. Clean implementation: The do_zip/1 helper correctly handles the "stop at shortest" semantics without unnecessary complexity.

Issues (Must Fix)

None. The implementation is complete and correct.

Suggestions (Optional)

1. OPTIONAL: Update architecture.md roadmap - docs/architecture.md:719
   • Observation: The Phase 3 roadmap line lists merge, concat but not zip
   • Note: The Combine Operations table (lines 177-179) correctly includes all three operations, so the spec is complete
   • Recommendation: Consider updating line 719 to include zip for consistency: - Combine: \merge`, `concat`, `zip``
   • Complexity: Trivial (doc-only change)
   • Recommendation: Could be deferred to a separate doc cleanup PR

Security

No concerns. The operations work only on in-memory data structures and use existing safe patterns for expression evaluation. No external I/O or unsafe operations are introduced.

Verdict

Approve. The implementation is complete, well-tested, and follows established patterns. The minor doc inconsistency in architecture.md is optional and doesn't affect functionality.

[andreasronge]

@claude please fix this issue from PR review:

Issue 1: Missing zip in Phase 3 roadmap documentation

Location: docs/architecture.md:719
Problem: The Phase 3 roadmap line lists only merge and concat but is missing zip, even though this PR implements all three combine operations. The operations table (lines 177-179) correctly includes all three.
Fix: Update line 719 from:

- Combine: `merge`, `concat`

to:

- Combine: `merge`, `concat`, `zip`

Please fix this in a single commit with minimal changes.

[andreasronge]

Auto-Triage Summary

Decisions Made

#	Issue	Decision	Action
1	Missing zip in Phase 3 roadmap (architecture.md:719)	FIX_NOW	Included in fix request above

Reasoning

Issue #1: Documentation inconsistency

• Complexity: Simple/Mechanical - trivial doc-only change, just adding one word
• Scope: IN-SCOPE - The PR implements merge, concat, AND zip. The roadmap should list all three operations. The PR already updated the operations table but missed the roadmap line, making this incomplete documentation work.
• Decision: FIX_NOW - This is mechanical, directly related to the PR's purpose, and completes the documentation that was partially updated in this PR.

Status

• FIX_NOW items: 1 (batched in single fix comment above)
• Issues created: 0
• Items ignored: 0

The review was positive with an "Approve" verdict. The single optional suggestion was assessed as in-scope incomplete documentation work and has been forwarded to @claude for fixing.

[andreasronge]

Fix Applied

Updated the Phase 3 roadmap in docs/architecture.md to include zip alongside merge and concat under the Combine operations section.

Changes Made:

• Line 719: Updated roadmap from Combine: merge, concat to Combine: merge, concat, zip

Verification:

• All precommit checks passed (formatting, compilation, credo, tests)
• Change is minimal and focused, affecting only the roadmap documentation
• Operations table (lines 177-179) was already correct with all three operations

Commit: e3c48fb - docs: add zip to Phase 3 roadmap in architecture.md

[github-actions]

PR Review: Implement combine operations (merge, concat, zip) for Phase 3

Summary

This PR adds the final Phase 3 operations (merge, concat, zip) for combining data structures. The implementation is clean, follows existing patterns well, and includes comprehensive test coverage. Ready for merge.

What's Good

• Consistent patterns: Implementation follows existing patterns from and/or operations using Enum.reduce_while and Interpreter.eval/2
• Comprehensive validation: All three operations validate required fields and recursive expressions
• Strong test coverage: 36 new tests covering happy paths, edge cases, error handling, and validation
• Good error messages: Type errors are clear and specific to each operation
• Documentation updates: Module doc and architecture.md Phase 3 roadmap updated to include zip
• Edge cases handled well: Empty inputs, single element, type mismatches, and unequal-length lists for zip

Issues (Must Fix)

None identified. The implementation is complete and follows the patterns established by existing operations.

Suggestions (Optional)

1. OPTIONAL: Performance consideration for concat - operations.ex:680

   • Observation: Uses acc ++ list which is O(n) per iteration
   • Benefit: Could use [list | acc] then Enum.reverse/1 and Enum.concat/1 for O(n) total
   • Complexity: Moderate refactor
   • Recommendation: Not needed for typical DSL usage; only relevant for very large list counts. Defer if desired.

2. OPTIONAL: Consider using Enum.zip_with/2 for zip - operations.ex:720-738

   • Observation: Custom do_zip implementation is correct but Elixir has built-in Enum.zip_with/2
   • Benefit: Simpler code, well-tested standard library
   • Complexity: Low - would be Enum.zip_with(evaluated_lists, & &1)
   • Recommendation: Current implementation is correct and readable. Nice-to-have cleanup.

Security

No concerns. The operations:

• Only work on data already evaluated through the interpreter
• Don't introduce new external access patterns
• Type check inputs before processing
• Error propagation follows existing safe patterns

Verdict

Approve - Well-implemented feature completing Phase 3. All tests pass, code follows established patterns, and documentation is updated. The implementation is solid and ready for merge.

[andreasronge]

Auto-Triage Summary (Cycle 2)

Decisions Made

#	Issue	Decision	Action
1	concat performance optimization (operations.ex:680)	DEFER_ISSUE	Created issue #38
2	Use Enum.zip_with/2 for zip operation (operations.ex:720-738)	DEFER_ISSUE	Created issue #39

Reasoning

Issue #1: concat performance optimization

• Complexity: Moderate refactor - requires changing reduce pattern and post-processing
• Scope: OUT OF SCOPE - PR implements working combine operations; performance optimization is not part of stated purpose
• From review: Explicitly noted as "Not needed for typical DSL usage" and "only relevant for very large list counts"
• Per PR guidelines: Performance optimizations (unless critical) should be deferred
• Decision: DEFER_ISSUE - Valid optimization but non-critical and out of scope

Issue #2: Use Enum.zip_with/2 for zip

• Complexity: Low - simple stdlib replacement
• Scope: OUT OF SCOPE - PR correctly implements zip functionality; this is a refactoring suggestion
• From review: Explicitly called "nice-to-have cleanup" and notes current implementation is "correct and readable"
• Decision: DEFER_ISSUE - Valid improvement for maintainability but not required for this PR

Status

• FIX_NOW items: 0
• Issues created: 2 ([From PR #37] Optimize concat operation performance for many lists #38, [From PR #37] Consider using Enum.zip_with/2 for zip operation #39)
• Items ignored: 0

Next Steps

Both suggestions are valid improvements but out of scope for this PR. The PR correctly implements all three combine operations with proper tests and documentation. Since there are no FIX_NOW items remaining, adding ready-to-merge label.