-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix(sdk): README doc-lies, actionable errors, and a compile-checked Hello World #1523
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
ytallo
wants to merge
3
commits into
main
Choose a base branch
from
sdk-dx-fixes
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| name: SDK README Guard | ||
|
|
||
| on: | ||
| push: | ||
| branches: [main] | ||
| paths: | ||
| - 'sdk/**/README.md' | ||
| - 'sdk/README.md' | ||
| - 'sdk/packages/rust/iii/examples/readme_hello.rs' | ||
| - 'sdk/packages/rust/iii/src/**' | ||
| - 'sdk/packages/python/iii/src/**' | ||
| - 'sdk/fixtures/readme-guard/**' | ||
| - 'scripts/readme-guard/**' | ||
| - '.github/workflows/sdk-readme.yml' | ||
| pull_request: | ||
| branches: [main] | ||
| paths: | ||
| - 'sdk/**/README.md' | ||
| - 'sdk/README.md' | ||
| - 'sdk/packages/rust/iii/examples/readme_hello.rs' | ||
| - 'sdk/packages/rust/iii/src/**' | ||
| - 'sdk/packages/python/iii/src/**' | ||
| - 'sdk/fixtures/readme-guard/**' | ||
| - 'scripts/readme-guard/**' | ||
| - '.github/workflows/sdk-readme.yml' | ||
| workflow_dispatch: | ||
|
|
||
| concurrency: | ||
| group: ${{ github.workflow }}-${{ github.ref }} | ||
| cancel-in-progress: ${{ github.event_name == 'pull_request' }} | ||
|
|
||
| # This workflow runs in warn-only mode so it can ship alongside the first round | ||
| # of README fixes without blocking merges. After the follow-up PR flips | ||
| # `continue-on-error` to `false`, any new README drift will fail CI. | ||
| jobs: | ||
| rust-readme: | ||
| name: Rust — cargo check --example readme_hello | ||
| runs-on: ubuntu-latest | ||
| continue-on-error: true | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - uses: dtolnay/rust-toolchain@stable | ||
|
|
||
| - uses: Swatinem/rust-cache@v2 | ||
|
|
||
| - name: Compile the canonical Rust Hello World | ||
| run: cargo check -p iii-sdk --example readme_hello | ||
|
|
||
| python-readme: | ||
| name: Python — attribute-probe against real `iii.III` | ||
| runs-on: ubuntu-latest | ||
| continue-on-error: true | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: '3.12' | ||
|
|
||
| - name: Install guard deps + editable SDK | ||
| run: | | ||
| python -m pip install --upgrade pip | ||
| python -m pip install markdown-it-py | ||
| python -m pip install -e sdk/packages/python/iii | ||
|
|
||
| - name: Verify golden-good fixture passes | ||
| run: python scripts/readme-guard/check_python_readme.py sdk/fixtures/readme-guard/golden-good-python.md | ||
|
|
||
| - name: Verify golden-bad fixture fails (meta-test) | ||
| # The guard *must* exit non-zero on this fixture. If it exits 0, the | ||
| # guard is silently broken and would let real bugs ship. | ||
| run: | | ||
| set +e | ||
| python scripts/readme-guard/check_python_readme.py sdk/fixtures/readme-guard/golden-bad-python.md | ||
| rc=$? | ||
| set -e | ||
| if [ "$rc" = "0" ]; then | ||
| echo "::error::Guard passed the golden-bad fixture — it is silently broken." | ||
| exit 1 | ||
| fi | ||
| echo "Guard correctly rejected golden-bad (exit $rc)." | ||
|
|
||
| - name: Run guard against real READMEs | ||
| run: | | ||
| python scripts/readme-guard/check_python_readme.py \ | ||
| sdk/packages/python/iii/README.md \ | ||
| sdk/packages/node/iii/README.md \ | ||
| sdk/README.md |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,135 @@ | ||
| #!/usr/bin/env python3 | ||
| """Verify Python README code blocks use only real `III` attributes. | ||
|
|
||
| Catches the class of bug where a README teaches `iii.connect()` while the | ||
| actual class exposes only `connect_async()`. Compile-only checks (py_compile, | ||
| mypy without type stubs) pass such code because `iii.connect()` is valid | ||
| Python syntax — it AttributeErrors only at runtime. | ||
|
|
||
| Usage: | ||
| python check_python_readme.py README.md [README.md ...] | ||
|
|
||
| Exit code 0 = all snippets reference real attributes, 1 = at least one | ||
| snippet calls a method that does not exist on `iii.III`. | ||
|
|
||
| The script uses `markdown-it-py` to parse code fences correctly (no regex) | ||
| and `ast` + `inspect` to resolve attribute references against the real | ||
| class. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import ast | ||
| import inspect | ||
| import sys | ||
| from pathlib import Path | ||
|
|
||
| try: | ||
| from markdown_it import MarkdownIt | ||
| except ImportError: # pragma: no cover - bootstrap guidance | ||
| sys.stderr.write( | ||
| "check_python_readme: markdown-it-py is required. " | ||
| "Install with `pip install markdown-it-py`.\n" | ||
| ) | ||
| sys.exit(2) | ||
|
|
||
|
|
||
| def extract_python_fences(md_text: str) -> list[tuple[int, str]]: | ||
| """Return (line_number, source) for each ```python fenced block.""" | ||
| md = MarkdownIt("commonmark") | ||
| tokens = md.parse(md_text) | ||
| fences: list[tuple[int, str]] = [] | ||
| for tok in tokens: | ||
| if tok.type != "fence": | ||
| continue | ||
| info = (tok.info or "").strip().lower() | ||
| # Accept `python`, `py`, or info strings that start with `python` | ||
| # (e.g., `python3`, `python,ignore`). | ||
| if info == "python" or info == "py" or info.startswith("python"): | ||
| line = (tok.map[0] + 1) if tok.map else 0 | ||
| fences.append((line, tok.content)) | ||
| return fences | ||
|
|
||
|
|
||
| def iii_class_attrs() -> set[str]: | ||
| """Return the set of public attribute names available on `iii.III`.""" | ||
| from iii.iii import III # noqa: E402 - runtime import | ||
|
|
||
| return {name for name, _ in inspect.getmembers(III) if not name.startswith("_")} | ||
|
|
||
|
|
||
| def attr_calls_on_iii_var(source: str) -> list[tuple[int, str]]: | ||
| """Find method calls of the form `iii.<name>(...)` in a snippet. | ||
|
|
||
| Returns a list of (lineno, attr_name). Heuristic: we treat any bare | ||
| identifier named ``iii`` as an `III` instance. The Hello World pattern | ||
| `iii = register_worker(...)` makes this reliable. | ||
| """ | ||
| try: | ||
| tree = ast.parse(source) | ||
| except SyntaxError: | ||
| # Syntax errors are a separate class of problem; `py_compile` already | ||
| # catches them. Skip attribute checking when parse fails. | ||
| return [] | ||
|
|
||
| calls: list[tuple[int, str]] = [] | ||
| for node in ast.walk(tree): | ||
| if not isinstance(node, ast.Call): | ||
| continue | ||
| fn = node.func | ||
| if not isinstance(fn, ast.Attribute): | ||
| continue | ||
| if not isinstance(fn.value, ast.Name): | ||
| continue | ||
| if fn.value.id != "iii": | ||
| continue | ||
| calls.append((node.lineno, fn.attr)) | ||
| return calls | ||
|
|
||
|
|
||
| def check_file(path: Path, allowed_attrs: set[str]) -> list[str]: | ||
| errors: list[str] = [] | ||
| md_text = path.read_text() | ||
| for block_line, source in extract_python_fences(md_text): | ||
| for call_lineno, attr in attr_calls_on_iii_var(source): | ||
| if attr in allowed_attrs: | ||
| continue | ||
| # Allow the dynamic `attr` sentinel names we know about: | ||
| # e.g., handler closures aren't on `iii`. | ||
| errors.append( | ||
| f"{path}:{block_line + call_lineno - 1}: " | ||
| f"iii.{attr}() is called but `iii.III` has no public " | ||
| f"attribute named `{attr}`. " | ||
| f"Did the README outlive a refactor?" | ||
| ) | ||
| return errors | ||
|
|
||
|
|
||
| def main() -> int: | ||
| if len(sys.argv) < 2: | ||
| sys.stderr.write(f"usage: {sys.argv[0]} README.md [README.md ...]\n") | ||
| return 2 | ||
|
|
||
| try: | ||
| allowed = iii_class_attrs() | ||
| except Exception as exc: # pragma: no cover - import failure path | ||
| sys.stderr.write( | ||
| f"check_python_readme: failed to import `iii.III` to probe its " | ||
| f"attributes: {exc}\n" | ||
| "Make sure the iii-sdk package is installed (editable mode is " | ||
| "fine) before running this check.\n" | ||
| ) | ||
| return 2 | ||
|
|
||
| all_errors: list[str] = [] | ||
| for arg in sys.argv[1:]: | ||
| all_errors.extend(check_file(Path(arg), allowed)) | ||
|
|
||
| for msg in all_errors: | ||
| sys.stderr.write(msg + "\n") | ||
|
|
||
| return 0 if not all_errors else 1 | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| sys.exit(main()) | ||
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| # Golden bad (Python) | ||
|
|
||
| This fixture MUST make the guard exit 1 — it contains the exact bug the | ||
| guard was built to catch: `iii.connect()` is valid Python syntax but the | ||
| `iii.III` class does not expose a public `connect` method. | ||
|
|
||
| ```python | ||
| from iii import register_worker | ||
|
|
||
| iii = register_worker("ws://localhost:49134") | ||
|
|
||
| def greet(data): | ||
| return {"message": f"Hello, {data['name']}!"} | ||
|
|
||
| iii.register_function("greet", greet) | ||
| iii.connect() # <-- intentional bug: no such method | ||
| ``` |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| # Golden good (Python) | ||
|
|
||
| This fixture exists so CI can prove the guard is *working* — not silently | ||
| letting everything pass. The guard must exit 0 on this file. | ||
|
|
||
| ```python | ||
| from iii import register_worker | ||
|
|
||
| iii = register_worker("ws://localhost:49134") | ||
|
|
||
| def greet(data): | ||
| return {"message": f"Hello, {data['name']}!"} | ||
|
|
||
| iii.register_function("greet", greet) | ||
|
|
||
| iii.register_trigger({ | ||
| "type": "http", | ||
| "function_id": "greet", | ||
| "config": {"api_path": "/greet", "http_method": "POST"}, | ||
| }) | ||
|
|
||
| result = iii.trigger({"function_id": "greet", "payload": {"name": "world"}}) | ||
| ``` |
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.