Conversation
|
The latest Buf updates on your PR. Results from workflow buf-pr / buf (pull_request).
|
Summary
Test & Lint SummaryCommands:
Results:
|
|
Requesting review. This PR adds the Chat service API schemas — both proto (gRPC) and OpenAPI (REST). CI is green (buf lint + OpenAPI validation passed). |
noa-lucent
left a comment
noa-lucent
left a comment
There was a problem hiding this comment.
Clean, well-structured addition. Proto and OpenAPI schemas are consistent with each other and with the existing repo conventions. All 5 RPCs and 5 REST endpoints from the issue spec are covered, cursor-based pagination is correctly applied, and identity is properly kept out of request bodies.
Two minor notes left inline — nothing blocking.
| fileIds: | ||
| type: array | ||
| items: { type: string, format: uuid } | ||
| description: File references (UUIDs). May be empty when body is non-empty. |
There was a problem hiding this comment.
[minor] This schema has no required list, so {} is a valid request body at the schema level. The "at least one of body or fileIds" rule is documented in the description but is invisible to schema validators and codegen.
Consider adding a comment or using an OpenAPI description on the schema itself to make it explicit this is enforced server-side. Alternatively, if there's a dominant use case (e.g., most messages have a body), you could require body and keep fileIds optional — though that changes the contract slightly.
Not a blocker since the constraint is well-documented in descriptions here and in the proto, but worth noting for consumers relying on schema validation.
| @@ -0,0 +1,51 @@ | |||
| get: | |||
| operationId: getChats | |||
There was a problem hiding this comment.
[nit] The chat path files include operationId (e.g., getChats, createChat, getChatMessages, etc.) while the existing llm/team/files specs do not use operationId at all. This is good practice for codegen and documentation, but creates a style inconsistency across the repo.
Not blocking — ideally the older specs would be backfilled with operationId too. Just flagging the drift.
4 participants
Summary
proto/agynio/api/chat/v1/chat.protoopenapi/chat/v1Testing
buf lintredocly bundle openapi/team/v1/openapi.yaml -o dist/team-v1.yamlredocly bundle openapi/files/v1/openapi.yaml -o dist/files-v1.yamlredocly bundle openapi/llm/v1/openapi.yaml -o dist/llm-v1.yamlredocly bundle openapi/chat/v1/openapi.yaml -o dist/chat-v1.yamlspectral lint dist/team-v1.yamlspectral lint dist/files-v1.yamlspectral lint dist/llm-v1.yamlspectral lint dist/chat-v1.yamlCloses #26