Skip to content

os/v3 puppy: the image seeds extra_models_dir, so models staged in /data/models reach the catalogue - #1151

Merged
bong-water-water-bong merged 2 commits into
mainfrom
os/v3-extra-models-dir
Nov 5, 2026
Merged

bong-water-water-bong merged 2 commits into
mainfrom
os/v3-extra-models-dir

Conversation

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator

Root cause

lemond only scans a models directory when extra_models_dir is set — ScanCatalog::enumerate() returns an empty map for an empty setting (src/cpp/server/models/scan_catalog.cpp:179), and nothing in the image ever seeded it. The embedded 10lemonade seeded log_level and the rotation caps only.

So on a booted image, files staged in /data/models were invisible and the catalogue read none downloaded while they sat on the medium. c26 works on a stick only because the stick writer stages its own config.json containing that key — a frugal/installed or dd-written image has no writer, which is where the gap bites.

This was first observed from the outside by pi-tui on a booted c25/c26 candidate (~/briefs/TTUI-EVIDENCE-c26-console-acceptance.md: catalogue empty, then "with the exact config.json the stick writer stages, it listed and served the model immediately"), and it is brief item 5's "models_dir config", which c26 carried on the stick but not in the image.

Measured before/after — device-free, listing only, on the shipping payload (rc-2026-11-05-c26c)

A real gguf staged in models/, lemond on 127.0.0.1, GET /api/v1/models?show_all=true, no model loaded, no GPU:

config result
exactly what 10lemonade seeds today (log_level + rotation) staged file absent, 213 rows
same config + extra_models_dir 214 rows; the extra one is Qwen3-0.6B-Q8_0 | recipe llamacpp | downloaded true | source extra_models_dir

(One test artefact worth recording: my first attempt staged the model as a symlink to a weights path that is itself a dead link on this box, so both arms read 213 and the key looked innocent. With a real file the difference is exactly one row.)

The fix

os/v3/puppy/embed/etc/init.d/10lemonade:

  • creates the models directory — /data/models when a /data partition is mounted (the path staging and the docs use), $LEM/models in the RAM case;
  • seeds "extra_models_dir" into a fresh config alongside the existing keys;
  • into an existing config only when the key is absent, via setdefault, so a value written by the stick writer or an operator is never overwritten.

Seeding proved against three known configs, extracting the real block out of the shipped script and running it: fresh → key + dir + valid JSON; log-level-only → keeps its log_level: info, host, port, gains the key; stick-writer's config → untouched, extra_models_dir/models_dir preserved verbatim.

Gate

vm-check gains U3 models dir: it reads the configured path out of /data/lemonade/config/config.json and requires that directory to exist. Its own command was run against three configs as a known-answer probe — seeded ones print path=…/models models-dir-seeded, and the pre-fix config prints path=<unset> and the row goes red, so the row catches the regression rather than describing the fix.

bash -n clean on both files; shellcheck -S warning reports only the pre-existing SC3045 at 10lemonade:40 (ulimit -l, unchanged by this PR and identical on main); tools/copyright.py --check rc 0 with a clean tree.

Not in this PR

  • No image re-cut: this lands for the next cycle's cut. c26's stick is unaffected because its staged config already carries the key.
  • 10lemonade:40's ulimit -l unlimited is a bashism run under #!/bin/sh; on dash it fails silently (stderr suppressed), so the lock-limit bump never happens. Pre-existing on main, a separate issue, not touched here.

…ata/models reach the catalogue

lemond scans a models directory only when `extra_models_dir` is set: ScanCatalog::enumerate()
returns an empty map for an empty setting (scan_catalog.cpp:179), and nothing in the image seeded it.
The embedded 10lemonade seeded log_level and rotation only, so on a booted image anything staged in
/data/models was invisible and the catalogue read "none downloaded" while the files sat on the medium.
c26 works on a stick only because the stick writer stages its own config.json; the image must not
depend on that (a frugal/installed or dd-written image has no writer).

Measured on the shipping payload (lemonade-onebit-rc-2026-11-05-c26c), device-free, listing only:
  config = exactly what 10lemonade seeds (log_level + rotation) -> staged gguf absent, 213 rows
  same config + "extra_models_dir"                              -> 214 rows, the extra one is
    Qwen3-0.6B-Q8_0 | recipe llamacpp | downloaded true | source extra_models_dir

Fix: 10lemonade creates the models directory (/data/models, or $LEM/models with no /data) and seeds
the key - in a fresh config, and into an existing one only when the key is absent, so a config the
stick writer or an operator wrote keeps its value. Seeding ran against three known configs (fresh,
log-level-only, stick-writer's) with valid JSON out and the third left byte-identical.

vm-check gains "U3 models dir": it reads the configured path out of the config and requires it to
exist. On the pre-fix image its own command prints path=<unset> and the row goes red, which is the
point.

@bong-water-water-bong bong-water-water-bong left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

VERDICT: READY

Root cause verified at the pinned lemonade, not at the description. At third_party/lemonade = f44ff953bcd8c1bf (main), src/cpp/server/models/scan_catalog.cpp:172-181:

std::map<std::string, ModelInfo> ScanCatalog::enumerate(const std::set<fs::path>& registered_paths) const {
    // If no extra models directory configured, return empty
    if (extra_models_dir_.empty()) { return discovered; }
    ...
    if (status_ec == std::errc::no_such_file_or_directory) { return discovered; }

Two consequences the PR gets right: an unset key enumerates nothing, and a key pointing at a missing directory enumerates nothing either — so the mkdir -p "$MODELS" in the same hunk is load-bearing, not tidying.

The seeding, run against the block's own bytes (lines 84-122 of the script, only $LEM substituted):

config on the medium result
none (fresh image) 10lemonade: seeded ... (log_level=debug, 8 MB x 3 rotation, extra_models_dir=...), file carries the key
existing, no key (older image / operator's) added extra_models_dir=...; log_level: trace kept, everything else kept
the stick writer's, key already present sha256 identical before and after; extra_models_dir keeps /data/models-from-writer

The new U3 models dir row, run with only the config path substituted into its own command string:

pre-fix config (no key)        -> path=<unset>                            (no sentinel -> FAIL, as intended)
post-fix config, dir exists    -> path=.../models  models-dir-seeded     (PASS)
key set but dir absent         -> path=.../missing-dir                    (no sentinel -> FAIL)
fresh seed, before the mkdir   -> path=.../models                         (no sentinel -> FAIL)
fresh seed, after the mkdir    -> path=.../models  models-dir-seeded     (PASS)

So the row reds on exactly the shipped defect and greens only when both halves are true; it also caught a half-fix (key without directory), which is the failure mode the enumerate() ENOENT branch would otherwise give silently.

Also: bash -n clean on both files; tools/copyright.py --check rc 0; shellcheck -S warning shows only the pre-existing SC3045 at 10lemonade:40 (ulimit -l, on main too); the rows list still parses (86 rows, the new one included); and the new block follows the existing json.dump(d, open(p,"w"), indent=2) idiom rather than inventing a second style.

Not re-derived: the 213 -> 214 catalogue rows on the shipping payload. That needs the image booted on the serial console, which is pi-os3's rig, not this box. The mechanism above is what makes those two numbers what they are, and the row is the cheap standing check for it.

@github-actions

github-actions Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

PR Reviewer Guide 🔍

(Review updated until commit 1b46509)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Missing directory creation for extra_models_dir

The script creates the MODELS directory (which points to /data/models) but does not ensure that the directory is properly initialized with the correct permissions or ownership before being used as extra_models_dir. This could lead to permission issues or failures if the directory is not accessible by lemond.

mkdir -p "$LEM/cache" "$LEM/config" "$MODELS"
Potential race condition in config.json modification

The script modifies the config.json file using Python, which may introduce a race condition if multiple processes attempt to modify it simultaneously. Since this is part of the init script, it's critical that the file is written atomically to prevent corruption.

            python3 - "$CONFJSON" "$MODELS" <<'PY' && echo "10lemonade: added extra_models_dir=$MODELS to $CONFJSON"
import json, sys
p, models = sys.argv[1], sys.argv[2]
d = json.load(open(p))
d.setdefault("extra_models_dir", models)
json.dump(d, open(p, "w"), indent=2)

@bong-water-water-bong

Copy link
Copy Markdown
Collaborator Author

pi-prs: re-verified on today's tree (no code change needed), independently of the PR's own harness — c0419331 still applies cleanly to origin/main a56f0e98 and neither touched file has moved since its base (#1111's vm-check.sh work was already in that base, so the new row sits on the current harness).

The row bites, and the change does what it says. I extracted the config-seeding region out of the branch's 10lemonade and ran it against three configs, then ran the row's own command against each result:

config what 10lemonade did what U3 models dir reads
none (fresh) seeded {log_level: debug, …, extra_models_dir: <models>} path=<models> models-dir-seeded → PASS
log_level: warning only honoured warning, added extra_models_dir (python3 setdefault) models-dir-seeded → PASS
the stick writer's (already has the key + port) honoured, nothing written — byte-identical (sha256sum -c clean) path=/data/models → PASS on the appliance, where that dir exists
pre-fix image config (main's 10lemonade) — path=<unset> → FAIL

That last line is the one that matters for a gate: on an unfixed image the row prints path=<unset> and fails rather than describing the fix, so it can only go green when the image really seeded the key.

Also worth knowing for whichever cycle ships it: mkdir -p $MODELS runs before the config is written, so /data/models exists on every boot regardless of the key — which is what makes the [ -d $p ] half of the row deterministic.

@bong-water-water-bong
bong-water-water-bong merged commit f406769 into main Nov 5, 2026
9 checks passed
@bong-water-water-bong
bong-water-water-bong deleted the os/v3-extra-models-dir branch November 5, 2026 21:58
@github-actions

github-actions Bot commented Nov 5, 2026

Copy link
Copy Markdown

Persistent review updated to latest commit 1b46509

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant