Open
Conversation
…pport Refactor REST and RPC API handlers to extract OpenAPI spec generation into dedicated classes (RestApiSpecGenerator, RPCApiSpecGenerator). Add support for queryOptions.slicing.fields with includedFilterKinds/excludedFilterKinds to control which filter operators appear in generated specs. REST generator controls individual filter query parameters; RPC generator controls per-operator properties in WhereInput schemas. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughAdds OpenAPI 3.1 generation for REST and RPC handlers: new spec generators, shared types and spec utilities, package dependency, comprehensive tests, and baseline OpenAPI YAML fixtures. Generation respects schema and queryOptions-driven slicing. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
Claude finished @ymc9's task —— View job Code Review for OpenAPI Spec Generation PRI've conducted a comprehensive review of this PR that adds OpenAPI spec generation with filter kind slicing support. Here's my detailed feedback: Review Tasks:
✅ StrengthsExcellent Architecture
Robust Filter Kind Slicing Implementation
Strong Test Coverage
|
ymc9
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (4)
packages/server/test/openapi/migrated/rest-migrated.test.ts (1)
198-208: Consider usingit.skip()ortest.skipIf()for clearer test reporting.When baseline files are missing, the test returns early with a
console.warn. This passes the test silently rather than marking it as skipped in test reports. Using Vitest's skip mechanism would provide better visibility.♻️ Alternative using skipIf
it.skipIf(!fs.existsSync(path.join(__dirname, 'baseline', 'rest-3.1.0.baseline.yaml')))( 'baseline comparison - rest-3.1.0', async () => { const baselinePath = path.join(__dirname, 'baseline', 'rest-3.1.0.baseline.yaml'); const baseline = YAML.parse(fs.readFileSync(baselinePath, 'utf-8')); expect(baseline.openapi).toBe('3.1.0'); expect(baseline.components?.schemas).toBeDefined(); expect(baseline.paths).toBeDefined(); } );Alternatively, keep the current approach if you prefer tests to pass when baselines are intentionally absent during development.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/test/openapi/migrated/rest-migrated.test.ts` around lines 198 - 208, The test "baseline comparison - rest-3.1.0" currently uses an early return after checking fs.existsSync(path.join(...)) which silently passes when the baseline is missing; change this to use Vitest's skip mechanism (e.g., it.skip or it.skipIf) so the test is explicitly marked skipped instead of returning early—remove the console.warn and early return, and wrap or declare the test using it.skipIf(!fs.existsSync(path.join(__dirname, 'baseline', 'rest-3.1.0.baseline.yaml'))) while keeping the body that reads YAML.parse(fs.readFileSync(...)) and the existing expects intact.packages/server/src/api/index.ts (1)
1-3: Consider exporting the concrete spec generator classes.The
RestApiSpecGeneratorandRPCApiSpecGeneratorclasses are exported from./restand./rpcrespectively (per context snippets), but not re-exported from this main barrel module. If external consumers should have direct access to these generators (independent of the handlers), consider adding them here for API surface consistency.🔧 Optional: Add generator class exports
export { RestApiHandler, type RestApiHandlerOptions } from './rest'; export { RPCApiHandler, type RPCApiHandlerOptions } from './rpc'; export type { OpenApiSpecGenerator, OpenApiSpecOptions } from './common/types'; +export { RestApiSpecGenerator } from './rest'; +export { RPCApiSpecGenerator } from './rpc';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/api/index.ts` around lines 1 - 3, Add re-exports for the concrete spec generator classes so external consumers can import them from the main barrel; specifically export RestApiSpecGenerator and RPCApiSpecGenerator from the module by adding "export { RestApiSpecGenerator } from './rest';" and "export { RPCApiSpecGenerator } from './rpc';" alongside the existing handler and type exports (keeping the existing RestApiHandler, RPCApiHandler, and OpenApiSpecGenerator/OpenApiSpecOptions exports).packages/server/test/openapi/migrated/rpc-migrated.test.ts (2)
165-197: Same recommendation: consider usingit.skip()for missing baselines.Consistent with the REST test file feedback, using Vitest's skip mechanism would provide clearer test reporting when baseline files are absent.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/test/openapi/migrated/rpc-migrated.test.ts` around lines 165 - 197, The three tests ('baseline comparison - rpc-3.1.0', 'baseline comparison - rpc-3.1.0-omit', 'baseline comparison - rpc-type-coverage-3.1.0') currently early-return with console.warn when baseline files are missing; replace that pattern with Vitest's skip so missing baselines show as skipped tests. For each test, compute baselinePath and then do: if (!fs.existsSync(baselinePath)) { it.skip('<same test title>', () => {}); return; } or better yet choose the test function dynamically (const maybeIt = fs.existsSync(baselinePath) ? it : it.skip) and call maybeIt('<same test title>', async () => { const baseline = YAML.parse(...); expect(...); }); ensuring the test name and assertions (YAML.parse and expect lines) remain unchanged.
128-129: Remove unnecessaryas anytype assertions.The
OpenAPIV3_1.InfoObjecttype from openapi-types includes bothdescriptionandsummaryas optional fields (per OpenAPI 3.1 spec). Theas anycasts on lines 128-129 are unnecessary and can be removed:expect(spec.info.description).toBe('A custom API'); expect(spec.info.summary).toBe('awesome api');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/test/openapi/migrated/rpc-migrated.test.ts` around lines 128 - 129, Remove the unnecessary `as any` casts on the OpenAPI info assertions: replace `expect((spec.info as any).description).toBe('A custom API')` and `expect((spec.info as any).summary).toBe('awesome api')` with `expect(spec.info.description).toBe('A custom API')` and `expect(spec.info.summary).toBe('awesome api')`; ensure `spec` is typed as an OpenAPIV3_1.Document (or otherwise that `spec.info` is recognized) so the `description` and `summary` properties on `spec.info` are accepted by TypeScript without casting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/server/src/api/common/spec-utils.ts`:
- Around line 24-39: The isOperationIncluded function treats a model-specific
slicing block as a full replacement of $all (using (slicing.models[modelKey] ??
slicing.models.$all)), which drops global rules; instead merge the $all block
with the model-specific block so each property (excludedOperations,
includedOperations) considers defaults from $all and overrides from
models[modelKey] before evaluating includes/excludes. Update isOperationIncluded
to compute a combined modelSlicing = { ...slicing.models.$all,
...slicing.models[modelKey] } (or equivalent property-level merge) and then
check modelSlicing.excludedOperations and modelSlicing.includedOperations; apply
the same merge-fix to the analogous logic later in the file (the field-slicing
check around lines 75-97).
In `@packages/server/src/api/rest/handler.ts`:
- Around line 1267-1298: processUpdate is manually constructing relation
payloads (set/connect) causing numeric IDs to remain strings and compound IDs to
be formatted differently than processCreate/processRelationshipCRUD; replace the
manual constructions with calls to makeIdConnect so relation payloads are
normalized. Concretely, inside the relationships loop where
updatePayload.data[key] is set for collection and non-collection branches, call
makeIdConnect(relationInfo.idFields, item.id) for each item (for collections use
set: enumerate(...).map(idObj => ({ [makeDefaultIdKey?]?: idObj }) if
makeIdConnect returns the nested unique object adjust accordingly) and for
single relations use connect: makeIdConnect(relationInfo.idFields,
data.data.id); keep updatePayload.include assignment but ensure IDs produced
match the shape used by processCreate/processRelationshipCRUD. Use the existing
symbols processUpdate, makeIdConnect, makeDefaultIdKey, processCreate,
processRelationshipCRUD, relationInfo, updatePayload and relationships to locate
and update the code.
- Around line 1817-1903: The bug is that item, curr and currType are created
once per query key and reused across enumerate(value) entries causing repeated
filter params (e.g. ?filter[id]=1&filter[id]=2) to overwrite the same object;
fix it by moving the creation/initialization of item, curr and currType inside
the for (const filterValue of enumerate(value)) loop so each filterValue builds
a fresh item object, reset curr = item and currType = typeInfo at the start of
each iteration, and then continue using existing logic (getIdFields,
getModelInfo, makeFilterValue, and pushing into items) so each value produces
its own independent filter object.
- Around line 510-512: handleGenericError currently appends err.stack into the
response body which leaks internals; change handleGenericError to only return a
generic error message via makeError('unknownError', err instanceof Error ?
err.message : 'Unknown error') and ensure the full error and stack are logged
internally (using the existing logger on the class or console.error) instead of
exposing the stack in the HTTP response; keep references to the
handleGenericError method and makeError call so the response content is
sanitized while retaining internal logs.
In `@packages/server/src/api/rest/openapi.ts`:
- Around line 73-89: generatePaths is currently adding collection and
single-resource paths unconditionally, causing operations like DELETE to be
included despite excludedOperations; update generatePaths to check
isOperationIncluded(modelName, operation) before adding each operation and when
calling buildCollectionPath and buildSinglePath so only allowed operations are
emitted—specifically, in generatePaths use isOperationIncluded for collection
ops (e.g., 'list','create') before assigning paths[`/${modelPath}`] and for
single-resource ops (e.g., 'read','update','delete') before assigning
paths[`/${modelPath}/{id}`]; also ensure the lower-level builders
(buildCollectionPath and buildSinglePath) accept or consult operation filters
(or are called selectively) so lines in the broader generation block (the later
code around 136-250) follow the same inclusion checks.
- Around line 191-250: The OpenAPI generator (buildSinglePath) is missing
operations that the runtime serves: add a PUT operation for /{model}/{id}
(operationId `replace<Model>` or `put<Model>` to match naming) with the same
requestBody schema as PATCH (`${modelName}UpdateRequest`) and appropriate
200/400/404 responses, and ensure relationship DELETE paths are generated for
/{model}/{id}/relationships/{field} (operationId like
`delete${modelName}Relationship${Field}`) returning 200/404; mirror the runtime
behavior found in RestApiHandler.handleRequest (lines handling PUT and DELETE
relationship) when creating these entries so the OpenAPI spec matches the live
endpoints.
- Around line 139-400: The responses currently reference
components/schemas/_errorResponse directly (invalid because Response Object refs
must target components/responses and Response Objects need a description);
update every occurrence inside buildCollectionPath (list/create),
buildSinglePath (get/patch/delete), buildFetchRelatedPath (get),
buildRelationshipPath (get/put/patch/post) and buildProcedureOperation to either
reference a Response Object at '#/components/responses/_errorResponse' or
replace the inline entry with a proper Response Object that includes a
description and content whose schema points to
'#/components/schemas/_errorResponse' (e.g. '400': { $ref:
'#/components/responses/_errorResponse' } or '400': { description: 'Error',
content: { 'application/vnd.api+json': { schema: { $ref:
'#/components/schemas/_errorResponse' } } } }); ensure every response entry that
previously used the schema $ref is converted accordingly.
In `@packages/server/src/api/rpc/openapi.ts`:
- Around line 73-98: The writeOps list includes ops createManyAndReturn and
updateManyAndReturn that cause references to
`${modelName}CreateManyAndReturnArgs` / `${modelName}UpdateManyAndReturnArgs`
but generateSchemas() never registers those schemas; update the generation logic
so either (A) remove those ops from writeOps or (B) add schema generation for
those args by extending opToArgsSchema/op mapping and ensure generateSchemas()
creates `${modelName}CreateManyAndReturnArgs` and
`${modelName}UpdateManyAndReturnArgs` (or maps them to existing
CreateMany/UpdateMany args) so buildPostOperation/buildPatchOperation produce
valid request-body component refs and no dangling refs. Ensure to touch
writeOps, opToArgsSchema, generateSchemas, and where
buildPostOperation/buildPatchOperation are invoked to keep names consistent.
- Around line 477-495: The recursive filter refs in the logical combinators
(properties['AND'], properties['OR'], properties['NOT']) point to
`#/components/schemas/_${_modelName}WhereInput` which doesn't match what
generateSchemas() emits (`${modelName}WhereInput`), causing dangling refs;
update those three $ref strings to reference the actual schema name produced by
generateSchemas (e.g., `#/components/schemas/${modelName}WhereInput`) or
otherwise make generateSchemas() emit the underscored name so the names match
(modify the ref strings in openapi.ts around the AND/OR/NOT blocks or adjust
generateSchemas() schema names accordingly).
- Around line 171-181: The response entries currently use a response-level $ref
to a Schema Object (e.g. { $ref: '#/components/schemas/_ErrorResponse' }) which
is invalid; update the responses in the OpenAPI generation (the responses object
built alongside modelName and op in packages/server/src/api/rpc/openapi.ts) to
either reference a Response Object (change to { $ref:
'#/components/responses/_ErrorResponse' } and add a
components.responses._ErrorResponse entry that points to the error schema) or
replace the direct $ref with a proper Response Object (include description and
content: application/json with schema: { $ref:
'#/components/schemas/_ErrorResponse' }); apply this change for all occurrences
of the error response generation so validators and generators target
`#/components/responses/` instead of `#/components/schemas/`.
- Around line 63-99: The generated OpenAPI paths always use the single-model
response schema `${modelName}Response`, causing client deserialization errors
for operations that return arrays, counts, aggregates or booleans; update the
path-building loop so each operation uses the correct response schema instead of
`${modelName}Response`. Add an op→response-schema mapping (e.g., findMany ->
`${modelName}[]` or `${modelName}ArrayResponse`,
count/createMany/updateMany/deleteMany -> `${modelName}BatchResponse` or a count
schema, aggregate -> `${modelName}AggregateResponse`, groupBy ->
`${modelName}GroupByResponse`, exists -> `${modelName}ExistsResponse`), and pass
that schema name into the builders (or extend
buildGetOperation/buildPostOperation/buildPatchOperation/buildDeleteOperation to
accept a responseSchemaName). Use the existing symbols opToArgsSchema,
buildGetOperation, buildPostOperation, buildPatchOperation,
buildDeleteOperation, isOperationIncluded and queryOptions to locate where to
inject the response schema selection and adjust the builders to reference the
provided response schema when constructing responses.
In
`@packages/server/test/openapi/migrated/baseline/rest-type-coverage-3.1.0.baseline.yaml`:
- Line 549: Fix the typo in the OpenAPI baseline description under the _links
schema: change the description string "Link for refetching the curent results"
to "Link for refetching the current results" so the word "curent" is corrected
to "current" in the description field.
- Around line 609-610: The YAML description for the schema property named
"zodErrors" has an unintended line break so the word "failure" is split onto its
own line; fix it by merging the lines into a single properly quoted or block
scalar YAML value (e.g., use a single-line string or a pipe/greater-than block)
so the description reads continuously as "Zod validation errors if the error is
due to data validation failure", and update the "description" value for the
zodErrors entry accordingly.
---
Nitpick comments:
In `@packages/server/src/api/index.ts`:
- Around line 1-3: Add re-exports for the concrete spec generator classes so
external consumers can import them from the main barrel; specifically export
RestApiSpecGenerator and RPCApiSpecGenerator from the module by adding "export {
RestApiSpecGenerator } from './rest';" and "export { RPCApiSpecGenerator } from
'./rpc';" alongside the existing handler and type exports (keeping the existing
RestApiHandler, RPCApiHandler, and OpenApiSpecGenerator/OpenApiSpecOptions
exports).
In `@packages/server/test/openapi/migrated/rest-migrated.test.ts`:
- Around line 198-208: The test "baseline comparison - rest-3.1.0" currently
uses an early return after checking fs.existsSync(path.join(...)) which silently
passes when the baseline is missing; change this to use Vitest's skip mechanism
(e.g., it.skip or it.skipIf) so the test is explicitly marked skipped instead of
returning early—remove the console.warn and early return, and wrap or declare
the test using it.skipIf(!fs.existsSync(path.join(__dirname, 'baseline',
'rest-3.1.0.baseline.yaml'))) while keeping the body that reads
YAML.parse(fs.readFileSync(...)) and the existing expects intact.
In `@packages/server/test/openapi/migrated/rpc-migrated.test.ts`:
- Around line 165-197: The three tests ('baseline comparison - rpc-3.1.0',
'baseline comparison - rpc-3.1.0-omit', 'baseline comparison -
rpc-type-coverage-3.1.0') currently early-return with console.warn when baseline
files are missing; replace that pattern with Vitest's skip so missing baselines
show as skipped tests. For each test, compute baselinePath and then do: if
(!fs.existsSync(baselinePath)) { it.skip('<same test title>', () => {}); return;
} or better yet choose the test function dynamically (const maybeIt =
fs.existsSync(baselinePath) ? it : it.skip) and call maybeIt('<same test
title>', async () => { const baseline = YAML.parse(...); expect(...); });
ensuring the test name and assertions (YAML.parse and expect lines) remain
unchanged.
- Around line 128-129: Remove the unnecessary `as any` casts on the OpenAPI info
assertions: replace `expect((spec.info as any).description).toBe('A custom
API')` and `expect((spec.info as any).summary).toBe('awesome api')` with
`expect(spec.info.description).toBe('A custom API')` and
`expect(spec.info.summary).toBe('awesome api')`; ensure `spec` is typed as an
OpenAPIV3_1.Document (or otherwise that `spec.info` is recognized) so the
`description` and `summary` properties on `spec.info` are accepted by TypeScript
without casting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f75a29d7-1077-45f0-bb75-c14f1dc714ee
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (24)
packages/server/package.jsonpackages/server/src/api/common/spec-utils.tspackages/server/src/api/common/types.tspackages/server/src/api/index.tspackages/server/src/api/rest/handler.tspackages/server/src/api/rest/index.tspackages/server/src/api/rest/openapi.tspackages/server/src/api/rpc/handler.tspackages/server/src/api/rpc/index.tspackages/server/src/api/rpc/openapi.tspackages/server/test/openapi/baseline/rest-3.1.0.baseline.yamlpackages/server/test/openapi/baseline/rest-type-coverage-3.1.0.baseline.yamlpackages/server/test/openapi/baseline/rpc-3.1.0-omit.baseline.yamlpackages/server/test/openapi/baseline/rpc-3.1.0.baseline.yamlpackages/server/test/openapi/baseline/rpc-type-coverage-3.1.0.baseline.yamlpackages/server/test/openapi/migrated/baseline/rest-3.1.0.baseline.yamlpackages/server/test/openapi/migrated/baseline/rest-type-coverage-3.1.0.baseline.yamlpackages/server/test/openapi/migrated/baseline/rpc-3.1.0-omit.baseline.yamlpackages/server/test/openapi/migrated/baseline/rpc-3.1.0.baseline.yamlpackages/server/test/openapi/migrated/baseline/rpc-type-coverage-3.1.0.baseline.yamlpackages/server/test/openapi/migrated/rest-migrated.test.tspackages/server/test/openapi/migrated/rpc-migrated.test.tspackages/server/test/openapi/rest-openapi.test.tspackages/server/test/openapi/rpc-openapi.test.ts
Comment on lines
+510
to
+512
| private handleGenericError(err: unknown): Response | PromiseLike<Response> { | ||
| return this.makeError('unknownError', err instanceof Error ? `${err.message}\n${err.stack}` : 'Unknown error'); | ||
| } |
Contributor
There was a problem hiding this comment.
Don't return stack traces in 500 responses.
handleGenericError() concatenates err.stack into the response body. That leaks internals on unexpected failures; keep the stack in logs only.
🔒 Suggested fix
private handleGenericError(err: unknown): Response | PromiseLike<Response> {
- return this.makeError('unknownError', err instanceof Error ? `${err.message}\n${err.stack}` : 'Unknown error');
+ log(this.log, 'error', 'unexpected REST handler error', err);
+ return this.makeError('unknownError', err instanceof Error ? err.message : 'Unknown error');
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/api/rest/handler.ts` around lines 510 - 512,
handleGenericError currently appends err.stack into the response body which
leaks internals; change handleGenericError to only return a generic error
message via makeError('unknownError', err instanceof Error ? err.message :
'Unknown error') and ensure the full error and stack are logged internally
(using the existing logger on the class or console.error) instead of exposing
the stack in the HTTP response; keep references to the handleGenericError method
and makeError call so the response content is sanitized while retaining internal
logs.
Comment on lines
+1267
to
+1298
| // turn relationships into query payload | ||
| if (relationships) { | ||
| for (const [key, data] of Object.entries<any>(relationships)) { | ||
| if (!data?.data) { | ||
| return this.makeError('invalidRelationData'); | ||
| } | ||
|
|
||
| const relationInfo = typeInfo.relationships[key]; | ||
| if (!relationInfo) { | ||
| return this.makeUnsupportedRelationshipError(type, key, 400); | ||
| } | ||
|
|
||
| if (relationInfo.isCollection) { | ||
| updatePayload.data[key] = { | ||
| set: enumerate(data.data).map((item: any) => ({ | ||
| [this.makeDefaultIdKey(relationInfo.idFields)]: item.id, | ||
| })), | ||
| }; | ||
| } else { | ||
| if (typeof data.data !== 'object') { | ||
| return this.makeError('invalidRelationData'); | ||
| } | ||
| updatePayload.data[key] = { | ||
| connect: { | ||
| [this.makeDefaultIdKey(relationInfo.idFields)]: data.data.id, | ||
| }, | ||
| }; | ||
| } | ||
| updatePayload.include = { | ||
| ...updatePayload.include, | ||
| [key]: { select: { [this.makeDefaultIdKey(relationInfo.idFields)]: true } }, | ||
| }; |
Contributor
There was a problem hiding this comment.
processUpdate() should reuse makeIdConnect() for relation payloads.
This branch manually builds set/connect inputs, unlike processCreate() and processRelationshipCRUD(). Numeric IDs stay as strings, and compound IDs are passed as the joined token instead of the nested unique object the other paths already build.
🛠️ Proposed fix
if (relationInfo.isCollection) {
updatePayload.data[key] = {
- set: enumerate(data.data).map((item: any) => ({
- [this.makeDefaultIdKey(relationInfo.idFields)]: item.id,
- })),
+ set: enumerate(data.data).map((item: any) =>
+ this.makeIdConnect(relationInfo.idFields, item.id),
+ ),
};
} else {
if (typeof data.data !== 'object') {
return this.makeError('invalidRelationData');
}
updatePayload.data[key] = {
- connect: {
- [this.makeDefaultIdKey(relationInfo.idFields)]: data.data.id,
- },
+ connect: this.makeIdConnect(relationInfo.idFields, data.data.id),
};
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/api/rest/handler.ts` around lines 1267 - 1298,
processUpdate is manually constructing relation payloads (set/connect) causing
numeric IDs to remain strings and compound IDs to be formatted differently than
processCreate/processRelationshipCRUD; replace the manual constructions with
calls to makeIdConnect so relation payloads are normalized. Concretely, inside
the relationships loop where updatePayload.data[key] is set for collection and
non-collection branches, call makeIdConnect(relationInfo.idFields, item.id) for
each item (for collections use set: enumerate(...).map(idObj => ({
[makeDefaultIdKey?]?: idObj }) if makeIdConnect returns the nested unique object
adjust accordingly) and for single relations use connect:
makeIdConnect(relationInfo.idFields, data.data.id); keep updatePayload.include
assignment but ensure IDs produced match the shape used by
processCreate/processRelationshipCRUD. Use the existing symbols processUpdate,
makeIdConnect, makeDefaultIdKey, processCreate, processRelationshipCRUD,
relationInfo, updatePayload and relationships to locate and update the code.
Comment on lines
+1817
to
+1903
| for (const [key, value] of Object.entries(query)) { | ||
| if (!value) { | ||
| continue; | ||
| } | ||
|
|
||
| // try matching query parameter key as "filter[x][y]..." | ||
| const match = key.match(this.filterParamPattern); | ||
| if (!match || !match.groups || !match.groups['match']) { | ||
| continue; | ||
| } | ||
|
|
||
| const filterKeys = match.groups['match'] | ||
| .replaceAll(/[[\]]/g, ' ') | ||
| .split(' ') | ||
| .filter((i) => i); | ||
|
|
||
| if (!filterKeys.length) { | ||
| continue; | ||
| } | ||
|
|
||
| // turn filter into a nested query object | ||
|
|
||
| const item: any = {}; | ||
| let curr = item; | ||
| let currType = typeInfo; | ||
|
|
||
| for (const filterValue of enumerate(value)) { | ||
| for (let i = 0; i < filterKeys.length; i++) { | ||
| // extract filter operation from (optional) trailing $op | ||
| let filterKey = filterKeys[i]!; | ||
| let filterOp: FilterOperationType | undefined; | ||
| const pos = filterKey.indexOf('$'); | ||
| if (pos > 0) { | ||
| filterOp = filterKey.substring(pos + 1) as FilterOperationType; | ||
| filterKey = filterKey.substring(0, pos); | ||
| } | ||
|
|
||
| if (!!filterOp && !FilterOperations.includes(filterOp)) { | ||
| return { | ||
| filter: undefined, | ||
| error: this.makeError('invalidFilter', `invalid filter operation: ${filterOp}`), | ||
| }; | ||
| } | ||
|
|
||
| const idFields = this.getIdFields(currType.name); | ||
| const fieldDef = | ||
| filterKey === 'id' | ||
| ? Object.values(currType.fields).find((f) => idFields.some((idf) => idf.name === f.name)) | ||
| : currType.fields[filterKey]; | ||
| if (!fieldDef) { | ||
| return { filter: undefined, error: this.makeError('invalidFilter') }; | ||
| } | ||
|
|
||
| if (!fieldDef.relation) { | ||
| // regular field | ||
| if (i !== filterKeys.length - 1) { | ||
| // must be the last segment of a filter | ||
| return { filter: undefined, error: this.makeError('invalidFilter') }; | ||
| } | ||
| curr[fieldDef.name] = this.makeFilterValue(fieldDef, filterValue, filterOp); | ||
| } else { | ||
| // relation field | ||
| if (i === filterKeys.length - 1) { | ||
| curr[fieldDef.name] = this.makeFilterValue(fieldDef, filterValue, filterOp); | ||
| } else { | ||
| // keep going | ||
| if (fieldDef.array) { | ||
| // collection filtering implies "some" operation | ||
| curr[fieldDef.name] = { some: {} }; | ||
| curr = curr[fieldDef.name].some; | ||
| } else { | ||
| curr = curr[fieldDef.name] = {}; | ||
| } | ||
| currType = this.getModelInfo(fieldDef.type)!; | ||
| } | ||
| } | ||
| } | ||
| items.push(item); | ||
| } | ||
| } | ||
|
|
||
| if (items.length === 0) { | ||
| return { filter: undefined, error: undefined }; | ||
| } else { | ||
| // combine filters with AND | ||
| return { filter: items.length === 1 ? items[0] : { AND: items }, error: undefined }; | ||
| } |
Contributor
There was a problem hiding this comment.
Repeated filter params collapse to the last value.
item, curr, and currType are created once per query key, then reused for every entry in enumerate(value). A request like ?filter[id]=1&filter[id]=2 overwrites the first filter and pushes the same object twice.
🐛 Proposed fix
- const item: any = {};
- let curr = item;
- let currType = typeInfo;
-
for (const filterValue of enumerate(value)) {
+ const item: any = {};
+ let curr = item;
+ let currType = typeInfo;
+
for (let i = 0; i < filterKeys.length; i++) {
// extract filter operation from (optional) trailing $op
let filterKey = filterKeys[i]!;
@@
}
items.push(item);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/api/rest/handler.ts` around lines 1817 - 1903, The bug is
that item, curr and currType are created once per query key and reused across
enumerate(value) entries causing repeated filter params (e.g.
?filter[id]=1&filter[id]=2) to overwrite the same object; fix it by moving the
creation/initialization of item, curr and currType inside the for (const
filterValue of enumerate(value)) loop so each filterValue builds a fresh item
object, reset curr = item and currType = typeInfo at the start of each
iteration, and then continue using existing logic (getIdFields, getModelInfo,
makeFilterValue, and pushing into items) so each value produces its own
independent filter object.
Comment on lines
+73
to
+89
| private generatePaths(): OpenAPIV3_1.PathsObject { | ||
| const paths: OpenAPIV3_1.PathsObject = {}; | ||
|
|
||
| for (const modelName of getIncludedModels(this.schema, this.queryOptions)) { | ||
| const modelDef = this.schema.models[modelName]!; | ||
| const idFields = this.getIdFields(modelDef); | ||
| if (idFields.length === 0) continue; | ||
|
|
||
| const modelPath = this.getModelPath(modelName); | ||
| const tag = lowerCaseFirst(modelName); | ||
|
|
||
| // Collection: GET (list) + POST (create) | ||
| paths[`/${modelPath}`] = this.buildCollectionPath(modelName, modelDef, tag) as any; | ||
|
|
||
| // Single resource: GET + PATCH + DELETE | ||
| paths[`/${modelPath}/{id}`] = this.buildSinglePath(modelName, tag) as any; | ||
|
|
Contributor
There was a problem hiding this comment.
REST path generation is ignoring includedOperations / excludedOperations.
Collection and single-resource methods are emitted unconditionally, so a sliced config like excludedOperations: ['delete'] still leaves DELETE /model/{id} in the spec. This needs to go through isOperationIncluded() the same way the RPC generator already does.
Also applies to: 136-250
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/api/rest/openapi.ts` around lines 73 - 89, generatePaths
is currently adding collection and single-resource paths unconditionally,
causing operations like DELETE to be included despite excludedOperations; update
generatePaths to check isOperationIncluded(modelName, operation) before adding
each operation and when calling buildCollectionPath and buildSinglePath so only
allowed operations are emitted—specifically, in generatePaths use
isOperationIncluded for collection ops (e.g., 'list','create') before assigning
paths[`/${modelPath}`] and for single-resource ops (e.g.,
'read','update','delete') before assigning paths[`/${modelPath}/{id}`]; also
ensure the lower-level builders (buildCollectionPath and buildSinglePath) accept
or consult operation filters (or are called selectively) so lines in the broader
generation block (the later code around 136-250) follow the same inclusion
checks.
Comment on lines
+73
to
+98
| // Write operations | ||
| const writeOps: Array<{ op: string; method: 'post' | 'patch' | 'delete' }> = [ | ||
| { op: 'create', method: 'post' }, | ||
| { op: 'createMany', method: 'post' }, | ||
| { op: 'createManyAndReturn', method: 'post' }, | ||
| { op: 'upsert', method: 'post' }, | ||
| { op: 'update', method: 'patch' }, | ||
| { op: 'updateMany', method: 'patch' }, | ||
| { op: 'updateManyAndReturn', method: 'patch' }, | ||
| { op: 'delete', method: 'delete' }, | ||
| { op: 'deleteMany', method: 'delete' }, | ||
| ]; | ||
|
|
||
| for (const { op, method } of writeOps) { | ||
| if (!isOperationIncluded(modelName, op, this.queryOptions)) continue; | ||
| const argsSchemaName = `${modelName}${this.opToArgsSchema(op)}`; | ||
| const buildOp = | ||
| method === 'post' | ||
| ? this.buildPostOperation | ||
| : method === 'patch' | ||
| ? this.buildPatchOperation | ||
| : this.buildDeleteOperation; | ||
| paths[`/${modelPath}/${op}`] = { | ||
| [method]: buildOp.call(this, modelName, op, tag, argsSchemaName), | ||
| }; | ||
| } |
Contributor
There was a problem hiding this comment.
createManyAndReturn and updateManyAndReturn reference components that are never generated.
These paths point at ${modelName}CreateManyAndReturnArgs / ${modelName}UpdateManyAndReturnArgs, but generateSchemas() never registers either schema. The emitted OpenAPI will contain dangling request-body refs for both endpoints.
Also applies to: 341-364
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/api/rpc/openapi.ts` around lines 73 - 98, The writeOps
list includes ops createManyAndReturn and updateManyAndReturn that cause
references to `${modelName}CreateManyAndReturnArgs` /
`${modelName}UpdateManyAndReturnArgs` but generateSchemas() never registers
those schemas; update the generation logic so either (A) remove those ops from
writeOps or (B) add schema generation for those args by extending
opToArgsSchema/op mapping and ensure generateSchemas() creates
`${modelName}CreateManyAndReturnArgs` and `${modelName}UpdateManyAndReturnArgs`
(or maps them to existing CreateMany/UpdateMany args) so
buildPostOperation/buildPatchOperation produce valid request-body component refs
and no dangling refs. Ensure to touch writeOps, opToArgsSchema, generateSchemas,
and where buildPostOperation/buildPatchOperation are invoked to keep names
consistent.
Comment on lines
+171
to
+181
| responses: { | ||
| '200': { | ||
| description: `Result of ${op}`, | ||
| content: { | ||
| 'application/json': { | ||
| schema: { $ref: `#/components/schemas/${modelName}Response` }, | ||
| }, | ||
| }, | ||
| }, | ||
| '400': { $ref: '#/components/schemas/_ErrorResponse' }, | ||
| }, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/server/src/api/rpc/openapi.ts | sed -n '160,200p'Repository: zenstackhq/zenstack
Length of output: 1732
🏁 Script executed:
cat -n packages/server/src/api/rpc/openapi.ts | sed -n '195,220p'Repository: zenstackhq/zenstack
Length of output: 1058
🏁 Script executed:
cat -n packages/server/src/api/rpc/openapi.ts | sed -n '225,250p'Repository: zenstackhq/zenstack
Length of output: 1124
🏁 Script executed:
cat -n packages/server/src/api/rpc/openapi.ts | sed -n '260,295p'Repository: zenstackhq/zenstack
Length of output: 1587
🌐 Web query:
OpenAPI 3.0 specification response $ref schema component validation
💡 Result:
In OpenAPI 3.0, when a response schema uses $ref to reference a schema component (e.g., responses: { "200": { content: { application/json: { schema: { $ref: '#/components/schemas/User' } } } }), the $ref is fully valid and supported. The official specification explicitly allows Reference Objects (containing $ref) in place of Schema Objects, including in response content schemas. Key validation aspects: - Document validation: OpenAPI documents are validated against the official JSON Schema for OAS 3.0 (available at spec.openapis.org), which permits $ref in schema fields of Media Type Objects within Response Objects. Tools like oascomply resolve $refs during parsing and validate that targets exist and are of correct type (e.g., a schema $ref must point to a Schema Object). - Resolution rules: $refs follow JSON Reference (RFC 3986), resolved relative to the current document. In OAS 3.0, a $ref object cannot have sibling properties (they are ignored), and it logically replaces itself with the target during resolution. - Runtime/data validation: The referenced schema (e.g., User) defines validation rules for actual response payloads. Tools like AJV (used in many OpenAPI validators) resolve $refs and apply the full schema (including discriminators, oneOf, etc.) to validate JSON payloads against the response schema. - Common pitfalls: $ref only allowed where spec permits (e.g., schema fields); invalid targets or circular refs cause validation errors in conformance tools. Examples from spec: - responses: { "200": { content: { application/json: { schema: { $ref: '#/components/schemas/Pet' } } } } This is standard and passes official validation.
Citations:
- 1: https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.0.3.md
- 2: https://swagger.io/docs/specification/v3_0/using-ref/
- 3: https://swagger.io/docs/specification/v3_0/describing-responses
- 4: https://spec.openapis.org/oas/v3.0.3.html
- 5: https://swagger.io/docs/specification/v3_0/components
- 6: https://spec.openapis.org/oas/3.0/schema/2024-10-18.html
- 7: https://github.com/oai/oascomply
Response-level $ref must target a Response Object, not a Schema Object.
At lines 180, 212, 244, 276, and 288, response definitions use { $ref: '#/components/schemas/_ErrorResponse' } directly at the response level. Per OpenAPI 3.0 specification, a response-level $ref must reference #/components/responses/, not #/components/schemas/. Validators and client generators will reject the document because the target is a Schema Object, not a Response Object.
Either create a Response Object in #/components/responses/ and reference it, or wrap the schema reference in a proper Response Object with description and content:
Fix pattern
- '400': { $ref: '#/components/schemas/_ErrorResponse' },
+ '400': {
+ description: 'Bad request',
+ content: {
+ 'application/json': {
+ schema: { $ref: '#/components/schemas/_ErrorResponse' },
+ },
+ },
+ },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| responses: { | |
| '200': { | |
| description: `Result of ${op}`, | |
| content: { | |
| 'application/json': { | |
| schema: { $ref: `#/components/schemas/${modelName}Response` }, | |
| }, | |
| }, | |
| }, | |
| '400': { $ref: '#/components/schemas/_ErrorResponse' }, | |
| }, | |
| responses: { | |
| '200': { | |
| description: `Result of ${op}`, | |
| content: { | |
| 'application/json': { | |
| schema: { $ref: `#/components/schemas/${modelName}Response` }, | |
| }, | |
| }, | |
| }, | |
| '400': { | |
| description: 'Bad request', | |
| content: { | |
| 'application/json': { | |
| schema: { $ref: '#/components/schemas/_ErrorResponse' }, | |
| }, | |
| }, | |
| }, | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/api/rpc/openapi.ts` around lines 171 - 181, The response
entries currently use a response-level $ref to a Schema Object (e.g. { $ref:
'#/components/schemas/_ErrorResponse' }) which is invalid; update the responses
in the OpenAPI generation (the responses object built alongside modelName and op
in packages/server/src/api/rpc/openapi.ts) to either reference a Response Object
(change to { $ref: '#/components/responses/_ErrorResponse' } and add a
components.responses._ErrorResponse entry that points to the error schema) or
replace the direct $ref with a proper Response Object (include description and
content: application/json with schema: { $ref:
'#/components/schemas/_ErrorResponse' }); apply this change for all occurrences
of the error response generation so validators and generators target
`#/components/responses/` instead of `#/components/schemas/`.
Comment on lines
+477
to
+495
| // Logical combinators | ||
| properties['AND'] = { | ||
| oneOf: [ | ||
| { $ref: `#/components/schemas/_${_modelName}WhereInput` }, | ||
| { type: 'array', items: { $ref: `#/components/schemas/_${_modelName}WhereInput` } }, | ||
| ], | ||
| }; | ||
| properties['OR'] = { | ||
| type: 'array', | ||
| items: { $ref: `#/components/schemas/_${_modelName}WhereInput` }, | ||
| }; | ||
| properties['NOT'] = { | ||
| oneOf: [ | ||
| { $ref: `#/components/schemas/_${_modelName}WhereInput` }, | ||
| { type: 'array', items: { $ref: `#/components/schemas/_${_modelName}WhereInput` } }, | ||
| ], | ||
| }; | ||
|
|
||
| return { type: 'object', properties }; |
Contributor
There was a problem hiding this comment.
The recursive WhereInput refs never resolve.
AND/OR/NOT point to #/components/schemas/_${modelName}WhereInput, but generateSchemas() only creates ${modelName}WhereInput. Every recursive filter schema therefore contains dangling refs.
🐛 Proposed fix
- { $ref: `#/components/schemas/_${_modelName}WhereInput` },
- { type: 'array', items: { $ref: `#/components/schemas/_${_modelName}WhereInput` } },
+ { $ref: `#/components/schemas/${_modelName}WhereInput` },
+ { type: 'array', items: { $ref: `#/components/schemas/${_modelName}WhereInput` } },
...
- items: { $ref: `#/components/schemas/_${_modelName}WhereInput` },
+ items: { $ref: `#/components/schemas/${_modelName}WhereInput` },
...
- { $ref: `#/components/schemas/_${_modelName}WhereInput` },
- { type: 'array', items: { $ref: `#/components/schemas/_${_modelName}WhereInput` } },
+ { $ref: `#/components/schemas/${_modelName}WhereInput` },
+ { type: 'array', items: { $ref: `#/components/schemas/${_modelName}WhereInput` } },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Logical combinators | |
| properties['AND'] = { | |
| oneOf: [ | |
| { $ref: `#/components/schemas/_${_modelName}WhereInput` }, | |
| { type: 'array', items: { $ref: `#/components/schemas/_${_modelName}WhereInput` } }, | |
| ], | |
| }; | |
| properties['OR'] = { | |
| type: 'array', | |
| items: { $ref: `#/components/schemas/_${_modelName}WhereInput` }, | |
| }; | |
| properties['NOT'] = { | |
| oneOf: [ | |
| { $ref: `#/components/schemas/_${_modelName}WhereInput` }, | |
| { type: 'array', items: { $ref: `#/components/schemas/_${_modelName}WhereInput` } }, | |
| ], | |
| }; | |
| return { type: 'object', properties }; | |
| // Logical combinators | |
| properties['AND'] = { | |
| oneOf: [ | |
| { $ref: `#/components/schemas/${_modelName}WhereInput` }, | |
| { type: 'array', items: { $ref: `#/components/schemas/${_modelName}WhereInput` } }, | |
| ], | |
| }; | |
| properties['OR'] = { | |
| type: 'array', | |
| items: { $ref: `#/components/schemas/${_modelName}WhereInput` }, | |
| }; | |
| properties['NOT'] = { | |
| oneOf: [ | |
| { $ref: `#/components/schemas/${_modelName}WhereInput` }, | |
| { type: 'array', items: { $ref: `#/components/schemas/${_modelName}WhereInput` } }, | |
| ], | |
| }; | |
| return { type: 'object', properties }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/api/rpc/openapi.ts` around lines 477 - 495, The recursive
filter refs in the logical combinators (properties['AND'], properties['OR'],
properties['NOT']) point to `#/components/schemas/_${_modelName}WhereInput`
which doesn't match what generateSchemas() emits (`${modelName}WhereInput`),
causing dangling refs; update those three $ref strings to reference the actual
schema name produced by generateSchemas (e.g.,
`#/components/schemas/${modelName}WhereInput`) or otherwise make
generateSchemas() emit the underscored name so the names match (modify the ref
strings in openapi.ts around the AND/OR/NOT blocks or adjust generateSchemas()
schema names accordingly).
| properties: | ||
| self: | ||
| type: string | ||
| description: Link for refetching the curent results |
Contributor
There was a problem hiding this comment.
Typo: "curent" should be "current".
Minor typo in the _links schema description.
✏️ Fix typo
self:
type: string
- description: Link for refetching the curent results
+ description: Link for refetching the current results📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| description: Link for refetching the curent results | |
| self: | |
| type: string | |
| description: Link for refetching the current results |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/server/test/openapi/migrated/baseline/rest-type-coverage-3.1.0.baseline.yaml`
at line 549, Fix the typo in the OpenAPI baseline description under the _links
schema: change the description string "Link for refetching the curent results"
to "Link for refetching the current results" so the word "curent" is corrected
to "current" in the description field.
Comment on lines
+609
to
+610
| description: Zod validation errors if the error is due to data validation | ||
| failure |
Contributor
There was a problem hiding this comment.
Fix YAML multiline formatting in description.
The description for zodErrors appears to have an unintended line break causing "failure" to appear on a separate line.
✏️ Fix formatting
zodErrors:
type: object
additionalProperties: true
- description: Zod validation errors if the error is due to data validation
- failure
+ description: Zod validation errors if the error is due to data validation failure📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| description: Zod validation errors if the error is due to data validation | |
| failure | |
| zodErrors: | |
| type: object | |
| additionalProperties: true | |
| description: Zod validation errors if the error is due to data validation failure |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/server/test/openapi/migrated/baseline/rest-type-coverage-3.1.0.baseline.yaml`
around lines 609 - 610, The YAML description for the schema property named
"zodErrors" has an unintended line break so the word "failure" is split onto its
own line; fix it by merging the lines into a single properly quoted or block
scalar YAML value (e.g., use a single-line string or a pipe/greater-than block)
so the description reads continuously as "Zod validation errors if the error is
due to data validation failure", and update the "description" value for the
zodErrors entry accordingly.
Contributor
|
Claude finished @ymc9's task —— View job Code Review for OpenAPI Spec Generation PR
|
Contributor
|
Claude finished @ymc9's task —— View job Code Review for OpenAPI Spec Generation PRI've conducted a comprehensive review of this PR that adds OpenAPI spec generation with filter kind slicing support. Here's my detailed feedback: Review Tasks:
✅ Major StrengthsExcellent Architecture & Design
Robust Filter Kind Slicing Implementation
Strong Test Coverage
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (3)
packages/server/src/api/rest/openapi.ts (3)
73-89:⚠️ Potential issue | 🟠 MajorHonor operation slicing before publishing REST methods.
generatePaths()always assigns both collection and single-resource path items, and the builders always return every method, soincludedOperations/excludedOperationscan't removeget,post,patch, ordeletefrom the emitted spec. Gate each method withisOperationIncluded(...)and skip empty path items.Also applies to: 136-188, 191-250
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/api/rest/openapi.ts` around lines 73 - 89, generatePaths currently unconditionally adds collection and single-resource PathItems by calling buildCollectionPath and buildSinglePath which themselves always include all methods; modify generatePaths and the builders (buildCollectionPath, buildSinglePath) to check isOperationIncluded(modelName, operation) before adding each operation (e.g., "get", "post", "patch", "delete") to the PathItem so omitted operations are not emitted, and ensure generatePaths skips adding a PathItem if it ends up empty after filtering; update logic used by getIncludedModels/getModelPath as needed so only included operations produce entries in the final PathsObject.
150-160:⚠️ Potential issue | 🔴 CriticalUse Response Objects for error statuses.
Every
400/404entry here points directly at#/components/schemas/_errorResponse. Under OpenAPI 3.1, a status-code response must be a Response Object or a Reference Object tocomponents/responses; schema refs only belong undercontent.*.schema. As written, these entries are invalid and also omit the requireddescription.#!/bin/bash set -euo pipefail file="packages/server/src/api/rest/openapi.ts" echo "Direct schema refs under response status codes:" rg -n "'(400|404)':\s*\{\s*\$ref:\s*'#/components/schemas/_errorResponse'" "$file" echo echo "Expected: no matches after wrapping these as Response Objects or moving them to components.responses."Also applies to: 175-185, 199-209, 225-236, 244-247, 278-281, 308-314, 325-328, 339-342, 360-363, 375-378
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/api/rest/openapi.ts` around lines 150 - 160, The response entries for error status codes (e.g., the '400' and '404' entries next to modelName in openapi.ts) incorrectly reference a schema (`#/components/schemas/_errorResponse`) directly instead of using a Response Object; update each such status-code entry to be either a Reference Object to components.responses (e.g., $ref: '#/components/responses/_errorResponse') or a full Response Object that includes a required description and a content block whose schema references `#/components/schemas/_errorResponse`, and apply the same change to all occurrences flagged (near the blocks that construct responses for modelNameListResponse and other endpoints). Ensure each response entry includes description when not using a $ref and replace direct schema refs under the status keys with the proper responses structure.
91-113:⚠️ Potential issue | 🟠 MajorRelation slicing still leaks endpoints and dangling refs.
generatePaths()never checksisFieldOmitted(modelName, fieldName, this.queryOptions), so omitted relation fields still get/{field}and/relationships/{field}paths. Separately,buildModelReadSchema()can still emit$ref: '#/components/schemas/${fieldDef.type}'for a kept relation whose target model was sliced out, which leaves a dangling ref because that component is never generated.Also applies to: 675-680
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/api/rest/openapi.ts` around lines 91 - 113, generatePaths() currently adds relation endpoints without checking isFieldOmitted and can refer to relation target schemas that were sliced out; update the loop over modelDef.fields in generatePaths() to skip any field where isFieldOmitted(modelName, fieldName, this.queryOptions) returns true and also ensure that before adding a relation path you verify the relation target model still exists in this.schema.models (reuse the existing relModelDef check) and that relModelDef was not sliced out; additionally, update buildModelReadSchema() to avoid emitting $ref to fieldDef.type when that target model has been omitted (check this.schema.models[fieldDef.type] or a slice/keep predicate) and emit an appropriate inline schema or remove the relation property instead to prevent dangling $ref; change references in generatePaths() to only call buildFetchRelatedPath and buildRelationshipPath when the field is not omitted and the target model is present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/server/src/api/rest/openapi.ts`:
- Around line 253-284: In buildFetchRelatedPath, the 200 response currently only
has a description; update the responses object so the '200' entry includes a
proper schema $ref: use `#/components/schemas/${fieldDef.type}ListResponse` when
fieldDef.array is true (collection) and
`#/components/schemas/${fieldDef.type}Response` when false (singular). Locate
the method buildFetchRelatedPath and modify the returned get.responses['200'] to
point at the correct schema based on isCollection (using
modelName/fieldName/fieldDef to determine the type).
- Around line 671-691: buildModelReadSchema currently emits fields at the top
level which causes the later allOf merge with the _resource wrapper to put
required attributes and relation refs at the wrong level; update
buildModelReadSchema so it returns an object with type: 'object' and two
properties: attributes (an object whose properties are the non-relation scalar
fields produced via fieldToSchema, with required moved into attributes.required)
and relationships (an object whose properties are the relation fields as
ReferenceObject refs to related model schemas), keeping existing checks
isFieldOmitted, fieldDef.omit, and this.queryOptions; ensure any array/optional
logic remains but that required only lists attribute names under
attributes.required and relation fields are only referenced under relationships
rather than at the top level.
- Around line 694-757: The builders (buildCreateRequestSchema and the similar
Update builder) currently skip fields only by fieldDef.omit but don't respect
per-request field slicing; modify buildCreateRequestSchema (and the Update
counterpart) to call isFieldOmitted(_modelName, fieldName, modelDef) for each
field and skip the field when it returns true (in addition to existing checks
like fieldDef.omit/updatedAt/foreignKeyFor/id with default); ensure you actually
use the _modelName parameter passed into buildCreateRequestSchema and mirror the
same change in the update schema builder so fields removed via queryOptions are
not included in CreateRequest/UpdateRequest.
In `@packages/server/src/api/rpc/openapi.ts`:
- Around line 401-412: The loop over modelDef.fields used by the
CreateInput/UpdateInput builders calls typeToSchema(fieldDef.type) directly,
which loses nullability and array shape; update the logic in the
CreateInput/UpdateInput builders (the block that fills properties and required)
to: 1) detect when fieldDef.type represents an array and set
properties[fieldName] to an OpenAPI array schema with items produced by
typeToSchema(baseType) instead of passing the array type through unchanged; 2)
preserve nullable by setting properties[fieldName].nullable = true when the
field type allows null (or by merging/using the nullable flag returned from
typeToSchema for the inner/base type); and 3) adjust the required calculation to
include non-optional array fields (remove the current !fieldDef.array guard) so
list columns that are required remain required; ensure you reference and update
the same symbols used now (typeToSchema, modelDef.fields, properties, required)
so CreateInput and UpdateInput both get the fix.
- Around line 380-386: The generator is marking non-array, non-optional relation
fields as required which is incorrect; update the required-pushing logic in the
loop over modelDef.fields (the block using isFieldOmitted, fieldToSchema and
this.queryOptions) to skip pushing relation fields into required—i.e., add a
relation-detection check (e.g., detect relation/object-kind fields via
fieldDef.kind/type/relationship flag used in your schema representation) and
change the condition to: if (!fieldDef.optional && !fieldDef.array &&
!isRelationField(fieldDef)) { required.push(fieldName); } so relation payloads
remain optional unless explicitly selected/included.
- Around line 688-735: buildFieldFilterSchema currently uses baseSchema =
this.typeToSchema(fieldDef.type) which strips array and optional wrappers so
filters like equals/in/notIn accept only the element type and cannot represent
null or arrays; change it to build a wrapped schema that respects fieldDef.array
and fieldDef.optional (e.g. if fieldDef.array wrap baseSchema into an array
schema for equals/in/notIn where appropriate, and if fieldDef.optional mark the
schema as nullable or include null in the type) and use that wrapped schema for
filterProps entries such as 'equals', 'not', 'in', 'notIn' (while keeping
element-level schemas for list operators like 'has'/'hasEvery'/'hasSome' if
intended); update references to this.typeToSchema, fieldDef.array, and
fieldDef.optional inside buildFieldFilterSchema to produce and use
baseSchemaWithWrappers instead of the bare baseSchema.
---
Duplicate comments:
In `@packages/server/src/api/rest/openapi.ts`:
- Around line 73-89: generatePaths currently unconditionally adds collection and
single-resource PathItems by calling buildCollectionPath and buildSinglePath
which themselves always include all methods; modify generatePaths and the
builders (buildCollectionPath, buildSinglePath) to check
isOperationIncluded(modelName, operation) before adding each operation (e.g.,
"get", "post", "patch", "delete") to the PathItem so omitted operations are not
emitted, and ensure generatePaths skips adding a PathItem if it ends up empty
after filtering; update logic used by getIncludedModels/getModelPath as needed
so only included operations produce entries in the final PathsObject.
- Around line 150-160: The response entries for error status codes (e.g., the
'400' and '404' entries next to modelName in openapi.ts) incorrectly reference a
schema (`#/components/schemas/_errorResponse`) directly instead of using a
Response Object; update each such status-code entry to be either a Reference
Object to components.responses (e.g., $ref:
'#/components/responses/_errorResponse') or a full Response Object that includes
a required description and a content block whose schema references
`#/components/schemas/_errorResponse`, and apply the same change to all
occurrences flagged (near the blocks that construct responses for
modelNameListResponse and other endpoints). Ensure each response entry includes
description when not using a $ref and replace direct schema refs under the
status keys with the proper responses structure.
- Around line 91-113: generatePaths() currently adds relation endpoints without
checking isFieldOmitted and can refer to relation target schemas that were
sliced out; update the loop over modelDef.fields in generatePaths() to skip any
field where isFieldOmitted(modelName, fieldName, this.queryOptions) returns true
and also ensure that before adding a relation path you verify the relation
target model still exists in this.schema.models (reuse the existing relModelDef
check) and that relModelDef was not sliced out; additionally, update
buildModelReadSchema() to avoid emitting $ref to fieldDef.type when that target
model has been omitted (check this.schema.models[fieldDef.type] or a slice/keep
predicate) and emit an appropriate inline schema or remove the relation property
instead to prevent dangling $ref; change references in generatePaths() to only
call buildFetchRelatedPath and buildRelationshipPath when the field is not
omitted and the target model is present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d8ec89b2-c972-4703-b6c5-f4719db96185
📒 Files selected for processing (2)
packages/server/src/api/rest/openapi.tspackages/server/src/api/rpc/openapi.ts
Comment on lines
+253
to
+284
| private buildFetchRelatedPath( | ||
| modelName: string, | ||
| fieldName: string, | ||
| fieldDef: FieldDef, | ||
| tag: string, | ||
| ): Record<string, any> { | ||
| const isCollection = !!fieldDef.array; | ||
| const params: any[] = [{ $ref: '#/components/parameters/id' }, { $ref: '#/components/parameters/include' }]; | ||
|
|
||
| if (isCollection && this.schema.models[fieldDef.type]) { | ||
| const relModelDef = this.schema.models[fieldDef.type]!; | ||
| params.push( | ||
| { $ref: '#/components/parameters/sort' }, | ||
| { $ref: '#/components/parameters/pageOffset' }, | ||
| { $ref: '#/components/parameters/pageLimit' }, | ||
| ...this.buildFilterParams(fieldDef.type, relModelDef), | ||
| ); | ||
| } | ||
|
|
||
| return { | ||
| get: { | ||
| tags: [tag], | ||
| summary: `Fetch related ${fieldDef.type} for ${modelName}`, | ||
| operationId: `get${modelName}_${fieldName}`, | ||
| parameters: params, | ||
| responses: { | ||
| '200': { description: `Related ${fieldDef.type} resource(s)` }, | ||
| '404': { $ref: '#/components/schemas/_errorResponse' }, | ||
| }, | ||
| }, | ||
| }; | ||
| } |
Contributor
There was a problem hiding this comment.
Document the 200 body for related-resource GETs.
buildFetchRelatedPath() knows both the related model and whether the relation is singular or plural, but the 200 response only has a description. That leaves every GET /{model}/{id}/{field} endpoint without a usable response schema. Point it at ${fieldDef.type}Response or ${fieldDef.type}ListResponse.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/api/rest/openapi.ts` around lines 253 - 284, In
buildFetchRelatedPath, the 200 response currently only has a description; update
the responses object so the '200' entry includes a proper schema $ref: use
`#/components/schemas/${fieldDef.type}ListResponse` when fieldDef.array is true
(collection) and `#/components/schemas/${fieldDef.type}Response` when false
(singular). Locate the method buildFetchRelatedPath and modify the returned
get.responses['200'] to point at the correct schema based on isCollection (using
modelName/fieldName/fieldDef to determine the type).
Comment on lines
+671
to
+691
| private buildModelReadSchema(modelName: string, modelDef: ModelDef): SchemaObject { | ||
| const properties: Record<string, SchemaObject | ReferenceObject> = {}; | ||
| const required: string[] = []; | ||
|
|
||
| for (const [fieldName, fieldDef] of Object.entries(modelDef.fields)) { | ||
| if (fieldDef.omit) continue; | ||
| if (isFieldOmitted(modelName, fieldName, this.queryOptions)) continue; | ||
|
|
||
| const schema = this.fieldToSchema(fieldDef); | ||
| properties[fieldName] = schema; | ||
|
|
||
| if (!fieldDef.optional && !fieldDef.array) { | ||
| required.push(fieldName); | ||
| } | ||
| } | ||
|
|
||
| const result: SchemaObject = { type: 'object', properties }; | ||
| if (required.length > 0) { | ||
| result.required = required; | ||
| } | ||
| return result; |
Contributor
There was a problem hiding this comment.
data currently requires the wrong JSON:API shape.
buildModelReadSchema() emits raw scalar/relation fields, then the response wrappers allOf-merge that object with _resource. The result puts required attributes at the top level and relation fields as nested model refs, instead of under attributes / relationships.
Also applies to: 816-848
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/api/rest/openapi.ts` around lines 671 - 691,
buildModelReadSchema currently emits fields at the top level which causes the
later allOf merge with the _resource wrapper to put required attributes and
relation refs at the wrong level; update buildModelReadSchema so it returns an
object with type: 'object' and two properties: attributes (an object whose
properties are the non-relation scalar fields produced via fieldToSchema, with
required moved into attributes.required) and relationships (an object whose
properties are the relation fields as ReferenceObject refs to related model
schemas), keeping existing checks isFieldOmitted, fieldDef.omit, and
this.queryOptions; ensure any array/optional logic remains but that required
only lists attribute names under attributes.required and relation fields are
only referenced under relationships rather than at the top level.
Comment on lines
+694
to
+757
| private buildCreateRequestSchema(_modelName: string, modelDef: ModelDef): SchemaObject { | ||
| const idFieldNames = new Set(modelDef.idFields); | ||
| const attributes: Record<string, SchemaObject | ReferenceObject> = {}; | ||
| const attrRequired: string[] = []; | ||
| const relationships: Record<string, SchemaObject | ReferenceObject> = {}; | ||
|
|
||
| for (const [fieldName, fieldDef] of Object.entries(modelDef.fields)) { | ||
| if (fieldDef.omit) continue; | ||
| if (fieldDef.updatedAt) continue; | ||
| if (fieldDef.foreignKeyFor) continue; | ||
| // Skip auto-generated id fields | ||
| if (idFieldNames.has(fieldName) && fieldDef.default !== undefined) continue; | ||
|
|
||
| if (fieldDef.relation) { | ||
| relationships[fieldName] = fieldDef.array | ||
| ? { | ||
| type: 'object', | ||
| properties: { | ||
| data: { | ||
| type: 'array', | ||
| items: { $ref: '#/components/schemas/_resourceIdentifier' }, | ||
| }, | ||
| }, | ||
| } | ||
| : { | ||
| type: 'object', | ||
| properties: { | ||
| data: { $ref: '#/components/schemas/_resourceIdentifier' }, | ||
| }, | ||
| }; | ||
| } else { | ||
| attributes[fieldName] = this.fieldToSchema(fieldDef); | ||
| if (!fieldDef.optional && fieldDef.default === undefined && !fieldDef.array) { | ||
| attrRequired.push(fieldName); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const dataProperties: Record<string, SchemaObject | ReferenceObject> = { | ||
| type: { type: 'string' }, | ||
| }; | ||
|
|
||
| if (Object.keys(attributes).length > 0) { | ||
| const attrSchema: SchemaObject = { type: 'object', properties: attributes }; | ||
| if (attrRequired.length > 0) attrSchema.required = attrRequired; | ||
| dataProperties['attributes'] = attrSchema; | ||
| } | ||
|
|
||
| if (Object.keys(relationships).length > 0) { | ||
| dataProperties['relationships'] = { type: 'object', properties: relationships }; | ||
| } | ||
|
|
||
| return { | ||
| type: 'object', | ||
| properties: { | ||
| data: { | ||
| type: 'object', | ||
| properties: dataProperties, | ||
| required: ['type'], | ||
| }, | ||
| }, | ||
| required: ['data'], | ||
| }; | ||
| } |
Contributor
There was a problem hiding this comment.
Field-level slicing is only applied to read schemas.
These builders skip fieldDef.omit, but they never call isFieldOmitted(...), so fields removed via queryOptions still show up in CreateRequest and UpdateRequest. _modelName being unused is the giveaway.
Minimal fix shape
- private buildCreateRequestSchema(_modelName: string, modelDef: ModelDef): SchemaObject {
+ private buildCreateRequestSchema(modelName: string, modelDef: ModelDef): SchemaObject {
const idFieldNames = new Set(modelDef.idFields);
const attributes: Record<string, SchemaObject | ReferenceObject> = {};
const attrRequired: string[] = [];
const relationships: Record<string, SchemaObject | ReferenceObject> = {};
for (const [fieldName, fieldDef] of Object.entries(modelDef.fields)) {
- if (fieldDef.omit) continue;
+ if (fieldDef.omit || isFieldOmitted(modelName, fieldName, this.queryOptions)) continue;
if (fieldDef.updatedAt) continue;
if (fieldDef.foreignKeyFor) continue;
// Skip auto-generated id fields
if (idFieldNames.has(fieldName) && fieldDef.default !== undefined) continue;- schemas[`${modelName}UpdateRequest`] = this.buildUpdateRequestSchema(modelDef);
+ schemas[`${modelName}UpdateRequest`] = this.buildUpdateRequestSchema(modelName, modelDef);- private buildUpdateRequestSchema(modelDef: ModelDef): SchemaObject {
+ private buildUpdateRequestSchema(modelName: string, modelDef: ModelDef): SchemaObject {
const attributes: Record<string, SchemaObject | ReferenceObject> = {};
const relationships: Record<string, SchemaObject | ReferenceObject> = {};
for (const [fieldName, fieldDef] of Object.entries(modelDef.fields)) {
- if (fieldDef.omit) continue;
+ if (fieldDef.omit || isFieldOmitted(modelName, fieldName, this.queryOptions)) continue;
if (fieldDef.updatedAt) continue;
if (fieldDef.foreignKeyFor) continue;Also applies to: 759-814
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/api/rest/openapi.ts` around lines 694 - 757, The builders
(buildCreateRequestSchema and the similar Update builder) currently skip fields
only by fieldDef.omit but don't respect per-request field slicing; modify
buildCreateRequestSchema (and the Update counterpart) to call
isFieldOmitted(_modelName, fieldName, modelDef) for each field and skip the
field when it returns true (in addition to existing checks like
fieldDef.omit/updatedAt/foreignKeyFor/id with default); ensure you actually use
the _modelName parameter passed into buildCreateRequestSchema and mirror the
same change in the update schema builder so fields removed via queryOptions are
not included in CreateRequest/UpdateRequest.
Comment on lines
+380
to
+386
| for (const [fieldName, fieldDef] of Object.entries(modelDef.fields)) { | ||
| if (fieldDef.omit) continue; | ||
| if (isFieldOmitted(modelName, fieldName, this.queryOptions)) continue; | ||
| properties[fieldName] = this.fieldToSchema(fieldDef); | ||
| if (!fieldDef.optional && !fieldDef.array) { | ||
| required.push(fieldName); | ||
| } |
Contributor
There was a problem hiding this comment.
Don't require relation fields in the shared model schema.
This generator already models select/include on the args side, so relation payloads are conditional. Marking every non-array, non-optional relation as required makes ordinary RPC responses invalid unless the caller explicitly requested that relation.
Suggested fix
for (const [fieldName, fieldDef] of Object.entries(modelDef.fields)) {
if (fieldDef.omit) continue;
if (isFieldOmitted(modelName, fieldName, this.queryOptions)) continue;
properties[fieldName] = this.fieldToSchema(fieldDef);
- if (!fieldDef.optional && !fieldDef.array) {
+ if (!fieldDef.optional && !fieldDef.array && !fieldDef.relation) {
required.push(fieldName);
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/api/rpc/openapi.ts` around lines 380 - 386, The generator
is marking non-array, non-optional relation fields as required which is
incorrect; update the required-pushing logic in the loop over modelDef.fields
(the block using isFieldOmitted, fieldToSchema and this.queryOptions) to skip
pushing relation fields into required—i.e., add a relation-detection check
(e.g., detect relation/object-kind fields via fieldDef.kind/type/relationship
flag used in your schema representation) and change the condition to: if
(!fieldDef.optional && !fieldDef.array && !isRelationField(fieldDef)) {
required.push(fieldName); } so relation payloads remain optional unless
explicitly selected/included.
Comment on lines
+401
to
+412
| for (const [fieldName, fieldDef] of Object.entries(modelDef.fields)) { | ||
| if (fieldDef.relation) continue; | ||
| if (fieldDef.foreignKeyFor) continue; | ||
| if (fieldDef.omit) continue; | ||
| if (fieldDef.updatedAt) continue; | ||
| // Skip auto-generated id fields | ||
| if (idFieldNames.has(fieldName) && fieldDef.default !== undefined) continue; | ||
|
|
||
| properties[fieldName] = this.typeToSchema(fieldDef.type); | ||
| if (!fieldDef.optional && fieldDef.default === undefined && !fieldDef.array) { | ||
| required.push(fieldName); | ||
| } |
Contributor
There was a problem hiding this comment.
CreateInput and UpdateInput drop nullability and list shape.
Both builders go straight through typeToSchema(fieldDef.type), so nullable scalars are emitted as non-null and scalar-list columns are emitted as single values. CreateInput also never marks required list fields as required because of the !fieldDef.array guard.
Suggested fix
- properties[fieldName] = this.typeToSchema(fieldDef.type);
- if (!fieldDef.optional && fieldDef.default === undefined && !fieldDef.array) {
+ const fieldSchema = fieldDef.array
+ ? { type: 'array', items: this.typeToSchema(fieldDef.type) }
+ : this.typeToSchema(fieldDef.type);
+ properties[fieldName] = fieldDef.optional ? { oneOf: [fieldSchema, { type: 'null' }] } : fieldSchema;
+ if (!fieldDef.optional && fieldDef.default === undefined) {
required.push(fieldName);
}
...
- properties[fieldName] = this.typeToSchema(fieldDef.type);
+ const fieldSchema = fieldDef.array
+ ? { type: 'array', items: this.typeToSchema(fieldDef.type) }
+ : this.typeToSchema(fieldDef.type);
+ properties[fieldName] = fieldDef.optional ? { oneOf: [fieldSchema, { type: 'null' }] } : fieldSchema;Also applies to: 425-432
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/api/rpc/openapi.ts` around lines 401 - 412, The loop over
modelDef.fields used by the CreateInput/UpdateInput builders calls
typeToSchema(fieldDef.type) directly, which loses nullability and array shape;
update the logic in the CreateInput/UpdateInput builders (the block that fills
properties and required) to: 1) detect when fieldDef.type represents an array
and set properties[fieldName] to an OpenAPI array schema with items produced by
typeToSchema(baseType) instead of passing the array type through unchanged; 2)
preserve nullable by setting properties[fieldName].nullable = true when the
field type allows null (or by merging/using the nullable flag returned from
typeToSchema for the inner/base type); and 3) adjust the required calculation to
include non-optional array fields (remove the current !fieldDef.array guard) so
list columns that are required remain required; ensure you reference and update
the same symbols used now (typeToSchema, modelDef.fields, properties, required)
so CreateInput and UpdateInput both get the fix.
Comment on lines
+688
to
+735
| private buildFieldFilterSchema(modelName: string, fieldName: string, fieldDef: FieldDef): SchemaObject | undefined { | ||
| const baseSchema = this.typeToSchema(fieldDef.type); | ||
| const filterProps: Record<string, SchemaObject | ReferenceObject> = {}; | ||
| const type = fieldDef.type; | ||
|
|
||
| // Equality operators | ||
| if (isFilterKindIncluded(modelName, fieldName, 'Equality', this.queryOptions)) { | ||
| filterProps['equals'] = baseSchema; | ||
| filterProps['not'] = baseSchema; | ||
| filterProps['in'] = { type: 'array', items: baseSchema }; | ||
| filterProps['notIn'] = { type: 'array', items: baseSchema }; | ||
| } | ||
|
|
||
| // Range operators (numeric/datetime types) | ||
| if ( | ||
| (type === 'Int' || type === 'Float' || type === 'BigInt' || type === 'Decimal' || type === 'DateTime') && | ||
| isFilterKindIncluded(modelName, fieldName, 'Range', this.queryOptions) | ||
| ) { | ||
| filterProps['lt'] = baseSchema; | ||
| filterProps['lte'] = baseSchema; | ||
| filterProps['gt'] = baseSchema; | ||
| filterProps['gte'] = baseSchema; | ||
| } | ||
|
|
||
| // Like operators (String type) | ||
| if (type === 'String' && isFilterKindIncluded(modelName, fieldName, 'Like', this.queryOptions)) { | ||
| filterProps['contains'] = { type: 'string' }; | ||
| filterProps['startsWith'] = { type: 'string' }; | ||
| filterProps['endsWith'] = { type: 'string' }; | ||
| filterProps['mode'] = { type: 'string', enum: ['default', 'insensitive'] }; | ||
| } | ||
|
|
||
| // List operators (array fields) | ||
| if (fieldDef.array && isFilterKindIncluded(modelName, fieldName, 'List', this.queryOptions)) { | ||
| filterProps['has'] = baseSchema; | ||
| filterProps['hasEvery'] = { type: 'array', items: baseSchema }; | ||
| filterProps['hasSome'] = { type: 'array', items: baseSchema }; | ||
| filterProps['isEmpty'] = { type: 'boolean' }; | ||
| } | ||
|
|
||
| if (Object.keys(filterProps).length === 0) return undefined; | ||
|
|
||
| const filterObject: SchemaObject = { type: 'object', properties: filterProps }; | ||
|
|
||
| // If Equality is included, allow shorthand (direct value) via oneOf | ||
| if (isFilterKindIncluded(modelName, fieldName, 'Equality', this.queryOptions)) { | ||
| return { oneOf: [baseSchema, filterObject] }; | ||
| } |
Contributor
There was a problem hiding this comment.
Filter values should mirror the field shape, not just the element type.
buildFieldFilterSchema() starts from typeToSchema(fieldDef.type), which strips both fieldDef.array and fieldDef.optional. That makes list equals/shorthand filters accept a single element and prevents null comparisons on nullable fields.
Suggested fix
- const baseSchema = this.typeToSchema(fieldDef.type);
+ const elementSchema = this.typeToSchema(fieldDef.type);
+ const valueSchema = fieldDef.optional
+ ? {
+ oneOf: [
+ fieldDef.array ? { type: 'array', items: elementSchema } : elementSchema,
+ { type: 'null' },
+ ],
+ }
+ : fieldDef.array
+ ? { type: 'array', items: elementSchema }
+ : elementSchema;
const filterProps: Record<string, SchemaObject | ReferenceObject> = {};
const type = fieldDef.type;
// Equality operators
if (isFilterKindIncluded(modelName, fieldName, 'Equality', this.queryOptions)) {
- filterProps['equals'] = baseSchema;
- filterProps['not'] = baseSchema;
- filterProps['in'] = { type: 'array', items: baseSchema };
- filterProps['notIn'] = { type: 'array', items: baseSchema };
+ filterProps['equals'] = valueSchema;
+ filterProps['not'] = valueSchema;
+ filterProps['in'] = { type: 'array', items: valueSchema };
+ filterProps['notIn'] = { type: 'array', items: valueSchema };
}
// Range operators (numeric/datetime types)
if (
(type === 'Int' || type === 'Float' || type === 'BigInt' || type === 'Decimal' || type === 'DateTime') &&
isFilterKindIncluded(modelName, fieldName, 'Range', this.queryOptions)
) {
- filterProps['lt'] = baseSchema;
- filterProps['lte'] = baseSchema;
- filterProps['gt'] = baseSchema;
- filterProps['gte'] = baseSchema;
+ filterProps['lt'] = elementSchema;
+ filterProps['lte'] = elementSchema;
+ filterProps['gt'] = elementSchema;
+ filterProps['gte'] = elementSchema;
}
...
if (fieldDef.array && isFilterKindIncluded(modelName, fieldName, 'List', this.queryOptions)) {
- filterProps['has'] = baseSchema;
- filterProps['hasEvery'] = { type: 'array', items: baseSchema };
- filterProps['hasSome'] = { type: 'array', items: baseSchema };
+ filterProps['has'] = elementSchema;
+ filterProps['hasEvery'] = { type: 'array', items: elementSchema };
+ filterProps['hasSome'] = { type: 'array', items: elementSchema };
filterProps['isEmpty'] = { type: 'boolean' };
}
...
- return { oneOf: [baseSchema, filterObject] };
+ return { oneOf: [valueSchema, filterObject] };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/api/rpc/openapi.ts` around lines 688 - 735,
buildFieldFilterSchema currently uses baseSchema =
this.typeToSchema(fieldDef.type) which strips array and optional wrappers so
filters like equals/in/notIn accept only the element type and cannot represent
null or arrays; change it to build a wrapped schema that respects fieldDef.array
and fieldDef.optional (e.g. if fieldDef.array wrap baseSchema into an array
schema for equals/in/notIn where appropriate, and if fieldDef.optional mark the
schema as nullable or include null in the type) and use that wrapped schema for
filterProps entries such as 'equals', 'not', 'in', 'notIn' (while keeping
element-level schemas for list operators like 'has'/'hasEvery'/'hasSome' if
intended); update references to this.typeToSchema, fieldDef.array, and
fieldDef.optional inside buildFieldFilterSchema to produce and use
baseSchemaWithWrappers instead of the bare baseSchema.
…n OpenAPI Extract model description from @@meta("description", "...") attribute and apply it to the generated schema object in both REST and RPC OpenAPI generators. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (7)
packages/server/src/api/rpc/openapi.ts (7)
482-497:⚠️ Potential issue | 🔴 CriticalRecursive
WhereInputrefs never resolve.
generateSchemas()only registers${modelName}WhereInput, butAND/OR/NOTpoint at#/components/schemas/_${modelName}WhereInput. Every recursive filter schema will contain dangling refs.Suggested fix
- { $ref: `#/components/schemas/_${_modelName}WhereInput` }, - { type: 'array', items: { $ref: `#/components/schemas/_${_modelName}WhereInput` } }, + { $ref: `#/components/schemas/${_modelName}WhereInput` }, + { type: 'array', items: { $ref: `#/components/schemas/${_modelName}WhereInput` } }, ... - items: { $ref: `#/components/schemas/_${_modelName}WhereInput` }, + items: { $ref: `#/components/schemas/${_modelName}WhereInput` }, ... - { $ref: `#/components/schemas/_${_modelName}WhereInput` }, - { type: 'array', items: { $ref: `#/components/schemas/_${_modelName}WhereInput` } }, + { $ref: `#/components/schemas/${_modelName}WhereInput` }, + { type: 'array', items: { $ref: `#/components/schemas/${_modelName}WhereInput` } },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/api/rpc/openapi.ts` around lines 482 - 497, The recursive WhereInput refs are pointing to `#/components/schemas/_`${modelName}WhereInput while generateSchemas() registers ${modelName}WhereInput, leaving dangling refs; update the schema registration in generateSchemas() to also register the underscored name (register a schema under _${modelName}WhereInput) or change the AND/OR/NOT refs in the block that sets properties['AND'], properties['OR'], properties['NOT'] to point to `#/components/schemas/`${modelName}WhereInput so the references match—locate generateSchemas() and the properties['AND'|'OR'|'NOT'] lines to make the names consistent.
75-98:⚠️ Potential issue | 🔴 Critical
createManyAndReturnandupdateManyAndReturnreference request components that are never generated.
opToArgsSchema()resolves these ops to${modelName}CreateManyAndReturnArgsand${modelName}UpdateManyAndReturnArgs, butgenerateSchemas()never registers either component. The resulting OpenAPI will contain dangling request-body refs for both endpoints unless these ops are mapped to existing args schemas or dedicated schemas are emitted.Also applies to: 155-157, 350-354
127-130:⚠️ Potential issue | 🔴 CriticalResponse-level
$refis pointing at the wrong component type.These
400entries sit inside a Responses Object, so a$refhere has to resolve to a Response Object, not#/components/schemas/_ErrorResponse. Validators and codegen will reject the document until this is wrapped as a real response or moved undercomponents.responses.Suggested fix pattern
- '400': { $ref: '#/components/schemas/_ErrorResponse' }, + '400': { + description: 'Bad request', + content: { + 'application/json': { + schema: { $ref: '#/components/schemas/_ErrorResponse' }, + }, + }, + },In OpenAPI 3.1, when a response entry inside `responses` uses `$ref`, can it target `#/components/schemas/...`, or must it reference a Response Object under `#/components/responses/...`?Also applies to: 172-182, 204-214, 236-246, 268-278, 287-290
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/api/rpc/openapi.ts` around lines 127 - 130, The responses objects currently use a $ref that points to a schema (e.g. '400': { $ref: '#/components/schemas/_ErrorResponse' }) which is invalid because response-level $ref must resolve to a Response Object; update each offending entry (the '400' response refs and the other similar entries around the other occurrences) to either reference a Response Object under `#/components/responses` (e.g. '#/components/responses/_ErrorResponse') or replace the $ref with an inline Response Object that includes description and content referencing the schema (e.g. description plus content: application/json with schema: { $ref: '#/components/schemas/_ErrorResponse' }); ensure you apply this change for every responses block that currently points directly at the schema.
414-416:⚠️ Potential issue | 🟠 Major
CreateInputandUpdateInputdrop nullable and list field shape.These builders go straight through
typeToSchema(fieldDef.type), so nullable columns are emitted as non-null and list columns are emitted as single values. Line 415 also skips required scalar-list fields because of!fieldDef.array.Also applies to: 436-436
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/api/rpc/openapi.ts` around lines 414 - 416, CreateInput/UpdateInput builders currently call typeToSchema(fieldDef.type) directly, which loses nullable and array shapes and skips required array fields; update the property generation so that if fieldDef.array is true you wrap the element schema into an array schema (type: "array", items: typeToSchema(fieldDef.type)), and if fieldDef.nullable is true you mark the schema as nullable or allow null (e.g. set schema.nullable = true or use anyOf with {type:null}); keep the required push logic independent of fieldDef.array so non-optional array fields are included in required, and continue to respect fieldDef.default and fieldDef.optional when deciding required. Ensure these changes are applied where CreateInput/UpdateInput properties are produced and where typeToSchema is currently called.
64-99:⚠️ Potential issue | 🔴 CriticalUse operation-specific response schemas.
All CRUD builders still hardcode
${modelName}Response, whosedatafield is a single-model ref. That is wrong forfindMany,createManyAndReturn,updateManyAndReturn,count,exists,aggregate,groupBy, and the batch ops, so the emitted spec will generate incorrect client types. Plumb a response schema name per op and generate the matching response components instead of reusing the single-model wrapper everywhere.Also applies to: 172-178, 205-210, 237-242, 269-274, 672-678
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/api/rpc/openapi.ts` around lines 64 - 99, The code always uses a single `${modelName}Response` wrapper which yields incorrect types for multi-item, count, exists, aggregate, groupBy and batch-returning ops; compute an operation-specific response schema name (e.g., via a new helper like opToResponseSchema(op, modelName) or a map for ops -> schema names) and pass that responseSchemaName into the OpenAPI builders instead of relying on `${modelName}Response`. Concretely, update the loop callers (the GET loop and the writeOps loop) to compute responseSchemaName and call this.buildGetOperation(this, ...) / this.buildPostOperation / this.buildPatchOperation / this.buildDeleteOperation with the extra responseSchemaName argument, then modify those builder methods (buildGetOperation, buildPostOperation, buildPatchOperation, buildDeleteOperation) to accept and use the provided responseSchemaName to reference and emit correct response components for findMany/createManyAndReturn/updateManyAndReturn/count/exists/aggregate/groupBy and batch ops. Ensure the response component generator creates list/number/boolean/aggregate/grouped shapes as appropriate for each op.
381-386:⚠️ Potential issue | 🟠 MajorDon't require relation fields in the shared model schema.
select/includemake relation payloads conditional, but Line 385 still marks every non-optional relation as required. That makes ordinary RPC responses invalid unless the caller explicitly included the relation.Suggested fix
- if (!fieldDef.optional && !fieldDef.array) { + if (!fieldDef.optional && !fieldDef.relation) { required.push(fieldName); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/api/rpc/openapi.ts` around lines 381 - 386, The loop over modelDef.fields is currently marking every non-optional, non-array field as required; change the condition that pushes into required (in the block using fieldDef, modelDef.fields, fieldToSchema, and isFieldOmitted) to also skip relation fields so relation/select/include-driven payloads are not required; implement this by detecting relation descriptors on fieldDef (e.g., a relation flag/property such as fieldDef.relation, fieldDef.targetModel, fieldDef.isRelation, or other project-specific relation marker) and only push fieldName into required when the field is non-optional, non-array, and not a relation.
693-740:⚠️ Potential issue | 🟠 MajorFilter operators should mirror the field's value schema.
buildFieldFilterSchema()starts from baretypeToSchema(fieldDef.type), so equality/shorthand filters on list fields accept a single element and nullable fields cannot compare againstnull. Keep an element schema forhas/range-style operators, but use a wrapped value schema forequals/not/in/notInand the shorthandoneOf.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/src/api/rpc/openapi.ts` around lines 693 - 740, buildFieldFilterSchema currently uses baseSchema everywhere so equality/shorthand filters treat array fields as single elements and nullable fields cannot be compared to null; fix by deriving a valueSchema that represents the field's full value (if fieldDef.array then valueSchema = { type: 'array', items: baseSchema } else valueSchema = baseSchema) and if fieldDef.nullable wrap valueSchema to allow null (e.g., oneOf [valueSchema, { type: 'null' }]); then use valueSchema for equals and not, use { type: 'array', items: valueSchema } for in and notIn, keep element-based baseSchema for list operators (has/hasEvery/hasSome), and return the shorthand oneOf as { oneOf: [valueSchema, filterObject] } in buildFieldFilterSchema.
🧹 Nitpick comments (2)
packages/server/test/openapi/rest-openapi.test.ts (2)
78-79: Consider using a proper OpenAPI type forspec.Using
anyloses type safety and IDE support. If an OpenAPI type is available from a package likeopenapi-types, it would help catch assertion errors at compile time.+import type { OpenAPIV3_1 } from 'openapi-types'; + describe('REST OpenAPI spec generation', () => { let handler: RestApiHandler; - let spec: any; + let spec: OpenAPIV3_1.Document;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/test/openapi/rest-openapi.test.ts` around lines 78 - 79, The test declares spec as type any which loses type safety; change spec's type to a proper OpenAPI type (e.g., OpenAPIV3.Document from the openapi-types package) by importing the type (import type { OpenAPIV3 } from 'openapi-types') and updating the declaration (let spec: OpenAPIV3.Document) so assertions against spec in this file and usage with RestApiHandler are type-checked by TypeScript.
191-203: Consider extracting the parameter name extraction into a helper.The pattern
listOp.parameters.map((p: any) => ('name' in p ? p.name : p.$ref))is repeated 6 times throughout the file. Extracting it would improve readability and maintainability.♻️ Proposed helper function
Add near the top of the file:
const getParamNames = (params: any[]) => params.map((p: any) => ('name' in p ? p.name : p.$ref));Then replace usages:
-const paramNames = listOp.parameters.map((p: any) => ('name' in p ? p.name : p.$ref)); +const paramNames = getParamNames(listOp.parameters);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/server/test/openapi/rest-openapi.test.ts` around lines 191 - 203, The tests repeat the extraction logic listOp.parameters.map((p: any) => ('name' in p ? p.name : p.$ref)) so add a small helper (e.g., getParamNames) and replace direct uses with getParamNames(listOp.parameters) to improve readability; implement the helper near the top of the test file and update each occurrence (references: getParamNames, listOp.parameters) to call it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/server/src/api/rpc/openapi.ts`:
- Around line 406-410: The loops that build
CreateInput/UpdateInput/WhereUniqueInput/WhereInput currently only check
fieldDef.omit and should instead use the same visibility logic as
buildModelOutputSchema; replace checks like "if (fieldDef.omit) continue" with a
call to isFieldOmitted(modelDef, fieldName, queryOptions) (or the existing
isFieldOmitted signature used by buildModelOutputSchema) so that
queryOptions.omit/slicing are honored for request and filter schemas; update the
loops that iterate modelDef.fields (the blocks constructing CreateInput,
UpdateInput, WhereUniqueInput, WhereInput) to call isFieldOmitted before adding
a field.
---
Duplicate comments:
In `@packages/server/src/api/rpc/openapi.ts`:
- Around line 482-497: The recursive WhereInput refs are pointing to
`#/components/schemas/_`${modelName}WhereInput while generateSchemas() registers
${modelName}WhereInput, leaving dangling refs; update the schema registration in
generateSchemas() to also register the underscored name (register a schema under
_${modelName}WhereInput) or change the AND/OR/NOT refs in the block that sets
properties['AND'], properties['OR'], properties['NOT'] to point to
`#/components/schemas/`${modelName}WhereInput so the references match—locate
generateSchemas() and the properties['AND'|'OR'|'NOT'] lines to make the names
consistent.
- Around line 127-130: The responses objects currently use a $ref that points to
a schema (e.g. '400': { $ref: '#/components/schemas/_ErrorResponse' }) which is
invalid because response-level $ref must resolve to a Response Object; update
each offending entry (the '400' response refs and the other similar entries
around the other occurrences) to either reference a Response Object under
`#/components/responses` (e.g. '#/components/responses/_ErrorResponse') or replace
the $ref with an inline Response Object that includes description and content
referencing the schema (e.g. description plus content: application/json with
schema: { $ref: '#/components/schemas/_ErrorResponse' }); ensure you apply this
change for every responses block that currently points directly at the schema.
- Around line 414-416: CreateInput/UpdateInput builders currently call
typeToSchema(fieldDef.type) directly, which loses nullable and array shapes and
skips required array fields; update the property generation so that if
fieldDef.array is true you wrap the element schema into an array schema (type:
"array", items: typeToSchema(fieldDef.type)), and if fieldDef.nullable is true
you mark the schema as nullable or allow null (e.g. set schema.nullable = true
or use anyOf with {type:null}); keep the required push logic independent of
fieldDef.array so non-optional array fields are included in required, and
continue to respect fieldDef.default and fieldDef.optional when deciding
required. Ensure these changes are applied where CreateInput/UpdateInput
properties are produced and where typeToSchema is currently called.
- Around line 64-99: The code always uses a single `${modelName}Response`
wrapper which yields incorrect types for multi-item, count, exists, aggregate,
groupBy and batch-returning ops; compute an operation-specific response schema
name (e.g., via a new helper like opToResponseSchema(op, modelName) or a map for
ops -> schema names) and pass that responseSchemaName into the OpenAPI builders
instead of relying on `${modelName}Response`. Concretely, update the loop
callers (the GET loop and the writeOps loop) to compute responseSchemaName and
call this.buildGetOperation(this, ...) / this.buildPostOperation /
this.buildPatchOperation / this.buildDeleteOperation with the extra
responseSchemaName argument, then modify those builder methods
(buildGetOperation, buildPostOperation, buildPatchOperation,
buildDeleteOperation) to accept and use the provided responseSchemaName to
reference and emit correct response components for
findMany/createManyAndReturn/updateManyAndReturn/count/exists/aggregate/groupBy
and batch ops. Ensure the response component generator creates
list/number/boolean/aggregate/grouped shapes as appropriate for each op.
- Around line 381-386: The loop over modelDef.fields is currently marking every
non-optional, non-array field as required; change the condition that pushes into
required (in the block using fieldDef, modelDef.fields, fieldToSchema, and
isFieldOmitted) to also skip relation fields so relation/select/include-driven
payloads are not required; implement this by detecting relation descriptors on
fieldDef (e.g., a relation flag/property such as fieldDef.relation,
fieldDef.targetModel, fieldDef.isRelation, or other project-specific relation
marker) and only push fieldName into required when the field is non-optional,
non-array, and not a relation.
- Around line 693-740: buildFieldFilterSchema currently uses baseSchema
everywhere so equality/shorthand filters treat array fields as single elements
and nullable fields cannot be compared to null; fix by deriving a valueSchema
that represents the field's full value (if fieldDef.array then valueSchema = {
type: 'array', items: baseSchema } else valueSchema = baseSchema) and if
fieldDef.nullable wrap valueSchema to allow null (e.g., oneOf [valueSchema, {
type: 'null' }]); then use valueSchema for equals and not, use { type: 'array',
items: valueSchema } for in and notIn, keep element-based baseSchema for list
operators (has/hasEvery/hasSome), and return the shorthand oneOf as { oneOf:
[valueSchema, filterObject] } in buildFieldFilterSchema.
---
Nitpick comments:
In `@packages/server/test/openapi/rest-openapi.test.ts`:
- Around line 78-79: The test declares spec as type any which loses type safety;
change spec's type to a proper OpenAPI type (e.g., OpenAPIV3.Document from the
openapi-types package) by importing the type (import type { OpenAPIV3 } from
'openapi-types') and updating the declaration (let spec: OpenAPIV3.Document) so
assertions against spec in this file and usage with RestApiHandler are
type-checked by TypeScript.
- Around line 191-203: The tests repeat the extraction logic
listOp.parameters.map((p: any) => ('name' in p ? p.name : p.$ref)) so add a
small helper (e.g., getParamNames) and replace direct uses with
getParamNames(listOp.parameters) to improve readability; implement the helper
near the top of the test file and update each occurrence (references:
getParamNames, listOp.parameters) to call it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e241c9a7-4616-40e5-9a80-757113f92040
📒 Files selected for processing (5)
packages/server/src/api/common/spec-utils.tspackages/server/src/api/rest/openapi.tspackages/server/src/api/rpc/openapi.tspackages/server/test/openapi/rest-openapi.test.tspackages/server/test/openapi/rpc-openapi.test.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/server/test/openapi/rpc-openapi.test.ts
- packages/server/src/api/common/spec-utils.ts
- packages/server/src/api/rest/openapi.ts
Comment on lines
+406
to
+410
| for (const [fieldName, fieldDef] of Object.entries(modelDef.fields)) { | ||
| if (fieldDef.relation) continue; | ||
| if (fieldDef.foreignKeyFor) continue; | ||
| if (fieldDef.omit) continue; | ||
| if (fieldDef.updatedAt) continue; |
Contributor
There was a problem hiding this comment.
Apply isFieldOmitted to request and filter schemas too.
buildModelOutputSchema() honors query-time field slicing, but CreateInput, UpdateInput, WhereUniqueInput, and WhereInput only check fieldDef.omit. With queryOptions.omit/slicing, hidden fields still show up in request/filter components, so the RPC spec advertises inputs the handler is supposed to hide.
Also applies to: 430-434, 445-460, 473-476
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/server/src/api/rpc/openapi.ts` around lines 406 - 410, The loops
that build CreateInput/UpdateInput/WhereUniqueInput/WhereInput currently only
check fieldDef.omit and should instead use the same visibility logic as
buildModelOutputSchema; replace checks like "if (fieldDef.omit) continue" with a
call to isFieldOmitted(modelDef, fieldName, queryOptions) (or the existing
isFieldOmitted signature used by buildModelOutputSchema) so that
queryOptions.omit/slicing are honored for request and filter schemas; update the
loops that iterate modelDef.fields (the blocks constructing CreateInput,
UpdateInput, WhereUniqueInput, WhereInput) to call isFieldOmitted before adding
a field.
Contributor
|
Claude finished @ymc9's task —— View job Code Review for OpenAPI Spec Generation PRI've conducted a comprehensive review of this PR that adds OpenAPI spec generation with filter kind slicing support. Here's my detailed feedback: Review Tasks:
✅ Major StrengthsExcellent Architecture & Design
Robust Filter Kind Slicing Implementation
Strong Test Coverage
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
RestApiSpecGeneratorandRPCApiSpecGeneratorclassesspec-utils.tswith helpers for model/operation/field/filter-kind inclusion checksqueryOptions.slicing.fieldswithincludedFilterKinds/excludedFilterKindsto control which filter operators appear in generated OpenAPI specsfilter[field][op]query parameters per filter kind (Equality, Like, Range, List)equals,contains,lt, etc.) inWhereInputschemas, withoneOfshorthand when Equality is includedqueryOptionssupport to handler constructors (with Zod validation) foromitandslicingconfigurationTest plan
npx vitest run packages/server/test/openapi/)excludedFilterKinds/includedFilterKindsWhereInputschemas reflect per-operator filter kind control$allfield-level slicing applies to all fields with field-specific overrides🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Chores