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.
In this repository42 other files it gives its agents.
- guidance/claude.mdCLAUDE.md
- factory-animationSkill
- factory-api-route-engineerSkill
- factory-apiSkill
- factory-authSkill
- factory-auth-wiring-specialistSkill
- factory-ciSkill
- factory-closeSkill
- factory-commitsSkill
- factory-data-layerSkill
- factory-data-pipeline-engineerSkill
- factory-data-pipelinesSkill
- factory-db-migration-engineerSkill
- factory-db-migrationSkill
- factory-db-schema-architectSkill
- factory-deploymentSkill
- factory-designSkill
- factory-entrySkill
- factory-feature-architectSkill
- factory-forms-builderSkill
- factory-formsSkill
- factory-frontend-engineerSkill
- factory-frontendSkill
- factory-kit-auditSkill
- factory-llm-workflow-engineerSkill
- factory-llm-workflowsSkill
- factory-observabilitySkill
- factory-pitfallsSkill
- factory-promptingSkill
- factory-promptSkill
- factory-releaseSkill
- factory-security-engineerSkill
- factory-securitySkill
- factory-setup-linearSkill
- factory-stackSkill
- factory-standupSkill
- factory-state-modelingSkill
- factory-submitSkill
- factory-testingSkill
- factory-verification-engineerSkill
- factory-verificationSkill
- factory-voiceSkill
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
- Distinctive: line 15
If the scope is ambiguous, ask.
- Distinctive: line 40
Prioritize findings: Use four tiers:
- Distinctive: line 46
Cite specifically: Every finding gets:
- Distinctive: line 59
Don't repeat the same finding: If a pattern is violated 5 times, mention it once at the top with "5 instances.
- Distinctive: line 132
Don't pad with generic best-practice advice: Every finding cites a factory-*.md skill and a real failure mode.
- Distinctive: line 133
Don't review code you haven't read: Always Read or grep the actual files.
- Distinctive: line 136
Don't speculate about intent: If unclear why a pattern was chosen, surface as an open question, not a critique.
- Distinctive: line 140
If the user asks "is this one-line change OK?" answer directly.
≈ “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.
Files
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)
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.
Which domain skills apply? Map changed files to skills:
features/*/columns.tsx,Drawer.tsx,Table.tsx→factory-frontend.mdfeatures/*/schema.ts, form components →factory-forms.mdfeatures/*/actions.ts,server/api/routers/*→factory-api.mddb/schema.ts,db/schemas/*→factory-data-layer.mdlib/auth/*, middleware →factory-auth.md- Anything touching KMS, encryption, redirects, rate limits →
factory-security.md workflows/*,nodes/*,*.pyLLM →factory-llm-workflows.mdscripts/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.
- 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
- 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)
- 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-*.mdskill)
- Look for what's NOT there. Hardest reviews are about missing patterns:
- Empty states on tables? Loading skeletons?
useTransitionon async submissions?safeNexton redirects?- Audit logging at mutation boundaries?
- Multi-tenant filter on queries?
- Encryption at rest for sensitive fields?
- 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
| Pattern | Severity | Skill |
| Hardcoded auth allowlist | Critical | factory-auth.md |
| Admin client exposed at module scope | Critical | factory-security.md |
UPDATE/DELETE without WHERE | Critical | factory-data-layer.md |
| PHI sent without BAA check | Critical | factory-security.md |
publicProcedure on data-mutation endpoint | Critical | factory-api.md |
| AI-generated code path mutating prod without review | Critical | factory-security.md |
| Mixing tRPC + server actions | High | factory-stack.md |
| Pydantic state for LangGraph | High | factory-llm-workflows.md |
| Monolithic 1k+ line form | High | factory-forms.md |
| In-memory rate limiter in prod (serverless) | High | factory-security.md |
| Two-way state-DB sync | High | factory-frontend.md |
Routing inline in add_conditional_edges | High | factory-llm-workflows.md |
Raw pg with row mappers | High | factory-data-layer.md |
| Triple-fallback auth surface | High | factory-auth.md |
| Server + client schema unified | Medium | factory-forms.md |
Missing useTransition on async submit | Medium | factory-forms.md |
| Raw palette color name | Medium | factory-frontend.md |
| Missing empty/loading state | Medium | factory-frontend.md |
| Inline currency math | Medium | factory-frontend.md |
| Missing audit log at mutation | Medium | factory-security.md |
| Missing trace ID middleware | Medium | factory-observability.md |
| Mixed migration-file naming | Low | factory-data-layer.md |
| Querying inside JSONB | Low to Medium | factory-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.
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
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.

