[From PR #160] Extend multi-arity support to get and get-in

[andreasronge]

Context

From PR #160 review: The {:multi_arity, tuple()} pattern was successfully implemented for sort-by to support both 2-arity and 3-arity forms.

Problem

Runtime.get/3 and Runtime.get_in/3 exist (with default values) but are only registered as 2-arity in env.ex:

• Line 72: {:get, {:normal, &Runtime.get/2}}
• Line 73: {:"get-in", {:normal, &Runtime.get_in/2}}

This means users cannot provide default values:

;; Currently not supported (should be):
(get m :key :default)
(get-in m [:a :b] :default)

Proposed Solution

Use the same {:multi_arity, tuple()} pattern to support optional default values:

{:get, {:multi_arity, {&Runtime.get/2, &Runtime.get/3}}},
{:"get-in", {:multi_arity, {&Runtime.get_in/2, &Runtime.get_in/3}}},

Benefits

• Consistent API - same pattern as sort-by
• More Clojure-like: (get m :key :default) and (get-in m [:a :b] :default)
• Useful for providing fallback values when keys don't exist

Implementation Steps

1. Update lib/ptc_runner/lisp/env.ex (lines 72-73):

   • Change {:get, {:normal, &Runtime.get/2}} to {:get, {:multi_arity, {&Runtime.get/2, &Runtime.get/3}}}
   • Change {:"get-in", {:normal, &Runtime.get_in/2}} to {:"get-in", {:multi_arity, {&Runtime.get_in/2, &Runtime.get_in/3}}}

2. Add unit tests in test/ptc_runner/lisp/eval_test.exs:

   • Test get with 2 args (existing behavior, regression test)
   • Test get with 3 args (key exists → returns value)
   • Test get with 3 args (key missing → returns default)
   • Test get-in with 2 args (existing behavior, regression test)
   • Test get-in with 3 args (path exists → returns value)
   • Test get-in with 3 args (path missing → returns default)
   • Test arity error with wrong argument count (1 arg or 4+ args)

3. Documentation: No changes required - architecture.md doesn't enumerate all builtins

Test Plan

Add tests to the existing describe "multi-arity builtins" block in eval_test.exs:

test "get with 2 arguments returns value or nil" do
  env = Env.initial()
  data = %{name: "Alice", age: 30}

  # Key exists
  call_ast = {:call, {:var, :get}, [{:var, :data}, {:keyword, :name}]}
  assert {:ok, "Alice", %{}} = Eval.eval(call_ast, %{}, %{}, Map.merge(env, %{data: data}), &dummy_tool/2)

  # Key missing
  call_ast = {:call, {:var, :get}, [{:var, :data}, {:keyword, :missing}]}
  assert {:ok, nil, %{}} = Eval.eval(call_ast, %{}, %{}, Map.merge(env, %{data: data}), &dummy_tool/2)
end

test "get with 3 arguments uses default for missing key" do
  env = Env.initial()
  data = %{name: "Alice"}

  # Key exists - returns value, not default
  call_ast = {:call, {:var, :get}, [{:var, :data}, {:keyword, :name}, {:string, "default"}]}
  assert {:ok, "Alice", %{}} = Eval.eval(call_ast, %{}, %{}, Map.merge(env, %{data: data}), &dummy_tool/2)

  # Key missing - returns default
  call_ast = {:call, {:var, :get}, [{:var, :data}, {:keyword, :missing}, {:string, "not found"}]}
  assert {:ok, "not found", %{}} = Eval.eval(call_ast, %{}, %{}, Map.merge(env, %{data: data}), &dummy_tool/2)
end

test "get-in with 3 arguments uses default for missing path" do
  env = Env.initial()
  data = %{user: %{name: "Alice"}}

  # Path exists
  call_ast = {:call, {:var, :"get-in"}, [{:var, :data}, {:vector, [{:keyword, :user}, {:keyword, :name}]}, {:string, "default"}]}
  assert {:ok, "Alice", %{}} = Eval.eval(call_ast, %{}, %{}, Map.merge(env, %{data: data}), &dummy_tool/2)

  # Path missing
  call_ast = {:call, {:var, :"get-in"}, [{:var, :data}, {:vector, [{:keyword, :user}, {:keyword, :age}]}, 0]}
  assert {:ok, 0, %{}} = Eval.eval(call_ast, %{}, %{}, Map.merge(env, %{data: data}), &dummy_tool/2)
end

Edge Cases

• (get nil :key :default) → should return :default (existing get/3 handles nil maps)
• (get-in {:a nil} [:a] :default) → should return nil (value exists), not :default
  • Note: Current get_in/3 implementation returns default if value is nil. This may need fixing to match Clojure semantics.

Related

• PR fix: Enable sort-by with comparator and builtin HOF arguments #160 introduces {:multi_arity, tuple()} pattern for sort-by

[andreasronge]

Review Complete

This issue has been reviewed and is ready for implementation.

Verification:

• ✅ Runtime.get/3 exists at runtime.ex:127-128
• ✅ Runtime.get_in/3 exists at runtime.ex:132-135
• ✅ multi_arity pattern is tested with sort-by (eval_test.exs:758-804)
• ✅ All 964 tests passing

Implementation Notes:

• Update env.ex lines 72-73 to use {:multi_arity, ...} pattern
• Follow the test pattern from sort-by multi-arity tests
• Include error case test for wrong argument count

@claude Please implement this issue

[andreasronge]

Issue Review: Extend multi-arity support to get and get-in

Summary: Straightforward enhancement to expose existing Runtime.get/3 and Runtime.get_in/3 functions via the {:multi_arity, tuple()} pattern introduced in PR #160.

Pre-Review Checks

• Codebase health: ✅ All 964 tests passing
• Higher-priority blockers: None - this is from PR review (from-pr-review label), which is priority 2
• Issue claims verified: ✅
  • Confirmed Runtime.get/3 and Runtime.get_in/3 exist (lines 127-137)
  • Confirmed env.ex only registers 2-arity versions (lines 72-73)
  • Confirmed {:multi_arity, tuple()} pattern works correctly in eval.ex

Analysis

1. Should This Be Done?: ✅ Yes - simple enhancement that completes the multi-arity support introduced in PR fix: Enable sort-by with comparator and builtin HOF arguments #160. Low risk, high utility for LLM-generated programs.

2. Simplifications: Already minimal - just changing two lines in env.ex plus tests.

3. Codebase Research: ✅ All referenced code verified to exist exactly as described.

4. Reuse Opportunities: ✅ Reuses existing {:multi_arity, tuple()} infrastructure from PR fix: Enable sort-by with comparator and builtin HOF arguments #160. Same test pattern as sort-by tests.

5. Refactoring Needs: None required.

6. Edge Cases: Added to issue body:

   • (get nil :key :default) - handled by existing get/3 clause
   • Potential issue: Current get_in/3 returns default when value is nil, not just when path is missing. This differs from Clojure semantics. Noted in issue for implementer awareness.

7. Issue Scope: ✅ Appropriately sized - 2-line change + tests, single PR.

8. Test Plan: ✅ Enhanced issue with specific test cases including regression tests for 2-arity behavior.

9. Solution Outline: ✅ Clear implementation steps with exact line numbers and code examples.

Changes Made to Issue Body

• Added "Problem" section to clarify what users cannot do currently
• Added concrete implementation steps with line numbers
• Added complete test plan with example test code
• Added edge cases section noting the get_in/3 nil-value semantics issue
• Added note about documentation (no changes needed)

Technical Issue Found

⚠️ Minor semantic concern: The current get_in/3 implementation (lines 132-137) returns the default value if the path value is nil:

def get_in(m, path, default) when is_map(m) do
  case Kernel.get_in(m, path) do
    nil -> default  # This triggers even if the value IS nil
    val -> val
  end
end

In Clojure, (get-in {:a nil} [:a] :default) returns nil (the value exists), not :default. This is a pre-existing issue but worth noting for the implementer to decide whether to fix it as part of this work or separately.

Recommendation

Ready for implementation with claude-approved label.

The enhancement is well-defined, low risk, and follows established patterns. The get_in/3 nil semantics issue is noted but should not block this work - it can be addressed in a follow-up if desired.

[andreasronge]

@claude Please implement this issue.

Implementation Notes:

• This is a straightforward extension following the {:multi_arity, tuple()} pattern from PR fix: Enable sort-by with comparator and builtin HOF arguments #160
• Update lib/ptc_runner/lisp/env.ex lines 72-73 to use multi_arity
• Add tests to the existing "multi-arity builtins" describe block in eval_test.exs
• Pay attention to the edge case noted in the issue regarding get-in semantics with nil values

[andreasronge]

Implementation Complete

I've successfully implemented issue #161. Here's what was done:

Changes Made

lib/ptc_runner/lisp/env.ex (lines 72-73):

• Updated get to use {:multi_arity, {&Runtime.get/2, &Runtime.get/3}}
• Updated get-in to use {:multi_arity, {&Runtime.get_in/2, &Runtime.get_in/3}}

test/ptc_runner/lisp/eval_test.exs:
Added 6 comprehensive tests in the "multi-arity builtins" describe block:

• get with 2 arguments (map, key) returns value or nil
• get with 3 arguments (map, key, default) returns value or default
• get arity error with wrong argument count
• get-in with 2 arguments (map, path) returns nested value
• get-in with 3 arguments (map, path, default) returns default when path not found
• get-in arity error with wrong argument count

Verification

• ✅ All 970 tests passing
• ✅ Format check passed
• ✅ Compilation with warnings-as-errors passed
• ✅ Credo linting passed
• ✅ Pre-commit hooks passed

Pull Request: #162

[andreasronge]

Implementation Complete

Successfully implemented issue #161. Here's what was done:

Changes Made

lib/ptc_runner/lisp/env.ex (lines 72-73):

• Updated get to use {:multi_arity, {&Runtime.get/2, &Runtime.get/3}}
• Updated get-in to use {:multi_arity, {&Runtime.get_in/2, &Runtime.get_in/3}}

test/ptc_runner/lisp/eval_test.exs:
Added 6 comprehensive tests in the "multi-arity builtins" describe block:

• get with 2 arguments (map, key) returns value or nil
• get with 3 arguments (map, key, default) returns value or default
• get arity error with wrong argument count
• get-in with 2 arguments (map, path) returns nested value
• get-in with 3 arguments (map, path, default) returns default when path not found
• get-in arity error with wrong argument count

Verification

✅ All 970 tests passing
✅ Format check passed
✅ Compilation with warnings-as-errors passed
✅ Credo linting passed
✅ Pre-commit hooks passed

Pull Request: #163