refactor(workflow-run): persist explicit node graph - #86
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found in this review.
Verified:
npm test -- --run src/entities/workflow-run/store.test.tsnpm run typecheck
Note: dependency installation completed with a Node engine warning because this sandbox uses Node 20 while react-router@8.3.0 declares Node >=22.22.0.
xyh202131
changed the title
|
@fennoai /review |
Review CompleteSubmitted a non-blocking PR review with one low-severity documentation finding: Verified locally:
Note: |
There was a problem hiding this comment.
Review findings:
- Low:
frontend/src/pages/home/index.tsx:32still documents the old eight-stepWORKFLOW_STEP_ORDERmodel (character-setup,template-candidate,complete-animation, etc.), but this PR replaces that public vocabulary with card-alignedWORKFLOW_STEP_ORDERScontainingcharacter/actionsteps and phase-based internals. Anyone updating the home workflow copy from this comment will now be pointed at deleted names and the wrong model shape. Please update or remove this comment alongside the WorkflowRun model change.
Verification run locally: npm run test, npm run typecheck, npm run lint, npm run build, and git diff --check 023bf0f86e8089681ae9a09427a12306b3c0b3dc...014e83a958d3b965b4ec5738e95ae805f9d188de. Note: npm ci emitted an engine warning because this sandbox uses Node v20.20.2 while react-router@8.3.0 declares Node >=22.22.0.
|
@fennoai /compact 根据当前PR 现状和评论列出未定事项，不要删除任何评论 |
未定事项
|
未定事项
|
There was a problem hiding this comment.
I found two persistence/hydration issues in the workflow-run changes.
Additional non-inline finding:
frontend/src/pages/home/index.tsx:32: The workflow guidance comment still saysWORKFLOW_STEP_ORDERhas eight steps and mapsaction-setup,first-frame,complete-animation, andexport, but this PR changes the source of truth to five steps withaction-generation. Please update this comment with the model change so future workflow edits do not follow stale guidance.
Verified: npm run typecheck; npm test -- src/entities/workflow-run/store.test.ts.
| value: unknown, | ||
| ): CompleteAnimationGenerationResult | null { | ||
| if (!isRecord(value) || value.type !== 'complete_animation') return null | ||
| if (!['walk', 'idle', 'attack', 'jump', 'custom'].includes(String(value.actionType))) return null |
There was a problem hiding this comment.
ActionType is currently just string, and the existing UI handles unknown action names by falling back to the raw type. This validator now rejects any completed animation whose actionType is not one of these five literals, so a workflow for a custom/template-defined action can be saved but then dropped on reload because isWorkflowStep calls this parser during hydration. Please validate a non-empty string here, or share a real action-type enum across the app instead of hard-coding this subset.
| migrateReferences(characterTemplate), | ||
| migrateReferences(templateCandidate), | ||
| { | ||
| ...migrateReferences(collapsedAction), |
There was a problem hiding this comment.
This collapses whichever legacy action step is active or failed, but only changes its type. A real v1 first-frame step will still carry a first_frame input/output shape, and action-setup may not have the firstFrameUrl required by the new action-generation validator. After migrateVersionThreeRun calls isWorkflowRun, those migrated runs fail validation and disappear from localStorage. Please normalize the collapsed step's input/output for each legacy step before the final validation.
- Remove localStorage persistence, replace with async API adapter (store.ts) - Remove Revision concept (no rollback/history per backend tree model) - Remove WorkflowDriver — set once, never read - Align GenerationType with backend: character_image / character_action - Flatten WorkflowRun: steps/generationStatus/exportStatus directly on run - Fix step model docs: first 3 steps serial, action-generation pairs concurrent - Sync backend infrastructure (ai_engine impl, new providers, config files) - Sync frontend infrastructure (shared/api, auth-session, export-package, etc.) - Update tests: async store contract, aligned generation types Addresses nighca review comments 1024XEngineer#1-1024XEngineer#6 on PR 1024XEngineer#86
15c963d to
07d0cd7
Compare
050fbf8 to
2020aa8
Compare
|
@fennoai 评价一下 |
|
An internal error occurred. Please try again later. |
| ) { | ||
| return false | ||
| } | ||
| return ( |
There was a problem hiding this comment.
Low: this accepts selectedImageUrl: null even when the card has already reached completed, so a hydrated WorkflowRun can look valid while still losing the confirmed candidate reference. The same issue applies to the action validator below. Please require the selected asset once the step is complete.
2020aa8 to
291c07e
Compare
291c07e to
2314ad8
Compare
eaec15b to
8cf6b31
Compare
xyh202131
changed the title
| } | ||
|
|
||
| /** 工作流图中的真实节点。前端和后端统一使用 node，不再保留 step 或假 root。 */ | ||
| export type WorkflowNode = CharacterWorkflowNode | ActionWorkflowNode |
There was a problem hiding this comment.
还有个细节问题，两种 node 就够了吗？确实不是每一次“用户动作”都需要表现为一个不一样的 node，可以多个动作合并在同一个 node 内（对应 node 的不同状态/phase），不过我记得按之前的 live demo，好像并不会合并得这么夸张？
3 participants
目的
统一前后端工作流概念：前端原
Step与后端Node表达的是同一种图节点。本 PR 删除双重建模，让WorkflowRun.nodes直接保存真实节点图，并把节点之间的边纳入持久化契约。当前设计
WorkflowRun是一次制作流程的持久化容器；Quick Start 与 Workflow Editor 只是两种独立界面，共用同一张节点图。WorkflowNode与编辑器卡片一一对应，目前包含 Character 与 Action 两类；生成、选择、审核等过程由节点内部phase表达。WorkflowRun → root node → steps，也不再保留Step、Revision 或 driver 概念。dependsOnNodeIds保存直接前置节点，因此边会随nodesJSON 一起落库，不再依赖数组位置推断。nodeId + taskId识别目标，避免重做前的晚到结果污染当前节点。持久化契约
前端只对接后端已经声明的四个资源接口：
POST /workflow-runsGET /workflow-runs/{id}PATCH /workflow-runs/{id}DELETE /workflow-runs/{id}请求和响应中的
nodes都是前端节点数组本身，不再额外包裹假根节点。响应水合时会校验节点字段、依赖 ID、重复 ID、悬空边、自依赖和环；已完成节点还必须保留已确认资产。本 PR 包含
nodeId / advanceNode / updateNode / restartFromNode接口边界。workflow-run/workflow-run等重复目录。明确不包含
验证
git diff --check通过。