types(acp): add missing parameter type annotations#754
Conversation
Add type annotations to parameters that were missing types in the ACP module: - test_utils.py: Add IO[bytes] type annotation for stdout parameter in UnbufferedJsonRpcReader.__init__() - token_streamer.py: Add Any type annotation for tool_call parameter in _handle_tool_call_streaming() (uses dynamic attribute access) Addresses item from code quality report. Closes #751 Co-authored-by: openhands <openhands@all-hands.dev>
|
@OpenHands make an empty commit to kick off CI and /iterate to green and approved by the AI reviewer from pr-review workflow. |
|
I'm on it! enyst can track my progress at all-hands.dev |
Co-authored-by: openhands <openhands@all-hands.dev>
all-hands-bot
left a comment
There was a problem hiding this comment.
🟢 Good taste - Clean, focused type annotation additions.
Both annotations are correct:
IO[bytes]for stdout is appropriate for the file-like object used withfileno()Anyfor tool_call is justified given the dynamic attribute access viagetattr
[RISK ASSESSMENT]
- [Overall PR]
⚠️ Risk Assessment: 🟢 LOW
Type annotations in Python are purely for static analysis and have zero runtime impact. No behavior changes, no breaking changes, no security implications.
VERDICT:
✅ Worth merging: Improves type safety with zero risk.
KEY INSIGHT:
Using Any for dynamically-accessed objects is the pragmatic choice that preserves type checking elsewhere without forcing rigid type definitions where they don't fit.
Was this automated review useful? React with 👍 or 👎 to this review to help us measure review quality.
Workflow run: https://github.com/OpenHands/OpenHands-CLI/actions/runs/26401712227
|
Since my last summary, there have been no additional changes or actions.
|
Summary
Add type annotations to parameters that were missing types in the ACP module.
Changes
test_utils.py: AddIO[bytes]type annotation forstdoutparameter inUnbufferedJsonRpcReader.__init__()token_streamer.py: AddAnytype annotation fortool_callparameter in_handle_tool_call_streaming()(uses dynamic attribute access viagetattr)Related Issue
Addresses findings from #751
Testing
make lint)make test- 1353 tests)This PR was automatically generated by the Code Quality Report workflow.
🚀 Try this PR