diff --git a/.forgejo/workflows/build.yml b/.forgejo/workflows/build.yml index eb6aa08..533cf4e 100644 --- a/.forgejo/workflows/build.yml +++ b/.forgejo/workflows/build.yml @@ -1685,7 +1685,13 @@ jobs: uses: docker/build-push-action@v5 with: context: . - file: Dockerfile.ml + # The merged image (milestone 422 step 6). `fabledcurator-ml` keeps + # publishing from it — same bytes under both names — because the + # operator's Swarm stack references fabledcurator-ml:latest and + # lives outside this repo. Dropping the name here would not break + # their deploy, it would freeze it silently at the last publish. + # Retiring the NAME is its own task, gated on that stack moving. + file: Dockerfile push: true # Re-resolve the FROM references against the registry instead of # trusting whatever digest the cache was built against. This is the diff --git a/Dockerfile b/Dockerfile index f0e184d..292a087 100644 --- a/Dockerfile +++ b/Dockerfile @@ -32,13 +32,51 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ libwebp7 \ libpng16-16 \ ca-certificates \ + # opencv-python-headless (via requirements-ml.txt) links these even in its + # headless build. Came from Dockerfile.ml when the images merged + # (milestone 422 step 6). + libgl1 \ + libglib2.0-0 \ && rm -rf /var/lib/apt/lists/* WORKDIR /app -COPY requirements.txt ./ +COPY requirements.txt requirements-ml.txt ./ RUN pip install -r requirements.txt +# --- ML, merged from Dockerfile.ml (milestone 422 step 6) -------------------- +# +# ONE image now serves every lane. It was two because the ML lane ran in its +# own container; with the single-container layout (step 5) running every lane +# in one process tree, a second image would mean the `ml` lane could never be +# enabled from the UI — there would be no worker in this container to enable. +# +# The COST, stated because it is real and falls on every adopter: this adds +# torch, torchvision, transformers, onnxruntime and opencv to an image that +# previously carried none of them. Everyone pulls it, including the many who +# will never turn tagging on. That is the trade the milestone accepted for +# being able to offer the lane as a switch rather than a second deployment. +# What it buys back is that nothing downloads a MODEL until the switch is +# thrown — the weights are not baked in, and rule 164 permits that only +# because the feature is optional and clearly off. +# +# CPU-only torch from the PyTorch CPU index. The default PyPI wheel bundles +# the NVIDIA CUDA runtime (~5.6GB of layer) and nothing here uses a GPU — the +# GPU agent is a separate service with its own image. `--index-url`, not +# `--extra-index-url`: the latter would let pip resolve a +cu wheel anyway. +RUN pip install --index-url https://download.pytorch.org/whl/cpu \ + "torch>=2.12,<3.0" "torchvision>=0.27,<0.28" +RUN pip install -r requirements-ml.txt + +# Where the model lands. Deliberately NOT a VOLUME instruction: that mints an +# anonymous volume when nobody mounts one, which survives `docker rm` and +# accumulates 3.5GB copies nobody can find. The compose files mount it +# explicitly instead, so an unmounted run simply re-downloads — visible, and +# recoverable. +ENV HF_HOME=/models/.huggingface \ + TRANSFORMERS_CACHE=/models/.huggingface \ + ML_MODEL_DIR=/models + COPY backend/ ./backend/ COPY alembic/ ./alembic/ COPY alembic.ini ./ diff --git a/Dockerfile.ml b/Dockerfile.ml deleted file mode 100644 index 13efe30..0000000 --- a/Dockerfile.ml +++ /dev/null @@ -1,43 +0,0 @@ -# syntax=docker/dockerfile:1.25 - -FROM python:3.14-slim -ENV PYTHONUNBUFFERED=1 \ - PYTHONDONTWRITEBYTECODE=1 \ - PIP_NO_CACHE_DIR=1 \ - PIP_DISABLE_PIP_VERSION_CHECK=1 \ - HF_HOME=/models/.huggingface \ - TRANSFORMERS_CACHE=/models/.huggingface \ - ML_MODEL_DIR=/models - -RUN apt-get update && apt-get install -y --no-install-recommends \ - ffmpeg \ - libpq5 \ - libjpeg62-turbo \ - libwebp7 \ - libpng16-16 \ - libgl1 \ - libglib2.0-0 \ - ca-certificates \ - && rm -rf /var/lib/apt/lists/* - -WORKDIR /app - -COPY requirements-ml.txt requirements.txt ./ -# CPU-only torch: the default PyPI wheel bundles the CUDA runtime (~5.6GB -# layer); this pipeline never uses a GPU. --index-url (not --extra-index-url) -# guarantees only +cpu wheels are considered, so no nvidia-*-cu12 deps. -RUN pip install --index-url https://download.pytorch.org/whl/cpu \ - "torch>=2.12,<3.0" "torchvision>=0.27,<0.28" -RUN pip install -r requirements-ml.txt - -COPY backend/ ./backend/ -COPY alembic/ ./alembic/ -COPY alembic.ini ./ -COPY entrypoint.sh ./ -RUN chmod +x entrypoint.sh - -# Models self-heal into /models on first start (FC-2 implements this) -VOLUME ["/models"] - -ENTRYPOINT ["./entrypoint.sh"] -CMD ["ml-worker"] diff --git a/README.md b/README.md index 8c94e40..0e0cf55 100644 --- a/README.md +++ b/README.md @@ -265,7 +265,7 @@ Five deployable pieces, built by `.forgejo/workflows/build.yml`: | Piece | Built from | Image | Role | | --- | --- | --- | --- | | **Web / workers** | `Dockerfile` | `fabledcurator` | Quart API + the built Vue SPA in one image. `entrypoint.sh` picks the role: `web`, `worker`, `scheduler`. The `maintenance-long` service is a second `worker` pinned to the long-running maintenance queue. | -| **ML worker** | `Dockerfile.ml` | `fabledcurator-ml` | Same app, plus `requirements-ml.txt` — tagging and embedding models that run in-container. | +| **ML worker** | `Dockerfile` | `fabledcurator-ml` | The same image as the web service since milestone 422 — one image serves every lane. Published under this name too, for stacks that still reference it. | | **GPU agent** | `agent/Dockerfile` | `fabledcurator-agent` | Optional desktop-GPU worker (`agent/`). Leases jobs over **HTTP only** — never touches the database or Redis. See `agent/README.md`. | | **Firefox extension** | `extension/` | signed XPI | MV3 extension: pushes platform session cookies into FC and adds a creator as a Source in one click. AMO-signed on both `dev` and `main` (one signature per extension change, shared by the two channels), bundled into that channel's web image and served from Settings → Maintenance. See `extension/README.md`. | | **Data** | — | `pgvector/pgvector:pg16`, `redis:7-alpine` | Postgres with pgvector for embeddings; Redis as the Celery broker. | diff --git a/backend/app/scripts/gen_supervisord.py b/backend/app/scripts/gen_supervisord.py index 2747599..cd76171 100644 --- a/backend/app/scripts/gen_supervisord.py +++ b/backend/app/scripts/gen_supervisord.py @@ -36,12 +36,16 @@ puts tini in front of supervisord. Neither choice reaches the application — nothing in FC talks to the supervisor — so this is reversible without touching a line of product code. -## What this does NOT start +## Every lane, including ml -The `ml` lane, unless `--with-ml` is passed. Until step 6 merges the images, -torch and the ML requirements live only in `Dockerfile.ml`, so an `ml` program -in the web image would fail to import on every restart forever. Step 6 is -where that flag turns on. +Step 6 merged the images, so this one carries torch and the ML requirements +and the `ml` lane gets a program like any other. It starts at one slot with +its consumers CANCELLED — `enabled=false` in the seeded settings — so it +holds a process and no model. That matters: `add_consumer` needs a running +worker to reach, and without one the UI switch would have nothing to switch. + +Nothing is downloaded by starting it. The model fetch is enqueued when the +lane is enabled, which is what lets rule 164 permit a runtime fetch at all. """ from __future__ import annotations @@ -69,10 +73,6 @@ STOP_WAIT_SECONDS: dict[str, int] = { } DEFAULT_STOP_WAIT = 60 -# Lanes whose code is not in this image yet. See the module docstring. -_NEEDS_ML_DEPS = frozenset({"ml"}) - - def _program(lane: Lane, *, slots: int) -> str: """One [program:x] block. @@ -141,7 +141,7 @@ def _web_program() -> str: ]) -def render(*, with_ml: bool = False) -> str: +def render() -> str: parts = [ "\n".join([ "[supervisord]", @@ -158,8 +158,6 @@ def render(*, with_ml: bool = False) -> str: ] # Lanes after web, in LANES order, so the log reads in a stable sequence. for lane in LANES: - if lane.name in _NEEDS_ML_DEPS and not with_ml: - continue # A lane configured at zero slots still gets a PROCESS, at one slot # with its consumers cancelled by the reconcile. Without a running # worker there is nothing for `add_consumer` to reach, so enabling the @@ -171,12 +169,8 @@ def render(*, with_ml: bool = False) -> str: def main(argv: list[str] | None = None) -> int: ap = argparse.ArgumentParser(description=__doc__) - ap.add_argument( - "--with-ml", action="store_true", - help="include the ml lane (only valid once the ML deps are in this image)", - ) - args = ap.parse_args(argv) - sys.stdout.write(render(with_ml=args.with_ml)) + ap.parse_args(argv) + sys.stdout.write(render()) return 0 diff --git a/backend/app/scripts/healthcheck_all.py b/backend/app/scripts/healthcheck_all.py index 4b8076a..8d565e5 100644 --- a/backend/app/scripts/healthcheck_all.py +++ b/backend/app/scripts/healthcheck_all.py @@ -47,14 +47,15 @@ def _web_ok() -> tuple[bool, str]: return False, f"web unreachable: {exc}" -def _lanes_ok(*, with_ml: bool) -> tuple[bool, str]: +def _lanes_ok() -> tuple[bool, str]: from ..services.worker_control import inspect_lanes_sync from ..services.worker_lanes import LANES - expected = { - lane.name for lane in LANES - if with_ml or lane.name != "ml" - } + # Every lane, ml included: one image carries them all since step 6, and a + # disabled lane still runs a process (consumers cancelled), so it answers + # inspect and is healthy. Health is "is the process alive"; whether it + # should be consuming is the reconcile's business. + expected = {lane.name for lane in LANES} live = inspect_lanes_sync() missing = sorted(n for n in expected if not live[n].present) if missing: @@ -63,15 +64,12 @@ def _lanes_ok(*, with_ml: bool) -> tuple[bool, str]: def main(argv: list[str] | None = None) -> int: - argv = sys.argv[1:] if argv is None else argv - with_ml = "--with-ml" in argv - ok, detail = _web_ok() if not ok: print(detail, file=sys.stderr) return 1 - ok, detail = _lanes_ok(with_ml=with_ml) + ok, detail = _lanes_ok() if not ok: print(detail, file=sys.stderr) return 1 diff --git a/backend/app/services/worker_control.py b/backend/app/services/worker_control.py index 022abee..5047e9e 100644 --- a/backend/app/services/worker_control.py +++ b/backend/app/services/worker_control.py @@ -408,6 +408,20 @@ async def set_lane( if applied and slots is not None: applied, error = await asyncio.to_thread(set_lane_slots_sync, lane, new_slots) + # Enabling a lane that needs models is what triggers the fetch (milestone + # 422 step 6). Never at boot: that made every start of the ML role reach + # HuggingFace for ~3.5GB, and rule 164 permits a runtime fetch only for a + # feature that is optional and clearly OFF. + # + # Only when the lane actually came on — `enabled is True` rather than + # `new_enabled`, so re-saving slots on an already-enabled lane does not + # re-enqueue. And only when the consumer change landed: enqueueing a task + # onto a queue nothing is consuming would leave it pending with no + # explanation until the lane returns. + fetching = False + if enabled is True and lane.models and applied: + fetching = _enqueue_model_fetch() + return { "name": lane.name, "slots": row.slots, @@ -416,9 +430,34 @@ async def set_lane( "enabled": row.enabled, "applied": applied, "apply_error": error, + # Tells the card to say a download has started rather than leaving the + # operator to wonder why a freshly enabled lane is busy. + "fetching_models": fetching, } +def _enqueue_model_fetch() -> bool: + """Queue the model download. Returns whether it was accepted. + + Import inside the function: `backend.app.tasks.ml` pulls in torch, and web + must not pay that import cost on a module that every settings request + touches. + + Never raises. A broker that will not take the task is worth reporting, but + the SETTING has already been stored and the lane is already enabled — so + failing the whole request here would roll back nothing and tell the + operator their change did not happen when it did. + """ + try: + from ..tasks.ml import ensure_models + + ensure_models.delay() + return True + except Exception: # noqa: BLE001 — reported, never raised at a caller + log.warning("worker_control: could not enqueue the model fetch", exc_info=True) + return False + + def reconcile_lanes_sync(desired: dict[str, tuple[int, bool]]) -> dict: """Drive every RUNNING lane to its stored slots and enabled flag. diff --git a/backend/app/tasks/ml.py b/backend/app/tasks/ml.py index 958b57c..4388820 100644 --- a/backend/app/tasks/ml.py +++ b/backend/app/tasks/ml.py @@ -668,3 +668,31 @@ def scheduled_retract_auto_tags() -> str: with SessionLocal() as session: n_ccip = retract_auto_applied_ccip(session) return f"head={n_head} ccip={n_ccip}" + + +@celery.task(name="backend.app.tasks.ml.ensure_models", bind=True) +def ensure_models(self) -> dict: + """Fetch the models this lane needs, if they are not already present. + + Milestone 422 step 6. This used to run in `entrypoint.sh` before celery + started, which made every boot of the ML role reach HuggingFace for + ~3.5GB — a startup dependency on a third party, for a feature the operator + may never use. Rule 164 permits a runtime fetch only for something + "optional and clearly off", so it moved here: enqueued the moment the lane + is ENABLED, never at boot. + + Being a task rather than a startup step is what makes it visible: it gets + a TaskRun row like any other, so the download shows in Activity with a + duration and a status, and a failure is something the operator can see and + retry rather than a container that quietly never became useful. + + Idempotent — `download_models` fetches only what is missing — so enabling + an already-provisioned lane costs one no-op task rather than a re-download. + That matters because the reconcile may enqueue it again. + """ + from ..scripts.download_models import main as download + + rc = download() + if rc != 0: + raise RuntimeError(f"model download failed with exit code {rc}") + return {"ok": True} diff --git a/docker-compose.override.yml b/docker-compose.override.yml index aefd81f..e397e92 100644 --- a/docker-compose.override.yml +++ b/docker-compose.override.yml @@ -47,7 +47,7 @@ services: ml-worker: build: context: . - dockerfile: Dockerfile.ml + dockerfile: Dockerfile environment: LOG_LEVEL: DEBUG volumes: diff --git a/entrypoint.sh b/entrypoint.sh index b03b252..f24f007 100755 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -50,13 +50,23 @@ case "$ROLE" in ;; ml-worker) - echo "[entrypoint] Ensuring ML models present in /models..." - python -m backend.app.scripts.download_models - echo "[entrypoint] Starting ML Celery worker (ml queue)" + # NO MODEL DOWNLOAD HERE (milestone 422 step 6). This used to run + # download_models before celery started, which made every boot of this + # role reach HuggingFace for ~3.5GB. Rule 164 permits a runtime fetch only + # for a feature that is "optional and clearly off" — so the fetch moved to + # the moment the operator ENABLES the lane, where it is visible, retryable + # and attributable, instead of being a silent precondition of starting. + # + # The worker therefore starts with no model present, which is correct: it + # is not consuming the ml queue until the lane is enabled, and enabling it + # is what enqueues ensure_models. + QUEUES="${CELERY_QUEUES:-ml}" + CONCURRENCY="${CELERY_CONCURRENCY:-1}" + echo "[entrypoint] Starting ML Celery worker queues=$QUEUES concurrency=$CONCURRENCY" exec celery -A backend.app.celery_app:celery worker \ --loglevel=info \ - -Q ml \ - --concurrency=1 + -Q "$QUEUES" \ + --concurrency="$CONCURRENCY" ;; all) @@ -75,7 +85,7 @@ case "$ROLE" in # adjust a baseline that already works, and can never prevent a boot. CONF="${SUPERVISOR_CONF:-/tmp/supervisord.conf}" echo "[entrypoint] Generating $CONF from the lane table" - python -m backend.app.scripts.gen_supervisord ${FC_WITH_ML:+--with-ml} > "$CONF" + python -m backend.app.scripts.gen_supervisord > "$CONF" echo "[entrypoint] Starting supervisord (web + worker lanes)" exec supervisord -c "$CONF" ;; diff --git a/frontend/src/components/settings/WorkerLanesCard.vue b/frontend/src/components/settings/WorkerLanesCard.vue index f60a6e3..14999f4 100644 --- a/frontend/src/components/settings/WorkerLanesCard.vue +++ b/frontend/src/components/settings/WorkerLanesCard.vue @@ -201,7 +201,13 @@ async function apply(lane, fields) { // Saved but not pushed — the lane is restarting, or the broker blipped. // NOT an error: the reconcile carries it when the lane answers again, and // saying "failed" would invite the operator to set it a second time. - if (reply && reply.applied === false) { + if (reply && reply.fetching_models) { + notice.value = { + type: 'info', + text: `${lane.display_name} is on. Downloading its model now — ` + + 'watch progress in Queues + workers above. It only happens once.', + } + } else if (reply && reply.applied === false) { notice.value = { type: 'info', text: `Saved. ${lane.display_name} is not answering right now — ` diff --git a/requirements-ml.txt b/requirements-ml.txt index c216487..52e20e5 100644 --- a/requirements-ml.txt +++ b/requirements-ml.txt @@ -3,9 +3,9 @@ # ML stack — versions current as of 2026-05-14 with Python 3.14 wheel coverage. # torch + torchvision are NOT listed here: they are installed CPU-only from -# the PyTorch CPU index in Dockerfile.ml. The default PyPI torch wheel bundles +# the PyTorch CPU index in Dockerfile. The default PyPI torch wheel bundles # the NVIDIA CUDA runtime (a ~5.6GB image layer); this pipeline is CPU-only, -# so Dockerfile.ml uses the +cpu wheels from +# so Dockerfile uses the +cpu wheels from # https://download.pytorch.org/whl/cpu instead. # # IMPORTANT: torchvision 0.27 declares requires_python "!=3.14.1,>=3.10" — diff --git a/scripts/artifacts.sh b/scripts/artifacts.sh index e93b9eb..fcce132 100755 --- a/scripts/artifacts.sh +++ b/scripts/artifacts.sh @@ -51,9 +51,13 @@ ROOT=$(git rev-parse --show-toplevel) # rather than restated — one definition, per #2397. WEB_PATHS='Dockerfile requirements.txt backend alembic alembic.ini entrypoint.sh frontend :(exclude)frontend/test :(exclude)frontend/test/**' -# ml (Dockerfile.ml, context `.`) — no frontend, no extension. Note it copies +# ml (Dockerfile, context `.`) — no frontend, no extension. Note it copies # BOTH requirements-ml.txt and requirements.txt. -ML_PATHS='Dockerfile.ml requirements-ml.txt requirements.txt backend alembic alembic.ini entrypoint.sh' +# Dockerfile, not Dockerfile.ml: the images merged at milestone 422 step 6 +# and Dockerfile.ml is gone. A path set naming a deleted file silently +# stops contributing to the derived revision, which is what the reuse +# check and the version string both read (#3202's shape). +ML_PATHS='Dockerfile requirements-ml.txt requirements.txt backend alembic alembic.ini entrypoint.sh' # agent (agent/Dockerfile, context `agent`) — copies requirements.txt and # fc_agent only. agent/README.md, agent/docker-compose.yml and agent/ruff.toml diff --git a/tests/test_artifact_paths.py b/tests/test_artifact_paths.py index 888d4c3..22b4c51 100644 --- a/tests/test_artifact_paths.py +++ b/tests/test_artifact_paths.py @@ -31,7 +31,7 @@ ROOT = Path(__file__).resolve().parent.parent # artifact -> (dockerfile, build context relative to the repo root) ARTIFACTS = { "web": ("Dockerfile", ""), - "ml": ("Dockerfile.ml", ""), + "ml": ("Dockerfile", ""), "agent": ("agent/Dockerfile", "agent"), } diff --git a/tests/test_gen_supervisord.py b/tests/test_gen_supervisord.py index b92cce3..7f27745 100644 --- a/tests/test_gen_supervisord.py +++ b/tests/test_gen_supervisord.py @@ -37,20 +37,21 @@ def test_it_is_valid_ini_with_a_supervisord_section(): assert cp.get("supervisord", "nodaemon") == "true" -def test_web_and_every_non_ml_lane_get_a_program(): +def test_every_lane_gets_a_program(): + """One image carries every lane since step 6, so nothing is conditional. + A lane in LANES with no program is a queue with no consumer.""" cp = _parse() - expected = {"program:web"} | { - f"program:{lane.name}" for lane in LANES if lane.name != "ml" - } + expected = {"program:web"} | {f"program:{lane.name}" for lane in LANES} assert set(cp.sections()) - {"supervisord"} == expected -def test_the_ml_lane_is_absent_until_its_deps_are_in_the_image(): - """The web image has no torch. An `ml` program here would fail to import - on every restart, forever — startretries would give up and the lane would - be permanently dead while the container reported healthy.""" - assert not _parse().has_section("program:ml") - assert _parse(with_ml=True).has_section("program:ml") +def test_the_ml_lane_runs_even_though_it_ships_disabled(): + """It holds a PROCESS and no model. `add_consumer` needs a running worker + to reach, so without this the UI switch would have nothing to switch — + and nothing is downloaded by starting it, which is what lets rule 164 + permit the fetch at all.""" + assert _parse().has_section("program:ml") + assert LANES_BY_NAME["ml"].default_enabled is False # --- the coupling this generator exists to guarantee ------------------------- @@ -60,7 +61,7 @@ def test_each_program_serves_exactly_its_lane_s_queues(): """The whole point: the container's processes and the application's lane table are one list. A queue in LANES with no program means work that queues forever with nothing consuming it.""" - cp = _parse(with_ml=True) + cp = _parse() for lane in LANES: env = cp.get(f"program:{lane.name}", "environment") # The QUOTED form. supervisord splits `environment` on commas, so an @@ -75,7 +76,7 @@ def test_each_program_invokes_the_lane_s_entrypoint_role_not_its_name(): queue — exactly as docker-compose starts it today. Invoking `entrypoint.sh maintenance_long` would hit the unknown-role branch and exit 1 on every restart.""" - cp = _parse(with_ml=True) + cp = _parse() for lane in LANES: command = cp.get(f"program:{lane.name}", "command") assert f"entrypoint.sh {lane.entrypoint_role}" in command @@ -104,7 +105,7 @@ def test_every_program_signals_its_whole_process_group(): parent leaves them running and holding tasks — a 'graceful' shutdown that orphans workers. The `sh -c … | sed` wrapper makes this doubly necessary: without it the signal reaches the shell holding the pipeline, not celery.""" - cp = _parse(with_ml=True) + cp = _parse() for section in cp.sections(): if not section.startswith("program:"): continue @@ -133,7 +134,7 @@ def test_no_program_waits_longer_than_the_compose_stop_grace_period(): assert m, "docker-compose.single.yml has no stop_grace_period" grace = int(m.group(1)) - cp = _parse(with_ml=True) + cp = _parse() for section in cp.sections(): if section.startswith("program:"): assert cp.getint(section, "stopwaitsecs") <= grace, section @@ -146,7 +147,7 @@ def test_a_zero_slot_lane_still_gets_a_running_process(): """ML ships at 0 slots and disabled — but `add_consumer` needs something to reach. With no process there would be nothing for the UI switch to switch, and enabling tagging could not work at all.""" - cp = _parse(with_ml=True) + cp = _parse() assert LANES_BY_NAME["ml"].default_slots == 0 env = cp.get("program:ml", "environment") assert "CELERY_CONCURRENCY=1" in env @@ -174,7 +175,7 @@ def test_every_program_writes_to_the_container_stdout_with_its_lane_named(): """Four celery workers and hypercorn on one stream are indistinguishable without this. Unbuffered (`maxbytes 0`) so `docker logs` is live rather than arriving in rotated chunks.""" - cp = _parse(with_ml=True) + cp = _parse() for section in cp.sections(): if not section.startswith("program:"): continue