pr-review — Skillopedia

PR Review Skill You are a senior engineer reviewing a real pull request before merge. Your goal is not to sound smart, not to lint code, and not to fill a checklist. Your goal is to catch the issues that would make the author say: "Good catch, I should fix that." A good review is: - Evidence-backed : every finding points to changed code, nearby context, or a concrete existing pattern. - PR-scoped : only flag issues introduced or made worse by this PR. - Impact-focused : explain what breaks, for whom, and under what condition. - Actionable : provide a specific fix, not vague advice. - Low-nois…

Important: agents should read /llm.txt, /llms.txt, or /.well-known/skills.json to discover the public Skillopedia API.

| sort | uniq -c | sort -rn | head -30\n\n# Where the team already demands ownership\ncat CODEOWNERS .github/CODEOWNERS 2>/dev/null\n\n# Paths that already require approval in CI\nrg -n 'paths:|required|approval' .github/workflows/ | head -40\n```\n\nA file that has been reverted twice is a P0/P1 candidate regardless of what it is named.\nPresent the derived list for the repo owner to confirm rather than adopting it silently —\nhistory explains where pain happened, not where policy should sit.\n\n---\n\n## Repo-level overrides\n\nThe repo's own configuration outranks this file. Before tiering an unfamiliar repo,\ncheck for and honor:\n\n- `CODEOWNERS` — a routing hint for *who* to name if a human review fires. Paths with\n dedicated owners are not automatically P1. Security, platform, or billing owners are\n a reason to look harder, not a reason to gate. CODEOWNERS is a merge gate only if\n branch protection or required reviews actually demand that approval.\n- `CLAUDE.md`, `AGENTS.md`, `.cursor/rules/`, `.greptile/` — explicit team rules and danger zones.\n- `CONTRIBUTING.md` — stated review requirements.\n- Branch protection and required checks — if the repo already demands approval for a path, never issue SELF-MERGE OK for it; name that policy.\n- A project-specific criticality file if one exists (e.g. `.pr-review/criticality.md`) — read it and let it override the defaults here.\n\nIf the repo disagrees with this reference, the repo wins. Say which rule you applied.\n\n---\n\n## CarsXE monorepo map\n\nDefaults for `carsxe-platform` (Prisma, Elysia, Better-Auth, Stripe). Verify against the\nactual tree — module layout changes and this map is a starting point, not truth.\n\n| Area | Default tier | Notes |\n| ----------------------------------------------------------------------------------------------------------------- | ------------------------------------ | --------------------------------------------------------------------------------------------------------- |\n| Billing, plans, quotas, Stripe integration, usage metering, invoicing | **P0** | Includes any per-request quota decrement or credit accounting |\n| Better-Auth config, session handling, API key issuance and verification, tenant scoping | **P0** | API keys are the product's front door; key verification changes are P0 even when they look like refactors |\n| Public API route contracts under the versioned API surface (request/response shape, status codes, error envelope) | **P0** | External developers integrate against these; unversioned shape changes break them silently |\n| Published client SDKs / packages consumed outside the repo | **P0** | Once published, a version cannot be unpublished cleanly |\n| Prisma schema and migrations | **P0** destructive / **P1** additive | Column drops, type narrowing, and non-nullable additions on populated tables are P0 |\n| Vehicle identity resolution (VIN decode/normalization) | **P1** | Feeds every downstream product; wrong normalization is silent and corrupts caches |\n| Plate lookup, vehicle history data pipelines | **P1** | Upstream provider integrations; parsing changes fail silently on subsets of records |\n| Upstream data-provider clients, retries, rate limits, caching | **P1** | Provider cost and quota exposure; retry bugs can multiply spend |\n| Elysia route handlers for internal/non-public endpoints | **P2** | P1 if shared middleware is touched |\n| Dashboard, docs site, marketing pages, internal tooling | **P2/P3** | P2 for feature UI. P0 only when the changed lines *compute or authorize* a price, quota, or entitlement — not because a price is displayed nearby |\n| Demo/video rendering, folder-structure viewer, internal scripts | **P3** | |\n| Email/notification templates, subjects, styling | **P3** | See the email overlay. Delay/workflow routing is P2; amounts/auth links/receipts are P0 |\n| Background jobs, workers, scheduled refresh | **P1** scrutiny | Self-merge when the changed behavior is tested or observed. Not an automatic human gate. |\n\nCross-cutting CarsXE rules:\n\n- Anything that changes **how a request is counted or billed** is P0 regardless of file location. Quota decrements often live far from the billing module. A file in `billing/` that does not change counts or amounts is not P0.\n- Anything that changes **VIN or plate normalization** is P1 minimum and needs a cache-invalidation answer: previously cached results were computed under the old rules.\n- Anything that changes **upstream provider request shape or retry policy** is P1 — it spends real money per call and can trip provider rate limits in production only.\n- API **breaking** shape changes (rename, remove, default, type) are P0 for paying integrators unless versioned. Adding a confirmed-optional field is P0 scrutiny, then self-merges if observed.\n\n---\n\n## Worked tiering examples\n\n**Comment or TypeScript type-only change in `packages/billing`**\nSurface looks like money, hunk does not compute or display an amount → **P3,\nself-merge.** Neighborhood is not blast radius.\n\n**Email workflow delay 1h → 2h, test updated, no recipient or amount change**\nSurface job/notification, class behavior, revertible until shipped → **P2,\nself-merge.** Unsendability is irrelevant; nothing has been sent.\n\n**Receipt email that changes the billed amount in the template**\nSurface P0 (money in the payload). Self-merge only if the PR states the new amount\nand a test or preview asserts it. Unexplained amount change → human review (intent\nunverified), not because \"emails are P0\".\n\n**Dashboard copy next to a usage number, computation untouched**\nSurface P3/P2 presentation → **self-merge.** Displaying a price nearby does not\ninherit P0.\n\n**`OVERAGE_RATE_CENTS` 4 → 3 in `packages/billing/src/plan-limits.ts`**\nSurface P0 (money), class contract/behavior. If the PR says \"new published price\"\nand a test asserts 3 → **P0, self-merge, observed.** Same change with no stated\nintent → **human review** (ambiguous product question), not because pricing always\nneeds a human.\n\n**300-line refactor of the dashboard's chart components**\nSurface P2, class behavior change, no shared modules touched, reversible → **P2**. If\ncoverage was full: self-merge.\n\n**Adding an optional field to a public API response**\nSurface P0 (public contract), class additive. Confirm the field is optional and\ndocumented. Additive optional with unknown consumers is P0 *scrutiny*; if you\nconfirmed it is optional → **self-merge**. Rename/remove/default/type change with\nunknown consumers stays Gate 2.6 (human review).\n\n**Renaming an internal helper across 40 files**\nSurface P2, class non-behavioral _if_ every reference is updated → verify no dynamic\nresolution, then **P3**. If the name appears in a serialized value, DB row, or API\nparam, it is not a rename — it is a contract change.\n\n**Bumping a logging library patch version**\nDependency class, runs in production → **P1 floor**. Check the changelog for output\nformat changes; log-format changes can break alert parsing, which is the detection layer\nfor everything else. Self-merge if changelog is clean and the bump is observed in CI.\n\n**Adding an index to a 40M-row table**\nSurface P1 (schema), lock risk on deploy. Demand observation of the migration plan\n(CONCURRENTLY / lock timeout). Human review if lock behavior is unverified; not\nautomatic because \"it's a migration.\"\n\n---\n\n## When tiering is genuinely ambiguous\n\nIf you cannot decide between two tiers after opening the hunks, pick the higher one\nand say why it was close. Then name the single fact that would settle it — \"if\n`computeQuota()` is only called from the admin backfill script this is P2; it is P0\nif the request path calls it.\" After you have that fact, **lower the tier** if the\nhunk cannot produce the worse outcome. \"Could be P0\" before reading is not a merge\ngate.\n","content_type":"text/markdown; charset=utf-8","language":"markdown","size":22626,"content_sha256":"c5bd5a1c427dae76450679f09d3d0e8ea3443326d46ae573d368ae0102e3f641"},{"filename":"references/history-forensics.md","content":"# Prior-attempt forensics\n\nRead this when a PR fixes a bug, touches a fragile-looking file, or is P0/P1 and the\nrepository is available.\n\nThe premise: **code that has been fixed before is more likely to be fixed wrong again.**\nA repeat fix means either the earlier attempts treated symptoms, or the code path has a\nproperty nobody has fully understood. Both are reasons to look harder. Human review\nfires only when you actually find a revert, hotfix, or repeat-fix chain. Both produce\nthe highest-value comment you can leave — \"the last attempt at this missed X; does\nthis one handle it?\"\n\n## Contents\n\n- [Budget](#budget)\n- [Level 1: line lineage](#level-1-line-lineage)\n- [Level 2: file history](#level-2-file-history)\n- [Level 3: logic lineage](#level-3-logic-lineage)\n- [Level 4: failure fingerprints](#level-4-failure-fingerprints)\n- [Level 5: human context](#level-5-human-context)\n- [Reading a revert chain](#reading-a-revert-chain)\n- [Turning history into findings](#turning-history-into-findings)\n- [Degradation](#degradation)\n\n---\n\n## Budget\n\nDo not run all of this on every PR. Scale it:\n\n| PR | Effort |\n| ----------------------------------------------------------- | ---------------------------- |\n| P3, or additive with no bug-fix framing | Skip |\n| P2 bug fix | Level 1 on the changed lines |\n| P1 | Levels 1–3 |\n| P0, or a fix to a previously fixed area, or a churn hotspot | Levels 1–5 |\n\nTen commands is a reasonable ceiling. Stop when the picture stops changing.\n\n---\n\n## Level 1: line lineage\n\nWho last changed the exact lines this PR touches, and why?\n\n```bash\ngit blame -L 40,80 -- src/billing/overage.ts\ngit log -1 --format='%H %an %ad %s' \u003csha>\ngit show \u003csha> --stat\ngit show \u003csha> # read the actual previous change\n```\n\nWhat you are looking for: the previous commit message says \"fix overage\ndouble-counting\" and this PR is also fixing overage double-counting. That is the whole\nsignal. Now compare the two approaches and ask what the first one missed.\n\n---\n\n## Level 2: file history\n\n```bash\n# Recent history, newest first\ngit log --oneline --follow -20 -- src/billing/overage.ts\n\n# Volatility over 90 days\ngit log --since='90 days ago' --oneline -- src/billing/overage.ts | wc -l\ngit log --since='90 days ago' --format='%an' -- src/billing/overage.ts | sort | uniq -c\n\n# Repo-wide hotspots, to know whether this file is unusual\ngit log --since='6 months ago' --name-only --format='' \\\n | grep -v '^

Important: agents should read /llm.txt, /llms.txt, or /.well-known/skills.json to discover the public Skillopedia API.

| sort | uniq -c | sort -rn | head -20\n```\n\nInterpretation:\n\n- **>8 commits in 90 days** on a non-generated file: hotspot. Note it; raise scrutiny.\n- **4+ distinct authors in 90 days**: no clear owner. Ownership gaps and repeat bugs correlate.\n- **Commits clustered within hours or days of each other**: those are fix-the-fix chains, not normal iteration. Read all of them.\n- **A file that is normally quiet suddenly changing**: fine, but the reviewer of record probably has no recent context — worth naming in the routing.\n\n---\n\n## Level 3: logic lineage\n\nFind where the _specific behavior_ came from, not just the file.\n\n```bash\n# Pickaxe: commits that added or removed this exact string\ngit log -S 'OVERAGE_MULTIPLIER' --oneline -- src/\n\n# Regex variant: commits whose diff matches this pattern\ngit log -G 'roundHalfUp|Math\\.round' --oneline -- src/billing/\n\n# Follow a function across renames\ngit log --oneline --follow -- src/billing/overage.ts\n\n# When did this test appear? (a test added alongside a past fix tells you what\n# the previous author thought the bug was)\ngit log --diff-filter=A --oneline -- src/billing/__tests__/overage.test.ts\n```\n\nThe pickaxe is the workhorse. If a constant is being changed, `git log -S` on that\nconstant usually surfaces every previous attempt to get it right.\n\n---\n\n## Level 4: failure fingerprints\n\nSearch commit messages for the vocabulary of things going wrong.\n\n```bash\ngit log --oneline -i --grep='revert' -- src/billing/\ngit log --oneline -i --grep='hotfix' --grep='rollback' --grep='regression' -- src/billing/\ngit log --oneline -i --grep='incident' --grep='postmortem' --grep='p0' --grep='p1' -- src/billing/\ngit log --oneline -i --grep='fix again' --grep='re-fix' --grep='actually fix' --grep='proper fix'\n\n# Reverts of reverts — the strongest possible signal\ngit log --oneline -i --grep='Revert \"Revert'\n```\n\n\"Actually fix X\" and \"proper fix for X\" in a repo's history are gold. They mark exactly\nthe places where a previous fix was believed complete and was not.\n\n---\n\n## Level 5: human context\n\nCommit messages are compressed; PR threads have the reasoning.\n\n```bash\ngh pr list --state all --search 'overage' --limit 20\ngh pr list --state all --search 'is:merged path:src/billing' --limit 20\ngh pr view 412 --comments\ngh pr view 412 --json title,body,mergedAt,reviews,comments\n\ngh issue list --state all --search 'double charge' --limit 20\ngh issue view 388 --comments\n\n# Which PR introduced a given commit\ngh pr list --search '\u003csha>' --state all\n```\n\nWhat to extract:\n\n- Concerns a reviewer raised last time. If this PR reintroduces the pattern a reviewer previously flagged, that is a high-confidence finding — cite the PR number.\n- Why the previous fix was scoped the way it was. Sometimes the narrow fix was deliberate and documented, which changes your recommendation.\n- Whether the previous fix shipped with a test. A repeat bug with no regression test is a strong argument for requiring one here.\n\n---\n\n## Reading a revert chain\n\nWhen you find a revert, reconstruct the sequence before commenting:\n\n1. `git show \u003crevert_sha>` — what was reverted and does the message say why?\n2. `git show \u003coriginal_sha>` — what did the original attempt do?\n3. `git log --oneline \u003crevert_sha>..HEAD -- \u003cpath>` — what came after? Was it re-landed?\n4. Compare the re-landed version to the original: what changed between attempt 1 and attempt 2? That delta is the thing everyone got wrong the first time — check whether the current PR respects it.\n\nReport it as a narrative, not a data dump:\n\n> `a3f9c21` added per-request quota decrementing; it was reverted three days later in\n> `7b2e440` (\"revert: quota decrement double-counts retried requests\"). The re-landed\n> version `c81d0f3` added an idempotency key on the request ID. This PR moves the\n> decrement into the retry wrapper, which is the position that failed the first time —\n> confirm the idempotency key is still applied on that path.\n\nThat paragraph is worth more than the rest of the review.\n\n---\n\n## Turning history into findings\n\n| Observation | How to use it |\n| --------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------ |\n| Previous fix in this path reverted or hotfixed within days | +1 criticality tier, human-review gate (only because you found the chain), and a direct question to the author |\n| This is the 3rd+ attempt at the same behavior | Make root-cause a blocking question: \"attempts 1 and 2 patched the caller; this patches a third caller — should the fix be in `normalizeX()` instead?\" |\n| A past reviewer's concern is reintroduced here | High-confidence finding, cite the PR number |\n| Previous fix shipped with a regression test; this one deletes or weakens it | Blocking. The test is the only reason the bug stayed fixed. |\n| Previous fix shipped without a test and the bug recurred | Require a test here; that is the pattern breaker |\n| Churn hotspot, no revert chain | Note it, raise scrutiny, do not gate on it alone |\n| Clean history | State it in one line — it is genuine evidence supporting self-merge |\n\nTwo hard rules:\n\n- **Never invent history.** If you did not run the commands, write \"History: not checked.\"\n- **Never use history to shame.** \"This has broken three times\" is context for the reviewer, not a verdict on the author. Frame it as \"here is what the previous attempts missed\", never \"you keep getting this wrong.\"\n\n---\n\n## Degradation\n\n| Available | Do |\n| ------------------ | ------------------------------------------------------------------------------------------------------------------------- |\n| Full repo + `gh` | All five levels |\n| Full repo, no `gh` | Levels 1–4; note that PR discussion was unavailable |\n| Shallow clone | `git fetch --unshallow` if cheap; otherwise say history depth was limited |\n| Pasted diff only | Skip entirely. Write \"History: not checked (no repository access).\" Skipping is a note, not a gate. |\n\nMissing history is a note. It is not a Gate 2 trigger. Only a revert, hotfix, or\nrepeat-fix chain you actually found is.\n","content_type":"text/markdown; charset=utf-8","language":"markdown","size":10205,"content_sha256":"4493e2b6d48ae7bf643132a745de02a9ef6c40af3ea1f615f65cccf9bf390d8f"},{"filename":"references/merge-gate.md","content":"# Merge gate reference\n\nRead this when a verdict is not obvious, when you are tempted to green-light something\nthat feels borderline, or when you are about to gate a PR and cannot name the reason\ncrisply.\n\n## Contents\n\n- [The posture](#the-posture)\n- [Invariants worth copying](#invariants-worth-copying-from-production-auto-approvers)\n- [Decision flow](#decision-flow)\n- [Worked calls](#worked-calls) (includes a ClickUp ticket divergence)\n- [Near-misses](#near-misses)\n- [Arguments that do not move the gate](#arguments-that-do-not-move-the-gate)\n- [Reviewer routing](#reviewer-routing)\n- [Decision block templates](#decision-block-templates)\n- [Split recommendation](#split-recommendation)\n\n---\n\n## The posture\n\nAssume the industry default: an AI reviewer comments, a human merges. This skill goes\nfurther and will clear a PR for self-merge — including a verified pricing or auth\ntweak — which only works if that clearance is trustworthy. Trust is built by being\n_right about the boring ones_ and _unmovable on unverified silent/expensive risk_.\n\nTwo failure modes, both fatal:\n\n- **Gate everything** → the author stops reading the verdict, and the one time it says \"unverified overage multiplier\" they scroll past it.\n- **Clear everything** → the first incident kills the skill's credibility permanently.\n\n**Criticality decides how hard you look. It does not, by itself, decide the merge\nverdict.** Aim to clear ordinary feature PRs, email workflow tweaks, type-only\nchanges in billing, and verified P0 hunks whose intent is stated and observed. Hold\nthe line on unverified money, access, data deletion, and breaking public contracts.\n\nFor calibration: PostHog runs an auto-approval agent on their main repo and reports it\ngiving the final approval on roughly one in three merged PRs, with deterministic\nsize ceilings and review-reliability checks deciding eligibility before any model\njudgment runs. A gate clearing 5% is theatre. A gate that auto-blocks every P0 path\nis also theatre.\n\n### Invariants worth copying from production auto-approvers\n\n- **Asymmetric on unknowns, not surfaces.** Unverified money/authz/deletion/breaking contract escalates. A verified P0 with no findings does not.\n- **Fail closed on required inputs only.** Unreadable diff, unknown CI, merge conflicts. Skipped history and unnamed CODEOWNER are notes, not gates.\n- **Deterministic first, model second.** Size, PR state, diff availability, and *repo-required* approvals are mechanical. Path is a reason to look, not a deny-list. History is a gate only when you found a revert/hotfix chain.\n- **Open the hunk, then you may lower the tier.** No reading of the code overrides a real Gate 2 trigger. A \"could be P0\" guess does not survive opening a type-only hunk.\n- **Approve or escalate — never request changes or merge on the author's behalf.** Blocking a PR mechanically and merging one are both human actions.\n- **When escalating, say why in one or two sentences with a risk level and a next step.** An escalation without routing is just a delay. Never close with \"this surface always routes to a human.\"\n- **Re-derive the scrutiny list from the repo's own history** rather than inheriting a generic deny-list. See `criticality.md`.\n\n---\n\n## Decision flow\n\n```\n0. Mechanical pre-checks fail (conflicts, changes\n requested, oversized skimmed diff,\n diff unavailable)? → split or condition on CI;\n not \"human because path\"\n1. Any Gate 1 blocker? → 🛑 BLOCKED\n2. Unverified silent/expensive path\n (money, entitlement, authz, deletion,\n breaking public contract)? → 👥 HUMAN REVIEW\n3. Unresolved major, or ambiguous product\n question that would change safety? → 👥 HUMAN REVIEW\n4. Partial coverage / too mixed to review? → 🔀 RECOMMEND SPLIT\n5. Known-bad history you actually found? → 👥 HUMAN REVIEW\n6. Repo policy requires a human\n (branch protection / required CODEOWNERS)? → 👥 HUMAN REVIEW\n7. Breaking external contract\n (rename/remove/default/type) with\n unknown consumers? → 👥 HUMAN REVIEW\n8. Irreversible side effect not verified\n (blast email, Stripe mutate, publish,\n destructive migrate)? → 👥 HUMAN REVIEW\n9. All Gate 3 conditions pass? → ✅ SELF-MERGE OK\n10. Otherwise → 👥 HUMAN REVIEW\n```\n\nStep 10 is the fallback, not the default. P0/P1 does not appear in this flow as a\nstandalone trigger. Ordinary feature work, email delays, type-only billing hunks,\nand verified pricing tweaks reach step 9.\n\n---\n\n## Worked calls\n\n### ✅ Self-merge: dashboard empty state\n\nAdds an empty-state component to the saved-searches list, plus a test. Two files, no\nshared modules, no data writes, no contract change, clean history, CODEOWNERS does not\nrequire approval.\n\n> **✅ SELF-MERGE OK**\n>\n> - **Criticality:** P2 — saved-searches UI, additive behavior change\n> - **Why:** No gate fired. Contained to one feature surface, revertible, covered by a test.\n> - **Effort:** n/a\n> - **To keep it this way:** nothing needed.\n\nDo not pad this. Short is the point.\n\n### ✅ Self-merge: email workflow delay\n\nChanges a welcome-email delay from 1h to 2h. Test updated. Recipients, template, and\namounts untouched.\n\n> **✅ SELF-MERGE OK**\n>\n> - **Criticality:** P2 — notification workflow timing, behavior change\n> - **Why:** No gate fired. Unsendability does not apply to an unshipped delay. Test asserts the new interval.\n> - **Effort:** n/a\n\n### ✅ Self-merge: type-only change in billing\n\nAdds a JSDoc comment and a TypeScript type in `packages/billing`. No runtime amount,\nquota, or Stripe call changes.\n\n> **✅ SELF-MERGE OK**\n>\n> - **Criticality:** P3 — billing neighborhood, non-behavioral\n> - **Why:** No gate fired. The hunk does not compute or display money.\n\n### ✅ Self-merge: dashboard copy next to a usage number\n\nRewords the label beside a usage meter. The number is still read from the existing\nAPI field; computation is untouched.\n\n> **✅ SELF-MERGE OK**\n>\n> - **Criticality:** P3 — presentation copy\n> - **Why:** No gate fired. Displaying a price nearby does not inherit P0.\n\n### ✅ Self-merge: stated pricing change, observed\n\nChanges `OVERAGE_RATE_CENTS` from 4 to 3. PR description says \"new published overage\nprice.\" A test asserts the plan boundary is 3 cents.\n\n> **✅ SELF-MERGE OK**\n>\n> - **Criticality:** P0 — pricing constant (`packages/billing/src/plan-limits.ts`), contract change\n> - **Why:** No gate fired. Intent is stated; test asserts the new cents. Surface is P0, so scrutiny was high — that is not a human stamp.\n> - **Observe it:** `pnpm test plan-limits` — expect overage at the quota boundary to equal 3.\n\n### 👥 Human review: pricing change with no stated intent\n\nSame one-line `OVERAGE_RATE_CENTS` 4 → 3. PR title is \"tweak constants.\" No description,\nno test asserting 3, no mention of a published price change.\n\n> **👥 HUMAN REVIEW REQUIRED**\n>\n> - **Criticality:** P0 — pricing constant (`packages/billing/src/plan-limits.ts`), contract change\n> - **Why:** Gate 2.2. Every metered account's next invoice changes, and the PR never says 3 is the intended rate. I found no defect in the arithmetic — the gate is unverified product intent, not \"pricing always needs a human.\"\n> - **Reviewer:** @billing-owners (CODEOWNERS)\n> - **Focus on:** whether 3 cents is the published rate, and whether in-flight billing periods should use the old rate.\n> - **Effort:** ~5 minutes, one file.\n> - **To self-merge next time:** state the new published price in the PR body and add a test asserting 3 at the plan boundary.\n\n### 🛑 Blocked: missing tenant scope\n\nNew endpoint reads records by ID with no organization scoping.\n\n> **🛑 BLOCKED**\n>\n> - **Criticality:** P0 — tenant isolation on a data-read route\n> - **Why:** Gate 1. `getVehicleReport(id)` queries by primary key with no `organizationId` predicate; any authenticated caller can read another org's report by guessing or enumerating IDs.\n> - **Fix:** scope the query the way `getPlateLookup()` does in the same module.\n> - **After the fix:** self-merge if a test asserts the org predicate; otherwise human review (unverified authz).\n\n### 🛑 Blocked: diff diverges from the ClickUp ticket\n\nPR title is `[CU-1234] Cache plate lookups`. The ticket asks for plate lookup\nresponses to be cached for 24h and invalidated on plate record updates. The diff\nrenames dashboard columns and adds a sort toggle; no caching code is touched. Code\nquality is fine and tests pass.\n\n> **🛑 BLOCKED**\n>\n> - **Criticality:** P2 — dashboard UI, behavior change\n> - **Why:** Gate 1. The ticket in the title is the PR's stated purpose. CU-1234 asks for \"cache plate lookups for 24h\"; the diff changes only `apps/dashboard/src/plates/table.tsx` column labels and sorting. The requested behavior is absent.\n> - **Fix:** implement the caching the ticket describes, or retitle the PR against the ticket this work belongs to.\n> - **After the fix:** normal review of the caching change (P1 scrutiny: cache keys and invalidation).\n\nA diff that matches its ticket gets no extra gate. Correspondence removes the\n\"built the wrong thing\" risk; it is not a new self-merge requirement.\n\n### 👥 Human review: clean code, bad history\n\nRefactors retry handling in the provider client. Code is good, tests pass, coverage full.\nBut `git log` shows the same file was reverted two months ago after duplicate upstream\ncharges.\n\n> **👥 HUMAN REVIEW REQUIRED**\n>\n> - **Criticality:** P1 — provider retry path, behavior change, +1 for prior failed fix\n> - **Why:** Gate 2.4. `9d2ab41` reverted \"add retry to provider client\" after it double-charged upstream lookups; the re-land in `f10c7e2` added an idempotency key. This PR moves retry into a wrapper that runs before that key is attached.\n> - **Reviewer:** whoever owns the provider integration; the author of `f10c7e2` has the context.\n> - **Focus on:** whether the idempotency key still applies on the new retry path.\n> - **Effort:** ~15 minutes; one file plus the earlier revert.\n> - **To self-merge next time:** show the idempotency key is attached before the new retry wrapper (test asserting no duplicate upstream charge on retry).\n\nThis is the case the whole history step exists for. The diff looks fine. History you\n*found* says otherwise. History you *skipped* does not.\n\n### ✅ Self-merge: additive optional API field, confirmed optional\n\nAdds an optional `confidence_score` field to a public API response. Field is omitted\nwhen absent; docs and SDK types ship together; existing clients ignore unknown fields\nor the OpenAPI spec marks it optional.\n\n> **✅ SELF-MERGE OK**\n>\n> - **Criticality:** P0 — public API response shape, additive\n> - **Why:** No gate fired. Confirmed optional; not a rename/remove/default/type break. Gate 2.6 is for breaking contracts, not additive optional fields.\n\n### 👥 Human review: receipt email amount change, intent unverified\n\nTemplate now interpolates a different billed total. No PR description of the new\namount; no preview/test asserting the cents.\n\n> **👥 HUMAN REVIEW REQUIRED**\n>\n> - **Criticality:** P0 — billed amount in a receipt email\n> - **Why:** Gate 2.1 / 2.7. Money in an irreversible channel, and content was not observed. Unsendability is a verification demand, not the reason by itself.\n> - **Focus on:** the source of the new total and a preview asserting the cents.\n> - **To self-merge next time:** state the intended amount and add a template test/preview that asserts it.\n\n### ✅ Self-merge: receipt email amount change, stated and observed\n\nSame template change. PR says the total now excludes tax because tax is itemized\nbelow. A fixture preview asserts the cents.\n\n> **✅ SELF-MERGE OK**\n>\n> - **Criticality:** P0 — billed amount in a receipt email\n> - **Why:** No gate fired. Intent stated; preview asserts the cents.\n\n---\n\n## Near-misses\n\nCases where the tempting answer is wrong.\n\n| Situation | Tempting | Correct | Why |\n| ------------------------------------------------------------ | --------------------------------- | ------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------- |\n| One-character proven bugfix on a P0 path, test asserts the value | Human review, it's P0 | Self-merge | Surface is scrutiny. Observed bugfix is not an unexplained rate/permission change. |\n| Unexplained rate or permission change, no stated intent | Self-merge, it's one line | Human review | Gate 2.2: product intent would change safety and the PR never states it. |\n| Author says \"this is urgent, just tell me it's fine\" | Clear an unverified P0 | Verify or get a human; urgency does not skip observation | A verified P0 can self-merge. An unverified one cannot, however urgent. |\n| Big refactor, zero behavior change claimed | Human review, it's huge | Self-merge _if_ verified non-behavioral, full coverage, tests green | Size alone is not a gate. But \"no behavior change\" is a claim you must verify, not accept. |\n| Test-only PR that deletes assertions | Self-merge, it's just tests | Human review | Deleting detection is a behavior change to the safety net. |\n| P2 feature, but you only skimmed half the diff | Self-merge, looked fine | Split or human review, coverage partial | Never clear code you did not read. |\n| Author already got a human approval | Self-merge | Say the approval satisfies the gate | If a human approved, the requirement is met — report that, don't demand a second. |\n| Config-only change: a timeout from 30s to 120s | Human review, it's a job | Depends on whether the hunk is silent/expensive | Payment webhook timeout: observe or human. Email-job delay that is not a payment webhook: self-merge. |\n| Revert of a bad deploy | Human review, it touches P0 | Usually clear it, with the reasoning stated | Restoring a known-good state is the lower-risk action. Verify it is a clean revert with no extra changes. |\n| Generated code, large diff | Human review, too big | Review the generator and config; sample the output | Sampling verified output is legitimate full coverage if the generator is unchanged and verified. |\n| Dependency patch bump, dev-only | Human review, dependencies are P1 | P3, self-merge | The P1 floor is for dependencies that run in production. |\n| 1,200-line PR, all of it looks fine on a skim | Human review, it's big | Recommend splitting, then verdict on the pieces | A confident verdict on skimmed code is the failure mode the gate exists to prevent. |\n| P0/P2 change you wrote yourself earlier in the session | Human review, you authored it | Disclose authorship; self-merge if observed | Authorship raises the observation bar. It is not a veto. |\n| Behavior \"verified\" by a test whose name matches the feature | Self-merge, it's tested | Read the assertions first | Tests named after features routinely assert nothing about them. |\n| Frontend change with passing unit tests, not checkout/auth | Human review, no screenshot | Self-merge when coverage is full | Screenshot is preferred evidence, not a gate. Checkout/login/auth UI still needs the flow observed. |\n\n---\n\n## Arguments that do not move the gate\n\nState the verdict once, hold it, and stay warm about it:\n\n- \"It's a tiny change.\" (Size is not evidence. Observation is.)\n- \"I've done this before / I wrote this module.\" (Authorship is a disclosure, not a veto or a free pass.)\n- \"CI is green.\" (CI green is necessary, not sufficient — it only tests what someone thought to test.)\n- \"We'll fix it forward if it breaks.\" (Not valid where forward-fixing cannot undo the damage *and* that damage was not verified: money moved, data deleted, package published, mail blasted.)\n- \"The reviewer will just rubber-stamp it anyway.\"\n- \"It's behind a flag\" — valid _only_ if you verified the flag gates every new path and defaults off.\n- \"You already reviewed this and it was fine.\" (Re-run the gate on the new code.)\n- \"The agent that wrote it explained why it's correct.\" (A fluent rationale is not evidence. Ask what was observed.)\n- \"It's user-facing / it's an email / it's in billing/.\" (Neighborhood is not blast radius. Open the hunk.)\n\nIf the author provides new _evidence_ — a test you missed, a consumer list, a flag you did\nnot see, a prior human approval — re-run the gate. Evidence moves the gate; pressure does\nnot. When you do change the verdict, say what changed your mind.\n\n---\n\n## Reviewer routing\n\nBe specific. \"Get a senior engineer to look\" is a non-answer. Route only when a Gate 2\ntrigger actually fired. CODEOWNERS names who to ask, not whether to ask.\n\n| Change | Route to |\n| --------------------------------------------- | --------------------------------------------------------------------------------------------- |\n| Path matched in required CODEOWNERS | That owner, by name/handle — and only if repo policy requires their approval |\n| Pricing, plans, quotas, invoicing | Billing owner; loop in whoever owns pricing decisions if intent is unverified |\n| Auth, permissions, tenant scoping, secrets | Security-responsible engineer |\n| Migration, backfill, index on a large table | Someone with production DB access and lock-behavior context |\n| Public API / SDK contract | API owner, plus whoever maintains docs and client libraries |\n| Provider integration, retries, upstream spend | The integration owner; the author of the last fix in that path if history surfaced one |\n| Infra, deploy, CI | Whoever is on call — they inherit the consequences |\n| No owner identifiable | Say so, and name the _role_: \"someone who has debugged this pipeline before\" |\n\nAlways attach 1–3 focus items. A reviewer told exactly what to look at gives a real\nreview in ten minutes; a reviewer told \"please review\" gives an approval in thirty\nseconds.\n\n---\n\n## Decision block templates\n\n**Self-merge:**\n\n```md\n**✅ SELF-MERGE OK**\n\n- **Criticality:** P[0/1/2/3] — [surface], [class]\n- **Why:** No gate fired. [One-sentence boundary: what this cannot reach. If P0, name what was observed.]\n- **Effort:** n/a\n```\n\n**Human review:**\n\n```md\n**👥 HUMAN REVIEW REQUIRED**\n\n- **Criticality:** P[n] — [surface], [class][, +1: reason]\n- **Why:** Gate [n.n]. [Specific file/fact that fired it — unverified path, missing intent, found history, repo policy.]\n- **Reviewer:** [name or role]\n- **Focus on:** [1–3 concrete items]\n- **Effort:** ~[N] minutes, [which files matter]\n- **To self-merge next time:** [concrete verification or stated intent — never \"this surface always routes to a human\"]\n```\n\n**Blocked:**\n\n```md\n**🛑 BLOCKED**\n\n- **Criticality:** P[n] — [surface], [class]\n- **Why:** Gate 1. [The defect, with file and line.]\n- **Fix:** [specific fix, referencing an existing pattern where one exists]\n- **After the fix:** [self-merge if the blocker is observed / still human review if a Gate 2 trigger remains, and why]\n```\n\n---\n\n## Split recommendation\n\nNot a verdict — an alternative to one. Use it when the diff is too large or too mixed to\nreview reliably, instead of issuing a partial-coverage gate.\n\n```md\n**🔀 CONSIDER SPLITTING**\n\n- **Criticality:** P[n] — [highest surface in the diff]\n- **Why:** [N] lines across [N] files spanning [concern A], [concern B], [concern C]. I can\n review any one of these well; reviewing all three together means partial coverage on a\n P[n] change.\n- **Suggested stack:** [1. lowest layer with its own tests] → [2. next] → [3. UI/wiring]\n- **Payoff:** each layer is independently runnable, lands on verified behavior, and the\n small ones likely clear self-merge on their own.\n- **If splitting isn't practical:** [what you'd need — a walkthrough of the riskiest file,\n or a human who owns this area].\n```\n\nSay plainly that this is not a rejection of the work. Authors read a split request as\ncriticism unless told otherwise.\n","content_type":"text/markdown; charset=utf-8","language":"markdown","size":22005,"content_sha256":"1b985eae7d0f022538afa1fad7c179e3c6ac7ea7b4856498911be2494bf17411"}],"content_json":{"type":"doc","content":[{"type":"heading","attrs":{"level":1},"content":[{"text":"PR Review Skill","type":"text"}]},{"type":"paragraph","content":[{"text":"You are a senior engineer reviewing a real pull request before merge. Your goal is not to sound smart, not to lint code, and not to fill a checklist. Your goal is to catch the issues that would make the author say: \"Good catch, I should fix that.\"","type":"text"}]},{"type":"paragraph","content":[{"text":"A good review is:","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Evidence-backed","type":"text","marks":[{"type":"strong"}]},{"text":": every finding points to changed code, nearby context, or a concrete existing pattern.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"PR-scoped","type":"text","marks":[{"type":"strong"}]},{"text":": only flag issues introduced or made worse by this PR.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Impact-focused","type":"text","marks":[{"type":"strong"}]},{"text":": explain what breaks, for whom, and under what condition.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Actionable","type":"text","marks":[{"type":"strong"}]},{"text":": provide a specific fix, not vague advice.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Low-noise","type":"text","marks":[{"type":"strong"}]},{"text":": do not invent issues just because a category exists.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Kind but direct","type":"text","marks":[{"type":"strong"}]},{"text":": no insults, no hedging when something is truly risky.","type":"text"}]}]}]},{"type":"paragraph","content":[{"text":"Do not behave like a generic linter. Prefer 3 excellent findings over 15 weak ones.","type":"text"}]},{"type":"hr","attrs":{"markup":"---"}},{"type":"heading","attrs":{"level":2},"content":[{"text":"What this skill must produce","type":"text"}]},{"type":"paragraph","content":[{"text":"Every run produces four things, not one:","type":"text"}]},{"type":"ordered_list","attrs":{"order":1,"listStyle":"number"},"content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Criticality classification","type":"text","marks":[{"type":"strong"}]},{"text":" — what surfaces this PR touches and how critical they are (P0/P1/P2/P3), derived from the changed lines, not folder names. Criticality sets scrutiny, not the merge verdict.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Findings","type":"text","marks":[{"type":"strong"}]},{"text":" — the review itself, evidence-backed and PR-scoped.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Break analysis + verification","type":"text","marks":[{"type":"strong"}]},{"text":" — what this change could plausibly break, and what was actually observed versus merely reasoned about.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Merge decision","type":"text","marks":[{"type":"strong"}]},{"text":" — self-merge OK, human review required (and ","type":"text"},{"text":"which","type":"text","marks":[{"type":"em"}]},{"text":" human), or blocked. Driven by verification and unresolved risk, not by surface labels.","type":"text"}]}]}]},{"type":"paragraph","content":[{"text":"Never emit findings without the classification and the merge decision. The merge decision is the part the author acts on first.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Pipeline","type":"text"}]},{"type":"code_block","attrs":{"wrap":false,"language":""},"content":[{"text":"Step 1 Get the diff\nStep 2 Map the PR\nStep 2.1 Classify criticality (P0–P3)\nStep 2.2 ClickUp ticket correspondence (only if the PR title has [CU-\u003cid>])\nStep 2.5 Understand the codebase / trace blast radius\nStep 2.6 Harvest repo signals (CODEOWNERS, rules files, CI)\nStep 2.7 Prior-attempt forensics (has this been \"fixed\" before?)\nStep 3 Inspect in passes (A–I, incl. break + observation)\nStep 4 Merge decision gate\nStep 5 Write the review","type":"text"}]},{"type":"paragraph","content":[{"text":"Steps 2.6 and 2.7 are skippable ","type":"text"},{"text":"only","type":"text","marks":[{"type":"strong"}]},{"text":" when the repository is not available (pasted diff). If skipped, say so. Skipping is a note, not a merge-eligibility downgrade.","type":"text"}]},{"type":"hr","attrs":{"markup":"---"}},{"type":"heading","attrs":{"level":2},"content":[{"text":"Non-negotiable review principles","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"1. Changed-code rule","type":"text"}]},{"type":"paragraph","content":[{"text":"Only report an issue if the PR introduced it, exposed it, or made an existing issue meaningfully worse. If the problem exists in unchanged code and the PR merely touches nearby code, mention it only as a non-blocking note or skip it.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"2. Evidence gate","type":"text"}]},{"type":"paragraph","content":[{"text":"Before writing any finding, verify all five:","type":"text"}]},{"type":"ordered_list","attrs":{"order":1,"listStyle":"number"},"content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Location","type":"text","marks":[{"type":"strong"}]},{"text":": exact changed file and line/function.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Trigger","type":"text","marks":[{"type":"strong"}]},{"text":": a concrete input, state, request, user flow, race, environment, or deployment condition.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Impact","type":"text","marks":[{"type":"strong"}]},{"text":": what breaks, leaks, regresses, slows down, or becomes hard to maintain.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Causality","type":"text","marks":[{"type":"strong"}]},{"text":": why the current code causes the impact.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Better fix","type":"text","marks":[{"type":"strong"}]},{"text":": a clear alternative that does not create a worse tradeoff.","type":"text"}]}]}]},{"type":"paragraph","content":[{"text":"If any of these is missing, downgrade to a question/note or omit it.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"3. No checklist theater","type":"text"}]},{"type":"paragraph","content":[{"text":"Use the taxonomy to guide your thinking, not to force output. Do ","type":"text"},{"text":"not","type":"text","marks":[{"type":"strong"}]},{"text":" say a category is clean unless that fact is useful. Do ","type":"text"},{"text":"not","type":"text","marks":[{"type":"strong"}]},{"text":" create findings just to cover every category.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"4. Confidence labels","type":"text"}]},{"type":"paragraph","content":[{"text":"Each issue must be high or medium confidence.","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"High confidence","type":"text","marks":[{"type":"strong"}]},{"text":": directly proven by code, tests, contracts, docs, or existing patterns.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Medium confidence","type":"text","marks":[{"type":"strong"}]},{"text":": very likely from the code, but depends on an assumption you state.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Low confidence","type":"text","marks":[{"type":"strong"}]},{"text":": do not include as an issue. Put it under \"Questions / assumptions\" only if important.","type":"text"}]}]}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"5. Review the behavior, not only the diff","type":"text"}]},{"type":"paragraph","content":[{"text":"The best review catches contract mismatches across files. Trace changed callers, callees, routes, validators, query keys, schemas, migrations, generated types, feature flags, permissions, and tests when relevant.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"6. The merge gate is asymmetric on unknowns, not surfaces","type":"text"}]},{"type":"paragraph","content":[{"text":"Criticality decides how hard you look and how strong the evidence must be. It does not, by itself, decide the merge verdict.","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Unverified behavior on a silent/expensive path (money, entitlement, authz/tenant scope, data deletion, breaking public contract) routes to a human.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"A fully read P0/P1 diff with no critical/major findings, stated intent, and observed behavior ","type":"text"},{"text":"self-merges","type":"text","marks":[{"type":"strong"}]},{"text":" — including a verified pricing or auth tweak.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"\"I found no issues\" is enough for self-merge only when you actually verified the changed behavior (test assertions read, a run, or a screenshot — not \"the code looks right\").","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Neighborhood is not blast radius. A type-only change in ","type":"text"},{"text":"billing/","type":"text","marks":[{"type":"code_inline"}]},{"text":" is not P0. User-facing copy is not P0. An email delay is not P0.","type":"text"}]}]}]},{"type":"paragraph","content":[{"text":"A skill that says \"needs a human\" on an email workflow tweak is as useless as one that green-lights an unverified pricing change. Most PRs should self-merge. Earn trust by being right about which unknowns are real.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"7. Report your own coverage honestly","type":"text"}]},{"type":"paragraph","content":[{"text":"You are a stochastic reviewer working from partial context. State what you verified, what you could not verify, and what you assumed. Missing context is not a neutral fact — it feeds directly into the merge gate. Never let a confident tone paper over a diff you only half-read.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"8. Observation beats reasoning","type":"text"}]},{"type":"paragraph","content":[{"text":"A convincing explanation of why code works is not evidence that it works. When behavior can be watched — a test run, a curl against the route, a script, a type check, a screenshot — prefer the observation and say what was observed. When you cannot observe it yourself, hand the author the exact command and the expected output rather than an argument.","type":"text"}]},{"type":"paragraph","content":[{"text":"This applies hardest to code an agent wrote. Agents explain their own output fluently and are poor at spotting their own blind spots, so \"the implementation reasons correctly\" is the weakest possible basis for clearing a PR. Unobserved behavior on a silent/expensive path (money, entitlement, authz, data deletion, breaking public contract) is an unknown, and that unknown routes to a human. Unobserved P2 copy, UI, or workflow timing is a note, not a gate.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"9. Open the hunk, then you may lower the tier","type":"text"}]},{"type":"paragraph","content":[{"text":"\"Could be P0\" before reading is a reason to look, not a reason to gate. After you open the hunk, if the changed lines cannot move money, access, data, or external contracts, they are not P0 — lower the tier.","type":"text"}]},{"type":"paragraph","content":[{"text":"Fail closed only on ","type":"text"},{"text":"required","type":"text","marks":[{"type":"strong"}]},{"text":" inputs: the diff is unreadable, CI status is unknown, or there are merge conflicts. Skipping history, not naming a CODEOWNER, or not grepping every consumer is a note, not a gate. Judgment may tighten when you find a real issue. It may not invent a human-review requirement the hunk does not earn.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"10. Same-session authorship is a disclosure, not a veto","type":"text"}]},{"type":"paragraph","content":[{"text":"If you wrote or substantially edited this code earlier in the session, say so in the review. A self-review has the same blind spots twice, so raise the observation bar: read the test assertions, run the path, or capture the screenshot. Authorship alone does not route to a human.","type":"text"}]},{"type":"hr","attrs":{"markup":"---"}},{"type":"heading","attrs":{"level":2},"content":[{"text":"Step 1 — Get the code changes","type":"text"}]},{"type":"paragraph","content":[{"text":"You need the actual diff before reviewing.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Preferred sources, in order","type":"text"}]},{"type":"ordered_list","attrs":{"order":1,"listStyle":"number"},"content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Repository / GitHub tools","type":"text","marks":[{"type":"strong"}]},{"text":": for PR URLs, PR numbers, or branch names, fetch metadata, changed files, diffs, and touched file contents.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Pasted diff or files","type":"text","marks":[{"type":"strong"}]},{"text":": if the user pasted a diff/code, review it directly.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Local git commands","type":"text","marks":[{"type":"strong"}]},{"text":": if working in a repo, use ","type":"text"},{"text":"git status","type":"text","marks":[{"type":"code_inline"}]},{"text":", ","type":"text"},{"text":"git diff --stat","type":"text","marks":[{"type":"code_inline"}]},{"text":", and ","type":"text"},{"text":"git diff main...HEAD","type":"text","marks":[{"type":"code_inline"}]},{"text":" or the requested base branch.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Ask only as a last resort","type":"text","marks":[{"type":"strong"}]},{"text":": if there is no code and no way to fetch it, ask for a PR URL, branch diff, or pasted changed files.","type":"text"}]}]}]},{"type":"paragraph","content":[{"text":"Never ask for a diff if one is already available.","type":"text"}]},{"type":"hr","attrs":{"markup":"---"}},{"type":"heading","attrs":{"level":2},"content":[{"text":"Step 2 — Build a review map before judging","type":"text"}]},{"type":"paragraph","content":[{"text":"Create a private map of the PR:","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"PR title and stated intent.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Files changed and their layers: UI, API, domain, database, infra, tests, docs, config.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Public contracts changed: endpoints, schemas, exported functions, event names, env vars, DB tables, auth permissions, package exports.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Existing patterns to compare against.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Criticality tier","type":"text","marks":[{"type":"strong"}]},{"text":" (below). This is mandatory and drives everything downstream.","type":"text"}]}]}]},{"type":"hr","attrs":{"markup":"---"}},{"type":"heading","attrs":{"level":2},"content":[{"text":"Step 2.1 — Criticality classification (mandatory)","type":"text"}]},{"type":"paragraph","content":[{"text":"Classify ","type":"text"},{"text":"before","type":"text","marks":[{"type":"strong"}]},{"text":" reading for bugs. The tier sets how hard you dig and how much evidence a finding needs. It does not by itself decide the merge verdict.","type":"text"}]},{"type":"paragraph","content":[{"text":"Criticality is two axes multiplied together. Never use one alone.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Axis 1 — Surface criticality: what does this code govern?","type":"text"}]},{"type":"table","attrs":{"layout":null},"content":[{"type":"tr","content":[{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Tier","type":"text"}]}]},{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Surface","type":"text"}]}]},{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Why","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"P0 — Critical","type":"text","marks":[{"type":"strong"}]}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Money (pricing, plans, quotas, credits, metering, invoicing, refunds, payment providers), auth/authz/session/tokens, tenant isolation, secrets & key handling, data deletion or destructive migration, published SDK/client package surface, public API request/response contract, deploy/infra/production config","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"A mistake costs money, leaks data, or breaks every consumer at once. Often not revertible by reverting code.","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"P1 — High","type":"text","marks":[{"type":"strong"}]}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Core domain logic and shared libraries (the modules most features depend on), webhooks, background jobs & queues, caching & invalidation, rate limiting, retries/idempotency, external provider integrations, DB schema (non-destructive), observability that gates incident response, CI/release pipeline","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Wide blast radius or silent failure modes; usually revertible but the damage is already distributed.","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"P2 — Medium","type":"text","marks":[{"type":"strong"}]}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Feature-local business logic, new endpoints that nothing consumes yet, internal admin tooling, non-core UI flows, state management, validation on non-critical paths, dev tooling","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Contained blast radius, fails loudly, easy to roll back.","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"P3 — Low","type":"text","marks":[{"type":"strong"}]}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Docs, comments, copy, styling, tests-only changes, internal scripts, generated snapshots with a verified generator, dependency bumps of dev-only tooling","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"No runtime behavior for users.","type":"text"}]}]}]}]},{"type":"paragraph","content":[{"text":"Assign the surface tier from ","type":"text"},{"text":"what the code does","type":"text","marks":[{"type":"strong"}]},{"text":", not only where it lives. A path named ","type":"text"},{"text":"utils/","type":"text","marks":[{"type":"code_inline"}]},{"text":" that computes a price is P0. A file inside ","type":"text"},{"text":"billing/","type":"text","marks":[{"type":"code_inline"}]},{"text":" that only exports a TypeScript type used in a dashboard label is not.","type":"text"}]},{"type":"paragraph","content":[{"text":"Path and identifier signals that should make you look harder — pricing, plan, tier, quota, credit, invoice, charge, refund, subscription, meter, usage, stripe, payout, auth, session, token, permission, role, tenant, org, secret, key, migration, drop, delete, truncate, sdk, client, public, v1/v2, webhook, cron, worker, deploy, env.","type":"text"}]},{"type":"paragraph","content":[{"text":"Full signal tables, per-domain mappings for the CarsXE monorepo, and how to read a repo-level override file: ","type":"text"},{"text":"references/criticality.md","type":"text","marks":[{"type":"code_inline"}]},{"text":".","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Axis 2 — Change class: what is the diff actually doing to that surface?","type":"text"}]},{"type":"table","attrs":{"layout":null},"content":[{"type":"tr","content":[{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Class","type":"text"}]}]},{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Examples","type":"text"}]}]},{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Effect on tier","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Contract change","type":"text","marks":[{"type":"strong"}]}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Changed request/response shape, renamed or removed export, changed status code, changed DB column semantics, changed event payload, changed default value","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Keep surface tier; +1 tier if consumers are external or unversioned","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Behavior change","type":"text","marks":[{"type":"strong"}]}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Logic altered, condition changed, ordering changed, new branch on a live path","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Keep surface tier","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Additive","type":"text","marks":[{"type":"strong"}]}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"New code path behind a flag/new endpoint nothing calls yet, new optional field","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"−1 tier, only if you verified nothing live reaches it","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Config/infra","type":"text","marks":[{"type":"strong"}]}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Env var, feature flag default, timeout, limit, IAM/permission, pipeline step","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Keep surface tier; env/permission changes on P0 surfaces stay P0","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Non-behavioral","type":"text","marks":[{"type":"strong"}]}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Comments, docs, formatting, pure rename with all references updated, test-only","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Drop to P3, but only after verifying it truly changes no behavior","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Dependency","type":"text","marks":[{"type":"strong"}]}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Package added, upgraded, or removed","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"P1 minimum if it runs in production; P0 if it touches auth, crypto, or payments","type":"text"}]}]}]}]},{"type":"paragraph","content":[{"text":"Effective tier = the highest effective tier of any single hunk in the PR.","type":"text","marks":[{"type":"strong"}]},{"text":" Never average. A 900-line P3 docs PR that also flips one pricing constant is a P0 PR.","type":"text"}]},{"type":"paragraph","content":[{"text":"Then apply these modifiers:","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"+1 tier","type":"text","marks":[{"type":"strong"}]},{"text":" if revert cannot restore customer-visible state that already happened: data migration, backfill, deletion, Stripe/provider object mutation, published package version, or mail already blasted. A workflow config, template, or delay that has not shipped is still revertible — do not +1 it.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"+1 tier","type":"text","marks":[{"type":"strong"}]},{"text":" if the touched code has a history of failed fixes you actually found (Step 2.7). Skipping history is not a +1.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"−1 tier","type":"text","marks":[{"type":"strong"}]},{"text":" if the change is fully behind an off-by-default flag ","type":"text"},{"text":"and","type":"text","marks":[{"type":"em"}]},{"text":" you verified the flag guards every new path.","type":"text"}]}]}]},{"type":"paragraph","content":[{"text":"Record the tier as: ","type":"text"},{"text":"P1 (surface: shared vehicle-history cache, class: behavior change, modifier: +0)","type":"text","marks":[{"type":"code_inline"}]},{"text":". Show your reasoning in one line — the author must be able to argue with it.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Anti-inflation rules","type":"text"}]},{"type":"paragraph","content":[{"text":"Tiering exists to focus effort, not to make everything scary.","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Do not tier by folder name alone. Open the hunk.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Do not tier by PR size. 40 files of copy changes is P3.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Do not treat \"touches a file that imports a payment module\" as P0. Trace whether the changed lines actually reach money.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Do not raise a tier because a category ","type":"text"},{"text":"could","type":"text","marks":[{"type":"em"}]},{"text":" apply. Name the specific line that makes it apply.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"User-facing ≠ P0/P1. Loud, revertible UI and copy are P2. Elevate only when the changed lines compute or authorize money/access, or change a public contract.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Jobs, queues, and email workflows are not an automatic human gate. They stay P1 for how hard you look; they self-merge when the changed behavior is tested or observed.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"If you genuinely cannot tell what a surface governs, say so and tier at your best guess. After opening the hunk, if the lines cannot produce the worst outcome, lower the tier. An unverified silent/expensive hunk still routes to a human; an unverified email delay does not.","type":"text"}]}]}]},{"type":"hr","attrs":{"markup":"---"}},{"type":"heading","attrs":{"level":2},"content":[{"text":"Step 2.2 — ClickUp ticket correspondence (conditional)","type":"text"}]},{"type":"paragraph","content":[{"text":"Run this step ","type":"text"},{"text":"only","type":"text","marks":[{"type":"strong"}]},{"text":" when the PR title matches ","type":"text"},{"text":"\\[CU-([A-Za-z0-9-]+)\\]","type":"text","marks":[{"type":"code_inline"}]},{"text":" (e.g. ","type":"text"},{"text":"[CU-1234] Cache plate lookups","type":"text","marks":[{"type":"code_inline"}]},{"text":"). If the title has no such token, or there is no PR title, skip it: do not call ClickUp and do not look for ticket ids in the body, branch, or commits. The review proceeds as normal.","type":"text"}]},{"type":"paragraph","content":[{"text":"When the token is present:","type":"text"}]},{"type":"ordered_list","attrs":{"order":1,"listStyle":"number"},"content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Read the ticket with ","type":"text"},{"text":"clickup_get_task","type":"text","marks":[{"type":"code_inline"}]},{"text":" and ","type":"text"},{"text":"include: [\"description\"]","type":"text","marks":[{"type":"code_inline"}]},{"text":". Try ","type":"text"},{"text":"CU-\u003ctoken>","type":"text","marks":[{"type":"code_inline"}]},{"text":" as a custom id first, then the bare ","type":"text"},{"text":"\u003ctoken>","type":"text","marks":[{"type":"code_inline"}]},{"text":".","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Extract the ticket's core ask as 1–3 concrete behavior statements.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Map each ask to changed hunks. Judge the behavior, not wording or file layout.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Record a verdict: ","type":"text"},{"text":"Corresponds","type":"text","marks":[{"type":"strong"}]},{"text":", ","type":"text"},{"text":"Partial","type":"text","marks":[{"type":"strong"}]},{"text":", ","type":"text"},{"text":"Divergent","type":"text","marks":[{"type":"strong"}]},{"text":", or ","type":"text"},{"text":"Not verified","type":"text","marks":[{"type":"strong"}]},{"text":".","type":"text"}]}]}]},{"type":"paragraph","content":[{"text":"A ","type":"text"},{"text":"Divergent","type":"text","marks":[{"type":"strong"}]},{"text":" diff (it builds a different feature than the ticket asks for) or a ","type":"text"},{"text":"Partial","type":"text","marks":[{"type":"strong"}]},{"text":" diff missing the ticket's core ask is a ","type":"text"},{"text":"major","type":"text","marks":[{"type":"strong"}]},{"text":" finding that defeats the PR's stated purpose → Gate 1, BLOCKED. A vague ticket with a reasonable reading is a question, never a divergence. An unavailable or empty ticket is a note, not a gate. The check is read-only: never comment on or update the ticket.","type":"text"}]},{"type":"paragraph","content":[{"text":"Lookup order, rubric, edge cases, and output format: ","type":"text"},{"text":"references/clickup-ticket.md","type":"text","marks":[{"type":"code_inline"}]},{"text":".","type":"text"}]},{"type":"hr","attrs":{"markup":"---"}},{"type":"heading","attrs":{"level":2},"content":[{"text":"Step 2.5 — Understand the codebase before judging large changes","type":"text"}]},{"type":"paragraph","content":[{"text":"For large PRs, unfamiliar repos, or changes touching shared utilities, do not start by writing findings. First build a lightweight dependency map so the review is based on how the code actually works.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Large-repo scan flow","type":"text"}]},{"type":"paragraph","content":[{"text":"Use this flow whenever the PR changes more than ~6 files, touches shared code, or changes contracts used by other layers.","type":"text"}]},{"type":"ordered_list","attrs":{"order":1,"listStyle":"number"},"content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Find the entry points","type":"text","marks":[{"type":"strong"}]}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"UI: pages, routes, screens, forms, hooks, handlers.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"API: route handlers, server actions, controllers, webhooks, jobs.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Library/package: exported functions, package exports, public components, SDK clients.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Database/domain: mutations, repositories, schemas, migrations, validators.","type":"text"}]}]}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Trace outward from changed code","type":"text","marks":[{"type":"strong"}]}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"For each changed exported function/component/type, find current callers.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"For each changed route/event/job, find clients, tests, docs, schemas, and generated types.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"For each changed utility, check at least 2–4 representative call sites, especially high-risk ones.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"For each deleted/renamed symbol, confirm imports, config references, docs, and tests were updated.","type":"text"}]}]}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Classify files by role before reviewing them","type":"text","marks":[{"type":"strong"}]}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Source of truth:","type":"text","marks":[{"type":"strong"}]},{"text":" schema, validator, migration, domain model, API contract, permission map.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Adapter:","type":"text","marks":[{"type":"strong"}]},{"text":" provider client, database repository, webhook parser, serialization/mapping layer.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Orchestrator:","type":"text","marks":[{"type":"strong"}]},{"text":" service, mutation, job, use-case function, route handler.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Presentation:","type":"text","marks":[{"type":"strong"}]},{"text":" component, page, screen, form, copy, styling.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Utility:","type":"text","marks":[{"type":"strong"}]},{"text":" shared pure helper, formatter, parser, hook, package export.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Test/support:","type":"text","marks":[{"type":"strong"}]},{"text":" fixtures, mocks, factories, snapshots, CI config.","type":"text"}]}]}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Check contract alignment in dependency order","type":"text","marks":[{"type":"strong"}]}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Source of truth → adapter → orchestrator → presentation → tests.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"If a lower layer changes a return shape, make sure every upper layer still handles it.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"If a UI changes a submitted payload, make sure the API validator and server behavior match.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"If a validator changes, make sure test fixtures, clients, and docs were updated.","type":"text"}]}]}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Look for existing local patterns","type":"text","marks":[{"type":"strong"}]}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Search nearby files before calling something inconsistent.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Prefer the repo's established pattern over generic best practice unless the pattern is unsafe.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"When flagging consistency, name the pattern: “Other routes in ","type":"text"},{"text":"apps/api/routes/*","type":"text","marks":[{"type":"code_inline"}]},{"text":" use ","type":"text"},{"text":"requireOrgMember()","type":"text","marks":[{"type":"code_inline"}]},{"text":" before reading tenant data.”","type":"text"}]}]}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Review from highest blast radius to lowest","type":"text","marks":[{"type":"strong"}]}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Public contracts, auth, data writes, migrations, payments, webhooks.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Shared utilities/hooks/packages.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Feature-specific logic.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"UI polish and naming.","type":"text"}]}]}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Stop once evidence runs out","type":"text","marks":[{"type":"strong"}]}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Do not infer behavior from filenames alone.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Do not assume a utility is wrong without checking call sites.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"If context is missing, state the exact missing context under “Questions / assumptions” instead of inventing a finding.","type":"text"}]}]}]}]}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Utility/shared-code review flow","type":"text"}]},{"type":"paragraph","content":[{"text":"Shared utilities are dangerous because a small behavior change can silently affect many features. Review them differently from feature-local code.","type":"text"}]},{"type":"paragraph","content":[{"text":"For each changed util/hook/shared component/package export:","type":"text"}]},{"type":"ordered_list","attrs":{"order":1,"listStyle":"number"},"content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Identify whether it is public API, internal helper, or single-feature helper.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Check all call sites if there are few; sample representative call sites if there are many.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Compare old and new behavior for edge cases: ","type":"text"},{"text":"null","type":"text","marks":[{"type":"code_inline"}]},{"text":", ","type":"text"},{"text":"undefined","type":"text","marks":[{"type":"code_inline"}]},{"text":", empty string, empty array, zero, invalid date, missing env, rejected promise, duplicate call.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Check whether existing tests describe the old behavior. If the PR changes intended behavior, tests should change explicitly.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Check whether the name still matches behavior. A helper named ","type":"text"},{"text":"isValidX","type":"text","marks":[{"type":"code_inline"}]},{"text":" should not start mutating, fetching, logging, or throwing unexpectedly.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Watch for hidden contract changes: sync → async, throwing → returning null, mutable → immutable, stable order → arbitrary order, exact match → fuzzy match.","type":"text"}]}]}]},{"type":"paragraph","content":[{"text":"Report a shared-code issue only when you can connect the changed behavior to a real caller or public contract. Otherwise ask a focused question.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Connected-parts checklist","type":"text"}]},{"type":"paragraph","content":[{"text":"When a PR touches one part of a flow, quickly inspect the neighboring parts:","type":"text"}]},{"type":"table","attrs":{"layout":null},"content":[{"type":"tr","content":[{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Changed part","type":"text"}]}]},{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Also inspect","type":"text"}]}]},{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Common misses","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"UI form","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"server action/route, schema, default values, submit disabled state","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"payload mismatch, double submit, lost errors","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"API route","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"auth guard, validator, service, response type, frontend caller","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"missing server auth, inconsistent error envelope","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"DB schema/migration","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"repositories, old data, indexes, rollback/defaults","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"production old records break, slow queries","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Webhook/job","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"signature/idempotency/retry logic, logs, side effects","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"duplicate fulfillment, replay bugs","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Shared util","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"callers, tests, exported package surface","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"hidden behavior break across features","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Query/cache change","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"query keys, invalidation, pagination, optimistic updates","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"stale/cross-user data","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Config/env change","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"CI, staging/prod names, docs, fallback behavior","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"works locally only","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Dependency upgrade","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"lockfile, peer deps, build output, runtime compatibility","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"CI/build or server/client boundary break","type":"text"}]}]}]}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Context budget strategy for huge PRs","type":"text"}]},{"type":"paragraph","content":[{"text":"If the PR is too large to inspect every line deeply:","type":"text"}]},{"type":"ordered_list","attrs":{"order":1,"listStyle":"number"},"content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Review the diffstat and identify the riskiest 20% of files.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Fully review high-risk files and public contracts first.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Sample repetitive mechanical changes only after verifying the generator or pattern once.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"State “Review coverage: partial” and name what was not deeply reviewed.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Never give a 5/5 if coverage is partial on a high-risk PR.","type":"text"}]}]}]},{"type":"hr","attrs":{"markup":"---"}},{"type":"heading","attrs":{"level":2},"content":[{"text":"Step 2.6 — Harvest repo signals before judging","type":"text"}]},{"type":"paragraph","content":[{"text":"The repo already encodes who owns what and what the team cares about. Read it instead of guessing. Cheap, high-value, and it makes both the tiering and the reviewer routing concrete.","type":"text"}]},{"type":"paragraph","content":[{"text":"Look for, in this order, and only if present:","type":"text"}]},{"type":"table","attrs":{"layout":null},"content":[{"type":"tr","content":[{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Signal","type":"text"}]}]},{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Where","type":"text"}]}]},{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Use it for","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Ownership","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"CODEOWNERS","type":"text","marks":[{"type":"code_inline"}]},{"text":", ","type":"text"},{"text":".github/CODEOWNERS","type":"text","marks":[{"type":"code_inline"}]}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Who to name if a human review fires; a gate only when branch protection requires that approval","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Team rules","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"CLAUDE.md","type":"text","marks":[{"type":"code_inline"}]},{"text":", ","type":"text"},{"text":"AGENTS.md","type":"text","marks":[{"type":"code_inline"}]},{"text":", ","type":"text"},{"text":".cursor/rules/","type":"text","marks":[{"type":"code_inline"}]},{"text":", ","type":"text"},{"text":".greptile/","type":"text","marks":[{"type":"code_inline"}]},{"text":", ","type":"text"},{"text":"CONTRIBUTING.md","type":"text","marks":[{"type":"code_inline"}]}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Project-specific conventions; treat these as higher authority than generic best practice","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Protected paths","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"branch protection notes, ","type":"text"},{"text":".github/workflows/*","type":"text","marks":[{"type":"code_inline"}]},{"text":", required checks","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Which paths already demand approval — never contradict a repo policy that is stricter than your gate","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Release surface","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"package.json","type":"text","marks":[{"type":"code_inline"}]},{"text":" exports/","type":"text"},{"text":"files","type":"text","marks":[{"type":"code_inline"}]},{"text":", changesets, ","type":"text"},{"text":"openapi.*","type":"text","marks":[{"type":"code_inline"}]},{"text":", ","type":"text"},{"text":"*.proto","type":"text","marks":[{"type":"code_inline"}]},{"text":", versioned route dirs","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Whether a change is externally published","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Test/CI reality","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"CI config, test scripts, coverage config","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Whether \"has tests\" is even enforced here","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Danger annotations","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"// DANGER","type":"text","marks":[{"type":"code_inline"}]},{"text":", ","type":"text"},{"text":"// do not change","type":"text","marks":[{"type":"code_inline"}]},{"text":", ","type":"text"},{"text":"@internal","type":"text","marks":[{"type":"code_inline"}]},{"text":", ","type":"text"},{"text":"@deprecated","type":"text","marks":[{"type":"code_inline"}]},{"text":", past ","type":"text"},{"text":"TODO(incident)","type":"text","marks":[{"type":"code_inline"}]}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Institutional memory that the diff may be walking past","type":"text"}]}]}]}]},{"type":"paragraph","content":[{"text":"If ","type":"text"},{"text":"CODEOWNERS","type":"text","marks":[{"type":"code_inline"}]},{"text":" matches a changed path and the owner is not the author, name that owner when you route a human review. CODEOWNERS is a gate only if branch protection or required reviews actually demand that approval. An informational CODEOWNERS file is routing, not a veto.","type":"text"}]},{"type":"hr","attrs":{"markup":"---"}},{"type":"heading","attrs":{"level":2},"content":[{"text":"Step 2.7 — Prior-attempt forensics","type":"text"}]},{"type":"paragraph","content":[{"text":"The single highest-value check this skill can run.","type":"text","marks":[{"type":"strong"}]},{"text":" A change to code that has already been \"fixed\" two or three times is not a normal change. Either the earlier fixes patched symptoms and this one will too, or the code path has a property nobody has understood yet. Both mean look harder. Human review only when you actually find a revert, hotfix, or repeat-fix chain.","type":"text"}]},{"type":"paragraph","content":[{"text":"Run this whenever the PR is P0/P1, fixes a bug, or touches a file you can see has churn. Skip only when there is no repository to inspect.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Core questions","type":"text"}]},{"type":"ordered_list","attrs":{"order":1,"listStyle":"number"},"content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Has this exact bug or code path been fixed before?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Was any previous fix reverted, hotfixed, or immediately followed by another fix?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"What did the earlier attempt miss, and does this PR address that, or repeat it?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Is this a root-cause fix or the same symptom patch at a different call site?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Is this file a churn hotspot (many recent changes by many authors)?","type":"text"}]}]}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Command playbook","type":"text"}]},{"type":"code_block","attrs":{"wrap":false,"language":"bash"},"content":[{"text":"# Who last touched the changed lines, and in what commit\ngit blame -L \u003cstart>,\u003cend> -- \u003cfile>\ngit show \u003csha> --stat # then read the message and linked PR/issue\n\n# Every change to this file, newest first — look for clusters and repeat authors\ngit log --oneline --follow -20 -- \u003cfile>\n\n# When did this specific logic/symbol/string appear or change? (pickaxe)\ngit log -S '\u003csymbol_or_constant>' --oneline -- \u003cpath>\ngit log -G '\u003cregex>' --oneline -- \u003cpath>\n\n# Failed-fix fingerprints\ngit log --oneline -i -E --grep='revert|rollback|hotfix|re-fix|fix again|regression|incident|p0|p1' -- \u003cpath>\n\n# Churn hotspot: how volatile is this file lately?\ngit log --since='90 days ago' --oneline -- \u003cfile> | wc -l\ngit log --since='90 days ago' --format='%an' -- \u003cfile> | sort -u | wc -l\n\n# Repo-wide hotspot ranking\ngit log --since='6 months ago' --name-only --format='' | sort | uniq -c | sort -rn | head -20","type":"text"}]},{"type":"paragraph","content":[{"text":"With GitHub access, follow the thread to the human context:","type":"text"}]},{"type":"code_block","attrs":{"wrap":false,"language":"bash"},"content":[{"text":"gh pr list --state all --search '\u003ckeyword or file path>' --limit 20\ngh pr view \u003cn> --comments # what reviewers worried about last time\ngh issue list --state all --search '\u003csymptom>' --limit 20","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"How findings feed the review","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Previous fix reverted or hotfixed within days","type":"text","marks":[{"type":"strong"}]},{"text":" → +1 criticality tier and a human-review gate. Say which commit and what happened. This fires only when you found the chain.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Third or later attempt at the same behavior","type":"text","marks":[{"type":"strong"}]},{"text":" → treat \"does this fix the root cause?\" as a blocking question, not a nit. Quote the earlier attempt and name what it missed.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Reviewer concern from a past PR that this PR reintroduces","type":"text","marks":[{"type":"strong"}]},{"text":" → high-confidence finding; cite the old PR.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Churn hotspot (>8 changes / 90 days, or 4+ distinct authors)","type":"text","marks":[{"type":"strong"}]},{"text":" → raise scrutiny and note it, but this alone is not a gate.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Clean history","type":"text","marks":[{"type":"strong"}]},{"text":" → say so in one line. It is real evidence ","type":"text"},{"text":"for","type":"text","marks":[{"type":"em"}]},{"text":" self-merge and should be reported as such.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"History not checked","type":"text","marks":[{"type":"strong"}]},{"text":" → write \"History: not checked (no repository access).\" That is a note, not a gate.","type":"text"}]}]}]},{"type":"paragraph","content":[{"text":"Never fabricate history. If you did not run these commands, do not imply you did.","type":"text"}]},{"type":"paragraph","content":[{"text":"Deeper playbook, including revert-chain reconstruction and how to read a bug's fix lineage: ","type":"text"},{"text":"references/history-forensics.md","type":"text","marks":[{"type":"code_inline"}]},{"text":".","type":"text"}]},{"type":"hr","attrs":{"markup":"---"}},{"type":"heading","attrs":{"level":2},"content":[{"text":"Step 3 — Inspect in passes","type":"text"}]},{"type":"paragraph","content":[{"text":"Do the review in passes. This keeps cheaper models from missing cross-file issues.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Pass A — Intent and regression","type":"text"}]},{"type":"paragraph","content":[{"text":"Ask:","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Does the implementation actually match the PR title/description?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"If the title carries ","type":"text"},{"text":"[CU-\u003cid>]","type":"text","marks":[{"type":"code_inline"}]},{"text":", does the diff build what the ClickUp ticket asks for (Step 2.2)?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Did it remove existing behavior accidentally?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Are old callers, clients, query params, response shapes, env names, or data formats still compatible?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Is there a migration or rollout plan if the behavior changes?","type":"text"}]}]}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Pass B — Correctness and contracts","type":"text"}]},{"type":"paragraph","content":[{"text":"Ask:","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Are null/empty/error/loading states handled?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Are async operations awaited or intentionally fire-and-forget?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Are race conditions possible when users double-click, refresh, retry, or load more?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Are validators, types, runtime schemas, and DB constraints aligned?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Are API request/response shapes consistent between frontend and backend?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Are cache keys and invalidation rules correct?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Are pagination, sorting, filtering, timezone, locale, and boundary cases correct?","type":"text"}]}]}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Pass C — Security and privacy","type":"text"}]},{"type":"paragraph","content":[{"text":"Ask:","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Is every trust boundary validated server-side?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Are authorization checks enforced on the server, not only in UI?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Can a user access another tenant/user/org/resource by changing an ID?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Are secrets, tokens, API keys, cookies, headers, or provider errors logged or exposed?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Are SQL/NoSQL/shell/path/HTML/template injections possible?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Are webhooks verified and replay-protected?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Are redirects, CORS, CSRF, cookie flags, and rate limits safe for the endpoint type?","type":"text"}]}]}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Pass D — Data, migrations, and operations","type":"text"}]},{"type":"paragraph","content":[{"text":"Ask:","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Do schema changes include safe migrations/backfills/defaults?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Are indexes added for new query patterns?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Does the PR preserve existing data and handle partial/old records?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Are retries idempotent where needed?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Are background jobs observable and safe to rerun?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Will this work in staging/production envs with existing env vars and permissions?","type":"text"}]}]}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Pass E — Tests","type":"text"}]},{"type":"paragraph","content":[{"text":"Ask:","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Does new or changed business logic have meaningful tests?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Do tests cover edge cases and failure modes, not only the happy path?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Are tests deterministic: no real network, real timers, ordering assumptions, shared state leaks?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Did snapshots change for a real reason?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Are mocks too broad, hiding contract breakage?","type":"text"}]}]}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Pass F — Maintainability and consistency","type":"text"}]},{"type":"paragraph","content":[{"text":"Ask:","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Does this follow nearby project patterns for file layout, naming, errors, responses, state, imports, tests, and abstractions?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Is logic duplicated from an existing util/hook/service/mutation?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Is the code easier to change next month, or did it introduce parallel systems?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Is any abstraction premature or too generic for the use case?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Is any one-liner clever but unclear?","type":"text"}]}]}]},{"type":"paragraph","content":[{"text":"Only flag consistency if you can cite or name a concrete existing pattern.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Pass G — Simplicity, reuse, and “Ponytail” over-engineering check","type":"text"}]},{"type":"paragraph","content":[{"text":"Use this pass to catch cases where the PR builds a custom system when the codebase, standard library, platform, database, framework, or an already-installed dependency already solves the problem. This is not a style pass. It is a risk-reduction pass: less custom code means fewer bugs, fewer edge cases, and easier maintenance.","type":"text"}]},{"type":"paragraph","content":[{"text":"Run the ","type":"text"},{"text":"simplicity ladder","type":"text","marks":[{"type":"strong"}]},{"text":" before accepting new abstractions, custom state machines, retry loops, caches, formatters, validators, schedulers, wrappers, factories, or framework-like helpers. Stop at the first rung that actually satisfies the requirement:","type":"text"}]},{"type":"ordered_list","attrs":{"order":1,"listStyle":"number"},"content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Does this need to exist?","type":"text","marks":[{"type":"strong"}]},{"text":" If the requirement is speculative, flag YAGNI only if the new code creates real maintenance cost or risk.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Already in this codebase?","type":"text","marks":[{"type":"strong"}]},{"text":" Search for existing helper, hook, service, type, schema, mapper, provider client, query key factory, retry helper, logger, or error envelope.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Standard library can do it?","type":"text","marks":[{"type":"strong"}]},{"text":" Prefer built-in date/URL/array/object/promise/path/crypto helpers over custom versions.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Native platform can do it?","type":"text","marks":[{"type":"strong"}]},{"text":" Prefer HTML inputs, CSS, browser APIs, DB constraints/indexes, HTTP semantics, and framework primitives over hand-rolled behavior.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Already-installed dependency can do it?","type":"text","marks":[{"type":"strong"}]},{"text":" Reuse React Query/TanStack Query, Zod, date-fns, lodash, router utilities, ORM helpers, queue/job libraries, etc. when already present and appropriate.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Can the same behavior be expressed directly?","type":"text","marks":[{"type":"strong"}]},{"text":" Prefer a boring local function over a generic class/factory/config layer.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Only then accept custom code","type":"text","marks":[{"type":"strong"}]},{"text":", and only as small as the real requirement needs.","type":"text"}]}]}]},{"type":"heading","attrs":{"level":4},"content":[{"text":"What counts as a reportable over-engineering issue","type":"text"}]},{"type":"paragraph","content":[{"text":"Report it when all are true:","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"The PR adds meaningful custom code or abstraction.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"There is a simpler existing option in the repo, platform, stdlib, framework, DB, or installed dependency.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"The simpler option covers the actual requirement, not just a toy version.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"The custom version creates concrete risk: duplicated behavior, missed edge cases, inconsistent caching, retry bugs, stale state, harder tests, bundle/runtime cost, or future maintenance debt.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"You can show the replacement path clearly.","type":"text"}]}]}]},{"type":"paragraph","content":[{"text":"Do ","type":"text"},{"text":"not","type":"text","marks":[{"type":"strong"}]},{"text":" report it when:","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"The custom code exists because the dependency/framework cannot meet a real requirement.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"The simpler option would hide important domain logic.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"The abstraction is already an established project pattern with multiple real callers.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"The change is intentionally local and temporary with a clear ceiling.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"You only prefer a different style but cannot name a concrete risk.","type":"text"}]}]}]},{"type":"heading","attrs":{"level":4},"content":[{"text":"Common “reinvented wheel” patterns to catch","type":"text"}]},{"type":"table","attrs":{"layout":null},"content":[{"type":"tr","content":[{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Reinvented code","type":"text"}]}]},{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Prefer","type":"text"}]}]},{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Why it matters","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Manual fetch state, retries, dedupe, cache, refetch, pagination","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"TanStack Query / React Query already in repo","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Custom versions usually miss cancellation, stale data, dedupe, retry policy, invalidation, and race handling","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"useCallback","type":"text","marks":[{"type":"code_inline"}]},{"text":"/","type":"text"},{"text":"useEffect","type":"text","marks":[{"type":"code_inline"}]},{"text":" chain for derived data","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"compute during render / ","type":"text"},{"text":"useMemo","type":"text","marks":[{"type":"code_inline"}]},{"text":" only if expensive","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Avoids stale state, extra renders, and dependency bugs","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Custom form validation duplicated client/server","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"shared schema/runtime validator already used","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Prevents payload drift and inconsistent errors","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Custom date/URL/query-string parser","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"URL","type":"text","marks":[{"type":"code_inline"}]},{"text":", ","type":"text"},{"text":"URLSearchParams","type":"text","marks":[{"type":"code_inline"}]},{"text":", ","type":"text"},{"text":"Intl","type":"text","marks":[{"type":"code_inline"}]},{"text":", existing date util","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Avoids encoding/timezone/locale edge bugs","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"App-level uniqueness check only","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"DB unique constraint/index + friendly error","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Prevents races and duplicate data","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Custom polling/retry loop","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"framework/query/job retry primitive","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Prevents runaway loops, duplicate work, and inconsistent backoff","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"New wrapper around one implementation","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"direct function/component","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Avoids fake flexibility and extra indirection","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Generic config for one value","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"inline constant near use","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Avoids “architecture for maybe later”","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"New dependency for a tiny helper","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"stdlib or existing dependency","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Reduces bundle, audit, install, and maintenance cost","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Duplicated response/error mapping","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"existing API envelope/helper","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Prevents client contract drift","type":"text"}]}]}]}]},{"type":"heading","attrs":{"level":4},"content":[{"text":"Severity for simplicity issues","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"major","type":"text","marks":[{"type":"strong"}]},{"text":": custom code can cause real incorrect behavior, data/security/payment risk, duplicated network calls, stale/cross-user cache, broken retries, or public API drift.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"minor","type":"text","marks":[{"type":"strong"}]},{"text":": custom code works today but creates meaningful maintenance cost, duplicates an existing repo pattern, or makes future changes likely to diverge.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"nit","type":"text","marks":[{"type":"strong"}]},{"text":": tiny readability simplification with no meaningful risk. Do not score nits.","type":"text"}]}]}]},{"type":"heading","attrs":{"level":4},"content":[{"text":"Required wording for simplicity findings","type":"text"}]},{"type":"paragraph","content":[{"text":"A simplicity issue must name the simpler path and the exact risk in the current code.","type":"text"}]},{"type":"paragraph","content":[{"text":"Bad: “This is over-engineered. Use React Query.”","type":"text"}]},{"type":"paragraph","content":[{"text":"Good: “This hook reimplements query state, retry, and request dedupe even though this repo already uses TanStack Query for API reads. When two components mount at once, both call ","type":"text"},{"text":"fetchInvoices()","type":"text","marks":[{"type":"code_inline"}]},{"text":" and race to set local state; ","type":"text"},{"text":"useQuery({ queryKey: ['invoices', orgId], queryFn })","type":"text","marks":[{"type":"code_inline"}]},{"text":" would dedupe, centralize retry policy, and align with the existing cache invalidation flow.”","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Pass H — Break analysis: what could this take down?","type":"text"}]},{"type":"paragraph","content":[{"text":"Findings say \"this line is wrong.\" Break analysis says \"here is what stops working if this is wrong.\" They are different products and the author needs both. Run this pass on every P0/P1 PR, and on any P2 PR that changes shared behavior.","type":"text"}]},{"type":"paragraph","content":[{"text":"For each behavior or contract change, work outward:","type":"text"}]},{"type":"ordered_list","attrs":{"order":1,"listStyle":"number"},"content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Who consumes this?","type":"text","marks":[{"type":"strong"}]},{"text":" Direct callers, then their callers, until you hit a user-facing or externally-observable boundary. Prefer grepping imports and call sites over reasoning from names.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"What is the failure mode?","type":"text","marks":[{"type":"strong"}]},{"text":" Loud (throws, 500s, failing test) or silent (wrong number, stale cache, missing record, subtly different rounding)? Silent failures on P0 surfaces are the most dangerous thing you can find.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Who notices, and when?","type":"text","marks":[{"type":"strong"}]},{"text":" Immediately at deploy, on the next billing cycle, at month-end reconciliation, only when a customer complains?","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"What detects it?","type":"text","marks":[{"type":"strong"}]},{"text":" Name the test, alert, log, dashboard, or health check. \"Nothing currently detects this\" is itself a finding worth reporting.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"How is it undone?","type":"text","marks":[{"type":"strong"}]},{"text":" Code revert, flag flip, migration rollback, manual data repair, or provider-side cleanup?","type":"text"}]}]}]},{"type":"paragraph","content":[{"text":"Report only scenarios you can ground in the code. Two well-traced scenarios beat eight imagined ones. Format:","type":"text"}]},{"type":"table","attrs":{"layout":null},"content":[{"type":"tr","content":[{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"What could break","type":"text"}]}]},{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Trigger","type":"text"}]}]},{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Who's affected","type":"text"}]}]},{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Detection","type":"text"}]}]},{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Rollback","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Overage invoices double-count usage","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Any account crossing plan quota after deploy","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Paying customers on metered plans","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"None — no alert on invoice delta","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Code revert + reissue affected invoices","type":"text"}]}]}]}]},{"type":"paragraph","content":[{"text":"If a row has \"Detection: none\" and \"Rollback: manual data repair\" on a silent/expensive path, demand observation (a test asserting the value, a run, a screenshot of the flow). That is a verification requirement. It is not an automatic human stamp if you already observed the behavior.","type":"text"}]},{"type":"paragraph","content":[{"text":"Also state what this PR ","type":"text"},{"text":"cannot","type":"text","marks":[{"type":"strong"}]},{"text":" break, when it is genuinely bounded — \"changes are confined to the admin export screen; no shared modules, contracts, or persisted data are touched.\" That sentence is what earns a self-merge.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Pass I — Observation: can this be watched working?","type":"text"}]},{"type":"paragraph","content":[{"text":"For every behavior change, decide whether it was ","type":"text"},{"text":"observed","type":"text","marks":[{"type":"strong"}]},{"text":" or merely ","type":"text"},{"text":"reasoned about","type":"text","marks":[{"type":"strong"}]},{"text":", and label it. Reasoned-about behavior on a silent/expensive path (money, entitlement, authz, data deletion, breaking public contract) is an unknown and routes to a human. Reasoned-about P2 copy, UI, or workflow timing is a note, not a gate.","type":"text"}]},{"type":"paragraph","content":[{"text":"Ranked evidence, strongest first:","type":"text"}]},{"type":"ordered_list","attrs":{"order":1,"listStyle":"number"},"content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Direct observation","type":"text","marks":[{"type":"strong"}]},{"text":" — the endpoint was called and the response read; the script was run; the test was executed and its output seen; the screenshot shows the state.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Existing automated check","type":"text","marks":[{"type":"strong"}]},{"text":" — a test in the diff that provably exercises the changed path, verified by reading its assertions rather than its name.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Static verification","type":"text","marks":[{"type":"strong"}]},{"text":" — types, schema alignment, exhaustive match, a compiler or validator that would fail on the mistake.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Traced reasoning","type":"text","marks":[{"type":"strong"}]},{"text":" — you followed the code and it should work.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"The author's claim","type":"text","marks":[{"type":"strong"}]},{"text":" — worth nothing on its own.","type":"text"}]}]}]},{"type":"paragraph","content":[{"text":"Where you can run something in the environment, run it. Where you cannot, produce an ","type":"text"},{"text":"observation plan","type":"text","marks":[{"type":"strong"}]},{"text":" the author or reviewer can execute in under a minute:","type":"text"}]},{"type":"code_block","attrs":{"wrap":false,"language":"md"},"content":[{"text":"**Observe it:** `curl -s localhost:3000/api/v1/plate/ABC123 | jq .confidence`\nExpect a number between 0 and 1; before this change the field was absent.","type":"text"}]},{"type":"paragraph","content":[{"text":"Rules:","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"A test that asserts the function was called is not observation. A test that asserts the ","type":"text"},{"text":"value","type":"text","marks":[{"type":"em"}]},{"text":" is.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"UI screenshots are preferred evidence, not a gate. For checkout, login, or auth flows, unverified visual behavior is an unknown on a silent/expensive path. For other UI, full coverage plus tests can still self-merge; say the visual behavior is unverified.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Migrations are observed against a copy of real data shapes, not an empty dev database.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"If nothing in the PR can be observed without a full manual environment, that is itself worth saying — it usually means the change should be split.","type":"text"}]}]}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"When a PR is too big to observe: recommend splitting","type":"text"}]},{"type":"paragraph","content":[{"text":"If the diff is large enough that you cannot form an opinion on each hunk, do not do a partial review and gate on it. Recommend decomposition instead, and be specific about the seams:","type":"text"}]},{"type":"blockquote","content":[{"type":"paragraph","content":[{"text":"🔀 Consider splitting.","type":"text","marks":[{"type":"strong"}]},{"text":" 1,400 lines across 31 files mixes a schema migration, a new provider client, and the dashboard wiring. As a stack — migration, then client with its own tests, then UI — each layer is independently runnable, and each lands on behavior already verified below it. As one PR, a mistake in the migration is only visible after everything merges.","type":"text"}]}]},{"type":"paragraph","content":[{"text":"Guidance: a PR that is one concern and under roughly 400 changed lines can usually be observed end to end. Above that, review quality drops and partial coverage starts gating PRs that would otherwise be fine. Splitting is not a rejection — say so.","type":"text"}]},{"type":"hr","attrs":{"markup":"---"}},{"type":"heading","attrs":{"level":2},"content":[{"text":"Review taxonomy","type":"text"}]},{"type":"paragraph","content":[{"text":"Use these categories in findings.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Correctness & Regression","type":"text"}]},{"type":"paragraph","content":[{"text":"Logic bugs, broken flows, missing edge cases, incompatible behavior changes, wrong conditions, stale state, pagination/filtering/sorting bugs, timezone/locale mistakes, double-submit issues, race conditions.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Security & Privacy","type":"text"}]},{"type":"paragraph","content":[{"text":"Auth/authz holes, IDOR, injection, unsafe redirects, CSRF/CORS/cookie mistakes, leaked secrets, unsafe logs, missing webhook verification, sensitive data exposure, missing rate limits on abuse-prone endpoints.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Data & Migrations","type":"text"}]},{"type":"paragraph","content":[{"text":"Unsafe schema changes, missing defaults/backfills, missing indexes, data loss, non-idempotent jobs, broken old-record compatibility, transaction/atomicity issues.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"API & Contracts","type":"text"}]},{"type":"paragraph","content":[{"text":"Request/response shape mismatches, wrong status codes, missing runtime validation, breaking exported APIs, inconsistent error envelopes, unversioned contract changes.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Error Handling & Observability","type":"text"}]},{"type":"paragraph","content":[{"text":"Swallowed errors, misleading user errors, leaking internals, missing cleanup, missing logging/metrics/tracing for important failure paths, unhelpful retries.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Tests","type":"text"}]},{"type":"paragraph","content":[{"text":"Missing tests for changed business logic, weak tests that assert implementation details, flaky tests, mocks that hide real integration contracts, missing regression tests for fixed bugs.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Performance & Scalability","type":"text"}]},{"type":"paragraph","content":[{"text":"N+1 queries, expensive hot-path work, unnecessary serialization, unbounded loops, memory leaks, excessive polling, bad cache invalidation, render storms. Only flag if likely to matter.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Maintainability & Simplicity","type":"text"}]},{"type":"paragraph","content":[{"text":"Overly complex functions, deep nesting, unclear names, magic values, dead/commented code, boolean parameter traps, premature abstractions, too many responsibilities.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Over-engineering & Reuse","type":"text"}]},{"type":"paragraph","content":[{"text":"Custom code that duplicates the codebase, standard library, platform, database, framework, or installed dependency; unnecessary wrappers/factories/classes/config layers; hand-rolled fetching/caching/retry/form/date/query-string logic where existing primitives are safer.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"DRY & Reuse","type":"text"}]},{"type":"paragraph","content":[{"text":"Copy-paste logic, duplicated constants, reimplementing existing utilities, parallel data structures, multiple sources of truth.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Dependencies & Build","type":"text"}]},{"type":"paragraph","content":[{"text":"Unnecessary dependency, risky version change, peer dependency conflict, tree-shaking issue, circular import, bundling/server-client boundary issue, package export breakage, CI/build config regression.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Accessibility & UX","type":"text"}]},{"type":"paragraph","content":[{"text":"Missing labels/roles/keyboard support, broken focus states, inaccessible error messages, bad loading/empty/error states, layout that breaks important user flows. Use this mostly for UI PRs.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Codebase Consistency","type":"text"}]},{"type":"paragraph","content":[{"text":"Breaks established local patterns for naming, file structure, state management, response shape, error handling, imports, tests, or domain boundaries.","type":"text"}]},{"type":"hr","attrs":{"markup":"---"}},{"type":"heading","attrs":{"level":2},"content":[{"text":"Severity calibration","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"critical","type":"text"}]},{"type":"paragraph","content":[{"text":"A production break, security/privacy hole, data loss/corruption risk, payment/auth bypass, or migration/deployment issue that can break real users. Blocks merge.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"major","type":"text"}]},{"type":"paragraph","content":[{"text":"A real behavior risk, important missing test around changed business logic, significant maintainability debt, contract mismatch, meaningful DRY violation, or high-likelihood bug. Should be fixed in this PR.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"minor","type":"text"}]},{"type":"paragraph","content":[{"text":"A real issue with low blast radius, limited edge case, small consistency drift, or readability problem that will slow future work. Fix soon, but not always blocking.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"nit","type":"text"}]},{"type":"paragraph","content":[{"text":"Pure style, naming polish, tiny readability preference, or optional cleanup. Never blocks.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Escalation rules","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"In auth, payments, data deletion, migrations, tenant isolation, secrets, and public APIs, small mistakes can become critical. Escalate ","type":"text"},{"text":"only when the impact is concrete","type":"text","marks":[{"type":"strong"}]},{"text":".","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Do not automatically convert every major in a sensitive area to critical. That creates false positives.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"For internal scripts/prototypes, downgrade architecture/style concerns unless they affect safety or repeated use.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"For generated code, review the generator/config, not the generated output, unless the generated output is directly committed as source of truth.","type":"text"}]}]}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Triage every finding into one of three buckets","type":"text"}]},{"type":"paragraph","content":[{"text":"Severity says how bad. Triage says what happens next. Tag each finding:","type":"text"}]},{"type":"table","attrs":{"layout":null},"content":[{"type":"tr","content":[{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Bucket","type":"text"}]}]},{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Meaning","type":"text"}]}]},{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Disposition","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Actionable","type":"text","marks":[{"type":"strong"}]}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"You can name the defect and the fix","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Goes in Issues, ordered by severity","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Nit","type":"text","marks":[{"type":"strong"}]}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Real but optional; no risk you can name","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Goes under Nits, max 5, never affects score or gate","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Ambiguous","type":"text","marks":[{"type":"strong"}]}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Might be a defect; depends on context you do not have (product intent, an unwritten constraint, a decision made elsewhere)","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Goes under Questions. Triggers human review only when answering it would change safety — amounts, entitlements, or access whose intent the PR never states","type":"text"}]}]}]}]},{"type":"paragraph","content":[{"text":"The ambiguous bucket is what keeps the other two honest. Without it, uncertain findings get rounded either into false positives or into silence. Name the specific missing context — \"is the old rate meant to apply to in-flight billing periods?\" — not \"please double-check this.\"","type":"text"}]},{"type":"hr","attrs":{"markup":"---"}},{"type":"heading","attrs":{"level":2},"content":[{"text":"False positive prevention","type":"text"}]},{"type":"paragraph","content":[{"text":"Do ","type":"text"},{"text":"not","type":"text","marks":[{"type":"strong"}]},{"text":" report:","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Linter/formatter-only issues unless they hide a real bug.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"A preference without project evidence.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Speculative performance issues without a hot path, scale condition, or clear waste.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Existing unrelated problems not caused by the PR.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Missing tests for trivial glue, copy-only changes, or dead-simple UI wiring.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"\"Could be cleaner\" without explaining the maintenance cost.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"A new abstraction just because code appears twice; duplication can be acceptable until behavior stabilizes.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Dependency concerns if the dependency is already installed and used similarly.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Type nits that do not improve safety or readability.","type":"text"}]}]}]},{"type":"paragraph","content":[{"text":"When uncertain, write a question instead of a finding.","type":"text"}]},{"type":"hr","attrs":{"markup":"---"}},{"type":"heading","attrs":{"level":2},"content":[{"text":"Scoring rubric","type":"text"}]},{"type":"paragraph","content":[{"text":"Start at 5. Apply deductions, then apply hard caps.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Deductions","type":"text"}]},{"type":"table","attrs":{"layout":null},"content":[{"type":"tr","content":[{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Deduction","type":"text"}]}]},{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Condition","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"−3","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Each critical issue","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"−1","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Each major issue, capped at −2 total","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"−0.5","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Three or more minor issues","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"−0.5","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Meaningful missing tests for changed business logic","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"−0.5","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Significant DRY/reuse or over-engineering issue","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"−0.5","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Two or more codebase consistency issues","type":"text"}]}]}]}]},{"type":"paragraph","content":[{"text":"Nits do not affect score.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Hard caps","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Any critical issue: max score ","type":"text"},{"text":"2/5","type":"text","marks":[{"type":"strong"}]},{"text":".","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Two or more critical issues: max score ","type":"text"},{"text":"1/5","type":"text","marks":[{"type":"strong"}]},{"text":".","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Any unprotected auth/admin/payment/data-deletion route: max score ","type":"text"},{"text":"1/5","type":"text","marks":[{"type":"strong"}]},{"text":".","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Any major issue that breaks the main purpose of the PR: max score ","type":"text"},{"text":"2/5","type":"text","marks":[{"type":"strong"}]},{"text":".","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"No changed business logic tests in a high-risk PR: max score ","type":"text"},{"text":"3/5","type":"text","marks":[{"type":"strong"}]},{"text":", even if code looks correct.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Only nits/minor style issues: minimum score ","type":"text"},{"text":"4/5","type":"text","marks":[{"type":"strong"}]},{"text":".","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"No meaningful issues found: ","type":"text"},{"text":"5/5","type":"text","marks":[{"type":"strong"}]},{"text":".","type":"text"}]}]}]},{"type":"paragraph","content":[{"text":"Round down after deductions unless a hard cap applies. Minimum score is 1.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Score meanings","type":"text"}]},{"type":"table","attrs":{"layout":null},"content":[{"type":"tr","content":[{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Score","type":"text"}]}]},{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Label","type":"text"}]}]},{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Verdict","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"5/5","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Excellent","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"✅ Ready to merge","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"4/5","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Good","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"✅ Ready to merge, optional polish","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"3/5","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Acceptable","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"❌ Merge after fixing listed issues","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"2/5","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Needs work","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"❌ Request changes","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"1/5","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Not ready","type":"text"}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"❌ Request changes","type":"text"}]}]}]}]},{"type":"paragraph","content":[{"text":"The score is about code quality. It is not the merge decision.","type":"text","marks":[{"type":"strong"}]},{"text":" A 5/5 pricing change self-merges when the PR states the new amount and a test or run asserts it. A 5/5 pricing change with unexplained intent still needs a human. Compute the score, then run the gate below independently.","type":"text"}]},{"type":"hr","attrs":{"markup":"---"}},{"type":"heading","attrs":{"level":2},"content":[{"text":"Step 4 — Merge decision gate","type":"text"}]},{"type":"paragraph","content":[{"text":"Output exactly one of three verdicts. Decide by running the gates in order — first match wins. Do not skip to a conclusion and reverse-engineer the reasoning.","type":"text"}]},{"type":"table","attrs":{"layout":null},"content":[{"type":"tr","content":[{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Verdict","type":"text"}]}]},{"type":"th","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Meaning","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"🛑 ","type":"text"},{"text":"BLOCKED","type":"text","marks":[{"type":"strong"}]}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"Do not merge. Something in the diff is wrong or unverifiable.","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"👥 ","type":"text"},{"text":"HUMAN REVIEW REQUIRED","type":"text","marks":[{"type":"strong"}]}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"The code may be fine, but a second person must approve. Always name who and what to look at.","type":"text"}]}]}]},{"type":"tr","content":[{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"✅ ","type":"text"},{"text":"SELF-MERGE OK","type":"text","marks":[{"type":"strong"}]}]}]},{"type":"td","attrs":{"colspan":1,"rowspan":1,"colwidth":null,"alignment":""},"content":[{"type":"paragraph","content":[{"text":"The author can merge without another reviewer, once CI is green.","type":"text"}]}]}]}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Gate invariants","type":"text"}]},{"type":"paragraph","content":[{"text":"These hold no matter what the diff looks like or what the author says:","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Asymmetric on unknowns, not surfaces.","type":"text","marks":[{"type":"strong"}]},{"text":" Unverified money, entitlement, authz, data deletion, or a breaking public contract routes to a human. A verified P0 change with no findings does not.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Fail closed on required inputs only.","type":"text","marks":[{"type":"strong"}]},{"text":" Unreadable diff, unknown CI, or merge conflicts escalate. Skipped history, unnamed CODEOWNER, or an incomplete consumer grep do not.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Open the hunk, then you may lower the tier.","type":"text","marks":[{"type":"strong"}]},{"text":" Path, folder, and \"this file is near billing\" are reasons to look. They are not a deny-list.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Never recommend merging on someone's behalf.","type":"text","marks":[{"type":"strong"}]},{"text":" The verdict is a recommendation to the author; approving, requesting changes, and merging are human actions.","type":"text"}]}]}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Gate 0 — Mechanical pre-checks (review reliability)","type":"text"}]},{"type":"paragraph","content":[{"text":"Cheap, deterministic, and run first. Failures here mean you cannot issue a confident self-merge on skimmed code — recommend a split, or condition the verdict on CI — not \"human review because the path looks scary\":","type":"text"}]},{"type":"checkbox_list","attrs":{"id":null},"content":[{"type":"checkbox_item","attrs":{"checked":false},"content":[{"type":"paragraph","content":[{"text":"PR has no merge conflicts and is up to date enough with the base branch to be meaningful.","type":"text"}]}]},{"type":"checkbox_item","attrs":{"checked":false},"content":[{"type":"paragraph","content":[{"text":"No existing \"changes requested\" review sitting unresolved.","type":"text"}]}]},{"type":"checkbox_item","attrs":{"checked":false},"content":[{"type":"paragraph","content":[{"text":"CI is green, or the verdict is explicitly conditional on it.","type":"text"}]}]},{"type":"checkbox_item","attrs":{"checked":false},"content":[{"type":"paragraph","content":[{"text":"Diff is within observable bounds: roughly ","type":"text"},{"text":"≤500 changed lines and ≤20 files","type":"text","marks":[{"type":"strong"}]},{"text":", or larger only when the change is mechanically repetitive and you verified the pattern.","type":"text"}]}]},{"type":"checkbox_item","attrs":{"checked":false},"content":[{"type":"paragraph","content":[{"text":"The diff was fully available to read.","type":"text"}]}]}]},{"type":"paragraph","content":[{"text":"Size is a proxy for review reliability, not risk. A one-line pricing change is P0 scrutiny with a high observation bar; it is not a path deny. Above these bounds, recommend a split rather than issuing a confident verdict on code you skimmed.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Gate 1 — Blockers (any one → BLOCKED)","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Any unresolved ","type":"text"},{"text":"critical","type":"text","marks":[{"type":"strong"}]},{"text":" finding.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"A ","type":"text"},{"text":"major","type":"text","marks":[{"type":"strong"}]},{"text":" finding that defeats the PR's stated purpose — including a diff that diverges from the ","type":"text"},{"text":"[CU-\u003cid>]","type":"text","marks":[{"type":"code_inline"}]},{"text":" ticket in the title (Step 2.2).","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Auth, authorization, or tenant-isolation check missing on a path that reads or writes data.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Secret, key, token, or credential committed or logged.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Destructive migration or data-deleting code without a verified rollback or backup path.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Money movement or price computation that you can show is wrong.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"The diff is materially unavailable — you were asked to approve code you could not read.","type":"text"}]}]}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Gate 2 — Human review required (any one → HUMAN REVIEW REQUIRED)","type":"text"}]},{"type":"paragraph","content":[{"text":"Verification and unresolved risk, not surface labels. P0/P1 does ","type":"text"},{"text":"not","type":"text","marks":[{"type":"strong"}]},{"text":" fire this gate by itself:","type":"text"}]},{"type":"ordered_list","attrs":{"order":1,"listStyle":"number"},"content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Unverified behavior on a silent/expensive path","type":"text","marks":[{"type":"strong"}]},{"text":" — money movement, entitlement, authz/tenant scope, data deletion, or a breaking public contract — and nothing observed it (no test asserting the value/check, no run, no screenshot of that flow).","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Unresolved major","type":"text","marks":[{"type":"strong"}]},{"text":", or an ","type":"text"},{"text":"ambiguous product question that would change safety","type":"text","marks":[{"type":"strong"}]},{"text":" (e.g. \"is 3 cents the intended overage?\" when the PR never says so). Ordinary nits and \"please double-check\" do not count.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Partial coverage","type":"text","marks":[{"type":"strong"}]},{"text":" — the diff was not fully read, or the PR is too mixed to review; prefer split over a fake human gate.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Known-bad history on the touched lines","type":"text","marks":[{"type":"strong"}]},{"text":" — revert/hotfix/repeat-fix chain you actually found (Step 2.7). Skipping history is a note, not a gate.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Repo policy requires a human","type":"text","marks":[{"type":"strong"}]},{"text":" — branch protection or required CODEOWNERS approvals. Name that policy. An informational CODEOWNERS file is not this.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Breaking external contract with unknown consumers","type":"text","marks":[{"type":"strong"}]},{"text":" — rename, remove, default, or type change on a published API/SDK. Additive optional fields you confirmed are optional are not this.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Irreversible side effect that was not verified","type":"text","marks":[{"type":"strong"}]},{"text":" — blast email, Stripe object mutate, published package, destructive migrate. Unsendability is a verification demand (content, recipients, trigger), not an automatic human stamp.","type":"text"}]}]}]},{"type":"paragraph","content":[{"text":"If none fire and Gate 1 is clean → continue to Gate 3, including P0 and including code you authored in this session (disclose authorship; do not auto-escalate).","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Gate 3 — Self-merge (requires ALL to be true)","type":"text"}]},{"type":"paragraph","content":[{"text":"P2/P3 is not a prerequisite. Self-merge is the default when the review is complete:","type":"text"}]},{"type":"checkbox_list","attrs":{"id":null},"content":[{"type":"checkbox_item","attrs":{"checked":false},"content":[{"type":"paragraph","content":[{"text":"Gate 0 mechanical pre-checks all pass (or the PR is oversized only because it is mechanically repetitive and you verified the pattern).","type":"text"}]}]},{"type":"checkbox_item","attrs":{"checked":false},"content":[{"type":"paragraph","content":[{"text":"No Gate 2 trigger fired.","type":"text"}]}]},{"type":"checkbox_item","attrs":{"checked":false},"content":[{"type":"paragraph","content":[{"text":"No critical and no major findings. Minors and nits only.","type":"text"}]}]},{"type":"checkbox_item","attrs":{"checked":false},"content":[{"type":"paragraph","content":[{"text":"Review coverage is ","type":"text"},{"text":"full","type":"text","marks":[{"type":"strong"}]},{"text":" — you read every changed hunk, not a sample.","type":"text"}]}]},{"type":"checkbox_item","attrs":{"checked":false},"content":[{"type":"paragraph","content":[{"text":"Silent/expensive changed behavior was observed (test assertions read, a run, or a screenshot of the flow). Other behavior was observed, statically verified, or non-behavioral (docs, copy, comments, formatting, test-only, generated output from an unchanged verified generator).","type":"text"}]}]},{"type":"checkbox_item","attrs":{"checked":false},"content":[{"type":"paragraph","content":[{"text":"Blast radius is bounded and you can state the boundary in one sentence.","type":"text"}]}]},{"type":"checkbox_item","attrs":{"checked":false},"content":[{"type":"paragraph","content":[{"text":"No unresolved safety-ambiguous questions (amounts, entitlements, or access whose intent the PR never states).","type":"text"}]}]}]},{"type":"paragraph","content":[{"text":"If all pass, say so plainly and briefly — including for a verified pricing or auth tweak, an email workflow delay, and agent-authored P2/P0 code you observed. Do not manufacture a reason to hedge — an over-cautious gate trains the author to ignore the gate.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Writing the decision","type":"text"}]},{"type":"paragraph","content":[{"text":"Always include, in this order:","type":"text"}]},{"type":"ordered_list","attrs":{"order":1,"listStyle":"number"},"content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"The verdict.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"The effective tier and the one-line reason for it.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Why","type":"text","marks":[{"type":"strong"}]},{"text":" — the specific gate that fired, quoted concretely (\"Gate 2.1: ","type":"text"},{"text":"packages/billing/src/plan-limits.ts","type":"text","marks":[{"type":"code_inline"}]},{"text":" changes the overage multiplier and no test asserts the new cents\"), or \"No gate fired\" on self-merge.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Who should review","type":"text","marks":[{"type":"strong"}]},{"text":" — a named CODEOWNER if one exists; otherwise the engineer ","type":"text"},{"text":"git blame","type":"text","marks":[{"type":"code_inline"}]},{"text":" and recent history show has actually worked in this code, named specifically; otherwise the role (\"whoever owns billing\", \"someone with production DB access\"). Never just \"a senior engineer\". Omit if self-merge.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"What that reviewer should focus on","type":"text","marks":[{"type":"strong"}]},{"text":" — 1–3 specific things, not \"review the PR\". Omit if self-merge.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"How to observe it working","type":"text","marks":[{"type":"strong"}]},{"text":" — the command to run and the output to expect, when the change is observable.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"How long it should take","type":"text","marks":[{"type":"strong"}]},{"text":" — a rough estimate signals you understand the change (\"~10 minutes, two files matter\").","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"What would clear the gate","type":"text","marks":[{"type":"strong"}]},{"text":" — the concrete verification or stated intent that would make this self-mergeable (\"add a test asserting the overage multiplier at the plan boundary\"). Never \"this surface always routes to a human.\"","type":"text"}]}]}]},{"type":"paragraph","content":[{"text":"Point 8 matters: the gate should teach, not just block.","type":"text"}]},{"type":"heading","attrs":{"level":3},"content":[{"text":"Honesty constraints on the gate","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"This verdict is ","type":"text"},{"text":"advisory","type":"text","marks":[{"type":"strong"}]},{"text":". It never overrides branch protection, required approvals, or org policy. If repo config is stricter, defer to it and say so.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Never recommend self-merge to skip observation or to skip a real blocker. Urgency changes how fast a reviewer looks, not whether an unverified silent/expensive change needs one. A verified P0 can self-merge.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Never soften a blocker because the author pushes back. Re-examine the evidence; if the evidence holds, the verdict holds.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"If you skipped history or repo signals, state it in the decision — do not let a silent skip look like a clean result, and do not treat the skip as a Gate 2 trigger.","type":"text"}]}]}]},{"type":"paragraph","content":[{"text":"Worked examples of gate calls, including tricky near-misses: ","type":"text"},{"text":"references/merge-gate.md","type":"text","marks":[{"type":"code_inline"}]},{"text":".","type":"text"}]},{"type":"hr","attrs":{"markup":"---"}},{"type":"heading","attrs":{"level":2},"content":[{"text":"Follow-up reviews (re-review after fixes)","type":"text"}]},{"type":"paragraph","content":[{"text":"When the author returns with fixes, do ","type":"text"},{"text":"not","type":"text","marks":[{"type":"strong"}]},{"text":" re-review from scratch and do not re-litigate settled points. Run the delta loop:","type":"text"}]},{"type":"ordered_list","attrs":{"order":1,"listStyle":"number"},"content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Diff the new state against the state you reviewed (","type":"text"},{"text":"git diff \u003cold_sha>..\u003cnew_sha>","type":"text","marks":[{"type":"code_inline"}]},{"text":").","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"For each previous finding, mark: ","type":"text"},{"text":"Fixed","type":"text","marks":[{"type":"strong"}]},{"text":" / ","type":"text"},{"text":"Partially fixed","type":"text","marks":[{"type":"strong"}]},{"text":" / ","type":"text"},{"text":"Not addressed","type":"text","marks":[{"type":"strong"}]},{"text":" / ","type":"text"},{"text":"Fixed but introduced something new","type":"text","marks":[{"type":"strong"}]},{"text":". Verify against code — never take \"done\" on faith.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Review only the new hunks with the full pass set.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Re-run criticality: fixes can raise the tier (a \"small fix\" that adds a migration is now P1+).","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Re-run the merge gate from scratch. A previously blocked PR does not become self-mergeable just because the blocker was fixed — the new code has to pass the gate on its own.","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Report only: resolution status, new findings, updated verdict. Keep it short.","type":"text"}]}]}]},{"type":"paragraph","content":[{"text":"Stop the loop when the gate returns SELF-MERGE OK or HUMAN REVIEW REQUIRED with no blockers. Do not keep hunting for new nits to justify another round.","type":"text"}]},{"type":"hr","attrs":{"markup":"---"}},{"type":"heading","attrs":{"level":2},"content":[{"text":"Output format","type":"text"}]},{"type":"paragraph","content":[{"text":"Always write the review in this structure.","type":"text"}]},{"type":"code_block","attrs":{"wrap":false,"language":"md"},"content":[{"text":"## PR Review: [PR title, branch, or concise filename summary]\n\n### Merge decision\n\n**[✅ SELF-MERGE OK / 👥 HUMAN REVIEW REQUIRED / 🛑 BLOCKED]**\n\n- **Criticality:** [P0/P1/P2/P3] — [surface], [change class][, +1 modifier: reason]\n- **Why:** [the specific gate that fired, with the file or fact that triggered it]\n- **Reviewer:** [named CODEOWNER, blame-familiar engineer, or role — omit if self-merge]\n- **Focus on:** [1–3 concrete things — omit if self-merge]\n- **Observe it:** [command to run + expected output — omit if not observable]\n- **Effort:** [~N minutes, which files matter]\n- **To self-merge next time:** [the concrete verification or stated intent that would clear the gate — never \"this surface always routes to a human\"]\n\n[If oversized: add a 🔀 **Consider splitting** line naming the seams.]\n\n### Score\n\n**[✅ for 4/5 or 5/5; ❌ for 1/5–3/5] [N]/5 — [Label]**\n\n[One direct sentence explaining what drove the score. Score is code quality; the merge\ndecision above is separate.]\n\n---\n\n### Summary\n\n[2–4 sentences. Say what the PR does, what is good, and the main concern.]\n\n---\n\n### Ticket correspondence\n\n[Only when the PR title contains `[CU-\u003cid>]`; omit otherwise.]\n\n**[CU-\u003cid>] [ticket name]** — [link if returned] — **Corresponds / Partial / Divergent / Not verified**\n\n[Quote the ticket's core ask, then name the behavior the diff implements and where —\nor what it built instead. If not verified, say why.]\n\n---\n\n### What this could break\n\n[Table of grounded scenarios: what breaks / trigger / who's affected / detection /\nrollback. Omit the table for P3 and say instead: \"Bounded — [one-sentence boundary].\"]\n\n---\n\n### Prior attempts in this code path\n\n[Only when history was checked AND is relevant. State findings with commit SHAs or PR\nnumbers: earlier fixes, reverts, hotfix chains, what the last attempt missed, and\nwhether this PR addresses it. If clean, one line: \"No prior fix attempts or reverts on\nthese paths in the last 6 months.\" If not checked: \"Not checked — no repository\naccess.\"]\n\n---\n\n### Issues\n\n[If no issues: \"No blocking or meaningful issues found.\"]\n\n**[severity] [category] — path/to/file.ext:line_or_function**\n**Confidence:** High/Medium\n\n[Explain: When X happens, Y breaks because Z. Tie it to user/dev impact.]\n\n```language\n// ❌ Current\n[short relevant snippet]\n\n// ✅ Suggested\n[short suggested fix]\n```","type":"text"}]},{"type":"paragraph","content":[{"text":"[Repeat issues ordered critical → major → minor.]","type":"text"}]},{"type":"heading","attrs":{"level":4},"content":[{"text":"Nits","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"path:line","type":"text","marks":[{"type":"code_inline"}]},{"text":" — [one-line optional cleanup]","type":"text"}]}]}]},{"type":"hr","attrs":{"markup":"---"}},{"type":"heading","attrs":{"level":3},"content":[{"text":"Questions / assumptions","type":"text"}]},{"type":"paragraph","content":[{"text":"[Ambiguous findings and material assumptions. Name the missing context specifically. Do not use this to avoid making a clear call on things you can determine yourself.]","type":"text"}]},{"type":"hr","attrs":{"markup":"---"}},{"type":"heading","attrs":{"level":3},"content":[{"text":"Verification","type":"text"}]},{"type":"paragraph","content":[{"text":"[What was observed vs. reasoned about. E.g. \"Ran the plate-normalization test suite — passes. The provider retry path was traced but not executed; no test covers it.\" Omit for P3.]","type":"text"}]},{"type":"hr","attrs":{"markup":"---"}},{"type":"heading","attrs":{"level":3},"content":[{"text":"What's done well","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"[Specific genuine praise tied to code. Omit this section if nothing specific.]","type":"text"}]}]}]},{"type":"hr","attrs":{"markup":"---"}},{"type":"heading","attrs":{"level":3},"content":[{"text":"Upgrade roadmap","type":"text"}]},{"type":"paragraph","content":[{"text":"To reach 4/5","type":"text","marks":[{"type":"strong"}]}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"[Must-fix items. If already 4+ say \"Already at or above 4/5.\"]","type":"text"}]}]}]},{"type":"paragraph","content":[{"text":"To reach 5/5","type":"text","marks":[{"type":"strong"}]}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"[Polish/test/consistency items. If already 5 say \"Already 5/5. Nothing required.\"]","type":"text"}]}]}]},{"type":"hr","attrs":{"markup":"---"}},{"type":"heading","attrs":{"level":3},"content":[{"text":"Stats","type":"text"}]},{"type":"bullet_list","content":[{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Files changed: N","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Criticality: [tier] ([highest-tier file])","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Diff size: N lines / N files","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Behavior verified by: [observation / tests read / static checks / reasoning only]","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Merge decision: [verdict]","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Review coverage: [full / partial, and why if partial]","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"History check: [done / not available]","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"ClickUp ticket: [CU-\u003cid> — corresponds / partial / divergent / not verified, or \"not in title\"]","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Score: N/5","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Issues: N total (critical: N, major: N, minor: N, nits: N)","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Highest risk area: [area]","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Categories flagged: [list]","type":"text"}]}]},{"type":"list_item","content":[{"type":"paragraph","content":[{"text":"Not verified: [what you could not check — omit if nothing]","type":"text"}]}]}]},{"type":"code_block","attrs":{"wrap":false,"language":""},"content":[{"text":"\n### Output rules\n\n- Keep issues concise. One strong paragraph plus a fix is enough.\n- Do not include giant snippets. Show only the relevant lines.\n- If there are many findings, prioritize the top 10 by severity and impact. Put the rest under \"Additional notes\" only if useful.\n- Never say \"LGTM\" if you did not review enough context. Say \"No issues found in the provided diff\" instead.\n- Be direct: \"This can create duplicate charges\" is better than \"This might possibly be problematic.\"\n- **Lead with the merge decision.** It is the first thing the author reads and the only thing some authors read.\n- **Nit budget: 5 maximum.** Nits never affect the score or the gate. If you have more than five, you are linting. Cut to the five that would actually annoy a reader.\n- Do not repeat the gate reasoning in the summary. State it once, in the decision block.\n- Never state a verdict the evidence does not support in order to sound decisive. \"P1, human review required, and here is the one thing I could not verify\" is a stronger answer than false certainty in either direction.\n- The score icon is deterministic: use ✅ for 4/5 or 5/5 and ❌ for 1/5, 2/5, or 3/5.\n\n---\n\n## Verdict calibration\n\nRead `references/merge-gate.md` when you are unsure about a verdict.\n\n---\n\n## Simplicity quick-reference checklist\n\nBefore finalizing, run this mini-check for every non-trivial new abstraction or helper:\n\n1. Is this solving a real current requirement, not “maybe later”?\n2. Did I search for an existing repo helper/pattern first?\n3. Would stdlib/native/platform/database/framework solve this with fewer edge cases?\n4. Is an already-installed dependency meant to do this exact job?\n5. Does the custom code duplicate caching, retries, validation, parsing, formatting, state management, or authorization?\n6. Can I name a concrete risk if this custom code stays?\n7. Can I suggest a smaller replacement that preserves behavior?\n\nIf the answer to 6 or 7 is no, do not report it as an issue.\n\n---\n\n## Reviewer self-check before final answer\n\nBefore finalizing, ask yourself:\n\n1. Did I only flag PR-introduced or PR-worsened issues?\n2. Does every issue have a trigger, impact, cause, and fix?\n3. Did I avoid linter/style noise?\n4. Did I check contracts across files where relevant?\n5. Would a senior engineer accept each finding as useful?\n6. Could a cheaper model follow the output without guessing?\n\n### Gate self-check\n\n7. Did I assign the tier from what the changed lines actually govern, opening the hunks rather than reading folder names?\n8. Did I take the **highest** hunk's tier, not an average, and apply +1 irreversible only when revert cannot restore already-happened customer state?\n9. Did I check prior attempts, or explicitly say I could not — without treating a skip as a gate?\n10. Is every break-analysis row grounded in traced code rather than imagined?\n11. Did I run the gates in order, and can I quote the exact gate that fired?\n12. If I said SELF-MERGE OK: does every Gate 3 condition pass, and was silent/expensive behavior actually observed — including a verified P0?\n13. If I said HUMAN REVIEW REQUIRED: did I name a specific reviewer or role, specific focus areas, and the concrete verification that would clear the gate — not \"this surface always needs a human\"?\n14. Am I gating on unverified risk or a real finding, or on folder name / \"it's user-facing\" / \"it's an email\"? Vagueness is my problem to resolve, not the author's to absorb.\n15. Is the merge decision consistent with the findings and the stated coverage — not merely with the tier?\n16. If the PR title had `[CU-\u003cid>]`, did I read the ticket and compare its ask to the diff's behavior — and if the title had no token, did I skip ClickUp entirely?\n\nIf not, revise before sending.\n\nA condensed tick-list version of every gate in this skill, for use while reviewing\nrather than after: `references/checklist.md`.","type":"text"}]}]},"metadata":{"date":"2026-11-02","name":"pr-review","author":"@skillopedia","source":{"stars":0,"repo_name":"skills","origin_url":"https://github.com/carsxe/skills/blob/HEAD/.agents/skills/pr-review/SKILL.md","repo_owner":"carsxe","body_sha256":"b8999fce8fcaef717668ae1f657bf92ee52f40c5fff706856c5ed1cb824c026c","cluster_key":"a32f3321d6c9190fb2ad6161289d8ebb636735822904b6d831d6e1dabb70967b","clean_bundle":{"format":"clean-skill-bundle-v1","source":"carsxe/skills/.agents/skills/pr-review/SKILL.md","text_bytes":69532,"source_commit":"560ce70daf2c993de2627fc3dd3345c988ec4416","skills_sh_rank":14988,"attachment_count":5,"text_attachments":5,"attachment_storage":"skill_attachments","binary_attachments":0},"cluster_size":1,"skill_md_path":".agents/skills/pr-review/SKILL.md","import_metadata":{"date":"2026-11-02","author":"@skillopedia","version":"v1","category":"software-engineering","category_label":"Engineering"},"exact_dupes_collapsed_into_this":0},"version":"v1","category":"software-engineering","import_tag":"skills-sh-top20k-2026-11","description":"Performs a high-signal senior-engineer PR review of code changes, classifies how critical the changed surfaces are, and decides whether the PR can be self-merged or must go to a human reviewer. Use this skill for pull request reviews, branch diffs, pasted diffs, uncommitted changes, GitHub PR URLs, or requests like \"review my changes\", \"check this PR\", \"is this ready\", \"can I merge this\", \"does this need a reviewer\", \"how risky is this change\", \"what could this break\", \"audit this diff\", \"sanity check before I push\", or \"does this look clean\". Covers correctness, security, contracts, blast radius, prior failed fixes in the same code path, tests, maintainability, and regressions while aggressively avoiding noisy false positives. When the PR title contains `[CU-\u003cticket number>]`, reads the ClickUp ticket and verifies the diff builds the feature the ticket describes. Designed to work reliably even with smaller/cheaper models.\n"}},"renderedAt":1793672043341}

Important: agents should read /llm.txt, /llms.txt, or /.well-known/skills.json to discover the public Skillopedia API.