diff --git a/dashboard/osa/index.html b/dashboard/osa/index.html
index 122882c..991725e 100644
--- a/dashboard/osa/index.html
+++ b/dashboard/osa/index.html
@@ -416,6 +416,50 @@
.admin-status.success { color: #059669; }
.admin-status.error { color: #dc2626; }
+ /* Feedback panel */
+ .feedback-toolbar {
+ display: flex;
+ align-items: center;
+ gap: 0.5rem;
+ margin-bottom: 0.75rem;
+ font-size: 0.82rem;
+ color: #64748b;
+ }
+ .feedback-toolbar label { display: inline-flex; align-items: center; gap: 0.4rem; cursor: pointer; }
+ .feedback-table-wrap {
+ max-height: 320px;
+ overflow-y: auto;
+ border: 1px solid #e2e8f0;
+ border-radius: 8px;
+ }
+ .feedback-table {
+ width: 100%;
+ border-collapse: collapse;
+ font-size: 0.82rem;
+ }
+ .feedback-table th, .feedback-table td {
+ text-align: left;
+ padding: 0.5rem 0.7rem;
+ border-bottom: 1px solid #e2e8f0;
+ vertical-align: top;
+ }
+ .feedback-table th {
+ position: sticky;
+ top: 0;
+ background: #f8fafc;
+ color: #475569;
+ font-weight: 600;
+ font-size: 0.72rem;
+ text-transform: uppercase;
+ letter-spacing: 0.03em;
+ }
+ .feedback-table tr:last-child td { border-bottom: none; }
+ .feedback-table td a { color: #2563eb; text-decoration: none; }
+ .feedback-table td a:hover { text-decoration: underline; }
+ .feedback-sentiment-up { color: #059669; font-weight: 600; }
+ .feedback-sentiment-down { color: #dc2626; font-weight: 600; }
+ .feedback-empty { color: #64748b; padding: 1rem; font-size: 0.85rem; }
+
/* Loading / error */
.loading { text-align: center; color: #64748b; padding: 2rem; }
.loading::after {
@@ -508,6 +552,14 @@
.admin-btn { background: #7ba3d4; color: #0d1117; }
.admin-btn:hover { background: #93b8de; }
.loading { color: #8a9bb5; }
+ .feedback-toolbar { color: #8a9bb5; }
+ .feedback-table-wrap { border-color: #30363d; }
+ .feedback-table th, .feedback-table td { border-color: #30363d; }
+ .feedback-table th { background: #1c2128; color: #8a9bb5; }
+ .feedback-table td a { color: #7ba3d4; }
+ .feedback-sentiment-up { color: #4ade80; }
+ .feedback-sentiment-down { color: #f87171; }
+ .feedback-empty { color: #8a9bb5; }
.error-msg { color: #f87171; background: #3b1219; border-color: #5c1d2a; }
.site-footer { color: #6b7b92; border-color: #30363d; }
.site-footer a { color: #7ba3d4; }
@@ -918,6 +970,8 @@
+ Admin: Feedback
+
`;
@@ -1167,6 +1221,8 @@ Failed to load feedback (HTTP ${resp.status})`;
+ }
+ return;
+ }
+ renderAdminFeedback(await resp.json(), communityId);
+ } catch (err) {
+ console.error('Failed to load feedback:', err);
+ container.innerHTML = '
Failed to load feedback: unexpected server response. Try refreshing the page.
';
+ }
+ }
+
+ function toggleFeedbackCommentsOnly(checked, communityId) {
+ feedbackCommentsOnly = checked;
+ loadAdminFeedback(decodeURIComponent(communityId));
+ }
+
+ function formatFeedbackTime(isoStr) {
+ if (!isoStr) return '—';
+ try {
+ const d = new Date(isoStr);
+ if (isNaN(d.getTime())) return isoStr;
+ return d.toLocaleString(undefined, {
+ year: 'numeric', month: 'short', day: 'numeric',
+ hour: '2-digit', minute: '2-digit',
+ });
+ } catch (err) { console.warn('Failed to parse feedback timestamp:', isoStr, err); return isoStr; }
+ }
+
+ function renderAdminFeedback(data, communityId) {
+ const container = document.getElementById('adminFeedback');
+ if (!container) return;
+
+ const summary = data.summary || {};
+ const up = summary.thumbs_up || 0;
+ const down = summary.thumbs_down || 0;
+ const satRate = (summary.satisfaction_rate === null || summary.satisfaction_rate === undefined)
+ ? '—' : `${(summary.satisfaction_rate * 100).toFixed(0)}%`;
+ const responseTotal = summary.response_total || 0;
+ const generalTotal = summary.general_total || 0;
+ const commentTotal = summary.comment_total || 0;
+
+ const summaryHtml = `
+
+
+
${up.toLocaleString()}
+
Thumbs Up
+
+
+
${down.toLocaleString()}
+
Thumbs Down
+
+
+
${satRate}
+
Satisfaction Rate
+
+
+
${responseTotal.toLocaleString()}
+
Responses Rated
+
+
+
${generalTotal.toLocaleString()}
+
General Feedback
+
+
+
${commentTotal.toLocaleString()}
+
Comments
+
+
+
+
`;
+
+ const entries = Array.isArray(data.entries) ? data.entries : [];
+ let tableHtml;
+ if (entries.length === 0) {
+ tableHtml = 'No feedback yet.
';
+ } else {
+ const rows = entries.map(e => {
+ const time = escapeHtml(formatFeedbackTime(e.timestamp));
+ const type = escapeHtml(e.feedback_type || '—');
+ let sentiment = '';
+ if (e.sentiment === 'up') sentiment = '▲ up';
+ else if (e.sentiment === 'down') sentiment = '▼ down';
+ const comment = e.comment ? escapeHtml(e.comment) : '—';
+ // Defense in depth: only render a clickable link for http(s) URLs
+ // (the backend already enforces this on write).
+ const isHttpUrl = typeof e.page_url === 'string' && /^https?:\/\//i.test(e.page_url);
+ const page = isHttpUrl
+ ? `link`
+ : '';
+ return `
+ | ${time} |
+ ${type} |
+ ${sentiment} |
+ ${comment} |
+ ${page} |
+
`;
+ }).join('');
+ tableHtml = `
+
+
+
+
+ | Time |
+ Type |
+ Sentiment |
+ Comment |
+ Page |
+
+
+ ${rows}
+
+
+
+
+
+
+
+
+
+ Shared with the ${escapeHtml(CONFIG.title.replace(' Assistant', ''))} community maintainers. Please do not include personal information.
+
+
+
+
+ Thanks for your feedback!
+
+
+
+
+
+ Thanks!
+
`
+ : '';
+
msgEl.innerHTML = `
${content}
+ ${feedbackRow}
`;
messagesEl.appendChild(msgEl);
});
@@ -2093,6 +2350,17 @@
});
});
+ // Per-response thumbs up/down buttons
+ messagesEl.querySelectorAll('.osa-message-feedback .osa-feedback-btn[data-feedback]').forEach(btn => {
+ btn.addEventListener('click', (e) => {
+ e.stopPropagation();
+ const row = btn.closest('.osa-message-feedback');
+ const msgIndex = parseInt(row.getAttribute('data-msg-index'), 10);
+ const sentiment = btn.getAttribute('data-feedback');
+ submitResponseFeedback(container, msgIndex, sentiment);
+ });
+ });
+
if (isLoading) {
const loadingEl = document.createElement('div');
loadingEl.className = 'osa-loading';
@@ -2236,7 +2504,9 @@
// Log tool completion
console.log('[OSA] Tool completed:', event.name);
} else if (event.event === 'session') {
- // Capture session ID early (sent at stream start)
+ // Capture session ID early (sent at stream start). request_id is
+ // intentionally NOT sent here; it arrives on the 'done' event so it
+ // only attaches to a reply that completed successfully.
if (event.session_id && typeof event.session_id === 'string') {
sessionId = event.session_id;
}
@@ -2251,6 +2521,9 @@
if (event.session_id && typeof event.session_id === 'string') {
sessionId = event.session_id;
}
+ if (event.request_id && typeof event.request_id === 'string') {
+ messages[messageIndex].requestId = event.request_id;
+ }
messages[messageIndex].content = accumulatedContent;
renderMessages(container);
try {
@@ -2473,7 +2746,11 @@
if (!answer) {
throw new Error('Invalid response from server');
}
- messages.push({ role: 'assistant', content: answer });
+ const assistantMsg = { role: 'assistant', content: answer };
+ if (data && typeof data.request_id === 'string') {
+ assistantMsg.requestId = data.request_id;
+ }
+ messages.push(assistantMsg);
try {
saveHistory();
} catch (saveError) {
@@ -2852,6 +3129,23 @@
}
});
+ // Feedback link + modal event listeners
+ const feedbackLink = container.querySelector('.osa-feedback-link');
+ const feedbackOverlay = container.querySelector('.osa-feedback-overlay');
+ const feedbackCloseBtn = container.querySelector('.osa-feedback-close-btn');
+ const feedbackCancelBtn = container.querySelector('.osa-feedback-cancel-btn');
+ const feedbackSendBtn = container.querySelector('.osa-feedback-send-btn');
+
+ feedbackLink?.addEventListener('click', () => openFeedback(container));
+ feedbackCloseBtn?.addEventListener('click', () => closeFeedback(container));
+ feedbackCancelBtn?.addEventListener('click', () => closeFeedback(container));
+ feedbackSendBtn?.addEventListener('click', () => submitGeneralFeedback(container));
+ feedbackOverlay?.addEventListener('click', (e) => {
+ if (e.target === feedbackOverlay) {
+ closeFeedback(container);
+ }
+ });
+
// Show/hide custom model input based on selection
modelSelect?.addEventListener('change', (e) => {
if (customModelField) {
diff --git a/src/api/main.py b/src/api/main.py
index 3d486c6..f248c3b 100644
--- a/src/api/main.py
+++ b/src/api/main.py
@@ -17,6 +17,7 @@
from src.api.routers import (
communities_router,
create_community_router,
+ feedback_router,
metrics_public_router,
metrics_router,
mirrors_router,
@@ -272,6 +273,9 @@ def register_routes(app: FastAPI) -> None:
# Communities metadata endpoint (public, for widget config)
app.include_router(communities_router)
+ # Feedback endpoint (public; anonymous widget users submit thumbs/comments)
+ app.include_router(feedback_router)
+
# Health check router
app.include_router(health_router)
@@ -323,7 +327,9 @@ async def root() -> dict[str, Any]:
endpoints["POST /sync/trigger"] = "Trigger sync (requires API key)"
endpoints["GET /metrics/overview"] = "Metrics overview (requires admin key)"
endpoints["GET /metrics/tokens"] = "Token breakdown (requires admin key)"
+ endpoints["GET /metrics/feedback"] = "User feedback (requires admin key)"
endpoints["GET /metrics/public/overview"] = "Public metrics overview"
+ endpoints["POST /feedback"] = "Submit user feedback (thumbs up/down or comment)"
endpoints["GET /health"] = "Health check"
return {
diff --git a/src/api/routers/__init__.py b/src/api/routers/__init__.py
index df2c667..d343526 100644
--- a/src/api/routers/__init__.py
+++ b/src/api/routers/__init__.py
@@ -2,6 +2,7 @@
from src.api.routers.communities import router as communities_router
from src.api.routers.community import create_community_router
+from src.api.routers.feedback import router as feedback_router
from src.api.routers.metrics import router as metrics_router
from src.api.routers.metrics_public import router as metrics_public_router
from src.api.routers.mirrors import router as mirrors_router
@@ -10,6 +11,7 @@
__all__ = [
"communities_router",
"create_community_router",
+ "feedback_router",
"metrics_public_router",
"metrics_router",
"mirrors_router",
diff --git a/src/api/routers/community.py b/src/api/routers/community.py
index 3e16e5a..0fc6426 100644
--- a/src/api/routers/community.py
+++ b/src/api/routers/community.py
@@ -145,6 +145,10 @@ class ChatResponse(BaseModel):
tool_calls: list[ToolCallInfo] = Field(
default_factory=list, description="Tools called during response generation"
)
+ request_id: str | None = Field(
+ default=None,
+ description="Per-request identifier the widget can attach to feedback",
+ )
class AskResponse(BaseModel):
@@ -154,6 +158,10 @@ class AskResponse(BaseModel):
tool_calls: list[ToolCallInfo] = Field(
default_factory=list, description="Tools called during response generation"
)
+ request_id: str | None = Field(
+ default=None,
+ description="Per-request identifier the widget can attach to feedback",
+ )
class SessionInfo(BaseModel):
@@ -1048,7 +1056,11 @@ async def ask(
ar = _extract_agent_result(result)
_set_metrics_on_request(http_request, awm, ar)
- return AskResponse(answer=ar.response_content, tool_calls=ar.tool_calls_info)
+ return AskResponse(
+ answer=ar.response_content,
+ tool_calls=ar.tool_calls_info,
+ request_id=getattr(http_request.state, "request_id", None),
+ )
except HTTPException:
raise
@@ -1155,6 +1167,7 @@ async def chat(
session_id=session.session_id,
message=ChatMessage(role="assistant", content=ar.response_content),
tool_calls=ar.tool_calls_info,
+ request_id=getattr(http_request.state, "request_id", None),
)
except ValueError as e:
@@ -1595,7 +1608,7 @@ async def _stream_ask_response(
data: {"event": "content", "content": "text chunk"}
data: {"event": "tool_start", "name": "tool_name", "input": {...}}
data: {"event": "tool_end", "name": "tool_name", "output": {...}}
- data: {"event": "done"}
+ data: {"event": "done", "request_id": "..."}
data: {"event": "error", "message": "error text"}
"""
start_time = time.monotonic()
@@ -1604,6 +1617,9 @@ async def _stream_ask_response(
total_input_tokens = 0
total_output_tokens = 0
+ # Per-request id (set by metrics middleware) so the widget can attach feedback.
+ request_id = getattr(http_request.state, "request_id", None) if http_request else None
+
try:
awm = create_community_assistant(
community_id,
@@ -1658,7 +1674,7 @@ async def _stream_ask_response(
}
yield f"data: {json.dumps(sse_event)}\n\n"
- sse_event = {"event": "done"}
+ sse_event = {"event": "done", "request_id": request_id}
yield f"data: {json.dumps(sse_event)}\n\n"
# Log metrics at end of streaming
@@ -1767,7 +1783,8 @@ async def _stream_chat_response(
data: {"event": "tool_start", "name": "tool_name", "input": {...}}
data: {"event": "tool_end", "name": "tool_name", "output": {...}}
data: {"event": "session", "session_id": "..."} (sent first)
- data: {"event": "done", "session_id": "..."}
+ data: {"event": "warning", "message": "..."} (optional, before done)
+ data: {"event": "done", "session_id": "...", "request_id": "..."}
data: {"event": "error", "message": "error text"}
"""
start_time = time.monotonic()
@@ -1776,6 +1793,13 @@ async def _stream_chat_response(
total_input_tokens = 0
total_output_tokens = 0
+ # The metrics middleware assigns a per-request UUID; expose it only on the
+ # final `done` event (below) so the widget attaches it only to a reply that
+ # completed normally. Error paths yield an `error` event instead and never
+ # reach `done`, so a partially-streamed or fully-errored reply carries no
+ # request_id. This also joins per-response feedback back to request_log.
+ request_id = getattr(http_request.state, "request_id", None) if http_request else None
+
# Send session_id immediately so the client captures it even if the
# stream is truncated by a proxy timeout.
sse_event = {"event": "session", "session_id": session.session_id}
@@ -1859,7 +1883,11 @@ async def _stream_chat_response(
}
yield f"data: {json.dumps(sse_event)}\n\n"
- sse_event = {"event": "done", "session_id": session.session_id}
+ sse_event = {
+ "event": "done",
+ "session_id": session.session_id,
+ "request_id": request_id,
+ }
yield f"data: {json.dumps(sse_event)}\n\n"
# Log metrics at end of streaming
diff --git a/src/api/routers/feedback.py b/src/api/routers/feedback.py
new file mode 100644
index 0000000..392f4e0
--- /dev/null
+++ b/src/api/routers/feedback.py
@@ -0,0 +1,145 @@
+"""User feedback API endpoint.
+
+Receives anonymous feedback from the chat widget (per-response thumbs up/down
+and free-text general feedback) and persists it to the metrics database for
+community admins to review on the status dashboard.
+
+This is a top-level (non community-prefixed) route because the widget posts to
+the Cloudflare Worker's global ``POST /feedback`` proxy, which forwards here;
+the community is carried in the request body, not the path.
+"""
+
+import logging
+import uuid
+from typing import Annotated, Any, Literal
+
+from fastapi import APIRouter, Header, HTTPException
+from pydantic import BaseModel, Field, field_validator, model_validator
+
+from src.api.routers.community import _is_authorized_origin
+from src.assistants import registry
+from src.metrics.db import FeedbackEntry, now_iso, write_feedback
+
+logger = logging.getLogger(__name__)
+
+router = APIRouter(tags=["Feedback"])
+
+_MAX_COMMENT_LEN = 5000
+
+
+class FeedbackRequest(BaseModel):
+ """Body for ``POST /feedback`` submitted by the widget.
+
+ Two shapes share this model:
+
+ * ``feedback_type="response"`` -- a thumbs up/down on one reply. ``sentiment``
+ is required; ``request_id`` / ``message_index`` link it to the answer.
+ * ``feedback_type="general"`` -- free-text feedback. ``comment`` is required;
+ ``sentiment`` is ignored.
+ """
+
+ community_id: str = Field(..., min_length=1, max_length=64)
+ feedback_type: Literal["response", "general"] = "response"
+ sentiment: Literal["up", "down"] | None = None
+ request_id: str | None = Field(default=None, max_length=128)
+ session_id: str | None = Field(default=None, max_length=128)
+ message_index: int | None = Field(default=None, ge=0)
+ comment: str | None = Field(default=None, max_length=_MAX_COMMENT_LEN)
+ page_url: str | None = Field(default=None, max_length=2048)
+
+ @field_validator("page_url")
+ @classmethod
+ def _validate_page_url_scheme(cls, url: str | None) -> str | None:
+ """Only accept http(s) URLs.
+
+ This endpoint is anonymous, and page_url is later rendered as a clickable
+ link in the admin dashboard. Rejecting non-http(s) schemes prevents a
+ stored 'javascript:'/'data:' link from becoming XSS against an admin.
+ """
+ if url is None:
+ return url
+ if not url.startswith(("http://", "https://")):
+ raise ValueError("page_url must start with http:// or https://")
+ return url
+
+ @model_validator(mode="before")
+ @classmethod
+ def _normalize(cls, data: Any) -> Any:
+ """Normalize the raw input before field validation.
+
+ Runs on the raw dict so the after-validator can be a pure guard: drop
+ any sentiment on general feedback (it is meaningless there) and collapse
+ a whitespace-only comment to None so the DB stays clean.
+ """
+ if not isinstance(data, dict):
+ return data
+ if data.get("feedback_type") == "general":
+ data = {**data, "sentiment": None}
+ comment = data.get("comment")
+ if isinstance(comment, str) and not comment.strip():
+ data = {**data, "comment": None}
+ return data
+
+ @model_validator(mode="after")
+ def _check_shape(self) -> "FeedbackRequest":
+ """Enforce the per-type required fields (pure guard, no mutation)."""
+ if self.feedback_type == "response" and self.sentiment is None:
+ raise ValueError("response feedback requires a sentiment ('up' or 'down')")
+ if self.feedback_type == "general" and not (self.comment and self.comment.strip()):
+ raise ValueError("general feedback requires a non-empty comment")
+ return self
+
+
+class FeedbackResponse(BaseModel):
+ """Acknowledgement returned to the widget."""
+
+ status: Literal["ok"] = "ok"
+ feedback_id: str
+
+
+@router.post("/feedback", response_model=FeedbackResponse)
+async def submit_feedback(
+ body: FeedbackRequest,
+ origin: Annotated[str | None, Header()] = None,
+) -> FeedbackResponse:
+ """Record a piece of user feedback.
+
+ Anonymous by design: widget users carry no API key. The community must be a
+ registered one. The Origin header is checked softly -- an unrecognized or
+ absent origin is logged (CLI, mobile, proxies strip it) but does not reject
+ the submission, mirroring how the chat endpoints degrade gracefully.
+
+ Abuse/DoS defense: the public path to this endpoint is the Cloudflare Worker
+ (`POST /feedback` -> handleFeedback -> rateLimitOrReject), which rate-limits
+ per client before proxying here. There is intentionally no in-process limiter;
+ if this endpoint is ever exposed without the Worker in front, add one.
+ """
+ info = registry.get(body.community_id)
+ if info is None:
+ raise HTTPException(status_code=404, detail=f"Unknown community: {body.community_id}")
+
+ if not _is_authorized_origin(origin, body.community_id):
+ logger.info(
+ "Feedback from unrecognized origin %r for community %s (accepted)",
+ origin,
+ body.community_id,
+ )
+
+ entry = FeedbackEntry(
+ feedback_id=str(uuid.uuid4()),
+ timestamp=now_iso(),
+ community_id=body.community_id,
+ feedback_type=body.feedback_type,
+ sentiment=body.sentiment,
+ request_id=body.request_id,
+ session_id=body.session_id,
+ message_index=body.message_index,
+ comment=body.comment,
+ page_url=body.page_url,
+ )
+
+ # write_feedback is best-effort: it logs and swallows storage errors (and
+ # escalates after repeated failures) rather than failing the user's request,
+ # so the widget always receives a clean acknowledgement.
+ write_feedback(entry)
+ return FeedbackResponse(feedback_id=entry.feedback_id)
diff --git a/src/api/routers/metrics.py b/src/api/routers/metrics.py
index 467c28c..a022d28 100644
--- a/src/api/routers/metrics.py
+++ b/src/api/routers/metrics.py
@@ -14,6 +14,8 @@
from src.metrics.db import metrics_connection
from src.metrics.queries import (
get_community_summary,
+ get_feedback_entries,
+ get_feedback_summary,
get_overview,
get_quality_summary,
get_token_breakdown,
@@ -71,6 +73,47 @@ async def token_breakdown(
)
+@router.get("/feedback")
+async def feedback(
+ auth: RequireScopedAuth,
+ community_id: str | None = Query(default=None, description="Filter by community"),
+ limit: int = Query(default=100, ge=1, le=500, description="Max comment rows to return"),
+ offset: int = Query(default=0, ge=0, description="Rows to skip (pagination)"),
+ comments_only: bool = Query(
+ default=False, description="Return only entries that carry a free-text comment"
+ ),
+) -> dict[str, Any]:
+ """Get user feedback (thumbs up/down counts and free-text comments).
+
+ Global admin keys can filter by any community (or see all). Per-community
+ keys are automatically scoped to their own community.
+ """
+ # Community-scoped keys always filter to their own community
+ effective_community = community_id
+ if auth.role == "community":
+ effective_community = auth.community_id
+
+ try:
+ with metrics_connection() as conn:
+ return {
+ "community_id": effective_community,
+ "summary": get_feedback_summary(conn, community_id=effective_community),
+ "entries": get_feedback_entries(
+ conn,
+ community_id=effective_community,
+ limit=limit,
+ offset=offset,
+ with_comment_only=comments_only,
+ ),
+ }
+ except sqlite3.Error:
+ logger.exception("Failed to query metrics database for feedback")
+ raise HTTPException(
+ status_code=503,
+ detail="Metrics database is temporarily unavailable.",
+ )
+
+
@router.get("/quality")
async def quality_overview(auth: RequireScopedAuth) -> dict[str, Any]:
"""Get quality metrics overview.
diff --git a/src/metrics/db.py b/src/metrics/db.py
index 35b6af5..9e2b621 100644
--- a/src/metrics/db.py
+++ b/src/metrics/db.py
@@ -1,7 +1,7 @@
"""Metrics storage layer using SQLite with WAL mode.
-Single SQLite database at {data_dir}/metrics.db stores all request logs.
-WAL mode enables concurrent reads during writes.
+Single SQLite database at {data_dir}/metrics.db stores request logs and user
+feedback. WAL mode enables concurrent reads during writes.
"""
import json
@@ -12,6 +12,7 @@
from dataclasses import dataclass, field
from datetime import UTC, datetime
from pathlib import Path
+from typing import Literal
from langchain_core.messages import AIMessage, BaseMessage
@@ -20,6 +21,14 @@
# Track consecutive log_request failures for escalation
_log_request_failures: int = 0
+# Track consecutive write_feedback failures for escalation
+_write_feedback_failures: int = 0
+
+# Allowed feedback type / sentiment values. These mirror the SQLite CHECK
+# constraints below and the FeedbackEntry / FeedbackRequest invariants.
+FEEDBACK_TYPES = ("response", "general")
+FEEDBACK_SENTIMENTS = ("up", "down")
+
METRICS_SCHEMA_SQL = """
CREATE TABLE IF NOT EXISTS request_log (
id INTEGER PRIMARY KEY AUTOINCREMENT,
@@ -49,6 +58,27 @@
ON request_log(timestamp);
CREATE INDEX IF NOT EXISTS idx_request_log_community_timestamp
ON request_log(community_id, timestamp);
+
+CREATE TABLE IF NOT EXISTS feedback_log (
+ id INTEGER PRIMARY KEY AUTOINCREMENT,
+ feedback_id TEXT NOT NULL,
+ timestamp TEXT NOT NULL,
+ community_id TEXT NOT NULL,
+ feedback_type TEXT NOT NULL CHECK (feedback_type IN ('response', 'general')),
+ sentiment TEXT CHECK (sentiment IN ('up', 'down') OR sentiment IS NULL),
+ request_id TEXT,
+ session_id TEXT,
+ message_index INTEGER,
+ comment TEXT,
+ page_url TEXT
+);
+
+CREATE UNIQUE INDEX IF NOT EXISTS idx_feedback_log_feedback_id
+ ON feedback_log(feedback_id);
+CREATE INDEX IF NOT EXISTS idx_feedback_log_community
+ ON feedback_log(community_id);
+CREATE INDEX IF NOT EXISTS idx_feedback_log_community_timestamp
+ ON feedback_log(community_id, timestamp);
"""
# Columns added after initial schema; ALTER TABLE for existing databases
@@ -83,6 +113,48 @@ class RequestLogEntry:
langfuse_trace_id: str | None = None
+@dataclass
+class FeedbackEntry:
+ """A single user-feedback entry for the metrics database.
+
+ Two flavors share one table:
+
+ * ``feedback_type="response"`` -- a thumbs up/down on a specific assistant
+ reply. ``sentiment`` is required; ``request_id`` (and/or ``message_index``)
+ links it back to the answer it rates.
+ * ``feedback_type="general"`` -- free-text feedback not tied to a single
+ reply. ``sentiment`` is typically ``None`` and ``comment`` carries the text.
+ """
+
+ feedback_id: str
+ timestamp: str
+ community_id: str
+ feedback_type: Literal["response", "general"]
+ sentiment: Literal["up", "down"] | None = None
+ request_id: str | None = None
+ session_id: str | None = None
+ message_index: int | None = None
+ comment: str | None = None
+ page_url: str | None = None
+
+ def __post_init__(self) -> None:
+ """Enforce the storage-layer invariants.
+
+ ``write_feedback`` is a public function callable outside the API router,
+ so this is the last checkpoint before a row reaches SQLite. Reject the
+ illegal shapes the satisfaction-rate query would otherwise be corrupted
+ by (a 'response' with no sentiment, or a 'general' carrying one).
+ """
+ if self.feedback_type not in FEEDBACK_TYPES:
+ raise ValueError(f"feedback_type must be one of {FEEDBACK_TYPES!r}")
+ if self.sentiment is not None and self.sentiment not in FEEDBACK_SENTIMENTS:
+ raise ValueError(f"sentiment must be one of {FEEDBACK_SENTIMENTS!r} or None")
+ if self.feedback_type == "response" and self.sentiment is None:
+ raise ValueError("response feedback requires a sentiment")
+ if self.feedback_type == "general" and self.sentiment is not None:
+ raise ValueError("general feedback must not carry a sentiment")
+
+
def get_metrics_db_path() -> Path:
"""Return path to the metrics SQLite database.
@@ -230,6 +302,67 @@ def log_request(entry: RequestLogEntry, db_path: Path | None = None) -> None:
conn.close()
+def write_feedback(entry: FeedbackEntry, db_path: Path | None = None) -> None:
+ """Insert a feedback entry into the database.
+
+ Mirrors :func:`log_request`: best-effort, never raises to the caller, and
+ escalates to a CRITICAL log after repeated failures so a broken disk or DB
+ surfaces in monitoring instead of silently dropping feedback.
+
+ Args:
+ entry: The feedback entry to insert.
+ db_path: Optional path override (for testing).
+ """
+ global _write_feedback_failures
+ conn: sqlite3.Connection | None = None
+ try:
+ # Acquire the connection inside the try so a connect failure (e.g. the
+ # data dir vanished) is handled here too and the function honors its
+ # "never raises" contract.
+ conn = get_metrics_connection(db_path)
+ conn.execute(
+ """
+ INSERT INTO feedback_log (
+ feedback_id, timestamp, community_id, feedback_type, sentiment,
+ request_id, session_id, message_index, comment, page_url
+ ) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
+ """,
+ (
+ entry.feedback_id,
+ entry.timestamp,
+ entry.community_id,
+ entry.feedback_type,
+ entry.sentiment,
+ entry.request_id,
+ entry.session_id,
+ entry.message_index,
+ entry.comment,
+ entry.page_url,
+ ),
+ )
+ conn.commit()
+ except sqlite3.Error:
+ _write_feedback_failures += 1
+ logger.exception(
+ "Failed to write feedback %s (community=%s, type=%s) [failure #%d]",
+ entry.feedback_id,
+ entry.community_id,
+ entry.feedback_type,
+ _write_feedback_failures,
+ )
+ if _write_feedback_failures >= 10:
+ logger.critical(
+ "Feedback DB write has failed %d times. "
+ "Possible disk/database issue requiring investigation.",
+ _write_feedback_failures,
+ )
+ else:
+ _write_feedback_failures = 0
+ finally:
+ if conn is not None:
+ conn.close()
+
+
def extract_token_usage(result: dict) -> tuple[int, int, int]:
"""Extract token usage from agent result messages.
diff --git a/src/metrics/queries.py b/src/metrics/queries.py
index 8c2d7be..fae34b8 100644
--- a/src/metrics/queries.py
+++ b/src/metrics/queries.py
@@ -593,6 +593,122 @@ def get_quality_summary(community_id: str, conn: sqlite3.Connection) -> dict[str
}
+# ---------------------------------------------------------------------------
+# Feedback queries
+# ---------------------------------------------------------------------------
+
+
+def get_feedback_summary(
+ conn: sqlite3.Connection,
+ community_id: str | None = None,
+) -> dict[str, Any]:
+ """Get aggregate feedback counts.
+
+ Args:
+ conn: SQLite connection (with row_factory=sqlite3.Row).
+ community_id: Filter to a single community, or None for all communities.
+
+ Returns:
+ Dict with thumbs_up, thumbs_down, response_total, general_total,
+ comment_total, and a satisfaction_rate (up / (up + down)).
+ """
+ where = ""
+ params: tuple = ()
+ if community_id:
+ where = "WHERE community_id = ?"
+ params = (community_id,)
+
+ row = conn.execute(
+ f"""
+ SELECT
+ COUNT(CASE WHEN sentiment = 'up' THEN 1 END) as thumbs_up,
+ COUNT(CASE WHEN sentiment = 'down' THEN 1 END) as thumbs_down,
+ COUNT(CASE WHEN feedback_type = 'response' THEN 1 END) as response_total,
+ COUNT(CASE WHEN feedback_type = 'general' THEN 1 END) as general_total,
+ COUNT(CASE WHEN comment IS NOT NULL AND TRIM(comment) != '' THEN 1 END)
+ as comment_total
+ FROM feedback_log
+ {where}
+ """,
+ params,
+ ).fetchone()
+
+ up = row["thumbs_up"]
+ down = row["thumbs_down"]
+ rated = up + down
+
+ return {
+ "community_id": community_id,
+ "thumbs_up": up,
+ "thumbs_down": down,
+ "response_total": row["response_total"],
+ "general_total": row["general_total"],
+ "comment_total": row["comment_total"],
+ "satisfaction_rate": round(up / rated, 4) if rated > 0 else None,
+ }
+
+
+def get_feedback_entries(
+ conn: sqlite3.Connection,
+ community_id: str | None = None,
+ limit: int = 100,
+ offset: int = 0,
+ with_comment_only: bool = False,
+) -> list[dict[str, Any]]:
+ """Get individual feedback rows, most recent first.
+
+ Args:
+ conn: SQLite connection.
+ community_id: Filter to a single community, or None for all communities.
+ limit: Maximum number of rows to return (clamped 1..500).
+ offset: Number of rows to skip (for pagination).
+ with_comment_only: If True, return only rows that carry a free-text comment.
+
+ Returns:
+ List of dicts with timestamp, community_id, feedback_type, sentiment,
+ comment, request_id, session_id, message_index, page_url.
+ """
+ limit = max(1, min(int(limit), 500))
+ offset = max(0, int(offset))
+
+ clauses: list[str] = []
+ params: list[Any] = []
+ if community_id:
+ clauses.append("community_id = ?")
+ params.append(community_id)
+ if with_comment_only:
+ clauses.append("comment IS NOT NULL AND TRIM(comment) != ''")
+ where = ("WHERE " + " AND ".join(clauses)) if clauses else ""
+
+ rows = conn.execute(
+ f"""
+ SELECT
+ timestamp, community_id, feedback_type, sentiment,
+ comment, request_id, session_id, message_index, page_url
+ FROM feedback_log
+ {where}
+ ORDER BY timestamp DESC, id DESC
+ LIMIT ? OFFSET ?
+ """,
+ (*params, limit, offset),
+ ).fetchall()
+
+ return [
+ {
+ "timestamp": r["timestamp"],
+ "community_id": r["community_id"],
+ "feedback_type": r["feedback_type"],
+ "sentiment": r["sentiment"],
+ "comment": r["comment"],
+ "request_id": r["request_id"],
+ "session_id": r["session_id"],
+ "message_index": r["message_index"],
+ "page_url": r["page_url"],
+ }
+ for r in rows
+ ]
+
+
def _percentile(sorted_values: list[float], pct: float) -> float | None:
"""Compute percentile from a sorted list of values.
diff --git a/tests/test_api/test_feedback_endpoint.py b/tests/test_api/test_feedback_endpoint.py
new file mode 100644
index 0000000..b307cf8
--- /dev/null
+++ b/tests/test_api/test_feedback_endpoint.py
@@ -0,0 +1,339 @@
+"""Tests for the feedback API: POST /feedback and GET /metrics/feedback."""
+
+import os
+
+import pytest
+from fastapi.testclient import TestClient
+
+from src.api.main import app
+from src.metrics.db import get_metrics_connection, init_metrics_db
+from src.metrics.queries import get_feedback_summary
+
+ADMIN_KEY = "test-feedback-admin-key"
+HED_KEY = "hed-community-key"
+
+
+@pytest.fixture
+def feedback_db(tmp_path):
+ """Isolated, empty metrics DB via DATA_DIR (real path resolution, no mocks).
+
+ get_metrics_db_path() reads DATA_DIR, so pointing it at tmp_path redirects
+ every metrics code path (writes, reads, middleware) to a throwaway DB.
+ """
+ os.environ["DATA_DIR"] = str(tmp_path)
+ init_metrics_db()
+ yield tmp_path / "metrics.db"
+ del os.environ["DATA_DIR"]
+
+
+@pytest.fixture
+def scoped_auth_env():
+ """Admin key + per-community key for hed."""
+ from src.api.config import get_settings
+
+ os.environ["API_KEYS"] = ADMIN_KEY
+ os.environ["REQUIRE_API_AUTH"] = "true"
+ os.environ["COMMUNITY_ADMIN_KEYS"] = f"hed:{HED_KEY}"
+ get_settings.cache_clear()
+ yield
+ del os.environ["API_KEYS"]
+ del os.environ["REQUIRE_API_AUTH"]
+ del os.environ["COMMUNITY_ADMIN_KEYS"]
+ get_settings.cache_clear()
+
+
+@pytest.fixture
+def client():
+ return TestClient(app)
+
+
+@pytest.mark.usefixtures("feedback_db")
+class TestSubmitFeedback:
+ """POST /feedback (anonymous, no auth)."""
+
+ def test_thumbs_up(self, client):
+ resp = client.post(
+ "/feedback",
+ json={
+ "community_id": "hed",
+ "feedback_type": "response",
+ "sentiment": "up",
+ "request_id": "req-1",
+ "session_id": "sess-1",
+ "message_index": 1,
+ },
+ )
+ assert resp.status_code == 200
+ body = resp.json()
+ assert body["status"] == "ok"
+ assert body["feedback_id"]
+
+ def test_thumbs_down_with_comment(self, client):
+ resp = client.post(
+ "/feedback",
+ json={
+ "community_id": "hed",
+ "feedback_type": "response",
+ "sentiment": "down",
+ "comment": "missed the BIDS sidecar question",
+ },
+ )
+ assert resp.status_code == 200
+
+ def test_general_feedback(self, client):
+ resp = client.post(
+ "/feedback",
+ json={
+ "community_id": "hed",
+ "feedback_type": "general",
+ "comment": "Please add more examples.",
+ },
+ )
+ assert resp.status_code == 200
+
+ def test_unknown_community_rejected(self, client):
+ resp = client.post(
+ "/feedback",
+ json={
+ "community_id": "not-a-real-community",
+ "feedback_type": "response",
+ "sentiment": "up",
+ },
+ )
+ assert resp.status_code == 404
+
+ def test_response_without_sentiment_rejected(self, client):
+ resp = client.post(
+ "/feedback",
+ json={"community_id": "hed", "feedback_type": "response"},
+ )
+ assert resp.status_code == 422
+
+ def test_general_without_comment_rejected(self, client):
+ resp = client.post(
+ "/feedback",
+ json={"community_id": "hed", "feedback_type": "general"},
+ )
+ assert resp.status_code == 422
+
+ def test_oversized_comment_rejected(self, client):
+ resp = client.post(
+ "/feedback",
+ json={
+ "community_id": "hed",
+ "feedback_type": "general",
+ "comment": "x" * 6000,
+ },
+ )
+ assert resp.status_code == 422
+
+ @pytest.mark.parametrize(
+ "bad_url",
+ [
+ "javascript:alert(1)",
+ "data:text/html,![]()
",
+ "ftp://evil.example/x",
+ ],
+ )
+ def test_non_http_page_url_rejected(self, client, bad_url):
+ # page_url is rendered as a link in the admin dashboard; only http(s)
+ # schemes are allowed to prevent stored XSS against admins.
+ resp = client.post(
+ "/feedback",
+ json={
+ "community_id": "hed",
+ "feedback_type": "response",
+ "sentiment": "up",
+ "page_url": bad_url,
+ },
+ )
+ assert resp.status_code == 422
+
+ def test_general_sentiment_is_stripped(self, feedback_db, client):
+ # A general payload that smuggles a sentiment must be stored with NULL
+ # sentiment so it never counts as a thumbs-down.
+ resp = client.post(
+ "/feedback",
+ json={
+ "community_id": "hed",
+ "feedback_type": "general",
+ "sentiment": "down",
+ "comment": "smuggled sentiment",
+ },
+ )
+ assert resp.status_code == 200
+ conn = get_metrics_connection(feedback_db)
+ try:
+ row = conn.execute(
+ "SELECT sentiment FROM feedback_log WHERE feedback_type='general'"
+ ).fetchone()
+ summary = get_feedback_summary(conn, community_id="hed")
+ finally:
+ conn.close()
+ assert row["sentiment"] is None
+ assert summary["thumbs_down"] == 0
+
+ def test_http_page_url_accepted(self, client):
+ resp = client.post(
+ "/feedback",
+ json={
+ "community_id": "hed",
+ "feedback_type": "response",
+ "sentiment": "up",
+ "page_url": "https://hedtags.org/page",
+ },
+ )
+ assert resp.status_code == 200
+
+ def test_persisted_and_readable(self, feedback_db, client):
+ client.post(
+ "/feedback",
+ json={"community_id": "hed", "feedback_type": "response", "sentiment": "up"},
+ )
+ conn = get_metrics_connection(feedback_db)
+ try:
+ summary = get_feedback_summary(conn, community_id="hed")
+ finally:
+ conn.close()
+ assert summary["thumbs_up"] == 1
+
+ def test_request_id_round_trips(self, feedback_db, client):
+ # The widget keys a vote on the request_id from the chat `done` event;
+ # the stored row must carry it so feedback joins back to request_log.
+ client.post(
+ "/feedback",
+ json={
+ "community_id": "hed",
+ "feedback_type": "response",
+ "sentiment": "down",
+ "request_id": "req-xyz-123",
+ "message_index": 4,
+ },
+ )
+ conn = get_metrics_connection(feedback_db)
+ try:
+ row = conn.execute("SELECT request_id, message_index FROM feedback_log").fetchone()
+ finally:
+ conn.close()
+ assert row["request_id"] == "req-xyz-123"
+ assert row["message_index"] == 4
+
+
+@pytest.mark.usefixtures("feedback_db", "scoped_auth_env")
+class TestReadFeedback:
+ """GET /metrics/feedback (scoped admin auth)."""
+
+ def _seed(self, client):
+ client.post(
+ "/feedback",
+ json={"community_id": "hed", "feedback_type": "response", "sentiment": "up"},
+ )
+ client.post(
+ "/feedback",
+ json={
+ "community_id": "eeglab",
+ "feedback_type": "response",
+ "sentiment": "down",
+ },
+ )
+
+ def test_admin_sees_all(self, client):
+ self._seed(client)
+ resp = client.get("/metrics/feedback", headers={"X-API-Key": ADMIN_KEY})
+ assert resp.status_code == 200
+ data = resp.json()
+ assert data["summary"]["thumbs_up"] == 1
+ assert data["summary"]["thumbs_down"] == 1
+
+ def test_admin_aggregates_across_communities(self, client):
+ # Two up votes in different communities must sum, proving the admin view
+ # is not silently scoped to one community.
+ client.post(
+ "/feedback",
+ json={"community_id": "hed", "feedback_type": "response", "sentiment": "up"},
+ )
+ client.post(
+ "/feedback",
+ json={"community_id": "eeglab", "feedback_type": "response", "sentiment": "up"},
+ )
+ resp = client.get("/metrics/feedback", headers={"X-API-Key": ADMIN_KEY})
+ assert resp.json()["summary"]["thumbs_up"] == 2
+
+ def test_comments_only_filter(self, client):
+ client.post(
+ "/feedback",
+ json={"community_id": "hed", "feedback_type": "response", "sentiment": "up"},
+ )
+ client.post(
+ "/feedback",
+ json={"community_id": "hed", "feedback_type": "general", "comment": "great tool"},
+ )
+ resp = client.get(
+ "/metrics/feedback",
+ params={"comments_only": "true"},
+ headers={"X-API-Key": HED_KEY},
+ )
+ entries = resp.json()["entries"]
+ assert len(entries) == 1
+ assert entries[0]["comment"] == "great tool"
+
+ def test_limit_above_max_rejected(self, client):
+ resp = client.get(
+ "/metrics/feedback",
+ params={"limit": 9999},
+ headers={"X-API-Key": HED_KEY},
+ )
+ assert resp.status_code == 422
+
+ def test_offset_pagination(self, client):
+ for i in range(3):
+ client.post(
+ "/feedback",
+ json={
+ "community_id": "hed",
+ "feedback_type": "general",
+ "comment": f"note {i}",
+ },
+ )
+ page1 = client.get(
+ "/metrics/feedback",
+ params={"limit": 2, "offset": 0},
+ headers={"X-API-Key": HED_KEY},
+ ).json()["entries"]
+ page2 = client.get(
+ "/metrics/feedback",
+ params={"limit": 2, "offset": 2},
+ headers={"X-API-Key": HED_KEY},
+ ).json()["entries"]
+ assert len(page1) == 2
+ assert len(page2) == 1
+ # No overlap between pages
+ seen = {e["comment"] for e in page1} | {e["comment"] for e in page2}
+ assert seen == {"note 0", "note 1", "note 2"}
+
+ def test_community_key_scoped_to_own(self, client):
+ self._seed(client)
+ resp = client.get("/metrics/feedback", headers={"X-API-Key": HED_KEY})
+ assert resp.status_code == 200
+ data = resp.json()
+ # hed key must only see hed's up vote, not eeglab's down vote
+ assert data["community_id"] == "hed"
+ assert data["summary"]["thumbs_up"] == 1
+ assert data["summary"]["thumbs_down"] == 0
+ assert all(e["community_id"] == "hed" for e in data["entries"])
+
+ def test_community_key_cannot_override_filter(self, client):
+ self._seed(client)
+ # hed key asks for eeglab; must still be scoped to hed
+ resp = client.get(
+ "/metrics/feedback",
+ params={"community_id": "eeglab"},
+ headers={"X-API-Key": HED_KEY},
+ )
+ data = resp.json()
+ assert data["community_id"] == "hed"
+ assert data["summary"]["thumbs_down"] == 0
+
+ def test_requires_auth(self, client):
+ resp = client.get("/metrics/feedback")
+ assert resp.status_code == 401
diff --git a/tests/test_metrics/test_feedback_db.py b/tests/test_metrics/test_feedback_db.py
new file mode 100644
index 0000000..515c246
--- /dev/null
+++ b/tests/test_metrics/test_feedback_db.py
@@ -0,0 +1,320 @@
+"""Tests for the feedback storage layer (feedback_log table + queries)."""
+
+import logging
+
+import pytest
+
+import src.metrics.db as db_module
+from src.metrics.db import (
+ FeedbackEntry,
+ get_metrics_connection,
+ init_metrics_db,
+ now_iso,
+ write_feedback,
+)
+from src.metrics.queries import get_feedback_entries, get_feedback_summary
+
+
+@pytest.fixture
+def reset_feedback_counter():
+ """Reset the module-global failure counter around tests that exercise it."""
+ db_module._write_feedback_failures = 0
+ yield
+ db_module._write_feedback_failures = 0
+
+
+@pytest.fixture
+def feedback_db(tmp_path):
+ """Create a temporary metrics database with feedback rows."""
+ db_path = tmp_path / "metrics.db"
+ init_metrics_db(db_path)
+
+ entries = [
+ FeedbackEntry(
+ feedback_id="f1",
+ timestamp="2025-01-15T10:00:00+00:00",
+ community_id="hed",
+ feedback_type="response",
+ sentiment="up",
+ request_id="r1",
+ session_id="s1",
+ message_index=0,
+ ),
+ FeedbackEntry(
+ feedback_id="f2",
+ timestamp="2025-01-15T11:00:00+00:00",
+ community_id="hed",
+ feedback_type="response",
+ sentiment="down",
+ request_id="r2",
+ session_id="s1",
+ message_index=2,
+ comment="answer was wrong about epochs",
+ ),
+ FeedbackEntry(
+ feedback_id="f3",
+ timestamp="2025-01-15T12:00:00+00:00",
+ community_id="hed",
+ feedback_type="general",
+ sentiment=None,
+ comment="love this assistant",
+ page_url="https://hedtags.org",
+ ),
+ # A different community, to prove scoping
+ FeedbackEntry(
+ feedback_id="f4",
+ timestamp="2025-01-15T13:00:00+00:00",
+ community_id="eeglab",
+ feedback_type="response",
+ sentiment="up",
+ ),
+ ]
+ for e in entries:
+ write_feedback(e, db_path=db_path)
+
+ return db_path
+
+
+class TestFeedbackTable:
+ """Schema creation for feedback_log."""
+
+ def test_table_created(self, tmp_path):
+ db_path = tmp_path / "metrics.db"
+ init_metrics_db(db_path)
+ conn = get_metrics_connection(db_path)
+ try:
+ tables = conn.execute(
+ "SELECT name FROM sqlite_master WHERE type='table' AND name='feedback_log'"
+ ).fetchall()
+ assert len(tables) == 1
+ finally:
+ conn.close()
+
+ def test_table_added_to_existing_db(self, tmp_path):
+ """A pre-existing DB without feedback_log gets the table on re-init.
+
+ This proves the CREATE TABLE IF NOT EXISTS path auto-provisions the
+ new table on already-deployed databases without a bespoke migration.
+ Simulated by initializing the full schema, dropping feedback_log to
+ recreate an "old" DB, then re-running init.
+ """
+ db_path = tmp_path / "metrics.db"
+ init_metrics_db(db_path)
+
+ conn = get_metrics_connection(db_path)
+ conn.execute("DROP TABLE feedback_log")
+ conn.commit()
+ conn.close()
+
+ # Re-init (mirrors a deploy bringing the new schema).
+ init_metrics_db(db_path)
+
+ conn = get_metrics_connection(db_path)
+ try:
+ tables = conn.execute(
+ "SELECT name FROM sqlite_master WHERE type='table' AND name='feedback_log'"
+ ).fetchall()
+ assert len(tables) == 1
+ finally:
+ conn.close()
+
+
+class TestWriteFeedback:
+ """write_feedback() persistence."""
+
+ def test_round_trip(self, tmp_path):
+ db_path = tmp_path / "metrics.db"
+ init_metrics_db(db_path)
+ write_feedback(
+ FeedbackEntry(
+ feedback_id="x1",
+ timestamp=now_iso(),
+ community_id="hed",
+ feedback_type="response",
+ sentiment="up",
+ request_id="req-abc",
+ ),
+ db_path=db_path,
+ )
+ conn = get_metrics_connection(db_path)
+ try:
+ row = conn.execute("SELECT * FROM feedback_log WHERE feedback_id = 'x1'").fetchone()
+ assert row["sentiment"] == "up"
+ assert row["request_id"] == "req-abc"
+ assert row["feedback_type"] == "response"
+ finally:
+ conn.close()
+
+ @pytest.mark.usefixtures("reset_feedback_counter")
+ def test_failure_counter_increments_and_resets(self, tmp_path):
+ # A UNIQUE feedback_id collision makes the second write fail at INSERT.
+ # write_feedback must swallow it, bump the counter, then reset on success.
+ db_path = tmp_path / "metrics.db"
+ init_metrics_db(db_path)
+ dup = FeedbackEntry(
+ feedback_id="dup",
+ timestamp=now_iso(),
+ community_id="hed",
+ feedback_type="response",
+ sentiment="up",
+ )
+ write_feedback(dup, db_path=db_path)
+ assert db_module._write_feedback_failures == 0
+ write_feedback(dup, db_path=db_path) # duplicate -> swallowed failure
+ assert db_module._write_feedback_failures == 1
+ write_feedback(
+ FeedbackEntry(
+ feedback_id="ok2",
+ timestamp=now_iso(),
+ community_id="hed",
+ feedback_type="response",
+ sentiment="up",
+ ),
+ db_path=db_path,
+ )
+ assert db_module._write_feedback_failures == 0
+
+ @pytest.mark.usefixtures("reset_feedback_counter")
+ def test_failure_escalates_to_critical(self, tmp_path, caplog):
+ db_path = tmp_path / "metrics.db"
+ init_metrics_db(db_path)
+ dup = FeedbackEntry(
+ feedback_id="d",
+ timestamp=now_iso(),
+ community_id="hed",
+ feedback_type="response",
+ sentiment="up",
+ )
+ write_feedback(dup, db_path=db_path) # first succeeds
+ with caplog.at_level(logging.CRITICAL):
+ for _ in range(10):
+ write_feedback(dup, db_path=db_path) # all collide
+ assert any(r.levelno == logging.CRITICAL for r in caplog.records)
+
+
+class TestFeedbackEntryInvariants:
+ """FeedbackEntry.__post_init__ rejects illegal shapes at the storage layer."""
+
+ def test_response_requires_sentiment(self):
+ with pytest.raises(ValueError, match="response feedback requires a sentiment"):
+ FeedbackEntry(
+ feedback_id="a",
+ timestamp=now_iso(),
+ community_id="hed",
+ feedback_type="response",
+ sentiment=None,
+ )
+
+ def test_general_must_not_carry_sentiment(self):
+ with pytest.raises(ValueError, match="general feedback must not carry a sentiment"):
+ FeedbackEntry(
+ feedback_id="b",
+ timestamp=now_iso(),
+ community_id="hed",
+ feedback_type="general",
+ sentiment="up",
+ comment="x",
+ )
+
+ def test_bad_feedback_type_rejected(self):
+ with pytest.raises(ValueError, match="feedback_type must be"):
+ FeedbackEntry(
+ feedback_id="c",
+ timestamp=now_iso(),
+ community_id="hed",
+ feedback_type="bogus",
+ )
+
+ def test_bad_sentiment_rejected(self):
+ with pytest.raises(ValueError, match="sentiment must be"):
+ FeedbackEntry(
+ feedback_id="d",
+ timestamp=now_iso(),
+ community_id="hed",
+ feedback_type="response",
+ sentiment="meh",
+ )
+
+
+class TestFeedbackSummary:
+ """get_feedback_summary() aggregation."""
+
+ def test_counts_for_community(self, feedback_db):
+ conn = get_metrics_connection(feedback_db)
+ try:
+ summary = get_feedback_summary(conn, community_id="hed")
+ finally:
+ conn.close()
+ assert summary["thumbs_up"] == 1
+ assert summary["thumbs_down"] == 1
+ assert summary["response_total"] == 2
+ assert summary["general_total"] == 1
+ assert summary["comment_total"] == 2 # f2 (note) + f3 (general)
+ assert summary["satisfaction_rate"] == 0.5
+
+ def test_counts_all_communities(self, feedback_db):
+ conn = get_metrics_connection(feedback_db)
+ try:
+ summary = get_feedback_summary(conn, community_id=None)
+ finally:
+ conn.close()
+ assert summary["thumbs_up"] == 2 # hed f1 + eeglab f4
+ assert summary["thumbs_down"] == 1
+
+ def test_satisfaction_rate_none_when_no_ratings(self, tmp_path):
+ db_path = tmp_path / "metrics.db"
+ init_metrics_db(db_path)
+ conn = get_metrics_connection(db_path)
+ try:
+ summary = get_feedback_summary(conn, community_id="hed")
+ finally:
+ conn.close()
+ assert summary["satisfaction_rate"] is None
+
+
+class TestFeedbackEntries:
+ """get_feedback_entries() listing."""
+
+ def test_scoped_to_community(self, feedback_db):
+ conn = get_metrics_connection(feedback_db)
+ try:
+ rows = get_feedback_entries(conn, community_id="hed")
+ finally:
+ conn.close()
+ assert len(rows) == 3
+ assert all(r["community_id"] == "hed" for r in rows)
+ # Most recent first
+ assert rows[0]["timestamp"] >= rows[-1]["timestamp"]
+
+ def test_comments_only_filter(self, feedback_db):
+ conn = get_metrics_connection(feedback_db)
+ try:
+ rows = get_feedback_entries(conn, community_id="hed", with_comment_only=True)
+ finally:
+ conn.close()
+ assert len(rows) == 2
+ assert all(r["comment"] for r in rows)
+
+ def test_limit_floor_clamped(self, feedback_db):
+ # limit=0 must be clamped up to 1 (without clamping, SQL LIMIT 0 would
+ # return zero rows). Asserting a non-empty result tests the clamp itself.
+ conn = get_metrics_connection(feedback_db)
+ try:
+ rows = get_feedback_entries(conn, community_id="hed", limit=0)
+ finally:
+ conn.close()
+ assert len(rows) == 1
+
+ def test_offset_pagination(self, feedback_db):
+ # hed has 3 entries; page through them with limit=2.
+ conn = get_metrics_connection(feedback_db)
+ try:
+ page1 = get_feedback_entries(conn, community_id="hed", limit=2, offset=0)
+ page2 = get_feedback_entries(conn, community_id="hed", limit=2, offset=2)
+ finally:
+ conn.close()
+ assert len(page1) == 2
+ assert len(page2) == 1
+ stamps = {r["timestamp"] for r in page1} | {r["timestamp"] for r in page2}
+ # No overlap, full coverage of the 3 hed rows.
+ assert len(stamps) == 3