Skip to content

types(acp): add missing parameter type annotations#754

Merged
enyst merged 2 commits into
mainfrom
fix/code-quality/add-return-type-hints
May 25, 2026
Merged

types(acp): add missing parameter type annotations#754
enyst merged 2 commits into
mainfrom
fix/code-quality/add-return-type-hints

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot commented May 25, 2026

Summary

Add type annotations to parameters that were missing types in the ACP module.

Changes

  • 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 via getattr)

Related Issue

Addresses findings from #751

Testing

  • Linting passes (make lint)
  • Tests pass (make test - 1353 tests)

This PR was automatically generated by the Code Quality Report workflow.


🚀 Try this PR

uvx --python 3.12 git+https://github.com/OpenHands/OpenHands-CLI.git@fix/code-quality/add-return-type-hints

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>
@enyst
Copy link
Copy Markdown
Member

enyst commented May 25, 2026

@OpenHands make an empty commit to kick off CI and /iterate to green and approved by the AI reviewer from pr-review workflow.

@openhands-ai
Copy link
Copy Markdown

openhands-ai Bot commented May 25, 2026

I'm on it! enyst can track my progress at all-hands.dev

Co-authored-by: openhands <openhands@all-hands.dev>
@enyst enyst requested a review from all-hands-bot May 25, 2026 12:58
Copy link
Copy Markdown
Collaborator

@all-hands-bot all-hands-bot left a comment

Choose a reason for hiding this comment

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

🟢 Good taste - Clean, focused type annotation additions.

Both annotations are correct:

  • IO[bytes] for stdout is appropriate for the file-like object used with fileno()
  • Any for tool_call is justified given the dynamic attribute access via getattr

[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

@github-actions
Copy link
Copy Markdown
Contributor Author

Coverage

Coverage Report •
FileStmtsMissCoverMissing
openhands_cli/acp_impl/events
   token_streamer.py1251191%149, 161, 165, 168, 173, 175, 177, 179, 244, 251, 271
TOTAL7062101185% 

@openhands-ai
Copy link
Copy Markdown

openhands-ai Bot commented May 25, 2026

Since my last summary, there have been no additional changes or actions.

  • ✅ The requested PR work was fully addressed: an empty commit was pushed, CI was triggered, checks went green, and the AI reviewer approved the PR.
  • ✅ All instructions were followed faithfully, including using the GitHub API/CLI, pushing only to the PR branch, and verifying final status.
  • ✅ The changes were concise: only the requested empty commit was added; no code or unrelated files were modified.
  • ✅ No extraneous changes need to be reverted.

@enyst enyst merged commit 154a983 into main May 25, 2026
13 checks passed
@enyst enyst deleted the fix/code-quality/add-return-type-hints branch May 25, 2026 13:25
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.

2 participants