Skip to content

feat: download only config-selected models in setup#26

Merged
tyvsmith merged 1 commit into
mainfrom
worktree-model-selection
May 17, 2026
Merged

feat: download only config-selected models in setup#26
tyvsmith merged 1 commit into
mainfrom
worktree-model-selection

Conversation

@tyvsmith
Copy link
Copy Markdown
Owner

Summary

  • Previously facelock setup always 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 pair
  • Changed the filter in both wizard_model_download and run_non_interactive to select exactly the models named in recognition.detector_model and recognition.embedder_model
  • First-run behavior is unchanged: config defaults resolve to scrfd_2.5g + w600k_r50 so those are still downloaded on fresh installs
  • Re-running setup after changing the config (e.g. switching to det_10g + glintr100 for high accuracy) now downloads only the newly-selected models
  • Already-present models are still verified and skipped (no re-download)
  • Added #[allow(dead_code)] to ModelEntry::optional which is still deserialized from the manifest TOML and used in tests

Re-opens #25 (auto-closed when its stacked base branch was deleted on #24 merge). Rebased onto current main.

Test plan

  • cargo build --workspace passes
  • cargo clippy --workspace -- -D warnings passes with zero warnings/errors
  • cargo test --workspace passes (all 274 tests pass)
  • Manually verify: configure det_10g.onnx + glintr100.onnx in config, run facelock setup --non-interactive, confirm only those two models are downloaded

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>
Copilot AI review requested due to automatic review settings May 17, 2026 18:03
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 updates facelock setup to download (and verify/skip) only the face recognition models explicitly selected in the user’s config, instead of always downloading the default non-optional models as well.

Changes:

  • Restrict model selection in both interactive and non-interactive setup flows to recognition.detector_model and recognition.embedder_model.
  • Add #[allow(dead_code)] to ModelEntry::optional since it’s still deserialized and used by tests but no longer referenced in non-test code.
Comments suppressed due to low confidence (1)

crates/facelock-cli/src/commands/setup.rs:1020

  • Same issue as the wizard: when either configured model filename is not found in the bundled manifest, this filter can produce needed.len() == 0 and non-interactive setup will continue to completion (writing the setup marker) without ensuring the configured models exist and are valid. Consider resolving each configured model individually (manifest entry -> download/verify; non-manifest -> verify file exists in model_dir and validate against the configured *_sha256, otherwise fail with an actionable error).
    // 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.filename == *configured_detector || m.filename == *configured_embedder)
        .collect();

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +509 to 516
// 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();
Comment on lines 1013 to 1020
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();
@tyvsmith tyvsmith merged commit 3f7d1a7 into main May 17, 2026
7 checks passed
tyvsmith added a commit that referenced this pull request May 17, 2026
Refresh CHANGELOG to capture work merged since the initial draft:
- Signed APT repository publishing with main/legacy channels (#21)
- E2E package install validation for .deb and .rpm (#23)
- Streaming model download progress (#24)
- Config-selected model downloads in setup (#26)
- Security hardening: SHA256-verified models, persistent rate
  limiting, D-Bus method-level authorization, hardened PAM env (#16)
- Runtime-selectable GPU execution provider (no compile-time flags)

Also fix pkg-validate.sh removal tests to only run when the package
is actually managed by dpkg/rpm — previously failed in test-deb /
test-rpm where files are copied into the container directly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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