2.5: Review Agent Instructions
Role Definition
Review Agent (SUP.2)
Primary Responsibility: Perform automated code reviews, verify MISRA compliance, check traceability, and report defects
ASPICE Process: SUP.2 Verification
Success Metrics:
- MISRA Detection: 90-95% (find violations automatically)
- Style Verification: 100% (automated checks for naming, indentation)
- Traceability Verification: 95-100% (check @implements tags)
Input Work Products
Required Inputs (Must Exist Before Starting)
1. Pull Request (Code Changes)
- Content: Source code diff, commit message, PR description
- Quality Criteria: Code compiles, unit tests pass (CI/CD pipeline green)
2. Coding Standards
- MISRA C:2012: Rules checklist
- Project Style Guide: Naming conventions, formatting rules
3. Requirements and Architecture
- Content: SRS, SAD (for traceability verification)
- Purpose: Verify that code links to requirements (@implements tags)
Execution Steps
Step-by-Step Workflow
Step 1: Run Automated MISRA Compliance Check
Action: Use static analysis tool (cppcheck, PC-lint, Coverity) to detect violations
Automated Check (cppcheck with MISRA addon):
#!/bin/bash
# Run cppcheck with MISRA C:2012 addon
cppcheck --addon=misra.json \
--xml \
--enable=all \
--suppress=misra-config \
src/*.c include/*.h 2> misra_violations.xml
# Parse XML output
python3 parse_misra_violations.py misra_violations.xml > misra_report.md
# Example output:
# ┌──────────────────────────────────────────────────────────────┐
# │ MISRA C:2012 Compliance Report │
# ├──────────────────────────────────────────────────────────────┤
# │ Required Rules: 2 violations [ERROR] │
# │ Advisory Rules: 5 violations [WARN] │
# │ Total: 7 violations │
# └──────────────────────────────────────────────────────────────┘
#
# Required Rule Violations (MUST FIX):
# 1. Rule 10.1 (Line 45, acc_controller.c): Operands shall not be of inappropriate essential type
# → uint16_t compared with signed int (use explicit cast)
#
# 2. Rule 21.3 (Line 128, sensor_fusion.c): malloc/free shall not be used
# → Dynamic memory allocation prohibited in safety-critical code (ASIL-B)
#
# Advisory Rule Violations (SHOULD FIX):
# 3. Rule 2.1 (Line 89, diagnostics.c): Unreachable code
# → Dead code after 'return' statement
#
# 4. Rule 8.13 (Line 156, can_driver.c): Pointer parameter could be const
# → Add 'const' qualifier for read-only parameter
#
# 5. Rule 15.5 (Line 203, acc_controller.c): Function has multiple exit points
# → Consider single return (less critical, coding style preference)
Review Agent Action:
- Required Violations → [ERROR] Block pull request, escalate to author
- Advisory Violations → [WARN] Allow merge but flag for future fix
- No Violations → [PASS] Approve MISRA compliance
Step 2: Verify Coding Style
Action: Check naming conventions, indentation, comment quality
Automated Style Checks:
1. Naming Conventions:
import re
def check_naming_conventions(file_path):
"""
Verify project naming conventions:
- Functions: snake_case (e.g., ACC_GetDistance)
- Variables: snake_case (e.g., distance_m)
- Constants: UPPERCASE (e.g., MAX_SPEED)
- Types: PascalCase_t (e.g., CAN_Message_t)
"""
violations = []
with open(file_path, 'r') as f:
lines = f.readlines()
for i, line in enumerate(lines):
# Check function definitions
func_match = re.search(r'^(\w+)\s+(\w+)\s*\(', line)
if func_match:
func_name = func_match.group(2)
if not re.match(r'^[A-Z][a-zA-Z0-9_]*$', func_name):
violations.append(f"Line {i+1}: Function '{func_name}' should use snake_case (e.g., ACC_GetDistance)")
# Check #define constants
define_match = re.search(r'#define\s+(\w+)', line)
if define_match:
const_name = define_match.group(1)
if not re.match(r'^[A-Z][A-Z0-9_]*$', const_name):
violations.append(f"Line {i+1}: Constant '{const_name}' should be UPPERCASE (e.g., MAX_SPEED)")
return violations
# Example
violations = check_naming_conventions("src/acc_controller.c")
if violations:
print("[ERROR] Naming Convention Violations:")
for v in violations:
print(f" - {v}")
else:
print("[PASS] Naming conventions compliant")
2. Indentation and Formatting (use clang-format):
# Check if code follows project .clang-format style
clang-format --dry-run --Werror src/*.c include/*.h
# If violations found, auto-fix (with human approval):
clang-format -i src/*.c include/*.h
3. Comment Quality:
def check_doxygen_completeness(file_path):
"""
Verify all functions have Doxygen headers with required fields
"""
missing_doxygen = []
# Parse C file, find function definitions
functions = extract_functions(file_path)
for func in functions:
if not has_doxygen_header(func):
missing_doxygen.append(f"Function '{func.name}' missing Doxygen header")
else:
# Check required Doxygen fields
doxygen = get_doxygen_header(func)
if '@brief' not in doxygen:
missing_doxygen.append(f"Function '{func.name}' missing @brief")
if '@param' not in doxygen and func.has_parameters():
missing_doxygen.append(f"Function '{func.name}' missing @param")
if '@return' not in doxygen:
missing_doxygen.append(f"Function '{func.name}' missing @return")
if '@implements' not in doxygen:
missing_doxygen.append(f"Function '{func.name}' missing @implements tag")
return missing_doxygen
Step 3: Verify Traceability
Action: Check that all code has @implements tags linking to requirements
Traceability Verification:
def verify_traceability(code_files, requirements_db):
"""
Verify bidirectional traceability:
1. All functions have @implements tags → Code → Requirements
2. All requirements have implementing code → Requirements → Code
"""
issues = []
# Check 1: Code → Requirements (forward traceability)
for file in code_files:
functions = extract_functions_with_implements_tags(file)
for func in functions:
if not func.implements_tag:
issues.append(f"[ERROR] {file}:{func.line} - Function '{func.name}' missing @implements tag")
else:
req_id = func.implements_tag
if req_id not in requirements_db:
issues.append(f"[ERROR] {file}:{func.line} - Orphan requirement '{req_id}' (not in SRS)")
# Check 2: Requirements → Code (backward traceability)
for req in requirements_db:
implementing_functions = find_functions_implementing(req.id, code_files)
if not implementing_functions:
issues.append(f"[WARN] Requirement '{req.id}' has no implementing code (not verified)")
return issues
# Example usage
requirements = load_requirements("docs/SRS.md")
code_files = ["src/acc_controller.c", "src/can_driver.c"]
traceability_issues = verify_traceability(code_files, requirements)
if not traceability_issues:
print("[PASS] Traceability: 100% (all code linked to requirements)")
else:
print(f"[ERROR] Traceability issues: {len(traceability_issues)}")
for issue in traceability_issues:
print(f" - {issue}")
Example Output:
[ERROR] Traceability issues: 3
- [ERROR] src/acc_controller.c:145 - Function 'ACC_Helper_CalculateAverage' missing @implements tag
- [ERROR] src/sensor_fusion.c:78 - Orphan requirement 'SWE-999' (not in SRS)
- [WARN] Requirement 'SWE-089' has no implementing code (not verified)
Step 4: Review Code Complexity
Action: Measure cyclomatic complexity (McCabe), flag overly complex functions
Complexity Analysis (using radon or lizard):
# Analyze cyclomatic complexity
radon cc src/*.c --min B
# Example output:
# src/acc_controller.c
# F 45:0 ACC_GetObstacleDistance - A (2) # Simple (complexity = 2)
# F 78:0 ACC_GetClosingSpeed - A (3) # Simple (complexity = 3)
# F 120:0 ACC_ControlLoop_10ms - C (12) # Complex (complexity = 12) [WARN]
#
# src/sensor_fusion.c
# F 34:0 SensorFusion_Kalman - E (25) # Very complex (complexity = 25) [ERROR]
Complexity Thresholds:
- A (1-5): Simple, no action needed [PASS]
- B (6-10): Moderate, acceptable [PASS]
- C (11-20): Complex, consider refactoring [WARN]
- D (21-50): Very complex, should refactor [ERROR]
- F (>50): Extremely complex, must refactor [ERROR]
Review Agent Action:
- Complexity > 20 → Flag for human review, suggest refactoring
Step 5: Generate Review Report
Action: Compile all findings into human-readable review report
Review Report Template:
# Code Review Report (SUP.2)
## Pull Request
- **PR ID**: #142
- **Title**: Implement ACC controller (SWE.3)
- **Author**: Implementation Agent
- **Files Changed**: 5 files (+285 lines, -12 lines)
- **Review Date**: 2025-12-17
- **Reviewer**: AI Review Agent
## Automated Checks
### 1. MISRA C:2012 Compliance
| Category | Violations | Status |
|----------|------------|--------|
| **Required Rules** | 2 violations | [ERROR] FAIL (must fix) |
| **Advisory Rules** | 5 violations | [WARN] WARNING |
**Required Violations (BLOCKING)**:
1. **Rule 10.1** (Line 45, acc_controller.c): Inappropriate type conversion
- Issue: `uint16_t` compared with `signed int` without cast
- Fix: Add explicit cast: `(int16_t)radar_data > threshold`
2. **Rule 21.3** (Line 128, sensor_fusion.c): Dynamic memory allocation
- Issue: `malloc()` used in ASIL-B code (prohibited)
- Fix: Replace with static buffer or stack allocation
**Verdict**: [ERROR] **BLOCK MERGE** (Required rule violations must be fixed)
---
### 2. Coding Style
| Check | Result | Status |
|-------|--------|--------|
| Naming Conventions | 3 violations | [WARN] WARNING |
| Indentation | 100% compliant | [PASS] PASS |
| Doxygen Completeness | 95% (1 missing @brief) | [WARN] WARNING |
**Style Issues**:
1. Line 156 (can_driver.c): Function `CAN_ReadMsg` should be `CAN_ReadMessage` (avoid abbreviations)
2. Line 203 (diagnostics.c): Variable `tmp` should be `temporary_buffer` (descriptive name)
3. Line 78 (acc_controller.c): Missing @brief in Doxygen header
**Verdict**: [WARN] **ALLOW MERGE** (style issues non-blocking, create follow-up ticket)
---
### 3. Traceability
- **Code → Requirements**: 98% (34/35 functions have @implements tags)
- **Requirements → Code**: 100% (all requirements have implementing code)
**Traceability Issues**:
1. Function `ACC_Helper_CalculateAverage` (line 145) missing @implements tag
- Likely a helper function (not directly linked to requirement)
- Recommendation: Add comment explaining purpose or link to parent requirement
**Verdict**: [PASS] **ACCEPTABLE** (minor gap, helper function)
---
### 4. Code Complexity
| Function | Complexity | Grade | Recommendation |
|----------|------------|-------|----------------|
| ACC_GetObstacleDistance | 2 | A | [PASS] Simple |
| ACC_GetClosingSpeed | 3 | A | [PASS] Simple |
| ACC_ControlLoop_10ms | 12 | C | [WARN] Consider refactoring |
| SensorFusion_Kalman | 25 | E | [ERROR] Should refactor |
**Recommendation**:
- `SensorFusion_Kalman` (complexity 25): Extract sub-functions for prediction/correction steps
- `ACC_ControlLoop_10ms` (complexity 12): Acceptable for control loop, but monitor if grows
**Verdict**: [WARN] **ACCEPTABLE** (flag for future refactoring)
---
## Summary
### Overall Verdict
[ERROR] **REQUEST CHANGES** (Required MISRA violations must be fixed)
### Required Actions (BLOCKING)
1. [ERROR] Fix MISRA Required Rule 10.1 (Line 45, acc_controller.c)
2. [ERROR] Fix MISRA Required Rule 21.3 (Line 128, sensor_fusion.c) - Remove `malloc()`
### Recommended Actions (NON-BLOCKING)
1. [WARN] Fix naming convention violations (3 issues)
2. [WARN] Add missing Doxygen @brief tag (line 78)
3. [WARN] Consider refactoring `SensorFusion_Kalman` (complexity 25)
4. [WARN] Add @implements tag to `ACC_Helper_CalculateAverage`
### Approval Conditions
- [ ] Fix all Required MISRA violations (2 issues)
- [ ] Re-run static analysis (cppcheck)
- [ ] Resubmit for review
**Estimated Fix Time**: 30 minutes
**Re-review Required**: Yes (after fixes)
---
## Human Reviewer Notes
[TBD - Human reviewer should add notes here after manual review]
**Signed**: _________________________
**Date**: _________________________
Output Work Products
What Review Agent Must Generate
1. MISRA Compliance Report
- Format: Markdown + XML (cppcheck output)
- Content: Violations list, severity, recommended fixes
2. Code Style Report
- Format: Markdown
- Content: Naming violations, indentation issues, Doxygen gaps
3. Traceability Report
- Format: Markdown
- Content: Missing @implements tags, orphan requirements, coverage percentage
4. Complexity Report
- Format: Markdown
- Content: Cyclomatic complexity per function, refactoring recommendations
5. Overall Review Report (see template above)
- Format: Markdown (posted as PR comment)
- Content: Summary of all checks, verdict (approve/request changes), action items
6. GitHub/GitLab PR Comment
## [AI] Automated Code Review (AI Review Agent)
### Verdict: [ERROR] **REQUEST CHANGES**
**Blocking Issues** (must fix before merge):
- [ERROR] MISRA Required Rule 10.1 violation (Line 45, acc_controller.c)
- [ERROR] MISRA Required Rule 21.3 violation (Line 128, sensor_fusion.c)
**Non-Blocking Issues** (fix in follow-up PR):
- [WARN] 3 naming convention violations
- [WARN] 1 missing Doxygen tag
- [WARN] 1 high-complexity function (consider refactoring)
**Details**: See full review report in artifacts (review_report.md)
**Action Required**: Fix blocking issues, re-run `cppcheck`, resubmit for review.
---
*Generated by AI Review Agent | ASPICE SUP.2 Verification*
Quality Criteria
Acceptance Criteria for Review Agent Output
Review Agent Quality Checklist:
──────────────────────────────────────────────────────
☐ MISRA Compliance
☐ All Required rule violations detected (100% recall)
☐ Advisory rule violations flagged
☐ Severity correctly classified (Required vs Advisory)
☐ Coding Style
☐ Naming conventions checked (100% coverage)
☐ Indentation verified (clang-format)
☐ Doxygen completeness checked (@brief, @param, @return)
☐ Traceability
☐ Missing @implements tags detected (95% recall)
☐ Orphan requirements detected (links to non-existent requirements)
☐ Requirements without code flagged
☐ Complexity
☐ Cyclomatic complexity measured (all functions)
☐ High-complexity functions flagged (>20)
☐ Reporting
☐ Review report complete (verdict, action items, estimates)
☐ PR comment posted (summary for human reviewer)
☐ Artifact links provided (MISRA XML, coverage HTML)
Verdict:
[PASS] PASS: Review report accurate and complete
[ERROR] FAIL: False positives/negatives, re-run analysis
Escalation Triggers
When Review Agent Must Escalate
1. Critical Safety Violation [ESCALATION]
- Trigger: Code violates safety requirements (e.g., buffer overflow in ASIL-B function)
- Action: Block merge immediately, escalate to safety engineer
- Example:
[ESCALATION] ESCALATION: Critical Safety Violation File: acc_controller.c, Line 156 Issue: Buffer overflow possible (array bounds not checked) Function: ACC_ControlLoop_10ms (ASIL-B safety-critical) ISO 26262 Requirement: All array accesses must be bounds-checked (Part 6, Clause 9.4.2) Action: BLOCK MERGE, human review required before fixing Assignee: @safety_engineer
2. False Positive Detection [ESCALATION]
- Trigger: AI detects issue, but human disagrees (tool limitation)
- Action: Escalate to human reviewer for manual verification
- Example:
[ESCALATION] ESCALATION: Potential False Positive Issue: MISRA Rule 10.1 violation (Line 45) AI Analysis: Unsigned/signed comparison without cast Human Verification Needed: Code may be correct (range guaranteed by caller) Recommendation: Human reviewer should verify or suppress with justification Assignee: @senior_engineer
3. Novel Pattern Not in Training Data [ESCALATION]
- Trigger: Code uses pattern AI doesn't recognize (e.g., custom macro, domain-specific idiom)
- Action: Escalate for human review
- Example:
[ESCALATION] ESCALATION: Unfamiliar Code Pattern File: autosar_rte.c, Line 234 Pattern: AUTOSAR RTE macro `Rte_Call_R_SensorData_Read(&data)` AI Analysis: Cannot verify correctness (AUTOSAR runtime environment outside training) Recommendation: Human AUTOSAR expert should review Assignee: @autosar_specialist
Examples
Complete Review Agent Workflow
Input: Pull Request #142 (5 files, 285 LOC)
Output 1: MISRA Report
- Required violations: 2 (blocking)
- Advisory violations: 5 (non-blocking)
Output 2: Style Report
- Naming violations: 3
- Doxygen gaps: 1
- Indentation: 100% compliant [PASS]
Output 3: Traceability Report
- Code → Requirements: 98% (34/35)
- Requirements → Code: 100% (12/12)
Output 4: Complexity Report
- High complexity (>20): 1 function (
SensorFusion_Kalman) - Average complexity: 6.2 (acceptable)
Output 5: Review Report
- Verdict: [ERROR] Request Changes (2 Required MISRA violations)
- Action items: 2 blocking, 4 non-blocking
- Estimated fix time: 30 minutes
Output 6: PR Comment
- Posted to GitHub/GitLab as automated comment
- Author notified: "Fix blocking issues, resubmit"
Human Review Time: 30 minutes (baseline: 90 minutes manual) → 67% time savings
Summary
Review Agent Key Responsibilities:
- MISRA Compliance: Detect violations (90-95% accuracy) and classify severity (Required vs. Advisory)
- Coding Style: Verify naming, indentation, and Doxygen completeness (100% automated)
- Traceability: Check @implements tags and detect orphan requirements (95-100% accuracy)
- Complexity Analysis: Measure cyclomatic complexity and flag refactoring candidates (100% coverage)
- Generate Review Report: Summary with verdict, action items, and estimates
Escalation: Critical safety violations, false positives, novel patterns outside training data
Success Metrics: 90-95% MISRA detection, 100% style verification, 95-100% traceability checks
Chapter 30 Complete: Process Execution Instructions provides step-by-step agent guidance for Requirements (SWE.1), Architecture (SWE.2), Implementation (SWE.3), Verification (SWE.4), and Review (SUP.2).
Next: Chapter 31 - Workflow Instructions (Git, Pull Requests, Testing, Releases)