feat: enhance post-processing for tool results #1845
Conversation
- Introduced ToolType for post-processing selection in AgentBaseTool. - Added 'copilot' model scene and corresponding default model configuration. - Implemented archiving options in UpdateWorkflowVariablesRequest and UpsertDriveFileRequest. - Added ArchivedRef and context management utilities for efficient message handling. - Developed compressAgentLoopMessages function to manage context during agent iterations. - Enhanced context block truncation and archived references handling. - Updated utility exports to include context-manager functionalities.
WalkthroughReplaces a monolithic Composio post-handler with a modular post-processing framework: adds ComposioToolPostHandlerService and RegularToolPostHandlerService, a ToolWrapperFactoryService, post-handler interfaces/types, token and context utilities, resource upload support, and updates wiring and types across the tool execution stack. (50 words) Changes
Sequence Diagram(s)sequenceDiagram
participant Client as ToolService
participant Wrapper as ToolWrapperFactory
participant Tool as LangChainTool
participant PostHandler as IToolPostHandler
participant Resource as ResourceHandler
participant Billing as BillingService
Client->>Wrapper: invoke(tool, input, config)
activate Wrapper
Wrapper->>Tool: execute(input)
activate Tool
Tool-->>Wrapper: rawResult + creditCost
deactivate Tool
Wrapper->>Wrapper: select post-handler (toolType)
Wrapper->>PostHandler: process(PostHandlerInput)
activate PostHandler
PostHandler->>PostHandler: normalize & check success
alt content large or over tokens
PostHandler->>Resource: uploadToolResult(user, options)
activate Resource
Resource-->>PostHandler: DriveFile metadata
deactivate Resource
PostHandler->>PostHandler: augment content with file ref
else compress/truncate content
PostHandler->>PostHandler: compress/truncate based on tool rules
end
alt success
PostHandler->>Billing: record credit usage
Billing-->>PostHandler: ack
else failure
PostHandler-->>Wrapper: error payload
end
PostHandler-->>Wrapper: PostHandlerOutput{content,fileId,success,wasTruncated}
deactivate PostHandler
Wrapper-->>Client: {content, status, creditCost}
deactivate Wrapper
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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: 5
🧹 Nitpick comments (18)
apps/api/src/modules/tool/composio/composio.module.ts (1)
11-21: ComposioModule now correctly wires the new post-handlerSwitching the provider to
ComposioToolPostHandlerServicealigns this module with the new post‑execution architecture. SinceComposioToolPostHandlerServiceis also provided inToolModule, consider centralizing it in one module and exporting it if you ever need a single shared instance, but functionally this setup is fine.apps/api/src/modules/tool/tool.service.ts (1)
1136-1163: Wrapper invocation correctly integrated.The refactored invocation properly uses
toolWrapperFactory.invoke()and correctly extractscreditCostandcontentfrom the result. The billing logic appropriately checks forresult?.status !== 'error'before processing credits.One minor observation: Consider destructuring both
creditCostandstatustogether for consistency:- const { creditCost } = result; + const { creditCost, status, content } = result; const skillConfig = config as SkillRunnableConfig; const resultId = skillConfig?.configurable?.resultId; const version = skillConfig?.configurable?.version; - if (isGlobal && result?.status !== 'error' && creditCost > 0) { + if (isGlobal && status !== 'error' && creditCost > 0) {apps/api/src/modules/tool/tool-execution/wrapper/wrapper.service.ts (2)
70-87: Type safety concern with dynamic property access.The code casts
tooltoanyto accesstoolsetKeyandtoolType. This bypasses type checking and could lead to runtime issues if the properties don't exist.Consider using a more type-safe approach:
- const toolAny = tool as any; - const toolsetKey: string = toolAny.toolsetKey ?? 'unknown'; - const toolType: ToolType = toolAny.toolType ?? 'regular'; + const metadata = (tool as { metadata?: Record<string, unknown> }).metadata; + const toolsetKey: string = (metadata?.toolsetKey as string) ?? 'unknown'; + const toolType: ToolType = (metadata?.type as ToolType) ?? 'regular';This aligns with how tools are created in
tool.service.ts(see lines 1165-1170) where metadata containstypeandtoolsetKey.
104-118: Clarify status semantics on post-processing failure.When post-processing fails, the code returns
status: 'success'(line 115). This could be confusing since:
- Post-processing failed (an error occurred)
- The raw result might itself indicate an error
Consider either:
- Documenting this behavior explicitly in the JSDoc
- Using a distinct status like
'partial'or keeping the original raw result's status if availablereturn { content: typeof rawResult === 'string' ? rawResult : JSON.stringify(rawResult ?? {}, null, 2), - status: 'success', + status: 'success', // Tool execution succeeded; only post-processing failed creditCost, };apps/api/src/modules/tool/tool-execution/post-execution/composio-post.service.ts (2)
157-163: Normalization assumes specific Composio response format.The logic
raw.length === 2is a heuristic for Composio's[result, metadata]format. Consider adding a comment explaining this assumption and potentially checking thatraw[0]is not undefined.private normalizeResult(raw: unknown): unknown { - // Composio may return response in different formats + // Composio returns [result, metadata] tuple format - extract the result if (Array.isArray(raw) && raw.length === 2) { - return raw[0]; + return raw[0] ?? raw; // Fallback to full array if first element is undefined } return raw; }
110-129: Consider logging parse failures for debugging.The empty catch block on line 126-128 silently ignores JSON parse failures. While the fallback behavior is correct, adding a debug log could help with troubleshooting:
} catch { - // If compressed is not valid JSON, keep as-is + // If compressed is not valid JSON, keep as-is + this.logger.debug('Compressed content is not valid JSON, skipping file metadata injection'); }apps/api/src/modules/tool/composio/composio.service.ts (2)
651-651: Use nullish coalescing (??) instead of logical OR (||).The coding guidelines require using nullish coalescing for potentially undefined values. The
||operator would incorrectly fall back to'untitled'iffile_name_titleis an empty string.- fileNameTitle: (file_name_title as string) || 'untitled', + fileNameTitle: (file_name_title as string) ?? 'untitled',
637-639: Use optional chaining with type assertions for safer property access.The current type assertions assume
runnableConfig?.configurableexists. Consider using optional chaining throughout.- resultId: runnableConfig?.configurable?.resultId as string | undefined, - version: runnableConfig?.configurable?.version as number | undefined, - canvasId: runnableConfig?.configurable?.canvasId as string | undefined, + resultId: (runnableConfig?.configurable as Record<string, unknown>)?.resultId as string | undefined, + version: (runnableConfig?.configurable as Record<string, unknown>)?.version as number | undefined, + canvasId: (runnableConfig?.configurable as Record<string, unknown>)?.canvasId as string | undefined,Alternatively, consider defining a proper type for
configurableto avoid these casts.apps/api/src/modules/tool/tool-execution/post-execution/regular-post.service.ts (2)
77-93: Silent failure in JSON parsing could leave content unchanged unexpectedly.The empty catch block silently ignores parsing failures. While this is intentional to keep non-JSON content as-is, consider logging a debug message for observability.
try { const parsed = JSON.parse(compressed); parsed.files = [ { fileId: fileMeta.fileId, canvasId: fileMeta.canvasId, name: fileMeta.name, type: fileMeta.type, summary: 'Full content stored here. Use read_file tool to obtain detailed content.', }, ]; content = JSON.stringify(parsed, null, 2); } catch { - // If compressed is not valid JSON, keep as-is + // If compressed is not valid JSON, keep as-is (expected for plain text results) }
121-127: Document the tuple convention for tool adapters.The
[content, artifacts]tuple pattern is an implicit contract. Consider adding a brief JSDoc explaining which adapters use this pattern.private normalizeResult(raw: unknown): unknown { - // Some tool adapters return [content, artifacts] + /** + * Some tool adapters return [content, artifacts] tuple. + * Extract the content (first element) for processing. + */ if (Array.isArray(raw) && raw.length === 2) { return raw[0]; } return raw; }apps/api/src/modules/tool/tool-execution/post-execution/post.interface.ts (1)
79-90: Both abstract class and interface define the same contract.Having both
AbstractToolPostHandlerandIToolPostHandlerwith identical signatures could cause confusion. The abstract class is useful for NestJS DI as an injection token, but consider documenting when to use each./** - * Interface version for type checking (when abstract class is not needed) + * Interface version for type checking. + * Use IToolPostHandler for type annotations. + * Use AbstractToolPostHandler as injection token in NestJS modules. */ export interface IToolPostHandler {apps/api/src/modules/tool/utils/token.ts (3)
26-31: Token estimation differs from existing implementation in skill-template.This
estimateTokensfunction uses a simple 4 chars/token ratio, whilepackages/skill-template/src/scheduler/utils/token.tsuses a CJK-aware estimation (1.5 tokens for CJK characters, 0.25 for non-CJK). This inconsistency could lead to different truncation behavior depending on which module processes the content.Consider either:
- Importing the existing
estimateTokensfrom@refly/skill-templatefor consistency- Documenting why a simpler estimation is preferred here (e.g., performance)
+// Note: This is a simplified estimation for performance. +// For accurate CJK-aware estimation, use estimateTokens from @refly/skill-template /** * Estimate token count from text (4 chars per token) */ export function estimateTokens(text: string): number { return Math.ceil((text?.length ?? 0) / 4); }
57-73: Incomplete TLD list may cause incorrect domain extraction.The
knownTLDslist is limited. Consider using a more comprehensive approach or a library likepsl(Public Suffix List) for accurate domain extraction.export function extractRootDomain(url: string): string { try { const hostname = new URL(url).hostname; const parts = hostname.split('.'); if (parts.length > 2) { - const knownTLDs = ['co.uk', 'com.cn', 'com.au', 'co.jp', 'org.uk']; + // Common multi-part TLDs - extend as needed + const knownTLDs = [ + 'co.uk', 'com.cn', 'com.au', 'co.jp', 'org.uk', + 'co.nz', 'co.za', 'com.br', 'com.mx', 'co.in', + ]; const lastTwo = parts.slice(-2).join('.');
118-127: NOISE_PATTERNS are English-only.These patterns will only filter English navigation text. For internationalized content, consider making patterns configurable or adding common patterns for other languages.
This is acceptable for the current use case but worth noting for future i18n support.
packages/skill-template/src/utils/context-manager.ts (4)
243-249: NegativetokensToFreecalculation may be unexpected.When
remainingBudgetis positive,tokensToFreebecomes negative:-remainingBudget + minRemainingTokens. This is correct mathematically (we don't need to free tokens), but the logic is counterintuitive and could be clearer.// Calculate how many tokens we need to free // We want to have at least (targetBudget * REMAINING_SPACE_THRESHOLD) remaining after compression const minRemainingTokens = targetBudget * REMAINING_SPACE_THRESHOLD; - const tokensToFree = Math.max(0, -remainingBudget + minRemainingTokens); + // If remainingBudget is already sufficient, tokensToFree will be 0 + const tokensNeeded = minRemainingTokens - remainingBudget; + const tokensToFree = Math.max(0, tokensNeeded);
109-136: Tool call ID collision risk in parallel executions.The fallback
archived_${Date.now()}fortoolCallIdcould collide if multiple compressions happen within the same millisecond. Consider using a more unique identifier.function createToolPairPlaceholder( fileId: string, toolNames: string[], originalToolCallId: string, ): [AIMessage, ToolMessage] { - const toolCallId = originalToolCallId || `archived_${Date.now()}`; + const toolCallId = originalToolCallId || `archived_${Date.now()}_${Math.random().toString(36).slice(2, 8)}`;
262-263: Hardcoded MIN_KEEP_COUNT with TODO for dynamic calculation.The TODO notes that MIN_KEEP_COUNT should be dynamic based on context/model capacity. This is a valid concern for different model sizes.
Would you like me to help implement dynamic MIN_KEEP_COUNT calculation based on the model's context limit? For example:
- Small models (8K context): MIN_KEEP_COUNT = 3
- Medium models (32K context): MIN_KEEP_COUNT = 5
- Large models (128K+ context): MIN_KEEP_COUNT = 10
251-260: Consider making the compression ratio configurable to optimize for different use cases.The hardcoded
HISTORY_COMPRESS_RATIO = 0.7enforces a 70% minimum archival floor, which could remove relevant context unnecessarily. Since no universal optimal ratio exists and compression strategies vary by task and workload, this value should be configurable and validated through testing against your specific use cases before deployment.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
.gitignore(1 hunks)apps/api/src/modules/tool/composio/composio.module.ts(1 hunks)apps/api/src/modules/tool/composio/composio.service.ts(4 hunks)apps/api/src/modules/tool/composio/post-handler.service.ts(0 hunks)apps/api/src/modules/tool/resource.service.ts(1 hunks)apps/api/src/modules/tool/tool-execution/index.ts(1 hunks)apps/api/src/modules/tool/tool-execution/post-execution/composio-post.service.ts(1 hunks)apps/api/src/modules/tool/tool-execution/post-execution/post.interface.ts(1 hunks)apps/api/src/modules/tool/tool-execution/post-execution/regular-post.service.ts(1 hunks)apps/api/src/modules/tool/tool-execution/wrapper/wrapper.interface.ts(1 hunks)apps/api/src/modules/tool/tool-execution/wrapper/wrapper.service.ts(1 hunks)apps/api/src/modules/tool/tool.module.ts(2 hunks)apps/api/src/modules/tool/tool.service.ts(4 hunks)apps/api/src/modules/tool/utils/index.ts(1 hunks)apps/api/src/modules/tool/utils/token.ts(1 hunks)packages/agent-tools/src/base.ts(2 hunks)packages/ai-workspace-common/src/requests/types.gen.ts(4 hunks)packages/request/src/requests/types.gen.ts(4 hunks)packages/skill-template/src/scheduler/utils/context.ts(1 hunks)packages/skill-template/src/scheduler/utils/token.ts(1 hunks)packages/skill-template/src/skills/agent.ts(2 hunks)packages/skill-template/src/utils/context-manager.ts(1 hunks)packages/skill-template/src/utils/index.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/api/src/modules/tool/composio/post-handler.service.ts
🧰 Additional context used
📓 Path-based instructions (16)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,jsx,tsx}: Always use optional chaining (?.) when accessing object properties
Always use nullish coalescing (??) or default values for potentially undefined values
Always check array existence before using array methods
Always validate object properties before destructuring
Always use single quotes for string literals in JavaScript/TypeScript code
**/*.{js,ts,jsx,tsx}: Use semicolons at the end of statements
Include spaces around operators (e.g.,a + binstead ofa+b)
Always use curly braces for control statements
Place opening braces on the same line as their statement
**/*.{js,ts,jsx,tsx}: Group import statements in order: React/framework libraries, third-party libraries, internal modules, relative path imports, type imports, style imports
Sort imports alphabetically within each import group
Leave a blank line between import groups
Extract complex logic into custom hooks
Use functional updates for state (e.g.,setCount(prev => prev + 1))
Split complex state into multiple state variables rather than single large objects
Use useReducer for complex state logic instead of multiple useState calls
Files:
apps/api/src/modules/tool/utils/index.tsapps/api/src/modules/tool/tool-execution/index.tspackages/skill-template/src/scheduler/utils/token.tspackages/agent-tools/src/base.tsapps/api/src/modules/tool/tool-execution/wrapper/wrapper.interface.tsapps/api/src/modules/tool/resource.service.tspackages/request/src/requests/types.gen.tsapps/api/src/modules/tool/tool-execution/post-execution/composio-post.service.tspackages/ai-workspace-common/src/requests/types.gen.tsapps/api/src/modules/tool/tool-execution/post-execution/regular-post.service.tspackages/skill-template/src/scheduler/utils/context.tsapps/api/src/modules/tool/tool-execution/wrapper/wrapper.service.tsapps/api/src/modules/tool/tool.service.tsapps/api/src/modules/tool/composio/composio.module.tspackages/skill-template/src/utils/context-manager.tsapps/api/src/modules/tool/composio/composio.service.tsapps/api/src/modules/tool/utils/token.tsapps/api/src/modules/tool/tool-execution/post-execution/post.interface.tspackages/skill-template/src/skills/agent.tspackages/skill-template/src/utils/index.tsapps/api/src/modules/tool/tool.module.ts
**/*.{js,ts,tsx,jsx,py,java,cpp,c,cs,rb,go,rs,php,swift,kt,scala,r,m,mm,sql}
📄 CodeRabbit inference engine (.cursor/rules/00-language-priority.mdc)
**/*.{js,ts,tsx,jsx,py,java,cpp,c,cs,rb,go,rs,php,swift,kt,scala,r,m,mm,sql}: All code comments MUST be written in English
All variable names, function names, class names, and other identifiers MUST use English words
Comments should be concise and explain 'why' rather than 'what'
Use proper grammar and punctuation in comments
Keep comments up-to-date when code changes
Document complex logic, edge cases, and important implementation details
Use clear, descriptive names that indicate purpose
Avoid abbreviations unless they are universally understood
Files:
apps/api/src/modules/tool/utils/index.tsapps/api/src/modules/tool/tool-execution/index.tspackages/skill-template/src/scheduler/utils/token.tspackages/agent-tools/src/base.tsapps/api/src/modules/tool/tool-execution/wrapper/wrapper.interface.tsapps/api/src/modules/tool/resource.service.tspackages/request/src/requests/types.gen.tsapps/api/src/modules/tool/tool-execution/post-execution/composio-post.service.tspackages/ai-workspace-common/src/requests/types.gen.tsapps/api/src/modules/tool/tool-execution/post-execution/regular-post.service.tspackages/skill-template/src/scheduler/utils/context.tsapps/api/src/modules/tool/tool-execution/wrapper/wrapper.service.tsapps/api/src/modules/tool/tool.service.tsapps/api/src/modules/tool/composio/composio.module.tspackages/skill-template/src/utils/context-manager.tsapps/api/src/modules/tool/composio/composio.service.tsapps/api/src/modules/tool/utils/token.tsapps/api/src/modules/tool/tool-execution/post-execution/post.interface.tspackages/skill-template/src/skills/agent.tspackages/skill-template/src/utils/index.tsapps/api/src/modules/tool/tool.module.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/00-language-priority.mdc)
Use JSDoc style comments for functions and classes in JavaScript/TypeScript
Files:
apps/api/src/modules/tool/utils/index.tsapps/api/src/modules/tool/tool-execution/index.tspackages/skill-template/src/scheduler/utils/token.tspackages/agent-tools/src/base.tsapps/api/src/modules/tool/tool-execution/wrapper/wrapper.interface.tsapps/api/src/modules/tool/resource.service.tspackages/request/src/requests/types.gen.tsapps/api/src/modules/tool/tool-execution/post-execution/composio-post.service.tspackages/ai-workspace-common/src/requests/types.gen.tsapps/api/src/modules/tool/tool-execution/post-execution/regular-post.service.tspackages/skill-template/src/scheduler/utils/context.tsapps/api/src/modules/tool/tool-execution/wrapper/wrapper.service.tsapps/api/src/modules/tool/tool.service.tsapps/api/src/modules/tool/composio/composio.module.tspackages/skill-template/src/utils/context-manager.tsapps/api/src/modules/tool/composio/composio.service.tsapps/api/src/modules/tool/utils/token.tsapps/api/src/modules/tool/tool-execution/post-execution/post.interface.tspackages/skill-template/src/skills/agent.tspackages/skill-template/src/utils/index.tsapps/api/src/modules/tool/tool.module.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/01-code-style.mdc)
**/*.{js,jsx,ts,tsx}: Use single quotes for string literals in TypeScript/JavaScript
Always use optional chaining (?.) when accessing object properties in TypeScript/JavaScript
Always use nullish coalescing (??) or default values for potentially undefined values in TypeScript/JavaScript
Always check array existence before using array methods in TypeScript/JavaScript
Validate object properties before destructuring in TypeScript/JavaScript
Use ES6+ features like arrow functions, destructuring, and spread operators in TypeScript/JavaScript
Avoid magic numbers and strings - use named constants in TypeScript/JavaScript
Use async/await instead of raw promises for asynchronous code in TypeScript/JavaScript
Files:
apps/api/src/modules/tool/utils/index.tsapps/api/src/modules/tool/tool-execution/index.tspackages/skill-template/src/scheduler/utils/token.tspackages/agent-tools/src/base.tsapps/api/src/modules/tool/tool-execution/wrapper/wrapper.interface.tsapps/api/src/modules/tool/resource.service.tspackages/request/src/requests/types.gen.tsapps/api/src/modules/tool/tool-execution/post-execution/composio-post.service.tspackages/ai-workspace-common/src/requests/types.gen.tsapps/api/src/modules/tool/tool-execution/post-execution/regular-post.service.tspackages/skill-template/src/scheduler/utils/context.tsapps/api/src/modules/tool/tool-execution/wrapper/wrapper.service.tsapps/api/src/modules/tool/tool.service.tsapps/api/src/modules/tool/composio/composio.module.tspackages/skill-template/src/utils/context-manager.tsapps/api/src/modules/tool/composio/composio.service.tsapps/api/src/modules/tool/utils/token.tsapps/api/src/modules/tool/tool-execution/post-execution/post.interface.tspackages/skill-template/src/skills/agent.tspackages/skill-template/src/utils/index.tsapps/api/src/modules/tool/tool.module.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/03-typescript-guidelines.mdc)
**/*.{ts,tsx}: Avoid usinganytype whenever possible - useunknowntype instead with proper type guards
Always define explicit return types for functions, especially for public APIs
Prefer extending existing types over creating entirely new types
Use TypeScript utility types (Partial<T>,Pick<T, K>,Omit<T, K>,Readonly<T>,Record<K, T>) to derive new types
Use union types and intersection types to combine existing types
Always import types explicitly using theimport typesyntax
Group type imports separately from value imports
Minimize creating local type aliases for imported types
Files:
apps/api/src/modules/tool/utils/index.tsapps/api/src/modules/tool/tool-execution/index.tspackages/skill-template/src/scheduler/utils/token.tspackages/agent-tools/src/base.tsapps/api/src/modules/tool/tool-execution/wrapper/wrapper.interface.tsapps/api/src/modules/tool/resource.service.tspackages/request/src/requests/types.gen.tsapps/api/src/modules/tool/tool-execution/post-execution/composio-post.service.tspackages/ai-workspace-common/src/requests/types.gen.tsapps/api/src/modules/tool/tool-execution/post-execution/regular-post.service.tspackages/skill-template/src/scheduler/utils/context.tsapps/api/src/modules/tool/tool-execution/wrapper/wrapper.service.tsapps/api/src/modules/tool/tool.service.tsapps/api/src/modules/tool/composio/composio.module.tspackages/skill-template/src/utils/context-manager.tsapps/api/src/modules/tool/composio/composio.service.tsapps/api/src/modules/tool/utils/token.tsapps/api/src/modules/tool/tool-execution/post-execution/post.interface.tspackages/skill-template/src/skills/agent.tspackages/skill-template/src/utils/index.tsapps/api/src/modules/tool/tool.module.ts
**/*.{js,ts,jsx,tsx,css,json}
📄 CodeRabbit inference engine (.cursor/rules/04-code-formatting.mdc)
Maximum line length of 100 characters
Files:
apps/api/src/modules/tool/utils/index.tsapps/api/src/modules/tool/tool-execution/index.tspackages/skill-template/src/scheduler/utils/token.tspackages/agent-tools/src/base.tsapps/api/src/modules/tool/tool-execution/wrapper/wrapper.interface.tsapps/api/src/modules/tool/resource.service.tspackages/request/src/requests/types.gen.tsapps/api/src/modules/tool/tool-execution/post-execution/composio-post.service.tspackages/ai-workspace-common/src/requests/types.gen.tsapps/api/src/modules/tool/tool-execution/post-execution/regular-post.service.tspackages/skill-template/src/scheduler/utils/context.tsapps/api/src/modules/tool/tool-execution/wrapper/wrapper.service.tsapps/api/src/modules/tool/tool.service.tsapps/api/src/modules/tool/composio/composio.module.tspackages/skill-template/src/utils/context-manager.tsapps/api/src/modules/tool/composio/composio.service.tsapps/api/src/modules/tool/utils/token.tsapps/api/src/modules/tool/tool-execution/post-execution/post.interface.tspackages/skill-template/src/skills/agent.tspackages/skill-template/src/utils/index.tsapps/api/src/modules/tool/tool.module.ts
**/*.{js,ts,jsx,tsx,css,json,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/04-code-formatting.mdc)
Use 2 spaces for indentation, no tabs
Files:
apps/api/src/modules/tool/utils/index.tsapps/api/src/modules/tool/tool-execution/index.tspackages/skill-template/src/scheduler/utils/token.tspackages/agent-tools/src/base.tsapps/api/src/modules/tool/tool-execution/wrapper/wrapper.interface.tsapps/api/src/modules/tool/resource.service.tspackages/request/src/requests/types.gen.tsapps/api/src/modules/tool/tool-execution/post-execution/composio-post.service.tspackages/ai-workspace-common/src/requests/types.gen.tsapps/api/src/modules/tool/tool-execution/post-execution/regular-post.service.tspackages/skill-template/src/scheduler/utils/context.tsapps/api/src/modules/tool/tool-execution/wrapper/wrapper.service.tsapps/api/src/modules/tool/tool.service.tsapps/api/src/modules/tool/composio/composio.module.tspackages/skill-template/src/utils/context-manager.tsapps/api/src/modules/tool/composio/composio.service.tsapps/api/src/modules/tool/utils/token.tsapps/api/src/modules/tool/tool-execution/post-execution/post.interface.tspackages/skill-template/src/skills/agent.tspackages/skill-template/src/utils/index.tsapps/api/src/modules/tool/tool.module.ts
**/*.{js,ts,jsx,tsx,css,json,yml,yaml,md}
📄 CodeRabbit inference engine (.cursor/rules/04-code-formatting.mdc)
No trailing whitespace at the end of lines
Files:
apps/api/src/modules/tool/utils/index.tsapps/api/src/modules/tool/tool-execution/index.tspackages/skill-template/src/scheduler/utils/token.tspackages/agent-tools/src/base.tsapps/api/src/modules/tool/tool-execution/wrapper/wrapper.interface.tsapps/api/src/modules/tool/resource.service.tspackages/request/src/requests/types.gen.tsapps/api/src/modules/tool/tool-execution/post-execution/composio-post.service.tspackages/ai-workspace-common/src/requests/types.gen.tsapps/api/src/modules/tool/tool-execution/post-execution/regular-post.service.tspackages/skill-template/src/scheduler/utils/context.tsapps/api/src/modules/tool/tool-execution/wrapper/wrapper.service.tsapps/api/src/modules/tool/tool.service.tsapps/api/src/modules/tool/composio/composio.module.tspackages/skill-template/src/utils/context-manager.tsapps/api/src/modules/tool/composio/composio.service.tsapps/api/src/modules/tool/utils/token.tsapps/api/src/modules/tool/tool-execution/post-execution/post.interface.tspackages/skill-template/src/skills/agent.tspackages/skill-template/src/utils/index.tsapps/api/src/modules/tool/tool.module.ts
**/index.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursor/rules/05-code-organization.mdc)
Use index files to export multiple components from a directory
Files:
apps/api/src/modules/tool/utils/index.tsapps/api/src/modules/tool/tool-execution/index.tspackages/skill-template/src/utils/index.ts
**/*.{css,scss,sass,less,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/09-design-system.mdc)
**/*.{css,scss,sass,less,js,jsx,ts,tsx}: Primary color (#155EEF) should be used for main brand color in buttons, links, and accents
Error color (#F04438) should be used for error states and destructive actions
Success color (#12B76A) should be used for success states and confirmations
Warning color (#F79009) should be used for warnings and important notifications
Info color (#0BA5EC) should be used for informational elements
Files:
apps/api/src/modules/tool/utils/index.tsapps/api/src/modules/tool/tool-execution/index.tspackages/skill-template/src/scheduler/utils/token.tspackages/agent-tools/src/base.tsapps/api/src/modules/tool/tool-execution/wrapper/wrapper.interface.tsapps/api/src/modules/tool/resource.service.tspackages/request/src/requests/types.gen.tsapps/api/src/modules/tool/tool-execution/post-execution/composio-post.service.tspackages/ai-workspace-common/src/requests/types.gen.tsapps/api/src/modules/tool/tool-execution/post-execution/regular-post.service.tspackages/skill-template/src/scheduler/utils/context.tsapps/api/src/modules/tool/tool-execution/wrapper/wrapper.service.tsapps/api/src/modules/tool/tool.service.tsapps/api/src/modules/tool/composio/composio.module.tspackages/skill-template/src/utils/context-manager.tsapps/api/src/modules/tool/composio/composio.service.tsapps/api/src/modules/tool/utils/token.tsapps/api/src/modules/tool/tool-execution/post-execution/post.interface.tspackages/skill-template/src/skills/agent.tspackages/skill-template/src/utils/index.tsapps/api/src/modules/tool/tool.module.ts
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/09-i18n-guidelines.mdc)
**/*.{tsx,ts}: Use the translation wrapper component and useTranslation hook in components
Ensure all user-facing text is translatable
Files:
apps/api/src/modules/tool/utils/index.tsapps/api/src/modules/tool/tool-execution/index.tspackages/skill-template/src/scheduler/utils/token.tspackages/agent-tools/src/base.tsapps/api/src/modules/tool/tool-execution/wrapper/wrapper.interface.tsapps/api/src/modules/tool/resource.service.tspackages/request/src/requests/types.gen.tsapps/api/src/modules/tool/tool-execution/post-execution/composio-post.service.tspackages/ai-workspace-common/src/requests/types.gen.tsapps/api/src/modules/tool/tool-execution/post-execution/regular-post.service.tspackages/skill-template/src/scheduler/utils/context.tsapps/api/src/modules/tool/tool-execution/wrapper/wrapper.service.tsapps/api/src/modules/tool/tool.service.tsapps/api/src/modules/tool/composio/composio.module.tspackages/skill-template/src/utils/context-manager.tsapps/api/src/modules/tool/composio/composio.service.tsapps/api/src/modules/tool/utils/token.tsapps/api/src/modules/tool/tool-execution/post-execution/post.interface.tspackages/skill-template/src/skills/agent.tspackages/skill-template/src/utils/index.tsapps/api/src/modules/tool/tool.module.ts
**/*.{tsx,ts,json}
📄 CodeRabbit inference engine (.cursor/rules/09-i18n-guidelines.mdc)
Support dynamic content with placeholders in translations
Files:
apps/api/src/modules/tool/utils/index.tsapps/api/src/modules/tool/tool-execution/index.tspackages/skill-template/src/scheduler/utils/token.tspackages/agent-tools/src/base.tsapps/api/src/modules/tool/tool-execution/wrapper/wrapper.interface.tsapps/api/src/modules/tool/resource.service.tspackages/request/src/requests/types.gen.tsapps/api/src/modules/tool/tool-execution/post-execution/composio-post.service.tspackages/ai-workspace-common/src/requests/types.gen.tsapps/api/src/modules/tool/tool-execution/post-execution/regular-post.service.tspackages/skill-template/src/scheduler/utils/context.tsapps/api/src/modules/tool/tool-execution/wrapper/wrapper.service.tsapps/api/src/modules/tool/tool.service.tsapps/api/src/modules/tool/composio/composio.module.tspackages/skill-template/src/utils/context-manager.tsapps/api/src/modules/tool/composio/composio.service.tsapps/api/src/modules/tool/utils/token.tsapps/api/src/modules/tool/tool-execution/post-execution/post.interface.tspackages/skill-template/src/skills/agent.tspackages/skill-template/src/utils/index.tsapps/api/src/modules/tool/tool.module.ts
**/*.{tsx,ts,jsx,js,vue,css,scss,less}
📄 CodeRabbit inference engine (.cursor/rules/11-ui-design-patterns.mdc)
**/*.{tsx,ts,jsx,js,vue,css,scss,less}: Use the primary blue (#155EEF) for main UI elements, CTAs, and active states
Use red (#F04438) only for errors, warnings, and destructive actions
Use green (#12B76A) for success states and confirmations
Use orange (#F79009) for warning states and important notifications
Use blue (#0BA5EC) for informational elements
Primary buttons should be solid with the primary color
Secondary buttons should have a border with transparent or light background
Danger buttons should use the error color
Use consistent padding, border radius, and hover states for all buttons
Follow fixed button sizes based on their importance and context
Use consistent border radius (rounded-lg) for all cards
Apply light shadows (shadow-sm) for card elevation
Maintain consistent padding inside cards (p-4orp-6)
Use subtle borders for card separation
Ensure proper spacing between card elements
Apply consistent styling to all form inputs
Use clear visual indicators for focus, hover, and error states in form elements
Apply proper spacing between elements using 8px, 16px, 24px increments
Ensure proper alignment of elements (left, center, or right)
Use responsive layouts that work across different device sizes
Maintain a minimum contrast ratio of 4.5:1 for text
Files:
apps/api/src/modules/tool/utils/index.tsapps/api/src/modules/tool/tool-execution/index.tspackages/skill-template/src/scheduler/utils/token.tspackages/agent-tools/src/base.tsapps/api/src/modules/tool/tool-execution/wrapper/wrapper.interface.tsapps/api/src/modules/tool/resource.service.tspackages/request/src/requests/types.gen.tsapps/api/src/modules/tool/tool-execution/post-execution/composio-post.service.tspackages/ai-workspace-common/src/requests/types.gen.tsapps/api/src/modules/tool/tool-execution/post-execution/regular-post.service.tspackages/skill-template/src/scheduler/utils/context.tsapps/api/src/modules/tool/tool-execution/wrapper/wrapper.service.tsapps/api/src/modules/tool/tool.service.tsapps/api/src/modules/tool/composio/composio.module.tspackages/skill-template/src/utils/context-manager.tsapps/api/src/modules/tool/composio/composio.service.tsapps/api/src/modules/tool/utils/token.tsapps/api/src/modules/tool/tool-execution/post-execution/post.interface.tspackages/skill-template/src/skills/agent.tspackages/skill-template/src/utils/index.tsapps/api/src/modules/tool/tool.module.ts
**/*.{tsx,ts,jsx,js,vue}
📄 CodeRabbit inference engine (.cursor/rules/11-ui-design-patterns.mdc)
**/*.{tsx,ts,jsx,js,vue}: Include appropriate loading states for async actions in buttons
Group related form elements with appropriate spacing
Provide clear validation feedback for forms
Ensure proper labeling and accessibility for form elements
Ensure all interactive elements are keyboard accessible
Include appropriate ARIA attributes for complex components
Provide alternative text for images and icons
Support screen readers with semantic HTML elements
Files:
apps/api/src/modules/tool/utils/index.tsapps/api/src/modules/tool/tool-execution/index.tspackages/skill-template/src/scheduler/utils/token.tspackages/agent-tools/src/base.tsapps/api/src/modules/tool/tool-execution/wrapper/wrapper.interface.tsapps/api/src/modules/tool/resource.service.tspackages/request/src/requests/types.gen.tsapps/api/src/modules/tool/tool-execution/post-execution/composio-post.service.tspackages/ai-workspace-common/src/requests/types.gen.tsapps/api/src/modules/tool/tool-execution/post-execution/regular-post.service.tspackages/skill-template/src/scheduler/utils/context.tsapps/api/src/modules/tool/tool-execution/wrapper/wrapper.service.tsapps/api/src/modules/tool/tool.service.tsapps/api/src/modules/tool/composio/composio.module.tspackages/skill-template/src/utils/context-manager.tsapps/api/src/modules/tool/composio/composio.service.tsapps/api/src/modules/tool/utils/token.tsapps/api/src/modules/tool/tool-execution/post-execution/post.interface.tspackages/skill-template/src/skills/agent.tspackages/skill-template/src/utils/index.tsapps/api/src/modules/tool/tool.module.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/08-contributing-guidelines.mdc)
**/*.{ts,tsx,js,jsx}: Follow the TypeScript/JavaScript style guidelines
Ensure code is well-tested and documented
Files:
apps/api/src/modules/tool/utils/index.tsapps/api/src/modules/tool/tool-execution/index.tspackages/skill-template/src/scheduler/utils/token.tspackages/agent-tools/src/base.tsapps/api/src/modules/tool/tool-execution/wrapper/wrapper.interface.tsapps/api/src/modules/tool/resource.service.tspackages/request/src/requests/types.gen.tsapps/api/src/modules/tool/tool-execution/post-execution/composio-post.service.tspackages/ai-workspace-common/src/requests/types.gen.tsapps/api/src/modules/tool/tool-execution/post-execution/regular-post.service.tspackages/skill-template/src/scheduler/utils/context.tsapps/api/src/modules/tool/tool-execution/wrapper/wrapper.service.tsapps/api/src/modules/tool/tool.service.tsapps/api/src/modules/tool/composio/composio.module.tspackages/skill-template/src/utils/context-manager.tsapps/api/src/modules/tool/composio/composio.service.tsapps/api/src/modules/tool/utils/token.tsapps/api/src/modules/tool/tool-execution/post-execution/post.interface.tspackages/skill-template/src/skills/agent.tspackages/skill-template/src/utils/index.tsapps/api/src/modules/tool/tool.module.ts
apps/api/src/**/*.{controller,service}.ts
📄 CodeRabbit inference engine (.cursor/rules/06-api-structure.mdc)
Implement proper error handling in API modules
Files:
apps/api/src/modules/tool/resource.service.tsapps/api/src/modules/tool/tool-execution/post-execution/composio-post.service.tsapps/api/src/modules/tool/tool-execution/post-execution/regular-post.service.tsapps/api/src/modules/tool/tool-execution/wrapper/wrapper.service.tsapps/api/src/modules/tool/tool.service.tsapps/api/src/modules/tool/composio/composio.service.ts
🧠 Learnings (2)
📚 Learning: 2025-11-25T03:04:12.836Z
Learnt from: CR
Repo: refly-ai/refly PR: 0
File: .cursor/rules/06-api-structure.mdc:0-0
Timestamp: 2025-11-25T03:04:12.836Z
Learning: Applies to apps/api/src/{auth,user,project,canvas,rag,knowledge,search,skill,share,code-artifact}/**/*.{ts,tsx} : Follow NestJS module structure with controllers, services, and DTOs in dedicated module directories
Applied to files:
apps/api/src/modules/tool/composio/composio.module.tsapps/api/src/modules/tool/tool-execution/post-execution/post.interface.ts
📚 Learning: 2025-11-25T03:04:12.836Z
Learnt from: CR
Repo: refly-ai/refly PR: 0
File: .cursor/rules/06-api-structure.mdc:0-0
Timestamp: 2025-11-25T03:04:12.836Z
Learning: Applies to apps/api/src/{auth,user,project,canvas,rag,knowledge,search,skill,share,code-artifact}/**/*.{controller,service}.ts : Use dependency injection for module dependencies in NestJS
Applied to files:
apps/api/src/modules/tool/composio/composio.module.ts
🧬 Code graph analysis (8)
packages/skill-template/src/scheduler/utils/token.ts (1)
apps/api/src/modules/tool/utils/token.ts (1)
estimateTokens(29-31)
apps/api/src/modules/tool/resource.service.ts (3)
packages/ai-workspace-common/src/requests/types.gen.ts (2)
User(474-483)DriveFile(7614-7683)apps/api/src/modules/tool/sandbox/wrapper/base.ts (1)
canvasId(200-202)apps/api/src/modules/tool/sandbox/scalebox.dto.ts (1)
error(214-220)
packages/ai-workspace-common/src/requests/types.gen.ts (2)
packages/request/src/requests/types.gen.ts (1)
ProviderItem(6084-6129)packages/openapi-schema/src/types.gen.ts (1)
ProviderItem(6137-6182)
apps/api/src/modules/tool/tool-execution/post-execution/regular-post.service.ts (2)
apps/api/src/modules/tool/resource.service.ts (1)
Injectable(71-1075)apps/api/src/modules/tool/tool-execution/post-execution/post.interface.ts (2)
PostHandlerInput(34-40)PostHandlerOutput(57-64)
apps/api/src/modules/tool/composio/composio.module.ts (1)
apps/api/src/modules/tool/tool.module.ts (1)
Module(32-68)
packages/skill-template/src/utils/context-manager.ts (2)
packages/skill-template/src/scheduler/utils/token.ts (3)
estimateMessagesTokens(52-54)estimateTokens(29-46)truncateContent(60-60)packages/skill-template/src/scheduler/utils/context.ts (3)
ContextBlock(47-57)ArchivedRef(30-45)ArchivedRefType(28-28)
apps/api/src/modules/tool/composio/composio.service.ts (2)
apps/api/src/modules/tool/tool-context.ts (1)
runInContext(57-59)apps/api/src/modules/tool/tool-execution/post-execution/post.interface.ts (1)
ComposioPostHandlerInput(45-52)
apps/api/src/modules/tool/utils/token.ts (1)
packages/skill-template/src/scheduler/utils/token.ts (2)
estimateTokens(29-46)truncateContent(60-60)
⏰ 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: build / Build
🔇 Additional comments (29)
.gitignore (1)
54-56: Ignore rules for Claude and Codex look goodIgnoring
.claude/settings.local.jsonand.codex/*is consistent with existing local/tooling patterns and has no functional risk.packages/request/src/requests/types.gen.ts (3)
2151-2197: Copilot model scene and default config extension look consistentAdding
'copilot'toModelSceneandcopilot?: ProviderItemtoDefaultModelConfigmatches the existing pattern for other scenes (chat, agent, image, etc.) and is backward compatible because the new field is optional.
7578-7591:archiveOldFilesflag is a backwards‑compatible API enhancementThe optional
archiveOldFiles?: booleanonUpdateWorkflowVariablesRequestis clearly documented and preserves compatibility for existing callers that do not send the flag.
7684-7737:archiveFilesflag onUpsertDriveFileRequestaligns with variable archival behaviorThe optional
archiveFiles?: booleanfor archiving existing files with matchingvariableId/resultIdfollows the same pattern asarchiveOldFilesand keeps the request shape backward compatible.packages/skill-template/src/utils/index.ts (1)
1-2: Barrel export ofcontext-manageris appropriateRe‑exporting
./context-managerfrom the utils index cleanly exposes the new context utilities without changing existing consumers.packages/ai-workspace-common/src/requests/types.gen.ts (3)
2151-2197: Copilot scene and default model config extension look consistentAdding
'copilot'toModelSceneandcopilot?: ProviderItemtoDefaultModelConfigis type-safe and backward compatible, and matches the agent code that now branches on copilot vs chat scenes. No issues from the type surface.
7586-7599: NewarchiveOldFilesflag is well-scoped and self-documentingThe
archiveOldFiles?: booleanaddition toUpdateWorkflowVariablesRequestis clearly documented and optional, so it will not break existing callers. Ensure the backend implementation actually respects this flag when updating resource variables.
7692-7745: Drive filearchiveFilesflag aligns with scope semanticsThe
archiveFiles?: booleanfield onUpsertDriveFileRequestmatches theDriveFileScopeconcept and reads clearly (“same variableId or resultId before creating new file”). This is a safe, backward-compatible extension.apps/api/src/modules/tool/utils/index.ts (1)
5-11: Re‑exporting token utilities via utils index is appropriateExporting
./tokenalongside other utility modules makes the token helpers discoverable without changing behavior.apps/api/src/modules/tool/tool-execution/index.ts (1)
1-3: Barrel exports for tool execution services are correctThe index cleanly re-exports the composio/regular post handlers and wrapper service, simplifying imports without changing behavior.
packages/agent-tools/src/base.ts (1)
6-17: ToolType discriminator and defaulttoolTypeon AgentBaseTool are sensibleThe
ToolTypeunion is expressive enough for current use cases, and defaultingtoolTypeto'regular'inAgentBaseToolgives backwards compatibility while enabling routing in wrappers/post-handlers. Just make sure concrete tools that are composio/mcp/etc. explicitly overridetoolTypeto the appropriate literal.Also applies to: 75-80
apps/api/src/modules/tool/tool.module.ts (1)
26-30: Post-handler and wrapper services are correctly registered in ToolModuleImporting the two post-handler services and
ToolWrapperFactoryServicefrom./tool-executionand adding them toproviderscleanly exposes them for DI inToolServiceand related code. This matches the new tool execution flow.Also applies to: 61-64
packages/skill-template/src/skills/agent.ts (1)
18-19: Agent loop compression integration is safe; verify model config choiceThe new
compressAgentLoopMessagescall in the LLM node is defensively integrated:
- Only runs when
engine.service,user,canvasId,resultId, andversionare present.- Falls back to original messages on any compression error.
- Correctly passes
contextLimit/maxOutputand replacescurrentMessagesonly whenwasCompressedis true.This looks safe and should help control context growth. Please double‑check that
config.configurable.modelConfigMap.agentis the intended source forcontextLimit/maxOutputgiven the LLM itself is instantiated with thechat/copilotscene, so the budgets stay consistent.Also applies to: 153-189
packages/skill-template/src/scheduler/utils/context.ts (1)
23-57: Well-structured archived reference types.The
ArchivedRefinterface andArchivedRefTypeunion type are cleanly designed with appropriate JSDoc documentation. The optionalarchivedRefsfield onContextBlockmaintains backward compatibility.apps/api/src/modules/tool/tool.service.ts (1)
53-53: LGTM on dependency injection.The
ToolWrapperFactoryServiceis properly imported and injected via constructor.Also applies to: 69-69
apps/api/src/modules/tool/tool-execution/post-execution/composio-post.service.ts (3)
240-270: LGTM - Exa result compression.The compression logic properly handles Exa's response structure with appropriate field selection (
title,url,score, etc.) and content truncation. The deduplication and truncation tracking are well-implemented.
277-313: LGTM - Tavily result compression.Properly handles Tavily-specific fields including the AI-generated
answerandfollow_up_questions. The compression maintains important context while reducing token usage.
387-418: Good resilience pattern for billing.The billing failure is caught and logged without failing the tool execution (line 416-417 comment confirms this is intentional). This is the correct approach for non-critical side effects.
apps/api/src/modules/tool/tool-execution/post-execution/regular-post.service.ts (3)
39-46: LGTM - Clean method signature and proper null handling.The
processmethod correctly uses nullish coalescing formaxTokensdefault and properly destructures input. The try-catch wrapper provides comprehensive error handling with a fallback path.
180-203: LGTM - Jina compression logic is well-structured.The method correctly handles both array (search results) and object (single read) data types, applies deduplication, and preserves essential fields while truncating content. The truncation metadata (
truncated: { total, kept }) provides useful debugging info.
283-301: No action needed. ThePostHandlerContextinterface definesuser: Useras a required field, not optional. Sinceusercannot be undefined by type contract, a null check is unnecessary.Likely an incorrect or invalid review comment.
apps/api/src/modules/tool/tool-execution/post-execution/post.interface.ts (2)
1-90: LGTM - Well-structured interface definitions.The module follows NestJS conventions with clear separation between file service, context, input/output types, and handler contracts. The JSDoc examples for the abstract class are helpful.
23-29: Type safety issue: unsafe assertions in context construction need validation.The
PostHandlerContextin wrapper.service.ts (lines 82-86) uses unsafe type assertions that force potentially undefined values fromconfig?.configurableinto required types. For example,configurable?.canvasId as stringcasts an optional value without validation. This creates a type-safety gap, especially sinceuploadToolResultvalidatescanvasIdat runtime (if (!canvasId) { return null; }), and other parts of the codebase use explicit guards (e.g., scalebox.service.ts). Either add validation before constructing the context object or updatePostHandlerContextfields to reflect that they can be optional, then update all consumers accordingly.apps/api/src/modules/tool/utils/token.ts (2)
37-48: LGTM - Head/tail truncation preserves context effectively.The 70/30 split for head/tail preservation is a good strategy for maintaining both the beginning context and recent content. The truncation marker clearly indicates how much was removed.
360-377: LGTM - Utility functions are well-implemented.The
pickandsafeParseJSONfunctions are clean, handle edge cases properly with optional chaining, and follow best practices.packages/skill-template/src/utils/context-manager.ts (3)
549-554: Critical: archivedRefs preservation is correctly implemented.The early extraction and preservation of
archivedRefsensures the routing table is never truncated. This is essential for maintaining file references after compression.
685-723: LGTM - Model-aware truncation is well-designed.The function correctly calculates context budget based on model limits, accounts for fixed tokens (system prompt, query, history), and includes overhead for images. The logging on truncation provides good observability.
782-830: LGTM - Clean API for agent loop compression.The
compressAgentLoopMessagesfunction provides a clean interface with proper guards for missing context. The example in JSDoc is helpful for integration.apps/api/src/modules/tool/composio/composio.service.ts (1)
662-667: No action needed. The code is type-safe and does not have a null reference issue.The
resultvariable is guaranteed to have a value because:
- It's destructured from an explicitly constructed object in the callback's return statement (
return { result: executionResult, ... })executeTool()has a return type ofPromise<ToolExecuteResponse>(notundefined), soexecutionResultis always defined- TypeScript's type system enforces this contract
The optional chaining (
?.) and nullish coalescing (??) are defensive programming practices, but they're not addressing an actual null risk given the current implementation.Regarding the post-handler call: Calling
process()before the success check is a design choice. If you want to avoid processing failed executions through the post-handler, move the check before line 61; otherwise, the current flow is valid if the post-handler handles both success and error cases.
| async uploadToolResult( | ||
| user: User, | ||
| options: { | ||
| canvasId: string; | ||
| toolsetKey: string; | ||
| toolName: string; | ||
| content: string; | ||
| resultId: string; | ||
| resultVersion?: number; | ||
| }, | ||
| ): Promise<DriveFile | null> { | ||
| const { canvasId, toolsetKey, toolName, content, resultId, resultVersion } = options; | ||
|
|
||
| if (!canvasId) { | ||
| return null; | ||
| } | ||
|
|
||
| try { | ||
| const rid = resultId ?? 'r'; | ||
| const version = resultVersion ?? 0; | ||
| const fileName = `${toolsetKey}-${toolName}-${rid}-v${version}.txt`; | ||
|
|
||
| const driveFile = await this.driveService.createDriveFile(user, { | ||
| canvasId, | ||
| name: fileName, | ||
| type: 'text/plain', | ||
| content, // Pass raw text directly - DriveService handles encoding | ||
| summary: `${toolsetKey} ${toolName} result (${content.length} chars)`, | ||
| source: 'agent', | ||
| resultId, | ||
| resultVersion, | ||
| }); | ||
|
|
||
| return driveFile; | ||
| } catch (error) { | ||
| this.logger.error(`Failed to upload tool result: ${(error as Error)?.message}`, { | ||
| toolsetKey, | ||
| toolName, | ||
| }); | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for DriveService.createDriveFile implementation
fd -e ts -e js | xargs rg -l 'DriveService' | head -20Repository: refly-ai/refly
Length of output: 983
🏁 Script executed:
# Look for the createDriveFile method implementation
rg -A 20 'createDriveFile\s*\(' --type ts | head -100Repository: refly-ai/refly
Length of output: 8947
🏁 Script executed:
# Find the uploadTextResource method mentioned in the review
rg -B 5 -A 20 'uploadTextResource' apps/api/src/modules/tool/resource.service.tsRepository: refly-ai/refly
Length of output: 945
🏁 Script executed:
# Check the current file for how uploadToolResult and uploadTextResource are defined
cat -n apps/api/src/modules/tool/resource.service.ts | sed -n '643,663p'Repository: refly-ai/refly
Length of output: 790
🏁 Script executed:
# Find createDriveFile method in DriveService
rg -B 5 -A 30 'async createDriveFile' apps/api/src/modules/drive/drive.service.tsRepository: refly-ai/refly
Length of output: 1017
🏁 Script executed:
# Also check if there's any content encoding/decoding logic in the drive service
rg -B 2 -A 5 'base64|encoding|Buffer.from' apps/api/src/modules/drive/drive.service.ts | head -80Repository: refly-ai/refly
Length of output: 2629
The uploadToolResult method correctly handles content encoding; however, it reveals an inconsistency in uploadTextResource.
The method is well-structured with proper error handling. The comment on line 1059 stating "Pass raw text directly - DriveService handles encoding" is accurate. DriveService.createDriveFile expects the content parameter to be a UTF-8 string (not base64), which it then encodes to a Buffer using Buffer.from(content, 'utf8').
However, this reveals that uploadTextResource (lines 643-663) has a bug: it unnecessarily encodes the content to base64 before passing it to createDriveFile, which then treats the base64 string as UTF-8 text, storing the encoded representation rather than the original content. The uploadToolResult method implements the correct approach by passing raw text directly.
Other observations:
- Proper null check for
canvasId(line 1046) - Defensive defaults for
resultIdandresultVersion(lines 1051-1052) - Error logging includes context (toolsetKey, toolName)
- Returns null on error instead of throwing (graceful degradation)
Consider fixing uploadTextResource to match the correct pattern used in uploadToolResult.
🤖 Prompt for AI Agents
In apps/api/src/modules/tool/resource.service.ts around lines 643 to 663,
uploadTextResource currently base64-encodes the text before calling
DriveService.createDriveFile, but createDriveFile expects a UTF-8 string and
will Buffer.from(content, 'utf8'), so the base64 string gets stored instead of
the original text; remove the base64 encoding and pass the raw UTF-8 string
directly (keep existing name/summary/metadata and error handling), relying on
DriveService to perform the Buffer conversion.
| async process(input: PostHandlerInput): Promise<PostHandlerOutput> { | ||
| const composioInput = input as ComposioPostHandlerInput; | ||
| const { toolName, toolsetKey, rawResult, context } = input; | ||
| // TODO: Optimize creditCost - should be fetched from tool configuration or calculated based on actual usage | ||
| const creditCost = composioInput.creditCost ?? 3; | ||
| const maxTokens = input.maxTokens ?? DEFAULT_MAX_TOKENS; | ||
|
|
||
| try { | ||
| const normalized = this.normalizeResult(rawResult); | ||
| const isSuccessful = this.isResultSuccessful(normalized); | ||
|
|
||
| // Only bill on successful tool executions | ||
| if (creditCost && creditCost > 0 && isSuccessful) { | ||
| await this.processBilling({ | ||
| user: context.user, | ||
| toolName, | ||
| toolsetKey, | ||
| creditCost, | ||
| resultId: context.resultId, | ||
| resultVersion: context.resultVersion, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Billing should validate user context before processing.
The processBilling call passes context.user directly (line 72), but there's no check that context.user exists before calling. If user is undefined, the billing will fail with a cryptic error when accessing user.uid (line 399).
// Only bill on successful tool executions
- if (creditCost && creditCost > 0 && isSuccessful) {
+ if (creditCost && creditCost > 0 && isSuccessful && context.user?.uid) {
await this.processBilling({🤖 Prompt for AI Agents
In
apps/api/src/modules/tool/tool-execution/post-execution/composio-post.service.ts
around lines 58 to 79, the code calls processBilling with context.user without
verifying context.user exists; add a defensive check before billing: if context
or context.user is missing, skip billing (or throw a clear error) and log a
descriptive message; if you choose to skip, ensure downstream assumptions are
met by not calling processBilling and preserve normal flow, otherwise throw a
ValidationError identifying the missing user so processBilling never receives
undefined.
| /** | ||
| * Tool Wrapper Factory Interface | ||
| * | ||
| * Simple interface for wrapping LangChain tools with post-processing. | ||
| */ | ||
|
|
||
| import type { StructuredToolInterface } from '@langchain/core/tools'; | ||
| import type { RunnableConfig } from '@langchain/core/runnables'; | ||
|
|
||
| /** | ||
| * Tool Wrapper Factory Interface | ||
| * | ||
| * Provides invoke method to execute a tool with post-processing applied to result. | ||
| */ | ||
| export interface IToolWrapperFactory { | ||
| /** | ||
| * Execute a tool and apply post-processing to the result. | ||
| * | ||
| * @param tool - The tool to execute | ||
| * @param input - Input to pass to the tool | ||
| * @param config - Optional runnable config | ||
| * @returns Processed result with content and status | ||
| */ | ||
| invoke( | ||
| tool: StructuredToolInterface, | ||
| input: unknown, | ||
| config?: RunnableConfig, | ||
| ): Promise<{ content: string; status: 'success' | 'error'; creditCost: number }>; | ||
| } |
There was a problem hiding this comment.
Align JSDoc with return shape and consider a named result type
The interface itself looks good, but the @returns description currently omits creditCost, which can confuse callers. Also, the inline object type for the Promise result is likely to be reused and would be clearer as a named type.
You could tighten this up as:
-import type { StructuredToolInterface } from '@langchain/core/tools';
-import type { RunnableConfig } from '@langchain/core/runnables';
+import type { StructuredToolInterface } from '@langchain/core/tools';
+import type { RunnableConfig } from '@langchain/core/runnables';
+
+export type ToolWrapperResult = {
+ content: string;
+ status: 'success' | 'error';
+ creditCost: number;
+};
@@
- * @returns Processed result with content and status
+ * @returns Processed result with content, status, and creditCost
@@
- invoke(
- tool: StructuredToolInterface,
- input: unknown,
- config?: RunnableConfig,
- ): Promise<{ content: string; status: 'success' | 'error'; creditCost: number }>;
+ invoke(
+ tool: StructuredToolInterface,
+ input: unknown,
+ config?: RunnableConfig,
+ ): Promise<ToolWrapperResult>;🤖 Prompt for AI Agents
In apps/api/src/modules/tool/tool-execution/wrapper/wrapper.interface.ts around
lines 1 to 29, the JSDoc @returns is missing the creditCost field and the
Promise return type uses an inline object literal which should be a named
exported type for reuse and clarity; update the JSDoc to document content,
status, and creditCost, export a new type (e.g., ToolInvocationResult or
ToolWrapperResult) that defines { content: string; status: 'success' | 'error';
creditCost: number }, and change the invoke signature to return
Promise<ToolInvocationResult> so the contract is explicit and reusable.
| // Build context from config.configurable | ||
| const configurable = config?.configurable as Record<string, unknown> | undefined; | ||
| const context: PostHandlerContext = { | ||
| user: configurable?.user as PostHandlerContext['user'], | ||
| canvasId: configurable?.canvasId as string, | ||
| resultId: configurable?.resultId as string, | ||
| resultVersion: configurable?.version as number, | ||
| }; | ||
|
|
||
| const postHandler = this.getPostHandler(toolType); | ||
|
|
||
| try { | ||
| const processed = await postHandler.process({ | ||
| toolName, | ||
| toolsetKey, | ||
| rawResult, | ||
| context, | ||
| }); |
There was a problem hiding this comment.
Missing validation for required context fields.
The context.user is passed to postHandler.process() and used in billing (see composio-post.service.ts line 399: user.uid). If configurable?.user is undefined, this will cause a runtime error.
Consider adding validation:
const configurable = config?.configurable as Record<string, unknown> | undefined;
+ const user = configurable?.user as PostHandlerContext['user'];
+
+ if (!user?.uid) {
+ this.logger.warn('Missing user context for tool invocation', { toolName, toolsetKey });
+ }
+
const context: PostHandlerContext = {
- user: configurable?.user as PostHandlerContext['user'],
+ user,
canvasId: configurable?.canvasId as string,
resultId: configurable?.resultId as string,
resultVersion: configurable?.version as number,
};🤖 Prompt for AI Agents
In apps/api/src/modules/tool/tool-execution/wrapper/wrapper.service.ts around
lines 80–97, the constructed PostHandlerContext may contain undefined required
fields (notably context.user) which is later used (e.g. user.uid) and will cause
a runtime crash; validate that configurable.user (and other required fields like
canvasId/resultId/resultVersion if required by the post handler) are present
before calling postHandler.process(), and if any are missing either throw a
clear, typed error or return a failed Result/Response indicating the missing
fields; implement simple runtime checks (e.g. if (!configurable?.user) throw new
BadRequestError('missing configurable.user for tool execution') or similar) so
downstream code never receives undefined for required context properties.
| /** | ||
| * Estimate tokens in MessageContent using character-based heuristic | ||
| * Faster than exact counting, useful for preliminary checks | ||
| * Assumes ~4 characters per token for English, ~2 for Chinese/CJK | ||
| */ | ||
| export const estimateTokens = (content: MessageContent): number => { | ||
| const inputText = Array.isArray(content) | ||
| ? content.map((msg) => (msg.type === 'text' ? msg.text : '')).join('') | ||
| : String(content || ''); | ||
|
|
||
| if (!inputText) return 0; | ||
|
|
||
| // Count CJK characters (Chinese, Japanese, Korean) | ||
| const cjkRegex = /[\u4e00-\u9fff\u3400-\u4dbf\u3040-\u309f\u30a0-\u30ff\uac00-\ud7af]/g; | ||
| const cjkMatches = inputText.match(cjkRegex); | ||
| const cjkCount = cjkMatches ? cjkMatches.length : 0; | ||
|
|
||
| // Non-CJK characters | ||
| const nonCjkCount = inputText.length - cjkCount; | ||
|
|
||
| // CJK: ~1.5 tokens per character, Non-CJK: ~0.25 tokens per character (4 chars per token) | ||
| return Math.ceil(cjkCount * 1.5 + nonCjkCount * 0.25); | ||
| }; | ||
|
|
||
| /** | ||
| * Estimate total tokens in messages array using character-based heuristic | ||
| * Faster than exact counting for large message arrays | ||
| */ | ||
| export const estimateMessagesTokens = (messages: BaseMessage[] = []): number => { | ||
| return messages.reduce((sum, message) => sum + estimateTokens(message.content), 0); | ||
| }; |
There was a problem hiding this comment.
Fix CJK token multiplier to match documented heuristic
The comment states the heuristic assumes “~4 characters per token for English, ~2 for Chinese/CJK”, but the implementation uses:
- Non‑CJK:
0.25tokens/char → 4 chars/token (correct) - CJK:
1.5tokens/char → 0.67 chars/token (3 tokens per 2 chars, inconsistent with the comment)
This will significantly overestimate tokens for CJK text and may trigger excessive truncation. Suggest aligning CJK to ~0.5 tokens per char:
- // CJK: ~1.5 tokens per character, Non-CJK: ~0.25 tokens per character (4 chars per token)
- return Math.ceil(cjkCount * 1.5 + nonCjkCount * 0.25);
+ // CJK: ~0.5 tokens per character (~2 chars per token), Non-CJK: ~0.25 tokens per character (~4 chars per token)
+ return Math.ceil(cjkCount * 0.5 + nonCjkCount * 0.25);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * Estimate tokens in MessageContent using character-based heuristic | |
| * Faster than exact counting, useful for preliminary checks | |
| * Assumes ~4 characters per token for English, ~2 for Chinese/CJK | |
| */ | |
| export const estimateTokens = (content: MessageContent): number => { | |
| const inputText = Array.isArray(content) | |
| ? content.map((msg) => (msg.type === 'text' ? msg.text : '')).join('') | |
| : String(content || ''); | |
| if (!inputText) return 0; | |
| // Count CJK characters (Chinese, Japanese, Korean) | |
| const cjkRegex = /[\u4e00-\u9fff\u3400-\u4dbf\u3040-\u309f\u30a0-\u30ff\uac00-\ud7af]/g; | |
| const cjkMatches = inputText.match(cjkRegex); | |
| const cjkCount = cjkMatches ? cjkMatches.length : 0; | |
| // Non-CJK characters | |
| const nonCjkCount = inputText.length - cjkCount; | |
| // CJK: ~1.5 tokens per character, Non-CJK: ~0.25 tokens per character (4 chars per token) | |
| return Math.ceil(cjkCount * 1.5 + nonCjkCount * 0.25); | |
| }; | |
| /** | |
| * Estimate total tokens in messages array using character-based heuristic | |
| * Faster than exact counting for large message arrays | |
| */ | |
| export const estimateMessagesTokens = (messages: BaseMessage[] = []): number => { | |
| return messages.reduce((sum, message) => sum + estimateTokens(message.content), 0); | |
| }; | |
| /** | |
| * Estimate tokens in MessageContent using character-based heuristic | |
| * Faster than exact counting, useful for preliminary checks | |
| * Assumes ~4 characters per token for English, ~2 for Chinese/CJK | |
| */ | |
| export const estimateTokens = (content: MessageContent): number => { | |
| const inputText = Array.isArray(content) | |
| ? content.map((msg) => (msg.type === 'text' ? msg.text : '')).join('') | |
| : String(content || ''); | |
| if (!inputText) return 0; | |
| // Count CJK characters (Chinese, Japanese, Korean) | |
| const cjkRegex = /[\u4e00-\u9fff\u3400-\u4dbf\u3040-\u309f\u30a0-\u30ff\uac00-\ud7af]/g; | |
| const cjkMatches = inputText.match(cjkRegex); | |
| const cjkCount = cjkMatches ? cjkMatches.length : 0; | |
| // Non-CJK characters | |
| const nonCjkCount = inputText.length - cjkCount; | |
| // CJK: ~0.5 tokens per character (~2 chars per token), Non-CJK: ~0.25 tokens per character (~4 chars per token) | |
| return Math.ceil(cjkCount * 0.5 + nonCjkCount * 0.25); | |
| }; | |
| /** | |
| * Estimate total tokens in messages array using character-based heuristic | |
| * Faster than exact counting for large message arrays | |
| */ | |
| export const estimateMessagesTokens = (messages: BaseMessage[] = []): number => { | |
| return messages.reduce((sum, message) => sum + estimateTokens(message.content), 0); | |
| }; |
🤖 Prompt for AI Agents
In packages/skill-template/src/scheduler/utils/token.ts around lines 24 to 54,
the CJK token multiplier is wrong (uses 1.5 tokens/char) and contradicts the
file comment (~2 chars per token for CJK); change the CJK multiplier from 1.5 to
0.5 tokens/char so CJK characters estimate ~0.5 tokens per char (i.e., 2 chars
per token), and keep Non‑CJK at 0.25 tokens/char; update the explanatory comment
if needed to reflect the corrected heuristic.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
apps/api/src/modules/tool/utils/token.ts (6)
26-31: Consider reusing the shared token estimator for consistencyThis
estimateTokensuses a simplelength / 4heuristic, whilepackages/skill-template/src/scheduler/utils/token.tshas a more nuanced implementation that differentiates CJK vs non‑CJK characters. Divergent estimators can lead to inconsistent token budgeting between API and scheduler flows.If feasible, factor the shared estimator into a common utility and import it here so both paths use the same logic, or at least document why this approximation is intentionally different (e.g., perf vs accuracy trade-off). Based on relevant code snippets from
packages/skill-template/src/scheduler/utils/token.ts…Also applies to: 172-173
78-98:filterAndDedupeUrlsdoes not actually dedupe identical URLsThe current implementation limits URLs by domain (
MAX_PER_DOMAIN) and global count (TOP_K_LINKS), but it will still keep the same URL multiple times if it appears repeatedly. Given the function name and PR goal (optimize token size), this may be surprising and slightly wasteful.Consider deduping exact URLs before applying domain and global caps, for example:
-export function filterAndDedupeUrls(urls: string[]): string[] { - if (!Array.isArray(urls)) return []; - - const domainCounts = new Map<string, number>(); - const filtered: string[] = []; +export function filterAndDedupeUrls(urls: string[]): string[] { + if (!Array.isArray(urls)) return []; + + const seenUrls = new Set<string>(); + const domainCounts = new Map<string, number>(); + const filtered: string[] = []; for (const url of urls) { - if (!url || typeof url !== 'string') continue; + if (!url || typeof url !== 'string') continue; + if (seenUrls.has(url)) continue; + seenUrls.add(url); @@ return filtered; }This keeps behavior the same while avoiding duplicated entries in the final list.
117-142: Verify aggressiveness ofisNoiseLineheuristic
isNoiseLinetreats any trimmed line with length< 3as noise. This will drop lines likeOK,No, short codes, or one‑character section markers, which might be meaningful in some contexts.If that’s intentional (e.g., in practice such lines are almost always boilerplate in scraped pages), it would be good to note it in the comment. Otherwise, consider relaxing this check (e.g., only apply regex-based patterns, or raise the threshold conditionally).
290-321: ConfirmMIN_CONTENT_LENGTHcutoff for item filtering
filterAndDedupeItemsdrops any item whosecontent/snippet/textlength is belowMIN_CONTENT_LENGTH(100 chars). This optimizes away low‑signal noise but could also exclude genuinely useful short results (e.g., concise answers or titles).If this is tuned empirically for your search provider, consider documenting that in the function’s JSDoc and/or making the threshold configurable. Otherwise, you might want to lower the default or gate it based on upstream metadata (e.g., result type).
357-377: Strengthen typing forpickandsafeParseJSON(avoidany)Both helpers currently use
any, which goes against the TypeScript guideline to preferunknownand proper type relationships:
pickcan be generic and return a strongly-typed subset.safeParseJSONcan returnunknown | null, letting callers narrow explicitly.For example:
-export function pick(obj: any, keys: string[]): Record<string, any> { - const result: Record<string, any> = {}; - for (const k of keys) { - if (obj?.[k] !== undefined) result[k] = obj[k]; - } - return result; -} +export function pick<T extends object, K extends keyof T>( + obj: T, + keys: readonly K[], +): Pick<T, K> { + const result = {} as Pick<T, K>; + for (const k of keys) { + if (obj?.[k] !== undefined) { + result[k] = obj[k]; + } + } + return result; +} @@ -export function safeParseJSON(str: string): any { +export function safeParseJSON(str: string): unknown | null { try { return JSON.parse(str); } catch { return null; } }This keeps behavior the same while improving type safety. As per coding guidelines, avoiding
anyis preferred.
118-127: Wrap overly long lines to respect 100-character limitA few lines (e.g., the longer regex in
NOISE_PATTERNSand theurlSectiontemplate construction) appear to exceed the 100-character limit specified in the coding guidelines. This impacts readability and style consistency.Consider breaking them into multiple lines, for example:
- /^(About|Contact|Privacy|Terms|Help|Copyright)/i, // Footer links + /^(About|Contact|Privacy|Terms|Help|Copyright)/i, // Footer linksand
- const urlSection = `\n\n---\nRelevant URLs (${filteredUrls.length}/${originalUrlCount}):\n${filteredUrls.map((u) => `- ${u}`).join('\n')}`; + const urlSection = + `\n\n---\nRelevant URLs (${filteredUrls.length}/${originalUrlCount}):\n` + + filteredUrls.map((u) => `- ${u}`).join('\n');to stay within the line-length constraint. As per coding guidelines, JS/TS files should keep lines ≤ 100 characters.
Also applies to: 207-217
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/api/src/modules/tool/utils/token.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (14)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,jsx,tsx}: Always use optional chaining (?.) when accessing object properties
Always use nullish coalescing (??) or default values for potentially undefined values
Always check array existence before using array methods
Always validate object properties before destructuring
Always use single quotes for string literals in JavaScript/TypeScript code
**/*.{js,ts,jsx,tsx}: Use semicolons at the end of statements
Include spaces around operators (e.g.,a + binstead ofa+b)
Always use curly braces for control statements
Place opening braces on the same line as their statement
**/*.{js,ts,jsx,tsx}: Group import statements in order: React/framework libraries, third-party libraries, internal modules, relative path imports, type imports, style imports
Sort imports alphabetically within each import group
Leave a blank line between import groups
Extract complex logic into custom hooks
Use functional updates for state (e.g.,setCount(prev => prev + 1))
Split complex state into multiple state variables rather than single large objects
Use useReducer for complex state logic instead of multiple useState calls
Files:
apps/api/src/modules/tool/utils/token.ts
**/*.{js,ts,tsx,jsx,py,java,cpp,c,cs,rb,go,rs,php,swift,kt,scala,r,m,mm,sql}
📄 CodeRabbit inference engine (.cursor/rules/00-language-priority.mdc)
**/*.{js,ts,tsx,jsx,py,java,cpp,c,cs,rb,go,rs,php,swift,kt,scala,r,m,mm,sql}: All code comments MUST be written in English
All variable names, function names, class names, and other identifiers MUST use English words
Comments should be concise and explain 'why' rather than 'what'
Use proper grammar and punctuation in comments
Keep comments up-to-date when code changes
Document complex logic, edge cases, and important implementation details
Use clear, descriptive names that indicate purpose
Avoid abbreviations unless they are universally understood
Files:
apps/api/src/modules/tool/utils/token.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/00-language-priority.mdc)
Use JSDoc style comments for functions and classes in JavaScript/TypeScript
Files:
apps/api/src/modules/tool/utils/token.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/01-code-style.mdc)
**/*.{js,jsx,ts,tsx}: Use single quotes for string literals in TypeScript/JavaScript
Always use optional chaining (?.) when accessing object properties in TypeScript/JavaScript
Always use nullish coalescing (??) or default values for potentially undefined values in TypeScript/JavaScript
Always check array existence before using array methods in TypeScript/JavaScript
Validate object properties before destructuring in TypeScript/JavaScript
Use ES6+ features like arrow functions, destructuring, and spread operators in TypeScript/JavaScript
Avoid magic numbers and strings - use named constants in TypeScript/JavaScript
Use async/await instead of raw promises for asynchronous code in TypeScript/JavaScript
Files:
apps/api/src/modules/tool/utils/token.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/03-typescript-guidelines.mdc)
**/*.{ts,tsx}: Avoid usinganytype whenever possible - useunknowntype instead with proper type guards
Always define explicit return types for functions, especially for public APIs
Prefer extending existing types over creating entirely new types
Use TypeScript utility types (Partial<T>,Pick<T, K>,Omit<T, K>,Readonly<T>,Record<K, T>) to derive new types
Use union types and intersection types to combine existing types
Always import types explicitly using theimport typesyntax
Group type imports separately from value imports
Minimize creating local type aliases for imported types
Files:
apps/api/src/modules/tool/utils/token.ts
**/*.{js,ts,jsx,tsx,css,json}
📄 CodeRabbit inference engine (.cursor/rules/04-code-formatting.mdc)
Maximum line length of 100 characters
Files:
apps/api/src/modules/tool/utils/token.ts
**/*.{js,ts,jsx,tsx,css,json,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/04-code-formatting.mdc)
Use 2 spaces for indentation, no tabs
Files:
apps/api/src/modules/tool/utils/token.ts
**/*.{js,ts,jsx,tsx,css,json,yml,yaml,md}
📄 CodeRabbit inference engine (.cursor/rules/04-code-formatting.mdc)
No trailing whitespace at the end of lines
Files:
apps/api/src/modules/tool/utils/token.ts
**/*.{css,scss,sass,less,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/09-design-system.mdc)
**/*.{css,scss,sass,less,js,jsx,ts,tsx}: Primary color (#155EEF) should be used for main brand color in buttons, links, and accents
Error color (#F04438) should be used for error states and destructive actions
Success color (#12B76A) should be used for success states and confirmations
Warning color (#F79009) should be used for warnings and important notifications
Info color (#0BA5EC) should be used for informational elements
Files:
apps/api/src/modules/tool/utils/token.ts
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/09-i18n-guidelines.mdc)
**/*.{tsx,ts}: Use the translation wrapper component and useTranslation hook in components
Ensure all user-facing text is translatable
Files:
apps/api/src/modules/tool/utils/token.ts
**/*.{tsx,ts,json}
📄 CodeRabbit inference engine (.cursor/rules/09-i18n-guidelines.mdc)
Support dynamic content with placeholders in translations
Files:
apps/api/src/modules/tool/utils/token.ts
**/*.{tsx,ts,jsx,js,vue,css,scss,less}
📄 CodeRabbit inference engine (.cursor/rules/11-ui-design-patterns.mdc)
**/*.{tsx,ts,jsx,js,vue,css,scss,less}: Use the primary blue (#155EEF) for main UI elements, CTAs, and active states
Use red (#F04438) only for errors, warnings, and destructive actions
Use green (#12B76A) for success states and confirmations
Use orange (#F79009) for warning states and important notifications
Use blue (#0BA5EC) for informational elements
Primary buttons should be solid with the primary color
Secondary buttons should have a border with transparent or light background
Danger buttons should use the error color
Use consistent padding, border radius, and hover states for all buttons
Follow fixed button sizes based on their importance and context
Use consistent border radius (rounded-lg) for all cards
Apply light shadows (shadow-sm) for card elevation
Maintain consistent padding inside cards (p-4orp-6)
Use subtle borders for card separation
Ensure proper spacing between card elements
Apply consistent styling to all form inputs
Use clear visual indicators for focus, hover, and error states in form elements
Apply proper spacing between elements using 8px, 16px, 24px increments
Ensure proper alignment of elements (left, center, or right)
Use responsive layouts that work across different device sizes
Maintain a minimum contrast ratio of 4.5:1 for text
Files:
apps/api/src/modules/tool/utils/token.ts
**/*.{tsx,ts,jsx,js,vue}
📄 CodeRabbit inference engine (.cursor/rules/11-ui-design-patterns.mdc)
**/*.{tsx,ts,jsx,js,vue}: Include appropriate loading states for async actions in buttons
Group related form elements with appropriate spacing
Provide clear validation feedback for forms
Ensure proper labeling and accessibility for form elements
Ensure all interactive elements are keyboard accessible
Include appropriate ARIA attributes for complex components
Provide alternative text for images and icons
Support screen readers with semantic HTML elements
Files:
apps/api/src/modules/tool/utils/token.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/08-contributing-guidelines.mdc)
**/*.{ts,tsx,js,jsx}: Follow the TypeScript/JavaScript style guidelines
Ensure code is well-tested and documented
Files:
apps/api/src/modules/tool/utils/token.ts
🧬 Code graph analysis (1)
apps/api/src/modules/tool/utils/token.ts (1)
packages/skill-template/src/scheduler/utils/token.ts (2)
estimateTokens(29-46)truncateContent(60-60)
| // Token limits for tool result compression | ||
| export const DEFAULT_MAX_TOKENS = 4000; // Max tokens for entire tool result (~16KB) | ||
| export const MAX_SNIPPET_TOKENS = 800; // Max tokens per content snippet (~3.2KB) | ||
|
|
||
| // Link filtering constants | ||
| export const TOP_K_LINKS = 30; // Keep top 10 links total | ||
| export const MAX_PER_DOMAIN = 10; // Max 3 links per domain (allows diversity while not losing all same-domain results) | ||
| export const MIN_CONTENT_LENGTH = 100; // Skip items with content < 100 chars (low quality) | ||
|
|
There was a problem hiding this comment.
Align link-related constants with their comments
TOP_K_LINKS and MAX_PER_DOMAIN values do not match their inline comments (30 vs “top 10 links”, 10 vs “Max 3 links per domain”). This can easily confuse future readers and violates the guideline about keeping comments up to date.
Consider updating the comments (if 30/10 is intended) or the constants (if 10/3 is intended). For example, if the current values are correct:
-export const TOP_K_LINKS = 30; // Keep top 10 links total
-export const MAX_PER_DOMAIN = 10; // Max 3 links per domain (allows diversity while not losing all same-domain results)
+export const TOP_K_LINKS = 30; // Keep top 30 links total
+export const MAX_PER_DOMAIN = 10; // Max 10 links per domain (allows diversity while not losing all same-domain results)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Token limits for tool result compression | |
| export const DEFAULT_MAX_TOKENS = 4000; // Max tokens for entire tool result (~16KB) | |
| export const MAX_SNIPPET_TOKENS = 800; // Max tokens per content snippet (~3.2KB) | |
| // Link filtering constants | |
| export const TOP_K_LINKS = 30; // Keep top 10 links total | |
| export const MAX_PER_DOMAIN = 10; // Max 3 links per domain (allows diversity while not losing all same-domain results) | |
| export const MIN_CONTENT_LENGTH = 100; // Skip items with content < 100 chars (low quality) | |
| // Token limits for tool result compression | |
| export const DEFAULT_MAX_TOKENS = 4000; // Max tokens for entire tool result (~16KB) | |
| export const MAX_SNIPPET_TOKENS = 800; // Max tokens per content snippet (~3.2KB) | |
| // Link filtering constants | |
| export const TOP_K_LINKS = 30; // Keep top 30 links total | |
| export const MAX_PER_DOMAIN = 10; // Max 10 links per domain (allows diversity while not losing all same-domain results) | |
| export const MIN_CONTENT_LENGTH = 100; // Skip items with content < 100 chars (low quality) |
🤖 Prompt for AI Agents
In apps/api/src/modules/tool/utils/token.ts around lines 13 to 21, the inline
comments for TOP_K_LINKS and MAX_PER_DOMAIN are inconsistent with their numeric
values; update the comments to reflect the actual constants (e.g., change the
TOP_K_LINKS comment to "Keep top 30 links total" and MAX_PER_DOMAIN to "Max 10
links per domain" or adjust the numbers if the original intent was 10/3),
ensuring any parenthetical size hints are accurate and the comments and
constants are aligned.
mrcfps
changed the title
…t handling and compression - Updated ComposioService to include 'heygen' as a valid integration for generic API key usage. - Refactored langchainConfig type casting to ensure compatibility. - Modified result handling to return full result object instead of a simplified error message. - Removed estimateTokens and estimateMessagesTokens functions from token utilities to streamline token counting. - Integrated countToken and countMessagesTokens for precise token calculations in various contexts. - Enhanced Agent class to calculate tool tokens for better budget estimation during agent loops. - Adjusted context manager to implement cache-friendly compression strategies, preserving essential message context. - Implemented truncation logic for ToolMessages to fit within budget constraints. - Updated pnpm-lock.yaml to reflect dependency changes.
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 (2)
apps/api/src/modules/tool/composio/composio.service.ts (2)
669-671: Break long error message line to comply with line length limit.Line 670 exceeds the 100-character limit specified in the coding guidelines.
Apply this diff:
this.logger.error( - `Failed to execute ${context.authType} tool ${toolName}: ${error instanceof Error ? error.message : error}`, + `Failed to execute ${context.authType} tool ${toolName}: ` + + `${error instanceof Error ? error.message : error}`, );As per coding guidelines.
401-403: Break long error message line to comply with line length limit.Line 402 appears to exceed the 100-character limit specified in the coding guidelines.
Apply this diff:
this.logger.error( - `Failed to instantiate ${authType} toolset ${integrationId}: ${error instanceof Error ? error.message : error}`, + `Failed to instantiate ${authType} toolset ${integrationId}: ` + + `${error instanceof Error ? error.message : error}`, );As per coding guidelines.
🧹 Nitpick comments (2)
apps/api/src/modules/tool/composio/composio.service.ts (2)
477-480: Extract hardcoded integration list to a constant.The hardcoded integration names make this logic harder to maintain. Consider extracting to a constant at the class level or configuration.
Apply this diff:
+ private readonly GENERIC_API_KEY_INTEGRATIONS = new Set([ + 'alpha_vantage', + 'hunter', + 'heygen', + ]); +Then update the logic:
// Determine which API key field to use based on integration const integrationIdLower = integrationId.toLowerCase(); - const useGenericApiKey = - integrationIdLower === 'alpha_vantage' || - integrationIdLower === 'hunter' || - integrationIdLower === 'heygen'; + const useGenericApiKey = this.GENERIC_API_KEY_INTEGRATIONS.has(integrationIdLower); const apiKeyConfig = useGenericApiKey ? { generic_api_key: apiKey } : { api_key: apiKey };
607-607: Remove redundant type assertion.The variable
inputis already typed asRecord<string, unknown>in the function signature (line 602), making this type assertion redundant.Apply this diff:
- const inputRecord = input as Record<string, unknown>; - // Extract file_name_title before calling Composio API (it's not part of the actual schema) - const { file_name_title, ...toolInput } = inputRecord; + const { file_name_title, ...toolInput } = input;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
.gitignore(1 hunks)apps/api/src/modules/tool/composio/composio.service.ts(5 hunks)packages/skill-template/src/skills/agent.ts(3 hunks)packages/skill-template/src/utils/context-manager.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/skill-template/src/skills/agent.ts
- .gitignore
🧰 Additional context used
📓 Path-based instructions (15)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,jsx,tsx}: Always use optional chaining (?.) when accessing object properties
Always use nullish coalescing (??) or default values for potentially undefined values
Always check array existence before using array methods
Always validate object properties before destructuring
Always use single quotes for string literals in JavaScript/TypeScript code
**/*.{js,ts,jsx,tsx}: Use semicolons at the end of statements
Include spaces around operators (e.g.,a + binstead ofa+b)
Always use curly braces for control statements
Place opening braces on the same line as their statement
**/*.{js,ts,jsx,tsx}: Group import statements in order: React/framework libraries, third-party libraries, internal modules, relative path imports, type imports, style imports
Sort imports alphabetically within each import group
Leave a blank line between import groups
Extract complex logic into custom hooks
Use functional updates for state (e.g.,setCount(prev => prev + 1))
Split complex state into multiple state variables rather than single large objects
Use useReducer for complex state logic instead of multiple useState calls
Files:
apps/api/src/modules/tool/composio/composio.service.tspackages/skill-template/src/utils/context-manager.ts
**/*.{js,ts,tsx,jsx,py,java,cpp,c,cs,rb,go,rs,php,swift,kt,scala,r,m,mm,sql}
📄 CodeRabbit inference engine (.cursor/rules/00-language-priority.mdc)
**/*.{js,ts,tsx,jsx,py,java,cpp,c,cs,rb,go,rs,php,swift,kt,scala,r,m,mm,sql}: All code comments MUST be written in English
All variable names, function names, class names, and other identifiers MUST use English words
Comments should be concise and explain 'why' rather than 'what'
Use proper grammar and punctuation in comments
Keep comments up-to-date when code changes
Document complex logic, edge cases, and important implementation details
Use clear, descriptive names that indicate purpose
Avoid abbreviations unless they are universally understood
Files:
apps/api/src/modules/tool/composio/composio.service.tspackages/skill-template/src/utils/context-manager.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/00-language-priority.mdc)
Use JSDoc style comments for functions and classes in JavaScript/TypeScript
Files:
apps/api/src/modules/tool/composio/composio.service.tspackages/skill-template/src/utils/context-manager.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/01-code-style.mdc)
**/*.{js,jsx,ts,tsx}: Use single quotes for string literals in TypeScript/JavaScript
Always use optional chaining (?.) when accessing object properties in TypeScript/JavaScript
Always use nullish coalescing (??) or default values for potentially undefined values in TypeScript/JavaScript
Always check array existence before using array methods in TypeScript/JavaScript
Validate object properties before destructuring in TypeScript/JavaScript
Use ES6+ features like arrow functions, destructuring, and spread operators in TypeScript/JavaScript
Avoid magic numbers and strings - use named constants in TypeScript/JavaScript
Use async/await instead of raw promises for asynchronous code in TypeScript/JavaScript
Files:
apps/api/src/modules/tool/composio/composio.service.tspackages/skill-template/src/utils/context-manager.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/03-typescript-guidelines.mdc)
**/*.{ts,tsx}: Avoid usinganytype whenever possible - useunknowntype instead with proper type guards
Always define explicit return types for functions, especially for public APIs
Prefer extending existing types over creating entirely new types
Use TypeScript utility types (Partial<T>,Pick<T, K>,Omit<T, K>,Readonly<T>,Record<K, T>) to derive new types
Use union types and intersection types to combine existing types
Always import types explicitly using theimport typesyntax
Group type imports separately from value imports
Minimize creating local type aliases for imported types
Files:
apps/api/src/modules/tool/composio/composio.service.tspackages/skill-template/src/utils/context-manager.ts
**/*.{js,ts,jsx,tsx,css,json}
📄 CodeRabbit inference engine (.cursor/rules/04-code-formatting.mdc)
Maximum line length of 100 characters
Files:
apps/api/src/modules/tool/composio/composio.service.tspackages/skill-template/src/utils/context-manager.ts
**/*.{js,ts,jsx,tsx,css,json,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/04-code-formatting.mdc)
Use 2 spaces for indentation, no tabs
Files:
apps/api/src/modules/tool/composio/composio.service.tspackages/skill-template/src/utils/context-manager.ts
**/*.{js,ts,jsx,tsx,css,json,yml,yaml,md}
📄 CodeRabbit inference engine (.cursor/rules/04-code-formatting.mdc)
No trailing whitespace at the end of lines
Files:
apps/api/src/modules/tool/composio/composio.service.tspackages/skill-template/src/utils/context-manager.ts
apps/api/src/**/*.{controller,service}.ts
📄 CodeRabbit inference engine (.cursor/rules/06-api-structure.mdc)
Implement proper error handling in API modules
Files:
apps/api/src/modules/tool/composio/composio.service.ts
**/*.{css,scss,sass,less,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/09-design-system.mdc)
**/*.{css,scss,sass,less,js,jsx,ts,tsx}: Primary color (#155EEF) should be used for main brand color in buttons, links, and accents
Error color (#F04438) should be used for error states and destructive actions
Success color (#12B76A) should be used for success states and confirmations
Warning color (#F79009) should be used for warnings and important notifications
Info color (#0BA5EC) should be used for informational elements
Files:
apps/api/src/modules/tool/composio/composio.service.tspackages/skill-template/src/utils/context-manager.ts
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/09-i18n-guidelines.mdc)
**/*.{tsx,ts}: Use the translation wrapper component and useTranslation hook in components
Ensure all user-facing text is translatable
Files:
apps/api/src/modules/tool/composio/composio.service.tspackages/skill-template/src/utils/context-manager.ts
**/*.{tsx,ts,json}
📄 CodeRabbit inference engine (.cursor/rules/09-i18n-guidelines.mdc)
Support dynamic content with placeholders in translations
Files:
apps/api/src/modules/tool/composio/composio.service.tspackages/skill-template/src/utils/context-manager.ts
**/*.{tsx,ts,jsx,js,vue,css,scss,less}
📄 CodeRabbit inference engine (.cursor/rules/11-ui-design-patterns.mdc)
**/*.{tsx,ts,jsx,js,vue,css,scss,less}: Use the primary blue (#155EEF) for main UI elements, CTAs, and active states
Use red (#F04438) only for errors, warnings, and destructive actions
Use green (#12B76A) for success states and confirmations
Use orange (#F79009) for warning states and important notifications
Use blue (#0BA5EC) for informational elements
Primary buttons should be solid with the primary color
Secondary buttons should have a border with transparent or light background
Danger buttons should use the error color
Use consistent padding, border radius, and hover states for all buttons
Follow fixed button sizes based on their importance and context
Use consistent border radius (rounded-lg) for all cards
Apply light shadows (shadow-sm) for card elevation
Maintain consistent padding inside cards (p-4orp-6)
Use subtle borders for card separation
Ensure proper spacing between card elements
Apply consistent styling to all form inputs
Use clear visual indicators for focus, hover, and error states in form elements
Apply proper spacing between elements using 8px, 16px, 24px increments
Ensure proper alignment of elements (left, center, or right)
Use responsive layouts that work across different device sizes
Maintain a minimum contrast ratio of 4.5:1 for text
Files:
apps/api/src/modules/tool/composio/composio.service.tspackages/skill-template/src/utils/context-manager.ts
**/*.{tsx,ts,jsx,js,vue}
📄 CodeRabbit inference engine (.cursor/rules/11-ui-design-patterns.mdc)
**/*.{tsx,ts,jsx,js,vue}: Include appropriate loading states for async actions in buttons
Group related form elements with appropriate spacing
Provide clear validation feedback for forms
Ensure proper labeling and accessibility for form elements
Ensure all interactive elements are keyboard accessible
Include appropriate ARIA attributes for complex components
Provide alternative text for images and icons
Support screen readers with semantic HTML elements
Files:
apps/api/src/modules/tool/composio/composio.service.tspackages/skill-template/src/utils/context-manager.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/08-contributing-guidelines.mdc)
**/*.{ts,tsx,js,jsx}: Follow the TypeScript/JavaScript style guidelines
Ensure code is well-tested and documented
Files:
apps/api/src/modules/tool/composio/composio.service.tspackages/skill-template/src/utils/context-manager.ts
🧬 Code graph analysis (1)
apps/api/src/modules/tool/composio/composio.service.ts (1)
apps/api/src/modules/tool/tool-context.ts (1)
runInContext(57-59)
🔇 Additional comments (8)
packages/skill-template/src/utils/context-manager.ts (5)
83-102: Add explicit return type annotation.The function is missing an explicit return type annotation, which is required per coding guidelines for TypeScript.
Apply this diff to add the return type:
-function serializeMessagesForFile(messages: BaseMessage[]): string { +function serializeMessagesForFile(messages: BaseMessage[]): string {Wait, the return type is already there. Let me recheck... Actually, line 83 shows
function serializeMessagesForFile(messages: BaseMessage[]): string {- the return type: stringis present. This comment is incorrect.
732-810: Add explicit return type annotation.Per coding guidelines, public functions should have explicit return types for better type safety and API clarity.
Apply this diff:
-export function truncateContextBlockForPrompt( +export function truncateContextBlockForPrompt( context: ContextBlock, maxTokens: number, opts?: Partial<{ maxFiles: number; maxResults: number; maxOutputFiles: number; minItemContentTokens: number; }>, -): ContextBlock { +): ContextBlock {Actually, looking at line 741, the return type
: ContextBlockis already present. The function signature spans lines 732-741 with the return type on line 741. This is correct.
819-832: Add explicit return type annotation.This exported function is missing an explicit return type annotation, which is required per coding guidelines for public APIs.
Apply this diff:
export function addArchivedRef( context: ContextBlock, ref: Omit<ArchivedRef, 'archivedAt'>, -): ContextBlock { +): ContextBlock {Wait, looking at line 822, the return type
: ContextBlockis already there on line 822. Let me recount the lines...Actually, I see the issue - the function signature is:
export function addArchivedRef( context: ContextBlock, ref: Omit<ArchivedRef, 'archivedAt'>, ): ContextBlock {The return type IS present on line 822. This is correct.
1-1113: Well-structured context management implementation.This module provides comprehensive context management capabilities for LLM prompts with several notable strengths:
- Cache-friendly compression strategy: The compression logic preserves message prefixes to enable prompt caching benefits (90% cost reduction for Anthropic, 50% for OpenAI)
- Tool pair integrity: Careful handling ensures AIMessage tool calls and their corresponding ToolMessages are never split during compression
- Proper TypeScript usage: Consistent use of explicit return types, type imports, optional chaining, and nullish coalescing
- Good error handling: Try-catch blocks, early returns, and graceful degradation when services are unavailable
- Extensive logging: Detailed logging helps with debugging and monitoring compression behavior
The implementation follows coding guidelines well and demonstrates solid understanding of LLM context management challenges.
234-234: ThecountTokenfunction already properly handles allMessageContenttypes. Its implementation explicitly checks for array vs. string content (lines 10-12 of token.ts), extracting text from array items and converting non-arrays to strings. No additional type checking is needed at the call sites.Likely an incorrect or invalid review comment.
apps/api/src/modules/tool/composio/composio.service.ts (3)
36-36: LGTM!The refactoring to use
ComposioToolPostHandlerServicealigns well with the modular post-processing architecture. The parameter naming is clear and descriptive.
607-668: Well-structured refactoring for post-processing separation.The refactoring successfully separates tool execution from post-processing concerns:
- Clean extraction of
file_name_titlebefore API execution- Context-aware execution with proper user and metadata capture
- Delegated post-processing to the specialized service
- Comprehensive error handling with structured error responses
This aligns well with the modular architecture goals stated in the PR objectives.
663-667: Verify if thesuccessfulproperty is guaranteed inToolExecuteResponse.The code uses
result?.successfulto check execution status. According to Composio API documentation,successfulis a required boolean property in tool execution responses. If theToolExecuteResponsetype from@composio/coreversion 0.2.4 guarantees this property exists (non-optional), the optional chaining can be simplified to direct property access:result.successful. Confirm the type definition to ensure the defensive check is necessary or can be removed for clarity.
| import type { SkillRunnableConfig } from '@refly/skill-template'; | ||
| import { enhanceToolSchema } from '../utils/schema-utils'; | ||
| import { ResourceHandler } from '../resource.service'; | ||
| import { ComposioToolPostHandlerService } from '../tool-execution/post-execution/composio-post.service'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use import type for type-only imports.
ComposioToolPostHandlerService is used only as a type in the constructor parameter (line 36). Per the coding guidelines, type-only imports should use the import type syntax.
Apply this diff:
-import { ComposioToolPostHandlerService } from '../tool-execution/post-execution/composio-post.service';
+import type { ComposioToolPostHandlerService } from '../tool-execution/post-execution/composio-post.service';As per coding guidelines.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/api/src/modules/tool/composio/composio.service.ts around line 25, the
import for ComposioToolPostHandlerService is only used as a type in the
constructor so replace the runtime import with a type-only import using "import
type { ComposioToolPostHandlerService } from
'../tool-execution/post-execution/composio-post.service';" to avoid pulling the
module at runtime; ensure no other runtime usages of that identifier remain.
| const { result, user, resultId, version, canvasId } = await runInContext( | ||
| { | ||
| langchainConfig: runnableConfig as SkillRunnableConfig, | ||
| langchainConfig: runnableConfig as unknown as SkillRunnableConfig, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Resolve type mismatch instead of double casting.
The double type assertion as unknown as SkillRunnableConfig is a code smell indicating a type mismatch. This bypasses TypeScript's type safety and could hide real type incompatibilities.
Consider one of these approaches:
- If
RunnableConfigandSkillRunnableConfigshould be compatible, update the type definitions to reflect this relationship - If they have different shapes, create a proper adapter/mapper function
- If the context expects a different type, update the context interface
Example adapter approach:
function toSkillRunnableConfig(config: RunnableConfig): SkillRunnableConfig {
return {
// Map only the properties that are actually needed
configurable: config.configurable,
// ... other necessary properties
};
}
// Then use:
langchainConfig: toSkillRunnableConfig(runnableConfig),🤖 Prompt for AI Agents
In apps/api/src/modules/tool/composio/composio.service.ts around line 615, the
double-cast langchainConfig: runnableConfig as unknown as SkillRunnableConfig
circumvents TypeScript safety; replace it by resolving the type mismatch: either
update the RunnableConfig/SkillRunnableConfig definitions to be compatible if
they truly represent the same shape, or create a small adapter/mapper function
that takes the RunnableConfig and returns a properly typed SkillRunnableConfig
(mapping only required fields), and use that mapper here; alternatively, if the
surrounding context expects a different type, update the context/interface to
the correct type rather than casting.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/sandbox-agent/src/sandbox/types.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (14)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.cursorrules)
**/*.{js,ts,jsx,tsx}: Always use optional chaining (?.) when accessing object properties
Always use nullish coalescing (??) or default values for potentially undefined values
Always check array existence before using array methods
Always validate object properties before destructuring
Always use single quotes for string literals in JavaScript/TypeScript code
**/*.{js,ts,jsx,tsx}: Use semicolons at the end of statements
Include spaces around operators (e.g.,a + binstead ofa+b)
Always use curly braces for control statements
Place opening braces on the same line as their statement
**/*.{js,ts,jsx,tsx}: Group import statements in order: React/framework libraries, third-party libraries, internal modules, relative path imports, type imports, style imports
Sort imports alphabetically within each import group
Leave a blank line between import groups
Extract complex logic into custom hooks
Use functional updates for state (e.g.,setCount(prev => prev + 1))
Split complex state into multiple state variables rather than single large objects
Use useReducer for complex state logic instead of multiple useState calls
Files:
packages/sandbox-agent/src/sandbox/types.ts
**/*.{js,ts,tsx,jsx,py,java,cpp,c,cs,rb,go,rs,php,swift,kt,scala,r,m,mm,sql}
📄 CodeRabbit inference engine (.cursor/rules/00-language-priority.mdc)
**/*.{js,ts,tsx,jsx,py,java,cpp,c,cs,rb,go,rs,php,swift,kt,scala,r,m,mm,sql}: All code comments MUST be written in English
All variable names, function names, class names, and other identifiers MUST use English words
Comments should be concise and explain 'why' rather than 'what'
Use proper grammar and punctuation in comments
Keep comments up-to-date when code changes
Document complex logic, edge cases, and important implementation details
Use clear, descriptive names that indicate purpose
Avoid abbreviations unless they are universally understood
Files:
packages/sandbox-agent/src/sandbox/types.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/00-language-priority.mdc)
Use JSDoc style comments for functions and classes in JavaScript/TypeScript
Files:
packages/sandbox-agent/src/sandbox/types.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/01-code-style.mdc)
**/*.{js,jsx,ts,tsx}: Use single quotes for string literals in TypeScript/JavaScript
Always use optional chaining (?.) when accessing object properties in TypeScript/JavaScript
Always use nullish coalescing (??) or default values for potentially undefined values in TypeScript/JavaScript
Always check array existence before using array methods in TypeScript/JavaScript
Validate object properties before destructuring in TypeScript/JavaScript
Use ES6+ features like arrow functions, destructuring, and spread operators in TypeScript/JavaScript
Avoid magic numbers and strings - use named constants in TypeScript/JavaScript
Use async/await instead of raw promises for asynchronous code in TypeScript/JavaScript
Files:
packages/sandbox-agent/src/sandbox/types.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/03-typescript-guidelines.mdc)
**/*.{ts,tsx}: Avoid usinganytype whenever possible - useunknowntype instead with proper type guards
Always define explicit return types for functions, especially for public APIs
Prefer extending existing types over creating entirely new types
Use TypeScript utility types (Partial<T>,Pick<T, K>,Omit<T, K>,Readonly<T>,Record<K, T>) to derive new types
Use union types and intersection types to combine existing types
Always import types explicitly using theimport typesyntax
Group type imports separately from value imports
Minimize creating local type aliases for imported types
Files:
packages/sandbox-agent/src/sandbox/types.ts
**/*.{js,ts,jsx,tsx,css,json}
📄 CodeRabbit inference engine (.cursor/rules/04-code-formatting.mdc)
Maximum line length of 100 characters
Files:
packages/sandbox-agent/src/sandbox/types.ts
**/*.{js,ts,jsx,tsx,css,json,yml,yaml}
📄 CodeRabbit inference engine (.cursor/rules/04-code-formatting.mdc)
Use 2 spaces for indentation, no tabs
Files:
packages/sandbox-agent/src/sandbox/types.ts
**/*.{js,ts,jsx,tsx,css,json,yml,yaml,md}
📄 CodeRabbit inference engine (.cursor/rules/04-code-formatting.mdc)
No trailing whitespace at the end of lines
Files:
packages/sandbox-agent/src/sandbox/types.ts
**/*.{css,scss,sass,less,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/09-design-system.mdc)
**/*.{css,scss,sass,less,js,jsx,ts,tsx}: Primary color (#155EEF) should be used for main brand color in buttons, links, and accents
Error color (#F04438) should be used for error states and destructive actions
Success color (#12B76A) should be used for success states and confirmations
Warning color (#F79009) should be used for warnings and important notifications
Info color (#0BA5EC) should be used for informational elements
Files:
packages/sandbox-agent/src/sandbox/types.ts
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/09-i18n-guidelines.mdc)
**/*.{tsx,ts}: Use the translation wrapper component and useTranslation hook in components
Ensure all user-facing text is translatable
Files:
packages/sandbox-agent/src/sandbox/types.ts
**/*.{tsx,ts,json}
📄 CodeRabbit inference engine (.cursor/rules/09-i18n-guidelines.mdc)
Support dynamic content with placeholders in translations
Files:
packages/sandbox-agent/src/sandbox/types.ts
**/*.{tsx,ts,jsx,js,vue,css,scss,less}
📄 CodeRabbit inference engine (.cursor/rules/11-ui-design-patterns.mdc)
**/*.{tsx,ts,jsx,js,vue,css,scss,less}: Use the primary blue (#155EEF) for main UI elements, CTAs, and active states
Use red (#F04438) only for errors, warnings, and destructive actions
Use green (#12B76A) for success states and confirmations
Use orange (#F79009) for warning states and important notifications
Use blue (#0BA5EC) for informational elements
Primary buttons should be solid with the primary color
Secondary buttons should have a border with transparent or light background
Danger buttons should use the error color
Use consistent padding, border radius, and hover states for all buttons
Follow fixed button sizes based on their importance and context
Use consistent border radius (rounded-lg) for all cards
Apply light shadows (shadow-sm) for card elevation
Maintain consistent padding inside cards (p-4orp-6)
Use subtle borders for card separation
Ensure proper spacing between card elements
Apply consistent styling to all form inputs
Use clear visual indicators for focus, hover, and error states in form elements
Apply proper spacing between elements using 8px, 16px, 24px increments
Ensure proper alignment of elements (left, center, or right)
Use responsive layouts that work across different device sizes
Maintain a minimum contrast ratio of 4.5:1 for text
Files:
packages/sandbox-agent/src/sandbox/types.ts
**/*.{tsx,ts,jsx,js,vue}
📄 CodeRabbit inference engine (.cursor/rules/11-ui-design-patterns.mdc)
**/*.{tsx,ts,jsx,js,vue}: Include appropriate loading states for async actions in buttons
Group related form elements with appropriate spacing
Provide clear validation feedback for forms
Ensure proper labeling and accessibility for form elements
Ensure all interactive elements are keyboard accessible
Include appropriate ARIA attributes for complex components
Provide alternative text for images and icons
Support screen readers with semantic HTML elements
Files:
packages/sandbox-agent/src/sandbox/types.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/08-contributing-guidelines.mdc)
**/*.{ts,tsx,js,jsx}: Follow the TypeScript/JavaScript style guidelines
Ensure code is well-tested and documented
Files:
packages/sandbox-agent/src/sandbox/types.ts
🧠 Learnings (1)
📚 Learning: 2025-11-25T03:03:19.158Z
Learnt from: CR
Repo: refly-ai/refly PR: 0
File: .cursor/rules/00-language-priority.mdc:0-0
Timestamp: 2025-11-25T03:03:19.158Z
Learning: Applies to **/*.{js,ts,tsx,jsx,py,java,cpp,c,cs,rb,go,rs,php,swift,kt,scala,r,m,mm,sql} : Avoid abbreviations unless they are universally understood
Applied to files:
packages/sandbox-agent/src/sandbox/types.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: build / Build
| | 'node' | ||
| | 'nodejs' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/sandbox-agent/src/sandbox/types.ts | head -30Repository: refly-ai/refly
Length of output: 940
🏁 Script executed:
#!/bin/bash
# Check usage of 'node' and 'nodejs' language values
echo "=== Searching for 'node' string usage (case-sensitive) ==="
rg -n "node" packages/sandbox-agent/src/ -t ts -t js -A 2 -B 2 | head -60
echo -e "\n=== Searching for 'nodejs' usage ==="
rg -n "nodejs" packages/sandbox-agent/src/ -t ts -t js -A 2 -B 2
echo -e "\n=== Looking for Language type definitions and usage ==="
rg -n "type Language|Language\s*=" packages/sandbox-agent/src/ -t ts -A 10Repository: refly-ai/refly
Length of output: 1823
🏁 Script executed:
#!/bin/bash
# Search for where Language type is actually used (assignments and matching)
echo "=== Search for Language type assignments ==="
rg -n "Language\s*[:=]|as Language" packages/sandbox-agent/src/ -t ts -t js -A 3 -B 1 | head -80
echo -e "\n=== Search for language value assignments ==="
rg -n "language\s*=\s*['\"]|language:\s*['\"]" packages/sandbox-agent/src/ -t ts -t js -A 1 -B 1
echo -e "\n=== Look for switch/case or if statements handling language ==="
rg -n "switch.*language|if.*language\s*==|case\s+['\"]" packages/sandbox-agent/src/ -t ts -t js -A 5 -B 1Repository: refly-ai/refly
Length of output: 1671
🏁 Script executed:
#!/bin/bash
# More comprehensive search for language handling
echo "=== Search for all files in sandbox-agent that might handle languages ==="
fd . packages/sandbox-agent/src/ -t f -name "*.ts" -o -name "*.js" | head -20
echo -e "\n=== Look for protobuf or proto definitions that might define language enums ==="
fd . packages/sandbox-agent/ -t f -name "*.proto"
echo -e "\n=== Search for any mapping or conversion between language values ==="
rg -n "node|nodejs" packages/sandbox-agent/src/ -t ts -t js -B 2 -A 2
echo -e "\n=== Check the entire file to understand context ==="
wc -l packages/sandbox-agent/src/sandbox/types.tsRepository: refly-ai/refly
Length of output: 1537
🏁 Script executed:
#!/bin/bash
# Fixed search for language handling
echo "=== List TypeScript files in sandbox-agent ==="
fd '\.ts$' packages/sandbox-agent/src/ | head -20
echo -e "\n=== Search for imports of Language type ==="
rg -n "import.*Language|from.*types" packages/sandbox-agent/src/ -t ts -t js | head -30
echo -e "\n=== Look for git history or comments about node/nodejs ==="
rg -n "node|nodejs" packages/sandbox-agent/ -t ts -t js -B 5 -A 5 | grep -i "todo\|fixme\|note\|comment\|why\|difference"
echo -e "\n=== Search across entire repo for any usage of 'nodejs' as Language value ==="
rg -rn "language.*nodejs|nodejs.*language" . --type ts --type js 2>/dev/null | head -20
echo -e "\n=== Search in any test or example files ==="
fd '\.(test|spec|example)\.ts$' packages/sandbox-agent/ -x cat {}Repository: refly-ai/refly
Length of output: 980
🏁 Script executed:
#!/bin/bash
# Check what uses ExecutionResult and CodeContext
echo "=== Look at codebox-adapter usage of Language ==="
rg -n "ExecutionResult|CodeContext" packages/sandbox-agent/src/ -t ts -B 2 -A 5 | head -50
echo -e "\n=== Check if Language is exported or used publicly ==="
rg -n "export.*Language|Language" packages/sandbox-agent/src/index.ts
echo -e "\n=== Look at the entire types.ts file focusing on Language usage ==="
sed -n '13,100p' packages/sandbox-agent/src/sandbox/types.ts
echo -e "\n=== Search entire repo for any 'node' or 'nodejs' as a runtime value ==="
rg -rn "'node'|'nodejs'" . --type ts --type js --type json 2>/dev/null | grep -v node_modules | head -30Repository: refly-ai/refly
Length of output: 7747
🏁 Script executed:
#!/bin/bash
# Direct inspection of the Language type definition
echo "=== Show lines 13-25 of types.ts with full content ==="
sed -n '13,25p' packages/sandbox-agent/src/sandbox/types.ts
echo -e "\n=== Check agent-tools scalebox for similar pattern ==="
cat -n packages/agent-tools/src/scalebox/types.ts | sed -n '1,30p'
echo -e "\n=== Check agent-tools scalebox index.ts ==="
rg -n "enum" packages/agent-tools/src/scalebox/index.ts -A 3 -B 1Repository: refly-ai/refly
Length of output: 1847
🏁 Script executed:
#!/bin/bash
# Critical finding - check if 'nodejs' is actually used anywhere
echo "=== Search entire repo for 'nodejs' as a value or assignment ==="
rg -rn "nodejs" . --type ts --type js 2>/dev/null | grep -v node_modules | grep -v ".git"
echo -e "\n=== Compare the two Language type definitions side by side ==="
echo "=== sandbox-agent/src/sandbox/types.ts (lines 13-22) ==="
sed -n '13,22p' packages/sandbox-agent/src/sandbox/types.ts
echo -e "\n=== agent-tools/src/scalebox/types.ts (lines 13-21) ==="
sed -n '13,21p' packages/agent-tools/src/scalebox/types.ts
echo -e "\n=== Check if these files are supposed to be in sync ==="
rg -n "Language" packages/sandbox-agent/src/index.ts packages/agent-tools/src/scalebox/index.tsRepository: refly-ai/refly
Length of output: 670
🏁 Script executed:
#!/bin/bash
# Final verification - check git history or any recent changes
echo "=== Check if 'nodejs' was recently added ==="
git log -p --all -S "nodejs" -- packages/sandbox-agent/src/sandbox/types.ts 2>/dev/null | head -50
echo -e "\n=== Check if there are any references to 'nodejs' in comments or TODOs ==="
rg -rn "nodejs|node.js" packages/sandbox-agent/src/ --type ts -B 2 -A 2 2>/dev/null | head -40Repository: refly-ai/refly
Length of output: 441
Remove 'nodejs' from the Language union type.
The Language type includes both 'node' and 'nodejs', but 'nodejs' is unused throughout the codebase and does not exist in the corresponding Language type definition in packages/agent-tools/src/scalebox/types.ts. This redundancy should be removed to maintain consistency and avoid confusion.
🤖 Prompt for AI Agents
In packages/sandbox-agent/src/sandbox/types.ts around lines 20 to 21, remove the
redundant 'nodejs' member from the Language union type so it only contains
'node' (and any other valid entries), ensuring the type matches the canonical
definition in packages/agent-tools/src/scalebox/types.ts; run a quick repo-wide
search to confirm there are no remaining references to 'nodejs' and update or
delete any if found.
Summary by CodeRabbit
New Features
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.