Fix handling of empty steering responses in tool call middleware #637

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Open

matdev83 wants to merge 1 commit into dev

from codex/fix-a-bug-in-reactor-tool-97b52g

Open

Fix handling of empty steering responses in tool call middleware #637

matdev83 wants to merge 1 commit into dev from codex/fix-a-bug-in-reactor-tool-97b52g

+45 −1

Conversation

@matdev83

Copy link

Owner

@matdev83 matdev83 commented Oct 12, 2025 •

edited by coderabbitai bot

Loading

Summary

ensure the tool call reactor middleware still swallows calls when handlers return an empty replacement response
add a regression unit test covering the empty replacement scenario

Testing

python -m pytest -o addopts='' tests/unit/core/services/test_tool_call_reactor_middleware.py
python -m pytest -o addopts='' (fails: missing pytest_asyncio, respx, pytest_httpx optional dependencies)

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

Summary by CodeRabbit

Bug Fixes
- Empty replacement responses from tool-driven actions are now respected rather than ignored, preventing unintended fallback content and ensuring outputs can be intentionally blank when appropriate.
- Preserves related metadata flags to accurately reflect when a tool call was swallowed.
Tests
- Added unit test to validate behavior when a swallowed tool call provides an empty replacement, ensuring the response content is empty and metadata is correctly set.

@matdev83


 Fix empty replacement handling in tool call reactor

00a31b8

@matdev83 matdev83 added the codex label

Oct 12, 2025

— with ChatGPT Codex Connector

@coderabbitai

Copy link

coderabbitai bot commented Oct 12, 2025 •

edited

Loading

Walkthrough

Updated middleware to treat empty replacement responses as valid by switching from a truthiness check to an explicit None check. Added a unit test to validate that an empty string replacement produces empty content while marking the tool call as swallowed.

Changes

Cohort / File(s)	Summary
Middleware logic update `src/core/services/tool_call_reactor_middleware.py`	Changed condition from truthy check to `is not None` for `replacement_response`, allowing empty strings to be used as valid replacements.
Unit tests `tests/unit/core/services/test_tool_call_reactor_middleware.py`	Added test `test_process_with_empty_replacement` asserting empty replacement content is respected and `tool_call_swallowed` metadata is set; note duplication of the test within the file in the diff.

Sequence Diagram(s)

sequenceDiagram
 autonumber
 participant C as Client
 participant M as ToolCallReactorMiddleware
 participant T as Tool Call Handler
 C->>M: process(request)
 M->>T: evaluate tool call
 T-->>M: swallow_result (replacement_response may be "", None, or text)
 alt replacement_response is not None
 note right of M: Use replacement even if ""
 M-->>C: ProcessedResponse(content = replacement_response, metadata.tool_call_swallowed = true)
 else replacement_response is None
 M-->>C: ProcessedResponse(content = original, metadata.tool_call_swallowed = false)
 end

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I nibbled the code, a delicate bite,
Turned "falsy" to fine in the moonlit night.
An empty whisper now counts as a say,
Middleware dances the swallow-away.
Tests hop beside, two footprints in snow—
A bunny approves: on with the flow! 🐇✨

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 identifies the main change—improving the middleware’s handling of empty steering responses—and matches the adjustments made to the tool call reactor and related tests in this PR. It is concise, specific, and directly reflects the intent without extraneous detail.
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-a-bug-in-reactor-tool-97b52g

📜 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 00a31b8.

📒 Files selected for processing (2)

src/core/services/tool_call_reactor_middleware.py (1 hunks)
tests/unit/core/services/test_tool_call_reactor_middleware.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:

src/core/services/tool_call_reactor_middleware.py
tests/unit/core/services/test_tool_call_reactor_middleware.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_middleware.py

🧬 Code graph analysis (1)

tests/unit/core/services/test_tool_call_reactor_middleware.py (4)

tests/unit/test_tool_call_loop_middleware.py (1)

tool_call_response (43-72)

src/core/interfaces/response_processor_interface.py (1)

ProcessedResponse (8-30)

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

ToolCallReactionResult (47-57)

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

process (57-220)

⏰ 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: Analyze (python)

🔇 Additional comments (2)

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

198-198: LGTM! Critical fix for empty replacement handling.

Switching from truthiness check to explicit is not None correctly allows empty strings as valid replacement content. This aligns with the type signature str | None for replacement_response and enables handlers to swallow tool calls silently by returning an empty string.

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

309-352: LGTM! Solid regression test.

This test properly validates the empty replacement scenario, ensuring that handlers can swallow tool calls with empty content while still marking them as swallowed in metadata. The assertions are comprehensive and align with the middleware fix.

Note: The AI summary mentions test duplication, but I only see this test defined once in the provided code.

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

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix handling of empty steering responses in tool call middleware #637

Are you sure you want to change the base?

Fix handling of empty steering responses in tool call middleware #637

Conversation

@matdev83 matdev83 commented Oct 12, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

Summary

Testing

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Oct 12, 2025 •

edited

Loading

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Fix handling of empty steering responses in tool call middleware #637

Are you sure you want to change the base?

Fix handling of empty steering responses in tool call middleware #637

Conversation

@matdev83 matdev83 commented Oct 12, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Oct 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

@matdev83 matdev83 commented Oct 12, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Oct 12, 2025 •

edited

Loading