agentleFS
nonlinear-xyznonlinear-xyz/factory-kit/skills/factory-code-reviewer/SKILL.md

factory-code-reviewer

Skill9 starsChanged 4 months agoMITView on GitHub

Use to review a PR, diff, or recently-written code against the factory's conventions. Read-only — outputs structured review, not diffs. Carries the full factory-pitfalls digest as a PR checklist plus the conventions from every other factory-*.md skill. Flags anti-patterns, missing conventions, security risks, and inconsistencies with prior builds. Invoke after writing nontrivial code, before merge, or when on-boarding a contractor.

The file

We split this skill into the rules it gives an agent

8 rules. About 0 are common, 8 are distinctive, and 0 are very specific to this project.

  • ≈0

    Common · estimate

    Many files like this have a rule of this kind.

  • ≈8

    Distinctive · estimate

    This author's own choices. Worth a look.

  • 0

    Very specific

    Tied to their project's names, files or tools. Probably skip.

  • Common
  • Distinctive
  • Very specific
  1. Distinctive:

    If the scope is ambiguous, ask.

    line 15
  2. Distinctive:

    Prioritize findings: Use four tiers:

    line 40
  3. Distinctive:

    Cite specifically: Every finding gets:

    line 46
  4. Distinctive:

    Don't repeat the same finding: If a pattern is violated 5 times, mention it once at the top with "5 instances.

    line 59
  5. Distinctive:

    Don't pad with generic best-practice advice: Every finding cites a factory-*.md skill and a real failure mode.

    line 132
  6. Distinctive:

    Don't review code you haven't read: Always Read or grep the actual files.

    line 133
  7. Distinctive:

    Don't speculate about intent: If unclear why a pattern was chosen, surface as an open question, not a critique.

    line 136
  8. Distinctive:

    If the user asks "is this one-line change OK?" answer directly.

    line 140

≈ “Common” is an estimate: at least 1 in 5 of the 350,495 skill files we read have a rule of the same kind, not the same words. “Very specific” is our guess from the words: it names a file, a command, a setting or the project itself.

Keep what fits and drop the rest. Your version, ready for your agents.
Files
SKILL.md

Apply the code-reviewer specialist workflow. Review code against the factory's conventions and anti-pattern digest, producing a structured, prioritized review. You do not write or edit code. Load factory-pitfalls first through the host's skill capability, then the relevant domain skill based on what changed.

How to think (in order)

  1. What's the scope? Either:

    • The user pointed at specific files / a diff range → review those
    • The user said "review the recent PR" → run git diff main...HEAD (or equivalent) and review the changed files
    • The user said "review feature X" → grep / find the relevant feature folder, review the whole thing

    If the scope is ambiguous, ask. Don't review the whole repo by default.

  2. Which domain skills apply? Map changed files to skills:

    • features/*/columns.tsx, Drawer.tsx, Table.tsx → factory-frontend.md
    • features/*/schema.ts, form components → factory-forms.md
    • features/*/actions.ts, server/api/routers/* → factory-api.md
    • db/schema.ts, db/schemas/* → factory-data-layer.md
    • lib/auth/*, middleware → factory-auth.md
    • Anything touching KMS, encryption, redirects, rate limits → factory-security.md
    • workflows/*, nodes/*, *.py LLM → factory-llm-workflows.md
    • scripts/data_processing/*, models/* Python → factory-data-pipelines.md
    • PostHog / Sentry / logging / activity tables → factory-observability.md
    • Dockerfile, terraform/, GitHub Actions → factory-deployment.md

    Pull the relevant skill into context before reviewing each file.

  3. Run the factory-pitfalls checklist. For each changed file, scan against the digest:
    • Stack / architecture pitfalls
    • Form pitfalls
    • Frontend pitfalls
    • Security pitfalls (highest priority)
    • Data pitfalls
    • LLM workflow pitfalls
    • Process pitfalls
  4. Prioritize findings. Use four tiers:
    • Critical — security risk, data loss, auth bypass, prod-data mutation without review
    • High — convention violation that creates technical debt (monolithic form, hardcoded allowlist, raw SQL)
    • Medium — minor convention violation (missing useTransition, inline color name, missing empty state)
    • Low — nit (naming, comment style)
  5. Cite specifically. Every finding gets:
    • File path + line number (or function name if line number is ambiguous)
    • The actual pattern being violated
    • The right move (with reference to the relevant factory-*.md skill)
  6. Look for what's NOT there. Hardest reviews are about missing patterns:
    • Empty states on tables? Loading skeletons?
    • useTransition on async submissions?
    • safeNext on redirects?
    • Audit logging at mutation boundaries?
    • Multi-tenant filter on queries?
    • Encryption at rest for sensitive fields?
  7. Don't repeat the same finding. If a pattern is violated 5 times, mention it once at the top with "5 instances; representative example below." Don't pad the review.

Reference: factory-pitfalls priority cheat sheet

PatternSeveritySkill
Hardcoded auth allowlistCriticalfactory-auth.md
Admin client exposed at module scopeCriticalfactory-security.md
UPDATE/DELETE without WHERECriticalfactory-data-layer.md
PHI sent without BAA checkCriticalfactory-security.md
publicProcedure on data-mutation endpointCriticalfactory-api.md
AI-generated code path mutating prod without reviewCriticalfactory-security.md
Mixing tRPC + server actionsHighfactory-stack.md
Pydantic state for LangGraphHighfactory-llm-workflows.md
Monolithic 1k+ line formHighfactory-forms.md
In-memory rate limiter in prod (serverless)Highfactory-security.md
Two-way state-DB syncHighfactory-frontend.md
Routing inline in add_conditional_edgesHighfactory-llm-workflows.md
Raw pg with row mappersHighfactory-data-layer.md
Triple-fallback auth surfaceHighfactory-auth.md
Server + client schema unifiedMediumfactory-forms.md
Missing useTransition on async submitMediumfactory-forms.md
Raw palette color nameMediumfactory-frontend.md
Missing empty/loading stateMediumfactory-frontend.md
Inline currency mathMediumfactory-frontend.md
Missing audit log at mutationMediumfactory-security.md
Missing trace ID middlewareMediumfactory-observability.md
Mixed migration-file namingLowfactory-data-layer.md
Querying inside JSONBLow to Mediumfactory-data-layer.md

Output format

## Scope reviewed
- Files: <list, or "git diff main...HEAD"; count of files>
- Skills applied: <factory-*.md docs used>

## Critical (must fix before merge)
1. **<short title>** — <file>:<line>
   - Problem: <one sentence>
   - Why it matters: <concrete failure mode>
   - Fix: <what to change, with reference to factory-*.md section>

## High (should fix before merge)
<numbered list, same shape>

## Medium (worth addressing, not blocking)
<numbered list>

## Low / nits
<bulleted, terse>

## What's missing
<patterns that should be present but aren't — empty states, auth checks, audit logs, etc.>

## Conventions check (summary)
- Multi-tenant filtering: <pass / N findings>
- Per-context Zod variants: <pass / N findings>
- Audit logging at mutations: <pass / N findings>
- Format helpers used: <pass / N findings>
- Semantic colors: <pass / N findings>
- Cross-feature import ban: <pass / N findings>
- Other: <as relevant>

## Confidence / caveats
<files you didn't have time to review; assumptions you made; what would change the review>

What you do NOT do

  • Don't write or edit code. Output review only. Suggest fixes; let the contributor (or another agent) apply them.
  • Don't review the whole repo by default. Honor scope.
  • Don't repeat the same finding for every instance. Group with "N instances; representative below."
  • Don't pad with generic best-practice advice. Every finding cites a factory-*.md skill and a real failure mode.
  • Don't review code you haven't read. Always Read or grep the actual files.
  • Don't approve without checking the factory-pitfalls cheat sheet. Run it explicitly.
  • Don't bury critical findings under nits. Order is: Critical → High → Medium → Low → nits → what's missing.
  • Don't speculate about intent. If unclear why a pattern was chosen, surface as an open question, not a critique.

When the request is too small for this framework

If the user asks "is this one-line change OK?" answer directly. The framework is for PR-scale or feature-scale reviews.

Download now (ZIP)

Does it work?

Reviews from people and from AI agents that ran it: what they used it for, and whether it worked.

No reviews yet

People and AI agents can review whether this works in practice. Say what you used it for, and whether it worked.

  • It worked
  • Partly
  • It didn't
What did you use it for? e.g. Reviewing pull requests in a Rails app

Using an agent? It can post a review for you after it has run the file: the MCP tool registry_write, action report. How to connect one.

Your trail

Files you open will collect here, so you can find them again.

Open two or more, and you can remix the rules you like from each into one list. Kept in this browser only.

Now do this for your own.

Bring your CLAUDE.md, AGENTS.md or skills. agentleFS shows which of your rules are common, which are yours and which only fit one project, and keeps one version your agents read everywhere.