-
Notifications
You must be signed in to change notification settings - Fork 0
Make swarm tasks idempotent and add React visualization frontend #50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make swarm tasks idempotent and add React visualization frontend #50
Conversation
… for fill_running_tasks_idempotent
…tead of patch in each test
There was a problem hiding this 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 144 out of 156 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (1)
docs/documentation/callbacks.md:128
- The text says the default return field is
mageflow_results, but the example model still usesresults: str. Update the example to usemageflow_results(or rename the field in the snippet) to match the new default.
When no field is marked with ReturnValue, the return value of the function will be sent to the field named mageflow_results.
```python
class SuccessMessage(BaseModel):
results: str # The return value of the function will be sent here
field_int: int
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| async def chain_error_task(msg: EmptyModel, ctx: Context): | ||
| try: | ||
| chain_task_id = msg.model_dump()[CHAIN_TASK_ID_NAME] | ||
| task_data = HatchetInvoker(msg, ctx).task_ctx |
Copilot
AI
Jan 29, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
chain_error_task is typed as EmptyModel but then immediately expects chain_task_id to be present in msg.model_dump()[CHAIN_TASK_ID_NAME]. With EmptyModel this will raise KeyError (and it also disagrees with the unit tests which pass ChainCallbackMessage). Consider changing the workflow input model to a message type that includes chain_task_id (e.g. ChainCallbackMessage) and set input_validator for ON_CHAIN_ERROR accordingly so the field is guaranteed to exist.
| if task_id: | ||
| current_task = await TaskSignature.get_safe(task_id) | ||
| await current_task.done() | ||
| task_success_workflows = current_task.activate_success(result) |
Copilot
AI
Jan 29, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TaskSignature.get_safe(task_id) can return None (e.g., if the signature was already removed). await current_task.done() and current_task.activate_success(...) will then raise AttributeError. Add a None check (and decide whether to treat missing signatures as a no-op) before calling methods on current_task.
| jobs: | ||
| create-github-release: | ||
| needs: check-tests | ||
| if: github.ref_type == 'tag' && github.event.base_ref == 'refs/heads/main' |
Copilot
AI
Jan 29, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The job condition uses github.event.base_ref, which is typically empty for push events on tags. This will likely cause the release job to be skipped even when a tag is pushed. If you want to ensure the tag points to main, consider removing this check or validating that the tagged commit is contained in main via git merge-base --is-ancestor.
| if: github.ref_type == 'tag' && github.event.base_ref == 'refs/heads/main' | |
| if: github.ref_type == 'tag' |
Summary
Changes
fill_running_tasks,swarm_item_done,swarm_item_failed, and batch item operationsdoneandfailedstatus, move task data via messages instead of contextactions/,creation/,change_status/,idempotency/,publish/, andworkflows/folders with comprehensive coverageTesting
Closes #33