Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion backend/app/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
from app.models.chat import ChatMessage, FileAttachment, UrlAttachment, ChatConversation
from app.models.activity import ActivityEvent
from app.models.library import LibraryFolder, LibraryItem, Library
from app.models.feedback import ChatFeedback, ExtractionQualityRecord
from app.models.feedback import ChatFeedback, ExtractionQualityRecord, ProductFeedback
from app.models.verification import VerificationRequest, VerifiedItemMetadata, VerifiedCollection
from app.models.office import IntakeConfig, WorkItem
from app.models.automation import Automation
Expand Down Expand Up @@ -82,6 +82,7 @@
Library,
ChatFeedback,
ExtractionQualityRecord,
ProductFeedback,
VerificationRequest,
VerifiedItemMetadata,
VerifiedCollection,
Expand Down
3 changes: 2 additions & 1 deletion backend/app/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
from app.middleware.csrf import CSRFMiddleware
from app.observability import init_sentry
from app.rate_limit import limiter
from app.routers import activity, admin, audit, auth, automations, browser_automation, certification, chat, config, credentials, demo, documents, extractions, feedback, feedback_prompt, files, folders, graph_webhooks, knowledge, library, mgmt, notifications, office, optimizer_inbox, organizations, projects, reviews, spaces, support, teams, telemetry, verification, workflows
from app.routers import activity, admin, audit, auth, automations, browser_automation, certification, chat, config, credentials, demo, documents, extractions, feedback, feedback_admin, feedback_prompt, files, folders, graph_webhooks, knowledge, library, mgmt, notifications, office, optimizer_inbox, organizations, projects, reviews, spaces, support, teams, telemetry, verification, workflows


@lru_cache
Expand Down Expand Up @@ -211,6 +211,7 @@ async def send_with_headers(message: Message) -> None:
app.include_router(activity.router, prefix="/api/activity", tags=["activity"])
app.include_router(library.router, prefix="/api/library", tags=["library"])
app.include_router(feedback.router, prefix="/api/feedback", tags=["feedback"])
app.include_router(feedback_admin.router, prefix="/api/feedback/admin", tags=["feedback-admin"])
app.include_router(verification.router, prefix="/api/verification", tags=["verification"])
app.include_router(office.router, prefix="/api/office", tags=["office"])
app.include_router(automations.router, prefix="/api/automations", tags=["automations"])
Expand Down
33 changes: 33 additions & 0 deletions backend/app/models/feedback.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,36 @@ class ExtractionQualityRecord(Document):

class Settings:
name = "extraction_quality_record"


class ProductFeedback(Document):
"""Free-form, non-negative product feedback that isn't tied to a single
chat message or extraction — a user telling us what's working, or an idea.

Negatives deliberately stay on the support-ticket rails (they need triage,
assignment, and a status that can close). This collection is the home for
positive/idea signal so it stops dying write-only: the "What's Working"
admin surface reads it alongside thumbs-up chat feedback and high extraction
star ratings.
"""

# "positive" = something worked well; "idea" = a suggestion offered warmly,
# not a defect report. No "negative" — that path creates a support ticket.
sentiment: str = "positive"
message: str
# Where it was captured, so the admin feed can group by surface and we can
# add new capture points without schema churn (e.g. "support_panel").
source: str = "support_panel"
# Optional feature attribution when the surface knows it (e.g. "chat",
# "extraction"). Free-form on purpose — capture points vary.
feature: Optional[str] = None
user_id: Optional[str] = None
team_id: Optional[str] = None
created_at: datetime.datetime = Field(default_factory=datetime.datetime.utcnow)

class Settings:
name = "product_feedback"
indexes = [
"sentiment",
[("created_at", -1)],
]
31 changes: 30 additions & 1 deletion backend/app/routers/feedback.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

from app.dependencies import get_current_user
from app.models.user import User
from app.models.feedback import ChatFeedback, ExtractionQualityRecord
from app.models.feedback import ChatFeedback, ExtractionQualityRecord, ProductFeedback

router = APIRouter()

Expand Down Expand Up @@ -87,6 +87,35 @@ async def _maybe_trigger_kb_shadow_run(kb_uuid: str, user_id: str) -> None:
)


class ProductFeedbackRequest(BaseModel):
message: str = Field(min_length=1, max_length=4000)
# Only non-negative sentiments ride this path — a problem belongs on a
# support ticket (triage, assignment, a status that closes), not here.
sentiment: str = "positive"
source: str = "support_panel"
feature: Optional[str] = None


@router.post("/product")
async def submit_product_feedback(
req: ProductFeedbackRequest, user: User = Depends(get_current_user)
):
"""Capture free-form positive / idea feedback. Deliberately does NOT create
a support ticket, so praise never enters the triage queue or triggers the
ticket email/Teams fan-out. It surfaces in the 'What's Working' admin feed."""
sentiment = req.sentiment if req.sentiment in ("positive", "idea") else "positive"
record = ProductFeedback(
message=req.message.strip(),
sentiment=sentiment,
source=req.source,
feature=req.feature,
user_id=user.user_id,
team_id=str(user.current_team) if user.current_team else None,
)
await record.insert()
return {"complete": True}


@router.post("/chat")
async def submit_chat_feedback(req: ChatFeedbackRequest, user: User = Depends(get_current_user)):
record = ChatFeedback(
Expand Down
145 changes: 145 additions & 0 deletions backend/app/routers/feedback_admin.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
"""Admin read surface for positive feedback — the "What's Working" feed.

Positive signal used to be write-only: chat thumbs-up, high extraction star
ratings, and (now) ProductFeedback all landed in Mongo and were never read by a
human. This router is the read side that gives that signal a home. It unifies
three sources into one reverse-chronological feed plus a small stats rollup, so
support agents can finally see what users love — the mirror image of the
thumbs-down path that already drives KB optimization.

Support users / admins only, mirroring ``/api/support/stats``.
"""

import datetime
from typing import Optional

from fastapi import APIRouter, Depends, HTTPException, Query

from app.dependencies import get_current_user
from app.models.feedback import ChatFeedback, ExtractionQualityRecord, ProductFeedback
from app.models.user import User
from app.services import support_service

router = APIRouter()

# A star rating at or above this counts as positive signal for the feed.
POSITIVE_STAR_THRESHOLD = 4


async def _require_support(user: User) -> None:
if not await support_service.is_support_user(user):
raise HTTPException(status_code=403, detail="Not authorized")


def _item(source: str, sentiment: str, message: Optional[str], feature: Optional[str],
user_id: Optional[str], created_at: datetime.datetime) -> dict:
return {
"source": source,
"sentiment": sentiment,
"message": message,
"feature": feature,
"user_id": user_id,
"created_at": created_at.isoformat() if created_at else None,
"_sort": created_at or datetime.datetime.min,
}


@router.get("/positive")
async def list_positive_feedback(
user: User = Depends(get_current_user),
limit: int = Query(50, ge=1, le=200),
source: Optional[str] = Query(None, description="chat | extraction | product"),
):
"""Unified reverse-chronological feed of positive feedback across sources.

- chat: thumbs-up ChatFeedback that carries a comment (bare up-votes have
no words to show, so they inflate stats, not the feed).
- extraction: ExtractionQualityRecord at or above the star threshold.
- product: every ProductFeedback (already positive/idea by construction).
"""
await _require_support(user)

# Over-fetch per source, merge, then trim — a per-source limit can't know
# the global chronological cut-off ahead of the merge.
items: list[dict] = []

if source in (None, "chat"):
chats = await (
ChatFeedback.find(ChatFeedback.rating == "up")
.sort("-created_at")
.limit(limit)
.to_list()
)
for c in chats:
if c.comment and c.comment.strip():
items.append(_item("chat", "positive", c.comment, "chat",
c.user_id, c.created_at))

if source in (None, "extraction"):
extractions = await (
ExtractionQualityRecord.find(
ExtractionQualityRecord.star_rating >= POSITIVE_STAR_THRESHOLD
)
.sort("-created_at")
.limit(limit)
.to_list()
)
for e in extractions:
label = f"{e.star_rating}★ — {e.pdf_title}"
msg = f"{label}: {e.comment}" if (e.comment and e.comment.strip()) else label
items.append(_item("extraction", "positive", msg, "extraction",
e.user_id, e.created_at))

if source in (None, "product"):
products = await (
ProductFeedback.find()
.sort("-created_at")
.limit(limit)
.to_list()
)
for p in products:
items.append(_item("product", p.sentiment, p.message, p.feature,
p.user_id, p.created_at))

items.sort(key=lambda i: i["_sort"], reverse=True)
trimmed = items[:limit]
for i in trimmed:
i.pop("_sort", None)
return {"items": trimmed, "count": len(trimmed)}


@router.get("/stats")
async def positive_feedback_stats(user: User = Depends(get_current_user)):
"""Rollup for the 'What's Working' header: how much positive signal there
is, where it comes from, and the chat thumbs-up rate (the one sentiment
metric we already trend elsewhere)."""
await _require_support(user)

week_ago = datetime.datetime.utcnow() - datetime.timedelta(days=7)

chat_up = await ChatFeedback.find(ChatFeedback.rating == "up").count()
chat_down = await ChatFeedback.find(ChatFeedback.rating == "down").count()
extraction_positive = await ExtractionQualityRecord.find(
ExtractionQualityRecord.star_rating >= POSITIVE_STAR_THRESHOLD
).count()
product_total = await ProductFeedback.find().count()

product_week = await ProductFeedback.find(
ProductFeedback.created_at >= week_ago
).count()
chat_up_week = await ChatFeedback.find(
ChatFeedback.rating == "up", ChatFeedback.created_at >= week_ago
).count()

total_chat = chat_up + chat_down
thumbs_up_rate = round(chat_up / total_chat, 3) if total_chat else None

return {
"by_source": {
"chat": chat_up,
"extraction": extraction_positive,
"product": product_total,
},
"thumbs_up_rate": thumbs_up_rate,
"positive_last_7_days": product_week + chat_up_week,
}
134 changes: 134 additions & 0 deletions backend/tests/test_feedback_routes.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
"""Coverage for the positive-feedback surface: admin-gating on the read feed
and the off-ticket ProductFeedback write path."""

import secrets
from unittest.mock import AsyncMock, MagicMock, patch

import pytest
from httpx import ASGITransport, AsyncClient

from app.config import Settings
from app.utils.security import create_access_token

_TEST_SETTINGS = Settings(jwt_secret_key="test-secret-key", environment="development")


def _make_user(user_id: str = "testuser", *, is_admin: bool = False, current_team=None):
user = MagicMock()
user.id = "fake-id"
user.user_id = user_id
user.email = f"{user_id}@example.com"
user.name = "Test User"
user.is_admin = is_admin
user.current_team = current_team
user.is_demo_user = False
user.token_version = 0
user.demo_status = None
return user


def _auth(user_id: str = "testuser"):
token = create_access_token(user_id, _TEST_SETTINGS)
csrf = secrets.token_urlsafe(32)
return {"access_token": token, "csrf_token": csrf}, {"X-CSRF-Token": csrf}


@pytest.fixture
async def client():
with patch("app.main.init_db", new_callable=AsyncMock):
from app.main import app

async with AsyncClient(
transport=ASGITransport(app=app),
base_url="http://test",
) as ac:
yield ac


def _patch_auth(user):
"""Context managers that make get_current_user resolve to ``user``."""
return (
patch("app.dependencies.decode_token", return_value={"sub": user.user_id, "type": "access"}),
patch("app.dependencies.User"),
)


class TestPositiveFeedbackFeedAuthz:
@pytest.mark.asyncio
async def test_feed_forbidden_for_non_support(self, client):
user = _make_user("viewer")
cookies, headers = _auth("viewer")
patch_decode, patch_user = _patch_auth(user)
with (
patch_decode,
patch_user as MockUser,
patch("app.routers.feedback_admin.support_service.is_support_user",
new=AsyncMock(return_value=False)),
):
MockUser.find_one = AsyncMock(return_value=user)
resp = await client.get("/api/feedback/admin/positive", cookies=cookies, headers=headers)
assert resp.status_code == 403

@pytest.mark.asyncio
async def test_stats_forbidden_for_non_support(self, client):
user = _make_user("viewer")
cookies, headers = _auth("viewer")
patch_decode, patch_user = _patch_auth(user)
with (
patch_decode,
patch_user as MockUser,
patch("app.routers.feedback_admin.support_service.is_support_user",
new=AsyncMock(return_value=False)),
):
MockUser.find_one = AsyncMock(return_value=user)
resp = await client.get("/api/feedback/admin/stats", cookies=cookies, headers=headers)
assert resp.status_code == 403


class TestProductFeedbackWrite:
@pytest.mark.asyncio
async def test_product_feedback_inserts_without_ticket(self, client):
user = _make_user("author")
cookies, headers = _auth("author")
patch_decode, patch_user = _patch_auth(user)
instance = MagicMock()
instance.insert = AsyncMock()
with (
patch_decode,
patch_user as MockUser,
patch("app.routers.feedback.ProductFeedback", return_value=instance) as MockPF,
):
MockUser.find_one = AsyncMock(return_value=user)
resp = await client.post(
"/api/feedback/product",
json={"message": "The extraction saved me an hour!", "sentiment": "positive"},
cookies=cookies, headers=headers,
)
assert resp.status_code == 200
assert resp.json() == {"complete": True}
instance.insert.assert_awaited_once()
# It is a ProductFeedback, never a SupportTicket — praise stays off the queue.
MockPF.assert_called_once()

@pytest.mark.asyncio
async def test_negative_sentiment_is_coerced_to_positive(self, client):
"""A negative sentiment must never ride this path — it is coerced, so
the off-ticket collection only ever holds positive/idea signal."""
user = _make_user("author")
cookies, headers = _auth("author")
patch_decode, patch_user = _patch_auth(user)
instance = MagicMock()
instance.insert = AsyncMock()
with (
patch_decode,
patch_user as MockUser,
patch("app.routers.feedback.ProductFeedback", return_value=instance) as MockPF,
):
MockUser.find_one = AsyncMock(return_value=user)
resp = await client.post(
"/api/feedback/product",
json={"message": "this is broken", "sentiment": "negative"},
cookies=cookies, headers=headers,
)
assert resp.status_code == 200
assert MockPF.call_args.kwargs["sentiment"] == "positive"
Loading
Loading