Generate tree-structured raw MTProto documentation with examples#115
Generate tree-structured raw MTProto documentation with examples#115
Conversation
This change introduces a new documentation generator for raw MTProto functions and types. It adds: - A visual, recursive parameter tree showing nesting depth. - TL schema signatures for every function and type. - Fully inlined, working asynchronous code examples using `app.invoke`. - Clickable type references in the parameter tree using Sphinx `:obj:` directives. The `compiler/docs/compiler.py` has been updated to integrate these new sections into the generated RST files.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Reviewer's GuideImplements a TL-based documentation generator for raw MTProto types and functions, wiring it into the docs compiler so each generated page includes a TL schema signature, a recursive parameter tree, and realistic code examples based on parsed TL combinators. Sequence diagram for docs compiler using TLDocGenerator in generate_rawsequenceDiagram
participant Start as start
participant TLGen as TLDocGenerator
participant Compiler as compiler.generate_raw
participant Build as build_inner
participant Template as Sphinx_template
Start->>TLGen: TLDocGenerator(tl_paths)
activate TLGen
TLGen-->>Start: instance
deactivate TLGen
Start->>Compiler: generate_raw(TYPES_PATH, TYPES_BASE, tl_gen)
activate Compiler
loop walk raw entities
Compiler->>Build: build(path, level)
activate Build
Build->>TLGen: get_tl_signature(full_name)
TLGen-->>Build: tl_signature
Build->>TLGen: generate_tree(full_name)
TLGen-->>Build: parameter_tree
Build->>TLGen: generate_example(full_name, minimal)
TLGen-->>Build: example_code
Build->>Template: render page with tl_signature, parameter_tree, example_code
Template-->>Build: rst_page
Build-->>Compiler: write file
deactivate Build
end
deactivate Compiler
Start->>Compiler: generate_raw(FUNCTIONS_PATH, FUNCTIONS_BASE, tl_gen)
Start->>Compiler: generate_raw(BASE_PATH, BASE_BASE, tl_gen)
Class diagram for TLDocGenerator and related TL documentation typesclassDiagram
class Argument {
+str name
+str type
}
class Combinator {
+str section
+str qualname
+str id
+List~Argument~ args
+str qualtype
+str full_line
}
class TLDocGenerator {
-Dict~str, Combinator~ name_to_comb
-Dict~str, List~str~~ base_to_constructors
-List~Combinator~ combinators
+TLDocGenerator(tl_files)
+get_tl_signature(name) str
+get_full_class_path(name) str
+get_type_link(t) str
+generate_tree(name) str
+generate_example(name, minimal) str
-_parse_tl(tl_files) List~Combinator~
-_generate_tree_recursive(name, depth, prefix, visited, is_vector_item, is_base) List~str~
-get_default_value(arg_name, arg_type, depth, visited, minimal) str
}
class CORE_TYPES
Argument <|-- Combinator
Combinator "*" --> "*" Argument : uses
TLDocGenerator "1" --> "*" Combinator : parses and indexes
TLDocGenerator "1" --> "*" Argument : reads argument metadata
TLDocGenerator ..> CORE_TYPES : references
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the raw MTProto API documentation by introducing a new generation system. The primary goal is to make the documentation more comprehensive and user-friendly by providing structured parameter trees, direct TL schema signatures, and executable code examples. This change improves the clarity and utility of the API documentation for developers. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
generate_example, the call toget_default_valuepassesminimalas the fourth positional argument, which is actuallyvisited; this meansminimalis never honored andvisitedis misused—switch this to a keyword argument (minimal=minimal) to fix the behavior. - The
visitedparameter inget_default_valueis never updated with the current type, so it does not actually prevent recursive cycles; either incorporate the currentarg_typeintovisitedor drop the parameter to avoid confusion. - Several recursion/depth thresholds and pruning rules (e.g., depth > 6, specific arg-name whitelists, special base-type limits) are hard-coded throughout
TLDocGenerator; consider extracting these into named constants or configuration structures to make the behavior easier to tune and reason about.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `generate_example`, the call to `get_default_value` passes `minimal` as the fourth positional argument, which is actually `visited`; this means `minimal` is never honored and `visited` is misused—switch this to a keyword argument (`minimal=minimal`) to fix the behavior.
- The `visited` parameter in `get_default_value` is never updated with the current type, so it does not actually prevent recursive cycles; either incorporate the current `arg_type` into `visited` or drop the parameter to avoid confusion.
- Several recursion/depth thresholds and pruning rules (e.g., depth > 6, specific arg-name whitelists, special base-type limits) are hard-coded throughout `TLDocGenerator`; consider extracting these into named constants or configuration structures to make the behavior easier to tune and reason about.
## Individual Comments
### Comment 1
<location path="compiler/docs/tl_generator.py" line_range="168-177" />
<code_context>
+ val = self.get_default_value(arg_name, inner, depth + 1, visited.copy(), minimal=minimal)
+ indent = " " * (depth + 1)
+ item_indent = " " * (depth + 2)
+ if inner not in CORE_TYPES:
+ return f"[\n{item_indent}{val.replace('\n', '\n' + ' ')}\n{indent}]"
+ return f"[{val}]"
+
</code_context>
<issue_to_address>
**issue (bug_risk):** The f-string constructing the Vector default value is invalid due to a backslash in the expression.
In `get_default_value`, this return statement:
```python
return f"[\n{item_indent}{val.replace('\n', '\n' + ' ')}\n{indent}]"
```
will raise a `SyntaxError`, because f-string expressions cannot contain backslashes inside the string literal argument to `replace`. Compute the replaced value first, then use it in the f-string, e.g.:
```python
indented_val = val.replace("\n", "\n" + " ")
return f"[\n{item_indent}{indented_val}\n{indent}]"
```
</issue_to_address>
### Comment 2
<location path="compiler/docs/tl_generator.py" line_range="155" />
<code_context>
+ if name in ["InputPeerSelf", "InputPeerEmpty", "InputPeerChat", "InputUserSelf", "InputUserEmpty"] and depth > 2:
+ return []
+
+ filtered_args = [a for a in c.args if not (a.name.startswith("flags") and a.type == "#")]
+ for i, arg in enumerate(filtered_args):
+ is_last = (i == len(filtered_args) - 1)
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting shared argument filtering, prefix construction, and vector default-value logic into small helper methods to reduce duplication and clarify control flow without changing behaviour.
You can cut a noticeable amount of complexity and duplication with a few small extractions, without changing behavior.
### 1. Extract common argument filtering
The `flags`-filtering logic is duplicated in `_generate_tree_recursive` and `generate_example`. Centralising it reduces risk if the rule changes:
```python
def _filtered_args(self, c: Combinator) -> List[Argument]:
return [a for a in c.args if not (a.name.startswith("flags") and a.type == "#")]
```
Use it in both places:
```python
# _generate_tree_recursive
- filtered_args = [a for a in c.args if not (a.name.startswith("flags") and a.type == "#")]
+ filtered_args = self._filtered_args(c)
```
```python
# generate_example
- filtered_args = [a for a in c.args if not (a.name.startswith("flags") and a.type == "#")]
+ filtered_args = self._filtered_args(c)
```
### 2. Extract prefix / indentation logic for tree rendering
The `" "` vs `"│ "` logic is repeated and slightly obscures the high‑level flow. A tiny helper makes this clearer and less error‑prone:
```python
def _child_prefix(self, prefix: str, is_last: bool) -> str:
return prefix + (" " if is_last else "│ ")
```
Then update the recursive calls and lines:
```python
# in _generate_tree_recursive
- res.append(f"| {prefix}{'└── ' if is_last else '├── '}{self.get_type_link(constr)}")
+ branch = "└── " if is_last else "├── "
+ res.append(f"| {prefix}{branch}{self.get_type_link(constr)}")
- res.extend(self._generate_tree_recursive(constr, depth + 1, prefix + (" " if is_last else "│ "), new_visited, is_base=False))
+ res.extend(self._generate_tree_recursive(constr, depth + 1, self._child_prefix(prefix, is_last), new_visited, is_base=False))
# same pattern for other recursive calls:
- prefix + (" " if is_last else "│ ")
+ self._child_prefix(prefix, is_last)
```
This keeps the behaviour identical but makes the tree logic easier to scan.
### 3. Isolate vector default-value handling
`get_default_value` is doing several jobs; a minimal step is to pull vector-specific logic into a helper, shrinking the main function and making the recursion policy clearer:
```python
def _default_for_vector(self, arg_name: str, arg_type: str, depth: int, visited, minimal: bool) -> str:
inner = arg_type[7:-1]
val = self.get_default_value(arg_name, inner, depth + 1, visited.copy(), minimal=minimal)
indent = " " * (depth + 1)
item_indent = " " * (depth + 2)
if inner not in CORE_TYPES:
return f"[\n{item_indent}{val.replace('\n', '\n' + ' ')}\n{indent}]"
return f"[{val}]"
```
Then in `get_default_value`:
```python
- if arg_type.startswith("Vector<"):
- inner = arg_type[7:-1]
- val = self.get_default_value(arg_name, inner, depth + 1, visited.copy(), minimal=minimal)
- indent = " " * (depth + 1)
- item_indent = " " * (depth + 2)
- if inner not in CORE_TYPES:
- return f"[\n{item_indent}{val.replace('\n', '\n' + ' ')}\n{indent}]"
- return f"[{val}]"
+ if arg_type.startswith("Vector<"):
+ return self._default_for_vector(arg_name, arg_type, depth, visited, minimal)
```
This is a small change, but it makes `get_default_value`’s main flow less tangled and opens the door to further splitting (e.g. `_default_for_scalar`, `_default_for_constructor`) later without altering behaviour now.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| if inner not in CORE_TYPES: | ||
| res.extend(self._generate_tree_recursive(inner, depth + 1, prefix + (" " if is_last else "│ "), new_visited, is_vector_item=True)) | ||
| elif arg_type not in CORE_TYPES: | ||
| # If the type is a base type, we want to show its constructors | ||
| is_arg_base = arg_type in self.base_to_constructors | ||
| res.extend(self._generate_tree_recursive(arg_type, depth + 1, prefix + (" " if is_last else "│ "), new_visited, is_base=is_arg_base)) | ||
|
|
||
| return res | ||
|
|
||
| def get_default_value(self, arg_name, arg_type, depth=0, visited=None, minimal=False): |
There was a problem hiding this comment.
issue (bug_risk): The f-string constructing the Vector default value is invalid due to a backslash in the expression.
In get_default_value, this return statement:
return f"[\n{item_indent}{val.replace('\n', '\n' + ' ')}\n{indent}]"will raise a SyntaxError, because f-string expressions cannot contain backslashes inside the string literal argument to replace. Compute the replaced value first, then use it in the f-string, e.g.:
indented_val = val.replace("\n", "\n" + " ")
return f"[\n{item_indent}{indented_val}\n{indent}]"| if name in ["InputPeerSelf", "InputPeerEmpty", "InputPeerChat", "InputUserSelf", "InputUserEmpty"] and depth > 2: | ||
| return [] | ||
|
|
||
| filtered_args = [a for a in c.args if not (a.name.startswith("flags") and a.type == "#")] |
There was a problem hiding this comment.
issue (complexity): Consider extracting shared argument filtering, prefix construction, and vector default-value logic into small helper methods to reduce duplication and clarify control flow without changing behaviour.
You can cut a noticeable amount of complexity and duplication with a few small extractions, without changing behavior.
1. Extract common argument filtering
The flags-filtering logic is duplicated in _generate_tree_recursive and generate_example. Centralising it reduces risk if the rule changes:
def _filtered_args(self, c: Combinator) -> List[Argument]:
return [a for a in c.args if not (a.name.startswith("flags") and a.type == "#")]Use it in both places:
# _generate_tree_recursive
- filtered_args = [a for a in c.args if not (a.name.startswith("flags") and a.type == "#")]
+ filtered_args = self._filtered_args(c)# generate_example
- filtered_args = [a for a in c.args if not (a.name.startswith("flags") and a.type == "#")]
+ filtered_args = self._filtered_args(c)2. Extract prefix / indentation logic for tree rendering
The " " vs "│ " logic is repeated and slightly obscures the high‑level flow. A tiny helper makes this clearer and less error‑prone:
def _child_prefix(self, prefix: str, is_last: bool) -> str:
return prefix + (" " if is_last else "│ ")Then update the recursive calls and lines:
# in _generate_tree_recursive
- res.append(f"| {prefix}{'└── ' if is_last else '├── '}{self.get_type_link(constr)}")
+ branch = "└── " if is_last else "├── "
+ res.append(f"| {prefix}{branch}{self.get_type_link(constr)}")
- res.extend(self._generate_tree_recursive(constr, depth + 1, prefix + (" " if is_last else "│ "), new_visited, is_base=False))
+ res.extend(self._generate_tree_recursive(constr, depth + 1, self._child_prefix(prefix, is_last), new_visited, is_base=False))
# same pattern for other recursive calls:
- prefix + (" " if is_last else "│ ")
+ self._child_prefix(prefix, is_last)This keeps the behaviour identical but makes the tree logic easier to scan.
3. Isolate vector default-value handling
get_default_value is doing several jobs; a minimal step is to pull vector-specific logic into a helper, shrinking the main function and making the recursion policy clearer:
def _default_for_vector(self, arg_name: str, arg_type: str, depth: int, visited, minimal: bool) -> str:
inner = arg_type[7:-1]
val = self.get_default_value(arg_name, inner, depth + 1, visited.copy(), minimal=minimal)
indent = " " * (depth + 1)
item_indent = " " * (depth + 2)
if inner not in CORE_TYPES:
return f"[\n{item_indent}{val.replace('\n', '\n' + ' ')}\n{indent}]"
return f"[{val}]"Then in get_default_value:
- if arg_type.startswith("Vector<"):
- inner = arg_type[7:-1]
- val = self.get_default_value(arg_name, inner, depth + 1, visited.copy(), minimal=minimal)
- indent = " " * (depth + 1)
- item_indent = " " * (depth + 2)
- if inner not in CORE_TYPES:
- return f"[\n{item_indent}{val.replace('\n', '\n' + ' ')}\n{indent}]"
- return f"[{val}]"
+ if arg_type.startswith("Vector<"):
+ return self._default_for_vector(arg_name, arg_type, depth, visited, minimal)This is a small change, but it makes get_default_value’s main flow less tangled and opens the door to further splitting (e.g. _default_for_scalar, _default_for_constructor) later without altering behaviour now.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new documentation generation system for the raw MTProto API, which is a great enhancement. The new tl_generator.py file contains the core logic for parsing TL schema files and generating parameter trees and code examples.
My review focuses on the new tl_generator.py file, where I've identified a few areas for improvement regarding maintainability and robustness. Specifically, I've suggested:
- Using more robust parsing for TL files.
- Refactoring hardcoded values into constants for better readability and easier maintenance.
- Removing an unused parameter to improve code clarity.
These changes should make the new documentation generator more robust and easier to maintain in the long run. The overall implementation is well-done.
|
|
||
| if combinator_match := COMBINATOR_RE.match(line): | ||
| qualname, cid, qualtype = combinator_match.groups() | ||
| parts = line.split(" ") |
There was a problem hiding this comment.
Using line.split(' ') can be fragile if there are multiple spaces between parts of the TL definition, as it can produce empty strings in the parts list. It's more robust to use line.split() (with no arguments), which handles any amount of whitespace as a delimiter and discards empty strings.
| parts = line.split(" ") | |
| parts = line.split() |
| if arg_name == "self": arg_name = "is_self" | ||
| if arg_name == "from": arg_name = "from_peer" |
There was a problem hiding this comment.
For better maintainability and extensibility, consider using a dictionary for mapping reserved keywords instead of multiple if statements. This makes it easier to add more reserved keywords in the future.
You can define a RESERVED_KEYWORD_MAP constant at the module level:
RESERVED_KEYWORD_MAP = {
"self": "is_self",
"from": "from_peer",
}| if arg_name == "self": arg_name = "is_self" | |
| if arg_name == "from": arg_name = "from_peer" | |
| arg_name = RESERVED_KEYWORD_MAP.get(arg_name, arg_name) |
|
|
||
| if is_base and name in self.base_to_constructors: | ||
| constrs = self.base_to_constructors[name] | ||
| limit_expansion = name in ["InputPeer", "InputUser", "InputChannel", "InputFile", "InputMedia", "MessageEntity"] |
There was a problem hiding this comment.
To improve maintainability and readability, consider moving this hardcoded list of types to a module-level constant (as a set for efficient lookups). This makes it easier to manage the list of types for which tree expansion should be limited.
For example, you could define at the top of the file:
LIMITED_EXPANSION_TYPES = {
"InputPeer", "InputUser", "InputChannel", "InputFile", "InputMedia", "MessageEntity"
}And then use it here.
| limit_expansion = name in ["InputPeer", "InputUser", "InputChannel", "InputFile", "InputMedia", "MessageEntity"] | |
| limit_expansion = name in LIMITED_EXPANSION_TYPES |
| def get_default_value(self, arg_name, arg_type, depth=0, visited=None, minimal=False): | ||
| if visited is None: visited = set() |
There was a problem hiding this comment.
The visited parameter in get_default_value seems to be intended for cycle detection but is currently unused. The set is initialized but never populated or checked. Recursion is instead prevented by a hard depth limit (depth > 6).
To improve clarity and remove dead code, I suggest removing the visited parameter from this method's signature and from its recursive call sites (lines 208 and 232).
| def get_default_value(self, arg_name, arg_type, depth=0, visited=None, minimal=False): | |
| if visited is None: visited = set() | |
| def get_default_value(self, arg_name, arg_type, depth=0, minimal=False): |
1 participant
I have implemented a new documentation generation system for the raw MTProto API.
Key features:
await app.invoke(...)example. Nested types are inlined, and common fields likepeerandrandom_idare populated with realistic placeholders.Noneif not critical), deeply nested optional parameters are pruned to keep examples concise and useful.Changes were made to:
compiler/docs/tl_generator.py(New): Core logic for parsing TL files and generating trees/examples.compiler/docs/compiler.py: Integration with the documentation build process.compiler/docs/template/page.txt: Updated template for Sphinx RST pages.PR created automatically by Jules for task 8149885366910417951 started by @5hojib
Summary by Sourcery
Integrate a TL-based documentation generator to enrich raw MTProto API reference pages with schema signatures, parameter trees, and example code.
New Features:
Enhancements: