Skip to content

Allow users to choose the AI model when filling forms - #721

Closed
jansans04 wants to merge 16 commits into
developmentfrom
502-allow-users-choose-ai-model
Closed

jansans04 wants to merge 16 commits into
developmentfrom
502-allow-users-choose-ai-model

Conversation

@jansans04

Copy link
Copy Markdown
Member

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

  • Fill only ever used the model set in OLLAMA_MODEL. Changing it meant editing .env and restarting.
  • A model that was not installed came back as a generic 500 from the filler.
  • There was no way to download a model through the API, so a first-time user had to shell into the Ollama container.

What changed

  • GET /api/v1/forms/models returns current_model and 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 with installed: false.
  • POST /api/v1/forms/fill takes an optional model. Without it, OLLAMA_MODEL is used as before.
  • A model that is not installed returns 422 MODEL_NOT_INSTALLED instead of a 500. Async fill jobs keep the same code instead of TASK_FAILED.
  • POST /api/v1/forms/pull takes 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 an error line and no success line follows, since the HTTP status is already committed.
  • Model names are validated on fill and pull: trimmed, not empty, at most 128 characters, and limited to the characters Ollama model names use.
  • OLLAMA_PULL_TIMEOUT is separate from OLLAMA_TIMEOUT, because a download takes far longer than an inference call. Both are in docker/.env.example.
  • Contracts and docs/AI_MODELS.md updated.

Notes

  • GET /forms/models changed shape: models used to be list[str] and is now a list of objects. Inside this repo only the frontend reads it, and fetchModels() already adapts it. Anything outside the repo reading the old shape would break.
  • installed and /forms/pull only make sense for Ollama. They are kept in list_models() and pull_model() in forms.py so [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.
  • The dropdown is not in this PR. The frontend still needs adjusting to the final response and will follow separately.

Testing

  • python -m pytest: 184 passed. New tests cover a pull error mid-stream, invalid model names on fill and pull, MODEL_NOT_INSTALLED in an async job, a malformed tags response, fill without model, and the pull timeout.
  • Checked by hand against a running Ollama: .

Part of #502

jansans04 and others added 13 commits October 14, 2026 11:03
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>
Copilot AI balanced review requested due to automatic review settings October 14, 2026 15:18

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Comment thread app/api/routes/forms.py Outdated
"qwen2.5:3b",
"qwen2.5:7b",
"llama3.2:3b",
"mistral:7b",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get rid of this one

Comment thread app/core/config.py Outdated
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"))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

10 mins? what happens if takes more than 10? how much should it be taking?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

@jansans04
jansans04 requested review from marcvergees and a balanced review from Copilot October 17, 2026 09:29

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 marcvergees left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks 99% good to me. You just gotta change 1 thing and we'll be able to merge it.

Thanks @jansans04 !

Comment thread app/api/routes/forms.py Outdated

router = APIRouter(prefix="/forms", tags=["forms"])

RECOMMENDED_MODELS = [

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move this into config.py, general stuff should be allocated together

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've already patched it @jansans04.

@chetanr25

Copy link
Copy Markdown
Collaborator

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 GET /api/v1/system/llm/models API to get a list of all available models.

I have separated the LLM logic from forms, as it doesn't belong exclusively to forms. The previous LLM API under forms has been deprecated. You can find more information in the multi-LLM support documentation

Since we can now choose any model configured in .env, we can finally let the frontend select which LLM model to use.

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

  • Add a request body field or header to receive the LLM choice from the client application.
  • Validate whether the model chosen by the client is in the list of supported models. We can reuse the functions behind GET /api/v1/system/llm/models for this.
  • Use the LLM model chosen by the client for the request.

2. Frontend

  • Use the GET /api/v1/system/llm/models endpoint to render the list of models supported by the FireForm backend instance, based on its .env configuration.
  • Add search functionality to filter models.
  • When a model is selected, store it in the client's local storage and pass it with every request to an API that directly or indirectly uses the LLM service.

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.

CC: @marcvergees @vharkins1

@jansans04

Copy link
Copy Markdown
Member Author

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.

@jansans04

Copy link
Copy Markdown
Member Author

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.

@jansans04 jansans04 closed this Oct 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants