Skip to content

REFACTOR.md

Latest commit

 

History

History
87 lines (68 loc) · 6.09 KB

REFACTOR.md

File metadata and controls

87 lines (68 loc) · 6.09 KB

Refactoring Plan

Generated: 2026-02-12 Scope: tensors/ (all modules) + tests/

Summary

11 issues found: 3 high priority, 5 medium priority, 3 low priority.

High Priority

[ ] Extract common HTTP request pattern in api.py

  • Location: api.py:38-108
  • Problem: fetch_civitai_model_version, fetch_civitai_model, and fetch_civitai_by_hash share nearly identical try/except/error-handling blocks (3x duplication).
  • Impact: Changing error handling or adding retry logic requires editing 3+ places. Easy to introduce inconsistencies (e.g., fetch_civitai_model_version lacks the Progress spinner that the other two have).
  • Action: Extract a _api_get(url, api_key, console, spinner_msg=None) -> dict | None helper that handles GET, 404 check, error printing, and optional Progress spinner. Each fetch function becomes a one-liner calling this helper.

[ ] Eliminate module-level console singleton in cli.py

  • Location: cli.py:67
  • Problem: console = Console() at module level is used by 6 helper functions (_output_info_json, _save_metadata, _resolve_by_hash, _resolve_by_model_id, _prepare_download_dir, _display_download_info) via closure rather than parameter passing.
  • Impact: Impossible to inject a test console into helpers, prevents output capture in tests, creates hidden coupling. Commands pass console to API/display functions but helpers silently use the global.
  • Action: Add console: Console parameter to all helper functions. Pass the module-level console from command functions. This makes the dependency explicit and testable.

[ ] Remove or use unused safetensors dependency

  • Location: pyproject.toml:8
  • Problem: safetensors>=0.4.0 is listed as a runtime dependency but never imported anywhere. The code does manual binary parsing in safetensor.py.
  • Impact: Unnecessary install bloat. Users install a native extension library they don't need. Confusing for contributors.
  • Action: Remove safetensors from [project.dependencies] since manual parsing is the intended approach.

Medium Priority

[ ] Deduplicate output dict construction in cli.py

  • Location: cli.py:126-133 and cli.py:154-161
  • Problem: _output_info_json and _save_metadata construct the same dict structure independently.
  • Impact: Adding a new field to info output requires editing two places.
  • Action: Extract _build_info_dict(file_path, sha256_hash, local_metadata, civitai_data) -> dict and call it from both functions.

[ ] Extract API timeout and chunk size constants

  • Location: api.py:43,70,97,175 and safetensor.py:67
  • Problem: timeout=30.0 appears 4 times in api.py. Chunk sizes (1024 * 1024 in api.py, 1024 * 1024 * 8 in safetensor.py) are inline magic numbers.
  • Impact: Changing timeout or chunk size requires hunting through code.
  • Action: Add API_TIMEOUT = 30.0, DOWNLOAD_CHUNK_SIZE = 1024 * 1024 to config.py or at module top. Add HASH_CHUNK_SIZE = 1024 * 1024 * 8 to safetensor.py constants section.

[ ] Rename _format_size and _format_count to public API

  • Location: display.py:23,32
  • Problem: _format_size is imported and used by cli.py:36 as a cross-module API but has a private underscore prefix. Same for _format_count which could be useful externally.
  • Impact: Violates Python naming convention. Underscore-prefixed names signal "don't import this" but the codebase does.
  • Action: Rename to format_size and format_count. Update all imports.

[ ] Deduplicate Progress spinner setup in api.py

  • Location: api.py:61-66,88-93,166-171
  • Problem: The Progress(SpinnerColumn(), TextColumn(...), console=..., transient=True) pattern is repeated 3 times with identical configuration.
  • Impact: Changing spinner appearance requires 3 edits.
  • Action: Extract _spinner(console, description) context manager helper or a factory function. This pairs well with the HTTP helper extraction in High Priority #1.

[ ] Add shared console fixture to tests

  • Location: tests/test_tensors.py:284,291,298,...
  • Problem: Console(force_terminal=True, width=80) is created ~15 times across test methods.
  • Impact: Changing test console config requires editing every test.
  • Action: Add a console fixture to conftest.py and use it across all test classes.

Low Priority

[ ] Replace hardcoded command list in main()

  • Location: cli.py:403
  • Problem: arg not in ("info", "search", "get", "dl", "download", "config") is a manually maintained list of known commands for legacy invocation detection.
  • Impact: Adding a new command requires remembering to update this list, or legacy mode will swallow it.
  • Action: Derive the command list from app.registered_commands or Typer's command registry dynamically.

[ ] Strengthen display function tests

  • Location: tests/test_tensors.py:279-385
  • Problem: Most display tests only assert "should not raise" with no output verification.
  • Impact: Tests won't catch regressions in output format or content.
  • Action: Use Console(record=True) to capture output, then assert key strings appear in console.export_text().

[ ] Add tests for untested pure functions

  • Location: api.py:111-142,145-150,187-199,202-209
  • Problem: _build_search_params, _filter_results, _setup_resume, _get_dest_from_response have no test coverage. These are pure/near-pure functions that are easy to test.
  • Impact: Logic errors in search parameter building or resume setup won't be caught.
  • Action: Add direct unit tests for each function. These don't need HTTP mocking since they're pure logic.

Notes

  • High Priority #1 (HTTP helper) and Medium #6 (spinner dedup) should be done together since the extracted helper will naturally absorb the spinner logic.
  • High Priority #2 (console singleton) should be done before Medium #7 (test fixture) since making console a parameter enables proper test injection.
  • Medium #4 (output dict) is a quick win that can be done independently.
  • Start with High Priority #3 (remove unused dep) as it's the simplest and lowest risk change.