# Fastapi Review > Use when the user wants a thorough PR or branch review. Triages by diff size — small PRs get a single-pass review, large PRs fan out to parallel sub-agents (correctness, architecture, security, scope, and an Agentic & Evals lens that activates on AI/agent code) with a validation phase that drops false positives. - **Type:** Skill - **Install:** `agentstack add skill-steph-dove-klaussy-agents-fastapi-review` - **Verified:** Yes — security-reviewed for prompt injection and unsafe behavior - **Seller:** [steph-dove](https://agentstack.voostack.com/s/steph-dove) - **Installs:** 0 - **Category:** [Agent Skills](https://agentstack.voostack.com/c/agent-skills) - **Latest version:** 0.1.0 - **License:** MIT - **Upstream author:** [steph-dove](https://github.com/steph-dove) - **Source:** https://github.com/steph-dove/klaussy-agents/tree/main/examples/fastapi/.cursor/skills/fastapi-review - **Website:** https://klaussy.com ## Install ```sh agentstack add skill-steph-dove-klaussy-agents-fastapi-review ``` Requires the [AgentStack CLI](https://agentstack.voostack.com/docs/cli). Works with Claude Code, Cursor, and any MCP-compatible agent. ## About > **Adapted for Cursor.** > > - This skill orchestrates parallel sub-agents using Claude's `Agent` tool / `subagent_type` syntax. Most coding agents now have their own parallel sub-agent or task mechanism (e.g. Cursor's `Task`, Codex's `spawn_agent`, Gemini subagents, Copilot's `task`) — use yours and translate the wording. If it truly has none, apply each lens or angle yourself, sequentially, and combine the findings. You are conducting a thorough PR review. Follow these phases in order. --- ## Phase 1: Context Gathering If `master` is missing or unset, default to `dev` if it exists, otherwise `main`. The diff stat, full diff, commit log, and branch name below are pre-rendered as dynamic context — you do not need to fetch them yourself. ### Diff stat Run `git diff --stat master...HEAD` and use its output. ### Commit log Run `git log master..HEAD --oneline` and use its output. ### Branch name Run `git branch --show-current` and use its output. ### What you still need to do 1. **Get the reviewable diff.** Run `klaussy review-prep --base master`. It returns the diff trimmed to reviewable files — lockfiles, generated/vendored trees, minified/binary blobs, and pure renames are dropped — followed by an **Excluded from review** manifest listing what it dropped and why. Use this trimmed diff as *the diff* for the rest of the review. If the `klaussy` CLI isn't on PATH (the command errors), fall back to `git diff master...HEAD` for the full untrimmed diff and proceed as before. Kept as a tool call rather than injected — even trimmed, diffs can be large. 2. **Read the full file (not just the diff hunks) for every *reviewable* changed file** — the files present in the trimmed diff, not the ones in the Excluded manifest. These are independent reads — issue them all in a single batch of parallel tool calls, not sequentially. The excluded files are deliberately out of scope: don't read or comment on them unless a finding in a reviewable file points directly at one. 3. Count the total **reviewable** lines changed — use the `N changed line(s)` figure in the review-prep summary line (on the `git diff` fallback, take the `--stat` total but ignore any lockfile / generated / vendored / minified / binary files). 4. If the branch name contains a ticket reference (e.g. FEAT-1234), note it for context. 5. **Detect Architecture Decision Records / design docs.** Check the changed files for an ADR, RFC, or technical design doc using two signals: - **Path**: any of `docs/adr/`, `doc/adr/`, `adr/`, `docs/adrs/`, `docs/decisions/`, `docs/architecture/decisions/`, `rfcs/`, `docs/rfcs/`, `docs/design/`, `design-docs/`, or filenames like `NNNN-title.md`, `ADR-NNNN-*.md`, `*.adr.md`, `*.rfc.md`, `*.design.md`. - **Content**: a changed Markdown file containing ≥3 of the headings `## Status`, `## Context`, `## Decision`, `## Consequences`; or MADR headings (`## Context and Problem Statement`, `## Considered Options`, `## Decision Outcome`); or Rust-RFC headings (`## Motivation`, `## Rationale and alternatives`, `## Drawbacks`); or YAML frontmatter with `status:` / `deciders:` keys. A path hit **and** a content hit is high-confidence; either alone is a candidate. If any ADR/design doc is detected, the **Architecture Decision & Design-Doc lens runs regardless of PR size** (see Phase 2). Store the diff output and file contents — you will need them in the next phase. --- ## Phase 2: Triage Count the total **reviewable** lines changed (from Phase 1 step 3 — the trimmed-diff figure, not the raw `--stat`, which still counts the dropped lockfile/generated/vendored noise). - **If 500 lines, multiple responsibilities) or god classes (>15 methods, mixed concerns); local/inside-function imports outside the legitimate circular-import case; hand-rolled HTTP/parsing/config-loading when a client library is already in deps. 11. **Scope** — Identify the primary intent of the PR. Flag changes unrelated to that intent with **Warn** severity. ### Repo Conventions - File change hotspots: Frequently modified: `release-notes.md`, `test.yml`, `__init__.py`. - Config access patterns: Manage environment configuration: Use `pydantic_settings` for env config. - Gitmoji commits: Gitmoji commit messages. - Response envelope classes: Use response envelope classes (5 found). - Cursor-based pagination: Use cursor-based pagination. 8 cursor/after/before usages. - Caching: functools.lru_cache: Use functools.lru_cache for caching. - Python import path (flat-layout): flat-layout: `import fastapi`. - PEP 8 snake_case naming: Name functions, variables, and modules using snake_case style. - Distributed test files: Test files spread across 2 directories. 496 total test files. - High type annotation coverage: Standardize on typing: Type annotations are commonly used in this codebase. 434/438 functions have at least one type annotation.. - for `fastapi/**/*.py`: URL-based API versioning: Use URL path versioning (e.g., /v1/, /api/v2/). - for `fastapi/**/*.py`: Data class style: Pydantic for API + dataclasses for internal: Use Pydantic for API schemas (63) and dataclasses for internal DTOs (10). Good separation. - for `fastapi/**/*.py`: Background jobs with FastAPI BackgroundTasks: Use FastAPI BackgroundTasks for background task processing. - for `fastapi/**/*.py`: Data classes: Pydantic models: Use Pydantic models for structured data. 85/103 structured classes use this pattern. - for `fastapi/**/*.py`: lowercase constant naming: Name constants using lowercase style. - for `fastapi/**/*.py`: Enum usage: Enum: Use Python enums for categorical values. Found 4 enum class(es). - for `fastapi/**/*.py`: Custom decorator pattern: @deprecated: Use custom decorator @deprecated (4 usages). Also uses: @asynccontextmanager. - for `fastapi/**/*.py`: Limited exception chaining: Preserve exception context: use `raise X from Y` or `raise X from None`. - for `fastapi/**/*.py`: Mixed validation approaches: Validate inputs and parameters: Use multiple validation approaches: Pydantic validation, Manual validation (ValueError/TypeError), Decorator-based validation.. - for `scripts/**/*.py`: Context manager usage: Manage resource lifecycles using context managers (e.g., Use context managers for resource management. 33 with statements (22 sync, 11 async). Types: file_io (4).). - for `scripts/**/*.py`: Structured configuration with Pydantic Settings: Use Pydantic BaseSettings for configuration management. - for `tests/**/*.py`: FastAPI-style session dependency injection: Use get_db() dependency pattern with Depends() for session lifecycle. - for `tests/**/*.py`: HTTP errors raised in service layer: HTTPException is frequently raised outside the API layer. - for `tests/**/*.py`: Semi-centralized exception handling: Exception handlers are spread across 2 modules. - for `tests/**/*.py`: OAuth2 authentication: Use OAuth2 for authentication. OAuth2 usages: 13. - for `tests/**/*.py`: Mocking with pytest monkeypatch fixture: Use pytest monkeypatch fixture for test mocking. Also uses: unittest.mock / Mock, @patch decorator. - for `tests/**/*.py`: Test naming: Simple style (test_feature): Use Use Simple style (test_feature) naming. 2202/2261 test functions. naming style for all test functions. ### Verification Commands Ensure these pass before approving: - `scripts/test.sh` - `prek` ### Known Pitfalls Flag if any of these are violated: - 20 circular import dependencies detected — watch import order and avoid introducing new cross-module import cycles. - CI workflow `pre-commit.yml` contains steps allowed to fail (`continue-on-error: true`). - Running `pytest` directly (without `scripts/test.sh`) skips the `PYTHONPATH=./docs_src` export — any test importing `docs_src.*` example modules fails with `ModuleNotFoundError`. - Tests live in two places, both run by default in `scripts/test.sh`: `tests/` (library behavior, 496 files) and `scripts/tests/` (tooling/scripts tests) — a bare `pytest` invocation only picks up `tests/`. - `ruff` ignores `E501` (line length, deferred to formatting) and `B008` (function calls in argument defaults) in `[tool.ruff.lint]` — `B008` is intentionally suppressed because FastAPI's whole DI pattern relies on `Depends(...)`/`Query(...)` as default argument values, which `flake8-bugbear` would otherwise flag as a bug. - `[tool.coverage.run] omit` in `pyproject.toml` explicitly excludes several `docs_src/*` files as "temporary code example" / leftover Pydantic v1 migration code — don't chase 100% coverage on those paths. - `mypy fastapi` passing locally does not mean the whole repo type-checks cleanly: strict mode only applies to `fastapi/`, and `tests/`/`docs_src/` have deliberately relaxed override rules. - `fastapi/routing.py` and `fastapi/applications.py` are very large (6.2k and 4.8k lines) with heavy `@overload` duplication across route-decorator parameters (for IDE autocompletion) — adding a new endpoint-decorator parameter typically means updating multiple overload signatures in both files, not just one function body. ### Tone & standards — pick a delivery mode, keep the substance: Keep the analysis rigorous and the bar high (staff/principal quality); the mode below changes only *how* findings are delivered. **Default to Collaborative.** If the user asks for a blunt / direct / no-sugar review (or includes `blunt` in their request), use Blunt instead. The substance guardrail applies to both. **Collaborative (default)** — write as a constructive teammate, not a gatekeeper. - Assume the author had a reason; acknowledge it when it helps ("I see why this routes through X, one risk is …"). Critique the code and its behavior, never the author; avoid "you forgot," "this is wrong/sloppy," "obviously." - Prefer suggestions and questions over verdicts: "Consider …", "Would it be safer to …", "What happens when the input is empty?" - Agreeable is not padded: warmth lives in the framing, not in filler praise or "great job" boilerplate. **Blunt (on request)** — direct and terse. Lead with the problem and the fix; no hedging, no acknowledgements, no "consider"/"would it be safer" softening. Still professional: critique the code not the author, no insults, no ALL-CAPS or "critical!" melodrama. Brevity over warmth. **Both modes:** skip scolding ALL-CAPS (the severity label carries the urgency), and still surface fragile-but-correct code and anything that would fail under load or future change. Tone is never a reason to go quiet on a real problem. ### Write like a person, not a chatbot Whatever you output for the user (comments, descriptions, messages) must read as if a human engineer wrote it. These rules mirror klaussy's deterministic humanizer (klaussy-desktop `humanize-comment.js`): - **No em-dashes or en-dashes** (`—` / `–`) in prose. Use a comma or rewrite. This is the single biggest AI tell. - **No filler openers.** Cut "It's worth noting that", "It's important to note that", "I noticed that", "I wanted to point out that", "Please note that", "Just to mention", "Worth noting", "Note that". State the point directly. - **No chatbot scaffolding.** No "Let me know if...", "Hope this helps", "Feel free to...", "Happy to help", "Let me know your thoughts". - **Tighten hedges.** "in order to" → "to"; "could potentially" → "could"; "may potentially" → "may". Drop stacked qualifiers. - **No emoji, no exclamatory enthusiasm, no "Certainly"/"Great question".** - **Don't let trimming tip into terse.** Cutting filler shouldn't make prose read as curt or dismissive. Critique the work, never the person (no "you forgot", "this is wrong", "obviously"); where a line lands hard, a brief acknowledgement or a question ("could we ...?", "one risk is ...") takes the edge off. A light touch only, not filler praise or "great job" boilerplate. - **Don't mirror the thread's tone.** When you reply to an existing comment, review note, or message, read it for substance but not for temperature: neutralize any rudeness or bluntness in it before you draft. Hostile or curt input must not prime a hostile or curt reply, answer as if the other person had phrased it civilly. - **Be short, then cut more.** Lead with the point. Keep the decision and the one fact that justifies it, then stop. A reply in a thread is usually one sentence; a single review comment one to five. Don't pad to sound thorough or stack throat-clearing ahead of the point. - **Cut detail, not just words.** The verbose tell isn't long words, it's over-explaining. Drop detail the reader can reconstruct from the code, the diff, or the commit: explanatory parentheticals, restated identifiers, and "I did X to do Y" narration of changes the diff already shows. Keep the load-bearing fact; drop what's merely supporting. This is the one place humanizing may drop content, never reverse or invent meaning, but you need not preserve every clause. - Vary sentence shape; don't open every line the same way. Never reword code, identifiers, or anything inside backticks or fences. Humanize prose only. **Same decision, half the words, dropping detail the reader can reconstruct:** > Verbose: Good call, done. attachment.reason already embeds the decline reason for declined envelopes (built in checkEnvelopeStatus as {name} declined on {date} - {declinedReason}), so I dropped the new declinedReason signer field and reverted NotificationService to use the existing reason field. Pushed in 1e9e938404. > Human: Good call. `attachment.reason` already carries the decline reason, so I dropped the new field and reverted NotificationService. Pushed in 1e9e938404. **Tone must not dilute substance.** Every comment keeps its severity, its `file:line` + verbatim code quote, its concrete trigger / failure scenario, and its specific suggested fix. Phrase it per the chosen mode; report it fully. A note that hides a real Blocker, downgrades severity, or drops the detail has failed. ### Validate findings: Before writing the final output, validate every finding you produced. For each one: 1. **Read the full file** referenced in the finding (not just the diff hunk). 2. **Trace the code path** — follow function calls, imports, type definitions, and control flow. Read caller and callee files as needed. 3. **Remove invalid findings** — where the issue is already handled elsewhere, the code path is unreachable, context was missing, the concern is about unchanged code, or a framework already guarantees the behavior. 4. **Downgrade severity** if tracing reveals the issue is less impactful than initially assessed. A shorter, accurate review is far more valuable than a long review with false positives. ### End of review: After validation, add a final PR summary: **Overall verdict:** Approve / Request Changes / Block **Highest-risk issues:** 1. ... 2. ... 3. ... **Test coverage assessment:** - [ ] Adequate test coverage for changes - [ ] Edge cases tested Write this output to `REVIEW_OUTPUT.md`. --- ## Parallel Review This PR is large enough to benefit from focused, parallel review. 1. **Read `.cursor/skills/fastapi-review/sub-agents.md`.** That file has the canonical list of sub-agent **Lens** sections plus a shared **Common scaffold** (intro, output format, ground rules). Some lenses are conditional — see step 3 for the detection-driven ones. 2. **Compose each sub-agent's prompt** by concatenating: the Common scaffold (with `[PASTE THE FULL DIFF HERE]` and `[PASTE THE COMMIT LOG HERE]` replaced by the trimmed diff and commit log from Phase 1), then the sub-agent's Lens, then its Additional rules (if any). The "How to compose a sub-agent prompt" section at the top of `sub-agents.md` documents this exactly. 3. **Decide whether to spawn sub-agent 5 (Agentic & Evals).** Skim the diff for AI / agent / eval signals — changes under `**/skills/**`, `**/agents/**`, `**/.claude/**`, MCP server files (`mcp_*.{py,ts,js}`, `mcp-server*.*`, `.mcp.json`), eval suites (`**/evals/**`, `eval_*.{py,ts,js}`, `*.eval.*`), or imports of `anthropic` / `openai` / `langcha … ## Source & license This open-source skill is cataloged on AgentStack and links to its original source — we do not rehost the code. - **Author:** [steph-dove](https://github.com/steph-dove) - **Source:** [steph-dove/klaussy-agents](https://github.com/steph-dove/klaussy-agents) - **License:** MIT - **Homepage:** https://klaussy.com Install and usage instructions live in the source repository linked above. ## Pricing - **Free** — Free ## Security capabilities Automated source analysis of v0.1.0 — what this tool can access: - **Network access:** no - **Filesystem access:** no - **Shell / process execution:** no - **Environment & secrets:** no - **Dynamic code execution:** no *"Yes" means the capability is present in the source — more access means more to trust, not that it is unsafe.* ## Versions - **0.1.0** — security scan: passed — Imported from the upstream source. ## Links - Listing page: https://agentstack.voostack.com/l/skill-steph-dove-klaussy-agents-fastapi-review - Seller: https://agentstack.voostack.com/s/steph-dove - Browse the marketplace: https://agentstack.voostack.com/browse --- Listed on AgentStack — the marketplace for AI agent skills and MCP servers. Every listing is security-reviewed. Creators keep 70%.