OpenHands icon indicating copy to clipboard operation
OpenHands copied to clipboard

Replace all occurrences of sid with conversation_id

Open rbren opened this issue 6 months ago • 9 comments

This PR replaces all occurrences of sid with conversation_id throughout the codebase, focusing on the openhands/, evaluation/, and tests/ directories. This makes the naming more consistent and descriptive.

Changes:

  • Renamed sid to conversation_id in all Python files
  • Renamed generate_sid function to generate_conversation_id
  • Updated all imports and usages accordingly
  • Fixed mypy and ruff errors
  • Fixed test failures by correcting mock side_effect attribute

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

rbren avatar May 28 '25 13:05 rbren

@openhands update these tests

================================== FAILURES ===================================
_______________________ test_exception_during_execution _______________________

windows_bash_session = <openhands.runtime.utils.windows_bash.WindowsPowershellSession object at 0x000001B17F6F4B90>

    def test_exception_during_execution(windows_bash_session):
        """Test handling of exceptions during command execution."""
        # Patch the PowerShell class itself within the module where it's used
        patch_target = 'openhands.runtime.utils.windows_bash.PowerShell'
    
        # Create a mock PowerShell class
        mock_powershell_class = MagicMock()
        # Configure its Create method (which is called in execute) to raise an exception
        # This simulates an error during the creation of the PowerShell object itself.
        mock_powershell_class.Create.conversation_ide_effect = Exception(
            'Test exception from mocked Create'
        )
    
        with patch(patch_target, mock_powershell_class):
            action = CmdRunAction(command="Write-Output 'Test'")
            # Now, when execute calls PowerShell.Create(), it will hit our mock and raise the exception
>           result = windows_bash_session.execute(action)

tests\unit\test_windows_bash.py:352: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
openhands\runtime\utils\windows_bash.py:1141: in execute
    current_cwd = self._get_current_cwd()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <openhands.runtime.utils.windows_bash.WindowsPowershellSession object at 0x000001B17F6F4B90>

    def _get_current_cwd(self) -> str:
        """Gets the current working directory from the runspace."""
        # Use helper to run Get-Location
        results = self._run_ps_command('Get-Location')
    
        # --- Add more detailed check logging ---
>       if results and results.Count > 0:  # type: ignore[attr-defined]
E       TypeError: '>' not supported between instances of 'MagicMock' and 'int'

openhands\runtime\utils\windows_bash.py:579: TypeError
=========================== short test summary info ===========================
FAILED tests/unit/test_windows_bash.py::test_exception_during_execution - TypeError: '>' not supported between instances of 'MagicMock' and 'int'
================== 1 failed, 21 passed in 147.52s (0:02:27) ===================
Error: Process completed with exit code 1.

rbren avatar May 28 '25 14:05 rbren

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

openhands-ai[bot] avatar May 28 '25 14:05 openhands-ai[bot]

I successfully fixed the failing test in the PR #8753 on the replace-sid-with-conversation-id branch.

Issue Identified

The test was failing with a TypeError because there was a typo in the test code. The test was trying to use conversation_ide_effect instead of the correct side_effect method when configuring the mock PowerShell class.

Changes Made

  1. Fixed the typo in test_exception_during_execution by changing:

    mock_powershell_class.Create.conversation_ide_effect = Exception(...)
    

    to:

    mock_powershell_class.Create.side_effect = Exception(...)
    
  2. Committed the change with a descriptive commit message

  3. Pushed the changes to the remote branch

While we couldn't directly verify the test passes (since it's skipped on non-Windows platforms, which is expected behavior), the fix addresses the TypeError that was occurring in the test.

The changes have been successfully pushed to the replace-sid-with-conversation-id branch.

openhands-ai[bot] avatar May 28 '25 14:05 openhands-ai[bot]

@openhands fix the merge conflicts. There are a lot of them--fix them ALL

rbren avatar May 28 '25 19:05 rbren

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

openhands-ai[bot] avatar May 28 '25 19:05 openhands-ai[bot]

OpenHands encountered an error: ****.

See the conversation for more information.

openhands-ai[bot] avatar May 28 '25 20:05 openhands-ai[bot]

@openhands fix the failed tests

[gw1] FAILED tests/unit/resolver/github/test_send_pull_request.py::test_send_pull_request[ready-None-Custom PR Title] 
tests/unit/resolver/github/test_send_pull_request.py::test_send_pull_request[draft-develop-Another Custom Title] 
[gw0] PASSED tests/unit/frontend/test_translation_completeness.py::TestTranslationCompleteness::test_translation_completeness_check_runs 
tests/unit/resolver/github/test_guess_success.py::test_guess_success_multiline_explanation 
[gw0] PASSED tests/unit/resolver/github/test_guess_success.py::test_guess_success_multiline_explanation 
tests/unit/resolver/github/test_guess_success.py::test_pr_handler_guess_success_with_thread_comments 
[gw2] FAILED tests/unit/resolver/gitlab/test_gitlab_send_pull_request.py::test_send_pull_request[branch-None-None] 
tests/unit/resolver/gitlab/test_gitlab_send_pull_request.py::test_send_pull_request[draft-None-None] 
[gw0] PASSED tests/unit/resolver/github/test_guess_success.py::test_pr_handler_guess_success_with_thread_comments 
tests/unit/resolver/github/test_guess_success.py::test_pr_handler_guess_success_only_review_comments 
[gw0] PASSED tests/unit/resolver/github/test_guess_success.py::test_pr_handler_guess_success_only_review_comments 
tests/unit/resolver/github/test_guess_success.py::test_pr_handler_guess_success_no_comments 
[gw0] PASSED tests/unit/resolver/github/test_guess_success.py::test_pr_handler_guess_success_no_comments 
tests/unit/resolver/github/test_issue_handler.py::test_get_converted_issues_initializes_review_comments 
[gw1] FAILED tests/unit/resolver/github/test_send_pull_request.py::test_send_pull_request[draft-develop-Another Custom Title] 
tests/unit/resolver/github/test_send_pull_request.py::test_send_pull_request_with_reviewer 
[gw2] FAILED tests/unit/resolver/gitlab/test_gitlab_send_pull_request.py::test_send_pull_request[draft-None-None] 
tests/unit/resolver/gitlab/test_gitlab_send_pull_request.py::test_send_pull_request[ready-None-None] 
[gw0] FAILED tests/unit/resolver/github/test_issue_handler.py::test_get_converted_issues_initializes_review_comments 
tests/unit/resolver/github/test_issue_handler.py::test_get_converted_issues_handles_empty_body 
[gw1] FAILED tests/unit/resolver/github/test_send_pull_request.py::test_send_pull_request_with_reviewer 
tests/unit/resolver/github/test_send_pull_request.py::test_send_pull_request_target_branch_with_fork 
[gw3] PASSED tests/unit/test_agent_controller.py::test_run_controller_with_fatal_error 
tests/unit/test_agent_controller.py::test_run_controller_stop_with_stuck 
[gw2] FAILED tests/unit/resolver/gitlab/test_gitlab_send_pull_request.py::test_send_pull_request[ready-None-None] 
tests/unit/resolver/gitlab/test_gitlab_send_pull_request.py::test_send_pull_request[branch-feature-None] 
[gw0] FAILED tests/unit/resolver/github/test_issue_handler.py::test_get_converted_issues_handles_empty_body 
tests/unit/resolver/github/test_issue_handler.py::test_pr_handler_get_converted_issues_with_comments 
[gw1] FAILED tests/unit/resolver/github/test_send_pull_request.py::test_send_pull_request_target_branch_with_fork 
tests/unit/resolver/github/test_send_pull_request.py::test_send_pull_request_target_branch_with_additional_message 
[gw2] FAILED tests/unit/resolver/gitlab/test_gitlab_send_pull_request.py::test_send_pull_request[branch-feature-None] 
tests/unit/resolver/gitlab/test_gitlab_send_pull_request.py::test_send_pull_request[draft-develop-None] 
[gw1] FAILED tests/unit/resolver/github/test_send_pull_request.py::test_send_pull_request_target_branch_with_additional_message 
tests/unit/resolver/github/test_send_pull_request.py::test_send_pull_request_invalid_target_branch 
[gw1] PASSED tests/unit/resolver/github/test_send_pull_request.py::test_send_pull_request_invalid_target_branch 
tests/unit/resolver/github/test_send_pull_request.py::test_send_pull_request_git_push_failure 
[gw2] FAILED tests/unit/resolver/gitlab/test_gitlab_send_pull_request.py::test_send_pull_request[draft-develop-None] 
tests/unit/resolver/gitlab/test_gitlab_send_pull_request.py::test_send_pull_request[ready-staging-None] 
[gw0] FAILED tests/unit/resolver/github/test_issue_handler.py::test_pr_handler_get_converted_issues_with_comments 
tests/unit/resolver/github/test_issue_handler.py::test_get_issue_comments_with_specific_comment_id 
[gw0] PASSED tests/unit/resolver/github/test_issue_handler.py::test_get_issue_comments_with_specific_comment_id 
tests/unit/resolver/github/test_issue_handler.py::test_pr_handler_get_converted_issues_with_specific_thread_comment 
[gw1] FAILED tests/unit/resolver/github/test_send_pull_request.py::test_send_pull_request_git_push_failure 
tests/unit/resolver/github/test_send_pull_request.py::test_send_pull_request_permission_error 
[gw2] FAILED tests/unit/resolver/gitlab/test_gitlab_send_pull_request.py::test_send_pull_request[ready-staging-None] 
tests/unit/resolver/gitlab/test_gitlab_send_pull_request.py::test_send_pull_request[ready-None-Custom PR Title] 
[gw3] PASSED tests/unit/test_agent_controller.py::test_run_controller_stop_with_stuck 
tests/unit/test_agent_controller.py::test_max_iterations_extension 
[gw0] FAILED tests/unit/resolver/github/test_issue_handler.py::test_pr_handler_get_converted_issues_with_specific_thread_comment 
tests/unit/resolver/github/test_issue_handler.py::test_pr_handler_get_converted_issues_with_specific_review_thread_comment 
[gw1] FAILED tests/unit/resolver/github/test_send_pull_request.py::test_send_pull_request_permission_error 
tests/unit/resolver/github/test_send_pull_request.py::test_reply_to_comment 
[gw3] PASSED tests/unit/test_agent_controller.py::test_max_iterations_extension 
tests/unit/test_agent_controller.py::test_step_max_budget 
[gw1] PASSED tests/unit/resolver/github/test_send_pull_request.py::test_reply_to_comment 
tests/unit/resolver/github/test_send_pull_request.py::test_process_single_pr_update 
[gw2] FAILED tests/unit/resolver/gitlab/test_gitlab_send_pull_request.py::test_send_pull_request[ready-None-Custom PR Title] 
tests/unit/resolver/gitlab/test_gitlab_send_pull_request.py::test_send_pull_request[draft-develop-Another Custom Title] 

rbren avatar May 28 '25 20:05 rbren

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

openhands-ai[bot] avatar May 28 '25 20:05 openhands-ai[bot]

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

  • GitHub Actions are failing:
    • Run Python Unit Tests
  • There are merge conflicts

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

@OpenHands please fix the merge conflicts on PR #8753

or

@OpenHands please fix the failing actions on PR #8753

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 May 30 '25 09:05 openhands-ai[bot]