Merged
Conversation
Show open interest in USD by applying mark price and remove the redundant USDC suffix.
Use Effect Order module with tri-state cycling (desc → asc → none) for all market table columns via sortable headers with direction icons.
…et on markets change
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
📝 WalkthroughWalkthroughIntroduces a Hyperliquid real-time data service, adds mid-price streaming atoms and UI price-flash, expands API schemas (markets, candles, actions, transactions), refactors atom patterns to use AtomRefs/Atom.make and field atom APIs, updates wallet/signer typings, and tweaks build/config and various UI components. Changes
Sequence DiagramsequenceDiagram
participant App as App / Preload
participant Preload as PreloadAtoms
participant MidAtom as midPriceAtom
participant HL as HyperliquidService
participant WS as WebSocketTransport
participant UpdateMk as updateMarketsMidPriceAtom
participant Markets as marketsAtom
participant UI as Market UI
App->>Preload: useAtomMount(midPriceAtom)
Preload->>MidAtom: subscribe (runtimeAtom -> HyperliquidService)
MidAtom->>HL: use HyperliquidService.subscribeMidPrice()
HL->>WS: open WebSocket
WS-->>HL: stream AllMidsWsEvent
HL-->>MidAtom: emit stream (broadcastDynamic)
App->>Preload: useAtomMount(updateMarketsMidPriceAtom)
UpdateMk->>MidAtom: read mids
MidAtom-->>UpdateMk: price events
UpdateMk->>Markets: lookup market AtomRefs via marketsBySymbolAtom
Markets-->>UpdateMk: return AtomRefs
UpdateMk->>Markets: set markPrice on market AtomRefs
Markets->>UI: notify subscribers
UI->>UI: render PriceFlash animation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Philippoes
previously approved these changes
Apr 2, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/common/src/services/wallet/browser-signer.ts (1)
118-120:⚠️ Potential issue | 🟠 MajorDo not convert chain-switch failures into defects in
signTransaction.Line 119 uses
Effect.orDie, soSwitchChainErrorbecomes uncatchable and bypasses the typed error channel/UI recovery path.Suggested fix
- if (chainId !== wagmiAdapter.wagmiConfig.state.chainId) { - yield* switchChain(chainId).pipe(Effect.orDie); - } + if (chainId !== wagmiAdapter.wagmiConfig.state.chainId) { + yield* switchChain(chainId); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/common/src/services/wallet/browser-signer.ts` around lines 118 - 120, The current signTransaction flow converts chain-switch failures into defects by using Effect.orDie on the switchChain call; update signTransaction so switchChain(chainId) is not wrapped with orDie and instead propagates typed SwitchChainError through the effect error channel (e.g., remove Effect.orDie and use the effect's normal error propagation or map/catch to a typed error), ensuring errors from switchChain (referenced in switchChain and wagmiAdapter.wagmiConfig.state.chainId) are returnable/handleable by callers/UI rather than becoming uncatchable defects.packages/dashboard/src/components/modules/trade/order-form/index.tsx (1)
404-417:⚠️ Potential issue | 🟡 MinorOnly the final detail row should pass
isLast.
Fees,Taker Fee, andMaker Feeare all marked as last rows now, so this section renders stacked bottom borders between rows. LeaveisLaston the actual final row only.Suggested fix
<OrderDetailRow label="Fees" value={formatAmount(calculations.fees)} - isLast /> <OrderDetailRow label="Taker Fee" value={market.takerFee ? formatRate(market.takerFee) : "-"} - isLast /> <OrderDetailRow label="Maker Fee" value={market.makerFee ? formatRate(market.makerFee) : "-"} isLast />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/dashboard/src/components/modules/trade/order-form/index.tsx` around lines 404 - 417, The three OrderDetailRow instances for "Fees", "Taker Fee", and "Maker Fee" all wrongly pass isLast; only the true final row should set isLast. Update the OrderDetailRow calls in order-form/index.tsx so that only the "Maker Fee" row keeps isLast, and remove the isLast prop from the "Fees" and "Taker Fee" rows (locate the OrderDetailRow components that use label="Fees", label="Taker Fee", and label="Maker Fee").
🧹 Nitpick comments (1)
packages/dashboard/src/components/molecules/positions/positions-tab.tsx (1)
151-153: Pre-groupordersbymarketIdbefore mapping rows.Line 153 filters the full
ordersarray for every position, so render cost grows withpositions × orders. Building a lookup once above the map keeps each row O(1).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/dashboard/src/components/molecules/positions/positions-tab.tsx` around lines 151 - 153, The render is repeatedly filtering orders inside positions.map (see positions.map with positionRef -> marketId and computing marketOrders), causing O(positions × orders) cost; instead build a lookup (e.g., a Map keyed by order.marketId) once before the positions.map and then inside positions.map replace the filter with a constant-time lookup (use marketId from positionRef to fetch the orders array from that Map); update any references to marketOrders accordingly so each row lookup is O(1).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/common/src/atoms/markets-atoms.ts`:
- Around line 124-140: The updateMarketsMidPriceAtom handler is currently
writing a new market object for every symbol on each midPriceAtom emission and
allows malformed prices; change the loop in updateMarketsMidPriceAtom (which
reads midPriceAtom and marketsBySymbolAtom and uses marketRef.update) to: 1)
parse the incoming price safely (e.g., const parsed = Number(price)) and skip if
parsed is not a finite number, 2) read the current market.markPrice from
marketRef.value and compare to parsed, and only call marketRef.update(...) when
parsed differs from the existing markPrice (no-op skip), and 3) preserve
existing market object shape when updating (only replace markPrice) so
unnecessary rerenders are avoided. Ensure these checks happen before calling
marketRef.update in the same loop that iterates Record.toEntries(mids).
In `@packages/common/src/atoms/portfolio-atoms.ts`:
- Around line 116-135: The updatePositionsMidPriceAtom handler mutates
PositionDto only by setting markPrice, leaving server-derived fields
(unrealizedPnl, liquidationPrice, etc.) stale; change the
positionRef.value.update callback used in updatePositionsMidPriceAtom to compute
and set all dependent fields atomically (e.g., call or implement a helper like
computeDerivedPositionFields(position, newMarkPrice) that returns updated
unrealizedPnl, liquidationPrice, marginUsed, etc.) and merge that full derived
object into the position update so the ref remains internally consistent after
mark price changes.
In `@packages/common/src/components/molecules/price-flash.tsx`:
- Line 11: The ref is currently typed as useRef<HTMLElement>(null) but is
attached to a <span>, so update the ref declaration in price-flash.tsx to use
useRef<HTMLSpanElement>(null) (the symbol to change is the ref constant created
via useRef) so the ref type matches the DOM element and satisfies stricter
React/TypeScript typings.
In `@packages/common/src/hooks/use-order-form.ts`:
- Around line 164-180: handleLeverageChange sets leverage via setLeverage which
stores a clamped value in leverageAtom, but the subsequent positionSize
calculation still uses the raw incoming value; change the handler to compute and
use the same clamped leverage when calling calculateOrderPositionSize (either by
deriving clampedLeverage with the same clamp/validation logic used by
leverageAtom or by reading the updated leverage from the atom synchronously) so
that calculateOrderPercentage/calculateOrderPositionSize use the stored/clamped
leverage; update references in handleLeverageChange to use clampedLeverage when
computing percentage and positionSize (functions: handleLeverageChange,
setLeverage, leverageAtom, calculateOrderPercentage,
calculateOrderPositionSize).
- Around line 88-95: The code snapshots an AtomRef by mapping ref => ref.value
in currentPosition which breaks subscriptions; instead keep the AtomRef itself
when extracting from the record (stop dereferencing ref.value in the
Option.map/Option.getOrNull flow). Concretely, change the Record.get(...)
pipeline so it returns the AtomRef (or null) rather than its .value, and defer
calling useAtomValue/useAtom or subscribing to that AtomRef at the consumer site
where a live position value is needed; update references to currentPosition
accordingly so they call useAtomValue(currentPosition) (or handle a nullable
AtomRef) rather than relying on a stale snapshot.
In `@packages/common/src/lib/formatting.ts`:
- Around line 142-158: The comment/docs for formatTPOrSLSettings are stale: the
function now returns an object { tp, sl } (strings like "TP 1.23%" or "TP Off")
rather than a combined long/short string; update the documentation/comments that
describe formatTPOrSLSettings/TPOrSLSettings to state the new return contract
(object with tp and sl properties), show example return values, and remove any
wording about a single combined string so consumers are not misled.
In `@packages/common/src/services/api-client/api-schemas.ts`:
- Around line 960-969: The matchStatus branches for MarketsControllerGetCandles
and MarketsControllerGetMarket currently map 400/404 to Effect.void, which lets
undefined flow on the success path; update the
withResponse(HttpClientResponse.matchStatus({...})) handlers in
MarketsControllerGetCandles and MarketsControllerGetMarket to treat 400 and 404
as error cases (e.g., use decodeError with the appropriate error schema names or
call unexpectedStatus) instead of returning Effect.void so the methods only
succeed with CandlesResponseDto / MarketDetailDto and surface errors correctly.
Ensure you remove the Effect.void mappings and replace them with
decodeError("MarketsControllerGetCandles400", ...) /
decodeError("MarketsControllerGetMarket404", ...) or unexpectedStatus as
appropriate.
- Around line 789-799: The schema for signedMetadata (used in ActionDto) no
longer matches the docblock: the comment documents a TLV hex string but the
schema was changed to Record<string, unknown>, causing decoding errors when the
backend sends the hex string. Fix by making signedMetadata's schema reflect the
documented wire format (e.g., change the schema for signedMetadata back to a
nullable/optional string using S.String/S.optionalWith(S.String, { nullable:
true }) or the project's nullable helper), or if the backend truly now sends a
parsed object, update the docblock to describe the Record shape and update any
consumers of ActionDto; ensure you update the symbol signedMetadata in
api-schemas.ts and any ActionDto usages/tests to match the chosen
representation.
- Around line 46-47: The type for supportedActions was changed to a single
PerpActionTypes literal but the API returns an array; update the schema so
supportedActions is an array of PerpActionTypes (e.g., PerpActionTypes[]) in
packages/common/src/services/api-client/api-schemas.ts and ensure the mirrored
type in packages/common/src/services/api-client/client-factory.ts matches the
same array type; verify any decoding/validation logic that consumes
supportedActions still treats it as an array of action strings.
In `@packages/common/src/services/hyperliquid/index.ts`:
- Around line 28-30: The current Effect.catchAll usage is wrong because the
handler returns a plain GetCandleSnapshotError instance instead of an Effect;
replace the catchAll with Effect.mapError to remap the failure into a new
GetCandleSnapshotError. Locate the Effect.tryPromise(() =>
infoClient.candleSnapshot(params)) expression and change the chained
Effect.catchAll((cause) => new GetCandleSnapshotError({ cause })) to
Effect.mapError((cause) => new GetCandleSnapshotError({ cause })), ensuring the
error is transformed correctly by the Effect pipeline.
- Around line 50-62: The current subscribeMidPrice stream uses
Stream.broadcastDynamic({ capacity: "unbounded" }) for the high-frequency
allMids websocket, which can cause unbounded memory growth; change it to use a
bounded buffer or a sliding/dropping policy instead — replace
Stream.broadcastDynamic({ capacity: "unbounded" }) with a bounded numeric
capacity (e.g., capacity: N) or switch to Stream.share({ connector:
PubSub.sliding({ capacity: N }) }) or PubSub.dropping({ capacity: N }) so
subscribeMidPrice/backpressure is bounded and old or new messages are evicted as
appropriate.
In `@packages/common/src/services/wallet/browser-signer.ts`:
- Around line 93-96: Record.get(chainsMap, chainId.toString()) returns an
Option, so convert it to an Effect before mapping errors: wrap the Record.get
call with Effect.fromOption (or use Effect.fromOptionWith) and then apply
Effect.mapError to transform None into new SwitchChainError({ cause: new
ChainNotFoundError() }); update the code around the chain variable assignment
(where Record.get, chainsMap, chainId are used) to use Effect.fromOption +
Effect.mapError instead of calling Effect.mapError directly on Record.get's
result.
In `@packages/common/src/vite.config.ts`:
- Around line 29-30: createCommonViteConfig currently reuses module-scoped
plugin instances via commonPlugins, which causes shared state across configs;
change it to instantiate fresh plugins inside createCommonViteConfig by
replacing plugins: Object.values(commonPlugins) with a new array of plugin
factory calls (e.g., tanstackRouter({ target: "react", autoCodeSplitting: true
}), viteReact({ babel: { plugins: ["babel-plugin-react-compiler"] }}),
tailwindcss(), nodePolyfills({ include: ["buffer"] }), etc.), ensuring each call
returns a new plugin instance for createCommonViteConfig so widget and dashboard
get isolated plugin objects.
In `@packages/dashboard/src/atoms/selected-market-atom.ts`:
- Line 8: The init currently uses ctx.result(marketsAtom) which creates a
reactive dependency and causes the initializer to re-run whenever marketsAtom
updates, forcing the selected market back to the default (e.g., BTC/head);
change the initializer to use ctx.resultOnce(marketsAtom) (or guard the logic so
it only sets a value if selectedMarketAtom is unset) so the markets list is read
once for one-shot initialization and user selections are not overwritten on
subsequent marketsAtom updates.
In `@packages/dashboard/src/components/modules/trade/chart-v2.tsx`:
- Around line 33-99: The createStubDatafeed currently returns no usable data:
resolveSymbol hardcodes symbol metadata, getBars always calls onResult([],
{noData:true}), and subscribeBars/unsubscribeBars are no-ops, causing the chart
to show an empty state; update createStubDatafeed so resolveSymbol derives
metadata from the passed symbolName (or looks up a market config), implement
getBars to fetch historical candle data (or generate realistic stub candles) and
call onResult with a non-empty bars array and meta.noData=false, and implement
subscribeBars/unsubscribeBars to provide live updates (or simulate them) by
calling the provided realtime callback—target the resolveSymbol, getBars,
subscribeBars and unsubscribeBars handlers in createStubDatafeed to locate and
change the behavior.
- Around line 109-115: The component currently assumes TradingView assets are
present and returns null from createChartWidget when window.TradingView is
missing, which leaves users with a blank chart and clears loading state; update
the component to explicitly load the charting_library script (referencing
LIBRARY_PATH) before calling createChartWidget, add robust error handling when
window.TradingView or TV.widget is unavailable (surface a user-facing error
state and keep/retry loading instead of setting isLoading(false) silently), and
add a simple retry/backoff mechanism so createChartWidget/TradingViewWidget
initialization is retried on transient failures; also fix the comment near
isLoading to reflect the intended behavior (keep loading state when assets are
missing).
In
`@packages/dashboard/src/components/modules/trade/market-info/market-selector-popover.tsx`:
- Around line 172-187: The sort toggle button currently only changes icon/color,
so add ARIA attributes to expose state: update the button in
market-selector-popover.tsx (the button that calls onToggle(column) and uses
isActive/direction) to include an aria-pressed={isActive} and a descriptive
aria-label that combines the visible label and the sort state (e.g., "Name,
sorted ascending"/"Name, sorted descending"/"Name, not sorted") so screen
readers announce the current sort; ensure the aria-label uses the same label
variable and the direction variable to produce the correct phrase.
In `@packages/dashboard/src/components/modules/trade/order-form/index.tsx`:
- Around line 409-417: The Taker/Maker fee checks treat 0 as missing; change the
truthy checks to nullish checks so zero fees render correctly: where
OrderDetailRow uses market.takerFee and market.makerFee, replace the current
truthy condition with a null/undefined check (e.g., use != null or !== null &&
!== undefined) and call formatRate(...) when the fee is not null/undefined,
otherwise render "-". Ensure you update both uses (market.takerFee and
market.makerFee) near the OrderDetailRow components and keep formatRate for
actual values.
In `@packages/widget/index.html`:
- Around line 8-9: The document only sets mobile-web-app-capable (Android) but
not apple-mobile-web-app-capable, so the existing
apple-mobile-web-app-status-bar-style won't take effect on iOS; add a meta tag
with name="apple-mobile-web-app-capable" and content="yes" alongside the
existing meta tags (ensure the new tag appears before or near
apple-mobile-web-app-status-bar-style) so iOS Safari can enter standalone mode
and apply the status bar style.
In `@pnpm-workspace.yaml`:
- Around line 25-29: Update the `@tanstack/router-cli` entry in
pnpm-workspace.yaml from 1.159.4 to 1.159.5 so the CLI matches
`@tanstack/react-router` and `@tanstack/router-plugin`; locate the
'@tanstack/router-cli' line and change its version string to ^1.159.5 and then
run your lockfile install (pnpm install) to regenerate the lockfile.
---
Outside diff comments:
In `@packages/common/src/services/wallet/browser-signer.ts`:
- Around line 118-120: The current signTransaction flow converts chain-switch
failures into defects by using Effect.orDie on the switchChain call; update
signTransaction so switchChain(chainId) is not wrapped with orDie and instead
propagates typed SwitchChainError through the effect error channel (e.g., remove
Effect.orDie and use the effect's normal error propagation or map/catch to a
typed error), ensuring errors from switchChain (referenced in switchChain and
wagmiAdapter.wagmiConfig.state.chainId) are returnable/handleable by callers/UI
rather than becoming uncatchable defects.
In `@packages/dashboard/src/components/modules/trade/order-form/index.tsx`:
- Around line 404-417: The three OrderDetailRow instances for "Fees", "Taker
Fee", and "Maker Fee" all wrongly pass isLast; only the true final row should
set isLast. Update the OrderDetailRow calls in order-form/index.tsx so that only
the "Maker Fee" row keeps isLast, and remove the isLast prop from the "Fees" and
"Taker Fee" rows (locate the OrderDetailRow components that use label="Fees",
label="Taker Fee", and label="Maker Fee").
---
Nitpick comments:
In `@packages/dashboard/src/components/molecules/positions/positions-tab.tsx`:
- Around line 151-153: The render is repeatedly filtering orders inside
positions.map (see positions.map with positionRef -> marketId and computing
marketOrders), causing O(positions × orders) cost; instead build a lookup (e.g.,
a Map keyed by order.marketId) once before the positions.map and then inside
positions.map replace the filter with a constant-time lookup (use marketId from
positionRef to fetch the orders array from that Map); update any references to
marketOrders accordingly so each row lookup is O(1).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 334ef86d-0bea-4ee1-8b37-9bddc8fe1d66
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (56)
package.jsonpackages/common/package.jsonpackages/common/src/atoms/actions-atoms.tspackages/common/src/atoms/hyperliquid-atoms.tspackages/common/src/atoms/index.tspackages/common/src/atoms/markets-atoms.tspackages/common/src/atoms/order-form-atoms.tspackages/common/src/atoms/portfolio-atoms.tspackages/common/src/components/index.tspackages/common/src/components/molecules/leverage-dialog.tsxpackages/common/src/components/molecules/limit-price-dialog.tsxpackages/common/src/components/molecules/order-type-dialog.tsxpackages/common/src/components/molecules/price-flash.tsxpackages/common/src/components/ui/dialog.tsxpackages/common/src/domain/signer.tspackages/common/src/domain/wallet.tspackages/common/src/hooks/use-deposit-form.tspackages/common/src/hooks/use-order-form.tspackages/common/src/hooks/use-withdraw-form.tspackages/common/src/lib/formatting.tspackages/common/src/services/api-client/api-schemas.tspackages/common/src/services/api-client/client-factory.tspackages/common/src/services/hyperliquid/index.tspackages/common/src/services/index.tspackages/common/src/services/runtime.tspackages/common/src/services/wallet/browser-signer.tspackages/common/src/styles/base.csspackages/common/src/vite.config.tspackages/common/vite.config.tspackages/dashboard/package.jsonpackages/dashboard/src/atoms/selected-market-atom.tspackages/dashboard/src/components/modules/root/Preload.tsxpackages/dashboard/src/components/modules/trade/chart-v2.tsxpackages/dashboard/src/components/modules/trade/market-info/index.tsxpackages/dashboard/src/components/modules/trade/market-info/market-selector-popover.tsxpackages/dashboard/src/components/modules/trade/order-form/index.tsxpackages/dashboard/src/components/molecules/header/deposit/state.tsxpackages/dashboard/src/components/molecules/header/withdraw/state.tsxpackages/dashboard/src/components/molecules/positions/index.tsxpackages/dashboard/src/components/molecules/positions/positions-tab.tsxpackages/dashboard/vite.config.tspackages/widget/index.htmlpackages/widget/package.jsonpackages/widget/src/app.tsxpackages/widget/src/components/modules/Home/Positions/index.tsxpackages/widget/src/components/modules/Home/Positions/position-card.tsxpackages/widget/src/components/modules/Home/index.tsxpackages/widget/src/components/modules/Order/Overview/index.tsxpackages/widget/src/components/modules/PositionDetails/Close/state.tsxpackages/widget/src/components/modules/PositionDetails/Overview/Position/index.tsxpackages/widget/src/components/modules/PositionDetails/Overview/index.tsxpackages/widget/src/components/modules/PositionDetails/Overview/modify-dialog.tsxpackages/widget/src/components/modules/Root/PreloadAtoms.tsxpackages/widget/src/main.csspackages/widget/vite.config.tspnpm-workspace.yaml
packages/dashboard/src/components/modules/trade/market-info/market-selector-popover.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Line 25: Replace the transient CLI invocation in the CI step that runs
Playwright; the line using "pnpm dlx playwright install --with-deps" should be
changed to use the project-pinned binary via "pnpm exec" so the installed
Playwright version from the lockfile (e.g., 1.58.0) is used and browser binaries
stay deterministic; update the run step to call the project-local Playwright CLI
with the same args (install --with-deps) using pnpm exec instead of pnpm dlx.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 47aa355f-3511-4b78-aa9e-55675ed61a1f
📒 Files selected for processing (1)
.github/workflows/ci.yml
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
packages/common/src/services/hyperliquid/index.ts (1)
50-62:⚠️ Potential issue | 🟠 MajorUse bounded capacity or sliding strategy for high-frequency mid-price stream.
allMidsis a high-frequency WebSocket feed. WithStream.broadcastDynamic({ capacity: "unbounded" }), slow subscribers can accumulate an ever-growing backlog, causing memory growth and potential OOM.Consider using a bounded capacity or a sliding/dropping strategy:
).pipe(Stream.broadcastDynamic({ capacity: "unbounded" })); + // Bounded alternative: + // ).pipe(Stream.broadcastDynamic({ capacity: 100 }));Or for sliding behavior (evict oldest when full), use
PubSub.sliding:import { PubSub } from "effect"; // Replace broadcastDynamic with: .pipe(Stream.share({ connector: () => PubSub.sliding<AllMidsWsEvent>({ capacity: 100 }) }))Since mid-price data is time-sensitive, newer values are more useful than stale backlog—sliding semantics fit well here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/common/src/services/hyperliquid/index.ts` around lines 50 - 62, The current subscribeMidPrice stream uses Stream.broadcastDynamic({ capacity: "unbounded" }) which can cause unbounded backlog for the high-frequency allMids feed; replace the broadcastDynamic connector with a bounded or sliding connector (e.g., use Stream.share with a connector created via PubSub.sliding or a bounded PubSub) so slow subscribers drop/evict old mid-price events instead of growing memory; update the pipe on subscribeMidPrice to use Stream.share({ connector: () => PubSub.sliding<AllMidsWsEvent>({ capacity: 100 }) }) or an equivalent bounded strategy and ensure PubSub is imported.packages/common/src/atoms/portfolio-atoms.ts (1)
117-139:⚠️ Potential issue | 🟠 MajorDon't leave
PositionDtointernally inconsistent.This still patches only
markPrice, sounrealizedPnl,liquidationPrice, and the other server-derived fields become stale inside the same ref. The TODO documents it, but consumers can still render mixed-time data from one object. Either recompute the dependent fields atomically here or keep the live mark price separate from the server snapshot.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/common/src/atoms/portfolio-atoms.ts` around lines 117 - 139, updatePositionsMidPriceAtom currently only updates markPrice on the PositionDto inside positionRef, leaving unrealizedPnl, liquidationPrice and other derived fields stale; either compute and set those dependent fields atomically when updating the ref or separate live mark prices from the server snapshot. Fix by, inside updatePositionsMidPriceAtom where positionRef.value.update is called, derive the new unrealizedPnl and liquidationPrice using the existing position and market data (use the market from marketsBySymbolAtom and the current position from positionsAtom) and update the position object with markPrice plus the recomputed derived fields in a single update, or instead introduce a new liveMarkPrice atom keyed by walletAddress+marketId and stop mutating PositionDto here so PositionDto remains consistent.packages/common/src/hooks/use-order-form.ts (1)
168-182:⚠️ Potential issue | 🟡 MinorUse the clamped leverage for the amount recompute.
setLeverage(value)normalizes the input throughleverageAtom, butcalculateOrderPositionSizestill uses rawvalue. Any out-of-range input leaves the amount field inconsistent with the leverage that was actually stored.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/common/src/hooks/use-order-form.ts` around lines 168 - 182, The recompute after setLeverage uses the raw input `value` but the app stores a normalized/clamped leverage via `setLeverage`/`leverageAtom`, causing inconsistencies; update handleLeverageChange to use the clamped/normalized leverage when recalculating amount and position size (either read the normalized leverage from the atom/state after calling setLeverage or explicitly clamp `value` the same way the atom does) and then pass that clamped value into `calculateOrderPercentage` and `calculateOrderPositionSize` instead of the raw `value`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/common/src/atoms/portfolio-atoms.ts`:
- Around line 129-139: When processing Record.toEntries(mids) in this block,
coerce price into a numeric value once (e.g. const newMark = Number(price)) and
skip the update if Number produced NaN/Infinity or if newMark ===
position.markPrice; locate the lookup/update flow using Record.toEntries,
Record.get(markets), Record.get(positions), positionRef and its update method,
and change the update call to only run when the parsed numeric mark is valid
(isFinite) and different from the current position.markPrice to avoid
unnecessary writes and rerenders.
In `@packages/common/src/hooks/use-optional-atom-ref.ts`:
- Around line 7-19: Replace the useState/useEffect subscription with
React.useSyncExternalStore to avoid the race and remove unused state: call
useSyncExternalStore with a subscribe function that, when ref is truthy, returns
ref.subscribe (so the hook gets the unsubscribe) and otherwise returns a no-op,
and with a getSnapshot function that returns ref?.value ?? null; ensure the hook
returns that snapshot directly instead of ref?.value. Reference symbols:
useSyncExternalStore, ref.subscribe, ref?.value.
---
Duplicate comments:
In `@packages/common/src/atoms/portfolio-atoms.ts`:
- Around line 117-139: updatePositionsMidPriceAtom currently only updates
markPrice on the PositionDto inside positionRef, leaving unrealizedPnl,
liquidationPrice and other derived fields stale; either compute and set those
dependent fields atomically when updating the ref or separate live mark prices
from the server snapshot. Fix by, inside updatePositionsMidPriceAtom where
positionRef.value.update is called, derive the new unrealizedPnl and
liquidationPrice using the existing position and market data (use the market
from marketsBySymbolAtom and the current position from positionsAtom) and update
the position object with markPrice plus the recomputed derived fields in a
single update, or instead introduce a new liveMarkPrice atom keyed by
walletAddress+marketId and stop mutating PositionDto here so PositionDto remains
consistent.
In `@packages/common/src/hooks/use-order-form.ts`:
- Around line 168-182: The recompute after setLeverage uses the raw input
`value` but the app stores a normalized/clamped leverage via
`setLeverage`/`leverageAtom`, causing inconsistencies; update
handleLeverageChange to use the clamped/normalized leverage when recalculating
amount and position size (either read the normalized leverage from the
atom/state after calling setLeverage or explicitly clamp `value` the same way
the atom does) and then pass that clamped value into `calculateOrderPercentage`
and `calculateOrderPositionSize` instead of the raw `value`.
In `@packages/common/src/services/hyperliquid/index.ts`:
- Around line 50-62: The current subscribeMidPrice stream uses
Stream.broadcastDynamic({ capacity: "unbounded" }) which can cause unbounded
backlog for the high-frequency allMids feed; replace the broadcastDynamic
connector with a bounded or sliding connector (e.g., use Stream.share with a
connector created via PubSub.sliding or a bounded PubSub) so slow subscribers
drop/evict old mid-price events instead of growing memory; update the pipe on
subscribeMidPrice to use Stream.share({ connector: () =>
PubSub.sliding<AllMidsWsEvent>({ capacity: 100 }) }) or an equivalent bounded
strategy and ensure PubSub is imported.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 835e62b1-d3b3-42a6-984d-536132074c1b
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
.github/workflows/ci.ymlpackages/common/src/atoms/markets-atoms.tspackages/common/src/atoms/portfolio-atoms.tspackages/common/src/components/molecules/price-flash.tsxpackages/common/src/domain/signer.tspackages/common/src/domain/wallet.tspackages/common/src/hooks/use-optional-atom-ref.tspackages/common/src/hooks/use-order-form.tspackages/common/src/lib/formatting.tspackages/common/src/services/hyperliquid/index.tspackages/common/src/vite.config.tspackages/widget/index.htmlpnpm-workspace.yaml
✅ Files skipped from review due to trivial changes (3)
- .github/workflows/ci.yml
- packages/widget/index.html
- packages/common/src/components/molecules/price-flash.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/common/src/vite.config.ts
- packages/common/src/domain/signer.ts
- packages/common/src/lib/formatting.ts
- pnpm-workspace.yaml
- packages/common/src/domain/wallet.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
packages/common/src/services/api-client/api-schemas.ts (3)
1058-1065:⚠️ Potential issue | 🟠 MajorDon't return
voidon DTO-returning 400/404 responses.These methods are typed to succeed with
CandlesResponseDto/MarketDetailDtoonly, but Lines 1064 and 1073 turn 400/404 into a successfulundefined. Drop those branches sounexpectedStatusfails instead, or add explicit error DTOs if the API documents them.Suggested fix
"MarketsControllerGetCandles": (marketId, options) => HttpClientRequest.get(`/v1/markets/${marketId}/candles`).pipe( HttpClientRequest.setUrlParams({ "interval": options?.["interval"] as any, "from": options?.["from"] as any, "to": options?.["to"] as any }), withResponse(HttpClientResponse.matchStatus({ "2xx": decodeSuccess(CandlesResponseDto), "401": decodeError("MarketsControllerGetCandles401", MarketsControllerGetCandles401), "429": decodeError("MarketsControllerGetCandles429", MarketsControllerGetCandles429), - "400": () => Effect.void, orElse: unexpectedStatus })) ), "MarketsControllerGetMarketById": (marketId) => HttpClientRequest.get(`/v1/markets/${marketId}`).pipe( withResponse(HttpClientResponse.matchStatus({ "2xx": decodeSuccess(MarketDetailDto), "401": decodeError("MarketsControllerGetMarketById401", MarketsControllerGetMarketById401), "429": decodeError("MarketsControllerGetMarketById429", MarketsControllerGetMarketById429), - "404": () => Effect.void, orElse: unexpectedStatus })) ),Also applies to: 1068-1074
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/common/src/services/api-client/api-schemas.ts` around lines 1058 - 1065, The generated client currently treats 400/404 as successful no-op by returning Effect.void in the MarketsControllerGetCandles (and the similar Market detail call) response mapping; remove the 400/404 branches that return void so those statuses fall through to unexpectedStatus, or replace them with proper decodeError calls using the correct error DTOs if the API documents error bodies; locate the withResponse(HttpClientResponse.matchStatus(...)) block for MarketsControllerGetCandles and the corresponding MarketDetail mapping and delete the "() => Effect.void" entries for 400/404 (or swap them for decodeError("MarketsControllerGetCandles400", SomeErrorDto) / decodeError("MarketsControllerGetCandles404", SomeErrorDto)) so the method no longer silently returns undefined for DTO-returning endpoints.
47-47:⚠️ Potential issue | 🟠 MajorKeep
supportedActionsas an array.The field name and surrounding docs describe multiple actions, but this now accepts a single
PerpActionTypes. If the API still returns["open", "close", ...],ProviderDtodecoding breaks. Please mirror the same array type inpackages/common/src/services/api-client/client-factory.ts.Suggested fix
- "supportedActions": PerpActionTypes, + "supportedActions": S.Array(PerpActionTypes),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/common/src/services/api-client/api-schemas.ts` at line 47, The ProviderDto schema's supportedActions property was changed from an array to a single PerpActionTypes, causing decoding to fail when the API returns an array; update the type back to an array (e.g., PerpActionTypes[]) in packages/common/src/services/api-client/api-schemas.ts and make the corresponding change in packages/common/src/services/api-client/client-factory.ts so both schema and client factory expect an array for supportedActions and decoding succeeds consistently.
838-844:⚠️ Potential issue | 🟠 Major
signedMetadatano longer matches the documented wire format.The docblock describes a TLV hex string, but this schema expects an object. If the backend still sends the documented string,
ActionDtodecoding fails when metadata is populated. Please keep the mirror inpackages/common/src/services/api-client/client-factory.tsaligned with whichever representation is actually on the wire.Suggested fix if the wire format is still the documented TLV string
- "signedMetadata": S.optionalWith(S.Record({ key: S.String, value: S.Unknown }), { nullable: true }), + "signedMetadata": S.optionalWith(S.String, { nullable: true }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/common/src/services/api-client/api-schemas.ts` around lines 838 - 844, The schema for signedMetadata no longer matches the docblock TLV hex-string wire format and causes ActionDto decoding to fail; update the mirror so both sides agree: if the backend sends the TLV hex string, change the schema symbol "signedMetadata" in packages/common/src/services/api-client/api-schemas.ts to accept a string (e.g., S.optionalWith(S.String, { nullable: true })), and ensure the corresponding handling/mapping in packages/common/src/services/api-client/client-factory.ts still parses that hex TLV into the desired runtime object (or vice‑versa—if the backend sends an object, change the client-factory to emit the object); keep ActionDto decoding and "signedMetadata" types consistent across both files.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/common/src/services/api-client/api-schemas.ts`:
- Around line 816-819: The schema entry for "explorerUrl" in TransactionDto is
typed as an object but documented as a URL string; update the schema to accept a
nullable string (replace the current S.Record(...) usage with a string schema
allowing null/undefined) and then mirror that change in the client factory
decoding/encoding logic in client-factory (ensure places that read/write
explorerUrl expect a string | null). Also update the docblock only if you
instead decide the API truly returns an object; keep schema and client-factory
in sync with the chosen wire format.
---
Duplicate comments:
In `@packages/common/src/services/api-client/api-schemas.ts`:
- Around line 1058-1065: The generated client currently treats 400/404 as
successful no-op by returning Effect.void in the MarketsControllerGetCandles
(and the similar Market detail call) response mapping; remove the 400/404
branches that return void so those statuses fall through to unexpectedStatus, or
replace them with proper decodeError calls using the correct error DTOs if the
API documents error bodies; locate the
withResponse(HttpClientResponse.matchStatus(...)) block for
MarketsControllerGetCandles and the corresponding MarketDetail mapping and
delete the "() => Effect.void" entries for 400/404 (or swap them for
decodeError("MarketsControllerGetCandles400", SomeErrorDto) /
decodeError("MarketsControllerGetCandles404", SomeErrorDto)) so the method no
longer silently returns undefined for DTO-returning endpoints.
- Line 47: The ProviderDto schema's supportedActions property was changed from
an array to a single PerpActionTypes, causing decoding to fail when the API
returns an array; update the type back to an array (e.g., PerpActionTypes[]) in
packages/common/src/services/api-client/api-schemas.ts and make the
corresponding change in
packages/common/src/services/api-client/client-factory.ts so both schema and
client factory expect an array for supportedActions and decoding succeeds
consistently.
- Around line 838-844: The schema for signedMetadata no longer matches the
docblock TLV hex-string wire format and causes ActionDto decoding to fail;
update the mirror so both sides agree: if the backend sends the TLV hex string,
change the schema symbol "signedMetadata" in
packages/common/src/services/api-client/api-schemas.ts to accept a string (e.g.,
S.optionalWith(S.String, { nullable: true })), and ensure the corresponding
handling/mapping in packages/common/src/services/api-client/client-factory.ts
still parses that hex TLV into the desired runtime object (or vice‑versa—if the
backend sends an object, change the client-factory to emit the object); keep
ActionDto decoding and "signedMetadata" types consistent across both files.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6179a21e-9130-48b0-b74b-d170fae1844c
📒 Files selected for processing (6)
packages/common/src/atoms/actions-atoms.tspackages/common/src/domain/wallet.tspackages/common/src/hooks/use-optional-atom-ref.tspackages/common/src/services/api-client/api-schemas.tspackages/common/src/services/api-client/client-factory.tspackages/common/src/services/wallet/wallet-service.ts
✅ Files skipped from review due to trivial changes (1)
- packages/common/src/services/wallet/wallet-service.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/common/src/hooks/use-optional-atom-ref.ts
- packages/common/src/domain/wallet.ts
- packages/common/src/atoms/actions-atoms.ts
Philippoes
approved these changes
Apr 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Improvements
Chores