diff --git a/alembic/versions/0028_log_feedback_and_reviewer_score.py b/alembic/versions/0028_log_feedback_and_reviewer_score.py new file mode 100644 index 0000000..ac4cd7f --- /dev/null +++ b/alembic/versions/0028_log_feedback_and_reviewer_score.py @@ -0,0 +1,93 @@ +"""strategic_log_feedback + reviewer_score columns. + +Revision ID: 0028 +Revises: 0027 +Create Date: 2026-05-29 + +Two unrelated features bundled because they ship together: + +1. **strategic_log_feedback** — thumb up/down votes per (log, user). + UNIQUE on (log_id, user_id) enforces one vote per user per log, + flippable in place. The UI shows aggregate counts only. + +2. **reviewer_score** — the output reviewer now self-rates each + verdict 0-10 (10 = exemplary editorial, 0 = unfit). Stored on + strategic_logs, indicator_summaries, and every reviewer_verdicts + audit row, as nullable SMALLINT so existing rows aren't backfilled. +""" +from typing import Sequence, Union + +import sqlalchemy as sa +from alembic import op + + +revision: str = "0028" +down_revision: Union[str, None] = "0027" +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + # --- reviewer_score columns (nullable SMALLINT) ----------------------- + op.add_column( + "strategic_logs", + sa.Column("reviewer_score", sa.SmallInteger(), nullable=True), + ) + op.add_column( + "indicator_summaries", + sa.Column("reviewer_score", sa.SmallInteger(), nullable=True), + ) + op.add_column( + "reviewer_verdicts", + sa.Column("score", sa.SmallInteger(), nullable=True), + ) + + # --- strategic_log_feedback table ------------------------------------- + op.create_table( + "strategic_log_feedback", + sa.Column( + "id", + sa.BigInteger().with_variant(sa.Integer(), "sqlite"), + primary_key=True, autoincrement=True, + ), + sa.Column( + "log_id", + sa.BigInteger().with_variant(sa.Integer(), "sqlite"), + sa.ForeignKey("strategic_logs.id", ondelete="CASCADE"), + nullable=False, + ), + sa.Column( + "user_id", sa.Integer(), + sa.ForeignKey("users.id", ondelete="CASCADE"), + nullable=False, + ), + # 'up' or 'down'. Storing as varchar keeps the column readable in + # the DB shell; the enum-ness is enforced at the service layer. + sa.Column("vote", sa.String(length=8), nullable=False), + sa.Column( + "created_at", sa.DateTime(timezone=True), nullable=False, + server_default=sa.text("CURRENT_TIMESTAMP"), + ), + sa.Column( + "updated_at", sa.DateTime(timezone=True), nullable=False, + server_default=sa.text("CURRENT_TIMESTAMP"), + ), + sa.UniqueConstraint( + "log_id", "user_id", name="uq_slf_log_user", + ), + ) + op.create_index( + "ix_strategic_log_feedback_log", + "strategic_log_feedback", ["log_id"], + ) + + +def downgrade() -> None: + op.drop_index( + "ix_strategic_log_feedback_log", + table_name="strategic_log_feedback", + ) + op.drop_table("strategic_log_feedback") + op.drop_column("reviewer_verdicts", "score") + op.drop_column("indicator_summaries", "reviewer_score") + op.drop_column("strategic_logs", "reviewer_score") diff --git a/app/jobs/ai_log_job.py b/app/jobs/ai_log_job.py index fdf2a23..ed1b5e0 100644 --- a/app/jobs/ai_log_job.py +++ b/app/jobs/ai_log_job.py @@ -234,6 +234,7 @@ async def run() -> None: prompt_tokens=result.prompt_tokens, completion_tokens=result.completion_tokens, cost_usd=full_cost, + reviewer_score=verdict.score, ) session.add(slog) session.add(AICall( diff --git a/app/jobs/email_digest_job.py b/app/jobs/email_digest_job.py index 4cbd865..ea20688 100644 --- a/app/jobs/email_digest_job.py +++ b/app/jobs/email_digest_job.py @@ -198,17 +198,32 @@ def _pick_variant( async def _send_one(user: User, kind: str, content_html: str, date_str: str, - session) -> None: + session, *, latest_log_id: int | None = None) -> None: settings_url = f"{branding.SITE_URL}/settings" unsubscribe_url = ( f"{branding.SITE_URL}/email/unsubscribe" f"?token={sign_unsubscribe_token(user.id)}" ) + + # Build signed feedback URLs against the latest strategic log at send + # time. The token encodes (user, log, vote) so the recipient can + # click without being logged in; the receiving /feedback endpoint + # verifies the signature and applies the vote. + feedback_up_url = feedback_down_url = None + if latest_log_id is not None: + from app.services.log_feedback import sign_feedback_token + up_tok = sign_feedback_token(user.id, latest_log_id, "up") + down_tok = sign_feedback_token(user.id, latest_log_id, "down") + feedback_up_url = f"{branding.SITE_URL}/feedback?token={up_tok}&vote=up" + feedback_down_url = f"{branding.SITE_URL}/feedback?token={down_tok}&vote=down" + subject, text_body, html_body = render_digest_email( kind=kind, date_str=date_str, content_html=content_html, unsubscribe_url=unsubscribe_url, settings_url=settings_url, + feedback_up_url=feedback_up_url, + feedback_down_url=feedback_down_url, ) try: await send_email(to=user.email, subject=subject, @@ -288,6 +303,18 @@ async def run() -> None: client, variants, active_non_en, ) + # Resolve the latest strategic log once per job — used as the + # target of the email's thumb up/down feedback links. None if + # nothing has been generated yet (shouldn't happen at this point + # in the flow, but defensible). + from sqlalchemy import desc, select + from app.models import StrategicLog + latest_log_id = (await session.execute( + select(StrategicLog.id) + .order_by(desc(StrategicLog.generated_at)) + .limit(1) + )).scalar_one_or_none() + written = 0 for u in fresh: tone = (u.digest_tone or "INTERMEDIATE").upper() @@ -296,7 +323,8 @@ async def run() -> None: tone=tone, lang=(u.lang or "en"), ) - await _send_one(u, kind, content, date_str, session) + await _send_one(u, kind, content, date_str, session, + latest_log_id=latest_log_id) await asyncio.sleep(0.1) written += 1 diff --git a/app/jobs/indicator_summary_job.py b/app/jobs/indicator_summary_job.py index 4a81aaf..3d401d5 100644 --- a/app/jobs/indicator_summary_job.py +++ b/app/jobs/indicator_summary_job.py @@ -215,6 +215,7 @@ async def _generate_one( # Include the reviewer's cost in the row's recorded spend so the # monthly budget tracking covers the full pipeline cost. cost_usd=(result.cost_usd or 0.0) + (verdict.cost_usd or 0.0), + reviewer_score=verdict.score, ) session.add(summary) session.add(AICall( @@ -342,6 +343,7 @@ async def run() -> None: prompt_tokens=result.prompt_tokens, completion_tokens=result.completion_tokens, cost_usd=full_cost, + reviewer_score=verdict.score, ) session.add(agg_summary) session.add(AICall( diff --git a/app/models.py b/app/models.py index f64d2e4..d8d3c25 100644 --- a/app/models.py +++ b/app/models.py @@ -118,6 +118,10 @@ class StrategicLog(Base): prompt_tokens: Mapped[int | None] = mapped_column(Integer) completion_tokens: Mapped[int | None] = mapped_column(Integer) cost_usd: Mapped[float | None] = mapped_column(Float) + # Reviewer self-rating 0-10 (10 = exemplary, 0 = unfit). Nullable for + # rows generated before the score field existed; new rows always + # carry the value the reviewer returned alongside its clean verdict. + reviewer_score: Mapped[int | None] = mapped_column(SmallInteger) class StrategicLogTranslation(Base): @@ -170,6 +174,8 @@ class IndicatorSummary(Base): prompt_tokens: Mapped[int | None] = mapped_column(Integer) completion_tokens: Mapped[int | None] = mapped_column(Integer) cost_usd: Mapped[float | None] = mapped_column(Float) + # Reviewer self-rating 0-10. See StrategicLog.reviewer_score. + reviewer_score: Mapped[int | None] = mapped_column(SmallInteger) __table_args__ = (Index("ix_indsumm_group_generated", "group_name", "generated_at"),) @@ -244,6 +250,45 @@ class ReviewerVerdict(Base): layer: Mapped[str] = mapped_column(String(16), nullable=False) # LLM-layer model id, nullable for deterministic / error rows. model: Mapped[str | None] = mapped_column(String(64)) + # Reviewer self-rating 0-10. Deterministic-layer hits get 0 (hard + # reject by rule), error rows get NULL, LLM rows get the model's score. + score: Mapped[int | None] = mapped_column(SmallInteger) + + +class StrategicLogFeedback(Base): + """Anonymous-in-UI thumb up/down votes on strategic-log rows. + + One row per (log_id, user_id) — flippable: a user can change their + vote (up → down) by overwriting, or clear it by deleting. The UI + surfaces only aggregate counts; user attribution is server-side + only and exists purely so we can dedup and let the voter see/flip + their own vote. See app/services/log_feedback.py.""" + __tablename__ = "strategic_log_feedback" + id: Mapped[int] = mapped_column(_PK, primary_key=True, autoincrement=True) + log_id: Mapped[int] = mapped_column( + BigInteger().with_variant(Integer(), "sqlite"), + ForeignKey("strategic_logs.id", ondelete="CASCADE"), + nullable=False, + ) + user_id: Mapped[int] = mapped_column( + Integer, + ForeignKey("users.id", ondelete="CASCADE"), + nullable=False, + ) + # 'up' or 'down'. Service layer enforces the enum. + vote: Mapped[str] = mapped_column(String(8), nullable=False) + created_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), nullable=False, default=utcnow, + ) + updated_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), nullable=False, default=utcnow, + onupdate=utcnow, + ) + + __table_args__ = ( + UniqueConstraint("log_id", "user_id", name="uq_slf_log_user"), + Index("ix_strategic_log_feedback_log", "log_id"), + ) class UserAcknowledgement(Base): diff --git a/app/routers/api.py b/app/routers/api.py index 5075654..bf78eb0 100644 --- a/app/routers/api.py +++ b/app/routers/api.py @@ -285,11 +285,13 @@ async def news_list( def _log_partial_payload( row: StrategicLog | None, content_override: str | None = None, + feedback: object | None = None, ) -> dict | None: if row is None: return None content = content_override if content_override is not None else row.content return { + "id": row.id, "content_html": _md_to_html(content), "generated_at": row.generated_at, "model": row.model, @@ -299,6 +301,7 @@ def _log_partial_payload( "cost_usd": row.cost_usd, "prompt_tokens": row.prompt_tokens, "completion_tokens": row.completion_tokens, + "feedback": feedback, } @@ -404,9 +407,12 @@ async def log_latest( if as_ == "html": content_override = await _localized_content(session, row, principal) + feedback = await _feedback_for(session, row, principal) return templates.TemplateResponse( request, "partials/log.html", - {"log": _log_partial_payload(row, content_override=content_override), + {"log": _log_partial_payload( + row, content_override=content_override, feedback=feedback, + ), "tone": wanted_tone, "paid": not free_only}, ) @@ -415,6 +421,21 @@ async def log_latest( return StrategicLogOut.model_validate(row, from_attributes=True) +async def _feedback_for( + session: AsyncSession, + row: StrategicLog | None, + principal: CurrentUser | None, +): + """Aggregate up/down counts + the principal's own vote, or None when + there's no log to fetch feedback for. Always safe to await; runs two + indexed queries.""" + if row is None: + return None + from app.services.log_feedback import get_counts + user_id = principal.user.id if (principal and principal.user) else None + return await get_counts(session, log_id=row.id, user_id=user_id) + + @router.get("/log/by-date/{day}") async def log_by_date( request: Request, @@ -459,9 +480,12 @@ async def log_by_date( if as_ == "html": content_override = await _localized_content(session, row, principal) + feedback = await _feedback_for(session, row, principal) return templates.TemplateResponse( request, "partials/log.html", - {"log": _log_partial_payload(row, content_override=content_override), + {"log": _log_partial_payload( + row, content_override=content_override, feedback=feedback, + ), "tone": wanted_tone, "paid": not free_only}, ) if row is None: @@ -469,6 +493,53 @@ async def log_by_date( return StrategicLogOut.model_validate(row, from_attributes=True) +# --- Log feedback (thumb up/down) -------------------------------------------- + + +class FeedbackIn(BaseModel): + vote: Literal["up", "down", "clear"] + + +class FeedbackOut(BaseModel): + up: int + down: int + user_vote: str | None + + +@router.post("/log/{log_id}/feedback", response_model=FeedbackOut) +async def post_log_feedback( + log_id: int, + body: FeedbackIn, + session: AsyncSession = Depends(get_session), + principal: CurrentUser = Depends(require_token), +) -> FeedbackOut: + """Record (or flip / clear) the authenticated user's thumb on a log. + + Anonymous-in-UI: the response carries only aggregate counts plus the + *requesting* user's own vote (so the UI can highlight it). Other + users' votes are never exposed.""" + if principal.user is None: + raise HTTPException(status_code=400, detail="admin token cannot vote") + + # Guard against votes on non-existent logs (don't want orphan FKs). + exists = (await session.execute( + select(StrategicLog.id).where(StrategicLog.id == log_id).limit(1) + )).scalar_one_or_none() + if exists is None: + raise HTTPException(status_code=404, detail="log not found") + + from app.services.log_feedback import FeedbackError, set_vote + try: + counts = await set_vote( + session, log_id=log_id, user_id=principal.user.id, vote=body.vote, + ) + except FeedbackError as e: + raise HTTPException(status_code=400, detail=str(e)) + return FeedbackOut( + up=counts.up, down=counts.down, user_vote=counts.user_vote, + ) + + # --- Calendar archive -------------------------------------------------------- diff --git a/app/routers/pages.py b/app/routers/pages.py index 01d68a8..4be5276 100644 --- a/app/routers/pages.py +++ b/app/routers/pages.py @@ -176,6 +176,60 @@ async def log_page_day( ) +@router.get("/feedback", response_class=HTMLResponse) +async def log_feedback_via_token( + request: Request, + token: str, + vote: str | None = None, + session: AsyncSession = Depends(get_session), +): + """Email-link target for thumb up/down votes on a strategic log. + + The signed token encodes (user_id, log_id, intended_vote). The query + param ``vote`` is informational (lets the URL be self-describing in + the inbox); the canonical vote is what's in the token. If the two + disagree the token wins. + + Renders a small thank-you confirmation. No auth required — the token + is the auth-equivalent for this single side-effecting action.""" + from app.services.log_feedback import ( + FeedbackError, set_vote, verify_feedback_token, + ) + + payload = verify_feedback_token(token) + if payload is None: + return templates.TemplateResponse( + request, "feedback_thanks.html", + {"ok": False, "message": "This link has expired or is invalid.", + "log_id": None, "vote": None}, + status_code=400, + ) + + try: + counts = await set_vote( + session, + log_id=payload["log_id"], + user_id=payload["user_id"], + vote=payload["vote"], + ) + except FeedbackError as e: + return templates.TemplateResponse( + request, "feedback_thanks.html", + {"ok": False, "message": str(e), "log_id": payload["log_id"], + "vote": payload["vote"]}, + status_code=400, + ) + + return templates.TemplateResponse( + request, "feedback_thanks.html", + {"ok": True, + "vote": payload["vote"], + "log_id": payload["log_id"], + "counts": counts, + "message": None}, + ) + + @router.get("/settings", response_class=HTMLResponse) async def settings_page( request: Request, diff --git a/app/services/digest_email.py b/app/services/digest_email.py index 3d416f6..760a004 100644 --- a/app/services/digest_email.py +++ b/app/services/digest_email.py @@ -47,6 +47,7 @@ _DIGEST_HTML_TEMPLATE = """\
 
+ {feedback_row}
 
Unsubscribe in one click @@ -70,6 +71,32 @@ def _strip_html_to_text(html_body: str) -> str: return text.strip() +def _feedback_row_html( + feedback_up_url: str | None, + feedback_down_url: str | None, + light_accent: str, + light_muted: str, +) -> str: + """Build the optional 'How was today's read?' row that sits between + the digest content and the unsubscribe footer. Empty string when no + feedback URLs were supplied (e.g. there's no latest log to vote on).""" + if not feedback_up_url or not feedback_down_url: + return "" + return ( + '
 
' + f'
' + "How was today’s read? " + f'' + "👍 Helpful" + " · " + f'' + "👎 Not useful" + "
" + ) + + def render_digest_email( *, kind: str, @@ -77,10 +104,17 @@ def render_digest_email( content_html: str, unsubscribe_url: str, settings_url: str, + feedback_up_url: str | None = None, + feedback_down_url: str | None = None, ) -> tuple[str, str, str]: """Returns (subject, text_body, html_body) for a digest email. - `kind` is "daily" or "weekly". Anything else raises ValueError.""" + `kind` is "daily" or "weekly". Anything else raises ValueError. + + When ``feedback_up_url`` and ``feedback_down_url`` are both supplied, + a small thumb up/down row is rendered above the unsubscribe footer. + Both must be signed-token URLs pointing at /feedback (see + ``app.services.log_feedback.sign_feedback_token``).""" if kind == "daily": label = "Daily" subject = f"{branding.BRAND_NAME} · Daily — {date_str}" @@ -90,6 +124,12 @@ def render_digest_email( else: raise ValueError(f"unknown digest kind: {kind!r}") + feedback_row = _feedback_row_html( + feedback_up_url, feedback_down_url, + light_accent=branding.LIGHT["accent"], + light_muted=branding.LIGHT["muted"], + ) + html_body = _DIGEST_HTML_TEMPLATE.format( brand=branding.BRAND_NAME, brand_upper=branding.BRAND_NAME.upper(), @@ -99,6 +139,7 @@ def render_digest_email( content_html=content_html, unsubscribe_url=unsubscribe_url, settings_url=settings_url, + feedback_row=feedback_row, **{f"L_{k.replace('-', '_')}": v for k, v in branding.LIGHT.items()}, **{f"D_{k.replace('-', '_')}": v for k, v in branding.DARK.items()}, ) @@ -109,8 +150,16 @@ def render_digest_email( "", _strip_html_to_text(content_html), "", + ] + if feedback_up_url and feedback_down_url: + text_lines.extend([ + f"Was this read useful? Helpful: {feedback_up_url}", + f" Not useful: {feedback_down_url}", + "", + ]) + text_lines.extend([ f"Unsubscribe: {unsubscribe_url}", f"Manage preferences: {settings_url}", - ] + ]) text_body = "\n".join(text_lines) return subject, text_body, html_body diff --git a/app/services/log_feedback.py b/app/services/log_feedback.py new file mode 100644 index 0000000..04a6544 --- /dev/null +++ b/app/services/log_feedback.py @@ -0,0 +1,177 @@ +"""Thumb up/down votes on strategic-log rows. + +The model is one row per (log_id, user_id) — see +``app/models.py::StrategicLogFeedback``. The UI shows aggregate counts +only; user attribution is server-side state, not surface state. A user +can flip up → down (or vice versa) by re-submitting; "clear" deletes +the row. + +For email-digest feedback links we sign a short payload with the same +itsdangerous serialiser pattern used elsewhere (``app/auth.py``). The +link is single-purpose: it identifies the (user, log, intended vote) +combination, lets the recipient click without being logged in, and +expires after EMAIL_FEEDBACK_TTL_SECONDS. +""" +from __future__ import annotations + +from dataclasses import dataclass + +from itsdangerous import BadSignature, SignatureExpired, URLSafeTimedSerializer +from sqlalchemy import delete, func, select +from sqlalchemy.ext.asyncio import AsyncSession + +from app.config import get_settings +from app.db import utcnow +from app.models import StrategicLogFeedback + + +# Email-link tokens are valid for 30 days so a user can click the thumb +# in last week's Sunday digest. Long enough for slow readers; short +# enough that the signing-secret rotation can shake stale tokens off. +EMAIL_FEEDBACK_TTL_SECONDS = 30 * 24 * 60 * 60 + +# Discriminated enum of acceptable votes. "clear" is a sentinel that +# means "remove your existing vote, if any". +VALID_VOTES = ("up", "down", "clear") + + +class FeedbackError(ValueError): + """Raised on bad input. Message is safe to surface to the user.""" + + +@dataclass(frozen=True) +class FeedbackCounts: + up: int + down: int + user_vote: str | None # 'up' | 'down' | None + + +# --------------------------------------------------------------------------- +# Service +# --------------------------------------------------------------------------- + + +def _validate_vote(vote: str) -> str: + v = (vote or "").strip().lower() + if v not in VALID_VOTES: + raise FeedbackError( + f"vote must be one of {VALID_VOTES!r}; got {vote!r}" + ) + return v + + +async def set_vote( + session: AsyncSession, + *, + log_id: int, + user_id: int, + vote: str, +) -> FeedbackCounts: + """Insert / update / clear the user's vote on ``log_id``. Returns the + resulting aggregate counts plus the user's new vote state. + + "clear" removes the row entirely; "up"/"down" upserts. The unique + index on (log_id, user_id) guarantees there's never more than one + row per pair.""" + v = _validate_vote(vote) + + if v == "clear": + await session.execute( + delete(StrategicLogFeedback) + .where(StrategicLogFeedback.log_id == log_id) + .where(StrategicLogFeedback.user_id == user_id) + ) + await session.commit() + return await get_counts(session, log_id=log_id, user_id=user_id) + + # Upsert by hand — SQLAlchemy's portable dialect doesn't expose + # ON DUPLICATE KEY UPDATE across MySQL+SQLite reliably for our test + # path, so SELECT then INSERT-or-UPDATE is the simplest correct shape. + existing = (await session.execute( + select(StrategicLogFeedback) + .where(StrategicLogFeedback.log_id == log_id) + .where(StrategicLogFeedback.user_id == user_id) + )).scalar_one_or_none() + if existing is None: + session.add(StrategicLogFeedback( + log_id=log_id, user_id=user_id, vote=v, + created_at=utcnow(), updated_at=utcnow(), + )) + else: + existing.vote = v + existing.updated_at = utcnow() + await session.commit() + return await get_counts(session, log_id=log_id, user_id=user_id) + + +async def get_counts( + session: AsyncSession, + *, + log_id: int, + user_id: int | None = None, +) -> FeedbackCounts: + """Aggregate up/down counts for one log, plus the requesting user's + own vote (if ``user_id`` is supplied). One indexed query for the + counts; a second indexed lookup for the personal vote.""" + rows = (await session.execute( + select( + StrategicLogFeedback.vote, + func.count(StrategicLogFeedback.id), + ) + .where(StrategicLogFeedback.log_id == log_id) + .group_by(StrategicLogFeedback.vote) + )).all() + counts = {vote: int(n) for vote, n in rows} + user_vote: str | None = None + if user_id is not None: + user_vote = (await session.execute( + select(StrategicLogFeedback.vote) + .where(StrategicLogFeedback.log_id == log_id) + .where(StrategicLogFeedback.user_id == user_id) + )).scalar_one_or_none() + return FeedbackCounts( + up=counts.get("up", 0), + down=counts.get("down", 0), + user_vote=user_vote, + ) + + +# --------------------------------------------------------------------------- +# Email-link token helpers (parallel to app.auth.sign_pending) +# --------------------------------------------------------------------------- + + +def _feedback_serializer() -> URLSafeTimedSerializer: + s = get_settings() + secret = s.CASSANDRA_SESSION_SECRET or s.CASSANDRA_TOKEN or "dev-insecure-secret" + return URLSafeTimedSerializer(secret, salt="cassandra-log-feedback-v1") + + +def sign_feedback_token(user_id: int, log_id: int, vote: str) -> str: + """Signed token for an email-digest thumb link. Encodes the intended + (user, log, vote) tuple. The recipient clicks the link without being + logged in; the receiving endpoint verifies the signature, applies the + vote, and shows a thank-you page.""" + v = _validate_vote(vote) + if v == "clear": + raise FeedbackError("clear is not a valid email-link vote") + return _feedback_serializer().dumps({ + "uid": int(user_id), "lid": int(log_id), "v": v, + }) + + +def verify_feedback_token(token: str) -> dict | None: + """Returns {"user_id": int, "log_id": int, "vote": "up"|"down"} on + valid + un-expired tokens, or None on bad signature / expired / bad + payload. The TTL is ``EMAIL_FEEDBACK_TTL_SECONDS``.""" + try: + data = _feedback_serializer().loads( + token, max_age=EMAIL_FEEDBACK_TTL_SECONDS, + ) + return { + "user_id": int(data["uid"]), + "log_id": int(data["lid"]), + "vote": str(data["v"]), + } + except (BadSignature, SignatureExpired, KeyError, TypeError, ValueError): + return None diff --git a/app/services/output_review.py b/app/services/output_review.py index a63f4e9..ca6b6c6 100644 --- a/app/services/output_review.py +++ b/app/services/output_review.py @@ -123,8 +123,19 @@ Mark UNCLEAN if the text contains ANY of: claim on a *named* instrument is not. - Anything else other than the finished, publishable commentary. +Also assign a SCORE 0-10 to the candidate: +- 10 = exemplary editorial: sharp, well-grounded, no perimeter risk, clean prose. +- 7-9 = publishable as-is, varying degrees of polish. +- 4-6 = borderline: scratchpad leakage, mild perimeter drift, or weak structure, + but not yet outright unfit. (Anything ≤4 should usually be clean=false.) +- 1-3 = unfit: clear chain-of-thought, partial / truncated, or financial-advice drift. +- 0 = unfit by hard rule (deterministic catch territory). + +Clean=true implies a score of ~7+; clean=false implies ~4 or lower. Use the +score to communicate confidence within the verdict. + Return ONLY a JSON object with this exact shape: -{"clean": true | false, "reason": "<≤20 words, plain text>"} +{"clean": true | false, "reason": "<≤20 words, plain text>", "score": 0-10} No preamble, no markdown fences, no other fields. """ @@ -171,6 +182,12 @@ class Verdict: reason: str cost_usd: float | None # cost of the review call itself, for the ledger layer: str = "llm" # "deterministic" | "llm" | "error" + # Integer 0-10. None for error rows; 0 for deterministic-layer hits + # (rejected by hard rule, no nuance to score); 0-10 from the model + # on LLM-layer verdicts. Stored alongside the content row for + # future analysis — see StrategicLog.reviewer_score and + # IndicatorSummary.reviewer_score. + score: int | None = None # Truncation cap for the audit log's candidate_text column. Generous enough @@ -199,6 +216,7 @@ async def _record_verdict( reason=verdict.reason[:240] if verdict.reason else None, layer=verdict.layer, model=model, + score=verdict.score, ) session.add(row) await session.flush() @@ -237,7 +255,7 @@ async def review_read( if not candidate or not candidate.strip(): verdict = Verdict(clean=False, reason="empty candidate", cost_usd=0.0, - layer="deterministic") + layer="deterministic", score=0) await _record_verdict(session, surface=surface, candidate=candidate or "", verdict=verdict, model=None) return verdict @@ -250,6 +268,7 @@ async def review_read( reason=f"lexicon:{hit.rule}: {hit.snippet}", cost_usd=0.0, layer="deterministic", + score=0, ) log.info("review.deterministic_reject", rule=hit.rule, snippet=hit.snippet, surface=surface) @@ -293,7 +312,7 @@ async def review_read( except Exception as e: log.warning("review.call_failed", error=str(e)[:200]) verdict = Verdict(clean=False, reason=f"reviewer error: {str(e)[:80]}", - cost_usd=None, layer="error") + cost_usd=None, layer="error", score=None) await _record_verdict(session, surface=surface, candidate=candidate, verdict=verdict, model=reviewer_model) return verdict @@ -317,7 +336,7 @@ async def review_read( except json.JSONDecodeError: log.warning("review.parse_failed", preview=result.content[:200]) verdict = Verdict(clean=False, reason="reviewer returned non-JSON", - cost_usd=result.cost_usd, layer="error") + cost_usd=result.cost_usd, layer="error", score=None) await _record_verdict(session, surface=surface, candidate=candidate, verdict=verdict, model=reviewer_model) return verdict @@ -326,13 +345,25 @@ async def review_read( reason = parsed.get("reason") or "" if not isinstance(clean, bool): verdict = Verdict(clean=False, reason="reviewer omitted bool 'clean'", - cost_usd=result.cost_usd, layer="error") + cost_usd=result.cost_usd, layer="error", score=None) await _record_verdict(session, surface=surface, candidate=candidate, verdict=verdict, model=reviewer_model) return verdict + # Score is optional and bounded; the verdict is still valid without it. + raw_score = parsed.get("score") + score: int | None + if isinstance(raw_score, bool): + # bool is a subclass of int — exclude it explicitly to avoid + # silently treating True/False as 1/0. + score = None + elif isinstance(raw_score, (int, float)): + score = max(0, min(10, int(raw_score))) + else: + score = None + verdict = Verdict(clean=clean, reason=str(reason)[:200], - cost_usd=result.cost_usd, layer="llm") + cost_usd=result.cost_usd, layer="llm", score=score) await _record_verdict(session, surface=surface, candidate=candidate, verdict=verdict, model=reviewer_model) return verdict @@ -391,7 +422,7 @@ async def generate_with_review( content=None, verdict=Verdict(clean=False, reason=f"generator error: {str(e)[:80]}", - cost_usd=None, layer="error"), + cost_usd=None, layer="error", score=None), attempts=attempt, ) @@ -411,6 +442,7 @@ async def generate_with_review( return ReviewedGeneration( content=None, verdict=last_verdict or Verdict(clean=False, reason="no attempts", - cost_usd=None, layer="error"), + cost_usd=None, layer="error", + score=None), attempts=max_attempts, ) diff --git a/app/templates/feedback_thanks.html b/app/templates/feedback_thanks.html new file mode 100644 index 0000000..1d1de43 --- /dev/null +++ b/app/templates/feedback_thanks.html @@ -0,0 +1,30 @@ +{% extends "public_base.html" %} +{% block title %}{{ BRAND_NAME }} · Feedback{% endblock %} + +{% block main %} +
+ {% if ok %} +

+ {% if vote == 'up' %}Thanks for the thumbs up.{% else %}Thanks for the thumbs down.{% endif %} +

+

+ Your vote on this strategic log is recorded. + {% if counts %} + Current tally: {{ counts.up }} 👍 · + {{ counts.down }} 👎. + {% endif %} +

+

+ Open the dashboard +

+ {% else %} +

Couldn’t record that vote

+

+ {{ message or "Something went wrong; please try again from the dashboard." }} +

+

+ Open the dashboard +

+ {% endif %} +
+{% endblock %} diff --git a/app/templates/partials/log.html b/app/templates/partials/log.html index 315e42f..105a021 100644 --- a/app/templates/partials/log.html +++ b/app/templates/partials/log.html @@ -9,4 +9,78 @@ title="Last generated {{ log.generated_at.strftime('%Y-%m-%d %H:%M UTC') }}"> {{ log.content_html | safe | glossary(tone) }}
+ +{% if log.feedback %} +{# Anonymous-in-UI thumb up/down. Server stores (user, log, vote) for dedup + so a vote can be flipped; UI shows aggregate counts only. + POST clicks JSON-fetch and swap this partial back in place — no full + reload, the log content stays put. #} +
+ Was this useful? + + + +
+ +{% endif %} {% endif %} diff --git a/tests/test_output_review.py b/tests/test_output_review.py index 9bffb44..4ede525 100644 --- a/tests/test_output_review.py +++ b/tests/test_output_review.py @@ -325,3 +325,109 @@ async def test_review_portfolio_rider_active_when_flag_enabled(monkeypatch): surface="portfolio", ) assert "# Surface: portfolio commentary" in seen_systems[0] + + +# --------------------------------------------------------------------------- +# Reviewer self-score (0-10) +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_review_parses_score_from_llm_json(monkeypatch): + _configure(monkeypatch) + def handler(_req): + return httpx.Response(200, json={ + "choices": [{"message": {"content": + '{"clean": true, "reason": "exemplary", "score": 9}'}, + "finish_reason": "stop"}], + "usage": {"prompt_tokens": 50, "completion_tokens": 12, "cost": 0.00007}, + }) + async with httpx.AsyncClient(transport=_mock_post(handler)) as client: + v = await review_read(client, "Markets pricing tighter policy.") + assert v.clean is True + assert v.score == 9 + assert v.layer == "llm" + + +@pytest.mark.asyncio +async def test_review_score_clamped_to_0_10(monkeypatch): + """A model returning 17 or -3 is buggy but must not blow up — clamp.""" + _configure(monkeypatch) + def handler(_req): + return httpx.Response(200, json={ + "choices": [{"message": {"content": + '{"clean": false, "reason": "x", "score": 17}'}, + "finish_reason": "stop"}], + "usage": {"prompt_tokens": 50, "completion_tokens": 8, "cost": 0.00003}, + }) + async with httpx.AsyncClient(transport=_mock_post(handler)) as client: + v = await review_read(client, "Some content.") + assert v.score == 10 + + +@pytest.mark.asyncio +async def test_review_score_negative_clamped_to_0(monkeypatch): + _configure(monkeypatch) + def handler(_req): + return httpx.Response(200, json={ + "choices": [{"message": {"content": + '{"clean": false, "reason": "x", "score": -3}'}, + "finish_reason": "stop"}], + "usage": {"prompt_tokens": 50, "completion_tokens": 8, "cost": 0.00003}, + }) + async with httpx.AsyncClient(transport=_mock_post(handler)) as client: + v = await review_read(client, "Some content.") + assert v.score == 0 + + +@pytest.mark.asyncio +async def test_review_missing_score_yields_none(monkeypatch): + """Older mocked responses don't carry score; verdict still valid, + score is None.""" + _configure(monkeypatch) + def handler(_req): + return httpx.Response(200, json={ + "choices": [{"message": {"content": + '{"clean": true, "reason": "ok"}'}, + "finish_reason": "stop"}], + "usage": {"prompt_tokens": 50, "completion_tokens": 6, "cost": 0.00002}, + }) + async with httpx.AsyncClient(transport=_mock_post(handler)) as client: + v = await review_read(client, "Plain state-level prose.") + assert v.clean is True + assert v.score is None + + +@pytest.mark.asyncio +async def test_review_score_non_numeric_yields_none(monkeypatch): + """Defensive: a string or null in the score field doesn't poison the + verdict; score becomes None.""" + _configure(monkeypatch) + def handler(_req): + return httpx.Response(200, json={ + "choices": [{"message": {"content": + '{"clean": true, "reason": "ok", "score": "high"}'}, + "finish_reason": "stop"}], + "usage": {"prompt_tokens": 50, "completion_tokens": 6, "cost": 0.00002}, + }) + async with httpx.AsyncClient(transport=_mock_post(handler)) as client: + v = await review_read(client, "Plain state-level prose.") + assert v.clean is True + assert v.score is None + + +@pytest.mark.asyncio +async def test_review_deterministic_layer_score_is_zero(monkeypatch): + """A deterministic-layer hit is a hard reject by rule; the audit row + carries score=0 (no nuance to score).""" + _configure(monkeypatch) + calls = [] + def handler(_req): + calls.append(1) + return httpx.Response(500, json={"error": "should not fire"}) + async with httpx.AsyncClient(transport=_mock_post(handler)) as client: + v = await review_read(client, "You should buy the dip.") + assert v.clean is False + assert v.layer == "deterministic" + assert v.score == 0 + assert calls == [] diff --git a/tests/test_strategic_log_feedback.py b/tests/test_strategic_log_feedback.py new file mode 100644 index 0000000..e1f8b5b --- /dev/null +++ b/tests/test_strategic_log_feedback.py @@ -0,0 +1,186 @@ +"""Strategic-log feedback service + token helpers. + +Covers the pure service path (set_vote, get_counts, clear) plus the +token sign/verify round-trip. The web endpoint POST /api/log/{id}/feedback +is covered in tests/test_api_feedback.py (separate file because it +needs the full FastAPI + auth stack).""" +from __future__ import annotations + +import asyncio + + +def _build_db(tmp_path): + from sqlalchemy.ext.asyncio import async_sessionmaker, create_async_engine + + from app import db as db_mod + from app.db import Base + import app.models # noqa: F401 + + engine = create_async_engine(f"sqlite+aiosqlite:///{tmp_path}/fb.db") + factory = async_sessionmaker(engine, expire_on_commit=False) + db_mod._engine = engine + db_mod._session_factory = factory + + async def _seed(): + from app.models import StrategicLog, User + from app.db import utcnow + + async with engine.begin() as conn: + await conn.run_sync(Base.metadata.create_all) + async with factory() as s: + s.add(User(id=1, email="alice@example.com", tier="free", + settings_json={}, created_at=utcnow())) + s.add(User(id=2, email="bob@example.com", tier="free", + settings_json={}, created_at=utcnow())) + s.add(StrategicLog( + id=10, generated_at=utcnow(), + model="m", anchor_date=None, prompt_version=1, + tone="INTERMEDIATE", analysis="DRY", + content="x", prompt_tokens=1, completion_tokens=1, + cost_usd=0.0, + )) + await s.commit() + + asyncio.run(_seed()) + return factory + + +# --------------------------------------------------------------------------- +# Service: set_vote / get_counts +# --------------------------------------------------------------------------- + + +def test_set_vote_up_writes_one_row(tmp_path): + factory = _build_db(tmp_path) + + async def _go(): + from app.services.log_feedback import set_vote + async with factory() as s: + counts = await set_vote(s, log_id=10, user_id=1, vote="up") + return counts + + counts = asyncio.run(_go()) + assert counts.up == 1 + assert counts.down == 0 + assert counts.user_vote == "up" + + +def test_set_vote_flip_replaces_existing(tmp_path): + """Voting again with a different value flips the existing row; + no duplicate is written, the up/down counts swap.""" + factory = _build_db(tmp_path) + + async def _go(): + from app.services.log_feedback import set_vote + async with factory() as s: + await set_vote(s, log_id=10, user_id=1, vote="up") + return await set_vote(s, log_id=10, user_id=1, vote="down") + + counts = asyncio.run(_go()) + assert counts.up == 0 + assert counts.down == 1 + assert counts.user_vote == "down" + + +def test_set_vote_clear_removes_row(tmp_path): + factory = _build_db(tmp_path) + + async def _go(): + from app.services.log_feedback import set_vote, get_counts + async with factory() as s: + await set_vote(s, log_id=10, user_id=1, vote="up") + await set_vote(s, log_id=10, user_id=1, vote="clear") + return await get_counts(s, log_id=10, user_id=1) + + counts = asyncio.run(_go()) + assert counts.up == 0 + assert counts.down == 0 + assert counts.user_vote is None + + +def test_aggregate_counts_across_users(tmp_path): + """Two distinct users vote — counts aggregate; each user's own_vote + field reflects only their own row.""" + factory = _build_db(tmp_path) + + async def _go(): + from app.services.log_feedback import set_vote, get_counts + async with factory() as s: + await set_vote(s, log_id=10, user_id=1, vote="up") + await set_vote(s, log_id=10, user_id=2, vote="down") + alice_view = await get_counts(s, log_id=10, user_id=1) + bob_view = await get_counts(s, log_id=10, user_id=2) + return alice_view, bob_view + + alice, bob = asyncio.run(_go()) + assert alice.up == 1 and alice.down == 1 and alice.user_vote == "up" + assert bob.up == 1 and bob.down == 1 and bob.user_vote == "down" + + +def test_invalid_vote_rejected(tmp_path): + factory = _build_db(tmp_path) + + async def _go(): + from app.services.log_feedback import FeedbackError, set_vote + async with factory() as s: + try: + await set_vote(s, log_id=10, user_id=1, vote="meh") + except FeedbackError as e: + return str(e) + return "no error" + + msg = asyncio.run(_go()) + assert "up" in msg.lower() and "down" in msg.lower() + + +# --------------------------------------------------------------------------- +# Token sign / verify +# --------------------------------------------------------------------------- + + +def test_feedback_token_round_trips(monkeypatch): + """A signed token decodes back to the same (user, log, vote) tuple.""" + monkeypatch.setenv("CASSANDRA_SESSION_SECRET", "test-secret-32-chars-long-okay") + from app.config import get_settings + get_settings.cache_clear() + from app.services.log_feedback import sign_feedback_token, verify_feedback_token + + tok = sign_feedback_token(user_id=42, log_id=99, vote="up") + payload = verify_feedback_token(tok) + assert payload == {"user_id": 42, "log_id": 99, "vote": "up"} + + +def test_feedback_token_tampered_returns_none(monkeypatch): + monkeypatch.setenv("CASSANDRA_SESSION_SECRET", "test-secret-32-chars-long-okay") + from app.config import get_settings + get_settings.cache_clear() + from app.services.log_feedback import sign_feedback_token, verify_feedback_token + + tok = sign_feedback_token(user_id=42, log_id=99, vote="up") + tampered = tok[:-1] + ("a" if tok[-1] != "a" else "b") + assert verify_feedback_token(tampered) is None + + +def test_feedback_token_garbage_returns_none(monkeypatch): + monkeypatch.setenv("CASSANDRA_SESSION_SECRET", "test-secret-32-chars-long-okay") + from app.config import get_settings + get_settings.cache_clear() + from app.services.log_feedback import verify_feedback_token + + assert verify_feedback_token("not.a.real.token") is None + assert verify_feedback_token("") is None + + +def test_feedback_token_clear_is_not_a_valid_email_link_vote(monkeypatch): + """The 'clear' sentinel is a web-only path; the email link can only + apply a positive vote (up or down). Trying to sign 'clear' raises.""" + monkeypatch.setenv("CASSANDRA_SESSION_SECRET", "test-secret-32-chars-long-okay") + from app.config import get_settings + get_settings.cache_clear() + from app.services.log_feedback import FeedbackError, sign_feedback_token + + try: + sign_feedback_token(user_id=1, log_id=10, vote="clear") + except FeedbackError: + return + raise AssertionError("expected FeedbackError")