Skip to content

REVIEW.md

Latest commit

 

History

History
42 lines (30 loc) · 2.36 KB

REVIEW.md

File metadata and controls

42 lines (30 loc) · 2.36 KB

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