Optimize tool call reactor handler ordering cache

[matdev83]

Summary

• cache the sorted handler ordering in ToolCallReactorService to avoid repeated sorting on every tool call
• invalidate the cached ordering whenever handlers are registered or unregistered
• add regression tests that verify the cache refreshes when handlers are added or removed

Testing

• python -m pytest -c /tmp/pytest.ini tests/unit/core/services/test_tool_call_reactor_service.py
• python -m pytest -c /tmp/pytest.ini (fails: missing optional dev dependencies such as pytest-asyncio and respx in this environment)

---

https://chatgpt.com/codex/tasks/task_e_68ec2f427ce08333b4c1e4ecd4777ede

Summary by CodeRabbit

• Refactor
  • Optimized handler processing with cached, lazily evaluated priority ordering to reduce overhead and improve responsiveness during tool call processing. No change to user-facing behavior.
• Tests
  • Added unit tests to confirm cache invalidation on handler registration/unregistration and to verify correct priority handling flow.

[coderabbitai]

Walkthrough

Adds a lazily computed, cached priority ordering for tool call handlers with invalidation on register/unregister. Updates processing to use the cached order. Introduces unit tests verifying cache rebuild on registration and unregistration, ensuring correct priority handling and swallow behavior.

Changes

Cohort / File(s)	Summary
Core service: cached handler ordering src/core/services/tool_call_reactor_service.py	Adds _sorted_handlers cache with _invalidate_sorted_handlers and _get_sorted_handlers. process_tool_call now uses cached sorted handlers. Register/unregister paths invalidate cache. Minor refactor; unchanged external API.
Unit tests: cache invalidation behavior tests/unit/core/services/test_tool_call_reactor_service.py	Adds async tests for cache rebuild on handler register/unregister, asserting priority order, swallow behavior, and call counts via MockToolCallHandler.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Caller
  participant R as ToolCallReactorService
  participant H as Handlers (various priorities)
  note over R: On first use, compute and cache sorted handlers

  C->>R: process_tool_call(call)
  alt cache empty
    R->>R: _get_sorted_handlers() -> sort & cache
  else cache present
    R->>R: _get_sorted_handlers() -> return cached
  end
  loop in priority order
    R->>H: can_handle(call)?
    alt can_handle = true
      R->>H: handle(call)
      alt swallowed
        R-->>C: return handled result
        note over R: Stop on swallow
      else not swallowed
        note over R: Continue to next handler
      end
    else cannot handle
      note over R: Skip to next
    end
  end
  R-->>C: return default/no-op if none handled

Loading

sequenceDiagram
  autonumber
  participant Dev as Registrar
  participant R as ToolCallReactorService
  participant Cache as _sorted_handlers
  rect rgba(220,240,255,0.4)
  note over R,Cache: Cache invalidation paths
  Dev->>R: register(handler, priority)
  R->>Cache: _invalidate_sorted_handlers() -> None
  Dev->>R: unregister(handler)
  R->>Cache: _invalidate_sorted_handlers() -> None
  end
  note over R: Next process_tool_call recomputes sorted list

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

I sorted my snacks in a tidy cache,
With carrots on top for a quicker dash.
Register, unregister—flip! it’s anew,
Priorities shuffle like dew on the hew.
I nibble in order, no cycles to thrash—
A bunny with queues, avoiding the clash. 🥕✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title succinctly captures the core change of improving the caching mechanism for handler ordering in the tool call reactor, clearly reflecting the main performance optimization without extraneous details.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch codex/fix-performance-issues-in-codebase-9jwil1

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4b959ef and 4862a3e.

📒 Files selected for processing (2)

• src/core/services/tool_call_reactor_service.py (5 hunks)
• tests/unit/core/services/test_tool_call_reactor_service.py (1 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

**/*.py: Avoid using emojis in Python code comments or docstrings
Follow PEP 8 and use type hints for all functions
Import order must be: standard library, third-party, then local imports, separated by blank lines
Naming conventions: snake_case for variables and functions; PascalCase for classes
Error handling: use specific exceptions and include meaningful error messages
Prefer f-strings for string formatting

Files:

• tests/unit/core/services/test_tool_call_reactor_service.py
• src/core/services/tool_call_reactor_service.py

src/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

src/**/*.py: Catch specific exceptions; avoid broad except Exception blocks
If a broad exception must be caught, log with exc_info=True and re-raise a specific custom exception
Use the most specific exception class from src.core.common.exceptions that accurately describes the error
Provide clear, helpful error messages and include relevant details in the details dictionary of custom exceptions

Files:

• src/core/services/tool_call_reactor_service.py

🧬 Code graph analysis (2)

tests/unit/core/services/test_tool_call_reactor_service.py (2)

src/core/interfaces/tool_call_reactor_interface.py (2)

• ToolCallReactionResult (47-57)
• ToolCallContext (18-43)

src/core/services/tool_call_reactor_service.py (3)

• register_handler (86-103)
• process_tool_call (124-194)
• unregister_handler (105-122)

src/core/services/tool_call_reactor_service.py (2)

src/core/interfaces/tool_call_reactor_interface.py (2)

• IToolCallHandler (60-98)
• priority (74-75)

tests/unit/core/services/test_tool_call_reactor_service.py (1)

• priority (44-45)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: test (3.10)

🔇 Additional comments (7)

src/core/services/tool_call_reactor_service.py (5)

44-44: LGTM! Appropriate cache field initialization.

The cache field uses an immutable tuple type and is initialized to None for lazy evaluation, which is a solid optimization pattern.

---

46-49: LGTM! Clean cache invalidation.

The invalidation method is straightforward and correctly resets the cache to None.

---

51-62: LGTM! Well-implemented lazy caching pattern.

The method correctly implements lazy evaluation with caching. The use of tuple ensures immutability, and sorting by priority in descending order aligns with the documented behavior (higher priority handlers run first). Since there are no await points in this method, it's safe from race conditions in the async/await concurrency model.

---

83-83: LGTM! Cache invalidation correctly placed.

Cache invalidation is properly called after modifying the handlers dictionary in all three mutation methods (register_handler_sync, register_handler, and unregister_handler). The invalidation in async methods occurs within the lock, ensuring consistency.

Also applies to: 102-102, 121-121

---

164-164: LGTM! Efficient use of cached ordering.

Replacing the inline sorting with the cached _get_sorted_handlers() call achieves the performance optimization goal while maintaining correctness.

tests/unit/core/services/test_tool_call_reactor_service.py (2)

317-365: LGTM! Comprehensive test for cache invalidation on registration.

This test effectively verifies that:

1. The cache is primed with the initial low-priority handler
2. Registering a new high-priority handler invalidates the cache
3. The updated ordering is used in subsequent calls, with the high-priority handler swallowing before the low-priority handler can process

The assertions correctly check that the low-priority handler is not invoked again (line 364), confirming the high-priority handler swallowed the call.

---

366-416: LGTM! Thorough test for cache invalidation on unregistration.

This test properly validates that:

1. The high-priority handler processes calls before unregistration
2. Unregistering the high-priority handler invalidates the cache
3. The low-priority handler takes over after the cache is rebuilt

The assertions correctly verify that each handler is invoked the expected number of times, confirming the cache refresh logic works correctly.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!