Handle sync command service handlers

[matdev83]

Summary

• ensure command service adapters accept synchronous handler functions by normalizing return types
• add a regression test covering synchronous handler adaptation for ensure_command_service

Testing

• python -m pytest --override-ini addopts='' tests/unit/core/test_command_service_module.py
• python -m pytest --override-ini addopts=''

---

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

Summary by CodeRabbit

• New Features

  • Command processing now supports both synchronous and asynchronous handlers, improving flexibility.
  • Clearer runtime errors when handlers return invalid results.

• Tests

  • Added unit tests to verify synchronous callables are correctly wrapped and processed, ensuring expected outputs and flags remain accurate.

• Documentation

  • Clarified behavior around acceptable handler return types during command processing.

[coderabbitai]

Walkthrough

Updated FunctionCommandService.process_commands to support both synchronous and asynchronous handler results by detecting awaitables and returning ProcessedResult accordingly, with a TypeError for invalid returns. Added a unit test ensuring ensure_command_service correctly wraps a synchronous callable into an ICommandService.

Changes

Cohort / File(s)	Summary
Core command processing src/core/interfaces/command_service.py	Added inspect import; modified process_commands to handle awaitable handler results vs direct ProcessedResult; added runtime type check and error for invalid return types.
Unit tests tests/unit/core/test_command_service_module.py	Added test verifying ensure_command_service wraps a sync callable, producing expected ProcessedResult (uppercased messages, command_executed True, session in command_results).

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor Caller
    participant FCS as FunctionCommandService
    participant Handler
    participant PR as ProcessedResult

    Caller->>FCS: process_commands(commands, session)
    FCS->>Handler: invoke handler(commands, session)
    alt Handler returns awaitable
        Handler-->>FCS: Awaitable<ProcessedResult>
        FCS->>FCS: await result
        FCS-->>Caller: ProcessedResult
    else Handler returns ProcessedResult
        Handler-->>FCS: ProcessedResult
        FCS-->>Caller: ProcessedResult
    else Invalid return
        Handler-->>FCS: Other type
        FCS-->>Caller: TypeError
    end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I twitch my whiskers at sync and async calls,
Now both can hop through FunctionCommand halls.
With tests that thump in rhythmic delight,
We uppercase carrots in the moonlit night.
Results in the basket, neatly aligned—
A happy hare with a well-trained mind. 🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 16.67% 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 conveys the primary change of supporting synchronous handlers in the command service, directly reflecting the updates in process_commands and accompanying tests. It is concise and specific without any extraneous wording.

✨ 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-identified-frontend-bug-cidtne

---

📜 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 f4bc865.

📒 Files selected for processing (2)

• src/core/interfaces/command_service.py (2 hunks)
• tests/unit/core/test_command_service_module.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:

• tests/unit/core/test_command_service_module.py
• src/core/interfaces/command_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/interfaces/command_service.py

🧬 Code graph analysis (2)

tests/unit/core/test_command_service_module.py (2)

src/core/domain/processed_result.py (1)

• ProcessedResult (8-19)

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

• ensure_command_service (42-68)
• process_commands (25-39)

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

src/core/domain/processed_result.py (1)

• ProcessedResult (8-19)

🪛 Ruff (0.13.3)

src/core/interfaces/command_service.py

36-39: Avoid specifying long messages outside the exception class

(TRY003)

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

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

28-39: LGTM! Sync/async handler detection is correct.

The implementation correctly detects awaitable results using inspect.isawaitable, awaits them when necessary, returns immediate ProcessedResult values directly, and raises a clear TypeError for invalid returns. This properly normalizes both synchronous and asynchronous handler functions.

tests/unit/core/test_command_service_module.py (1)

52-67: LGTM! Comprehensive regression test for sync handler support.

The test properly validates that ensure_command_service correctly wraps a synchronous callable, verifying that the resulting ProcessedResult reflects the expected uppercased messages, command_executed is True, and command_results contains the session. This aligns well with the existing async wrapping test pattern and provides good coverage for the new functionality.

---

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

[matdev83]

PR #644 - Handle sync command service handlers - ALREADY IMPLEMENTED

Analysis Summary

This PR addressed the issue of ensuring command service adapters accept synchronous handler functions by normalizing return types.

Current Status: ALREADY IMPLEMENTED

After thorough analysis of the current codebase, all functionality described in this PR is already implemented and working correctly:

✅ Implementation Already Present

1. Sync/Async Handler Detection: The FunctionCommandService.process_commands() method already includes:

   • inspect.isawaitable() detection for handler results
   • Proper awaiting of async results
   • Direct return of sync ProcessedResult values
   • Clear TypeError for invalid return types

2. Type Support: The CommandServiceHandler type alias already supports:
   python CommandServiceHandler = Callable[ [list[Any], str], Awaitable[ProcessedResult] | ProcessedResult ]

3. Regression Test: The est_ensure_command_service_wraps_sync_callable() test is already present and validates:

   • Sync callable wrapping by �nsure_command_service()
   • Proper processing of sync handler results
   • Correct ProcessedResult with uppercased messages

✅ Test Results

All related tests pass successfully:

• est_ensure_command_service_accepts_valid_service - PASSED
• est_ensure_command_service_wraps_async_callable - PASSED
• est_ensure_command_service_wraps_sync_callable - PASSED
• est_ensure_command_service_rejects_none - PASSED
• est_ensure_command_service_rejects_invalid_type - PASSED

✅ Code Quality

• Implementation follows project coding standards
• Proper error handling with specific exceptions
• Comprehensive test coverage
• No linting issues

Conclusion

The functionality described in PR #644 has already been implemented in the current codebase. The project now properly supports both synchronous and asynchronous command service handlers with appropriate type detection, error handling, and comprehensive test coverage.

No further action is needed as this improvement is already available in the main development branch.