fix(sdk): README doc-lies, actionable errors, and a compile-checked Hello World#1523
Open
ytallo wants to merge 3 commits into
Open
fix(sdk): README doc-lies, actionable errors, and a compile-checked Hello World#1523ytallo wants to merge 3 commits into
ytallo wants to merge 3 commits into
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
13 tasks
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a GitHub Actions workflow and a Python readme-guard to validate README code samples; updates SDK READMEs, changelogs, examples, packaging metadata, error messages, tests, and adds Rust example/tests and Python readme fixtures for CI verification. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer / PR
participant GH as GitHub Actions (sdk-readme)
participant RustC as Rust Compiler
participant Py as Python runtime
participant Guard as check_python_readme.py
participant Repo as Repository (README/fixtures)
Dev->>GH: push PR triggers workflow
GH->>RustC: run Rust job (cargo check --example readme_hello)
RustC-->>GH: compile result (pass/fail)
GH->>Py: run Python job (setup py env)
Py->>Guard: execute guard against golden fixtures
Guard->>Repo: read README files & fixtures
Guard->>Py: import `iii.III` to probe attrs
Guard-->>Py: report violations (exit code)
GH-->>Dev: aggregate job statuses (warn-only)
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 unit tests (beta)
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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
sdk/packages/node/iii/README.md (1)
57-82:⚠️ Potential issue | 🟠 MajorFunction IDs use
.instead of the engine-standard::separator.
orders.createandanalytics.trackdon’t match the repo convention. Per coding guidelines, function IDs should use::(e.g.,orders::create,analytics::track). Since this README is the canonical Hello World many users copy-paste, it silently teaches the wrong convention and will drift from the other SDK snippets/docs.📝 Proposed fix
-iii.registerFunction('orders.create', async (input) => { +iii.registerFunction('orders::create', async (input) => { return { status_code: 201, body: { id: '123', item: input.body.item } } }) @@ iii.registerTrigger({ type: 'http', - function_id: 'orders.create', + function_id: 'orders::create', config: { api_path: '/orders', http_method: 'POST' }, }) @@ -const result = await iii.trigger({ function_id: 'orders.create', payload: { item: 'widget' } }) +const result = await iii.trigger({ function_id: 'orders::create', payload: { item: 'widget' } }) -iii.trigger({ function_id: 'analytics.track', payload: { event: 'page_view' }, action: TriggerAction.Void() }) +iii.trigger({ function_id: 'analytics::track', payload: { event: 'page_view' }, action: TriggerAction.Void() })As per coding guidelines: "Use
::separator for function IDs (e.g.,orders::validate,reports::daily-summary)".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/packages/node/iii/README.md` around lines 57 - 82, Update the README example to use the repo-standard "::" separator for function IDs: replace occurrences of 'orders.create' and 'analytics.track' with 'orders::create' and 'analytics::track' in the iii.registerFunction call, the iii.registerTrigger config (function_id), and both iii.trigger examples (including the TriggerAction.Void() example), ensuring all examples consistently use the "::" convention referenced in registerFunction, registerTrigger, and trigger.sdk/packages/python/iii/README.md (1)
64-84:⚠️ Potential issue | 🟡 MinorFunction IDs in examples use
.but engine standard is::.
orders.createappears in the three deeper examples. Engine convention (and the one used iniii.pydocstrings and other SDK docs) is::.Proposed fix
- return {"status_code": 201, "body": {"id": "123", "item": data["body"]["item"]}} - -iii.register_function("orders.create", create_order) + return {"status_code": 201, "body": {"id": "123", "item": data["body"]["item"]}} + +iii.register_function("orders::create", create_order) @@ iii.register_trigger({ "type": "http", - "function_id": "orders.create", + "function_id": "orders::create", "config": {"api_path": "/orders", "http_method": "POST"}, }) @@ -result = iii.trigger({"function_id": "orders.create", "payload": {"body": {"item": "widget"}}}) +result = iii.trigger({"function_id": "orders::create", "payload": {"body": {"item": "widget"}}}) @@ -iii.register_function({"id": "orders.create", "description": "Create a new order"}, create_order) +iii.register_function({"id": "orders::create", "description": "Create a new order"}, create_order)As per coding guidelines: "Use
::separator for function IDs (e.g.,orders::validate,reports::daily-summary)".🤖 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 64 - 84, Update the example function IDs to use the engine standard "::" separator instead of "."; specifically, in the create_order example change the id used in iii.register_function, the "function_id" value in the iii.register_trigger call, and the "function_id" in the iii.trigger invocation to use "orders::create" (references: create_order, iii.register_function, iii.register_trigger, iii.trigger).
🧹 Nitpick comments (6)
sdk/packages/rust/iii/src/error.rs (1)
8-14: RustNotConnectedlacks the failing URL that Node/Python include.Both
sdk/packages/node/iii/src/iii.ts:722andsdk/packages/python/iii/src/iii/iii.py:157interpolate the actual configured address (e.g.engine unreachable at ${this.address}). The Rust variant is a unit variant, so the message can only mention the defaultws://localhost:49134— when a user passed a different URL, the diagnostic silently misleads them.Consider promoting
NotConnectedtoNotConnected { address: String }(or{ address: String, max_retries: Option<u32> }) and interpolating it, so Rust parity with Node/Python is preserved.Note: this is a public API change on the error enum.
As per coding guidelines: "Make sure the shape of objects and arguments to functions and triggers are consistent within and between sdks (except for case)" and "Check for related inconsistencies with the other language sdks in sdk/".
🤖 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, The NotConnected error variant in the Error enum should include the actual address so diagnostics match Node/Python; change the unit variant NotConnected to a struct variant like NotConnected { address: String } (or NotConnected { address: String, max_retries: Option<u32> }) in sdk/packages/rust/iii/src/error.rs and update its #[error(...)] format string to interpolate {address} (and max_retries if added), then update all places that construct/return NotConnected to pass the configured address (and optional retries) to preserve public API intent and parity with other SDKs.sdk/packages/rust/iii/src/protocol.rs (2)
79-100: Struct-level doctest still showcases the verbose struct literal.Now that
TriggerRequest::new/.with_action/.with_timeout_msexist, the struct-level doctest on lines 82–100 (which pre-dated this PR) contradicts the guidance insdk/packages/rust/iii/CHANGELOG.mdthat treats the struct literal as an "Advanced escape hatch". Consider rewriting the example to lead with the builder and mention the struct literal only as an alternative.Proposed fix
/// ```rust -/// # use iii_sdk::protocol::{TriggerRequest, TriggerAction}; +/// # use iii_sdk::protocol::{TriggerRequest, TriggerAction}; /// # use serde_json::json; -/// // Simple call -/// TriggerRequest { -/// function_id: "my::function".to_string(), -/// payload: json!({ "key": "value" }), -/// action: None, -/// timeout_ms: None, -/// }; +/// // Preferred: builder +/// TriggerRequest::new("my::function", json!({ "key": "value" })); /// -/// // With action -/// TriggerRequest { -/// function_id: "my::function".to_string(), -/// payload: json!({}), -/// action: Some(TriggerAction::Enqueue { queue: "payments".to_string() }), -/// timeout_ms: None, -/// }; +/// // With action +/// TriggerRequest::new("my::function", json!({})) +/// .with_action(TriggerAction::Enqueue { queue: "payments".to_string() }); /// ```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/packages/rust/iii/src/protocol.rs` around lines 79 - 100, The struct-level doctest currently demonstrates building TriggerRequest via a verbose struct literal; update the example to use the builder helpers instead. Replace the literal examples with calls to TriggerRequest::new(...) for the simple case and chain .with_action(TriggerAction::Enqueue { queue: ... }) and/or .with_timeout_ms(...) for the variants, and keep a brief note that the struct literal remains an advanced escape hatch; update references to TriggerRequest, TriggerAction, TriggerRequest::new, with_action, and with_timeout_ms in the doctest accordingly.
333-343: Naming inconsistency:RegisterFunctionMessage::with_idvsTriggerRequest::new.Both constructors coexist in the same module but follow different naming conventions.
TriggerRequest::new()uses idiomatic Rust naming, whileRegisterFunctionMessage::with_id()uses a builder-style prefix. For consistency with the Rust convention (and the coding guideline to keep naming consistent within sdks), consider adding anew()alias or renamingwith_idto follow the same pattern.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/packages/rust/iii/src/protocol.rs` around lines 333 - 343, The constructor for RegisterFunctionMessage is named with_id, which is inconsistent with the idiomatic TriggerRequest::new naming; update RegisterFunctionMessage to provide a new() constructor (either add a new() that delegates to with_id or rename with_id to new) so both types use the same idiomatic factory name; modify the RegisterFunctionMessage impl to expose pub fn new(name: String) -> Self that creates the same struct as with_id (or replace all uses of with_id with new if renaming), and keep with_id as deprecated/removed accordingly to maintain API consistency with TriggerRequest::new..github/workflows/sdk-readme.yml (2)
7-25: Path filter omits Node SDK source but the guard validates the Node README.The
pathsfilter includessdk/packages/rust/iii/src/**andsdk/packages/python/iii/src/**, but notsdk/packages/node/iii/src/**. The final step validatessdk/packages/node/iii/README.mdwith the Python attribute-probe — which is fine because the guard only resolves againstiii.III(Python class) — but if you intend the guard to catch drift when Node SDK surface changes, you'd also need a Node-side probe. Otherwise, the current filter is consistent with scope: worth a brief comment here so future contributors don't wonder.🤖 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 7 - 25, The workflow's paths filter omits the Node SDK sources (sdk/packages/node/iii/src/**) but the guard validates sdk/packages/node/iii/README.md using the Python attribute-probe; either add 'sdk/packages/node/iii/src/**' to the paths list in .github/workflows/sdk-readme.yml so Node source changes trigger the job, or add a concise comment near the existing paths explaining that the guard intentionally only watches Python/Rust source and only validates the Node README via the Python probe; reference the paths block and the README validation of sdk/packages/node/iii/README.md when making the change.
84-89: Consider adding the rootsdk/README.mdNode snippets too.Running the Python guard against
sdk/packages/node/iii/README.mdis a bit of a category error — the guard parses ```python fences, so Node READMEs will simply have zero python blocks and silently pass. That's harmless but also provides no signal. Either drop the Node README from this list or add a matching Node-side guard in a follow-up.🤖 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 currently runs the Python README guard (scripts/readme-guard/check_python_readme.py) against three files including sdk/packages/node/iii/README.md which contains no Python fences; either remove sdk/packages/node/iii/README.md from the list passed to the "Run guard against real READMEs" step so the Python checker only runs against Python READMEs, or add a separate Node README guard step (e.g., a new check_node_readme.py or equivalent) and invoke it against sdk/packages/node/iii/README.md; update the workflow step(s) accordingly so each checker is only run against matching README files (reference: the "Run guard against real READMEs" step and the three README paths in the run block).sdk/packages/rust/iii/README.md (1)
76-77: Minor: fire-and-forget example doesn't need.await?on the user-visible path.For
TriggerAction::Void, showing.await?in the API table is technically fine (the call returns a future that resolves quickly) but may confuse readers who expect "fire-and-forget" to not block. Consider either keeping.await?with a brief note that it only awaits dispatch, or dropping it to emphasize non-blocking semantics — whichever matches the Node/Python README guidance for symmetry.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/packages/rust/iii/README.md` around lines 76 - 77, The fire-and-forget example currently shows awaiting the trigger future; update the Invoke (fire-and-forget) row so the user-visible example omits the `.await?` (i.e., show `iii.trigger(TriggerRequest::new(id, payload).with_action(TriggerAction::Void))`) to convey non-blocking semantics, and ensure the example uses the same convention as the Node/Python READMEs for symmetry; update any adjacent explanatory note if you prefer to keep a brief comment that the call returns a Future that can be awaited for dispatch confirmation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/readme-guard/check_python_readme.py`:
- Around line 37-105: The README guard silently misses invalid iii calls because
fence language detection and file handling are too permissive/fragile; update
extract_python_fences to only accept a whitelist of info values (e.g.,
{"python","py","python3"}) or split info tokens and match the first token
exactly instead of using info.startswith("python"), and add a defensive check in
check_file to raise SystemExit(2) if not path.is_file() before calling
path.read_text(); after changes, add a failing unit test exercising
check_file(path, ...) against sdk/fixtures/readme-guard/golden-bad-python.md to
ensure iii.connect() is detected (use functions extract_python_fences,
attr_calls_on_iii_var, iii_class_attrs, and check_file to locate behavior).
In `@sdk/packages/python/iii/CHANGELOG.md`:
- Around line 25-27: Update the Python changelog entry to use Python-style
snake_case names: replace Node-style `callVoid` and `triggerVoid` with
`call_void` and `trigger_void` so the Removed section reads that legacy methods
`call`, `call_void`, `trigger_void` were removed and users should use
`trigger()`; this aligns the Python entry with the Rust changelog which already
uses `call_void` and `trigger_void`.
In `@sdk/packages/rust/iii/examples/readme_hello.rs`:
- Line 41: The example currently calls the deprecated synchronous
iii.shutdown(); replace this call with the async variant by calling
iii.shutdown_async().await to ensure telemetry is flushed before process exit;
update the mirrored README example as well so the canonical example (the code
snippet containing iii.shutdown()/shutdown_async) uses shutdown_async().await
instead of shutdown().
In `@sdk/packages/rust/iii/README.md`:
- Line 56: The README and example use the synchronous call iii.shutdown(); in an
async context—replace calls to iii.shutdown() with the async variant
iii.shutdown_async().await so the shutdown runs in the async runtime and
telemetry is flushed; update the snippet in README.md and the matching example
file (examples/readme_hello.rs) to call shutdown_async().await in the same async
scope where other .await? calls occur.
---
Outside diff comments:
In `@sdk/packages/node/iii/README.md`:
- Around line 57-82: Update the README example to use the repo-standard "::"
separator for function IDs: replace occurrences of 'orders.create' and
'analytics.track' with 'orders::create' and 'analytics::track' in the
iii.registerFunction call, the iii.registerTrigger config (function_id), and
both iii.trigger examples (including the TriggerAction.Void() example), ensuring
all examples consistently use the "::" convention referenced in
registerFunction, registerTrigger, and trigger.
In `@sdk/packages/python/iii/README.md`:
- Around line 64-84: Update the example function IDs to use the engine standard
"::" separator instead of "."; specifically, in the create_order example change
the id used in iii.register_function, the "function_id" value in the
iii.register_trigger call, and the "function_id" in the iii.trigger invocation
to use "orders::create" (references: create_order, iii.register_function,
iii.register_trigger, iii.trigger).
---
Nitpick comments:
In @.github/workflows/sdk-readme.yml:
- Around line 7-25: The workflow's paths filter omits the Node SDK sources
(sdk/packages/node/iii/src/**) but the guard validates
sdk/packages/node/iii/README.md using the Python attribute-probe; either add
'sdk/packages/node/iii/src/**' to the paths list in
.github/workflows/sdk-readme.yml so Node source changes trigger the job, or add
a concise comment near the existing paths explaining that the guard
intentionally only watches Python/Rust source and only validates the Node README
via the Python probe; reference the paths block and the README validation of
sdk/packages/node/iii/README.md when making the change.
- Around line 84-89: The workflow currently runs the Python README guard
(scripts/readme-guard/check_python_readme.py) against three files including
sdk/packages/node/iii/README.md which contains no Python fences; either remove
sdk/packages/node/iii/README.md from the list passed to the "Run guard against
real READMEs" step so the Python checker only runs against Python READMEs, or
add a separate Node README guard step (e.g., a new check_node_readme.py or
equivalent) and invoke it against sdk/packages/node/iii/README.md; update the
workflow step(s) accordingly so each checker is only run against matching README
files (reference: the "Run guard against real READMEs" step and the three README
paths in the run block).
In `@sdk/packages/rust/iii/README.md`:
- Around line 76-77: The fire-and-forget example currently shows awaiting the
trigger future; update the Invoke (fire-and-forget) row so the user-visible
example omits the `.await?` (i.e., show `iii.trigger(TriggerRequest::new(id,
payload).with_action(TriggerAction::Void))`) to convey non-blocking semantics,
and ensure the example uses the same convention as the Node/Python READMEs for
symmetry; update any adjacent explanatory note if you prefer to keep a brief
comment that the call returns a Future that can be awaited for dispatch
confirmation.
In `@sdk/packages/rust/iii/src/error.rs`:
- Around line 8-14: The NotConnected error variant in the Error enum should
include the actual address so diagnostics match Node/Python; change the unit
variant NotConnected to a struct variant like NotConnected { address: String }
(or NotConnected { address: String, max_retries: Option<u32> }) in
sdk/packages/rust/iii/src/error.rs and update its #[error(...)] format string to
interpolate {address} (and max_retries if added), then update all places that
construct/return NotConnected to pass the configured address (and optional
retries) to preserve public API intent and parity with other SDKs.
In `@sdk/packages/rust/iii/src/protocol.rs`:
- Around line 79-100: The struct-level doctest currently demonstrates building
TriggerRequest via a verbose struct literal; update the example to use the
builder helpers instead. Replace the literal examples with calls to
TriggerRequest::new(...) for the simple case and chain
.with_action(TriggerAction::Enqueue { queue: ... }) and/or .with_timeout_ms(...)
for the variants, and keep a brief note that the struct literal remains an
advanced escape hatch; update references to TriggerRequest, TriggerAction,
TriggerRequest::new, with_action, and with_timeout_ms in the doctest
accordingly.
- Around line 333-343: The constructor for RegisterFunctionMessage is named
with_id, which is inconsistent with the idiomatic TriggerRequest::new naming;
update RegisterFunctionMessage to provide a new() constructor (either add a
new() that delegates to with_id or rename with_id to new) so both types use the
same idiomatic factory name; modify the RegisterFunctionMessage impl to expose
pub fn new(name: String) -> Self that creates the same struct as with_id (or
replace all uses of with_id with new if renaming), and keep with_id as
deprecated/removed accordingly to maintain API consistency with
TriggerRequest::new.
🪄 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: 3969ad5c-b542-4c8e-a670-6d479965928a
⛔ 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
+25
to
+27
| ### Removed | ||
| - Removed legacy `call`, `callVoid`, `triggerVoid` methods. Use `trigger()` | ||
| for all invocations: |
Contributor
There was a problem hiding this comment.
Python changelog uses Node-style camelCase method names.
The removed Python methods would be call, call_void, trigger_void (snake_case), not callVoid / triggerVoid. Compare with the Rust changelog which correctly lists call_void, trigger_void.
Proposed fix
-- Removed legacy `call`, `callVoid`, `triggerVoid` methods. Use `trigger()`
+- Removed legacy `call`, `call_void`, `trigger_void` methods. Use `trigger()`As per coding guidelines: "Make sure naming is consistent within and between sdks (except for case)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sdk/packages/python/iii/CHANGELOG.md` around lines 25 - 27, Update the Python
changelog entry to use Python-style snake_case names: replace Node-style
`callVoid` and `triggerVoid` with `call_void` and `trigger_void` so the Removed
section reads that legacy methods `call`, `call_void`, `trigger_void` were
removed and users should use `trigger()`; this aligns the Python entry with the
Rust changelog which already uses `call_void` and `trigger_void`.
| .await?; | ||
|
|
||
| println!("result: {result}"); | ||
| iii.shutdown(); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP -C2 '\bfn\s+shutdown(_async)?\b' sdk/packages/rust/iii/srcRepository: iii-hq/iii
Length of output: 2531
🏁 Script executed:
#!/bin/bash
# Check the current state of the example file
echo "=== Checking readme_hello.rs line 41 ==="
sed -n '35,45p' sdk/packages/rust/iii/examples/readme_hello.rs
echo ""
echo "=== Checking README.md around line 56 ==="
sed -n '50,65p' sdk/packages/rust/iii/README.mdRepository: iii-hq/iii
Length of output: 912
Use shutdown_async().await in the canonical CI-compiled example.
This file is the source-of-truth mirrored into the README, so using the deprecated shutdown() propagates the anti-pattern into every SDK user who copies the Hello World. shutdown_async().await is required to guarantee telemetry flush before exit.
♻️ Proposed change
println!("result: {result}");
- iii.shutdown();
+ iii.shutdown_async().await;
Ok(())Mirror the same change in sdk/packages/rust/iii/README.md line 56.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@sdk/packages/rust/iii/examples/readme_hello.rs` at line 41, The example
currently calls the deprecated synchronous iii.shutdown(); replace this call
with the async variant by calling iii.shutdown_async().await to ensure telemetry
is flushed before process exit; update the mirrored README example as well so
the canonical example (the code snippet containing
iii.shutdown()/shutdown_async) uses shutdown_async().await instead of
shutdown().
| .await?; | ||
|
|
||
| println!("result: {result}"); | ||
| iii.shutdown(); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm shutdown_async exists on III and whether shutdown is marked deprecated.
rg -nP -C2 '\bfn\s+shutdown(_async)?\s*\(' sdk/packages/rust/iii/src
rg -nP -C1 '#\[deprecated' sdk/packages/rust/iii/srcRepository: iii-hq/iii
Length of output: 2531
🏁 Script executed:
#!/bin/bash
# Get full context for shutdown() and shutdown_async() methods in iii.rs
echo "=== shutdown() method documentation and signature ==="
sed -n '815,835p' sdk/packages/rust/iii/src/iii.rs
echo ""
echo "=== shutdown_async() method documentation and signature ==="
sed -n '836,860p' sdk/packages/rust/iii/src/iii.rs
echo ""
echo "=== Current state of README.md around line 56 ==="
sed -n '50,65p' sdk/packages/rust/iii/README.md
echo ""
echo "=== Checking examples/readme_hello.rs for shutdown usage ==="
if [ -f "sdk/packages/rust/iii/examples/readme_hello.rs" ]; then
grep -n "shutdown" sdk/packages/rust/iii/examples/readme_hello.rs
else
echo "File not found"
fiRepository: iii-hq/iii
Length of output: 2656
Use shutdown_async().await instead of shutdown() in the canonical README example and examples file.
The README example is in an async context (as shown by the .await? calls); for async code, shutdown_async().await is the idiomatic choice. This aligns with SDK guidance to use shutdown_async().await for guaranteed telemetry flush before process exit, and both this README snippet and examples/readme_hello.rs should be updated consistently since they are kept in sync via CI.
Proposed changes
In sdk/packages/rust/iii/README.md line 56:
- iii.shutdown();
+ iii.shutdown_async().await;In sdk/packages/rust/iii/examples/readme_hello.rs line 41:
- iii.shutdown();
+ iii.shutdown_async().await;📝 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
| iii.shutdown(); | |
| iii.shutdown_async().await; |
🤖 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 56, The README and example use the
synchronous call iii.shutdown(); in an async context—replace calls to
iii.shutdown() with the async variant iii.shutdown_async().await so the shutdown
runs in the async runtime and telemetry is flushed; update the snippet in
README.md and the matching example file (examples/readme_hello.rs) to call
shutdown_async().await in the same async scope where other .await? calls occur.
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.
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, 3-argregister_trigger+TriggerRequest::new+ stale0.3version in Rust,?onregister_worker).TriggerRequest::new/with_action/with_timeout_msto the Rust SDK so the README (and 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.CHANGELOG.mdper SDK (Keep-a-Changelog) with packaging config so it 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()bug-shape thatpy_compilemisses.Produced from a
/devex-reviewaudit +/autoplandual-voice review. Dual-voice consensus also surfaced a bigger reframe (generate READMEs from tested source) — approved as a follow-up; this PR seeds the Rust foothold viaexamples/readme_hello.rsso future PRs can extend the pattern.Supersedes #1522 (closed due to branch rename).
Out of scope: 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 fmt --all -- --checkcleancargo check -p iii-sdk --example readme_hellocompilescargo test -p iii-sdk --test ergonomics— 7/7 passcargo test -p iii-sdk --lib— 52/52 passcargo package --listincludesCHANGELOG.md+examples/readme_hello.rspytest tests/test_init_api.py— 4/4 passpython -m build --wheelproduces wheel containingiii/CHANGELOG.mdpnpm --filter iii-sdk exec tsc --noEmitcleannpm pack --dry-runshipsdist/,README.md,CHANGELOG.md(41 files)iii.connect()), real READMEs exit 0continue-on-error: falseon both jobs insdk-readme.ymlonce this lands greenVerification commands
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Chores