Consolidate JS API gui module into common crate and rename GUI to OrderBuilder#2479
Open
Consolidate JS API gui module into common crate and rename GUI to OrderBuilder#2479
Conversation
Move core order builder logic (field values, deposits, token selection, validation, order operations, state management) into the common crate so it can be shared across targets. Add cfg gates for httpmock-dependent test submodules that cannot compile on WASM.
Delete the old gui/ module containing duplicated business logic and replace it with raindex_order_builder/ thin wrappers that delegate to the common crate's RaindexOrderBuilder. Update registry.rs references from DotrainOrderGui/GuiError to RaindexOrderBuilder/RaindexOrderBuilderWasmError.
The consolidation refactor dropped tsify annotations on DepositCalldataResult, IOVaultIds, and WithdrawCalldataResult, causing undefined Bytes/U256 types in generated TypeScript declarations.
- Export executeStateUpdateCallback as WASM method - Restore per-field callback in setFieldValues (was firing once per batch) - Add preserve_js_class to getAccountBalance so Float methods are available - Rename DotrainOrderGui to RaindexOrderBuilder in tests
Rename variables, functions, error variants, WASM method names, test descriptions, and documentation from gui terminology to builder terminology, aligning with the DotrainOrderGui to RaindexOrderBuilder class rename. YAML gui: config keys and settings crate types (GuiCfg, GuiDeploymentCfg, etc.) are unchanged as they map to the YAML schema.
…lder Renames gui.rs to order_builder.rs, changes all Gui-prefixed structs/enums/traits to use OrderBuilder prefix, updates the YAML keyword from "gui" to "builder", and fixes all references across settings, common, js_api crates, JS test files, and UI components.
…ferences Bump rain.interpreter submodule to pull in the OrderBuilderStateV1 rename from rain.metadata. Update all Rust types, imports, variables, wasm_bindgen getters, test fixtures, Svelte/TS references, and documentation to replace GUI terminology with builder/OrderBuilder throughout the codebase.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
WalkthroughA large refactor renaming and migrating the GUI-based order builder surface to a new RaindexOrderBuilder: adds a new raindex_order_builder module in common, introduces Wasm bindings under js_api for the builder, renames many settings/types from gui→order_builder, updates UI/webapp hooks/providers/components/tests, and adds dependencies and metadata handling for OrderBuilderStateV1. Changes
Sequence Diagram(s)sequenceDiagram
participant JS_Client
participant WASM_Raindex as RaindexOrderBuilder (WASM)
participant Inner_Rust as RaindexOrderBuilderInner
participant Settings as OrderBuilderCfg/Registry
JS_Client->>WASM_Raindex: newWithDeployment(dotrain, settings, deploymentKey, callback)
WASM_Raindex->>Inner_Rust: RaindexOrderBuilder::new_with_deployment(...)
Inner_Rust->>Settings: parse YAML -> get builder config
Settings-->>Inner_Rust: OrderBuilderCfg (deployments, fields, tokens)
Inner_Rust-->>WASM_Raindex: RaindexOrderBuilder instance
WASM_Raindex-->>JS_Client: returned RaindexOrderBuilder (WASM wrapper)
JS_Client->>WASM_Raindex: setSelectToken(key, address)
WASM_Raindex->>Inner_Rust: inner.set_select_token(...)
Inner_Rust->>Settings: update YAML record / validation
Inner_Rust-->>WASM_Raindex: Result (ok) -> triggers state callback to JS_Client
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 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
+1
to
+90
| use super::*; | ||
| use alloy::primitives::Address; | ||
| use rain_orderbook_common::raindex_order_builder::deposits as inner_deposits; | ||
|
|
||
| #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Tsify)] | ||
| pub struct TokenDeposit { | ||
| pub token: String, | ||
| pub amount: String, | ||
| #[tsify(type = "string")] | ||
| pub address: Address, | ||
| } | ||
| impl_wasm_traits!(TokenDeposit); | ||
|
|
||
| impl From<inner_deposits::TokenDeposit> for TokenDeposit { | ||
| fn from(td: inner_deposits::TokenDeposit) -> Self { | ||
| Self { | ||
| token: td.token, | ||
| amount: td.amount, | ||
| address: td.address, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| #[wasm_export] | ||
| impl RaindexOrderBuilder { | ||
| #[wasm_export( | ||
| js_name = "getDeposits", | ||
| unchecked_return_type = "TokenDeposit[]", | ||
| return_description = "Array of configured deposits with token info and amounts" | ||
| )] | ||
| pub fn get_deposits(&self) -> Result<Vec<TokenDeposit>, RaindexOrderBuilderWasmError> { | ||
| Ok(self | ||
| .inner | ||
| .get_deposits()? | ||
| .into_iter() | ||
| .map(Into::into) | ||
| .collect()) | ||
| } | ||
|
|
||
| #[wasm_export(js_name = "setDeposit", unchecked_return_type = "void")] | ||
| pub async fn set_deposit( | ||
| &mut self, | ||
| #[wasm_export(param_description = "Token key from the YAML configuration")] token: String, | ||
| #[wasm_export(param_description = "Deposit amount as a decimal string")] amount: String, | ||
| ) -> Result<(), RaindexOrderBuilderWasmError> { | ||
| self.inner.set_deposit(token, amount).await?; | ||
| self.execute_state_update_callback()?; | ||
| Ok(()) | ||
| } | ||
|
|
||
| #[wasm_export(js_name = "unsetDeposit", unchecked_return_type = "void")] | ||
| pub fn unset_deposit( | ||
| &mut self, | ||
| #[wasm_export(param_description = "Token key to remove deposit for")] token: String, | ||
| ) -> Result<(), RaindexOrderBuilderWasmError> { | ||
| self.inner.unset_deposit(token)?; | ||
| self.execute_state_update_callback()?; | ||
| Ok(()) | ||
| } | ||
|
|
||
| #[wasm_export( | ||
| js_name = "getDepositPresets", | ||
| unchecked_return_type = "string[]", | ||
| return_description = "Array of preset deposit amounts" | ||
| )] | ||
| pub fn get_deposit_presets( | ||
| &self, | ||
| #[wasm_export(param_description = "Token key to get presets for")] key: String, | ||
| ) -> Result<Vec<String>, RaindexOrderBuilderWasmError> { | ||
| Ok(self.inner.get_deposit_presets(key)?) | ||
| } | ||
|
|
||
| #[wasm_export( | ||
| js_name = "getMissingDeposits", | ||
| unchecked_return_type = "string[]", | ||
| return_description = "Array of token keys that need deposits" | ||
| )] | ||
| pub fn get_missing_deposits(&self) -> Result<Vec<String>, RaindexOrderBuilderWasmError> { | ||
| Ok(self.inner.get_missing_deposits()?) | ||
| } | ||
|
|
||
| #[wasm_export( | ||
| js_name = "hasAnyDeposit", | ||
| unchecked_return_type = "boolean", | ||
| return_description = "Whether any deposit has been set" | ||
| )] | ||
| pub fn has_any_deposit(&self) -> Result<bool, RaindexOrderBuilderWasmError> { | ||
| Ok(self.inner.has_any_deposit()?) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Please confirm Rust formatting/lint/test runs for the new WASM bindings.
Suggested commands:
nix develop -c cargo fmt --allnix develop -c rainix-rs-staticnix develop -c cargo buildnix develop -c cargo clippy --workspace --all-targets --all-features -D warningsnix develop -c cargo test --workspace
As per coding guidelines, “crates//*.rs: Run nix develop -c cargo build … clippy … cargo test …” and “/*.rs: Format Rust code with nix develop -c cargo fmt --all; Lint Rust code with nix develop -c rainix-rs-static …”.
🤖 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/deposits.rs` around lines 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.
Comment on lines
+138
to
142
| /// Retrieves the complete builder configuration including all deployments. | ||
| /// | ||
| /// This returns the parsed GUI section from the YAML, filtered to include only | ||
| /// This returns the parsed builder section from the YAML, filtered to include only | ||
| /// the current deployment. Use this to access order-level metadata. | ||
| /// |
Contributor
There was a problem hiding this comment.
Clarify getBuilderConfig docs (current deployment vs all).
The header says “complete configuration including all deployments,” but the description says it’s filtered to the current deployment.
✍️ Suggested wording
-/// Retrieves the complete builder configuration including all deployments.
-///
-/// This returns the parsed builder section from the YAML, filtered to include only
-/// the current deployment. Use this to access order-level metadata.
+/// Retrieves the builder configuration for the currently selected deployment.
+///
+/// This returns the parsed builder section from the YAML filtered to the current
+/// deployment. Use this to access order-level metadata.📝 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
| /// Retrieves the complete builder configuration including all deployments. | |
| /// | |
| /// This returns the parsed GUI section from the YAML, filtered to include only | |
| /// This returns the parsed builder section from the YAML, filtered to include only | |
| /// the current deployment. Use this to access order-level metadata. | |
| /// | |
| /// Retrieves the builder configuration for the currently selected deployment. | |
| /// | |
| /// This returns the parsed builder section from the YAML filtered to the current | |
| /// deployment. Use this to access order-level metadata. | |
| /// |
🤖 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 - 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.
Comment on lines
+273
to
276
| /// - `builder.name` - Order display name | ||
| /// - `builder.description` - Full order description | ||
| /// - `builder.short-description` - Brief summary (optional but recommended) | ||
| /// |
Contributor
There was a problem hiding this comment.
Docs say short-description is optional, but tests treat it as required.
If it’s required (as the tests indicate), the docs should reflect that.
✍️ Suggested wording
-/// - `builder.short-description` - Brief summary (optional but recommended)
+/// - `builder.short-description` - Brief summary (required)🤖 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 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.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
Chores