Skip to content

Gotchas

Append-only log of non-obvious mistakes — things that look right in this repo (or in vanilla Python) but aren't. Add a line whenever Claude is corrected on something a future session would also get wrong.

Format

- **Symptom or temptation** — what to do instead. (Why: brief reason / link to context.)

Keep entries one line where possible. If an entry needs more than two lines, it probably belongs in a dedicated doc — link from here instead.

Entries

  • Tempted to add method-layer polish around a one-call proof path — keep the method API plain: inline single-call helpers, avoid keyword-only * markers unless needed, prefer default-false booleans such as skip_pdf_hydration=False, and omit defaulted None args or pass the concrete effective value. (Why: PR #1195 review convention; less ceremony keeps proof orchestration easier to scan.)
  • Tempted to add a route/schema helper for a model's reusable response row shape — put the row serializer on the model and keep route schemas to endpoint envelopes. Example: Mailpiece.to_api_json() owns mailpiece fields, while serialize_order_mailpiece_response() only wraps { "mailpiece": ... }. (Why: this keeps serialization beside the model fields and avoids duplicating model-shaped dict builders across routes.)
  • Tempted to wrap new Flask route dict/list responses in jsonify(...) — return the dict/list or (body, status) tuple directly unless an existing local pattern requires otherwise. (Why: app/routes/CLAUDE.md defines plain Flask-serializable returns as the route convention; extra jsonify is noise in new code.)
  • Tempted to add a parent existence query before a scoped child lookup that already returns no visible row — prefer auth check → single scoped model lookup, and let the endpoint's missing-child response shape handle the empty result. Example: after route-level organization auth, GET /v2/organizations/:id/mailpieces/by-provider/:provider_id should call the organization-scoped mailpiece lookup once and return { "mailpiece": null } for no matching row, not first query organizations just to decide whether to return the same empty result. (Why: the extra query is usually a redundant round trip; if the product truly needs to distinguish missing parent from missing child, make that requirement explicit before adding it.)
  • Tempted to hide campaign visibility inside a narrowly named lookup method — keep the shared campaign visibility check explicit at the route boundary, then call the existing scoped model query directly when there is no extra orchestration. (Why: a get_by_email wrapper that also enforces business permissions obscures the authorisation boundary and adds tests/maintenance without reusable logic.)
  • Tempted to branch on settings.automated to determine campaign mode — use the canonical campaign.campaign_mode column (CampaignMode.Automated / OneOff) and never write automated by hand. (Why: settings.automated is a read-only mirror of campaign_mode, persisted in lockstep by set_campaign_mode() purely for direct-JSONB consumers; branching on it risks reading a stale value, and hand-writing it drifts from the canonical column. Retiring it is a separate gated ticket. See docs/models/campaign.md §Mode, ENG-3208.)
  • Tempted to send a partial settings body to the campaign PUT / PUT .../settings — send the full, current settings blob. (Why: settings is a whole-blob replace, so unmodeled keys / value overrides are dropped and omitted fields reset to payload defaults. A partial-merge PATCH is tracked in ENG-3234. See docs/models/campaign.md §Mode CAUTION.)
  • Tempted to use unittest.mock / MagicMock / @patch / with patch(...) — use monkeypatch.setattr("full.module.path", lambda_or_fn) instead. (Why: MagicMock returns truthy mocks for any attribute, silently hiding bugs. Full reasoning in app/tests/CLAUDE.md.)
  • Tempted to remove parens from except (ValueError, TypeError): — leave them. PEP 758 made parens optional in 3.14, but the parenthesized form is dominant in this codebase and the broader Python community; removing them is a drive-by edit that fights ruff and reviewer expectations.
  • Tempted to call app/core/integrations/* from app/routes/ — route through app/methods/ instead. (Why: layering invariant; routes are HTTP shape only.)
  • Tempted to ALTER TABLE … SET NOT NULL on a populated table in one shot — split into add-nullable-with-default → backfill → set-not-null. (Why: single-shot rewrites take a heavy lock.)