Optimize tool call reactor handler ordering cache

[matdev83]

Summary

• cache the priority-sorted handler list inside ToolCallReactorService and refresh it on registration changes
• add a regression test to ensure the cached ordering updates after a handler is unregistered

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 test dependencies such as pytest_asyncio, respx, pytest_httpx, hypothesis)

---

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

Summary by CodeRabbit

• Bug Fixes
  • Ensures the correct handler is selected after unregistering, preventing stale ordering issues during processing.
• Refactor
  • Introduced caching for handler prioritization to reduce redundant work and improve responsiveness, with automatic cache refresh on handler changes.
• Tests
  • Added coverage to verify the handler ordering cache refreshes correctly after a handler is removed.

[coderabbitai]

Walkthrough

Adds a cached, priority-ordered handler list to ToolCallReactorService, with cache invalidation on handler register/unregister. process_tool_call now uses the cached ordering. Introduces helper methods for cache management. Adds a unit test verifying cache refresh after unregister affects handler selection.

Changes

Cohort / File(s)	Summary of edits
Core service: handler ordering cache src/core/services/tool_call_reactor_service.py	Added _handlers_cache, _invalidate_handler_cache(), and _get_handlers_by_priority(). Hooked cache invalidation into register/unregister paths. Updated process_tool_call to consume cached, priority-sorted handlers. No changes to history/error handling.
Unit tests: cache refresh behavior tests/unit/core/services/test_tool_call_reactor_service.py	Added async test test_handler_cache_refresh_after_unregister to confirm handler ordering cache is refreshed after unregister, affecting which handler processes subsequent calls.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Client
  participant Reactor as ToolCallReactorService
  participant Cache as Handler Cache
  participant Handlers as Registered Handlers

  Client->>Reactor: register_handler(...)
  activate Reactor
  Reactor->>Handlers: Add handler
  Reactor->>Cache: Invalidate
  deactivate Reactor

  Client->>Reactor: process_tool_call(request)
  activate Reactor
  Reactor->>Cache: Get handlers by priority
  alt cache miss
    Cache->>Handlers: Read all handlers
    Handlers-->>Cache: Return set
    Cache-->>Reactor: Sorted handlers (desc priority)
  else cache hit
    Cache-->>Reactor: Cached sorted handlers
  end
  Reactor->>Handlers: Iterate and attempt handle
  Reactor-->>Client: Result
  deactivate Reactor

  Client->>Reactor: unregister_handler(id)
  activate Reactor
  Reactor->>Handlers: Remove handler
  Reactor->>Cache: Invalidate
  deactivate Reactor

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I cached my carrots, neat and high,
So hopping through, I barely sigh—
No re-sorting every bite,
Just freshest first by lunar light.
Unpluck a sprout? I clear the heap,
Then munch in order—swift and deep. 🥕✨

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 clearly reflects the main change of improving the handler ordering cache within the ToolCallReactorService by introducing caching and invalidation logic, and it is concise and specific enough to convey the PR’s primary focus.
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-qmgonx

---

📜 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 dd216cf and 298b0a8.

📒 Files selected for processing (2)

• src/core/services/tool_call_reactor_service.py (6 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 (3)

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

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

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

• priority (53-55)
• register_handler (397-403)
• unregister_handler (405-411)

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

• register_handler (68-85)
• process_tool_call (106-176)
• unregister_handler (87-104)

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). (2)

• GitHub Check: test (3.10)
• GitHub Check: Analyze (python)

🔇 Additional comments (5)

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

277-320: LGTM! Well-designed regression test.

This test effectively validates that the handler ordering cache is properly invalidated after unregistering a handler. The test logic is sound:

1. Registers two handlers with different priorities (primary=100, secondary=10)
2. Confirms the high-priority handler processes the first call
3. Unregisters the high-priority handler
4. Confirms the lower-priority handler now processes subsequent calls

The assertions correctly verify the call counts and swallow decisions at each step, ensuring the cache refresh behavior works as intended.

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

44-44: LGTM! Appropriate cache field initialization.

The cache is correctly typed as an optional tuple of handlers, initialized to None. Using a tuple ensures immutability, which prevents issues if handlers are modified while the cached snapshot is being iterated during process_tool_call.

---

65-65: LGTM! Cache invalidation correctly placed.

The cache is invalidated after each handler mutation (register_handler_sync, register_handler, unregister_handler), ensuring the next call to process_tool_call will use the updated handler ordering. The placement inside the lock (for async methods) ensures consistency with handler modifications.

Also applies to: 84-84, 103-103

---

146-146: LGTM! Efficient use of cached handler ordering.

Replacing the inline sorting with the cached getter eliminates redundant sorting on every process_tool_call invocation. Since the getter returns an immutable tuple, the snapshot is safe from concurrent handler modifications during iteration.

---

186-202: LGTM! Well-designed cache management.

The cache management methods are clean and correct:

• _invalidate_handler_cache(): Simple and effective cache clearing
• _get_handlers_by_priority(): Implements lazy computation with caching. The method is synchronous and runs atomically within the asyncio event loop, making it safe for concurrent process_tool_call invocations. The tuple return type ensures immutability.

The design optimizes the hot path (read-heavy process_tool_call) by avoiding locks, accepting the minor trade-off that concurrent tasks might redundantly compute the cache. This is a sensible performance optimization.

---

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

[matdev83]

Closing this pull request as it is a duplicate of #669. The changes from #669 will be used instead to avoid redundancy.