Skip to content
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

Remove TaskAction functionality #5959

Merged
merged 4 commits into from
Jan 2, 2025
Merged

Remove TaskAction functionality #5959

merged 4 commits into from
Jan 2, 2025

Conversation

rbren
Copy link
Collaborator

@rbren rbren commented Jan 1, 2025

This PR removes all TaskAction-related code from the codebase, including:

  • Removed tasks.py file containing AddTaskAction and ModifyTaskAction classes
  • Removed task action imports and references from:
    • openhands/events/action/__init__.py
    • openhands/events/serialization/action.py
    • openhands/controller/agent_controller.py
    • frontend/src/types/core/actions.ts
    • tests/unit/test_action_serialization.py
    • openhands/agenthub/dummy_agent/agent.py

All tests pass and the code has been formatted according to the project standards.

This should fix the issue where main appears to be broken by the command poetry run python3 openhands/core/main.py -t "do a flip" -d ./workspace/ -c DummyAgent


To run this PR locally, use the following command:

docker run -it --rm   -p 3000:3000   -v /var/run/docker.sock:/var/run/docker.sock   --add-host host.docker.internal:host-gateway   -e SANDBOX_RUNTIME_CONTAINER_IMAGE=docker.all-hands.dev/all-hands-ai/runtime:3935dfc-nikolaik   --name openhands-app-3935dfc   docker.all-hands.dev/all-hands-ai/openhands:3935dfc

@enyst
Copy link
Collaborator

enyst commented Jan 1, 2025

Two notes:

  • there are more occurrences, some of those here, such as the schema/action.py file
  • but one reference is in the planner agent's prompt:

by using the add_task and modify_task actions described below[...]

FWIW the planner agent did well in a manual test a few days ago.

Edited to add: it can only be tested manually currently.

Copy link
Collaborator

@tofarr tofarr left a comment

Choose a reason for hiding this comment

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

🍰 (Once Engle's notes are addressed) - But I think we probably need a follow up PR at some point to check our test coverage

@tofarr tofarr self-requested a review January 1, 2025 22:31
@rbren
Copy link
Collaborator Author

rbren commented Jan 2, 2025

Good catch Engel!

I think it makes sense to ditch the PlannerAgent for now. We can reintroduce it if we make planning a first-class concept again

@rbren rbren enabled auto-merge (squash) January 2, 2025 14:27
@enyst
Copy link
Collaborator

enyst commented Jan 2, 2025

Good catch Engel!

I think it makes sense to ditch the PlannerAgent for now. We can reintroduce it if we make planning a first-class concept again

There is this PR in progress, I hope it will allow us a cool alternative way of doing it.

@rbren rbren merged commit f846b31 into main Jan 2, 2025
15 checks passed
@rbren rbren deleted the remove-task-action branch January 2, 2025 15:11
SmartManoj added a commit to SmartManoj/Kevin that referenced this pull request Jan 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants