docs: Add repository guidelines and code review report
- Add AGENTS.md with comprehensive repository guidelines covering project structure, build/test commands, coding style, testing practices, and commit conventions - Add REVIEW_REPORT.md documenting code review findings including 4 actionable issues: logging permission errors, incorrect test imports, timezone conversion bugs, and dead error counter code - Include detailed recommendations for fix prioritization and test evidence from pytest runs - Provide reference documentation for future development and maintenance workflows
This commit is contained in:
@@ -0,0 +1,38 @@
|
|||||||
|
# Repository Guidelines
|
||||||
|
|
||||||
|
## Project Structure & Module Organization
|
||||||
|
- Core package lives in `src/vlm/`.
|
||||||
|
- CLI entrypoint is `src/vlm/cli.py` (`vlm` console script).
|
||||||
|
- Functional modules are split by concern: scanning (`scanner.py`), parsing (`parser.py`), analysis/planning/execution (`analysis.py`, `planner.py`, `executor.py`), state/reporting/logging (`state.py`, `reports.py`, `logging_config.py`).
|
||||||
|
- Tests live in `tests/` and mirror feature areas (for example `tests/test_scanner.py`, `tests/test_cli_state.py`).
|
||||||
|
- Project metadata and tool config are in `pyproject.toml`.
|
||||||
|
|
||||||
|
## Build, Test, and Development Commands
|
||||||
|
- `uv pip install -e .` installs the package in editable mode.
|
||||||
|
- `uv pip install -e ".[dev]"` installs dev dependencies (`pytest`, `hypothesis`).
|
||||||
|
- `pytest` runs the full test suite.
|
||||||
|
- `pytest tests/test_logging.py` runs a targeted test file during iteration.
|
||||||
|
- `vlm --help` verifies CLI startup and available commands.
|
||||||
|
|
||||||
|
## Coding Style & Naming Conventions
|
||||||
|
- Use Python 3.10+ idioms, 4-space indentation, and PEP 8 naming.
|
||||||
|
- Modules/functions/variables: `snake_case`; classes: `PascalCase`; constants: `UPPER_SNAKE_CASE`.
|
||||||
|
- Keep modules focused on a single responsibility; prefer small pure helpers in domain modules.
|
||||||
|
- Add type hints for public functions and non-trivial internal APIs.
|
||||||
|
- No formatter/linter is currently enforced in `pyproject.toml`; keep style consistent with existing files.
|
||||||
|
|
||||||
|
## Testing Guidelines
|
||||||
|
- Framework: `pytest`; property-based tests use `hypothesis`.
|
||||||
|
- Naming (enforced in config): files `test_*.py`, functions `test_*`, classes `Test*`.
|
||||||
|
- Add/extend tests with each behavior change, including CLI error paths and edge cases.
|
||||||
|
- Prefer narrow unit tests for module logic plus targeted CLI integration tests via `CliRunner`.
|
||||||
|
|
||||||
|
## Commit & Pull Request Guidelines
|
||||||
|
- Current history is minimal; use clear, imperative commit subjects (example: `fix logging fallback for unwritable log dir`).
|
||||||
|
- Keep commits focused; avoid mixing refactors and behavior changes unless tightly coupled.
|
||||||
|
- PRs should include: summary, rationale, test evidence (`pytest` output), and any CLI-visible output changes.
|
||||||
|
- Link related issues/tasks when applicable and call out config or migration impacts.
|
||||||
|
|
||||||
|
## Security & Configuration Tips
|
||||||
|
- Do not commit local paths, personal media metadata, or generated state/log artifacts.
|
||||||
|
- Validate config changes against `vlm --help` and at least one end-to-end CLI flow before merging.
|
||||||
@@ -0,0 +1,49 @@
|
|||||||
|
# Code Review Report
|
||||||
|
|
||||||
|
## Scope
|
||||||
|
- Reviewed Python sources under `src/vlm/`, tests under `tests/`, and docs (`README.md`, `AGENTS.md`).
|
||||||
|
- Executed test runs on February 9, 2026:
|
||||||
|
- `pytest -q`
|
||||||
|
- `pytest -q --ignore=tests/test_executor.py --ignore=tests/test_quarantine.py`
|
||||||
|
|
||||||
|
## Summary
|
||||||
|
- Found 4 actionable issues: 2 high-priority functional problems, 1 medium-priority data correctness issue, and 1 low-priority observability issue.
|
||||||
|
- Markdown docs are generally clear; no blocking doc defects were found.
|
||||||
|
|
||||||
|
## Findings
|
||||||
|
|
||||||
|
### P1 - CLI startup fails when log path is not writable
|
||||||
|
- Files: `src/vlm/logging_config.py:63`, `src/vlm/logging_config.py:87`
|
||||||
|
- `setup_logging()` unconditionally creates the log directory and rotating file handler.
|
||||||
|
- In restricted environments, this raises `PermissionError` and aborts CLI initialization (including read-only commands like `--help`).
|
||||||
|
- Impact: broad command/test failure in CI/sandbox/service-user contexts.
|
||||||
|
|
||||||
|
### P1 - Test imports use wrong module path
|
||||||
|
- Files:
|
||||||
|
- `tests/test_executor.py:13`
|
||||||
|
- `tests/test_executor.py:14`
|
||||||
|
- `tests/test_quarantine.py:8`
|
||||||
|
- `tests/test_quarantine.py:9`
|
||||||
|
- `tests/test_quarantine.py:10`
|
||||||
|
- Tests import `src.vlm...` instead of package imports `vlm...`, causing collection failure (`ModuleNotFoundError: No module named 'src'`).
|
||||||
|
|
||||||
|
### P2 - Timestamp conversion is incorrect for naive datetimes
|
||||||
|
- Files: `src/vlm/scanner.py:148`, `src/vlm/scanner.py:379`, `src/vlm/scanner.py:437`
|
||||||
|
- Naive local timestamps are later relabeled as UTC via `replace(tzinfo=timezone.utc)` instead of converted.
|
||||||
|
- Impact: exported timestamps can be shifted by local timezone offset.
|
||||||
|
|
||||||
|
### P3 - Scan error counter is dead code
|
||||||
|
- Files: `src/vlm/scanner.py:53`, `src/vlm/scanner.py:64`
|
||||||
|
- `error_count` is initialized/reported but never incremented.
|
||||||
|
- Impact: scan summary underreports error conditions.
|
||||||
|
|
||||||
|
## Test Evidence
|
||||||
|
- `pytest -q` failed at collection due to `src.vlm` imports in two test files.
|
||||||
|
- `pytest -q --ignore=tests/test_executor.py --ignore=tests/test_quarantine.py` reported 34 failures, dominated by logging startup failure:
|
||||||
|
- `PermissionError: [Errno 1] Operation not permitted: '/Users/windy/.vlm/logs/vlm.log'`
|
||||||
|
|
||||||
|
## Recommended Fix Order
|
||||||
|
1. Make logging setup fault-tolerant (fallback to console-only logging).
|
||||||
|
2. Correct test imports to `vlm...`.
|
||||||
|
3. Fix timezone handling for inventory timestamps.
|
||||||
|
4. Wire scan exception paths to increment `error_count`.
|
||||||
Reference in New Issue
Block a user