-
-
Notifications
You must be signed in to change notification settings - Fork 6.3k
Let recipes use the model loaded in Chat #4840
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 13 commits
aa438ce
0beffea
19d657a
694d3c4
b4e9ff6
8028495
754eb54
b351ffa
c83b3db
b3ae2dd
100ba0b
4c997e9
1b19db5
10f1fea
1259ebd
1cbc8a3
d21b077
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,9 +23,96 @@ | |
| RecipePayload, | ||
| ) | ||
|
|
||
| from datetime import timedelta | ||
|
|
||
| router = APIRouter() | ||
|
|
||
|
|
||
| def _inject_local_providers(recipe: dict[str, Any], request_base_url: str) -> None: | ||
| """ | ||
| Mutate recipe dict in-place: for any provider with is_local=True, | ||
| generate a 24h JWT and fill in the endpoint pointing at this server. | ||
| """ | ||
| providers = recipe.get("model_providers") | ||
| if not providers: | ||
| return | ||
|
|
||
| # Collect local providers and pop is_local from ALL dicts unconditionally | ||
| local_indices: list[int] = [] | ||
| for i, provider in enumerate(providers): | ||
| if not isinstance(provider, dict): | ||
| continue | ||
| if provider.pop("is_local", None): | ||
| local_indices.append(i) | ||
|
|
||
| if not local_indices: | ||
| return | ||
|
|
||
| # Only gate on model-loaded if a local provider is actually referenced | ||
| # by a model_config. Unused local provider nodes should not block runs. | ||
| local_names = {providers[i].get("name") for i in local_indices} | ||
| referenced_providers = { | ||
| mc.get("provider") | ||
| for mc in recipe.get("model_configs", []) | ||
| if isinstance(mc, dict) | ||
| } | ||
| if not local_names & referenced_providers: | ||
| # No model_config actually uses a local provider, just inject and move on | ||
| from urllib.parse import urlparse | ||
|
|
||
| parsed = urlparse(request_base_url) | ||
| port = parsed.port or 8888 | ||
| endpoint = f"http://127.0.0.1:{port}/v1" | ||
| for i in local_indices: | ||
| providers[i]["endpoint"] = endpoint | ||
| providers[i]["api_key"] = "" | ||
| providers[i]["provider_type"] = "openai" | ||
| return | ||
|
|
||
| # Verify a model is loaded. | ||
| # NOTE: This is a point-in-time check (TOCTOU). The model could be unloaded | ||
| # or swapped after this check but before the recipe subprocess calls /v1. | ||
| # The inference endpoint returns a clear 400 in that case. | ||
| # | ||
| # Imports are deferred to avoid circular dependencies with inference modules. | ||
| from routes.inference import get_llama_cpp_backend | ||
| from core.inference import get_inference_backend | ||
|
|
||
| llama = get_llama_cpp_backend() | ||
| model_loaded = llama.is_loaded | ||
| if not model_loaded: | ||
| backend = get_inference_backend() | ||
| model_loaded = bool(backend.active_model_name) | ||
| if not model_loaded: | ||
| raise ValueError( | ||
| "No model loaded in Chat. Load a model first, then run the recipe." | ||
| ) | ||
|
|
||
| from auth.authentication import ( | ||
| create_access_token, | ||
| ) # deferred: avoids circular import | ||
|
|
||
| # Uses the "unsloth" admin subject. If the user changes their password, | ||
| # the JWT secret rotates and this token becomes invalid mid-run. | ||
| # Acceptable for v1 — recipes typically finish well within one session. | ||
| token = create_access_token( | ||
| subject = "unsloth", | ||
| expires_delta = timedelta(hours = 24), | ||
| ) | ||
|
|
||
| # Always use loopback — request.base_url may reflect a proxy or 0.0.0.0 | ||
| from urllib.parse import urlparse # deferred: only needed for local providers | ||
|
|
||
| parsed = urlparse(request_base_url) | ||
| port = parsed.port or 8888 | ||
| endpoint = f"http://127.0.0.1:{port}/v1" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This endpoint is computed from the client-facing Useful? React with 👍 / 👎. |
||
|
|
||
| for i in local_indices: | ||
| providers[i]["endpoint"] = endpoint | ||
| providers[i]["api_key"] = token | ||
| providers[i]["provider_type"] = "openai" | ||
|
|
||
|
|
||
| def _normalize_run_name(value: Any) -> str | None: | ||
| if value is None: | ||
| return None | ||
|
|
@@ -40,7 +127,7 @@ def _normalize_run_name(value: Any) -> str | None: | |
|
|
||
|
|
||
| @router.post("/jobs", response_class = JSONResponse, response_model = JobCreateResponse) | ||
| def create_job(payload: RecipePayload): | ||
| def create_job(payload: RecipePayload, request: Request): | ||
| recipe = payload.recipe | ||
| if not recipe.get("columns"): | ||
| raise HTTPException(status_code = 400, detail = "Recipe must include columns.") | ||
|
|
@@ -67,6 +154,11 @@ def create_job(payload: RecipePayload): | |
| status_code = 400, detail = f"invalid run_config: {exc}" | ||
| ) from exc | ||
|
|
||
| try: | ||
| _inject_local_providers(recipe, str(request.base_url)) | ||
| except ValueError as exc: | ||
| raise HTTPException(status_code = 400, detail = str(exc)) from exc | ||
|
|
||
| mgr = get_job_manager() | ||
| try: | ||
| job_id = mgr.start(recipe = recipe, run = run) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -68,6 +68,15 @@ def _collect_validation_errors(recipe: dict[str, Any]) -> list[ValidateError]: | |
| return errors | ||
|
|
||
|
|
||
| def _patch_local_providers(recipe: dict[str, Any]) -> None: | ||
| """Strip is_local and fill a dummy endpoint so validation doesn't choke.""" | ||
| for provider in recipe.get("model_providers", []): | ||
| if not isinstance(provider, dict): | ||
| continue | ||
| if provider.pop("is_local", None): | ||
| provider["endpoint"] = "http://127.0.0.1" | ||
|
Comment on lines
+81
to
+82
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The validation path only rewrites local providers to Useful? React with 👍 / 👎. |
||
|
|
||
|
|
||
| @router.post("/validate", response_model = ValidateResponse) | ||
| def validate(payload: RecipePayload) -> ValidateResponse: | ||
| recipe = payload.recipe | ||
|
|
@@ -77,6 +86,8 @@ def validate(payload: RecipePayload) -> ValidateResponse: | |
| errors = [ValidateError(message = "Recipe must include columns.")], | ||
| ) | ||
|
|
||
| _patch_local_providers(recipe) | ||
|
|
||
| try: | ||
| validate_recipe(recipe) | ||
| except RuntimeError as exc: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,14 +26,17 @@ import { NameField } from "../shared/name-field"; | |
| type ModelConfigDialogProps = { | ||
| config: ModelConfig; | ||
| providerOptions: string[]; | ||
| localProviderNames: Set<string>; | ||
| onUpdate: (patch: Partial<ModelConfig>) => void; | ||
| }; | ||
|
|
||
| export function ModelConfigDialog({ | ||
| config, | ||
| providerOptions, | ||
| localProviderNames, | ||
| onUpdate, | ||
| }: ModelConfigDialogProps): ReactElement { | ||
| const isLinkedToLocal = localProviderNames.has(config.provider); | ||
| const [optionalOpen, setOptionalOpen] = useState(false); | ||
| const modelId = `${config.id}-model`; | ||
| const providerId = `${config.id}-provider`; | ||
|
|
@@ -84,7 +87,17 @@ export function ModelConfigDialog({ | |
| filteredItems={providerOptions} | ||
| filter={null} | ||
| value={config.provider || null} | ||
| onValueChange={(value) => updateField("provider", value ?? "")} | ||
| onValueChange={(value) => { | ||
| const selectedProvider = value ?? ""; | ||
| const isLocal = localProviderNames.has(selectedProvider); | ||
| if (isLocal && !config.model.trim()) { | ||
| onUpdate({ provider: selectedProvider, model: "local" }); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The new local/external model-id normalization only runs in Useful? React with 👍 / 👎. |
||
| } else if (!isLocal && config.model === "local") { | ||
| onUpdate({ provider: selectedProvider, model: "" }); | ||
| } else { | ||
| updateField("provider", selectedProvider); | ||
| } | ||
| }} | ||
| onInputValueChange={(value) => { | ||
| providerInputRef.current = value; | ||
| }} | ||
|
|
@@ -124,12 +137,12 @@ export function ModelConfigDialog({ | |
| <FieldLabel | ||
| label="Model ID" | ||
| htmlFor={modelId} | ||
| hint="The exact model name sent to the connection." | ||
| hint={isLinkedToLocal ? "Uses the model loaded in Chat. Any value works here." : "The exact model name sent to the connection."} | ||
| /> | ||
| <Input | ||
| id={modelId} | ||
| className="nodrag" | ||
| placeholder="gpt-4o-mini" | ||
| placeholder={isLinkedToLocal ? "local" : "gpt-4o-mini"} | ||
| value={config.model} | ||
| onChange={(event) => updateField("model", event.target.value)} | ||
| /> | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.