feat: generate frontend API contracts from OpenAPI - #228
Conversation
45354cb to
5809419
Compare
There was a problem hiding this comment.
Review — changes requested (one blocking item)
Strongest-engineered of the recent batch: pinned generator, committed output, a
real drift job, and the root cause of the non-determinism actually fixed rather
than worked around. It also lands all six of #114's acceptance criteria —
generated-contract.test.ts pinning the proof-workflow fields through exported
type aliases is a nicer approach than string assertions alone.
Verified against dev after #230 merged: no conflicts, and app/schemas/ai.py
passes ruff check, ruff format --check, and mypy. Nothing here is blocked by
the new static-analysis gate.
1. Remove stripDynamicDefaults (blocking)
§16's Determinism gate asks that non-determinism be "explained and bounded" — not
silently erased. stripDynamicDefaults recursively deletes any default key whose
value matches /^\d{4}-\d{2}-\d{2}T.*Z$/, anywhere in the document. But this PR
already fixes the real cause: app/schemas/ai.py moved checked_at from
datetime.now(UTC) to Field(default_factory=...).
With the sanitizer in place, the next import-time datetime.now() default someone
writes is swallowed instead of failing the drift check — the exact failure the
check exists to catch. It would also silently drop a legitimately static datetime
default from the contract.
I checked whether removing it is safe: grepping all of app/schemas/ and
app/core/ for import-time nondeterministic defaults returns exactly one hit —
ai.py:68, the line this PR fixes. Nothing else in the tree produces a timestamp
at import, so generation stays deterministic without the sanitizer.
Please delete the function and its call site. If other drift sources are
anticipated later, bound it to an explicit allowlist of schema paths — that's what
"explained and bounded" looks like.
2. Make getBackendDetails use the same guard
getBackendMessage and getBackendRequestId now route through isErrorResponse,
which is correct — ErrorResponse in app/core/exceptions.py declares both code
and message required with no defaults, so the narrowing is safe. But
getBackendDetails was left on the old loose typeof body === 'object' check, so
a body carrying details but no code renders details with no accompanying
message. Route all three through the guard.
3. Minor
[...generated].findIndex((character, index) => current[index] !== character)
iterates by code point while indexing current by UTF-16 code unit, so the
reported offset drifts once any non-ASCII enters the contract. Error message only
— fine as a follow-up.
Also worth confirming the new js-yaml: 4.3.0 override is a deliberate transitive
pin for openapi-typescript rather than incidental.
Please fill in Roadmap alignment
Missing, and CONTRIBUTING.md §0 makes it a merge gate. Map it to W0 / Truth
contract (schema policy), not W5 / Interfaces — this hardens an existing client
against evidence loss rather than adding a surface, so W0 keeps it clear of
GATE A. §16 Contract gate names "generated client checks" and "stable errors"
verbatim; §28 criterion is Trusted output.
Fix 1 and 2 and this is good to merge.
6bd6832 to
987ce27
Compare
|
Approved. Generating the frontend client from OpenAPI (with a --check drift guard) encodes the contract-authority rule properly, and the checked_at default_factory fix is a real bug squash. Good moat-deepening work. |
parthrohit22
left a comment
There was a problem hiding this comment.
Approved. OpenAPI-driven client generation with drift guard; checked_at default_factory bug fix. Tests present.
Summary
openapi-typescript@7.13.0.ErrorResponsevalidation.checked_atdefault so OpenAPI generation is deterministic.Validation
npm --prefix apps/frontend run generate:api-contract -- --checknpm --prefix apps/frontend run buildnpm --prefix apps/frontend run lintRoadmap alignment
Closes #114