code-review

code-review

Perform a comprehensive code review of all changes on the currently checked out branch compared to main. Scope is limited to the branch diff — for a whole-codebase audit, use /codebase-audit instead.

This is a mixed-stack repo: a Python 3.12 backend (FastAPI, SQLModel over SQLite, a typer CLI exposed as job-applier) under src/job_applier/, and a SvelteKit frontend (Svelte 5 runes, TypeScript) under web/src/. Reviews must cover whichever side(s) the diff touches.

When to Use

• User asks to review the current branch or an open PR
• User asks for a code review before merging
• User invokes /code-review

Instructions

Step 1: Identify Changes

Determine the diff between the current branch and main:

git diff main...HEAD --name-only
git diff main...HEAD --stat

If there are no changes vs main, inform the user and stop.

Collect the full diff for context:

git diff main...HEAD

Step 2: Spawn Parallel Review Agents

Launch nine agents in parallel using the Agent tool (subagent_type: general-purpose). Each agent receives the list of changed files and the full diff, and is responsible for one review category.

Critical preamble — include in every agent prompt:

You are performing an exhaustive code review of a Python (FastAPI / SQLModel / typer) + SvelteKit (TypeScript, Svelte 5 runes) project. You must check EVERY item in the checklist below — do not skip items or stop early. For each item, read the relevant code and report either a finding or "PASS". When an item asks you to grep or search, you MUST actually run the search and report the results — do not assume. Your review must be deterministic: given the same code, you should always find the same issues.

Each agent returns a structured report with:

• Severity: critical, warning, or suggestion
• File path and line number (or range)
• Clear description of the issue
• Suggested fix or code snippet when appropriate

Agent 1: Test Quality & Coverage

1. Read CLAUDE.md — backend tests are pytest in tests/test_<name>.py, use fixtures from tests/conftest.py (e.g. the make_raw RawJob factory) and @pytest.mark.parametrize for table-driven cases; frontend tests are vitest in web/src/lib/*.test.ts or web/src/lib/__tests__/*.test.ts using @testing-library/svelte. Test command: make test (make test-api = pytest, make test-web = vitest).

2. List all changed source files under src/job_applier/ (.py) and web/src/ (.ts, .svelte, excluding *.test.ts). For EACH: a. Search for a corresponding test (tests/test_<module>.py for backend; colocated *.test.ts for frontend helpers). Report whether one exists. b. If a test exists, read it. Verify every new/changed public function, API endpoint, filter rule, source adapter, or exported helper has at least one test. c. If NO test file exists, flag per user preference ("always write tests"): new modules / new filter rules / new API routes / new source adapters = critical; new public functions on existing modules = warning; cosmetic-only = suggestion.

3. Read every changed test file. For each test: a. Assertions must test outcomes (returned FilterResult.status, persisted rows, response bodies, store state), not implementation details. b. Flag tests that only assert "is not None" / "does not raise" without checking behavior. c. Prefer fixture reuse: flag backend tests that hand-roll a RawJob instead of using the make_raw factory when the factory would do. d. For filter-rule changes, flag missing parametrized cases — the existing suite pins both pass and drop directions per rule; new rules should follow suit. e. Frontend: flag component/store tests that don't exercise the runes-driven state transition they claim to cover.

4. For every new if / match / early-return branch added in the diff: check that at least one test exercises it. Untested filter outcomes (passed / manual / dropped) or error branches → warning.

5. For every new API endpoint, CLI command, or source adapter: check that a test covers the success path AND at least one failure/validation/empty-result path.

Agent 2: DRY Code

1. Read every changed source file (not tests) under src/job_applier/ and web/src/. For each: a. Any code block (3+ lines) repeated more than once in the same file → warning. b. Any block nearly identical to code in another changed file → warning.

2. For each pattern found: grep the full repo (src/job_applier/, web/src/) for ALL occurrences. Only flag if it appears 3+ times OR extracting it meaningfully reduces maintenance burden.

3. Repeated string/integer literals used as source names, filter reasons, status strings, API paths, or DB column names across changed files — flag magic values appearing 3+ times (they belong in a constant, enum, or the shared types in web/src/lib/api.ts).

4. Source-adapter duplication: the per-company adapters (greenhouse, lever, ashby, workday, workable, smartrecruiters) share shape via the SourceAdapter protocol in src/job_applier/sources/base.py. If the diff adds a new adapter or copies fetch/parse boilerplate that base helpers already provide, suggest consolidation.

5. Do NOT flag: test verbosity, trivial property/getter repetition, or two-site patterns in clearly different contexts.

Agent 3: Architecture

User is a senior engineer who values cohesive boundaries and dislikes god objects and shared-state desync. Weight findings accordingly.

1. Read CLAUDE.md — layering: FastAPI app + Pydantic schemas in src/job_applier/api/; the frontend talks to it only through web/src/lib/api.ts; source adapters implement the SourceAdapter protocol and register in src/job_applier/sources/__init__.py; the hard filter lives in src/job_applier/filters/rules.py; schema + migrations in src/job_applier/models/db.py.

2. For every changed module: a. God object check: lines of code, number of public functions, number of distinct responsibilities. Flag any module growing past ~400 lines or mixing clearly distinct concerns (e.g. HTTP fetching + parsing + persistence in one function). b. Shared-state desync: does the change introduce a new source of truth that duplicates state the DB already owns (scores, dedupe hashes, application status)? Flag as critical if two places now own the same data. Score writes must go through the active MatchScore + MatchScoreHistory snapshot path — flag direct mutations that skip history. c. Layer coupling: does new frontend code reach the backend by raw fetch instead of the typed api client in web/src/lib/api.ts? Flag as warning. (api.ts browser-safety is covered by Agent 9.) d. LLM boundary: the LLM is NEVER called server-side — scoring/drafting happen via slash commands in .claude/commands/. Grep the diff for anthropic, ANTHROPIC_API_KEY, or any server-side model call. Flag as critical.

3. For new source adapters: confirm they implement the SourceAdapter protocol and are registered in sources/__init__.py; per-company sources must read slugs from the SourceSlug table at runtime, not hardcode them (companies.py is seed-only). Flag deviations.

4. Right shape for data: backend — SQLModel table vs Pydantic schema vs plain dataclass (RawJob). Frontend — Svelte 5 rune store (*.svelte.ts) vs +page.server.ts loader data vs component-local state. Flag mismatches (e.g. cross-route state stuffed into a component instead of a rune store like draftCart.svelte.ts).

5. Feature cohesion: does the change place new files in the correct package (api/, sources/, filters/, models/) or route folder? Flag files that cut across boundaries without justification.

Agent 4: Python / TypeScript Best Practices

1. Read CLAUDE.md for project conventions. Lint is make lint (ruff check src/); frontend typecheck is cd web && npm run check.

2. For every changed .py file: a. Type hints: public functions have typed parameters and return types; modules start with from __future__ import annotations (established style across src/job_applier/ and tests/). Flag missing as warning. b. No bare except: and no except Exception that silently swallows. (Error semantics in depth are Agent 5's job; here just flag the syntactic violation.) c. No stray print() in library code. Grep for print( in src/job_applier/; in CLI code use typer.echo, elsewhere use logging. warning. d. f-strings over %-formatting or .format() in new code. suggestion. e. Ruff cleanliness: flag unused imports, unsorted imports, unused variables that make lint would catch.

3. For every changed .ts / .svelte file: a. Explicit types on exported functions; no any leaking into the typed api surface. Flag any as warning. b. import type for type-only imports. c. No console.log left in committed code. Grep web/src/ for console.log(. warning. d. Code should pass npm run check (svelte-check). Flag obvious type errors.

4. Repo-specific hard rules (carry verbatim — these catch more than "follow conventions"): a. No anthropic SDK / API-key handling server-side — grep new .py files for import anthropic, from anthropic, ANTHROPIC_API_KEY. Flag as critical. (See also Agent 3.) b. No LinkedIn / Indeed scraping — grep new source code (especially under src/job_applier/sources/) for linkedin.com / indeed.com endpoints or scraping logic. ToS violation and explicit project rule. critical. c. Generated-draft purity — if the diff touches .claude/commands/draft.md or draft-rendering code, confirm no em dashes (—), en dashes (–), or smart quotes (" " ' ') a

Content truncated.