Add CLAUDE.md and .claude/rules/ for Claude Code guidance#1255
Add CLAUDE.md and .claude/rules/ for Claude Code guidance#1255sbryngelson merged 7 commits intomasterfrom
Conversation
Adds project-level instructions for Claude Code interactive sessions and automated PR reviews. Core CLAUDE.md covers commands, development workflow contract, architecture, and critical rules. Domain-specific knowledge (Fortran conventions, GPU/MPI patterns, parameter system, common pitfalls) lives in modular .claude/rules/ files. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
📝 WalkthroughWalkthroughAdds five new documentation files under Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
codeant-ai
bot
added
the
size:L
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
|
CodeAnt AI finished reviewing your PR. |
There was a problem hiding this comment.
Pull request overview
This PR introduces comprehensive documentation for Claude Code AI assistance in the MFC codebase. It establishes project identity, workflow contracts, and domain-specific knowledge for both interactive development sessions and automated code reviews.
Changes:
- Adds
CLAUDE.mdas the core instruction file covering project identity, CLI commands, development workflow, architecture, and critical coding rules - Adds modular
.claude/rules/directory with four domain-specific knowledge files for Fortran conventions, GPU/MPI patterns, parameter system flow, and common pitfalls
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| CLAUDE.md | Core Claude Code instruction file with project overview, CLI reference, workflow contract, and critical rules |
| .claude/rules/parameter-system.md | Documents Python→Fortran parameter flow and 3-location checklist for adding parameters |
| .claude/rules/gpu-and-mpi.md | Details GPU macro abstraction system, preprocessor defines, compiler-backend matrix, and MPI patterns |
| .claude/rules/fortran-conventions.md | Specifies Fortran naming conventions, precision types, module structure, and forbidden patterns |
| .claude/rules/common-pitfalls.md | Lists common error patterns including array bounds, blast radius warnings, and PR checklist |
The GPU section previously only mentioned OpenACC. Now reflects that MFC supports both OpenACC and OpenMP target offload via backend-agnostic GPU_* Fypp macros. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Expand 3-location to 4-location checklist: add m_global_parameters.fpp as required step for declaring Fortran variables (without this, adding a parameter to the namelist in m_start_up.fpp causes a compile error) - Fix fastjsonschema attribution: JSON schema validation is in case.py and params/registry.py, not in case_validator.py which does physics constraint checking Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Claude Code Review (updated)Head SHA: cb58059 Files changed: 5
Summary of changes
FindingsIssue 1 - Wrong Fypp call prefix for all GPU macros (
|
In mixed-precision mode (--mixed), stp is set to half_precision (kind=2), not single_precision, as confirmed in src/common/m_precision_select.f90. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- GPU macros use $: prefix (inline), not @: — matches all 400+ usages in src/ - GPU_PARALLEL, GPU_DATA, GPU_HOST_DATA are block macros using #:call/#:endcall - Add block macro usage example - Fix GPU_ROUTINE args: parallelism='[seq]', not function_name= - Split forbidden patterns into precheck-enforced vs convention-enforced (goto, COMMON, save, stop are not checked by precheck.sh) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
CLAUDE.md (1)
113-125: Add a language to the fenced code block to satisfy MD040.markdownlint flags the architecture block as missing a language specifier.
🔧 Proposed fix
-``` +```text src/ common/ # Shared code (used by ALL three executables — wide blast radius) pre_process/ # Grid generation and initial conditions simulation/ # CFD solver (GPU-accelerated via OpenACC / OpenMP target offload) post_process/ # Data output and visualization toolchain/ # Python CLI, build system, testing, parameter management mfc/params/definitions.py # ~3300 parameter definitions (source of truth) mfc/case_validator.py # Physics constraint validation mfc/test/ # Test runner and case generation examples/ # Example simulation cases (case.py files) tests/ # 500+ regression test golden files -``` +```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CLAUDE.md` around lines 113 - 125, The fenced code block in CLAUDE.md is missing a language specifier causing markdownlint MD040; update the opening fence from ``` to ```text (or another appropriate language like ```none) so the block becomes language-tagged, ensuring the directory tree snippet is fenced as ```text and the closing fence stays ```.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/rules/parameter-system.md:
- Around line 26-29: Update the heading "Adding a New Parameter (3-location
checklist)" to match the body by changing it to "Adding a New Parameter
(4-location checklist)" (or "4-step checklist") so the heading and the line "YOU
MUST update all 4 locations" are consistent; locate the heading string in the
file and edit only that text.
---
Nitpick comments:
In `@CLAUDE.md`:
- Around line 113-125: The fenced code block in CLAUDE.md is missing a language
specifier causing markdownlint MD040; update the opening fence from ``` to
```text (or another appropriate language like ```none) so the block becomes
language-tagged, ensuring the directory tree snippet is fenced as ```text and
the closing fence stays ```.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.claude/rules/fortran-conventions.md.claude/rules/gpu-and-mpi.md.claude/rules/parameter-system.mdCLAUDE.md
🚧 Files skipped from review as they are similar to previous changes (2)
- .claude/rules/gpu-and-mpi.md
- .claude/rules/fortran-conventions.md
…patterns - Fix "3-location checklist" heading to "4-location" (parameter-system.md) - Fix "all 3 locations" to "all 4 locations" in PR checklist (common-pitfalls.md) - Remove non-existent $:END_GPU_LOOP() from block macro example (gpu-and-mpi.md) - Add d0 literals, double precision, dcos/dsin/dtan to forbidden patterns list - Reference toolchain/bootstrap/precheck.sh for full forbidden pattern list - Fix Frontier system slug: OpenACC/OpenMP, not just OpenACC - Clarify stp as "field data arrays and I/O", not just "I/O" - Document @:PROHIBIT, @:ASSERT, @:LOG error-checking macros - Document @:PREFER_GPU unified memory macro - Document m_checker*.fpp Fortran-side runtime validation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Claude Code Review (updated)
6 files changed:
Summary
FindingsIssue 1 — Wrong Fypp call prefix for GPU macros in
|
| File | Says |
|---|---|
CLAUDE.md line 139 |
✅ "in 4 places" |
parameter-system.md line 26 |
❌ "3-location checklist" heading, then "all 4 locations" in body |
common-pitfalls.md line 39 |
❌ "all 3 locations updated" |
The correct count is 4 (definitions.py, m_global_parameters.fpp, m_start_up.fpp, case_validator.py). The heading in parameter-system.md and the checklist item in common-pitfalls.md should both say "4".
Generated with Claude Code
Claude Code Review (updated)Head SHA: 3286cfa 6 files changed:
Summary
Findings1. Internal contradiction in
|
- Fix parameter checklist: first 3 mandatory, #4 only if constraints exist - Fix namelist format: &user_inputs ... &end/ (not &user_inputs / ... / &end) - Update counts: ~3,400 params (was ~3,300), 560+ tests (was 500+) - Add ghost cell allocation pattern (-buff_size:m+buff_size, idwint/idwbuff) - Add field variable indexing system (cont_idx, mom_idx, E_idx, adv_idx, sys_size) - Add test system details: programmatic generation, CaseGeneratorStack, UUID hashing - Add scalar_field uses stp (not wp) to precision types - Add analytical IC available variables (x, y, z, xc, yc, zc, lx, ly, lz, r, e, t) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.claude/rules/fortran-conventions.md (1)
8-15: Module Structure section is missing two enforced requirements.The section covers
implicit noneand explicitintentbut omits:
private/publicdeclarations — every new.fppmodule must include explicitprivate/publicdeclarations for all entities.docs/module_categories.jsonregistration — new modules must be registered there; the precheck linter will fail if missing.✏️ Proposed additions
## Module Structure Every Fortran module follows this pattern: - File: `m_<feature>.fpp` - Module: `module m_<feature>` - `implicit none` required +- Explicit `private` by default; explicitly list all `public` symbols +- Register module in `docs/module_categories.json` (precheck linter enforces this) - Explicit `intent(in)`, `intent(out)`, or `intent(inout)` on ALL subroutine/function arguments - Initialization subroutine: `s_initialize_<feature>_module` - Finalization subroutine: `s_finalize_<feature>_module`Based on learnings: every new
.fppmodule must include explicitprivate/publicdeclarations for all entities, and new modules must be registered indocs/module_categories.json(precheck linter will fail if missing).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/rules/fortran-conventions.md around lines 8 - 15, Update the "Module Structure" section to require (1) explicit private/public declarations for every entity in new modules (files following the m_<feature>.fpp pattern and modules named module m_<feature>) and (2) registration of the new module in docs/module_categories.json so the precheck linter passes; mention that these rules apply to all modules including their initialization/finalization subroutines s_initialize_<feature>_module and s_finalize_<feature>_module and that missing either will cause the precheck to fail.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.claude/rules/fortran-conventions.md:
- Around line 8-15: Update the "Module Structure" section to require (1)
explicit private/public declarations for every entity in new modules (files
following the m_<feature>.fpp pattern and modules named module m_<feature>) and
(2) registration of the new module in docs/module_categories.json so the
precheck linter passes; mention that these rules apply to all modules including
their initialization/finalization subroutines s_initialize_<feature>_module and
s_finalize_<feature>_module and that missing either will cause the precheck to
fail.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.claude/rules/common-pitfalls.md.claude/rules/fortran-conventions.md.claude/rules/gpu-and-mpi.md.claude/rules/parameter-system.mdCLAUDE.md
🚧 Files skipped from review as they are similar to previous changes (2)
- .claude/rules/common-pitfalls.md
- .claude/rules/gpu-and-mpi.md
Claude Code Review (updated)
6 files changed:
Summary
Findings1. Misleading GPU_PARALLEL + GPU_LOOP block macro exampleFile: .claude/rules/gpu-and-mpi.md lines 44–53 The documented example shows a pattern with GPU_PARALLEL wrapping GPU_LOOP, but this pattern does not exist anywhere in the simulation codebase. All 331+ parallel spatial loops use GPU_PARALLEL_LOOP/END_GPU_PARALLEL_LOOP inline macros. GPU_PARALLEL is used only 3 times, all wrapping scalar reductions (minval/maxval), never wrapping a loop. More critically, this example would produce silent serial execution on Cray and AMD compilers via the OpenMP backend. OMP_LOOP (the underlying OpenMP implementation of GPU_LOOP) emits an empty string for Cray (CCE) and AMD (LLVMFlang) compilers, leaving the loop with no scheduling directive inside a target teams region. Suggested fix: Replace the block macro usage example with the actual GPU_PARALLEL_LOOP pattern that is used throughout the codebase. 2. Compiler-backend matrix has incorrect entries for Intel ifx and GNU gfortranFile: .claude/rules/gpu-and-mpi.md lines 65–74 The matrix marks both Intel ifx and GNU gfortran as No for --gpu mp (OpenMP target offload). However, CMakeLists.txt has explicit compiler-specific handling for both:
The corrected matrix should mark Intel ifx as Yes and GNU gfortran as Yes (experimental, AMD GCN only) for --gpu mp. 3. AMD flang inconsistency across documentation filesAMD flang is treated inconsistently across the docs:
The last two points in common-pitfalls.md are self-contradictory: AMD flang is simultaneously not in the required compiler list and required for GPU builds. An AI assistant or developer following the all four compilers portability requirement would never flag AMD flang regressions. Suggested fix: Add a note distinguishing CI-gated compilers (the four) from additionally supported GPU compilers (AMD flang for --gpu mp), or explicitly add AMD flang to the compiler list everywhere it appears. |
1 participant
User description
Summary
CLAUDE.md(168 lines) with project identity, CLI commands, development workflow contract, system identification/module loading, architecture overview, critical rules, and code review priorities.claude/rules/files for domain-specific knowledge:fortran-conventions.md— naming, precision system (wp/stp), forbidden patterns, module structuregpu-and-mpi.md— GPU macro abstraction (GPU_*→ OpenACC/OpenMP target offload), preprocessor defines, compiler-backend matrix, MPI halo exchange patternsparameter-system.md— Python→Fortran parameter flow, 3-location checklist for new parameterscommon-pitfalls.md— array bounds, blast radius warnings, compiler portability, PR checklistCore CLAUDE.md stays under the ~200-line threshold for optimal instruction adherence. Rules files load automatically as part of Claude Code's memory system.
Designed for two audiences: Claude Code interactive development sessions and automated PR reviews.
Test plan
CLAUDE.mdloads correctly: runclaudein repo root and check/memory.claude/rules/files load alongside core: check/memoryoutput./mfc.shcommands rather than raw CMake/Python🤖 Generated with Claude Code
Summary by CodeRabbit
CodeAnt-AI Description
Add developer guide and rule set for contributors and automated reviewers
What Changed
Impact
✅ Shorter onboarding for new contributors✅ Fewer build/test mistakes from missing parameter or target updates✅ Clearer GPU/MPI and precision guidance reducing platform-specific errors💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.