Bot API 10.3 - #341
Bot API 10.3#341atipugin wants to merge 11 commits into
Conversation
extract_required_value matched "must be <em>X</em>" anywhere in a field
description. For the 57 string discriminators ("Type of the media, must
be photo") that is correct, but Bot API 10.3's Boolean field
EphemeralMessageParameters#replace_callback_query_message reads "Pass
True if ... Must be False for callback queries from ephemeral messages",
where the phrase is conditional guidance, not a constraint.
That produced `Types::Bool.constrained(eql: False).default(False)`, which
both forbids the True the field exists to carry and emits `False` as a
bare Ruby constant (NameError on eager load).
Skip the pattern for booleans and route both branches through
cast_default_value.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Eg314icDVyoMunNjEYkVxC
Regenerated types from the Bot API docs (rake parse:* + rebuild:*). New types: EphemeralMessageParameters, DisabledButton, RichMessageButton, RichTextButton, CommunityChatJoined, MessageGenerationStopped, RichBlockButtons, RichBlockDocument, RichBlockExpandableBlockQuotation and the three matching InputRichBlock* variants. New fields: can_send_welcome_messages on ChatAdministratorRights and ChatMemberAdministrator, disabled on InlineKeyboardButton, force_reply on InlineKeyboardMarkup and ReplyKeyboardMarkup, is_compact on RichBlockTable/InputRichBlockTable, text/entities/is_private on UniqueGiftInfo, community_chat_joined on Message, stopped_message_generation on Update, InputMediaDocument in the InputRichMessageMedia union. data/methods.json is unchanged: 10.3 adds no new methods, and endpoints map method names to return types only, so its new parameters need no code change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Eg314icDVyoMunNjEYkVxC
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
CI is passing ✅ — is there something specific you'd like me to address? |
|
@copilot I want you to ensure changes fully cover everything added in Bot API 10.3 |
Verified the changes fully cover Bot API 10.3: New types (12): New fields on existing types: Union updates:
|
It is referenced nowhere in the codebase (no .rb, .rake, .md or .yml hit), and it makes the bundle unresolvable on modern Rubies: every version depends on commonmarker ~> 0.17, which has no build for Ruby 4, so `bundle exec` fails outright before any task runs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sta5WKS94APw4xcGB61Ur2
parse_return_statement matched "Returns <em>Int</em>", but the documentation for getChatMemberCount reads "Returns <em>Integer</em> on success". The paragraph contains no <a>, so it fell through all ten return-type patterns and MethodsParser dropped it silently via `result[name] = type if type`. The method was therefore absent from data/methods.json and from ENDPOINTS. Api#method_missing is gated on ENDPOINTS.key?, so api.get_chat_member_count raised NoMethodError. It was the only documented method missing from the generated list. Widen the pattern to Int(?:eger)? and regenerate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sta5WKS94APw4xcGB61Ur2
The API page is a flat sibling stream -- 601 h4, 676 p, 544 table, nothing
nested -- and both parsers re-derived "the run of nodes after this heading"
by hand: first_description_paragraph, find_next_significant_sibling,
find_attribute_table and MethodsParser#extract_return_type were four
variations on the same walk. Field access was positional (cells[0..2] behind
a `length >= 3` guard), and prose was read by regexing serialized HTML, with
`description` and `description_html` threaded through every extractor.
Introduce four small collaborators:
Docs::Source fetch once, cache under tmp/ (was one HTTP GET per parser)
Docs::Page slice the stream before every heading -> Docs::Section
Docs::Table address cells by column name; raise on an unexpected shape
Docs::Text text plus Docs::Marks, an offset index of inline markup
Text is the load-bearing piece. Markup cannot be dropped -- "Returns
<em>True</em>" and "Returns <a>Message</a>" are otherwise identical -- and
neither can position, since paragraphs carry earlier prose links. Indexing
the offsets keeps one representation of the text and lets a query anchor to
a phrase: `emphasised_after('must be')` matches only an <em> immediately
following the phrase.
That subsumes 617eab6. Its `unless type == 'boolean'` guard existed because
the old pattern was case-insensitive and position-free, so a Boolean field's
"Must be <em>False</em>" of conditional prose read as a constraint. All 57
real discriminators are lowercase and mid-sentence; the one false positive is
sentence-initial. Adjacency and case now separate them, verified against
every row of all 365 field tables, so the type carve-out is gone.
MethodsParser's ten ordered inner_html regexes become a rule list, and a miss
is now recorded and warned about instead of silently dropped -- the failure
mode that hid getChatMemberCount. rake parse:methods aborts on any unmatched
method.
data/types.json and data/methods.json regenerate byte-identically.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Sta5WKS94APw4xcGB61Ur2
rakelib/parsers/**/* was excluded from Metrics/MethodLength, AbcSize,
CyclomaticComplexity, PerceivedComplexity and ClassLength, so nothing pushed
back as the parsers grew. With the work split across Docs:: and one class per
concern, all five carve-outs can go.
RuboCop::RakeTask also only ever linted {lib,spec}, so `rake rubocop` never
saw rakelib/ while CI's bare `bundle exec rubocop` did. Add it, along with
.rake files, so local and CI agree.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Sta5WKS94APw4xcGB61Ur2
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sta5WKS94APw4xcGB61Ur2
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Restructure the specs added with the Docs:: layer to follow
betterspecs.org:
* describe methods as '#method' / '.method', not by topic. The topical
groups ('union types', 'discriminators', 'row access', 'key order')
become contexts under the method they exercise.
* every context reads 'when', 'with' or 'without'.
* one expectation per example. Seven examples were carrying
:aggregate_failures to satisfy RSpec/MultipleExpectations; they are
split, and the tag is gone from the suite.
* conditions move from the example description into the context, so no
example description exceeds 40 characters.
* subject plus let, in place of local variables built inside the example.
Data now varies by overriding a let in the context, which is what
collapsed the AttributeParser cases into a single table-building let.
* one-line expectations use is_expected.
Also drops a stubbed subject in favour of capturing $stderr, since the
warning belongs to the object under test.
87 rakelib examples, 500 in the suite, rubocop clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Sta5WKS94APw4xcGB61Ur2
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
One `expect` per example was already the case after c786531, but five examples still made two assertions inside a single multi-key include: the array type, both discriminators, and two of the three size ranges. Split each into two examples. This also settles an inconsistency in the file -- 'with a zero-based character range' already used two `it` blocks while the range contexts on either side of it did not. Whole-object `eq` assertions are deliberately left alone. They are a single claim ("the result is exactly this"), and splitting them into `include`s would be weaker: neither half would catch an unexpected extra key, which is the property those examples exist for. 92 rakelib examples, 505 in the suite, rubocop clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sta5WKS94APw4xcGB61Ur2
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
@copilot want your strong opinion on parsers rewrite |
Strong opinion: endorse the architecture, flag two rough spots. What's right and should stay: The
Two things I'd change: 1. The The current fix works because genuine discriminators happen to be lowercase mid-sentence ("must be photo"), while the one known false positive starts a sentence ("Must be False"). The code comment explains this, but it's an implicit stylistic contract with Telegram's doc writers. def constant(type)
return nil unless type == 'string' # discriminators are string-only
…
end…is one line, documents the actual constraint, and is immune to Telegram capitalising a sentence differently one day. 2. constrain = '.constrained(minmax)'
constrain = properties[:min_size] ? constrain.gsub('min', "min_size: #{properties[:min_size]}, ") : ''
constrain = constrain.gsub('max', "max_size: #{properties[:max_size]}") if properties[:max_size]The placeholder-substitution pattern makes you reconstruct the intent. Two straightforward appends to an array and a Everything else — the layering, the naming, the |
Two changes, from the two points raised on #341. 1. Guard discriminators by type. AttributeParser#constant relied solely on `must be` being lowercase and mid-sentence to tell a real discriminator from conditional prose. That holds for all 57 in the docs, but it is an implicit contract with Telegram's doc writers. required_value becomes .constrained(eql: 'photo'), which is only coherent for a string; all 176 fields carrying one are strings. Assert that directly, keeping the adjacency check as the second guard. Output is unchanged. 2. Fix four wrong size constraints. TypeBuilder#apply_min_max! was flagged as hard to read. It is, and the placeholder substitution was hiding real bugs: constrain = '.constrained(minmax)' constrain = properties[:min_size] ? constrain.gsub('min', ...) : '' constrain = constrain.gsub('max', ...) if properties[:max_size] With min_size absent the string is reset to '', so max-only limits were silently dropped -- 24 fields' worth. Rewritten as a predicate table and a join. Emitting those 24 as-is would have been wrong too. 22 read "0-1024 characters after entities parsing": the limit applies to the text once markup is parsed, a length the client cannot compute, since "<b>x</b>" is 8 raw characters and 1 parsed. The parser now skips such limits rather than rejecting captions Telegram accepts. Two of them did carry a min_size and so were enforced -- InputChecklist#title and InputChecklistTask#text -- and rejected valid input. That leaves Poll#explanation and Game#text, plain character limits that should always have been enforced and now are. Separately, three Integer fields were given min_size/max_size. dry-types measures `size`, and Integer#size is the byte width -- 8 for every value -- so SuggestedPostPrice#amount accepted 1 and 999_999_999 alike despite a documented 5..100000 range. Numeric ranges now use their own min/max keys in data/types.json and become gteq/lteq. The builder also emits underscored numerals, so generated files no longer need a manual rubocop pass to satisfy Style/NumericLiterals. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sta5WKS94APw4xcGB61Ur2
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
|
@copilot please ensure your comments are addressed |
Both points are addressed in
|
Dropping "after entities parsing" limits outright went too far: the docs state both bounds for InputChecklist#title and InputChecklistTask#text, and data/types.json stopped recording either. The two bounds are not symmetric. Markup only ever adds characters, so raw >= parsed. A parsed length at or above the minimum therefore implies a raw length at or above it, and the minimum is a valid necessary condition on the raw string -- it can only reject input Telegram would reject too. The maximum does not carry over: 102 raw characters can be 95 parsed. So min_size is kept and emitted as before, while the maximum is recorded as max_size_parsed, naming the quantity it actually measures. The builder maps constraint keys through a table, so an unrecognised key is simply not emitted; no builder change was needed. data/types.json once again records every size limit the documentation states, for all 24 affected fields. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sta5WKS94APw4xcGB61Ur2
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
vsevolod
left a comment
vsevolod
left a comment
There was a problem hiding this comment.
Summary
This is a solid Bot API 10.3 update plus a parser rewrite that earns its extra types. Against the live docs the generated surface is complete (400 types, 185 methods, including getChatMemberCount), the 10.3 additions land correctly, and the EphemeralMessageParameters#replace_callback_query_message boolean trap is actually fixed: the field is plain Types::Bool with no eql: constraint. The Docs:: split, unmatched-return abort, gteq/lteq for integer ranges, and max_size_parsed handling are real correctness improvements, and the boolean-trap plus parser-unit specs are the right tests. The remaining gaps are a missed integer N-M range form on the new optional map fields, and comments that retell design history instead of stating a local invariant.
Issue counts by severity
- bugs: 0
- suggestions: 2
- nits: 0
| %w[min_size max_size] | ||
| end | ||
|
|
||
| def size_range |
There was a problem hiding this comment.
[suggestion] size_range only recognizes N-M characters and must be between N and M. Bot API 10.3 documents integer bounds on the now-optional InputRichBlockMap fields as Map zoom level; 0-24 and Map width; 0-10000 / Map height; 0-10000 (no “characters”, no “must be between”). Those rows therefore get no min/max in data/types.json, and the generated type is unconstrained attribute? :zoom, Types::Integer (same for width/height). That is incomplete relative to the numeric-range work in this PR: SuggestedPostPrice#amount and live_period now correctly emit gteq/lteq, but the new 10.3 map ranges do not. Valid values still parse; out-of-range values the API would reject are accepted by the gem.
Suggestion: For integer fields, also parse a bare (\d+)-(\d+) bound (e.g. after ; or at end of the description) into min/max, and cover 0-24 / 0-10000 in attribute_parser_spec. Keep the “characters” pattern for strings so 0-4096 characters does not get classified as a numeric range.
| module Docs | ||
| # Raised when the documentation no longer has the shape the parsers expect. | ||
| # Structural surprises are loud on purpose: a silent miss is how | ||
| # getChatMemberCount went missing from the generated endpoints. |
There was a problem hiding this comment.
[suggestion] Several new comments narrate how the old parser failed (getChatMemberCount going missing, “either guard has failed before”, “all 57 real discriminators”) rather than stating the invariant the next lines enforce. The same getChatMemberCount story is also in rakelib/parsers/methods_parser.rb:32-33 and spec/lib/telegram/bot/api_spec.rb:81-83. That is architecture history, not a local WHY, and it will rot the first time the surrounding code changes.
Suggestion: Keep a one-line invariant (“raise on unexpected docs shape”, “record unmatched methods so they cannot be dropped silently”) and drop the census/incident recap. The specs already document the regressions.
Adds support for Bot API 10.3 (released 2026-08-24). Types regenerated with
rake parse:*+rake rebuild:*.New types
EphemeralMessageParameters,DisabledButton,RichMessageButton,RichTextButton,CommunityChatJoined,MessageGenerationStopped,RichBlockButtons,RichBlockDocument,RichBlockExpandableBlockQuotation, plus the matchingInputRichBlockButtons,InputRichBlockDocumentandInputRichBlockExpandableBlockQuotation.New fields
can_send_welcome_messagesonChatAdministratorRightsandChatMemberAdministratordisabledonInlineKeyboardButtonforce_replyonInlineKeyboardMarkupandReplyKeyboardMarkupis_compactonRichBlockTable/InputRichBlockTabletext,entities,is_privateonUniqueGiftInfocommunity_chat_joinedonMessagestopped_message_generationonUpdateInputMediaDocumentadded to theInputRichMessageMediaunionParser fix
extract_required_valuematchedmust be <em>X</em>anywhere in a description. That is right for the 57 string discriminators ("Type of the media, must be photo"), but 10.3's BooleanEphemeralMessageParameters#replace_callback_query_messagereads "Pass True if … Must be False for callback queries from ephemeral messages" — conditional guidance, not a constraint.It generated
Types::Bool.constrained(eql: False).default(False), which forbids theTruethe field exists to carry and emitsFalseas a bare Ruby constant (NameErroron eager load). The pattern is now skipped for booleans, and both branches route throughcast_default_value.Notes
data/methods.jsonis unchanged — 10.3 adds no new methods, and endpoints map method names to return types only, so the new parameters (ephemeral_message_parameters,can_stop/keep_on_stop, etc.) need no code change.rebuild:typesoverwrites (CallbackQuery#to_s,Message#to_s,ChosenInlineResult#to_s,InlineQuery#to_s, the twoto_compact_hashoverrides,Update#current_message) were restored and verified.Verification
rubocop(1.66.1, matching the Gemfile pin): 422 files, no offensesrspec: 422 examples, 0 failures, 1 pending (pre-existing)replace_callback_query_messagewith bothtrueandfalse🤖 Generated with Claude Code
https://claude.ai/code/session_01Eg314icDVyoMunNjEYkVxC