feat: add message rating feature

[pawelkiszczak]

Changes

Summary

Add a functionality of message rating in the template to further enhance its functionality.

Business Context

Strengthening open source offering.

Changes

• add rate message functionality and implement it across whole template
• add admin view with ratings, statistics

Verification

• manual tests
• unit tests
• GH Actions tests

Linked Issues

#56

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[DEENUU1]

Code Review: PR #59 — Message Rating Feature

PR: #59
Author: pawelkiszczak
Files changed: 50 | +5879 / -700
CI: Lint, Type Check, Security — pass. Tests pending.

---

Overview

Adds like/dislike rating on AI assistant messages across all 3 DB backends (PostgreSQL, SQLite, MongoDB). Includes admin dashboard with statistics/export, message_saved WS event, WebSocket auth refactor, and SQLite path change.

Architecture

Rating flow: RatingButtons (frontend) → POST /conversations/{id}/messages/{mid}/rate → MessageRatingService.rate_message() → message_rating_repo.create_rating() → MessageRating model.

Admin flow: admin/ratings page → GET /admin/ratings + /summary + /export → MessageRatingService.list_ratings() / get_summary().

The architecture follows project conventions (model → repo → service → schema → route) — good.

---

Issues by Severity

Critical

1. get_current_user_ws breaking change — returns None instead of raising

# Before: raised AuthenticationError
# After: returns None
async def get_current_user_ws(...) -> User | None:

Any existing WS endpoint using this dependency now silently receives None as user and continues execution. Only agent.py was updated to check if user is None: return. If someone added a custom WS endpoint relying on the old behavior, it silently breaks with AttributeError on first user.id access.

Suggestion: Either keep the exception (handle it with try/except in agent.py), or add a wrapper dependency get_required_user_ws() that raises if None.

---

2. admin_ratings.py — router = None fallback when JWT disabled

{%- else %}
# Admin ratings router - JWT not enabled
router = None  # type: ignore
{%- endif %}

Meanwhile in __init__.py:

{%- if cookiecutter.use_jwt %}
from app.api.routes.v1 import admin_ratings, auth, users
...
v1_router.include_router(admin_ratings.router, ...)
{%- endif %}

This is technically safe because the import is guarded by use_jwt, but the router = None in the module is a landmine — anyone importing the module directly or in tests will get AttributeError: 'NoneType' object has no attribute 'routes'. Better to always define router = APIRouter() (empty) instead of None.

---

High

3. Rate endpoint — awkward Response construction for 200 vs 201

@router.post(
    "/{conversation_id}/messages/{message_id}/rate",
    status_code=status.HTTP_201_CREATED,
)
async def rate_message(...) -> Any:
    rating, is_new = await rating_service.rate_message(...)
    if is_new:
        return rating
    else:
        return Response(
            content=rating.model_dump_json(),
            status_code=status.HTTP_200_OK,
            media_type="application/json",
        )

Problems:

• Manually constructing Response bypasses FastAPI's response model serialization
• No response_model validation on the 200 path
• Import of Response inside function body

Suggestion: Use PUT (idempotent upsert) with status_code=200 and return the rating. Or use separate POST (create) and PATCH (update) endpoints. If you must distinguish 201/200, use JSONResponse:

from fastapi.responses import JSONResponse

@router.put(
    "/{conversation_id}/messages/{message_id}/rate",
    response_model=MessageRatingRead,
)
async def rate_message(...):
    rating, is_new = await rating_service.rate_message(...)
    status_code = 201 if is_new else 200
    return JSONResponse(
        content=MessageRatingRead.model_validate(rating).model_dump(mode="json"),
        status_code=status_code,
    )

Or even simpler — just always return 200 since the client doesn't care about the distinction:

@router.put("/{conversation_id}/messages/{message_id}/rate", response_model=MessageRatingRead)
async def rate_message(...) -> MessageRatingRead:
    rating, _ = await rating_service.rate_message(...)
    return rating

---

4. Missing response_model=None on DELETE rating endpoint

@router.delete(
    "/{conversation_id}/messages/{message_id}/rate",
    status_code=status.HTTP_204_NO_CONTENT,
)
async def remove_rating(...) -> None:

This will fail with newer FastAPI versions (same bug we already fixed in other endpoints in this session). Needs response_model=None.

---

5. format parameter shadows Python builtin

@router.get("/export")
async def export_ratings(
    format: str = Query("json", ...),  # shadows builtin format()

Rename to export_format or output_format.

---

6. Export hardcoded limit of 10,000

items, _ = await rating_service.list_ratings(
    skip=0,
    limit=10000,  # Large limit for export
    ...
)

For production with high traffic, 10k ratings loaded into memory at once could cause issues. Consider streaming with cursor-based pagination or at least making the limit configurable.

---

7. type: ignore[attr-defined] — monkey-patching Message objects

for msg in items:
    msg.user_rating = user_ratings.get(msg.id)  # type: ignore[attr-defined]
    msg.rating_count = rating_counts.get(msg.id)  # type: ignore[attr-defined]

Setting arbitrary attributes on SQLAlchemy model instances is fragile. If the model uses __slots__ or strict attribute checking, this will break. Better approach: return a DTO/schema object instead of patching the ORM model, or add user_rating and rating_count as column_property / hybrid_property on the model.

---

Medium

8. requests dependency added but unused

# pyproject.toml
dependencies = [
    ...
    "requests>=2.33.0",  # <-- not used anywhere in the diff
]

Remove it or move to dev dependencies if needed for tests.

---

9. Duplicate CSV export code — 3x copy-paste

The CSV generation logic in admin_ratings.py is identical across PostgreSQL, SQLite, and MongoDB variants (~30 lines each). Extract a helper:

def _generate_csv_response(items: list[MessageRatingWithDetails]) -> StreamingResponse:
    output = StringIO()
    writer = csv.writer(output)
    writer.writerow(["ID", "Message ID", ...])
    for item in items:
        writer.writerow([str(item.id), ...])
    output.seek(0)
    return StreamingResponse(output, media_type="text/csv", ...)

---

10. MongoDB parse_obj deprecated

return MessageRating.parse_obj(result)  # Pydantic v1 method

Use MessageRating.model_validate(result) for Pydantic v2.

---

11. with_comments_only filter catches empty strings

if with_comments_only:
    query = query.where(MessageRating.comment.isnot(None))

This doesn't filter out empty strings (""). A user could submit comment="" and it would pass this filter. Add:

query = query.where(MessageRating.comment.isnot(None)).where(MessageRating.comment != "")

---

12. Rating value not constrained at DB level

rating: Mapped[int] = mapped_column(Integer, nullable=False)  # 1 or -1

Comment says 1 or -1 but nothing prevents inserting 0, 5, or -100. Add a CheckConstraint:

__table_args__ = (
    UniqueConstraint("message_id", "user_id", name="uq_message_user_rating"),
    CheckConstraint("rating IN (1, -1)", name="ck_rating_value"),
)

---

13. conversation_id inconsistency across DB backends

• MongoDB model stores conversation_id on MessageRating
• PostgreSQL/SQLite models do NOT store it (derive it through Message → Conversation join)
• But create_rating repo function accepts conversation_id parameter on all backends and silently ignores it on SQL backends

Either add the column to SQL models or remove the parameter from SQL repo functions.

---

14. Unused Boolean import

# SQLite model
from sqlalchemy import Boolean, ForeignKey, Integer, String, Text, UniqueConstraint
# Boolean is never used

---

15. .actrc change unrelated to feature

Switching from full-latest to act-latest for local CI runner is unrelated. Should be a separate commit.

---

Low

16. _sanitize_comment double-escapes on update

The validator runs on every MessageRatingCreate, including updates. If a user updates their rating, the comment gets HTML-escaped again (& → &). The sanitization should only run once on raw input.

---

17. Missing newline at end of message_rating.py schema

    ratings_by_day: list[dict[str, Any]]
\ No newline at end of file

---

18. datetime.now() without timezone in export headers

"Content-Disposition": f'...ratings_export_{datetime.now().strftime(...)}.csv"'

Use datetime.now(UTC) for consistency.

---

19. Admin ratings page — missing pagination

The list_ratings_admin endpoint supports pagination (skip, limit) but the frontend admin page should implement infinite scroll or page buttons. Currently unclear if it does from the diff.

---

20. RatingValue enum not enforced in MongoDB

# MongoDB model
rating: Literal[1, -1]

Good — uses Literal for validation. But SQL models use plain int. Consider using RatingValue enum from schemas in validation layer consistently.

---

Security

• Admin endpoints guarded by CurrentAdmin — good
• Rating ownership enforced (user can only rate/update/delete their own) — good
• Comment sanitization with XSS protection (html.escape) — good
• No SQL injection risk (uses ORM) — good
• Unique constraint prevents duplicate ratings — good
• response_model=None missing on DELETE

---

Positive Observations

• All 3 DB backends implemented consistently
• UniqueConstraint("message_id", "user_id") is the right approach
• Comment sanitization is thoughtful (strips control chars, HTML escapes, length limit)
• message_saved WS event enables real-time rating by persisted message ID
• Test coverage looks thorough (test_message_ratings.py)
• RatingValue enum prevents magic numbers in frontend
• Optimistic UI updates in RatingButtons component
• Feedback dialog for dislikes is good UX
• Daily breakdown in summary enables trend analysis
• Admin export (JSON + CSV) is practical

---

Summary

Severity	Count	Key Items
Critical	2	WS auth breaking change, router = None
High	5	Response construction, missing response_model=None, format shadow, 10k limit, monkey-patching
Medium	8	Unused dep, code duplication, deprecated API, filter bug, DB constraint
Low	5	Double-escape, missing newline, tz, pagination, enum consistency

Recommendation: Fix critical and high issues before merge. Medium items can be addressed in follow-up.

[DEENUU1]

Issues

1. rate_message endpoint — JSONResponse bypass still present

@router.post(
    "/{conversation_id}/messages/{message_id}/rate",
    response_model=MessageRatingRead,
    status_code=status.HTTP_200_OK,   # ← fixed to 200
)
async def rate_message(...) -> Any:
    from fastapi.responses import JSONResponse  # ← import inside function

    rating, is_new = await rating_service.rate_message(...)
    if is_new:
        return rating                                    # ← response_model applies
    else:
        return JSONResponse(                            # ← bypasses response_model
            content=rating.model_dump(mode="json"),
            status_code=status.HTTP_200_OK,             # ← same as default!
        )

Both branches now return 200, so the is_new check only adds confusion. The JSONResponse path bypasses FastAPI's response_model=MessageRatingRead serialization — fields not in MessageRatingRead won't be stripped, and fields with aliases won't be renamed. Additionally, from fastapi.responses import JSONResponse is imported inside the function body (repeated 3× for each DB backend).

Suggested fix — simplest correct version:

@router.post(
    "/{conversation_id}/messages/{message_id}/rate",
    response_model=MessageRatingRead,
)
async def rate_message(...) -> MessageRatingRead:
    rating, _ = await rating_service.rate_message(...)
    return rating

If you must distinguish 201 vs 200 (e.g. for clients that care), use JSONResponse for both paths:

from fastapi.responses import JSONResponse  # top-level import

@router.post("...", response_model=MessageRatingRead)
async def rate_message(...) -> Any:
    rating, is_new = await rating_service.rate_message(...)
    return JSONResponse(
        content=MessageRatingRead.model_validate(rating).model_dump(mode="json"),
        status_code=201 if is_new else 200,
    )

---

2. user_name attribute inconsistency — potential AttributeError

# In message_rating.py service (list_ratings / export_all_ratings):
user_name=item.user.full_name if item.user else None,  # ← .full_name

# In conversation.py export (export_all):
"user_name": getattr(user, "name", None),              # ← .name (with safe fallback)

The User model has either full_name or name — not both. These two paths are inconsistent. If the User model has name (not full_name), the service will silently return None for all user_name fields on the ratings list and export endpoints. If the model has neither, it raises AttributeError on line 3085.

The conversation.py export uses getattr(user, "name", None) which is safe. The service should do the same, or both should be aligned to the actual User model attribute.

---

3. Monkey-patching ORM models with type: ignore[attr-defined]

# conversation.py — list_messages enrichment (PostgreSQL, SQLite)
for msg in items:
    msg.user_rating = user_ratings.get(msg.id)  # type: ignore[attr-defined]
    msg.rating_count = rating_counts.get(msg.id)  # type: ignore[attr-defined]

This is still present in both PostgreSQL and SQLite variants. Setting arbitrary attributes on SQLAlchemy ORM instances works (SQLAlchemy models don't use __slots__), but:

• The attributes are ephemeral and not tracked by SA — if the object is accidentally passed back into a session-managed operation, the extra attrs will be lost silently.
• SA's inspection tools (e.g., inspect(msg)) won't see these fields.
• The # type: ignore masks the issue rather than solving it.

MessageRead.user_rating and MessageRead.rating_count fields were correctly added to the schema, but the ORM-to-schema conversion still relies on patching the ORM object instead of constructing the schema directly.

Better approach: Return MessageRead objects directly from list_messages (not ORM instances) when rating enrichment is needed:

result = []
for msg in items:
    msg_schema = MessageRead.model_validate(msg)
    msg_schema.user_rating = user_ratings.get(msg.id)
    msg_schema.rating_count = rating_counts.get(msg.id)
    result.append(msg_schema)
return result, total

Or add user_rating and rating_count as transient mapped attributes on the Message model.

---

4. MongoDB with_comments count in summary is inconsistent

# list_ratings query (fixed in this PR):
query_dict["comment"] = {"$nin": [None, ""]}   # ← correct, filters empty strings

# get_rating_summary pipeline (NOT fixed):
"with_comments": {
    "$sum": {"$cond": [{"$ne": ["$comment", None]}, 1, 0]}  # ← misses ""
},

The list_ratings query correctly excludes empty strings when with_comments_only=True, but the summary's with_comments counter counts ratings where comment != None, including comment = "". The two counts will disagree when empty-string comments exist.

Fix:

"with_comments": {
    "$sum": {"$cond": [{"$and": [
        {"$ne": ["$comment", None]},
        {"$ne": ["$comment", ""]}
    ]}, 1, 0]}
},

---

5. db.expunge(user) — residual DetachedInstanceError risk

# deps.py — get_current_user_ws (PostgreSQL + SQLite variants)
db.expunge(user)
return user

This correctly fixes the "instance not bound to a Session" error after the context manager exits. However, if any WS handler accesses a lazy-loaded relationship on the returned user object (e.g., user.conversations, or any backref), it will raise DetachedInstanceError — a different but equally confusing error.

The current agent.py usage only accesses user.id and user.is_active, so this is safe today. But it's a latent risk for future handlers.

Consider eager-loading all needed attributes before expunge:

# Ensure eager-load of commonly-accessed fields
await db.refresh(user)  # loads all mapped columns
db.expunge(user)
return user

Or use make_transient(user) from sqlalchemy.orm to make the object fully independent (no SA tracking whatsoever).

6. .actrc change still in PR

The switch from full-latest to act-latest for the local CI runner is a developer tooling change unrelated to the message rating feature. Should be a separate commit. Minor nit.

7. from fastapi.responses import JSONResponse inside function body

This import appears inside the function body in conversations.py across all three DB backend variants. Top-level imports are the Python convention and avoid the overhead of re-resolving the import on every request (minor, but unnecessary).

8. Export chunk iteration — SQLite async def returns sync generator

# admin_ratings.py SQLite export endpoint:
async def export_ratings(...) -> Any:           # async def
    chunks = rating_service.export_all_ratings(...)  # sync generator
    return _generate_export_response(list(chunks), export_format)

The list(chunks) call on a sync generator inside async def works but blocks the event loop while iterating. Since SQLite is already synchronous, this is acceptable — but worth noting for future migration.

9. Admin ratings frontend pagination

The list_ratings_admin endpoint supports skip/limit (max 100), but the admin frontend page likely loads with the defaults (skip=0, limit=50). If the page doesn't implement pagination controls (infinite scroll or page buttons), admins with > 50 ratings will see a truncated list. The export endpoint covers the full dataset, but the list view should too.

[DEENUU1]

1. user.full_name — inconsistency with User model (carried from R2-2)

In message_rating.py service (PostgreSQL, SQLite, MongoDB):

user_name=item.user.full_name if item.user else None,

And in conversation.py export:

"user_name": user.full_name if user else None,

Problem: it's unclear whether the User model has a full_name attribute. If the User model defines name instead of full_name, this will cause an AttributeError at runtime. This issue was reported in Review #2 and is still present — usage is consistent (full_name everywhere), but the User model must be verified to confirm full_name exists.

Risk: Runtime AttributeError on admin endpoints (ratings list, export, conversations export).

Fix: Verify the User model. If it has name instead of full_name — fix it. If it has full_name — issue closed.

---

2. Token proxy endpoint exposes access token as JSON

// frontend/src/app/api/auth/token/route.ts
export async function GET(request: NextRequest) {
  const accessToken = request.cookies.get("access_token")?.value;
  // ... validates with backend ...
  return NextResponse.json({ access_token: accessToken });
}

This endpoint converts an httpOnly cookie into a JSON response readable by JavaScript. This effectively negates the purpose of the httpOnly cookie. Any XSS on the frontend can now extract the token by calling fetch("/api/auth/token").

Suggestion: Instead of exposing the token:

• Use a server-side WebSocket proxy (Next.js middleware), or
• Add CSRF token validation on this endpoint, or
• At minimum, add a comment explaining the tradeoff and restrict access (e.g., Origin header check)

Note: This may be a deliberate tradeoff — WebSocket API requires a token in URL/header, and httpOnly cookies are not accessible from JS. But it should be explicitly documented.

3. conversationId on ChatMessage is required when JWT enabled, but initialized as empty string

// types/chat.ts
export interface ChatMessage {
  // ...
  conversationId: string;  // required, not optional
}

// use-chat.ts
conversationId: effectiveConversationId,  // can be "" (empty string)

A new message in a new conversation doesn't have a conversationId yet (the conversation is created on the backend after the first message). Rating buttons check !conversationId || conversationId === "" and disable themselves — this is OK. But:

• conversationId: string should be conversationId?: string (optional) instead of forcing empty string
• An empty string is not a valid UUID, which makes type safety weaker

Minor issue, but causes unnecessary complexity in RatingButtons.

---

4. message_saved event — fallback heuristic for finding messages is fragile

// use-chat.ts — fallback when currentMessageId is null
updateMessagesWhere(
  (msg) => msg.role === "assistant" && msg.id.length > 20 && !msg.id.includes("-"),
  (msg) => ({ ...msg, id: message_id })
);

The heuristic msg.id.length > 20 && !msg.id.includes("-") tries to distinguish nanoid from UUID. Problems:

• nanoid's default alphabet includes - (A-Za-z0-9_-), so nanoid IDs can contain dashes
• This logic may misidentify the wrong message

Fix: Use a dedicated flag (e.g., isTemporaryId: true) instead of heuristics on ID format.

---

5. Admin conversations page loads ALL conversations at once

// admin/conversations/page.tsx
const data = await apiClient.get<ConversationExport>("/v1/admin/conversations");

The frontend calls /v1/admin/conversations -> proxy to /v1/conversations/export which loads up to 10,000 conversations with messages into memory. Under production load (e.g., 5000 conversations x 20 messages) this could be hundreds of MB in a single JSON response.

Suggestion: Add a dedicated admin list endpoint with pagination (without messages), instead of reusing the export endpoint.

---

6. Export proxy in Next.js — backendFetch may parse CSV as JSON

// frontend/src/app/api/v1/admin/ratings/export/route.ts
const data = await backendFetch(url, { ... });
// ...
if (export_format === "csv") {
  return new NextResponse(data as string, { ... });
}

backendFetch likely calls response.json() internally. If the backend returns CSV (content-type text/csv), then response.json() will throw a parse error. The implementation of backendFetch needs to be verified — it may need a raw text mode for CSV.

---

7. Empty rating_filter sent as empty string instead of omitted

// admin/ratings/page.tsx
fetch(`/api/v1/admin/ratings?...&rating_filter=${
  filter === "all" ? "" : filter === "positive" ? "1" : "-1"
}&with_comments_only=${commentsOnly}`)

When filter === "all", the URL contains rating_filter= (empty string). Backend defines rating_filter: int | None = Query(None). FastAPI may interpret the empty string as "" and throw a validation error instead of treating it as None.

Fix: Don't add the parameter to the URL when filter is "all":

const params = new URLSearchParams();
if (filter !== "all") params.set("rating_filter", filter === "positive" ? "1" : "-1");

---

8. recharts dependency not added to package.json

Admin ratings page imports:

import { BarChart, Bar, XAxis, YAxis, Tooltip, ResponsiveContainer, CartesianGrid } from "recharts";

I don't see recharts added to package.json in the diff. It may already be there — needs verification.

---

9. Enrichment logic duplication in conversation.py (3 backends)

Message enrichment with ratings (user_rating, rating_count) is duplicated across PostgreSQL, SQLite, and MongoDB variants of list_messages(). That's ~40 lines of code per backend. Not critical (template architecture may require this), but worth noting the maintenance cost.

---

10. ConversationMessage.conversation_id not defined in types

// chat-container.tsx
conversationId: msg.conversation_id,  // <-- where does this field come from?

ConversationMessage in types/conversation.ts does not define conversation_id. This field needs to be added to the interface, or read from the conversation context.

---

11. Export via window.open relies on cookie-based auth

// admin/ratings/page.tsx
window.open(`/api/v1/admin/ratings/export?${params.toString()}`, "_blank");

window.open doesn't send custom headers. The Next.js proxy endpoint reads the cookie, so this should work because the browser sends cookies automatically. But it's worth verifying with CORS/SameSite settings in production.

[pawelkiszczak]

R3-1

User model has a full_name attribute, therefore it is obsolete.

full_name: str | None = Field(default=None, max_length=255)

R3-2

Added SECURITY NOTE explaining the trade-off plus an Origin header check.

R3-3

Changed conversationId?: string to be optional in interface.

R3-4

Replaced msg.id.length with isTemporaryId. Fallback matcher now checks !!msg.isTemporaryId.

R3-5

Added this functionality in latest commit.

R3-6

Added raw?: boolean option to fetch method. If raw: true returns raw text instead of JSON.parse. Export route uses raw: export_format === 'csv'

R3-7

Main fetch uses URLSearchParams and only adds rating_filter when filter is not all. Handler then deletes param if empty.

R3-8

Already present in package.json.

R3-9

Architectural decision due to Jinja templating. Will not change since it's like this by design.

R3-10

conversationId: string present in types/conversation.ts.

R3-11

Commet added in ratings/page.tsx, no structural change. Works because Next.js proxy reads cookie on the server side.

[DEENUU1]

Critical

4.1 LIKE Injection in admin conversation search (PostgreSQL/SQLite)

File: template/.../backend/app/repositories/conversation.py (both async and sync variants)

if search:
    query = query.where(
        (Conversation.title.ilike(f"%{search}%"))
        | Conversation.id.cast(String).ilike(f"{search}%")
    )

SQLAlchemy properly parameterizes the value (no SQL injection), but LIKE-specific wildcards % and _ are not escaped. An admin user could:

• Search % to match everything (information disclosure / performance)
• Craft %a%a%a%a%a%a%a% patterns causing expensive backtracking on large datasets

The same pattern appears in the count query below it.

Fix:

safe_search = search.replace("%", r"\%").replace("_", r"\_")
query = query.where(
    (Conversation.title.ilike(f"%{safe_search}%"))
    | Conversation.id.cast(String).ilike(f"{safe_search}%")
)

Severity note: This is admin-only, so the blast radius is limited. Downgrade to High if admin users are trusted.

---

4.2 MongoDB regex injection in admin conversation search

File: template/.../backend/app/repositories/conversation.py (MongoDB variant)

match_filter["$or"] = [
    {"title": {"$regex": search, "$options": "i"}},
    {"_id_str": {"$regex": f"^{search}", "$options": "i"}},
]

Raw user input goes directly into a MongoDB $regex operator. A malicious admin could:

• Submit .* to match everything
• Submit catastrophic backtracking patterns like (a+)+$ causing ReDoS (server-side)

Same issue in the count pipeline below.

Fix:

import re
safe_search = re.escape(search)
match_filter["$or"] = [
    {"title": {"$regex": safe_search, "$options": "i"}},
    {"_id_str": {"$regex": f"^{safe_search}", "$options": "i"}},
]

---

High

4.3 No conversation ownership validation on rate/remove endpoints

Files: template/.../backend/app/api/routes/v1/conversations.py, template/.../backend/app/services/message_rating.py

The rate_message and remove_rating endpoints accept conversation_id and message_id as path params. The service validates that the message belongs to the conversation (_validate_message_in_conversation), but never validates that the conversation belongs to current_user.

This means User A can rate messages in User B's private conversation if they know (or guess) the conversation_id and message_id (both UUIDs, so hard to guess — but not impossible with the admin conversations API leaking IDs, or via browser history).

Present in all 3 database backends.

Fix: Add ownership check in service:

async def _validate_conversation_ownership(
    self, conversation_id: UUID, user_id: UUID
) -> None:
    conv = await conversation_repo.get_conversation(self.db, conversation_id)
    if not conv:
        raise NotFoundError(...)
    if conv.user_id != user_id:
        raise NotFoundError(message="Conversation not found")

Or add the check at the route level before calling the service.

---

4.4 Token proxy origin check is bypassable

File: template/.../frontend/src/app/api/auth/token/route.ts

const origin = request.headers.get("origin");
const host = request.headers.get("host");
if (origin && host && !origin.includes(host)) {
    return NextResponse.json({ error: "Forbidden" }, { status: 403 });
}

origin.includes(host) uses substring matching. If host is app.com, then evil-app.com passes the check. The SECURITY NOTE added per R3-2 documents the tradeoff (good), but the origin check itself is flawed.

Fix:

if (origin && host) {
    try {
        const originUrl = new URL(origin);
        if (originUrl.host !== host) {
            return NextResponse.json({ error: "Forbidden" }, { status: 403 });
        }
    } catch {
        return NextResponse.json({ error: "Forbidden" }, { status: 403 });
    }
}

---

Medium

4.5 updateMessagesWhere fallback in message_saved can still overwrite multiple messages

File: template/.../frontend/src/hooks/use-chat.ts

R3-4 was fixed (replaced heuristic with isTemporaryId flag — good). But the fallback path still uses updateMessagesWhere:

// Fallback: find the most recent assistant message with a temp ID
const { updateMessagesWhere } = useChatStore.getState();
updateMessagesWhere(
    (msg) => msg.role === "assistant" && !!msg.isTemporaryId,
    (msg) => ({ ...msg, id: message_id, isTemporaryId: false })
);

If the user sends messages rapidly and multiple assistant responses have isTemporaryId: true, all of them get the same message_id. This breaks React key uniqueness and makes subsequent ratings go to the wrong message.

Fix: Only update the last matching message:

const messages = useChatStore.getState().messages;
const lastTemp = [...messages].reverse().find(
    msg => msg.role === "assistant" && msg.isTemporaryId
);
if (lastTemp) {
    updateMessage(lastTemp.id, (msg) => ({
        ...msg, id: message_id, isTemporaryId: false
    }));
}

---

4.6 URL parameter mismatch between docs and code

File: template/.../docs/howto/use-ratings.md vs frontend code

Documentation says:

http://localhost:3000/chat?c=550e8400-e29b-41d4-a716-446655440000

But the actual code uses ?id=:

• chat/page.tsx: searchParams.get("id")
• Admin conversations link: href={'/chat?id=${conv.id}'}
• Admin ratings link: href={'/chat?id=${rating.conversation_id}'}

Fix: Update docs to use ?id= or update code to use ?c=. Pick one, apply everywhere.

---

4.7 MessageRating model imported unconditionally in models/__init__.py

File: template/.../backend/app/db/models/__init__.py

{%- set _ = models.append("MessageRating") %}
from app.db.models.message_rating import MessageRating

Not wrapped in {%- if cookiecutter.use_jwt %}. In practice JWT is always enabled in this template so this is harmless, but it's inconsistent with other conditional imports in the same file. If a future change makes JWT optional, this will break.

---

4.8 Conversation export still uses hardcoded 10k limit (carried from R1-6)

File: template/.../backend/app/services/conversation.py

items, _ = await self.list_conversations(skip=0, limit=10000, include_archived=True)

Ratings export was fixed to use chunked generator (EXPORT_CHUNK_SIZE = 5000). Conversation export still loads up to 10k conversations with all their messages into memory at once. For consistency and scalability, this should use chunking too.

---

Low

4.9 from typing import Any repeated 3 times in admin_ratings.py

{%- if cookiecutter.use_postgresql %}
from typing import Any
from uuid import UUID
{%- elif cookiecutter.use_mongodb %}
from typing import Any
{%- else %}
from typing import Any
{%- endif %}

Move from typing import Any before the conditional block.

---

4.10 Optional[str] vs str | None style inconsistency in MongoDB model

File: template/.../backend/app/db/models/message_rating.py (MongoDB)

from typing import Optional
comment: Optional[str] = None
updated_at: Optional[datetime] = None

Rest of the PR uses str | None. Minor style nit.

---

4.11 Unused String import in PostgreSQL+SQLModel model variant

from sqlalchemy import CheckConstraint, Column, ForeignKey, Integer, String, Text, UniqueConstraint

String is imported but not used in the PostgreSQL+SQLModel variant (uses PG_UUID, not String(36)).

---

4.12 alembic/env.py includes unrelated model imports

The PR adds imports for Conversation, Message, ToolCall, ChatFile, RAGDocument, SyncLog, SyncSource, Session, Webhook, WebhookDelivery — most unrelated to the rating feature. These were likely needed to fix Alembic autogenerate, but should be mentioned in the PR description as a side-fix.