Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Summary
This PR implements a comprehensive @ context feature for the copilot system, allowing users to reference and interact with various types of contextual information during conversations. The changes introduce a new ChatContext union type supporting 6 different context kinds: past_chat, workflow, blocks, logs, knowledge, and templates. Users can now type @ followed by context names to include relevant information in their copilot interactions.
The implementation spans multiple layers of the application:
Frontend UI Changes: The user input component (user-input.tsx) receives a major enhancement with over 1400 lines of new code implementing a sophisticated mention menu system with keyboard navigation, real-time filtering, and visual highlighting. The copilot message component now displays context chips above messages and highlights @ mentions in text.
Tool System Expansion: Five new client-side tools are added to support workflow operations (list_user_workflows, get_workflow_from_name, get_global_workflow_variables, set_global_workflow_variables) and OAuth integration (oauth_request_access). These tools follow the established BaseClientTool pattern with proper state management and error handling.
API and Backend Integration: New API endpoints are created (copilot chats route), existing endpoints are enhanced with context support, and a comprehensive context processing system (process-contents.ts) handles fetching and formatting contextual data from various sources including databases and registries.
State Management: The copilot store is updated to handle context data throughout the message lifecycle, including context validation, persistence in contentBlocks, and restoration from saved messages.
Infrastructure Updates: Chat title generation is enhanced to support both OpenAI and Azure OpenAI providers with model upgrades, and UI components are updated to properly display the new workflow variable operations with structured table layouts.
The feature integrates seamlessly with the existing copilot architecture while significantly expanding its capabilities to work with broader workspace context beyond just the immediate conversation.
Confidence score: 3/5
- This PR introduces significant complexity with potential stability risks due to extensive new code and multiple type safety issues
- Score reflects concerns about type assertions to 'any', complex state management, and insufficient error handling in critical paths
- Pay close attention to
user-input.tsx(1400+ lines),process-contents.ts(type safety issues), and tool registry files (type assertions)
Context used:
Context - Avoid using type assertions to 'any' in TypeScript. Instead, ensure proper type definitions are used to maintain type safety. (link)
23 files reviewed, 28 comments
| gdrive_request_access: toolCallSSEFor( | ||
| 'gdrive_request_access', | ||
| ToolArgSchemas.gdrive_request_access | ||
| ToolArgSchemas.gdrive_request_access as any |
There was a problem hiding this comment.
logic: Type assertion to 'any' bypasses TypeScript safety. Should define proper schema instead.
| ToolArgSchemas.gdrive_request_access as any | |
| ToolArgSchemas.gdrive_request_access |
Context Used: Context - Avoid using type assertions to 'any' in TypeScript. Instead, ensure proper type definitions are used to maintain type safety. (link)
| {String((toolCall.parameters as any).method || '').toUpperCase() || | ||
| 'GET'} |
There was a problem hiding this comment.
style: Multiple type assertions to any throughout this section violate TypeScript safety. Consider defining proper interfaces for tool parameters.
Context Used: Context - Avoid using type assertions to 'any' in TypeScript. Instead, ensure proper type definitions are used to maintain type safety. (link)
| ...(contexts && contexts.length > 0 && { contexts }), | ||
| ...(contexts && | ||
| contexts.length > 0 && { | ||
| contentBlocks: [ | ||
| { type: 'contexts', contexts: contexts as any, timestamp: Date.now() }, | ||
| ] as any, | ||
| }), |
There was a problem hiding this comment.
logic: The contexts are stored twice in the message object - once as a top-level property and once in contentBlocks. This duplication could lead to synchronization issues if they get out of sync.
| key={idx} | ||
| className='grid grid-cols-3 items-center gap-0 px-2 py-1.5' |
There was a problem hiding this comment.
logic: Using array index as React key can cause rendering issues if operations order changes. Consider using a unique identifier if available.
| onClosed as EventListener, | ||
| { | ||
| once: true, | ||
| } as any |
There was a problem hiding this comment.
style: Type assertion to any bypasses TypeScript's event listener options checking
| } as any | |
| } satisfies AddEventListenerOptions |
Context Used: Context - Avoid using type assertions to 'any' in TypeScript. Instead, ensure proper type definitions are used to maintain type safety. (link)
| const varsRecord = (json?.data as Record<string, any>) || {} | ||
| // Convert to name/value pairs for clarity | ||
| const variables = Object.values(varsRecord).map((v: any) => ({ | ||
| name: String(v?.name || ''), | ||
| value: (v as any)?.value, | ||
| })) |
There was a problem hiding this comment.
logic: Using multiple type assertions to any could cause runtime errors if the API response structure changes. Consider defining proper types for the expected response format.
Context Used: Context - Avoid using type assertions to 'any' in TypeScript. Instead, ensure proper type definitions are used to maintain type safety. (link)
| {(Array.isArray((message as any).contexts) && (message as any).contexts.length > 0) || | ||
| (Array.isArray(message.contentBlocks) && | ||
| (message.contentBlocks as any[]).some((b: any) => b?.type === 'contexts')) ? ( |
There was a problem hiding this comment.
style: Complex conditional logic spread across multiple lines makes this hard to read and maintain. Consider extracting this into a helper function or using early returns.
| {(() => { | ||
| const direct = Array.isArray((message as any).contexts) | ||
| ? ((message as any).contexts as any[]) | ||
| : [] | ||
| const block = Array.isArray(message.contentBlocks) | ||
| ? (message.contentBlocks as any[]).find((b: any) => b?.type === 'contexts') | ||
| : null | ||
| const fromBlock = Array.isArray((block as any)?.contexts) | ||
| ? ((block as any).contexts as any[]) | ||
| : [] | ||
| const allContexts = direct.length > 0 ? direct : fromBlock | ||
| const MAX_VISIBLE = 4 | ||
| const visible = showAllContexts | ||
| ? allContexts | ||
| : allContexts.slice(0, MAX_VISIBLE) | ||
| return ( | ||
| <> | ||
| {visible.map((ctx: any, idx: number) => ( | ||
| <span | ||
| key={`ctx-${idx}-${ctx?.label || ctx?.kind}`} | ||
| className='inline-flex items-center gap-1 rounded-full bg-[color-mix(in_srgb,var(--brand-primary-hover-hex)_14%,transparent)] px-1.5 py-0.5 text-[11px] text-foreground' | ||
| title={ctx?.label || ctx?.kind} | ||
| > | ||
| {ctx?.kind === 'past_chat' ? ( | ||
| <Bot className='h-3 w-3 text-muted-foreground' /> | ||
| ) : ctx?.kind === 'workflow' ? ( | ||
| <Workflow className='h-3 w-3 text-muted-foreground' /> | ||
| ) : ctx?.kind === 'blocks' ? ( | ||
| <Blocks className='h-3 w-3 text-muted-foreground' /> | ||
| ) : ctx?.kind === 'knowledge' ? ( | ||
| <LibraryBig className='h-3 w-3 text-muted-foreground' /> | ||
| ) : ctx?.kind === 'templates' ? ( | ||
| <Shapes className='h-3 w-3 text-muted-foreground' /> | ||
| ) : ( | ||
| <Info className='h-3 w-3 text-muted-foreground' /> | ||
| )} | ||
| <span className='max-w-[140px] truncate'> | ||
| {ctx?.label || ctx?.kind} | ||
| </span> | ||
| </span> | ||
| ))} | ||
| {allContexts.length > MAX_VISIBLE && ( | ||
| <button | ||
| type='button' | ||
| onClick={() => setShowAllContexts((v) => !v)} | ||
| className='inline-flex items-center gap-1 rounded-full bg-[color-mix(in_srgb,var(--brand-primary-hover-hex)_10%,transparent)] px-1.5 py-0.5 text-[11px] text-foreground hover:bg-[color-mix(in_srgb,var(--brand-primary-hover-hex)_14%,transparent)]' | ||
| title={ | ||
| showAllContexts | ||
| ? 'Show less' | ||
| : `Show ${allContexts.length - MAX_VISIBLE} more` | ||
| } | ||
| > | ||
| {showAllContexts | ||
| ? 'Show less' | ||
| : `+${allContexts.length - MAX_VISIBLE} more`} | ||
| </button> | ||
| )} | ||
| </> | ||
| ) | ||
| })()} |
There was a problem hiding this comment.
style: This IIFE contains complex state logic that's difficult to follow. The context merging logic should be extracted into a custom hook or utility function for better testability and reusability.
Context Used: Context - If a switch statement is large and handles multiple cases, extract each case into separate functions for better maintainability. (link)
| {(() => { | ||
| const text = message.content || '' | ||
| const contexts: any[] = Array.isArray((message as any).contexts) | ||
| ? ((message as any).contexts as any[]) | ||
| : [] | ||
| const labels = contexts.map((c) => c?.label).filter(Boolean) as string[] | ||
| if (!labels.length) return <WordWrap text={text} /> | ||
|
|
||
| const escapeRegex = (s: string) => s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') | ||
| const pattern = new RegExp(`@(${labels.map(escapeRegex).join('|')})`, 'g') | ||
|
|
||
| const nodes: React.ReactNode[] = [] | ||
| let lastIndex = 0 | ||
| let match: RegExpExecArray | null | ||
| while ((match = pattern.exec(text)) !== null) { | ||
| const i = match.index | ||
| const before = text.slice(lastIndex, i) | ||
| if (before) nodes.push(before) | ||
| const mention = match[0] | ||
| nodes.push( | ||
| <span | ||
| key={`mention-${i}-${lastIndex}`} | ||
| className='rounded-[6px] bg-[color-mix(in_srgb,var(--brand-primary-hover-hex)_14%,transparent)] px-1' | ||
| > | ||
| {mention} | ||
| </span> | ||
| ) | ||
| lastIndex = i + mention.length | ||
| } | ||
| const tail = text.slice(lastIndex) | ||
| if (tail) nodes.push(tail) | ||
| return nodes | ||
| })()} |
There was a problem hiding this comment.
style: Another IIFE with complex regex logic and React node manipulation. This mention highlighting logic should be extracted into a separate function or custom hook.
Context Used: Context - If a switch statement is large and handles multiple cases, extract each case into separate functions for better maintainability. (link)
| const contexts: any[] = Array.isArray((message as any).contexts) | ||
| ? ((message as any).contexts as any[]) | ||
| : [] |
There was a problem hiding this comment.
style: Using any[] type assertions throughout the contexts handling. Consider defining proper TypeScript interfaces for better type safety.
Context Used: Context - Avoid using type assertions to 'any' in TypeScript. Instead, ensure proper type definitions are used to maintain type safety. (link)
Sg312
force-pushed
the
feat/copilot-read-other-workflows
branch
from
9ce1a1b to
939c93a
Compare
* Copilot updates * Set/get vars * Credentials opener v1 * Progress * Checkpoint? * Context v1 * Workflow references * Add knowledge base context * Blocks * Templates * Much better pills * workflow updates * Major ui * Workflow box colors * Much i mproved ui * Improvements * Much better * Add @ icon * Welcome page * Update tool names * Matches * UPdate ordering * Good sort * Good @ handling * Update placeholder * Updates * Lint * Almost there * Wrapped up? * Lint * Builid error fix * Build fix? * Lint * Fix load vars
* Copilot updates * Set/get vars * Credentials opener v1 * Progress * Checkpoint? * Context v1 * Workflow references * Add knowledge base context * Blocks * Templates * Much better pills * workflow updates * Major ui * Workflow box colors * Much i mproved ui * Improvements * Much better * Add @ icon * Welcome page * Update tool names * Matches * UPdate ordering * Good sort * Good @ handling * Update placeholder * Updates * Lint * Almost there * Wrapped up? * Lint * Builid error fix * Build fix? * Lint * Fix load vars
1 participant
Summary
Adds @ context to copilot
Also adds tools to view/set vars
Type of Change
Testing
Manual
Checklist