Draft
Conversation
📝 WalkthroughWalkthroughAdds optional Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Engine
participant Config
participant Auth
participant vNextAPI as vNext API
participant LegacyAPI as Legacy API
Client->>Engine: quick_translate(..., engine_id?)
Engine->>Config: validate api_url, engine_id
Engine->>Engine: generate _session_id, set _is_vnext
Engine->>Auth: _ensure_client() -> choose header
alt vNext (engine_id present)
Auth-->>Engine: X-API-Key header
Engine->>vNextAPI: POST /process/{engine_id}/localize {sessionId, sourceLocale, targetLocale, data, ...}
vNextAPI-->>Engine: localized response
else Legacy (no engine_id)
Auth-->>Engine: Authorization: Bearer token
Engine->>LegacyAPI: POST /i18n or /recognize {...}
LegacyAPI-->>Engine: localized response
end
Engine->>Client: return translated content
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Adds vNext API support to the Python SDK by enabling an engine_id-driven mode that switches base URL/auth headers and routes certain calls to new vNext endpoints while keeping legacy behavior unchanged when engine_id is absent.
Changes:
- Extend
EngineConfigwith optionalengine_idand auto-selecthttps://api.lingo.devas the default whenengine_idis set. - Branch authentication (
X-API-KeyvsAuthorization: Bearer) and endpoint paths for_localize_chunk,recognize_locale, andwhoami(GET/users/mefor vNext). - Add
engine_idsupport toquick_translate/quick_batch_translateand introduce unit tests covering vNext behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/lingodotdev/engine.py |
Adds engine_id config, vNext detection, auth/header branching, and vNext endpoint routing. |
tests/test_engine.py |
Adds unit tests validating vNext URL/body/header behavior and the default API URL switch. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/lingodotdev/engine.py (1)
574-583: Update quick-helper docstrings forengine_id.
quick_translateandquick_batch_translatenow acceptengine_id, but the Args sections don’t document it yet.Also applies to: 636-645
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lingodotdev/engine.py` around lines 574 - 583, Update the docstrings for both quick_translate and quick_batch_translate to document the new engine_id parameter: add an Args entry describing engine_id (type Optional[str], purpose to override/select the specific engine instance, default behavior when None) and include any notes about compatibility with api_url or fast flags; ensure the parameter name matches the function signatures (engine_id) and appears in the Args sections near api_url/fast so readers can find it easily.tests/test_engine.py (1)
646-677: Add a regression test for empty/whitespaceengine_id.The new suite covers vNext happy paths well, but it should also assert that invalid values like
""/" "are rejected to prevent malformed vNext routing/auth behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_engine.py` around lines 646 - 677, Add regression tests for empty/whitespace engine_id by creating LingoDotDevEngine instances with engine_id = "" and engine_id = " " and asserting these are treated as invalid: verify engine._is_vnext is False, engine.config.engine_id is falsy/normalized (e.g. None or empty after stripping), and the api_url is not switched to "https://api.lingo.dev" (i.e. remains the non-vNext/default value); add these as new test methods alongside TestVNextEngine to prevent malformed vNext routing/auth behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lingodotdev/engine.py`:
- Around line 220-223: The vNext URL assembly uses f-strings with
self.config.api_url which can end with a trailing slash and produce double
slashes; fix by normalizing once before concatenation (e.g., base_api =
self.config.api_url.rstrip("/")) and then build URLs with
f"{base_api}/process/{self.config.engine_id}/localize" (and similarly for other
vNext endpoints). Update the occurrences where url is created under
self._is_vnext (including the blocks around the current url in the method using
self._is_vnext and the other two locations mentioned) to use the normalized
base_api variable so no double slashes are produced.
- Line 21: Reject empty-string engine_id values and make vNext detection
truthy-based: add validation where engine_id is set (e.g., in the Engine
dataclass __post_init__ or the constructor that assigns engine_id) to raise an
error if engine_id is None or engine_id.strip() == "" (use ValueError) so blank
strings don't pass config, and change the _is_vnext check to use a truthiness
test (e.g., return bool(self.engine_id) or bool(self.engine_id and
self.engine_id.strip())) instead of comparison to None so empty strings are
treated as falsy and will not produce malformed paths like /process//localize;
update all places referencing engine_id and _is_vnext accordingly.
In `@tests/test_engine.py`:
- Line 675: The file tests/test_engine.py fails Black formatting; run the
formatter (e.g., execute `black --write .` or `black tests/test_engine.py`) and
commit the changes so the instantiation line `engine =
LingoDotDevEngine({"api_key": "key", "api_url": "https://api.test.com"})` and
the other affected ranges (around lines with that instantiation and the blocks
at 693 and 708-710) are reformatted to comply with Black; ensure you run Black
locally, stage the modified file(s), and push the commit to unblock the CI.
---
Nitpick comments:
In `@src/lingodotdev/engine.py`:
- Around line 574-583: Update the docstrings for both quick_translate and
quick_batch_translate to document the new engine_id parameter: add an Args entry
describing engine_id (type Optional[str], purpose to override/select the
specific engine instance, default behavior when None) and include any notes
about compatibility with api_url or fast flags; ensure the parameter name
matches the function signatures (engine_id) and appears in the Args sections
near api_url/fast so readers can find it easily.
In `@tests/test_engine.py`:
- Around line 646-677: Add regression tests for empty/whitespace engine_id by
creating LingoDotDevEngine instances with engine_id = "" and engine_id = " "
and asserting these are treated as invalid: verify engine._is_vnext is False,
engine.config.engine_id is falsy/normalized (e.g. None or empty after
stripping), and the api_url is not switched to "https://api.lingo.dev" (i.e.
remains the non-vNext/default value); add these as new test methods alongside
TestVNextEngine to prevent malformed vNext routing/auth behavior.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lingodotdev/engine.py (1)
596-603:⚠️ Potential issue | 🟡 MinorDocument
engine_idin quick helperArgssections.
Line 590andLine 652addengine_id, but the docstrings still omit it.Docstring patch
Args: content: Text string or dict to translate api_key: Your Lingo.dev API key target_locale: Target language code (e.g., 'es', 'fr') source_locale: Source language code (optional, auto-detected if None) api_url: API endpoint URL fast: Enable fast mode for quicker translations + engine_id: Optional vNext engine ID (enables vNext routing when provided)Args: content: Text string or dict to translate api_key: Your Lingo.dev API key target_locales: List of target language codes (e.g., ['es', 'fr', 'de']) source_locale: Source language code (optional, auto-detected if None) api_url: API endpoint URL fast: Enable fast mode for quicker translations + engine_id: Optional vNext engine ID (enables vNext routing when provided)Also applies to: 658-665
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lingodotdev/engine.py` around lines 596 - 603, The docstrings for the quick helper(s) omitted the newly added engine_id parameter; update the Args sections for the quick helper functions that now accept engine_id to include a brief description for the engine_id parameter (e.g., "engine_id: ID of the translation engine to use (optional); defaults to the account's default engine"), ensuring you add the engine_id entry alongside the existing entries (content, api_key, target_locale, source_locale, api_url, fast) so both occurrences of the quick helper docstrings mention engine_id.
🧹 Nitpick comments (2)
src/lingodotdev/engine.py (1)
34-45: Normalize first, then apply default-host vNext switching.
Line 37checks for the default URL before trailing-slash normalization. Withengine_idset,api_url="https://engine.lingo.dev/"won’t switch tohttps://api.lingo.dev, even though it is the same default host form.Proposed adjustment
`@validator`("api_url", pre=True, always=True) `@classmethod` def validate_api_url(cls, v: Optional[str], values: Dict[str, Any]) -> str: - if v is None or v == "https://engine.lingo.dev": - engine_id = values.get("engine_id") - if engine_id: - return "https://api.lingo.dev" - if v is None: - return "https://engine.lingo.dev" - if not v.startswith(("http://", "https://")): + normalized = (v or "https://engine.lingo.dev").strip().rstrip("/") + if not normalized.startswith(("http://", "https://")): raise ValueError("API URL must be a valid HTTP/HTTPS URL") - return v.rstrip("/") + if values.get("engine_id") and normalized == "https://engine.lingo.dev": + return "https://api.lingo.dev" + return normalized🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lingodotdev/engine.py` around lines 34 - 45, The validator validate_api_url should normalize trailing slashes before comparing against the default host so that values like "https://engine.lingo.dev/" are treated the same as "https://engine.lingo.dev"; modify validate_api_url to strip a trailing "/" from v (if v is not None) at the start, then perform the engine_id-based host switch to "https://api.lingo.dev", then validate scheme with startswith and finally return the normalized value; reference api_url, validate_api_url, and engine_id when locating the code to change.tests/test_engine.py (1)
675-685: Add regression coverage for default legacy URL with trailing slash +engine_id.You cover custom trailing-slash normalization; add the canonical default case (
"https://engine.lingo.dev/") to lock expected vNext host switching behavior.Suggested test
def test_api_url_trailing_slash_stripped(self): """Test that trailing slash is stripped from api_url""" engine = LingoDotDevEngine( { "api_key": "key", "engine_id": "eng", "api_url": "https://custom.api.com/", } ) assert engine.config.api_url == "https://custom.api.com" + def test_default_legacy_url_with_trailing_slash_switches_to_vnext_host(self): + """Default legacy URL form with trailing slash should still route to api.lingo.dev.""" + engine = LingoDotDevEngine( + { + "api_key": "key", + "engine_id": "eng", + "api_url": "https://engine.lingo.dev/", + } + ) + assert engine.config.api_url == "https://api.lingo.dev"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_engine.py` around lines 675 - 685, Add a regression test that mirrors test_api_url_trailing_slash_stripped but uses the legacy default URL "https://engine.lingo.dev/" plus an engine_id to ensure trailing-slash normalization and the default vNext host behavior; create a new test (e.g., test_default_legacy_url_trailing_slash_with_engine_id) that constructs LingoDotDevEngine with {"api_key":"key","engine_id":"eng","api_url":"https://engine.lingo.dev/"} and asserts engine.config.api_url == "https://engine.lingo.dev" (or the expected vNext host if the code should transform it), using the LingoDotDevEngine constructor and config.api_url symbol references to locate the logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/lingodotdev/engine.py`:
- Around line 596-603: The docstrings for the quick helper(s) omitted the newly
added engine_id parameter; update the Args sections for the quick helper
functions that now accept engine_id to include a brief description for the
engine_id parameter (e.g., "engine_id: ID of the translation engine to use
(optional); defaults to the account's default engine"), ensuring you add the
engine_id entry alongside the existing entries (content, api_key, target_locale,
source_locale, api_url, fast) so both occurrences of the quick helper docstrings
mention engine_id.
---
Nitpick comments:
In `@src/lingodotdev/engine.py`:
- Around line 34-45: The validator validate_api_url should normalize trailing
slashes before comparing against the default host so that values like
"https://engine.lingo.dev/" are treated the same as "https://engine.lingo.dev";
modify validate_api_url to strip a trailing "/" from v (if v is not None) at the
start, then perform the engine_id-based host switch to "https://api.lingo.dev",
then validate scheme with startswith and finally return the normalized value;
reference api_url, validate_api_url, and engine_id when locating the code to
change.
In `@tests/test_engine.py`:
- Around line 675-685: Add a regression test that mirrors
test_api_url_trailing_slash_stripped but uses the legacy default URL
"https://engine.lingo.dev/" plus an engine_id to ensure trailing-slash
normalization and the default vNext host behavior; create a new test (e.g.,
test_default_legacy_url_trailing_slash_with_engine_id) that constructs
LingoDotDevEngine with
{"api_key":"key","engine_id":"eng","api_url":"https://engine.lingo.dev/"} and
asserts engine.config.api_url == "https://engine.lingo.dev" (or the expected
vNext host if the code should transform it), using the LingoDotDevEngine
constructor and config.api_url symbol references to locate the logic.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/test_engine.py (1)
655-658: Useasyncio.run()for async cleanup to avoid deprecation warning.
asyncio.get_event_loop()is deprecated in Python 3.10+ and will raise an error in Python 3.12+ when there is no current event loop. Replace withasyncio.run(), which is the recommended pattern for synchronous code that needs to run async operations.♻️ Suggested fix
def teardown_method(self): """Clean up engine client""" if self.engine._client and not self.engine._client.is_closed: - asyncio.get_event_loop().run_until_complete(self.engine.close()) + asyncio.run(self.engine.close())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_engine.py` around lines 655 - 658, The teardown_method currently uses asyncio.get_event_loop().run_until_complete to close the engine client, which triggers deprecation warnings; update teardown_method so that when self.engine._client exists and is not closed you call asyncio.run(self.engine.close()) instead of get_event_loop().run_until_complete, keeping the same condition around self.engine._client and self.engine._client.is_closed and invoking the existing engine.close() coroutine.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/test_engine.py`:
- Around line 655-658: The teardown_method currently uses
asyncio.get_event_loop().run_until_complete to close the engine client, which
triggers deprecation warnings; update teardown_method so that when
self.engine._client exists and is not closed you call
asyncio.run(self.engine.close()) instead of get_event_loop().run_until_complete,
keeping the same condition around self.engine._client and
self.engine._client.is_closed and invoking the existing engine.close()
coroutine.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Add vNext API support to the Python SDK. When engine_id is provided in config, the SDK uses the new api.lingo.dev API with X-API-Key auth and vNext endpoint paths. When engine_id is not provided, the SDK continues to use the legacy engine.lingo.dev API with Bearer token auth. All existing methods work transparently with both APIs.
Type of Change
Changes:
Testing
Checklist
Summary by CodeRabbit
New Features
Tests