Update Registry, Strat commit#1970
Closed
Siddharth2207 wants to merge 1 commit intomainfrom
Closed
Conversation
Contributor
WalkthroughThis change updates hardcoded GitHub raw content URLs in both a constant and a fetch call to reference a new commit hash for the Changes
Possibly related PRs
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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: 2
🔭 Outside diff range comments (1)
packages/webapp/src/lib/constants.ts (1)
1-3: Avoid hard-coding the same commit hash in multiple placesThis constant is great, but the new hash is still duplicated in
routes/+layout.tsand its tests. Import this constant there and build the URL with a template string to guarantee the two always stay in sync.- 'https://raw.githubusercontent.com/rainlanguage/rain.strategies/89cb164fcd8e314483d496287635869281f63236/registry'; +// points to `<commit>/registry` +export const REGISTRY_URL = + `https://raw.githubusercontent.com/rainlanguage/rain.strategies/${'89cb164fcd8e314483d496287635869281f63236'}/registry`;(see follow-up diffs in the layout file).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
packages/webapp/src/lib/constants.ts(1 hunks)packages/webapp/src/routes/+layout.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T23:09:46.344Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must reassess the PR size after each new commit to ensure the report includes the latest changes, and calculate the total size up to the very latest commit.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:26:31.566Z
Learning: For PR #1884 and future PRs in rainlanguage/rain.orderbook repository, provide PR size reports in a separate comment with ONLY the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number", without any additional text or explanation. Reports should exclude irrelevant files such as lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T22:14:22.646Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any additional text or formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T22:14:22.646Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, I must consistently report the PR size after EVERY new commit in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number" without any formatting blocks. The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and can be included either as part of the review or as a separate comment.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T23:34:42.328Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, the PR size report must use the format:
TOTAL:number
ADDITIONS:number
DELETIONS:number
with no extra text, formatting blocks, or explanations, and must still exclude lock files and reassess after each new commit.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T23:09:27.578Z
Learning: For PR #1884 in rainlanguage/rain.orderbook repository, I must always reassess the PR size after each new commit, calculating the total changes up to the very latest commit and ensuring accuracy of the report. The calculation must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock) and report in the exact format "TOTAL=number\nADDITIONS=number\nDELETIONS=number".
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T21:25:47.873Z
Learning: After each new commit review for rainlanguage/rain.orderbook repository, generate a PR size report in a separate comment with the format: "TOTAL=number ADDITIONS=number DELETIONS=number". This report should exclude irrelevant files such as lock files (e.g., package-lock.json, cargo.lock).
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T22:46:08.530Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, the PR size report should be the only content in the comment - no text before it, no text after it, no formatting blocks, just the raw report in the exact format: "TOTAL=number\nADDITIONS=number\nDELETIONS=number". The report must exclude lock files (package-lock.json, yarn.lock, pnpm-lock.yaml, cargo.lock).
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T22:47:21.927Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, the PR size report must always be placed at the very beginning of any comment, before any learning used section or other content.
Learnt from: rouzwelt
PR: rainlanguage/rain.orderbook#0
File: :0-0
Timestamp: 2025-05-21T22:47:21.927Z
Learning: For PR #1884 in rainlanguage/rain.orderbook, the PR size report must always be placed at the very beginning of any comment, before any learning used section or other content.
packages/webapp/src/routes/+layout.ts (12)
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1916
File: packages/ui-components/src/lib/__fixtures__/settings-12-11-24.json:182-195
Timestamp: 2025-06-10T12:04:54.107Z
Learning: In test fixture files like `packages/ui-components/src/lib/__fixtures__/settings-12-11-24.json`, network configuration inconsistencies (such as matchain using Polygon's RPC, chainId, and currency while having its own network key) are acceptable since they are used for testing purposes only.
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1785
File: packages/webapp/src/routes/+layout.ts:22-22
Timestamp: 2025-05-13T10:19:19.558Z
Learning: Making the settings URL configurable via environment variables was considered out of scope for PR #1785, which focused specifically on adding error handling for config fetching.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1917
File: tauri-app/src/lib/stores/order.ts:10-15
Timestamp: 2025-06-11T11:27:14.391Z
Learning: In this codebase, Svelte writable/derived stores (e.g., `subgraph` in `tauri-app/src/lib/stores/settings.ts`) expose a custom asynchronous `.load()` helper that retrieves the current value, so calls like `await subgraph.load()` are valid.
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1565
File: packages/webapp/src/routes/deploy/layout.test.ts:12-29
Timestamp: 2025-04-07T08:18:36.473Z
Learning: In test files for this project, hardingjam prefers to use custom mocks (such as for localStorage) rather than relying on environment-provided implementations, as this allows for spying on individual methods and having precise control over implementation details for more robust testing.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1926
File: packages/ui-components/src/lib/__mocks__/stores.ts:13-17
Timestamp: 2025-06-30T14:17:16.626Z
Learning: User findolor reports that vi.mock(import('@rainlanguage/orderbook'), async (importOriginal) => { ... }) syntax works in their testing environment, despite official Vitest documentation indicating the first argument should be a string. This suggests there may be specific Vitest versions or configurations that support dynamic import() as the first argument to vi.mock().
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1956
File: packages/ui-components/src/__tests__/VaultDetail.test.ts:177-181
Timestamp: 2025-07-04T09:14:35.748Z
Learning: In the rain.orderbook project, maintainer findolor is fine with mutating mock data directly in test files using @ts-expect-error comments, preferring this pragmatic approach over immutable patterns like spread operators for test data updates.
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1565
File: packages/webapp/src/__tests__/registryManager.test.ts:20-21
Timestamp: 2025-04-09T09:40:03.131Z
Learning: When mocking browser APIs like localStorage for tests, it's not always necessary to fully implement all interface properties/methods with dynamic behavior. A hardcoded value (like setting length: 0) can be sufficient when the tests don't rely on that specific property.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1891
File: packages/webapp/src/routes/deploy/[strategyName]/[deploymentKey]/page.test.ts:66-80
Timestamp: 2025-06-08T18:43:51.842Z
Learning: In the rain.orderbook webapp test files, when mocking objects like the transaction manager, it's acceptable to use simple empty objects with @ts-expect-error comments rather than providing complete mock implementations with all properties and methods.
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1700
File: tauri-app/src/lib/mocks/mockConfigSource.ts:6-6
Timestamp: 2025-04-28T10:58:11.124Z
Learning: In mock data for testing in this codebase, it's acceptable to use URL placeholders like 'https://mainnet.infura.io/v3/YOUR-PROJECT-ID' as they clearly indicate the expected format for actual implementation.
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1559
File: packages/ui-components/src/__tests__/OrderOrVaultHash.test.ts:94-94
Timestamp: 2025-04-04T11:25:21.518Z
Learning: In the rain.orderbook project, minimal test fixtures are preferred over complete mocks that implement the entire interface. Type casting (e.g., `as unknown as SgVault`) is an acceptable approach to maintain both minimal fixtures and TypeScript type compatibility.
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1597
File: packages/ui-components/src/__tests__/OrderDetail.test.ts:120-120
Timestamp: 2025-04-08T09:18:46.653Z
Learning: In test files for the Rain Orderbook project, it's acceptable to bypass TypeScript's strict typing using constructs like `as unknown as [Type]` to create simplified mock objects that don't need to implement the entire interface.
Learnt from: findolor
PR: rainlanguage/rain.orderbook#1936
File: packages/ui-components/vite.config.ts:21-23
Timestamp: 2025-06-17T14:55:22.914Z
Learning: In the rain.orderbook project, the Vite configuration uses `'import.meta.vitest': 'undefined'` (as a string) combined with conditional `if (import.meta.vitest)` checks for in-source testing. The mock files are excluded from test execution using `exclude: ['src/lib/__mocks__/**/*.ts']`. This configuration successfully allows dev builds to work without `vi` undefined errors, despite the theoretical expectation that the string "undefined" would be truthy and cause issues.
packages/webapp/src/lib/constants.ts (5)
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1565
File: packages/webapp/src/lib/services/loadRegistryUrl.ts:5-19
Timestamp: 2025-04-09T12:58:03.399Z
Learning: In the rain.orderbook application, URL validation for registry URLs is handled at the frontend/component level before the `loadRegistryUrl` function is called, so additional validation within this function is not necessary.
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1565
File: packages/webapp/src/__tests__/registryManager.test.ts:227-238
Timestamp: 2025-04-09T13:00:24.640Z
Learning: In the rain.orderbook project, error handling was added to the RegistryManager to handle localStorage exceptions, addressing part of the verification agent's suggestions. The team should also consider testing extremely long URLs that might approach browser limits.
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1705
File: packages/ui-components/src/lib/providers/registry/RegistryManager.ts:123-137
Timestamp: 2025-05-09T12:28:36.931Z
Learning: Handling relative URLs in the `updateUrlWithRegistry` method of the RegistryManager class is not a requirement for the registry URL management system.
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1565
File: packages/webapp/src/routes/deploy/+layout.svelte:12-16
Timestamp: 2025-04-09T09:59:04.121Z
Learning: In the Rain Orderbook application, a full page reload (window.location.reload()) is needed when changing the registry URL to ensure proper state reinitialization, rather than using SvelteKit's soft navigation methods.
Learnt from: hardingjam
PR: rainlanguage/rain.orderbook#1565
File: packages/webapp/src/lib/services/loadRegistryUrl.ts:21-95
Timestamp: 2025-04-09T12:57:52.799Z
Learning: In the loadRegistryUrl function, invalid URL format validation is handled by the fetchRegistryDotrains function rather than explicit URL format validation within loadRegistryUrl itself. The error handling in the catch block will handle any failures from the fetch operation.
⏰ Context from checks skipped due to timeout of 90000ms (16)
- GitHub Check: test
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-legal)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-test)
- GitHub Check: standard-tests (ubuntu-latest, test-js-bindings)
- GitHub Check: standard-tests (ubuntu-latest, rainix-wasm-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-artifacts)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-test)
- GitHub Check: standard-tests (ubuntu-latest, rainix-sol-static)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-static)
- GitHub Check: test
- GitHub Check: standard-tests (ubuntu-latest, ob-rs-test, true)
- GitHub Check: standard-tests (ubuntu-latest, rainix-rs-artifacts, true)
- GitHub Check: Deploy-Preview
- GitHub Check: build-tauri (ubuntu-22.04, true)
- GitHub Check: test
- GitHub Check: git-clean
Comment on lines
247
to
249
| expect(mockFetch).toHaveBeenCalledWith( | ||
| 'https://raw.githubusercontent.com/rainlanguage/rain.strategies/a42d1f611f2169302c04ea809c09a7e37177cf0b/settings.yaml' | ||
| 'https://raw.githubusercontent.com/rainlanguage/rain.strategies/89cb164fcd8e314483d496287635869281f63236/settings.yaml' | ||
| ); |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Mirror the constant in the test to stay DRY
Once the production code imports REGISTRY_URL, the expectation here should too; otherwise a future hash bump will break tests.
- expect(mockFetch).toHaveBeenCalledWith(
- 'https://raw.githubusercontent.com/rainlanguage/rain.strategies/89cb164fcd8e314483d496287635869281f63236/settings.yaml'
- );
+ expect(mockFetch).toHaveBeenCalledWith(`${REGISTRY_URL}/../settings.yaml`);Remember to import { REGISTRY_URL } from '$lib/constants' at the top of the vitest block.
📝 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
| expect(mockFetch).toHaveBeenCalledWith( | |
| 'https://raw.githubusercontent.com/rainlanguage/rain.strategies/a42d1f611f2169302c04ea809c09a7e37177cf0b/settings.yaml' | |
| 'https://raw.githubusercontent.com/rainlanguage/rain.strategies/89cb164fcd8e314483d496287635869281f63236/settings.yaml' | |
| ); | |
| - expect(mockFetch).toHaveBeenCalledWith( | |
| - 'https://raw.githubusercontent.com/rainlanguage/rain.strategies/89cb164fcd8e314483d496287635869281f63236/settings.yaml' | |
| - ); | |
| + expect(mockFetch).toHaveBeenCalledWith(`${REGISTRY_URL}/../settings.yaml`); |
🤖 Prompt for AI Agents
In packages/webapp/src/routes/+layout.ts around lines 247 to 249, the test uses
a hardcoded URL string that includes a commit hash, which can break tests when
the hash changes. To fix this, import the REGISTRY_URL constant from
'$lib/constants' at the top of the test file and replace the hardcoded URL in
the expect(mockFetch).toHaveBeenCalledWith call with REGISTRY_URL to keep the
test DRY and resilient to changes.
Comment on lines
22
to
24
| const response = await fetch( | ||
| 'https://raw.githubusercontent.com/rainlanguage/rain.strategies/a42d1f611f2169302c04ea809c09a7e37177cf0b/settings.yaml' | ||
| 'https://raw.githubusercontent.com/rainlanguage/rain.strategies/89cb164fcd8e314483d496287635869281f63236/settings.yaml' | ||
| ); |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use the shared constant rather than another string literal
The exact hash already lives in src/lib/constants.ts. Import it once and derive the YAML path to eliminate future drift.
-import type { AppStoresInterface } from '@rainlanguage/ui-components';
+import type { AppStoresInterface } from '@rainlanguage/ui-components';
+import { REGISTRY_URL } from '$lib/constants';
...
- const response = await fetch(
- 'https://raw.githubusercontent.com/rainlanguage/rain.strategies/89cb164fcd8e314483d496287635869281f63236/settings.yaml'
- );
+ const response = await fetch(`${REGISTRY_URL}/../settings.yaml`);(Adjust the path composition if you prefer a dedicated STRATEGY_RAW_BASE constant.)
📝 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
| const response = await fetch( | |
| 'https://raw.githubusercontent.com/rainlanguage/rain.strategies/a42d1f611f2169302c04ea809c09a7e37177cf0b/settings.yaml' | |
| 'https://raw.githubusercontent.com/rainlanguage/rain.strategies/89cb164fcd8e314483d496287635869281f63236/settings.yaml' | |
| ); | |
| import type { AppStoresInterface } from '@rainlanguage/ui-components'; | |
| import { REGISTRY_URL } from '$lib/constants'; | |
| ... | |
| const response = await fetch(`${REGISTRY_URL}/../settings.yaml`); |
🤖 Prompt for AI Agents
In packages/webapp/src/routes/+layout.ts around lines 22 to 24, replace the
hardcoded commit hash string in the fetch URL with the shared constant imported
from src/lib/constants.ts to avoid duplication and potential drift. Import the
constant at the top of the file and use it to construct the full URL
dynamically, optionally creating a dedicated base URL constant if preferred for
clarity.
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.
Motivation
Update Registry, Strat commit
Checks
By submitting this for review, I'm confirming I've done the following:
Summary by CodeRabbit