Skip to content

Define model display order once on ModelId; derive sort and UI index from it #14

Description

@BumpyClock

Problem

Model display order is encoded in four independent locations. Adding or reordering a model requires synchronized edits across all four, and existing tests mostly validate that the duplication stayed in sync rather than guarding behavior.

1. crates/shared/src/settings.rs:113–120ModelId::ALL (catalog order):

pub const ALL: &[Self] = &[
    Self::ParakeetV3,
    Self::NemotronSpeechStreamingEn06b,
    Self::WhisperSmall,
    Self::WhisperMedium,
    Self::WhisperTurbo,
    Self::WhisperLarge,
];

2. crates/core/src/transcription/model/mod.rs:105–109list_models sort key (service layer):

list.sort_by_key(|model| match model.id {
    ModelId::ParakeetV3 => (0_u8, model.size_mb),
    ModelId::NemotronSpeechStreamingEn06b => (1_u8, model.size_mb),
    _ => (2_u8, model.size_mb),
});

3. crates/app/src/ui/settings/sections/models/mod.rs:223–232model_id_index (UI element IDs):

const fn model_id_index(id: ModelId) -> usize {
    match id {
        ModelId::ParakeetV3 => 0,
        ModelId::NemotronSpeechStreamingEn06b => 1,
        ModelId::WhisperSmall => 2,
        ModelId::WhisperMedium => 3,
        ModelId::WhisperTurbo => 4,
        ModelId::WhisperLarge => 5,
    }
}

4. docs/design/unified-model-selection.md:394 — design-doc sort sentence:

Keep existing: Parakeet first, then Whisper models sorted by size ascending.

(The doc predates Nemotron and does not mention it — a silent staleness.)

All four must agree on variant count and relative order. Today they happen to agree; the risk is that the next model addition touches ModelId::ALL and catalog.rs but misses list_models or model_id_index, causing wrong UI ordering or a panicking element-ID collision that is only caught at runtime.

Why it matters

  • Synchronized-edit footgun. Every new model variant is a four-file change with no compile-time enforcement. The compiler does not warn if a new variant falls into the _ => arm of the sort or gets an auto-assigned usize in a separate match arm.
  • Silent staleness. model_id_index and the sort key are const fn / inline — no warning if they go out of sync with ALL. The design doc already drifted (no mention of Nemotron at line 394).
  • Tests verify alignment, not correctness. list_models_order_parakeet_nemotron_then_whisper (tests.rs:16–42) and model_id_index_covers_all_variants_uniquely (mod.rs:248–254) are green only because all four copies currently agree. They would not catch a new variant added to ALL but missed in list_models.

Proposed change

  1. Add ModelId::display_order() -> usize (or reuse the positional index of ALL) in crates/shared/src/settings.rs. This becomes the single authoritative rank for a variant.
  2. Rewrite list_models sort to call model.id.display_order() instead of the hard-coded match on bucket integers (mod.rs:105–109).
  3. Rewrite model_id_index to call ModelId::ALL.iter().position(|v| *v == id).unwrap() (or delegate to display_order()), removing the redundant match (mod.rs:223–232). Because model_id_index is const fn, a display_order() -> usize helper on ModelId that does the position search suffices (or promote to a non-const helper if needed by the GPUI element-ID type).
  4. Update docs/design/unified-model-selection.md:394 to mention the ordering rule now lives in ModelId::ALL.
  5. Trim or convert alignment-only tests. list_models_keeps_parakeet_first and list_models_order_parakeet_nemotron_then_whisper can be collapsed to a single property test asserting list_models() output order matches ModelId::ALL order. model_id_index_covers_all_variants_uniquely can be replaced by a doc-comment assertion or kept as a compile-time sanity check — but should not be the only guard.

No behavior changes for existing models.

Recommendation / triage

DO — define order once on ModelId (e.g. a display_order() method or reuse ALL) and derive list_models + model_id_index from it. Moderate effort; removes a real synchronized-edit footgun for future models. The Nemotron addition is recent evidence this footgun is live: nemotron-support-implementation-plan.md:184 explicitly called out "Adjust list sort if needed so Parakeet then Nemotron then Whisper" as a checklist item — meaning the author knew it was a separate place to update.

Acceptance criteria

  • ModelId::display_order() (or equivalent) defined once in crates/shared/src/settings.rs; returns a stable usize rank derived from ALL position.
  • list_models sort in crates/core/src/transcription/model/mod.rs delegates to display_order() — no free-standing match on bucket integers.
  • model_id_index in crates/app/src/ui/settings/sections/models/mod.rs delegates to the same helper — no free-standing match.
  • Adding a new variant to ModelId::ALL automatically propagates correct order to both list_models and model_id_index with no other edits required.
  • docs/design/unified-model-selection.md:394 updated to reference ModelId::ALL as the ordering authority.
  • Behavior unchanged: list_models() output order is identical to today's (Parakeet → Nemotron → Whisper by size). Verified by the surviving ordering test.
  • Tests that survive: one ordering test asserting list_models() output matches ModelId::ALL order; model_id_index_covers_all_variants_uniquely may be kept or replaced by a static assertion. The redundant list_models_keeps_parakeet_first test may be removed.
  • cargo test green across ansible-shared, ansible-core, and ansible-app.

Traceability

  • clawpatch finding ID: fnd_sig-feat-service-60d65539ae-581a_828c66acc2
  • Revalidate command: clawpatch revalidate --finding fnd_sig-feat-service-60d65539ae-581a_828c66acc2 --reasoning-effort high
  • No directly coupled issue. Related context: docs/design/nemotron-support-implementation-plan.md:156,184 (Nemotron ordering checklist items that triggered discovery of the duplication).

Metadata

Metadata

Assignees

No one assigned

    Labels

    deslopifyFound by clawpatch deslopify reviewtech-debtCode quality / maintainability cleanup

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions