Skip to content

Ensure Anthropic fallback request processor keeps app state#608

Merged
matdev83 merged 1 commit intodevfrom
codex/fix-improper-di-usage-in-codebase
Oct 15, 2025
Merged

Ensure Anthropic fallback request processor keeps app state#608
matdev83 merged 1 commit intodevfrom
codex/fix-improper-di-usage-in-codebase

Conversation

@matdev83
Copy link
Copy Markdown
Owner

@matdev83 matdev83 commented Oct 12, 2025

Summary

  • ensure the Anthropic controller's manual RequestProcessor fallback wires through the application state dependency
  • add a regression test covering the fallback path to verify the request processor retains the DI-provided application state

Testing

  • python -m pytest -o addopts='' tests/unit/anthropic_frontend_tests/test_anthropic_controller_di_fallback.py
  • python -m pytest -o addopts=''

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

Summary by CodeRabbit

  • Refactor
    • Improved request processing by integrating application state, enhancing consistency and reliability across controller operations. No user-facing changes.
  • Tests
    • Added unit tests to verify fallback behavior and correct state propagation in the controller when certain services are not explicitly registered, strengthening overall stability.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Oct 12, 2025

Walkthrough

Adds app_state to RequestProcessor construction in the Anthropic controller. Introduces a unit test verifying DI fallback constructs a RequestProcessor with ApplicationStateService when IRequestProcessor is absent.

Changes

Cohort / File(s) Summary
Controller wiring
src/core/app/controllers/anthropic_controller.py
Passes app_state=app_state when creating RequestProcessor, aligning with the updated constructor signature.
Tests: DI fallback
tests/unit/anthropic_frontend_tests/test_anthropic_controller_di_fallback.py
Adds stubs and a test ensuring that, without IRequestProcessor in DI, the controller’s fallback RequestProcessor is created and receives ApplicationStateService.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Test as Test
  participant DI as ServiceProvider
  participant C as AnthropicController
  participant RP as RequestProcessor
  participant AS as ApplicationStateService

  Test->>DI: get_anthropic_controller()
  note right of DI: IRequestProcessor not registered
  DI-->>C: AnthropicController (constructed)
  C->>DI: Resolve dependencies
  DI-->>AS: ApplicationStateService
  C->>RP: new RequestProcessor(app_state=AS)
  note over C,RP: Fallback path wires app_state
  Test-->>C: Assertions on internal RP.app_state
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

In burrows of code, I twitch my nose,
A stateful carrot now neatly flows.
When DI’s pantry forgets a treat,
The fallback chef makes it complete.
Requestors nibble, tests hop with cheer—
App state snug, no bugs to fear. 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 18.75% 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 primary change—ensuring the fallback request processor in the Anthropic controller retains the application state—and directly reflects the modifications made in the DI wiring and added test coverage without extraneous detail.
✨ 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-improper-di-usage-in-codebase

📜 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 d89f274 and 0ada648.

📒 Files selected for processing (2)
  • src/core/app/controllers/anthropic_controller.py (1 hunks)
  • tests/unit/anthropic_frontend_tests/test_anthropic_controller_di_fallback.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/app/controllers/anthropic_controller.py
  • tests/unit/anthropic_frontend_tests/test_anthropic_controller_di_fallback.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/app/controllers/anthropic_controller.py
🧬 Code graph analysis (2)
src/core/app/controllers/anthropic_controller.py (1)
tests/unit/core/services/test_edit_precision_response_middleware.py (1)
  • app_state (18-20)
tests/unit/anthropic_frontend_tests/test_anthropic_controller_di_fallback.py (3)
src/core/app/controllers/anthropic_controller.py (2)
  • AnthropicController (39-380)
  • get_anthropic_controller (383-566)
src/core/di/container.py (3)
  • ServiceCollection (250-445)
  • add_instance (324-333)
  • build_service_provider (335-337)
src/core/services/application_state_service.py (1)
  • ApplicationStateService (18-237)
🪛 Ruff (0.13.3)
tests/unit/anthropic_frontend_tests/test_anthropic_controller_di_fallback.py

38-38: Unused method argument: session_id

(ARG002)


50-50: Unused method argument: request

(ARG002)


52-52: Unused method argument: allow_failover

(ARG002)


53-53: Unused method argument: context

(ARG002)


64-64: Unused method argument: backend

(ARG002)


64-64: Unused method argument: model

(ARG002)


73-73: Unused method argument: session_id

(ARG002)


74-74: Unused method argument: context

(ARG002)


79-79: Unused method argument: session_id

(ARG002)


88-88: Unused method argument: middleware

(ARG002)


88-88: Unused method argument: priority

(ARG002)


100-100: Unused method argument: context

(ARG002)


101-101: Unused method argument: session_id

(ARG002)


102-102: Unused method argument: backend

(ARG002)


103-103: Unused method argument: model

(ARG002)


104-104: Unused method argument: key_name

(ARG002)


105-105: Unused method argument: request_payload

(ARG002)


112-112: Unused method argument: context

(ARG002)


113-113: Unused method argument: session_id

(ARG002)


114-114: Unused method argument: backend

(ARG002)


115-115: Unused method argument: model

(ARG002)


116-116: Unused method argument: key_name

(ARG002)


117-117: Unused method argument: response_content

(ARG002)


124-124: Unused method argument: context

(ARG002)


125-125: Unused method argument: session_id

(ARG002)


126-126: Unused method argument: backend

(ARG002)


127-127: Unused method argument: model

(ARG002)


128-128: Unused method argument: key_name

(ARG002)

⏰ 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 (5)
src/core/app/controllers/anthropic_controller.py (1)

546-546: LGTM! Correct wiring of app_state dependency.

The addition of app_state=app_state properly passes the DI-resolved ApplicationStateService instance to the fallback-constructed RequestProcessor. This ensures the processor has access to application state even when IRequestProcessor is not registered in the provider.

tests/unit/anthropic_frontend_tests/test_anthropic_controller_di_fallback.py (4)

1-34: LGTM! Clean import structure.

The imports follow the coding guidelines correctly with standard library, third-party, and local imports separated appropriately.


36-135: Stub implementations are minimal and appropriate for testing.

The stub classes (_StubCommandService, _StubBackendService, _StubResponseProcessor, _StubWireCapture) correctly implement their respective interfaces with minimal behavior needed for testing the DI fallback path.

Static analysis flags unused method parameters throughout these stubs. These warnings are false positives—the parameters are required for interface compliance and intentionally unused in these minimal test doubles.


137-177: LGTM! Comprehensive test provider setup.

The helper function correctly builds a service provider with all necessary dependencies except IRequestProcessor, which triggers the fallback construction path being tested. The setup is thorough and includes all services required by the fallback logic.


179-191: LGTM! Test validates the regression scenario effectively.

The test correctly verifies that:

  1. IRequestProcessor is absent from the provider (line 184)
  2. The fallback construction succeeds and returns a valid AnthropicController (lines 186-187)
  3. The fallback-constructed RequestProcessor receives the DI-managed ApplicationStateService instance (line 191)

This provides good regression coverage for the PR's objective of ensuring app_state is wired through the fallback path.


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

@codecov
Copy link
Copy Markdown

codecov Bot commented Oct 12, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@matdev83
Copy link
Copy Markdown
Owner Author

APPROVED

This PR successfully fixes the DI issue in the Anthropic controller's fallback RequestProcessor path.

Review Findings:

  • ✅ Minimal, focused change addressing the specific dependency injection problem
  • ✅ Added comprehensive test coverage for the fallback behavior
  • ✅ All Anthropic-related tests pass (66/66)
  • ✅ Backward compatible - app_state is optional parameter
  • ✅ Ensures proper dependency propagation in fallback scenarios

The fix adds the missing parameter when creating RequestProcessor in the fallback path, preventing potential issues where the fallback processor wouldn't have access to application state.

Safe to merge. 🚀

@matdev83 matdev83 merged commit 465b0bc into dev Oct 15, 2025
7 checks passed
@matdev83 matdev83 deleted the codex/fix-improper-di-usage-in-codebase branch November 4, 2025 22:54
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