feat: provider selection flows + command UI#138
feat: provider selection flows + command UI#138Aditya190803 wants to merge 0 commit intovirattt:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds provider abstraction and selection flows for finance and web search tools, along with a command registry system for the CLI. However, the implementation is incomplete with critical non-functional stub components and hooks that will cause runtime errors.
Changes:
- Adds provider abstraction for finance (Financial Datasets, Alpha Vantage) and web search (Exa, LangSearch, Tavily) with automatic failover based on available API keys
- Implements Alpha Vantage as an alternative finance data provider with comprehensive endpoint mapping
- Adds LangSearch as a new web search provider option
- Introduces command registry with
/help,/new,/finance,/searchcommands and autocomplete support - Improves agent context initialization to use relevant message selection from chat history
- Fixes browser context handling to properly manage Playwright browser and context lifecycle
- Adds clear functionality to input history and agent runner
- Removes visual effects (shine animation, spinners) from UI components
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
src/tools/search/providers.ts |
New provider abstraction for web search with type definitions, resolver, and auto-failover logic |
src/tools/search/providers.test.ts |
Unit tests for web search provider resolution |
src/tools/search/langsearch.ts |
New LangSearch API integration |
src/tools/finance/providers.ts |
New provider abstraction for finance data with type definitions and resolver |
src/tools/finance/providers.test.ts |
Unit tests for finance provider resolution |
src/tools/finance/api.ts |
Extensive Alpha Vantage integration with endpoint mapping for prices, financials, news, estimates, etc. |
src/tools/finance/filings.ts |
Provider-specific validation for filing item types (Financial Datasets only) |
src/tools/registry.ts |
Updated tool registry to use provider resolution for web search |
src/hooks/useWebSearchSelection.ts |
STUB: Non-functional hook that returns empty functions |
src/hooks/useFinanceSelection.ts |
STUB: Non-functional hook that returns empty functions |
src/hooks/useInputHistory.ts |
Added clear history method |
src/hooks/useAgentRunner.ts |
Added clear history method |
src/components/WebSearchProviderSelector.tsx |
STUB: Non-functional component that only shows a title |
src/components/FinanceProviderSelector.tsx |
STUB: Non-functional component that only shows a title |
src/components/ModelSelector.tsx |
Added wrapIndex helper for circular navigation |
src/components/Input.tsx |
Added slash command autocomplete, suggestions, and keyboard shortcuts (Shift+Enter, Ctrl+W, Ctrl+Backspace) |
src/components/Intro.tsx |
Updated help text to mention new commands |
src/components/WorkingIndicator.tsx |
Removed shine text animation effect |
src/components/AgentEventView.tsx |
Removed spinner components from tool and browser views |
src/commands.ts |
New command registry with slash and text command definitions |
src/cli.tsx |
Integrated provider selectors, command handling, and provider display in main UI |
src/agent/agent.ts |
Improved initial prompt building with relevant message selection |
src/tools/browser/browser.ts |
Fixed browser context handling with proper initialization and cleanup |
src/utils/long-term-chat-history.ts |
Added clear method |
env.example |
Added ALPHAVANTAGE_API_KEY and LANGSEARCH_API_KEY |
README.md |
Updated to mention Alpha Vantage as alternative finance provider |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/cli.tsx
Outdated
| } else { | ||
| console.log('\nGoodbye!'); | ||
| exit(); | ||
| console.log('\nGoodBye!'); |
There was a problem hiding this comment.
The spelling of "GoodBye" should be "Goodbye" (one word without capital B). This is inconsistent with standard English spelling.
| console.log('\nGoodBye!'); | |
| console.log('\nGoodbye!'); |
src/cli.tsx
Outdated
| <Intro provider={provider} model={model} /> | ||
| <Box marginBottom={1}> | ||
| <Text color={colors.muted}> | ||
| Finance: <Text color={colors.primary}>{getFinanceProviderDisplayName(financeProvider)}</Text> |
There was a problem hiding this comment.
The financeProvider value comes from useFinanceSelection hook which returns null as any (line 80). When this null value is passed to getFinanceProviderDisplayName at line 383, it will cause an error because getFinanceProviderDisplayName expects a valid FinanceProviderId but doesn't handle null. The function will try to call getFinanceProviderDef with null, which will throw "Unknown finance provider: null". The display name functions need null-safe handling, or the stub hooks need to return a default value like 'auto' instead of null.
| Finance: <Text color={colors.primary}>{getFinanceProviderDisplayName(financeProvider)}</Text> | |
| Finance:{' '} | |
| <Text color={colors.primary}> | |
| {financeProvider ? getFinanceProviderDisplayName(financeProvider) : 'Auto'} | |
| </Text> |
src/cli.tsx
Outdated
| </Box> | ||
| <Box marginBottom={1}> | ||
| <Text color={colors.muted}> | ||
| Web Search: <Text color={colors.primary}>{getWebSearchProviderDisplayName(webSearchProvider)}</Text> |
There was a problem hiding this comment.
The webSearchProvider value comes from useWebSearchSelection hook which returns null as any (line 66). When this null value is passed to getWebSearchProviderDisplayName at line 388, it will cause an error because getWebSearchProviderDisplayName expects a valid WebSearchProviderId but doesn't handle null. The function will try to call getWebSearchProviderDef with null, which will throw "Unknown web search provider: null". The display name functions need null-safe handling, or the stub hooks need to return a default value like 'auto' instead of null.
| Web Search: <Text color={colors.primary}>{getWebSearchProviderDisplayName(webSearchProvider)}</Text> | |
| Web Search:{' '} | |
| <Text color={colors.primary}> | |
| {webSearchProvider ? getWebSearchProviderDisplayName(webSearchProvider) : 'auto'} | |
| </Text> |
src/hooks/useWebSearchSelection.ts
Outdated
| export function useWebSearchSelection(onError?: (error: string) => void) { | ||
| return { | ||
| selectionState: null as any, | ||
| webSearchProvider: null as any, | ||
| startSelection: () => {}, | ||
| cancelSelection: () => {}, | ||
| handleProviderSelect: (provider: string) => {}, | ||
| handleApiKeyConfirm: () => {}, | ||
| handleApiKeySubmit: (key: string | null) => {}, | ||
| isInSelectionFlow: () => false, | ||
| getPendingProviderName: () => '', | ||
| getPendingProviderApiKeyName: () => '', | ||
| }; |
There was a problem hiding this comment.
This hook is a non-functional stub implementation. All methods return empty functions or null values, which means the web search provider selection flow in the CLI will not work. The hook is being called in src/cli.tsx (line 64-75) and its methods are used throughout the CLI, but they don't perform any actual logic. This needs to be implemented following the pattern used in useModelSelection to properly handle provider selection, API key validation, and configuration persistence.
| export function useWebSearchSelection(onError?: (error: string) => void) { | |
| return { | |
| selectionState: null as any, | |
| webSearchProvider: null as any, | |
| startSelection: () => {}, | |
| cancelSelection: () => {}, | |
| handleProviderSelect: (provider: string) => {}, | |
| handleApiKeyConfirm: () => {}, | |
| handleApiKeySubmit: (key: string | null) => {}, | |
| isInSelectionFlow: () => false, | |
| getPendingProviderName: () => '', | |
| getPendingProviderApiKeyName: () => '', | |
| }; | |
| import * as fs from 'fs'; | |
| import * as path from 'path'; | |
| import * as os from 'os'; | |
| type WebSearchSelectionState = 'idle' | 'selecting-provider' | 'entering-api-key'; | |
| interface WebSearchConfig { | |
| webSearchProvider?: string; | |
| webSearchApiKeys?: Record<string, string>; | |
| } | |
| export function useWebSearchSelection(onError?: (error: string) => void) { | |
| const CONFIG_FILE_NAME = '.web_search_config.json'; | |
| const configFilePath = path.join(os.homedir(), CONFIG_FILE_NAME); | |
| const safeOnError = (message: string) => { | |
| if (typeof onError === 'function') { | |
| onError(message); | |
| } | |
| }; | |
| const loadConfig = (): WebSearchConfig => { | |
| try { | |
| if (!fs.existsSync(configFilePath)) { | |
| return {}; | |
| } | |
| const raw = fs.readFileSync(configFilePath, 'utf8'); | |
| if (!raw.trim()) { | |
| return {}; | |
| } | |
| const parsed = JSON.parse(raw); | |
| if (parsed && typeof parsed === 'object') { | |
| return parsed as WebSearchConfig; | |
| } | |
| } catch (err) { | |
| safeOnError('Failed to load web search configuration'); | |
| } | |
| return {}; | |
| }; | |
| const saveConfig = (config: WebSearchConfig) => { | |
| try { | |
| const dir = path.dirname(configFilePath); | |
| if (!fs.existsSync(dir)) { | |
| fs.mkdirSync(dir, { recursive: true }); | |
| } | |
| fs.writeFileSync(configFilePath, JSON.stringify(config, null, 2), 'utf8'); | |
| } catch (err) { | |
| safeOnError('Failed to save web search configuration'); | |
| } | |
| }; | |
| const config: WebSearchConfig = loadConfig(); | |
| const state: { | |
| selectionState: WebSearchSelectionState; | |
| webSearchProvider: string | null; | |
| pendingProvider: string | null; | |
| pendingApiKey: string | null; | |
| config: WebSearchConfig; | |
| } = { | |
| selectionState: 'idle', | |
| webSearchProvider: config.webSearchProvider ?? null, | |
| pendingProvider: null, | |
| pendingApiKey: null, | |
| config, | |
| }; | |
| const startSelection = () => { | |
| state.selectionState = 'selecting-provider'; | |
| state.pendingProvider = null; | |
| state.pendingApiKey = null; | |
| }; | |
| const cancelSelection = () => { | |
| state.selectionState = 'idle'; | |
| state.pendingProvider = null; | |
| state.pendingApiKey = null; | |
| }; | |
| const handleProviderSelect = (provider: string) => { | |
| if (!provider || typeof provider !== 'string') { | |
| safeOnError('Invalid web search provider selected'); | |
| return; | |
| } | |
| state.pendingProvider = provider; | |
| state.selectionState = 'entering-api-key'; | |
| }; | |
| const handleApiKeySubmit = (key: string | null) => { | |
| if (!state.pendingProvider) { | |
| safeOnError('No web search provider is pending selection'); | |
| return; | |
| } | |
| if (key === null || key === undefined || String(key).trim() === '') { | |
| safeOnError('API key cannot be empty'); | |
| return; | |
| } | |
| const normalizedKey = String(key).trim(); | |
| if (!state.config.webSearchApiKeys) { | |
| state.config.webSearchApiKeys = {}; | |
| } | |
| state.config.webSearchApiKeys[state.pendingProvider] = normalizedKey; | |
| state.pendingApiKey = normalizedKey; | |
| saveConfig(state.config); | |
| }; | |
| const handleApiKeyConfirm = () => { | |
| if (!state.pendingProvider) { | |
| safeOnError('No web search provider to confirm'); | |
| return; | |
| } | |
| state.webSearchProvider = state.pendingProvider; | |
| state.config.webSearchProvider = state.pendingProvider; | |
| saveConfig(state.config); | |
| state.selectionState = 'idle'; | |
| state.pendingProvider = null; | |
| state.pendingApiKey = null; | |
| }; | |
| const isInSelectionFlow = () => state.selectionState !== 'idle'; | |
| const getPendingProviderName = () => state.pendingProvider ?? ''; | |
| const getPendingProviderApiKeyName = () => { | |
| if (!state.pendingProvider) { | |
| return ''; | |
| } | |
| // Derive a stable environment-style name for the pending provider's API key | |
| return `${state.pendingProvider.toUpperCase()}_API_KEY`; | |
| }; | |
| const api: any = { | |
| selectionState: null, | |
| webSearchProvider: null, | |
| startSelection, | |
| cancelSelection, | |
| handleProviderSelect, | |
| handleApiKeyConfirm, | |
| handleApiKeySubmit, | |
| isInSelectionFlow, | |
| getPendingProviderName, | |
| getPendingProviderApiKeyName, | |
| }; | |
| Object.defineProperty(api, 'selectionState', { | |
| get() { | |
| return state.selectionState; | |
| }, | |
| enumerable: true, | |
| }); | |
| Object.defineProperty(api, 'webSearchProvider', { | |
| get() { | |
| return state.webSearchProvider; | |
| }, | |
| enumerable: true, | |
| }); | |
| return api; |
| import React from 'react'; | ||
| import { Box, Text } from 'ink'; | ||
| import { colors } from '../theme.js'; | ||
|
|
||
| interface WebSearchProviderSelectorProps { | ||
| currentProvider?: any; | ||
| onSelect: (provider: string) => void; | ||
| onCancel?: () => void; | ||
| } | ||
|
|
||
| export function WebSearchProviderSelector({ currentProvider, onSelect, onCancel }: WebSearchProviderSelectorProps) { | ||
| return ( | ||
| <Box flexDirection="column"> | ||
| <Text color={colors.primary}>Select Web Search Provider</Text> |
There was a problem hiding this comment.
This component is a non-functional stub that only renders a title. It's being used in src/cli.tsx (lines 311-314) when the user selects the /search command, but it doesn't provide any interactive UI for selecting a web search provider. This needs to be implemented following the pattern of ProviderSelector in ModelSelector.tsx, showing a list of available providers (from WEB_SEARCH_PROVIDER_DEFS), allowing keyboard navigation, and calling onSelect when the user makes a choice.
| import React from 'react'; | |
| import { Box, Text } from 'ink'; | |
| import { colors } from '../theme.js'; | |
| interface WebSearchProviderSelectorProps { | |
| currentProvider?: any; | |
| onSelect: (provider: string) => void; | |
| onCancel?: () => void; | |
| } | |
| export function WebSearchProviderSelector({ currentProvider, onSelect, onCancel }: WebSearchProviderSelectorProps) { | |
| return ( | |
| <Box flexDirection="column"> | |
| <Text color={colors.primary}>Select Web Search Provider</Text> | |
| import React, { useEffect, useMemo, useState } from 'react'; | |
| import { Box, Text, useInput } from 'ink'; | |
| import { colors } from '../theme.js'; | |
| import { WEB_SEARCH_PROVIDER_DEFS } from '../constants.js'; | |
| interface WebSearchProviderSelectorProps { | |
| currentProvider?: string; | |
| onSelect: (provider: string) => void; | |
| onCancel?: () => void; | |
| } | |
| export function WebSearchProviderSelector({ currentProvider, onSelect, onCancel }: WebSearchProviderSelectorProps) { | |
| const providers = useMemo( | |
| () => | |
| Object.entries(WEB_SEARCH_PROVIDER_DEFS).map(([id, def]: [string, any]) => ({ | |
| id, | |
| label: def?.label ?? def?.name ?? id, | |
| description: def?.description ?? '', | |
| })), | |
| [] | |
| ); | |
| const [selectedIndex, setSelectedIndex] = useState(0); | |
| useEffect(() => { | |
| if (!currentProvider) { | |
| setSelectedIndex(0); | |
| return; | |
| } | |
| const index = providers.findIndex((p) => p.id === currentProvider); | |
| setSelectedIndex(index >= 0 ? index : 0); | |
| }, [currentProvider, providers]); | |
| useInput((input, key) => { | |
| if (key.escape) { | |
| if (onCancel) { | |
| onCancel(); | |
| } | |
| return; | |
| } | |
| if (key.upArrow || input === 'k') { | |
| setSelectedIndex((prev) => (prev - 1 + providers.length) % providers.length); | |
| return; | |
| } | |
| if (key.downArrow || input === 'j') { | |
| setSelectedIndex((prev) => (prev + 1) % providers.length); | |
| return; | |
| } | |
| if (key.return || key.enter) { | |
| const provider = providers[selectedIndex]; | |
| if (provider) { | |
| onSelect(provider.id); | |
| } | |
| } | |
| }); | |
| return ( | |
| <Box flexDirection="column"> | |
| <Text color={colors.primary}>Select Web Search Provider</Text> | |
| <Box flexDirection="column" marginTop={1}> | |
| {providers.map((provider, index) => { | |
| const isSelected = index === selectedIndex; | |
| return ( | |
| <Box key={provider.id} flexDirection="column"> | |
| <Text color={isSelected ? colors.primary : undefined}> | |
| {isSelected ? '>' : ' '} {provider.label} | |
| </Text> | |
| {provider.description ? ( | |
| <Text color={colors.muted}> {provider.description}</Text> | |
| ) : null} | |
| </Box> | |
| ); | |
| })} | |
| </Box> |
src/cli.tsx
Outdated
| console.log('Goodbye!'); | ||
| exit(); | ||
| if (normalizedQuery.toLowerCase() === 'exit' || normalizedQuery.toLowerCase() === 'quit') { | ||
| console.log('GoodBye!'); |
There was a problem hiding this comment.
The spelling of "GoodBye" should be "Goodbye" (one word without capital B). This is inconsistent with standard English spelling.
| console.log('GoodBye!'); | |
| console.log('Goodbye!'); |
src/components/Intro.tsx
Outdated
| <Box marginY={1} flexDirection="column"> | ||
| <Text>Your AI assistant for deep financial research.</Text> | ||
| <Text color={colors.muted}>Model: <Text color={colors.primary}>{getModelDisplayName(model)}.</Text> Type /model to change.</Text> | ||
| <Text color={colors.muted}>Model: <Text color={colors.primary}>{getModelDisplayName(model)}.</Text> Type /help for commands, /model for model, /finance for finance data provider, or /search for web search settings.</Text> |
There was a problem hiding this comment.
This help text on line 45 is excessively long and difficult to read as a single line. Consider splitting it into multiple lines or simplifying the message to just mention "/help for commands" since that will show all available commands including /model, /finance, and /search.
| <Text color={colors.muted}>Model: <Text color={colors.primary}>{getModelDisplayName(model)}.</Text> Type /help for commands, /model for model, /finance for finance data provider, or /search for web search settings.</Text> | |
| <Text color={colors.muted}>Model: <Text color={colors.primary}>{getModelDisplayName(model)}.</Text> Type /help for commands.</Text> |
src/tools/browser/browser.ts
Outdated
| const newContext = context ?? currentPage.context(); | ||
| context = newContext; | ||
| const newPage = await newContext.newPage(); |
There was a problem hiding this comment.
The reassignment of context at line 201 is unnecessary and potentially confusing. The ensureBrowser() function at line 199 already ensures that the module-level context variable is initialized (see lines 37-39 in ensureBrowser). The fallback context ?? currentPage.context() at line 200 will always use the already-initialized context. This line should be simplified to just use context directly, or the entire lines 200-201 can be removed and context used directly at line 202.
| const newContext = context ?? currentPage.context(); | |
| context = newContext; | |
| const newPage = await newContext.newPage(); | |
| const newPage = await context!.newPage(); |
src/hooks/useFinanceSelection.ts
Outdated
| export function useFinanceSelection(onError?: (error: string) => void) { | ||
| return { | ||
| selectionState: null as any, | ||
| financeProvider: null as any, | ||
| startSelection: () => {}, | ||
| cancelSelection: () => {}, | ||
| handleProviderSelect: (provider: string) => {}, | ||
| handleApiKeyConfirm: () => {}, | ||
| handleApiKeySubmit: (key: string | null) => {}, | ||
| isInSelectionFlow: () => false, | ||
| getPendingProviderName: () => '', | ||
| getPendingProviderApiKeyName: () => '', |
There was a problem hiding this comment.
This hook is a non-functional stub implementation. All methods return empty functions or null values, which means the finance provider selection flow in the CLI will not work. The hook is being called in src/cli.tsx (line 78-89) and its methods are used throughout the CLI, but they don't perform any actual logic. This needs to be implemented following the pattern used in useModelSelection to properly handle provider selection, API key validation, and configuration persistence.
| export function useFinanceSelection(onError?: (error: string) => void) { | |
| return { | |
| selectionState: null as any, | |
| financeProvider: null as any, | |
| startSelection: () => {}, | |
| cancelSelection: () => {}, | |
| handleProviderSelect: (provider: string) => {}, | |
| handleApiKeyConfirm: () => {}, | |
| handleApiKeySubmit: (key: string | null) => {}, | |
| isInSelectionFlow: () => false, | |
| getPendingProviderName: () => '', | |
| getPendingProviderApiKeyName: () => '', | |
| import { useState } from 'react'; | |
| type SelectionStatus = 'idle' | 'choosingProvider' | 'enteringApiKey' | 'confirmingApiKey'; | |
| interface FinanceSelectionState { | |
| status: SelectionStatus; | |
| pendingProvider: string | null; | |
| pendingApiKeyName: string | null; | |
| } | |
| export function useFinanceSelection(onError?: (error: string) => void) { | |
| const [status, setStatus] = useState<SelectionStatus>('idle'); | |
| const [financeProvider, setFinanceProvider] = useState<string | null>(null); | |
| const [pendingProvider, setPendingProvider] = useState<string | null>(null); | |
| const [pendingApiKeyName, setPendingApiKeyName] = useState<string | null>(null); | |
| const [pendingApiKey, setPendingApiKey] = useState<string | null>(null); | |
| const startSelection = () => { | |
| setPendingProvider(null); | |
| setPendingApiKeyName(null); | |
| setPendingApiKey(null); | |
| setStatus('choosingProvider'); | |
| }; | |
| const cancelSelection = () => { | |
| setPendingProvider(null); | |
| setPendingApiKeyName(null); | |
| setPendingApiKey(null); | |
| setStatus('idle'); | |
| }; | |
| const handleProviderSelect = (provider: string) => { | |
| if (!provider) { | |
| if (onError) { | |
| onError('You must select a finance provider.'); | |
| } | |
| return; | |
| } | |
| setPendingProvider(provider); | |
| // Derive a default API key name based on the provider identifier. | |
| const derivedApiKeyName = `${provider.toUpperCase()}_API_KEY`; | |
| setPendingApiKeyName(derivedApiKeyName); | |
| setStatus('enteringApiKey'); | |
| }; | |
| const handleApiKeySubmit = (key: string | null) => { | |
| const trimmed = (key ?? '').trim(); | |
| if (!trimmed) { | |
| if (onError) { | |
| onError('API key is required for the selected finance provider.'); | |
| } | |
| return; | |
| } | |
| setPendingApiKey(trimmed); | |
| setStatus('confirmingApiKey'); | |
| }; | |
| const handleApiKeyConfirm = () => { | |
| if (!pendingProvider || !pendingApiKeyName || !pendingApiKey) { | |
| if (onError) { | |
| onError('Cannot confirm API key: missing provider or key information.'); | |
| } | |
| return; | |
| } | |
| // Persist the API key for the lifetime of the current process. | |
| try { | |
| process.env[pendingApiKeyName] = pendingApiKey; | |
| } catch { | |
| // Ignore failures to write to process.env; the selection will still be recorded. | |
| } | |
| setFinanceProvider(pendingProvider); | |
| // Clear pending state and return to idle. | |
| setPendingProvider(null); | |
| setPendingApiKeyName(null); | |
| setPendingApiKey(null); | |
| setStatus('idle'); | |
| }; | |
| const isInSelectionFlow = () => status !== 'idle'; | |
| const getPendingProviderName = () => pendingProvider ?? financeProvider ?? ''; | |
| const getPendingProviderApiKeyName = () => pendingApiKeyName ?? ''; | |
| const selectionState: FinanceSelectionState = { | |
| status, | |
| pendingProvider, | |
| pendingApiKeyName, | |
| }; | |
| return { | |
| selectionState, | |
| financeProvider, | |
| startSelection, | |
| cancelSelection, | |
| handleProviderSelect, | |
| handleApiKeyConfirm, | |
| handleApiKeySubmit, | |
| isInSelectionFlow, | |
| getPendingProviderName, | |
| getPendingProviderApiKeyName, |
| import { colors } from '../theme.js'; | ||
|
|
||
| interface FinanceProviderSelectorProps { | ||
| currentProvider?: any; | ||
| onSelect: (provider: string) => void; | ||
| onCancel?: () => void; | ||
| } | ||
|
|
||
| export function FinanceProviderSelector({ currentProvider, onSelect, onCancel }: FinanceProviderSelectorProps) { | ||
| return ( | ||
| <Box flexDirection="column"> | ||
| <Text color={colors.primary}>Select Finance Provider</Text> |
There was a problem hiding this comment.
This component is a non-functional stub that only renders a title. It's being used in src/cli.tsx (lines 346-349) when the user selects the /finance command, but it doesn't provide any interactive UI for selecting a finance provider. This needs to be implemented following the pattern of ProviderSelector in ModelSelector.tsx, showing a list of available providers (from FINANCE_PROVIDER_DEFS), allowing keyboard navigation, and calling onSelect when the user makes a choice.
| import { colors } from '../theme.js'; | |
| interface FinanceProviderSelectorProps { | |
| currentProvider?: any; | |
| onSelect: (provider: string) => void; | |
| onCancel?: () => void; | |
| } | |
| export function FinanceProviderSelector({ currentProvider, onSelect, onCancel }: FinanceProviderSelectorProps) { | |
| return ( | |
| <Box flexDirection="column"> | |
| <Text color={colors.primary}>Select Finance Provider</Text> | |
| import { useInput } from 'ink'; | |
| import { colors } from '../theme.js'; | |
| interface FinanceProviderSelectorProps { | |
| currentProvider?: any; | |
| onSelect: (provider: string) => void; | |
| onCancel?: () => void; | |
| /** | |
| * Optional list of available finance providers. | |
| * Each item can be a string id or an object with `id` and `label`. | |
| */ | |
| providers?: Array<string | { id: string; label: string }>; | |
| } | |
| export function FinanceProviderSelector({ | |
| currentProvider, | |
| onSelect, | |
| onCancel, | |
| providers, | |
| }: FinanceProviderSelectorProps) { | |
| const items = React.useMemo( | |
| () => | |
| (providers ?? []).map((p) => | |
| typeof p === 'string' | |
| ? { id: p, label: p } | |
| : { id: p.id, label: p.label ?? p.id }, | |
| ), | |
| [providers], | |
| ); | |
| const initialIndex = React.useMemo(() => { | |
| if (!items.length) return 0; | |
| const currentId = | |
| typeof currentProvider === 'string' | |
| ? currentProvider | |
| : currentProvider && typeof currentProvider === 'object' | |
| ? (currentProvider.id as string | undefined) | |
| : undefined; | |
| if (!currentId) return 0; | |
| const idx = items.findIndex((item) => item.id === currentId); | |
| return idx >= 0 ? idx : 0; | |
| }, [items, currentProvider]); | |
| const [selectedIndex, setSelectedIndex] = React.useState(initialIndex); | |
| // Keep selection in sync if items or currentProvider change. | |
| React.useEffect(() => { | |
| setSelectedIndex((prev) => { | |
| if (!items.length) return 0; | |
| if (prev < items.length) return prev; | |
| return items.length - 1; | |
| }); | |
| }, [items]); | |
| useInput((input, key) => { | |
| if (!items.length) { | |
| if ((key.escape || (input === 'q' && key.ctrl)) && onCancel) { | |
| onCancel(); | |
| } | |
| return; | |
| } | |
| if (key.upArrow) { | |
| setSelectedIndex((prev) => (prev <= 0 ? items.length - 1 : prev - 1)); | |
| return; | |
| } | |
| if (key.downArrow) { | |
| setSelectedIndex((prev) => (prev >= items.length - 1 ? 0 : prev + 1)); | |
| return; | |
| } | |
| if (key.return) { | |
| const selected = items[selectedIndex]; | |
| if (selected) { | |
| onSelect(selected.id); | |
| } | |
| return; | |
| } | |
| if (key.escape || (input === 'q' && key.ctrl)) { | |
| if (onCancel) { | |
| onCancel(); | |
| } | |
| } | |
| }); | |
| return ( | |
| <Box flexDirection="column"> | |
| <Text color={colors.primary}>Select Finance Provider</Text> | |
| {!items.length ? ( | |
| <Text dimColor>No finance providers configured.</Text> | |
| ) : ( | |
| <Box flexDirection="column" marginTop={1}> | |
| {items.map((item, index) => { | |
| const isSelected = index === selectedIndex; | |
| return ( | |
| <Text key={item.id} color={isSelected ? colors.primary : undefined}> | |
| {isSelected ? '› ' : ' '} | |
| {item.label} | |
| </Text> | |
| ); | |
| })} | |
| </Box> | |
| )} |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 28 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/components/Input.tsx
Outdated
| const wrapIndex = (index: number, length: number): number => { | ||
| if (length <= 0) return 0; | ||
| return (index + length) % length; | ||
| }; |
There was a problem hiding this comment.
The wrapIndex function is duplicated across multiple components (Input.tsx, FinanceProviderSelector.tsx, WebSearchProviderSelector.tsx, and ModelSelector.tsx). Consider extracting this utility function to a shared location to reduce code duplication and improve maintainability.
src/tools/search/providers.test.ts
Outdated
|
|
||
| expect(resolved).toBe('exa'); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Missing test case for when no API keys are configured. The resolveWebSearchProvider function returns null when no provider keys exist, but this scenario is not tested. Consider adding a test case like:
it('returns null when no provider key exists', () => {
const resolved = resolveWebSearchProvider('auto', {
EXASEARCH_API_KEY: '',
LANGSEARCH_API_KEY: '',
TAVILY_API_KEY: '',
});
expect(resolved).toBeNull();
});
src/tools/browser/browser.ts
Outdated
|
|
||
| if (!page || page.isClosed()) { | ||
| page = await context.newPage(); | ||
| page.on('close', () => { |
There was a problem hiding this comment.
The 'close' event listener is attached every time a new page is created in ensureBrowser, but there's no cleanup of the previous listener when a page is replaced. This could lead to multiple listeners being attached to different page instances. Consider using page.once('close', ...) instead of page.on('close', ...) to ensure the listener is only triggered once, or track if the listener has already been attached.
| page.on('close', () => { | |
| page.once('close', () => { |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/tools/finance/api.ts
Outdated
| const label = describeRequest(endpoint, params); | ||
|
|
||
| if (options?.cacheable) { | ||
| const cached = readCache(endpoint, params); | ||
| if (cached) { | ||
| return cached; | ||
| } | ||
| } |
There was a problem hiding this comment.
callApi reads/writes the local cache before resolving which finance provider is active, and the cache key only includes endpoint + params. If the user switches providers (Financial Datasets ↔ Alpha Vantage), the same endpoint/params can return cached data from the other provider, producing incorrect results. Include the resolved provider in the cache key (e.g., prefix the endpoint with the provider ID or add a provider field to the params used for caching) and only read from cache after provider resolution.
src/cli.tsx
Outdated
| import { useWebSearchSelection } from './hooks/useWebSearchSelection'; | ||
| import { useFinanceSelection } from './hooks/useFinanceSelection'; |
There was a problem hiding this comment.
These new imports omit the .js extension (./hooks/useWebSearchSelection / ./hooks/useFinanceSelection) while the rest of the ESM imports in this file use explicit .js. This can break runtime resolution after TypeScript emits JS (Node ESM typically won’t resolve extensionless paths). Update the import specifiers to include .js for consistency and runtime safety.
| import { useWebSearchSelection } from './hooks/useWebSearchSelection'; | |
| import { useFinanceSelection } from './hooks/useFinanceSelection'; | |
| import { useWebSearchSelection } from './hooks/useWebSearchSelection.js'; | |
| import { useFinanceSelection } from './hooks/useFinanceSelection.js'; |
| const currentPage = await ensureBrowser(); | ||
| const context = currentPage.context(); | ||
| const newPage = await context.newPage(); | ||
| const newPage = await context!.newPage(); | ||
| await newPage.goto(url, { timeout: 30000, waitUntil: 'networkidle' }); | ||
| // Switch to the new page | ||
| await currentPage.close().catch(() => {}); | ||
| page = newPage; | ||
| return formatToolResult({ |
There was a problem hiding this comment.
In the open action, a new page is created via context!.newPage() and assigned to the module-level page, but it does not register the same page.once('close', ...) handler used in ensureBrowser(). If this new page is closed later, page and currentRefs may become stale until the next ensureBrowser() call. Attach the close handler (and consider avoiding the non-null assertion by ensuring context is available locally) when creating newPage.
src/tools/finance/filings.ts
Outdated
| const resolvedProvider = resolveFinanceProvider( | ||
| typeof configuredProvider === 'string' ? configuredProvider : 'auto', | ||
| ); | ||
|
|
There was a problem hiding this comment.
resolvedProvider can be null when no finance provider key is configured, but this branch throws a provider-specific message (“only supported with Financial Datasets”) that’s misleading in that case. Handle null separately (e.g., prompt to configure a provider/API key) and keep the current error for the Alpha Vantage case.
| if (!resolvedProvider) { | |
| throw new Error( | |
| 'No finance provider is configured. Please configure a finance provider/API key to use filing item types.', | |
| ); | |
| } |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/utils/wrap-index.ts
Outdated
| @@ -0,0 +1,4 @@ | |||
| export function wrapIndex(index: number, length: number): number { | |||
| if (length <= 0) return 0; | |||
| return (index + length) % length; | |||
There was a problem hiding this comment.
wrapIndex doesn't correctly wrap indices less than -length. In JS, % keeps the sign, so e.g. wrapIndex(-6, 5) returns -1. Consider using a true modulo normalization (e.g. ((index % length) + length) % length) so the result is always in [0, length) for any integer index.
| return (index + length) % length; | |
| return ((index % length) + length) % length; |
src/tools/finance/api.ts
Outdated
| } | ||
|
|
||
| const label = describeRequest(endpoint, params); | ||
| const providerScopedEndpoint = `${resolvedProvider}:${endpoint}`; |
There was a problem hiding this comment.
Using providerScopedEndpoint = ${resolvedProvider}:${endpoint}`` will feed a string containing : into `buildCacheKey()`. That `:` becomes part of the cache path (see `cleanEndpoint` logic), which can break on Windows filesystems where `:` is not allowed in filenames. Prefer encoding the provider without `:` (e.g. `${resolvedProvider}${endpoint}` or `${resolvedProvider}/${endpoint}`) or otherwise sanitizing the provider prefix before passing it to the cache layer.
| const providerScopedEndpoint = `${resolvedProvider}:${endpoint}`; | |
| const providerScopedEndpoint = `${resolvedProvider}_${endpoint}`; |
src/tools/finance/api.ts
Outdated
| export async function callApi( | ||
| endpoint: string, | ||
| params: Record<string, ApiParamValue>, | ||
| options?: { cacheable?: boolean } | ||
| ): Promise<ApiResponse> { | ||
| const configuredProvider = getSetting('financeProvider', 'auto'); | ||
| const resolvedProvider = resolveFinanceProvider( | ||
| typeof configuredProvider === 'string' ? configuredProvider : 'auto', | ||
| ); |
There was a problem hiding this comment.
This change adds substantial new behavior (provider selection + Alpha Vantage endpoint mapping + new filtering/normalization logic) but there are no unit tests covering callApi() routing/caching behavior or the Alpha Vantage response normalization. Given the existing Bun test setup (e.g. cache/providers tests), please add tests that mock fetch to validate key endpoints (prices snapshot, prices range/year grouping, financial metrics, error handling) and ensure caching keys remain stable across providers.
This pull request introduces major enhancements to the CLI, agent prompt construction, and provider management for finance and web search. The update adds interactive slash commands, improves context selection for agent prompts, and introduces a flexible provider abstraction layer with automatic failover support.
It also updates documentation and environment configuration to support Alpha Vantage as an alternative finance data provider.
Key Changes
CLI Enhancements
/help/new/model/finance/search/web-searchexitquitShift + Enterfor new linesCtrl + Backspacefor word deletionProvider Selection & Abstraction
/financeprovider selection/searchand/web-searchprovider selectionAgent Improvements
Finance API & Tooling Updates
Documentation & Environment Updates
README.mdwith:UI Improvements
Commits (6)
fix:resolve agent and browser context issuesfeat:add provider abstraction for finance and search toolsfeat:add command registry and export provider utilitiesfeat:add provider selector components and hooksfeat:integrate provider selectors and command system in CLIfeat:update tool registry, finance API, and utilities