Files
gpu-rent/docs/reviews/2026-08-21-review-2.md
T
Leonid Pershin dc1fde9e3e Refactor LLM runtime handling and enhance CLI documentation
- Updated `resolve_llm_runtime` to prioritize live configuration over legacy notes, ensuring accurate runtime resolution.
- Enhanced `tunnel_forwards` to prefer current configuration for LLM runtime, improving tunnel setup logic.
- Improved idle-killer logic to handle stale markers and provide clearer warnings in the status output.
- Updated CLI documentation in `cli.md` to reflect changes in command behavior and runtime handling.
- Enhanced tests to validate new runtime resolution logic and ensure proper handling of configuration states.
2026-08-21 05:40:22 +03:00

5.4 KiB
Raw Blame History

Project Review — 2026-08-21 (2)

Prior Reviews Summary

Based on the last 3 review files analysed in Phase 0 (only 2026-08-21-review-1.md exists).

Still Open (carried forward)

None.

Resolved Since Last Review

  • [source: 2026-08-21-review-1, tasks 113] All prior tasks closed (wait_ssh, bootstrapped clear, access_rules attempt, GIT scrub, SG prune, CIDR example, SSH reuse in sync, light bootstrap flag, phase bootstrapping, Civitai non-blocking, session tests, journalctl logs, architecture docs).

Phase 1: Code Quality

SOLID

  • session.cmd_up / _bind_access still orchestrate cloud + SSH + bootstrap + provision + LLM + snapshot + notify (god-flow). Acceptable for a small CLI, but growing with LLM.
  • New modules (llm_runtime, setup_wizard, access_card) are reasonably scoped.

Performance

  • arm_idle_killer still opens many short SSH sessions (put_text / run_ssh per step) — sync_files was fixed in review-1; idle-arm was not.
  • Every up still runs full doctor (Keystone + Glance + …) even when user already ran doctor.

Correctness & Bugs

  • Ollama pull skip can treat a different tag of the same model as “already present” (listed() stores bare name; match uses startswith).
  • Pull marker .gpu-rent-ollama-pulling can stick after SSH timeout → idle-killer forever busy.
  • provision_llm exceptions swallowed → tunnel/access card can show LLM URLs while install failed; notes.llm_runtime may stay stale.
  • Light bootstrap requires state.bootstrapped and VM marker; stop clears local bootstrapped → almost always full apt after stop→up (conflicts with review-1 perf goal).
  • clone_ext SystemExit(2) on origin mismatch skips token file unlink in main().
  • App cred access_rules include /v2.1/servers/* (any server) plus unrestricted fallback; not revoked on stop.

Code Quality

  • Dead branch in push_tree (_is_weight_name + empty pass).
  • OllamaModelEntry.default written/parsed but unused in pull.
  • Invalid LLM_RUNTIME in env silently coerced to none.

Phase 2: Logical Consistency

Domain & Application Layer

  • Docs/architecture updated for LLM; product still SwarmUI-first with opt-in LLM — consistent with decisions.

Data Flow

  • Tunnel / open --llm / access card prefer state.notes["llm_runtime"] over live config — desync if provision failed or vars changed without re-provision.

State Management

  • LLM_RUNTIME=none does not stop/disable remote LLM systemd units → GPU may stay occupied.
  • Soft-fail wait_backend_idle / soft-fail arm_idle_killer still yield ready_cloud / “ready” UX while killer may be blind.

Consistency

  • Error handling: mix of hard CloudError (bootstrap) vs soft log (LLM, killer, ready). Uneven.

Phase 3: UI/UX (CLI)

Usability

  • Bare gpu-rent invokes full interactive up (expensive surprise).
  • Full doctor table on every up is noisy.
  • Interactive Ollama preset typo raises raw ValueError.
  • Access card at tunnel end is a clear UX win.

Visual & Consistency

  • Rich access panel is coherent; toast text still SwarmUI-only (minor).

Interaction & Feedback

  • --no-tunnel still shows access card with tunnel hint — good.
  • Soft LLM failure gives little signal beyond a log line.

Accessibility

  • N/A (CLI).

Tasks

  • 1. [Security] Narrow idle-killer access_rules to this server_id only (drop /servers/*); fail closed or warn loudly instead of unrestricted fallback; revoke app cred on stop/destroysrc/gpu_rent/idle_killer.py line 57
  • 2. [Security] Ensure GIT_TOKEN file is always removed (try/finally around main, convert SystemExit paths to raised errors) — src/gpu_rent/remote/clone_ext.py line 116
  • 3. [Bug] Fix Ollama pull skip: match exact tag only (do not skip foo:7b because foo:3b exists) — src/gpu_rent/remote/ollama_pull.py line 14
  • 4. [Bug] Age out or clear stale .gpu-rent-ollama-pulling in idle-killer (e.g. ignore marker older than N minutes) — src/gpu_rent/remote/idle_killer.py line 93
  • 5. [Logic] Do not swallow provision_llm failures; update notes.llm_runtime only after success; prefer cfg over stale notes when they disagree — src/gpu_rent/provision.py line 340
  • 6. [Logic] When LLM_RUNTIME=none (or switching runtime), stop/disable previous gpu-rent-ollama / gpu-rent-llamacpp units — src/gpu_rent/provision.py line 262
  • 7. [Performance] Light bootstrap when VM marker exists even if local bootstrapped=False after stop (or restore bootstrapped from marker after SSH) — src/gpu_rent/session.py line 101
  • 8. [UX] Bare gpu-rent should show help or a short menu, not auto-up; keep launcher GPU_RENT_DEFAULT_ARGS for double-click — src/gpu_rent/cli.py line 53
  • 9. [UX] Quiet doctor on up (summary / only failures) unless --verbosesrc/gpu_rent/cli.py line 403
  • 10. [Logic] Surface idle-killer arm failure as warning in status/access card, not silent soft-log only — src/gpu_rent/provision.py line 344
  • 11. [CodeQuality] Remove dead _is_weight_name pass in push_tree; use OllamaModelEntry.default or drop the field — src/gpu_rent/sync_files.py line 44
  • 12. [Security] Pin/checksum Ollama install script and llama.cpp release asset (or document supply-chain risk) — src/gpu_rent/remote/install_ollama.sh line 20