# Code Review

> Code quality checklist and review automation scripts for quark-engine PRs

- Skill: `ev-flow/code-review` (Agent Skill, multi-file: 9 files)
- Install (CLI): `npx skillmds@latest add ev-flow/code-review`
- Raw SKILL.md: https://api.skillmd.com/api/skills/ev-flow/code-review/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Coding & Dev Tools
- Author: ev-flow (https://skillmd.com/u/ev-flow)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/ev-flow/code-review

---


# Code Review

## Code Quality Checklist

Based on the project's review standard. Each item is tagged with a review role.

### 1. Functional Correctness
- [ ] [Staff Eng] All required features from the issue/task are implemented
- [ ] [Staff Eng] No uncaught exceptions or warnings during execution
- [ ] [Staff Eng] Exception handling catches and reports all error conditions
- [ ] [Staff Eng] No logic errors that pass CI but would fail in production (look for deep bugs that automated tests cannot catch)
- [ ] [Staff Eng] No completeness gaps — no missing expected behaviors or edge cases; actively flag any that are found

### 2. Code Design
- [ ] [Staff Eng] No duplicate functionality already present in the codebase (check `quark/utils/tools.py` and `BaseApkinfo` before adding helpers)
- [ ] [Staff Eng] Each function does exactly one thing
- [ ] [Lang] Function signatures have correct input/output types (use type annotations consistent with existing code)
- [ ] [Eng Manager] Architecture, data flow, and boundaries are explicitly defined — all hidden assumptions must be surfaced
- [ ] [Eng Manager] Edge cases and boundary conditions are clearly specified and enforced
- [ ] [Lang] Follows backend & API design patterns (REST, database, cache layer conventions — no ad-hoc assembly)
- [ ] [QA] Run SonarLint and resolve all detected issues

### 3. Code Style
- [ ] [Lang] Variables and classes use noun/noun-phrase names; functions start with a verb
- [ ] [Lang] Format with `black --line-length 79 quark/` before committing — **automate**: `bash .claude/skills/code-review/scripts/check_style.sh <files>`
- [ ] [Lang] Run `pylint` on **only the files changed in this PR** and resolve all reported issues in those files (do not fix pre-existing issues in untouched files — it creates noise and defocuses the PR) — **automate**: `bash .claude/skills/code-review/scripts/check_style.sh <files>`
- [ ] [Lang] Run spell-checker on code — **automate**: `bash .claude/skills/code-review/scripts/check_spelling.sh <files>`
- [ ] [Lang] No lines exceed 79 characters
- [ ] [Lang] All functions and variables have complete, correct type hints — **automate**: `bash .claude/skills/code-review/scripts/check_types.sh <files>`

### 4. Testing & Regression Prevention
- [ ] [QA] New/modified functions are covered by tests
- [ ] [QA] Tests validate both expected and unexpected inputs
- [ ] [QA] Every bug fix is accompanied by a regression test that would have caught it — the same bug must never recur
- [ ] [QA] Aim for 100% coverage on changed code; replace any yolo/vibe-coded patches with proper tests — **automate**: `bash .claude/skills/code-review/scripts/check_coverage.sh <module>`

### 5. Security
- [ ] [Lang] Run language-level security scan (e.g., `bandit` for Python) and confirm no vulnerabilities — **automate**: `bash .claude/skills/code-review/scripts/check_security.sh <files>`
- [ ] [CSO] Only raise a security warning when confidence is ≥ 8/10 and independently verified — zero false positives
- [ ] [CSO] Every identified vulnerability (OWASP Top 10 or STRIDE) includes a concrete exploit scenario explaining how an attacker would actually abuse it in a real environment

### 6. Debugging Standards
- [ ] [Debugger] No fix is applied without a systematic root-cause investigation: trace data flow and test hypotheses before touching code
- [ ] [Debugger] If three consecutive fix attempts fail, stop and escalate rather than continuing to patch blindly

### 7. PR Preparation
- [ ] [Staff Eng] Commit message follows conventional commit format and passes sentence-quality checks — **automate**: `bash .claude/skills/code-review/scripts/check_commit.sh`
- [ ] [Staff Eng] PR title and description reference the relevant issue
- [ ] [QA] Smoke test counts updated if behavior counts changed (see `@.claude/skills/workflows.md` → Updating Smoke Test Counts)
- [ ] [Lang] If any `quark.script` public API changed: update `docs/source/quark_script.rst` and compile docs locally to confirm no errors

## Review Automation Scripts

Bundled scripts in `.claude/skills/code-review/scripts/` automate the checklist items marked with **automate** above.

**Prerequisites:** `pip install black pylint mypy codespell bandit pytest-cov` (or use `uvx` for one-off runs).

### Review workflow

#### Phase 1 — Understand

1. Read the changed files and their surrounding context (callers, callees, related modules)
2. Understand the feature's intent from the linked issue, branch name, and commit messages. **Ask questions if unclear.**
3. Identify how the changes fit into the existing architecture (e.g., does a new method in `TableObject` affect all callers in `PyEval`?)
4. Check whether the change duplicates, conflicts with, or misuses existing patterns in the codebase

#### Phase 2 — Plan

5. Produce a **test plan blueprint** based on the Code Quality Checklist:
   - List every test item (automated scripts and manual checks), with a brief description of what each verifies
   - Present the blueprint to the user for review
   - **Wait for user confirmation before executing any tests**

#### Phase 3 — Automated checks

6. Run all automated checks: `bash .claude/skills/code-review/scripts/run_all.sh <changed-files-or-dir>`
7. Fix failures and re-run until all checks pass. If a failure is due to a missing dependency, install and retry up to **3 attempts**; after that, **skip** the check and record it for the final summary.

#### Phase 4 — Manual review

9. Review checklist items without an **automate** marker — these require manual judgment, informed by the context gathered in Phase 1

#### Phase 5 — Finalize

10. Mark the review as complete only when both automated and manual checks pass
11. In the final summary, include a **Skipped Tests** section listing any checks skipped due to unresolvable tooling errors, with error details

## Summary Format

Present the review result as a **single consolidated table**. Each row is one check (automated or manual) with its result and issue description.

**Only show issues.** Do not include a "What's Good" section. End by asking the user if they want any issues fixed.

See [examples/summary.md](examples/summary.md) for the expected output format.

