Two unrelated features bundled because they ship together and share
migration 0028.
Strategic-log feedback (thumb up/down):
- New strategic_log_feedback table with UNIQUE(log_id, user_id) so each
user has one vote per log, flippable in place (up -> down -> clear).
UI shows aggregate counts only.
- app/services/log_feedback.py: set_vote, get_counts, sign/verify
feedback tokens (same itsdangerous pattern as auth.sign_pending,
30-day TTL for email links).
- POST /api/log/{id}/feedback: web vote, auth required, returns counts
+ the requesting user's own vote.
- GET /feedback?token=...&vote=...: email-link target, no auth, signed
token encodes (user, log, vote), renders feedback_thanks.html.
- partials/log.html: thumbs row below content, JS-driven swap via the
POST endpoint. Dashboard latest-log card and /log page both render
this partial via htmx, so the buttons appear in all three surfaces.
- digest emails: a "How was today's read?" row above the unsub footer,
with signed-token URLs against the latest StrategicLog at send time.
Plain-text fallback included.
Reviewer self-score (0-10):
- _SYSTEM_PROMPT asks for an integer score with anchors (10 exemplary,
5 borderline, 0 unfit). Verdict gains score: int | None.
- Deterministic-layer hits get score=0 (hard rule, no nuance);
error rows get None; LLM rows get the model's score clamped 0..10.
- ReviewerVerdict.score, StrategicLog.reviewer_score, and
IndicatorSummary.reviewer_score all new SMALLINT NULL columns.
- ai_log_job + indicator_summary_job persist verdict.score onto their
content rows when committing the row alongside content.
Tests:
- tests/test_strategic_log_feedback.py: vote, flip, clear, aggregate
across users, invalid vote, token round-trip + tamper + garbage +
'clear' not signable for email path.
- tests/test_output_review.py: score parsing, clamping (>10, <0),
missing/non-numeric -> None, deterministic-layer score=0.
Full suite: 427 passed (was 412), 5 skipped, no regressions.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
448 lines
20 KiB
Python
448 lines
20 KiB
Python
"""Two-layer reviewer for AI-generated reads.
|
||
|
||
Architecture (both layers fail-closed):
|
||
1. Deterministic lexicon/regex pre-check (cheap, zero-variance) — see
|
||
``app.services.output_review_lexicon``. Hard-catches the obvious
|
||
advice / forecast / level-trigger patterns.
|
||
2. LLM nuance check (Haiku via OpenRouter) — catches the subtle cases
|
||
the lexicon misses (chain-of-thought leakage, forward-state-as-
|
||
description, MAR forward price opinions on named instruments).
|
||
|
||
Either layer can reject. A reject drops the candidate; the previously
|
||
cached good version stays visible. Every verdict (pass and fail) is
|
||
persisted to ``reviewer_verdicts`` — the regulator-facing audit trail.
|
||
|
||
The reviewer is deliberately a tiny, JSON-shaped classifier — same
|
||
JSON-mode mechanism as the generator, so the verdict can't be lost in
|
||
prose. Cost is ~$0.0001 per LLM check; latency ~1-2 s in the hourly job.
|
||
"""
|
||
from __future__ import annotations
|
||
|
||
import json
|
||
from dataclasses import dataclass
|
||
|
||
import httpx
|
||
from sqlalchemy.ext.asyncio import AsyncSession
|
||
|
||
from app.config import get_settings
|
||
from app.logging import get_logger
|
||
from app.models import ReviewerVerdict
|
||
from app.services.openrouter import call_llm
|
||
from app.services.output_review_lexicon import check as lexicon_check
|
||
|
||
log = get_logger("output_review")
|
||
|
||
|
||
# The reviewer runs through OpenRouter against a small, non-thinking
|
||
# model. DeepSeek-V4-flash (our generator default) emits internal
|
||
# chain-of-thought before its JSON output even when the prompt forbids
|
||
# it, which truncates the JSON at any reasonable max_tokens cap and
|
||
# breaks the parser. Anthropic's Haiku family answers structured-output
|
||
# tasks tersely and deterministically — no chain-of-thought tax. Cost
|
||
# is ~$0.0001-$0.0003 per review depending on candidate length.
|
||
DEFAULT_REVIEWER_MODEL = "anthropic/claude-haiku-4.5"
|
||
|
||
|
||
_SYSTEM_PROMPT = """\
|
||
You are a strict editor for a financial-markets dashboard. The author
|
||
was asked to produce editorial commentary on public market data for
|
||
human readers. You receive the proposed text — it may be a one-line
|
||
read, a multi-paragraph daily log, a portfolio analysis, a chat
|
||
reply, or an email digest — and decide if it is publishable as-is.
|
||
|
||
Mark CLEAN only if the text reads like finished editorial commentary
|
||
a reader could see on a public dashboard without confusion.
|
||
|
||
Editorial framework you should KNOW about (don't flag these):
|
||
This dashboard's voice deliberately contrasts a "rational" read
|
||
(fundamentals, policy regime, valuation) with an "irrational" read
|
||
(positioning, narrative momentum, flows) and names the gap between
|
||
them. Section labels like "Rational:" / "Irrational:" (or "Bull /
|
||
Bear", or any explicit "X vs Y" contrast) are STRUCTURAL DEVICES,
|
||
not the author thinking on the page. Treat them as finished prose.
|
||
The Italian / Spanish / French / German equivalents
|
||
("Razionalmente / Irrazionalmente", "Racionalmente / Irracionalmente",
|
||
"Rationnellement / Irrationnellement", "Rational / Irrational") are
|
||
the same device translated and equally fine.
|
||
|
||
Mark UNCLEAN if the text contains ANY of:
|
||
- Chain-of-thought / scratchpad markers — the author thinking on the
|
||
page rather than presenting finished commentary. Phrases like
|
||
"Let me", "Let's see", "we need to", "actually" (correcting itself),
|
||
"wait", "hmm", "or rather", "I should". Rhetorical questions used
|
||
as structure are fine; questions that the author then answers in
|
||
front of the reader (self-questioning) are not.
|
||
- Self-questioning parentheticals: "Q1 2026? Actually Q4 2025?",
|
||
"is it X or Y?", any place where the author appears to be working
|
||
out the answer in front of the reader. The "rational vs irrational"
|
||
contrast above is NOT self-questioning — the author is presenting
|
||
both reads as parallel takes, not asking which one is correct.
|
||
- Meta-commentary about the task, output format, word limits, or
|
||
instructions — e.g. "as required by the constraints", "the prompt
|
||
asks", "let me address each".
|
||
- Partial / truncated content. Starts mid-word, mid-number, mid-clause,
|
||
ends mid-thought.
|
||
- Visible internal numbers without clear meaning ("change 1y +5.9%?"),
|
||
raw column names ("as_of 2026-01-01"), or any debug-like fragments.
|
||
- FINANCIAL ADVICE or any phrasing that recommends an action the
|
||
reader should take. This service is editorial commentary on public
|
||
data, not investment advice; the operator is not licensed to give
|
||
it. Reject any of:
|
||
* Buy/sell/hold/accumulate/trim/exit/enter/rotate language.
|
||
* Allocation guidance ("overweight", "underweight",
|
||
"X% in bonds", "increase exposure to").
|
||
* Price targets or specific level predictions ("will reach $X",
|
||
"target Y", "expect Z by year-end").
|
||
* Personalised framing ("you should", "investors should",
|
||
"consider buying", "we recommend").
|
||
DESCRIPTIVE / INTERPRETIVE language about market state is fine —
|
||
"valuations are stretched", "real yields are restrictive", "rates
|
||
and credit disagree". The test: does the text describe a STATE, or
|
||
does it suggest an ACTION? States are fine; actions are not.
|
||
- FORWARD PREDICTION DRESSED AS PRESENT-STATE DESCRIPTION. A claim
|
||
about direction smuggled into state-shaped language is still a
|
||
forecast, and an action-focused rule misses it.
|
||
* Allowed (pure state): "valuations are stretched", "real yields
|
||
are restrictive", "positioning is crowded".
|
||
* Reject (state + direction): "valuations are stretched and unlikely
|
||
to hold", "the path of least resistance is lower", "risk is
|
||
skewed to the downside", "the setup favours further weakness".
|
||
If the sentence implies which way prices go next, flag it.
|
||
- MAR INVESTMENT-RECOMMENDATION — a forward-looking opinion on the
|
||
price or value of a specific NAMED INSTRUMENT, even without an
|
||
explicit numeric target and even without a "you should" verb.
|
||
* Reject: "Brent is likely to consolidate near $90–93", "the
|
||
base case is a move to X for gold", "TSLA appears poised to
|
||
drift lower", "EUR/USD looks set to test new highs".
|
||
* Allowed: regime-level forward observation that does NOT name a
|
||
specific instrument's price trajectory ("the policy mix is still
|
||
tightening", "real yields are likely to stay restrictive while
|
||
the labour market holds up").
|
||
This rule applies to single tickers, single commodities, and single
|
||
FX pairs equally. A cross-asset *regime* claim is fine; a price
|
||
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>", "score": 0-10}
|
||
No preamble, no markdown fences, no other fields.
|
||
"""
|
||
|
||
|
||
# Surface-specific rider appended to the system prompt when the caller
|
||
# passes a known `surface` to review_read(). The portfolio rider — the only
|
||
# entry here — only fires when PORTFOLIO_AI_ENABLED is on AND the caller
|
||
# explicitly tags surface="portfolio". With the flag off (default) the
|
||
# strict base rules govern every surface; future re-enable requires both
|
||
# flipping the flag and a deliberate code review of this loosened block.
|
||
_SURFACE_RIDERS = {
|
||
"portfolio": """\
|
||
|
||
# Surface: portfolio commentary
|
||
This text describes a real investor's holdings. DESCRIPTIVE risk
|
||
language is the whole point of this surface and must NOT be flagged
|
||
as financial advice. The following ARE fine:
|
||
- Naming portfolio attributes: "high concentration", "single-name
|
||
exposure", "currency risk is unhedged", "FX exposure", "elevated
|
||
risk", "stretched valuations", "concentration is manageable", "low
|
||
diversification".
|
||
- Impersonal observation about a position's behaviour or sensitivity:
|
||
"the position warrants monitoring", "carries vulnerability to a
|
||
policy shock", "is sensitive to rate moves".
|
||
|
||
ONLY flag EXPLICIT calls to action where a verb or directive is
|
||
aimed at the reader:
|
||
- Imperative verbs in the second person: "buy X", "sell Y",
|
||
"trim Z", "hedge", "rotate into".
|
||
- "You should", "investors should", "consider X-ing", "we recommend".
|
||
- Specific allocation prescriptions: "go 20% bonds", "overweight
|
||
tech", "underweight defensives".
|
||
- Price-target predictions: "will reach $X by year-end".
|
||
- Forward conditional judgements on a held position ("the thesis
|
||
fails if rates retrace", "this view depends on X holding"): these
|
||
read as "you should be watching for X" and are out of scope.
|
||
""",
|
||
}
|
||
|
||
|
||
@dataclass(frozen=True)
|
||
class Verdict:
|
||
clean: bool
|
||
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
|
||
# to keep useful context, bounded to keep pathological inputs from blowing
|
||
# the row size.
|
||
_AUDIT_CANDIDATE_MAX = 16_000
|
||
|
||
|
||
async def _record_verdict(
|
||
session: AsyncSession | None,
|
||
*,
|
||
surface: str | None,
|
||
candidate: str,
|
||
verdict: Verdict,
|
||
model: str | None,
|
||
) -> None:
|
||
"""Best-effort persist the verdict to ``reviewer_verdicts``. Errors here
|
||
must NEVER mask the caller's verdict — wrap and swallow."""
|
||
if session is None:
|
||
return
|
||
try:
|
||
row = ReviewerVerdict(
|
||
surface=surface,
|
||
candidate_text=candidate[:_AUDIT_CANDIDATE_MAX],
|
||
clean=verdict.clean,
|
||
reason=verdict.reason[:240] if verdict.reason else None,
|
||
layer=verdict.layer,
|
||
model=model,
|
||
score=verdict.score,
|
||
)
|
||
session.add(row)
|
||
await session.flush()
|
||
except Exception as e:
|
||
log.warning("review.audit_write_failed", error=str(e)[:200])
|
||
|
||
|
||
async def review_read(
|
||
client: httpx.AsyncClient,
|
||
candidate: str,
|
||
surface: str | None = None,
|
||
*,
|
||
session: AsyncSession | None = None,
|
||
) -> Verdict:
|
||
"""Run the two-layer reviewer on `candidate`.
|
||
|
||
Layer 1: deterministic lexicon/regex pre-check. On hit, returns
|
||
``clean=False, reason="lexicon:<rule>"``, layer="deterministic" — no
|
||
LLM call is made.
|
||
|
||
Layer 2: LLM nuance check (Haiku via OpenRouter). On JSON-mode
|
||
success, returns whatever the model decided. Any error — provider
|
||
failure, JSON parse failure, missing field, wrong type — yields a
|
||
CONSERVATIVE verdict (clean=False) so the caller drops the candidate.
|
||
The previously cached good summary stays visible on the dashboard.
|
||
|
||
`surface` selects a surface-specific rider — see _SURFACE_RIDERS. The
|
||
only entry, "portfolio", is additionally gated behind
|
||
PORTFOLIO_AI_ENABLED so it cannot loosen review when the feature is
|
||
paused.
|
||
|
||
When ``session`` is provided, every verdict (pass and fail, every
|
||
layer) is persisted to ``reviewer_verdicts`` for the audit trail.
|
||
Persistence errors are swallowed."""
|
||
settings = get_settings()
|
||
|
||
if not candidate or not candidate.strip():
|
||
verdict = Verdict(clean=False, reason="empty candidate", cost_usd=0.0,
|
||
layer="deterministic", score=0)
|
||
await _record_verdict(session, surface=surface, candidate=candidate or "",
|
||
verdict=verdict, model=None)
|
||
return verdict
|
||
|
||
# Layer 1 — deterministic lexicon pre-check. Cheap, zero-variance.
|
||
hit = lexicon_check(candidate)
|
||
if hit is not None:
|
||
verdict = Verdict(
|
||
clean=False,
|
||
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)
|
||
await _record_verdict(session, surface=surface, candidate=candidate,
|
||
verdict=verdict, model=None)
|
||
return verdict
|
||
|
||
# Layer 2 — LLM nuance check. The portfolio rider is doubly gated: by
|
||
# PORTFOLIO_AI_ENABLED here, and by the only caller's own flag-gate.
|
||
system_prompt = _SYSTEM_PROMPT
|
||
if (
|
||
surface
|
||
and surface in _SURFACE_RIDERS
|
||
and (surface != "portfolio" or settings.PORTFOLIO_AI_ENABLED)
|
||
):
|
||
system_prompt = system_prompt + _SURFACE_RIDERS[surface]
|
||
|
||
messages = [
|
||
{"role": "system", "content": system_prompt},
|
||
# Sent as a fenced user turn so the model can't confuse the
|
||
# candidate with instructions, even if the candidate happens to
|
||
# contain prompt-like prose.
|
||
{"role": "user", "content": f"Candidate read:\n```\n{candidate}\n```"},
|
||
]
|
||
reviewer_model = getattr(settings, "REVIEWER_MODEL", None) or DEFAULT_REVIEWER_MODEL
|
||
try:
|
||
result = await call_llm(
|
||
client, messages,
|
||
# Pin to OpenRouter so a non-DeepSeek model like Haiku is
|
||
# actually reachable; the default provider chain would try
|
||
# DeepSeek native first and 404 on the Anthropic model name.
|
||
provider="openrouter",
|
||
model=reviewer_model,
|
||
# 300 tokens is well above the ~30-token JSON verdict.
|
||
# Haiku doesn't pad with hidden reasoning the way DeepSeek
|
||
# does, so we don't need the 800-token headroom required to
|
||
# absorb the generator's chain-of-thought.
|
||
max_tokens=300,
|
||
response_format={"type": "json_object"},
|
||
)
|
||
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", score=None)
|
||
await _record_verdict(session, surface=surface, candidate=candidate,
|
||
verdict=verdict, model=reviewer_model)
|
||
return verdict
|
||
|
||
# Haiku (and several other models) occasionally wrap their JSON
|
||
# output in a markdown code fence even with response_format set —
|
||
# ```json\n{...}\n``` — so strip a single leading/trailing fence
|
||
# before parsing. We do this defensively for any model; it's a
|
||
# no-op for callers that already emit bare JSON.
|
||
raw = result.content.strip()
|
||
if raw.startswith("```"):
|
||
first_nl = raw.find("\n")
|
||
if first_nl != -1:
|
||
raw = raw[first_nl + 1:]
|
||
if raw.rstrip().endswith("```"):
|
||
raw = raw.rstrip()[:-3].rstrip()
|
||
raw = raw.strip()
|
||
|
||
try:
|
||
parsed = json.loads(raw)
|
||
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", score=None)
|
||
await _record_verdict(session, surface=surface, candidate=candidate,
|
||
verdict=verdict, model=reviewer_model)
|
||
return verdict
|
||
|
||
clean = parsed.get("clean")
|
||
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", 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", score=score)
|
||
await _record_verdict(session, surface=surface, candidate=candidate,
|
||
verdict=verdict, model=reviewer_model)
|
||
return verdict
|
||
|
||
|
||
# --- Regenerate-on-fail helper -----------------------------------------------
|
||
|
||
|
||
# Maximum regenerate attempts before falling back to stale-cache. Two retries
|
||
# is the sweet spot: cheap enough on cost ($0.0003-$0.001 extra per rejected
|
||
# draft), enough to absorb the occasional model drift, and capped so a
|
||
# genuinely bad prompt template doesn't loop forever.
|
||
MAX_REGENERATE_ATTEMPTS = 2
|
||
|
||
|
||
@dataclass(frozen=True)
|
||
class ReviewedGeneration:
|
||
"""Result of generate_with_review. ``content`` is the accepted text (or
|
||
None if all attempts were rejected — caller falls back to stale cache).
|
||
``verdict`` is the final reviewer verdict (which may be a reject when
|
||
content is None). ``attempts`` counts how many generator calls ran."""
|
||
content: str | None
|
||
verdict: Verdict
|
||
attempts: int
|
||
|
||
|
||
async def generate_with_review(
|
||
*,
|
||
client: httpx.AsyncClient,
|
||
generate,
|
||
surface: str | None = None,
|
||
session: AsyncSession | None = None,
|
||
max_attempts: int = MAX_REGENERATE_ATTEMPTS + 1, # 1 initial + N retries
|
||
) -> ReviewedGeneration:
|
||
"""Run a generator, review it, and on reject feed the reviewer's reason
|
||
back into the generator for up to ``max_attempts - 1`` retries before
|
||
falling back. Returns ``ReviewedGeneration(content=None, ...)`` when
|
||
every attempt was rejected — caller decides how to fall back (typically
|
||
keep the stale cached version).
|
||
|
||
``generate`` is an async callable ``(reject_reason: str | None) -> str``
|
||
that produces a candidate. The reject_reason is None on the first call
|
||
and the prior verdict's reason on subsequent calls — the generator is
|
||
expected to thread it into its system prompt as guidance.
|
||
|
||
Every attempt's verdict is audited via the session — see review_read."""
|
||
last_verdict: Verdict | None = None
|
||
for attempt in range(1, max_attempts + 1):
|
||
reject_reason = last_verdict.reason if last_verdict else None
|
||
try:
|
||
candidate = await generate(reject_reason)
|
||
except Exception as e:
|
||
log.warning("review.generator_failed",
|
||
attempt=attempt, error=str(e)[:200])
|
||
return ReviewedGeneration(
|
||
content=None,
|
||
verdict=Verdict(clean=False,
|
||
reason=f"generator error: {str(e)[:80]}",
|
||
cost_usd=None, layer="error", score=None),
|
||
attempts=attempt,
|
||
)
|
||
|
||
verdict = await review_read(client, candidate, surface, session=session)
|
||
if verdict.clean:
|
||
return ReviewedGeneration(
|
||
content=candidate, verdict=verdict, attempts=attempt,
|
||
)
|
||
|
||
log.info("review.regenerate_attempt",
|
||
attempt=attempt, max=max_attempts,
|
||
reason=verdict.reason[:80] if verdict.reason else None,
|
||
layer=verdict.layer)
|
||
last_verdict = verdict
|
||
|
||
# Exhausted — every attempt was rejected.
|
||
return ReviewedGeneration(
|
||
content=None,
|
||
verdict=last_verdict or Verdict(clean=False, reason="no attempts",
|
||
cost_usd=None, layer="error",
|
||
score=None),
|
||
attempts=max_attempts,
|
||
)
|