diff --git a/backend/app/routers/letters.py b/backend/app/routers/letters.py index 4812bbb..12ee48b 100644 --- a/backend/app/routers/letters.py +++ b/backend/app/routers/letters.py @@ -26,6 +26,7 @@ from app.services.pdf_pages import PdfRenderError from app.services.persistence import persist_extraction from app.services.storage import detect_magic_mime, save_letter_file +from app.services.urgency import urgency_for_deadline router = APIRouter(prefix="/api/letters", tags=["letters"]) @@ -310,18 +311,26 @@ def list_letters( stmt = stmt.where(Letter.category == parsed_category) stmt = stmt.order_by(Letter.created_at.desc()) items = list(db.scalars(stmt).all()) - return [ - LetterListItem( - id=letter.id, - letter_type=letter.letter_type or letter.document_type, - category=letter.category, - risk_score=letter.risk_score, - deadline_date=letter.deadline_date, - status=letter.status, - created_at=letter.created_at, + result = [] + for letter in items: + # Compute both fields from a single `today` snapshot so a midnight + # rollover between two separate `date.today()` calls can never yield + # a contradictory (days_remaining, urgency) pair for the same letter. + days_remaining, urgency = urgency_for_deadline(letter.deadline_date) + result.append( + LetterListItem( + id=letter.id, + letter_type=letter.letter_type or letter.document_type, + category=letter.category, + risk_score=letter.risk_score, + deadline_date=letter.deadline_date, + days_remaining=days_remaining, + urgency=urgency, + status=letter.status, + created_at=letter.created_at, + ) ) - for letter in items - ] + return result # --- GET /api/letters/{id}/process (SSE) --------------------------------- diff --git a/backend/app/schemas.py b/backend/app/schemas.py index 04a5558..4b87fe1 100644 --- a/backend/app/schemas.py +++ b/backend/app/schemas.py @@ -60,6 +60,8 @@ class LetterListItem(BaseModel): category: DocumentCategory risk_score: int deadline_date: Optional[date] = None + days_remaining: Optional[int] = None + urgency: Literal["red", "yellow", "green"] status: LetterStatus created_at: datetime diff --git a/backend/app/services/__init__.py b/backend/app/services/__init__.py index 3818b10..a824203 100644 --- a/backend/app/services/__init__.py +++ b/backend/app/services/__init__.py @@ -13,6 +13,7 @@ from app.services.persistence import persist_extraction from app.services.risk import compute_risk from app.services.storage import detect_magic_mime, is_pdf, save_letter_file, user_dir +from app.services.urgency import compute_days_remaining, compute_urgency __all__ = [ "DOCUMENT_CATEGORIES", @@ -31,4 +32,6 @@ "is_pdf", "save_letter_file", "user_dir", + "compute_days_remaining", + "compute_urgency", ] diff --git a/backend/app/services/urgency.py b/backend/app/services/urgency.py new file mode 100644 index 0000000..1fe1c05 --- /dev/null +++ b/backend/app/services/urgency.py @@ -0,0 +1,77 @@ +"""Shared urgency computation — canonical thresholds for red/yellow/green. + +Computed at read time from `Letter.deadline_date`; never persisted. This is +the single source of truth for the urgency thresholds so callers (currently +`GET /api/letters`'s `list_letters`, and later KLAR-T2's SSE `urgency` event) +derive the same color from the same days-remaining calculation instead of +duplicating the boundary logic. + +Thresholds: + - red: days_remaining <= 7 (including negative/overdue values) + - yellow: 8 <= days_remaining <= 21 + - green: days_remaining > 21, OR deadline_date is None +""" + +from datetime import date +from typing import Literal + +Urgency = Literal["red", "yellow", "green"] + + +def compute_days_remaining(deadline_date: date | None, *, today: date | None = None) -> int | None: + """None if no deadline; otherwise (deadline_date - today).days. + + May be negative if the deadline is already overdue. Pure calendar-day + arithmetic via `date.today()` — matches the convention already used by + `services/risk.py`'s `_deadline_proximity`. + + `today` may be passed explicitly so callers that also need `urgency` can + snapshot "today" once and derive both values from it atomically (see + `urgency_for_deadline`) instead of each function calling `date.today()` + independently, which risks a midnight rollover producing contradictory + days_remaining/urgency pairs. + """ + if deadline_date is None: + return None + if today is None: + today = date.today() + return (deadline_date - today).days + + +def urgency_from_days_remaining(days_remaining: int | None) -> Urgency: + """Derive the urgency color from an already-computed days_remaining. + + Pure function of the days value — no clock access — so it can never + disagree with the days_remaining it was derived from. + """ + if days_remaining is None: + return "green" + if days_remaining <= 7: + return "red" + if days_remaining <= 21: + return "yellow" + return "green" + + +def compute_urgency(deadline_date: date | None) -> Urgency: + """red <= 7 days; yellow 8-21 days; green > 21 days or no deadline. + + Convenience wrapper for callers that only need the color, not the days + count. Callers that need BOTH values for the same letter must use + `urgency_for_deadline` instead, to guarantee they're computed against the + same `today` snapshot. + """ + return urgency_from_days_remaining(compute_days_remaining(deadline_date)) + + +def urgency_for_deadline(deadline_date: date | None) -> tuple[int | None, Urgency]: + """Compute (days_remaining, urgency) atomically from a single `today`. + + This is the function callers needing both fields (e.g. `GET /api/letters`) + should use: it snapshots `date.today()` exactly once so the two returned + values can never contradict each other across a midnight rollover. + """ + today = date.today() + days_remaining = compute_days_remaining(deadline_date, today=today) + urgency = urgency_from_days_remaining(days_remaining) + return days_remaining, urgency diff --git a/backend/tests/test_urgency.py b/backend/tests/test_urgency.py new file mode 100644 index 0000000..7b18140 --- /dev/null +++ b/backend/tests/test_urgency.py @@ -0,0 +1,197 @@ +"""Unit tests for backend/app/services/urgency.py — pure functions, no DB. + +Covers the 4 required cases (tracker KLAR-T3 acceptance criterion 4): + - red (near/overdue deadline) + - yellow (8-21 days out) + - green (>21 days out) + - green (no deadline) + +Boundary values (7, 8, 21, 22) are asserted directly per criterion 2. +Dates are computed relative to `date.today()` so tests are never date-flaky. +""" + +from datetime import date, timedelta +from uuid import uuid4 + +from app.database import engine +from app.models import DocumentCategory, Letter, LetterStatus, User +from app.routers.letters import list_letters +from app.services.urgency import ( + compute_days_remaining, + compute_urgency, + urgency_for_deadline, +) +from sqlmodel import Session + + +def _in(days: int) -> date: + return date.today() + timedelta(days=days) + + +def test_red_at_seven_days_boundary(): + deadline = _in(7) + assert compute_days_remaining(deadline) == 7 + assert compute_urgency(deadline) == "red" + + +def test_red_when_overdue(): + deadline = _in(-3) + assert compute_days_remaining(deadline) == -3 + assert compute_urgency(deadline) == "red" + + +def test_yellow_at_eight_days_boundary(): + deadline = _in(8) + assert compute_days_remaining(deadline) == 8 + assert compute_urgency(deadline) == "yellow" + + +def test_yellow_at_twenty_one_days_boundary(): + deadline = _in(21) + assert compute_days_remaining(deadline) == 21 + assert compute_urgency(deadline) == "yellow" + + +def test_green_at_twenty_two_days_boundary(): + deadline = _in(22) + assert compute_days_remaining(deadline) == 22 + assert compute_urgency(deadline) == "green" + + +def test_green_when_no_deadline(): + assert compute_days_remaining(None) is None + assert compute_urgency(None) == "green" + + +# -------------------------------------------------------------------------- +# Atomic (days_remaining, urgency) pair — Review 1 blocking issue 1. +# +# `urgency_for_deadline` must snapshot `date.today()` exactly once and derive +# both values from it, so the pair can never contradict itself even if a +# midnight rollover happens between what would otherwise be two separate +# `date.today()` calls (e.g. days_remaining=8 but urgency="red"). +# -------------------------------------------------------------------------- + + +def test_urgency_for_deadline_returns_consistent_pair_red(): + deadline = _in(7) + days_remaining, urgency = urgency_for_deadline(deadline) + assert days_remaining == 7 + assert urgency == "red" + + +def test_urgency_for_deadline_returns_consistent_pair_yellow(): + deadline = _in(8) + days_remaining, urgency = urgency_for_deadline(deadline) + assert days_remaining == 8 + assert urgency == "yellow" + + +def test_urgency_for_deadline_returns_consistent_pair_green(): + deadline = _in(22) + days_remaining, urgency = urgency_for_deadline(deadline) + assert days_remaining == 22 + assert urgency == "green" + + +def test_urgency_for_deadline_returns_consistent_pair_no_deadline(): + days_remaining, urgency = urgency_for_deadline(None) + assert days_remaining is None + assert urgency == "green" + + +def test_urgency_for_deadline_immune_to_simulated_midnight_rollover(monkeypatch): + """Simulates the exact bug the atomic helper prevents. + + If `compute_days_remaining` and `urgency_from_days_remaining` each called + `date.today()` independently, a clock that advances between the two calls + could produce e.g. days_remaining=8 (computed against yesterday) paired + with urgency derived against today's (later) date. `urgency_for_deadline` + snapshots `today` once, so the pair is always self-consistent regardless + of how many times the underlying clock is queried elsewhere. + """ + import app.services.urgency as urgency_mod + + deadline = _in(8) # 8 days from the real "today" -> yellow, not red. + + real_date = urgency_mod.date + call_count = {"n": 0} + + class _RollingDate(real_date): + @classmethod + def today(cls): + # First call (inside urgency_for_deadline) returns the real today; + # any further call would return a day later, simulating a + # midnight rollover mid-computation. Because urgency_for_deadline + # only calls date.today() once, this rollover must never surface. + call_count["n"] += 1 + if call_count["n"] == 1: + return real_date.today() + return real_date.today() + timedelta(days=1) + + monkeypatch.setattr(urgency_mod, "date", _RollingDate) + + days_remaining, urgency = urgency_for_deadline(deadline) + assert call_count["n"] == 1 + assert days_remaining == 8 + assert urgency == "yellow" + + +# -------------------------------------------------------------------------- +# Endpoint response shape — Review 1 blocking issue 2. +# +# Criterion 1 requires `GET /api/letters` items to include both +# `days_remaining` and `urgency`. This calls the `list_letters` handler +# directly (same pattern as `test_scanned_pdf_graceful.py`'s +# `_call_post_letter`) against real DB rows with varying deadlines, and +# asserts the returned `LetterListItem`s carry both fields with the right +# values — without spinning up a full HTTP/TestClient integration test. +# -------------------------------------------------------------------------- + + +def _make_user(db: Session) -> User: + user = User(email=f"urgency-{uuid4()}@example.com", language="en") + db.add(user) + db.commit() + db.refresh(user) + return user + + +def _make_letter(db: Session, user_id, *, deadline_date, letter_type="Formal Letter"): + letter = Letter( + user_id=user_id, + language="en", + status=LetterStatus.UPLOADED, + original_file="/tmp/whatever.pdf", + letter_type=letter_type, + category=DocumentCategory.OTHER, + deadline_date=deadline_date, + ) + db.add(letter) + db.commit() + db.refresh(letter) + return letter + + +def test_list_letters_response_includes_days_remaining_and_urgency(): + with Session(engine) as db: + user = _make_user(db) + _make_letter(db, user.id, deadline_date=_in(3)) # red + _make_letter(db, user.id, deadline_date=_in(10)) # yellow + _make_letter(db, user.id, deadline_date=_in(30)) # green + _make_letter(db, user.id, deadline_date=None) # green, no deadline + + items = list_letters(status=None, category=None, db=db, user=user) + + by_deadline_days = {item.days_remaining: item for item in items} + + assert len(items) == 4 + for item in items: + # Both fields must be present on every item (criterion 1) ... + assert hasattr(item, "days_remaining") + assert hasattr(item, "urgency") + + assert by_deadline_days[3].urgency == "red" + assert by_deadline_days[10].urgency == "yellow" + assert by_deadline_days[30].urgency == "green" + assert by_deadline_days[None].urgency == "green"