Skip to content

feat: Add keyword/string type coercion to where clause comparisons (#232)#233

Merged
andreasronge merged 1 commit intomainfrom
claude/issue-232-20251211-122317
Dec 11, 2025
Merged

feat: Add keyword/string type coercion to where clause comparisons (#232)#233
andreasronge merged 1 commit intomainfrom
claude/issue-232-20251211-122317

Conversation

@andreasronge
Copy link
Copy Markdown
Owner

Summary

  • Implements automatic coercion of keywords to strings in equality operators (=, not=) and membership operators (in, includes)
  • Allows LLM-generated keywords to match string data values without explicit conversion
  • Excludes booleans from coercion to prevent true matching "true"
  • Improves LLM ergonomics by reducing silent failures when keywords are generated instead of strings

Implementation

  • Added normalize_for_comparison/1 helper that coerces non-boolean atoms to their string representation
  • Updated safe_eq/2, safe_in/2, and safe_includes/2 to use the coercion helper
  • Ordering comparisons (>, <, >=, <=) intentionally excluded from coercion (maintain strict type checking)

Testing

  • Added 7 new tests covering keyword/string coercion scenarios:
    • Basic keyword-to-string equality matching
    • Membership testing with in operator
    • List membership testing with includes operator
    • Boolean non-coercion (true/false don't match "true"/"false")
    • Empty string matching
  • All 1088 tests pass, no regressions

Documentation

  • Updated Section 7.1 of ptc-lisp-specification.md with coercion rules and examples
  • Clearly documented what types coerce and which operations use coercion
  • Noted that ordering comparisons do not use coercion

Closes #232

)

Implement automatic coercion of keywords to strings in equality operators (=, not=),
and membership operators (in, includes). This improves LLM ergonomics by allowing
keywords generated by LLMs to match string data values without explicit conversion.

- Add normalize_for_comparison helper to coerce non-boolean atoms to strings
- Update safe_eq, safe_in, safe_includes to use coercion
- Exclude booleans from coercion (true/false do not coerce to strings)
- Add comprehensive tests covering keyword/string matching
- Document coercion behavior in Section 7.1 of ptc-lisp-specification.md
- Note that ordering comparisons (>, <, >=, <=) do NOT coerce

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

PR Review: Add keyword/string type coercion to where clause comparisons (#232)

Summary

This PR implements automatic coercion of keywords to strings in equality and membership operators, allowing LLM-generated keywords to match string data values. The implementation is clean, follows the issue specification exactly, and includes comprehensive tests.

What's Good

  • Clean implementation: The normalize_for_comparison/1 helper is well-designed and reused across safe_eq, safe_in, and safe_includes
  • Correct boolean handling: The not is_boolean(value) guard correctly prevents true/false from matching their string equivalents
  • Intentional scope: Ordering comparisons (>, <, >=, <=) are correctly excluded from coercion as specified
  • Good test coverage: 7 tests cover the key scenarios including positive cases, boolean non-coercion, and empty string matching
  • Documentation updated: Section 7.1 of the Lisp specification clearly documents the coercion rules with examples

Issues (Must Fix)

None.

Suggestions (Optional)

  1. Test setup extraction - Nice-to-have
    • Suggestion: The new tests in flex_access_test.exs:64-166 have repeated context setup patterns (items with status/name, tags, active fields). A setup block could reduce some repetition.
    • Benefit: Slightly reduced boilerplate
    • Complexity: Low, but the current tests are clear and self-documenting as-is
    • Recommendation: Not urgent - the test duplication is minor and each test is easy to read in isolation

Security

No concerns. The coercion only applies to keywords (atoms), which are safe in the BEAM. The boolean guard prevents any unexpected behavior with true/false values.

Documentation

Documentation updated appropriately in docs/ptc-lisp-specification.md Section 7.1. The coercion rules are clearly explained with examples, and the note about ordering comparisons not using coercion is included.

Verdict

Approve - The implementation matches the issue specification exactly, tests are comprehensive, and documentation is updated. Clean, minimal change that solves the stated problem without over-engineering.

@github-actions github-actions bot added the auto-triage-pending Auto-triage in progress label Dec 11, 2025
@andreasronge
Copy link
Copy Markdown
Owner Author

Auto-Triage Summary

Review Analysis

The PR review approved the changes with no must-fix issues. There was one optional suggestion about extracting test setup to reduce boilerplate.

Decisions Made

# Issue Decision Reasoning
1 Extract test setup patterns in flex_access_test.exs:64-166 IGNORE Each test uses different data to test different behaviors (equality, inequality, membership, boolean non-coercion, etc.). The "duplication" is natural test structure, not boilerplate. Extracting to shared setup would reduce clarity by separating test data from test logic. The reviewer noted "tests are clear and self-documenting as-is" and "not urgent". Project guidelines emphasize test clarity over DRY principles for tests.

Status

  • FIX_NOW items: 0
  • Issues created: 0
  • Items ignored: 1

Conclusion

No action items. The PR is approved and ready to merge once CI passes.

@andreasronge andreasronge added the ready-to-merge PR is ready to be merged label Dec 11, 2025
@github-actions github-actions bot added auto-triage-cycle-1 Auto-triage cycle 1 auto-triage-complete Auto-triage completed and removed auto-triage-pending Auto-triage in progress labels Dec 11, 2025
@andreasronge andreasronge enabled auto-merge (squash) December 11, 2025 12:27
@andreasronge andreasronge merged commit 6aef427 into main Dec 11, 2025
4 checks passed
@andreasronge andreasronge deleted the claude/issue-232-20251211-122317 branch December 11, 2025 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-triage-complete Auto-triage completed auto-triage-cycle-1 Auto-triage cycle 1 ready-to-merge PR is ready to be merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

1 participant