Skip to content

fix(models): self-healing GGUF loading + self-diagnosing model status#19

Merged
FzzySwtr merged 2 commits into
mainfrom
claude/app-models-env-issue-15db45
Jul 17, 2026
Merged

fix(models): self-healing GGUF loading + self-diagnosing model status#19
FzzySwtr merged 2 commits into
mainfrom
claude/app-models-env-issue-15db45

Conversation

@FzzySwtr

Copy link
Copy Markdown
Contributor

Problem

The app reported "degraded / LLM missing" even when the GGUF models were present in the environment. Root cause: the server loaded models exactly once at process startup and never retried, so if the files weren't visible at that instant it stayed degraded permanently. Two real ways this happens:

  1. Files provisioned after the container starts → the server already gave up.
  2. The mount resolves somewhere the server isn't looking — Compose resolves ./gguf_models relative to the compose file, and the container runs as non-root uid 10001, so a directory it can't read looks identical to "empty". The only diagnostic was an opaque relative path.

Ingestion had the mirror bug: it exited 1 when the embedding model was absent, so the search index never built either — the "0 chunks / 0 files" in the reported screenshot.

This fixes the repo behavior for everyone, not a single environment.

What changed

Server (src/server.cpp)

  • g_llm / g_embeddings are now std::atomic<X*> (transition null→loaded exactly once, never revert), so a background thread can publish them lock-free while request handlers run on cpp-httplib's worker pool. Handlers acquire-load into a local and use that for the whole request (no check-then-use race).
  • A background model_loader thread (started before listen(), joined before the shutdown deletes) loads each model as soon as its file is present and settled (FILE_SETTLE_SECONDS), independently per pointer so it never reloads over a live slot (which would leak a multi-GB model every scan). Dropping GGUF files in later now works with no restart — the same way ingestion already auto-indexes documents.
  • /status gains gguf_dir, llm_gguf_present, embedding_gguf_present.

Diagnostics (src/config.h)

  • JIC_GGUF_DIR overrides the model directory (fixes the wrong-mount case).
  • describe_model_path() logs the resolved absolute path + present/size, or the directory listing / (unreadable: Permission denied) when missing. Iterates with directory_iterator::increment(ec) so it never throws.

Ingestion (src/ingestion.cpp)

  • Waits/retries for the embedding model (settle-checked) instead of crash-looping; builds the index once the model appears.
  • No longer poisons the index: a failed embedding is skipped instead of stored as a zero vector, and a file whose chunks all failed is left unmarked to retry; partial failures are logged.

Models (src/embeddings.h, src/llm.h)

  • init() is idempotent (frees prior model/ctx) so retries can't leak; get_embedding() returns an empty vector on failure (was an EMBEDDING_DIM zero vector) and guards a null context.

Provisioning / UI

  • fetch-models.sh: download to .part then atomic mv, so a running container never mmaps a half-written GGUF (SIGBUS) and a broken download can't land at the real path.
  • app.js: degraded banner now says models load automatically (no restart) and shows the resolved gguf_dir.

How it was validated

  • Reproduced then fixed on real 2 GB / 84 MB models: start with an empty models dir → degraded → drop the GGUFs in after boot → server auto-loads with no restart, /query answers, ingestion builds the index (10/10 end-to-end).
  • CI parity: test-config.sh 38/38 · degraded test-server.sh 21/21 (docker-smoke) · style-canon lint OK · unit tests OK · clean image build · container screenshot gate passes.
  • The design and the implemented diff were each put through an adversarial multi-agent review; all surviving findings were low-severity and are addressed or documented in-code.

Reviewer notes

  • The atomic pointers only ever go null→loaded once and are deleted only after listen() joins all handler workers and the loader is joined — no use-after-free.
  • The container runs as uid 10001, so a host gguf_models dir must stay world-readable (0755/0644); the new /status fields + logs now diagnose that case explicitly.
  • docker-compose.yml / Dockerfile are intentionally untouched; JIC_GGUF_DIR defaults to ./gguf_models.
  • Known low-severity follow-ups left as-is: a settled-but-corrupt GGUF logs once then keeps retrying; a partial mid-file embedding failure commits what stored and WARNs (no auto re-ingest, since add_batch is INSERT-not-upsert).

🤖 Generated with Claude Code

The server loaded models exactly once at startup, so if the GGUF files
weren't visible at that instant it stayed permanently "degraded / LLM
missing" with no retry — even when the models were present. This bit two
real cases: files provisioned after the container starts, and a bind mount
resolving somewhere the server wasn't looking (compose resolves
./gguf_models against the compose file, and the container runs as non-root
uid 10001, so an unreadable dir looks identical to "empty"). The only clue
was an opaque relative path. Ingestion had the mirror problem: it exited 1
when the embedding model was absent, so the index never built either
("0 chunks / 0 files").

Server (src/server.cpp):
- g_llm / g_embeddings are now std::atomic<X*> (null->loaded exactly once,
  never revert) so a background loader can publish them lock-free while
  request handlers run on cpp-httplib's worker pool; handlers acquire-load
  into a local and use that for the whole request (no check-then-use race).
- A background model_loader thread (started before listen(), joined before
  the shutdown deletes) loads each model as soon as its file is present and
  settled (FILE_SETTLE_SECONDS), independently per pointer so it never
  reloads over a live slot (which would leak a multi-GB model each scan).
  Dropping the GGUF files in later now "just works" with no restart, the
  same way ingestion already auto-indexes documents.
- /status gains gguf_dir, llm_gguf_present, embedding_gguf_present.

Diagnostics (src/config.h):
- JIC_GGUF_DIR overrides the model directory (fixes the wrong-mount case).
- describe_model_path() logs the resolved absolute path + present/size, or
  the directory listing / "unreadable: Permission denied" when missing.
  Iterates with directory_iterator::increment(ec) so it never throws.

Ingestion (src/ingestion.cpp):
- Waits/retries for the embedding model (settle-checked) instead of
  crash-looping; builds the index once the model appears.
- No longer poisons the index: a failed embedding is skipped rather than
  stored as a zero vector, and a file whose chunks all failed is left
  unmarked to retry; partial failures are logged.

Models (src/embeddings.h, src/llm.h):
- init() is idempotent (frees prior model/ctx) so retries can't leak.
- get_embedding() returns an empty vector on failure (was an EMBEDDING_DIM
  zero vector) and guards a null context.

fetch-models.sh: download to .part then atomic mv, so a running container
never mmaps a half-written GGUF (SIGBUS) and a broken download can't land
at the real path.

app.js: degraded banner now says models load automatically (no restart)
and shows the resolved gguf_dir from /status.

Verified: unit tests, test-config.sh (38/38), style-canon lint, clean image
build, degraded test-server.sh (21/21, CI docker-smoke parity), and an
end-to-end auto-load run — start with an empty models dir, drop the real
2GB/84MB GGUFs in after boot, and the server loads them with no restart,
/query answers, and ingestion builds the index.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Copilot AI review requested due to automatic review settings July 17, 2026 04:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR makes GGUF model provisioning resilient and self-diagnosing across the server and ingestion services, so the app can start in degraded mode, automatically recover once models appear, and clearly report where it is looking and why it can’t load.

Changes:

  • Server: publish LLM/embedding generators via atomic pointers and load them asynchronously in a background model_loader; /status now reports the resolved GGUF directory and model file presence.
  • Config/diagnostics: add JIC_GGUF_DIR override and richer describe_model_path() logging for missing/unreadable model paths.
  • Ingestion + embeddings: wait/retry for the embedding model instead of exiting; treat embedding failures as “no embedding” (empty vector) to avoid poisoning the index.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/server.cpp Background model loading + atomic publication; /status diagnostics and degraded-mode behavior updates.
src/config.h Adds JIC_GGUF_DIR, resolved path reporting, and detailed “why model missing” diagnostics.
src/ingestion.cpp Wait/retry for embedding model availability; skip failed embeddings and avoid marking fully-failed files processed.
src/embeddings.h Makes init() idempotent and returns empty vectors on embedding failures; guards null context.
src/llm.h Makes init() idempotent to support retry semantics without leaks.
public/app.js Improves degraded banner messaging and displays the server-reported model directory.
helper-scripts/fetch-models.sh Downloads to .part then atomically renames to avoid half-written GGUF files being mmap’d.

Comment thread src/server.cpp Outdated
Comment thread src/config.h
…eadable-dir diagnostic

- server.cpp: the degraded /query 503 message now points at the resolved
  model directory (resolved_gguf_dir(), which honours JIC_GGUF_DIR and matches
  /status) instead of a hardcoded "gguf_models/", so an overridden mount is
  reported correctly.
- config.h: describe_model_path() no longer mislabels an unreadable model
  directory as "does not exist" — when is_directory() itself fails and sets an
  error_code (e.g. a non-traversable parent), it now reports the actual reason.

Verified: unit tests, degraded test-server.sh (21/21), test-config.sh (38/38),
clean image build; /query message follows JIC_GGUF_DIR=/models at runtime.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
@FzzySwtr
FzzySwtr merged commit e6b8e57 into main Jul 17, 2026
6 checks passed
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.

2 participants