Merge branch 'main' into openhands-fix-issue-5320

openhands/runtime/impl/remote/remote_runtime.py

@@ -10,7 +10,6 @@ import requests
 import tenacity
 
 from openhands.core.config import AppConfig
-from openhands.core.logger import openhands_logger as logger
 from openhands.events import EventStream
 from openhands.events.action import (
     BrowseInteractiveAction,

openhands/server/data_models/feedback.py

@@ -1,5 +1,5 @@
 import json
-from typing import Any, Literal, Optional
+from typing import Any, Literal
 
 import requests
 from pydantic import BaseModel
@@ -15,7 +15,10 @@ class FeedbackDataModel(BaseModel):
         'positive', 'negative'
     ]  # TODO: remove this, its here for backward compatibility
     permissions: Literal['public', 'private']
-    trajectory: Optional[list[dict[str, Any]]]
+    trajectory: list[dict[str, Any]] | None
+    model: str | None = None
+    provider: str | None = None
+    agent: str | None = None
 
 
 FEEDBACK_URL = 'https://share-od-trajectory-3u9bw9tx.uc.gateway.dev/share_od_trajectory'

openhands/server/routes/feedback.py

@@ -41,6 +41,13 @@ async def submit_feedback(request: Request):
     trajectory = []
     async for event in async_stream:
         trajectory.append(event_to_dict(event))
+
+    # Get model and agent info from config
+    llm_config = request.state.conversation.config.get_llm_config()
+    model = llm_config.model
+    provider = llm_config.custom_llm_provider or ''
+    agent = request.state.conversation.config.default_agent
+
     feedback = FeedbackDataModel(
         email=body.get('email', ''),
         version=body.get('version', ''),
@@ -48,6 +55,9 @@ async def submit_feedback(request: Request):
         polarity=body.get('polarity', ''),
         feedback=body.get('polarity', ''),
         trajectory=trajectory,
+        model=model,
+        provider=provider,
+        agent=agent,
     )
     try:
         feedback_data = await call_sync_from_async(store_feedback, feedback)

openhands/server/session/README.md

@@ -8,19 +8,19 @@ interruptions are recoverable.
 There are 3 main server side event handlers:
 
 * `connect` - Invoked when a new connection to the server is established. (This may be via http or WebSocket)
-* `oh_action` - Invoked when a connected client sends an event (Such as `INIT` or a prompt for the Agent) - 
+* `oh_action` - Invoked when a connected client sends an event (Such as `INIT` or a prompt for the Agent) -
    this is distinct from the `oh_event` sent from the server to the client.
 * `disconnect` - Invoked when a connected client disconnects from the server.
 
 ## Init
 Each connection has a unique id, and when initially established, is not associated with any session. An
-`INIT` event must be sent to the server in order to attach a connection to a session. The `INIT` event 
-may optionally include a GitHub token and a token to connect to an existing session. (Which may be running 
-locally or may need to be hydrated). If no token is received as part of the init event, it is assumed a 
+`INIT` event must be sent to the server in order to attach a connection to a session. The `INIT` event
+may optionally include a GitHub token and a token to connect to an existing session. (Which may be running
+locally or may need to be hydrated). If no token is received as part of the init event, it is assumed a
 new session should be started.
 
 ## Disconnect
-The (manager)[manager.py] manages connections and sessions. Each session may have zero or more connections 
+The (manager)[manager.py] manages connections and sessions. Each session may have zero or more connections
 associated with it, managed by invocations of `INIT` and disconnect. When a session no longer has any
 connections associated with it, after a set amount of time (determined by `config.sandbox.close_delay`),
-the session and runtime are passivated (So will need to be rehydrated to continue.) 
+the session and runtime are passivated (So will need to be rehydrated to continue.)

openhands/server/session/session.py

@@ -8,7 +8,6 @@ from openhands.core.config import AppConfig
 from openhands.core.const.guide_url import TROUBLESHOOTING_URL
 from openhands.core.logger import openhands_logger as logger
 from openhands.core.schema import AgentState
-from openhands.core.schema.action import ActionType
 from openhands.core.schema.config import ConfigType
 from openhands.events.action import MessageAction, NullAction
 from openhands.events.event import Event, EventSource
@@ -23,7 +22,6 @@ from openhands.events.stream import EventStreamSubscriber
 from openhands.llm.llm import LLM
 from openhands.server.session.agent_session import AgentSession
 from openhands.storage.files import FileStore
-from openhands.utils.async_utils import call_coro_in_bg_thread
 
 ROOM_KEY = 'room:{sid}'
 

poetry.lock

@@ -7145,6 +7145,23 @@ files = [
 py = "*"
 pytest = ">=3.10"
 
+[[package]]
+name = "pytest-mock"
+version = "3.14.0"
+description = "Thin-wrapper around the mock package for easier use with pytest"
+optional = false
+python-versions = ">=3.8"
+files = [
+    {file = "pytest-mock-3.14.0.tar.gz", hash = "sha256:2719255a1efeceadbc056d6bf3df3d1c5015530fb40cf347c0f9afac88410bd0"},
+    {file = "pytest_mock-3.14.0-py3-none-any.whl", hash = "sha256:0b72c38033392a5f4621342fe11e9219ac11ec9d375f8e2a0c164539e0d70f6f"},
+]
+
+[package.dependencies]
+pytest = ">=6.2.5"
+
+[package.extras]
+dev = ["pre-commit", "pytest-asyncio", "tox"]
+
 [[package]]
 name = "pytest-xdist"
 version = "3.6.1"

pyproject.toml

@@ -81,6 +81,7 @@ ruff = "0.8.0"
 mypy = "1.13.0"
 pre-commit = "4.0.1"
 build = "*"
+pytest-mock = "^3.14.0"
 
 [tool.poetry.group.test.dependencies]
 pytest = "*"

tests/unit/test_feedback.py

@@ -0,0 +1,71 @@
+import pytest
+from fastapi.responses import JSONResponse
+
+from openhands.core.config.agent_config import AgentConfig
+from openhands.core.config.app_config import AppConfig
+from openhands.core.config.llm_config import LLMConfig
+from openhands.events.stream import EventStream
+from openhands.server.data_models.feedback import FeedbackDataModel
+from openhands.server.routes.feedback import submit_feedback
+from openhands.storage.memory import InMemoryFileStore
+
+
+class MockConversation:
+    def __init__(self, config: AppConfig, event_stream: EventStream):
+        self.config = config
+        self.event_stream = event_stream
+
+
+class MockRequest:
+    def __init__(self, body: dict, conversation: MockConversation):
+        self.body = body
+        self.state = type('State', (), {'conversation': conversation})
+
+    async def json(self):
+        return self.body
+
+
+@pytest.mark.asyncio
+async def test_submit_feedback_includes_model_agent_info(mocker):
+    # Mock the store_feedback function
+    mock_store_feedback = mocker.patch(
+        'openhands.server.routes.feedback.store_feedback',
+        return_value={'message': 'Success', 'feedback_id': '123', 'password': 'pass'},
+    )
+
+    # Create a mock config with model and agent info
+    config = AppConfig()
+    config.llms['llm'] = LLMConfig(model='gpt-4', custom_llm_provider='openai')
+    config.agents['test-agent'] = AgentConfig()
+    config.default_agent = 'test-agent'
+
+    # Create a mock event stream
+    event_stream = EventStream('test-sid', InMemoryFileStore())
+
+    # Create a mock conversation
+    conversation = MockConversation(config, event_stream)
+
+    # Create a mock request
+    request = MockRequest(
+        {
+            'email': 'test@example.com',
+            'version': '1.0',
+            'permissions': 'private',
+            'polarity': 'positive',
+        },
+        conversation,
+    )
+
+    # Call submit_feedback
+    response = await submit_feedback(request)
+
+    # Verify the response
+    assert isinstance(response, JSONResponse)
+    assert response.status_code == 200
+
+    # Verify store_feedback was called with correct data
+    mock_store_feedback.assert_called_once()
+    feedback_model: FeedbackDataModel = mock_store_feedback.call_args[0][0]
+    assert feedback_model.model == 'gpt-4'
+    assert feedback_model.provider == 'openai'
+    assert feedback_model.agent == 'test-agent'