Fastapi Review

> 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.

1. 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.lrucache: Use functools.lrucache for caching.
• Python import path (flat-layout): flat-layout: import fastapi.
• PEP 8 snakecase naming: Name functions, variables, and modules using snakecase 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 (testfeature): Use Use Simple style (testfeature) 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
• Source: steph-dove/klaussy-agents
• License: MIT
• Homepage: https://klaussy.com

Install and usage instructions live in the source repository linked above.