# 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