Conversation
Root README: - Add API surface table across all three SDKs - Fix Node.js init from new III() to init() - Fix all examples from call() to trigger() - Add deprecation note for call/callVoid/call_void Node.js: - Fix init from new III() to init() - Fix invocations from call/callVoid to trigger/triggerVoid - Add API table, subpath exports table - Add deprecation section Python: - Fix invocations from call/call_void to trigger/trigger_void - Add connection note (explicit await iii.connect() required) - Add API table and deprecation section Rust: - Fix invocations from call() to trigger() - Add API table, streams example, OTel feature docs - Add notes about register_function behavior - Add deprecation section
|
Important Review skippedThis PR was authored by the user configured for CodeRabbit reviews. By default, CodeRabbit skips reviewing PRs authored by this user. It's recommended to use a dedicated user account to post CodeRabbit review feedback. To trigger a single review, invoke the You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughDocumentation rebranded from "III SDK" to "iii SDK" and reorganized. READMEs now document init/connect and trigger/trigger_void usage (call/callVoid deprecated), introduce an API surface table and Resources, and standardize examples around a "greet" function across Node.js, Python, and Rust. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/python/iii/README.md`:
- Line 37: The example assigns the trigger response to result but never shows
it; update the Quick Start snippet around the iii.trigger("greet", {"name":
"world"}) call to print the response (e.g., call print(result)) so readers see
the returned value; locate the line using the result variable and the
iii.trigger call and add a print of result immediately after it.
- Around line 34-35: The Quick Start's config dict uses "api_path": "greet"
which is inconsistent with the HTTP Trigger Example's "api_path": "/todo";
update the Quick Start example to use a leading slash (e.g., "/greet") so the
api_path format is consistent across README examples, and scan other examples in
the file for the config={"api_path": ...} pattern to normalize all occurrences
to the chosen convention.
In `@packages/rust/iii/README.md`:
- Line 85: The README line incorrectly describes connect using
associated-function notation; update the wording to method notation to match how
it’s called (e.g., change the phrase "III::connect" to "iii.connect()" or
"III#connect / III::connect -> iii.connect()" so readers see it is an instance
method called like iii.connect().await?); ensure the text references the
instance call pattern (iii.connect().await?) rather than the static/associated
form (III::connect).
- Around line 67-72: The example snippet uses await but is not inside an async
main, so it won't compile; wrap the calls in an async context by adding a tokio
async entry (e.g., add #[tokio::main] async fn main() -> Result<(), Box<dyn
std::error::Error>> { ... }) and inside it create and connect the client (use
III::new(...) and call iii.connect().await? before calling iii.stream()), then
call stream.set(...).await? and stream.get(...).await? and return Ok(()) so the
snippet compiles and handles the Result.
In `@README.md`:
- Around line 45-47: The README's Python example defines async def main() and
calls iii.connect() and iii.trigger(...) but never invokes main(); add a final
call to run the coroutine (use asyncio.run(main())) and ensure asyncio is
imported at the top of the snippet so main() is actually executed; update the
snippet containing main(), iii.connect(), and iii.trigger(...) to include import
asyncio and asyncio.run(main()).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
README.mdpackages/node/iii/README.mdpackages/python/iii/README.mdpackages/rust/iii/README.md
- iii.stream() doesn't exist, use Streams::new(iii.clone()) - Streams helper has set_field/update/increment, not set/get
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/rust/iii/README.md (2)
67-72:⚠️ Potential issue | 🟡 MinorStreams snippet still lacks async context — previously flagged.
The snippet uses
.await?but is not wrapped in an async function with aResultreturn type. Without an async entry point (e.g.,#[tokio::main] async fn main() -> Result<...>), this code will not compile when copy-pasted.🔧 Suggested fix: wrap in async context
-```rust -use iii_sdk::Streams; - -let streams = Streams::new(iii.clone()); -streams.set_field("room::123", "users", json!(["alice", "bob"])).await?; -``` +```rust +use iii_sdk::{III, Streams}; +use serde_json::json; + +#[tokio::main] +async fn main() -> Result<(), Box<dyn std::error::Error>> { + let iii = III::new("ws://127.0.0.1:49134"); + iii.connect().await?; + + let streams = Streams::new(iii.clone()); + streams.set_field("room::123", "users", json!(["alice", "bob"])).await?; + Ok(()) +} +``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@packages/rust/iii/README.mdaround lines 67 - 72, The snippet calls .await?
on streams.set_field but isn't in an async context; wrap the example in an async
entry function (e.g., add #[tokio::main] async fn main() -> Result<(), Box>), construct/connect an III instance (use III::new(...) and
iii.connect().await?), import serde_json::json, then create Streams via
Streams::new(iii.clone()) and call streams.set_field(...).await? inside that
async main, returning Ok(()) at the end.</details> --- `85-85`: _⚠️ Potential issue_ | _🟡 Minor_ **`III::connect` uses wrong notation — previously flagged.** In Rust, `::` denotes an associated (static) function. Since `connect` is an instance method (called as `iii.connect().await?`), the documentation should use method notation to avoid confusion. The Quick Start example (line 29) and API table (line 55) correctly show the instance method form. <details> <summary>🔧 Suggested fix</summary> ```diff -- `III::connect` starts a background task and handles reconnection automatically +- `iii.connect()` starts a background task and handles reconnection automatically🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rust/iii/README.md` at line 85, The README incorrectly documents the instance method using associated-function notation "III::connect"; update all occurrences (e.g., the line saying "III::connect starts a background task...") to use method-instance notation consistent with the Quick Start and API table — e.g., refer to the method as iii.connect() (or III.connect when describing the method generically) and ensure any example shows the await call form (iii.connect().await?) and the API table entry similarly uses the instance-method form.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/rust/iii/README.md`:
- Around line 67-72: The snippet calls .await? on streams.set_field but isn't in
an async context; wrap the example in an async entry function (e.g., add
#[tokio::main] async fn main() -> Result<(), Box<dyn std::error::Error>>),
construct/connect an III instance (use III::new(...) and iii.connect().await?),
import serde_json::json, then create Streams via Streams::new(iii.clone()) and
call streams.set_field(...).await? inside that async main, returning Ok(()) at
the end.
- Line 85: The README incorrectly documents the instance method using
associated-function notation "III::connect"; update all occurrences (e.g., the
line saying "III::connect starts a background task...") to use method-instance
notation consistent with the Quick Start and API table — e.g., refer to the
method as iii.connect() (or III.connect when describing the method generically)
and ensure any example shows the await call form (iii.connect().await?) and the
API table entry similarly uses the instance-method form.
- Python: add print(result), fix api_path consistency (remove leading slash) - Rust: use connect() instance method notation in Notes section - Root: add import asyncio and asyncio.run(main()) to Python snippet
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/rust/iii/README.md (1)
67-72:⚠️ Potential issue | 🟡 MinorStreams snippet still isn’t copy-paste compilable.
await?requires async context, andjson!is used without importingserde_json::jsonin this snippet. Please wrap this example in an asyncmain(or clearly mark it as partial).Suggested doc fix
```rust -use iii_sdk::Streams; - -let streams = Streams::new(iii.clone()); -streams.set_field("room::123", "users", json!(["alice", "bob"])).await?; +use iii_sdk::{III, Streams}; +use serde_json::json; + +#[tokio::main] +async fn main() -> Result<(), Box<dyn std::error::Error>> { + let iii = III::new("ws://127.0.0.1:49134"); + iii.connect().await?; + + let streams = Streams::new(iii.clone()); + streams + .set_field("room::123", "users", json!(["alice", "bob"])) + .await?; + Ok(()) +}</details> ```shell #!/bin/bash # Verify Streams snippet still contains await outside an async wrapper in README context. rg -n -C3 '### Streams|await\?|use serde_json::json|tokio::main' packages/rust/iii/README.md🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rust/iii/README.md` around lines 67 - 72, The README example uses await? and json! outside an async context and without importing serde_json::json; fix by turning the snippet into a complete async main example: add the serde_json::json import, create and connect an III client (III::new and connect), annotate an async entry (e.g. #[tokio::main]) and move the Streams::new and streams.set_field(...).await? call inside that async function so the example is copy-paste compilable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/rust/iii/README.md`:
- Around line 67-72: The README example uses await? and json! outside an async
context and without importing serde_json::json; fix by turning the snippet into
a complete async main example: add the serde_json::json import, create and
connect an III client (III::new and connect), annotate an async entry (e.g.
#[tokio::main]) and move the Streams::new and streams.set_field(...).await? call
inside that async function so the example is copy-paste compilable.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/python/iii/README.md (1)
60-83:⚠️ Potential issue | 🟡 MinorMake the HTTP example runnable end-to-end.
This snippet defines
main()but never runs it, so copy-paste execution is incomplete. Addimport asyncioandasyncio.run(main())after the function.🔧 Suggested doc fix
from iii import III, ApiRequest, ApiResponse +import asyncio @@ async def main(): await iii.connect() @@ iii.register_trigger( type="http", function_id="api.post.todo", config={ "api_path": "todo", "http_method": "POST", "description": "Create a new todo" } ) + +asyncio.run(main())🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/python/iii/README.md` around lines 60 - 83, The example defines async main() but never runs it; to make the HTTP example runnable end-to-end, import asyncio at the top and invoke the coroutine with asyncio.run(main()) after the main() definition so the III client connects and the trigger is registered (refer to the III instance, the create_todo function, and the main() coroutine).
♻️ Duplicate comments (1)
packages/rust/iii/README.md (1)
67-72:⚠️ Potential issue | 🔴 CriticalStreams example still lacks async context and won't compile.
Despite the past review comment being marked as addressed, the streams snippet still contains
.await?without an async function wrapper. This will fail to compile when a developer copies it.🔧 Required fix to add async context
-```rust -use iii_sdk::Streams; - -let streams = Streams::new(iii.clone()); -streams.set_field("room::123", "users", json!(["alice", "bob"])).await?; -``` +```rust +use iii_sdk::{III, Streams}; +use serde_json::json; + +#[tokio::main] +async fn main() -> Result<(), Box<dyn std::error::Error>> { + let iii = III::new("ws://127.0.0.1:49134"); + iii.connect().await?; + + let streams = Streams::new(iii.clone()); + streams.set_field("room::123", "users", json!(["alice", "bob"])).await?; + + Ok(()) +} +``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@packages/rust/iii/README.mdaround lines 67 - 72, The snippet uses .await?
but lacks an async context; wrap the example in an async runtime (e.g., add
#[tokio::main] async fn main() -> Result<..., ...>), import serde_json::json,
construct an III client with III::new and call iii.connect().await? before
creating Streams::new(iii.clone()), then call streams.set_field(...).await? and
return Ok(()). Ensure imports include iii_sdk::{III, Streams} and
serde_json::json so the example compiles.</details> </blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (1)</summary><blockquote> <details> <summary>packages/rust/iii/README.md (1)</summary><blockquote> `87-87`: **Consider splitting the long sentence for readability.** The note about `unregister_trigger_type` is a single 30+ word sentence. Consider breaking it into two sentences for clarity. <details> <summary>♻️ Proposed refactor</summary> ```diff -- The engine protocol supports `registertriggertype` but does not include `unregistertriggertype`; `unregister_trigger_type` only removes local handlers +- The engine protocol supports `registertriggertype` but does not include `unregistertriggertype`. The `unregister_trigger_type` method only removes local handlers🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/rust/iii/README.md` at line 87, Split the long note into two clear sentences: state first that the engine protocol supports `registertriggertype` but does not include `unregistertriggertype`, and follow with a second sentence that `unregister_trigger_type` only removes local handlers; update the README reference to `registertriggertype`, `unregistertriggertype`, and `unregister_trigger_type` accordingly for readability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/python/iii/README.md`:
- Around line 60-83: The example defines async main() but never runs it; to make
the HTTP example runnable end-to-end, import asyncio at the top and invoke the
coroutine with asyncio.run(main()) after the main() definition so the III client
connects and the trigger is registered (refer to the III instance, the
create_todo function, and the main() coroutine).
---
Duplicate comments:
In `@packages/rust/iii/README.md`:
- Around line 67-72: The snippet uses .await? but lacks an async context; wrap
the example in an async runtime (e.g., add #[tokio::main] async fn main() ->
Result<..., ...>), import serde_json::json, construct an III client with
III::new and call iii.connect().await? before creating
Streams::new(iii.clone()), then call streams.set_field(...).await? and return
Ok(()). Ensure imports include iii_sdk::{III, Streams} and serde_json::json so
the example compiles.
---
Nitpick comments:
In `@packages/rust/iii/README.md`:
- Line 87: Split the long note into two clear sentences: state first that the
engine protocol supports `registertriggertype` but does not include
`unregistertriggertype`, and follow with a second sentence that
`unregister_trigger_type` only removes local handlers; update the README
reference to `registertriggertype`, `unregistertriggertype`, and
`unregister_trigger_type` accordingly for readability.
- Python/Rust API tables now match Node.js (add register_trigger_type) - Fix Python version requirement 3.8+ to 3.10+ - Fix Rust MSRV 1.70+ to 1.85+ - Fix Python ApiResponse example (status/data to status_code/body) - Add Registering Functions/Triggers/Custom Trigger Types/Invoking sections to Python and Rust (matching Node.js) - Rust Deprecated section now includes call_void()
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/rust/iii/README.md (1)
101-106:⚠️ Potential issue | 🟡 MinorStreams example still lacks async context.
The snippet uses
.await?on line 105 but is not wrapped in an async function. A developer copying this directly will encounter a compile error. For consistency with the Quick Start example (lines 22-48), consider showing the complete async context.🔧 Suggested fix
-```rust -use iii_sdk::Streams; - -let streams = Streams::new(iii.clone()); -streams.set_field("room::123", "users", json!(["alice", "bob"])).await?; -``` +```rust +use iii_sdk::{III, Streams}; +use serde_json::json; + +#[tokio::main] +async fn main() -> Result<(), Box<dyn std::error::Error>> { + let iii = III::new("ws://127.0.0.1:49134"); + iii.connect().await?; + + let streams = Streams::new(iii.clone()); + streams.set_field("room::123", "users", json!(["alice", "bob"])).await?; + + Ok(()) +} +``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@packages/rust/iii/README.mdaround lines 101 - 106, The Streams example is
missing an async runtime and necessary imports; update the snippet to show a
complete async context by importing III and serde_json::json, annotating an
async entry (e.g., #[tokio::main] async fn main()), creating and connecting an
III client (III::new(...) and iii.connect().await?), then construct
Streams::new(iii.clone()) and call streams.set_field(...).await? inside that
async main before returning Ok(()).</details> </blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (2)</summary><blockquote> <details> <summary>packages/rust/iii/README.md (1)</summary><blockquote> `85-89`: **Custom trigger type example references undefined `my_handler`.** Line 88 uses `my_handler` without defining it or explaining its expected type/signature. Consider either providing a complete example handler or adding a comment clarifying what the handler should be. <details> <summary>💡 Suggested improvement</summary> Option 1: Add a placeholder comment ```diff ```rust +// Assuming you have a handler function defined: +// async fn my_handler(config: Value, ctx: TriggerContext) -> Result<(), Error> { ... } iii.register_trigger_type("webhook", "External webhook trigger", my_handler);Option 2: Provide a minimal handler example ```diff ```rust +use iii_sdk::TriggerContext; + +async fn my_handler(config: Value, ctx: TriggerContext) -> Result<(), Box<dyn std::error::Error>> { + // Custom trigger logic here + Ok(()) +} + iii.register_trigger_type("webhook", "External webhook trigger", my_handler);</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@packages/rust/iii/README.mdaround lines 85 - 89, The example calls
iii.register_trigger_type("webhook", "External webhook trigger", my_handler) but
my_handler is undefined; add either a short placeholder comment describing the
expected handler signature or a minimal example handler implementation.
Specifically, show a function matching the expected signature (e.g., async fn
my_handler(config: Value, ctx: TriggerContext) -> Result<..., ...>) or add a
comment above the register_trigger_type call that documents the parameter types
and return type expected by my_handler and imports like TriggerContext/Value so
readers can copy the example directly.</details> </blockquote></details> <details> <summary>README.md (1)</summary><blockquote> `88-88`: **Consider clarifying language-specific deprecations.** The deprecation note uses `callVoid() / call_void()` and `triggerVoid() / trigger_void()`, which mixes JavaScript/Python naming conventions in a single line. While not incorrect, it may be clearer to specify which variant applies to which language, especially since the API Surface table above shows the differences explicitly. <details> <summary>💡 Optional improvement</summary> ```diff -> `call()` and `callVoid()` / `call_void()` exist as deprecated aliases. Use `trigger()` and `triggerVoid()` / `trigger_void()`. +> **Deprecated:** `call()`, `callVoid()` (Node.js), and `call_void()` (Python/Rust) are deprecated aliases for `trigger()`, `triggerVoid()` (Node.js), and `trigger_void()` (Python/Rust).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 88, Clarify the deprecation note by specifying language-specific names: state that in JavaScript/TypeScript the old aliases are call() and callVoid() (use trigger() and triggerVoid()), and in Python the old aliases are call() and call_void() (use trigger() and trigger_void()); update the sentence to separate the JS and Python variants and mirror the API Surface table so readers can immediately see which identifier applies to which language (refer to symbols call(), callVoid(), call_void(), trigger(), triggerVoid(), trigger_void()).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/rust/iii/README.md`:
- Around line 101-106: The Streams example is missing an async runtime and
necessary imports; update the snippet to show a complete async context by
importing III and serde_json::json, annotating an async entry (e.g.,
#[tokio::main] async fn main()), creating and connecting an III client
(III::new(...) and iii.connect().await?), then construct
Streams::new(iii.clone()) and call streams.set_field(...).await? inside that
async main before returning Ok(()).
---
Nitpick comments:
In `@packages/rust/iii/README.md`:
- Around line 85-89: The example calls iii.register_trigger_type("webhook",
"External webhook trigger", my_handler) but my_handler is undefined; add either
a short placeholder comment describing the expected handler signature or a
minimal example handler implementation. Specifically, show a function matching
the expected signature (e.g., async fn my_handler(config: Value, ctx:
TriggerContext) -> Result<..., ...>) or add a comment above the
register_trigger_type call that documents the parameter types and return type
expected by my_handler and imports like TriggerContext/Value so readers can copy
the example directly.
In `@README.md`:
- Line 88: Clarify the deprecation note by specifying language-specific names:
state that in JavaScript/TypeScript the old aliases are call() and callVoid()
(use trigger() and triggerVoid()), and in Python the old aliases are call() and
call_void() (use trigger() and trigger_void()); update the sentence to separate
the JS and Python variants and mirror the API Surface table so readers can
immediately see which identifier applies to which language (refer to symbols
call(), callVoid(), call_void(), trigger(), triggerVoid(), trigger_void()).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.mdpackages/python/iii/README.mdpackages/rust/iii/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/python/iii/README.md
There was a problem hiding this comment.
I pushed a commit to this PR so make sure to pull before making the below edits. Let me know if any questions.
- There's packages/python/README.md, which is a bit confusing, maybe incorporate it into your python readme?
- Remove the iii-example directories and instead link users to the Quickstart docs as the example. This will help maintainability for us while also encouraging users to start really using and learning about iii.
- API tables on node/python/rust READMEs should have the same first column as on the root README
- Function namespaces are using dot notation and not
:: - Remove "Custom Trigger Types" as it's too advanced for a README
-
api_paths should include the leading/ - Do Python or Rust have any modules yet like node does? Will need to add
- Main README should have descriptions for the API surface like the other READMEs do
- Examples vary for registering functions and triggers between the READMEs, they should all functionally do the same thing
|
Hey @anthonyiscoding, Yeah I have the same thought should / be everywhere? Like in website as well. |
|
hey @anthonyiscoding, All three READMEs already use dot notation for function namespaces ( |
- Add registerTrigger/register_trigger to root Hello World examples - Add Description column to root API table - Replace iii-example links with Quickstart docs link - Align SDK API tables to Operation | Signature | Description format - Remove Custom Trigger Types sections and register_trigger_type from API tables - Fix all api_path values to include leading / - Align function/trigger examples to orders.create across all SDKs - Add Modules section to Python and Rust READMEs - Merge packages/python/README.md dev commands into Python SDK README - Delete redundant packages/python/README.md
I misread, all is good here. |
anthonyiscoding
left a comment
anthonyiscoding
left a comment
There was a problem hiding this comment.
I reviewed the SDKs and made one minor fix.
The root README is about the iii-sdk but it should either be about just iii or about iii first and then link out and describe the other things within the monorepo so update that.
For the coderabbit comments you don't need to do ones that don't make sense just make sure you implement any that do.
- Add iii description before diving into SDK details - Add API descriptions after the comparison table - Link to per-SDK READMEs for language-specific details - Address Anthony's review feedback from iii-hq/sdk#28
- Add iii description before diving into SDK details - Add API descriptions after the comparison table - Link to per-SDK READMEs for language-specific details - Address Anthony's review feedback from iii-hq/sdk#28
2 participants
No description provided.