fix: Resolve remaining Clojure conformance test failures

[andreasronge]

Summary

Fix multiple issues causing 15 Clojure conformance tests to fail after the initial Clojure validation implementation.

Changes

ClojureValidator EDN parsing (fixed 12 test failures)

• Add support for parsing Clojure lists (1 2 3) in parse_simple_edn
• Use read-string to safely parse EDN output before JSON conversion in parse_edn_output
• This prevents lists like (2 4) from being interpreted as function calls when converting to JSON

Variadic multiplication (* 5) (fixed 1 failure)

• Fix single-argument handler in apply_fun to return the value for all variadic ops
• Previously only - (negation) was handled correctly, others returned {:error, {:not_callable, ...}}

Variadic division (/ 100 5 2) (fixed 1 failure)

• Change division from :normal (binary only) to :variadic_nonempty in env.ex
• Now correctly reduces left-to-right: (/ 100 5 2) = 100 / 5 / 2 = 10

2-arity reduce (reduce + [1 2 3]) (fixed 1 failure)

• Add reduce/2 in runtime.ex that uses first element as initial value
• Change binding to :multi_arity to support both 2 and 3 argument forms

Test plan

All 63 Clojure conformance tests now pass:

mix test --only clojure
# 63 tests, 0 failures

Full test suite also passes:

mix test --exclude clojure --exclude e2e
# 1126 tests, 0 failures

Closes #245

🤖 Generated with Claude Code

[github-actions]

PR Review: fix: Resolve remaining Clojure conformance test failures

Summary

This PR adds comprehensive Babashka-based Clojure validation infrastructure and fixes four Clojure conformance issues (EDN parsing, variadic multiplication, variadic division, and 2-arity reduce). The implementation is well-structured with proper test infrastructure for ongoing Clojure compatibility verification.

What's Good

• Well-organized implementation with clear separation of concerns (ClojureValidator module, test helpers, mix task)
• Shared test helper module (ClojureTestHelpers) eliminates duplication across test files
• Proper use of @describetag :clojure and require_babashka() setup for graceful test skipping when Babashka is unavailable
• Documentation updates in docs/guide.md explaining Clojure validation workflow
• Clean implementation of fixes in env.ex, eval.ex, and runtime.ex
• The mix ptc.install_babashka task handles cross-platform installation cleanly

Issues (Must Fix)

None identified. The code changes are well-implemented and follow existing patterns.

Suggestions (Optional)

1. File Size - test files exceed 800 line threshold - Nice-to-have

   • test/ptc_runner/lisp/integration_test.exs: 920 lines (+131 in this PR)
   • test/ptc_runner/lisp/runtime_test.exs: 1059 lines (+268 in this PR)
   • Suggestion: Consider splitting Clojure conformance tests into separate files in a future PR (e.g., test/ptc_runner/lisp/clojure_conformance_test.exs for runtime conformance and test/ptc_runner/lisp/clojure_syntax_test.exs for syntax validation)
   • Benefit: Better organization and parallel test execution
   • Complexity: Low - mechanical extraction with no behavior changes
   • Recommendation: Create GitHub issue for future work

2. Demo report files - Out of scope consideration

   • The PR adds several demo report files (demo/deepseek_json_report2.md, demo/deepseek_report2.md, etc.)
   • These appear to be LLM test run outputs rather than code artifacts
   • Suggestion: Consider whether these should be gitignored or stored elsewhere to avoid bloating the repository
   • Recommendation: Create GitHub issue if this is a concern

Security

No concerns. The Babashka installer downloads from official GitHub releases and the validator executes user-provided Lisp programs in a subprocess, which is appropriate for test-time validation.

Documentation

Appropriate updates made:

• docs/guide.md updated with Clojure validation section explaining mix ptc.install_babashka and mix test --only clojure
• Module documentation in ClojureValidator and ClojureTestHelpers is comprehensive

Verdict

Approve - The implementation is clean, well-tested, and follows project conventions. The file size concerns are minor and can be addressed in a follow-up PR.

[andreasronge]

Auto-Triage Summary

Decisions Made

#	Issue	Decision	Action
1	File size - test files exceed 800 line threshold	DEFER_ISSUE	Created issue #251
2	Demo report files gitignore consideration	IGNORE	Speculative concern; demo reports provide valuable documentation of test results

Rationale

Issue #1 - File size concern (DEFER_ISSUE)

• This follows an established pattern in this codebase (previous issues: [From PR #172] Split eval_test.exs into multiple files #173, Refactor: Split test/ptc_runner_test.exs into logical smaller files #95, [From PR #243] Split large transformation_test.exs into smaller test files #244, [From PR #237] Split operations.ex into smaller modules by operation category #238)
• The suggestion is mechanical (low complexity) and valuable for test organization
• Out of scope for this PR, but worth tracking for future work
• Created [From PR #250] Split large Clojure test files for better organization #251 to track splitting Clojure conformance tests into dedicated files

Issue #2 - Demo reports (IGNORE)

• The review frames this as "consider whether" (speculative)
• Demo reports are intentionally checked in and provide documentation
• No concrete problem to fix
• The demo/ directory structure appears intentional

Status

• FIX_NOW items: 0
• Issues created: 1 ([From PR #250] Split large Clojure test files for better organization #251)
• Items ignored: 1
• Label added: ready-to-merge

The PR has been approved with no blocking issues. All suggestions have been appropriately triaged.

[github-actions]

PR Review: fix: Resolve remaining Clojure conformance test failures

Summary

This PR fixes 15 remaining Clojure conformance test failures by addressing four issues: EDN parsing for lists in ClojureValidator, variadic multiplication with single argument, variadic division left-to-right reduction, and 2-arity reduce support. The implementation is clean and well-tested.

What's Good

• Well-organized fixes: Each issue is addressed with minimal, targeted changes
• Proper separation in env.ex: The change from :normal to :variadic_nonempty for division and :multi_arity for reduce follows established patterns
• Good test coverage: New Clojure conformance tests added with proper @describetag :clojure tagging
• Shared test helper module: ClojureTestHelpers properly centralized in test/support/ following project conventions
• CI correctly configured: Tests excluded with --exclude clojure since Babashka is not installed in CI
• Correct fix for variadic ops: The apply_fun change in eval.ex correctly handles single-argument calls for all variadic operators (not just -)

Issues (Must Fix)

None identified. The code changes are well-implemented and follow existing patterns.

Suggestions (Optional)

1. File Size - test files exceed 800 line threshold - Nice-to-have

   • test/ptc_runner/lisp/integration_test.exs: 896 lines (+113 in this PR)
   • test/ptc_runner/lisp/runtime_test.exs: 1027 lines (+244 in this PR)
   • Suggestion: Consider splitting Clojure conformance tests into separate files in a future PR (e.g., test/ptc_runner/lisp/clojure_conformance_test.exs)
   • Benefit: Better organization and parallel test execution
   • Complexity: Low - mechanical extraction with no behavior changes
   • Note: Issue [From PR #250] Split large Clojure test files for better organization #251 already exists for this

2. Demo report files - Out of scope consideration

   • The PR adds 8 demo report markdown files to demo/
   • These appear to be LLM test run outputs documenting test results
   • Suggestion: If these are frequently regenerated, consider adding them to .gitignore to avoid bloating the repository
   • Recommendation: Low priority, can be ignored if they serve documentation purposes

Security

No concerns. The Babashka validator executes Clojure code in a subprocess with a timeout, which is appropriate for test-time validation. The installer downloads from official GitHub releases.

Documentation

• CI workflow correctly updated to exclude Clojure tests (Babashka not available in CI)
• Test helpers are well-documented with @moduledoc and usage examples
• The ClojureValidator module has comprehensive documentation

Verdict

Approve - The implementation is clean, follows project conventions, and correctly fixes the Clojure conformance test failures. All tests pass. The file size concern is tracked in issue #251 and is not blocking.

[andreasronge]

Auto-Triage Summary (Cycle 2)

Review Analysis

Analyzed two review comments from github-actions bot. Both reviews provide APPROVE verdicts with only optional suggestions.

Decisions Made

#	Issue	Decision	Action
1	File size - test files exceed 800 line threshold	ALREADY_TRACKED	Issue #251 exists
2	Demo report files gitignore consideration	IGNORE	Low priority, speculative concern

Rationale

Issue #1 - File size (ALREADY_TRACKED)

• Previous triage correctly created issue [From PR #250] Split large Clojure test files for better organization #251
• Both review comments acknowledge this is tracked
• No additional action needed

Issue #2 - Demo reports (IGNORE)

• Framed as "consider whether" (speculative)
• Previous triage correctly determined these provide documentation value
• No concrete problem identified
• Consistent with first triage decision

Status

• FIX_NOW items: 0
• New issues created: 0 (existing issue [From PR #250] Split large Clojure test files for better organization #251 covers file size concern)
• Items ignored: 1
• PR remains ready-to-merge ✓

Conclusion: The previous triage handled all review suggestions appropriately. No additional fixes or issues needed. The PR is ready for merge once all checks pass.