feat: add model detail view (#129)

[Yegorov]

Closes #129

[greptile-apps]

Greptile Summary

This PR adds a model detail view to both classifier models <name> and classifier models --local <name>, introducing a new detect_registry_and_model helper that parses the @registry:model argument format.

• Remote detail view: when a model name (or @registry:model spec) is passed, list_remote_models fetches the index and renders a structured detail card instead of the table row.
• Local detail view: list_local_models similarly accepts an optional model name argument and renders type, categories, version, and size for a single cached model.
• New tests: four tests cover the remote/local detail paths including the not-found and custom-registry cases, though none exercise a model entry missing the categories key.

Confidence Score: 3/5

The new detail view paths will crash at runtime for any model whose JSON lacks a categories key — a realistic scenario for older cached models or third-party registries.

Both list_remote_models and list_local_models call .map / .keys directly on info['categories'] with no nil guard. Any model entry that omits that field triggers a NoMethodError rather than a graceful message, making the detail view unreliable against real-world data.

The nil-categories guard fixes belong in lib/classifier/cli.rb at lines 395 and 455. The test fixtures always include categories, so the crash path is untested.

Important Files Changed

Filename	Overview
lib/classifier/cli.rb	Adds model detail view to both remote and local listing commands; introduces detect_registry_and_model helper. Two NoMethodError crash paths exist when a model's JSON lacks a categories key.
test/cli/registry_commands_test.rb	Adds four new tests covering remote detail view, not-found path, custom-registry detail, and local detail view. All tests use fixtures that always include categories, so the nil-categories crash path is not exercised.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["classifier models [arg]"] --> B{--local flag?}
    B -- no --> C[list_remote_models]
    B -- yes --> D[list_local_models]

    C --> E[detect_registry_and_model]
    E --> F[parse_registry + DEFAULT_REGISTRY]
    F --> G[fetch_registry_index]
    G --> H{model arg present?}
    H -- yes --> I[find model in index]
    I --> J{found?}
    J -- no --> K[output: No model found]
    J -- yes --> L[render detail card]
    H -- no --> M[render table rows for all models]

    D --> N{model_arg present?}
    N -- yes --> O[find in local models list]
    O --> P{found?}
    P -- no --> Q[output: No local model found]
    P -- yes --> R[load_model_info + render detail card]
    N -- no --> S[render table rows for all local models]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["classifier models [arg]"] --> B{--local flag?}
    B -- no --> C[list_remote_models]
    B -- yes --> D[list_local_models]

    C --> E[detect_registry_and_model]
    E --> F[parse_registry + DEFAULT_REGISTRY]
    F --> G[fetch_registry_index]
    G --> H{model arg present?}
    H -- yes --> I[find model in index]
    I --> J{found?}
    J -- no --> K[output: No model found]
    J -- yes --> L[render detail card]
    H -- no --> M[render table rows for all models]

    D --> N{model_arg present?}
    N -- yes --> O[find in local models list]
    O --> P{found?}
    P -- no --> Q[output: No local model found]
    P -- yes --> R[load_model_info + render detail card]
    N -- no --> S[render table rows for all local models]

Loading

Prompt To Fix All With AI

Fix the following 4 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 4
lib/classifier/cli.rb:395
**`NoMethodError` crash when `categories` is missing** — `info['categories']` returns `nil` for any model entry that omits the key, so calling `.map` on it raises `NoMethodError`. The `|| ''` at the end is applied to the result of `join`, not the array, so it cannot rescue a nil receiver.

```suggestion
        categories = (info['categories'] || []).map(&:downcase).join(', ')
```

### Issue 2 of 4
lib/classifier/cli.rb:455
**`NoMethodError` crash when `categories` is missing in local model** — `info['categories']` can be `nil` if the stored model file doesn't include that key (e.g. models saved before this field was standard). Calling `.keys` on `nil` will raise `NoMethodError`. There is no fallback default here at all, unlike the remote path.

```suggestion
        categories = (info['categories'] || {}).keys.map(&:downcase).join(', ')
```

### Issue 3 of 4
lib/classifier/cli.rb:385-411
**Nested conditional instead of early return** — both `list_remote_models` and `list_local_models` wrap the default list-rendering path in an `else` branch. Per the repo's style guide, the single-model branch should early-return so the loop is the unconditional tail, with no nesting required. The same pattern appears in `list_local_models` at the equivalent block (lines ~445–471).

In this review, suggest simpl... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))

### Issue 4 of 4
lib/classifier/cli.rb:847-849
**Redundant comment restates the method name** — "Detect registry and model" adds no information beyond what the method name already says. Per the repo's De-AI style guide, comments that restate the code should be removed. The `@rbs` annotation below it is a keeper.

```suggestion
    # @rbs (String?) -> [String?, String?]
    def detect_registry_and_model(arg)
```

In this review, suggest simpl... ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))

_{Reviews (1): Last reviewed commit: "feat: add model detail view (#129)" | Re-trigger Greptile}

[Yegorov]

@greptile-apps Tell me how to correct the comments (Metrics/AbcSize: Assignment Branch Condition size for test_models_local_model_detail_view is too high. [17.12/15]) from houndci-bot?

[greptile-apps]

The ABC size of 17.12 comes from the method having too many branches (method calls). Let me count: File.join ×3, FileUtils.mkdir_p, File.write ×2, run_cli, result[:output] ×4, result[:exit_code], result[:error], plus assertions = ~17 branches.

The cleanest fix is to extract the repeated file-setup into a helper (used by both test_models_local_model_detail_view and test_models_local_model_detail_view_if_not_found), and cache result[:output] in a local:

def create_local_model_fixtures(models_dir)
  FileUtils.mkdir_p(models_dir)
  File.write(File.join(models_dir, 'spam-filter.json'), @model_json)
  File.write(File.join(models_dir, 'sentiment.json'), @model_json)
end

def test_models_local_model_detail_view
  create_local_model_fixtures(File.join(@cache_dir, 'models'))

  result = run_cli('models', '--local', 'spam-filter')
  output = result[:output]

  assert_equal 0, result[:exit_code]
  assert_match('Name: spam-filter', output)
  assert_match('Type: bayes', output)
  assert_match('Categories: spam, ham', output)
  assert_empty result[:error]
end

This reduces the method's branch count from ~17 to ~8 (ABC ≈ 9), well under the 15 limit. Apply the same pattern to test_models_local_model_detail_view_if_not_found since it has identical setup.

[Yegorov]

@cardmagic can you take a look, please?

[cardmagic]

The detail view truncates the description to 40 chars via desc.slice(0, 40). That 40-char cap makes sense for the table row below (it keeps columns aligned), but the point of a detail view is to show the full record. A model with a longer description gets silently cut off. I confirmed a 77-char description renders as Description: A very long description that clearly exc. The existing tests do not catch it because every fixture description is under 40 chars.

Suggest rendering the full description here, e.g. pass desc: instead of desc.slice(0, 40), and consider adding a test with a description longer than 40 chars.

[cardmagic]

return *arg.split(":") produces a variable-length array, so the actual return shape does not match the declared [String?, String?]:

• @user/repo:model:extra returns ["@user/repo", "model", "extra"], and the third element is silently dropped when the caller destructures registry, model = ...
• spam:filter (no @) returns ["spam", "filter"], so a plain model name that happens to contain a colon gets misread as registry:model

Using arg.split(":", 2) bounds it to two parts and keeps the return consistent with the RBS annotation. Both inputs are unlikely, so this is minor.

[cardmagic]

index["models"] is a Hash keyed by model name, so .find { |name, _| name == model } is an O(n) scan doing what a direct key lookup does in O(1). The block param name also shadows the outer name.

Since the keys are the names, you can simplify to a direct lookup and use model as the display name:

info = index["models"][model]
if info.nil?
  @output << "No model #{model.inspect} found in registry"
  return
end

Clearer and avoids the shadowing.

[cardmagic]

thanks for all these wonderful additions btw! Really nice to be collaborating on this with you!