feat: make asset searching more powerful and flexible #137
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds contract-address lookup and filtering to asset search and the getAssets tool; centralizes asset enrichment via a new hydrateAsset helper; AssetService gains contract indexing plus Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Agent
participant Tool as getAssetsTool
participant Service as AssetService
participant Enricher as hydrateAsset
participant Market as MarketData
Client->>Tool: request({searchTerm|assetId|contractAddress}, filters)
Tool->>Service: route lookup
alt assetId provided
Service->>Service: getByAssetId(assetId)
Service-->>Tool: staticAsset?
else contractAddress provided
Service->>Service: searchByContract(contractAddress, network?)
Service-->>Tool: matching staticAssets
else searchTerm provided
Service->>Service: searchWithFilters(term, {network, assetType, pools})
Service-->>Tool: filtered staticAssets
end
loop for each staticAsset
Tool->>Enricher: hydrateAsset(staticAsset, network?)
Enricher->>Market: fetch (optional) market data (CoinGecko)
Market-->>Enricher: price/market info (or none)
Enricher-->>Tool: enriched asset
end
Tool-->>Client: GetAssetsOutput (assets[])
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 (6)
packages/utils/src/AssetService.ts (3)
14-14: Contract index population matches CAIP‑19 layout; minor resilience improvement possibleIndexing non‑slip44 assets by the substring after the post‑slash colon is consistent with CAIP‑19’s
assetReferenceand should work well with your dataset. The only potential sharp edge is if futureassetIds violate the expectedchainId/namespace:referenceshape, in which caseindexOf(':', slashIdx)could be-1and you’d end up indexing under the entire string. If you expect the asset catalog to evolve, consider a small guard (e.g., skip whencolonIdx <= slashIdx) or centralizing CAIP‑19 parsing in a helper to keep this logic in one place.Also applies to: 26-26, 41-50
144-158: searchByContract behavior is sound; consider normalizing and DRY‑ing network filteringThe contract lookup is case‑insensitive and correctly reuses the same network/chainId resolution pattern as the symbol/name searches. Two small polish ideas:
contractAddresscould be.trim().toLowerCase()to avoid failures from accidental whitespace.- The network resolution and
(assetNetwork === normalized || asset.chainId[.toLowerCase()] === candidateChainId)pattern is now duplicated in three methods; extracting a tiny helper to compute the predicate would reduce drift between them.
160-189: Filtered search semantics look correct; be aware of assetType × pools corner cases
searchWithFilterscleanly layersassetTypeandpoolsfilters over the scoredsearch()results, and the defaults (assetType = 'all',pools = 'exclude') align with the tool description. A couple of subtle behaviors to keep in mind:
- “Native” is defined purely as
assetId.includes('/slip44:'), which is fine today but will silently misclassify if new namespaces are added for other native-like assets.- Because
assetTypeis applied beforepools, combinations likeassetType: 'native', pools: 'only'will (correctly but perhaps surprisingly) result in an empty list in most cases; if that’s not desired, you may want to special-casepools === 'only'to ignoreassetType.apps/agentic-server/src/tools/getAssets.ts (3)
65-92: Nice centralization of enrichment; consider typing and network fallback tweaks
enrichAndReturnis a good consolidation of the “StaticAsset → AssetWithMarketData” flow, and theassetIdToCoingecko+ graceful fallback pattern looks solid. Two small improvements you might consider:
- Type the
staticAssetparameter asStaticAsset(or a named subset) instead of inlining the shape to avoid drifting from the canonical type.- If
chainIdToNetwork[staticAsset.chainId]is everundefinedand nonetworkis provided, you silently setnetworkto''; logging or surfacing that mismatch could make bad catalog entries easier to detect.
94-116: Lookup order and new contract path look good; returning only the top match is a design choiceThe resolution order
assetId → contractAddress → searchTermis clear and predictable, and delegating all three paths throughenrichAndReturnkeeps the enrichment logic consistent. The only behavioral constraint to call out is that both the contract and search paths take only the first matching asset ([0]), even though the underlying service can return multiple matches (e.g., multi‑chain symbols or contract collisions). If you anticipate UIs that want to present a choice, you may eventually want a variant that returns the full (or top‑N) list.
121-123: Guard in executeGetAssets avoids pointless work; optional normalization for searchTermShort‑circuiting
executeGetAssetswhen all ofsearchTerm,assetId, andcontractAddressare falsy is sensible. If agents or UIs might send whitespace‑only search terms, you could defensively checksearchTerm?.trim()here to avoid treating" "as a real query and accidentally returning broad matches, but that’s strictly a polish improvement.
📜 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 (2)
apps/agentic-server/src/tools/getAssets.ts(2 hunks)packages/utils/src/AssetService.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/utils/src/AssetService.ts (2)
packages/types/src/asset.ts (2)
StaticAsset(30-30)asset(3-26)packages/types/src/network.ts (2)
networkToChainIdMap(63-82)chainIdToNetwork(84-86)
⏰ 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 (1)
apps/agentic-server/src/tools/getAssets.ts (1)
130-147: Tool description matches behavior and clearly documents new filtersThe updated description and METHODS/FILTERS sections align well with the implementation:
assetTypeandpoolsbehaviors mirrorsearchWithFilters, and the note about native assets being prioritized on symbol matches matches the scoring logic inAssetService.scoreMatch. This should make the tool significantly easier for agents to use correctly.
premiumjibles
force-pushed
the
flexible-assets-
branch
from
536df73 to
66f556c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/agentic-server/src/tools/getAssets.ts (1)
27-27: Documentation inconsistency remains unaddressed.The past review comment correctly identified that the description
"(requires network)"is inconsistent with the actual behavior. The schema marksnetworkas optional, andsearchByContract(line 104) acceptsundefinednetwork. The tool description at line 136 correctly showsnetwork?as optional.This should be updated to either:
- Remove
"(requires network)"- Replace with
"(optional; without network returns first match across all networks)"
🧹 Nitpick comments (1)
apps/agentic-server/src/tools/getAssets.ts (1)
66-74: Consider extracting the inline type definition.The 9-line inline type for
staticAssetmakes the function signature harder to scan. Extracting it to a named type (e.g.,StaticAssetorBaseAssetData) would improve readability and enable reuse if needed elsewhere.type StaticAssetData = { assetId: string chainId: string symbol: string name: string precision: number color?: string icon: string } async function enrichAndReturn( staticAsset: StaticAssetData, network?: string ): Promise<GetAssetsOutput> { // ... }
📜 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 (2)
apps/agentic-server/src/tools/getAssets.ts(2 hunks)packages/utils/src/AssetService.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/utils/src/AssetService.ts
⏰ 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 (4)
apps/agentic-server/src/tools/getAssets.ts (4)
39-63: LGTM!The error handling and fallback logic are well-implemented. Returning
nullon failure allows the caller to gracefully fall back to the base asset without market data.
94-116: LGTM!The three-path routing (assetId → contractAddress → searchTerm) is clear and well-structured. The null checks before calling
enrichAndReturnproperly handle cases where searches return no results.
118-128: LGTM!The routing logic correctly handles all three search methods. Returning an empty result when no search parameters are provided is a reasonable defensive approach, though you could optionally enforce at least one parameter at the schema level if desired.
130-147: LGTM!The tool description is comprehensive and accurately documents the three search methods and filter options. Line 136 correctly shows
network?as optional, which accurately reflects the implementation behavior. The usage guidance about native asset prioritization and pool filtering is helpful.
premiumjibles
force-pushed
the
flexible-assets-
branch
from
66f556c to
870109d
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apps/agentic-server/src/tools/getAssets.ts (2)
119-136: Tool description now matches schema and runtime behaviorThe updated
getAssetsTool.description:
- Documents three methods with
network?clearly optional for CONTRACT,- Explains
assetTypeandpoolsfilters and their defaults,- Notes that native assets are prioritized for symbol matches, matching the scoring logic in
AssetService.search,- Clarifies that the tool returns data suitable for a single UI card.
This resolves the earlier mismatch between the schema and description around contract search and optional network.
65-81: Empty-string network fallback in hydrateAsset remains problematic
hydrateAssetcurrently computes:const inferredNetwork = network ?? chainIdToNetwork[staticAsset.chainId] ?? ''and then sets
asset.network = inferredNetwork. When neithernetworknorchainIdToNetwork[staticAsset.chainId]is defined, this yieldsnetwork: '', which is almost certainly not a valid network identifier and can break downstream consumers expecting a real network or a clearly absent value.Given this was already flagged earlier, it would be good to resolve it here. For example:
async function hydrateAsset(staticAsset: StaticAsset, network?: Network): Promise<GetAssetsOutput> { const inferredNetwork = network ?? chainIdToNetwork[staticAsset.chainId] if (!inferredNetwork) { // Option A: fail fast throw new Error(`[hydrateAsset] Could not infer network for assetId: ${staticAsset.assetId}`) // Option B: make Asset.network optional and allow undefined, if you choose that broader change } const asset: Asset = { ...staticAsset, color: staticAsset.color ?? '', price: '0', network: inferredNetwork, } // rest unchanged... }You could also keep
Asset.networkasstringbut at least avoid''and prefer either throwing or some explicit fallback strategy.
🧹 Nitpick comments (4)
packages/utils/src/AssetService.ts (3)
14-15: Contract index implementation for non-slip44 assets looks soundBuilding
assetsByContractonly for non-/slip44:assetIds and extracting the contract asassetId.substring(colonIdx + 1).toLowerCase()matches CAIP-19 token ids (e.g.,eip155:1/erc20:0x...). This should support efficient case-insensitive contract lookups.If you ever ingest non-standard assetIds, a small defensive guard around
colonIdx === -1would make this more robust, but it's not strictly necessary given the current data pipeline.Also applies to: 24-27, 28-50
115-129: Search aggregation and scoring pipeline looks correctCombining symbol and name results into a
Map<AssetId, StaticAsset>to dedupe, then applyingscoreMatchand sorting by score before projecting back to assets preserves uniqueness and ordering by relevance as intended. This is consistent with the scoring logic and avoids duplicate assets from multiple search paths.
141-170: Pool/type filtering is aligned with doc semantics; consider not overriding explicit isPool=false
searchWithFilterscorrectly:
- Delegates to
search(term, network)for relevance ranking,- Splits
assetTypeinto native (/slip44:), token (non-/slip44:), or all,- Applies pool filtering based on
'exclude' | 'include' | 'only', with'include'acting as a no-op.
isPool(asset)currently returnsasset.isPool || asset.symbol.includes('/'). This can override an explicitisPool === falseif a symbol contains/. If you expect any such assets, a safer pattern would be:private isPool(asset: StaticAsset): boolean { return asset.isPool ?? asset.symbol.includes('/') }so that explicit
falsewins over the heuristic.apps/agentic-server/src/tools/getAssets.ts (1)
24-30: Schema additions and descriptions match the new behaviorsThe updated
getAssetsSchema:
- Adds
contractAddress,assetType, andpools,- Clarifies
searchTermas “Search by name or symbol,”- Marks
networkas optional “Chain to search on,”which is consistent with how
getAssetWithMarketDataandassetServicenow behave. If you ever want stricter validation (e.g., require at least one ofsearchTerm/assetId/contractAddress), that can be layered via a refinement, but not required for correctness.
📜 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 (2)
apps/agentic-server/src/tools/getAssets.ts(3 hunks)packages/utils/src/AssetService.ts(7 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
packages/utils/src/AssetService.ts (2)
packages/types/src/asset.ts (2)
StaticAsset(30-30)asset(3-26)packages/types/src/network.ts (2)
Network(44-44)networkToChainIdMap(63-82)
apps/agentic-server/src/tools/getAssets.ts (4)
packages/types/src/network.ts (2)
NETWORKS(23-42)chainIdToNetwork(84-86)packages/types/src/asset.ts (3)
StaticAsset(30-30)asset(3-26)Asset(28-28)packages/caip/src/adapters/coingecko/index.ts (1)
assetIdToCoingecko(59-59)packages/utils/src/AssetService.ts (1)
assetService(177-177)
⏰ 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 (7)
packages/utils/src/AssetService.ts (4)
2-3: Typed imports for Network and chainId mapping look consistentThe added
Network/StaticAssettype import andnetworkToChainIdMapimport align with the new Network-aware search APIs; no issues here.
65-75: Network-aware symbol search is straightforward and type-safeLowercasing the input symbol and then filtering matches by
asset.chainId === networkToChainIdMap[network]is a clean way to scope results to a network while keeping the API Network-typed.
97-113: Name search behavior (exact + partial, then optional network filter) is reasonableThe split between exact name matches and partial matches via
assetsByName.entries()and the subsequent chainId filter whennetworkis provided gives predictable behavior and keeps network scoping consistent withsearchBySymbol. No correctness issues spotted.
131-139: Contract search correctly respects optional network filteringLowercasing the contract address, returning all matches by default, and then narrowing by
chainIdwhennetworkis supplied matches the intended semantics (with multi-chain contracts handled by the filter). The behavior is clear, andresults[0]selection is handled at the caller.apps/agentic-server/src/tools/getAssets.ts (3)
2-2: StaticAsset import is appropriately used for hydration typingImporting
StaticAssetalongsideAssetkeepshydrateAsset’s signature precise and documents that hydration starts from static data; no issues here.
83-105: Selection and hydration order for assetId/contract/searchTerm is coherent
getAssetWithMarketDatanow:
- Resolves by
assetIdviaassetService.getAssetand hydrates,- Falls back to
contractAddressusingassetService.searchByContract(contractAddress, network)[0],- Then to
searchTermviaassetService.searchWithFilters(searchTerm, { network, assetType, pools })[0],- Returns
{ assets: [] }when nothing matches.This preserves a clear precedence order and keeps the tool’s output shape stable (0 or 1 asset), which matches the card-focused UI description.
107-116: executeGetAssets gating logic correctly includes contractAddressAdding
contractAddressto the early-return condition ensures all three entry paths (assetId,contractAddress,searchTerm) are handled consistently before falling back to an empty result. No issues here.
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.