OpenGait/.sisyphus/notepads/demo-visualizer/learnings.md

## Task 1: CLI Flag Addition

- Added --visualize boolean flag to opengait/demo/__main__.py
- Uses argparse with action="store_true" as requested
- Passes visualize=args.visualize to ScoliosisPipeline constructor
- Note: ScoliosisPipeline does not yet accept visualize parameter (Task 3 will add this)
- All existing CLI options preserved with same defaults as pipeline.py click definitions


## Task 1 Final State

- Single if __name__ == "__main__" block
- Uses inspect.signature to conditionally pass visualize
- All CLI options preserved with correct defaults
- --visualize flag present and functional


## Task 2: OpenCVVisualizer Implementation

### Completed
- Created opengait/demo/visualizer.py with OpenCVVisualizer class
- Internal state self.mask_mode = 0 (0: Both, 1: Raw, 2: Normalized)
- Method update(frame, bbox, track_id, mask_raw, silhouette, label, confidence, fps)
- Two windows: main stream (bbox + text overlay) and segmentation (mode-dependent view)
- Key handling: m cycles mask mode, q returns False to signal quit
- close() method calls cv2.destroyAllWindows()

### Key Implementation Details
- Silhouette shape assumed 64x44 (from preprocess.py SIL_HEIGHT/SIL_WIDTH)
- Upscaled to 256x176 for display using INTER_NEAREST to preserve pixelation
- Handles missing inputs gracefully (bbox/mask_raw/silhouette can be None)
- Converts grayscale arrays to BGR for consistent display
- Mode indicator text shown in segmentation window for operator clarity

### Type Checking Notes
- OpenCV MatLike type conflicts with NDArray annotations
- These are acceptable warnings - runtime behavior is correct
- Used _ = cv2.function() pattern to suppress unused return value warnings


## Task 2 Retry: Type Annotation Fixes

### Problem
LSP diagnostics showed basedpyright errors due to OpenCV MatLike type conflicting with strict NDArray[np.uint8] annotations.

### Solution
- Added ImageArray type alias = NDArray[np.uint8]
- Used typing.cast() to convert OpenCV return values (MatLike) to ImageArray
- Applied cast() on all cv2.cvtColor() and cv2.resize() calls that return MatLike

### Changes Made
- Added: from typing import cast
- Added: ImageArray = NDArray[np.uint8] type alias
- Modified: All methods returning NDArray[np.uint8] now use cast() for OpenCV calls
- Modified: Parameter types changed from NDArray[np.uint8] to ImageArray for consistency

### Verification
- lsp_diagnostics: 0 errors (4 warnings about Any types remain - acceptable)
- Import test: uv run python -c "from opengait.demo.visualizer import OpenCVVisualizer; print(ok)" -> ok
- No TODO/FIXME/HACK placeholders introduced


## Task 3: Pipeline Integration - COMPLETED

### Implementation Summary
- Added `_visualizer: object | None` attribute to `ScoliosisPipeline` class
- Updated `__init__` to accept `visualize: bool = False` parameter
- Conditionally instantiates `OpenCVVisualizer` when `visualize=True`
- Updated `_select_silhouette` to return 4-tuple: `(silhouette, mask_raw, bbox, track_id)`
- Updated `process_frame` to:
  - Unpack 4-tuple from `_select_silhouette`
  - Return visualization payload dict with `mask_raw`, `bbox`, `silhouette`, `track_id`, `label`, `confidence`
  - Return payload in all paths (preprocess-only, not-ready-to-classify, and classified)
- Updated `run()` to:
  - Compute per-frame EMA FPS with alpha=0.1 smoothing
  - Call `self._visualizer.update()` with all required parameters
  - Break loop if visualizer returns `False` (user pressed 'q')
- Updated `close()` to close visualizer via `self._visualizer.close()` in finally path

### Key Design Decisions
- Used `object | None` type for `_visualizer` to avoid circular import issues
- EMA FPS uses alpha=0.1 for reasonable smoothing while maintaining responsiveness
- Visualization payload returned in all paths ensures consistent behavior
- Visualizer cleanup happens in `close()` which is called in `finally` block

### Verification Results
- `uv run python -m opengait.demo --help` - PASSED
- `uv run python -m opengait.demo --source foo --checkpoint bar --config baz --device cpu --visualize` - PASSED (reaches file-not-found as expected)
- Constructor accepts `visualize` parameter - PASSED

## Task 3 Regression Fix - COMPLETED

### Problem
LSP errors at lines 343-350 due to calling `.get()` / `.update()` on `object` typed values (`viz_payload`, `_visualizer`).

### Solution
Used `cast()` to tell the type checker the actual types:
1. `viz_payload` is cast to `dict[str, object]` before calling `.get()`
2. `self._visualizer` is cast to `object` and methods are accessed via `getattr()`

### Key Changes
- In `run()`: `viz_dict = cast(dict[str, object], viz_payload)` before `.get()` calls
- In `run()`: `visualizer = cast(object, self._visualizer)` then `getattr(visualizer, "update", None)`
- In `close()`: Same pattern for calling `.close()` on visualizer

### Verification
- `lsp_diagnostics(opengait/demo/pipeline.py)` - ZERO ERRORS (only acceptable warnings)
- `uv run python -m opengait.demo --help` - PASSED
- `uv run python -m opengait.demo --source foo --checkpoint bar --config baz --device cpu --visualize` - PASSED (reaches file-not-found)



## Task 4: CLI Validation Pattern

### Lesson
When migrating from click to argparse, ensure ALL behavior is preserved:
- Input validation (both CLI-level and runtime)
- Exit code mapping for different error types
- User-friendly error messages

### Pattern for CLI Entry Points
```python
if __name__ == "__main__":
    args = parser.parse_args()
    
    # CLI-level validation
    if args.preprocess_only and not args.silhouette_export_path:
        print("Error: ...", file=sys.stderr)
        raise SystemExit(2)
    
    try:
        validate_runtime_inputs(...)
        pipeline = ScoliosisPipeline(...)
        raise SystemExit(pipeline.run())
    except ValueError as err:
        print(f"Error: {err}", file=sys.stderr)
        raise SystemExit(2) from err
    except RuntimeError as err:
        print(f"Runtime error: {err}", file=sys.stderr)
        raise SystemExit(1) from err
```

### Key Takeaway
Exit code parity matters for test suites and shell scripting. ValueError (user input errors) -> 2, RuntimeError (system/runtime errors) -> 1.

## Task 5: YOLO Model Path Relocation

### Task Summary
Moved yolo11n-seg.pt from repo root to ckpt/yolo11n-seg.pt and updated all in-repo references.

### Files Modified
1. opengait/demo/__main__.py - Updated --yolo-model default
2. opengait/demo/pipeline.py - Updated --yolo-model default
3. tests/demo/test_pipeline.py - Updated YOLO_MODEL_PATH

### Verification
- All 3 references now point to ckpt/yolo11n-seg.pt
- Tests pass: 12 passed, 1 skipped in 37.35s



## Oracle Caveat Fix #1: Simplified inspect-based conditional logic

### Change Summary
- Removed `import inspect` from opengait/demo/__main__.py
- Removed `inspect.signature(ScoliosisPipeline.__init__)` call
- Removed conditional `if "visualize" in sig.parameters:` check
- Now passes `visualize=args.visualize` directly in pipeline_kwargs

### Rationale
The inspect-based conditional was unnecessary complexity for an intra-package API contract.
ScoliosisPipeline.__init__ explicitly accepts the visualize parameter, so the runtime
check provided no value and added maintenance burden.

### Verification
- `uv run python -m opengait.demo --help` - PASSED (all CLI options preserved)
- `uv run pytest tests/demo/test_pipeline.py -q` - PASSED (12 passed, 1 skipped)
- `lsp_diagnostics(opengait/demo/__main__.py)` - No new errors (only pre-existing warnings)

### Behavior Parity
CLI behavior unchanged. --visualize flag still supported and passed to pipeline.
Default yolo path preserved: ckpt/yolo11n-seg.pt


## Oracle Caveat Fix #2: Explicit Typing for Visualizer

### Change Summary
- Replaced loose `_visualizer: object | None` with explicit `OpenCVVisualizer | None` using TYPE_CHECKING forward reference
- Removed `cast(object, ...)` and `getattr(..., "update"/"close")` indirection in favor of direct method calls
- Added explicit casts for values extracted from viz_payload dict to satisfy type checker

### Rationale
The previous implementation used `object` typing and runtime introspection (`getattr`) to avoid circular imports and maintain optional dependency semantics. However, this sacrificed type safety and code clarity. Using `TYPE_CHECKING` allows us to:
1. Import the actual type for static analysis without runtime import
2. Call methods directly on the typed visualizer instance
3. Maintain lazy import behavior (OpenCVVisualizer still only imported when visualize=True)

### Key Changes
1. Added TYPE_CHECKING block with forward import: `from .visualizer import OpenCVVisualizer`
2. Changed `_visualizer` type annotation from `object | None` to `OpenCVVisualizer | None`
3. In `run()`: Removed `cast(object, self._visualizer)` and `getattr(visualizer, "update", None)` pattern
   - Now calls `self._visualizer.update(...)` directly
   - Added explicit casts for dict values extracted from viz_payload
4. In `close()`: Removed `cast(object, self._visualizer)` and `getattr(visualizer, "close", None)` pattern
   - Now calls `self._visualizer.close()` directly

### Verification
- `lsp_diagnostics(opengait/demo/pipeline.py)` - ZERO ERRORS (only pre-existing warnings unrelated to visualizer)
- `uv run pytest tests/demo/test_pipeline.py -q` - PASSED (12 passed, 1 skipped)
- Runtime behavior unchanged: lazy import preserved, EMA FPS calculation unchanged, quit handling unchanged

### Type Safety Improvement
Before: Runtime introspection required, no static type checking on visualizer methods
After: Full static type checking on visualizer.update() and visualizer.close() calls, proper type inference for all parameters

## Oracle Non-blocking Improvement: _prepare_both_view Redundant Work Removal

### Change Summary
- Modified `_prepare_both_view` in `opengait/demo/visualizer.py` to eliminate wasted text-rendering work
- Previously: Called `_prepare_raw_view` and `_prepare_normalized_view` which drew mode indicators, then converted to grayscale (destroying the text), then drew combined indicator
- Now: Inlines the view preparation logic without mode indicators, preserving only the final combined indicator

### Rationale
The sub-view mode indicators ("Raw Mask" and "Normalized") were being drawn and immediately destroyed by grayscale conversion before stacking. This was pure overhead with no visual effect. The final combined indicator ("Both: Raw | Normalized") is the only one visible to users.

### Behavior Preservation
- Visual output unchanged: Final combined mode indicator still displayed
- Mode toggle semantics untouched: mask_mode cycling (0->1->2->0) unchanged
- Placeholder handling preserved: None inputs still produce zero-filled arrays
- All existing tests pass: 12 passed, 1 skipped

### Code Quality Impact
- Reduced unnecessary OpenCV text rendering operations
- Eliminated redundant BGR->Gray->BGR conversions on sub-views
- Improved maintainability by making the wasted work explicit (removed)

### Verification
- `lsp_diagnostics(opengait/demo/visualizer.py)` - 0 errors (4 pre-existing Any warnings)
- `uv run pytest tests/demo/test_pipeline.py -q` - PASSED (12 passed, 1 skipped)


## Oracle Non-blocking Cleanup: Duplicate Import Removal

### Change Summary
- Removed duplicated import block at lines 17-28 in `tests/demo/test_pipeline.py`
- Duplicated imports: `json`, `pickle`, `Path`, `subprocess`, `sys`, `time`, `Final`, `cast`, `pytest`, `torch`, `ScoNetDemo`
- Kept first import block (lines 1-15) intact

### Rationale
Identical import block appeared twice consecutively—likely from a merge conflict resolution or copy-paste error. No functional impact, but code hygiene improvement.

### Verification
- `uv run pytest tests/demo/test_pipeline.py -q` - PASSED (12 passed, 1 skipped)
- `lsp_diagnostics(tests/demo/test_pipeline.py)` - No new errors (only pre-existing warnings)

### Behavior Preservation
- No test logic modified
- All imports required by tests remain available
- Import order and formatting unchanged


## Dependency Configuration: cvmmap-client Local Path Source

### Task Summary
Added `cvmmap-client` as a dependency sourced from local path `/home/crosstyan/Code/cvmmap-python-client`.

### Changes Made
1. **pyproject.toml**:
   - Added `"cvmmap-client"` to `[project] dependencies list
   - Added `[tool.uv.sources]` section with path mapping:
     ```toml
     [tool.uv.sources]
     cvmmap-client = { path = "/home/crosstyan/Code/cvmmap-python-client" }
     ```

2. **uv.lock**: Updated automatically via `uv lock` to include:
   - `cvmmap-client v0.1.0` (from file:// path)
   - `pyzmq v27.1.0` (transitive dependency)

### Verification
- `uv lock` - PASSED (resolved 104 packages)
- `uv run python -c "from cvmmap import CvMmapClient; print('ok')"` - PASSED

### Key Points
- Package name in provider repo: `cvmmap-client` (distribution name)
- Import path: `from cvmmap import CvMmapClient` (module name)
- uv path sources require absolute paths
- Lockfile captures the path dependency for reproducibility