OpenHands icon indicating copy to clipboard operation
OpenHands copied to clipboard

Add stricter type checking for conversation manager and utils

Open raymyers opened this issue 7 months ago • 2 comments

  • [ ] This change is worth documenting at https://docs.all-hands.dev/
  • [ ] Include this change in the Release Notes. If checked, you must provide an end-user friendly description for your change below

End-user friendly description of the problem this fixes or functionality this introduces.


Summarize what the PR does, explaining any non-trivial design decisions.

This configures mypy to require type annotations including return types on a subset of files in openhands.server. Requiring these should catching missing None checks such as this recent runtime exception:

openhands/server/conversation_manager/standalone_conversation_manager.py", line 136, in detach_from_conversation
    sid = conversation.sid
          ^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'sid'

Eventually, everything in openhands.server and anything we consider an integration point should probably have this enabled, but starting with a small change.


Link of any specific issues this addresses:


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:9e578e9-nikolaik   --name openhands-app-9e578e9   docker.all-hands.dev/all-hands-ai/openhands:9e578e9

raymyers avatar Jun 05 '25 08:06 raymyers

As expected, the CI linter catches these missing types now...

openhands/server/conversation_manager/conversation_manager.py:50: error: Function is missing a return type annotation  [no-untyped-def]
openhands/server/conversation_manager/conversation_manager.py:50: note: Use "-> None" if function does not return a value
openhands/server/conversation_manager/conversation_manager.py:54: error: Function is missing a type annotation  [no-untyped-def]
openhands/server/conversation_manager/conversation_manager.py:64: error: Function is missing a return type annotation  [no-untyped-def]
openhands/server/conversation_manager/conversation_manager.py:106: error: Function is missing a return type annotation  [no-untyped-def]
openhands/server/conversation_manager/conversation_manager.py:110: error: Function is missing a return type annotation  [no-untyped-def]
openhands/server/conversation_manager/conversation_manager.py:114: error: Function is missing a return type annotation  [no-untyped-def]
openhands/server/conversation_manager/standalone_conversation_manager.py:64: error: Function is missing a return type annotation  [no-untyped-def]
openhands/server/conversation_manager/standalone_conversation_manager.py:68: error: Function is missing a type annotation  [no-untyped-def]
openhands/server/conversation_manager/standalone_conversation_manager.py:135: error: Function is missing a return type annotation  [no-untyped-def]
openhands/server/conversation_manager/standalone_conversation_manager.py:1[47](https://github.com/All-Hands-AI/OpenHands/actions/runs/15462728466/job/43527634975?pr=8911#step:6:48): error: Function is missing a return type annotation  [no-untyped-def]
openhands/server/conversation_manager/standalone_conversation_manager.py:327: error: Function is missing a return type annotation  [no-untyped-def]
openhands/server/conversation_manager/standalone_conversation_manager.py:340: error: Function is missing a return type annotation  [no-untyped-def]
openhands/server/conversation_manager/standalone_conversation_manager.py:353: error: Function is missing a return type annotation  [no-untyped-def]
openhands/server/conversation_manager/standalone_conversation_manager.py:358: error: Function is missing a return type annotation  [no-untyped-def]
openhands/server/conversation_manager/standalone_conversation_manager.py:406: error: Function is missing a type annotation  [no-untyped-def]
openhands/server/conversation_manager/standalone_conversation_manager.py:418: error: Function is missing a return type annotation  [no-untyped-def]
openhands/server/conversation_manager/standalone_conversation_manager.py:418: error: Function is missing a type annotation for one or more arguments  [no-untyped-def]
openhands/server/conversation_manager/standalone_conversation_manager.py:474: error: Function is missing a return type annotation  [no-untyped-def]
openhands/server/conversation_manager/standalone_conversation_manager.py:[48](https://github.com/All-Hands-AI/OpenHands/actions/runs/15462728466/job/43527634975?pr=8911#step:6:49)6: error: Function is missing a return type annotation  [no-untyped-def]
openhands/server/conversation_manager/standalone_conversation_manager.py:[49](https://github.com/All-Hands-AI/OpenHands/actions/runs/15462728466/job/43527634975?pr=8911#step:6:50)4: error: Function is missing a return type annotation  [no-untyped-def]
openhands/server/conversation_manager/docker_nested_conversation_manager.py:59: error: Function is missing a return type annotation  [no-untyped-def]
openhands/server/conversation_manager/docker_nested_conversation_manager.py:59: note: Use "-> None" if function does not return a value
openhands/server/conversation_manager/docker_nested_conversation_manager.py:63: error: Function is missing a type annotation  [no-untyped-def]
openhands/server/conversation_manager/docker_nested_conversation_manager.py:73: error: Function is missing a return type annotation  [no-untyped-def]
openhands/server/conversation_manager/docker_nested_conversation_manager.py:144: error: Function is missing a return type annotation  [no-untyped-def]
openhands/server/conversation_manager/docker_nested_conversation_manager.py:185: error: Function is missing a return type annotation  [no-untyped-def]
openhands/server/conversation_manager/docker_nested_conversation_manager.py:274: error: Function is missing a return type annotation  [no-untyped-def]
openhands/server/conversation_manager/docker_nested_conversation_manager.py:278: error: Function is missing a return type annotation  [no-untyped-def]
openhands/server/conversation_manager/docker_nested_conversation_manager.py:282: error: Function is missing a return type annotation  [no-untyped-def]
openhands/server/conversation_manager/docker_nested_conversation_manager.py:285: error: Function is missing a type annotation  [no-untyped-def]
openhands/server/conversation_manager/docker_nested_conversation_manager.py:3[56](https://github.com/All-Hands-AI/OpenHands/actions/runs/15462728466/job/43527634975?pr=8911#step:6:57): error: Function is missing a return type annotation  [no-untyped-def]
openhands/server/conversation_manager/docker_nested_conversation_manager.py:366: error: Function is missing a return type annotation  [no-untyped-def]
openhands/server/conversation_manager/docker_nested_conversation_manager.py:398: error: Function is missing a return type annotation  [no-untyped-def]
openhands/server/conversation_manager/docker_nested_conversation_manager.py:408: error: Function is missing a return type annotation  [no-untyped-def]
openhands/server/utils.py:20: error: Function is missing a return type annotation  [no-untyped-def]

raymyers avatar Jun 05 '25 08:06 raymyers

Looks like there are a few issues preventing this PR from being merged!

  • GitHub Actions are failing:
    • Lint

If you'd like me to help, just leave a comment, like

@OpenHands please fix the failing actions on PR #8911

Feel free to include any additional details that might help me get this PR into a better state.

You can manage your notification settings

openhands-ai[bot] avatar Jun 05 '25 09:06 openhands-ai[bot]

Closed because the missing validation is better achieved by making sure the globals like conversation_manager had type anotations.

raymyers avatar Jun 12 '25 18:06 raymyers