Conversation
Contributor
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
🦋 Changeset detectedLatest commit: 47dabea The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## chore/api-openapi-metadata #41645 +/- ##
==============================================================
+ Coverage 68.77% 68.80% +0.02%
==============================================================
Files 4156 4195 +39
Lines 159825 159714 -111
Branches 28070 28039 -31
==============================================================
- Hits 109927 109892 -35
+ Misses 44722 44641 -81
- Partials 5176 5181 +5
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
ggazzo
force-pushed
the
chore/api-openapi-import-prose
branch
14 times, most recently
from
August 3, 2026 20:25
7d592f1 to
58490bb
Compare
…s repo The summaries, descriptions, payload examples and tags of our endpoints have been maintained by hand in RocketChat/Rocket.Chat-Open-API, where nothing checks them against the code and they drift silently. Of the 588 operations described there, 561 match a route registered by this repository, so the prose can live next to the routes it documents and be reviewed with the code that changes them. This imports the prose of every typed route that has one - 359 operations: - summary and description, including the per endpoint changelog tables the definitions repository keeps; - the named request and response examples; - the tags, so the generated document groups endpoints the same way the published documentation does instead of the ad hoc tags we had. Types are deliberately NOT imported - the schemas here are the source of truth and are validated at runtime, while the hand-written ones are looser. What is left out is the routes still registered through the legacy `addRoute`, which have nowhere to carry documentation until they are typed. `scripts/import-openapi-prose.ts` performs the import, matching operations by method and path, so re-running it after the definitions repository changes is a single command. Examples land in a sibling `<file>.examples.ts` module to keep the route options readable.
…ects The examples import surfaced a pre-existing violation in businessHours.ts and customField.ts, where the sibling imports sat after a deeper parent one. It went unnoticed because the lint task was reading them from cache.
`dm.*` and `im.*` are the same endpoint under two names and declare their options in a shared object, which the import skipped: it only recognized options written inline at the call site. It now resolves the identifier to its declaration, and the definitions repository describes only one name of each pair, so the twin's prose is reused. Fourteen more operations documented, and the edits are applied from the last offset backwards - shared objects are declared before the call sites that use them, so they no longer come out in file order.
Importing `OpenAPIDocumentation` here put every example in the type graph of every endpoint that uses them. The modules only need to describe the shape of a payload example, so they declare it themselves and the cross package edge is gone.
Declaring the tags in the route options put them in the type every caller of the endpoint sees, and 368 of them collapsed it: `Type 'Endpoints' recursively references itself as a base type`, fifty times over, with two thousand errors cascading through the client. Reproducing it locally needed the build artifacts out of the program - their presence was hiding the failure that CI kept reporting. The grouping now lives in `operationTags`, a generated map from operation to tags that the document builder applies while assembling the paths. Same document, none of the type weight.
The wiring was lost resolving a rebase conflict, leaving the generated map with no consumer and the operations ungrouped.
ggazzo
force-pushed
the
chore/api-openapi-import-prose
branch
from
August 3, 2026 21:01
58490bb to
47dabea
Compare
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.
Stacked on #41638 — review that one first; this PR targets its branch, not
develop.Proposed changes (including videos or screenshots)
The summaries, descriptions and payload examples of our REST endpoints live in RocketChat/Rocket.Chat-Open-API, hand-maintained YAML that nothing checks against the code. I audited it against the document our own framework generates:
/api/docs/json?withUndocumented=true)So most of the prose can simply move next to the route it documents, where it is reviewed with the code that changes it.
This PR imports the 226 operations that are already typed (90% of the typed call sites in the repository):
summaryanddescription, including the per-endpoint changelog tables the definitions repository keeps (### Changelogappears in 66% of its descriptions);Types are deliberately not imported. The schemas in this repository are the source of truth and are validated at runtime; the hand-written ones are looser (
requiredappears 284 times across 10901 type nodes) and would be a regression.How it was done
scripts/import-openapi-prose.tsmatches operations by method and path (normalizing:idto{id}), injects the prose into the route options, and writes the bulky examples into a sibling<file>.examples.tsso the endpoint definitions stay readable:The script is idempotent (it skips call sites that already declare a
summary) and only keeps examples for status codes the route actually declares.Issue(s)
Steps to test or reproduce
curl -s localhost:3000/api/docs/json | jq '.paths["/api/v1/channels.history"].get | {summary, description, examples: (.responses."200".content."application/json".examples | keys)}'/api-docs, group Channels: every operation now has a summary, a description with its changelog, and selectable example scenarios in the response panel.bun scripts/import-openapi-prose.ts --spec ../Rocket.Chat-Open-API --dry-run apps/meteor/server/api/v1/channels.tsreportswould import 0.Further comments
Re-running the import after the definitions repository changes is a single command:
bun scripts/import-openapi-prose.ts --spec ../Rocket.Chat-Open-API apps/meteor/server/api/v1/*.tsIt is idempotent: call sites that already declare a
summaryare skipped, and only examples for status codes the route actually declares are kept.Two findings from the audit that are worth handling separately:
addRouteand have nowhere to put documentation until they are typed./api/v1/livechat/sms-incoming/{service}and friends); the definitions repository should drop them.