feat(hooks): add hook combination and per-call hooks support

[jxnl]

Description

This PR adds powerful hook combination capabilities and per-call hook support to the Instructor library, enabling flexible event handling and monitoring at both the client and individual request levels.

Changes

Hook Combination Support

• ** method**: Combine two Hooks instances using the operator
• ** method**: Add hooks to an existing instance using the operator
• ** class method**: Combine multiple Hooks instances at once
• ** method**: Create deep copies of Hooks instances

Per-Call Hooks Support

• Added parameter to all create methods:
• Per-call hooks are automatically combined with client hooks using the operator
• Client hooks remain unchanged and functional after per-call hook usage
• Maintains backward compatibility - existing code continues to work unchanged

Examples and Documentation

• Hook Combination Example: Demonstrates different ways to combine hooks
• Per-Call Hooks Example: Shows how to use hooks at the individual request level
• Comprehensive examples with logging, metrics, debugging, and performance monitoring

Key Features

1. Flexible Hook Composition: Mix and match different hook sets for different use cases
2. Per-Request Customization: Add specific monitoring or debugging to individual calls
3. Client Hook Preservation: Client-level hooks continue working normally
4. Backward Compatibility: No breaking changes to existing code
5. Type Safety: Maintains existing Protocol-based type checking

Usage Examples

Hook Combination

# Combine hooks using + operator
combined_hooks = logging_hooks + metrics_hooks

# Add hooks in-place using += operator
logging_hooks += debug_hooks

# Combine multiple hooks at once
all_hooks = Hooks.combine(logging_hooks, metrics_hooks, debug_hooks)

Per-Call Hooks

# Client has global hooks
client = instructor.from_openai(openai.OpenAI(), hooks=client_hooks)

# Add debug hooks for specific call
user = client.chat.completions.create(
    model="gpt-3.5-turbo",
    messages=[{"role": "user", "content": "Extract: Alice is 25"}],
    response_model=User,
    hooks=debug_hooks  # Per-call hooks combined with client hooks
)

Testing

• Verified client hooks are preserved when no per-call hooks provided
• Verified per-call hooks are properly combined with client hooks
• Verified client hooks remain immutable after per-call hook usage
• All existing functionality continues to work unchanged

This PR was written by Cursor

---

Important

Add hook combination and per-call hooks support to Instructor library, enabling flexible event handling and maintaining backward compatibility.

• Hook Combination:
  • Add __add__, __iadd__, and combine() methods in Hooks class for combining hook instances.
  • copy() method added to create deep copies of Hooks instances.
• Per-Call Hooks:
  • Add hooks parameter to create, create_partial, create_iterable, and create_with_completion methods in client.py.
  • Combine per-call hooks with client hooks using + operator.
  • Ensure client hooks remain unchanged after per-call hook usage.
• Backward Compatibility:
  • Existing code remains functional without changes.
• Testing:
  • Verified preservation of client hooks and correct combination with per-call hooks.

^{This description was created by for 2e0ed6d. You can customize this summary. It will automatically update as commits are pushed.}

[cloudflare-workers-and-pages]

Deploying with    Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Updated (UTC)
✅ Deployment successful! View logs	instructor	2e0ed6d	Sep 04 2025, 08:06 PM

[claude]

Claude finished @jxnl's task —— View job

---

Code Review: Hook Combination and Per-Call Hooks Support

I've completed a thorough review of this PR that adds hook combination and per-call hooks support. Here's my comprehensive analysis:

Todo List

• Read and analyze all changed files
• Review hook combination implementation in hooks.py
• Review per-call hooks integration in client.py
• Check example files for best practices
• Assess test coverage and identify gaps
• Check for potential bugs, performance, and security issues
• Provide comprehensive feedback

✅ Code Quality & Implementation

The implementation demonstrates solid engineering practices:

Hook Combination Logic (hooks.py:229-347):

• Well-designed operator overloading with __add__ and __iadd__ methods
• Proper type checking with NotImplemented returns
• Clean deep copy semantics in the copy() method
• Comprehensive class method for combining multiple instances

Client Integration (client.py:371-374):

• Consistent pattern across all create methods (create, create_partial, create_iterable, create_with_completion)
• Proper hook combination using self.hooks + hooks syntax
• Client hooks remain immutable as claimed

⚠️ Issues Identified

1. Memory Management Concerns

File: hooks.py:258, 262, 293

combined._handlers[hook_name].extend(handlers.copy())  # Line 258
combined._handlers[hook_name].extend(handlers.copy())  # Line 262  
self._handlers[hook_name].extend(handlers.copy())      # Line 293

Issue: Calling handlers.copy() on a list creates a shallow copy, meaning the same handler function objects are shared between hook instances. This could lead to:

• Unintended side effects if handlers maintain internal state
• Memory leaks if handlers create closures that capture large objects
• Unexpected behavior when removing handlers from one instance affects others

Recommendation: Consider deep copying handler functions or document the shallow copy behavior clearly.

2. Missing Input Validation

File: client.py:373-374

if hooks is not None:
    combined_hooks = self.hooks + hooks

Issue: No validation that the hooks parameter is actually a Hooks instance. While the __add__ method returns NotImplemented, this will cause a runtime error rather than a clear error message.

Recommendation: Add type checking:

if hooks is not None:
    if not isinstance(hooks, Hooks):
        raise TypeError(f"Expected Hooks instance, got {type(hooks)}")
    combined_hooks = self.hooks + hooks

3. Performance Impact Not Addressed

Issue: Hook combination creates new instances and copies handlers each time, which could impact performance in high-frequency scenarios.

Potential Solutions:

• Consider lazy evaluation for hook combination
• Add performance considerations to documentation
• Implement hook caching for repeated combinations

4. Error Handling Edge Cases

File: hooks.py:137-145

The existing error handling catches all exceptions but may mask hook combination issues. Consider adding specific handling for hook-related errors vs. handler execution errors.

🧪 Test Coverage Gaps

Critical Missing Tests:

1. Hook Combination Testing: No tests for __add__, __iadd__, combine(), or copy() methods
2. Per-Call Hook Integration: No tests verifying client hooks + per-call hooks combination
3. Error Scenarios: No tests for invalid hook types passed to create methods
4. Memory Behavior: No tests ensuring client hooks remain unchanged after per-call usage
5. Handler Execution Order: No verification that combined hooks execute in the correct order

Test File: tests/llm/test_openai/test_hooks.py only covers basic hook registration/removal, not the new functionality.

Recommended Test Cases:

def test_hook_combination_add_operator():
    # Test hooks1 + hooks2 creates new instance
    
def test_hook_combination_iadd_operator():  
    # Test hooks1 += hooks2 modifies original
    
def test_per_call_hooks_combination():
    # Test client hooks + per-call hooks work together
    
def test_client_hooks_immutability():
    # Ensure client hooks unchanged after per-call usage

def test_combined_hooks_execution_order():
    # Verify hooks execute in expected order

🔒 Security Considerations

Low Risk: This feature primarily affects internal event handling and doesn't expose new attack vectors. The main concern is around error handling potentially masking security-relevant exceptions.

🏃♂️ Performance Considerations

Moderate Impact: Hook combination involves copying handler lists and creating new instances. For applications with frequent hook usage, consider:

• Caching combined hook instances
• Lazy evaluation strategies
• Performance benchmarks for high-frequency scenarios

📚 Documentation & Examples

Strengths:

• Comprehensive examples demonstrating all features
• Clear docstrings with usage examples
• Good progression from simple to complex usage

Minor Improvements:

• Add performance considerations to docstrings
• Document shallow copy behavior of handlers
• Add migration guide for users with custom hook implementations

🔄 Backward Compatibility

✅ Excellent: All existing code continues to work unchanged. The new features are purely additive.

Summary

This is a solid implementation of a useful feature with good API design and comprehensive examples. The main concerns are around memory management, missing validation, and insufficient test coverage for the new functionality.

Recommendation: Approve with requested changes - Address the memory management and validation issues, then add comprehensive tests for the new features.

Priority Fixes:

1. Add input validation for hooks parameter
2. Document shallow copy behavior or implement deep copying
3. Add comprehensive tests for hook combination and per-call hooks
4. Consider performance implications for high-frequency usage

---

[claude]

Claude finished @jxnl's task —— View job

---

Comprehensive PR Review

I've completed my review of this PR that adds hook combination and per-call hooks support. Here's my comprehensive analysis:

Todo List

• Read and analyze all changed files
• Review hook combination implementation in hooks.py
• Review per-call hooks integration in client.py
• Check example files for best practices
• Assess test coverage and identify gaps
• Check for potential bugs, performance, and security issues
• Provide comprehensive feedback

Review Summary

This PR adds solid hook combination and per-call hooks functionality. The implementation is generally well-designed and maintains backward compatibility. Here are my findings:

✅ Strengths

1. Excellent Type Safety: Strong typing with proper Protocol definitions and overloads
2. Backward Compatibility: All existing code continues to work unchanged
3. Clean API Design: Intuitive operator overloading (+, +=) and class method (combine())
4. Comprehensive Documentation: Good docstrings with examples throughout
5. Error Handling: Proper exception handling and warnings in hook execution
6. Immutability Preservation: Client hooks remain unchanged when per-call hooks are used

⚠️ Issues Found

Critical Issues

1. Memory Leak Risk in __add__ method (hooks.py:258,262):

   combined._handlers[hook_name].extend(handlers.copy())  # Should be list.copy(), not handlers.copy()

   The current code calls .copy() on the handlers list, but since these are function references, this creates shallow copies. Should be:

   combined._handlers[hook_name].extend(handlers[:])  # or list(handlers)

2. Same issue in copy() method (hooks.py:346):

   new_hooks._handlers[hook_name].extend(handlers.copy())  # Same issue

Medium Issues

3. Missing Test Coverage: No tests for the new hook combination features (__add__, __iadd__, combine(), copy()) or per-call hooks functionality

4. Missing Example Files: PR description mentions example files but they don't exist in the repository

5. Inconsistent Error Handling: Hook combination methods don't validate that all instances are actually Hooks objects until runtime

Minor Issues

6. Type Annotation: combine() method could use more specific typing:

   @classmethod  
   def combine(cls, *hooks_instances: Hooks) -> Hooks:  # Current
   # Could be more specific about empty args case

7. Performance: Multiple list extensions in hook combination could be optimized

🧪 Test Coverage Gaps

The existing tests only cover basic hook functionality but miss:

• Hook combination with + and += operators
• Hooks.combine() class method
• Hooks.copy() method
• Per-call hooks integration
• Edge cases (empty hooks, None hooks, type errors)

🏗️ Architecture Assessment

Good Design Decisions:

• Using defaultdict for handlers storage
• Operator overloading follows Python conventions
• Per-call hooks properly combined with client hooks
• Immutable operations preserve original instances

Areas for Improvement:

• Could benefit from validation decorators
• Hook execution could use context managers for better error isolation

🔒 Security & Performance

• Security: No security concerns identified
• Performance: Hook combination creates copies, which is safe but could be expensive with many handlers
• Memory: Potential minor memory overhead from list copying

📋 Recommendations

1. Fix Critical Bugs: Address the .copy() method calls in hook combination
2. Add Comprehensive Tests: Test all new functionality including edge cases
3. Add Example Files: Create the mentioned example files for documentation
4. Consider Performance: For heavy usage, consider lazy evaluation or copy-on-write
5. Validation: Add runtime type validation for better error messages

📊 Overall Assessment

Score: 7/10 - Solid implementation with good design principles, but needs bug fixes and better test coverage before merging.

The core functionality is well-designed and the API is intuitive. The main blockers are the memory-related bugs in the copying methods and missing test coverage. Once those are addressed, this would be a valuable addition to the library.

---

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed 2e0ed6d in 45 seconds. Click for details.

• Reviewed 368 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 2 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. examples/hooks/combination_example.py:1

• Draft comment:
  The entire combination example file has been removed. Ensure that equivalent documentation or examples are provided elsewhere (e.g., in the docs or tests) and update mkdocs.yml if these examples are relocated.
• Reason this comment was not posted:
  Confidence changes required: 80% <= threshold 85% None

2. examples/hooks/per_call_hooks_example.py:1

• Draft comment:
  The per-call hooks example file has been removed entirely. Please confirm that similar examples are available in the updated documentation or tests to illustrate per-call hook usage, and update mkdocs.yml if necessary.
• Reason this comment was not posted:
  Confidence changes required: 80% <= threshold 85% None

Workflow ID: wflow_3cV87ZX1JhFRRJo0

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed everything up to 7ce209d in 1 minute and 57 seconds. Click for details.

• Reviewed 851 lines of code in 4 files
• Skipped 0 files when reviewing.
• Skipped posting 1 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. instructor/core/client.py:394

• Draft comment:
  There's a placeholder comment "{{ edit_1 }}" in the parameter line. If this is not intended for production, please remove or update it.
• Reason this comment was not posted:
  Comment was on unchanged code.

Workflow ID: wflow_johoPM9XMPT124u7

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

The hook combination logic (i.e. 'combined_hooks = self.hooks; if hooks is not None: combined_hooks = self.hooks + hooks') is repeated in multiple methods. Consider extracting this pattern into a helper method to reduce duplication and improve maintainability.