Skip to content

user feeback validation changes LCORE-197 #93

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
Jun 13, 2025
Merged

user feeback validation changes LCORE-197 #93

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
b25af3f
user feeback validation changes
omkarjoshi0304 Jun 11, 2025
cff6715
Update main.py
omkarjoshi0304 Jun 12, 2025
bb14bac
Fix feedback field validation, remove test data, align with code qual…
omkarjoshi0304 Jun 13, 2025
c74db01
Update main.py
omkarjoshi0304 Jun 13, 2025
8f1ea75
Remove data/ from .gitignore
omkarjoshi0304 Jun 13, 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
10 changes: 8 additions & 2 deletions src/models/requests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

from typing import Optional, Self

from pydantic import BaseModel, model_validator, field_validator
from pydantic import BaseModel, model_validator, field_validator, Field
from llama_stack_client.types.agents.turn_create_params import Document

from utils import suid
Expand Down Expand Up @@ -159,7 +159,13 @@ class FeedbackRequest(BaseModel):
user_question: str
llm_response: str
sentiment: Optional[int] = None
user_feedback: Optional[str] = None
# Optional user feedback limited to 1–4096 characters to prevent abuse.
user_feedback: Optional[str] = Field(
default=None,
max_length=4096,
description="Feedback on the LLM response.",
examples=["I'm not satisfied with the response because it is too vague."],
)

# provides examples for /docs endpoint
model_config = {
Expand Down
14 changes: 13 additions & 1 deletion tests/unit/models/test_requests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import pytest

from pydantic import ValidationError
Copy link

@lpiwowar lpiwowar Jun 12, 2025

Choose a reason for hiding this comment

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

I would keep the empty line here:).

Suggested change
from pydantic import ValidationError
import pytest
from pydantic import ValidationError
from models.requests import QueryRequest, Attachment, FeedbackRequest

From PEP-8:

Imports should be grouped in the following order:

    1. Standard library imports.
    2. Related third party imports.
    3. Local application/library specific imports.

You should put a blank line between each group of imports.

https://peps.python.org/pep-0008/#imports

Copy link
Contributor

@tisnik tisnik Jun 12, 2025

Choose a reason for hiding this comment

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

thanks @lpiwowar for review! Agree with your comments

from models.requests import QueryRequest, Attachment, FeedbackRequest


Expand Down Expand Up @@ -176,3 +176,15 @@ def test_check_sentiment_or_user_feedback(self) -> None:
sentiment=None,
user_feedback=None,
)

def test_feedback_too_long(self) -> None:
"""Test that user feedback is limited to 4096 characters."""
with pytest.raises(
ValidationError, match="should have at most 4096 characters"
):
FeedbackRequest(
conversation_id="12345678-abcd-0000-0123-456789abcdef",
user_question="What is this?",
llm_response="Some response",
user_feedback="a" * 4097,
)