chore: consolidate agent instructions into .claude/CLAUDE.md - #1855
Conversation
Replace the root CLAUDE.md and .claude/rules/common-pitfalls.md with a single .claude/CLAUDE.md. Add AGENTS.md as a symlink to it so Codex loads the same file with no per-user configuration; a repo-level .codex/config.toml is not read by Codex. Written with assistance from Claude Code.
The review workflow read the now-removed root CLAUDE.md and cited its Code Review Priorities section; both reads were guarded with '|| true', so the review step would have run with empty standards rather than failing. lint_source.py pointed contributors at the removed .claude/rules/common-pitfalls.md; its docstring already carries the full explanation, so the pointer now names the compiler-bugs reproducer only. Written with assistance from Claude Code.
|
Claude Code Review Head SHA: 1e643af Files changed:
Findings:
|
The removed .claude/rules/common-pitfalls.md held traps whose failure modes are silent. Its content is redistributed to the docs page that owns each subject rather than to a new page: indexing/ghost-cell and parameter-plumbing traps into contributing.md's Common Pitfalls section, compiler and backend traps into a new Silent-Failure Traps section in gpuParallelization.md, and test-selection traps into testing.md. Material already covered in those pages (Riemann j/j+1 indexing, the add-a-parameter procedure and its still-manual list, the AMD case-opt bound pattern) was dropped rather than duplicated. The testing.md --only bullet described substring matching and is corrected to whole-element matching. Written with assistance from Claude Code.
Three checks in lint_docs.py fail on master. Two single-backtick spans contain a single quote, which Doxygen treats as ending the span, so they need double backticks. The testing.md cross-reference to the new pitfalls section needs a matching {#selection-and-execution-pitfalls} anchor on that header.
Written with assistance from Claude Code.
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Pull request overview
Consolidates AI/agent instruction sources by replacing the root CLAUDE.md and .claude/rules/common-pitfalls.md with a single .claude/CLAUDE.md, and wires tooling/docs to reference the new locations.
Changes:
- Remove legacy instruction files and add
.claude/CLAUDE.mdas the new instruction source. - Update workflow/tooling references to avoid dangling reads and stale citations.
- Relocate “silent failure / pitfalls” guidance into existing docs pages, and clarify
--onlysemantics in testing docs.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| toolchain/mfc/lint_source.py | Updates references in docstring and lint error message away from removed pitfalls doc. |
| docs/documentation/testing.md | Clarifies --only semantics and adds a “Selection and Execution Pitfalls” section. |
| docs/documentation/gpuParallelization.md | Adds “Silent-Failure Traps” section documenting measured GPU/compiler pitfalls. |
| docs/documentation/contributing.md | Adds indexing/parameter “pitfalls” content formerly in the removed common pitfalls doc. |
| CLAUDE.md | Removes the legacy root instruction file. |
| AGENTS.md | Adds Codex-discoverable entry intended to point to .claude/CLAUDE.md. |
| .github/workflows/claude-code-review.yml | Updates workflow to read .claude/CLAUDE.md and removes outdated citation reference. |
| .claude/rules/common-pitfalls.md | Removes the legacy common pitfalls doc (content moved into docs). |
| .claude/CLAUDE.md | Adds the consolidated instruction file. |
Suppressed comments (1)
AGENTS.md:1
- If the intent is for
AGENTS.mdto be a symlink (as described in the PR), it’s worth confirming the PR actually records it as a symlink in git (mode 120000) rather than a regular text file containing a path. If it lands as a plain file, Codex may not follow it as intended; if symlinks are not reliably supported in some contributor environments, a fallback (e.g., duplicating the small file content) may be more robust.
* New branches cannot be made on MFlowCode/MFC, they are made on forks
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| * made using AI tools like Claude Code and Codex should say so. | ||
| * are made from those MFC forks | ||
| * that change CFD result need verification PR is correct |
| * New code should follow the DRY principle and also make side-effect code DRY as well | ||
| * Comments should be as short as possible without sacrificing value | ||
| * GPU macros should follow existing the source's GPU macro principles and patterns | ||
| * Functions/subroutines/modules shorter is better while being correctness, fast, and separating concerns |
| is affected, and the loop counts when it sits in anything the routine calls. Copy the element | ||
| to a scalar before the call and receive results into a scalar. See | ||
| .claude/rules/common-pitfalls.md and sbryngelson/compiler-bugs cce/acc-routine-element-by-reference. | ||
| sbryngelson/compiler-bugs cce/acc-routine-element-by-reference. |
| e = _ELEMENT_ARG.match(arg) | ||
| if e and ":" not in arg and not _VALUE_CALL_NAMES.match(e.group(1)): | ||
| errors.append(f" {rel}:{line_no} `{arg}` into `{name}` (a device routine with a seq loop): pass a scalar, see common-pitfalls.md") | ||
| errors.append(f" {rel}:{line_no} `{arg}` into `{name}` (a device routine with a seq loop): pass a scalar, see sbryngelson/compiler-bugs cce/acc-routine-element-by-reference") |
Point the device-routine lint at documentation that exists. The error message now names the Silent-Failure Traps section of gpuParallelization.md, which is repo-local and survives changes to the external reproducer repo, and that section gains the {#silent-failure-traps} anchor Doxygen needs. The docstring keeps the full context: the docs section, MFC issue 1815, and a full URL to the compiler-bugs reproducer, whose path was verified to exist.
Also fix trailing whitespace and three grammar slips in .claude/CLAUDE.md.
Written with assistance from Claude Code.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1855 +/- ##
=======================================
Coverage 61.26% 61.26%
=======================================
Files 84 84
Lines 22330 22330
Branches 3265 3265
=======================================
Hits 13680 13680
Misses 6207 6207
Partials 2443 2443 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Summary
Consolidates the repository's agent instruction files into a single source of truth at
.claude/CLAUDE.md, and makes that same file load in Codex via anAGENTS.mdsymlink.CLAUDE.md(root, 147 lines) and.claude/rules/common-pitfalls.md(173 lines) are removed..claude/CLAUDE.md(15 lines) replaces both.AGENTS.mdis a symlink to.claude/CLAUDE.md(mode120000), so Codex picks up the same rules with no per-user configuration.Net: +16 / -320.
Why the symlink rather than a config file
Codex does not read a repository-level
.codex/config.toml. This was measured rather than assumed, in a scratch repo:.codex/config.tomlwithproject_doc_fallback_filenames = [".claude/CLAUDE.md"]-c project_doc_fallback_filenames=[...].codex/config.toml+--strict-configAGENTS.mdsymlinked to.claude/CLAUDE.md, no config at allThe fallback-filename mechanism itself works and accepts a path with a directory component; the repo-level config file is simply not a location Codex reads. The symlink needs no configuration from any contributor.
Claude Code loads
.claude/CLAUDE.mddirectly (it is a first-class instruction-file location) and does not readAGENTS.md, so there is no double-loading.Dangling references, fixed here
Two places pointed at the removed files. Both failed silently rather than erroring, so neither would have shown up in CI:
.github/workflows/claude-code-review.ymlrancat CLAUDE.mdand cited the "Code Review Priorities" section of the old root file. Both reads are guarded with2>/dev/null || true, so the review step would have proceeded with empty review standards. It now reads.claude/CLAUDE.md; the priority list is already inlined in the workflow, so the citation was dropped rather than repointed.toolchain/mfc/lint_source.pynamed.claude/rules/common-pitfalls.mdin a lint error message and in the check's docstring. The docstring already carries the full CCE explanation, so both now cite thesbryngelson/compiler-bugsreproducer only.The workflow's
.claude/rules/*.mdreads are left in place. That directory no longer exists, but all three call sites are already guarded no-ops and will pick up rule files again if any are added.Verified: the workflow YAML parses, and
check_device_routine_element_argsruns against the tree and reports 0 findings, unchanged.Where the pitfalls content went
.claude/rules/common-pitfalls.mddocumented traps whose failure modes are silent, so it is relocated rather than dropped. Each block went to the page that owns its subject instead of to a new page:buff_size,eqn_idxcontributing.md, "Array Bounds and Indexing"patch_ib, checker placement, analytic ICs, case-opt rebuildcontributing.md, new "Parameter Plumbing"GPU_LOOPserialization,ftn-7066, CCE element-by-reference,m_thermochemtraps,USING_AMDguardsgpuParallelization.md, new "Silent-Failure Traps"--onlysemantics,define_case_dreachability,--no-build, stale binaries, worktree hook,/tmptesting.md, new "Selection and Execution Pitfalls"Material already covered in those pages was dropped rather than duplicated: Riemann
j/j+1indexing, the add-a-parameter procedure and its still-manual list, and the AMD case-optimization bound pattern in theGPU_ROUTINEhelper section.One inconsistency surfaced and is fixed:
testing.mddescribed--onlyas matching traces that "contain a certain feature," but the filter matches whole trace elements and ANDs labels, which is why--only bubblessilently matches nothing.All three destination pages are already in the
DOCSlist thatlint_docs.pyscans, so the added file-path references are now checked.check_docs,check_cite_keys, andcheck_param_refsall report 0 errors against the tree.Note on tooling
The commit was made with assistance from Claude Code. The consolidation decision, the contents of
.claude/CLAUDE.md, and the removal of the previous instruction files are the author's; the Codex behaviour above was verified experimentally before the approach was chosen.