Skip to content

Commit f34bc8c

Browse files
Add docstring guidelines and fix key docstrings for MDX compatibility (#2452)
Co-authored-by: openhands <openhands@all-hands.dev>
1 parent a604ee3 commit f34bc8c

File tree

7 files changed

+407
-46
lines changed

7 files changed

+407
-46
lines changed
Lines changed: 297 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,297 @@
1+
#!/usr/bin/env python3
2+
"""Validate docstrings conform to MDX-compatible formatting guidelines.
3+
4+
This script checks that docstrings in the SDK use patterns that render correctly
5+
in Mintlify MDX documentation. It validates:
6+
7+
1. No REPL-style examples (>>>) - should use fenced code blocks instead
8+
2. Shell/config examples use fenced code blocks (prevents # becoming headers)
9+
10+
Run with: python scripts/check_docstrings.py
11+
Exit code 0 = all checks pass, 1 = violations found
12+
"""
13+
14+
import ast
15+
import sys
16+
from dataclasses import dataclass
17+
from pathlib import Path
18+
19+
20+
# Directories to check
21+
SDK_PATHS = [
22+
"openhands-sdk/openhands/sdk",
23+
]
24+
25+
# Files/directories to skip
26+
SKIP_PATTERNS = [
27+
"__pycache__",
28+
".pyc",
29+
"test_",
30+
"_test.py",
31+
]
32+
33+
# Core public API files to check strictly (these are documented on the website)
34+
# Other files will be checked but only emit warnings, not failures
35+
STRICT_CHECK_FILES = [
36+
"agent/agent.py",
37+
"llm/llm.py",
38+
"conversation/conversation.py",
39+
"tool/tool.py",
40+
"workspace/base.py",
41+
"observability/laminar.py",
42+
]
43+
44+
45+
@dataclass
46+
class Violation:
47+
"""A docstring formatting violation."""
48+
49+
file: Path
50+
line: int
51+
name: str
52+
rule: str
53+
message: str
54+
is_strict: bool = False # True if this is in a strictly-checked file
55+
56+
57+
def should_skip(path: Path) -> bool:
58+
"""Check if a path should be skipped."""
59+
path_str = str(path)
60+
return any(pattern in path_str for pattern in SKIP_PATTERNS)
61+
62+
63+
def check_repl_examples(
64+
docstring: str, name: str, lineno: int, file: Path
65+
) -> list[Violation]:
66+
"""Check for REPL-style examples (>>>).
67+
68+
These should be replaced with fenced code blocks for better MDX rendering.
69+
"""
70+
violations = []
71+
lines = docstring.split("\n")
72+
73+
for i, line in enumerate(lines):
74+
stripped = line.strip()
75+
if stripped.startswith(">>>"):
76+
violations.append(
77+
Violation(
78+
file=file,
79+
line=lineno + i,
80+
name=name,
81+
rule="no-repl-examples",
82+
message=(
83+
"Use fenced code blocks (```python) instead of >>> REPL style. "
84+
"REPL examples don't render well in MDX documentation."
85+
),
86+
)
87+
)
88+
# Only report once per docstring
89+
break
90+
91+
return violations
92+
93+
94+
def check_unfenced_shell_config(
95+
docstring: str, name: str, lineno: int, file: Path
96+
) -> list[Violation]:
97+
"""Check for shell/config examples that aren't in fenced code blocks.
98+
99+
Lines starting with # outside code blocks become markdown headers.
100+
"""
101+
violations = []
102+
lines = docstring.split("\n")
103+
in_code_block = False
104+
105+
for i, line in enumerate(lines):
106+
stripped = line.strip()
107+
108+
# Track code block state
109+
if stripped.startswith("```"):
110+
in_code_block = not in_code_block
111+
continue
112+
113+
# Skip if inside a code block
114+
if in_code_block:
115+
continue
116+
117+
# Check for shell-style comments that look like config
118+
# Pattern: line starts with # and previous line has = (config pattern)
119+
if stripped.startswith("#") and not stripped.startswith("# "):
120+
# This is likely a shell comment without space (less common in prose)
121+
continue
122+
123+
# Check for unfenced config: KEY=VALUE followed by # comment
124+
if i > 0:
125+
prev_line = lines[i - 1].strip() if i > 0 else ""
126+
# If previous line looks like config (VAR=value) and this is a # comment
127+
if "=" in prev_line and prev_line.split("=")[0].isupper():
128+
if stripped.startswith("# "):
129+
violations.append(
130+
Violation(
131+
file=file,
132+
line=lineno + i,
133+
name=name,
134+
rule="fenced-shell-config",
135+
message=(
136+
"Shell/config examples with # comments should be "
137+
"in ```bash code blocks. Otherwise # becomes a "
138+
"markdown header."
139+
),
140+
)
141+
)
142+
# Only report once per docstring
143+
break
144+
145+
return violations
146+
147+
148+
def check_docstring(
149+
docstring: str, name: str, lineno: int, file: Path
150+
) -> list[Violation]:
151+
"""Run all checks on a docstring."""
152+
if not docstring:
153+
return []
154+
155+
violations = []
156+
violations.extend(check_repl_examples(docstring, name, lineno, file))
157+
violations.extend(check_unfenced_shell_config(docstring, name, lineno, file))
158+
return violations
159+
160+
161+
def get_docstrings_from_file(file: Path) -> list[tuple[str, str, int]]:
162+
"""Extract all docstrings from a Python file.
163+
164+
Returns list of (name, docstring, lineno) tuples.
165+
"""
166+
try:
167+
source = file.read_text()
168+
tree = ast.parse(source)
169+
except (SyntaxError, UnicodeDecodeError) as e:
170+
print(f"Warning: Could not parse {file}: {e}", file=sys.stderr)
171+
return []
172+
173+
docstrings = []
174+
175+
for node in ast.walk(tree):
176+
name = None
177+
lineno = 0
178+
docstring = None
179+
180+
if isinstance(node, ast.Module):
181+
docstring = ast.get_docstring(node)
182+
name = file.stem
183+
lineno = 1
184+
elif isinstance(node, ast.ClassDef):
185+
docstring = ast.get_docstring(node)
186+
name = node.name
187+
lineno = node.lineno
188+
elif isinstance(node, ast.FunctionDef | ast.AsyncFunctionDef):
189+
docstring = ast.get_docstring(node)
190+
name = node.name
191+
lineno = node.lineno
192+
193+
if docstring and name:
194+
docstrings.append((name, docstring, lineno))
195+
196+
return docstrings
197+
198+
199+
def is_strict_file(file: Path, repo_root: Path) -> bool:
200+
"""Check if a file is in the strict check list."""
201+
try:
202+
rel_path = file.relative_to(repo_root / "openhands-sdk/openhands/sdk")
203+
return any(str(rel_path) == strict for strict in STRICT_CHECK_FILES)
204+
except ValueError:
205+
return False
206+
207+
208+
def check_file(file: Path, repo_root: Path) -> list[Violation]:
209+
"""Check all docstrings in a file."""
210+
violations = []
211+
is_strict = is_strict_file(file, repo_root)
212+
213+
for name, docstring, lineno in get_docstrings_from_file(file):
214+
file_violations = check_docstring(docstring, name, lineno, file)
215+
for v in file_violations:
216+
v.is_strict = is_strict
217+
violations.extend(file_violations)
218+
219+
return violations
220+
221+
222+
def main() -> int:
223+
"""Run docstring checks on all SDK files."""
224+
repo_root = Path(__file__).parent.parent.parent
225+
226+
all_violations: list[Violation] = []
227+
files_checked = 0
228+
229+
for sdk_path in SDK_PATHS:
230+
path = repo_root / sdk_path
231+
if not path.exists():
232+
print(f"Warning: Path not found: {path}", file=sys.stderr)
233+
continue
234+
235+
for py_file in path.rglob("*.py"):
236+
if should_skip(py_file):
237+
continue
238+
239+
files_checked += 1
240+
violations = check_file(py_file, repo_root)
241+
all_violations.extend(violations)
242+
243+
# Separate strict violations (errors) from warnings
244+
strict_violations = [v for v in all_violations if v.is_strict]
245+
warning_violations = [v for v in all_violations if not v.is_strict]
246+
247+
# Report warnings (non-strict files)
248+
if warning_violations:
249+
count = len(warning_violations)
250+
print(f"\n⚠️ Found {count} docstring warning(s) in non-core files:\n")
251+
252+
by_file: dict[Path, list[Violation]] = {}
253+
for v in warning_violations:
254+
by_file.setdefault(v.file, []).append(v)
255+
256+
for file, violations in sorted(by_file.items()):
257+
rel_path = file.relative_to(repo_root)
258+
print(f"📄 {rel_path}")
259+
for v in violations:
260+
print(f" Line {v.line}: {v.name} ({v.rule})")
261+
print()
262+
263+
# Report errors (strict files)
264+
if strict_violations:
265+
count = len(strict_violations)
266+
print(f"\n❌ Found {count} docstring error(s) in core API files:\n")
267+
268+
by_file: dict[Path, list[Violation]] = {}
269+
for v in strict_violations:
270+
by_file.setdefault(v.file, []).append(v)
271+
272+
for file, violations in sorted(by_file.items()):
273+
rel_path = file.relative_to(repo_root)
274+
print(f"📄 {rel_path}")
275+
for v in violations:
276+
print(f" Line {v.line}: {v.name}")
277+
print(f" Rule: {v.rule}")
278+
print(f" {v.message}")
279+
print()
280+
281+
print("=" * 60)
282+
print("To fix these issues:")
283+
print(" 1. Replace >>> examples with ```python code blocks")
284+
print(" 2. Wrap shell/config examples in ```bash code blocks")
285+
print("=" * 60)
286+
return 1
287+
288+
if warning_violations:
289+
count = len(warning_violations)
290+
print(f"✅ Core API files pass. {count} warnings in other files.")
291+
else:
292+
print(f"✅ All {files_checked} files pass docstring checks")
293+
return 0
294+
295+
296+
if __name__ == "__main__":
297+
sys.exit(main())
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
---
2+
# .github/workflows/check-docstrings.yml
3+
name: Check Docstrings
4+
5+
on:
6+
push:
7+
branches: [main]
8+
pull_request:
9+
branches: ['**']
10+
11+
jobs:
12+
check-docstrings:
13+
runs-on: ubuntu-24.04
14+
15+
steps:
16+
- name: Checkout code
17+
uses: actions/checkout@v5
18+
19+
- name: Set up Python
20+
uses: actions/setup-python@v6
21+
with:
22+
python-version: '3.13'
23+
24+
- name: Check docstring formatting
25+
run: python .github/scripts/check_docstrings.py

openhands-sdk/openhands/sdk/agent/agent.py

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,11 +80,21 @@ class Agent(CriticMixin, AgentBase):
8080
AgentBase and implements the agent execution logic. Critic-related functionality
8181
is provided by CriticMixin.
8282
83+
Attributes:
84+
llm: The language model instance used for reasoning.
85+
tools: List of tools available to the agent.
86+
name: Optional agent identifier.
87+
system_prompt: Custom system prompt (uses default if not provided).
88+
8389
Example:
84-
>>> from openhands.sdk import LLM, Agent, Tool
85-
>>> llm = LLM(model="claude-sonnet-4-20250514", api_key=SecretStr("key"))
86-
>>> tools = [Tool(name="TerminalTool"), Tool(name="FileEditorTool")]
87-
>>> agent = Agent(llm=llm, tools=tools)
90+
```python
91+
from openhands.sdk import LLM, Agent, Tool
92+
from pydantic import SecretStr
93+
94+
llm = LLM(model="claude-sonnet-4-20250514", api_key=SecretStr("key"))
95+
tools = [Tool(name="TerminalTool"), Tool(name="FileEditorTool")]
96+
agent = Agent(llm=llm, tools=tools)
97+
```
8898
"""
8999

90100
@model_validator(mode="before")

openhands-sdk/openhands/sdk/conversation/conversation.py

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -40,17 +40,21 @@ class Conversation:
4040
is remote.
4141
4242
Example:
43-
>>> from openhands.sdk import LLM, Agent, Conversation
44-
>>> from openhands.sdk.plugin import PluginSource
45-
>>> llm = LLM(model="claude-sonnet-4-20250514", api_key=SecretStr("key"))
46-
>>> agent = Agent(llm=llm, tools=[])
47-
>>> conversation = Conversation(
48-
... agent=agent,
49-
... workspace="./workspace",
50-
... plugins=[PluginSource(source="github:org/security-plugin", ref="v1.0")],
51-
... )
52-
>>> conversation.send_message("Hello!")
53-
>>> conversation.run()
43+
```python
44+
from openhands.sdk import LLM, Agent, Conversation
45+
from openhands.sdk.plugin import PluginSource
46+
from pydantic import SecretStr
47+
48+
llm = LLM(model="claude-sonnet-4-20250514", api_key=SecretStr("key"))
49+
agent = Agent(llm=llm, tools=[])
50+
conversation = Conversation(
51+
agent=agent,
52+
workspace="./workspace",
53+
plugins=[PluginSource(source="github:org/security-plugin", ref="v1.0")],
54+
)
55+
conversation.send_message("Hello!")
56+
conversation.run()
57+
```
5458
"""
5559

5660
@overload

0 commit comments

Comments
 (0)