Skip to content

feat: add object operation to construct maps with evaluated values (#253)#254

Merged
andreasronge merged 2 commits intomainfrom
claude/issue-253-20251212-102542
Dec 12, 2025
Merged

feat: add object operation to construct maps with evaluated values (#253)#254
andreasronge merged 2 commits intomainfrom
claude/issue-253-20251212-102542

Conversation

@andreasronge
Copy link
Copy Markdown
Owner

Summary

Implements issue #253: adds the object operation to PTC-JSON that constructs maps with evaluated expression values, fixing the memory contract that was previously impossible to express correctly.

Changes

  • Schema: Added object operation definition with fields parameter (map type)
  • Validator: Added validate_object/2 for recursive field validation of operation values
  • Operations: Implemented eval_object and evaluate_object_field_value to evaluate fields with expressions
  • System Prompt: Updated memory example to use object instead of the invalid merge pattern
  • Tests: Added comprehensive unit tests covering:
    • All literal values
    • Expression values with var references
    • Mixed literal and expression values
    • Nested object operations
    • Empty fields
    • E2E memory contract test (store and retrieve)
  • Documentation: Updated ptc-json-specification.md with object operation description
  • Schema File: Regenerated priv/ptc_schema.json with new operation
  • Schema Tests: Updated to expect 42 operations instead of 41

Test Plan

✅ All new tests pass
✅ All existing tests pass
✅ Credo passes
✅ Code format passes
✅ Compilation succeeds with warnings as errors

Verification

The implementation enables the memory contract to work correctly:

{"program":{"op":"object","fields":{"my-key":{"op":"var","name":"x"},"result":42}}}

Closes #253

🤖 Generated with Claude Code

Co-Authored-By: Claude Haiku 4.5 noreply@anthropic.com

Adds the 'object' operation to PTC-JSON that enables constructing maps where
values can be evaluated expressions. This fixes a critical gap in the memory
contract and allows the system prompt example to work correctly.

- Add object operation definition to schema (fields: map type)
- Add validate_object/2 for recursive field validation in validator
- Implement eval_object and evaluate_object_field_value in operations
- Update system prompt memory example to use object instead of invalid merge pattern
- Add comprehensive unit and E2E tests for object operation
- Update ptc-json-specification.md documentation
- Regenerate priv/ptc_schema.json with new operation
- Update schema tests to expect 42 operations instead of 41

Closes #253

🤖 Generated with Claude Code

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

PR Review: feat: add object operation to construct maps with evaluated values

Summary

This PR implements issue #253 by adding the object operation to PTC-JSON, enabling construction of maps with evaluated expression values. This fixes a critical gap where the system prompt memory example was previously invalid. The implementation follows existing patterns well, though validation tests specified in the issue are missing.

What's Good

  • Follows existing patterns: The implementation mirrors merge/concat/zip operations for consistency
  • Comprehensive happy-path tests: Good coverage of literal values, expressions, mixed values, nested objects, var references, empty fields
  • E2E memory contract test: Validates the core use case that motivated this feature
  • Documentation updated: docs/ptc-json-specification.md and priv/ptc_schema.json properly updated
  • System prompt fixed: The invalid merge pattern in the system prompt is now correctly replaced with object
  • Schema tests updated: Correctly changed operation count from 41 to 42

Issues (Must Fix)

  1. Missing validation tests - test/ptc_runner/json/operations/collection_test.exs

    • Problem: The issue feat: Add object operation to construct maps with evaluated values #253 test plan specifically required: "Error in field expression propagates error". This test is missing. Additionally, for consistency with merge/concat/zip operations (which all have validation failure tests), object should have similar tests.
    • Impact: Incomplete test coverage compared to similar operations and missing test case from issue requirements
    • Fix: Add the following tests to collection_test.exs:
      • test "object with error in field expression propagates error" - use an invalid operation in a field value to verify error propagation
      • test "object validation fails when fields is missing" - verify validation error when fields is omitted
      • test "object validation fails when fields is not a map" - verify validation error when fields is wrong type (e.g., a list)
    • Complexity: Mechanical (follow same pattern as merge/concat/zip validation tests at lines 111-140, 215-240, 322-350)
  2. Moduledoc not updated - lib/ptc_runner/json/operations.ex:5-9

    • Problem: The moduledoc lists operations by phase but object is not included in the Phase 3 list where merge, concat, zip appear
    • Impact: Documentation inconsistency
    • Fix: Add object to Phase 3 in the moduledoc, e.g., "Phase 3: let, if, and, or, not, object, merge, concat, zip"
    • Complexity: Trivial (single-word addition)

Suggestions (Optional)

  1. Consider test for invalid op in nested expression - Nice-to-have
    • Suggestion: Add a test verifying that a nested object with an invalid inner operation properly propagates the validation error
    • Benefit: Ensures recursive validation works correctly
    • Recommendation: Could be combined with the error propagation test above

Security

No security concerns. The operation evaluates expressions through the existing sandboxed interpreter.

Documentation

  • docs/ptc-json-specification.md updated with object operation description
  • priv/ptc_schema.json regenerated
  • ✅ System prompt example fixed in lib/ptc_runner/schema.ex
  • ⚠️ Minor: Moduledoc in operations.ex needs object added to Phase 3

Verdict

Request Changes - The implementation is correct and follows existing patterns well, but missing tests specified in issue #253 must be added for completeness. The fixes are mechanical and follow existing patterns in the codebase.

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

@claude please fix these issues from PR review:

Issue 1: Missing validation tests for object operation

Location: test/ptc_runner/json/operations/collection_test.exs
Problem: Issue #253 test plan required "Error in field expression propagates error" test. Additionally, for consistency with merge/concat/zip operations (which all have validation failure tests at lines 111-140, 215-240, 322-350), object should have similar validation tests.
Fix: Add the following three tests to collection_test.exs after the existing object tests (after line 702):

  1. test "object with error in field expression propagates error" - use an invalid operation in a field value to verify error propagation (follow pattern from merge/concat/zip error tests)
  2. test "object validation fails when fields is missing" - verify validation error when fields parameter is omitted (follow pattern at line 123)
  3. test "object validation fails when fields is not a map" - verify validation error when fields is wrong type like a list or number (follow pattern at line 132)

Issue 2: Moduledoc not updated with object operation

Location: lib/ptc_runner/json/operations.ex:5-9
Problem: The moduledoc lists operations by phase but object is not included in the Phase 3 list where merge, concat, zip appear.
Fix: Add object to Phase 3 in the moduledoc at line 7. Change "Phase 3: let, if, and, or, not, merge, concat, zip" to "Phase 3: let, if, and, or, not, object, merge, concat, zip"

Please fix ALL issues above in a SINGLE commit. Make minimal changes to resolve each issue.

@andreasronge
Copy link
Copy Markdown
Owner Author

Auto-Triage Summary

Decisions Made

# Issue Decision Action
1 Missing validation tests (error propagation, missing fields, invalid type) FIX_NOW Included in fix request above
2 Moduledoc not updated with object in Phase 3 list FIX_NOW Included in fix request above

Reasoning

  • Issue 1: Marked as FIX_NOW because (1) issue feat: Add object operation to construct maps with evaluated values #253 test plan explicitly required "Error in field expression propagates error" test, (2) validation tests are mechanical additions following existing merge/concat/zip patterns at lines 111-140, 215-240, 322-350, (3) this is incomplete work from the PR's stated purpose
  • Issue 2: Marked as FIX_NOW because it's a trivial single-word addition directly related to the PR's purpose of adding the object operation

Status

  • FIX_NOW items: 2 (batched in single fix comment above)
  • Issues created: 0
  • Items ignored: 0

All issues are mechanical fixes that follow existing patterns in the codebase. Once fixes are applied, the PR will be complete per issue #253 requirements.

@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 12, 2025
…uledoc

- Add three validation tests for object operation in collection_test.exs:
  * Test error propagation in field expressions
  * Test validation fails when fields parameter is missing
  * Test validation fails when fields parameter is not a map
- Update moduledoc in operations.ex to include 'object' in Phase 3 operations list
- Add catch-all clause in validator.ex to properly handle invalid field types

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

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
@andreasronge
Copy link
Copy Markdown
Owner Author

Fix Summary

All issues from the PR review have been addressed:

Issue 1: Missing validation tests for object operation ✅

Added three validation tests to test/ptc_runner/json/operations/collection_test.exs:

  1. Test "object with error in field expression propagates error" - Verifies that invalid operations in field values properly propagate validation errors
  2. Test "object validation fails when fields is missing" - Verifies validation error when the fields parameter is omitted
  3. Test "object validation fails when fields is not a map" - Verifies validation error when fields parameter is wrong type (e.g., a list)

Issue 2: Moduledoc not updated with object operation ✅

Updated lib/ptc_runner/json/operations.ex moduledoc at line 7:

  • Changed: "Phase 3: let, if, and, or, not, merge, concat, zip"
  • To: "Phase 3: let, if, and, or, not, object, merge, concat, zip"

Additional Fix ✅

Fixed the validator in lib/ptc_runner/json/validator.ex to properly validate the fields parameter type:

  • Added catch-all clause to validate_object_fields/1 to return proper validation error when fields is not a map

All tests pass and precommit checks are clean.

@github-actions
Copy link
Copy Markdown
Contributor

PR Review: feat: add object operation to construct maps with evaluated values

Summary

This PR implements issue #253 by adding the object operation to PTC-JSON that constructs maps with evaluated expression values. It fixes a critical gap where the system prompt memory example was invalid. The implementation is well-structured, follows existing patterns, and all issues from the previous review have been addressed.

What's Good

  • Correct implementation: The object operation follows the same patterns used by merge, concat, and zip - using Enum.reduce_while for iteration and consistent error handling
  • Comprehensive test coverage: 12 tests covering literal values, expressions, mixed values, nested objects, var references, empty fields, undefined vars, E2E memory contract, and validation errors
  • Validation tests added: Following the reviewer's feedback, validation tests now match the pattern of other combine operations (merge, concat, zip)
  • Documentation complete: docs/ptc-json-specification.md updated with operation description and examples, following the structure of existing operations
  • Schema files updated: priv/ptc_schema.json regenerated, schema tests updated to expect 42 operations
  • System prompt fixed: The invalid merge pattern in lib/ptc_runner/schema.ex is now correctly replaced with object
  • Moduledoc updated: Phase 3 operations list in lib/ptc_runner/json/operations.ex now includes object

Issues (Must Fix)

None. All issues from the previous review have been properly addressed.

Suggestions (Optional)

None. The implementation is appropriately scoped and complete.

Security

No security concerns. The operation evaluates expressions through the existing sandboxed interpreter with established resource limits.

Documentation

  • docs/ptc-json-specification.md - Operation description and example added
  • priv/ptc_schema.json - Schema regenerated
  • lib/ptc_runner/schema.ex - System prompt example fixed
  • lib/ptc_runner/json/operations.ex - Moduledoc updated

Verdict

Approve - The implementation is complete, well-tested, and follows established codebase patterns. All requirements from issue #253 are met, including the validation tests that were added after the previous review. The 448 JSON tests and 72 schema tests pass.

@github-actions github-actions bot added the auto-triage-pending Auto-triage in progress label Dec 12, 2025
@andreasronge andreasronge added the ready-to-merge PR is ready to be merged label Dec 12, 2025
@andreasronge
Copy link
Copy Markdown
Owner Author

Auto-Triage Summary (Cycle 1 of 3)

Status: Already Triaged ✅

This PR has already completed a full triage cycle:

  1. Initial Review (github-actions): Identified 2 issues → Request Changes
  2. Triage (@andreasronge): Both issues marked as FIX_NOW
  3. Fixes Applied (@andreasronge): All issues resolved in follow-up commit
  4. Second Review (github-actions): Approve verdict with no remaining issues

Current State

  • Verdict: Approve
  • Issues Found: 0
  • Action Taken: Added ready-to-merge label

The PR is ready for auto-merge once all checks pass.

@github-actions github-actions bot added auto-triage-cycle-2 Auto-triage cycle 2 and removed auto-triage-cycle-1 Auto-triage cycle 1 auto-triage-pending Auto-triage in progress labels Dec 12, 2025
@andreasronge andreasronge enabled auto-merge (squash) December 12, 2025 10:40
@andreasronge andreasronge merged commit e87efdc into main Dec 12, 2025
4 checks passed
@andreasronge andreasronge deleted the claude/issue-253-20251212-102542 branch December 12, 2025 10:40
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-2 Auto-triage cycle 2 ready-to-merge PR is ready to be merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: Add object operation to construct maps with evaluated values

1 participant