[REF-1010] feat: implement patch_workflow_tool for efficient workflow plan editing#1978
[REF-1010] feat: implement patch_workflow_tool for efficient workflow plan editing#1978
Conversation
…eration - Added RedisService for locking mechanism to prevent concurrent updates on workflow plans. - Introduced genWorkflowPlanID function for consistent ID generation. - Updated patching logic to utilize the new locking mechanism and improved error handling. - Modified response structure in GenerateWorkflow class to return planId and version only.
…okerService - Introduced copilotSessionId property in SkillRunnableConfig interface for enhanced configuration options. - Updated SkillInvokerService to conditionally assign copilotSessionId from data if available, improving flexibility in session management.
…kflow plan handling - Introduced getLatestWorkflowPlan method in ReflyService interface to retrieve the latest workflow plan based on copilotSessionId. - Updated copilot index logic to handle cases where planId is not provided, fetching the latest plan if necessary. - Modified workflowPlanPatchSchema to make planId optional, allowing for dynamic selection of the latest plan within a session.
- Revised the description of the PatchWorkflow class to clarify the purpose and available operations for modifying workflow plans. - Streamlined operation examples and added notes on usage, emphasizing the distinction between `generate_workflow` and `patch_workflow` tools. - Improved clarity on optional fields and the order of operations in the workflow plan patching process.
- Consolidated WorkflowPlanData and WorkflowPlan into a single WorkflowPlan type for clarity. - Introduced WorkflowPlanRecord to encapsulate additional properties like planId and version. - Updated various services and interfaces to utilize the new types, ensuring consistency across the codebase. - Enhanced error handling and validation for workflow tasks and variables during patch operations.
- Updated WorkflowPlan model to include version and uid fields for better tracking. - Refactored service methods to accept user and params objects, improving parameter handling and consistency. - Enhanced getWorkflowPlanDetail method to utilize user context and versioning for fetching plans. - Improved generateWorkflowPlan and patchWorkflowPlan methods to streamline parameter passing and validation. - Updated controller to include a new endpoint for retrieving workflow plan details, ensuring proper user authentication and query handling.
…eval - Introduced GetWorkflowSummary class to retrieve the current workflow plan structure, including tasks and variables. - Updated ToolService to integrate getWorkflowSummary, allowing for better context management during workflow modifications. - Enhanced documentation to guide users on when to utilize getWorkflowSummary for recalling task/variable IDs and understanding workflow structure.
…d components - Introduced useUpdateCanvasTitle hook for managing canvas title updates, including auto-naming and manual renaming. - Refactored CopilotMessage, CanvasRenameModal, and CanvasTitle components to utilize the new hook, improving code maintainability and performance. - Removed redundant remote title update logic from components, centralizing it within the new hook. - Enhanced title update handling in CanvasRenameModal and CanvasTitle to ensure proper state management and user experience.
…ayout - Added CopilotSummaryRenderer for displaying workflow summary status in a compact format. - Updated ToolCall component to conditionally render CopilotSummaryRenderer based on tool name. - Refactored ListFilesRenderer and ReadFileRenderer to include durationText and improve styling consistency. - Adjusted result message component layout for better spacing and visual clarity. - Enhanced i18n support by adding translations for copilot summary messages.
📝 WalkthroughWalkthroughAdds a versioned WorkflowPlan Prisma model, new WorkflowPlanService with Redis locking and controller endpoint, copilot tools for generate/patch/summary, ReflyService extensions, client APIs/hooks/types, UI components/hooks for rendering and title management, and workflow patching/validation primitives. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant Service as WorkflowPlanService
participant DB as PrismaDB
participant Redis
rect rgb(220,235,255)
Note over Client,API: Generate workflow plan
Client->>API: reflyService.generateWorkflowPlan(data, copilotSessionId, resultId, resultVersion)
API->>Service: generateWorkflowPlan(user, params)
Service->>DB: query latest by copilot_session_id
DB-->>Service: latest record
Service->>Service: compute new version & planId
Service->>DB: create new WorkflowPlan record
DB-->>Service: created record
Service-->>API: WorkflowPlanRecord (planId, version)
API-->>Client: planId & version
end
rect rgb(220,255,220)
Note over Client,Redis: Patch workflow plan (concurrency-safe)
Client->>API: reflyService.patchWorkflowPlan(planId, operations, resultId, resultVersion)
API->>Service: patchWorkflowPlan(user, params)
Service->>Redis: lockPlan(planId, options)
Redis-->>Service: lock acquired
Service->>DB: fetch current plan by planId/latest version
DB-->>Service: current plan JSON
Service->>Service: applyWorkflowPatchOperations(currentPlan, operations)
Service->>DB: insert new version with patch metadata
Service->>Redis: release lock
Service-->>API: patched WorkflowPlanRecord
API-->>Client: planId, version, summary
end
rect rgb(255,240,220)
Note over Client,Service: Get latest / summary
Client->>API: reflyService.getLatestWorkflowPlan(copilotSessionId)
API->>Service: getLatestWorkflowPlan(user, { copilotSessionId })
Service->>DB: query latest by copilot_session_id
DB-->>Service: WorkflowPlanRecord or null
Service-->>API: WorkflowPlanRecord/null
API-->>Client: plan summary or null
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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 |
mrcfps
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/openapi-schema/src/schemas.gen.ts (1)
11027-11071: Fix mismatch betweenrequireddescription and schema default in WorkflowVariable
WorkflowVariable.requirednow says “Defaults to false.” but the schema does not actually specify adefault. That can mislead consumers and generated clients.Consider either adding
default: falseor adjusting the description so they are aligned; adding the default keeps the improved description and is minimally invasive.Proposed schema tweak to add the default
required: { type: 'boolean', - description: 'Whether the variable is required. Defaults to false.', + description: 'Whether the variable is required. Defaults to false.', + default: false, },(Optionally, if your public API docs are intended to be English‑only, you may also want to replace the Chinese example string for
descriptionwith an English example for consistency, but that is cosmetic.)
🧹 Nitpick comments (18)
packages/ai-workspace-common/src/components/result-message/index.tsx (1)
92-92: Consider using spacing that aligns with the 8px increment guideline.The change from
my-2tomy-1reduces the vertical margin to 4px, which doesn't follow the guideline of using 8px, 16px, or 24px increments for spacing. Consider keepingmy-2or adjusting to an alternative spacing value that aligns with the increment guideline if tighter spacing is needed.As per coding guidelines: "Apply proper spacing between elements using 8px, 16px, 24px increments"
🔎 Suggested adjustment
- className="my-1" + className="my-2"packages/canvas-common/src/workflow-plan.test.ts (3)
20-30: Approve title addition; improve parameter types.The addition of the
titleproperty aligns well with the new WorkflowPlan schema. However, theanytype usage for parameters violates the TypeScript coding guidelines.🔎 Suggested type improvements
const createWorkflowPlan = ( - tasks: any[], + tasks: Array<ReturnType<typeof createTask>>, products: Array<{ id: string; type: any; title: string }> = [], - variables: any[] = [], + variables: Array<Record<string, unknown>> = [], ) => ({ title: 'Test Workflow Plan', tasks, products, variables, });As per coding guidelines, avoid
anytype whenever possible.
77-92: Remove unnecessary type assertion.The explicit
as any[]cast at line 79 violates TypeScript coding guidelines. The type can be properly inferred or explicitly typed.🔎 Suggested fix
const taskNode = result.nodes[0]; expect(taskNode.data?.metadata?.selectedToolsets).toHaveLength(2); - const selectedToolsets = taskNode.data?.metadata?.selectedToolsets as any[]; + const selectedToolsets = taskNode.data?.metadata?.selectedToolsets; expect(selectedToolsets[0]).toEqual({ type: 'regular', id: 'toolset1',If type safety is needed, use a proper type guard instead of
any.As per coding guidelines, avoid using
anytype.
113-117: Remove unnecessary type assertion.Another instance of
as any[]that should be avoided per TypeScript coding guidelines.🔎 Suggested fix
const taskNode = result.nodes[0]; expect(taskNode.data?.metadata?.selectedToolsets).toHaveLength(1); - const selectedToolsets = taskNode.data?.metadata?.selectedToolsets as any[]; + const selectedToolsets = taskNode.data?.metadata?.selectedToolsets; expect(selectedToolsets[0].id).toBe('valid');As per coding guidelines, avoid using
anytype.packages/ai-workspace-common/src/hooks/canvas/use-update-canvas-title.ts (1)
12-23: Consider logging errors for debugging.The
updateRemoteCanvasTitlefunction silently returnsundefinedon error. While this is fine for control flow, consider logging the error for debugging purposes.🔎 Optional: Add error logging
async function updateRemoteCanvasTitle(canvasId: string, newTitle: string) { const { data, error } = await getClient().updateCanvas({ body: { canvasId, title: newTitle, }, }); if (error || !data?.success) { + console.error('Failed to update canvas title:', error); return; } return data.data?.title; }packages/ai-workspace-common/src/components/canvas/top-toolbar/canvas-title.tsx (1)
122-135: MissingsetEditedTitlein dependency array.While
setEditedTitlefromuseStateis technically stable, ESLint'sexhaustive-depsrule typically flags this. Consider adding it for consistency and to satisfy linting.🔎 Optional: Add setEditedTitle to dependencies
[handleSubmit, canvasTitle, setMode], + [handleSubmit, canvasTitle, setMode, setEditedTitle],apps/api/src/modules/skill/skill-invoker.service.ts (1)
250-252: Make copilotSessionId presence check explicit rather than truthyUsing a truthy check means an empty string would be ignored. If you ever allow non-null/empty values, consider checking for undefined/null instead.
Suggested tweak
- if (data.copilotSessionId) { - config.configurable.copilotSessionId = data.copilotSessionId; - } + if (data.copilotSessionId != null) { + config.configurable.copilotSessionId = data.copilotSessionId; + }apps/api/src/modules/workflow/workflow.module.ts (1)
15-39: Consider reusing WorkflowPlanModule instead of providing WorkflowPlanService directlyYou now provide
WorkflowPlanServicehere whileWorkflowPlanModulealso provides/exports it. That yields separate provider instances instead of a single shared one and slightly duplicates module wiring.If you want a single source of truth, consider importing
WorkflowPlanModulehere and droppingWorkflowPlanServicefrom thisprovidersarray.packages/agent-tools/src/builtin/index.ts (1)
202-224: New workflow plan ToolsetDefinitions look consistentThe
BuiltinGenerateWorkflowPlanDefinitionandBuiltinPatchWorkflowPlanDefinitionshapes match existing simple toolset definitions (key + label/description), so they should integrate cleanly with the existing tool inventory.If these are meant to be system-only or highlighted differently, you might optionally add
builtin,internal, ordomainmetadata for consistency with other definitions, but it is not required.apps/api/src/modules/workflow/workflow.controller.ts (1)
1-10: New workflow plan detail endpoint is correctly integratedThe
WorkflowPlanServiceinjection and/v1/workflow/plan/detailhandler cleanly delegate togetWorkflowPlanDetailand return aGetWorkflowPlanDetailResponseviabuildSuccessResponse.ParseIntPipe({ optional: true })onversionmatches the intended optional query semantics.For consistency with the other detail endpoints (
executionId/canvasIdchecks), you may optionally add an explicitplanIdpresence check here and throwParamsErrorearly, instead of relying solely on the service to validate it.Also applies to: 21-22, 26-26, 33-35, 127-139
packages/ai-workspace-common/src/components/markdown/plugins/tool-call/render.tsx (1)
3-3: Copilot tool rendering is sound but could be slightly more robustThe dedicated copilot branch,
CopilotWorkflowPlanintegration, and passingdurationTextthrough toInternalToolRenderer/CopilotSummaryRendererall look good and keep the UI aligned with the new workflow‑plan flow.Two small robustness tweaks to consider:
- For
generate_workflow/patch_workflow, you currently parse onlyprops['data-tool-result']. If a message is rendered with just adata-tool-call-idand no inlined result, the component will show the “generating/invalidData” state even thoughuseGetToolCallResultmay have the final payload. ReusingresultContentor falling back tofetchedData.data.result.outputhere would make this path behave like other tools.safeParseJSON(resultStr)?.data as WorkflowPlanis a fairly loose cast; if the stored payload is only{ planId, version }, you might want to narrow the type to that minimal shape (or use a runtime guard) rather than claiming it is a fullWorkflowPlan.Also applies to: 21-21, 235-265, 316-323
apps/api/prisma/schema.prisma (1)
334-364: WorkflowPlan model fits the feature; consider strengthening the primary identifierThe
WorkflowPlanmodel fields and indexes align with the service behavior (versioned plans perplanId, tied to results and copilot sessions).Given that each logical plan/version pair should be unique, you might consider upgrading
@@index([planId, version])to@@unique([planId, version])to guard against accidental duplicate rows for the same plan version and make lookups more robust in the face of bugs or missed locking.packages/openapi-schema/schema.yml (1)
11995-12068: TightenWorkflowPlanRecordtyping and clarifyWorkflowTask.toolsetssemanticsTwo small schema refinements to consider:
In
WorkflowPlanRecord,planIdandversionare effectively mandatory for persisted records but are not declared inrequired. Marking them as required would give client generators stricter, more accurate types.In
WorkflowTask,toolsetsisstring[]with description “Toolset ID”. Elsewhere the API usesGenericToolsetfor toolset structures. If these are specificallytoolsetIds (and not keys), calling that out explicitly in the description (or renaming totoolsetIdsin a future revision) would make the contract clearer for clients.These are non-blocking and can be deferred, but they would improve type-safety and clarity.
packages/openapi-schema/src/schemas.gen.ts (1)
10587-10642: WorkflowPlan/WorkflowPlanRecord schemas look good; consider tighteningversiontypeThe plan/record layering via
allOfmatches existing record-style schemas and should work well with clients.Optionally, if plan versions are always whole-number increments, you might change
versionfromtype: 'number'totype: 'integer'to avoid accidental non‑integer values and get stricter client typings.packages/ai-workspace-common/src/components/markdown/plugins/tool-call/copilot-workflow-plan.tsx (1)
172-180: Query may fail ifplanIdis undefined.The query is enabled when
isEmptyis true, butdata.planIdmight be undefined based on theWorkflowPlanRecordtype. Consider adding a check to ensure the query only runs whenplanIdis available.🔎 Proposed fix
- const isEmpty = isPlanDataEmpty(data); + const isEmpty = isPlanDataEmpty(data); + const canFetch = isEmpty && !!data.planId; const { data: workflowPlanData, isLoading } = useGetWorkflowPlanDetail( { query: { planId: data.planId, version: data.version }, }, undefined, { - enabled: isEmpty, + enabled: canFetch, }, );apps/api/src/modules/workflow/workflow-plan.service.ts (1)
163-168: Error handling could fail with non-Error objects.Accessing
error?.messageassumeserroris an Error object. If a non-Error value is thrown, this could cause issues.🔎 Proposed fix
try { currentData = safeParseJSON(currentPlan.data); } catch (error) { - this.logger.error(`Failed to parse workflow plan data: ${error?.message ?? error}`); + this.logger.error(`Failed to parse workflow plan data: ${error instanceof Error ? error.message : String(error)}`); currentData = { title: '', tasks: [], variables: [] }; }packages/ai-workspace-common/src/components/canvas/copilot/copilot-message.tsx (2)
128-144: Missing error handling for API call.If
getWorkflowPlanDetailfails, the error is silently ignored andfinalPlanremains unchanged (potentially with undefinedtasks). Consider handling the error case to provide user feedback.🔎 Proposed fix
if (workflowPlan.tasks === undefined) { setLoading(true); try { - const { data } = await getClient().getWorkflowPlanDetail({ + const { data, error } = await getClient().getWorkflowPlanDetail({ query: { planId: workflowPlan.planId, version: workflowPlan.version, }, }); + if (error) { + console.error('Failed to fetch workflow plan details:', error); + return; + } if (data?.data) { finalPlan = data?.data; } } finally { setLoading(false); } }
177-194:logEventin dependency array is unnecessary.
logEventis a stable import that won't change between renders. Including it in the dependency array is harmless but adds noise. You could wrap it withuseCallbackor simply exclude it if your linter allows.
...ages/ai-workspace-common/src/components/markdown/plugins/tool-call/copilot-workflow-plan.tsx
Show resolved
Hide resolved
- Merged import statements for WorkflowVariable and WorkflowPlan from '@refly/openapi-schema' to streamline code and improve readability.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/api/src/modules/copilot-autogen/copilot-autogen.dto.ts
🧰 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/copilot-autogen/copilot-autogen.dto.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/copilot-autogen/copilot-autogen.dto.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/copilot-autogen/copilot-autogen.dto.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/copilot-autogen/copilot-autogen.dto.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/copilot-autogen/copilot-autogen.dto.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/copilot-autogen/copilot-autogen.dto.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/copilot-autogen/copilot-autogen.dto.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/copilot-autogen/copilot-autogen.dto.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/copilot-autogen/copilot-autogen.dto.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/copilot-autogen/copilot-autogen.dto.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/copilot-autogen/copilot-autogen.dto.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/copilot-autogen/copilot-autogen.dto.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/copilot-autogen/copilot-autogen.dto.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/copilot-autogen/copilot-autogen.dto.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
…nents - Changed import statement in copilot-autogen.dto.ts to use type imports for better clarity. - Updated loading message in CopilotWorkflowPlan component to utilize i18n translation for improved localization support. - Removed outdated preset scenarios and context menu translations from i18n files to streamline the translation structure.
- Added validation for copilotSessionId, resultId, and resultVersion to ensure required session context is present before generating and patching workflow plans. - Utilized optional chaining and nullish coalescing for safer property access and default values. - Improved error messages for better clarity on missing session context during workflow operations.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/agent-tools/src/copilot/index.ts (1)
221-240:input.planIdis accepted but never used.The schema declares
planIdas an optional parameter, but the implementation always callsgetLatestWorkflowPlanwithcopilotSessionId, ignoring any providedplanId. If a user provides a specificplanId, they would expect that plan to be retrieved, not the latest one.🔎 Proposed fix
async _call( - _input: z.infer<typeof this.schema>, + input: z.infer<typeof this.schema>, _: unknown, config: RunnableConfig, ): Promise<ToolCallResult> { try { const { reflyService, user } = this.params; - const copilotSessionId = config.configurable?.copilotSessionId; - - if (!copilotSessionId) { + + let plan; + + if (input.planId) { + // If planId is provided, fetch that specific plan + // Note: May need a getWorkflowPlanById method in ReflyService + plan = await reflyService.getWorkflowPlanById(user, { planId: input.planId }); + } else { + const copilotSessionId = config.configurable?.copilotSessionId; + if (!copilotSessionId) { + return { + status: 'error', + data: { error: 'copilotSessionId is required when planId is not provided' }, + summary: 'Missing session context', + }; + } + plan = await reflyService.getLatestWorkflowPlan(user, { copilotSessionId }); + } + + if (!plan) { return { - status: 'error', - data: { error: 'copilotSessionId is required to retrieve the workflow plan' }, - summary: 'Missing session context', + status: 'success', + data: { exists: false }, + summary: 'No workflow plan found.', }; } - - const plan = await reflyService.getLatestWorkflowPlan(user, { - copilotSessionId, - });Note: This assumes
reflyService.getWorkflowPlanByIdexists. If not, you'll need to add it to theReflyServiceinterface and implement it, or removeplanIdfrom the schema if single-plan retrieval isn't supported.
🧹 Nitpick comments (5)
packages/agent-tools/src/copilot/index.ts (5)
11-12: Useimport typesyntax for type-only imports.
UserandRunnableConfigappear to be type definitions and should be imported using theimport typesyntax to clearly distinguish type imports from value imports.🔎 Proposed refactor
-import { ReflyService } from '../builtin/interface'; -import { User } from '@refly/openapi-schema'; -import { RunnableConfig } from '@langchain/core/runnables'; +import { ReflyService } from '../builtin/interface'; +import type { User } from '@refly/openapi-schema'; +import type { RunnableConfig } from '@langchain/core/runnables';As per coding guidelines, TypeScript type imports should use the
import typesyntax and be grouped separately from value imports.
20-85: Consider adding JSDoc comments for better documentation.The
GenerateWorkflowclass and its methods lack JSDoc documentation. Adding JSDoc comments would improve code maintainability and provide better IDE support.Example JSDoc structure
/** * Tool for generating a complete workflow plan from scratch. * Validates input against workflowPlanSchema and persists via ReflyService. */ export class GenerateWorkflow extends AgentBaseTool<CopilotToolParams> { // ... /** * Generate a workflow plan with the given input. * @param input - Workflow plan data conforming to workflowPlanSchema * @param _ - Unused parameter * @param config - Runnable config containing session context * @returns Tool call result with planId and version on success */ async _call( input: z.infer<typeof this.schema>, _: unknown, config: RunnableConfig, ): Promise<ToolCallResult> { // ... } }As per coding guidelines, JSDoc-style comments should be used for functions and classes in TypeScript.
87-191: Consider adding JSDoc comments for better documentation.The
PatchWorkflowclass and its methods lack JSDoc documentation. Adding JSDoc comments would improve code maintainability and provide better IDE support.As per coding guidelines, JSDoc-style comments should be used for functions and classes in TypeScript.
263-263: Consider using a named constant for the truncation limit.The hardcoded value
100for truncating task prompts could be extracted into a named constant for better maintainability.🔎 Proposed refactor
+const TASK_PROMPT_TRUNCATE_LENGTH = 100; + export class GetWorkflowSummary extends AgentBaseTool<CopilotToolParams> { // ... tasks: plan.tasks?.map((t) => ({ id: t.id, title: t.title, dependentTasks: t.dependentTasks, toolsets: t.toolsets, - prompt: truncateContent(t.prompt, 100), + prompt: truncateContent(t.prompt, TASK_PROMPT_TRUNCATE_LENGTH), })),As per coding guidelines, avoid magic numbers and strings - use named constants instead.
201-282: Consider adding JSDoc comments for better documentation.The
GetWorkflowSummaryclass and its methods lack JSDoc documentation. Adding JSDoc comments would improve code maintainability and provide better IDE support.As per coding guidelines, JSDoc-style comments should be used for functions and classes in TypeScript.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/api/src/modules/copilot-autogen/copilot-autogen.dto.tspackages/agent-tools/src/copilot/index.tspackages/ai-workspace-common/src/components/markdown/plugins/tool-call/copilot-workflow-plan.tsxpackages/i18n/src/en-US/ui.tspackages/i18n/src/zh-Hans/ui.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/api/src/modules/copilot-autogen/copilot-autogen.dto.ts
- packages/ai-workspace-common/src/components/markdown/plugins/tool-call/copilot-workflow-plan.tsx
🧰 Additional context used
📓 Path-based instructions (18)
**/*.{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/i18n/src/en-US/ui.tspackages/agent-tools/src/copilot/index.tspackages/i18n/src/zh-Hans/ui.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/i18n/src/en-US/ui.tspackages/agent-tools/src/copilot/index.tspackages/i18n/src/zh-Hans/ui.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/i18n/src/en-US/ui.tspackages/agent-tools/src/copilot/index.tspackages/i18n/src/zh-Hans/ui.ts
**/{i18n,locales,translations,lang}/**/*.{json,ts,tsx,js,jsx,properties}
📄 CodeRabbit inference engine (.cursor/rules/00-language-priority.mdc)
Use Chinese for all user-facing communication
Files:
packages/i18n/src/en-US/ui.tspackages/i18n/src/zh-Hans/ui.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/i18n/src/en-US/ui.tspackages/agent-tools/src/copilot/index.tspackages/i18n/src/zh-Hans/ui.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/i18n/src/en-US/ui.tspackages/agent-tools/src/copilot/index.tspackages/i18n/src/zh-Hans/ui.ts
**/*.{js,ts,jsx,tsx,css,json}
📄 CodeRabbit inference engine (.cursor/rules/04-code-formatting.mdc)
Maximum line length of 100 characters
Files:
packages/i18n/src/en-US/ui.tspackages/agent-tools/src/copilot/index.tspackages/i18n/src/zh-Hans/ui.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/i18n/src/en-US/ui.tspackages/agent-tools/src/copilot/index.tspackages/i18n/src/zh-Hans/ui.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/i18n/src/en-US/ui.tspackages/agent-tools/src/copilot/index.tspackages/i18n/src/zh-Hans/ui.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/i18n/src/en-US/ui.tspackages/agent-tools/src/copilot/index.tspackages/i18n/src/zh-Hans/ui.ts
packages/i18n/src/{en-US,zh-Hans}/**
📄 CodeRabbit inference engine (.cursor/rules/09-i18n-guidelines.mdc)
Add new translation keys to both language files (en-US and zh-Hans)
Files:
packages/i18n/src/en-US/ui.tspackages/i18n/src/zh-Hans/ui.ts
packages/i18n/src/**
📄 CodeRabbit inference engine (.cursor/rules/09-i18n-guidelines.mdc)
packages/i18n/src/**: Maintain consistency in translation key naming conventions
Use descriptive translation keys that reflect the content
Group related translation keys together in the translation file structure
Use namespaces for different sections of the application in translation keys
Follow a hierarchical structure for nested components in translation key organization
Files:
packages/i18n/src/en-US/ui.tspackages/i18n/src/zh-Hans/ui.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/i18n/src/en-US/ui.tspackages/agent-tools/src/copilot/index.tspackages/i18n/src/zh-Hans/ui.ts
**/*.{tsx,ts,json}
📄 CodeRabbit inference engine (.cursor/rules/09-i18n-guidelines.mdc)
Support dynamic content with placeholders in translations
Files:
packages/i18n/src/en-US/ui.tspackages/agent-tools/src/copilot/index.tspackages/i18n/src/zh-Hans/ui.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/i18n/src/en-US/ui.tspackages/agent-tools/src/copilot/index.tspackages/i18n/src/zh-Hans/ui.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/i18n/src/en-US/ui.tspackages/agent-tools/src/copilot/index.tspackages/i18n/src/zh-Hans/ui.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/i18n/src/en-US/ui.tspackages/agent-tools/src/copilot/index.tspackages/i18n/src/zh-Hans/ui.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:
packages/agent-tools/src/copilot/index.ts
🧠 Learnings (4)
📚 Learning: 2025-11-25T03:04:41.334Z
Learnt from: CR
Repo: refly-ai/refly PR: 0
File: .cursor/rules/09-i18n-guidelines.mdc:0-0
Timestamp: 2025-11-25T03:04:41.334Z
Learning: Applies to packages/i18n/src/{en-US,zh-Hans}/** : Add new translation keys to both language files (en-US and zh-Hans)
Applied to files:
packages/i18n/src/en-US/ui.tspackages/i18n/src/zh-Hans/ui.ts
📚 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 **/{i18n,locales,translations,lang}/**/*.{json,ts,tsx,js,jsx,properties} : Use Chinese for all user-facing communication
Applied to files:
packages/i18n/src/en-US/ui.tspackages/i18n/src/zh-Hans/ui.ts
📚 Learning: 2025-11-25T03:04:41.334Z
Learnt from: CR
Repo: refly-ai/refly PR: 0
File: .cursor/rules/09-i18n-guidelines.mdc:0-0
Timestamp: 2025-11-25T03:04:41.334Z
Learning: Applies to **/*.{tsx,ts} : Ensure all user-facing text is translatable
Applied to files:
packages/i18n/src/en-US/ui.tspackages/i18n/src/zh-Hans/ui.ts
📚 Learning: 2025-11-25T03:04:41.334Z
Learnt from: CR
Repo: refly-ai/refly PR: 0
File: .cursor/rules/09-i18n-guidelines.mdc:0-0
Timestamp: 2025-11-25T03:04:41.334Z
Learning: Applies to packages/i18n/src/** : Use descriptive translation keys that reflect the content
Applied to files:
packages/i18n/src/en-US/ui.tspackages/i18n/src/zh-Hans/ui.ts
🧬 Code graph analysis (1)
packages/agent-tools/src/copilot/index.ts (5)
packages/ai-workspace-common/src/requests/types.gen.ts (1)
User(474-483)packages/request/src/requests/types.gen.ts (1)
User(474-483)packages/openapi-schema/src/types.gen.ts (1)
User(474-483)packages/agent-tools/src/builtin/interface.ts (1)
ReflyService(50-196)packages/canvas-common/src/workflow-plan.ts (4)
workflowPlanSchema(63-69)normalizeWorkflowPlan(383-397)workflowPlanPatchSchema(126-136)parseWorkflowPlanPatch(334-353)
⏰ 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 (3)
packages/i18n/src/en-US/ui.ts (2)
3214-3217: LGTM!The new
copilotSummarytranslation key follows the naming convention of sibling keys (readFile,listFiles) and provides a clear, descriptive English value. Based on learnings, ensure the corresponding key exists in the zh-Hans locale file.
3223-3224: LGTM!The
loadingWorkflowkey is appropriately placed under thecopilotsection and follows the established pattern for loading state messages (e.g., using ellipsis). Based on learnings, ensure the corresponding key exists in the zh-Hans locale file.packages/i18n/src/zh-Hans/ui.ts (1)
3319-3319: The corresponding translation key already exists inpackages/i18n/src/en-US/ui.tsat line 3216 with the value'Reading workflow summary'. Both language files have been properly updated as required.
… interface - Implemented getWorkflowPlanById method in SkillEngineService to retrieve workflow plan details by ID. - Updated WorkflowPlanService to return WorkflowPlanRecord type for getWorkflowPlanDetail method. - Added getWorkflowPlanById method to ReflyService interface for consistency in workflow plan retrieval.
- Updated SkillInvokerService to prioritize messages from action_messages for history reconstruction, improving clarity and performance. - Introduced reconstructLangchainMessagesFromDB method to separate AI and tool messages without XML formatting. - Modified populateSkillResultHistory to include messages for better result detail. - Enhanced context extraction in scheduler utils to prefer messages over steps, ensuring cleaner content handling and preventing model confusion with XML patterns. - Added utility functions for stripping tool_use blocks and extracting content from ActionResult.
- Updated SkillInvokerService to utilize nullish coalescing for safer property access and improved message handling. - Refactored tool message reconstruction to ensure proper formatting and prevent missing tool call references. - Enhanced token counting utilities to include tool call data for more accurate token calculations. - Improved message validation in truncation logic to account for tool calls, ensuring better content management.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
apps/api/src/modules/skill/skill.service.ts (1)
1129-1134: Consider adding error handling for individual result fetches.The
Promise.allwill reject if any singlegetActionResultcall fails, potentially losing all successfully fetched results. Consider usingPromise.allSettledor adding try-catch within the map callback, similar to the pattern used inpopulateSkillContext(lines 1078-1091).🔎 Proposed fix with error handling
const finalResults: ActionResult[] = await Promise.all( Array.from(latestResultsMap.keys()).map(async (resultId) => { - const resultDetail = await this.actionService.getActionResult(user, { resultId }); - return actionResultPO2DTO(resultDetail); + try { + const resultDetail = await this.actionService.getActionResult(user, { resultId }); + return actionResultPO2DTO(resultDetail); + } catch (error) { + this.logger.error( + `Failed to get action result detail for resultId ${resultId}: ${error?.message}`, + ); + return null; + } }), - ); + ).then((results) => results.filter(Boolean) as ActionResult[]);packages/skill-template/src/scheduler/utils/context.ts (1)
108-131: Avoid usinganytype; use proper types fromActionMessageandActionStep.The callback parameters are typed as
any, butActionResulthas typed arraysmessages?: Array<ActionMessage>andsteps?: Array<ActionStep>. Import and use these types for better type safety.🔎 Proposed fix
-import { ActionResult, SkillContext } from '@refly/openapi-schema'; +import { ActionMessage, ActionResult, ActionStep, SkillContext } from '@refly/openapi-schema';function extractResultContent(result: ActionResult): string { if (!result) return ''; // Priority 1: Use messages if available (preferred) if (result.messages && Array.isArray(result.messages) && result.messages.length > 0) { return result.messages - .filter((msg: any) => msg?.type === 'ai') // Only include AI messages - .map((msg: any) => { + .filter((msg: ActionMessage) => msg?.type === 'ai') // Only include AI messages + .map((msg: ActionMessage) => { // Combine reasoning content and regular content - const reasoning = msg?.reasoningContent || ''; - const content = msg?.content || ''; + const reasoning = (msg as any)?.reasoningContent ?? ''; + const content = msg?.content ?? ''; return reasoning ? `${reasoning}\n\n${content}` : content; }) .filter(Boolean) .join('\n\n'); } // Priority 2: Fallback to steps for backward compatibility if (result.steps && Array.isArray(result.steps)) { - return result.steps.map((step: any) => step?.content || '').join('\n\n'); + return result.steps.map((step: ActionStep) => step?.content ?? '').join('\n\n'); } return ''; }As per coding guidelines, prefer explicit types over
any. Note: ifreasoningContentis not in theActionMessagetype definition, consider extending the type or using a type assertion for that specific property.packages/skill-template/src/scheduler/utils/truncator.ts (2)
18-21: Use type guards instead ofanyfor safer type handling.The
(message as any)type assertion bypasses TypeScript's type safety. Consider using a type guard with optional chaining to handletool_callsmore safely.🔎 Proposed type-safe approach
+// Type guard for messages with tool calls +const hasToolCalls = (message: BaseMessage): message is BaseMessage & { tool_calls: any[] } => { + return 'tool_calls' in message && Array.isArray((message as any).tool_calls); +}; + export const isEmptyMessage = (message: BaseMessage) => { // If the message has tool calls, it is not empty - if ((message as any).tool_calls && (message as any).tool_calls.length > 0) { + if (hasToolCalls(message) && message.tool_calls.length > 0) { return false; }As per coding guidelines, avoid using
anytype and always use optional chaining when accessing object properties.
54-54: Apply consistent type-safe handling fortool_calls.Similar to the issue on lines 18-21, this line uses
(message as any)which bypasses type safety. Use the same type guard approach for consistency.🔎 Proposed fix using type guard
export const truncateMessages = ( messages: BaseMessage[] = [], maxMessages = MAX_MESSAGES, maxMessageTokens = MAX_MESSAGE_TOKENS, maxMessagesTotalTokens = MAX_MESSAGES_TOTAL_TOKENS, ): BaseMessage[] => { let totalTokens = 0; const truncatedMessages: BaseMessage[] = []; for (let i = messages.length - 1; i >= Math.max(0, messages.length - maxMessages); i--) { const message = messages[i]; let content = message.content; - let tokens = countToken(content, (message as any).tool_calls); + let tokens = countToken(content, hasToolCalls(message) ? message.tool_calls : undefined);As per coding guidelines, avoid using
anytype and prefer type guards for type narrowing.packages/skill-template/src/scheduler/utils/token.ts (2)
85-88: Use optional chaining for safer property access.The condition can be simplified using optional chaining, which is more idiomatic and safer.
🔎 Proposed improvement
// Add tool calls to input text for token counting - if (toolCalls && toolCalls.length > 0) { + if (toolCalls?.length) { inputText += JSON.stringify(toolCalls); }As per coding guidelines, always use optional chaining (
?.) when accessing object properties.
97-102: Add explicit return type and use type guards.This function has two issues:
- Uses
(message as any)which bypasses type safety- Missing explicit return type declaration
🔎 Proposed improvements
-export const countMessagesTokens = (messages: BaseMessage[] = []) => { +export const countMessagesTokens = (messages: BaseMessage[] = []): number => { return messages.reduce( - (sum, message) => sum + countToken(message.content, (message as any).tool_calls), + (sum, message) => sum + countToken( + message.content, + hasToolCalls(message) ? message.tool_calls : undefined + ), 0, ); };Note: This assumes the
hasToolCallstype guard is defined as suggested in the truncator.ts file review.As per coding guidelines, always define explicit return types for functions and avoid using
anytype.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/api/src/modules/skill/skill-invoker.service.tsapps/api/src/modules/skill/skill.service.tspackages/skill-template/src/scheduler/utils/context.tspackages/skill-template/src/scheduler/utils/token.tspackages/skill-template/src/scheduler/utils/truncator.ts
🧰 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:
packages/skill-template/src/scheduler/utils/truncator.tspackages/skill-template/src/scheduler/utils/token.tsapps/api/src/modules/skill/skill.service.tsapps/api/src/modules/skill/skill-invoker.service.tspackages/skill-template/src/scheduler/utils/context.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/skill-template/src/scheduler/utils/truncator.tspackages/skill-template/src/scheduler/utils/token.tsapps/api/src/modules/skill/skill.service.tsapps/api/src/modules/skill/skill-invoker.service.tspackages/skill-template/src/scheduler/utils/context.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/skill-template/src/scheduler/utils/truncator.tspackages/skill-template/src/scheduler/utils/token.tsapps/api/src/modules/skill/skill.service.tsapps/api/src/modules/skill/skill-invoker.service.tspackages/skill-template/src/scheduler/utils/context.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/skill-template/src/scheduler/utils/truncator.tspackages/skill-template/src/scheduler/utils/token.tsapps/api/src/modules/skill/skill.service.tsapps/api/src/modules/skill/skill-invoker.service.tspackages/skill-template/src/scheduler/utils/context.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/skill-template/src/scheduler/utils/truncator.tspackages/skill-template/src/scheduler/utils/token.tsapps/api/src/modules/skill/skill.service.tsapps/api/src/modules/skill/skill-invoker.service.tspackages/skill-template/src/scheduler/utils/context.ts
**/*.{js,ts,jsx,tsx,css,json}
📄 CodeRabbit inference engine (.cursor/rules/04-code-formatting.mdc)
Maximum line length of 100 characters
Files:
packages/skill-template/src/scheduler/utils/truncator.tspackages/skill-template/src/scheduler/utils/token.tsapps/api/src/modules/skill/skill.service.tsapps/api/src/modules/skill/skill-invoker.service.tspackages/skill-template/src/scheduler/utils/context.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/skill-template/src/scheduler/utils/truncator.tspackages/skill-template/src/scheduler/utils/token.tsapps/api/src/modules/skill/skill.service.tsapps/api/src/modules/skill/skill-invoker.service.tspackages/skill-template/src/scheduler/utils/context.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/skill-template/src/scheduler/utils/truncator.tspackages/skill-template/src/scheduler/utils/token.tsapps/api/src/modules/skill/skill.service.tsapps/api/src/modules/skill/skill-invoker.service.tspackages/skill-template/src/scheduler/utils/context.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/skill-template/src/scheduler/utils/truncator.tspackages/skill-template/src/scheduler/utils/token.tsapps/api/src/modules/skill/skill.service.tsapps/api/src/modules/skill/skill-invoker.service.tspackages/skill-template/src/scheduler/utils/context.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/skill-template/src/scheduler/utils/truncator.tspackages/skill-template/src/scheduler/utils/token.tsapps/api/src/modules/skill/skill.service.tsapps/api/src/modules/skill/skill-invoker.service.tspackages/skill-template/src/scheduler/utils/context.ts
**/*.{tsx,ts,json}
📄 CodeRabbit inference engine (.cursor/rules/09-i18n-guidelines.mdc)
Support dynamic content with placeholders in translations
Files:
packages/skill-template/src/scheduler/utils/truncator.tspackages/skill-template/src/scheduler/utils/token.tsapps/api/src/modules/skill/skill.service.tsapps/api/src/modules/skill/skill-invoker.service.tspackages/skill-template/src/scheduler/utils/context.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/skill-template/src/scheduler/utils/truncator.tspackages/skill-template/src/scheduler/utils/token.tsapps/api/src/modules/skill/skill.service.tsapps/api/src/modules/skill/skill-invoker.service.tspackages/skill-template/src/scheduler/utils/context.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/skill-template/src/scheduler/utils/truncator.tspackages/skill-template/src/scheduler/utils/token.tsapps/api/src/modules/skill/skill.service.tsapps/api/src/modules/skill/skill-invoker.service.tspackages/skill-template/src/scheduler/utils/context.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/skill-template/src/scheduler/utils/truncator.tspackages/skill-template/src/scheduler/utils/token.tsapps/api/src/modules/skill/skill.service.tsapps/api/src/modules/skill/skill-invoker.service.tspackages/skill-template/src/scheduler/utils/context.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/skill/skill.service.tsapps/api/src/modules/skill/skill-invoker.service.ts
🧬 Code graph analysis (3)
packages/skill-template/src/scheduler/utils/truncator.ts (1)
packages/skill-template/src/scheduler/utils/token.ts (1)
countToken(80-91)
packages/skill-template/src/scheduler/utils/token.ts (1)
packages/utils/src/token.ts (1)
countToken(66-78)
packages/skill-template/src/scheduler/utils/context.ts (3)
packages/ai-workspace-common/src/requests/types.gen.ts (1)
ActionResult(2043-2176)packages/openapi-schema/src/types.gen.ts (1)
ActionResult(2043-2176)packages/request/src/requests/types.gen.ts (1)
ActionResult(2043-2176)
⏰ 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 (10)
apps/api/src/modules/skill/skill-invoker.service.ts (6)
5-12: LGTM!The imports are correctly updated to include
ToolMessagefrom LangChain andActionMessagefrom the schema, which are required for the new DB-based message reconstruction logic.Also applies to: 23-23
138-145: LGTM!The new preferred path correctly checks for pre-fetched messages and delegates to
reconstructLangchainMessagesFromDB. The early return pattern keeps the logic clean.
178-250: LGTM with minor type safety observation.The
reconstructLangchainMessagesFromDBmethod is well-structured with proper null handling. The defensive check at lines 211-217 ensures Bedrock compatibility by not emitting orphanToolMessages without matching tool calls.The
as anycasts on lines 204, 208, 214, 220, and 241 suggest theActionMessagetype definition might benefit from more precise typing fortoolCallMetaandtoolCallResultfields. Consider updating the schema types if this pattern becomes more widespread.
339-341: LGTM!The conditional propagation of
copilotSessionIdto the configurable object is correctly implemented.
354-357: LGTM!The call to
buildLangchainMessagesis correctly updated to use the new signature without the explicitstepsparameter.
147-176: LGTM!The backward compatibility path for step-based message reconstruction is well-preserved. The fallback logic correctly handles cases where the new
messagesfield is not available.packages/skill-template/src/scheduler/utils/context.ts (4)
6-12: Well-documented regex patterns.The regex patterns are well-designed with clear comments explaining their purpose. The non-greedy matching (
*?) and the handling of both markdown code blocks and standalone XML tags covers the expected formats appropriately.
84-96: Clean implementation with proper null handling.The function correctly handles edge cases with the early return for falsy content. The two-pass regex replacement strategy appropriately handles both markdown-wrapped and standalone XML formats.
183-197: Good refactoring of content extraction flow.The sorting logic now correctly uses
extractResultContentfor consistent content length estimation. The two-step extraction and sanitization process (extract → strip tool_use blocks) is clear and properly integrated with the token-based selection logic.
212-225: LGTM!The default title change to "Untitled Agent" is appropriate for the agent results context. The
outputFilesmapping correctly uses optional chaining and nullish coalescing for safe property access.
| */ | ||
| export const countToken = (content: MessageContent) => { | ||
| const inputText = Array.isArray(content) | ||
| export const countToken = (content: MessageContent, toolCalls?: any[]) => { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add explicit return type and avoid any type.
The function signature has two issues:
- Uses
any[]fortoolCallsparameter, violating the guideline to avoidanytype - Missing explicit return type declaration
🔎 Proposed fix with proper types
-export const countToken = (content: MessageContent, toolCalls?: any[]) => {
+export const countToken = (content: MessageContent, toolCalls?: unknown[]): number => {As per coding guidelines, always define explicit return types for functions (especially for public APIs) and avoid using any type—use unknown instead with proper type guards.
📝 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.
| export const countToken = (content: MessageContent, toolCalls?: any[]) => { | |
| export const countToken = (content: MessageContent, toolCalls?: unknown[]): number => { |
🤖 Prompt for AI Agents
In packages/skill-template/src/scheduler/utils/token.ts around line 80, the
function signature uses `any[]` for toolCalls and lacks an explicit return type;
change the parameter type to a safer type (e.g., `unknown[]` or a defined
`ToolCall` interface) and add an explicit return type (likely `: number`).
Update the function implementation to narrow/check the `toolCalls` items with
type guards or validations before using them so the compiler knows their shape,
and ensure the function returns a number in all code paths.
… plan editing (#1978) * WIP: add new workflow plan schema and persistence * feat: enhance workflow plan service with Redis locking and new ID generation - Added RedisService for locking mechanism to prevent concurrent updates on workflow plans. - Introduced genWorkflowPlanID function for consistent ID generation. - Updated patching logic to utilize the new locking mechanism and improved error handling. - Modified response structure in GenerateWorkflow class to return planId and version only. * feat: add copilotSessionId to SkillRunnableConfig and update SkillInvokerService - Introduced copilotSessionId property in SkillRunnableConfig interface for enhanced configuration options. - Updated SkillInvokerService to conditionally assign copilotSessionId from data if available, improving flexibility in session management. * feat: add getLatestWorkflowPlan method to ReflyService and update workflow plan handling - Introduced getLatestWorkflowPlan method in ReflyService interface to retrieve the latest workflow plan based on copilotSessionId. - Updated copilot index logic to handle cases where planId is not provided, fetching the latest plan if necessary. - Modified workflowPlanPatchSchema to make planId optional, allowing for dynamic selection of the latest plan within a session. * refactor: update workflow plan description and enhance operation details - Revised the description of the PatchWorkflow class to clarify the purpose and available operations for modifying workflow plans. - Streamlined operation examples and added notes on usage, emphasizing the distinction between `generate_workflow` and `patch_workflow` tools. - Improved clarity on optional fields and the order of operations in the workflow plan patching process. * refactor: update workflow plan types and improve handling - Consolidated WorkflowPlanData and WorkflowPlan into a single WorkflowPlan type for clarity. - Introduced WorkflowPlanRecord to encapsulate additional properties like planId and version. - Updated various services and interfaces to utilize the new types, ensuring consistency across the codebase. - Enhanced error handling and validation for workflow tasks and variables during patch operations. * feat: adapt workflow plan display in copilot sessions - Updated WorkflowPlan model to include version and uid fields for better tracking. - Refactored service methods to accept user and params objects, improving parameter handling and consistency. - Enhanced getWorkflowPlanDetail method to utilize user context and versioning for fetching plans. - Improved generateWorkflowPlan and patchWorkflowPlan methods to streamline parameter passing and validation. - Updated controller to include a new endpoint for retrieving workflow plan details, ensuring proper user authentication and query handling. * feat: add getWorkflowSummary tool for enhanced workflow context retrieval - Introduced GetWorkflowSummary class to retrieve the current workflow plan structure, including tasks and variables. - Updated ToolService to integrate getWorkflowSummary, allowing for better context management during workflow modifications. - Enhanced documentation to guide users on when to utilize getWorkflowSummary for recalling task/variable IDs and understanding workflow structure. * feat: implement canvas title update functionality and refactor related components - Introduced useUpdateCanvasTitle hook for managing canvas title updates, including auto-naming and manual renaming. - Refactored CopilotMessage, CanvasRenameModal, and CanvasTitle components to utilize the new hook, improving code maintainability and performance. - Removed redundant remote title update logic from components, centralizing it within the new hook. - Enhanced title update handling in CanvasRenameModal and CanvasTitle to ensure proper state management and user experience. * feat: enhance tool call rendering with copilot summary and improved layout - Added CopilotSummaryRenderer for displaying workflow summary status in a compact format. - Updated ToolCall component to conditionally render CopilotSummaryRenderer based on tool name. - Refactored ListFilesRenderer and ReadFileRenderer to include durationText and improve styling consistency. - Adjusted result message component layout for better spacing and visual clarity. - Enhanced i18n support by adding translations for copilot summary messages. * refactor: consolidate imports in copilot-autogen.dto.ts - Merged import statements for WorkflowVariable and WorkflowPlan from '@refly/openapi-schema' to streamline code and improve readability. * refactor: update imports and enhance loading message in Copilot components - Changed import statement in copilot-autogen.dto.ts to use type imports for better clarity. - Updated loading message in CopilotWorkflowPlan component to utilize i18n translation for improved localization support. - Removed outdated preset scenarios and context menu translations from i18n files to streamline the translation structure. * refactor: enhance session context validation in GenerateWorkflow class - Added validation for copilotSessionId, resultId, and resultVersion to ensure required session context is present before generating and patching workflow plans. - Utilized optional chaining and nullish coalescing for safer property access and default values. - Improved error messages for better clarity on missing session context during workflow operations. * feat: add getWorkflowPlanById method to SkillEngineService and update interface - Implemented getWorkflowPlanById method in SkillEngineService to retrieve workflow plan details by ID. - Updated WorkflowPlanService to return WorkflowPlanRecord type for getWorkflowPlanDetail method. - Added getWorkflowPlanById method to ReflyService interface for consistency in workflow plan retrieval. * refactor: enhance skill invoker service and context handling - Updated SkillInvokerService to prioritize messages from action_messages for history reconstruction, improving clarity and performance. - Introduced reconstructLangchainMessagesFromDB method to separate AI and tool messages without XML formatting. - Modified populateSkillResultHistory to include messages for better result detail. - Enhanced context extraction in scheduler utils to prefer messages over steps, ensuring cleaner content handling and preventing model confusion with XML patterns. - Added utility functions for stripping tool_use blocks and extracting content from ActionResult. * refactor: enhance SkillInvokerService and token counting utilities - Updated SkillInvokerService to utilize nullish coalescing for safer property access and improved message handling. - Refactored tool message reconstruction to ensure proper formatting and prevent missing tool call references. - Enhanced token counting utilities to include tool call data for more accurate token calculations. - Improved message validation in truncation logic to account for tool calls, ensuring better content management.
2 participants
Summary by CodeRabbit
New Features
Improvements
Docs & Localization
✏️ Tip: You can customize this high-level summary in your review settings.