From 8fe5f573a50a6ab6a00f065dacd729dd43857f2e Mon Sep 17 00:00:00 2001 From: Krstan Vjestica Date: Mon, 27 Jul 2026 12:15:00 +0200 Subject: [PATCH 01/12] docs: point the contributing link at the published site forail-deploy no longer carries the guide's markdown in the repo -- the sources live on the host and the site is what is published. Link the site page instead of a file path that now 404s. --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 37ec9d2..0a5140b 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -2,7 +2,7 @@ Thanks for your interest in contributing! -The full contributing guide — git workflow, commit conventions, coding standards, PR process — lives in the [forail-deploy repository](https://github.com/forail-platform/forail-devops/blob/main/docs/10-contributing-guide.md). Please read it before submitting a pull request. +The full contributing guide — git workflow, commit conventions, coding standards, PR process — lives in the [Forail developer docs](https://forail-platform.github.io/dev/contributing.html). Please read it before submitting a pull request. ## What lives here From 32e39984277c599d416dccf8b443a3588052ed8a Mon Sep 17 00:00:00 2001 From: Krstan Vjestica Date: Tue, 28 Jul 2026 20:40:00 +0200 Subject: [PATCH 02/12] fix: run Ollama as its own pinned service, not a binary lifted from its image The image copied /bin/ollama out of ollama/ollama:latest and nothing else. That silently stopped working: Ollama keeps its inference engine in /usr/lib/ollama (llama-server, libggml, the CUDA backends), so the copied binary could start a server and answer /api/tags -- enough for the health check to look fine -- while every generation failed with error starting llama-server: llama-server binary not found Reproduced against the published 2026.06.0 image, which carries Ollama 0.30.8 without that directory. It has never been able to load a model. Rather than chase upstream's internal file layout, Ollama now runs as its own service: - Pinned to ollama/ollama:0.30.10. Tracking `latest` is precisely what let an upstream change break inference without a line of our code changing. - Only the model server benefits from a GPU, so only it has to carry that requirement -- in Kubernetes just that pod needs a GPU node. - No published port. Ollama has no authentication, so it stays reachable only from the API, and the integration overlay keeps it off the Forail network. The API now waits for Ollama on startup and pulls models over its HTTP API, since the ollama CLI is no longer in the image. The wait is bounded at 300s and exits with an actionable message instead of hanging forever. --- Dockerfile | 31 +++++++++++------- docker-compose.integration.yml | 4 +++ docker-compose.yml | 30 ++++++++++++----- entrypoint.sh | 60 ++++++++++++++++++++++------------ 4 files changed, 85 insertions(+), 40 deletions(-) diff --git a/Dockerfile b/Dockerfile index 0ed4398..80340df 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,7 +1,16 @@ -### Forail Assistant — All-in-one image -### Ollama (LLM) + ChromaDB (embedded) + FastAPI in a single container - -FROM ollama/ollama:latest AS ollama +### Forail Assistant — API image +### FastAPI (RAG pipeline) + ChromaDB (embedded). +### +### Ollama is NOT bundled here. It runs as its own service so that only the +### model server needs a GPU (and, in Kubernetes, only that pod needs to land +### on a GPU node). See docker-compose.yml, or the forail-assistant-ollama +### Deployment in the Helm chart. +### +### The previous all-in-one layout copied /bin/ollama out of the official +### image on its own. That silently stopped working: modern Ollama keeps the +### inference engine in /usr/lib/ollama (llama-server, libggml, the CUDA +### backends), so the copied binary could start a server but never load a +### model — every request came back 500. FROM python:3.12-slim @@ -13,9 +22,6 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ WORKDIR /app -# Copy Ollama binary from official image -COPY --from=ollama /bin/ollama /usr/local/bin/ollama - # Install Python dependencies COPY requirements.txt . RUN pip install --no-cache-dir -r requirements.txt @@ -28,11 +34,14 @@ COPY docs_to_index/ ./docs_to_index/ COPY entrypoint.sh /entrypoint.sh RUN chmod +x /entrypoint.sh -# Directories for data persistence -RUN mkdir -p /data/ollama /data/chroma +# Directory for the ChromaDB index. Model blobs live in the Ollama service's +# own volume, not here. +RUN mkdir -p /data/chroma -ENV OLLAMA_MODELS=/data/ollama -ENV FORAIL_ASSISTANT_OLLAMA_BASE_URL=http://localhost:11434 +# Default points at the Ollama service by its compose/Service name. Both the +# compose file and the Helm chart set this explicitly; the default only keeps +# a bare `docker run` on the same network working. +ENV FORAIL_ASSISTANT_OLLAMA_BASE_URL=http://ollama:11434 ENV FORAIL_ASSISTANT_OLLAMA_MODEL=gemma3:1b ENV FORAIL_ASSISTANT_CHROMA_HOST=localhost ENV FORAIL_ASSISTANT_CHROMA_PORT=8000 diff --git a/docker-compose.integration.yml b/docker-compose.integration.yml index 7bd70c1..229221e 100644 --- a/docker-compose.integration.yml +++ b/docker-compose.integration.yml @@ -1,5 +1,9 @@ # Integration overlay for forail-deploy # Usage: docker compose -f docker-compose.yml -f docker-compose.integration.yml up -d +# +# Only the API joins the Forail network. The ollama service stays on this +# stack's default network alone — it has no authentication, so nothing outside +# the assistant should be able to reach it. services: forail-assistant: networks: diff --git a/docker-compose.yml b/docker-compose.yml index cac3f01..df0e983 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -1,22 +1,35 @@ services: + # Model server. Pinned deliberately: this used to be `ollama/ollama:latest` + # scavenged for its binary, and an upstream layout change broke inference + # without changing a line of our code. Bump this tag on purpose, not by drift. + ollama: + image: ollama/ollama:0.30.10 + volumes: + - ollama_models:/root/.ollama + # No published port on purpose — Ollama has no authentication. It is + # reachable only by services on this compose network. + healthcheck: + test: ["CMD-SHELL", "ollama list > /dev/null 2>&1 || exit 1"] + interval: 10s + timeout: 5s + start_period: 30s + retries: 5 + restart: unless-stopped + forail-assistant: build: . image: ghcr.io/forail-platform/forail-assistant:latest + depends_on: + ollama: + condition: service_healthy ports: - "8100:8100" volumes: - assistant_data:/data environment: + - FORAIL_ASSISTANT_OLLAMA_BASE_URL=http://ollama:11434 - FORAIL_ASSISTANT_OLLAMA_MODEL=gemma3:1b - FORAIL_ASSISTANT_LOG_LEVEL=INFO - # Uncomment for GPU support: - # deploy: - # resources: - # reservations: - # devices: - # - driver: nvidia - # count: 1 - # capabilities: [gpu] healthcheck: test: ["CMD", "curl", "-sf", "http://localhost:8100/api/v1/health"] interval: 30s @@ -27,3 +40,4 @@ services: volumes: assistant_data: + ollama_models: diff --git a/entrypoint.sh b/entrypoint.sh index 7910430..e923b82 100644 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -1,33 +1,51 @@ #!/bin/bash set -e -echo "==> Starting Ollama server..." -ollama serve & -OLLAMA_PID=$! +OLLAMA_URL="${FORAIL_ASSISTANT_OLLAMA_BASE_URL:-http://ollama:11434}" +LLM_MODEL="${FORAIL_ASSISTANT_OLLAMA_MODEL:-gemma3:1b}" +EMBED_MODEL="${FORAIL_ASSISTANT_OLLAMA_EMBED_MODEL:-nomic-embed-text}" -# Wait for Ollama to be ready -echo "==> Waiting for Ollama..." -until curl -sf http://localhost:11434/ > /dev/null 2>&1; do - sleep 1 +# Ollama runs in its own container now, so wait for it rather than starting it. +# Bounded: hanging here forever just turns a missing dependency into a pod that +# never reports anything useful. +echo "==> Waiting for Ollama at ${OLLAMA_URL}..." +for attempt in $(seq 1 150); do + if curl -sf "${OLLAMA_URL}/" > /dev/null 2>&1; then + echo "==> Ollama ready." + break + fi + if [ "$attempt" -eq 150 ]; then + echo "FATAL: Ollama not reachable at ${OLLAMA_URL} after 300s." >&2 + echo " Check that the ollama service is running and that" >&2 + echo " FORAIL_ASSISTANT_OLLAMA_BASE_URL points at it." >&2 + exit 1 + fi + sleep 2 done -echo "==> Ollama ready." -# Pull models if not present -if ! ollama list 2>/dev/null | grep -q "${FORAIL_ASSISTANT_OLLAMA_MODEL:-gemma3:1b}"; then - echo "==> Pulling model ${FORAIL_ASSISTANT_OLLAMA_MODEL:-gemma3:1b}..." - ollama pull "${FORAIL_ASSISTANT_OLLAMA_MODEL:-gemma3:1b}" -fi +# The ollama CLI is no longer in this image, so models are pulled over the API. +ensure_model() { + local model="$1" + if curl -sf "${OLLAMA_URL}/api/tags" | grep -q "${model}"; then + echo "==> Model ${model} already present." + return + fi + echo "==> Pulling model ${model}..." + curl -sf -X POST "${OLLAMA_URL}/api/pull" \ + -H 'Content-Type: application/json' \ + -d "{\"model\": \"${model}\"}" > /dev/null + echo "==> Pulled ${model}." +} -if ! ollama list 2>/dev/null | grep -q "nomic-embed-text"; then - echo "==> Pulling embedding model nomic-embed-text..." - ollama pull nomic-embed-text -fi +ensure_model "${LLM_MODEL}" +ensure_model "${EMBED_MODEL}" echo "==> Starting ChromaDB..." -# needtofix L13: ChromaDB (and Ollama) bind 0.0.0.0 with no auth. This is safe -# only because they are confined to this pod/container and not exposed by a -# Service/port. Do NOT publish these ports; if a shared instance is ever -# needed, put an authenticating proxy in front. +# needtofix L13: ChromaDB binds 0.0.0.0 with no auth. This is safe only because +# it is confined to this pod/container and not exposed by a Service/port. Do NOT +# publish this port; if a shared instance is ever needed, put an authenticating +# proxy in front. The same applies to the Ollama service next door — it has no +# auth either, so it stays on the internal network with no published port. chroma run --host 0.0.0.0 --port 8000 --path /data/chroma > /dev/null 2>&1 & CHROMA_PID=$! From ddbbd6123bdbe6046532a9bd7ef06d6668c8fc3c Mon Sep 17 00:00:00 2001 From: Krstan Vjestica Date: Wed, 29 Jul 2026 19:05:00 +0200 Subject: [PATCH 03/12] feat: optional GPU overlay for the model server Kept as an overlay instead of folded into docker-compose.yml because a devices reservation is a hard requirement: on a host without a GPU the stack refuses to start rather than quietly running on CPU. Opting in is explicit: docker compose -f docker-compose.yml -f docker-compose.gpu.yml up -d Ollama falls back to CPU silently when it cannot see a device, so the file documents how to confirm it actually took: `logs ollama | grep "inference compute"` must report a CUDA library and non-zero VRAM, not library=cpu. Measured on a Ryzen 9 5900X / RTX 3080 12GB with gemma3:1b, warm, same questions against the same index: ~680 B/s on 24 CPU threads against ~3900 B/s on the GPU. Time to first token barely moves (~0.5s either way) -- that is RAG retrieval, not generation. --- docker-compose.gpu.yml | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 docker-compose.gpu.yml diff --git a/docker-compose.gpu.yml b/docker-compose.gpu.yml new file mode 100644 index 0000000..3f2acfd --- /dev/null +++ b/docker-compose.gpu.yml @@ -0,0 +1,30 @@ +# GPU overlay — hands an NVIDIA GPU to the Ollama service. +# +# Kept as an overlay rather than folded into docker-compose.yml because a +# `devices` reservation is a hard requirement: on a host without a GPU the +# stack refuses to start instead of quietly running on CPU. +# +# Requires on the host: +# - NVIDIA driver +# - nvidia-container-toolkit, registered with Docker: +# sudo nvidia-ctk runtime configure --runtime=docker +# sudo systemctl restart docker +# +# Verify the host is ready before using this file: +# docker run --rm --gpus all nvidia/cuda:12.4.0-base-ubuntu22.04 nvidia-smi +# +# Usage: +# docker compose -f docker-compose.yml -f docker-compose.gpu.yml up -d +# +# Confirm Ollama actually picked the GPU up (it falls back to CPU silently): +# docker compose logs ollama | grep "inference compute" +# The line must report a CUDA library and non-zero VRAM, not `library=cpu`. +services: + ollama: + deploy: + resources: + reservations: + devices: + - driver: nvidia + count: 1 + capabilities: [gpu] From 3b664140baf9c0df080dda539335e9cd9f165417 Mon Sep 17 00:00:00 2001 From: Krstan Vjestica Date: Fri, 31 Jul 2026 18:22:00 +0200 Subject: [PATCH 04/12] test: assert the config defaults, not the ambient environment test_default_settings built a Settings() and checked its values, but pydantic-settings reads FORAIL_ASSISTANT_* out of the environment first. The shipped image sets those (it points the API at the ollama service rather than localhost), so running the suite inside that image failed on a correct default. Clear the prefix from the environment before constructing Settings, so the test means what its name says wherever it runs. --- tests/test_config.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/test_config.py b/tests/test_config.py index 71c5dda..2f4c721 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,9 +1,19 @@ """Tests for configuration module.""" +import os + from app.config import Settings -def test_default_settings(): +def test_default_settings(monkeypatch): + # The shipped image sets FORAIL_ASSISTANT_* variables (it points the API at + # the ollama service, not localhost), so without clearing them this asserts + # the environment rather than the code's defaults — and fails when run + # inside that image. + for name in list(os.environ): + if name.startswith("FORAIL_ASSISTANT_"): + monkeypatch.delenv(name) + s = Settings() assert s.ollama_base_url == "http://localhost:11434" assert s.ollama_model == "gemma3:1b" From 8be414a34c59a9a2ef66a6a48208cfa0efc0677e Mon Sep 17 00:00:00 2001 From: Krstan Vjestica Date: Mon, 3 Aug 2026 16:10:00 +0200 Subject: [PATCH 05/12] docs: describe the two-service layout and the measured GPU numbers The README still promised a single all-in-one container with Ollama bundled inside, which is no longer true and was the arrangement that could not run a model in the first place. - New architecture diagram: API (FastAPI + embedded ChromaDB) alongside Ollama, with why they are apart -- GPU requirement on the model server alone, and no published port because Ollama has no authentication. - Hardware section replaced with numbers actually measured here rather than estimates, plus the command to confirm Ollama took the GPU plus the toolkit prerequisite. It falls back to CPU silently, so "check, do not assume". - FORAIL_ASSISTANT_RAG_TOP_K documented as 3, which is what the code has always defaulted to; the table said 5. --- README.md | 69 ++++++++++++++++++++++++++++++++++++++----------------- 1 file changed, 48 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 798cb4a..4f36852 100644 --- a/README.md +++ b/README.md @@ -10,21 +10,25 @@ AI-powered assistant for the Forail infrastructure automation platform. Uses a l ## Overview -Forail Assistant is an **optional, standalone service** that can be plugged into or removed from any Forail deployment. It runs as a **single all-in-one container** with Ollama (LLM) and ChromaDB (embedded) bundled inside. +Forail Assistant is an **optional, standalone service** that can be plugged into or removed from any Forail deployment. It runs as **two containers**: the API (FastAPI + embedded ChromaDB) and Ollama, the model server. ``` -┌──────────────────┐ ┌──────────────────────────────────────┐ -│ Forail Frontend │────▶│ Forail Assistant │ -│ (React chat) │ SSE │ ┌──────────┐ ┌──────────────────┐ │ -└──────────────────┘ │ │ Ollama │ │ FastAPI │ │ - │ │ gemma3:1b │ │ (RAG pipeline) │ │ - │ └──────────┘ └────────┬──────────┘ │ - │ ┌────────▼──────────┐ │ - │ │ ChromaDB (embed) │ │ - │ └───────────────────┘ │ - └──────────────────────────────────────┘ +┌──────────────────┐ ┌───────────────────────────────┐ ┌──────────────┐ +│ Forail Frontend │────▶│ Forail Assistant API │────▶│ Ollama │ +│ (React chat) │ SSE │ ┌──────────────────────────┐ │HTTP │ gemma3:1b │ +└──────────────────┘ │ │ FastAPI (RAG pipeline) │ │ │ (GPU here) │ + │ └────────────┬─────────────┘ │ └──────────────┘ + │ ┌────────────▼─────────────┐ │ + │ │ ChromaDB (embedded) │ │ + │ └──────────────────────────┘ │ + └───────────────────────────────┘ ``` +They are separate on purpose: only the model server benefits from a GPU, so +only it carries the GPU requirement. In Kubernetes that means just one pod has +to land on a GPU node while the API stays schedulable anywhere. Ollama has no +authentication, so it is never given a published port — only the API talks to it. + ## Features - **Contextual help** — knows which page the user is on @@ -36,9 +40,12 @@ Forail Assistant is an **optional, standalone service** that can be plugged into ## Quick Start ```bash -# Start the assistant (all-in-one: Ollama + ChromaDB + FastAPI) +# Start the assistant (API + Ollama) docker compose up -d +# ...or with GPU acceleration for the model server (see Hardware below) +docker compose -f docker-compose.yml -f docker-compose.gpu.yml up -d + # Wait ~2 minutes for Ollama to load the model on first start, # then index documentation curl -X POST http://localhost:8100/api/v1/index @@ -49,7 +56,7 @@ curl -X POST http://localhost:8100/api/v1/chat \ -d '{"message": "How do I create a job template?"}' ``` -> **Note:** On first start, the entrypoint automatically pulls the LLM model (`gemma3:1b`) and embedding model (`nomic-embed-text`). The healthcheck `start_period` is 120 seconds to allow time for this. +> **Note:** On first start, the API waits for Ollama and then pulls the LLM model (`gemma3:1b`) and embedding model (`nomic-embed-text`) over Ollama's API. The healthcheck `start_period` is 120 seconds to allow time for this. ## Integration with Forail @@ -68,21 +75,41 @@ All settings via environment variables with `FORAIL_ASSISTANT_` prefix: | Variable | Default | Description | |----------|---------|-------------| -| `FORAIL_ASSISTANT_OLLAMA_BASE_URL` | `http://localhost:11434` | Ollama API URL (localhost — runs inside the same container) | +| `FORAIL_ASSISTANT_OLLAMA_BASE_URL` | `http://localhost:11434` | Ollama API URL. The code defaults to localhost for local development; the image and the Helm chart both override it to point at the Ollama service | | `FORAIL_ASSISTANT_OLLAMA_MODEL` | `gemma3:1b` | LLM model | | `FORAIL_ASSISTANT_OLLAMA_EMBED_MODEL` | `nomic-embed-text` | Embedding model | -| `FORAIL_ASSISTANT_CHROMA_HOST` | `localhost` | ChromaDB host (localhost — embedded in the same container) | +| `FORAIL_ASSISTANT_CHROMA_HOST` | `localhost` | ChromaDB host (localhost — embedded in the API container) | | `FORAIL_ASSISTANT_CHROMA_PORT` | `8000` | ChromaDB port | -| `FORAIL_ASSISTANT_RAG_TOP_K` | `5` | Number of docs to retrieve | +| `FORAIL_ASSISTANT_RAG_TOP_K` | `3` | Number of docs to retrieve | | `FORAIL_ASSISTANT_LOG_LEVEL` | `INFO` | Logging level | ## Hardware Requirements -| Setup | RAM | GPU | Response Time | -|-------|-----|-----|---------------| -| CPU-only (phi3:mini) | 8 GB | None | 10-20s | -| GPU (mistral:7b) | 16 GB | 8 GB VRAM | 2-5s | -| GPU (llama3.1:8b) | 32 GB | 12 GB VRAM | 1-3s | +The GPU overlay needs the NVIDIA driver plus `nvidia-container-toolkit` +registered with Docker (`nvidia-ctk runtime configure --runtime=docker`). +Without it the reservation fails and the stack refuses to start — deliberately, +so that a missing GPU is loud rather than a silent fall back to CPU. + +Ollama picks CPU silently when it cannot see a device. Always confirm: + +```bash +docker compose logs ollama | grep "inference compute" +# GPU: library=CUDA ... description="NVIDIA GeForce RTX 3080" total="11.6 GiB" +# CPU: library=cpu ... name=cpu +``` + +Measured on a Ryzen 9 5900X / RTX 3080 12GB, `gemma3:1b`, warm (model already +resident), same two questions against the same index: + +| Setup | Time to first token | Generation throughput | +|-------|--------------------|----------------------| +| CPU (24 threads) | ~0.5s | ~680 B/s | +| GPU (RTX 3080) | ~0.5s | ~3900 B/s | + +Time to first token is dominated by RAG retrieval, so it barely moves; the GPU +buys roughly **5–6× generation throughput**. That matters most as a headroom +budget: it is what makes a larger, more accurate model affordable at all, since +an 8B-class model on CPU is slower again by a wide margin. ## Development From 1d1f316b7626664cd154d2a2ca74dcde33305256 Mon Sep 17 00:00:00 2001 From: Krstan Vjestica Date: Tue, 4 Aug 2026 18:35:00 +0200 Subject: [PATCH 06/12] docs: changelog for the Ollama split, pinning, and GPU support Records the failure mode explicitly -- health checks passing while every generation returned 500 -- because that combination is what kept it hidden, and because the published 2026.06.0 image is affected. --- CHANGELOG.md | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index bbaddc3..f5da444 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,34 @@ All notable changes to the Forail Assistant will be documented in this file. ## [Unreleased] +### Fixed +- **The assistant could not run a model at all.** The image copied `/bin/ollama` + out of `ollama/ollama:latest` and nothing else, but Ollama keeps its inference + engine in `/usr/lib/ollama` (`llama-server`, `libggml`, the CUDA backends). + The server started and answered `/api/tags`, so health checks looked fine, + while every generation failed with + `error starting llama-server: llama-server binary not found` — HTTP 500. This + affects the published `2026.06.0` image, which ships Ollama 0.30.8 without + that directory. + +### Changed +- **Ollama now runs as its own service** instead of being bundled into the API + image, and its version is **pinned** (`ollama/ollama:0.30.10`) rather than + tracking `latest`. Tracking `latest` is what let an upstream layout change + break inference without a line of our code changing. Splitting it also means + only the model server needs a GPU: in Kubernetes just that pod requests + `nvidia.com/gpu` and lands on a GPU node, while the API stays schedulable + anywhere. +- The API waits for Ollama on startup and pulls models over its HTTP API (the + `ollama` CLI is no longer present in the image). The wait is bounded at 300s + and exits with a clear message instead of hanging. + +### Added +- `docker-compose.gpu.yml` overlay that attaches an NVIDIA GPU to the model + server. Kept separate so a host without a GPU fails loudly instead of quietly + running on CPU. Measured on an RTX 3080 with `gemma3:1b`: ~5–6× generation + throughput over 24-thread CPU inference. + ### Security - **CORS** no longer combines a wildcard origin with credentials (a wildcard now disables `allow_credentials`). From d25c975b2b73405664005973f2ce57d340fe7cdc Mon Sep 17 00:00:00 2001 From: Krstan Vjestica Date: Thu, 6 Aug 2026 11:20:00 +0200 Subject: [PATCH 07/12] docs: architecture describes two services, not one container The diagram and the component notes still said FastAPI, Ollama and ChromaDB share a container and that the entrypoint starts Ollama itself. None of that has been true since the split. Also records why the copied-binary layout was abandoned, so the pinned upstream image does not look like an arbitrary preference. --- docs/architecture.md | 47 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 38 insertions(+), 9 deletions(-) diff --git a/docs/architecture.md b/docs/architecture.md index baeca14..0aa014a 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -12,20 +12,34 @@ Forail Assistant is a standalone microservice that provides AI-powered help for │ SSE (Server-Sent Events) ▼ ┌─────────────────────────────────┐ - │ Forail Assistant (all-in-one) │ + │ Forail Assistant (API) │ │ │ │ FastAPI /api/v1/chat → SSE │ │ /api/v1/health→ JSON │ │ /api/v1/index → reindex│ │ │ - │ Ollama gemma3:1b (LLM) │ - │ nomic-embed-text (emb) │ - │ │ │ ChromaDB (embedded vector store) │ + └───────────────┬──────────────────┘ + │ HTTP (internal network only) + ▼ + ┌─────────────────────────────────┐ + │ Ollama (model server) │ + │ │ + │ gemma3:1b (generation) │ + │ nomic-embed-text (embeddings) │ + │ │ + │ optional: NVIDIA GPU │ └─────────────────────────────────┘ ``` -> **Note:** All three components (FastAPI, Ollama, ChromaDB) run inside a single Docker container. The entrypoint script starts Ollama and ChromaDB as background processes before launching the FastAPI server. +> **Note:** These are **two services**, not one container. The API image carries +> FastAPI and an embedded ChromaDB; Ollama runs beside it from its own pinned +> upstream image. In Compose that is the `ollama` service, in Kubernetes the +> `forail-assistant-ollama` Deployment. Only the model server needs a GPU or a +> large volume, so only it has to land on a GPU node. +> +> Ollama has no authentication. It is reachable on the internal network alone — +> no published port in Compose, and a ClusterIP Service in Kubernetes. ## Components @@ -44,13 +58,28 @@ The core service, responsible for: - `app/indexer.py` — Document loading, chunking, indexing - `app/config.py` — Pydantic settings from environment -### Ollama (LLM Server) +### Ollama (Model Server) -Runs the language model locally inside the same container. Two models are used: +Runs the language models locally, in its own container. Two models are used: - **gemma3:1b** (default, or configured model) — for chat generation - **nomic-embed-text** — for generating document/query embeddings -The Ollama binary is copied from the official `ollama/ollama` image at build time. GPU acceleration is optional but recommended for larger models. +It runs from the official `ollama/ollama` image, **pinned** to an exact tag. It +used to be a single binary copied out of that image at build time, and that +broke silently: modern Ollama keeps its inference engine in `/usr/lib/ollama` +(`llama-server`, `libggml`, the CUDA backends), so the copied binary could start +a server and answer `/api/tags` — passing the health check — while every +generation returned HTTP 500. Running the upstream image whole, at a tag we +choose deliberately, is what closes that class of failure. + +The API never shells out to the `ollama` CLI; it is not in the API image at all. +Models are pulled over Ollama's HTTP API on first start, and the API waits for +the model server before it accepts traffic (bounded at 300s, then it exits with +an actionable message rather than hanging). + +GPU acceleration is optional and applies to this service alone — the +`docker-compose.gpu.yml` overlay in Compose, `assistant.ollama.gpu.enabled` in +the Helm chart. See the README for measured numbers. ### ChromaDB (Vector Store) @@ -90,7 +119,7 @@ Stores document chunks as vectors for similarity search. When a user asks a ques - **No Django dependency** — FastAPI is lighter, async-native, no ORM needed - **Optional** — can be added/removed without touching core Forail -- **Independent scaling** — can run on a separate GPU server +- **Independent scaling** — the model server is a separate service and can sit on its own GPU node - **Independent release cycle** — update models without redeploying Forail ### Why Ollama (Not OpenAI/Claude API)? From b84bae584599f7f999d01aa2d213aeaca1b02c61 Mon Sep 17 00:00:00 2001 From: Krstan Vjestica Date: Fri, 7 Aug 2026 18:05:00 +0200 Subject: [PATCH 08/12] docs: correct the Ollama URL default and how models are pulled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The base-URL row explained localhost as "runs inside the same container", which is exactly what stopped being true, and the model-change recipe told the reader to exec the ollama CLI in the API container — it is not in that image any more, so the command fails. Point both at the model server. --- docs/configuration.md | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 65f97a2..9edf6bd 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -10,7 +10,7 @@ All configuration is via environment variables with the `FORAIL_ASSISTANT_` pref | Variable | Default | Description | |----------|---------|-------------| -| `FORAIL_ASSISTANT_OLLAMA_BASE_URL` | `http://localhost:11434` | Ollama API base URL (localhost — runs inside the same container) | +| `FORAIL_ASSISTANT_OLLAMA_BASE_URL` | `http://localhost:11434` | Ollama API base URL. Ollama is a **separate service**, so this default only fits a hand-rolled setup with one on the host; the shipped image, the Compose file and the Helm chart all override it (`http://ollama:11434`, `http://forail-assistant-ollama:11434`) | | `FORAIL_ASSISTANT_OLLAMA_MODEL` | `gemma3:1b` | LLM model for chat generation | | `FORAIL_ASSISTANT_OLLAMA_EMBED_MODEL` | `nomic-embed-text` | Model for generating embeddings | | `FORAIL_ASSISTANT_OLLAMA_TIMEOUT` | `120` | Timeout in seconds for Ollama requests | @@ -54,13 +54,22 @@ All configuration is via environment variables with the `FORAIL_ASSISTANT_` pref To change the model: ```bash -# Pull new model (exec into the all-in-one container) -docker compose exec forail-assistant ollama pull llama3.1:8b - -# Restart with new model +# Restart with the new model — the API pulls it over Ollama's HTTP API on +# start, so nothing has to be pulled by hand. FORAIL_ASSISTANT_OLLAMA_MODEL=llama3.1:8b docker compose up -d ``` +To pull one ahead of time, address the model server directly. The `ollama` CLI +is no longer in the API image — it is in the Ollama service: + +```bash +docker compose exec ollama ollama pull llama3.1:8b +``` + +Anything above `gemma3:1b` wants the GPU overlay +(`-f docker-compose.gpu.yml`); the VRAM column above is the model server's +requirement alone, since it is the only service doing inference. + --- ## Document Sources From b0e1cc71dd945f1a84da8f51cd2a612e8aef2c62 Mon Sep 17 00:00:00 2001 From: Krstan Vjestica Date: Mon, 10 Aug 2026 12:40:00 +0200 Subject: [PATCH 09/12] docs: deployment guide for the two-service layout Four things in here were not just stale but actively wrong after the split: the service list, the GPU section (it told the reader to uncomment a block that no longer exists, on the wrong service), teardown that stops the API and leaves the model server up, and a backup procedure that claimed one volume holds both the index and the models. --- docs/deployment.md | 76 ++++++++++++++++++++++++++++++---------------- 1 file changed, 50 insertions(+), 26 deletions(-) diff --git a/docs/deployment.md b/docs/deployment.md index 97aa74e..805bd8b 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -9,17 +9,28 @@ cd forail-assistant docker compose up -d ``` -This starts a **single all-in-one container** (`forail-assistant`) that bundles: -- **Ollama** — LLM server (internal, port 11434) -- **ChromaDB** — vector store (embedded, port 8000) -- **FastAPI** — API server (exposed on port 8100) +This starts **two containers**: + +| Service | Contains | Ports | +|---------|----------|-------| +| `forail-assistant` | FastAPI API server + embedded ChromaDB (port 8000, in-container) | 8100 published | +| `ollama` | The model server, from the pinned `ollama/ollama` image | none published | + +Ollama is deliberately given no published port: it has no authentication, so it +is reachable only by the API over the Compose network. Each service keeps its +own volume — `assistant_data` for the vector index, `ollama_models` for model +blobs — so rebuilding one does not discard the other's data. ### First-Time Setup -On first start, the entrypoint automatically pulls the LLM model (`gemma3:1b`) and embedding model (`nomic-embed-text`). Allow ~2 minutes for the initial model download. +On first start the API waits for the model server to come up, then pulls the LLM +model (`gemma3:1b`) and the embedding model (`nomic-embed-text`) over Ollama's +HTTP API. Allow ~2 minutes for the initial download. The wait is bounded at +300s; past that the API exits with the URL it was trying to reach rather than +hanging. ```bash -# 1. Start the container +# 1. Start both services docker compose up -d # 2. Wait for health check to pass (start_period is 120s) @@ -74,24 +85,27 @@ The Forail frontend automatically detects the assistant by calling `/assistant/a ## GPU Support -For GPU-accelerated inference, uncomment the GPU section in `docker-compose.yml`: - -```yaml -services: - forail-assistant: - deploy: - resources: - reservations: - devices: - - driver: nvidia - count: 1 - capabilities: [gpu] +Inference happens in the `ollama` service alone, so that is the only service the +GPU is handed to. It ships as an overlay rather than a commented-out block: + +```bash +docker compose -f docker-compose.yml -f docker-compose.gpu.yml up -d ``` +The reservation is a hard requirement, on purpose — on a host without a usable +GPU the stack refuses to start instead of quietly falling back to CPU. + Requirements: -- NVIDIA GPU with 8+ GB VRAM -- nvidia-container-toolkit installed on the host -- Docker configured with nvidia runtime +- NVIDIA GPU (8+ GB VRAM for an 8B-class model; `gemma3:1b` needs ~2 GB) +- `nvidia-container-toolkit` installed and registered with Docker: + `sudo nvidia-ctk runtime configure --runtime=docker && sudo systemctl restart docker` + +Ollama falls back to CPU silently when it cannot see a device, so confirm: + +```bash +docker compose logs ollama | grep "inference compute" +# must report library=CUDA and non-zero VRAM, not library=cpu +``` --- @@ -112,10 +126,11 @@ Response time will be 10-20 seconds instead of 2-5 seconds. To remove the assistant from a running Forail deployment: ```bash -# Stop assistant service +# Stop the assistant services (both of them — stopping only the API leaves +# the model server running and holding its volume) docker compose -f docker-compose.yml \ -f /path/to/forail-assistant/docker-compose.integration.yml \ - down forail-assistant + down forail-assistant ollama # Or if running standalone cd forail-assistant && docker compose down -v @@ -127,10 +142,15 @@ The Forail platform continues to work normally. The chat button disappears autom ## Backup and Restore -All persistent data (ChromaDB + Ollama models) is stored in the `assistant_data` volume mounted at `/data`: +Persistent data lives in two volumes, one per service: + +| Volume | Mounted at | Holds | +|--------|-----------|-------| +| `assistant_data` | `/data` in the API container | the ChromaDB vector index | +| `ollama_models` | `/root/.ollama` in the model server | pulled model blobs | ```bash -# Backup +# Backup the vector index docker run --rm -v forail-assistant_assistant_data:/data -v $(pwd)/backups:/backup \ alpine tar czf /backup/assistant-data.tar.gz /data @@ -139,7 +159,11 @@ docker run --rm -v forail-assistant_assistant_data:/data -v $(pwd)/backups:/back alpine tar xzf /backup/assistant-data.tar.gz -C / ``` -> **Tip:** Re-indexing docs (`curl -X POST http://localhost:8100/api/v1/index?rebuild=true`) is fast and often easier than restoring ChromaDB data. Model re-download is automatic on first start if models are missing. +> **Tip:** Neither volume actually has to be backed up. Re-indexing +> (`curl -X POST http://localhost:8100/api/v1/index?rebuild=true`) rebuilds the +> index from `docs_to_index/`, and models re-download on first start when +> `ollama_models` is empty. Back `ollama_models` up only to avoid re-pulling +> several GB on a metered or air-gapped host. --- From 6645da0d1ed1d9793c7a2c64d109491c7bd4bae8 Mon Sep 17 00:00:00 2001 From: Krstan Vjestica Date: Wed, 12 Aug 2026 09:15:00 +0200 Subject: [PATCH 10/12] docs: disaster recovery covers both volumes, not one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The document opened by saying all persistent state lives in one place; since the split there are two claims, and following it would have backed up the index while silently skipping several GB of model blobs — the one thing an air-gapped host cannot re-fetch. Adds the unreachable-model-server case, which is now a distinct failure with its own message. --- docs/disaster-recovery.md | 55 ++++++++++++++++++++++++--------------- 1 file changed, 34 insertions(+), 21 deletions(-) diff --git a/docs/disaster-recovery.md b/docs/disaster-recovery.md index f1920c8..be30a9d 100644 --- a/docs/disaster-recovery.md +++ b/docs/disaster-recovery.md @@ -1,19 +1,23 @@ # Disaster Recovery — ChromaDB Index & Models -The assistant keeps **all** persistent state in one place: +The assistant runs as two services, and each keeps its own persistent state: -| Deployment | Location | Holds | -|------------|----------|-------| -| Docker Compose | volume `assistant_data` mounted at `/data` | ChromaDB vector store + pulled Ollama models | -| Kubernetes (forail-helm) | PVC `forail-assistant-data` (default **20Gi**) at `/data` | same | +| Deployment | API — vector index | Model server — model blobs | +|------------|--------------------|----------------------------| +| Docker Compose | volume `assistant_data` at `/data` | volume `ollama_models` at `/root/.ollama` | +| Kubernetes (forail-helm) | PVC `forail-assistant-data` (default **5Gi**) at `/data` | PVC `forail-assistant-ollama-models` (default **20Gi**) at `/root/.ollama` | -Inside `/data`: +What each holds: -- **ChromaDB** — the embedded vector store; collection `forail_docs` +- **ChromaDB** (API) — the embedded vector store; collection `forail_docs` (configurable via `FORAIL_ASSISTANT_CHROMA_COLLECTION`). This is the RAG index built from your documentation. -- **Ollama models** — the pulled LLM (`gemma3:1b` by default) and the - embedding model (`nomic-embed-text`). +- **Ollama models** (model server) — the pulled LLM (`gemma3:1b` by default) + and the embedding model (`nomic-embed-text`). + +Keeping them apart is the point: model blobs are far larger than the index and +outlive a rebuild of the API, so the index claim is sized by the corpus rather +than by the model. ## Key principle: the index is *rebuildable*, not precious @@ -30,10 +34,11 @@ kubectl -n forail exec deploy/forail-assistant -- \ curl -sX POST "http://localhost:8100/api/v1/index?rebuild=true" ``` -Likewise, **Ollama models re-download automatically** on first start if -they are missing. So the realistic worst case — total loss of `/data` — -recovers by: start the container (models re-pull) → re-index (index -rebuilds). No restore from backup is strictly required. +Likewise, **Ollama models re-download automatically** if they are missing — +the API pulls them over Ollama's HTTP API when it starts. So the realistic +worst case — losing both volumes — recovers by: start the stack (models +re-pull) → re-index (index rebuilds). No restore from backup is strictly +required. > This is why the assistant is safe to run with `assistant.enabled=false` > by default and to add/remove freely: it carries no irreplaceable state. @@ -45,32 +50,40 @@ rebuilds). No restore from backup is strictly required. | **Index empty / never built** | Chat answers with no doc context | `POST /api/v1/index` | | **Index stale** (docs changed) | Answers cite old content | `POST /api/v1/index?rebuild=true` | | **Index corrupted** | Chat errors, ChromaDB read failures in logs | Delete the Chroma dir under `/data`, restart, then `?rebuild=true` | -| **Models missing** | Health check fails on startup, "model not found" | Just restart — entrypoint re-pulls `gemma3:1b` + `nomic-embed-text` | -| **Total `/data` loss** | Fresh/empty volume | Restart (models re-pull) → `POST /api/v1/index?rebuild=true` | -| **PVC lost (k8s)** | Pod stuck / volume gone | Recreate PVC (helm re-apply), pod re-pulls models, re-index | +| **Models missing** | Health check fails on startup, "model not found" | Just restart the API — it re-pulls `gemma3:1b` + `nomic-embed-text` over Ollama's API | +| **Model server down / unreachable** | API exits after 300s with `Ollama not reachable at ...` | Check the `ollama` service (Compose) or the `forail-assistant-ollama` Deployment and its Service, then restart the API | +| **Total volume loss** | Fresh/empty volumes | Restart (models re-pull) → `POST /api/v1/index?rebuild=true` | +| **PVC lost (k8s)** | Pod stuck / volume gone | Recreate the PVC (helm re-apply); `forail-assistant-ollama-models` re-pulls, `forail-assistant-data` re-indexes | -## Optional: back up `/data` to skip re-download/re-index +## Optional: back up the volumes to skip re-download/re-index Re-indexing and model re-download are usually faster than a restore, but for air-gapped hosts (no registry to re-pull models from) or very large -corpora, back up the volume: +corpora, back the volumes up. On an air-gapped host the **model** volume is +the one that matters — the index can always be rebuilt from `docs_to_index/`, +while models cannot be pulled at all: ```bash -# Compose — backup +# Compose — backup (index, then models) docker run --rm -v forail-assistant_assistant_data:/data -v "$(pwd)/backups":/backup \ alpine tar czf /backup/assistant-data.tar.gz /data +docker run --rm -v forail-assistant_ollama_models:/models -v "$(pwd)/backups":/backup \ + alpine tar czf /backup/ollama-models.tar.gz /models # Compose — restore docker run --rm -v forail-assistant_assistant_data:/data -v "$(pwd)/backups":/backup \ alpine tar xzf /backup/assistant-data.tar.gz -C / +docker run --rm -v forail-assistant_ollama_models:/models -v "$(pwd)/backups":/backup \ + alpine tar xzf /backup/ollama-models.tar.gz -C / ``` ```bash -# Kubernetes — snapshot the PVC with your CSI VolumeSnapshot class, or copy it out: +# Kubernetes — snapshot both PVCs with your CSI VolumeSnapshot class, or copy them out: kubectl -n forail exec deploy/forail-assistant -- tar czf - /data > assistant-data.tar.gz +kubectl -n forail exec deploy/forail-assistant-ollama -- tar czf - /root/.ollama > ollama-models.tar.gz ``` -For air-gapped clusters, back up `/data` **after** the first successful +For air-gapped clusters, back both up **after** the first successful model pull + index so the restore is fully self-contained. ## Recovery objectives From 0f1b2f9553cebe3e23cf59e5296db556ea98f9ce Mon Sep 17 00:00:00 2001 From: Krstan Vjestica Date: Thu, 13 Aug 2026 16:30:00 +0200 Subject: [PATCH 11/12] docs: the indexed image table no longer claims Ollama is in our image This file is part of docs_to_index/, so the assistant answers from it. Left alone it would have kept telling users the model server ships inside the application image. --- docs_to_index/deployment/08-ci-cd-pipeline.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs_to_index/deployment/08-ci-cd-pipeline.md b/docs_to_index/deployment/08-ci-cd-pipeline.md index 69143c0..4c13b5f 100644 --- a/docs_to_index/deployment/08-ci-cd-pipeline.md +++ b/docs_to_index/deployment/08-ci-cd-pipeline.md @@ -60,7 +60,8 @@ No third-party credentials are required — everything runs in the GitHub-hosted | `ghcr.io/forail-platform/forail-backend:` | Same | Version-tagged (CalVer) | | `ghcr.io/forail-platform/forail-frontend:latest` | `forail-frontend/Dockerfile` | React SPA + nginx | | `ghcr.io/forail-platform/forail-frontend:` | Same | Version-tagged | -| `ghcr.io/forail-platform/forail-assistant:latest` | `forail-assistant/Dockerfile` | FastAPI + Ollama + ChromaDB (preview) | +| `ghcr.io/forail-platform/forail-assistant:latest` | `forail-assistant/Dockerfile` | FastAPI + ChromaDB (preview) — the model server is not in this image | +| `ollama/ollama:` | upstream | Model server for the assistant. Not built here; pinned in `images.assistantOllama` | | `ghcr.io/forail-platform/forail-operator:` | `forail-operator/Dockerfile` | Kubernetes operator | All images are **public** — no pull secret required for `docker pull` or `helm install`. From 93c7671c3ae80e2987677d25280de4a48f508b7d Mon Sep 17 00:00:00 2001 From: Krstan Vjestica Date: Wed, 19 Aug 2026 20:10:00 +0200 Subject: [PATCH 12/12] fix: nothing bounded the size or duration of a chat request `chat_max_concurrency` caps how many generations run at once and says nothing about how large or how long any one of them is. Four callers could hold every slot for the full Ollama timeout with a prompt the size of a book, and the service would look healthy the whole time. - The message is capped at 4000 characters (413 over it) and a blank one is rejected outright rather than sent to the model. - History is trimmed to the most recent 20 turns and 16000 characters rather than rejected: dropping the oldest turns costs a little context, while a 413 in the middle of a conversation ends it. History matters more than the message here -- every turn is re-sent to the model and paid for again on the next request. - The page context is truncated to 200 characters. It is a route, not a payload. - A generation gets a 180s deadline independent of the model's own timeout. One that will not stop still ends, because the slot it holds is one of only four. Defaults are settings, so an operator who wants a longer conversation can have one. Documented alongside the chat token, whose empty default leaves the endpoint open -- the chart now always sets it. --- app/config.py | 15 ++++++ app/main.py | 59 ++++++++++++++++++++-- docs/configuration.md | 11 +++++ tests/test_api.py | 110 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 192 insertions(+), 3 deletions(-) diff --git a/app/config.py b/app/config.py index c4a6992..0662cee 100644 --- a/app/config.py +++ b/app/config.py @@ -41,6 +41,21 @@ class Settings(BaseSettings): # exhausts GPU/CPU. Excess requests get 429. chat_max_concurrency: int = 4 + # Bounds on a single request (Codex M3). The concurrency cap limits how many + # generations run at once, but says nothing about how large or how long any + # one of them is -- four callers could hold every slot for the full Ollama + # timeout with a prompt the size of a book. + # + # A question is a question: 4000 characters is longer than anyone types. + chat_max_message_chars: int = 4000 + # Turns of prior conversation kept. Each one is re-sent to the model, so an + # unbounded history is an unbounded prompt, paid for on every request. + chat_max_history_turns: int = 20 + chat_max_history_chars: int = 16000 + # Hard ceiling on one streamed response, independent of the model's own + # timeout. A generation that will not stop still ends. + chat_deadline_seconds: int = 180 + model_config = {"env_prefix": "FORAIL_ASSISTANT_"} diff --git a/app/main.py b/app/main.py index abacf92..d2929d0 100644 --- a/app/main.py +++ b/app/main.py @@ -4,6 +4,7 @@ import hmac import json import logging +import time from fastapi import FastAPI, Header, HTTPException from fastapi.middleware.cors import CORSMiddleware @@ -72,6 +73,46 @@ class ChatRequest(BaseModel): history: list[dict] | None = None +def _bounded_request(req: "ChatRequest") -> tuple[str, list[dict]]: + """ + The message and history this request is allowed to spend, or 413. + + The concurrency cap limits how many generations run at once and says nothing + about how large any one of them is: four callers could hold every slot for + the full Ollama timeout with a prompt the size of a book. History matters + more than the message, because every turn is re-sent to the model and paid + for again on the next request. + """ + message = (req.message or "").strip() + if not message: + raise HTTPException(status_code=400, detail="message must not be empty") + if len(message) > settings.chat_max_message_chars: + raise HTTPException( + status_code=413, + detail=f"message must be at most {settings.chat_max_message_chars} characters", + ) + + history = req.history or [] + if not isinstance(history, list): + raise HTTPException(status_code=400, detail="history must be a list") + + # Trimmed rather than rejected: dropping the oldest turns degrades the answer + # a little, while a 413 in the middle of a conversation ends it. + history = history[-settings.chat_max_history_turns:] + budget = settings.chat_max_history_chars + kept: list[dict] = [] + for turn in reversed(history): + if not isinstance(turn, dict): + continue + cost = len(str(turn.get("content", ""))) + if cost > budget: + break + budget -= cost + kept.append(turn) + kept.reverse() + return message, kept + + class HealthResponse(BaseModel): status: str version: str @@ -129,18 +170,30 @@ async def chat(req: ChatRequest, authorization: str | None = Header(default=None if _chat_semaphore.locked(): raise HTTPException(status_code=429, detail="Assistant busy, retry shortly") + message, history = _bounded_request(req) + page_context = "" if req.context and req.context.get("page"): - page_context = req.context["page"] + page_context = str(req.context["page"])[:200] async def event_generator(): async with _chat_semaphore: + deadline = time.monotonic() + settings.chat_deadline_seconds try: async for token in stream_chat( - message=req.message, + message=message, page_context=page_context, - history=req.history, + history=history, ): + # A generation that will not stop still has to end: the + # slot it holds is one of only chat_max_concurrency. + if time.monotonic() > deadline: + logger.warning( + "Chat generation exceeded %ss deadline; cutting the stream", + settings.chat_deadline_seconds, + ) + yield {"data": json.dumps({"error": "response timed out", "done": True})} + return yield {"data": json.dumps({"token": token})} yield {"data": json.dumps({"done": True})} except Exception: diff --git a/docs/configuration.md b/docs/configuration.md index 9edf6bd..4dd39b0 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -23,6 +23,17 @@ All configuration is via environment variables with the `FORAIL_ASSISTANT_` pref | `FORAIL_ASSISTANT_CHROMA_PORT` | `8000` | ChromaDB port | | `FORAIL_ASSISTANT_CHROMA_COLLECTION` | `forail_docs` | Collection name for indexed documents | +### Request Limits + +| Variable | Default | Description | +|----------|---------|-------------| +| `FORAIL_ASSISTANT_CHAT_TOKEN` | `""` | Bearer token required on `/api/v1/chat`. **Empty means the endpoint is open** — set it whenever the service is reachable by anything you do not control | +| `FORAIL_ASSISTANT_CHAT_MAX_CONCURRENCY` | `4` | Concurrent generations; excess requests get 429 | +| `FORAIL_ASSISTANT_CHAT_MAX_MESSAGE_CHARS` | `4000` | Longest accepted question; over it returns 413 | +| `FORAIL_ASSISTANT_CHAT_MAX_HISTORY_TURNS` | `20` | Prior turns kept. Trimmed, not rejected — every turn is re-sent to the model on each request | +| `FORAIL_ASSISTANT_CHAT_MAX_HISTORY_CHARS` | `16000` | Total history size kept, oldest turns dropped first | +| `FORAIL_ASSISTANT_CHAT_DEADLINE_SECONDS` | `180` | Hard ceiling on one streamed response. A generation that will not stop still ends, so it cannot hold a concurrency slot indefinitely | + ### RAG Settings | Variable | Default | Description | diff --git a/tests/test_api.py b/tests/test_api.py index 2b60176..aae88a3 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -156,3 +156,113 @@ def test_openapi_schema(self, client): schema = resp.json() assert "/api/v1/health" in schema["paths"] assert "/api/v1/chat" in schema["paths"] + + +class TestChatRequestBounds: + """ + Codex M3: the concurrency cap limits how many generations run at once and + says nothing about how large or how long any one of them is. Four callers + could hold every slot for the full Ollama timeout with a prompt the size of + a book. + """ + + def _stream(self): + async def mock_stream(*args, **kwargs): + yield "ok" + return mock_stream + + def test_oversized_message_is_refused(self, client): + from app.config import settings + + resp = client.post( + "/api/v1/chat", + json={"message": "x" * (settings.chat_max_message_chars + 1)}, + ) + assert resp.status_code == 413 + + def test_message_at_the_limit_is_accepted(self, client): + from app.config import settings + + with patch("app.main.stream_chat", side_effect=self._stream()): + resp = client.post( + "/api/v1/chat", + json={"message": "x" * settings.chat_max_message_chars}, + ) + assert resp.status_code == 200 + + def test_blank_message_is_refused(self, client): + resp = client.post("/api/v1/chat", json={"message": " "}) + assert resp.status_code == 400 + + def test_history_is_trimmed_to_the_most_recent_turns(self, client): + # Trimmed rather than rejected: dropping the oldest turns costs a little + # context, while a 413 mid-conversation ends it. + from app.config import settings + + captured = {} + + async def mock_stream(*args, **kwargs): + captured.update(kwargs) + yield "ok" + + history = [{"role": "user", "content": f"turn {i}"} for i in range(200)] + with patch("app.main.stream_chat", side_effect=mock_stream): + resp = client.post("/api/v1/chat", json={"message": "hi", "history": history}) + + assert resp.status_code == 200 + assert len(captured["history"]) <= settings.chat_max_history_turns + # The turns kept are the recent ones, not the first ones. + assert captured["history"][-1]["content"] == "turn 199" + + def test_history_is_trimmed_by_total_size(self, client): + from app.config import settings + + captured = {} + + async def mock_stream(*args, **kwargs): + captured.update(kwargs) + yield "ok" + + history = [{"role": "user", "content": "x" * 5000} for _ in range(10)] + with patch("app.main.stream_chat", side_effect=mock_stream): + client.post("/api/v1/chat", json={"message": "hi", "history": history}) + + total = sum(len(t["content"]) for t in captured["history"]) + assert total <= settings.chat_max_history_chars + + def test_page_context_is_truncated(self, client): + captured = {} + + async def mock_stream(*args, **kwargs): + captured.update(kwargs) + yield "ok" + + with patch("app.main.stream_chat", side_effect=mock_stream): + client.post( + "/api/v1/chat", + json={"message": "hi", "context": {"page": "/x" * 5000}}, + ) + assert len(captured["page_context"]) <= 200 + + def test_a_generation_that_will_not_stop_is_cut(self, client): + # The slot it holds is one of only chat_max_concurrency, so an endless + # stream is a denial of service against the other three. A deadline + # already in the past is the same code path as one that runs out. + from app.config import settings + + emitted = 0 + + async def endless(*args, **kwargs): + nonlocal emitted + for _ in range(100): + emitted += 1 + yield "token" + + with patch("app.main.stream_chat", side_effect=endless), \ + patch.object(settings, "chat_deadline_seconds", -1): + resp = client.post("/api/v1/chat", json={"message": "hi"}) + + assert resp.status_code == 200 + assert "timed out" in resp.text + # Cut, not drained: the generator does not run to completion. + assert emitted < 100