# API Surface Review

> Review changes to Polar's API contract — Pydantic schemas, FastAPI endpoints, OpenAPI output and the generated SDKs. Checks public vs private exposure, schema shape and naming, and whether the change breaks merchants or the generated clients. Use when a diff touches schemas.py, endpoints.py, docs/openapi.json or sdk/, or when the user asks whether an API change is breaking.

- Skill: `polarsource/api-surface-review` (Agent Skill)
- Install (CLI): `npx skillmds@latest add polarsource/api-surface-review`
- Raw SKILL.md: https://api.skillmd.com/api/skills/polarsource/api-surface-review/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Integrations & APIs
- License: MIT
- Author: polarsource (https://skillmd.com/u/polarsource)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/polarsource/api-surface-review

---


# API Surface Review (Polar)

The handle is the **diff**. The evidence is **what this change does to the API contract**, and
whether any of it is breaking.

This is where Polar's most expensive review mistakes live. A schema change does not stay in
the repo: it flows into `docs/openapi.json`, the generated TypeScript client, the Speakeasy
SDKs under `sdk/`, and out to merchants who already wrote code against it.

## Scope

Diffs touching `**/schemas.py`, `**/endpoints.py`, `polar/openapi.py`, `docs/openapi.json`,
`sdk/`, `clients/packages/client/`.

**CI already does part of this job.** The `OpenAPI Diff` workflow posts the schema delta as a
PR comment, and `OpenAPI Client Regeneration Check` fails if the client is stale. Do not
recompute either by hand. Read the diff comment if it exists and judge whether the delta is
acceptable; that judgement is the thing CI cannot make.

Owned elsewhere: `ADR-0002` (status-coded `PolarError`), `ADR-0007` (no default in output
schemas) → `adr-check`. Endpoint conventions written in `server/AGENTS.md` — `response_model`
and ORM returns, POST/PATCH/DELETE status codes, `ListResource`, `responses=`, the
`PolarRequestValidationError` boundary → `conventions-check`. Existing shared types →
`reuse-check`.

## Checks

### 1. Public or private

`APITag` has two values, `public` and `private` (`polar/openapi.py`). Public means documented
and in the SDKs.

- Set it deliberately on every new router and route. Never hide something with
  `include_in_schema=False`; use `tags=[APITag.private]`.
- Tag the route, not the router, when only one route should change. The pattern: keep
  `APITag.public` on what stays, add `APITag.private` to the deprecated one.
- Private endpoints (payouts are the standing example) are not in the SDK. Do not reason about
  them as public API.
- **On every new field, ask: does a merchant need to see this?** Internal `Organization`
  settings keep leaking into the public API this way.

### 2. Schema shape

- Output fields are required, nullable where needed. A `default` makes them optional in
  OpenAPI, which generates a wrongly-optional TS field. (ADR-0007 — name it, do not restate it.)
- Type fields with their enum, not `str`.
- Reuse a related resource's base schema rather than redefining its fields:
  `class DisputeCustomer(CustomerBase): ...`.
- A value that already exists as a model property is a plain field, not a reimplemented
  `computed_field`.
- Descriptions are user-facing: no implementation detail, and write `None` not `null`.
- Any field writing to a SQL `INT` column is `Int32`, so out-of-range input is rejected at
  validation instead of overflowing.
- Constrain enums by listing allowed values (`reason: Literal[RefundReason.foo, ...]`) rather
  than a hand-rolled validator. Pydantic validates, OpenAPI documents.
- Discriminated unions get a plain `Discriminator` plus `SetSchemaReference`, so the union
  lands in OpenAPI properly.
- Do not over-constrain fields fed by a payment processor. *"We'll never know what Stripe or
  other payment processor will send us."*

### 3. Naming

The union takes the clean name, the variants get qualified: `CustomerCreate` is the union,
`CustomerIndividualCreate` and `CustomerTeamCreate` are members. Same for `CustomerState`.

Fields are `snake_case`. Prefer explicit over clever (`payment_method_type`, not
`payment_method`). Do not overload an existing concept with a new meaning.

### 4. Breaking changes

Breaking = removing a field, flipping required/optional, changing a type, renaming anything,
or changing a status code a client branches on.

Before removing a field or behaviour:

1. **Check Logfire for real usage.** This is what François actually does. Merchants create
   things programmatically that nobody expects.
2. If it is used, keep a compatibility layer and use `SkipJsonSchema` so runtime still accepts
   the value while it disappears from future SDKs.
3. If it is genuinely breaking, say so plainly. That is a human decision, not something to
   wave through.

### 5. Route shape

Only the parts `AGENTS.md` does not already cover:

- Trailing slash on root endpoints.
- Errors on the same route need distinct status codes, or their schemas collide in the OpenAPI
  output.
- A query parameter must act as a pure filter. If it changes the response shape, the resource
  is wrong and it wants its own route.
- Removing an endpoint and updating its frontend caller in one PR is a deploy hazard →
  `ship-safety` owns the split.

## Output

```
## API Surface

### 🔴 Blocking
- `file:line` — <what breaks, and for whom>. Fix: <fix>

### 🟠 Should fix
- `file:line` — <claim>. Fix: <fix>

### 🟡 Question
- `file:line` — <question>

### Notes
- public surface delta: <one line, or "see the OpenAPI Diff PR comment">

### Verdict
✅ Clean  |  ❌ n blocking, n should-fix
```

Anything blocking needs a human decision before merge.