fix(sdk): README doc-lies, actionable errors, and a compile-checked Hello World#1522
Closed
ytallo wants to merge 2 commits into
Closed
fix(sdk): README doc-lies, actionable errors, and a compile-checked Hello World#1522ytallo wants to merge 2 commits into
ytallo wants to merge 2 commits into
Conversation
Rust: - Add `TriggerRequest::new(function_id, payload)` with `.with_action(action)` / `.with_timeout_ms(ms)` chainable helpers. The root SDK README already documented `TriggerRequest::new` — this makes it real. - Rewrite `IIIError::NotConnected` to include the engine URL, the `iii --config config.yaml` command, and a link to https://iii.dev/docs/install. - New `tests/ergonomics.rs` unit tests for the constructors and the new error-message contract (7 tests, all green). Python: - `_wait_until_connected` raises `ConnectionError` with the target URL, how to start the engine, and the docs link. - New test in `test_init_api.py` asserting the error contains each piece. Node: - Max-retries failure log names the URL, the start command, and the docs link. No existing call site changes. Purely additive in Rust, string-only changes in Python/Node. All existing SDK tests pass unchanged.
READMEs (Python, Rust, Root, Node):
- Python: remove non-existent `iii.connect()` call, switch Hello World
to string-form `register_function("greet", handler)`, add Prerequisites
block.
- Rust: bump install line from `iii-sdk = "0.3"` (lied about version) to
`"0.11"`, rewrite Hello World with signatures that actually compile
(`RegisterFunction::new_async`, `TriggerRequest::new`, no `?` on
`register_worker`), promote the `IIITrigger::Http(...).for_function(...)`
typed builder as the recommended trigger API, keep raw struct-literal
form as an advanced escape hatch, add Prerequisites block.
- Root: rewrite Rust Hello World to match the per-SDK README (drop fake
`TriggerRequest::new` without the impl), spell out `TriggerAction.Void()`
for Python (previously marked "Same" as Node), add a shared
Prerequisites block.
- Node: add Prerequisites block (snippets were already correct).
Source-of-truth Rust example:
- New `sdk/packages/rust/iii/examples/readme_hello.rs` — the canonical
Rust Hello World, verified by CI via `cargo check --example readme_hello`.
The README links to this file so drift is surface-visible.
CHANGELOGs + packaging:
- Bootstrap `CHANGELOG.md` per SDK in Keep-a-Changelog format with an
[Unreleased] entry describing this PR and one retro entry for the
`call`/`callVoid`/`triggerVoid` removal.
- Node `package.json`: add explicit `files` array including `CHANGELOG.md`.
- Python `pyproject.toml`: add `[tool.hatch.build.targets.wheel.force-include]`
so the top-level `CHANGELOG.md` ships inside the wheel at
`iii/CHANGELOG.md`, plus `[tool.hatch.build.targets.sdist]`
include list and a `Changelog` project URL.
- Verified: `npm pack --dry-run`, `python -m build --wheel`, and
`cargo package --list` all include CHANGELOG.md for their respective
registries.
CI README guard (warn-only):
- New `.github/workflows/sdk-readme.yml` with two non-blocking jobs:
Rust runs `cargo check --example readme_hello`; Python runs an
attribute-probe that parses README code fences via `markdown-it-py`
and verifies every `iii.<method>()` call resolves against the real
`iii.III` class — catches the `iii.connect()` class of bug that
`python -m py_compile` would miss.
- New `scripts/readme-guard/check_python_readme.py` — the attribute-probe.
- New meta-guard fixtures at `sdk/fixtures/readme-guard/` (golden-good
and golden-bad) so a separate CI step proves the guard itself is
working. Workflow runs warn-only for this PR; a follow-up flips
`continue-on-error: false` once the three SDKs are green.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughThis PR establishes automated validation of SDK README examples and improves developer experience across Python, Node, and Rust SDKs through updated documentation, enhanced error messages, new builder APIs, and changelog files. A GitHub Actions workflow runs CI checks to ensure Rust and Python code examples in READMEs remain valid. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsTimed out fetching pipeline failures after 30000ms Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
sdk/packages/python/iii/README.md (1)
25-48:⚠️ Potential issue | 🟡 MinorHello World should end with
iii.shutdown().The referenced guideline for this file says the Hello World should "reflect the current
trigger(..., action=TriggerAction.Void())andiii.shutdown()usage". The current snippet never callsshutdown(), which leaves the background event-loop thread running (it's non-daemon — seeself._thread = threading.Thread(target=self._loop.run_forever, daemon=False)iniii.py) and will hang a copy-pasted script on exit.📝 Suggested addition
result = iii.trigger({"function_id": "greet", "payload": {"name": "world"}}) print(result) # {"message": "Hello, world!"} + +iii.shutdown()As per coding guidelines: "For fire-and-forget/shutdown, reflect the current
trigger(..., action=TriggerAction.Void())andiii.shutdown()usage."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/packages/python/iii/README.md` around lines 25 - 48, Update the example to perform a fire-and-forget trigger and then cleanly stop the worker: call trigger with action=TriggerAction.Void() (or equivalent) for the fire-and-forget invocation and then call iii.shutdown() at the end to stop the background event-loop thread created by register_worker; ensure the example imports/uses TriggerAction and demonstrates the shutdown call after the trigger so a copy-pasted script will not hang.
🧹 Nitpick comments (1)
sdk/packages/rust/iii/src/error.rs (1)
8-14: Carry the target URL inNotConnectedfor parity with Node/Python.The Node (
iii.ts:722) and Python (iii.py:156) implementations interpolate the actual engine address into the error message, which is critical when multiple services or environment-specific URLs are in play. The Rust implementation uses a unit variant and can only mention the defaultws://localhost:49134, making the error misleading for users who passed a custom URL toregister_worker(...).Promote the variant to carry the address:
♻️ Proposed refactor
- #[error( - "iii is not connected: engine unreachable. Verify the engine is running \ - (`iii --config config.yaml`) and that the WebSocket URL passed to \ - `register_worker` matches (default: ws://localhost:49134). \ - See https://iii.dev/docs/install" - )] - NotConnected, + #[error( + "iii is not connected: engine unreachable at {address}. Verify the engine is running \ + (`iii --config config.yaml`) and that the WebSocket URL passed to \ + `register_worker` matches (default: ws://localhost:49134). \ + See https://iii.dev/docs/install" + )] + NotConnected { address: String },The
addressis available at all 5 construction sites (iii.rs:1152,iii.rs:1418, and 3 sites inchannels.rs). The ergonomics test (tests/ergonomics.rs:9) will need a field assertion update.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/packages/rust/iii/src/error.rs` around lines 8 - 14, Change the NotConnected unit variant in the Error enum in error.rs to carry the engine address (e.g., NotConnected(String)) and update its Display/#[error(...)] message to interpolate that address instead of the hardcoded default; then update all construction sites that create NotConnected (references in iii.rs at the two locations and the three sites in channels.rs) to pass the actual address String, and adjust the ergonomics test in tests/ergonomics.rs to assert the new field or updated message accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/sdk-readme.yml:
- Around line 6-25: Update the path filters in .github/workflows/sdk-readme.yml
so the workflow actually triggers when Node SDK sources change: add the Node
package paths (for example include 'sdk/packages/node/iii/src/**' and
'sdk/packages/node/iii/README.md' or broader 'sdk/packages/node/**') to both the
push and pull_request path lists so the guard step that probes
sdk/packages/node/iii/README.md and sdk/README.md runs when Node API-source
files are modified.
- Around line 84-89: The workflow step invoking check_python_readme.py is
incorrectly being run against the Node README (sdk/packages/node/iii/README.md),
which is a no-op because check_python_readme.py parses Python fenced blocks;
remove sdk/packages/node/iii/README.md from the arguments to the
check_python_readme.py invocation OR add a separate Node-specific guard:
create/run a Node/TS checker (e.g., check_node_readme.js or a tsc/ts-node
type-check command) against sdk/packages/node/iii/README.md and update the
workflow step to call that checker instead of passing the Node README into
check_python_readme.py.
In `@scripts/readme-guard/check_python_readme.py`:
- Around line 68-73: The parser currently swallows SyntaxError by returning []
after ast.parse(source) fails; change this so a SyntaxError in README Python
fences is treated as a test failure — raise or append a descriptive error
(include the snippet context) instead of returning an empty list in the
try/except around ast.parse in the function that parses fences; additionally,
wrap the calls to attr_calls_on_iii_var inside check_file (the block around the
lines currently invoking attr_calls_on_iii_var) in a try/except that catches
SyntaxError and reports it (rather than skipping), so malformed Python snippets
are reported as errors with clear context.
In `@sdk/packages/python/iii/CHANGELOG.md`:
- Around line 25-30: The changelog entry uses camelCase method names; update it
to Python-style snake_case: replace `call`, `callVoid`, `triggerVoid` with
`call`, `call_void`, `trigger_void` in the Removed section and ensure the usage
example references `iii.trigger()` and the Python `TriggerAction.Void()`
invocation; adjust the text so Python developers will see the correct snake_case
symbols (`call`, `call_void`, `trigger_void`, `trigger`, `TriggerAction.Void`)
when searching the codebase.
In `@sdk/packages/rust/iii/README.md`:
- Line 87: Examples use dotted function IDs; update them to use :: namespace
separators for consistency. Locate calls to iii.register_function and uses of
RegisterFunction::new_async (e.g., RegisterFunction::new_async("orders.create",
...)) and change string IDs from "orders.create" style to "orders::create" (and
any other dotted IDs in the README examples) so they match the :: convention
used elsewhere (e.g., stream::set/stream::update).
In `@sdk/README.md`:
- Around line 111-112: Update the cross-SDK API table to reflect that Python's
trigger() is synchronous (remove the leading "await" in the Python column for
the Invoke rows) and change any "Node/Python" example that currently uses JS
object literal syntax to use Python dict syntax instead (e.g., use
{"function_id": ..., "payload": ...} style for Python rather than JS object
notation); specifically edit the entries referencing iii.trigger(),
TriggerAction.Void(), and TriggerRequest examples so Python examples omit await
and use Python dicts where Node/Python is indicated.
---
Outside diff comments:
In `@sdk/packages/python/iii/README.md`:
- Around line 25-48: Update the example to perform a fire-and-forget trigger and
then cleanly stop the worker: call trigger with action=TriggerAction.Void() (or
equivalent) for the fire-and-forget invocation and then call iii.shutdown() at
the end to stop the background event-loop thread created by register_worker;
ensure the example imports/uses TriggerAction and demonstrates the shutdown call
after the trigger so a copy-pasted script will not hang.
---
Nitpick comments:
In `@sdk/packages/rust/iii/src/error.rs`:
- Around line 8-14: Change the NotConnected unit variant in the Error enum in
error.rs to carry the engine address (e.g., NotConnected(String)) and update its
Display/#[error(...)] message to interpolate that address instead of the
hardcoded default; then update all construction sites that create NotConnected
(references in iii.rs at the two locations and the three sites in channels.rs)
to pass the actual address String, and adjust the ergonomics test in
tests/ergonomics.rs to assert the new field or updated message accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 52b9c046-da35-4bab-a198-9c686aa266df
⛔ Files ignored due to path filters (1)
sdk/packages/python/iii/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
.github/workflows/sdk-readme.ymlscripts/readme-guard/check_python_readme.pysdk/README.mdsdk/fixtures/readme-guard/golden-bad-python.mdsdk/fixtures/readme-guard/golden-good-python.mdsdk/packages/node/iii/CHANGELOG.mdsdk/packages/node/iii/README.mdsdk/packages/node/iii/package.jsonsdk/packages/node/iii/src/iii.tssdk/packages/python/iii/CHANGELOG.mdsdk/packages/python/iii/README.mdsdk/packages/python/iii/pyproject.tomlsdk/packages/python/iii/src/iii/iii.pysdk/packages/python/iii/tests/test_init_api.pysdk/packages/rust/iii/CHANGELOG.mdsdk/packages/rust/iii/README.mdsdk/packages/rust/iii/examples/readme_hello.rssdk/packages/rust/iii/src/error.rssdk/packages/rust/iii/src/protocol.rssdk/packages/rust/iii/tests/ergonomics.rs
Comment on lines
+6
to
+25
| 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' |
Contributor
There was a problem hiding this comment.
Path filters omit Node/root SDK sources that the guard probes.
The final guard step probes sdk/packages/node/iii/README.md and sdk/README.md, but the triggers do not include sdk/packages/node/iii/src/** or anything that would catch Node API-surface churn. If a Node public API renames, the Node README could regress without this workflow running. Either narrow the guard targets (see sibling comment) or expand the path filters to match what you're actually checking.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/sdk-readme.yml around lines 6 - 25, Update the path
filters in .github/workflows/sdk-readme.yml so the workflow actually triggers
when Node SDK sources change: add the Node package paths (for example include
'sdk/packages/node/iii/src/**' and 'sdk/packages/node/iii/README.md' or broader
'sdk/packages/node/**') to both the push and pull_request path lists so the
guard step that probes sdk/packages/node/iii/README.md and sdk/README.md runs
when Node API-source files are modified.
Comment on lines
+84
to
+89
| - 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 |
Contributor
There was a problem hiding this comment.
Probing the Node README with the Python attribute probe is a no-op.
check_python_readme.py parses Python fenced blocks and checks iii.<attr>(...) against iii.iii.III. The Node README contains TypeScript/JS code blocks, so this step will almost certainly find nothing and exit 0 regardless of drift — it provides false confidence. Either drop sdk/packages/node/iii/README.md from this invocation, or add a separate Node guard (e.g., a TS type-check of the Node Hello World) and run that against the Node README.
📝 Minimal fix
- 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📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - 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 | |
| - name: Run guard against real READMEs | |
| run: | | |
| python scripts/readme-guard/check_python_readme.py \ | |
| sdk/packages/python/iii/README.md \ | |
| sdk/README.md |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/sdk-readme.yml around lines 84 - 89, The workflow step
invoking check_python_readme.py is incorrectly being run against the Node README
(sdk/packages/node/iii/README.md), which is a no-op because
check_python_readme.py parses Python fenced blocks; remove
sdk/packages/node/iii/README.md from the arguments to the check_python_readme.py
invocation OR add a separate Node-specific guard: create/run a Node/TS checker
(e.g., check_node_readme.js or a tsc/ts-node type-check command) against
sdk/packages/node/iii/README.md and update the workflow step to call that
checker instead of passing the Node README into check_python_readme.py.
Comment on lines
+68
to
+73
| 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 [] |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether syntax errors are suppressed here and whether another workflow step explicitly compiles README fences.
rg -n -C4 'except SyntaxError|return \[\]' scripts/readme-guard/check_python_readme.py
rg -n -C3 'py_compile|compileall|check_python_readme' --glob '*.yml' --glob '*.yaml'Repository: iii-hq/iii
Length of output: 493
🏁 Script executed:
#!/bin/bash
# Search for py_compile and compileall usage throughout the codebase
echo "=== Searching for py_compile usage ==="
rg -r 'py_compile' --no-heading
echo ""
echo "=== Searching for compileall usage ==="
rg -r 'compileall' --no-heading
echo ""
echo "=== Searching for check_python_readme invocation in all files ==="
rg -r 'check_python_readme' --no-headingRepository: iii-hq/iii
Length of output: 1407
🏁 Script executed:
#!/bin/bash
# View the full context of the check_file function and lines 93-94
echo "=== Full check_python_readme.py file ==="
wc -l scripts/readme-guard/check_python_readme.py
echo ""
cat -n scripts/readme-guard/check_python_readme.pyRepository: iii-hq/iii
Length of output: 5648
🏁 Script executed:
#!/bin/bash
# Check for workflow files and their content
echo "=== Workflow files ==="
fd -e yml -e yaml | head -20
echo ""
echo "=== Check for any CI/workflow references to readme or syntax ==="
rg -l 'readme|syntax' --glob '*.yml' --glob '*.yaml' --glob '.github'Repository: iii-hq/iii
Length of output: 912
🏁 Script executed:
#!/bin/bash
cat -n .github/workflows/sdk-readme.ymlRepository: iii-hq/iii
Length of output: 3619
🏁 Script executed:
#!/bin/bash
cat -n sdk/fixtures/readme-guard/golden-bad-python.mdRepository: iii-hq/iii
Length of output: 655
Fail closed on Python syntax errors in README fences.
Right now a malformed Python snippet returns [] instead of reporting an error, allowing this guard to exit successfully when it should fail. The comment claiming py_compile already catches syntax errors is incorrect—the workflow contains no such compile step.
Lines 93–94 in check_file also need wrapping to catch and report SyntaxError from attr_calls_on_iii_var instead of silently skipping syntax-invalid blocks.
Proposed fix
- 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 []
+ tree = ast.parse(source)
@@
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):
+ try:
+ calls = attr_calls_on_iii_var(source)
+ except SyntaxError as exc:
+ errors.append(
+ f"{path}:{block_line + (exc.lineno or 1) - 1}: "
+ f"Python README snippet has invalid syntax: {exc.msg}"
+ )
+ continue
+
+ for call_lineno, attr in calls:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/readme-guard/check_python_readme.py` around lines 68 - 73, The parser
currently swallows SyntaxError by returning [] after ast.parse(source) fails;
change this so a SyntaxError in README Python fences is treated as a test
failure — raise or append a descriptive error (include the snippet context)
instead of returning an empty list in the try/except around ast.parse in the
function that parses fences; additionally, wrap the calls to
attr_calls_on_iii_var inside check_file (the block around the lines currently
invoking attr_calls_on_iii_var) in a try/except that catches SyntaxError and
reports it (rather than skipping), so malformed Python snippets are reported as
errors with clear context.
Comment on lines
+25
to
+30
| ### Removed | ||
| - Removed legacy `call`, `callVoid`, `triggerVoid` methods. Use `trigger()` | ||
| for all invocations: | ||
| ```python | ||
| iii.trigger({"function_id": id, "payload": data, "action": TriggerAction.Void()}) | ||
| ``` |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for any historical Python definitions/usages of the legacy invocation methods.
rg -nP --type=py -C2 '\b(call_void|trigger_void|callVoid|triggerVoid)\b'
git log --all --oneline -S 'call_void' -- 'sdk/packages/python' | head
git log --all --oneline -S 'trigger_void' -- 'sdk/packages/python' | headRepository: iii-hq/iii
Length of output: 773
🏁 Script executed:
#!/bin/bash
# First, read the CHANGELOG.md to see full context around lines 25-30
cat -n sdk/packages/python/iii/CHANGELOG.md | head -40Repository: iii-hq/iii
Length of output: 1577
🏁 Script executed:
#!/bin/bash
# Check the commit that mentions unifying Python SDK public API
git show f1440898 --stat | head -20Repository: iii-hq/iii
Length of output: 1125
🏁 Script executed:
#!/bin/bash
# Look at the diff for Python SDK changes in that commit, focusing on removed methods
git show f1440898 -- sdk/packages/python/ | grep -A5 -B5 'call\|trigger' | head -100Repository: iii-hq/iii
Length of output: 4611
🏁 Script executed:
#!/bin/bash
# Check the parent commit before the refactor to see the old Python SDK API
git show f1440898~1:sdk/packages/python/iii/src/iii/__init__.py 2>/dev/null | head -100Repository: iii-hq/iii
Length of output: 2107
🏁 Script executed:
#!/bin/bash
# Also check if there's an older version with these methods by looking at the SDK structure
git log --all --oneline --follow -- 'sdk/packages/python/iii/src/iii/__init__.py' | head -20Repository: iii-hq/iii
Length of output: 1121
🏁 Script executed:
#!/bin/bash
# Search for any mention of 'call' method in Python SDK history
git log --all -p -- 'sdk/packages/python' | grep -B5 -A5 'def call' | head -60Repository: iii-hq/iii
Length of output: 2117
Correct CHANGELOG to use Python-style method names.
The Python SDK removed call, call_void (not callVoid), and trigger_void (not triggerVoid). Python developers will not find camelCase names when grepping their codebase. Update the entry:
- Removed legacy `call`, `call_void`, `trigger_void` methods. Use `trigger()`
for all invocations:
```python
iii.trigger({"function_id": id, "payload": data, "action": TriggerAction.Void()})
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @sdk/packages/python/iii/CHANGELOG.md around lines 25 - 30, The changelog
entry uses camelCase method names; update it to Python-style snake_case: replace
call, callVoid, triggerVoid with call, call_void, trigger_void in
the Removed section and ensure the usage example references iii.trigger() and
the Python TriggerAction.Void() invocation; adjust the text so Python
developers will see the correct snake_case symbols (call, call_void,
trigger_void, trigger, TriggerAction.Void) when searching the codebase.
</details>
<!-- fingerprinting:phantom:poseidon:nectarine:1db9236b-3c5d-439f-9488-ed2fa9699b73 -->
<!-- This is an auto-generated comment by CodeRabbit -->
| use serde_json::{Value, json}; | ||
|
|
||
| iii.register_function("orders.create", |input: Value| async move { | ||
| iii.register_function(RegisterFunction::new_async("orders.create", |input: Value| async move { |
Contributor
There was a problem hiding this comment.
Use :: separators for namespaced function IDs in examples.
These snippets use dotted IDs while the same README already uses stream::set/stream::update; keeping examples on :: avoids teaching two ID conventions.
Proposed fix
-iii.register_function(RegisterFunction::new_async("orders.create", |input: Value| async move {
+iii.register_function(RegisterFunction::new_async("orders::create", |input: Value| async move {
@@
- .for_function("orders.create"),
+ .for_function("orders::create"),
@@
- function_id: "orders.create".into(),
+ function_id: "orders::create".into(),
@@
- .trigger(TriggerRequest::new("orders.create", json!({ "body": { "item": "widget" } })))
+ .trigger(TriggerRequest::new("orders::create", json!({ "body": { "item": "widget" } })))
@@
- TriggerRequest::new("analytics.track", json!({ "event": "page_view" }))
+ TriggerRequest::new("analytics::track", json!({ "event": "page_view" }))
@@
- TriggerRequest::new("orders.process", json!({ "order_id": "456" }))
+ TriggerRequest::new("orders::process", json!({ "order_id": "456" }))As per coding guidelines, use :: separator for function IDs.
Also applies to: 103-105, 118-118, 132-145
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sdk/packages/rust/iii/README.md` at line 87, Examples use dotted function
IDs; update them to use :: namespace separators for consistency. Locate calls to
iii.register_function and uses of RegisterFunction::new_async (e.g.,
RegisterFunction::new_async("orders.create", ...)) and change string IDs from
"orders.create" style to "orders::create" (and any other dotted IDs in the
README examples) so they match the :: convention used elsewhere (e.g.,
stream::set/stream::update).
Comment on lines
+111
to
+112
| | Invoke (await) | `await iii.trigger({ function_id, payload })` | `await iii.trigger({"function_id": id, "payload": data})` | `iii.trigger(TriggerRequest::new(id, payload)).await?` | Invoke a function and wait for the result | | ||
| | Invoke (fire-and-forget) | `iii.trigger({ function_id, payload, action: TriggerAction.Void() })` | `iii.trigger({"function_id": id, "payload": data, "action": TriggerAction.Void()})` | `iii.trigger(TriggerRequest::new(id, payload).with_action(TriggerAction::Void)).await?` | Invoke without waiting | |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm whether Python `III.trigger` is synchronous and locate README guidance that still says to await it.
rg -n -C3 '^\s*(async\s+)?def\s+trigger\s*\(' --glob 'iii.py'
rg -n -C2 'await\s+iii\.trigger|Node/Python|TriggerAction\.Void\(\)' --glob 'README.md'Repository: iii-hq/iii
Length of output: 7795
Fix Python trigger() invocation and removed-method note in cross-SDK API table
Line 111 incorrectly shows await iii.trigger(...) for Python, but Python's trigger() is synchronous. Additionally, line 120's removed-method note uses JS object syntax for "Node/Python" when Python requires dict syntax.
Proposed fix
-| Invoke (await) | `await iii.trigger({ function_id, payload })` | `await iii.trigger({"function_id": id, "payload": data})` | `iii.trigger(TriggerRequest::new(id, payload)).await?` | Invoke a function and wait for the result |
+| Invoke (wait for result) | `await iii.trigger({ function_id, payload })` | `iii.trigger({"function_id": id, "payload": data})` | `iii.trigger(TriggerRequest::new(id, payload)).await?` | Invoke a function and wait for the result |
@@
-> `call`, `callVoid`, `triggerVoid` (and Python/Rust equivalents) have been removed. Use `trigger()` for all invocations. For fire-and-forget, use `trigger({ function_id, payload, action: TriggerAction.Void() })` (Node/Python) or `TriggerRequest::new(id, payload).with_action(TriggerAction::Void)` (Rust).
+> `call`, `callVoid`, `triggerVoid` (and Python/Rust equivalents) have been removed. Use `trigger()` for all invocations. For fire-and-forget, use `iii.trigger({ function_id, payload, action: TriggerAction.Void() })` in Node, `iii.trigger({"function_id": id, "payload": data, "action": TriggerAction.Void()})` in Python, or `TriggerRequest::new(id, payload).with_action(TriggerAction::Void)` in Rust.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sdk/README.md` around lines 111 - 112, Update the cross-SDK API table to
reflect that Python's trigger() is synchronous (remove the leading "await" in
the Python column for the Invoke rows) and change any "Node/Python" example that
currently uses JS object literal syntax to use Python dict syntax instead (e.g.,
use {"function_id": ..., "payload": ...} style for Python rather than JS object
notation); specifically edit the entries referencing iii.trigger(),
TriggerAction.Void(), and TriggerRequest examples so Python examples omit await
and use Python dicts where Node/Python is indicated.
11 tasks
Contributor
Author
|
Superseded by #1523 (branch renamed from |
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
1 participant
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.
Summary
iii.connect()in Python,register_trigger(3-arg)+TriggerRequest::new+ stale0.3version in Rust,?onregister_worker).TriggerRequest::new/with_action/with_timeout_msto the Rust SDK so the README (and your callers) can stop writing the four-field struct literal for the 80% case.NotConnectederror per SDK from "iii is not connected" to a message that names the URL, how to start the engine, and links to https://iii.dev/docs/install — the first real error new devs hit is now actionable.CHANGELOG.mdper SDK (Keep-a-Changelog) with packaging config so it actually ships from crates.io / npm / PyPI.examples/readme_hello.rsand (b) probesiii.<method>()calls in Python READMEs against the realiii.IIIclass — catching the exactiii.connect()shape of bug thatpy_compilemisses.This was produced after a
/devex-reviewaudit and/autoplanreview; the audit found four P0 doc lies plus surface-level ergonomic gaps. Dual-voice consensus also surfaced a bigger reframe (generate READMEs from tested source) — approved as follow-up; this PR seeds the Rust foothold viaexamples/readme_hello.rsso that future PRs can extend the pattern.Out of scope (deferred to follow-up): rewriting every error message, splitting 1–2k line SDK core files, full deprecation infrastructure, live playground / generated API reference.
Files touched
src/protocol.rs,src/error.rs,tests/ergonomics.rs(new),examples/readme_hello.rs(new),README.md,CHANGELOG.md(new)src/iii/iii.py,tests/test_init_api.py,README.md,pyproject.toml,CHANGELOG.md(new),uv.lock(version sync)src/iii.ts,README.md,package.json,CHANGELOG.md(new)sdk/README.md.github/workflows/sdk-readme.yml(new),scripts/readme-guard/check_python_readme.py(new),sdk/fixtures/readme-guard/(new)Test plan
cargo check -p iii-sdk --example readme_hello— compilescargo test -p iii-sdk --test ergonomics— 7/7 pass (constructors + error-message contract)cargo test -p iii-sdk --lib— 52/52 passcargo package --listinsdk/packages/rust/iii/includes CHANGELOG.md andexamples/readme_hello.rspytest tests/test_init_api.py— 4/4 pass (includes newtest_not_connected_error_is_actionable)python -m build --wheelproduces a wheel containingiii/CHANGELOG.mdChangelogURL renders on PyPI-style metadatapnpm --filter iii-sdk exec tsc --noEmit— cleanpnpm --filter iii-sdk build— cleannpm pack --dry-run— tarball includesdist/,README.md,CHANGELOG.md(41 files)iii.connect()), all three real READMEs exit 0mainif desired (branch was cut 1 commit behind; no conflicts expected since the intervening commit touchesinstall.shonly)continue-on-error: falseon the two jobs insdk-readme.ymlonce this lands greenVerification commands
Summary by CodeRabbit
Documentation
Bug Fixes