Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 84 additions & 0 deletions .claude/skills/openmc-code-review/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
---
name: openmc-code-review
description: Reviews code changes in the OpenMC codebase against OpenMC's contribution criteria (correctness, testing, physics soundness, style, design, performance, docs, dependencies). Use when asked to review a PR, branch, patch, or set of code changes in OpenMC, or when an author wants to self-review changes before submitting a PR.
---

Apply repository-wide guidance from `AGENTS.md` (architecture, build/test workflow, branch conventions, style, and OpenMC-specific expectations). This skill adds a structured review process and rubric on top of that baseline knowledge.

## Determine Review Context

1. **Identify what to review.** Determine the review context based on what the user provides or what can be inferred:
- **Default (no explicit context):** Compare the current branch against `develop` using `git diff develop...HEAD` to see only commits unique to the current branch.
- **User specifies a base branch or commit range:** Use that instead.
- **PR context provided by tooling:** Use the base/head refs supplied by the tool.
2. **Read changed files in context** — look at surrounding code, related modules, and existing codebase style to judge consistency.
3. **Determine review mode:**
- **Self-review mode** (author preparing changes before submission): Emphasize likely reviewer objections, missing tests/docs, API naming consistency, and quick fixes that would speed up review.
- **Reviewer mode** (default): Provide a standard structured review suitable for a code review or PR review.

If the user explicitly requests self-review, use self-review mode. Otherwise, default to reviewer mode.

## Review Criteria

Assess each of the following areas, noting any issues found. If an area looks good, briefly confirm it passes.

### Purpose and Scope
- Do the changes have a clear, well-defined purpose?
- Are the changes of **general enough interest** to warrant inclusion in the main OpenMC codebase, or would they be better suited as a downstream extension?

### Correctness and Testing
- Do the changes compile and appear functionally correct?
- Are appropriate **unit tests** added in `tests/unit_tests/` for new Python API features?
- Are appropriate **regression tests** added in `tests/regression_tests/` for new simulation capabilities?
- Are edge cases and error conditions handled and tested?

### Physics Soundness (when applicable)
- When the changes implement new physics, are the **equations, methods, and approaches physically sound**?
- Are the algorithms consistent with established references? Are those references cited in comments or documentation?
- Are there numerical stability or accuracy concerns with the implementation?

### Code Quality and Style
- Does the C++ code conform to the OpenMC style guide: `CamelCase` classes, `snake_case` functions/variables, trailing underscores for class members, C++17 idioms, `openmc::vector` instead of `std::vector`?
- Does the Python code conform to PEP 8, use numpydoc docstrings, `pathlib.Path` for filesystem operations, and `openmc.checkvalue` for input validation?
- Are the changes (API design, naming, abstractions, file organization) **consistent with the rest of the codebase**?

### Design
- Is the design as simple as it could be while still meeting the requirements?
- Are there **alternative designs** that would achieve the same purpose with greater simplicity or better integration with existing infrastructure?
- Does the API feel natural and follow the conventions established elsewhere in OpenMC?

### Memory and Performance
- Are there obvious memory leaks or unsafe memory management patterns in C++ code?
- Do the changes introduce unnecessary performance regressions or greatly increased memory usage?
- Do the changes introduce dynamic memory allocation (e.g., `new`/`delete`, heap-allocating containers, `std::make_shared`, `std::make_unique`) inside the main particle transport loop (`transport_history_based` and `transport_event_based`)? This is undesirable for two reasons: it degrades thread scalability due to contention on the global allocator, and it precludes future GPU execution where dynamic allocation is not available.

### Documentation
- Are new features, input parameters, and Python API additions **documented** (docstrings, `docs/source/`)?
- Are new XML input attributes described in the input reference?
- Are any deprecations or breaking changes clearly noted?

### Dependencies
- Do the changes introduce any new external software dependencies?
- If so, are they justified, optional where possible, and consistent with OpenMC's existing dependency policy?

## Output Format

Produce your review as a structured report with the following sections:

**Context**: State the review mode (self-review or reviewer) and what is being compared (e.g., "current branch vs. `develop`", or the specific commit range/PR).

**Summary**: A short paragraph describing what the changes do and your overall assessment.

**Detailed Findings**: For each criterion above, provide a brief assessment. Use `✓` for items that pass and flag issues with severity:
- `[Minor]` — Style nits, small improvements, non-blocking suggestions
- `[Moderate]` — Issues worth addressing but not strictly blocking
- `[Major]` — Problems that should be resolved before merging

Group findings into:
1. **Blocking issues** — Would justify requesting changes before merge
2. **Non-blocking suggestions** — Improvements that could be addressed now or later
3. **Questions for the author** — Ambiguities or design choices worth clarifying

**Recommended Action**:
- *Reviewer mode*: One of — *Approve*, *Approve with minor suggestions*, *Request changes*, or *Reject* — with a brief justification.
- *Self-review mode*: A prioritized list of pre-submission fixes — missing tests, undocumented parameters, naming inconsistencies, or anything else likely to slow down review.
8 changes: 8 additions & 0 deletions .github/agents/Review.agent.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
name: Review
description: Reviews code changes on the current branch, evaluating them against OpenMC's contribution criteria and providing structured feedback. Useful for PR reviews and pre-submission self-review.
argument-hint: Optionally provide a focus area (e.g., "focus on physics correctness", "check Python API design", "self-review before PR"). If omitted, a full review is performed.
---
You are an expert code reviewer for OpenMC. Use the `openmc-code-review` skill to perform a structured review of the code changes on the current branch.

If the user provides a focus area, prioritize that section of the review. If the user requests a self-review, use the skill's self-review mode.
1 change: 1 addition & 0 deletions .github/copilot-instructions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
When reviewing code changes in this repository, use the `openmc-code-review` skill.
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ OpenMC uses a git flow branching model with two primary branches:

### Instructions for Code Review

When analyzing code changes on a feature or bugfix branch (e.g., when a user asks "what do you think of these changes?"), **compare the branch changes against `develop`, not `master`**. Pull requests are submitted to merge into `develop`, so differences relative to `develop` represent the actual proposed changes. Comparing against `master` will include unrelated changes from other features that have already been merged to `develop`.
When reviewing code changes in this repository, use the `openmc-code-review` skill.

### Workflow for contributors

Expand Down