feat: redesign mlxcel list output (modified, --json/-q/-v, styling)

[inureyes]

Summary

Improve the presentation/UX of mlxcel list (the local-models inventory introduced by #138): a MODIFIED time column, machine-readable (--json) and quiet (-q) output, a -v/--verbose PATH restore, --sort, NAME capping/ellipsis, and TTY-aware styling that respects NO_COLOR. Builds on #138 and is purely additive — it does not change which models are listed.

Default-layout change (call-out)

This changes the default mlxcel list table. Columns are now NAME / SIZE / MODIFIED (PATH is dropped from the default view, restored only under -v/--verbose), and the header becomes the compact N model(s) · <total> · <root> form with $HOME contracted to ~. It is not a breaking flag removal, but scripts that scraped the old REPO ID / SIZE / PATH table or the Downloaded models (...) header should switch to --json (stable field names) or -q (repo-ids only) — both unstyled and pipe-safe.

What changed

• src/downloader/store.rs: add StoredModel.modified: Option<SystemTime>, populated from the snapshot directory mtime at both list_models_under construction sites (std::fs::metadata(path).modified().ok()); None on stat failure.
• src/commands/models.rs: renderer redesign. Rendering functions take injected now + use_color (never read the clock or is_terminal() internally) so golden tests are deterministic and filesystem-free. Adds pure humanize_relative(Duration) (just now / min / hour(s) / day(s) / week(s) up to 4 weeks / month(s) / year(s), pluralized; future mtime → "just now"), NAME cap at NAME_COL_MAX = 40 with … ellipsis, --json via a dedicated Serialize struct {repo_id, size_bytes, path, modified} (modified = Unix epoch seconds or null), -q repo-ids-only, --sort name|size|modified, and a stdout-oriented should_color_stdout() mirroring the NO_COLOR + is_terminal() semantics of should_show_progress(). Styling dims only MODIFIED cells + the header root (NAME stays plain); --json/-q are never styled. The feat: Make mlxcel list show local models by default; move the architecture catalog to mlxcel arch #138 empty-store hint (incl. the mlxcel arch redirect) is preserved.
• src/main.rs: extend ListArgs with --json, -q/--quiet, -v/--verbose, --sort (clap ValueEnum, default name); clap-enforced mutual exclusivity (--json vs --quiet/--verbose; --quiet vs --verbose; --sort allowed with any). Dispatch threads the options through to run_list_local.
• src/commands/models.rs (tests) + src/main_tests.rs: reworked golden tests (default / json / quiet / verbose / empty), humanize_relative boundary tests, a no-ANSI styling assertion + a color-on assertion, sort tests, and CLI-surface tests (flag defaults, parsing, conflicts, unknown---sort rejection).

Test plan

• cargo fmt
• cargo clippy --bin mlxcel --tests -- -D warnings (clean)
• cargo test --bin mlxcel (117 passed, 0 failed)
• Manual end-to-end against a fabricated store: default (plain, NAME/SIZE/MODIFIED, no PATH), -v (PATH restored), -q, --json (numeric/null modified), --sort size, empty-store hint, $HOME→~ header contraction, and on a pty: dim escapes present with NAME plain, NO_COLOR=1 + --json unstyled.

Closes #141

[inureyes]

Implementation Review Summary

Intent

Additively improve mlxcel list UX on top of #138: NAME/SIZE/MODIFIED columns (PATH dropped from default, restored under -v), a relative MODIFIED time, --json/-q/-v output modes, --sort name|size|modified, NAME ellipsis, and TTY-gated ANSI dimming that respects NO_COLOR.

Findings Addressed

• None. No CRITICAL/HIGH/MEDIUM/LOW findings — the implementation is correct and complete.

Remaining Items

• None.

Verification against issue #141

• Data model — StoredModel.modified: Option<SystemTime> added; populated at both list_models_under sites (bare-id + owner/name) via std::fs::metadata(path).and_then(|m| m.modified()).ok() → None on stat failure. No other construction site exists; no destructuring without .. (all field access is by-name).
• Humanizer — humanize_relative bucket edges are consistent (each unit's lower edge → value 1): <60s just now; min/hour/day/week; 28d weeks-cutoff falls into months with .max(1) clamp → "1 month" (no "0 months" gap); years. Pluralization correct (min never pluralizes by design; value==1 singular). Future mtime (duration_since Err) → "just now". Boundary tests confirm 59s, 59min, 23h, 6d, 27d→3 weeks, 28d→1 month.
• Renderer purity — now + use_color injected via ListOptions; renderers read neither the clock nor is_terminal(). Golden tests are filesystem-free and clock-pinned.
• TTY styling — should_color_stdout() mirrors should_show_progress() (NO_COLOR non-empty → off; non-TTY → off) but gates on stdout. Dim applies only to MODIFIED cells + header root; NAME stays plain. --json/-q are never styled (use_color forced off for them in run_list_local). Padding happens before dimming, so ANSI never counts toward column width (verified in verbose mode where MODIFIED is a middle column and PATH still aligns).
• --json — stable {repo_id, size_bytes, path, modified} via a dedicated Serialize struct; modified = epoch seconds or null (pre-epoch/absent → null); valid JSON; empty store → [].
• -q/--quiet — repo-ids only, no header/columns/ANSI; empty store emits nothing.
• -v/--verbose — restores the absolute PATH column.
• --sort — name asc; size largest-first (ties → repo-id); modified recent-first with None last (ties → repo-id). Applied in run_list_local before rendering, so it affects table and --json alike.
• clap conflicts — --json conflicts_with_all [quiet, verbose]; --quiet conflicts_with verbose. This yields full mutual exclusivity across json/quiet/verbose; --sort allowed with any. Parse tests assert each conflict and the unknown---sort rejection.
• feat: Make mlxcel list show local models by default; move the architecture catalog to mlxcel arch #138 preserved — empty-store hint still redirects to mlxcel arch; list dispatch still routes to the local store via list_models_with_override; the old rm path is untouched.
• NAME capping — ellipsize_name truncates by char (not byte), appending …; width math uses chars().count(). Empirically confirmed Rust {:<width$} pads strings by char count, so the multibyte ellipsis preserves alignment (a case ASCII-only goldens wouldn't catch).
• Test adequacy — golden tests for default/json/quiet/verbose/empty, humanizer boundaries, a no-ANSI assertion + a color-on assertion, sort tests, and CLI-surface tests are all present and meaningful.
• Conventions / reuse — compact_size (pre-existing from feat: Make mlxcel list show local models by default; move the architecture catalog to mlxcel arch #138) is reused, not reimplemented; imports ordered std→external→internal; SortKey derives the conventional trait set + clap::ValueEnum; structural impact confined to the list-render surface.

Build verification (warm, narrow commands)

• cargo test --bin mlxcel → 117 passed, 0 failed
• cargo test --lib downloader::store → 27 passed, 0 failed
• cargo clippy --bin mlxcel --tests -- -D warnings → clean

Note: a supplementary Codex review was attempted but could not run — its sandbox blocked shell access (bwrap: loopback: Failed RTM_NEWADDR) and the branch is not on a remote it could reach — so it produced no findings. This review stands on direct source analysis.

Verdict: the PR correctly and completely implements #141.

[inureyes]

Security & Performance Review — no blocking findings

Reviewed the full change surface against the security/performance/code-quality checklists, with cargo clippy --bin mlxcel --tests -- -D warnings (clean), cargo test --bin mlxcel (117 passed), and cargo test --lib downloader::store (27 passed) on the warm target. Verdict: no CRITICAL/HIGH/MEDIUM findings; no auto-fix applied (none warranted).

1. Panic paths — clear

No reachable panic on user- or filesystem-influenced input.

• humanize_relative — only integer division by nonzero constants plus .max(1); no subtraction/overflow. Duration::as_secs() is u64.
• format_modified — the future-mtime (duration_since Err) branch is handled explicitly → just now; no unwrap.
• ellipsize_name — saturating_sub(1) and chars().take().collect() (codepoint-safe, never slices mid-UTF-8).
• render_json — the only unwrap-family call is unwrap_or_else(|_| "[]") (cannot panic); pre-epoch mtimes map to None via duration_since(UNIX_EPOCH).ok().
• The header total models.iter().map(|m| m.size_bytes).sum::<u64>() (line 336) is byte-identical to pre-Improve the mlxcel list output UI (columns, modified-time, --json/--quiet, TTY styling) #141 code (was line 96) and not a realistic overflow vector (>18 EB of on-disk model data); release builds wrap rather than panic. Pre-existing, not introduced here.

2. Performance — clear

• The added std::fs::metadata(path).and_then(|m| m.modified()).ok() is one stat(2) per snapshot, placed inside the loop that already runs dir_size() (a full recursive walk) and snapshot_is_complete() per snapshot. O(1) per already-enumerated directory; no new traversal, no extra walk.
• sort_models is slice::sort_by → O(n log n) with cheap comparators.
• Rendering is single-pass (render_local_models does a fixed number of O(n) passes for cell pre-render + width + write; render_json/render_quiet one pass each). No O(n²).

3. JSON correctness / injection — safe

The sole serialization site is serde_json::to_string_pretty(&rows) over a typed #[derive(Serialize)] JsonModel; no manual JSON string concatenation anywhere. Verified empirically that serde_json escapes a raw ESC (0x1b) embedded in a repo_id/path field into � (zero raw 0x1b bytes in output) — so on-disk directory names cannot break the JSON structure or smuggle terminal escapes through --json.

4. Terminal-escape consideration — pre-existing, NOT worsened (informational)

The human table prints repo_id (and path under -v) raw, and a repo_id derived from a Unix directory name can contain a raw ESC (0x1b is valid UTF-8, so it survives the to_str() filter in list_models_under). This is a pre-existing property: the pre-#141 table already wrote model.repo_id / model.path.display() unescaped to stdout.

This PR does not worsen it:

• ellipsize_name only truncates/collects chars — it neither adds nor removes escapes.
• The new ANSI dim() wrapping is applied only to the MODIFIED cell (program-generated relative-time strings) and the header store-root (operator-supplied --models-dir/env/$HOME, also printed raw pre-Improve the mlxcel list output UI (columns, modified-time, --json/--quiet, TTY styling) #141) — never to NAME/PATH, and the wrapper is fixed program text that emits the underlying bytes unchanged.
• The script-facing outputs are safe: --json escapes control chars (verified above); -q prints the bare repo_id the user must already pass to mlxcel rm, and is intended for piping into another program, not styled terminal rendering — no new byte reaches the terminal that the default table didn't already emit.

Severity: informational / pre-existing (user owns their own model store; no new attack surface). If hardening is ever desired, a separate follow-up could sanitize control bytes in repo_id for the human table only — out of scope for this PR.

Other

• No new unsafe, no concurrency primitives, and zero changes to the rm/deletion path (run_remove/confirm_removal/remove_model_with_override untouched).
• clap conflicts_with rules for --json/-q/-v are covered by main_tests.rs (conflict + default + unknown---sort rejection tests).

LGTM from a security/performance standpoint.

[inureyes]

PR Finalization Complete

Summary

• Tests: verified — 117 passed, 0 failed (no new tests needed; coverage confirmed complete per pre-flight)
• Documentation: updated CHANGELOG.md and README.md
• Lint/Format: cargo fmt clean, cargo clippy --bin mlxcel --tests -- -D warnings clean

Changes made (commit 13513a4)

CHANGELOG.md — added a new bullet under in , placed immediately after the #138 BREAKING entry and before . Describes: NAME/SIZE/MODIFIED default table, PATH moved behind -v/--verbose, humanized mtime from snapshot directory mtime, --json stable array output, -q quiet mode, --sort, and TTY-aware styling respecting NO_COLOR.

README.md — replaced the stale comment on line 84 ("List downloaded models with their on-disk size and path") with the accurate default description, and added three example lines showing --json, -q, and -v modes. Confirmed no other README or docs prose describes the old REPO ID / SIZE / PATH table shape.

All checks passing. Ready for merge.