# Ns Reviewer

> (NS) Senior Tech Lead review: SOLID, clean code, performance, security, testability. `Approved` only at score 10; score 9 = `Rejected`. Use after code changes, before PRs, at implementation closure, or code/PR/issue review gate — even without naming this skill. GitLab `ISSUE_URL`: Issue review mode. Do NOT write code-review-report.md. Do NOT use for root-cause debugging (ns-investigator).

- Skill: `nextstage-brasil/ns-reviewer` (Agent Skill, multi-file: 4 files)
- Install (CLI): `npx skillmds@latest add nextstage-brasil/ns-reviewer`
- Raw SKILL.md: https://api.skillmd.com/api/skills/nextstage-brasil/ns-reviewer/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Security
- License: Apache-2.0
- Author: nextstage-brasil (https://skillmd.com/u/nextstage-brasil)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/nextstage-brasil/ns-reviewer

---


# Code Reviewer

Deep constructive review vs project rules + acceptance criteria.

## Caller contract (workflow callers)

Invoker: `ns-coder`, `ns-autonomous`, `ns-execution-gitlab-issue` (direct or harness **`reviewer-agent`**):

- Caller **must** run this skill — **MUST** dispatch `reviewer-agent` when available (`../../ns-harness/references/subagent-dispatch.md`); else read this `SKILL.md` in-session. No Cursor Task personas (`senior-tech-lead-reviewer`, `bugbot`, `security-review`) or improvised review.
- Gate rules: `references/review-gate-workflow.md`.
- Every response to workflow caller **must** end exact line: `Code Review: {Approved|Rejected|Blocked}`. Overall score in **Executive Summary**.

## Session boot

See `../../ns-harness/references/session-boot.md`. **Complete Session boot (blocking)** before any other step — cold start this reviewer run; mid-session skip only if steps 1–6 done and files unchanged. Never tool-Read `AGENTS.md`.

## Workflow

1. **Session boot** — Cold start `reviewer-agent` / this skill: Session boot steps 1–6 in `session-boot.md`. Already booted same agent run (1–6 done), files unchanged: no re-read. Still judge diff vs `AGENTS.md` (in context) + project rules.
2. `git diff` (`git status` if needed).
3. Focus modified files + surrounding context.
4. Start immediately — no permission ask.

### Ad-hoc diff mode (from `ns-coder`)

Invoker passes working-tree diff only (no `ISSUE_URL`, no version-closure path):

1. Review `git diff` on working tree.
2. Apply **Score gate** + severity below.
3. Last line to parent: `Code Review: {Approved|Rejected|Blocked}`

### Version closure

1. Apply **Score gate**; end chat with `Code Review: {Approved|Rejected|Blocked}`.
2. **Spec-conformance (when version `source/` or `spec-coverage.md` exists):** walk `spec-coverage.md` **row-by-row**; walk `ui-contract.md` when present; walk Layout SSoT **Quick visual checklist** when cited (`reference-sources.md` `role: ui-layout`, or tasks cite `*-visual.md`). Unmet checklist items = findings at same bar as missing `ui-contract` elements. Missing ledger evidence → **Rejected**.
3. **Do not** write `code-review-report.md` (or any persistent review report).
4. `Rejected` / `Blocked`: **minimal fix map** in response (`references/review-fix-map.template.md`) — data another agent needs. No human prose, no positive findings, no history.
5. `Approved`: Executive Summary + score + verdict line only (no fix map).

### Issue review mode

Invoker passes `ISSUE_URL` (or `project_id` + `issue_iid`):

1. Delegate issue context to `ns-execution-gitlab-issue` context flow or `gitlab-issue-context-agent` — no `read_issue` if synthesis block provided.
2. Diff `origin/<target>...origin/<source>` from synthesis — never wrong branch.
3. **Requirement proof gate:** every AC needs behavioral evidence; producer-only code without consumer = Critical.
4. **Verdict (exactly one):** `Approved` | `Rejected` | `Blocked` — **Score gate** below.
5. Post internal GitLab comment via `mcp-gitlab-usage` — first line: `Code Review | YYYY-MM-DD HH:MM (UTC) | Verdict: {Approved|Rejected|Blocked}`
6. Last line to parent: `Code Review: {Approved|Rejected|Blocked}`
7. `Rejected`/`Blocked`: GitLab comment = same minimal fix-map facts.

## Score gate (all modes)

Every review **must** include overall score **1–10**. **`Approved`** = zero Criticals **and** score **= 10** only. Callers close on `Approved` — `references/review-gate-workflow.md`. Score **9** = **`Rejected`** (Lift toward 10).

| Score | Meaning | Verdict impact |
|-------|---------|----------------|
| **10** | Ship as-is | **`Approved`** (only eligible score) |
| **9** | Near-bar leftover (redundancy / non-uniform) | **`Rejected`** — Lift |
| **≤8** | Below bar / score cap | **`Rejected`** |

**`Approved` only when all true:**

1. Zero Critical findings
2. Overall score **= 10**/10
3. Issue review mode: every AC PASS with behavioral evidence

**`Rejected` when:** any Critical, **or** score ≤ **9**, **or** (Issue mode) any AC fails behavioral proof.

### Scoring unit

Score **quality of touched module/file after diff**, not hunk-alone correctness. Minimal patch leaving/worsening SSoT/DRY/OCP in file **cannot** score 9–10.

### Score caps (lowest that fits)

| Condition in touched module | Max score |
|-----------------------------|-----------|
| New/changed behavior with config/lookup **split across 2+ places** (SSoT) | **7** |
| Same resolution block copied in **2+ functions** in diff scope (DRY) | **7** |
| Predictable extension requires editing **3+ points** in same file (weak OCP, e.g. provider) | **7** |
| New/changed **exported / public** symbol (module `return { }`, `module.exports`, `export`, `public`/`public static`, window/global attach) with **no caller outside defining module** (grep: other files + same-file call sites that are not the export list) | **7** |
| Diff correct, zero Critical, mediocre / inconsistent pattern in file | **7–8** |
| gate/role helper or domain constant swapped without cited project-rule clause | **6** |

Internal/private/nested helpers **do not** count. **Do not** cap on function count. Example: 6 exported IP helpers vs 2 caller ops (`validateOriginIpInput`, `originIpMatches`); private `originIpKey` = OK.

**9:** zero Critical **and** smells above absent or resolved in touched module; predictable extension = one SSoT.

**10:** same as 9 **plus** no obvious fallback/redundancy; uniform pattern across file.

### Anti-inflation

- **Forbidden:** “minimal diff / tests pass / AC ok ⇒ 10”
- **Required** in Executive Summary: one sentence justifying score vs rubric (e.g. “cap 7 — apiKey outside preset”)

### Smell severity (SSoT / DRY / weak OCP / orphan public)

Split SSoT, duplicated resolution, **or orphan public/exported surface** in touched module ≥ **Warning**. Prefer **Warning + score cap** over auto-Critical. **Critical** for bugs, security, AC failures. Score ≤ **9** forces `Rejected`.

## Review priorities

Within each section, order by severity:

1. Critical (must fix before merge)
2. Warning (should fix)
3. Suggestion (consider)

Diff touches `agent-api` (or LangGraph runtime paths): load `../ns-langgraph-agents/references/anti-patterns.md`. Placement, Prompt inject, Bind parity, Spec drift, colon wire names = Critical if violated.

### AI / agent diff criteria

Keep anti-patterns delegation above. Extra criteria when diff touches agent/LLM surfaces:

| Finding | Severity |
| ------- | -------- |
| Irreversible write reachable without a gate | **Critical** |
| Prompt body changed without version bump | **Critical** |
| `UPDATE` / `DELETE` against audit history | **Critical** |
| Equality assertion on model output (exact string) instead of accepted range / contract | **Warning** + score cap **7–8** |
| New LLM call with no token/cost accounting | **Warning** (no extra score cap unless combined with other smells) |

### SOLID and clean code

- SRP, OCP, LSP, ISP, DIP
- Naming, cyclomatic complexity, error handling, dead code

### Performance, security, testability

- N+1, unnecessary renders, missing validation
- SQL injection, XSS, sensitive logs
- Coupling that blocks testing

### Actor and gate semantics (rules-first)

Before proposing change to session gate, middleware, or role check:

- Read project rules for **named actor semantics**. Homonyms not interchangeable.
- Forbidden to swap helper by label similarity. **Critical** when rules define distinct actors.
- Diff introducing actor/gate spec never named in rules: **Warning**.
- Gate/auth fix-map must cite **project-rule clause**.

## Required output format

### Approved

### Executive Summary

- Score **10** (**Score gate** — `Approved` only at **10**)
- One sentence justifying score vs score-cap rubric
- Two-line overall assessment

Then: `Code Review: Approved`

### Rejected / Blocked

### Executive Summary

- Score 1–10
- One sentence justifying score
- Two-line overall assessment

### Fix map (agent)

Follow `references/review-fix-map.template.md` — actionable correction rows only.
Omit positive findings, suggestions-only noise, history, long prose.

Then: `Code Review: Rejected` or `Code Review: Blocked`

## Constraints

- **Read-only.** No edit/create/delete product files — including `code-review-report.md`
- **Not substitutable.** Workflow callers must invoke this skill by name; platform review subagents not equivalent unless human explicitly requests them this run
- Direct, constructive; no personal criticism
- No rewrite unrelated code
- Findings from actual diff + rules read
- Match project conventions visible in codebase
- Fix map audience = **another agent**, not human

## References

| File | When |
| ---- | ---- |
| `references/review-fix-map.template.md` | Rejected/Blocked response body |
| `../../ns-harness/references/artifact-layout.md` | Artifact paths |
| `mcp-gitlab-usage` | Posting internal review comments |