feat: add strategy-builder CLI subcommand#2544
Open
hardyjosh wants to merge 5 commits into2026-02-23-js-api-consolidationfrom
Open
feat: add strategy-builder CLI subcommand#2544hardyjosh wants to merge 5 commits into2026-02-23-js-api-consolidationfrom
hardyjosh wants to merge 5 commits into2026-02-23-js-api-consolidationfrom
Conversation
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a new Changes
Sequence DiagramsequenceDiagram
participant User as User/CLI
participant CLI as StrategyBuilder
participant HTTP as HTTP Client
participant Registry as Registry Server
participant OrderBuilder as RaindexOrderBuilder
participant Output as Stdout
User->>CLI: invoke with registry, strategy, deployment, owner, bindings
CLI->>HTTP: GET registry URL
HTTP->>Registry: request registry
Registry-->>HTTP: registry text
HTTP-->>CLI: registry data
CLI->>OrderBuilder: new_with_deployment(registry.settings?, deployment)
CLI->>OrderBuilder: set_field_value(...) for each binding
CLI->>OrderBuilder: set_select_token(...) async for each select
CLI->>OrderBuilder: set_deposit(...) async for each deposit
OrderBuilder-->>CLI: get_deployment_transaction_args(owner) -> (approvals, calldata, meta_call?)
CLI->>Output: print approvals
CLI->>Output: print address:calldata
CLI->>Output: print optional meta-call
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/cli/src/commands/strategy_builder.rs`:
- Around line 48-55: parse_key_value_pairs currently silently overwrites
duplicate keys; update it to detect duplicate keys and return an error instead
of replacing the previous entry. In the function parse_key_value_pairs(args:
&[String]) -> Result<HashMap<String, String>>, after splitting into key/value
check whether map.contains_key(&key.to_string()) (or lookups using &key) and if
present return an anyhow::anyhow! error indicating a duplicate override for that
key and the original arg; only insert when the key is new. Ensure the function
returns an Err on duplicates so callers cannot proceed with silent collisions.
- Around line 86-89: The parser currently uses line.split_once(' ') which fails
on tabs or multiple/mixed whitespace; change the logic that extracts key and
url_str from each line to use line.split_whitespace() (collect into fields),
validate that fields.len() == 2 and return the same anyhow::anyhow!("invalid
registry line (expected 'key url'): {line}') error if not, then set key =
fields[0] and url_str = fields[1] before calling
order_urls.insert(key.to_string(), url_str.trim().to_string()) so the code
accepts any whitespace delimiter.
- Around line 59-65: The fetch_text function currently calls reqwest::get
without a timeout and can block indefinitely; change it to build and reuse a
reqwest::Client with an explicit timeout (e.g., Duration::from_secs(10)) and
perform client.get(url.as_str()).send().await instead of reqwest::get, ensuring
you propagate errors as before; update fetch_text to accept or create that
client (or create a local client with Client::builder().timeout(...).build()?)
and use response.text().await? to return the body. Also add the necessary
std::time::Duration import and preserve the existing HTTP error handling around
response.status() and anyhow::bail! when non-success.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c75248ce-047a-4e27-b189-fdb73b5de209
📒 Files selected for processing (4)
crates/cli/Cargo.tomlcrates/cli/src/commands/mod.rscrates/cli/src/commands/strategy_builder.rscrates/cli/src/lib.rs
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
crates/cli/src/commands/strategy_builder.rs (1)
48-60:⚠️ Potential issue | 🟡 MinorReject empty
KEYvalues duringKEY=VALUEparsing.
=valuecurrently passes parsing and creates an empty key, which degrades CLI validation and error clarity. Validate non-empty trimmed keys at parse time and add a test for it.Suggested patch
fn parse_key_value_pairs(args: &[String]) -> Result<HashMap<String, String>> { let mut map = HashMap::new(); for arg in args { - let (key, value) = arg + let (raw_key, value) = arg .split_once('=') .ok_or_else(|| anyhow::anyhow!("expected KEY=VALUE, got: {arg}"))?; + let key = raw_key.trim(); + if key.is_empty() { + anyhow::bail!("expected non-empty KEY in KEY=VALUE, got: {arg}"); + } if map.contains_key(key) { anyhow::bail!("duplicate key: {key}"); } map.insert(key.to_string(), value.to_string()); } Ok(map) } @@ fn parse_key_value_pairs_duplicate_key_fails() { let args = vec!["key=first".to_string(), "key=second".to_string()]; let err = parse_key_value_pairs(&args).unwrap_err().to_string(); assert!(err.contains("duplicate key: key"), "got: {err}"); } + + #[test] + fn parse_key_value_pairs_empty_key_fails() { + let args = vec!["=value".to_string()]; + let err = parse_key_value_pairs(&args).unwrap_err().to_string(); + assert!(err.contains("expected non-empty KEY"), "got: {err}"); + } }Also applies to: 165-202
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/cli/src/commands/strategy_builder.rs` around lines 48 - 60, parse_key_value_pairs accepts inputs like "=value" producing empty keys; update the function (parse_key_value_pairs) to trim the key part, reject empty keys by returning an error (e.g., bail or anyhow::anyhow! with a clear message referencing the original arg), and keep the duplicate-key check and insertion behavior. Apply the same non-empty-trimmed-key validation to the other parsing occurrence in this file (the duplicate block around the later function), and add a unit test that asserts parsing "=value" (and keys with only whitespace) returns an error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@crates/cli/src/commands/strategy_builder.rs`:
- Around line 48-60: parse_key_value_pairs accepts inputs like "=value"
producing empty keys; update the function (parse_key_value_pairs) to trim the
key part, reject empty keys by returning an error (e.g., bail or anyhow::anyhow!
with a clear message referencing the original arg), and keep the duplicate-key
check and insertion behavior. Apply the same non-empty-trimmed-key validation to
the other parsing occurrence in this file (the duplicate block around the later
function), and add a unit test that asserts parsing "=value" (and keys with only
whitespace) returns an error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5ccb4e7f-ad28-409d-a17f-fce8982ea92f
📒 Files selected for processing (2)
crates/cli/Cargo.tomlcrates/cli/src/commands/strategy_builder.rs
hardyjosh
force-pushed
the
strategy-builder-cli
branch
from
ba92f20 to
9a5eb91
Compare
Contributor
Author
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
This was referenced Apr 12, 2026
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/cli/src/commands/strategy_builder.rs`:
- Around line 77-82: The error message builds its "available" list from
registry.get_order_keys() which relies on map iteration order and yields
non-deterministic text; change the code that assigns available (from
registry.get_order_keys().unwrap_or_default()) to collect into a Vec, sort it
(e.g., sort_unstable or sort()), and then use that sorted Vec in the anyhow!
message so the "Available: {:?}" output is deterministic; update the variable
named available near the anyhow! call in strategy_builder.rs 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 81056df2-7093-4391-9355-e31e7fee657e
📒 Files selected for processing (5)
crates/cli/Cargo.tomlcrates/cli/src/commands/mod.rscrates/cli/src/commands/strategy_builder.rscrates/cli/src/lib.rscrates/settings/src/remote/tokens.rs
Comment on lines
+77
to
+82
| let available = registry.get_order_keys().unwrap_or_default(); | ||
| anyhow::anyhow!( | ||
| "strategy '{}' not found in registry. Available: {:?}", | ||
| self.strategy, | ||
| available | ||
| ) |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Make “available strategies” output deterministic in not-found errors.
Line 77 currently uses map key iteration order, which can produce unstable error text. Sorting before formatting improves reproducibility (especially for tests and user support).
Proposed refinement
- let available = registry.get_order_keys().unwrap_or_default();
+ let mut available = registry.get_order_keys().unwrap_or_default();
+ available.sort();
anyhow::anyhow!(
"strategy '{}' not found in registry. Available: {:?}",
self.strategy,
available
)📝 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
| let available = registry.get_order_keys().unwrap_or_default(); | |
| anyhow::anyhow!( | |
| "strategy '{}' not found in registry. Available: {:?}", | |
| self.strategy, | |
| available | |
| ) | |
| let mut available = registry.get_order_keys().unwrap_or_default(); | |
| available.sort(); | |
| anyhow::anyhow!( | |
| "strategy '{}' not found in registry. Available: {:?}", | |
| self.strategy, | |
| available | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/cli/src/commands/strategy_builder.rs` around lines 77 - 82, The error
message builds its "available" list from registry.get_order_keys() which relies
on map iteration order and yields non-deterministic text; change the code that
assigns available (from registry.get_order_keys().unwrap_or_default()) to
collect into a Vec, sort it (e.g., sort_unstable or sort()), and then use that
sorted Vec in the anyhow! message so the "Available: {:?}" output is
deterministic; update the variable named available near the anyhow! call in
strategy_builder.rs accordingly.
4 tasks
Adds a new CLI subcommand that generates deployment calldata from a
remote registry strategy. Fetches the registry file, resolves the
strategy's dotrain + settings, creates a RaindexOrderBuilder, applies
field/token/deposit overrides from flags, and outputs `<to>:<calldata>`
lines to stdout for piping into a transaction submitter.
Usage:
raindex strategy-builder \
--registry <url> --strategy <name> --deployment <key> \
--owner <addr> \
--set-field binding=value \
--select-token slot=addr \
--set-deposit token=amount
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replace manual registry/settings/order fetching with DotrainRegistry from js_api, which already handles all of that. The CLI now depends on rain_orderbook_js_api (compiles natively since PR #2455) and uses DotrainRegistry::new(url) to fetch the registry, then extracts dotrain content and settings to create the RaindexOrderBuilder.
hardyjosh
force-pushed
the
strategy-builder-cli
branch
from
72e5a29 to
ea4b4dd
Compare
findolor
approved these changes
May 1, 2026
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
2 participants
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.
Motivation
The Raindex orderbook protocol has a rich GUI builder flow in the webapp for configuring and deploying strategies. There is currently no way to do the same from a terminal or from a non-interactive agent — the webapp is the only entry point. That forces anyone automating a deployment (CI, scripts, AI agents) to either drive the browser or hand-roll the calldata.
Solution
Add a
strategy-buildersubcommand to the raindex CLI that generates deployment calldata from a remote registry strategy.Outputs one
<address>:<calldata>line per transaction on stdout — approvals first, then the deployment multicall, then optional metaboard meta emission. Each line is one signable transaction.Implementation reuses
RaindexOrderBuilderfrom the common crate (same object the webapp drives) andDotrainRegistryfrom the js_api crate, so the CLI and webapp use identical resolution semantics.Follow-up PRs in this stack add
--interactive(#2546),--tokens(#2549), the template-fallback operator (#2551), and--describe(#2548).Checks
Summary by CodeRabbit
New Features
strategy-buildercommand that generates deployment calldata from registry strategies with configurable field bindings, token selections, and deposit amounts.Improvements