Migrate some required functions from plugin sdk to lab facade#1363
Merged
Migrate some required functions from plugin sdk to lab facade#1363
Conversation
…ab-app into migrate/plugin-sdk-local-provider
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new library-agnostic text generation module with a Local HTTP backend, exposes generation model creation and a synchronous job-data setter on the Lab facade, and bumps package versions/dependencies to 0.0.81; also adds a smoke test for loading the local generation model. Changes
Sequence DiagramsequenceDiagram
participant Client as Lab Client
participant Facade as Lab Facade
participant GenModule as generation.py
participant HTTPServer as Local HTTP Server
Client->>Facade: load_generation_model(config)
Facade->>GenModule: load_generation_model(config)
GenModule->>GenModule: Normalize config & validate provider
GenModule->>GenModule: Create LocalHTTPGenerationModel(base_url, model, api_key)
GenModule-->>Facade: Return GenerationModel instance
Facade-->>Client: Return GenerationModel instance
Client->>Facade: set_job_data_field(key, value)
Facade->>Facade: Call async updater synchronously
Facade-->>Client: Field updated
Client->>GenModule: generate(prompt, system_prompt)
GenModule->>HTTPServer: POST /chat/completions (JSON)
HTTPServer-->>GenModule: JSON response with messages
GenModule->>GenModule: Parse & extract first message content
GenModule-->>Client: Return generated text
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lab-sdk/src/lab/generation.py`:
- Line 9: The lab.generation module imports the external library requests but
that package is not listed in the lab-sdk project's dependencies; update
lab-sdk/pyproject.toml to add "requests" (with an appropriate version
constraint) to the project dependencies so installs include it and runtime
import errors are avoided—ensure the new entry is placed in the
[tool.poetry.dependencies] or equivalent dependencies section that the project
uses.
- Around line 12-29: Add proper type hints to _run_async: declare a TypeVar T
and annotate the parameter as coro: Coroutine[Any, Any, T] and the return type
as -> T; also import TypeVar, Coroutine, and Any from typing at the top of the
module. Update the signature to def _run_async(coro: Coroutine[Any, Any, T]) ->
T: and add the corresponding from typing import TypeVar, Coroutine, Any
(ensuring no unused-import errors) so static type checkers can validate the
async/sync bridging in the _run_async helper.
In `@lab-sdk/tests/test_lab_facade.py`:
- Around line 1003-1028: Add type hints to the pytest fixtures in the test
function test_lab_load_generation_model_smoke: annotate tmp_path as Path and
monkeypatch as pytest.MonkeyPatch, and give the function an explicit return type
of None; also add the required imports "from pathlib import Path" and "import
pytest" at the top of the file (or apply equivalent typing across all test
functions if you prefer whole-file consistency).
Comment on lines
+12
to
+29
| def _run_async(coro): | ||
| """ | ||
| Helper to run async code from sync context. | ||
| Mirrors lab_facade._run_async behavior. | ||
| """ | ||
| try: | ||
| asyncio.get_running_loop() | ||
| running = True | ||
| except RuntimeError: | ||
| running = False | ||
|
|
||
| if not running: | ||
| return asyncio.run(coro) | ||
|
|
||
| loop = asyncio.get_event_loop() | ||
| if loop.is_closed(): | ||
| return asyncio.run(coro) | ||
| return loop.run_until_complete(coro) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n lab-sdk/src/lab/generation.py | head -40Repository: transformerlab/transformerlab-app
Length of output: 1216
🏁 Script executed:
rg -n "def _run_async" lab-sdk/src/lab/generation.py -A 20Repository: transformerlab/transformerlab-app
Length of output: 588
🏁 Script executed:
rg -n "^from typing import|^import" lab-sdk/src/lab/generation.py | head -15Repository: transformerlab/transformerlab-app
Length of output: 178
Add type hints to _run_async.
The function signature is missing argument and return type hints, which violates the mandatory type hints requirement for Python files.
✍️ Suggested update
-from typing import Any, Dict, Optional
+from typing import Any, Coroutine, Dict, Optional, TypeVar
+
+T = TypeVar("T")
-def _run_async(coro):
+def _run_async(coro: Coroutine[Any, Any, T]) -> T:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lab-sdk/src/lab/generation.py` around lines 12 - 29, Add proper type hints to
_run_async: declare a TypeVar T and annotate the parameter as coro:
Coroutine[Any, Any, T] and the return type as -> T; also import TypeVar,
Coroutine, and Any from typing at the top of the module. Update the signature to
def _run_async(coro: Coroutine[Any, Any, T]) -> T: and add the corresponding
from typing import TypeVar, Coroutine, Any (ensuring no unused-import errors) so
static type checkers can validate the async/sync bridging in the _run_async
helper.
Comment on lines
+1003
to
+1028
| def test_lab_load_generation_model_smoke(tmp_path, monkeypatch): | ||
| _fresh(monkeypatch) | ||
| home = tmp_path / ".tfl_home" | ||
| ws = tmp_path / ".tfl_ws" | ||
| home.mkdir() | ||
| ws.mkdir() | ||
| monkeypatch.setenv("TFL_HOME_DIR", str(home)) | ||
| monkeypatch.setenv("TFL_WORKSPACE_DIR", str(ws)) | ||
|
|
||
| from lab.lab_facade import Lab | ||
| from lab.generation import LocalHTTPGenerationModel | ||
|
|
||
| lab = Lab() | ||
| # load_generation_model should NOT require lab.init; it's stateless config | ||
| model = lab.load_generation_model( | ||
| { | ||
| "provider": "local", | ||
| "base_url": "http://localhost:9999/v1", | ||
| "model": "test-model", | ||
| "api_key": "test-key", | ||
| } | ||
| ) | ||
|
|
||
| assert isinstance(model, LocalHTTPGenerationModel) | ||
| assert model.base_url == "http://localhost:9999/v1" | ||
| assert model.model == "test-model" |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the test function signature and surrounding code
rg -n --type=py -C5 "def test_lab_load_generation_model_smoke" lab-sdk/tests/test_lab_facade.pyRepository: transformerlab/transformerlab-app
Length of output: 432
🏁 Script executed:
#!/bin/bash
# Check the current imports in the test file
head -50 lab-sdk/tests/test_lab_facade.py | rg -n "^import|^from"Repository: transformerlab/transformerlab-app
Length of output: 138
🏁 Script executed:
#!/bin/bash
# Check if Path and pytest are already imported in the file
rg -n "from pathlib import|import pytest" lab-sdk/tests/test_lab_facade.pyRepository: transformerlab/transformerlab-app
Length of output: 59
🏁 Script executed:
#!/bin/bash
# Search entire file for any pytest or Path imports
rg -n "pytest|from pathlib" lab-sdk/tests/test_lab_facade.pyRepository: transformerlab/transformerlab-app
Length of output: 59
🏁 Script executed:
#!/bin/bash
# Check a few other test function signatures to see if they have type hints
rg -n "^def test_" lab-sdk/tests/test_lab_facade.py | head -10Repository: transformerlab/transformerlab-app
Length of output: 625
🏁 Script executed:
#!/bin/bash
# Get the complete imports section of the file
sed -n '1,100p' lab-sdk/tests/test_lab_facade.py | cat -nRepository: transformerlab/transformerlab-app
Length of output: 3494
Add type hints for pytest fixtures in the new smoke test.
The new test function lacks argument and return type hints, which violates the mandatory type hints coding guideline. However, note that the entire test file currently lacks type hints on all test functions; adding them to this function alone would create an inconsistency with the rest of the file. Consider either:
- Adding type hints only to this function (requires importing
Pathfrompathlibandpytest.MonkeyPatch) - Applying type hints across all test functions in the file for consistency
✍️ Suggested update
-def test_lab_load_generation_model_smoke(tmp_path, monkeypatch):
+def test_lab_load_generation_model_smoke(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:Requires adding these imports at the top of the file if not already present:
from pathlib import Path
import pytest🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lab-sdk/tests/test_lab_facade.py` around lines 1003 - 1028, Add type hints to
the pytest fixtures in the test function test_lab_load_generation_model_smoke:
annotate tmp_path as Path and monkeypatch as pytest.MonkeyPatch, and give the
function an explicit return type of None; also add the required imports "from
pathlib import Path" and "import pytest" at the top of the file (or apply
equivalent typing across all test functions if you prefer whole-file
consistency).
dadmobile
requested changes
Feb 23, 2026
dadmobile
approved these changes
Feb 23, 2026
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
2 participants
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.
Summary by CodeRabbit
Release Notes - Version 0.0.81
New Features
Chores
Tests