Skip to content

[RHDHPAI-1027] Add dynamic feedback status update endpoint #458

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Aug 29, 2025
Merged

[RHDHPAI-1027] Add dynamic feedback status update endpoint #458

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6a48436
add endpoint for toggling feedback enablement for life of service
Jdubrick Aug 25, 2025
25c4203
update format and apply linter to feedback toggle
Jdubrick Aug 26, 2025
8ecd41f
add new endpoint to openapi json
Jdubrick Aug 27, 2025
ec5e7a7
move to PUT endpoint to set feedback status
Jdubrick Aug 28, 2025
d683993
update description to denote per-worker changes
Jdubrick Aug 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 99 additions & 0 deletions docs/openapi.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,46 @@
}
}
}
},
"put": {
"tags": [
"feedback"
],
"summary": "Update Feedback Status",
"description": "Handle feedback status update requests.\n\nTakes a request with the desired state of the feedback status.\nReturns the updated state of the feedback status based on the request's value.\nThese changes are for the life of the service and are on a per-worker basis.\n\nReturns:\n StatusResponse: Indicates whether feedback is enabled.",
"operationId": "update_feedback_status_v1_feedback_status_put",
"requestBody": {
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/FeedbackStatusUpdateRequest"
}
}
},
"required": true
},
"responses": {
"200": {
"description": "Successful Response",
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/FeedbackStatusUpdateResponse"
}
}
}
},
"422": {
"description": "Validation Error",
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/HTTPValidationError"
}
}
}
}
}
}
},
"/v1/conversations": {
Expand Down Expand Up @@ -712,6 +752,7 @@
"title": "Actions"
}
},
"additionalProperties": false,
"type": "object",
"required": [
"role",
Expand Down Expand Up @@ -843,6 +884,7 @@
]
}
},
"additionalProperties": false,
"type": "object",
"title": "AuthenticationConfiguration",
"description": "Authentication configuration."
Expand All @@ -857,6 +899,7 @@
"title": "Access Rules"
}
},
"additionalProperties": false,
"type": "object",
"title": "AuthorizationConfiguration",
"description": "Authorization configuration."
Expand Down Expand Up @@ -933,6 +976,7 @@
]
}
},
"additionalProperties": false,
"type": "object",
"title": "CORSConfiguration",
"description": "CORS configuration."
Expand Down Expand Up @@ -1000,6 +1044,7 @@
"default": {}
}
},
"additionalProperties": false,
"type": "object",
"required": [
"name",
Expand Down Expand Up @@ -1223,6 +1268,7 @@
"title": "System Prompt"
}
},
"additionalProperties": false,
"type": "object",
"title": "Customization",
"description": "Service customization."
Expand Down Expand Up @@ -1250,6 +1296,7 @@
]
}
},
"additionalProperties": false,
"type": "object",
"title": "DatabaseConfiguration",
"description": "Database configuration."
Expand Down Expand Up @@ -1446,6 +1493,47 @@
}
]
},
"FeedbackStatusUpdateRequest": {
"properties": {
"status": {
"type": "boolean",
"title": "Status",
"description": "Desired state of feedback enablement, must be False or True",
"default": false,
"examples": [
true,
false
]
}
},
"type": "object",
"title": "FeedbackStatusUpdateRequest",
"description": "Model representing a feedback status update request.\n\nAttributes:\n status: Value of the desired feedback enabled state.\n\nExample:\n ```python\n feedback_request = FeedbackRequest(\n status=false\n )\n ```"
},
"FeedbackStatusUpdateResponse": {
"properties": {
"status": {
"additionalProperties": true,
"type": "object",
"title": "Status"
}
},
"type": "object",
"required": [
"status"
],
"title": "FeedbackStatusUpdateResponse",
"description": "Model representing a response to a feedback status update request.\n\nAttributes:\n status: The previous and current status of the service and who updated it.\n\nExample:\n ```python\n status_response = StatusResponse(\n status={\n \"previous_status\": true,\n \"updated_status\": false,\n \"updated_by\": \"user/test\"\n },\n )\n ```",
"examples": [
{
"status": {
"previous_status": true,
"updated_by": "user/test",
"updated_status": false
}
}
]
},
Comment on lines +1513 to +1536
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 28, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Constrain response schema to explicit fields.

Publish stable keys for clients.

 "FeedbackStatusUpdateResponse": {
     "properties": {
         "status": {
-            "additionalProperties": true,
-            "type": "object",
-            "title": "Status"
+            "type": "object",
+            "title": "Status",
+            "additionalProperties": false,
+            "properties": {
+                "previous_status": { "type": "boolean" },
+                "updated_status":  { "type": "boolean" },
+                "updated_by":      { "type": "string" }
+            },
+            "required": ["previous_status", "updated_status", "updated_by"]
         }
     },
     "type": "object",
     "required": [
         "status"
     ],
     "title": "FeedbackStatusUpdateResponse",
-    "description": "Model representing a response to a feedback status update request.\n\nAttributes:\n    status: The previous and current status of the service and who updated it.\n\nExample:\n    ```python\n    status_response = StatusResponse(\n        status={\n            \"previous_status\": true,\n            \"updated_status\": false,\n            \"updated_by\": \"user/test\"\n        },\n    )\n    ```",
+    "description": "Model representing a response to a feedback status update request.\n\nAttributes:\n    status: The previous and current status of the service and who updated it.\n\nExample:\n    ```python\n    status_response = FeedbackStatusUpdateResponse(\n        status={\n            \"previous_status\": true,\n            \"updated_status\": false,\n            \"updated_by\": \"user/test\"\n        }\n    )\n    ```",
     "examples": [
         {
             "status": {
                 "previous_status": true,
                 "updated_by": "user/test",
                 "updated_status": false
             }
         }
     ]
 },
🤖 Prompt for AI Agents 
In docs/openapi.json around lines 1506 to 1529, the FeedbackStatusUpdateResponse
currently allows arbitrary keys (additionalProperties: true) for the status
object and has an inconsistent example name; change the status schema to
enumerate explicit properties (previous_status: boolean, updated_status:
boolean, updated_by: string), mark them required as appropriate, remove
additionalProperties, and update the description/example to reference
FeedbackStatusUpdateResponse consistently so clients have a stable, validated
response shape.

"ForbiddenResponse": {
"properties": {
"detail": {
Expand Down Expand Up @@ -1503,6 +1591,7 @@
"title": "Default Provider"
}
},
"additionalProperties": false,
"type": "object",
"title": "InferenceConfiguration",
"description": "Inference configuration."
Expand Down Expand Up @@ -1569,6 +1658,7 @@
}
}
},
"additionalProperties": false,
"type": "object",
"required": [
"url"
Expand Down Expand Up @@ -1596,6 +1686,7 @@
"title": "Role Rules"
}
},
"additionalProperties": false,
"type": "object",
"title": "JwtConfiguration",
"description": "JWT configuration."
Expand Down Expand Up @@ -1625,6 +1716,7 @@
"title": "Roles"
}
},
"additionalProperties": false,
"type": "object",
"required": [
"jsonpath",
Expand Down Expand Up @@ -1701,6 +1793,7 @@
"title": "Library Client Config Path"
}
},
"additionalProperties": false,
"type": "object",
"title": "LlamaStackConfiguration",
"description": "Llama stack configuration."
Expand All @@ -1721,6 +1814,7 @@
"title": "Url"
}
},
"additionalProperties": false,
"type": "object",
"required": [
"name",
Expand Down Expand Up @@ -1828,6 +1922,7 @@
"title": "Ca Cert Path"
}
},
"additionalProperties": false,
"type": "object",
"required": [
"db",
Expand Down Expand Up @@ -2131,6 +2226,7 @@
"title": "Db Path"
}
},
"additionalProperties": false,
"type": "object",
"required": [
"db_path"
Expand Down Expand Up @@ -2192,6 +2288,7 @@
}
}
},
"additionalProperties": false,
"type": "object",
"title": "ServiceConfiguration",
"description": "Service configuration."
Expand Down Expand Up @@ -2263,6 +2360,7 @@
"title": "Tls Key Password"
}
},
"additionalProperties": false,
"type": "object",
"title": "TLSConfiguration",
"description": "TLS configuration."
Expand Down Expand Up @@ -2321,6 +2419,7 @@
"title": "Transcripts Storage"
}
},
"additionalProperties": false,
"type": "object",
"title": "UserDataCollection",
"description": "User data collection configuration."
Expand Down
44 changes: 43 additions & 1 deletion src/app/endpoints/feedback.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""Handler for REST API endpoint for user feedback."""

import logging
import threading
from typing import Annotated, Any
from pathlib import Path
import json
Expand All @@ -12,10 +13,11 @@
from authorization.middleware import authorize
from configuration import configuration
from models.config import Action
from models.requests import FeedbackRequest
from models.requests import FeedbackRequest, FeedbackStatusUpdateRequest
from models.responses import (
ErrorResponse,
FeedbackResponse,
FeedbackStatusUpdateResponse,
StatusResponse,
UnauthorizedResponse,
ForbiddenResponse,
Expand All @@ -25,6 +27,7 @@
logger = logging.getLogger(__name__)
router = APIRouter(prefix="/feedback", tags=["feedback"])
auth_dependency = get_auth_dependency()
feedback_status_lock = threading.Lock()

# Response for the feedback endpoint
feedback_response: dict[int | str, dict[str, Any]] = {
Expand Down Expand Up @@ -174,3 +177,42 @@ def feedback_status() -> StatusResponse:
return StatusResponse(
functionality="feedback", status={"enabled": feedback_status_enabled}
)


@router.put("/status")
@authorize(Action.ADMIN)
async def update_feedback_status(
feedback_update_request: FeedbackStatusUpdateRequest,
auth: Annotated[AuthTuple, Depends(auth_dependency)],
) -> FeedbackStatusUpdateResponse:
"""
Handle feedback status update requests.

Takes a request with the desired state of the feedback status.
Returns the updated state of the feedback status based on the request's value.
These changes are for the life of the service and are on a per-worker basis.

Returns:
StatusResponse: Indicates whether feedback is enabled.
"""
user_id, _, _ = auth
requested_status = feedback_update_request.get_value()

with feedback_status_lock:
previous_status = (
configuration.user_data_collection_configuration.feedback_enabled
)
configuration.user_data_collection_configuration.feedback_enabled = (
requested_status
)
updated_status = (
configuration.user_data_collection_configuration.feedback_enabled
)

return FeedbackStatusUpdateResponse(
status={
"previous_status": previous_status,
"updated_status": updated_status,
"updated_by": user_id,
}
)
25 changes: 25 additions & 0 deletions src/models/requests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,3 +380,28 @@ def check_feedback_provided(self) -> Self:
"'sentiment', 'user_feedback', or 'categories'"
)
return self


class FeedbackStatusUpdateRequest(BaseModel):
"""Model representing a feedback status update request.

Attributes:
status: Value of the desired feedback enabled state.

Example:
```python
feedback_request = FeedbackRequest(
status=false
)
```
"""

status: bool = Field(
False,
description="Desired state of feedback enablement, must be False or True",
examples=[True, False],
)

def get_value(self) -> bool:
"""Return the value of the status attribute."""
return self.status
37 changes: 37 additions & 0 deletions src/models/responses.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -570,3 +570,40 @@ class ErrorResponse(BaseModel):
]
}
}


class FeedbackStatusUpdateResponse(BaseModel):
"""
Model representing a response to a feedback status update request.

Attributes:
status: The previous and current status of the service and who updated it.

Example:
```python
status_response = StatusResponse(
status={
"previous_status": true,
"updated_status": false,
"updated_by": "user/test"
},
)
```
"""

status: dict

# provides examples for /docs endpoint
model_config = {
"json_schema_extra": {
"examples": [
{
"status": {
"previous_status": True,
"updated_status": False,
"updated_by": "user/test",
},
}
]
}
}
Comment on lines +575 to +609
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 28, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Make response shape explicit; fix example class name and booleans.

  • Status should be a typed object, not a free-form dict.
  • Docstring example references StatusResponse and uses lowercase true/false; should reference FeedbackStatusUpdateResponse and use Python booleans.

Apply:

-class FeedbackStatusUpdateResponse(BaseModel):
+class FeedbackStatusUpdateResponse(BaseModel):
@@
-    Example:
-        ```python
-        status_response = StatusResponse(
-            status={
-                "previous_status": true,
-                "updated_status": false,
-                "updated_by": "user/test"
-            },
-        )
-        ```
+    Example:
+        ```python
+        status_response = FeedbackStatusUpdateResponse(
+            status={
+                "previous_status": True,
+                "updated_status": False,
+                "updated_by": "user/test",
+            }
+        )
+        ```
@@
-    status: dict
+    status: "FeedbackStatusDelta"

Add this model (top-level in this file):

class FeedbackStatusDelta(BaseModel):
    previous_status: bool = Field(..., description="State before the update")
    updated_status: bool = Field(..., description="State after the update")
    updated_by: str = Field(..., description="User ID of the updater")
🤖 Prompt for AI Agents 
In src/models/responses.py around lines 575-609, the
FeedbackStatusUpdateResponse currently types status as dict and its
docstring/example incorrectly names StatusResponse and uses lowercase booleans;
replace the free-form dict with a concrete FeedbackStatusDelta model, update the
docstring example to use FeedbackStatusUpdateResponse and Python booleans, and
update the status annotation to FeedbackStatusDelta. Add a top-level
FeedbackStatusDelta BaseModel with previous_status: bool, updated_status: bool,
and updated_by: str (using Field with descriptions) and ensure Field (and
BaseModel) are imported where needed. Update the json_schema_extra example to
use True/False and the new class name.

Loading