Skip to content

fix(runtime): satisfy ty in runtime tests#529

Open
Subhajitdas99 wants to merge 1 commit into
GetBindu:mainfrom
Subhajitdas99:fix/runtime-tests-ty-cleanup
Open

fix(runtime): satisfy ty in runtime tests#529
Subhajitdas99 wants to merge 1 commit into
GetBindu:mainfrom
Subhajitdas99:fix/runtime-tests-ty-cleanup

Conversation

@Subhajitdas99
Copy link
Copy Markdown
Contributor

@Subhajitdas99 Subhajitdas99 commented May 9, 2026

Summary

  • Problem: Updated runtime tests fail ty due to stale type: ignore comments and nullable await_args access in mocked async calls. The runtime provider code also imports the optional boxd dependency, which ty could not resolve in the default test environment.
  • Why it matters: These typing issues block unrelated PRs in CI even when their own changes are clean.
  • What changed: Removed unused type: ignore directives, added explicit non-None assertions before accessing await_args.args / await_args.kwargs, and added minimal local boxd stub files so ty can resolve the optional runtime dependency during static analysis.
  • What did NOT change: No production runtime behavior changes, no CLI behavior changes, and no test logic changes beyond type-safety guards and type-checking stubs.

Change Type (select all that apply)

  • Bug fix
  • Feature
  • Refactor
  • Documentation
  • Security hardening
  • Tests
  • Chore/infra

Scope (select all touched areas)

  • Server / API endpoints
  • Extensions (DID, x402, etc.)
  • Storage backends
  • Scheduler backends
  • Observability / monitoring
  • Authentication / authorization
  • CLI / utilities
  • Tests
  • Documentation
  • CI/CD / infra

Linked Issue/PR

User-Visible / Behavior Changes

None.

Security Impact (required)

  • New permissions/capabilities? (Yes/No) No
  • Secrets/credentials handling changed? (Yes/No) No
  • New/changed network calls? (Yes/No) No
  • Database schema/migration changes? (Yes/No) No
  • Authentication/authorization changes? (Yes/No) No
  • If any Yes, explain risk + mitigation: None

Verification

Environment

  • OS: Windows 11
  • Python version: 3.12.9
  • Storage backend: Not applicable
  • Scheduler backend: Not applicable

Steps to Test

  1. Run uv run pre-commit run --all-files
  2. Confirm ty no longer reports unused type: ignore comments in the runtime test base file
  3. Confirm ty no longer reports nullable await_args access or unresolved boxd imports in the runtime provider/test paths

Expected Behavior

  • Pre-commit no longer fails on these runtime typing issues.
  • Test behavior remains unchanged.

Actual Behavior

  • Removed the stale suppressions, added explicit non-None guards, and provided minimal local boxd stubs for static analysis.
  • Pre-commit passed locally on commit.

Evidence (attach at least one)

  • Failing test before + passing after
  • Test output / logs
  • Screenshot / recording
  • Performance metrics (if relevant)

Human Verification (required)

  • Verified scenarios: Verified the affected runtime tests now guard nullable await_args access and no longer rely on unused type: ignore comments. Verified ty can resolve the optional boxd import path via local stubs.
  • Edge cases checked: Checked both log-streaming assertions and shell exec assertions where mocked await_args is read. Checked that the minimal stub surface covers the imported runtime symbols used by static analysis.
  • What you did NOT verify: Did not change or re-verify production runtime behavior against a live boxd installation, since this PR is focused on static-analysis/test-path cleanup.

Compatibility / Migration

  • Backward compatible? (Yes/No) Yes
  • Config/env changes? (Yes/No) No
  • Database migration needed? (Yes/No) No
  • If yes, exact upgrade steps: None

Failure Recovery (if this breaks)

  • How to disable/revert this change quickly: Revert this PR.
  • Files/config to restore:
    • tests/unit/runtime/test_base.py
    • tests/unit/runtime/test_boxd_provider.py
    • tests/unit/runtime/test_cli_shell_logs.py
    • boxd/__init__.pyi
    • boxd/aio.pyi
    • boxd/errors.pyi
  • Known bad symptoms reviewers should watch for: Unexpected test assertion failures if a mocked async call stops being awaited, or future boxd imports require additional stubbed symbols for static analysis.

Risks and Mitigations

  • Risk: The local boxd stubs may need expansion if additional optional SDK symbols are imported later.
    • Mitigation: Kept the stubs intentionally minimal and limited to the symbols currently referenced by ty in the runtime provider/tests.

Checklist

  • Tests pass (uv run pytest)
  • Pre-commit hooks pass (uv run pre-commit run --all-files)
  • Documentation updated (if needed)
  • Security impact assessed
  • Human verification completed
  • Backward compatibility considered

Summary by CodeRabbit

  • New Features

    • Added configuration structures for container setup, including proxy, lifecycle, and resource allocation options.
    • Added async context-manager support for compute operations.
    • Added NotFoundError exception type.
  • Tests

    • Enhanced test assertions with additional null-safety checks.
    • Simplified test implementations for provider instantiation.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 9, 2026

Review Change Stack
No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2c6d87d0-fbe9-4160-9dfe-0b3cb6e5bb65

📥 Commits

Reviewing files that changed from the base of the PR and between 23becac and 9d1d192.

📒 Files selected for processing (6)
  • boxd/__init__.pyi
  • boxd/aio.pyi
  • boxd/errors.pyi
  • tests/unit/runtime/test_base.py
  • tests/unit/runtime/test_boxd_provider.py
  • tests/unit/runtime/test_cli_shell_logs.py

📝 Walkthrough

Walkthrough

This PR introduces type stubs for box configuration, async compute management, and exception types, along with test refactoring to add null-safety assertions and remove type-ignore comments. Five new/updated files define configuration contracts and improve test reliability.

Changes

Box Configuration and Async Compute APIs

Layer / File(s) Summary
Configuration Data Contracts
boxd/__init__.pyi
ProxyEntry, NetworkConfig, LifecycleConfig, and BoxConfig form a composition hierarchy defining box resource configuration, networking, and lifecycle management.
Async Context Manager Interface
boxd/aio.pyi
Compute class implements async context-manager protocol with __aenter__ returning Compute and __aexit__ handling cleanup.
Exception Definitions
boxd/errors.pyi
NotFoundError exception type added as a public exception.
Test Code Cleanup and Simplification
tests/unit/runtime/test_base.py
Removes type: ignore comments from FakeProvider and simplifies method bodies to ellipsis form; RuntimeProvider instantiation assertion simplified.
Test Assertion Safeguards
tests/unit/runtime/test_boxd_provider.py, tests/unit/runtime/test_cli_shell_logs.py
Four test methods add explicit null checks for fake_box.exec.await_args before accessing command arguments in log-streaming and shell tests.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 Behold, a box of types so neat,
With proxies, lifecycle, and compute so sweet,
The tests now guard their null checks tight,
Type stubs defined, the contracts bright!
No more ignores—just clean code right. 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 23.53% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: fixing type-checking failures in runtime tests by satisfying the 'ty' type checker.
Description check ✅ Passed The description is comprehensive and follows the template structure with all major sections completed, including problem statement, change details, verification steps, security assessment, and checklist.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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.

1 participant