From 976a1fa1d0dc6611f36d825983d85a920aa49855 Mon Sep 17 00:00:00 2001 From: windyboy Date: Mon, 9 Feb 2026 17:55:34 +0800 Subject: [PATCH] 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 --- AGENTS.md | 38 +++++++++++++++++++++++++++++++++++++ REVIEW_REPORT.md | 49 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 87 insertions(+) create mode 100644 AGENTS.md create mode 100644 REVIEW_REPORT.md diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..95cc4dd --- /dev/null +++ b/AGENTS.md @@ -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. diff --git a/REVIEW_REPORT.md b/REVIEW_REPORT.md new file mode 100644 index 0000000..de0163b --- /dev/null +++ b/REVIEW_REPORT.md @@ -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`.