Skip to content

chore: port an api as experiment#35261

Open
asukaminato0721 wants to merge 4 commits intolanggenius:mainfrom
asukaminato0721:35141
Open

chore: port an api as experiment#35261
asukaminato0721 wants to merge 4 commits intolanggenius:mainfrom
asukaminato0721:35141

Conversation

@asukaminato0721
Copy link
Copy Markdown
Contributor

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

part of #35141

Screenshots

Before After
... ...

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

Copilot AI review requested due to automatic review settings April 15, 2026 10:09
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Apr 15, 2026
@asukaminato0721 asukaminato0721 changed the title chore: port a api as experiment chore: port an api as experiment Apr 15, 2026
@github-actions
Copy link
Copy Markdown
Contributor

Pyrefly Diff

No changes detected.

@github-actions
Copy link
Copy Markdown
Contributor

Pyrefly Diff

No changes detected.

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

Note

Copilot was unable to run its full agentic suite in this review.

Ports the completion task “stop” endpoint to a new canonical API shape (DELETE on task resource) while keeping the legacy POST /stop route for backward compatibility.

Changes:

  • Introduced CompletionTaskApi using DELETE /completion-messages/<task_id> to stop a running task.
  • Refactored stop logic into a shared helper and marked the legacy POST stop route as deprecated.
  • Updated unit tests to cover the new DELETE route and verify AppTaskService.stop_task arguments.

Reviewed changes

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

File Description
api/controllers/web/completion.py Adds shared stop helper, introduces DELETE task route, and deprecates legacy POST /stop endpoint.
api/tests/unit_tests/controllers/web/test_completion.py Adds/updates unit tests for the new DELETE endpoint and legacy stop behavior.

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

Comment thread api/controllers/web/completion.py Outdated
Comment thread api/controllers/web/completion.py Outdated
Comment thread api/controllers/web/completion.py
Comment on lines +122 to 126
class TestCompletionStopApi:
@patch("controllers.web.completion.AppTaskService.stop_task")
def test_legacy_post_stop_success(self, mock_stop: MagicMock, app: Flask) -> None:
with app.test_request_context("/completion-messages/task-1/stop", method="POST"):
result, status = CompletionStopApi().post(_completion_app(), _end_user(), "task-1")
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

The legacy deprecated POST endpoint is tested for success, but there’s no longer a test asserting it raises NotCompletionAppError on a non-completion app mode (similar to what TestCompletionTaskApi.test_wrong_mode_raises does for DELETE). Adding that negative-path test would help prevent regression while the deprecated endpoint remains supported.

Copilot uses AI. Check for mistakes.
@github-actions
Copy link
Copy Markdown
Contributor

Pyrefly Diff

No changes detected.

@asukaminato0721 asukaminato0721 requested a review from hyoban April 15, 2026 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants