Skip to content

perf: workflow runtime#6562

Open
c121914yu wants to merge 8 commits intolabring:mainfrom
c121914yu:fix-issue
Open

perf: workflow runtime#6562
c121914yu wants to merge 8 commits intolabring:mainfrom
c121914yu:fix-issue

Conversation

@c121914yu
Copy link
Collaborator

No description provided.

Copilot AI review requested due to automatic review settings March 15, 2026 02:57
@c121914yu c121914yu changed the title perf: workflow run time perf: workflow runtime Mar 15, 2026
@github-actions
Copy link

github-actions bot commented Mar 15, 2026

Preview sandbox Image:

registry.cn-hangzhou.aliyuncs.com/fastgpt/fastgpt-pr:fatsgpt_sandbox_6b0383cf82c86be24706847bebb54fe8c780bc3f

@github-actions
Copy link

github-actions bot commented Mar 15, 2026

Preview mcp_server Image:

registry.cn-hangzhou.aliyuncs.com/fastgpt/fastgpt-pr:fatsgpt_mcp_server_6b0383cf82c86be24706847bebb54fe8c780bc3f

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to improve workflow dispatch runtime behavior by replacing recursive queue processing with an iterative loop, alongside a few logging/schema tweaks that support debugging and runtime visibility.

Changes:

  • Refactor workflow queue execution from recursive processActiveNode to an iterative startProcessing loop with a processingActive guard.
  • Adjust logging: remove per-node debug log, add a workflow-run start info log, and increase console pretty-logger object inspection depth.
  • Broaden JSON Schema enum parsing to accept non-string values.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
projects/app/package.json Bumps app version.
packages/service/core/workflow/dispatch/index.ts Replaces recursive queue processing with an iterative loop; adjusts skip-node handling and workflow start logging.
packages/service/common/logger/sinks.ts Increases console pretty formatter inspection depth for nested objects.
packages/global/core/app/jsonschema.ts Widens JSON Schema enum type from string[] to any[].

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

@github-actions
Copy link

github-actions bot commented Mar 15, 2026

Preview fastgpt Image:

registry.cn-hangzhou.aliyuncs.com/fastgpt/fastgpt-pr:fatsgpt_6b0383cf82c86be24706847bebb54fe8c780bc3f

Copy link
Collaborator Author

@c121914yu c121914yu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📊 代码审查总结

详细的审查意见请查看下方的行级评论。


// 枚举和常量
enum: z.array(z.string()).optional(), // 枚举值
enum: z.array(z.any()).optional(), // 枚举值
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 类型放宽: z.array(z.string())z.array(z.any()) 虽然修复了 enum 值不限于 string 的问题(JSON Schema 的 enum 确实支持任意类型),但 z.any() 完全绕过了运行时校验。

建议使用更精确的联合类型:

enum: z.array(z.union([z.string(), z.number(), z.boolean(), z.null()])).optional(),

@c121914yu
Copy link
Collaborator Author

PR Review: perf: workflow runtime

📊 变更概览

📁 变更分类

类别 文件 说明
🔧 核心优化 dispatch/index.ts WorkflowQueue 递归→迭代重构
🐛 Bug 修复 jsonschema.ts enum 类型从 string 放宽为 any
🔧 日志 sinks.ts console 输出深度设为 Infinity
🎨 前端重构 7 个组件文件 placeholder→description 语义拆分
📦 版本 package.json 4.14.8.1 → 4.14.8.4
📄 文档 2 个 .md 文件 性能分析报告(建议移除)

✅ 优点

  1. 递归→迭代重构方向正确: 将 processActiveNode 的递归调用改为 startProcessing 的 while 循环,避免了深层工作流中的调用栈堆积和 Promise 链累积,这是本 PR 最有价值的改动
  2. 并发控制更清晰: 使用 Promise.race(runningNodePromises) 等待节点完成,比原来的 fire-and-forget + 递归回调更容易理解控制流
  3. 前端语义拆分合理: placeholderdescription 的职责分离是正确的设计决策
  4. skip 节点改为 await: processSkipNodes 从 fire-and-forget 改为 await,避免了 skip 节点处理的竞态问题

⚠️ 问题汇总

🔴 严重问题(2 个,必须修复)

  1. inspectOptions: { depth: Infinity } (sinks.ts:70) — 生产环境中可能导致性能问题、内存爆炸和敏感信息泄露。建议按环境区分深度
  2. skip 节点错误被静默吞掉 (dispatch/index.ts:496) — .catch 只记日志不传播错误,可能导致工作流静默挂起

🟡 建议改进(4 个)

  1. 并发时序依赖需要注释 (dispatch/index.ts:420) — addActiveNodestartProcessing 的交互存在微妙的时序依赖,建议添加注释说明
  2. 防御性分支可能导致空转 (dispatch/index.ts:448) — "理论上不应出现"的分支如果触发会 busy-wait,建议添加日志或计数器
  3. enum 类型过度放宽 (jsonschema.ts:22) — z.any() 完全绕过校验,建议用 z.union 限定常见类型
  4. 分析文档不应入库 (.claude/issue/) — 1480 行临时分析报告占 PR 93% 的新增行数,建议移除

🟢 可选优化(2 个)

  1. let nodePromise 可改为 constdispatch/index.ts:467
  2. 前端 placeholder prop 在拆分后可能丢失 input 占位文字,需确认是否预期

🧪 测试建议

  1. 构造 50+ 节点的工作流,验证迭代模式下内存和调用栈不再增长
  2. 测试 skip 节点执行失败时工作流是否能正常终止(而非挂起)
  3. 验证 debug 模式下 processSkipNodescontinue 路径的正确性
  4. 检查前端表单在 description 拆分后,input 框是否仍有合适的占位提示

💬 总体评价

  • 代码质量: ⭐⭐⭐⭐☆ (4/5)
  • 安全性: ⭐⭐⭐☆☆ (3/5) — depth: Infinity 有风险
  • 性能: ⭐⭐⭐⭐☆ (4/5) — 核心优化方向正确
  • 可维护性: ⭐⭐⭐⭐☆ (4/5) — 需要更多注释说明时序

🚀 审查结论

需修改。核心的递归→迭代重构是好的改进,但需要:

  1. 修复 depth: Infinity 的生产风险
  2. 完善 skip 节点的错误处理
  3. 移除不应入库的分析文档

📍 详细代码评论

已在以下位置添加了具体的行级评论:

  • packages/service/core/workflow/dispatch/index.ts: L420, L448, L467, L496
  • packages/service/common/logger/sinks.ts: L70
  • packages/global/core/app/jsonschema.ts: L22
  • .claude/issue/workflow-deep-analysis.md: L1
  • projects/app/src/components/core/app/formRender/LabelAndForm.tsx: L36

Copy link
Collaborator Author

@c121914yu c121914yu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📊 第二轮代码审查

相比上一轮,大部分问题已修复。详细意见见行级评论。

@c121914yu
Copy link
Collaborator Author

PR Review 第二轮: perf: workflow runtime

📊 变更概览

✅ 上轮问题修复情况

问题 严重度 状态
depth: Infinity 生产风险 🔴 ✅ 已修复 → depth: 5
runningNodeCount 值拷贝 bug 🔴 ✅ 已修复 → 直接用 runningNodePromises.size
letconst nodePromise 🟢 ✅ 已采纳
skip 错误日志补充上下文 🔴 ✅ 部分采纳(加了 nodeName
防御性分支空转风险 🟡 ⏳ 未处理
skip 节点错误被吞掉 🔴 ⏳ 未处理(核心逻辑风险)
z.any() 类型过宽 🟡 ⏳ 未处理
分析文档不应入库 🟡 ⏳ 未处理

⚠️ 剩余问题

🟡 建议改进(非阻塞,可后续处理)

  1. 防御性分支空转startProcessing 中 "理论上不应出现" 的 else 分支建议加 warn 日志
  2. skip 节点错误处理.catch 吞掉错误可能导致工作流静默挂起,建议后续迭代中完善
  3. z.any() 类型 — 建议收窄为 z.union([z.string(), z.number(), z.boolean(), z.null()])
  4. 分析文档.claude/issue/ 下两个 md 文件建议合并前移除

💬 总体评价

  • 代码质量: ⭐⭐⭐⭐☆ (4/5)
  • 安全性: ⭐⭐⭐⭐☆ (4/5) — depth 已修复
  • 性能: ⭐⭐⭐⭐☆ (4/5)
  • 可维护性: ⭐⭐⭐⭐☆ (4/5)

🚀 审查结论

基本可以合并。核心的递归→迭代重构逻辑正确,上轮的严重问题已修复。剩余 4 个 🟡 建议为非阻塞项,可在后续迭代中处理。合并前建议移除 .claude/issue/ 下的分析文档。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants