fix: improve wallet connection and portfolio query reliability #170
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis change refactors the account lookup tool throughout the agentic stack by renaming from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/agentic-server/src/routes/chat.ts (1)
97-119: Consider simplifying the conditional routing logic.The logic is functionally correct but could be more readable. The double-check pattern (line 109 includes check, then line 112 undefined check) is safe but verbose.
Consider this cleaner approach:
lookupExternalAddress: { description: lookupExternalAddressTool.description, inputSchema: lookupExternalAddressTool.inputSchema, execute: async (args: GetAccountInput) => { console.log('[Tool] lookupExternalAddress:', JSON.stringify(args, null, 2)) const { address, network } = args const chainId = networkToChainIdMap[network] const connectedAddress = walletContext.connectedWallets?.[chainId]?.address if (connectedAddress && address.toLowerCase() === connectedAddress.toLowerCase()) { console.log('[Tool] lookupExternalAddress: Redirecting to portfolioTool (connected wallet detected)') - const evmSolanaNetwork = EVM_SOLANA_NETWORKS.includes(network as EvmSolanaNetwork) - ? (network as EvmSolanaNetwork) - : undefined - if (evmSolanaNetwork) { - return portfolioTool.execute({ networks: [evmSolanaNetwork] }, walletContext) - } + if (EVM_SOLANA_NETWORKS.includes(network as EvmSolanaNetwork)) { + return portfolioTool.execute({ networks: [network as EvmSolanaNetwork] }, walletContext) + } } return lookupExternalAddressTool.execute(args) }, },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
apps/agentic-chat/src/components/CustomConnectButton.tsx(2 hunks)apps/agentic-chat/src/components/toolUIRegistry.tsx(1 hunks)apps/agentic-chat/src/components/tools/GetAccountUI.tsx(1 hunks)apps/agentic-server/src/index.ts(1 hunks)apps/agentic-server/src/routes/chat.ts(2 hunks)apps/agentic-server/src/tools/getAccount.ts(3 hunks)apps/agentic-server/src/tools/portfolio.ts(1 hunks)apps/agentic-server/src/utils/balanceHelpers.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (5)
apps/agentic-chat/src/components/toolUIRegistry.tsx (1)
apps/agentic-chat/src/components/tools/GetAccountUI.tsx (1)
GetAccountUI(4-29)
apps/agentic-chat/src/components/tools/GetAccountUI.tsx (1)
apps/agentic-chat/src/components/tools/toolUIHelpers.tsx (1)
useToolStateRender(10-31)
apps/agentic-server/src/tools/getAccount.ts (3)
packages/types/src/network.ts (1)
networkToChainIdMap(63-82)apps/agentic-server/src/utils/addressValidation.ts (1)
validateAddress(6-14)apps/agentic-server/src/index.ts (1)
lookupExternalAddressTool(12-12)
apps/agentic-server/src/tools/portfolio.ts (1)
apps/agentic-server/src/tools/getAccount.ts (1)
executeGetAccount(27-94)
apps/agentic-server/src/routes/chat.ts (4)
apps/agentic-server/src/index.ts (4)
lookupExternalAddressTool(12-12)GetAccountInput(14-14)GetAccountInput(15-15)portfolioTool(40-40)apps/agentic-server/src/tools/getAccount.ts (2)
lookupExternalAddressTool(96-101)GetAccountInput(19-19)packages/types/src/network.ts (3)
networkToChainIdMap(63-82)EVM_SOLANA_NETWORKS(59-59)EvmSolanaNetwork(61-61)apps/agentic-server/src/tools/portfolio.ts (1)
portfolioTool(182-187)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: main
🔇 Additional comments (11)
apps/agentic-server/src/index.ts (1)
12-12: LGTM! Export rename is consistent.The export rename from
getAccountTooltolookupExternalAddressToolcorrectly aligns with the tool's updated purpose of looking up external wallet addresses.apps/agentic-server/src/tools/portfolio.ts (1)
84-84: LGTM! Parameter update aligns with schema changes.The parameter change from
{ account, network }to{ address: account, network }correctly matches the updatedGetAccountInputschema.apps/agentic-server/src/utils/balanceHelpers.ts (1)
9-12: LGTM! Cleaner parameter passing.Using the shorthand
{ address, network }is more idiomatic and aligns with the updated schema.apps/agentic-chat/src/components/toolUIRegistry.tsx (1)
29-29: LGTM! Registry key updated consistently.The registry key
lookupExternalAddresscorrectly aligns with the renamed server-side tool.apps/agentic-chat/src/components/tools/GetAccountUI.tsx (2)
8-18: LGTM! Good backward compatibility.The fallback
input?.address ?? input?.accountensures compatibility during the transition while the label update correctly reflects the tool's purpose.
20-24: LGTM! Clear status messages.The updated messages ("Looking up", "Failed to look up") are consistent with the external address lookup workflow.
apps/agentic-server/src/tools/getAccount.ts (4)
8-8: LGTM! Address validation import added.The new import enables proper address validation before querying external addresses.
11-15: Excellent input description for LLM guidance.The detailed description with examples ("0x..." for EVM, "base58" for Solana) and the explicit warning "NEVER pass descriptions like 'connected wallet'" helps guide the LLM to use this tool correctly.
33-34: LGTM! Address validation added for security.Validating the address format before making external API calls is a good security practice that will catch malformed addresses early.
96-98: Clear tool description distinguishes from portfolioTool.The description effectively communicates when to use this tool versus
portfolioTool, which should help the LLM make the right choice.apps/agentic-server/src/routes/chat.ts (1)
107-108: Good case-insensitive address comparison.The case-insensitive comparison correctly handles Ethereum addresses which are case-insensitive (ignoring EIP-55 checksums).
premiumjibles
force-pushed
the
wallet-connect-reliability
branch
from
fdf06bf to
fa3498a
Compare
premiumjibles
force-pushed
the
wallet-connect-reliability
branch
from
fa3498a to
eb7eade
Compare
premiumjibles
force-pushed
the
wallet-connect-reliability
branch
from
eb7eade to
e269c5e
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/agentic-server/src/tools/getAccount.ts (1)
1-101: Consider renaming the file for consistency.The file is named
getAccount.tsbut now exportslookupExternalAddressTooland uses[lookupExternalAddress]logging. Additionally, theGetAccountOutputtype andexecuteGetAccountfunction retain the old naming. While the code functions correctly, renaming the file tolookupExternalAddress.tsand updating the type/function names would improve clarity and reduce confusion for future maintainers.Suggested renaming:
- File:
getAccount.ts→lookupExternalAddress.ts- Type:
GetAccountOutput→LookupExternalAddressOutput- Function:
executeGetAccount→executeLookupExternalAddressapps/agentic-server/src/utils/balanceHelpers.ts (1)
9-12: Correct alignment with the updated schema.The change from
account: addresstoaddress: addressproperly aligns with the renamed schema field. Consider using ES6 shorthand property syntax for slightly cleaner code.Apply this diff to use ES6 shorthand:
const accountData = await executeGetAccount({ - address, + address, network, })Actually, the current code already uses the shorthand if it's just
address,- please verify the actual formatting. If it's written asaddress: address,, then apply:const accountData = await executeGetAccount({ - address: address, + address, network, })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
apps/agentic-chat/src/components/toolUIRegistry.tsx(1 hunks)apps/agentic-chat/src/components/tools/GetAccountUI.tsx(1 hunks)apps/agentic-server/src/index.ts(1 hunks)apps/agentic-server/src/routes/chat.ts(2 hunks)apps/agentic-server/src/tools/getAccount.ts(3 hunks)apps/agentic-server/src/tools/portfolio.ts(1 hunks)apps/agentic-server/src/utils/balanceHelpers.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/agentic-server/src/routes/chat.ts
- apps/agentic-chat/src/components/toolUIRegistry.tsx
- apps/agentic-server/src/index.ts
- apps/agentic-server/src/tools/portfolio.ts
🧰 Additional context used
🧬 Code graph analysis (2)
apps/agentic-chat/src/components/tools/GetAccountUI.tsx (1)
apps/agentic-chat/src/components/tools/toolUIHelpers.tsx (1)
useToolStateRender(10-31)
apps/agentic-server/src/tools/getAccount.ts (3)
packages/types/src/network.ts (1)
networkToChainIdMap(63-82)apps/agentic-server/src/utils/addressValidation.ts (1)
validateAddress(6-14)apps/agentic-server/src/index.ts (1)
lookupExternalAddressTool(12-12)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: main
🔇 Additional comments (6)
apps/agentic-server/src/tools/getAccount.ts (4)
8-9: LGTM!The import is correctly placed and the validation function is properly utilized later in the code.
11-15: Excellent schema description clarity.The emphatic description clearly distinguishes between external addresses and placeholder text. This should help prevent the LLM from passing invalid inputs like "connected wallet" or "my wallet".
28-34: LGTM! Validation and logging updates are well-implemented.The validation is correctly placed after the chainId is determined (since the validation function requires it), and the destructuring pattern
{ address: account, network }maintains internal backward compatibility while supporting the new schema.
96-101: Excellent tool description clarity.The renamed export and emphatic description effectively distinguish this tool's purpose (external addresses only) from the portfolio tool (connected wallet). The explicit guidance should significantly improve tool selection reliability.
apps/agentic-chat/src/components/tools/GetAccountUI.tsx (2)
9-18: Excellent backward compatibility and defensive programming.The fallback logic
input?.address ?? input?.accountensures the UI works with both old and new message formats, and the type guards prevent runtime errors. The label update to "external address" aligns well with the tool's renamed purpose.
21-24: LGTM! Message updates are consistent.The updated loading and error messages ("Looking up" and "Failed to look up") are appropriate for the external address lookup context. All messages consistently use
accountDetailsTextfor cohesive user feedback.
Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.