loader/_docs/04_refactoring/01-quality-cleanup/list-of-changes.md

List of Changes

Run: 01-quality-cleanup Mode: automatic Source: self-discovered Date: 2026-04-13

Summary

Address thread safety issues, replace unsafe manual cryptographic padding with library implementations, remove dead code, and fix minor configurability/error-handling gaps.

Changes

C01: Thread-safe ApiClient singleton

• File(s): main.py
• Problem: get_api_client() checks if api_client is None and assigns without a lock. Concurrent requests can create duplicate instances, with the second overwriting the first.
• Change: Protect the singleton initialization with a threading.Lock using the double-checked locking pattern.
• Rationale: FastAPI serves concurrent requests via threads; the global singleton must be safe under concurrency.
• Risk: low
• Dependencies: None

C02: Encapsulate unlock state

• File(s): main.py
• Problem: unlock_state and unlock_error are module-level globals mutated via a lock. This pattern scatters state management across the module and makes the state machine hard to reason about.
• Change: Move unlock state and error into a small state holder class that encapsulates the lock and provides thread-safe read/write methods.
• Rationale: Single Responsibility — state management belongs in one place, not spread across endpoint handlers and background tasks.
• Risk: low
• Dependencies: None

C03: Use library PKCS7 unpadder in Security.decrypt_to

• File(s): security.pyx
• Problem: decrypt_to manually reads the last byte to determine padding length. It does not validate that all N trailing bytes equal N (PKCS7 requirement). Corrupted ciphertext silently produces garbage. The padding.PKCS7(128).unpadder() is already imported but unused for decryption.
• Change: Replace manual unpadding (lines 38-44) with the library's padding.PKCS7(128).unpadder(), matching the encrypt path.
• Rationale: The library validates all padding bytes and raises on corruption instead of silently returning garbage.
• Risk: medium — changes decryption output on invalid input (correctly raises instead of silently passing)
• Dependencies: None

C04: Use library PKCS7 unpadder in decrypt_archive

• File(s): binary_split.py
• Problem: decrypt_archive manually reads the last byte of the decrypted file to strip padding (lines 46-53). Same incomplete PKCS7 validation as C03.
• Change: After decryption, use the cryptography library's PKCS7 unpadder to strip padding instead of manual byte inspection and file truncation.
• Rationale: Same as C03 — proper validation, raises on corruption.
• Risk: medium — same reasoning as C03
• Dependencies: None

C05: Configurable log file path

• File(s): constants.pyx
• Problem: Log file sink is hardcoded to "Logs/log_loader_{time:YYYYMMDD}.txt". No environment variable override. Creates unexpected directories outside Docker.
• Change: Read the log directory from an environment variable (e.g., LOG_DIR) with the current value as default.
• Rationale: Configurability across development and production environments.
• Risk: low
• Dependencies: None

C06: Log os.remove failure instead of swallowing

• File(s): main.py
• Problem: os.remove(tar_path) catches OSError and passes silently (lines 143-146). Hides information about filesystem issues.
• Change: Log the exception at warning level instead of silently passing.
• Rationale: Per project rules, errors should not be silently suppressed.
• Risk: low
• Dependencies: None

C07: Remove dead methods from ApiClient

• File(s): api_client.pyx, api_client.pxd
• Problem: 5 methods are defined and declared but never called from any source file: get_user, list_files, check_resource, upload_to_cdn, download_from_cdn.
• Change: Delete the 5 orphan method definitions from api_client.pyx and their declarations from api_client.pxd.
• Rationale: Dead code misleads readers and breaks when its dependencies evolve. Git history preserves it.
• Risk: low
• Dependencies: None

C08: Remove dead constants

• File(s): constants.pyx, constants.pxd
• Problem: 5 constants in constants.pyx are never referenced outside their own file: CONFIG_FILE, QUEUE_CONFIG_FILENAME, AI_ONNX_MODEL_FILE, MODELS_FOLDER, ALIGNMENT_WIDTH. Additionally, 3 variables are declared in constants.pxd with no definition in constants.pyx: QUEUE_MAXSIZE, COMMANDS_QUEUE, ANNOTATIONS_QUEUE.
• Change: Delete the 5 unused constant definitions from constants.pyx and all 8 orphan declarations from constants.pxd (keeping only CDN_CONFIG, SMALL_SIZE_KB, log, logerror).
• Rationale: Dead code rots. Orphan .pxd declarations with no backing definition are especially misleading.
• Risk: low
• Dependencies: None

C09: CDN upload failure silently ignored

• File(s): api_client.pyx
• Problem: upload_big_small_resource (line 203) calls self.cdn_manager.upload() which returns False on failure, but the return value is never checked. A failed CDN upload means the big part is only saved locally — the resource cannot be downloaded from other devices.
• Change: Check the return value of cdn_manager.upload and raise on failure, matching the pattern in the (currently orphan) upload_to_cdn method.
• Rationale: Silent upload failure leads to incomplete resources that cannot be downloaded from CDN on other devices.
• Risk: medium — previously-silent failures will now raise exceptions
• Dependencies: None

C10: API upload failure silently swallowed

• File(s): api_client.pyx
• Problem: upload_file (line 91-102) catches all exceptions and only logs them. The caller (upload_big_small_resource at line 206) never knows the API upload failed. The small part is silently lost.
• Change: Let the exception propagate to the caller instead of catching and logging. The caller's error handling (main.py endpoint) will convert it to an appropriate HTTP error.
• Rationale: The upload endpoint should report failure, not return {"status": "ok"} when the small part was never uploaded.
• Risk: medium — previously-silent failures will now raise exceptions
• Dependencies: None