Skip to content

feat: implement call operation for tool invocation (fixes #40)#41

Merged
andreasronge merged 1 commit intomainfrom
claude/issue-40-implement
Dec 2, 2025
Merged

feat: implement call operation for tool invocation (fixes #40)#41
andreasronge merged 1 commit intomainfrom
claude/issue-40-implement

Conversation

@andreasronge
Copy link
Copy Markdown
Owner

Summary

Implements the call operation for Phase 4 (Tool Integration), enabling PTC programs to invoke registered tool functions with provided arguments and handle their results.

What Changed

Validator (lib/ptc_runner/validator.ex)

  • Added validation clause for "call" operation before catch-all clause
  • Validates required "tool" field (must be string)
  • Validates optional "args" field (must be map if provided)

Operations (lib/ptc_runner/operations.ex)

  • Added eval function for "call" operation
  • Invokes registered tool by name with args (defaults to %{} if not provided)
  • Handles all edge cases:
    • Missing tool: returns error with "Tool not found" message
    • Tool returns {:error, reason}: propagates as execution error
    • Tool raises exception: caught and converted to execution error
    • Tool has wrong arity: caught with clear error message using is_function(tool_fn, 1)

Tests (test/ptc_runner_test.exs)

  • 13 comprehensive tests covering all acceptance criteria:
    • Unit tests: tool with args, tool without args, all error cases
    • Validation tests: missing tool field, args not a map
    • E2E test: call tool as first step in pipe with filter and count
    • E2E test: call tool with let binding to store result

Test Results

All 236 tests pass (including 3 doctests, 13 new tests for call operation).

Checklist

  • Validation pattern matches literal and let operations
  • Operation pattern matches let and other operations
  • Implementation addresses review suggestion (using is_function(tool_fn, 1))
  • All acceptance criteria covered
  • Edge cases handled (missing tool, error returns, exceptions, wrong arity)
  • E2E tests demonstrate pipe and let integration
  • Code formatted and passes all quality checks

Fixes #40

🤖 Generated with Claude Code

Add call operation to allow PTC programs to invoke registered tool functions.
This is the core feature of Phase 4 (Tool Integration), enabling LLMs to
generate programs that call user-defined tools and process their results.

Changes:
- Add validation clause for "call" operation in validator.ex
  - Validates required "tool" field
  - Validates optional "args" field (must be map if provided)

- Add eval function for "call" operation in operations.ex
  - Invokes registered tool by name with provided arguments
  - Defaults to empty map for args if not provided
  - Handles missing tool errors
  - Handles tool error returns
  - Catches and converts tool exceptions
  - Uses is_function(tool_fn, 1) for arity validation

- Add comprehensive test suite covering:
  - Unit tests for all acceptance criteria
  - Tool argument handling
  - Error cases (missing tool, error tuple, exceptions, wrong arity)
  - Validation errors
  - E2E test with pipe integration
  - E2E test with let binding

All tests pass (236 tests, 0 failures).

🤖 Generated with Claude Code

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

github-actions bot commented Dec 2, 2025

PR Review: feat: implement call operation for tool invocation

Summary

This PR implements the call operation for Phase 4 (Tool Integration), enabling PTC programs to invoke registered tool functions. The implementation is well-structured, follows existing patterns correctly, and includes comprehensive tests. Ready for merge.

What's Good

  • Follows existing patterns exactly: Validator clause uses require_field/3 helper and placement before catch-all matches the pattern from other operations
  • Clean operation implementation: Uses is_function(tool_fn, 1) for arity validation as suggested in the issue review
  • Comprehensive error handling: Covers all edge cases (missing tool, error returns, exceptions, wrong arity)
  • Strong test coverage: 11 tests covering unit, validation, and E2E scenarios
  • Well-integrated E2E tests: Tests demonstrate call working in pipes and with let bindings

Issues (Must Fix)

None identified. The implementation is complete and consistent.

Suggestions (Optional)

  1. Test for nil tools map - Nice-to-have

    • Suggestion: Consider adding a test for when context.tools is nil (though Context.new/2 defaults to %{} so this shouldn't happen in practice)
    • Benefit: Documents the expected behavior explicitly
    • Complexity: Trivial
    • Recommendation: Low priority, current behavior is correct since Map.get(nil, key) raises
  2. Tool validation at registration - Out of scope

    • Suggestion: Could validate tool arities when passed to PtcRunner.run/2 rather than at call time
    • Benefit: Earlier error detection
    • Complexity: Moderate (needs design decisions)
    • Recommendation: Create GitHub issue if desired for future work

Security

No concerns. The implementation:

  • Tools are user-provided functions, not arbitrary code execution
  • Uses try/rescue to catch exceptions from tools
  • Arguments are raw values from parsed JSON, not dynamically evaluated

Verdict

Approve - The implementation is clean, well-tested, and follows established patterns. All acceptance criteria from issue #40 are met:

  • call operation invokes registered tools with args
  • ✅ Missing tool returns execution error
  • ✅ Tool returning {:error, reason} is propagated
  • ✅ Tool exceptions are caught and converted
  • ✅ Empty args defaults to %{}
  • ✅ Works as first step in pipe
  • ✅ Works with let bindings

@andreasronge
Copy link
Copy Markdown
Owner Author

Auto-Triage Summary

Decisions Made

# Issue Decision Action
1 Test for nil tools map IGNORE Reviewer correctly noted this is low priority. Context.new/2 defaults to %{}, so context.tools is never nil in practice. Current behavior (Map.get would raise on nil) is actually good defensive property. No functional benefit to adding this test.
2 Tool validation at registration DEFER_ISSUE Created issue #42. This is a valuable architectural enhancement (earlier error detection) but requires design decisions and is out of scope for the call operation implementation.

Status

Next Steps

No fixes needed. The PR is approved and ready to merge. All acceptance criteria from issue #40 are met with comprehensive test coverage.

@andreasronge andreasronge added the ready-to-merge PR is ready to be merged label Dec 2, 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 2, 2025
@andreasronge andreasronge enabled auto-merge (squash) December 2, 2025 08:17
@andreasronge andreasronge merged commit 4e304b8 into main Dec 2, 2025
3 checks passed
@andreasronge andreasronge deleted the claude/issue-40-implement branch December 2, 2025 08:17
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.

[Phase 4] Implement call operation for tool invocation

1 participant