Skip to content

Conversation

@matdev83
Copy link
Owner

@matdev83 matdev83 commented Oct 12, 2025

Summary

  • expose the configured buffer cap from ToolCallRepairService
  • enforce the cap inside the streaming tool call repair processor to drop excess data
  • cover the new safeguards with unit tests for truncation and zero-cap behaviour

Testing

  • python -m pytest -o addopts="" tests/unit/core/services/test_tool_call_repair.py (fails: ModuleNotFoundError: No module named 'pytest_mock')
  • python -m pytest -o addopts="" (fails: multiple ModuleNotFoundError exceptions for missing dev dependencies such as pytest_asyncio, pytest_httpx, respx, hypothesis)

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

Summary by CodeRabbit

  • New Features
    • Introduced configurable per-stream buffer limit for tool-call repair during streaming, with a sensible default.
  • Bug Fixes
    • Correctly enforces buffer caps using byte-size (UTF-8) measurement, trimming oldest data when limits are exceeded and handling zero/unlimited configurations predictably.
    • Improved handling of invalid or negative configuration values.
  • Tests
    • Added unit tests covering buffer-cap behavior, including over-cap trimming and zero-cap scenarios, to ensure consistent streaming behavior.

@coderabbitai
Copy link

coderabbitai bot commented Oct 12, 2025

Walkthrough

Adds per-stream buffer cap enforcement to ToolCallRepairProcessor and exposes max_buffer_bytes configuration via ToolCallRepairService. Processor computes and caches a cap, enforces it per stream (UTF-8 byte-sized, trims oldest on overflow, special cases 0 and negative), and integrates it into processing. Tests cover overflow and zero-cap behaviors.

Changes

Cohort / File(s) Summary
Processor buffering and enforcement
src/core/services/streaming/tool_call_repair_processor.py
Adds cap resolution and per-stream buffer enforcement methods; integrates cap application into processing loop; handles defaults, invalid, zero, and negative caps; trims oldest bytes using UTF-8 sizing and warns on drops.
Service configuration for buffer cap
src/core/services/tool_call_repair_service.py
Normalizes max_buffer_bytes in constructor (int cast, default 64 KiB, clamp negatives to 0); adds public accessor property max_buffer_bytes.
Unit tests for buffering behavior
tests/unit/core/services/test_tool_call_repair.py
Adds async tests for cap overflow trimming and zero-cap behavior; patches repair invocation; adjusts imports (move StreamingContent import to top).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Client
  participant Stream as Stream Source
  participant Processor as ToolCallRepairProcessor
  participant Service as ToolCallRepairService
  participant Repair as Repair Logic

  Note over Processor,Service: Initialization
  Service->>Service: Normalize max_buffer_bytes (int/default/clamp)
  Processor->>Service: Read max_buffer_bytes
  Processor->>Processor: Cache resolved buffer cap

  Client->>Stream: Send chunk(s)
  loop For each chunk
    Stream->>Processor: on_chunk(data)
    Processor->>Processor: Append to per-stream buffer
    Processor->>Processor: Enforce cap (UTF-8 size, trim oldest if exceeded)
    alt Cap == 0
      Processor->>Processor: Drop buffer and warn
    else Cap < 0
      Processor->>Processor: Allow unlimited
    end
    Processor->>Repair: Attempt tool_call repair with current buffer
    Repair-->>Processor: Result/No-op
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

I buffered my bytes in a tidy heap,
Nibbled the oldest when the cap ran deep.
Zero? I drop them—quick hop, no fuss!
Negative? Boundless—carrots for us.
With trims and tests, I thump in delight—
Streams stay swift, and repairs hop right. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly describes the main change by stating that bounds are being added to the streaming tool call buffer, which directly corresponds to the implementation of per-stream buffering caps in the processor and service. It uses clear, specific language without extraneous details, making it easy for a reviewer to understand the change at a glance. This aligns with best practices for concise and meaningful PR titles.
✨ 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-remote-attack-vector-bug-u6qsgh

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/core/services/streaming/tool_call_repair_processor.py (1)

141-142: Remove unreachable negative cap check.

Lines 141-142 check for cap < 0, but this condition is unreachable because _resolve_buffer_cap (line 121-126) already normalizes negative values to 0 during initialization.

Apply this diff to remove the dead code:

-        if cap < 0:
-            return buffer
-
         if not buffer:
📜 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 b4f8476.

📒 Files selected for processing (3)
  • src/core/services/streaming/tool_call_repair_processor.py (3 hunks)
  • src/core/services/tool_call_repair_service.py (1 hunks)
  • tests/unit/core/services/test_tool_call_repair.py (2 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_repair_service.py
  • src/core/services/streaming/tool_call_repair_processor.py
  • tests/unit/core/services/test_tool_call_repair.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_repair_service.py
  • src/core/services/streaming/tool_call_repair_processor.py
🧬 Code graph analysis (2)
src/core/services/streaming/tool_call_repair_processor.py (1)
src/core/interfaces/tool_call_repair_service_interface.py (1)
  • IToolCallRepairService (7-21)
tests/unit/core/services/test_tool_call_repair.py (3)
src/core/domain/streaming_content.py (1)
  • StreamingContent (10-158)
src/core/services/tool_call_repair_service.py (2)
  • ToolCallRepairService (12-218)
  • max_buffer_bytes (45-48)
src/core/services/streaming/tool_call_repair_processor.py (2)
  • ToolCallRepairProcessor (18-160)
  • process (29-105)
⏰ 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_repair_service.py (1)

38-48: LGTM! Clean buffer cap implementation.

The initialization and property exposure are well-implemented:

  • Defensive casting with sensible defaults (64 KiB)
  • Proper handling of negative values (clamped to 0)
  • Clear documentation
tests/unit/core/services/test_tool_call_repair.py (2)

6-6: Good refactoring.

Moving the StreamingContent import to module level follows Python best practices and improves readability.


154-195: Excellent test coverage for buffer cap enforcement.

These tests comprehensively verify the new buffering behavior:

  • Truncation when the cap is exceeded
  • Buffer dropping when cap is zero
  • Proper interaction with the repair service

The tests validate both the stored buffer state and the data passed to the repair logic, ensuring end-to-end correctness.

src/core/services/streaming/tool_call_repair_processor.py (4)

27-27: Good initialization.

Caching the buffer cap at initialization time is efficient and follows the principle of computing configuration once.


40-40: Correct enforcement placement.

Enforcing the cap immediately after buffer accumulation ensures that the buffer never exceeds the configured limit before being processed.


107-127: Robust buffer cap resolution.

The defensive implementation handles invalid inputs gracefully:

  • Safe attribute access with default fallback
  • Type validation with fallback to default
  • Normalization of negative values to zero with warning

This ensures the processor always has a valid cap regardless of the service implementation.


129-160: Solid enforcement logic with correct truncation strategy.

The implementation correctly:

  • Drops buffers entirely when cap is zero (lines 133-139)
  • Measures buffer size in UTF-8 bytes for accuracy (lines 147-148)
  • Keeps the newest data by truncating from the end (line 152), which is appropriate for streaming scenarios
  • Handles UTF-8 decoding errors gracefully (line 160)
  • Provides clear warning logs with relevant details (lines 154-159)

The truncation strategy (keeping the last cap bytes) ensures the most recent streaming data is retained, which aligns well with the goal of detecting tool calls in ongoing streams.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant