Closed
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR introduces the YieldPlus feature for tracking leveraged yield positions, including new API queries for position data and DEX K-line candles, a new YieldPlus page with interactive UI components, refactored transaction and table components, K-line chart visualization, and comprehensive translations across multiple languages. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App as YieldPlus Page
participant Hooks as React Hooks
participant API as API Client
participant DEX as DEX API
User->>App: View YieldPlus Page
App->>Hooks: useGetYieldPlusPositions(accountAddress)
Hooks->>API: getYieldPlusPositions()
API->>API: Fetch positions from REST
API->>Hooks: getPools() for each position
Hooks->>API: Concurrent pool resolution
API->>Hooks: Return enriched positions
App->>Hooks: useGetDexKlineCandles(token, interval)
Hooks->>DEX: Fetch K-line candles
DEX->>Hooks: Return candle data
Hooks->>App: Positions + Candles
App->>App: Calculate financial metrics (PnL, liquidation price, APY)
App->>User: Render interactive positions table with K-line chart
User->>App: Click position row
App->>App: Toggle accordion / Load RowFooter
App->>Hooks: useGetAccountTransactionHistory(positionAccountAddress)
Hooks->>API: Fetch transactions for position
API->>Hooks: Return transaction history
Hooks->>App: Transactions data
App->>User: Display status and transaction tabs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Greptile SummaryThis PR wires up real DEX K-line data to the YieldPlus page, replacing hardcoded placeholder values with a REST fetch for historical candles ( Key changes:
Confidence Score: 3/5
Important Files Changed
Last reviewed commit: "feat: add more timer..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
Author
|
@greptile |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/evm/src/utilities/restService.ts (1)
30-38:⚠️ Potential issue | 🟡 MinorMissing URL encoding for query parameter values.
Now that
createQueryParamsis exported and used across modules, the lack of URL encoding could cause issues with special characters in parameter values.🐛 Proposed fix
export const createQueryParams = (params: Record<string, unknown>) => { const paramArray = Object.entries(params).map(([key, value]) => { if (value !== undefined && value !== null) { - return `${key}=${value}`; + return `${encodeURIComponent(key)}=${encodeURIComponent(String(value))}`; } return ''; }); return paramArray.filter(p => p).join('&'); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/evm/src/utilities/restService.ts` around lines 30 - 38, The createQueryParams function does not URL-encode keys or values; update createQueryParams to call encodeURIComponent on both the key and the stringified value (e.g., encodeURIComponent(String(value))) before joining, skip undefined/nulls as before, and continue filtering out empty entries so special characters are safely encoded; reference createQueryParams to locate the change.
🧹 Nitpick comments (11)
apps/evm/src/utilities/restService.ts (1)
54-64: Duplicate code block.The
Accept-Versionheader is set twice with identical logic at lines 54-58 and 60-64.♻️ Remove duplicate block
if (next) { _set(headers, 'Accept-Version', 'next'); } else { _set(headers, 'Accept-Version', 'stable'); } - if (next) { - _set(headers, 'Accept-Version', 'next'); - } else { - _set(headers, 'Accept-Version', 'stable'); - }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/evm/src/utilities/restService.ts` around lines 54 - 64, Remove the duplicated Accept-Version header logic in restService.ts: the conditional block that calls _set(headers, 'Accept-Version', next|stable) appears twice; keep a single instance (referencing the same _set function, headers variable, and the next flag) and delete the redundant second block so the header is only set once.apps/evm/src/libs/ws/WsClient.ts (2)
76-81: Consider adding exponential backoff and retry limits for reconnection.The current implementation uses a fixed 3-second delay and will retry indefinitely while channels remain active. This could cause issues if the server is down for an extended period (battery drain on mobile, excessive logs, etc.).
♻️ Suggested implementation with exponential backoff
export abstract class WsClient { private ws: WebSocket | null = null; private reconnectTimer: ReturnType<typeof setTimeout> | null = null; private isConnecting = false; private readonly activeChannels = new Set<string>(); + private reconnectAttempts = 0; + private static readonly MAX_RECONNECT_ATTEMPTS = 10; + private static readonly BASE_DELAY_MS = 1000; + private static readonly MAX_DELAY_MS = 30000; // ... other methods ... - private scheduleReconnect(delayMs = 3000): void { + private scheduleReconnect(): void { + if (this.reconnectAttempts >= WsClient.MAX_RECONNECT_ATTEMPTS) { + console.warn('WebSocket max reconnect attempts reached'); + return; + } + const delayMs = Math.min( + WsClient.BASE_DELAY_MS * 2 ** this.reconnectAttempts, + WsClient.MAX_DELAY_MS, + ); + this.reconnectAttempts++; this.reconnectTimer = setTimeout(() => { this.reconnectTimer = null; this.connect(); }, delayMs); }Also reset
reconnectAttempts = 0inonopencallback upon successful connection.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/evm/src/libs/ws/WsClient.ts` around lines 76 - 81, The reconnect logic in scheduleReconnect currently uses a fixed 3s delay and can run indefinitely; change scheduleReconnect to implement exponential backoff with a baseDelay (e.g., 3000ms), a backoff multiplier (e.g., 2), and a maxDelay cap, and add a maxReconnectAttempts limit; increment reconnectAttempts each time scheduleReconnect schedules a retry and clear/stop scheduling when reconnectAttempts >= maxReconnectAttempts, and ensure reconnectTimer behavior remains unchanged. Also reset reconnectAttempts = 0 in the WebSocket onopen handler (the onopen callback) upon successful connection so future disconnects start backoff from the baseDelay. Ensure to reference and update the existing fields/methods: scheduleReconnect, reconnectTimer, reconnectAttempts, connect, and the onopen handler.
55-55: WebSocket data may not always be a string.The
MessageEvent.dataproperty can bestring,Blob, orArrayBufferdepending on thebinaryTypesetting. If the server ever sends binary data, casting tostringwill produce unexpected results.♻️ Suggested validation
- this.ws.onmessage = ({ data }) => this.onMessage(data as string); + this.ws.onmessage = ({ data }) => { + if (typeof data === 'string') { + this.onMessage(data); + } + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/evm/src/libs/ws/WsClient.ts` at line 55, The assignment that assumes MessageEvent.data is always a string is unsafe; update the this.ws.onmessage handler in WsClient so it detects the data type and converts it to a string before calling onMessage: if data is a string call this.onMessage(data), if data is a Blob use await data.text() (so make the handler async or dispatch to an async helper), and if data is an ArrayBuffer decode it (e.g., using TextDecoder) to a string; ensure onMessage is invoked only with a string and handle/propagate any conversion errors appropriately.apps/evm/src/clients/api/__mocks__/index.ts (1)
678-686: Mock queryKey differs from real implementation.The real
useGetDexKlineCandleshook usesqueryKey: [FunctionKey.GET_DEX_KLINE_CANDLES, { ...params, chainId }], but this mock uses a simpler key without parameters. This is consistent with other mocks in this file, but could cause issues if tests rely on specific queryKey matching for cache invalidation or query filtering.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/evm/src/clients/api/__mocks__/index.ts` around lines 678 - 686, The mock useGetDexKlineCandles uses a simplified queryKey that doesn't match the real hook; update the mock so useGetDexKlineCandles returns useQuery with queryKey: [FunctionKey.GET_DEX_KLINE_CANDLES, { ...params, chainId }] (i.e., include the params object and chainId like the real implementation) and keep queryFn: getDexKlineCandles so tests that inspect or invalidate queries by key will match the real behavior.apps/evm/src/clients/api/index.ts (1)
217-227: Pre-existing duplicate exports detected.Lines 217-218 and 223-224 both export
getBurnedWBnbanduseGetBurnedWBnb. Similarly, lines 220-221 and 226-227 both exportgetImportablePositionsanduseGetImportablePositions. While this doesn't break functionality (re-exports are idempotent), it adds noise.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/evm/src/clients/api/index.ts` around lines 217 - 227, Duplicate re-exports for './queries/getBurnedWBnb' (exporting getBurnedWBnb and useGetBurnedWBnb) and './queries/getImportablePositions' (exporting getImportablePositions and useGetImportablePositions) should be removed; edit the export block to keep only one export line pair for each module (leave a single "export * from './queries/getBurnedWBnb';" and "export * from './queries/getBurnedWBnb/useGetBurnedWBnb';" and a single "export * from './queries/getImportablePositions';" and "export * from './queries/getImportablePositions/useGetImportablePositions';"), deleting the duplicate occurrences to eliminate noise.apps/evm/src/clients/api/queries/getDexKlineCandles/useDexKlineWebSocket.ts (1)
18-24: Singleton WebSocket client has no teardown mechanism.The
sharedClientsingleton is created lazily but never destroyed. This is typically fine for app-lifetime resources, but ifdexWsUrlcould change at runtime (e.g., config reload), the stale client would persist.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/evm/src/clients/api/queries/getDexKlineCandles/useDexKlineWebSocket.ts` around lines 18 - 24, The singleton sharedClient (variable) created by getSharedClient can become stale if config.dexWsUrl changes and has no teardown; add a lifecycle API to allow closing/recreating the client: implement a closeSharedClient (or resetSharedClient) function that calls the DexKlineWsClient instance's close/terminate method, awaits cleanup if needed, and sets sharedClient = null, and modify getSharedClient to detect a changed config.dexWsUrl by storing the current url (e.g., lastWsUrl) and if it differs, call the new close/reset function before constructing a new DexKlineWsClient(config.dexWsUrl); expose the close/reset function where the app can call it on config reload.apps/evm/src/clients/api/queries/getDexKlineCandles/index.ts (2)
38-40: Consider including HTTP status in error for debugging.The error thrown on non-OK responses doesn't include the HTTP status code or response body, making debugging harder. Consider enriching the error context.
♻️ Suggested improvement for error context
if (!response.ok) { - throw new VError({ type: 'unexpected', code: 'somethingWentWrong' }); + throw new VError({ + type: 'unexpected', + code: 'somethingWentWrong', + data: { status: response.status, statusText: response.statusText }, + }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/evm/src/clients/api/queries/getDexKlineCandles/index.ts` around lines 38 - 40, The error thrown when response.ok is false in the fetch block (the throw new VError(...) line) should include HTTP status and response body for debugging: capture response.status and response.statusText and attempt to read a safe body (await response.text() or json() inside a try/catch), then rethrow a VError (or augment the existing VError) including those fields (e.g., message with status/statusText and a property like info: { status, statusText, body }) so callers can see HTTP details; update the code in the function that performs the fetch (the response handling in index.ts) to build and throw this enriched VError.
33-36: Consider adding request timeout handling.The
fetchcall has no timeout, which could cause the request to hang indefinitely if the server doesn't respond. Consider usingAbortControllerwith a timeout.♻️ Suggested timeout implementation
+ const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 30000); // 30s timeout + const response = await fetch( `${config.dexApiUrl}/k-line/candles?${queryParams}`, - { headers: { Accept: 'application/json' } }, + { headers: { Accept: 'application/json' }, signal: controller.signal }, ); + + clearTimeout(timeoutId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/evm/src/clients/api/queries/getDexKlineCandles/index.ts` around lines 33 - 36, The fetch call in getDexKlineCandles can hang because it lacks timeout handling; wrap the fetch call with an AbortController: create an AbortController, start a setTimeout that calls controller.abort() after a chosen timeout, pass controller.signal into fetch (the call that constructs `response`), and clear the timeout after the fetch completes; also handle the abort case in the surrounding function (e.g., catch and translate an AbortError to a timed-out error or return a controlled error response). Ensure you reference `config.dexApiUrl`, `queryParams`, and the `response` assignment when adding the controller/signal so the change is localized to that fetch invocation.apps/evm/src/clients/api/queries/getDexKlineCandles/useGetDexKlineCandles.ts (1)
13-20: Platform mapping duplicated across files.Both this file and
useDexKlineWebSocket.tsmaintain separate chain-to-platform mappings (DEX_PLATFORM_BY_CHAIN_IDwith strings here,PLATFORM_ID_BY_CHAIN_IDwith numbers there). Consider consolidating into a shared constants file to ensure consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/evm/src/clients/api/queries/getDexKlineCandles/useGetDexKlineCandles.ts` around lines 13 - 20, DEX_PLATFORM_BY_CHAIN_ID here is duplicated with PLATFORM_ID_BY_CHAIN_ID in another module; extract a single source of truth by creating a shared constant (e.g., export const DEX_PLATFORM_BY_CHAIN_ID or PLATFORM_MAP) and replace the local maps in useGetDexKlineCandles.ts and useDexKlineWebSocket.ts to import that shared constant; update usage sites to adapt to the shared shape (string keys/values or numeric IDs) and convert callers if needed so both getDexKlineCandles and useDexKlineWebSocket reference the same mapping symbol.apps/evm/src/components/KLineChart/index.tsx (1)
106-116: Consider extracting duplicatesetDataLoaderconfiguration.The
subscribeBarandunsubscribeBarhandlers are duplicated between the initial setup (lines 110-115) and the data-change effect (lines 147-152). Extracting a helper could reduce duplication.♻️ Potential refactor to reduce duplication
+ const createDataLoader = (candleData: KLineData[]) => ({ + getBars: ({ callback }: { callback: (data: KLineData[]) => void }) => { + callback(candleData); + }, + subscribeBar: ({ callback }: { callback: (data: KLineData) => void }) => { + subscribeCallbackRef.current = callback; + }, + unsubscribeBar: () => { + subscribeCallbackRef.current = null; + }, + }); // In mount effect: - chart.setDataLoader({ - getBars: ({ callback }) => { - callback(latestDataRef.current); - }, - subscribeBar: ({ callback }) => { - subscribeCallbackRef.current = callback; - }, - unsubscribeBar: () => { - subscribeCallbackRef.current = null; - }, - }); + chart.setDataLoader(createDataLoader(latestDataRef.current)); // In data effect: - chartRef.current?.setDataLoader({ - getBars: ({ callback }) => { - callback(data); - }, - subscribeBar: ({ callback }) => { - subscribeCallbackRef.current = callback; - }, - unsubscribeBar: () => { - subscribeCallbackRef.current = null; - }, - }); + chartRef.current?.setDataLoader(createDataLoader(data));Also applies to: 140-154
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/evm/src/components/KLineChart/index.tsx` around lines 106 - 116, Duplicate setDataLoader subscription logic should be extracted into a small helper to avoid repeating subscribeBar/unsubscribeBar setup; create a function (e.g., initChartDataLoader or configureChartSubscriptions) that accepts the chart instance and wires getBars (using latestDataRef.current), subscribeBar (assigning subscribeCallbackRef.current), and unsubscribeBar (clearing subscribeCallbackRef.current), then call that helper both in the initial chart setup and inside the data-change effect instead of duplicating the subscribe/unsubscribe blocks.apps/evm/src/clients/api/queries/getDexKlineCandles/DexKlineWsClient.ts (1)
59-59: Protect fanout from subscriber exceptions.A thrown error in one callback can interrupt delivery to the remaining subscribers on the same channel. Wrap each callback invocation defensively.
Suggested refactor
- this.callbacks.get(channel)?.forEach(cb => cb(candle)); + for (const cb of this.callbacks.get(channel) ?? []) { + try { + cb(candle); + } catch { + // Optional: report error via app logger/telemetry + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/evm/src/clients/api/queries/getDexKlineCandles/DexKlineWsClient.ts` at line 59, The fanout loop in DexKlineWsClient that currently does "this.callbacks.get(channel)?.forEach(cb => cb(candle))" must be defensive: iterate the subscriber list (this.callbacks.get(channel)) and invoke each callback inside a try/catch so a thrown error from one cb does not stop delivery to others; on catch, log or handle the error (use the class logger or console with context including channel and the cb identity if available) and continue to the next subscriber. Ensure you update the method that processes incoming candles (where "callbacks" and "channel" are referenced) to use this per-callback try/catch pattern.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/evm/src/clients/api/queries/getDexKlineCandles/DexKlineWsClient.ts`:
- Around line 54-57: The current check only verifies usdOhlcv length; update
DexKlineWsClient handling so before constructing DexKlineCandle you validate
that each element in usdOhlcv (o,h,l,c,v,ts) is a finite number (and timestamp
is an integer/finite number) and not NaN/undefined; if any value fails
validation, bail out (return) or skip the message to avoid poisoning chart
state, otherwise safely use the numeric values to build the candle object;
reference usdOhlcv, DexKlineCandle, and the candle construction site to locate
where to add these checks.
In `@apps/evm/src/clients/api/queries/getDexKlineCandles/types.ts`:
- Around line 30-39: ApiDexKlineCandle currently documents the 7th element
("ut") as optional but defines it as required (nullable), causing 6-element
payloads to be rejected; update the tuple type ApiDexKlineCandle so the 7th
element is optional (e.g., make the final tuple element have an optional
modifier: (number | null)?), and keep the inline comment describing ut as
optional so 6- and 7-element arrays are both accepted.
---
Outside diff comments:
In `@apps/evm/src/utilities/restService.ts`:
- Around line 30-38: The createQueryParams function does not URL-encode keys or
values; update createQueryParams to call encodeURIComponent on both the key and
the stringified value (e.g., encodeURIComponent(String(value))) before joining,
skip undefined/nulls as before, and continue filtering out empty entries so
special characters are safely encoded; reference createQueryParams to locate the
change.
---
Nitpick comments:
In `@apps/evm/src/clients/api/__mocks__/index.ts`:
- Around line 678-686: The mock useGetDexKlineCandles uses a simplified queryKey
that doesn't match the real hook; update the mock so useGetDexKlineCandles
returns useQuery with queryKey: [FunctionKey.GET_DEX_KLINE_CANDLES, { ...params,
chainId }] (i.e., include the params object and chainId like the real
implementation) and keep queryFn: getDexKlineCandles so tests that inspect or
invalidate queries by key will match the real behavior.
In `@apps/evm/src/clients/api/index.ts`:
- Around line 217-227: Duplicate re-exports for './queries/getBurnedWBnb'
(exporting getBurnedWBnb and useGetBurnedWBnb) and
'./queries/getImportablePositions' (exporting getImportablePositions and
useGetImportablePositions) should be removed; edit the export block to keep only
one export line pair for each module (leave a single "export * from
'./queries/getBurnedWBnb';" and "export * from
'./queries/getBurnedWBnb/useGetBurnedWBnb';" and a single "export * from
'./queries/getImportablePositions';" and "export * from
'./queries/getImportablePositions/useGetImportablePositions';"), deleting the
duplicate occurrences to eliminate noise.
In `@apps/evm/src/clients/api/queries/getDexKlineCandles/DexKlineWsClient.ts`:
- Line 59: The fanout loop in DexKlineWsClient that currently does
"this.callbacks.get(channel)?.forEach(cb => cb(candle))" must be defensive:
iterate the subscriber list (this.callbacks.get(channel)) and invoke each
callback inside a try/catch so a thrown error from one cb does not stop delivery
to others; on catch, log or handle the error (use the class logger or console
with context including channel and the cb identity if available) and continue to
the next subscriber. Ensure you update the method that processes incoming
candles (where "callbacks" and "channel" are referenced) to use this
per-callback try/catch pattern.
In `@apps/evm/src/clients/api/queries/getDexKlineCandles/index.ts`:
- Around line 38-40: The error thrown when response.ok is false in the fetch
block (the throw new VError(...) line) should include HTTP status and response
body for debugging: capture response.status and response.statusText and attempt
to read a safe body (await response.text() or json() inside a try/catch), then
rethrow a VError (or augment the existing VError) including those fields (e.g.,
message with status/statusText and a property like info: { status, statusText,
body }) so callers can see HTTP details; update the code in the function that
performs the fetch (the response handling in index.ts) to build and throw this
enriched VError.
- Around line 33-36: The fetch call in getDexKlineCandles can hang because it
lacks timeout handling; wrap the fetch call with an AbortController: create an
AbortController, start a setTimeout that calls controller.abort() after a chosen
timeout, pass controller.signal into fetch (the call that constructs
`response`), and clear the timeout after the fetch completes; also handle the
abort case in the surrounding function (e.g., catch and translate an AbortError
to a timed-out error or return a controlled error response). Ensure you
reference `config.dexApiUrl`, `queryParams`, and the `response` assignment when
adding the controller/signal so the change is localized to that fetch
invocation.
In `@apps/evm/src/clients/api/queries/getDexKlineCandles/useDexKlineWebSocket.ts`:
- Around line 18-24: The singleton sharedClient (variable) created by
getSharedClient can become stale if config.dexWsUrl changes and has no teardown;
add a lifecycle API to allow closing/recreating the client: implement a
closeSharedClient (or resetSharedClient) function that calls the
DexKlineWsClient instance's close/terminate method, awaits cleanup if needed,
and sets sharedClient = null, and modify getSharedClient to detect a changed
config.dexWsUrl by storing the current url (e.g., lastWsUrl) and if it differs,
call the new close/reset function before constructing a new
DexKlineWsClient(config.dexWsUrl); expose the close/reset function where the app
can call it on config reload.
In
`@apps/evm/src/clients/api/queries/getDexKlineCandles/useGetDexKlineCandles.ts`:
- Around line 13-20: DEX_PLATFORM_BY_CHAIN_ID here is duplicated with
PLATFORM_ID_BY_CHAIN_ID in another module; extract a single source of truth by
creating a shared constant (e.g., export const DEX_PLATFORM_BY_CHAIN_ID or
PLATFORM_MAP) and replace the local maps in useGetDexKlineCandles.ts and
useDexKlineWebSocket.ts to import that shared constant; update usage sites to
adapt to the shared shape (string keys/values or numeric IDs) and convert
callers if needed so both getDexKlineCandles and useDexKlineWebSocket reference
the same mapping symbol.
In `@apps/evm/src/components/KLineChart/index.tsx`:
- Around line 106-116: Duplicate setDataLoader subscription logic should be
extracted into a small helper to avoid repeating subscribeBar/unsubscribeBar
setup; create a function (e.g., initChartDataLoader or
configureChartSubscriptions) that accepts the chart instance and wires getBars
(using latestDataRef.current), subscribeBar (assigning
subscribeCallbackRef.current), and unsubscribeBar (clearing
subscribeCallbackRef.current), then call that helper both in the initial chart
setup and inside the data-change effect instead of duplicating the
subscribe/unsubscribe blocks.
In `@apps/evm/src/libs/ws/WsClient.ts`:
- Around line 76-81: The reconnect logic in scheduleReconnect currently uses a
fixed 3s delay and can run indefinitely; change scheduleReconnect to implement
exponential backoff with a baseDelay (e.g., 3000ms), a backoff multiplier (e.g.,
2), and a maxDelay cap, and add a maxReconnectAttempts limit; increment
reconnectAttempts each time scheduleReconnect schedules a retry and clear/stop
scheduling when reconnectAttempts >= maxReconnectAttempts, and ensure
reconnectTimer behavior remains unchanged. Also reset reconnectAttempts = 0 in
the WebSocket onopen handler (the onopen callback) upon successful connection so
future disconnects start backoff from the baseDelay. Ensure to reference and
update the existing fields/methods: scheduleReconnect, reconnectTimer,
reconnectAttempts, connect, and the onopen handler.
- Line 55: The assignment that assumes MessageEvent.data is always a string is
unsafe; update the this.ws.onmessage handler in WsClient so it detects the data
type and converts it to a string before calling onMessage: if data is a string
call this.onMessage(data), if data is a Blob use await data.text() (so make the
handler async or dispatch to an async helper), and if data is an ArrayBuffer
decode it (e.g., using TextDecoder) to a string; ensure onMessage is invoked
only with a string and handle/propagate any conversion errors appropriately.
In `@apps/evm/src/utilities/restService.ts`:
- Around line 54-64: Remove the duplicated Accept-Version header logic in
restService.ts: the conditional block that calls _set(headers, 'Accept-Version',
next|stable) appears twice; keep a single instance (referencing the same _set
function, headers variable, and the next flag) and delete the redundant second
block so the header is only set once.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 29f7a4b2-ebbe-40f8-be7c-2ee5df2e855a
📒 Files selected for processing (18)
apps/evm/src/clients/api/__mocks__/index.tsapps/evm/src/clients/api/index.tsapps/evm/src/clients/api/queries/getDexKlineCandles/DexKlineWsClient.tsapps/evm/src/clients/api/queries/getDexKlineCandles/index.tsapps/evm/src/clients/api/queries/getDexKlineCandles/types.tsapps/evm/src/clients/api/queries/getDexKlineCandles/useDexKlineWebSocket.tsapps/evm/src/clients/api/queries/getDexKlineCandles/useGetDexKlineCandles.tsapps/evm/src/components/KLineChart/index.tsxapps/evm/src/config/envVariables.tsapps/evm/src/config/index.tsapps/evm/src/constants/functionKey.tsapps/evm/src/hooks/useIsFeatureEnabled/index.tsxapps/evm/src/libs/ws/WsClient.tsapps/evm/src/libs/ws/index.tsapps/evm/src/pages/YieldPlus/PairInfo/index.tsxapps/evm/src/pages/YieldPlus/index.tsxapps/evm/src/setupTests.tsxapps/evm/src/utilities/restService.ts
Contributor
Author
|
@greptile |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/evm/src/clients/api/queries/getDexKlineCandles/types.ts (1)
30-39:⚠️ Potential issue | 🟠 Major
ApiDexKlineCandlestill contradicts optionalutcontract.Line 30 documents
ut?as optional, but Line 38 makes the 7th tuple element required (nullable only). This can mis-type valid 6-element candles.Suggested fix
export type ApiDexKlineCandle = [ number, // o - open price number, // h - highest price number, // l - lowest price number, // c - close price number, // v - volume number, // ts - opening time (Unix ms) - number | null, // ut - unique traders (optional) + (number | null)?, // ut - unique traders (optional) ];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/evm/src/clients/api/queries/getDexKlineCandles/types.ts` around lines 30 - 39, ApiDexKlineCandle’s seventh tuple entry is documented as optional but currently typed as required (number | null); update the tuple so the seventh element is optional. In apps/evm/src/clients/api/queries/getDexKlineCandles/types.ts change the tuple declaration for ApiDexKlineCandle so the 7th element is typed as (number | null)? (i.e., make the ut element optional) to allow 6- and 7-element candles.
🧹 Nitpick comments (5)
apps/evm/src/libs/ws/WsClient.ts (1)
80-85: Consider exponential backoff for reconnection.The fixed 3-second delay works but exponential backoff with a cap would be more resilient under sustained failures (e.g., network outages), reducing unnecessary retry load and improving recovery behavior.
♻️ Proposed exponential backoff pattern
+ private reconnectAttempt = 0; + private readonly maxReconnectDelay = 30000; - private scheduleReconnect(delayMs = 3000): void { + private scheduleReconnect(): void { + const delayMs = Math.min(1000 * 2 ** this.reconnectAttempt, this.maxReconnectDelay); + this.reconnectAttempt++; this.reconnectTimer = setTimeout(() => { this.reconnectTimer = null; this.connect(); }, delayMs); } // Reset attempt counter on successful connection: this.ws.onopen = () => { this.isConnecting = false; + this.reconnectAttempt = 0; for (const channel of this.activeChannels) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/evm/src/libs/ws/WsClient.ts` around lines 80 - 85, Replace the fixed delay in scheduleReconnect with an exponential backoff: add a numeric state (e.g., this.reconnectAttempts) that increments each time scheduleReconnect runs, compute delayMs = Math.min(baseDelayMs * 2 ** this.reconnectAttempts, maxDelayMs) (choose baseDelayMs like 3000 and a max cap), call setTimeout to invoke this.connect(), store/clear this.reconnectTimer as before, and reset this.reconnectAttempts to 0 on successful connection inside connect() (or on open event) so retries start from base again; keep existing reconnectTimer cancellation behavior unchanged.apps/evm/src/components/KLineChart/index.tsx (1)
106-116: Consider extracting duplicatedsetDataLoaderconfiguration.The
setDataLoaderconfiguration withgetBars,subscribeBar, andunsubscribeBaris duplicated in both the initial setup effect (lines 106-116) and the data-driven effect (lines 143-153). This creates a maintenance burden where changes need to be synchronized in two places.♻️ Proposed refactor to extract loader factory
+ const createDataLoader = (candles: KLineData[]) => ({ + getBars: ({ callback }: { callback: (data: KLineData[]) => void }) => { + callback(candles); + }, + subscribeBar: ({ callback }: { callback: (data: KLineData) => void }) => { + subscribeCallbackRef.current = callback; + }, + unsubscribeBar: () => { + subscribeCallbackRef.current = null; + }, + }); // In initial setup effect: - chart.setDataLoader({ - getBars: ({ callback }) => { - callback(latestDataRef.current); - }, - subscribeBar: ({ callback }) => { - subscribeCallbackRef.current = callback; - }, - unsubscribeBar: () => { - subscribeCallbackRef.current = null; - }, - }); + chart.setDataLoader(createDataLoader(latestDataRef.current)); // In data effect: - chartRef.current?.setDataLoader({ - getBars: ({ callback }) => { - callback(data); - }, - subscribeBar: ({ callback }) => { - subscribeCallbackRef.current = callback; - }, - unsubscribeBar: () => { - subscribeCallbackRef.current = null; - }, - }); + chartRef.current?.setDataLoader(createDataLoader(data));Also applies to: 143-153
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/evm/src/components/KLineChart/index.tsx` around lines 106 - 116, Extract the duplicated setDataLoader configuration into a single factory/helper and use it in both places: create a function (e.g., buildDataLoader) that returns an object with getBars, subscribeBar, and unsubscribeBar implementations referencing latestDataRef and subscribeCallbackRef, then call chart.setDataLoader(buildDataLoader()) wherever the loader is currently duplicated (both in the initial setup effect and the data-driven effect); ensure function uses the same closure variables (latestDataRef, subscribeCallbackRef, chart) so the behavior remains identical and any future changes are made in one place.apps/evm/src/pages/YieldPlus/index.tsx (1)
29-35: Consider edge case when wrapped token lookup fails for native tokens.When
longToken.isNativeis true and no wrapped token is found, this falls back tolongToken.address. Native token addresses typically can't be used directly in DEX queries. The fallback may cause API errors or return no data.♻️ Suggested improvement
const longTokenDexAddress = useMemo(() => { if (!longToken.isNative) return longToken.address; const wrappedToken = tokens.find(t => areAddressesEqual(t.tokenWrapped?.address ?? '', longToken.address), ); - return wrappedToken?.address ?? longToken.address; + return wrappedToken?.address; }, [longToken, tokens]);Then disable the kline queries when
longTokenDexAddressis undefined:const { data: klineData } = useGetDexKlineCandles({ address: longTokenDexAddress, interval: KLINE_INTERVAL, limit: KLINE_LIMIT, -}); +}, { enabled: !!longTokenDexAddress });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/evm/src/pages/YieldPlus/index.tsx` around lines 29 - 35, The current useMemo for longTokenDexAddress returns longToken.address when a native token's wrapped lookup fails, which can lead to invalid DEX queries; change the logic in the useMemo (the longTokenDexAddress computation that uses areAddressesEqual, longToken and tokens) to return undefined (or null) when longToken.isNative and no wrappedToken is found instead of falling back to longToken.address, and then ensure any kline queries or callers that depend on longTokenDexAddress are guarded/disabled when it is undefined (e.g., skip or early-return in the kline query hook or component that uses longTokenDexAddress).apps/evm/src/clients/api/queries/getDexKlineCandles/index.ts (1)
33-40: Consider adding a timeout for the fetch request.The
fetchcall has no timeout, which could cause the request to hang indefinitely if the DEX API is unresponsive. Consider usingAbortControllerwith a timeout.♻️ Proposed refactor with timeout
+const FETCH_TIMEOUT_MS = 30_000; + export const getDexKlineCandles = async ({ platform, address, interval, limit, from, to, unit, tokenIndex, }: GetDexKlineCandlesInput): Promise<GetDexKlineCandlesOutput> => { const queryParams = createQueryParams({ platform, address, interval, limit, from, to, unit, tokenIndex, }); + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), FETCH_TIMEOUT_MS); + - const response = await fetch( - `${config.dexApiUrl}/k-line/candles?${queryParams}`, - { headers: { Accept: 'application/json' } }, - ); + const response = await fetch( + `${config.dexApiUrl}/k-line/candles?${queryParams}`, + { headers: { Accept: 'application/json' }, signal: controller.signal }, + ).finally(() => clearTimeout(timeoutId));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/evm/src/clients/api/queries/getDexKlineCandles/index.ts` around lines 33 - 40, The fetch call that requests `${config.dexApiUrl}/k-line/candles?${queryParams}` should use an AbortController with a timeout to avoid hanging; create an AbortController, pass its signal into fetch, start a setTimeout that calls controller.abort() after a configurable ms, clear the timeout once the response arrives, and catch an aborted/timeout error to throw a distinct VError (e.g., type 'timeout' or code 'requestTimedOut') instead of the generic unexpected VError; update the fetch/response handling in the getDexKlineCandles flow to include these changes and ensure the timeout is cleaned up on success or error.apps/evm/src/clients/api/queries/getDexKlineCandles/useGetDexKlineCandles.ts (1)
29-49: Narrow hook options to prevent overridingqueryKey/queryFnand add explicit platform guard.Spreading
optionstyped asPartial<QueryObserverOptions<...>>allows callers to overridequeryKeyorqueryFn, which can silently break cache consistency. Additionally, while theenabledflag prevents query execution whenplatformis undefined, using an explicit error guard inqueryFnis more defensive than relying on a non-null assertion.Suggested change
type Options = QueryObserverOptions< GetDexKlineCandlesOutput, Error, GetDexKlineCandlesOutput, GetDexKlineCandlesOutput, UseGetDexKlineCandlesQueryKey >; export const useGetDexKlineCandles = ( params: TrimmedGetDexKlineCandlesInput, - options?: Partial<Options>, + options?: Partial<Omit<Options, 'queryKey' | 'queryFn'>>, ) => { const { chainId } = useChainId(); const platform = DEX_PLATFORM_BY_CHAIN_ID.get(chainId); return useQuery({ queryKey: [FunctionKey.GET_DEX_KLINE_CANDLES, { ...params, chainId }], - queryFn: () => getDexKlineCandles({ platform: platform!, ...params }), + queryFn: () => { + if (!platform) { + throw new Error(`Unsupported chainId for DEX kline candles: ${chainId}`); + } + return getDexKlineCandles({ platform, ...params }); + }, staleTime: Number.POSITIVE_INFINITY, ...options, enabled: !!platform && (options?.enabled ?? true), }); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/evm/src/clients/api/queries/getDexKlineCandles/useGetDexKlineCandles.ts` around lines 29 - 49, The hook useGetDexKlineCandles currently spreads a Partial<Options> into useQuery which lets callers override core fields (queryKey/queryFn) and also uses a non-null assertion on platform; fix it by narrowing or omitting queryKey and queryFn from the passed options (e.g. pick/omit so you spread only safe fields) and build queryKey/queryFn locally, and change the queryFn to defensively guard against missing platform (throw or return early) instead of using platform!; update useQuery call in useGetDexKlineCandles to always supply the internally constructed queryKey and a queryFn that checks DEX_PLATFORM_BY_CHAIN_ID.get(chainId) and calls getDexKlineCandles({ platform, ...params }) only when platform is present, while keeping the enabled flag logic intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/evm/src/libs/ws/WsClient.ts`:
- Line 59: The onmessage handler in WsClient (this.ws.onmessage) is casting
MessageEvent.data to string which can be Blob or ArrayBuffer in browsers; update
the handler to detect the runtime type and convert non-string payloads to string
before calling onMessage: if typeof data === 'string' call this.onMessage(data),
if data is an ArrayBuffer use TextDecoder to decode it, and if data is a Blob
use FileReader (or blob.text()) to asynchronously obtain text then call
this.onMessage; alternatively ensure ws.binaryType is set and handle accordingly
so onMessage always receives a proper string.
---
Duplicate comments:
In `@apps/evm/src/clients/api/queries/getDexKlineCandles/types.ts`:
- Around line 30-39: ApiDexKlineCandle’s seventh tuple entry is documented as
optional but currently typed as required (number | null); update the tuple so
the seventh element is optional. In
apps/evm/src/clients/api/queries/getDexKlineCandles/types.ts change the tuple
declaration for ApiDexKlineCandle so the 7th element is typed as (number |
null)? (i.e., make the ut element optional) to allow 6- and 7-element candles.
---
Nitpick comments:
In `@apps/evm/src/clients/api/queries/getDexKlineCandles/index.ts`:
- Around line 33-40: The fetch call that requests
`${config.dexApiUrl}/k-line/candles?${queryParams}` should use an
AbortController with a timeout to avoid hanging; create an AbortController, pass
its signal into fetch, start a setTimeout that calls controller.abort() after a
configurable ms, clear the timeout once the response arrives, and catch an
aborted/timeout error to throw a distinct VError (e.g., type 'timeout' or code
'requestTimedOut') instead of the generic unexpected VError; update the
fetch/response handling in the getDexKlineCandles flow to include these changes
and ensure the timeout is cleaned up on success or error.
In
`@apps/evm/src/clients/api/queries/getDexKlineCandles/useGetDexKlineCandles.ts`:
- Around line 29-49: The hook useGetDexKlineCandles currently spreads a
Partial<Options> into useQuery which lets callers override core fields
(queryKey/queryFn) and also uses a non-null assertion on platform; fix it by
narrowing or omitting queryKey and queryFn from the passed options (e.g.
pick/omit so you spread only safe fields) and build queryKey/queryFn locally,
and change the queryFn to defensively guard against missing platform (throw or
return early) instead of using platform!; update useQuery call in
useGetDexKlineCandles to always supply the internally constructed queryKey and a
queryFn that checks DEX_PLATFORM_BY_CHAIN_ID.get(chainId) and calls
getDexKlineCandles({ platform, ...params }) only when platform is present, while
keeping the enabled flag logic intact.
In `@apps/evm/src/components/KLineChart/index.tsx`:
- Around line 106-116: Extract the duplicated setDataLoader configuration into a
single factory/helper and use it in both places: create a function (e.g.,
buildDataLoader) that returns an object with getBars, subscribeBar, and
unsubscribeBar implementations referencing latestDataRef and
subscribeCallbackRef, then call chart.setDataLoader(buildDataLoader()) wherever
the loader is currently duplicated (both in the initial setup effect and the
data-driven effect); ensure function uses the same closure variables
(latestDataRef, subscribeCallbackRef, chart) so the behavior remains identical
and any future changes are made in one place.
In `@apps/evm/src/libs/ws/WsClient.ts`:
- Around line 80-85: Replace the fixed delay in scheduleReconnect with an
exponential backoff: add a numeric state (e.g., this.reconnectAttempts) that
increments each time scheduleReconnect runs, compute delayMs =
Math.min(baseDelayMs * 2 ** this.reconnectAttempts, maxDelayMs) (choose
baseDelayMs like 3000 and a max cap), call setTimeout to invoke this.connect(),
store/clear this.reconnectTimer as before, and reset this.reconnectAttempts to 0
on successful connection inside connect() (or on open event) so retries start
from base again; keep existing reconnectTimer cancellation behavior unchanged.
In `@apps/evm/src/pages/YieldPlus/index.tsx`:
- Around line 29-35: The current useMemo for longTokenDexAddress returns
longToken.address when a native token's wrapped lookup fails, which can lead to
invalid DEX queries; change the logic in the useMemo (the longTokenDexAddress
computation that uses areAddressesEqual, longToken and tokens) to return
undefined (or null) when longToken.isNative and no wrappedToken is found instead
of falling back to longToken.address, and then ensure any kline queries or
callers that depend on longTokenDexAddress are guarded/disabled when it is
undefined (e.g., skip or early-return in the kline query hook or component that
uses longTokenDexAddress).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 09658a34-b745-487e-b187-89a2e60f5c2b
📒 Files selected for processing (18)
apps/evm/src/clients/api/__mocks__/index.tsapps/evm/src/clients/api/index.tsapps/evm/src/clients/api/queries/getDexKlineCandles/DexKlineWsClient.tsapps/evm/src/clients/api/queries/getDexKlineCandles/index.tsapps/evm/src/clients/api/queries/getDexKlineCandles/types.tsapps/evm/src/clients/api/queries/getDexKlineCandles/useDexKlineWebSocket.tsapps/evm/src/clients/api/queries/getDexKlineCandles/useGetDexKlineCandles.tsapps/evm/src/components/KLineChart/index.tsxapps/evm/src/config/envVariables.tsapps/evm/src/config/index.tsapps/evm/src/constants/functionKey.tsapps/evm/src/hooks/useIsFeatureEnabled/index.tsxapps/evm/src/libs/ws/WsClient.tsapps/evm/src/libs/ws/index.tsapps/evm/src/pages/YieldPlus/PairInfo/index.tsxapps/evm/src/pages/YieldPlus/index.tsxapps/evm/src/setupTests.tsxapps/evm/src/utilities/restService.ts
Contributor
Contributor
Author
|
@greptile |
cuzz-venus
commented
Mar 19, 2026
Contributor
Author
cuzz-venus
left a comment
cuzz-venus
left a comment
There was a problem hiding this comment.
no need change, it will always string
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/evm/src/utilities/restService.ts (1)
54-64:⚠️ Potential issue | 🟠 MajorDuplicate
Accept-Versionheader logic.Lines 54-58 and 60-64 are identical blocks that both set the
Accept-Versionheader. This appears to be an accidental duplication that should be removed.🐛 Proposed fix to remove duplicate block
if (next) { _set(headers, 'Accept-Version', 'next'); } else { _set(headers, 'Accept-Version', 'stable'); } - if (next) { - _set(headers, 'Accept-Version', 'next'); - } else { - _set(headers, 'Accept-Version', 'stable'); - } - if (token) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/evm/src/utilities/restService.ts` around lines 54 - 64, Remove the duplicated Accept-Version header block: there are two identical if/else blocks that call _set(headers, 'Accept-Version', next ? 'next' : 'stable'); keep only one. Locate the duplicate logic that references the variables next and headers and the _set function in restService.ts and delete the redundant second block so the header is set exactly once.
♻️ Duplicate comments (1)
apps/evm/src/libs/ws/WsClient.ts (1)
59-59:⚠️ Potential issue | 🟡 MinorWebSocket
datamay not always be a string.In browser environments,
MessageEvent.datacan be aBloborArrayBufferdepending on thebinaryTypesetting. The castdata as stringcould cause issues if the server sends binary data. Consider adding a type guard.🛡️ Proposed fix to handle data type safely
- this.ws.onmessage = ({ data }) => this.onMessage(data as string); + this.ws.onmessage = ({ data }) => { + if (typeof data === 'string') { + this.onMessage(data); + } + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/evm/src/libs/ws/WsClient.ts` at line 59, The onmessage handler currently casts MessageEvent.data to string which can break for Blob/ArrayBuffer; update the this.ws.onmessage assignment in WsClient so it type-guards MessageEvent.data, handling string directly, calling blob.text() for Blob, and using TextDecoder for ArrayBuffer (or Uint8Array) before passing the resulting string into onMessage; ensure any asynchronous blob.text() promise is awaited or handled so onMessage receives a string consistently.
🧹 Nitpick comments (9)
apps/evm/src/pages/YieldPlus/PairInfo/index.tsx (1)
151-158: Consider neutral styling whenchangePercentageis undefined.When
changePercentageis undefined, the placeholder text displays in green, which may misleadingly suggest positive data. Consider using a neutral color (e.g.,text-grey) for the undefined case.♻️ Suggested change
<p className={cn( 'text-b1s', - changePercentage === undefined || changePercentage >= 0 ? 'text-green' : 'text-red', + changePercentage === undefined + ? 'text-grey' + : changePercentage >= 0 + ? 'text-green' + : 'text-red', )} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/evm/src/pages/YieldPlus/PairInfo/index.tsx` around lines 151 - 158, The text color currently uses changePercentage === undefined || changePercentage >= 0 to choose 'text-green', which makes undefined values appear positive; update the conditional in the JSX that builds the className (the cn call rendering readableChangePercentage) to handle undefined separately so undefined uses a neutral class like 'text-grey' and only values >= 0 use 'text-green' while negatives use 'text-red' (refer to changePercentage and readableChangePercentage in the component to locate the JSX).apps/evm/src/utilities/restService.ts (1)
31-37: Consider URL-encoding query parameter values.Values containing special characters (e.g.,
&,=, spaces) will corrupt the query string. UseencodeURIComponentfor robustness.♻️ Proposed fix
export const createQueryParams = (params: Record<string, unknown>) => { const paramArray = Object.entries(params).map(([key, value]) => { if (value !== undefined && value !== null) { - return `${key}=${value}`; + return `${encodeURIComponent(key)}=${encodeURIComponent(String(value))}`; } return ''; }); return paramArray.filter(p => p).join('&'); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/evm/src/utilities/restService.ts` around lines 31 - 37, The query-building logic maps Object.entries(params) into paramArray but doesn't URL-encode keys or values, so special characters can break the query string; update the code that constructs paramArray (the mapping over params / the paramArray variable) to call encodeURIComponent on both key and value (e.g., encodeURIComponent(key) and encodeURIComponent(value)) while preserving the existing checks for undefined/null, then continue filtering and joining as before.apps/evm/src/components/KLineChart/index.tsx (1)
143-153: Consider extracting thesetDataLoaderconfiguration.The
subscribeBar/unsubscribeBarhandlers at lines 110-115 and 147-152 are duplicated. Extracting a helper would reduce repetition, though this is optional given the small scope.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/evm/src/components/KLineChart/index.tsx` around lines 143 - 153, The setDataLoader configuration is duplicated for subscribeBar/unsubscribeBar; refactor by extracting a helper that returns the handlers and reuse it in both places: create a function (e.g., buildBarSubscriptionHandlers) that captures subscribeCallbackRef and returns an object with subscribeBar(callbackSetter) and unsubscribeBar() or directly returns { subscribeBar, unsubscribeBar } to pass into chartRef.current?.setDataLoader and elsewhere; ensure getBars remains unchanged and that subscribeCallbackRef is assigned/cleared inside the helper so both occurrences call chartRef.current?.setDataLoader({ getBars, ...buildBarSubscriptionHandlers() }).apps/evm/src/clients/api/queries/getDexKlineCandles/index.ts (1)
33-39: Consider adding a timeout to prevent indefinite hangs.The
fetchcall has no timeout, which could cause the request to hang indefinitely if the DEX API is unresponsive. This could degrade user experience.♻️ Proposed fix using AbortController
export const getDexKlineCandles = async ({ platform, address, interval, limit, from, to, unit, tokenIndex, }: GetDexKlineCandlesInput): Promise<GetDexKlineCandlesOutput> => { const queryParams = createQueryParams({ platform, address, interval, limit, from, to, unit, tokenIndex, }); + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 30000); + - const response = await fetch(`${config.dexApiUrl}/k-line/candles?${queryParams}`, { - headers: { Accept: 'application/json' }, - }); + const response = await fetch(`${config.dexApiUrl}/k-line/candles?${queryParams}`, { + headers: { Accept: 'application/json' }, + signal: controller.signal, + }).finally(() => clearTimeout(timeoutId));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/evm/src/clients/api/queries/getDexKlineCandles/index.ts` around lines 33 - 39, The fetch to `${config.dexApiUrl}/k-line/candles` in getDexKlineCandles can hang indefinitely; fix it by using an AbortController: create an AbortController, start a setTimeout (e.g., 5s) that calls controller.abort(), pass controller.signal to fetch, and clear the timeout after fetch completes; also catch the abort case and throw a VError with a clear timeout-type/code instead of the generic error so callers can distinguish timeouts from other failures (reference: getDexKlineCandles, response, config.dexApiUrl).apps/evm/src/clients/api/queries/getDexKlineCandles/useDexKlineWebSocket.ts (1)
18-24: Module-level singleton WebSocket client is never cleaned up.The
sharedClientis lazily created but never destroyed. While this is likely intentional for performance (keeping the connection alive across page navigations), the WebSocket connection will persist until the browser tab is closed, even if the user navigates away from the YieldPlus page entirely.Consider whether this is the desired behavior, or if cleanup logic should be added for when no subscriptions remain.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/evm/src/clients/api/queries/getDexKlineCandles/useDexKlineWebSocket.ts` around lines 18 - 24, The module-level singleton sharedClient (created by getSharedClient) is never cleaned up, so the DexKlineWsClient WebSocket stays open indefinitely; add lifecycle/cleanup logic: implement a dispose/close method on DexKlineWsClient (if not present), track active subscriptions count in the module (increment when subscribing, decrement when unsubscribing), and when count reaches zero call sharedClient.close()/dispose() and set sharedClient = null to fully tear down the connection; update subscription management code to use these counters and ensure getSharedClient still lazily creates a client when needed.apps/evm/src/pages/YieldPlus/index.tsx (1)
56-61: Consider edge case when live candle updates during initial fetch.The
changePercentagecalculation compares the latest candle (live or fetched) againstklineData?.candles?.at(-2). If the live candle arrives before/during the initial fetch or there's only one historical candle,prevCandlewill beundefinedand the percentage returnsundefined(handled correctly).However, when comparing
liveCandleagainst the second-to-last fetched candle, you may skip showing the change from the last fetched candle to the live candle's predecessor. This might be intentional since the live candle updates the last bar, but worth confirming the UX is as expected.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/evm/src/pages/YieldPlus/index.tsx` around lines 56 - 61, The changePercentage useMemo can miss a valid previous candle when liveCandle exists because prevCandle is always taken as klineData?.candles?.at(-2); update the logic in changePercentage (referencing changePercentage, liveCandle, and klineData?.candles?.at) so that if liveCandle is present and there is at least one fetched candle you treat the last fetched candle (at(-1)) as prevCandle, otherwise fall back to at(-2); keep the existing guard for undefined/zero and return undefined when no valid prevCandle exists.apps/evm/src/libs/ws/WsClient.ts (2)
60-60: Silent error handling may hinder debugging.The error handler closes the socket without logging. Consider adding minimal logging to aid debugging connection issues in production.
♻️ Proposed enhancement
- this.ws.onerror = () => this.ws?.close(); + this.ws.onerror = (event) => { + console.warn('[WsClient] WebSocket error, closing connection', event); + this.ws?.close(); + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/evm/src/libs/ws/WsClient.ts` at line 60, The onerror handler on WsClient currently closes the socket silently; modify the this.ws.onerror callback to capture the error/event (accept the event param) and log a concise message including the event or error details before calling this.ws.close(); use the class logger if available (e.g., this.logger.error) or console.error as a fallback, and preserve the existing close behavior so the socket still shuts down after logging.
80-85: Consider exponential backoff for reconnection.The fixed 3-second delay works, but exponential backoff with jitter is more resilient against server overload during outages and reduces thundering-herd effects.
♻️ Proposed enhancement
+ private reconnectAttempts = 0; + private readonly maxReconnectDelay = 30000; + - private scheduleReconnect(delayMs = 3000): void { + private scheduleReconnect(): void { + const baseDelay = 1000; + const delay = Math.min(baseDelay * 2 ** this.reconnectAttempts, this.maxReconnectDelay); + const jitter = Math.random() * 1000; this.reconnectTimer = setTimeout(() => { this.reconnectTimer = null; + this.reconnectAttempts++; this.connect(); - }, delayMs); + }, delay + jitter); }Reset
reconnectAttemptsto 0 inonopenhandler.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/evm/src/libs/ws/WsClient.ts` around lines 80 - 85, Replace the fixed 3s reconnect delay in scheduleReconnect with an exponential-backoff plus jitter calculation based on a reconnectAttempts counter: increment reconnectAttempts before scheduling, compute delay = min(maxDelayMs, baseDelayMs * 2**reconnectAttempts) and add some random jitter (± up to e.g. 20% of delay), then set reconnectTimer and call this.connect() after that delay; ensure you reference scheduleReconnect and reconnectTimer when changing behavior and reset reconnectAttempts = 0 in the WebSocket onopen handler so successful connections clear the backoff.apps/evm/src/clients/api/queries/getDexKlineCandles/types.ts (1)
50-62: Consider removing the index signature if not needed.The
[key: string]: unknownindex signature onDexKlineCandleallows arbitrary additional properties. If the candle shape is well-defined and no extra fields are expected, removing this provides stricter type safety and better autocompletion.♻️ Proposed fix
export interface DexKlineCandle { timestamp: number; open: number; high: number; low: number; close: number; volume: number; - [key: string]: unknown; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/evm/src/clients/api/queries/getDexKlineCandles/types.ts` around lines 50 - 62, The DexKlineCandle interface currently includes an open index signature (`[key: string]: unknown`) which permits arbitrary properties; if the candle shape is fixed, remove that index signature from DexKlineCandle to enforce stricter typing, keep the explicit fields (timestamp, open, high, low, close, volume), and then update any places that depended on extra keys (casts, spreads, or dynamic property access) to either add explicit fields or use a separate record type where truly needed; ensure GetDexKlineCandlesOutput remains unchanged and run type checks to find the usages to adjust.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/evm/src/config/envVariables.ts`:
- Around line 43-48: Add VITE_DEX_API_URL and VITE_DEX_WS_URL to the project's
example environment file and/or documentation so developers know the required
endpoints and expected format; update .env.example (or relevant docs) to include
entries for VITE_DEX_API_URL and VITE_DEX_WS_URL with brief placeholder values
and a short comment, referencing the variables used in
apps/evm/src/config/envVariables.ts (VITE_DEX_API_URL, VITE_DEX_WS_URL) and the
fallback behavior in apps/evm/src/config/index.ts so the example aligns with the
code.
---
Outside diff comments:
In `@apps/evm/src/utilities/restService.ts`:
- Around line 54-64: Remove the duplicated Accept-Version header block: there
are two identical if/else blocks that call _set(headers, 'Accept-Version', next
? 'next' : 'stable'); keep only one. Locate the duplicate logic that references
the variables next and headers and the _set function in restService.ts and
delete the redundant second block so the header is set exactly once.
---
Duplicate comments:
In `@apps/evm/src/libs/ws/WsClient.ts`:
- Line 59: The onmessage handler currently casts MessageEvent.data to string
which can break for Blob/ArrayBuffer; update the this.ws.onmessage assignment in
WsClient so it type-guards MessageEvent.data, handling string directly, calling
blob.text() for Blob, and using TextDecoder for ArrayBuffer (or Uint8Array)
before passing the resulting string into onMessage; ensure any asynchronous
blob.text() promise is awaited or handled so onMessage receives a string
consistently.
---
Nitpick comments:
In `@apps/evm/src/clients/api/queries/getDexKlineCandles/index.ts`:
- Around line 33-39: The fetch to `${config.dexApiUrl}/k-line/candles` in
getDexKlineCandles can hang indefinitely; fix it by using an AbortController:
create an AbortController, start a setTimeout (e.g., 5s) that calls
controller.abort(), pass controller.signal to fetch, and clear the timeout after
fetch completes; also catch the abort case and throw a VError with a clear
timeout-type/code instead of the generic error so callers can distinguish
timeouts from other failures (reference: getDexKlineCandles, response,
config.dexApiUrl).
In `@apps/evm/src/clients/api/queries/getDexKlineCandles/types.ts`:
- Around line 50-62: The DexKlineCandle interface currently includes an open
index signature (`[key: string]: unknown`) which permits arbitrary properties;
if the candle shape is fixed, remove that index signature from DexKlineCandle to
enforce stricter typing, keep the explicit fields (timestamp, open, high, low,
close, volume), and then update any places that depended on extra keys (casts,
spreads, or dynamic property access) to either add explicit fields or use a
separate record type where truly needed; ensure GetDexKlineCandlesOutput remains
unchanged and run type checks to find the usages to adjust.
In `@apps/evm/src/clients/api/queries/getDexKlineCandles/useDexKlineWebSocket.ts`:
- Around line 18-24: The module-level singleton sharedClient (created by
getSharedClient) is never cleaned up, so the DexKlineWsClient WebSocket stays
open indefinitely; add lifecycle/cleanup logic: implement a dispose/close method
on DexKlineWsClient (if not present), track active subscriptions count in the
module (increment when subscribing, decrement when unsubscribing), and when
count reaches zero call sharedClient.close()/dispose() and set sharedClient =
null to fully tear down the connection; update subscription management code to
use these counters and ensure getSharedClient still lazily creates a client when
needed.
In `@apps/evm/src/components/KLineChart/index.tsx`:
- Around line 143-153: The setDataLoader configuration is duplicated for
subscribeBar/unsubscribeBar; refactor by extracting a helper that returns the
handlers and reuse it in both places: create a function (e.g.,
buildBarSubscriptionHandlers) that captures subscribeCallbackRef and returns an
object with subscribeBar(callbackSetter) and unsubscribeBar() or directly
returns { subscribeBar, unsubscribeBar } to pass into
chartRef.current?.setDataLoader and elsewhere; ensure getBars remains unchanged
and that subscribeCallbackRef is assigned/cleared inside the helper so both
occurrences call chartRef.current?.setDataLoader({ getBars,
...buildBarSubscriptionHandlers() }).
In `@apps/evm/src/libs/ws/WsClient.ts`:
- Line 60: The onerror handler on WsClient currently closes the socket silently;
modify the this.ws.onerror callback to capture the error/event (accept the event
param) and log a concise message including the event or error details before
calling this.ws.close(); use the class logger if available (e.g.,
this.logger.error) or console.error as a fallback, and preserve the existing
close behavior so the socket still shuts down after logging.
- Around line 80-85: Replace the fixed 3s reconnect delay in scheduleReconnect
with an exponential-backoff plus jitter calculation based on a reconnectAttempts
counter: increment reconnectAttempts before scheduling, compute delay =
min(maxDelayMs, baseDelayMs * 2**reconnectAttempts) and add some random jitter
(± up to e.g. 20% of delay), then set reconnectTimer and call this.connect()
after that delay; ensure you reference scheduleReconnect and reconnectTimer when
changing behavior and reset reconnectAttempts = 0 in the WebSocket onopen
handler so successful connections clear the backoff.
In `@apps/evm/src/pages/YieldPlus/index.tsx`:
- Around line 56-61: The changePercentage useMemo can miss a valid previous
candle when liveCandle exists because prevCandle is always taken as
klineData?.candles?.at(-2); update the logic in changePercentage (referencing
changePercentage, liveCandle, and klineData?.candles?.at) so that if liveCandle
is present and there is at least one fetched candle you treat the last fetched
candle (at(-1)) as prevCandle, otherwise fall back to at(-2); keep the existing
guard for undefined/zero and return undefined when no valid prevCandle exists.
In `@apps/evm/src/pages/YieldPlus/PairInfo/index.tsx`:
- Around line 151-158: The text color currently uses changePercentage ===
undefined || changePercentage >= 0 to choose 'text-green', which makes undefined
values appear positive; update the conditional in the JSX that builds the
className (the cn call rendering readableChangePercentage) to handle undefined
separately so undefined uses a neutral class like 'text-grey' and only values >=
0 use 'text-green' while negatives use 'text-red' (refer to changePercentage and
readableChangePercentage in the component to locate the JSX).
In `@apps/evm/src/utilities/restService.ts`:
- Around line 31-37: The query-building logic maps Object.entries(params) into
paramArray but doesn't URL-encode keys or values, so special characters can
break the query string; update the code that constructs paramArray (the mapping
over params / the paramArray variable) to call encodeURIComponent on both key
and value (e.g., encodeURIComponent(key) and encodeURIComponent(value)) while
preserving the existing checks for undefined/null, then continue filtering and
joining as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cc227676-66fa-4017-9ef0-0b105f43fceb
📒 Files selected for processing (19)
apps/evm/src/clients/api/__mocks__/index.tsapps/evm/src/clients/api/index.tsapps/evm/src/clients/api/queries/getDexKlineCandles/DexKlineWsClient.tsapps/evm/src/clients/api/queries/getDexKlineCandles/index.tsapps/evm/src/clients/api/queries/getDexKlineCandles/types.tsapps/evm/src/clients/api/queries/getDexKlineCandles/useDexKlineWebSocket.tsapps/evm/src/clients/api/queries/getDexKlineCandles/useGetDexKlineCandles.tsapps/evm/src/components/KLineChart/index.tsxapps/evm/src/config/envVariables.tsapps/evm/src/config/index.tsapps/evm/src/constants/functionKey.tsapps/evm/src/hooks/useIsFeatureEnabled/index.tsxapps/evm/src/libs/ws/WsClient.tsapps/evm/src/libs/ws/index.tsapps/evm/src/pages/YieldPlus/PairInfo/index.tsxapps/evm/src/pages/YieldPlus/__tests__/index.spec.tsxapps/evm/src/pages/YieldPlus/index.tsxapps/evm/src/setupTests.tsxapps/evm/src/utilities/restService.ts
Comment on lines
+43
to
+48
| VITE_DEX_API_URL: | ||
| typeof process !== 'undefined' | ||
| ? process.env.VITE_DEX_API_URL | ||
| : import.meta.env.VITE_DEX_API_URL, | ||
| VITE_DEX_WS_URL: | ||
| typeof process !== 'undefined' ? process.env.VITE_DEX_WS_URL : import.meta.env.VITE_DEX_WS_URL, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if new env variables are documented and have fallback handling.
# Check for .env.example or similar documentation
fd -t f -e example -e sample | xargs rg -l 'DEX_API_URL\|DEX_WS_URL' 2>/dev/null || echo "No .env example files found with DEX vars"
# Check config/index.ts for fallback handling
rg -n 'dexApiUrl|dexWsUrl' apps/evm/src/config/Repository: VenusProtocol/venus-protocol-interface
Length of output: 390
Add these environment variables to .env.example or documentation.
Code pattern is correct and follows established conventions. Fallback handling in apps/evm/src/config/index.ts (lines 50-51) is properly implemented with empty string defaults. However, VITE_DEX_API_URL and VITE_DEX_WS_URL are missing from example configuration files, making it unclear to developers how to configure these required endpoints.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/evm/src/config/envVariables.ts` around lines 43 - 48, Add
VITE_DEX_API_URL and VITE_DEX_WS_URL to the project's example environment file
and/or documentation so developers know the required endpoints and expected
format; update .env.example (or relevant docs) to include entries for
VITE_DEX_API_URL and VITE_DEX_WS_URL with brief placeholder values and a short
comment, referencing the variables used in apps/evm/src/config/envVariables.ts
(VITE_DEX_API_URL, VITE_DEX_WS_URL) and the fallback behavior in
apps/evm/src/config/index.ts so the example aligns with the code.
Contributor
Author
|
@greptile |
Contributor
Author
|
@greptile |
Contributor
Author
|
@greptile |
Member
|
Work merged with #5463 |
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
3 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.
Jira ticket(s)
VEN-626
Changes
Summary by CodeRabbit
New Features
Tests