Add type coercion to where clause comparisons (keyword/string)

[andreasronge]

Summary

Add automatic type coercion to where clause comparisons so that keywords (atoms) and strings are compared by value, enabling LLM-generated code like (where :department = :engineering) to match string data values like "engineering".

Context

Architecture reference: PTC-Lisp Specification - Section 7.1 where
Dependencies: None
Related issues: None

Current State

Verified: The comparison helpers in lib/ptc_runner/lisp/eval.ex (lines 545-570) use strict equality without type coercion:

defp safe_eq(a, b), do: a == b  # No type coercion

When filtering data with string values using keyword literals:

(where :department = :engineering)  ; Returns [] when data has "engineering"
(where :department = "engineering") ; Works correctly

The field accessor already supports flexible key access via flex_get_in in lib/ptc_runner/lisp/runtime.ex, but value comparison does not.

This causes LLM-generated programs to fail silently when they use keywords instead of strings for comparison values - the filter returns empty results instead of matching data.

Acceptance Criteria

• (where :field = :keyword) matches string values (e.g., :engineering matches "engineering")
• (where :field = "string") matches keyword values (e.g., "engineering" matches :engineering)
• in operator handles mixed keyword/string membership (e.g., (where :status in [:active :pending]) matches "active")
• includes operator handles keyword/string comparison for list membership
• Ordering operators (>, <, >=, <=) do NOT coerce types - comparing atom to string returns false (consistent with nil handling)
• Exact type matches still work (no performance regression for matching types)
• Boolean atoms (true, false) are NOT coerced to strings "true", "false"
• Existing tests pass
• E2E test demonstrates keyword-to-string matching in filter pipeline

Implementation Hints

Files to modify:

• lib/ptc_runner/lisp/eval.ex - Add type coercion clauses to safe_eq, safe_in, safe_includes
• test/ptc_runner/lisp/flex_access_test.exs - Add tests for keyword/string value coercion

Patterns to follow:

• The existing flex_get pattern in lib/ptc_runner/lisp/runtime.ex (lines 19-39) shows how to handle atom/string conversion

Implementation approach:

# In safe_eq - add before catch-all clause:
defp safe_eq(a, b) when is_atom(a) and not is_boolean(a) and is_binary(b), do: to_string(a) == b
defp safe_eq(a, b) when is_binary(a) and is_atom(b) and not is_boolean(b), do: a == to_string(b)
defp safe_eq(a, b), do: a == b

# In safe_in - normalize values before membership check:
defp safe_in(value, coll) when is_list(coll) do
  normalized = normalize_for_comparison(value)
  Enum.any?(coll, fn item -> normalize_for_comparison(item) == normalized end)
end

defp normalize_for_comparison(v) when is_atom(v) and not is_boolean(v), do: to_string(v)
defp normalize_for_comparison(v), do: v

# In safe_includes - for list membership, use the same normalize_for_comparison approach
defp safe_includes(coll, value) when is_list(coll) do
  normalized = normalize_for_comparison(value)
  Enum.any?(coll, fn item -> normalize_for_comparison(item) == normalized end)
end

Design decision - Ordering operators:
Ordering operators (>, <, >=, <=) should NOT coerce atom/string types. Comparing :active > "active" would be semantically meaningless. These should return false (matching nil behavior) rather than attempting string comparison. No changes needed to safe_cmp.

Edge cases to consider:

• nil should NOT be coerced (already handled by existing nil guards)
• Boolean atoms (true, false) should NOT be coerced to strings - use is_boolean/1 guard
• Numbers should NOT be affected by this change
• Empty atom :"" coerces to empty string "" (correct behavior)

Test Plan

Unit tests:

• safe_eq(:keyword, "string") returns true
• safe_eq("string", :keyword) returns true
• safe_eq(:keyword, :keyword) returns true (exact match still works)
• safe_eq(true, "true") returns false (booleans not coerced)
• safe_eq(false, "false") returns false (booleans not coerced)
• safe_in(:active, ["active", "pending"]) returns true
• safe_in("active", [:active, :pending]) returns true
• safe_includes([:active, :pending], "active") returns true

E2E test:

test "where clause coerces keywords to strings" do
  program = ~S|(->> ctx/items (filter (where :status = :active)))|
  context = %{"items" => [
    %{"status" => "active", "name" => "A"},
    %{"status" => "inactive", "name" => "B"}
  ]}

  assert {:ok, [%{"status" => "active", "name" => "A"}], _, _} =
           PtcRunner.Lisp.run(program, context: context)
end

test "in operator coerces keywords to strings" do
  program = ~S|(->> ctx/items (filter (where :status in [:active :pending])))|
  context = %{"items" => [
    %{"status" => "active"},
    %{"status" => "pending"},
    %{"status" => "closed"}
  ]}

  assert {:ok, [%{"status" => "active"}, %{"status" => "pending"}], _, _} =
           PtcRunner.Lisp.run(program, context: context)
end

Out of Scope

• Changing how field keys are accessed (already handled by flex_get)
• Coercing numbers to strings or vice versa
• Coercing booleans to strings
• Adding warning/logging when coercion occurs
• Coercion for ordering operators (>, <, >=, <=)

Documentation Updates

• docs/ptc-lisp-specification.md - Update Section 7.1 to mention that comparison values support keyword/string coercion (similar to the existing "Flexible Key Access" section)

[andreasronge]

Issue Review: Add type coercion to where clause comparisons (keyword/string)

Summary: Well-specified enhancement to improve LLM ergonomics by allowing keyword literals to match string data values in where clause comparisons.

Pre-Review Checks:

• Codebase health: pass (1081 tests, 0 failures)
• Higher-priority blockers: none (no open bugs or from-pr-review issues)
• Issue claims verified: yes (line numbers corrected from 544-570 to 545-570, code patterns confirmed)

Analysis:

1. Should This Be Done?: Yes. This is a pragmatic enhancement aligned with the project's LLM-friendliness design goal. LLMs commonly generate keywords for values, and silent failures (empty results) are frustrating to debug.

2. Simplifications: The approach is already minimal - just add coercion clauses to existing helper functions. No unnecessary abstractions.

3. Codebase Research: Verified:

   • safe_eq, safe_in, safe_includes at lines 545-570 in lib/ptc_runner/lisp/eval.ex
   • flex_get pattern at lines 19-39 in lib/ptc_runner/lisp/runtime.ex provides the model
   • Test file exists at test/ptc_runner/lisp/flex_access_test.exs

4. Reuse Opportunities: The normalize_for_comparison helper can be reused across safe_eq, safe_in, and safe_includes.

5. Refactoring Needs: None. The existing structure supports this cleanly.

6. Edge Cases:

   • Added explicit boolean guard (not is_boolean/1) to prevent true matching "true"
   • Clarified that ordering operators should NOT coerce (returns false like nil handling)
   • Empty atom :"" correctly coerces to ""

7. Issue Scope: Appropriate for one PR. Three functions to modify with straightforward pattern matching additions.

8. Test Plan: Good coverage - unit tests for edge cases plus E2E tests demonstrating the feature.

9. Solution Outline: Clear and actionable. A junior developer could implement this.

Technical Issues Found:

• None significant

Changes made to issue body:

• Corrected line numbers (544-570 → 545-570)
• Added is_boolean/1 guards to implementation example to prevent boolean coercion
• Clarified that ordering operators (>, <, >=, <=) should NOT coerce - added to "Out of Scope" and acceptance criteria
• Added design decision section explaining ordering operator behavior
• Added test case for false/"false" non-coercion
• Added test case for safe_includes with list membership

Recommended Labels: ready-for-implementation, claude-approved, enhancement

Verdict: Ready for implementation

[andreasronge]

@claude Please implement this issue.

Implementation guidance:

• Follow the implementation approach in the issue body for safe_eq, safe_in, and safe_includes in lib/ptc_runner/lisp/eval.ex
• Add tests to test/ptc_runner/lisp/flex_access_test.exs following the test plan
• Update docs/ptc-lisp-specification.md Section 7.1 to document keyword/string coercion behavior
• Run mix test to ensure all tests pass before creating PR

[andreasronge]

Issue #232 implemented successfully!

Changes Made

• Added helper function that coerces keywords to strings while preserving boolean types
• Updated , , and to use type coercion for flexible comparisons
• Added 7 comprehensive tests covering keyword/string coercion scenarios
• Updated docs/ptc-lisp-specification.md Section 7.1 with coercion behavior and examples

Verification

✅ All 1088 tests pass (0 failures)
✅ Credo code quality checks passed
✅ PR created: #233

The implementation allows LLM-generated keywords to seamlessly match string data values in where clauses, improving usability while maintaining strict type checking for booleans and ordering comparisons.