Files
claude-howto/03-skills/code-review/templates/finding-template.md
Luong NGUYEN 6d1e0ae4af refactor(ci): shift quality checks to pre-commit, CI as 2nd pass (#34)
* refactor(ci): shift quality checks to pre-commit, CI as 2nd pass

- Remove ci.yml (lint, security, pytest were only for EPUB scripts)
- Move EPUB build to pre-commit local hook (runs on .md changes)
- Add check_cross_references.py, check_mermaid.py, check_links.py scripts
- Add markdown-lint, cross-references, mermaid-syntax, link-check as
  pre-commit hooks — mirrors all 4 CI doc-check jobs locally
- Remove spell check job from docs-check.yml (breaks on translations)
- Refactor docs-check.yml to reuse scripts/ instead of inline Python
- Add .markdownlint.json config shared by pre-commit and CI
- Update CONTRIBUTING.md with required dependencies and hook table

* fix(ci): resolve all CI check failures in docs-check workflow

- fix(check_cross_references): skip code blocks and inline code spans
  to avoid false positives from documentation examples; fix emoji
  heading anchor generation (rstrip not strip); add blog-posts,
  openspec, prompts, .agents to IGNORE_DIRS; ignore README.backup.md
- fix(check_links): strip trailing Markdown punctuation from captured
  URLs; add wikipedia, api.github.com to SKIP_DOMAINS; add placeholder
  URL patterns to SKIP_URL_PATTERNS; add .agents/.claude to IGNORE_DIRS
- fix(check_mermaid): add --no-sandbox puppeteer config support via
  MERMAID_PUPPETEER_NO_SANDBOX env var for GitHub Actions Linux runners
- fix(docs-check.yml): pass MERMAID_PUPPETEER_NO_SANDBOX=true to mermaid job
- fix(content): repair broken anchors in README.md, 09-advanced-features;
  fix #plugins -> #claude-code-plugins in claude_concepts_guide.md;
  remove non-existent ./docs/performance.md placeholder links; fix
  dependabot alerts URL in SECURITY_REPORTING.md; update auto-mode URL
  in resources.md; use placeholder pattern for 07-plugins example URL
- remove README.backup.md (stale file)

* fix(check-scripts): fix strip_code_blocks regex and URL fragment handling

- fix regex in strip_code_blocks to avoid conflicting MULTILINE+DOTALL
  flags that could fail to strip indented code fences; use DOTALL only
- strip URL fragments (#section) before dispatching link checks to avoid
  false-positive 404s on valid URLs with anchor fragments

* fix(check-scripts): fix anchor stripping, cross-ref enforcement, and mermaid temp file cleanup

- heading_to_anchor: use .strip("-") instead of .rstrip("-") to also strip leading hyphens
  produced by emoji-prefixed headings, preventing false-positive anchor errors
- check_cross_references: always exit with main()'s return code — filesystem checks
  should block pre-commit unconditionally, not silently pass on errors
- check_mermaid: wrap file-processing loop in try/finally so the puppeteer config
  temp file is cleaned up even if an unexpected exception (e.g. UnicodeDecodeError) occurs
- docs-check.yml: remove now-unused CROSS_REF_STRICT env var

* fix(scripts): fix anchor stripping and mermaid output path

- Replace .strip('-') with .rstrip('-') in heading_to_anchor() so leading
  hyphens from emoji-prefixed headings are preserved, matching GitHub's
  anchor generation behaviour.
- Use Path.with_suffix('.svg') in check_mermaid.py instead of
  str.replace('.mmd', '.svg') to avoid replacing all occurrences of .mmd
  in the full temp path.
2026-04-02 02:20:45 +02:00

2.4 KiB

Code Review Finding Template

Use this template when documenting each issue found during code review.


Issue: [TITLE]

Severity

  • Critical (blocks deployment)
  • High (should fix before merge)
  • Medium (should fix soon)
  • Low (nice to have)

Category

  • Security
  • Performance
  • Code Quality
  • Maintainability
  • Testing
  • Design Pattern
  • Documentation

Location

File: src/components/UserCard.tsx

Lines: 45-52

Function/Method: renderUserDetails()

Issue Description

What: Describe what the issue is.

Why it matters: Explain the impact and why this needs to be fixed.

Current behavior: Show the problematic code or behavior.

Expected behavior: Describe what should happen instead.

Code Example

Current (Problematic)

// Shows the N+1 query problem
const users = fetchUsers();
users.forEach(user => {
  const posts = fetchUserPosts(user.id); // Query per user!
  renderUserPosts(posts);
});

Suggested Fix

// Optimized with JOIN query
const usersWithPosts = fetchUsersWithPosts();
usersWithPosts.forEach(({ user, posts }) => {
  renderUserPosts(posts);
});

Impact Analysis

Aspect Impact Severity
Performance 100+ queries for 20 users High
User Experience Slow page load High
Scalability Breaks at scale Critical
Maintainability Hard to debug Medium
  • Similar issue in AdminUserList.tsx line 120
  • Related PR: #456
  • Related issue: #789

Additional Resources

Reviewer Notes

  • This is a common pattern in this codebase
  • Consider adding this to the code style guide
  • Might be worth creating a helper function

Author Response (for feedback)

To be filled by the code author:

  • Fix implemented in commit: abc123
  • Fix status: Complete / In Progress / Needs Discussion
  • Questions or concerns: (describe)

Finding Statistics (for Reviewer)

When reviewing multiple findings, track:

  • Total Issues Found: X
  • Critical: X
  • High: X
  • Medium: X
  • Low: X

Recommendation: Approve / ⚠️ Request Changes / 🔄 Needs Discussion

Overall Code Quality: 1-5 stars