Conversation
## Spec Change Impact
### Changes:
- **Operation Added**: `Search` now supports `best_match` relevance sorting as an option.
- **Modified Resource**: The `Search` resource has been updated to include the `best_match` sorting parameter.
### SDK Updates:
All SDKs require regeneration due to the spec change.
### Breaking Changes:
No breaking changes. This is a non-breaking addition of functionality.
### SDK Checklist:
- [ ] Go
- [ ] TypeScript
- [ ] Ruby
- [ ] Kotlin
- [ ] Swift |
Review carefully before merging. Consider a major version bump. |
There was a problem hiding this comment.
1 issue found across 16 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="ruby/scripts/generate-services.rb">
<violation number="1" location="ruby/scripts/generate-services.rb:573">
P2: The multi-line description fix is applied to query params but not to body params, which have the same vulnerability. If any body param description in the OpenAPI spec contains a newline, the generated Ruby file will have the same syntax-breaking bare newline in the YARD comment.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9869fc033b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Pull request overview
This PR aligns the SDKs’ search sort option with observed server behavior by adding relevance sorting (best_match), removing the non-functional updated_at mode, and hardening Ruby service code generation for multi-line parameter descriptions.
Changes:
- Update Search sort semantics: add
best_matchand removeupdated_atfrom the documented sort modes. - Add Go request-level tests to verify
sort=best_matchand omission ofsortwhen not provided. - Fix Ruby service generator to safely render multi-line query param descriptions without producing invalid Ruby.
Reviewed changes
Copilot reviewed 8 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| typescript/tests/services/search.test.ts | Updates TS test coverage to use best_match sort. |
| typescript/src/generated/services/search.ts | Updates generated TS Search options documentation/type for sort. |
| typescript/src/generated/schema.d.ts | Updates OpenAPI-derived TS declarations for the search sort param description. |
| typescript/src/generated/openapi-stripped.json | Updates generated OpenAPI (TS package) search sort parameter description. |
| typescript/src/generated/metadata.json | Regenerates TS metadata timestamp. |
| spec/basecamp.smithy | Updates Smithy docs for search sort field and adds multi-line documentation for sort. |
| ruby/test/basecamp/services/search_service_test.rb | Updates Ruby test to use best_match sort. |
| ruby/scripts/generate-services.rb | Escapes multi-line query param descriptions into valid YARD comments. |
| ruby/lib/basecamp/services/search_service.rb | Updates handwritten Ruby service docs for new sort modes. |
| ruby/lib/basecamp/generated/types.rb | Regenerates Ruby types timestamp header. |
| ruby/lib/basecamp/generated/services/search_service.rb | Regenerates Ruby service with multi-line sort param docs. |
| ruby/lib/basecamp/generated/metadata.json | Regenerates Ruby metadata timestamp. |
| openapi.json | Updates published OpenAPI search sort parameter description. |
| go/pkg/generated/client.gen.go | Updates generated Go client param docs for search sort. |
| go/pkg/basecamp/search_test.go | Adds Go httptest-based coverage for best_match and “no sort param” cases. |
| go/pkg/basecamp/search.go | Updates Go SDK SearchOptions.Sort doc to reflect best_match behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Replace "created_at|updated_at" with "best_match|created_at" in the Smithy SearchSortField enum. updated_at was never a distinct server-side sort mode (fell through to created_at desc). best_match triggers Elasticsearch dis_max + recency-boosted function_score. Regenerated all downstream artifacts.
The generator emitted raw newlines from OpenAPI descriptions into Ruby YARD @param tags, producing syntax errors. Continuation lines now get a "# " prefix so multi-line descriptions stay inside comments.
Update hand-written Go and Ruby service comments to document best_match and created_at as the valid sort values. Add Go httptest request-level tests verifying sort=best_match is sent and sort is absent when omitted. Update TS and Ruby tests from updated_at to best_match.
Summary
best_matchtoSearchSortFieldfor relevance-based search ordering (Elasticsearchdis_max+ recency-boostedfunction_score)updated_atfrom the sort enum — it was never a distinct server-side sort mode (fell through tocreated_at desc)Breaking change:
updated_atis removed from the TS union type for search sort. It never did anything distinct server-side, so this is a correction rather than a feature removal.API docs vs server behavior: The published API docs document no
sortparameter. The server (bc3/app/models/search.rb) acceptssort=best_matchfor relevance and defaults tocreated_at descotherwise. The SDK is aligning to observed server behavior, not published docs.Test plan
TestSearchService_Search_BestMatchSort,TestSearchService_Search_NoSort)updated_attobest_matchupdated_attobest_matchmake checkpasses (all five SDKs, lint, conformance)Summary by cubic
Adds
best_matchrelevance sorting to search and removesupdated_atfrom sort values. SDKs and schemas now match server behavior: usesort=best_matchfor relevance; otherwise results default tocreated_at(newest first). Added Go request-level tests and updated TS/Ruby tests and comments.Migration
sort: "updated_at"withsort: "best_match"or omit it to usecreated_atby default."updated_at"is no longer accepted in the sort type; update any calls and tests.Bug Fixes
Written for commit ad53f55. Summary will update on new commits.