Consolidate JS API gui module into common crate and rename GUI to OrderBuilder#2479
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces the GUI-focused DotrainOrderGui surface with a RaindexOrderBuilder surface across Rust core, settings/YAML (root key ChangesSingle coordinated migration: GUI → RaindexOrderBuilder
Sequence Diagram(s)sequenceDiagram
participant JS as JS_Client
participant WASM as RaindexOrderBuilder (WASM)
participant INNER as RaindexOrderBuilderInner
participant RPC as Network/RPC
JS->>WASM: newWithDeployment(dotrain, settings, deploymentKey, callback)
WASM->>INNER: new_with_deployment(...), parse YAML, select deployment
INNER->>INNER: build OrderBuilderCfg, compute dotrain_hash
INNER-->>WASM: return builder instance
JS->>WASM: setSelectToken(key, address)
WASM->>INNER: inner.set_select_token(key, address)
INNER->>RPC: fetch ERC20 decimals/name/symbol
RPC-->>INNER: token metadata
INNER->>INNER: persist token record into dotrain YAML
INNER-->>WASM: Ok -> WASM executes state-update-callback -> JS callback invoked
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![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: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (17)
packages/ui-components/src/lib/services/registry.ts (1)
83-94:⚠️ Potential issue | 🟠 MajorCorrect the error property accessor from
.msgto.readableMsg.Line 86 uses
result.error.msgbut the@rainlanguage/orderbookAPI documentation (and other parts of the codebase) consistently useresult.error.readableMsg. The Rust bindings exportto_readable_msg()which becomesreadableMsgin JavaScript. Other UI components correctly accessresult.error.readableMsg(e.g., VaultsListTable.svelte, OrderDetail.svelte); registry.ts and a few other files use the incorrect.msgproperty, which will fail at runtime.Inconsistent usage in codebase
Files using incorrect
.msg:
- packages/ui-components/src/lib/services/registry.ts:86
- packages/ui-components/src/lib/services/handleShareChoices.ts:12
- packages/ui-components/src/lib/components/deployment/TokenIOInput.svelte:24, 40
Files using correct
.readableMsg:
- packages/ui-components/src/lib/providers/transactions/TransactionManager.ts:59
- packages/ui-components/src/lib/components/tables/VaultsListTable.svelte:76, 125, 176, 191
- packages/ui-components/src/lib/components/detail/OrderDetail.svelte:90
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui-components/src/lib/services/registry.ts` around lines 83 - 94, The code in RaindexOrderBuilder.getOrderDetails handling incorrectly accesses result.error.msg; update the error property access to result.error.readableMsg in the failure branch where you throw (inside the function returning { valid: true, data: ... } path) so the thrown Error uses the exported readableMsg; locate the usage in registry.ts around the variable registryDotrain and replace `.msg` with `.readableMsg` (and mirror the same fix in any other files listed like handleShareChoices.ts and TokenIOInput.svelte if present).packages/ui-components/src/lib/components/deployment/ComposedRainlangModal.svelte (1)
1-20:⚠️ Potential issue | 🟠 MajorProvide the required webapp screenshot for this UI change.
Frontend modifications in ui-components require a screenshot of the built webapp; please attach one and include the build/preview steps used.
As per coding guidelines: "packages/{webapp,ui-components}/**/*.{ts,tsx,svelte,js,jsx}: If you modify frontend code or functionality affecting the frontend, you MUST provide a screenshot of the built webapp reflecting your change".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui-components/src/lib/components/deployment/ComposedRainlangModal.svelte` around lines 1 - 20, The PR modifies the UI in ComposedRainlangModal.svelte (notably the generateRainlang function and UI around rainlangText/open) but lacks the required built-app screenshot and preview instructions; please attach a screenshot showing the modal/UI state after building the webapp and include concise build/preview steps (commands used, environment, branch/commit, and URL or local port used) so reviewers can reproduce the UI (e.g., build/install, run dev/preview, navigate to the component and invoke generateRainlang to show the modal).packages/ui-components/src/lib/components/deployment/TokenSelectionModal.svelte (1)
5-22:⚠️ Potential issue | 🟠 MajorProvide an updated webapp screenshot for this UI change.
This swap affects the frontend token source; please attach a screenshot from the built webapp and confirm build/preview.As per coding guidelines:
packages/{webapp,ui-components}/**/*.{ts,tsx,svelte,js,jsx}: If you modify frontend code or functionality affecting the frontend, you MUST provide a screenshot of the built webapp reflecting your change; Frontend verification requires building and previewing the webapp withnix develop -c npm run build -w@rainlanguage/webappfollowed by `nix develop -c npm run preview -w `@rainlanguage/webapp.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui-components/src/lib/components/deployment/TokenSelectionModal.svelte` around lines 5 - 22, The reviewer requires a built-webapp screenshot showing the TokenSelectionModal change: rebuild and preview the webapp and attach a screenshot showing the updated token source UI (TokenSelectionModal.svelte / loadTokens -> builder.getAllTokens). Run the prescribed commands to verify the frontend: `nix develop -c npm run build -w `@rainlanguage/webapp`` then `nix develop -c npm run preview -w `@rainlanguage/webapp``, open the page where TokenSelectionModal is used, capture a clear screenshot of the modal/search results, and add that image to the PR with a short confirmation comment stating the build and preview commands succeeded.packages/ui-components/src/lib/components/deployment/SelectToken.svelte (1)
26-40:⚠️ Potential issue | 🟡 MinorSilent NPE when
result.valueis null on mountWhen
getTokenInforeturns{ value: null }(no token configured yet), line 32 setstokenInfo = nulland line 33 immediately throwsTypeError: Cannot read properties of nullwhen accessingresult.value.address. The outercatch {}silently swallows it, so the UI is unaffected, but the exception is unintended. The fix is optional chaining.🐛 Proposed fix
tokenInfo = result.value; - if (result.value.address) { + if (result.value?.address) { inputValue = result.value.address; onSelectTokenSelect(token.key); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui-components/src/lib/components/deployment/SelectToken.svelte` around lines 26 - 40, The onMount block may throw a TypeError when builder.getTokenInfo returns { value: null }; update the logic in the onMount async handler (the function that calls builder.getTokenInfo) to guard access to result.value before reading properties: check result.value exists (or use optional chaining) before assigning tokenInfo, reading result.value.address, setting inputValue, or calling onSelectTokenSelect(token.key), and preserve the existing error handling behavior.crates/settings/src/yaml/emitter.rs (1)
14-33: 🧹 Nitpick | 🔵 TrivialBreaking change: existing
gui:YAML documents will be silently dropped on emitThe rename of the canonical key from
"gui"to"builder"means any existing deployment YAML that contains agui:top-level section will be silently removed whenemit_documentsruns (keys not inCANONICAL_ROOT_KEYSare filtered out). Additionally,OrderBuilderCfg::parse_from_yaml_optionalwill returnNoneinstead of an error for oldgui:files, so there is no failure signal to users.If this is an upgrade path for existing users, consider adding a guard in
validate_and_emit_documentsthat detects agui:key and either migrates it tobuilder:or emits a clear diagnostic error, to avoid silent data loss during the transition.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/settings/src/yaml/emitter.rs` around lines 14 - 33, The CANONICAL_ROOT_KEYS constant was changed to "builder" which causes top-level "gui" keys to be filtered out in emit_documents and silent for users because OrderBuilderCfg::parse_from_yaml_optional returns None; update validate_and_emit_documents to detect a legacy "gui" top-level key (or detect it in emit_documents) and either migrate it to "builder" before filtering or emit a clear diagnostic error instead of silently dropping it, and ensure OrderBuilderCfg::parse_from_yaml_optional surfaces an error or warning when a legacy "gui" document is encountered so users are notified of the required change.packages/ui-components/src/__tests__/ComposedRainlangModal.test.ts (1)
1-37:⚠️ Potential issue | 🟡 MinorAdd passthrough mock for
@rainlanguage/orderbookConsistent with other component tests in this codebase (e.g.,
VaultIdInformation.test.ts,SelectToken.test.ts), this test requires a passthroughvi.mock('@rainlanguage/orderbook')block. The project's test configuration requires this mock even when no exports are overridden, because the hook dependency indirectly loads types from that package.Add the following after the
svelte-codemirror-editormock:vi.mock('@rainlanguage/orderbook', async (importOriginal) => { return { ...(await importOriginal()) }; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui-components/src/__tests__/ComposedRainlangModal.test.ts` around lines 1 - 37, Test fails because the package '@rainlanguage/orderbook' needs a passthrough mock; add a vi.mock for '@rainlanguage/orderbook' immediately after the svelte-codemirror-editor mock that calls importOriginal and returns a spread of await importOriginal() so the original exports are preserved (place this mock near the other vi.mock calls in ComposedRainlangModal.test.ts alongside useRaindexOrderBuilder and mockBuilder setup).packages/webapp/src/lib/services/handleAddOrder.ts (1)
1-14: 🧹 Nitpick | 🔵 TrivialPlease confirm required webapp verification steps (and attach screenshot).
This change touches webapp frontend code. Please provide:
nix develop -c npm run build -w@rainlanguage/webapp``nix develop -c npm run svelte-lint-format-check -w@rainlanguage/webapp``nix develop -c npm run test -w@rainlanguage/webapp``nix develop -c npm run format,nix develop -c npm run lint,nix develop -c npm run check(for TS/Svelte)- Webapp build + preview and a screenshot of the built webapp reflecting the change.
As per coding guidelines: “packages/webapp//*.{ts,tsx,svelte,js,jsx}: … run
nix develop -c npm run build -w@rainlanguage/webapp, … `nix develop -c npm run svelte-lint-format-check -w `@rainlanguage/webapp, …nix develop -c npm run test -w@rainlanguage/webapp``”; “/.{ts,tsx,svelte}: Format TypeScript/Svelte code withnix develop -c npm run format… Lint … Run type checks …”; and “packages/{webapp,ui-components}/**/.{ts,tsx,svelte,js,jsx}: … MUST provide a screenshot … build … preview …”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webapp/src/lib/services/handleAddOrder.ts` around lines 1 - 14, This PR touches frontend file handleAddOrder.ts (imports like RaindexOrderBuilder, RaindexClient, QKEY_ORDERS), so run the required webapp verification: execute nix develop -c npm run build -w `@rainlanguage/webapp`, nix develop -c npm run svelte-lint-format-check -w `@rainlanguage/webapp`, and nix develop -c npm run test -w `@rainlanguage/webapp`; run formatting, linting and type checks via nix develop -c npm run format, nix develop -c npm run lint, and nix develop -c npm run check; then start a preview of the built webapp and attach a screenshot that clearly shows the changed behavior/UI (include timestamp or browser URL in the shot) to the PR comments so reviewers can confirm the frontend change.packages/orderbook/test/js_api/dotrainRegistry.test.ts (1)
1-6: 🧹 Nitpick | 🔵 TrivialPlease confirm orderbook package build/lint/test + TS format/check steps.
For changes in
packages/orderbook, please provide:
nix develop -c npm run build:orderbooknix develop -c npm run check -w@rainlanguage/orderbook``nix develop -c npm run test -w@rainlanguage/orderbook``nix develop -c npm run format,nix develop -c npm run lint,nix develop -c npm run check(for TS/Svelte)As per coding guidelines: “packages/orderbook//*.{ts,tsx,js,jsx}: … run
nix develop -c npm run build:orderbook…nix develop -c npm run check -w@rainlanguage/orderbook… `nix develop -c npm run test -w `@rainlanguage/orderbook”; and “/*.{ts,tsx,svelte}: Format TypeScript/Svelte code withnix develop -c npm run format… Lint … Run type checks …”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/orderbook/test/js_api/dotrainRegistry.test.ts` around lines 1 - 6, This change touches the orderbook package tests — ensure the package builds, type-checks, formats and tests by running the required nix/npm commands: execute `nix develop -c npm run build:orderbook` to build, `nix develop -c npm run check -w `@rainlanguage/orderbook`` to run TypeScript checks for the package (covers symbols like OrderbookYaml and DotrainRegistry used in dotrainRegistry.test.ts), and `nix develop -c npm run test -w `@rainlanguage/orderbook`` to run the package tests; also run project-wide formatting, linting and checks with `nix develop -c npm run format`, `nix develop -c npm run lint`, and `nix develop -c npm run check` to ensure TS/Svelte formatting and lint rules are satisfied before merging.packages/ui-components/src/lib/components/deployment/FieldDefinitionInput.svelte (1)
1-55: 🧹 Nitpick | 🔵 TrivialPlease confirm ui-components build/lint/test + TS/Svelte format/check.
Suggested commands:
nix develop -c npm run build -w@rainlanguage/ui-components``nix develop -c npm run svelte-lint-format-check -w@rainlanguage/ui-components``nix develop -c npm run test -w@rainlanguage/ui-components``nix develop -c npm run formatnix develop -c npm run lintnix develop -c npm run checkAs per coding guidelines, “packages/ui-components//*.{ts,tsx,svelte,js,jsx}: … run build … lint … test …” and “/*.{ts,tsx,svelte}: Format … Lint … Run type checks …”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui-components/src/lib/components/deployment/FieldDefinitionInput.svelte` around lines 1 - 55, Run the full UI components verification for packages/ui-components to ensure build, lint, format and type checks pass for this change in FieldDefinitionInput.svelte: run the build and tests (e.g., npm run build -w `@rainlanguage/ui-components` and npm run test -w `@rainlanguage/ui-components`), run Svelte/TS formatting and linting (e.g., npm run svelte-lint-format-check -w `@rainlanguage/ui-components`, npm run format, npm run lint) and run type checks (npm run check) before merging; if any errors reference FieldDefinitionInput.svelte (including onMount, handlePresetClick, handleCustomInputChange or useRaindexOrderBuilder usage), fix formatting/type/lint issues and re-run the commands until all checks succeed.packages/webapp/src/routes/deploy/[orderName]/[deploymentKey]/+page.svelte (1)
69-88:⚠️ Potential issue | 🟠 MajorProvide a screenshot of the built webapp reflecting this change.
Frontend verification is mandatory for Svelte component changes. Build, preview, and run the required checks:
nix develop -c npm run build -w@rainlanguage/webapp``nix develop -c npm run preview -w@rainlanguage/webapp``nix develop -c npm run svelte-lint-format-check -w@rainlanguage/webapp``nix develop -c npm run test -w@rainlanguage/webapp``🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/webapp/src/routes/deploy/`[orderName]/[deploymentKey]/+page.svelte around lines 69 - 88, The PR changes the conditional rendering in +page.svelte for RaindexOrderBuilderProvider/DeploymentSteps; provide a frontend verification screenshot of the built webapp showing the builder view (when builder is truthy) and include the image in the PR, and run the full Svelte build/preview/lint/test checks: execute `nix develop -c npm run build -w `@rainlanguage/webapp``, then `nix develop -c npm run preview -w `@rainlanguage/webapp`` to capture the running UI screenshot for the route /deploy/[orderName]/[deploymentKey], and finally run `nix develop -c npm run svelte-lint-format-check -w `@rainlanguage/webapp`` and `nix develop -c npm run test -w `@rainlanguage/webapp`` to show lint/format/tests pass; attach the screenshot and CI output to the PR for review.packages/ui-components/src/__tests__/DeploymentSteps.test.ts (1)
107-114:⚠️ Potential issue | 🟡 MinorAlign
getCurrentDeploymentmock with the builder result shape.
Most mocked builder methods return{ value }, butgetCurrentDeploymentreturnsmockDeploymentdirectly. The actual builder API returnsWasmEncodedResult<OrderBuilderDeploymentCfg>with shape{ value, error }. While the first test doesn't exercise this mock, later tests override it correctly with the wrapped format. For consistency across the file and alignment with the API contract, wrap the initial mock:🔧 Proposed fix
- getCurrentDeployment: vi.fn().mockReturnValue(mockDeployment), + getCurrentDeployment: vi.fn().mockReturnValue({ value: mockDeployment }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ui-components/src/__tests__/DeploymentSteps.test.ts` around lines 107 - 114, The mock for builderInstance.getCurrentDeployment returns mockDeployment directly but should match the builder API shape WasmEncodedResult<OrderBuilderDeploymentCfg> (an object with value and error); update the initial mock on builderInstance (getCurrentDeployment) to return { value: mockDeployment, error: null } (or appropriate error) so it aligns with other mocks like getAllFieldDefinitions and with later tests that override it.packages/orderbook/test/js_api/builder.test.ts (1)
1-23: 🧹 Nitpick | 🔵 TrivialPlease verify Orderbook TS build/test + format/lint/check runs.
Confirm
npm run build:orderbook,npm run check -w@rainlanguage/orderbook, `npm run test -w `@rainlanguage/orderbook, and the package-levelformat/lint/checksteps were executed.As per coding guidelines,
packages/orderbook/**/*.{ts,tsx,js,jsx}: ... run build:orderbook / check / testand**/*.{ts,tsx,svelte}: Format ... Lint ... Run check ....🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/orderbook/test/js_api/builder.test.ts` around lines 1 - 23, Run and verify the Orderbook package build/test/lint/check steps: execute npm run build:orderbook, npm run check -w `@rainlanguage/orderbook`, and npm run test -w `@rainlanguage/orderbook` locally and confirm green results; also run the package-level format/lint/check steps for packages/orderbook (format, lint, and type-check) and correct any failures found (fix TypeScript errors, formatting, or lint violations) so tests like packages/orderbook/test/js_api/builder.test.ts import symbols (e.g., RaindexOrderBuilder, OrderbookYaml) from the compiled output correctly and CI will pass.crates/js_api/src/registry.rs (2)
415-522: 🧹 Nitpick | 🔵 TrivialLeverage
?to simplify builder error propagation.
DotrainRegistryErroralready has#[from]forRaindexOrderBuilderWasmError, so explicitmap_errisn’t needed.♻️ Simplified error handling
- let builder = result.map_err(DotrainRegistryError::BuilderError)?; - Ok(builder) + Ok(result?)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/js_api/src/registry.rs` around lines 415 - 522, The match produces a Result<RaindexOrderBuilder, RaindexOrderBuilderWasmError> named result which is then converted with map_err into DotrainRegistryError; since DotrainRegistryError implements #[from] for RaindexOrderBuilderWasmError you can replace the explicit map_err with the try operator. In get_order_builder, change the final conversion from "let builder = result.map_err(DotrainRegistryError::BuilderError)?; Ok(builder)" (or similar) to simply use the try operator on result (e.g., let builder = result?; or return Ok(result?);) so the RaindexOrderBuilderWasmError is automatically converted into DotrainRegistryError.
12-115:⚠️ Potential issue | 🟡 MinorFix
getOrderBuilderexample argument order.
The example passesstateCallbackas the third argument, but the API expectsserializedStatethird andstateUpdateCallbackfourth. Passing a function in position 3 will fail conversion.✍️ Suggested doc fix
-/// const builder = await registry.getOrderBuilder("fixed-limit", "mainnet", stateCallback); +/// const builder = await registry.getOrderBuilder("fixed-limit", "mainnet", undefined, stateCallback);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/js_api/src/registry.rs` around lines 12 - 115, The example call to DotrainRegistry.getOrderBuilder passes stateCallback as the third argument but the API expects serializedState as the third and stateUpdateCallback as the fourth; update the example so DotrainRegistry.new(...) is followed by registry.getOrderBuilder("fixed-limit", "mainnet", serializedState, stateCallback) (i.e., supply a serializedState value or null/undefined in the third position and move stateCallback to the fourth) so it matches the getOrderBuilder parameter order used in the implementation.crates/settings/src/order_builder.rs (2)
1-16: 🧹 Nitpick | 🔵 TrivialRun the required Rust format/lint/test commands for this crate.
Please runnix develop -c cargo fmt --all,nix develop -c rainix-rs-static,nix develop -c cargo build,nix develop -c cargo clippy --workspace --all-targets --all-features -D warnings, andnix develop -c cargo test --workspacefor these changes.As per coding guidelines “
crates/**/*.rs: Runnix develop -c cargo buildto check Rust dependency readiness before development; for Rust crates incrates/*, run linting withnix develop -c cargo clippy --workspace --all-targets --all-features -D warnings; for Rust crates incrates/*, run tests withnix develop -c cargo test --workspaceor targeting specific packages with--package <crate>; and**/*.rs: Format Rust code withnix develop -c cargo fmt --alland lint withnix develop -c rainix-rs-staticwith preconfigured flags.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/settings/src/order_builder.rs` around lines 1 - 16, Run the Rust formatting, static linting, build, clippy checks, and tests for this crate to ensure changes compiling and follow style rules: execute `nix develop -c cargo fmt --all`, `nix develop -c rainix-rs-static`, `nix develop -c cargo build`, `nix develop -c cargo clippy --workspace --all-targets --all-features -D warnings`, and `nix develop -c cargo test --workspace`; verify the module that defines Context and OrderBuilderContextTrait (and related types like DeploymentCfg, TokenCfg) in this crate compiles cleanly and fixes any formatting or clippy warnings reported.
468-503:⚠️ Potential issue | 🟠 MajorShort-description should remain optional in order details.
parse_order_detailsnow requiresshort-description, butOrderBuilderCfgtreats it as optional (andparse_from_yaml_optionalallows it). This will error on valid configs that omit the field.🛠️ Suggested fix
- let short_description = require_string( - get_hash_value(builder, "short-description", Some("builder".to_string()))?, - None, - Some("builder".to_string()), - )?; + let short_description = get_hash_value_as_option(builder, "short-description") + .map(|v| { + v.as_str().ok_or(YamlError::Field { + kind: FieldErrorKind::InvalidType { + field: "short-description".to_string(), + expected: "a string".to_string(), + }, + location: "builder".to_string(), + }) + }) + .transpose()? + .map(|s| s.to_string()); return Ok(NameAndDescriptionCfg { name, description, - short_description: Some(short_description), + short_description, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/settings/src/order_builder.rs` around lines 468 - 503, parse_order_details currently treats "short-description" as required; change the logic so "short-description" is fetched with the optional YAML helper (instead of require_string) so missing short-description returns None rather than an error, then set NameAndDescriptionCfg.short_description to that Option; update the short_description retrieval that currently calls require_string(get_hash_value(...), ...) to use the optional string helper (or equivalent optional accessor) with get_hash_value(builder, "short-description", Some("builder".to_string())) so valid configs without short-description no longer error in parse_order_details.crates/common/src/raindex_order_builder/order_operations.rs (1)
241-288:⚠️ Potential issue | 🟠 MajorAvoid re-approving when allowance already exceeds the deposit.
Using!eqtriggers approvals even when allowance > required, leading to unnecessary transactions. Compare against the required allowance instead.🛠️ Suggested fix
- let allowance_float = Float::from_fixed_decimal(token_allowance.allowance, decimals)?; - - if !allowance_float.eq(*deposit_amount)? { + let required_allowance = deposit_amount.to_fixed_decimal(decimals)?; + if token_allowance.allowance < required_allowance { let calldata = approveCall { spender: tx_args.orderbook_address, - amount: deposit_amount.to_fixed_decimal(decimals)?, + amount: required_allowance, } .abi_encode();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/common/src/raindex_order_builder/order_operations.rs` around lines 241 - 288, The current check uses !allowance_float.eq(*deposit_amount) which causes approval when allowance is greater than the required amount; update generate_approval_calldatas to only create an approval calldata when the existing allowance is strictly less than the required deposit. Specifically, compare allowance_float against the required deposit amount in the same units (convert deposit_amount to the same fixed-decimal representation as allowance_float) and replace the eq-based condition with a "allowance < required" check (references: generate_approval_calldatas, allowance_float, deposit_amount, check_allowance, to_fixed_decimal).
🤖 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/js_api/ARCHITECTURE.md`:
- Line 7: Update the phrasing to use hyphenated compound adjectives: change
"WASM output" to "WASM-output" and "JS facing" to "JS-facing" in the sentence
containing "Crate type: `cdylib` (WASM output). Most modules are
`#[cfg(target_family = \"wasm\")]` as they are JS facing." and apply the same
hyphenation fix to the repeated occurrences around lines 118–120.
- Around line 40-41: The docs currently omit the optional settings parameter in
JS examples for RaindexOrderBuilder; update the descriptions and examples for
RaindexOrderBuilder.newWithDeployment and RaindexOrderBuilder.newFromState to
mention the optional settings argument (inserted before the deployment or
serialized args) and show examples that pass either a settings object or
undefined; ensure getDeploymentKeys remains documented as-is and add a brief
note near the newWithDeployment/newFromState entries indicating the parameter
order and optionality so readers know to include undefined when skipping
settings.
In `@crates/js_api/src/raindex_order_builder/deposits.rs`:
- Around line 1-90: The WASM bindings in TokenDeposit and the
RaindexOrderBuilder methods (TokenDeposit, impl
From<inner_deposits::TokenDeposit>, get_deposits, set_deposit, unset_deposit,
get_deposit_presets, get_missing_deposits, has_any_deposit) need to be validated
by running Rust formatting, build, clippy and tests; run the suggested commands
(nix develop -c cargo fmt --all, nix develop -c rainix-rs-static, nix develop -c
cargo build, nix develop -c cargo clippy --workspace --all-targets
--all-features -D warnings, nix develop -c cargo test --workspace), fix any
formatting/lint/build/test errors reported (adjust types, imports, async
signatures, error conversions, or missing trait impls in the referenced
functions/impls), then re-run the commands until all pass.
In `@crates/js_api/src/raindex_order_builder/mod.rs`:
- Around line 138-142: The doc header for getBuilderConfig is inconsistent (says
"complete configuration including all deployments" but function filters to the
current deployment); update the function's top-level Rust doc comment in
raindex_order_builder::getBuilderConfig to clearly state it returns the parsed
builder section from the YAML filtered to the current deployment (not all
deployments), and adjust the summary line and first sentence to match the
detailed description so readers know this returns order-level metadata scoped to
the current deployment.
- Around line 273-276: The docs state that `builder.short-description` is
optional but tests expect it to be required; update the documentation comment
(the block that lists `builder.name`, `builder.description`, and
`builder.short-description`) to mark `builder.short-description` as required
(remove "optional but recommended" and state it is required), or alternatively
change the tests that enforce `short-description` to treat it as optional—choose
the approach consistent with the intended API and keep the wording for
`builder.name` and `builder.description` unchanged.
In `@crates/js_api/src/raindex_order_builder/select_tokens.rs`:
- Around line 1-2: The crate changes in select_tokens.rs need verification that
Rust build/lint/test steps were executed: run in the crate root (and workspace)
cargo build, cargo fmt, rainix-rs-static (nix develop -c ... if your repo uses
that wrapper), cargo clippy --workspace --all-targets --all-features -D
warnings, and cargo test --workspace; then update the PR or commit message with
a confirmation and any clippy/fmt fixes applied (or include the generated format
changes), and ensure the repository-level CI (nix/cargo steps) passes—refer to
this file select_tokens.rs and the crate js_api to locate where to run these
checks.
In `@packages/ui-components/src/__tests__/SelectToken.test.ts`:
- Around line 67-75: The beforeEach currently calls vi.clearAllMocks() after
setting up mocks which can hide prior call history if order changes; move
vi.clearAllMocks() to the top of the beforeEach so you clear previous mocks/call
history first, then initialize mockStateUpdateCallback, set
mockBuilder.setSelectToken's implementation, and call (useRaindexOrderBuilder as
Mock).mockReturnValue(mockBuilder) to set up the mocks in the correct, idiomatic
order.
In `@packages/ui-components/src/__tests__/TokenSelectionModal.test.ts`:
- Around line 38-40: The test mocks the wrong module specifier: replace the
relative path in the vi.mock call with the same module specifier used in the
import so the mock resolves correctly; update the vi.mock(...) entry to mock
"$lib/hooks/useRaindexOrderBuilder" and keep the exported shape
(useRaindexOrderBuilder: vi.fn()) so the imported hook in
TokenSelectionModal.test.ts is properly mocked.
In `@packages/ui-components/src/lib/components/deployment/DepositInput.svelte`:
- Around line 1-16: This change touches the DepositInput.svelte UI component in
packages/ui-components—please run the full frontend verification: build the
package (nix develop -c npm run build -w `@rainlanguage/ui-components`), run
svelte lint/format check (nix develop -c npm run svelte-lint-format-check -w
`@rainlanguage/ui-components`), run unit tests (nix develop -c npm run test -w
`@rainlanguage/ui-components`), and run project-wide formatting, linting and type
checks (nix develop -c npm run format, nix develop -c npm run lint, nix develop
-c npm run check); also build and preview the webapp, take and attach a
screenshot showing the updated DepositInput.svelte UI (and include test/build
logs or artifacts), then update the PR with these artifacts to confirm frontend
verification was completed.
In `@packages/ui-components/src/lib/components/detail/OrderDetail.svelte`:
- Around line 314-320: This change adds a "Builder State" TabItem rendering
formatBuilderState(data.orderBuilderState) in OrderDetail.svelte; attach a
screenshot of the webapp preview showing the Builder State tab open and the JSON
output (confirming data.orderBuilderState renders correctly) and run the
required verification commands: build and lint/test the ui-components package
(npm run format, svelte-lint-format-check, build, test for
`@rainlanguage/ui-components`) and build/preview the webapp (npm run build and npm
run preview for `@rainlanguage/webapp`) as listed in the review so CI/local checks
pass; include the screenshot and a short note that all listed commands completed
successfully.
In `@packages/ui-components/src/lib/hooks/useRaindexOrderBuilder.ts`:
- Line 2: The import of RaindexOrderBuilder is currently a runtime import but
it's only used as a type in getContext<RaindexOrderBuilder>; change it to a
type-only import so the module doesn't pull in the WASM/runtime side-effect at
import time. Replace the existing import with a type import for
RaindexOrderBuilder and keep the usage in getContext<RaindexOrderBuilder>
unchanged so tooling and runtime behave correctly.
In `@packages/ui-components/src/lib/services/registry.ts`:
- Around line 1-3: The PR lacks frontend verification artifacts for changes in
the order validation service
(packages/ui-components/src/lib/services/registry.ts which imports
InvalidOrderDetail, ValidOrderDetail and RaindexOrderBuilder); please run the
project checks and build for `@rainlanguage/ui-components` using nix develop -c
npm run format -w `@rainlanguage/ui-components`, ... -w
`@rainlanguage/ui-components` (format, lint, check, build,
svelte-lint-format-check, test), then build and preview the webapp with nix
develop -c npm run build -w `@rainlanguage/webapp` and nix develop -c npm run
preview -w `@rainlanguage/webapp`, exercise the UI flows that use the order
validation (create/submit various valid and invalid orders via the components
relying on RaindexOrderBuilder and ValidOrderDetail/InvalidOrderDetail), capture
a screenshot of the preview showing successful validation behavior and include
the screenshot and a brief bullet list of manual verification steps in the PR
description.
---
Outside diff comments:
In `@crates/common/src/raindex_order_builder/order_operations.rs`:
- Around line 241-288: The current check uses
!allowance_float.eq(*deposit_amount) which causes approval when allowance is
greater than the required amount; update generate_approval_calldatas to only
create an approval calldata when the existing allowance is strictly less than
the required deposit. Specifically, compare allowance_float against the required
deposit amount in the same units (convert deposit_amount to the same
fixed-decimal representation as allowance_float) and replace the eq-based
condition with a "allowance < required" check (references:
generate_approval_calldatas, allowance_float, deposit_amount, check_allowance,
to_fixed_decimal).
In `@crates/js_api/src/registry.rs`:
- Around line 415-522: The match produces a Result<RaindexOrderBuilder,
RaindexOrderBuilderWasmError> named result which is then converted with map_err
into DotrainRegistryError; since DotrainRegistryError implements #[from] for
RaindexOrderBuilderWasmError you can replace the explicit map_err with the try
operator. In get_order_builder, change the final conversion from "let builder =
result.map_err(DotrainRegistryError::BuilderError)?; Ok(builder)" (or similar)
to simply use the try operator on result (e.g., let builder = result?; or return
Ok(result?);) so the RaindexOrderBuilderWasmError is automatically converted
into DotrainRegistryError.
- Around line 12-115: The example call to DotrainRegistry.getOrderBuilder passes
stateCallback as the third argument but the API expects serializedState as the
third and stateUpdateCallback as the fourth; update the example so
DotrainRegistry.new(...) is followed by registry.getOrderBuilder("fixed-limit",
"mainnet", serializedState, stateCallback) (i.e., supply a serializedState value
or null/undefined in the third position and move stateCallback to the fourth) so
it matches the getOrderBuilder parameter order used in the implementation.
In `@crates/settings/src/order_builder.rs`:
- Around line 1-16: Run the Rust formatting, static linting, build, clippy
checks, and tests for this crate to ensure changes compiling and follow style
rules: execute `nix develop -c cargo fmt --all`, `nix develop -c
rainix-rs-static`, `nix develop -c cargo build`, `nix develop -c cargo clippy
--workspace --all-targets --all-features -D warnings`, and `nix develop -c cargo
test --workspace`; verify the module that defines Context and
OrderBuilderContextTrait (and related types like DeploymentCfg, TokenCfg) in
this crate compiles cleanly and fixes any formatting or clippy warnings
reported.
- Around line 468-503: parse_order_details currently treats "short-description"
as required; change the logic so "short-description" is fetched with the
optional YAML helper (instead of require_string) so missing short-description
returns None rather than an error, then set
NameAndDescriptionCfg.short_description to that Option; update the
short_description retrieval that currently calls
require_string(get_hash_value(...), ...) to use the optional string helper (or
equivalent optional accessor) with get_hash_value(builder, "short-description",
Some("builder".to_string())) so valid configs without short-description no
longer error in parse_order_details.
In `@crates/settings/src/yaml/emitter.rs`:
- Around line 14-33: The CANONICAL_ROOT_KEYS constant was changed to "builder"
which causes top-level "gui" keys to be filtered out in emit_documents and
silent for users because OrderBuilderCfg::parse_from_yaml_optional returns None;
update validate_and_emit_documents to detect a legacy "gui" top-level key (or
detect it in emit_documents) and either migrate it to "builder" before filtering
or emit a clear diagnostic error instead of silently dropping it, and ensure
OrderBuilderCfg::parse_from_yaml_optional surfaces an error or warning when a
legacy "gui" document is encountered so users are notified of the required
change.
In `@packages/orderbook/test/js_api/builder.test.ts`:
- Around line 1-23: Run and verify the Orderbook package build/test/lint/check
steps: execute npm run build:orderbook, npm run check -w
`@rainlanguage/orderbook`, and npm run test -w `@rainlanguage/orderbook` locally and
confirm green results; also run the package-level format/lint/check steps for
packages/orderbook (format, lint, and type-check) and correct any failures found
(fix TypeScript errors, formatting, or lint violations) so tests like
packages/orderbook/test/js_api/builder.test.ts import symbols (e.g.,
RaindexOrderBuilder, OrderbookYaml) from the compiled output correctly and CI
will pass.
In `@packages/orderbook/test/js_api/dotrainRegistry.test.ts`:
- Around line 1-6: This change touches the orderbook package tests — ensure the
package builds, type-checks, formats and tests by running the required nix/npm
commands: execute `nix develop -c npm run build:orderbook` to build, `nix
develop -c npm run check -w `@rainlanguage/orderbook`` to run TypeScript checks
for the package (covers symbols like OrderbookYaml and DotrainRegistry used in
dotrainRegistry.test.ts), and `nix develop -c npm run test -w
`@rainlanguage/orderbook`` to run the package tests; also run project-wide
formatting, linting and checks with `nix develop -c npm run format`, `nix
develop -c npm run lint`, and `nix develop -c npm run check` to ensure TS/Svelte
formatting and lint rules are satisfied before merging.
In `@packages/ui-components/src/__tests__/ComposedRainlangModal.test.ts`:
- Around line 1-37: Test fails because the package '@rainlanguage/orderbook'
needs a passthrough mock; add a vi.mock for '@rainlanguage/orderbook'
immediately after the svelte-codemirror-editor mock that calls importOriginal
and returns a spread of await importOriginal() so the original exports are
preserved (place this mock near the other vi.mock calls in
ComposedRainlangModal.test.ts alongside useRaindexOrderBuilder and mockBuilder
setup).
In `@packages/ui-components/src/__tests__/DeploymentSteps.test.ts`:
- Around line 107-114: The mock for builderInstance.getCurrentDeployment returns
mockDeployment directly but should match the builder API shape
WasmEncodedResult<OrderBuilderDeploymentCfg> (an object with value and error);
update the initial mock on builderInstance (getCurrentDeployment) to return {
value: mockDeployment, error: null } (or appropriate error) so it aligns with
other mocks like getAllFieldDefinitions and with later tests that override it.
In
`@packages/ui-components/src/lib/components/deployment/ComposedRainlangModal.svelte`:
- Around line 1-20: The PR modifies the UI in ComposedRainlangModal.svelte
(notably the generateRainlang function and UI around rainlangText/open) but
lacks the required built-app screenshot and preview instructions; please attach
a screenshot showing the modal/UI state after building the webapp and include
concise build/preview steps (commands used, environment, branch/commit, and URL
or local port used) so reviewers can reproduce the UI (e.g., build/install, run
dev/preview, navigate to the component and invoke generateRainlang to show the
modal).
In
`@packages/ui-components/src/lib/components/deployment/FieldDefinitionInput.svelte`:
- Around line 1-55: Run the full UI components verification for
packages/ui-components to ensure build, lint, format and type checks pass for
this change in FieldDefinitionInput.svelte: run the build and tests (e.g., npm
run build -w `@rainlanguage/ui-components` and npm run test -w
`@rainlanguage/ui-components`), run Svelte/TS formatting and linting (e.g., npm
run svelte-lint-format-check -w `@rainlanguage/ui-components`, npm run format, npm
run lint) and run type checks (npm run check) before merging; if any errors
reference FieldDefinitionInput.svelte (including onMount, handlePresetClick,
handleCustomInputChange or useRaindexOrderBuilder usage), fix
formatting/type/lint issues and re-run the commands until all checks succeed.
In `@packages/ui-components/src/lib/components/deployment/SelectToken.svelte`:
- Around line 26-40: The onMount block may throw a TypeError when
builder.getTokenInfo returns { value: null }; update the logic in the onMount
async handler (the function that calls builder.getTokenInfo) to guard access to
result.value before reading properties: check result.value exists (or use
optional chaining) before assigning tokenInfo, reading result.value.address,
setting inputValue, or calling onSelectTokenSelect(token.key), and preserve the
existing error handling behavior.
In
`@packages/ui-components/src/lib/components/deployment/TokenSelectionModal.svelte`:
- Around line 5-22: The reviewer requires a built-webapp screenshot showing the
TokenSelectionModal change: rebuild and preview the webapp and attach a
screenshot showing the updated token source UI (TokenSelectionModal.svelte /
loadTokens -> builder.getAllTokens). Run the prescribed commands to verify the
frontend: `nix develop -c npm run build -w `@rainlanguage/webapp`` then `nix
develop -c npm run preview -w `@rainlanguage/webapp``, open the page where
TokenSelectionModal is used, capture a clear screenshot of the modal/search
results, and add that image to the PR with a short confirmation comment stating
the build and preview commands succeeded.
In `@packages/ui-components/src/lib/services/registry.ts`:
- Around line 83-94: The code in RaindexOrderBuilder.getOrderDetails handling
incorrectly accesses result.error.msg; update the error property access to
result.error.readableMsg in the failure branch where you throw (inside the
function returning { valid: true, data: ... } path) so the thrown Error uses the
exported readableMsg; locate the usage in registry.ts around the variable
registryDotrain and replace `.msg` with `.readableMsg` (and mirror the same fix
in any other files listed like handleShareChoices.ts and TokenIOInput.svelte if
present).
In `@packages/webapp/src/lib/services/handleAddOrder.ts`:
- Around line 1-14: This PR touches frontend file handleAddOrder.ts (imports
like RaindexOrderBuilder, RaindexClient, QKEY_ORDERS), so run the required
webapp verification: execute nix develop -c npm run build -w
`@rainlanguage/webapp`, nix develop -c npm run svelte-lint-format-check -w
`@rainlanguage/webapp`, and nix develop -c npm run test -w `@rainlanguage/webapp`;
run formatting, linting and type checks via nix develop -c npm run format, nix
develop -c npm run lint, and nix develop -c npm run check; then start a preview
of the built webapp and attach a screenshot that clearly shows the changed
behavior/UI (include timestamp or browser URL in the shot) to the PR comments so
reviewers can confirm the frontend change.
In `@packages/webapp/src/routes/deploy/`[orderName]/[deploymentKey]/+page.svelte:
- Around line 69-88: The PR changes the conditional rendering in +page.svelte
for RaindexOrderBuilderProvider/DeploymentSteps; provide a frontend verification
screenshot of the built webapp showing the builder view (when builder is truthy)
and include the image in the PR, and run the full Svelte build/preview/lint/test
checks: execute `nix develop -c npm run build -w `@rainlanguage/webapp``, then
`nix develop -c npm run preview -w `@rainlanguage/webapp`` to capture the running
UI screenshot for the route /deploy/[orderName]/[deploymentKey], and finally run
`nix develop -c npm run svelte-lint-format-check -w `@rainlanguage/webapp`` and
`nix develop -c npm run test -w `@rainlanguage/webapp`` to show lint/format/tests
pass; attach the screenshot and CI output to the PR for review.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (81)
ai_commands/sdk-documentation-update.mdcrates/bindings/ARCHITECTURE.mdcrates/common/Cargo.tomlcrates/common/src/add_order.rscrates/common/src/dotrain_order.rscrates/common/src/lib.rscrates/common/src/parsed_meta.rscrates/common/src/raindex_client/orders.rscrates/common/src/raindex_order_builder/deposits.rscrates/common/src/raindex_order_builder/field_values.rscrates/common/src/raindex_order_builder/mod.rscrates/common/src/raindex_order_builder/order_operations.rscrates/common/src/raindex_order_builder/select_tokens.rscrates/common/src/raindex_order_builder/state_management.rscrates/common/src/raindex_order_builder/validation.rscrates/common/src/test_helpers.rscrates/js_api/ARCHITECTURE.mdcrates/js_api/src/gui/deposits.rscrates/js_api/src/gui/field_values.rscrates/js_api/src/gui/select_tokens.rscrates/js_api/src/lib.rscrates/js_api/src/raindex_order_builder/deposits.rscrates/js_api/src/raindex_order_builder/field_values.rscrates/js_api/src/raindex_order_builder/mod.rscrates/js_api/src/raindex_order_builder/order_operations.rscrates/js_api/src/raindex_order_builder/select_tokens.rscrates/js_api/src/raindex_order_builder/state_management.rscrates/js_api/src/registry.rscrates/settings/ARCHITECTURE.mdcrates/settings/src/deployment.rscrates/settings/src/lib.rscrates/settings/src/order.rscrates/settings/src/order_builder.rscrates/settings/src/yaml/context.rscrates/settings/src/yaml/dotrain.rscrates/settings/src/yaml/emitter.rscrates/settings/src/yaml/mod.rscrates/settings/src/yaml/orderbook.rslib/rain.interpreterpackages/orderbook/ARCHITECTURE.mdpackages/orderbook/README.mdpackages/orderbook/test/js_api/builder.test.tspackages/orderbook/test/js_api/dotrainRegistry.test.tspackages/ui-components/ARCHITECTURE.mdpackages/ui-components/src/__tests__/ComposedRainlangModal.test.tspackages/ui-components/src/__tests__/DeploymentSteps.test.tspackages/ui-components/src/__tests__/DepositInput.test.tspackages/ui-components/src/__tests__/DotrainRegistryProvider.test.tspackages/ui-components/src/__tests__/FieldDefinitionInput.test.tspackages/ui-components/src/__tests__/OrderDetail.test.tspackages/ui-components/src/__tests__/SelectToken.test.tspackages/ui-components/src/__tests__/TokenIOInput.test.tspackages/ui-components/src/__tests__/TokenSelectionModal.test.tspackages/ui-components/src/__tests__/handleShareChoices.test.tspackages/ui-components/src/__tests__/registry.test.tspackages/ui-components/src/lib/components/deployment/ComposedRainlangModal.sveltepackages/ui-components/src/lib/components/deployment/DeploymentSteps.sveltepackages/ui-components/src/lib/components/deployment/DepositInput.sveltepackages/ui-components/src/lib/components/deployment/FieldDefinitionInput.sveltepackages/ui-components/src/lib/components/deployment/SelectToken.sveltepackages/ui-components/src/lib/components/deployment/TokenIOInput.sveltepackages/ui-components/src/lib/components/deployment/TokenSelectionModal.sveltepackages/ui-components/src/lib/components/detail/OrderDetail.sveltepackages/ui-components/src/lib/errors/DeploymentStepsError.tspackages/ui-components/src/lib/hooks/useGui.test.tspackages/ui-components/src/lib/hooks/useGui.tspackages/ui-components/src/lib/hooks/useRaindexOrderBuilder.test.tspackages/ui-components/src/lib/hooks/useRaindexOrderBuilder.tspackages/ui-components/src/lib/index.tspackages/ui-components/src/lib/providers/GuiProvider.sveltepackages/ui-components/src/lib/providers/RaindexOrderBuilderProvider.sveltepackages/ui-components/src/lib/services/handleShareChoices.tspackages/ui-components/src/lib/services/registry.tspackages/ui-components/test-setup.tspackages/webapp/ARCHITECTURE.mdpackages/webapp/src/__tests__/handleAddOrder.test.tspackages/webapp/src/lib/services/handleAddOrder.tspackages/webapp/src/lib/services/handleUpdateBuilderState.tspackages/webapp/src/routes/deploy/[orderName]/[deploymentKey]/+page.sveltepackages/webapp/src/routes/deploy/[orderName]/[deploymentKey]/fullDeployment.test.tspackages/webapp/src/routes/deploy/[orderName]/[deploymentKey]/page.test.ts
💤 Files with no reviewable changes (6)
- packages/ui-components/src/lib/hooks/useGui.test.ts
- packages/ui-components/src/lib/hooks/useGui.ts
- crates/js_api/src/gui/select_tokens.rs
- crates/js_api/src/gui/deposits.rs
- crates/js_api/src/gui/field_values.rs
- packages/ui-components/src/lib/providers/GuiProvider.svelte
|
|
||
| **Build & Targets** | ||
| - Crate type: `cdylib` (WASM output). Most modules are `#[cfg(target_family = "wasm")]` as they are JS/GUI facing. | ||
| - Crate type: `cdylib` (WASM output). Most modules are `#[cfg(target_family = "wasm")]` as they are JS facing. |
Contributor
There was a problem hiding this comment.
Minor grammar: add hyphens for compound adjectives.
✍️ Suggested edits
-- Crate type: `cdylib` (WASM output). Most modules are `#[cfg(target_family = "wasm")]` as they are JS facing.
+- Crate type: `cdylib` (WASM output). Most modules are `#[cfg(target_family = "wasm")]` as they are JS-facing.
-- Provides `to_readable_msg()` with end-user friendly explanations.
+- Provides `to_readable_msg()` with end-user-friendly explanations.Also applies to: 118-120
🧰 Tools
🪛 LanguageTool
[grammar] ~7-~7: Use a hyphen to join words.
Context: ...target_family = "wasm")]as they are JS facing. - Key dependencies:wasm-bindge...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/js_api/ARCHITECTURE.md` at line 7, Update the phrasing to use
hyphenated compound adjectives: change "WASM output" to "WASM-output" and "JS
facing" to "JS-facing" in the sentence containing "Crate type: `cdylib` (WASM
output). Most modules are `#[cfg(target_family = \"wasm\")]` as they are JS
facing." and apply the same hyphenation fix to the repeated occurrences around
lines 118–120.
Comment on lines
+40
to
+41
| - `RaindexOrderBuilder.getDeploymentKeys(dotrain: string) -> string[]` parses `builder.deployments`. | ||
| - `RaindexOrderBuilder.newWithDeployment(dotrain, selectedDeployment, stateUpdateCallback?) -> RaindexOrderBuilder` validates the deployment and bootstraps an order builder instance. |
Contributor
There was a problem hiding this comment.
Document the optional settings parameter in JS examples.
newWithDeployment/newFromState include an optional settings param before the deployment/serialized args; the docs should reflect that or show undefined.
📚 Suggested doc updates
- - `RaindexOrderBuilder.newWithDeployment(dotrain, selectedDeployment, stateUpdateCallback?) -> RaindexOrderBuilder`
+ - `RaindexOrderBuilder.newWithDeployment(dotrain, settings?, selectedDeployment, stateUpdateCallback?) -> RaindexOrderBuilder`
- - `const builder = await RaindexOrderBuilder.newWithDeployment(dotrain, deploymentKey, onStateChanged?)`
+ - `const builder = await RaindexOrderBuilder.newWithDeployment(dotrain, undefined, deploymentKey, onStateChanged?)`
- - Persist UI state: read `serializeState()`; restore later with `RaindexOrderBuilder.newFromState(dotrain, serialized, callback?)`.
+ - Persist UI state: read `serializeState()`; restore later with `RaindexOrderBuilder.newFromState(dotrain, undefined, serialized, callback?)`.Also applies to: 186-189
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/js_api/ARCHITECTURE.md` around lines 40 - 41, The docs currently omit
the optional settings parameter in JS examples for RaindexOrderBuilder; update
the descriptions and examples for RaindexOrderBuilder.newWithDeployment and
RaindexOrderBuilder.newFromState to mention the optional settings argument
(inserted before the deployment or serialized args) and show examples that pass
either a settings object or undefined; ensure getDeploymentKeys remains
documented as-is and add a brief note near the newWithDeployment/newFromState
entries indicating the parameter order and optionality so readers know to
include undefined when skipping settings.
Comment on lines
+38
to
40
| vi.mock('../lib/hooks/useRaindexOrderBuilder', () => ({ | ||
| useRaindexOrderBuilder: vi.fn() | ||
| })); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, check the actual content of the test file, specifically looking at imports and the mock
sed -n '1,50p' packages/ui-components/src/__tests__/TokenSelectionModal.test.ts | cat -nRepository: rainlanguage/rain.orderbook
Length of output: 1857
🏁 Script executed:
#!/bin/bash
# Check if there's a Vitest or Vite config that defines the $lib alias
fd -e config.ts -e vite.config.ts packages/ui-components | head -5Repository: rainlanguage/rain.orderbook
Length of output: 464
🏁 Script executed:
#!/bin/bash
# Search for other vi.mock patterns in test files to see if relative or alias is standard
rg "vi\.mock\(" packages/ui-components/src/__tests__/ | head -20Repository: rainlanguage/rain.orderbook
Length of output: 2330
🏁 Script executed:
#!/bin/bash
# Check the Vite config for $lib alias setup
cat packages/ui-components/vite.config.ts | head -50Repository: rainlanguage/rain.orderbook
Length of output: 888
🏁 Script executed:
#!/bin/bash
# Search for other cases where vi.mock uses relative path while component/test imports using $lib
rg -B2 "from '\$lib" packages/ui-components/src/__tests__/*.test.ts | rg -A2 "vi\.mock\(" | head -30Repository: rainlanguage/rain.orderbook
Length of output: 53
🏁 Script executed:
#!/bin/bash
# Check if there are other mismatch cases (relative path mocks with $lib imports)
rg "vi\.mock\('\.\./" packages/ui-components/src/__tests__/ -A1 -B5 | head -50Repository: rainlanguage/rain.orderbook
Length of output: 4640
🏁 Script executed:
#!/bin/bash
# Verify if the test file actually runs and if the mock is being applied by checking test patterns
grep -A 20 "beforeEach\|it(" packages/ui-components/src/__tests__/TokenSelectionModal.test.ts | head -40Repository: rainlanguage/rain.orderbook
Length of output: 1374
🏁 Script executed:
#!/bin/bash
# Check if there's any evidence of tests passing or failing with this mock setup
# Look for how the mock is actually used in tests
grep -n "useRaindexOrderBuilder\|mockBuilder\|getAllTokens" packages/ui-components/src/__tests__/TokenSelectionModal.test.ts | head -20Repository: rainlanguage/rain.orderbook
Length of output: 1083
Use the same module specifier for import and mock.
useRaindexOrderBuilder is imported with the $lib alias (line 7) but mocked with a relative path (line 38). Module mocking in Vitest requires exact specifier matching. Change the mock to use $lib to align with the import.
🔧 Suggested fix
-vi.mock('../lib/hooks/useRaindexOrderBuilder', () => ({
+vi.mock('$lib/hooks/useRaindexOrderBuilder', () => ({
useRaindexOrderBuilder: vi.fn()
}));📝 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
| vi.mock('../lib/hooks/useRaindexOrderBuilder', () => ({ | |
| useRaindexOrderBuilder: vi.fn() | |
| })); | |
| vi.mock('$lib/hooks/useRaindexOrderBuilder', () => ({ | |
| useRaindexOrderBuilder: vi.fn() | |
| })); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui-components/src/__tests__/TokenSelectionModal.test.ts` around
lines 38 - 40, The test mocks the wrong module specifier: replace the relative
path in the vi.mock call with the same module specifier used in the import so
the mock resolves correctly; update the vi.mock(...) entry to mock
"$lib/hooks/useRaindexOrderBuilder" and keep the exported shape
(useRaindexOrderBuilder: vi.fn()) so the imported hook in
TokenSelectionModal.test.ts is properly mocked.
Comment on lines
1
to
+16
| <script lang="ts"> | ||
| import { type GuiDepositCfg, type TokenDeposit, type TokenInfo } from '@rainlanguage/orderbook'; | ||
| import { | ||
| type OrderBuilderDepositCfg, | ||
| type TokenDeposit, | ||
| type TokenInfo | ||
| } from '@rainlanguage/orderbook'; | ||
| import { Input } from 'flowbite-svelte'; | ||
| import ButtonSelectOption from './ButtonSelectOption.svelte'; | ||
| import DeploymentSectionHeader from './DeploymentSectionHeader.svelte'; | ||
| import { CloseCircleSolid } from 'flowbite-svelte-icons'; | ||
| import { onMount } from 'svelte'; | ||
| import { useGui } from '$lib/hooks/useGui'; | ||
| import { useRaindexOrderBuilder } from '$lib/hooks/useRaindexOrderBuilder'; | ||
|
|
||
| export let deposit: GuiDepositCfg; | ||
| export let deposit: OrderBuilderDepositCfg; | ||
|
|
||
| const gui = useGui(); | ||
| const builder = useRaindexOrderBuilder(); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Please confirm required UI-component and frontend verification steps were run (and attach screenshot).
This change touches frontend UI components, so please provide:
nix develop -c npm run build -w@rainlanguage/ui-components``nix develop -c npm run svelte-lint-format-check -w@rainlanguage/ui-components``nix develop -c npm run test -w@rainlanguage/ui-components``nix develop -c npm run format,nix develop -c npm run lint,nix develop -c npm run check(for TS/Svelte)- Webapp build + preview and a screenshot of the built webapp reflecting the change.
As per coding guidelines: “packages/ui-components//*.{ts,tsx,svelte,js,jsx}: … run nix develop -c npm run build -w @rainlanguage/ui-components, … `nix develop -c npm run svelte-lint-format-check -w `@rainlanguage/ui-components, … nix develop -c npm run test -w @rainlanguage/ui-components``”; “/.{ts,tsx,svelte}: Format TypeScript/Svelte code with nix develop -c npm run format … Lint … Run type checks …”; and “packages/{webapp,ui-components}/**/.{ts,tsx,svelte,js,jsx}: … MUST provide a screenshot … build … preview …”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui-components/src/lib/components/deployment/DepositInput.svelte`
around lines 1 - 16, This change touches the DepositInput.svelte UI component in
packages/ui-components—please run the full frontend verification: build the
package (nix develop -c npm run build -w `@rainlanguage/ui-components`), run
svelte lint/format check (nix develop -c npm run svelte-lint-format-check -w
`@rainlanguage/ui-components`), run unit tests (nix develop -c npm run test -w
`@rainlanguage/ui-components`), and run project-wide formatting, linting and type
checks (nix develop -c npm run format, nix develop -c npm run lint, nix develop
-c npm run check); also build and preview the webapp, take and attach a
screenshot showing the updated DepositInput.svelte UI (and include test/build
logs or artifacts), then update the PR with these artifacts to confirm frontend
verification was completed.
Comment on lines
+314
to
320
| {#if data.orderBuilderState} | ||
| <TabItem title="Builder State"> | ||
| <div class="mb-4"> | ||
| <div class="overflow-auto rounded-lg border bg-gray-50 p-4 dark:bg-gray-800"> | ||
| <pre class="text-sm" data-testid="gui-state-json"> | ||
| {formatGuiState(data.dotrainGuiState)} | ||
| <pre class="text-sm" data-testid="builder-state-json"> | ||
| {formatBuilderState(data.orderBuilderState)} | ||
| </pre> |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Builder State tab wiring looks good — please attach UI verification artifacts.
Please attach a screenshot from the webapp preview showing the Builder State tab, and confirm the UI-components/webapp checks for this change.
nix develop -c npm run format -w `@rainlanguage/ui-components`
nix develop -c npm run lint -w `@rainlanguage/ui-components`
nix develop -c npm run check -w `@rainlanguage/ui-components`
nix develop -c npm run build -w `@rainlanguage/ui-components`
nix develop -c npm run svelte-lint-format-check -w `@rainlanguage/ui-components`
nix develop -c npm run test -w `@rainlanguage/ui-components`
nix develop -c npm run build -w `@rainlanguage/webapp`
nix develop -c npm run preview -w `@rainlanguage/webapp`As per coding guidelines: packages/{webapp,ui-components}/**/*.{ts,tsx,svelte,js,jsx}: If you modify frontend code or functionality affecting the frontend, you MUST provide a screenshot of the built webapp reflecting your change; frontend verification requires building and previewing the webapp with nix develop -c npm run build -w @rainlanguage/webapp followed by `nix develop -c npm run preview -w `@rainlanguage/webapp. packages/ui-components/**/*.{ts,tsx,svelte,js,jsx}: For UI components package (@rainlanguage/ui-components), run nix develop -c npm run build -w @rainlanguage/ui-components after making changes; run linting with `nix develop -c npm run svelte-lint-format-check -w `@rainlanguage/ui-components; run tests with nix develop -c npm run test -w @rainlanguage/ui-components``. **/*.{ts,tsx,svelte}: Format TypeScript/Svelte code with `nix develop -c npm run format` in each package; Lint TypeScript/Svelte code with `nix develop -c npm run lint` in each package; Run type checks on TypeScript/Svelte with `nix develop -c npm run check` in each package.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui-components/src/lib/components/detail/OrderDetail.svelte` around
lines 314 - 320, This change adds a "Builder State" TabItem rendering
formatBuilderState(data.orderBuilderState) in OrderDetail.svelte; attach a
screenshot of the webapp preview showing the Builder State tab open and the JSON
output (confirming data.orderBuilderState renders correctly) and run the
required verification commands: build and lint/test the ui-components package
(npm run format, svelte-lint-format-check, build, test for
`@rainlanguage/ui-components`) and build/preview the webapp (npm run build and npm
run preview for `@rainlanguage/webapp`) as listed in the review so CI/local checks
pass; include the screenshot and a short note that all listed commands completed
successfully.
| @@ -0,0 +1,12 @@ | |||
| import { getContext } from 'svelte'; | |||
| import { RaindexOrderBuilder } from '@rainlanguage/orderbook'; | |||
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Prefer import type for type-only usage of RaindexOrderBuilder
RaindexOrderBuilder is used exclusively as a generic type parameter in getContext<RaindexOrderBuilder>. A type-only import makes the intent explicit and avoids a runtime side-effect import of the WASM class.
♻️ Suggested change
-import { RaindexOrderBuilder } from '@rainlanguage/orderbook';
+import type { RaindexOrderBuilder } from '@rainlanguage/orderbook';📝 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
| import { RaindexOrderBuilder } from '@rainlanguage/orderbook'; | |
| import type { RaindexOrderBuilder } from '@rainlanguage/orderbook'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui-components/src/lib/hooks/useRaindexOrderBuilder.ts` at line 2,
The import of RaindexOrderBuilder is currently a runtime import but it's only
used as a type in getContext<RaindexOrderBuilder>; change it to a type-only
import so the module doesn't pull in the WASM/runtime side-effect at import
time. Replace the existing import with a type import for RaindexOrderBuilder and
keep the usage in getContext<RaindexOrderBuilder> unchanged so tooling and
runtime behave correctly.
Comment on lines
1
to
3
| import type { InvalidOrderDetail, ValidOrderDetail } from '$lib/types/order'; | ||
| import { DotrainOrderGui } from '@rainlanguage/orderbook'; | ||
| import { RaindexOrderBuilder } from '@rainlanguage/orderbook'; | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/ui-components/src/lib/services/registry.ts | head -100Repository: rainlanguage/rain.orderbook
Length of output: 3613
🏁 Script executed:
cat packages/ui-components/package.json | grep -A 3 '"name"'Repository: rainlanguage/rain.orderbook
Length of output: 272
🏁 Script executed:
cat packages/webapp/package.json | grep -A 3 '"name"'Repository: rainlanguage/rain.orderbook
Length of output: 177
🏁 Script executed:
cd packages && ls -la | grep package.jsonRepository: rainlanguage/rain.orderbook
Length of output: 53
🏁 Script executed:
rg "RaindexOrderBuilder" -A 5 -B 2 packages/Repository: rainlanguage/rain.orderbook
Length of output: 50386
🏁 Script executed:
find . -name "*.ts" -o -name "*.tsx" | head -20 | xargs grep -l "getOrderDetails" 2>/dev/null | head -5Repository: rainlanguage/rain.orderbook
Length of output: 119
🏁 Script executed:
grep -A 10 "getOrderDetails" packages/orderbook/test/js_api/builder.test.ts | head -20Repository: rainlanguage/rain.orderbook
Length of output: 727
🏁 Script executed:
rg "WasmEncodedResult|error.*msg|error.*readableMsg" packages/orderbook -A 2 -B 2 | head -40Repository: rainlanguage/rain.orderbook
Length of output: 3518
Provide frontend verification steps and screenshot for this ui-components change.
This file is a service affecting frontend order validation. Per coding guidelines, run nix develop -c npm run format -w @rainlanguage/ui-components, `nix develop -c npm run lint -w `@rainlanguage/ui-components, nix develop -c npm run check -w @rainlanguage/ui-components, `nix develop -c npm run build -w `@rainlanguage/ui-components, nix develop -c npm run svelte-lint-format-check -w @rainlanguage/ui-components, and `nix develop -c npm run test -w `@rainlanguage/ui-components. Then build and preview the webapp with nix develop -c npm run build -w @rainlanguage/webapp followed by `nix develop -c npm run preview -w `@rainlanguage/webapp, and attach a screenshot of the preview.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui-components/src/lib/services/registry.ts` around lines 1 - 3, The
PR lacks frontend verification artifacts for changes in the order validation
service (packages/ui-components/src/lib/services/registry.ts which imports
InvalidOrderDetail, ValidOrderDetail and RaindexOrderBuilder); please run the
project checks and build for `@rainlanguage/ui-components` using nix develop -c
npm run format -w `@rainlanguage/ui-components`, ... -w
`@rainlanguage/ui-components` (format, lint, check, build,
svelte-lint-format-check, test), then build and preview the webapp with nix
develop -c npm run build -w `@rainlanguage/webapp` and nix develop -c npm run
preview -w `@rainlanguage/webapp`, exercise the UI flows that use the order
validation (create/submit various valid and invalid orders via the components
relying on RaindexOrderBuilder and ValidOrderDetail/InvalidOrderDetail), capture
a screenshot of the preview showing successful validation behavior and include
the screenshot and a brief bullet list of manual verification steps in the PR
description.
Contributor
How to use the Graphite Merge QueueAdd the label Raindex-queue to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
This was referenced Apr 13, 2026
This was referenced Apr 15, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
crates/common/src/add_order.rs (1)
330-348:⚠️ Potential issue | 🟠 MajorValidate the builder-state hash before emitting meta.
At Line 334,
subjectcomes from the decodedOrderBuilderStateV1, but the emitted payload is built fromself.dotrain. If those ever diverge, this will publish the current dotrain under the wrong subject hash and poison metaboard lookups for that state. Please compare the embeddeddotrain_hash()with the hash ofself.dotrainand fail on mismatch.🛡️ Suggested guard
use rain_metadata::{ + types::dotrain::source_v1::DotrainSourceV1, types::dotrain::order_builder_state_v1::OrderBuilderStateV1, types::raindex_signed_context_oracle::RaindexSignedContextOracleV1, ContentEncoding, ContentLanguage, ContentType, Error as RainMetaError, KnownMagic, RainMetaDocumentV1Item, }; @@ { Some(doc) => { let builder_state = OrderBuilderStateV1::try_from(doc.clone())?; let subject_hash = builder_state.dotrain_hash(); + let expected_hash = DotrainSourceV1(self.dotrain.clone()).hash(); + if subject_hash != expected_hash { + return Err(AddOrderArgsError::InvalidArgs( + "order-builder state dotrain hash does not match emitted dotrain" + .to_string(), + )); + } let meta_document = RainMetaDocumentV1Item { payload: ByteBuf::from(self.dotrain.as_bytes()), magic: KnownMagic::DotrainSourceV1,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/common/src/add_order.rs` around lines 330 - 348, The code is emitting meta using self.dotrain but sets subject from builder_state.dotrain_hash() (OrderBuilderStateV1::dotrain_hash()), which can diverge; compute the hash of self.dotrain (same hashing used by OrderBuilderStateV1::dotrain_hash()), compare it to builder_state.dotrain_hash(), and if they differ return an error (or propagate a failure) before constructing RainMetaDocumentV1Item and returning emitMetaCall; update the block around OrderBuilderStateV1 handling (where subject is set and meta is built) to perform this validation and fail on mismatch.crates/common/src/dotrain_order.rs (1)
757-814:⚠️ Potential issue | 🟠 MajorMerge all
buildersources before pruning.
create_with_profile()supports layeredsettings, but this helper stops at the first document that containsbuilderand returnsNoneif that document does not also contain the selected deployment. That meansgenerate_dotrain_for_deployment()can silently drop the builder block when root metadata lives in frontmatter and deployment config is added or overridden in a later settings file. Please mergebuilderacross all documents, or at least keep scanning until the selected deployment is found, before filteringbuilder.deployments.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/common/src/dotrain_order.rs` around lines 757 - 814, The helper clone_builder_config_for_deployment currently stops at the first document that contains a "builder" and returns None if that document lacks the selected deployment; change it to merge/accumulate the "builder" sections from all documents (merging top-level builder keys and merging the "deployments" maps across documents) and only then prune deployments to the single deployment_key (or at least continue scanning all documents until that deployment is found). Specifically, in clone_builder_config_for_deployment, iterate all documents to collect/merge StrictYaml::Hash entries for "builder" (merge keys into builder_hash, and for "deployments" merge each deployments_hash into a combined deployments map), then after the merge locate deployment_key in the combined deployments map, create filtered_deployments with that single entry, insert it back into the merged builder_hash, and return Ok(Some(StrictYaml::Hash(builder_hash))); ensure errors are preserved when types are not maps and only return Ok(None) if no builder or no matching deployment is present after the full merge.crates/common/src/raindex_client/orders.rs (1)
1437-1453:⚠️ Potential issue | 🟠 MajorKeep a legacy GUI-state fallback in
fetch_dotrain_source().This now returns early unless
ParsedMeta::OrderBuilderStateV1exists. Orders that were already published with the old GUI metadata will never satisfy that match, sodotrainSourcestops hydrating for historical orders after the rename.crates/common/src/parsed_meta.rs:1-50reinforces this because the legacy parsed variant is no longer available here either.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/common/src/raindex_client/orders.rs` around lines 1437 - 1453, fetch_dotrain_source() currently returns early when ParsedMeta::OrderBuilderStateV1 isn't found, causing legacy GUI-state orders to never hydrate; update fetch_dotrain_source() to fallback to the legacy parsed meta variant (the older GUI/state representation) when ParsedMeta::OrderBuilderStateV1 is missing—search in the function for self.parsed_meta.iter().find_map(...) and the use of order_builder_state.dotrain_hash(), and change the match to first try ParsedMeta::OrderBuilderStateV1 then fallback to the legacy variant (or reconstruct equivalent state) so dotrainSource continues to be populated for historical orders while keeping the existing get_metaboard_client() logic and subject_hash usage.
♻️ Duplicate comments (2)
crates/js_api/src/raindex_order_builder/mod.rs (2)
270-291:⚠️ Potential issue | 🟡 Minor
getOrderDetailsdocs still markbuilder.short-descriptionas optional.The tests in this file treat missing
builder.short-descriptionas a validation error, so the required-fields list andreturn_descriptionare still misleading.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/js_api/src/raindex_order_builder/mod.rs` around lines 270 - 291, The docs for getOrderDetails are inconsistent: update the comment block and the wasm_export attribute for the getOrderDetails function so that `builder.short-description` is listed as required (remove "optional but recommended") and adjust the return_description to reflect that the short description is always returned; reference the `getOrderDetails` function and the `builder.short-description` field in your edits so the documentation matches the validation logic in the tests.
138-158:⚠️ Potential issue | 🟡 Minor
getBuilderConfigdocs still read like this returns all deployments.The method is described as returning the complete builder config, but the text below it says it is filtered to the current deployment. Please make the summary and
return_descriptionmatch the scoped behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/js_api/src/raindex_order_builder/mod.rs` around lines 138 - 158, Update the getBuilderConfig doc comments and wasm_export metadata to reflect that the returned config is scoped/filtered to the current deployment rather than the complete set of deployments: change the summary sentence to state it "Retrieves the builder configuration scoped to the current deployment" and update the wasm_export return_description from "Complete builder configuration with name, description, and deployments" to something like "Builder configuration for the current deployment (name, description, and its deployment-specific settings)"; ensure references to "all deployments" in the doc block are removed or replaced with "current deployment" and keep the js_name and unchecked_return_type unchanged.
🧹 Nitpick comments (1)
crates/common/src/raindex_order_builder/state_management.rs (1)
29-35: Add an explicit version envelope to the serialized builder state.
serialize_state()andnew_from_state()round-trip a rawSerializedBuilderStatepayload, so the next field add/remove/rename turns persisted state into an opaque decode failure with no migration path. Since this blob is now a public cross-platform format, it should be versioned up front.♻️ Possible shape
+#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] +struct VersionedSerializedBuilderState { + version: u8, + state: SerializedBuilderState, +} ... - let bytes = bincode::serialize(&state)?; + let bytes = bincode::serialize(&VersionedSerializedBuilderState { + version: 1, + state, + })?; ... - let state: SerializedBuilderState = bincode::deserialize(&bytes)?; + let wrapper: VersionedSerializedBuilderState = bincode::deserialize(&bytes)?; + if wrapper.version != 1 { + return Err(RaindexOrderBuilderError::UnsupportedStateVersion(wrapper.version)); + } + let state = wrapper.state;Also applies to: 192-279
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/common/src/raindex_order_builder/state_management.rs` around lines 29 - 35, The serialized payload lacks a version envelope—add a top-level version field to the persisted format and update serialization/deserialization to use it: define a new wrapper type (e.g., VersionedSerializedBuilderState or add a `version: u32` field to SerializedBuilderState) and change serialize_state() to emit that versioned wrapper, and change new_from_state() to parse the version, validate/handle known versions (return a clear error or perform migration for unknown/older versions), ensuring any future field additions can be migrated; update references to SerializedBuilderState, serialize_state, and new_from_state 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 `@crates/js_api/src/raindex_order_builder/mod.rs`:
- Around line 86-106: The examples for RaindexOrderBuilder.newWithDeployment are
passing the deployment key as the second argument which is actually the settings
parameter; update both examples to call newWithDeployment(dotrainYaml, settings,
"mainnet-order", stateUpdateCallback) (i.e., provide a settings object or
null/{} as the second argument and pass the deployment key as the third
argument), and ensure the optional stateUpdateCallback is the fourth parameter
so selectedDeployment is set correctly.
---
Outside diff comments:
In `@crates/common/src/add_order.rs`:
- Around line 330-348: The code is emitting meta using self.dotrain but sets
subject from builder_state.dotrain_hash() (OrderBuilderStateV1::dotrain_hash()),
which can diverge; compute the hash of self.dotrain (same hashing used by
OrderBuilderStateV1::dotrain_hash()), compare it to
builder_state.dotrain_hash(), and if they differ return an error (or propagate a
failure) before constructing RainMetaDocumentV1Item and returning emitMetaCall;
update the block around OrderBuilderStateV1 handling (where subject is set and
meta is built) to perform this validation and fail on mismatch.
In `@crates/common/src/dotrain_order.rs`:
- Around line 757-814: The helper clone_builder_config_for_deployment currently
stops at the first document that contains a "builder" and returns None if that
document lacks the selected deployment; change it to merge/accumulate the
"builder" sections from all documents (merging top-level builder keys and
merging the "deployments" maps across documents) and only then prune deployments
to the single deployment_key (or at least continue scanning all documents until
that deployment is found). Specifically, in clone_builder_config_for_deployment,
iterate all documents to collect/merge StrictYaml::Hash entries for "builder"
(merge keys into builder_hash, and for "deployments" merge each deployments_hash
into a combined deployments map), then after the merge locate deployment_key in
the combined deployments map, create filtered_deployments with that single
entry, insert it back into the merged builder_hash, and return
Ok(Some(StrictYaml::Hash(builder_hash))); ensure errors are preserved when types
are not maps and only return Ok(None) if no builder or no matching deployment is
present after the full merge.
In `@crates/common/src/raindex_client/orders.rs`:
- Around line 1437-1453: fetch_dotrain_source() currently returns early when
ParsedMeta::OrderBuilderStateV1 isn't found, causing legacy GUI-state orders to
never hydrate; update fetch_dotrain_source() to fallback to the legacy parsed
meta variant (the older GUI/state representation) when
ParsedMeta::OrderBuilderStateV1 is missing—search in the function for
self.parsed_meta.iter().find_map(...) and the use of
order_builder_state.dotrain_hash(), and change the match to first try
ParsedMeta::OrderBuilderStateV1 then fallback to the legacy variant (or
reconstruct equivalent state) so dotrainSource continues to be populated for
historical orders while keeping the existing get_metaboard_client() logic and
subject_hash usage.
---
Duplicate comments:
In `@crates/js_api/src/raindex_order_builder/mod.rs`:
- Around line 270-291: The docs for getOrderDetails are inconsistent: update the
comment block and the wasm_export attribute for the getOrderDetails function so
that `builder.short-description` is listed as required (remove "optional but
recommended") and adjust the return_description to reflect that the short
description is always returned; reference the `getOrderDetails` function and the
`builder.short-description` field in your edits so the documentation matches the
validation logic in the tests.
- Around line 138-158: Update the getBuilderConfig doc comments and wasm_export
metadata to reflect that the returned config is scoped/filtered to the current
deployment rather than the complete set of deployments: change the summary
sentence to state it "Retrieves the builder configuration scoped to the current
deployment" and update the wasm_export return_description from "Complete builder
configuration with name, description, and deployments" to something like
"Builder configuration for the current deployment (name, description, and its
deployment-specific settings)"; ensure references to "all deployments" in the
doc block are removed or replaced with "current deployment" and keep the js_name
and unchecked_return_type unchanged.
---
Nitpick comments:
In `@crates/common/src/raindex_order_builder/state_management.rs`:
- Around line 29-35: The serialized payload lacks a version envelope—add a
top-level version field to the persisted format and update
serialization/deserialization to use it: define a new wrapper type (e.g.,
VersionedSerializedBuilderState or add a `version: u32` field to
SerializedBuilderState) and change serialize_state() to emit that versioned
wrapper, and change new_from_state() to parse the version, validate/handle known
versions (return a clear error or perform migration for unknown/older versions),
ensuring any future field additions can be migrated; update references to
SerializedBuilderState, serialize_state, and new_from_state 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8b07ca34-f708-4421-8be1-6a5cbe732e93
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
Cargo.tomlcrates/common/Cargo.tomlcrates/common/src/add_order.rscrates/common/src/dotrain_order.rscrates/common/src/lib.rscrates/common/src/parsed_meta.rscrates/common/src/raindex_client/orders.rscrates/common/src/raindex_order_builder/mod.rscrates/common/src/raindex_order_builder/order_operations.rscrates/common/src/raindex_order_builder/select_tokens.rscrates/common/src/raindex_order_builder/state_management.rscrates/common/src/test_helpers.rscrates/js_api/src/raindex_order_builder/mod.rscrates/js_api/src/registry.rscrates/settings/src/deployment.rs
✅ Files skipped from review due to trivial changes (4)
- crates/common/src/lib.rs
- crates/settings/src/deployment.rs
- crates/common/src/test_helpers.rs
- crates/common/src/parsed_meta.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- crates/common/Cargo.toml
- crates/js_api/src/registry.rs
Contributor
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)
.github/copilot-instructions.md (1)
66-78:⚠️ Potential issue | 🔴 CriticalThe fallback command sequence is broken—referenced directories do not exist.
Lines 70–72 reference directories that don't exist in this repository:
lib/rain.interpreter/lib/rain.interpreter.interface/lib/rain.math.float(missing)lib/rain.interpreter/lib/rain.metadata(missing)
lib/rain.interpreterexists but is empty. Thecdcommands in the fallback will fail immediately, defeating the purpose of the fallback. Either update the paths to match the actual repository structure or remove these commands if they are no longer needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/copilot-instructions.md around lines 66 - 78, The fallback steps in .github/copilot-instructions.md reference non-existent directories (the cd targets in the bash commands: lib/rain.interpreter/lib/rain.interpreter.interface/lib/rain.math.float and lib/rain.interpreter/lib/rain.metadata) so the fallback will fail; update the sequence by correcting or removing those cd commands: either replace those paths with the actual repository paths under lib/rain.interpreter (or other current package locations) or remove the three failing lines entirely, and ensure the remaining commands (including ./prep-all.sh, nix develop -c forge install, the rainix-* prelude invocations, and the npm build commands) reflect the repo’s real structure.
🧹 Nitpick comments (1)
.github/copilot-instructions.md (1)
80-83: Remove trailing whitespace on the EOF blank line (if any).Line 82 indicates an end-of-file whitespace/formatting adjustment where a trailing-whitespace blank line was “preserved” while adding a trailing newline. If CI includes trailing-whitespace enforcement, that preserved whitespace can still fail formatting checks.
Consider removing any spaces/tabs on the final blank line so the file ends with a clean empty line.
Proposed whitespace-only cleanup
- +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/copilot-instructions.md around lines 80 - 83, Remove any trailing spaces or tabs on the final blank line after the line that starts with "Goal: all CI checks in `.github/workflows` pass." so the file ends with a single clean newline (no trailing whitespace); open the file, trim whitespace from the EOF blank line and save so formatting/lint CI stops flagging preserved trailing whitespace.
🤖 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 @.github/copilot-instructions.md:
- Around line 66-78: The fallback steps in .github/copilot-instructions.md
reference non-existent directories (the cd targets in the bash commands:
lib/rain.interpreter/lib/rain.interpreter.interface/lib/rain.math.float and
lib/rain.interpreter/lib/rain.metadata) so the fallback will fail; update the
sequence by correcting or removing those cd commands: either replace those paths
with the actual repository paths under lib/rain.interpreter (or other current
package locations) or remove the three failing lines entirely, and ensure the
remaining commands (including ./prep-all.sh, nix develop -c forge install, the
rainix-* prelude invocations, and the npm build commands) reflect the repo’s
real structure.
---
Nitpick comments:
In @.github/copilot-instructions.md:
- Around line 80-83: Remove any trailing spaces or tabs on the final blank line
after the line that starts with "Goal: all CI checks in `.github/workflows`
pass." so the file ends with a single clean newline (no trailing whitespace);
open the file, trim whitespace from the EOF blank line and save so
formatting/lint CI stops flagging preserved trailing whitespace.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a8201279-76b9-476b-bba5-8ce11707682b
📒 Files selected for processing (3)
.github/copilot-instructions.mdpointers.shprep-base.sh
hardyjosh
approved these changes
Apr 30, 2026
Contributor
Merge activity
|
…erBuilder (#2479) ## Dependent PRs - rainlanguage/specs#46 - rainlanguage/rain.strategies#89 - rainlanguage/rainlang#483 ## Motivation The order builder logic (formerly "GUI") lived entirely in `crates/js_api/src/gui/`, tightly coupled to WASM and `JsValue`. This made it impossible to reuse from the CLI, REST API, or other Rust consumers. Additionally, the "GUI" naming was misleading since the logic is not UI-specific — it handles field values, deposits, token selection, state serialization, and order calldata generation. ## Solution - **Ported core logic to `crates/common/src/raindex_order_builder/`** — field values, deposits, select tokens, order operations, state management, and validation now live in a platform-agnostic common crate with no WASM dependencies. - **Reduced `crates/js_api/src/raindex_order_builder/`** to thin WASM wrappers that delegate to the common crate, keeping only `JsValue` conversion and callback handling. - **Renamed all GUI terminology to OrderBuilder** across the entire codebase: - Rust types: `DotrainOrderGui` → `RaindexOrderBuilder`, `DotrainGuiStateV1` → `OrderBuilderStateV1`, `GuiCfg` → `OrderBuilderCfg`, etc. - YAML keyword: `gui:` → `builder:` - Settings crate: `gui.rs` → `order_builder.rs` - Svelte/TS: components, providers, hooks, tests, and documentation updated - Test file renamed: `gui.test.ts` → `builder.test.ts` - WASM getter: `dotrainGuiState` → `orderBuilderState` - **Bumped `lib/rain.interpreter` submodule** to pull in `OrderBuilderStateV1` rename from `rain.metadata`. - Updated all ARCHITECTURE.md files and README documentation. ## Checks By submitting this for review, I'm confirming I've done the following: - [ ] made this PR as small as possible - [ ] unit-tested any new functionality - [x] linked any relevant issues or PRs - [ ] included screenshots (if this involves a front-end change) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** - Replaces the GUI surface with a unified "Order Builder": deposits, field values, select-tokens, token info & balances, validation, state serialization/restoration, calldata/deployment-args generation, and related UI provider/hook + WASM/JS bindings. * **Documentation** - README and architecture docs updated to show builder-centric examples and workflow. * **Chores** - Added encoding/hash utility dependencies (base64, bincode, sha2). <!-- end of auto-generated comment: release notes by coderabbit.ai -->
graphite-app
Bot
force-pushed
the
2026-02-23-js-api-consolidation
branch
from
e7b44db to
4d894ae
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/ui-components/src/lib/components/deployment/DeploymentSteps.svelte (1)
1-333:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPlease attach a screenshot of the built webapp for this frontend change.
This file changes deployment UI behavior/content, but no screenshot is included in the review context.
As per coding guidelines
packages/{webapp,ui-components}/**/*.{svelte,ts,tsx,js,jsx}: “If you modify frontend code or functionality affecting the frontend, you MUST provide a screenshot of the built webapp reflecting your change.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ui-components/src/lib/components/deployment/DeploymentSteps.svelte` around lines 1 - 333, You modified the deployment UI (DeploymentSteps.svelte: components/select UI like SelectToken, Toggle showAdvancedOptions, Deploy button/Spinner and sections) but didn't attach a screenshot; build the webapp, open the page showing the changed DeploymentSteps UI (showing Select Tokens, the Toggle for advanced options, deposit/token IO inputs and the Deploy button/Spinner), take a clear screenshot, and attach it to the PR—add it to the PR description and upload the image to the repo (e.g. packages/ui-components/.github/pr-screenshots or the PR attachments) so reviewers can verify the visual change.crates/common/src/raindex_order_builder/order_operations.rs (2)
559-565:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't block deployment on Metaboard availability.
emit_meta_callis optional, but any Metaboard query failure or empty address set currently failsget_deployment_transaction_args. That makes the main deployment path in CLI/REST/UI depend on an external subgraph being up. Please degrade gracefully here instead of propagating the Metaboard error as a hard failure.Also applies to: 598-609
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/common/src/raindex_order_builder/order_operations.rs` around lines 559 - 565, The current logic in get_deployment_transaction_args treats any failure from should_emit_meta_call()/get_metaboard_client()/client.get_metaboard_addresses() and an empty address set (RaindexOrderBuilderError::NoAddressInMetaboardSubgraph) as a hard error; change it to degrade gracefully by treating Metaboard failures as "no meta call": call should_emit_meta_call() as before, but when creating the metaboard client or fetching addresses (get_metaboard_client(), client.get_metaboard_addresses()) handle errors and empty results by logging/debugging the error and returning Ok(false) for the emit_meta_call branch instead of propagating the error, and apply the same pattern to the similar block around the 598-609 code so get_deployment_transaction_args does not fail the deployment when the Metaboard subgraph is unavailable.
270-283:⚠️ Potential issue | 🟠 Major | ⚡ Quick winOnly emit
approvewhen the allowance is too small.This branch fires on any
!=, so it still generates approval calldata when the user already has a larger allowance or when the requested deposit is0. That adds unnecessary approval txs and can fail on zero-first ERC20s after a prior non-zero approval.Suggested fix
- let token_allowance = self.check_allowance(&deposit_args, &owner).await?; - let allowance_float = Float::from_fixed_decimal(token_allowance.allowance, decimals)?; - - if !allowance_float.eq(*deposit_amount)? { + let token_allowance = self.check_allowance(&deposit_args, &owner).await?; + let required_allowance = deposit_amount.to_fixed_decimal(decimals)?; + + if token_allowance.allowance < required_allowance { let calldata = approveCall { spender: tx_args.orderbook_address, - amount: deposit_amount.to_fixed_decimal(decimals)?, + amount: required_allowance, } .abi_encode();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/common/src/raindex_order_builder/order_operations.rs` around lines 270 - 283, The code currently emits an approve calldata whenever allowance_float != *deposit_amount, which also triggers when allowance is larger than needed or when deposit_amount is zero; change the condition in the block that builds approveCall/ApprovalCalldata (around check_allowance, token_allowance, allowance_float, deposit_amount, approveCall, to_fixed_decimal) to only create the approval when allowance_float is strictly less than the requested deposit_amount and skip generating any approval when deposit_amount is zero (or equivalent zero-check on deposit_amount) so approvals are only emitted when allowance is insufficient.
♻️ Duplicate comments (1)
crates/js_api/ARCHITECTURE.md (1)
40-42:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDocument the optional
settingsparameter and correct argument order in builder examples.
RaindexOrderBuilder.newWithDeployment(...)andRaindexOrderBuilder.newFromState(...)examples still omit the optionalsettingsargument (inserted before deployment key / serialized state). This makes the usage docs inconsistent with the current API surface.📚 Suggested doc fix
- - `RaindexOrderBuilder.newWithDeployment(dotrain, selectedDeployment, stateUpdateCallback?) -> RaindexOrderBuilder` + - `RaindexOrderBuilder.newWithDeployment(dotrain, settings?, selectedDeployment, stateUpdateCallback?) -> RaindexOrderBuilder` - - `RaindexOrderBuilder.newFromState(dotrain, serialized, callback?) -> RaindexOrderBuilder` + - `RaindexOrderBuilder.newFromState(dotrain, settings?, serialized, callback?) -> RaindexOrderBuilder` - - `const builder = await RaindexOrderBuilder.newWithDeployment(dotrain, deploymentKey, onStateChanged?)` + - `const builder = await RaindexOrderBuilder.newWithDeployment(dotrain, undefined, deploymentKey, onStateChanged?)` - - Persist UI state: read `serializeState()`; restore later with `RaindexOrderBuilder.newFromState(dotrain, serialized, callback?)`. + - Persist UI state: read `serializeState()`; restore later with `RaindexOrderBuilder.newFromState(dotrain, undefined, serialized, callback?)`.Also applies to: 107-107, 186-189
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/js_api/ARCHITECTURE.md` around lines 40 - 42, Update the examples for RaindexOrderBuilder to include the optional settings parameter and fix the argument order: when calling RaindexOrderBuilder.newWithDeployment(...) and RaindexOrderBuilder.newFromState(...), pass the settings object as the first argument before the deployment key or serialized state, and update all example invocations (including the ones around the noted blocks) so they match the current function signatures and parameter order.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ui-components/src/lib/components/deployment/DeploymentSteps.svelte`:
- Line 46: The prop onDeploy is currently typed/used as a synchronous callback
and is not awaited, causing checkingDeployment to be reset in finally before the
async deploy completes and allowing duplicate submissions; update the prop
signature to return a Promise (e.g., onDeploy: (raindexClient: RaindexClient,
builder: RaindexOrderBuilder) => Promise<void>), and in the deploy handler(s)
(the function that sets checkingDeployment and calls onDeploy at the end of the
file/around the 221-241 block) await the call to onDeploy inside the try block
so the finally block only runs after the async work finishes, preventing
premature re-enabling of the deploy button.
---
Outside diff comments:
In `@crates/common/src/raindex_order_builder/order_operations.rs`:
- Around line 559-565: The current logic in get_deployment_transaction_args
treats any failure from
should_emit_meta_call()/get_metaboard_client()/client.get_metaboard_addresses()
and an empty address set
(RaindexOrderBuilderError::NoAddressInMetaboardSubgraph) as a hard error; change
it to degrade gracefully by treating Metaboard failures as "no meta call": call
should_emit_meta_call() as before, but when creating the metaboard client or
fetching addresses (get_metaboard_client(), client.get_metaboard_addresses())
handle errors and empty results by logging/debugging the error and returning
Ok(false) for the emit_meta_call branch instead of propagating the error, and
apply the same pattern to the similar block around the 598-609 code so
get_deployment_transaction_args does not fail the deployment when the Metaboard
subgraph is unavailable.
- Around line 270-283: The code currently emits an approve calldata whenever
allowance_float != *deposit_amount, which also triggers when allowance is larger
than needed or when deposit_amount is zero; change the condition in the block
that builds approveCall/ApprovalCalldata (around check_allowance,
token_allowance, allowance_float, deposit_amount, approveCall, to_fixed_decimal)
to only create the approval when allowance_float is strictly less than the
requested deposit_amount and skip generating any approval when deposit_amount is
zero (or equivalent zero-check on deposit_amount) so approvals are only emitted
when allowance is insufficient.
In `@packages/ui-components/src/lib/components/deployment/DeploymentSteps.svelte`:
- Around line 1-333: You modified the deployment UI (DeploymentSteps.svelte:
components/select UI like SelectToken, Toggle showAdvancedOptions, Deploy
button/Spinner and sections) but didn't attach a screenshot; build the webapp,
open the page showing the changed DeploymentSteps UI (showing Select Tokens, the
Toggle for advanced options, deposit/token IO inputs and the Deploy
button/Spinner), take a clear screenshot, and attach it to the PR—add it to the
PR description and upload the image to the repo (e.g.
packages/ui-components/.github/pr-screenshots or the PR attachments) so
reviewers can verify the visual change.
---
Duplicate comments:
In `@crates/js_api/ARCHITECTURE.md`:
- Around line 40-42: Update the examples for RaindexOrderBuilder to include the
optional settings parameter and fix the argument order: when calling
RaindexOrderBuilder.newWithDeployment(...) and
RaindexOrderBuilder.newFromState(...), pass the settings object as the first
argument before the deployment key or serialized state, and update all example
invocations (including the ones around the noted blocks) so they match the
current function signatures and parameter order.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dd17c8cf-d34a-40bc-9160-e788b8fa873d
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (88)
.github/copilot-instructions.mdCargo.tomlai_commands/sdk-documentation-update.mdcrates/bindings/ARCHITECTURE.mdcrates/common/Cargo.tomlcrates/common/src/add_order.rscrates/common/src/dotrain_order.rscrates/common/src/lib.rscrates/common/src/parsed_meta.rscrates/common/src/raindex_client/orders.rscrates/common/src/raindex_order_builder/deposits.rscrates/common/src/raindex_order_builder/field_values.rscrates/common/src/raindex_order_builder/mod.rscrates/common/src/raindex_order_builder/order_operations.rscrates/common/src/raindex_order_builder/select_tokens.rscrates/common/src/raindex_order_builder/state_management.rscrates/common/src/raindex_order_builder/validation.rscrates/common/src/test_helpers.rscrates/js_api/ARCHITECTURE.mdcrates/js_api/src/gui/deposits.rscrates/js_api/src/gui/field_values.rscrates/js_api/src/gui/select_tokens.rscrates/js_api/src/lib.rscrates/js_api/src/raindex_order_builder/deposits.rscrates/js_api/src/raindex_order_builder/field_values.rscrates/js_api/src/raindex_order_builder/mod.rscrates/js_api/src/raindex_order_builder/order_operations.rscrates/js_api/src/raindex_order_builder/select_tokens.rscrates/js_api/src/raindex_order_builder/state_management.rscrates/js_api/src/registry.rscrates/settings/ARCHITECTURE.mdcrates/settings/src/deployment.rscrates/settings/src/lib.rscrates/settings/src/order.rscrates/settings/src/order_builder.rscrates/settings/src/yaml/context.rscrates/settings/src/yaml/dotrain.rscrates/settings/src/yaml/emitter.rscrates/settings/src/yaml/mod.rscrates/settings/src/yaml/orderbook.rscrates/test_fixtures/Cargo.tomllib/rain.interpreterpackages/orderbook/ARCHITECTURE.mdpackages/orderbook/README.mdpackages/orderbook/test/js_api/builder.test.tspackages/orderbook/test/js_api/dotrainRegistry.test.tspackages/ui-components/ARCHITECTURE.mdpackages/ui-components/src/__tests__/ComposedRainlangModal.test.tspackages/ui-components/src/__tests__/DeploymentSteps.test.tspackages/ui-components/src/__tests__/DepositInput.test.tspackages/ui-components/src/__tests__/DotrainRegistryProvider.test.tspackages/ui-components/src/__tests__/FieldDefinitionInput.test.tspackages/ui-components/src/__tests__/OrderDetail.test.tspackages/ui-components/src/__tests__/SelectToken.test.tspackages/ui-components/src/__tests__/TokenIOInput.test.tspackages/ui-components/src/__tests__/TokenSelectionModal.test.tspackages/ui-components/src/__tests__/handleShareChoices.test.tspackages/ui-components/src/__tests__/registry.test.tspackages/ui-components/src/lib/components/deployment/ComposedRainlangModal.sveltepackages/ui-components/src/lib/components/deployment/DeploymentSteps.sveltepackages/ui-components/src/lib/components/deployment/DepositInput.sveltepackages/ui-components/src/lib/components/deployment/FieldDefinitionInput.sveltepackages/ui-components/src/lib/components/deployment/SelectToken.sveltepackages/ui-components/src/lib/components/deployment/TokenIOInput.sveltepackages/ui-components/src/lib/components/deployment/TokenSelectionModal.sveltepackages/ui-components/src/lib/components/detail/OrderDetail.sveltepackages/ui-components/src/lib/errors/DeploymentStepsError.tspackages/ui-components/src/lib/hooks/useGui.test.tspackages/ui-components/src/lib/hooks/useGui.tspackages/ui-components/src/lib/hooks/useRaindexOrderBuilder.test.tspackages/ui-components/src/lib/hooks/useRaindexOrderBuilder.tspackages/ui-components/src/lib/index.tspackages/ui-components/src/lib/providers/GuiProvider.sveltepackages/ui-components/src/lib/providers/RaindexOrderBuilderProvider.sveltepackages/ui-components/src/lib/services/handleShareChoices.tspackages/ui-components/src/lib/services/registry.tspackages/ui-components/test-setup.tspackages/webapp/ARCHITECTURE.mdpackages/webapp/src/__tests__/handleAddOrder.test.tspackages/webapp/src/lib/constants.tspackages/webapp/src/lib/services/handleAddOrder.tspackages/webapp/src/lib/services/handleUpdateBuilderState.tspackages/webapp/src/routes/deploy/[orderName]/[deploymentKey]/+page.sveltepackages/webapp/src/routes/deploy/[orderName]/[deploymentKey]/fullDeployment.test.tspackages/webapp/src/routes/deploy/[orderName]/[deploymentKey]/page.test.tspointers.shprep-base.shsrc/concrete/parser/OrderBookV6SubParser.sol
💤 Files with no reviewable changes (6)
- packages/ui-components/src/lib/hooks/useGui.ts
- packages/ui-components/src/lib/providers/GuiProvider.svelte
- crates/js_api/src/gui/deposits.rs
- packages/ui-components/src/lib/hooks/useGui.test.ts
- crates/js_api/src/gui/select_tokens.rs
- crates/js_api/src/gui/field_values.rs
✅ Files skipped from review due to trivial changes (12)
- crates/test_fixtures/Cargo.toml
- lib/rain.interpreter
- crates/common/src/lib.rs
- ai_commands/sdk-documentation-update.md
- packages/webapp/ARCHITECTURE.md
- crates/settings/src/yaml/orderbook.rs
- crates/settings/src/order.rs
- packages/webapp/src/lib/services/handleUpdateBuilderState.ts
- packages/orderbook/ARCHITECTURE.md
- crates/common/src/test_helpers.rs
- packages/ui-components/ARCHITECTURE.md
- crates/bindings/ARCHITECTURE.md
🚧 Files skipped from review as they are similar to previous changes (56)
- packages/ui-components/src/lib/components/deployment/ComposedRainlangModal.svelte
- Cargo.toml
- packages/ui-components/src/lib/index.ts
- .github/copilot-instructions.md
- crates/js_api/src/lib.rs
- packages/ui-components/src/lib/services/registry.ts
- packages/ui-components/src/lib/providers/RaindexOrderBuilderProvider.svelte
- packages/ui-components/src/lib/hooks/useRaindexOrderBuilder.test.ts
- crates/common/Cargo.toml
- packages/webapp/src/lib/constants.ts
- packages/ui-components/src/tests/OrderDetail.test.ts
- crates/common/src/dotrain_order.rs
- packages/ui-components/src/tests/handleShareChoices.test.ts
- packages/ui-components/src/lib/errors/DeploymentStepsError.ts
- packages/ui-components/src/lib/services/handleShareChoices.ts
- crates/settings/src/yaml/mod.rs
- packages/ui-components/src/lib/components/deployment/FieldDefinitionInput.svelte
- packages/ui-components/src/lib/components/deployment/TokenSelectionModal.svelte
- packages/ui-components/test-setup.ts
- packages/webapp/src/lib/services/handleAddOrder.ts
- crates/settings/src/yaml/emitter.rs
- crates/js_api/src/raindex_order_builder/select_tokens.rs
- packages/webapp/src/tests/handleAddOrder.test.ts
- packages/ui-components/src/tests/TokenIOInput.test.ts
- crates/settings/src/lib.rs
- packages/ui-components/src/lib/hooks/useRaindexOrderBuilder.ts
- packages/ui-components/src/tests/DeploymentSteps.test.ts
- crates/js_api/src/raindex_order_builder/field_values.rs
- crates/common/src/add_order.rs
- crates/js_api/src/raindex_order_builder/state_management.rs
- packages/ui-components/src/tests/registry.test.ts
- crates/common/src/raindex_order_builder/field_values.rs
- packages/ui-components/src/lib/components/deployment/DepositInput.svelte
- crates/common/src/parsed_meta.rs
- crates/common/src/raindex_order_builder/deposits.rs
- crates/js_api/src/registry.rs
- crates/js_api/src/raindex_order_builder/order_operations.rs
- crates/settings/src/yaml/dotrain.rs
- packages/ui-components/src/tests/FieldDefinitionInput.test.ts
- packages/webapp/src/routes/deploy/[orderName]/[deploymentKey]/+page.svelte
- crates/common/src/raindex_order_builder/select_tokens.rs
- crates/common/src/raindex_client/orders.rs
- packages/ui-components/src/tests/DotrainRegistryProvider.test.ts
- packages/ui-components/src/lib/components/detail/OrderDetail.svelte
- crates/settings/src/yaml/context.rs
- packages/ui-components/src/lib/components/deployment/SelectToken.svelte
- packages/ui-components/src/tests/SelectToken.test.ts
- packages/ui-components/src/tests/TokenSelectionModal.test.ts
- crates/common/src/raindex_order_builder/validation.rs
- crates/settings/src/order_builder.rs
- crates/common/src/raindex_order_builder/mod.rs
- crates/js_api/src/raindex_order_builder/mod.rs
- crates/common/src/raindex_order_builder/state_management.rs
- packages/orderbook/test/js_api/builder.test.ts
- crates/js_api/src/raindex_order_builder/deposits.rs
- crates/settings/src/deployment.rs
| export let orderDetail: NameAndDescriptionCfg; | ||
| /** Handlers for deployment modals */ | ||
| export let onDeploy: (raindexClient: RaindexClient, gui: DotrainOrderGui) => void; | ||
| export let onDeploy: (raindexClient: RaindexClient, builder: RaindexOrderBuilder) => void; |
Contributor
There was a problem hiding this comment.
Await onDeploy and type it as async-capable to prevent duplicate submissions.
checkingDeployment is reset in finally before an async deploy finishes because onDeploy(...) is not awaited. This can re-enable the button too early and trigger duplicate deploy attempts.
✅ Suggested fix
-export let onDeploy: (raindexClient: RaindexClient, builder: RaindexOrderBuilder) => void;
+export let onDeploy: (
+ raindexClient: RaindexClient,
+ builder: RaindexOrderBuilder
+) => void | Promise<void>;
...
- return onDeploy(raindexClient, builder);
+ await onDeploy(raindexClient, builder);
+ return;Also applies to: 221-241
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/ui-components/src/lib/components/deployment/DeploymentSteps.svelte`
at line 46, The prop onDeploy is currently typed/used as a synchronous callback
and is not awaited, causing checkingDeployment to be reset in finally before the
async deploy completes and allowing duplicate submissions; update the prop
signature to return a Promise (e.g., onDeploy: (raindexClient: RaindexClient,
builder: RaindexOrderBuilder) => Promise<void>), and in the deploy handler(s)
(the function that sets checkingDeployment and calls onDeploy at the end of the
file/around the 221-241 block) await the call to onDeploy inside the try block
so the finally block only runs after the async work finishes, preventing
premature re-enabling of the deploy button.
Contributor
|
@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment: S/M/L PR Classification Guidelines:This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed. Small (S)Characteristics:
Review Effort: Would have taken 5-10 minutes Examples:
Medium (M)Characteristics:
Review Effort: Would have taken 15-30 minutes Examples:
Large (L)Characteristics:
Review Effort: Would have taken 45+ minutes Examples:
Additional Factors to ConsiderWhen deciding between sizes, also consider:
Notes:
|
Contributor
|
Tip For best results, initiate chat on the files or code changes. SIZE=L |
thedavidmeister
pushed a commit
that referenced
this pull request
May 19, 2026
…aindex Resolves 37 conflicts: - UD deletes: dropped vercel-preview-pr-target.yaml (removed in #2573) and the leftover js_api/gui/{deposits,select_tokens}.rs (logic moved to common/raindex_order_builder/ via #2479). - UU code conflicts: kept the orderbook->raindex rename across crates, packages, and Solidity while taking in main's new functionality (multi-subgraph trades_list/trades_count, GUI->OrderBuilder rename, BaseRainterpreterSubParser->BaseRainlangSubParser, ContextProfile::Gui ->ContextProfile::Builder, get_orderbook_yaml->get_raindex_yaml, Orderbook::multicallCall->Raindex::multicallCall). - Lockfiles: Cargo.lock regenerated via cargo generate-lockfile; package-lock.json via npm install --package-lock-only. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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.
Dependent PRs
Motivation
The order builder logic (formerly "GUI") lived entirely in
crates/js_api/src/gui/, tightly coupled to WASM andJsValue. This made it impossible to reuse from the CLI, REST API, or other Rust consumers. Additionally, the "GUI" naming was misleading since the logic is not UI-specific — it handles field values, deposits, token selection, state serialization, and order calldata generation.Solution
crates/common/src/raindex_order_builder/— field values, deposits, select tokens, order operations, state management, and validation now live in a platform-agnostic common crate with no WASM dependencies.crates/js_api/src/raindex_order_builder/to thin WASM wrappers that delegate to the common crate, keeping onlyJsValueconversion and callback handling.DotrainOrderGui→RaindexOrderBuilder,DotrainGuiStateV1→OrderBuilderStateV1,GuiCfg→OrderBuilderCfg, etc.gui:→builder:gui.rs→order_builder.rsgui.test.ts→builder.test.tsdotrainGuiState→orderBuilderStatelib/rain.interpretersubmodule to pull inOrderBuilderStateV1rename fromrain.metadata.Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit
New Features
Documentation
Chores