ProjectTech4DevAI / ProjectTech4DevAI/kaapi-backend

API Audit: Strengthening type safety

Abierto
#1,074 0 comentarios 0 reacciones 1 asignado Ver en GitHub

@Prajna1999 ya está trabajando en esto.

Desde el 24/7/2026.

Lenguaje dominante
Python
Estrellas
18
Forks
10
Merge medio
2 d 20 h
PR fusionados (30 d)
14

Descripción

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

Guía de contribución

Abrir la guía de contribución

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.