Repository navigation
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| "qwen2.5:3b", | ||
| "qwen2.5:7b", | ||
| "llama3.2:3b", | ||
| "mistral:7b", |
| OLLAMA_HOST = os.getenv("OLLAMA_HOST", "http://localhost:11434").rstrip("/") | ||
| OLLAMA_MODEL = os.getenv("OLLAMA_MODEL", "qwen2.5:1.5b") | ||
| OLLAMA_TIMEOUT = int(os.getenv("OLLAMA_TIMEOUT", "300")) | ||
| OLLAMA_PULL_TIMEOUT = int(os.getenv("OLLAMA_PULL_TIMEOUT", "600")) |
There was a problem hiding this comment.
10 mins? what happens if takes more than 10? how much should it be taking?
There was a problem hiding this comment.
It wasn't a total limit: it's the requests timeout on a streamed response,
so it's the max wait between two chunks, and Ollama sends progress
continuously. But 10 minutes of silence was too generous for a stuck download.
Changes:
- connecting is now limited to 10 s, separate from the idle timeout
- default lowered from 600 to 120 s
- docs and .env.example now describe it as an idle timeout
- added a test for a mid-stream timeout (error line, no success)
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
marcvergees
left a comment
There was a problem hiding this comment.
It looks 99% good to me. You just gotta change 1 thing and we'll be able to merge it.
Thanks @jansans04 !
|
|
||
| router = APIRouter(prefix="/forms", tags=["forms"]) | ||
|
|
||
| RECOMMENDED_MODELS = [ |
There was a problem hiding this comment.
move this into config.py, general stuff should be allocated together
|
Hey @jansans04, I have implemented external multi-LLM support in PR #725. In the same PR, I introduced a few APIs, and you can find their descriptions #725 (comment) We can now use the I have separated the LLM logic from Since we can now choose any model configured in For the frontend, whenever an API call is made that subsequently uses an LLM, such as forms in our case, we can select a model on the frontend and pass it in the request body or headers. The selected LLM can be stored locally in the client application and passed with every request related to LLM calls. According to me, we should make the following changes: 1. Backend
2. Frontend
I think this PR can be closed, as PR #725 already implements what is required. Apologies, as I didn't notice this PR while I was working on #725. |
|
Thanks @chetanr25, and no worries, it happens. #725 covers choosing provider and model, so I'll close this one and open a new PR on top of your registry with what is still missing: installing Ollama models and showing which are installed. I'll also adapt the model selector I already have in the frontend (fireform-frontend#46) to GET /system/llm/models, so there is no need to build it twice. |
|
Superseded by #733. #725 now covers choosing provider and model and validating the pair, so the part of this PR that is still needed (installing Ollama models with progress, and the installed/downloadable state) is rebuilt on top of the new registry there. Thanks @marcvergees for the review: the points about mistral:7b and the pull timeout are applied in the new PR. Closing this one. |
Allow users to choose the AI model when filling forms
Backend for the model picker in #502. The frontend needs to know which models exist, pick one per request, and install one that is missing.
Why
OLLAMA_MODEL. Changing it meant editing.envand restarting.What changed
GET /api/v1/forms/modelsreturnscurrent_modeland a list of{name, installed, recommended}. Recommended models are always listed, plus anything Ollama has installed. If Ollama is down or answers with something unreadable, it still returns the recommended models, all withinstalled: false.POST /api/v1/forms/filltakes an optionalmodel. Without it,OLLAMA_MODELis used as before.MODEL_NOT_INSTALLEDinstead of a 500. Async fill jobs keep the same code instead ofTASK_FAILED.POST /api/v1/forms/pulltakes a model name and streams Ollama's progress as NDJSON. It ends with{"status":"success"}. If the pull fails after the stream has started, the error is sent as anerrorline and no success line follows, since the HTTP status is already committed.OLLAMA_PULL_TIMEOUTis separate fromOLLAMA_TIMEOUT, because a download takes far longer than an inference call. Both are indocker/.env.example.docs/AI_MODELS.mdupdated.Notes
GET /forms/modelschanged shape:modelsused to belist[str]and is now a list of objects. Inside this repo only the frontend reads it, andfetchModels()already adapts it. Anything outside the repo reading the old shape would break.installedand/forms/pullonly make sense for Ollama. They are kept inlist_models()andpull_model()informs.pyso [FEAT] Support LLM providers other than Ollama on development #716 can move them behind its provider layer, where non-Ollama providers will need a different answer.Testing
python -m pytest: 184 passed. New tests cover a pull error mid-stream, invalid model names on fill and pull,MODEL_NOT_INSTALLEDin an async job, a malformed tags response, fill withoutmodel, and the pull timeout.Part of #502