Generate complete reply coverage contracts - #28
Merged
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 23 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (108)
Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
8 issues found across 110 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="sig/x_twitter_scraper/models/trend_list_response.rbs">
<violation number="1" location="sig/x_twitter_scraper/models/trend_list_response.rbs:74">
P2: `Trend#to_hash[:url]` can be nil when `url` is omitted, but the new `trend` and `to_hash` records expose it as a guaranteed `String`, allowing statically accepted calls such as `.upcase` to fail at runtime. The hash declarations should model `url` as nullable/optional consistently with the initializer and reader.</violation>
</file>
<file name="lib/x_twitter_scraper/models/x/account_connection_attempt_retrieve_response.rb">
<violation number="1" location="lib/x_twitter_scraper/models/x/account_connection_attempt_retrieve_response.rb:27">
P2: SDK upgrades now raise `NameError` for callers that pattern-match or instantiate the previously published variant constants, including challenge enum constants, because they are renamed rather than aliased. Retaining deprecated aliases in Ruby/RBS/RBI, or shipping and documenting this as an explicit breaking release, would make the compatibility impact clear.</violation>
</file>
<file name="sig/x_twitter_scraper/models/x/tweet_author.rbs">
<violation number="1" location="sig/x_twitter_scraper/models/x/tweet_author.rbs:8">
P2: Typed consumers lose the inherited `UserProfile` contract because this alias and the matching `initialize`/`to_hash` signatures expose only `followers` and `verified`, even though runtime `TweetAuthor` instances retain all superclass fields. The generated contract should merge the inherited required and optional profile fields rather than replace them.</violation>
</file>
<file name="lib/x_twitter_scraper/models/x/tweet_get_replies_params.rb">
<violation number="1" location="lib/x_twitter_scraper/models/x/tweet_get_replies_params.rb:127">
P2: Complete-mode calls remain type-valid with `cursor` and filter parameters even though the new contract excludes those combinations. Since `dump_request` serializes every supplied field, consider separate parameter alternatives or validation that rejects incompatible keys before sending the request.</violation>
</file>
<file name="lib/x_twitter_scraper/models/paginated_tweets.rb">
<violation number="1" location="lib/x_twitter_scraper/models/paginated_tweets.rb:37">
P3: Pagination users lose the guidance needed to distinguish filtered/credit-limited pages and continue via `next_cursor`; the replacement “see PaginatedTweets” link does not preserve those details. Restoring the prior behavior notes alongside the new complete-mode fields would keep the public contract actionable.</violation>
</file>
<file name="sig/x_twitter_scraper/models/search_tweet.rbs">
<violation number="1" location="sig/x_twitter_scraper/models/search_tweet.rbs:292">
P2: Hash results are typed as containing every newly added optional field, so Steep can allow nil-unsafe access such as `tweet.to_hash[:article]` even when runtime omitted `article`. These fields should use optional record keys (`?article:`, `?id:`, etc.) in the added aliases and `to_hash` records.</violation>
</file>
<file name="lib/x_twitter_scraper/models/support/ticket_retrieve_response.rb">
<violation number="1" location="lib/x_twitter_scraper/models/support/ticket_retrieve_response.rb:55">
P3: Non-empty ticket responses remain untested because `MockPayload.example` always supplies an empty `messages` array. Consider adding a coercion test with one message to cover the new required fields and `Sender` enum.</violation>
</file>
<file name="sig/x_twitter_scraper/models/paginated_tweets.rbs">
<violation number="1" location="sig/x_twitter_scraper/models/paginated_tweets.rbs:12">
P2: Hashes from `PaginatedTweets.new(...).to_hash` may omit `diagnostic` and `nested_replies`, but both new record signatures claim the keys and non-nil values are always present. Mark these fields as optional (`?diagnostic:` / `?nested_replies:`) in both `paginated_tweets` and `to_hash` so RBS consumers do not rely on values absent at runtime.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
| ?rank: Integer | ||
| ?rank: Integer, | ||
| ?tweet_volume: Integer?, | ||
| ?url: String |
There was a problem hiding this comment.
P2: Trend#to_hash[:url] can be nil when url is omitted, but the new trend and to_hash records expose it as a guaranteed String, allowing statically accepted calls such as .upcase to fail at runtime. The hash declarations should model url as nullable/optional consistently with the initializer and reader.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At sig/x_twitter_scraper/models/trend_list_response.rbs, line 74:
<comment>`Trend#to_hash[:url]` can be nil when `url` is omitted, but the new `trend` and `to_hash` records expose it as a guaranteed `String`, allowing statically accepted calls such as `.upcase` to fail at runtime. The hash declarations should model `url` as nullable/optional consistently with the initializer and reader.</comment>
<file context>
@@ -48,18 +58,30 @@ module XTwitterScraper
- ?rank: Integer
+ ?rank: Integer,
+ ?tweet_volume: Integer?,
+ ?url: String
) -> void
</file context>
| # @return [Array<XTwitterScraper::Models::Support::TicketRetrieveResponse::Message::Attachment>, nil] | ||
| optional :attachments, | ||
| # @return [Array<XTwitterScraper::Models::Support::TicketRetrieveResponse::Message::Attachment>] | ||
| required :attachments, |
There was a problem hiding this comment.
P3: Non-empty ticket responses remain untested because MockPayload.example always supplies an empty messages array. Consider adding a coercion test with one message to cover the new required fields and Sender enum.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/x_twitter_scraper/models/support/ticket_retrieve_response.rb, line 55:
<comment>Non-empty ticket responses remain untested because `MockPayload.example` always supplies an empty `messages` array. Consider adding a coercion test with one message to cover the new required fields and `Sender` enum.</comment>
<file context>
@@ -11,70 +11,70 @@ module Support
- # @return [Array<XTwitterScraper::Models::Support::TicketRetrieveResponse::Message::Attachment>, nil]
- optional :attachments,
+ # @return [Array<XTwitterScraper::Models::Support::TicketRetrieveResponse::Message::Attachment>]
+ required :attachments,
-> { XTwitterScraper::Internal::Type::ArrayOf[XTwitterScraper::Models::Support::TicketRetrieveResponse::Message::Attachment] }
</file context>
Comment on lines
37
to
41
| # Some parameter documentations has been truncated, see | ||
| # {XTwitterScraper::Models::PaginatedTweets} for more details. | ||
| # | ||
| # An empty page can still have has_next_page true after filtering. | ||
| # |
There was a problem hiding this comment.
P3: Pagination users lose the guidance needed to distinguish filtered/credit-limited pages and continue via next_cursor; the replacement “see PaginatedTweets” link does not preserve those details. Restoring the prior behavior notes alongside the new complete-mode fields would keep the public contract actionable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/x_twitter_scraper/models/paginated_tweets.rb, line 37:
<comment>Pagination users lose the guidance needed to distinguish filtered/credit-limited pages and continue via `next_cursor`; the replacement “see PaginatedTweets” link does not preserve those details. Restoring the prior behavior notes alongside the new complete-mode fields would keep the public contract actionable.</comment>
<file context>
@@ -22,16 +22,286 @@ class PaginatedTweets < XTwitterScraper::Internal::Type::BaseModel
+ optional :nested_replies, -> { XTwitterScraper::Internal::Type::ArrayOf[XTwitterScraper::SearchTweet] }
+
+ # @!method initialize(has_next_page:, next_cursor:, tweets:, diagnostic: nil, nested_replies: nil)
+ # Some parameter documentations has been truncated, see
+ # {XTwitterScraper::Models::PaginatedTweets} for more details.
+ #
</file context>
Suggested change
| # Some parameter documentations has been truncated, see | |
| # {XTwitterScraper::Models::PaginatedTweets} for more details. | |
| # | |
| # An empty page can still have has_next_page true after filtering. | |
| # | |
| # Paginated tweet results. The item count can be lower than pageSize when the | |
| # source returns fewer tweets, filters remove tweets, or remaining credits cover | |
| # fewer results. Follow next_cursor while has_next_page is true. An empty page can | |
| # still have has_next_page true after filtering. Zero affordable results returns | |
| # 402 insufficient_credits. | |
| # |
furkanerday
requested changes
Jul 31, 2026
furkanerday
left a comment
furkanerday
left a comment
Collaborator
There was a problem hiding this comment.
Blocking findings remain, and 19 current review threads are unresolved. Representative contract defects:
-
Optional runtime fields are incorrectly declared required/non-nil in RBS records and to_hash contracts, including sig/x_twitter_scraper/models/trend_list_response.rbs:33-42 and 77-85, tweet_detail.rbs:8-48 and 252+, embedded_tweet.rbs:7-47, paginated_tweets.rbs:7+, tweet_media.rbs:7-29 and 137-158, and search_tweet.rbs:7-47 and 251+. This rejects valid runtime objects.
-
Hash-coercible nested runtime inputs are typed class-only at tweet_detail.rbs:69-89 and 220-243 and tweet_media.rbs:72-102 and 126-132. TweetAuthor also loses the inherited UserProfile contract at sig/x_twitter_scraper/models/x/tweet_author.rbs:8-21.
-
lib/x_twitter_scraper/models/x/account_connection_attempt_retrieve_response.rb:9+ renames public variant constants without compatibility aliases.
-
The documented 424 incomplete-replies path still has no typed IncompleteTweetReplies model/error-path test.
Resolve every actionable thread and these runtime/RBS compatibility gaps, then re-request review.
Regenerate every SDK from the exact production OpenAPI contract.
kriptoburak
force-pushed
the
codex/reply-extraction-contract-20260731
branch
from
July 31, 2026 14:15
e12e544 to
d03581f
Compare
Signed-off-by: kriptoburak <kriptoburak@users.noreply.github.com>
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
2 participants
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.
What changed
6613f3219.Why
X can rank, hide, or omit replies counted on a post. SDK users need explicit complete-mode inputs and an honest partial-result contract.
Validation
The generated output passed its complete language-specific test, lint, coverage, audit, licensing, and reproducibility gates before publication. Production REST, MCP, pagination, load, and Apify canaries are green. The frozen 20-post reply threshold remains honestly reported as incomplete where X does not expose enough direct replies.
Summary by cubic
Adds complete reply coverage with a new complete mode and a typed replies response that separates nested replies and includes diagnostics. Regenerates the SDK from the production OpenAPI and aligns rich read-model types across tweets, users, media, trends, support tickets, and account-connection flows.
New Features
X::TweetGetRepliesResponse: new return type fortweets#get_replies; addsdiagnosticandnested_replieson top ofPaginatedTweets.X::TweetGetRepliesParams: addsmode(:complete) andlimitfor complete-coverage fetching.EmbeddedTweet,SearchTweet, andX::TweetDetailadd shared fields (e.g.article,card,community_note,edit,note_tweet,place,possibly_sensitive,previous_counts,view_state).AccountCreateResponseis now a typed union; attempt retrieval is discriminated bystatus(:pending,:success,:failed,:requires_email_code); challenge/reauthstatusis:active.Status/Senderenums;TweetMediagains geometry/accessibility/crop metadata; trends addpromoted_content,tweet_volume,url;UserProfileadds affiliation/verification details;X::TweetAuthornow requiresfollowersandverified.Migration
tweets#get_repliesnow returnsX::TweetGetRepliesResponse. Usemodeandlimitfor complete mode; complete mode accepts onlylimit(drop cursor, pageSize/count, time ranges, and tweet filters). Readdiagnosticfor coverage;nested_repliesare separate from direct coverage.status; challenge/reauthstatusis:active.ContentDisclosure::AIGenerated#can_edit,UserProfile#can_dm, communityis_member/role.Written for commit 443ea3b. Summary will update on new commits.
Note
Add complete reply coverage contracts with structured response types and expanded tweet/user models
TweetGetRepliesResponsewith aDiagnosticsubmodel andTweetGetRepliesParamsgainslimitandmode(:complete) parameters;get_repliesnow deserializes into this typed response instead ofPaginatedTweets.TweetDetail,SearchTweet, andEmbeddedTweetwith shared optional fields:article,card,community_note,edit,note_tweet,place,possibly_sensitive,previous_counts, andview_state, each backed by new nested model classes.AccountCreateResponse's unknown converter with a typed union (SanitizedXAccount,XAccountConnectionAttemptPending,XAccountConnectionChallenge); renamesAccountConnectionAttemptRetrieveResponsevariants toPending,Success,Failed,RequiresEmailCodeand adds astatusdiscriminator.UserProfilewith 14 new fields (e.g.highlights_info,identity_verification,affiliates_highlighted_label) and removescan_dm,viewer_followed_by,viewer_following; expandsTweetMediawith geometry, accessibility, and crop metadata.TicketListResponse,TicketRetrieveResponse, andTicketUpdateResponsefields from optional to required and narrowsstatus/senderto typed enum modules.UserProfile,TweetAuthor, and the support ticket models now require previously optional fields at initialization; callers constructing these models directly will break.Changes since #28 opened
::Hash[Symbol, top]instead of specific structured types [443ea3b]ObjectandStatusmodules with enum constants to all variant classes inModels::X::AccountConnectionAttemptRetrieveResponse[443ea3b]Models::X::AccountConnectionAttemptRetrieveResponsemodule mapping to existing variant classes [443ea3b]Internal::Type::Unionmodule to includeRequiresEmailCodevariant in pattern-matching examples [443ea3b]Resources::X::Communitiesclass method [443ea3b]Macroscope summarized d03581f.