Skip to content

refactor(api): migrate console.app.workflow to BaseModel#36216

Merged
asukaminato0721 merged 4 commits into
langgenius:mainfrom
cqjjjzr:refac-basemodel
May 18, 2026
Merged

refactor(api): migrate console.app.workflow to BaseModel#36216
asukaminato0721 merged 4 commits into
langgenius:mainfrom
cqjjjzr:refac-basemodel

Conversation

@cqjjjzr
Copy link
Copy Markdown
Contributor

@cqjjjzr cqjjjzr commented May 15, 2026

Important

  1. Make sure you have read our contribution guidelines
  2. Ensure there is an associated issue and you have been assigned to it
  3. Use the correct syntax to link this PR: Fixes #<issue number>.

Summary

See #28015 . Replace all inline Flask-RESTX request/response payload and query param models to Pydantic BaseModel. Also done minor cleanups for duplicate code.

Affected:

  • console.app.workflow
  • console.app.rag_pipeline_workflow

A minor frontend-backend disparency is discovered and fixed. In web/models/pipeline.ts, RAGPipelineVariable has allowed_file_extensions instead of allow_file_extension.

Other affected frontend parts:

  • web/service/use-pipeline.ts, use-workflow.ts, workflow.ts: Align the types of APIs which may return null, as in the API docs.
  • web/app/components/rag-pipeline/hooks/use-pipeline-config.ts, web/app/components/workflow-app/hooks/use-workflow-init.ts, app/components/rag-pipeline/hooks/__tests__/use-pipeline-config.spec.ts: due to the change of contract, the ?? 0 has to be brought forward from setPublishedAt

Integration tests are changed because the former ones are asserting against the original raw dict return value.

Checklist

  • This change requires a documentation update, included: Dify Document
  • I understand that this PR may be closed in case there was no previous discussion or issues. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.
  • I ran make lint && make type-check (backend) and cd web && pnpm exec vp staged (frontend) to appease the lint gods

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 15, 2026

Pyrefly Diff

base → PR
--- /tmp/pyrefly_base.txt	2026-05-18 07:23:27.101520352 +0000
+++ /tmp/pyrefly_pr.txt	2026-05-18 07:23:16.942496449 +0000
@@ -1824,16 +1824,6 @@
    --> tests/unit_tests/controllers/console/app/test_app_response_models.py:112:12
 ERROR Object of class `object` has no attribute `exec_module` [missing-attribute]
    --> tests/unit_tests/controllers/console/app/test_app_response_models.py:113:5
-ERROR Argument `SimpleNamespace` is not assignable to parameter `workflow` with type `Workflow` in function `controllers.console.app.workflow._parse_file` [bad-argument-type]
-  --> tests/unit_tests/controllers/console/app/test_workflow.py:26:40
-ERROR Argument `SimpleNamespace` is not assignable to parameter `workflow` with type `Workflow` in function `controllers.console.app.workflow._parse_file` [bad-argument-type]
-  --> tests/unit_tests/controllers/console/app/test_workflow.py:44:42
-ERROR Object of class `FunctionType` has no attribute `assert_called_once_with` [missing-attribute]
-   --> tests/unit_tests/controllers/console/app/test_workflow.py:406:5
-ERROR Object of class `FunctionType` has no attribute `call_count` [missing-attribute]
-   --> tests/unit_tests/controllers/console/app/test_workflow.py:438:12
-ERROR `in` is not supported between `Literal['Maximum']` and `None` [not-iterable]
-   --> tests/unit_tests/controllers/console/app/test_workflow.py:466:12
 ERROR Missing argument `app_model` in function `handler` [missing-argument]
   --> tests/unit_tests/controllers/console/app/test_wraps.py:21:19
 ERROR Unexpected keyword argument `app_id` in function `handler` [unexpected-keyword]

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 15, 2026

Pyrefly Type Coverage

Metric Base PR Delta
Type coverage 0.00% 43.71% +43.71%
Strict coverage 0.00% 43.24% +43.24%
Typed symbols 0 22,152 +22,152
Untyped symbols 0 28,835 +28,835
Modules 0 2558 +2,558

@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. refactor labels May 15, 2026
@cqjjjzr cqjjjzr changed the title direfactor(api): migrate console.app.workflow to BaseModel refactor(api): migrate console.app.workflow to BaseModel May 15, 2026
@cqjjjzr cqjjjzr force-pushed the refac-basemodel branch from 0b0571f to 5d8fdc2 Compare May 15, 2026 09:06
@asukaminato0721 asukaminato0721 requested a review from Copilot May 15, 2026 13:18
@asukaminato0721 asukaminato0721 enabled auto-merge May 15, 2026 13:18
@asukaminato0721 asukaminato0721 self-assigned this May 15, 2026
Copy link
Copy Markdown
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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@asukaminato0721
Copy link
Copy Markdown
Contributor

@dosu how about this pr

@dosubot
Copy link
Copy Markdown

dosubot Bot commented May 15, 2026

This PR is part of the broader Flask-RESTX → Pydantic migration effort tracked in issue #28015 [1], which has already migrated ~64% of targeted files across 23+ PRs. It's well-aligned with that effort.

What it does:

  • Replaces @marshal_with decorators and inline Flask-RESTX model definitions with Pydantic BaseModel response serialization in console.app.workflow and console.app.rag_pipeline_workflow
  • Introduces new Pydantic models (WorkflowResponse, ConversationVariableResponse, WorkflowOnlineUsersResponse, etc.) with validators for timestamp normalization and secret variable masking
  • Removes the now-obsolete api/fields/online_user_fields.py file
  • Updates OpenAPI/Swagger docs and the schema guide [2]

Positives:

  • Comprehensive test updates (190+ lines added to unit tests)
  • Documentation and API schema kept in sync
  • Follows the established migration pattern consistently

Minor concerns:

  • The "minor frontend-backend disparity" fix is mentioned but not detailed — would be good to clarify what changed for reviewers
  • The documentation checklist item is unchecked, though schema docs appear updated inline [2]

Overall this looks like a solid, well-scoped contribution that advances the migration roadmap toward eventual FastAPI adoption.

To reply, just mention @dosu.


Share context across your team and agents. Try Dosu.

Leave Feedback Learn about dify with Dosu

Comment thread api/controllers/console/app/workflow.py Outdated
Comment thread api/controllers/console/app/workflow.py Outdated
Copy link
Copy Markdown
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

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

Comments suppressed due to low confidence (2)

api/controllers/console/datasets/rag_pipeline/rag_pipeline_workflow.py:483

  • Same as the draft workflow endpoint: @marshal_with was removed but this route does not declare a @console_ns.response schema, so the OpenAPI docs lose the response model. Please add @console_ns.response(200, ..., console_ns.models[WorkflowResponse.__name__]) (and the non-200 responses) for the GET handler.
@console_ns.route("/rag/pipelines/<uuid:pipeline_id>/workflows/publish")
class PublishedRagPipelineApi(Resource):
    @setup_required
    @login_required
    @account_initialization_required
    @edit_permission_required
    @get_rag_pipeline
    def get(self, pipeline: Pipeline):

api/controllers/console/datasets/rag_pipeline/rag_pipeline_workflow.py:651

  • RagPipelineByIdApi.patch removed @marshal_with and returns a WorkflowResponse dict via dump_response(...), but the route has no @console_ns.response declaration. Add an explicit response schema for 200 (and existing 4xx responses) so the update endpoint remains properly documented in OpenAPI.
@console_ns.route("/rag/pipelines/<uuid:pipeline_id>/workflows/<string:workflow_id>")
class RagPipelineByIdApi(Resource):
    @setup_required
    @login_required
    @account_initialization_required
    @edit_permission_required
    @get_rag_pipeline
    def patch(self, pipeline: Pipeline, workflow_id: str):

Comment thread api/controllers/console/app/workflow.py
auto-merge was automatically disabled May 15, 2026 15:09

Head branch was pushed to by a user without write access

@cqjjjzr cqjjjzr force-pushed the refac-basemodel branch from 5d8fdc2 to 173d5e9 Compare May 15, 2026 15:09
@dosubot dosubot Bot added size:XL This PR changes 500-999 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels May 15, 2026
Copy link
Copy Markdown
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

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

Comments suppressed due to low confidence (3)

api/controllers/console/app/workflow.py:948

  • The endpoint documents a 404 ("Published workflow not found"), but the implementation returns None when no published workflow exists, which will produce a 200 with a null body. Please align behavior and docs: either raise NotFound when workflow is None, or remove the 404 response (and explicitly document that 200 may return null).
        console_ns.models[WorkflowResponse.__name__],
    )
    @console_ns.response(404, "Published workflow not found")
    @setup_required

api/controllers/console/datasets/rag_pipeline/rag_pipeline_workflow.py:625

  • This wrapper response is manually serialized via WorkflowPaginationResponse.model_validate(...).model_dump(...). Since dump_response(...) now handles from_attributes=True + model_dump(mode='json'), consider using it here as well for consistency with controllers/API_SCHEMA_GUIDE.md.
            return WorkflowPaginationResponse.model_validate(
                {
                    "items": workflows,
                    "page": page,
                    "limit": limit,

api/openapi/markdown/console-swagger.md:14229

  • WorkflowResponse’s schema table is missing several Type values (and environment_variables renders as [ ] without an item type). Please adjust the schema/markdown generation so array item types and nullable primitives are represented, otherwise the published API docs become ambiguous.
| conversation_variables | [ [WorkflowConversationVariableResponse](#workflowconversationvariableresponse) ] |  | No |
| created_at |  |  | No |
| created_by |  |  | No |
| environment_variables | [  ] |  | No |
| features | object |  | No |

Comment thread api/controllers/console/app/workflow.py Outdated
Comment thread api/controllers/console/app/workflow.py
Comment thread api/controllers/console/datasets/rag_pipeline/rag_pipeline_workflow.py Outdated
Comment thread api/openapi/markdown/console-swagger.md Outdated
Comment thread api/openapi/markdown/console-swagger.md Outdated
Comment thread api/openapi/markdown/console-swagger.md Outdated
Comment thread api/openapi/markdown/console-swagger.md Outdated
@cqjjjzr cqjjjzr force-pushed the refac-basemodel branch from 173d5e9 to 6dd1f7c Compare May 16, 2026 10:40
@cqjjjzr cqjjjzr requested a review from Copilot May 16, 2026 10:40
@github-actions github-actions Bot added the web This relates to changes on the web. label May 16, 2026
Copy link
Copy Markdown
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

Copilot reviewed 17 out of 18 changed files in this pull request and generated 1 comment.

@cqjjjzr cqjjjzr force-pushed the refac-basemodel branch 2 times, most recently from 84c352e to d031c3c Compare May 16, 2026 11:27
@cqjjjzr
Copy link
Copy Markdown
Contributor Author

cqjjjzr commented May 16, 2026

@CodingOnStar Please have a look on the touched frontend part.

There are two changes:

  • web/service/use-pipeline.ts, use-workflow.ts, workflow.ts: Align the types of APIs which may return null, as in the API docs.
  • web/app/components/rag-pipeline/hooks/use-pipeline-config.ts, web/app/components/workflow-app/hooks/use-workflow-init.ts, app/components/rag-pipeline/hooks/__tests__/use-pipeline-config.spec.ts: due to the change of contract, the ?? 0 has to be brought forward from setPublishedAt

@autofix-ci autofix-ci Bot requested review from Yeuoly and crazywoola as code owners May 18, 2026 07:22
@dosubot dosubot Bot added size:XXL This PR changes 1000+ lines, ignoring generated files. and removed size:XL This PR changes 500-999 lines, ignoring generated files. labels May 18, 2026
@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label May 18, 2026
@asukaminato0721 asukaminato0721 added this pull request to the merge queue May 18, 2026
Merged via the queue into langgenius:main with commit 2eb37ca May 18, 2026
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer refactor size:XXL This PR changes 1000+ lines, ignoring generated files. web This relates to changes on the web.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants