From df15fcd91ad53c0e55dcd5dff7f079fd9b5eae04 Mon Sep 17 00:00:00 2001 From: Yung-Yu Chen Date: Wed, 13 May 2026 21:46:04 +0800 Subject: [PATCH] Replace slash commands with make targets, convert reviewers to skills - Inline the canonical `make build/test/lint` invocations into `CLAUDE.md` and delete `.claude/commands/{build,pytest,gtest,lint,format}.md`. - Move `.claude/agents/{cpp,python}-style-review.md` to `.claude/skills//SKILL.md` so the rules apply in the caller's context. - Mark `make format` / `make pyformat` as work-in-progress in `CLAUDE.md` and the Makefile, and drop them from `.claude/settings.json` allowlist. - Update `CLAUDE.md` and `contrib/prompt/general-rule.md` to reference skills (not subagents) and the surviving make targets. Related to #773 and #774. --- .claude/commands/build.md | 18 --- .claude/commands/format.md | 21 ---- .claude/commands/gtest.md | 22 ---- .claude/commands/lint.md | 19 --- .claude/commands/pytest.md | 23 ---- .claude/settings.json | 2 - .../cpp-style-review/SKILL.md} | 31 +++-- .../python-style-review/SKILL.md} | 30 +++-- CLAUDE.md | 117 ++++++++++-------- Makefile | 4 + contrib/prompt/general-rule.md | 9 +- 11 files changed, 107 insertions(+), 189 deletions(-) delete mode 100644 .claude/commands/build.md delete mode 100644 .claude/commands/format.md delete mode 100644 .claude/commands/gtest.md delete mode 100644 .claude/commands/lint.md delete mode 100644 .claude/commands/pytest.md rename .claude/{agents/cpp-style-reviewer.md => skills/cpp-style-review/SKILL.md} (69%) rename .claude/{agents/python-style-reviewer.md => skills/python-style-review/SKILL.md} (58%) diff --git a/.claude/commands/build.md b/.claude/commands/build.md deleted file mode 100644 index 3b304d1b4..000000000 --- a/.claude/commands/build.md +++ /dev/null @@ -1,18 +0,0 @@ ---- -description: Build modmesh (defaults to the Python extension) -argument-hint: [optional make args, e.g. VERBOSE=1 or target name] -allowed-tools: Bash(make:*) ---- - -Build modmesh by running `make $ARGUMENTS` from the repo root. If -`$ARGUMENTS` is empty, run `make` (default target builds the -`_modmesh` Python extension into `modmesh/`). - -Report: -- On success, a one-line summary; surface any non-fatal warnings. -- On failure, list the first few errors as - `path:line -- error -- one-line diagnosis`. Do not paste full - logs. - -End with: `verdict: clean | issues found | blocking`. `blocking` -iff the build failed. diff --git a/.claude/commands/format.md b/.claude/commands/format.md deleted file mode 100644 index 2c93c0e9d..000000000 --- a/.claude/commands/format.md +++ /dev/null @@ -1,21 +0,0 @@ ---- -description: Auto-format C++ (clang-format -i) and Python (black) -argument-hint: [optional: pyformat, or leave empty for both] -allowed-tools: Bash(make format:*), Bash(make pyformat:*), Bash(make cformat:*), Bash(git diff:*), Bash(git status:*) ---- - -Auto-format modmesh sources. - -- If `$ARGUMENTS` is empty, run `make format` (formats both C++ - and Python). -- If `$ARGUMENTS` is `pyformat`, run `make pyformat` (Python only). -- If `$ARGUMENTS` is `cformat`, note that `make cformat` only - checks formatting; running `make format` is the correct way to - apply C++ fixes. - -After formatting, run `git status --short` and list which files -were modified. If nothing changed, say "already formatted". - -End with: `verdict: clean | issues found | blocking`. `clean` iff -`make format` succeeded (whether or not files changed). `blocking` -iff the formatter tooling is missing or errored. diff --git a/.claude/commands/gtest.md b/.claude/commands/gtest.md deleted file mode 100644 index 6844c03a0..000000000 --- a/.claude/commands/gtest.md +++ /dev/null @@ -1,22 +0,0 @@ ---- -description: Build and run C++ tests (googletest) -argument-hint: [optional --gtest_filter=Suite.Test or other gtest flags] -allowed-tools: Bash(make gtest:*), Bash(./build/*/gtests/run_gtest:*), Bash(find:*), Bash(ls:*) ---- - -Run modmesh's C++ tests. - -- If `$ARGUMENTS` is empty, run `make gtest` (builds and runs the - full suite). -- If `$ARGUMENTS` contains gtest flags (e.g. `--gtest_filter=...`), - locate the gtest binary under `build/*/gtests/run_gtest` (build - first with `make gtest` if it's missing) and invoke it with - `$ARGUMENTS`. - -Report: -- Run / fail / time on one line. -- For failures, list `Suite.Test -- first assertion failure with - file:line`. No full output. - -End with: `verdict: clean | issues found | blocking`. `blocking` -iff the binary failed to build. diff --git a/.claude/commands/lint.md b/.claude/commands/lint.md deleted file mode 100644 index 130276a00..000000000 --- a/.claude/commands/lint.md +++ /dev/null @@ -1,19 +0,0 @@ ---- -description: Run every lint check (cformat, cinclude, flake8, checkascii, checktws) -argument-hint: [optional individual target, e.g. flake8, cformat] -allowed-tools: Bash(make lint:*), Bash(make cformat:*), Bash(make cinclude:*), Bash(make flake8:*), Bash(make checkascii:*), Bash(make checktws:*) ---- - -Run modmesh's lint checks. - -- If `$ARGUMENTS` is empty, run `make lint` (runs all of cformat, - cinclude, flake8, checkascii, checktws). -- If `$ARGUMENTS` names a single target (`cformat`, `cinclude`, - `flake8`, `checkascii`, `checktws`), run `make $ARGUMENTS`. - -Report each failing check as `target -- path:line -- rule`. Do not -suggest fixes for `cformat` violations -- they should be applied -via `/format` (or `make format`) instead. - -End with: `verdict: clean | issues found | blocking`. `blocking` -iff any check that gates CI failed (all five do). diff --git a/.claude/commands/pytest.md b/.claude/commands/pytest.md deleted file mode 100644 index 00d24a837..000000000 --- a/.claude/commands/pytest.md +++ /dev/null @@ -1,23 +0,0 @@ ---- -description: Run Python tests (pytest) -argument-hint: [optional pytest args or test file/function paths] -allowed-tools: Bash(make pytest:*), Bash(make run_pilot_pytest:*), Bash(find:*), Bash(ls:*) ---- - -Run the modmesh Python test suite. - -- If `$ARGUMENTS` is empty, run `make pytest` (full suite; builds - the extension first if needed). -- If `$ARGUMENTS` contains pytest args or a path, forward them via - `make pytest PYTEST_OPTS='$ARGUMENTS'`. This is the project's - documented entry point (see comments in `Makefile`) and preserves - the `PYTHONPATH=$(MODMESH_ROOT)` env that bare `pytest` would - miss on macOS (SIP strips `DYLD_LIBRARY_PATH`). - -Report: -- Pass / fail / time on one line. -- For failures, list `test_id -- first failure line` (no full - tracebacks). If the failure is in a fixture or setup, say so. - -End with: `verdict: clean | issues found | blocking`. `blocking` -iff the test run could not start (build failure, import error). diff --git a/.claude/settings.json b/.claude/settings.json index 9bbe08670..e1393aa86 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -6,8 +6,6 @@ "Bash(make gtest:*)", "Bash(make pilot:*)", "Bash(make lint:*)", - "Bash(make format:*)", - "Bash(make pyformat:*)", "Bash(make cformat:*)", "Bash(make flake8:*)", "Bash(make cinclude:*)", diff --git a/.claude/agents/cpp-style-reviewer.md b/.claude/skills/cpp-style-review/SKILL.md similarity index 69% rename from .claude/agents/cpp-style-reviewer.md rename to .claude/skills/cpp-style-review/SKILL.md index c0f827127..36e5e42ae 100644 --- a/.claude/agents/cpp-style-reviewer.md +++ b/.claude/skills/cpp-style-review/SKILL.md @@ -1,13 +1,14 @@ --- -name: cpp-style-reviewer -description: Judgment-call C++ review for modmesh (m_ prefix, function-body placement, SimpleCollector preference, pybind11 binding split, const_cast). Use proactively after editing files in cpp/. -tools: Read, Grep, Glob, Bash -model: sonnet +name: cpp-style-review +description: Apply modmesh's judgment-call C++ style rules (m_ prefix, function-body placement, SimpleCollector preference, pybind11 binding split, const_cast) to changed lines in cpp/ or gtests/. Use after editing C++ sources. +tools: Read, Grep, Glob, Edit, Bash --- -You are a C++ code reviewer for modmesh. Authoritative reference is `STYLE.md` -at the repo root; `CLAUDE.md` is a summary. If they disagree, follow `STYLE.md` -and flag the drift in your verdict. +# C++ Style Review (modmesh) + +Authoritative reference is `STYLE.md` at the repo root; `CLAUDE.md` is a +summary. If they disagree, follow `STYLE.md` and flag the drift in the +verdict. ## Scope @@ -20,7 +21,7 @@ include-style, line length) are handled by `.claude/hooks/check-source.sh` (PostToolUse). Do not duplicate them. If the hook somehow missed one, mention it briefly but don't re-implement the check here. -## Judgment-call rules you check +## Judgment-call rules **Naming** - Classes / structs: `CamelCase`. @@ -59,11 +60,13 @@ it briefly but don't re-implement the check here. `cpp/**/*.{cpp,hpp,c,h}` and `gtests/**/*.cpp`. 2. For each file, read only the diff hunks (use `git diff` output). 3. Apply the rules above to changed lines. -4. Output each finding as `path:line -- rule -- one-line fix suggestion`. +4. Output each finding as `path:line -- rule -- (fix applied | suggestion): + `. 5. End with a single verdict line: `verdict: clean | issues found | blocking`. + Use `clean` only when no findings remain after any hand-fixes. `blocking` is reserved for things `make lint` would reject (which the hooks -already cover). Findings from this agent are typically `issues found`. +already cover). Findings from this skill are typically `issues found`. ## Output @@ -71,7 +74,11 @@ already cover). Findings from this agent are typically `issues found`. - Don't paste long code excerpts; point to `file:line`. - Be explicit when uncertain ("not sure whether X is intentional -- please confirm"). -- Suggest `/format` (or `make format`) for pure formatting nits; do not - hand-fix yourself. +- For clang-format violations, don't hand-fix -- suggest + `make FORCE_CLANG_FORMAT=inplace cformat` to auto-fix. For `cinclude` + findings (include ordering, angle brackets) and other non-auto-fixable + nits, try to hand-fix. + +Do not run `make pyformat` or `make format`. They are still work in progress. diff --git a/.claude/agents/python-style-reviewer.md b/.claude/skills/python-style-review/SKILL.md similarity index 58% rename from .claude/agents/python-style-reviewer.md rename to .claude/skills/python-style-review/SKILL.md index 2f810bb3b..7b888d45b 100644 --- a/.claude/agents/python-style-reviewer.md +++ b/.claude/skills/python-style-review/SKILL.md @@ -1,13 +1,14 @@ --- -name: python-style-reviewer -description: Judgment-call Python review for modmesh (naming, test intent, project conventions). Use proactively after editing files in modmesh/ or tests/. -tools: Read, Grep, Glob, Bash -model: sonnet +name: python-style-review +description: Apply modmesh's judgment-call Python style rules (naming, project conventions, test intent) to changed lines in modmesh/ or tests/. Use after editing Python sources. +tools: Read, Grep, Glob, Edit, Bash --- -You are a Python code reviewer for modmesh. Authoritative reference is -`STYLE.md` at the repo root; `CLAUDE.md` is a summary. If they disagree, -follow `STYLE.md` and flag the drift in your verdict. +# Python Style Review (modmesh) + +Authoritative reference is `STYLE.md` at the repo root; `CLAUDE.md` is a +summary. If they disagree, follow `STYLE.md` and flag the drift in the +verdict. ## Scope @@ -17,9 +18,9 @@ unchanged lines -- out of scope per Rule 3 (surgical changes). Deterministic checks (ASCII, trailing whitespace, modeline, 79-char limit, flake8) are handled by `.claude/hooks/check-source.sh` -(PostToolUse) and `make flake8`. Do not duplicate them. +(PostToolUse) and `make flake8`. Do not duplicate them. -## Judgment-call rules you check +## Judgment-call rules **Naming** - Classes: `CamelCase`. @@ -42,16 +43,19 @@ limit, flake8) are handled by `.claude/hooks/check-source.sh` 2. Read diff hunks only. 3. Apply rules to changed lines. 4. Output each finding as - `path:line -- rule -- one-line fix suggestion`. + `path:line -- rule -- (fix applied | suggestion): `. 5. End with a single verdict line: - `verdict: clean | issues found | blocking`. + `verdict: clean | issues found | blocking`. Use `clean` only when no + findings remain after any hand-fixes. ## Output - Bullets only. - Don't paste long code excerpts; point to `file:line`. - Be explicit when uncertain. -- Suggest `/format` (or `make pyformat`) for pure formatting nits; do - not hand-fix yourself. +- Try to hand-fix formatting nits. `make flake8` verifies but does not fix. + +Do not run `make pyformat`, which is not set up to conform to the existing +Python coding style yet. diff --git a/CLAUDE.md b/CLAUDE.md index fab3a5ab6..c313f59cd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -17,33 +17,24 @@ unstructured meshes. The codebase emphasizes: ## Claude Code Tooling -This repository ships a `.claude/` directory with permissions, hooks, slash -commands, and subagents tuned to this codebase. General behavioral rules live -in `contrib/prompt/general-rule.md` (not auto-imported). This section indexes +This repository ships a `.claude/` directory with permissions, hooks, and +skills tuned to this codebase. General behavioral rules live in +`contrib/prompt/general-rule.md` (not auto-imported). This section indexes the tools. -### Slash commands (`.claude/commands/`) +### Skills (`.claude/skills/`) -| Command | Wraps | Notes | -| ----------------- | ------------------------------ | -------------------------------------------------- | -| `/build [args]` | `make $ARGUMENTS` | Builds the Python extension by default. | -| `/pytest [args]` | `make pytest PYTEST_OPTS=...` | Forwards args via `PYTEST_OPTS`; default is full suite. | -| `/gtest [args]` | `make gtest` or `run_gtest` | Forwards `--gtest_filter=...` to the built binary. | -| `/lint [target]` | `make lint` | Or a single sub-target (`flake8`, `cinclude`, ...). | -| `/format [pyformat]` | `make format` | `pyformat` runs Python-only. | - -All commands end with `verdict: clean | issues found | blocking`. - -### Subagents (`.claude/agents/`) - -- `cpp-style-reviewer` -- judgment-call C++ review (`m_` prefix, function-body +- `cpp-style-review` -- judgment-call C++ review (`m_` prefix, function-body placement, `SimpleCollector` preference, pybind11 binding split, - `const_cast`). Scoped to `git diff`. Pinned `model: sonnet`. -- `python-style-reviewer` -- judgment-call Python review (naming, test intent, - project conventions). Scoped to `git diff`. Pinned `model: sonnet`. + `const_cast`). Scoped to `git diff`. Invoke after editing files in `cpp/` + or `gtests/`. +- `python-style-review` -- judgment-call Python review (naming, test intent, + project conventions). Scoped to `git diff`. Invoke after editing files in + `modmesh/` or `tests/`. -Deterministic style checks (ASCII, trailing whitespace, modeline, 79-char -Python lines) are owned by hooks, not subagents. +Skills inherit the caller's model rather than pinning their own. Deterministic +style checks (ASCII, trailing whitespace, modeline, 79-char Python lines) are +owned by hooks, not skills. ### Hooks (`.claude/hooks/`) @@ -62,19 +53,50 @@ Python lines) are owned by hooks, not subagents. - `statusLine` runs `.claude/statusline.sh` -- shows model, project, branch (with `*` if dirty), and context-window usage. -## Build Commands +## Build, Test, Lint, Format + +All workflows are driven through `make` from the repo root. The Makefile sets +`PYTHONPATH=$(MODMESH_ROOT)` so the in-tree `_modmesh` extension is picked up +without installation, and works around macOS SIP stripping `DYLD_LIBRARY_PATH`. + +**Build** + +- `make` -- build the `_modmesh` Python extension (default target). +- `make pilot` -- build the Qt pilot GUI binary. +- `make clean` / `make cmakeclean` -- remove build artifacts. + +**Test** + +- `make pytest` -- full Python test suite. +- `make pytest PYTEST_OPTS="tests/test_buffer.py::SimpleArrayBasicTC::test_sort"` + -- run a single test or subset; `PYTEST_OPTS` is forwarded verbatim to + pytest. +- `make run_pilot_pytest` -- Python tests that require the pilot GUI; + accepts `PYTEST_OPTS` the same way. +- `make gtest` -- build and run the full C++ test suite. +- `./build/rel/gtests/run_gtest --gtest_filter=Suite.Test` -- run a + single gtest after `make gtest` has built the binary + (`` is the Python major+minor, e.g. `314`). +- `make pyprof` -- run profiling benchmarks; results land in + `profiling/results/`. -Prefer the slash commands listed above (`/build`, `/pytest`, `/gtest`, -`/lint`, `/format`). The underlying `make` targets are: `make` (Python -extension; default), `make pytest`, `make gtest`, `make pilot`, -`make run_pilot_pytest`, `make pyprof`, `make lint` (or individual: -`cinclude`, `flake8`, `checkascii`, `checktws`, `cformat`), `make pyformat`, -`make format` (in-place C++ + Python), `make clean`, `make cmakeclean`. +**Lint** (`make lint` runs all five) -Any target whose tool (`clang-format`, `flake8`, `black`) is missing prints an -install hint and exits 1. `make cformat` also warns when the local -`clang-format` major version differs from the CI pin (`CLANG_FORMAT_CI_VERSION` -in the Makefile). +- `make cformat` -- check C++ formatting (read-only; use + `make FORCE_CLANG_FORMAT=inplace cformat` to fix). +- `make cinclude` -- check `#include` ordering and style. +- `make flake8` -- Python style and 79-char line limit. +- `make checkascii` -- reject non-ASCII bytes in source. +- `make checktws` -- reject trailing whitespace. + +**Format** + +Automatic formatting is still work in progress. Do not run `make format` or +`make pyformat`. + +Any target whose tool (`clang-format`, `flake8`) is missing prints an install +hint and exits 1. `make cformat` also warns when the local `clang-format` major +version differs from the CI pin (`CLANG_FORMAT_CI_VERSION` in the Makefile). ### Build Configuration @@ -140,17 +162,12 @@ Python interface in `modmesh/`: ### Testing Structure -- **Python tests** (`tests/`): pytest-based - - Named `test_*.py` - - Run with `make pytest` - -- **C++ tests** (`gtests/`): googletest-based - - Named `test_nopython_*.cpp` - - Run with `make gtest` +- **Python tests** (`tests/`): pytest-based, files named `test_*.py`. +- **C++ tests** (`gtests/`): googletest-based, files named + `test_nopython_*.cpp`. +- **Profiling benchmarks** (`profiling/`): files named `profile_*.py`. -- **Profiling benchmarks** (`profiling/`): performance tests - - Named `profile_*.py` - - Run with `make pyprof`; outputs to `profiling/results/` +See "Build, Test, Lint, Format" above for the `make` invocations. ## Code Style @@ -166,8 +183,8 @@ How style is enforced in this repo: - `.claude/hooks/check-source.sh` owns the deterministic checks (ASCII bytes, trailing whitespace, modeline at EOF, Python `>79`-char lines). -- The `cpp-style-reviewer` and `python-style-reviewer` subagents in - `.claude/agents/` own the judgment-call rules (`m_` prefix in context, +- The `cpp-style-review` and `python-style-review` skills in + `.claude/skills/` own the judgment-call rules (`m_` prefix in context, function-body placement, container choice, pybind11 binding split, test intent). They are scoped to `git diff`. @@ -182,16 +199,6 @@ management. ## Development Workflow -### Running Single Tests - -Prefer the slash commands (`/pytest `, -`/gtest --gtest_filter=Suite.Test`). Direct invocations: - -```bash -make pytest PYTEST_OPTS="tests/test_buffer.py::SimpleArrayBasicTC::test_sort" -./build/rel/gtests/run_gtest --gtest_filter=Suite.Test -``` - ### Build System Notes - CMake is the primary build system (minimum version 3.27) diff --git a/Makefile b/Makefile index 6b6d2e9e6..a5c0a4876 100644 --- a/Makefile +++ b/Makefile @@ -234,6 +234,8 @@ checktws: .PHONY: lint lint: cformat cinclude flake8 checkascii checktws +# FIXME: Do not run pyformat. It is not yet configured to conform to the style +# of the Python code in the code base. It is still work in progress. .PHONY: pyformat pyformat: @command -v $(BLACK) >/dev/null 2>&1 || { \ @@ -243,6 +245,8 @@ pyformat: } $(BLACK) $(BLACK_OPTS) . +# FIXME: Do not run format, which depends on pyformat, which is still work in +# progress. .PHONY: format format: pyformat @$(MAKE) FORCE_CLANG_FORMAT=inplace cformat diff --git a/contrib/prompt/general-rule.md b/contrib/prompt/general-rule.md index 8781deb11..bfe4956a1 100644 --- a/contrib/prompt/general-rule.md +++ b/contrib/prompt/general-rule.md @@ -26,7 +26,7 @@ success and iterate. Strong success criteria let you loop independently. Use Claude for: classification, drafting, summarization, extraction. Do NOT use Claude for: routing, retries, deterministic transforms. If code can answer, code answers. In this repo, hooks under `.claude/hooks/` own the deterministic -checks (ASCII, trailing whitespace, modeline, line length); subagents own the +checks (ASCII, trailing whitespace, modeline, line length); skills own the judgment calls. ## Rule 6 -- Token budgets surfaced, not hidden @@ -52,9 +52,10 @@ can't fail when business logic changes is wrong. ## Rule 10 -- Checkpoint after every significant step Summarize what was done, what's verified, what's left. Don't continue from a -state you can't describe back. If you lose track, stop and restate. The slash -commands (`/build`, `/pytest`, `/gtest`, `/lint`, `/format`) end with an -explicit verdict line; treat each as a checkpoint. +state you can't describe back. If you lose track, stop and restate. After +running `make` targets (`build`, `pytest`, `gtest`, `lint`), state an +explicit verdict (clean / issues found / blocking) and treat it as a +checkpoint. ## Rule 11 -- Match the codebase's conventions, even if you disagree Conformance > taste inside the codebase. If you genuinely think a convention is