Skip to content

Commit ed0b734

Browse files
committed
enhanced wamr AI review pipeline
1 parent db6acc4 commit ed0b734

11 files changed

Lines changed: 641 additions & 413 deletions

.claude/agents/tests-fix.md

Lines changed: 95 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,17 @@ model_name: main
4848

4949
---
5050

51+
## Phase 0.75: Static Analysis Fix
52+
53+
| Line | Category | Issue | Action | Result |
54+
|------|----------|-------|--------|--------|
55+
| 42 | bugprone-narrowing-conversions | `uint32` to `int32` | Changed type to `int32` ||
56+
| 156 | readability-convert-member-functions-to-static | method can be static | Added `static` keyword ||
57+
58+
**Summary**: N issues fixed
59+
60+
---
61+
5162
## Phase 1: Fix Alignment Issues
5263

5364
### Test: <TEST_CASE_NAME>
@@ -79,6 +90,7 @@ model_name: main
7990
| Category | Count |
8091
|----------|-------|
8192
| Quality Fixes | N |
93+
| Static Analysis Fixes | N |
8294
| Alignment Fixes | N |
8395
| New Tests Added | N |
8496
| Tests Skipped | N |
@@ -110,8 +122,8 @@ model_name: main
110122
1. Test case reviews with `Alignment: YES/NO` status
111123
2. `Recommendations` section for tests with `Alignment: NO`
112124
3. `Enhancement Recommendations` with suggested new test cases
113-
4. `Redundancy Cleanup` section (redundant tests already identified)
114-
5. `Quality Screening` section (diagnostic issues per test case)
125+
4. `Quality Screening` section (diagnostic issues per test case)
126+
5. `Static Analysis` section (clang-tidy warnings/errors)
115127

116128
**OPTIONAL INPUT (for RE-FIX mode)**:
117129
- Previous `*_fix.md` file (context of what was already attempted)
@@ -128,21 +140,11 @@ If `*_verify.md` is not provided, locate it automatically in the same directory
128140
│ PHASE 0: INITIALIZATION │
129141
│ - Parse INPUT FILE → extract test file path │
130142
│ - cd ~/zhenwei/wasm-micro-runtime/tests/unit │
131-
│ - Run: ./get_coverage.sh <TEST_FILE_PATH>
132-
│ - Record INITIAL_COVERAGE in output document
143+
│ - rm -rf build
144+
│ - cmake -S . -B build -DCOLLECT_CODE_COVERAGE=1 2>&1 | tail -10
133145
│ - cmake --build build/smart-tests/<MODULE_NAME> 2>&1 | tail -15 │
134-
└─────────────────────────────────────────────────────────────────────┘
135-
136-
137-
┌─────────────────────────────────────────────────────────────────────┐
138-
│ PHASE 0.25: REDUNDANCY CLEANUP (enforced by coverage gate) │
139-
│ - Read redundant test cases from review.md "Redundant Test Cases" │
140-
│ - Extract test case names from the table │
141-
│ - Delete redundant tests in bulk: │
142-
│ python3 delete_test_cases.py <TEST_FILE_PATH> <test1> <test2>... │
143-
│ - Rebuild module │
144-
│ - Verify coverage NOT dropped vs INITIAL (if dropped → REVERT) │
145-
│ - Record deletions as part of Phase 0.5 table (Action=Deleted) │
146+
│ - Run: ./get_coverage.sh <TEST_FILE_PATH> │
147+
│ - Record INITIAL_COVERAGE in output document │
146148
└─────────────────────────────────────────────────────────────────────┘
147149
148150
@@ -159,6 +161,16 @@ If `*_verify.md` is not provided, locate it automatically in the same directory
159161
160162
161163
┌─────────────────────────────────────────────────────────────────────┐
164+
│ PHASE 0.75: STATIC ANALYSIS FIX (from review clang-tidy results) │
165+
│ - Read "Static Analysis" section from review.md │
166+
│ - For each clang-tidy warning/error: │
167+
│ - Apply fix based on category (type conversion, static, etc.) │
168+
│ - Rebuild to verify compilation (do NOT re-run clang-tidy) │
169+
│ - Record all fixes in output document │
170+
└─────────────────────────────────────────────────────────────────────┘
171+
172+
173+
┌─────────────────────────────────────────────────────────────────────┐
162174
│ PHASE 1: FIX ALIGNMENT ISSUES (for Alignment: NO tests) │
163175
│ - For each test with "Alignment: NO" in review: │
164176
│ - Apply recommended fix (rename/modify assertion/add setup) │
@@ -230,64 +242,21 @@ All commands execute from: `~/zhenwei/wasm-micro-runtime/tests/unit`
230242
```bash
231243
cd ~/zhenwei/wasm-micro-runtime/tests/unit
232244

233-
# Get initial coverage (RECORD THIS!)
234-
./get_coverage.sh <TEST_FILE_PATH>
245+
# Clean and configure build
246+
rm -rf build
247+
cmake -S . -B build -DCOLLECT_CODE_COVERAGE=1 2>&1 | tail -10
235248

236-
# Build if needed
249+
# Build module FIRST (required before get_coverage.sh)
237250
cmake --build build/smart-tests/<MODULE_NAME> 2>&1 | tail -15
251+
252+
# Get initial coverage (RECORD THIS!)
253+
./get_coverage.sh <TEST_FILE_PATH>
238254
```
239255

240256
**Create output file** following the EXACT format in "CRITICAL: OUTPUT FORMAT" above.
241257

242258
---
243259

244-
### PHASE 0.25: Redundancy Cleanup (enforced by coverage gate)
245-
246-
**Goal**: Remove truly redundant tests (0 incremental coverage) while guaranteeing overall coverage does not regress.
247-
248-
**Input**: Redundant test cases are already identified in the review.md file under "Redundant Test Cases" section.
249-
250-
**Workflow**:
251-
252-
1. **Read redundant test cases from review.md**:
253-
- Locate the "Redundant Test Cases" table in the review.md file
254-
- Extract test case names from the first column (format: `SuiteName.TestName` or full test case name)
255-
- Example table format:
256-
```markdown
257-
| Test Case | Reason |
258-
|-----------|--------|
259-
| `F32ConstTest.BoundaryValues_PreservesLimits` | No incremental coverage contribution |
260-
| `aot_resolve_import_func_SubModuleLoadFails_LogWarning` | No incremental coverage contribution |
261-
```
262-
263-
2. **Delete redundant tests in bulk**:
264-
```bash
265-
# Extract test case names and pass as arguments
266-
python3 delete_test_cases.py <TEST_FILE_PATH> <test_case1> <test_case2> <test_case3> ...
267-
268-
# Example:
269-
python3 delete_test_cases.py smart-tests/constants/enhanced_f32_const_test.cc \
270-
F32ConstTest.BoundaryValues_PreservesLimits \
271-
F32ConstTest.SubnormalValues_PreservesAccuracy \
272-
F32ConstTest.SpecialValues_PreservesIEEE754
273-
```
274-
275-
3. **Rebuild after deletion**:
276-
```bash
277-
cmake --build build/smart-tests/<MODULE_NAME> 2>&1 | tail -15
278-
```
279-
280-
4. **Coverage gate: MUST NOT drop vs INITIAL**:
281-
```bash
282-
./get_coverage.sh <TEST_FILE_PATH>
283-
```
284-
285-
**Accept/Reject**:
286-
- If coverage **drops** vs INITIAL → **REVERT** the file to pre-cleanup state and record as ❌ FAILED (coverage regression)
287-
- If coverage **maintained or improved** → keep changes and record deletions in Phase 0.5 table (Action=Deleted)
288-
289-
**Note**: The agent must parse the "Redundant Test Cases" table from review.md and extract test case names, then pass them as arguments to `delete_test_cases.py`. The script accepts test case names in either `SuiteName.TestName` format or full test case name format.
290-
291260
### PHASE 0.5: Quality Fix (from review + safety scan)
292261

293262
**Goal**: Apply quality fixes based on review findings, plus a quick safety scan to catch missed issues.
@@ -304,6 +273,40 @@ cmake --build build/smart-tests/<MODULE_NAME> 2>&1 | tail -15
304273

305274
---
306275

276+
### PHASE 0.75: Static Analysis Fix (from review clang-tidy results)
277+
278+
**Goal**: Fix clang-tidy warnings/errors identified in the review report's "Static Analysis" section.
279+
280+
**Input**: Read the "Static Analysis" section from `*_review.md`, which contains a table of clang-tidy findings.
281+
282+
**Workflow**:
283+
284+
1. **Parse static analysis findings from review.md**:
285+
- Locate the "Static Analysis" or "clang-tidy Results" section
286+
- Extract each warning/error with: Line, Category, Message
287+
288+
2. **Apply fixes based on category**:
289+
| Category | Common Fix |
290+
|----------|------------|
291+
| `bugprone-narrowing-conversions` | Change variable type or add explicit cast |
292+
| `readability-convert-member-functions-to-static` | Add `static` keyword to method |
293+
| `readability-implicit-bool-conversion` | Add explicit `!= nullptr` or `!= 0` |
294+
| `misc-non-private-member-variables-in-classes` | Change to private or add accessor |
295+
| `modernize-use-trailing-return-type` | Convert to `auto func() -> ReturnType` |
296+
297+
3. **Rebuild after fixes**:
298+
```bash
299+
cmake --build build/smart-tests/<MODULE_NAME> 2>&1 | tail -10
300+
```
301+
302+
**Record all fixes in output document's Phase 0.75 table.**
303+
304+
**Note**:
305+
- If a clang-tidy warning is suppressed in project's `.clang-tidy` config, it can be skipped.
306+
- Do NOT re-run clang-tidy for verification (current build uses gcc, not clang toolchain). Verification is done by verify agent.
307+
308+
---
309+
307310
### RE-FIX Mode (Closed-Loop Iteration)
308311

309312
When invoked for RE-FIX (typically because Compliance < 100% in `*_verify.md`):
@@ -338,7 +341,8 @@ For each test with `Alignment: NO` in review:
338341
- **⚠️ MANDATORY**: Check exit code and output - test must execute successfully with NO failures
339342
- **Note**: `<TEST_CASE_NAME>` is the specific test case name (e.g., `F32ConstTest.BasicConstants_ReturnsCorrectValues`), not the class name
340343
- If ctest fails: document specific error and mark as ❌ FAILED (revert changes)
341-
5. **Verify coverage**: `python3 is_test_case_useful.py <TEST_FILE> <TEST_CASE>`
344+
5. **Verify coverage**: `python3 is_test_case_useful.py <TEST_FILE_PATH> <SuiteName.TestName>`
345+
- **Note**: `<SuiteName.TestName>` format required (e.g., `F32ConstTest.BasicConstants_ReturnsCorrectValues`)
342346
6. **Accept/Reject**:
343347
- Coverage maintained/improved AND ctest passes → ✅ FIXED
344348
- Coverage dropped (per-test OR overall gate) → ❌ FAILED (revert changes)
@@ -373,22 +377,37 @@ You MUST attempt to generate EVERY suggested test case from review. NEVER skip e
373377
1. **Generate** test code following discovered patterns
374378
2. **Append** to test file
375379
3. **Rebuild**: `cmake --build build/smart-tests/<MODULE_NAME> 2>&1 | tail -10`
380+
4. If build fails: **delete the test immediately** using `python3 delete_test_cases.py`, document error and mark as ⏭️ SKIPPED
376381

377382
**Step 4: Verify ctest execution**
378383
- Run: `ctest --test-dir build/smart-tests/<MODULE_NAME> -R "<TEST_CASE_NAME>" --output-on-failure`
379384
- **⚠️ MANDATORY**: Check exit code and output - test must execute successfully with NO failures
380385
- **Note**: `<TEST_CASE_NAME>` is the specific new test case name (e.g., `F32ConstTest.NewTestName`), not the class name
381-
- If ctest fails: document specific error and mark as ⏭️ SKIPPED (do not proceed to Step 5)
386+
- If ctest fails: **delete the test immediately** using `python3 delete_test_cases.py`, document specific error and mark as ⏭️ SKIPPED
382387

383388
**Step 5: Verify coverage contribution**
384-
- Run: `python3 is_test_case_useful.py <TEST_FILE> <NEW_TEST_CASE>`
389+
- Run: `python3 is_test_case_useful.py <TEST_FILE_PATH> <SuiteName.TestName>`
390+
- **Note**: Use `SuiteName.TestName` format (e.g., `F32ConstTest.NewTestName`)
385391
- **⚠️ MANDATORY**: Test must contribute to coverage (new lines > 0)
386392

387393
**Step 6: Accept/Reject Decision**
388394
- Coverage improved (new lines > 0) AND ctest passes (no failures) AND overall gate not dropped → ✅ ADDED
389-
- No coverage contribution after implementation → ⏭️ SKIPPED (delete test case)
390-
- ctest fails (test execution errors) → ⏭️ SKIPPED (document specific ctest error from Step 4)
391-
- Build fails with technical blocker → ⏭️ SKIPPED (document specific build error)
395+
- No coverage contribution after implementation → ⏭️ SKIPPED (**MUST delete test case immediately**)
396+
- ctest fails (test execution errors) → ⏭️ SKIPPED (**MUST delete test case immediately**)
397+
- Build fails with technical blocker → ⏭️ SKIPPED (**MUST delete test case immediately**)
398+
399+
**⚠️ CRITICAL: Deletion of SKIPPED Tests**
400+
401+
When a new test case is marked as SKIPPED, you **MUST immediately delete it** before proceeding to the next test. Use:
402+
403+
```bash
404+
python3 delete_test_cases.py <TEST_FILE_PATH> <SuiteName.TestName>
405+
```
406+
407+
**Why this is mandatory:**
408+
1. Leaving SKIPPED tests in the file causes compilation errors (duplicate definitions)
409+
2. If fix agent runs multiple times, duplicate tests accumulate
410+
3. SKIPPED means "rejected" - the test code should not remain in the file
392411

393412
**Step 7: Documentation**
394413
Record each new test in output document's Phase 2 table with:
@@ -486,11 +505,11 @@ Record each new test in output document's Phase 2 table with:
486505
# Working directory
487506
cd ~/zhenwei/wasm-micro-runtime/tests/unit
488507

489-
# Get coverage
490-
./get_coverage.sh <TEST_FILE>
508+
# Get coverage (TEST_FILE_PATH = relative path, e.g., smart-tests/constants/enhanced_i32_const_test.cc)
509+
./get_coverage.sh <TEST_FILE_PATH>
491510

492-
# Check if test case useful
493-
python3 is_test_case_useful.py <TEST_FILE> <TEST_CASE_NAME>
511+
# Check if test case useful (use SuiteName.TestName format)
512+
python3 is_test_case_useful.py <TEST_FILE_PATH> <SuiteName.TestName>
494513

495514
# Build module
496515
cmake --build build/smart-tests/<MODULE_NAME> 2>&1 | tail -15

.claude/agents/tests-pipeline.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,8 +64,8 @@ After EVERY step completion, you MUST:
6464

6565
| Sub-Agent | Purpose | Input | Output |
6666
|-----------|---------|-------|--------|
67-
| tests-review | Analyzes test coverage and quality | test `.cc` file path | `*_review.md` |
68-
| tests-fix | Applies review recommendations | `*_review.md` path | `*_fix.md` |
67+
| tests-review | Analyzes coverage, deletes redundant tests, runs clang-tidy | test `.cc` file path | `*_review.md` + modified `.cc` |
68+
| tests-fix | Applies review recommendations (quality, static analysis, alignment, new tests) | `*_review.md` path | `*_fix.md` |
6969
| tests-verify | Validates fix compliance | `*_fix.md` path | `*_verify.md` |
7070

7171
## Pipeline Flow

0 commit comments

Comments
 (0)