ProjectTech4DevAI / ProjectTech4DevAI/kaapi-backend

API Audit: Strengthening type safety

Aperta
#1,074 0 commenti 0 reazioni 1 assegnatario Vedi su GitHub

@Prajna1999 ci sta già lavorando.

Dal 24/7/2026.

Lingua principale
Python
Stelle
18
Fork
10
Merge medio
2g 20h
PR unite (30g)
14

Descrizione

Describe the current behavior

We added an automated breaking-change gate in #892 (.github/workflows/api-breaking-changes.yaml) that exports the OpenAPI schema for base vs. HEAD, diffs them with oasdiff, and blocks merges that introduce breaking changes (overridable with the breaking-change-approved label).

The gate is only as good as the schema it diffs. oasdiff can only see what OpenAPI encodes. Wherever a request/response body is typed as an opaque, permissive shape — dict[str, Any], bare Any, list[dict[str, Any]], or a route with no meaningful response_model — OpenAPI renders it as a formless object with no properties and no required list. To oasdiff, such a field looks identical before and after any change to its real contents, so genuine breaking changes pass straight through the gate undetected.

Concrete example (already happened): the credential contract.

  • credentials is typed list[dict[str, Any]] in app/models/onboarding.py and described as a generic "Dictionary mapping provider names to their credentials" in app/models/credentials.py.
  • The actual per-provider required fields live in a runtime Python dict — PROVIDER_CONFIGS in app/core/providers.py — enforced imperatively by validate_provider_credentials().
  • In #890 (2026-06-11), provider: google was changed so its required fields went from ["api_key"] to ["api_key", "project_id", "location", "sa_key", "gcs_bucket"]. Any client previously sending provider: google with only api_key now gets rejected with Missing required fields.
  • This is a textbook breaking change. The oasdiff gate had already been live for 9 days (#892 merged 2026-06-02). It said nothing — because the requirement was never in the schema, it was in a dict[str, Any] plus a runtime dict.

If a discriminated union had modeled the credentials (see "solution" below), OpenAPI would have carried a real required list per provider variant and oasdiff would have flagged #890 as breaking.

Describe the enhancement you'd like

A systematic audit + hardening pass across the whole API surface:

  1. Inventory every loose type on the request/response boundary. Starting data from a quick grep of backend/:
    dict[str, Any] / Dict[str, Any] <-- a lenient dict like this may not be the only culprit, need to check for other loose types also
  2. Triage each occurrence into:
  • Fixable → model it properly. Replace with a typed Pydantic model. For fields whose shape depends on a discriminator (the credentials case), use a Pydantic discriminated union so OpenAPI emits oneOf + discriminator, each variant carrying its own required fields:
class GoogleCredentials(BaseModel):
    provider: Literal["google"]
    api_key: str
    project_id: str
    location: str
    sa_key: dict
    gcs_bucket: str

class OpenAICredentials(BaseModel):
    provider: Literal["openai"]
    api_key: str
# ...
Credential = Annotated[
    Union[GoogleCredentials, OpenAICredentials, ...],
    Field(discriminator="provider"),
]
  • Genuinely dynamic → document + guard another way. Some payloads are legitimately free-form (arbitrary provider passthrough, user-defined metadata blobs). For these, oasdiff structurally can't help. Add a golden-file contract test instead: snapshot the effective contract (e.g. PROVIDER_CONFIGS → a checked-in providers.contract.json) and assert equality in CI, so any change fails the test with a visible diff and forces explicit review.
  1. Prevent regressions. Decide on a lint/CI rule that discourages introducing new dict[str, Any] / bare Any on request/response models without an explicit, reviewed opt-out, so the audit doesn't silently rot back.

Why is this enhancement needed?

  • Correctness of the guardrail: right now the breaking-change gate gives false confidence: a green check does not mean "no breaking changes," it means "no breaking changes in the parts of the contract we bothered to type strictly." #890 is proof of the blind spot and we have already faced the consequences.
  • Client trust: downstream consumers rely on our contracts, silent breaking failures surface as production failures

Guida per i contributori

Apri la guida per i contributori

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.