feat: download only config-selected models in setup#25
feat: download only config-selected models in setup#25tyvsmith wants to merge 1 commit intoworktree-agent-a10a4d5efrom
Conversation
Previously, setup always downloaded all non-optional models (scrfd_2.5g + w600k_r50) regardless of the active config, then additionally fetched any optional models the config referenced. This meant switching to high-accuracy models (det_10g + glintr100) still triggered a download of the unused default models. The filter now selects exactly the two models named in recognition.detector_model and recognition.embedder_model. On a fresh install the defaults resolve to the standard models, so behavior is unchanged for first-run users. Re-running setup after changing the config to a different model pair downloads the new models only. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates facelock setup so it downloads/validates only the detector + embedder models currently selected in the user’s config, rather than always pulling the default required models as well.
Changes:
- Narrow model selection in both interactive and non-interactive setup to
recognition.detector_model+recognition.embedder_model. - Add an allowance to keep deserializing
ModelEntry::optionalfrom the manifest without dead-code warnings.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Only download the models actually selected in the config. | ||
| // Non-optional (default) models that aren't selected are skipped — the user | ||
| // chose different models, so there is no point fetching the defaults too. | ||
| let needed: Vec<&ModelEntry> = manifest | ||
| .models | ||
| .iter() | ||
| .filter(|m| { | ||
| !m.optional || m.filename == *configured_detector || m.filename == *configured_embedder | ||
| }) | ||
| .filter(|m| m.filename == *configured_detector || m.filename == *configured_embedder) | ||
| .collect(); |
There was a problem hiding this comment.
The new filter only includes models present in the bundled manifest. If the config points to a model filename that is not in the manifest (e.g., a custom model the user placed in the model_dir), needed becomes empty and the wizard will print “All models are already present and verified.” even though it didn’t check anything. Please validate that both recognition.detector_model and recognition.embedder_model resolve to manifest entries (and error with a clear message if not), or explicitly handle custom models by verifying the configured file exists (and checksum if provided) instead of silently succeeding.
| // 3. Check and download only the models actually selected in the config. | ||
| // Non-optional (default) models that aren't referenced by the config are | ||
| // skipped — if the user chose different models there is no reason to fetch | ||
| // the defaults as well. On a first run the config defaults resolve to the | ||
| // standard models (scrfd_2.5g + w600k_r50) so those are still downloaded. | ||
| let configured_detector = &config.recognition.detector_model; | ||
| let configured_embedder = &config.recognition.embedder_model; | ||
|
|
||
| let needed: Vec<&ModelEntry> = manifest | ||
| .models | ||
| .iter() | ||
| .filter(|m| { | ||
| !m.optional || m.filename == *configured_detector || m.filename == *configured_embedder | ||
| }) | ||
| .filter(|m| m.filename == *configured_detector || m.filename == *configured_embedder) | ||
| .collect(); | ||
|
|
||
| println!("Checking {} model(s)...\n", needed.len()); |
There was a problem hiding this comment.
In non-interactive setup, the new needed filter can result in needed.len() == 0 when the configured model filename(s) are not present in the bundled manifest. In that case setup continues, writes the setup-complete marker, and exits successfully without downloading or verifying any models. Please fail fast if the configured models aren’t found in the manifest (or add explicit support for custom model verification using the config SHA256 fields) to avoid marking setup as complete with no usable models.
Summary
facelock setupalways downloaded all non-optional models (scrfd_2.5g + w600k_r50) plus any optional models the config referenced, even when the user had configured a completely different model pairwizard_model_downloadandrun_non_interactiveto select exactly the models named inrecognition.detector_modelandrecognition.embedder_model#[allow(dead_code)]toModelEntry::optionalwhich is still deserialized from the manifest TOML and used in testsStacks on top of #24 (streaming download fix).
Test plan
cargo build --workspacepassescargo clippy --workspace -- -D warningspasses with zero warnings/errorscargo test --workspacepasses (all 274 tests pass)det_10g.onnx+glintr100.onnxin config, runfacelock setup --non-interactive, confirm only those two models are downloaded🤖 Generated with Claude Code