Avoid redundant canonical chat request cloning

[matdev83]

Summary

• return canonical chat requests directly in TranslationService.to_domain_request instead of cloning them
• add a regression test to ensure canonical requests bypass re-validation and reuse the existing object

Testing

• python -m pytest -o addopts= tests/unit/core/services/test_translation_service.py::test_to_domain_request_returns_same_instance_for_canonical_request
• python -m pytest -o addopts= (fails: missing optional dev dependencies such as pytest-asyncio, respx, pytest-httpx)

---

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

Summary by CodeRabbit

• Refactor
  • Clarified internal request handling by using explicit branches for different input types, improving readability and maintainability without changing behavior or outputs.
• Tests
  • Added a unit test to verify canonical requests are returned unchanged, reinforcing contract and preventing regressions.

No user-facing changes; existing translation behavior remains the same.

[coderabbitai]

Walkthrough

Refactors TranslationService.to_domain_request to use explicit type-guard branches for canonical and chat requests, preserving behavior. Adds a unit test asserting that canonical requests are returned by identity. No public API changes or other logic modifications.

Changes

Cohort / File(s)	Summary
Service logic refactor src/core/services/translation_service.py	Replaced a combined isinstance check with two explicit branches: return canonical requests as-is; convert chat requests to canonical. Other pathways unchanged.
Unit tests tests/unit/core/services/test_translation_service.py	Added test verifying to_domain_request returns the same instance for CanonicalChatRequest inputs.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor Caller
    participant TS as TranslationService
    Caller->>TS: to_domain_request(request, provider)
    alt request is Canonical
        note right of TS: Return same instance (identity)
        TS-->>Caller: CanonicalChatRequest (same object)
    else request is ChatRequest
        note right of TS: Convert to canonical
        TS-->>Caller: CanonicalChatRequest (new object)
    else other/unchanged paths
        note right of TS: Existing handling unchanged
        TS-->>Caller: Result per existing logic
    end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I twitch my whiskers at branches two,
One path returns the same—who knew?
The other hops to shape and mold,
From chat to canon, neat and bold.
Tests now cheer with thumps of glee,
Identity preserved—carrot tea! 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.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 summarizes the main change of preventing redundant cloning of canonical chat requests in TranslationService and directly returning the existing instance. It is clear, specific, and immediately conveys the primary intent of the pull request to readers scanning the history.

✨ 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-1je7uj

---

📜 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 47cd9c3.

📒 Files selected for processing (2)

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

• src/core/services/translation_service.py
• tests/unit/core/services/test_translation_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/translation_service.py

🧬 Code graph analysis (1)

tests/unit/core/services/test_translation_service.py (2)

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

• to_domain_request (98-175)

src/core/domain/chat.py (2)

• CanonicalChatRequest (321-324)
• ChatMessage (90-138)

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

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

15-25: LGTM! Test correctly verifies the optimization.

The test appropriately uses an identity check (is) to confirm that canonical requests are returned without cloning, which aligns with the PR's performance optimization goal.

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

133-136: LGTM! Refactoring achieves the performance goal without breaking functionality.

The explicit type guards correctly handle the class hierarchy (CanonicalChatRequest extends ChatRequest) by checking the more specific type first. This optimization avoids unnecessary model_dump() and model_validate() calls for canonical requests while preserving the conversion behavior for non-canonical ChatRequest instances.

---

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

[matdev83]

This optimization is already present in the current dev branch at lines 136-137 in src/core/services/translation_service.py. The PR changes have been merged.