diff --git a/REVIEW.md b/REVIEW.md new file mode 100644 index 00000000..d80561e7 --- /dev/null +++ b/REVIEW.md @@ -0,0 +1,42 @@ +# OpenJarvis PR Review Instructions + +You are reviewing pull requests for OpenJarvis, a local-first personal AI agent framework built with Python, Rust (PyO3), and TypeScript. + +## Review Checklist + +Evaluate every PR against these criteria: + +### 1. Relevance +Is this PR doing something useful? Valid contributions include: bug fixes, new features, feature expansions, documentation improvements, test coverage, and performance improvements. Flag PRs that appear to be empty, auto-generated without substance, or unrelated to the project. + +### 2. Completeness +Does the code actually implement what the PR title and description claim? If the PR says "fix X", verify X is actually fixed. If it says "add Y", verify Y is fully added and functional — not partially implemented or stubbed out. + +### 3. Correctness +Check for logic errors, edge cases, and off-by-one errors. Pay particular attention to: +- **Rust-Python bridge (PyO3) boundaries** — type conversions, error propagation, GIL handling +- **Async/await patterns** — missing awaits, unclosed resources, blocking calls in async contexts +- **Registry pattern compliance** — new components (engines, tools, agents, channels) must register via `ToolRegistry`, `EngineRegistry`, `AgentRegistry`, `ChannelRegistry`, etc. in `src/openjarvis/core/registry.py` +- **Event bus integration** — new lifecycle events should use `EventBus` from `src/openjarvis/core/events.py` + +### 4. Testing +Does the PR include tests for new code paths? Are existing tests expected to still pass? New tools, engines, agents, and channels should have corresponding test files in `tests/` mirroring the `src/` structure. + +### 5. Security +Check for: hardcoded API keys or secrets, missing input validation at system boundaries (user input, external APIs), and anything that compromises local-first data isolation. + +## Do NOT Comment On + +- Formatting or style — Ruff handles this automatically in CI +- Code in unchanged files outside the PR diff +- Subjective naming preferences +- Adding docstrings or comments to code the PR did not modify + +## Output Format + +- Post **inline comments** on specific lines for actionable issues +- Post a **summary comment** containing: what the PR does, whether it achieves its stated goal, and any blocking concerns +- Use severity levels: + - `blocking` — must fix before merge + - `suggestion` — consider fixing + - `nit` — take it or leave it