Add RSwag coverage for chat API #210
Conversation
jjmata
added
the
codex
ChatGPT Codex Connector
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds RSpec and Rswag-based testing and OpenAPI generation: new RSpec configs and helpers, request specs for API v1 chats, a rake task for rswag, test gems in the Gemfile, a generated OpenAPI spec at Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant CLI as RSpec CLI
participant RSpec as RSpec (request specs)
participant Rails as Rails API
participant Rswag as Rswag/Swagger
participant OpenAPI as docs/api/openapi.yaml
Dev->>CLI: run targeted specs (rspec or rswag)
CLI->>RSpec: load spec_helper & rails_helper
RSpec->>Rails: perform HTTP requests (GET/POST/PATCH/DELETE)
Rails-->>RSpec: responses (status, body)
RSpec->>RSpec: validate responses against schemas
RSpec->>Rswag: emit request/response metadata
Rswag->>OpenAPI: generate/update `openapi.yaml`
OpenAPI-->>Dev: OpenAPI artifact written
RSpec-->>CLI: test results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (9)
Gemfile (1)
119-122: Group and pin testing/documentation gems for stability.
- Keep rswag-specs in :development,:test; serve rswag-api/ui in :development.
- Pin rspec-rails to the series compatible with Rails 7.2 (8.x) and rswag to stable 2.16 to avoid the 3.0.0.pre.
Based on learnings
Apply within this block:
-group :test do +group :test do gem "capybara" gem "selenium-webdriver" gem "mocha" gem "vcr" gem "webmock" gem "climate_control" gem "simplecov", require: false - gem "rspec-rails" - gem "rswag-api" - gem "rswag-specs" - gem "rswag-ui" + gem "rspec-rails", "~> 8.0" + gem "rswag-specs", "~> 2.16" endAnd add rswag-ui/api to the development group:
group :development do gem "hotwire-livereload" gem "letter_opener" gem "ruby-lsp-rails" gem "web-console" gem "faker" gem "benchmark-ips" gem "stackprof" gem "derailed_benchmarks" gem "foreman" + gem "rswag-ui", "~> 2.16" + gem "rswag-api", "~> 2.16" endIf you intentionally want to serve Swagger UI in production, we can revisit the grouping and mount path.
spec/requests/api/v1/chats_spec.rb (3)
125-134: Use OAS3request_bodyinstead ofparameter in: :body.For OpenAPI 3, prefer
request_bodywithcontentto avoid Swagger 2.0 remnants and improve schema fidelity.Example for “Create chat”:
- parameter name: :chat_params, in: :body, required: true, schema: { - type: :object, - properties: { - title: { type: :string, example: 'Monthly budget review' }, - message: { type: :string, description: 'Initial message in the chat' }, - model: { type: :string, description: 'Optional OpenAI model identifier' } - }, - required: %w[title message] - } + request_body do + required true + content 'application/json' do + schema type: :object, + properties: { + title: { type: :string, example: 'Monthly budget review' }, + message: { type: :string, description: 'Initial message in the chat' }, + model: { type: :string, description: 'Optional OpenAI model identifier' } + }, + required: %w[title message] + end + endRepeat similarly for the PATCH body and the message create body.
If your
spec/swagger_helper.rbis set to Swagger 2.0, keep current style; otherwise, this change aligns with OAS3. Please confirm youropenapiversion in swagger_helper.Also applies to: 201-207, 268-276
326-334: Avoidallow_any_instance_of; stub closer to the seam.
allow_any_instance_of(AssistantMessage)is broad and can hide regressions. Prefer stubbing the collaborator invoked by the retry action (e.g., service object) or create the minimal valid record.If there’s a specific validator or service, stub that class/method instead of the model instance. I can draft an example if you share the retry implementation.
84-101: Minor rswag polish.
security [ { bearerAuth: [] } ]is good; you can omit explicitAuthorizationheader parameter if you set a globalswagger_helpersecuritySchemesand use a request helper to inject the header. Optional only.produces/consumesare accepted; with OAS3contentthey become redundant. Keep or remove consistently.Also applies to: 118-134, 260-276, 316-325
spec/rails_helper.rb (2)
39-41: Fixture path config: verify key.Typical RSpec config uses
config.fixture_path = Rails.root.join('spec/fixtures'). Iffixture_pathsis intentional to support multiple dirs, keep; otherwise switch to singular.-config.fixture_paths = [ - Rails.root.join('spec/fixtures') -] +config.fixture_path = Rails.root.join('spec/fixtures')
66-67: Consider enabling type inference.
config.infer_spec_type_from_file_location!reduces the need fortype: :requestannotations. Optional.docs/api/openapi.yaml (3)
220-225: Remove redundant Authorization parameter.When using the
bearerAuthsecurity scheme (line 218), the Authorization header is automatically documented and should not be explicitly listed as a parameter. This redundancy appears in multiple endpoints.Apply this pattern to remove the redundant parameter:
security: - bearerAuth: [] - parameters: - - name: Authorization - in: header - required: true - schema: - type: string - description: Bearer token with read scope responses:This same issue occurs at:
- Lines 246-251 (POST /api/v1/chats)
- Lines 286-291 (/api/v1/chats/{id})
- Lines 366-371 (POST messages)
- Lines 419-424 (POST retry)
323-323: Remove empty parameters array.An empty
parameters: []array is redundant and can be omitted.security: - bearerAuth: [] - parameters: [] responses:Same issue at line 384.
51-55: Consider adding maxItems constraint to error details array.The static analysis tool suggests adding a maximum number of items to arrays. While not critical, adding
maxItemscan help document API behavior and prevent potential issues with excessively large error details arrays.If there's a practical limit on error details, consider:
oneOf: - type: array items: type: string + maxItems: 100 - type: object
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Gemfile.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
.rspec(1 hunks)Gemfile(1 hunks)docs/api/chats.md(1 hunks)docs/api/openapi.yaml(1 hunks)lib/tasks/rswag.rake(1 hunks)spec/rails_helper.rb(1 hunks)spec/requests/api/v1/chats_spec.rb(1 hunks)spec/spec_helper.rb(1 hunks)spec/swagger_helper.rb(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.rb
📄 CodeRabbit inference engine (AGENTS.md)
Ruby style: 2-space indentation; snake_case for methods/variables; CamelCase for classes/modules
Files:
spec/swagger_helper.rbspec/rails_helper.rbspec/spec_helper.rbspec/requests/api/v1/chats_spec.rb
**/*.{rb,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{rb,js,jsx,ts,tsx}: Make changes atomic, testable, and explain their impact briefly in code suggestions.
Respect existing tests and add tests when changing critical logic.
Files:
spec/swagger_helper.rbspec/rails_helper.rbspec/spec_helper.rbspec/requests/api/v1/chats_spec.rb
**/*.{rb,erb,haml,slim}
📄 CodeRabbit inference engine (.cursor/rules/general-rules.mdc)
**/*.{rb,erb,haml,slim}: UseCurrent.userfor the current user; do not usecurrent_user
UseCurrent.familyfor the current family; do not usecurrent_family
Ignore i18n methods; hardcode strings in English for now (do not useI18n.t,t, or similar)
Files:
spec/swagger_helper.rbspec/rails_helper.rbspec/spec_helper.rbspec/requests/api/v1/chats_spec.rb
🧠 Learnings (2)
📚 Learning: 2025-09-20T08:37:48.022Z
Learnt from: CR
PR: we-promise/sure#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-09-20T08:37:48.022Z
Learning: Applies to test/**/*_test.rb : Use VCR for external API tests.
Applied to files:
.rspec
📚 Learning: 2025-09-23T22:22:48.511Z
Learnt from: CR
PR: we-promise/sure#0
File: .cursor/rules/testing.mdc:0-0
Timestamp: 2025-09-23T22:22:48.511Z
Learning: Applies to test/**/*_test.rb : For tests needing many records, use Rails helpers to construct data and inline the creation in the test
Applied to files:
spec/rails_helper.rb
🪛 Checkov (3.2.334)
docs/api/openapi.yaml
[medium] 51-55: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
🪛 GitHub Check: ci / lint
lib/tasks/rswag.rake
[failure] 3-3:
Style/StringLiterals: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Cursor Bugbot
- GitHub Check: ci / test
🔇 Additional comments (5)
.rspec (1)
1-2: The review comment is incorrect; the .rspec file was not changed in this PR, and the pattern covers all existing tests.The git diff shows no changes to .rspec, and the pattern
spec/requests/api/v1/**/*_spec.rbwas already present in HEAD. The repository contains only one spec file (spec/requests/api/v1/chats_spec.rb), which resides in the path covered by the pattern. There are no non-API tests to skip. The review's assumption that "most tests" would be skipped does not match the actual codebase structure.Likely an incorrect or invalid review comment.
spec/requests/api/v1/chats_spec.rb (1)
184-191: The review comment is incorrect. Chat primary keys are UUID type, not integers.The schema confirms
create_table "chats", id: :uuid, and all test locations (184-191, 219-226, 293-300, 336-343) correctly useSecureRandom.uuidfor generating non-existent IDs. Using a valid UUID string with a UUID column will properly return 404 for missing records, not 500. The current implementation is already correct and requires no changes.Likely an incorrect or invalid review comment.
spec/swagger_helper.rb (1)
1-171: LGTM! Well-structured OpenAPI configuration.The swagger_helper configuration is comprehensive and follows rswag best practices. The schema definitions are thorough with proper types, constraints, and nullable fields. The security scheme is correctly configured for JWT bearer authentication.
spec/spec_helper.rb (1)
1-94: LGTM! Standard RSpec configuration.This is the standard RSpec configuration generated by
rspec:installwith sensible defaults. The active settings (include_chain_clauses, verify_partial_doubles, shared_context_metadata_behavior) are appropriate for modern RSpec usage.docs/api/chats.md (1)
1-59: LGTM! Clear and well-structured API documentation.The documentation effectively explains the OpenAPI-driven approach, provides clear instructions for regeneration, and properly references the generated specification. The reorganization around executable specs as the source of truth is a best practice for maintaining accurate API documentation.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Juan José Mata <[email protected]>
Signed-off-by: Juan José Mata <[email protected]>
This is the first task Codex planned here.
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68f61fd3a21c8332b4e087d220779cbf
Summary by CodeRabbit
Documentation
Tests
Chores