Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -30,3 +30,35 @@ PROCESS_CONCURRENCY=4

# Max attempts before a job moves from `failed` to permanently failed.
MAX_ATTEMPTS=3

## OIDC Authentication (verifier reviewer identity)

# When unset, the verifier falls back to either VERIFIER_PASSWORD-gated
# BasicAuth or no auth at all (current local-dev default). Set all five
# vars together to enable OIDC against api.wxyc.org/auth.

# Issuer URL of the WXYC Better Auth server. Production:
# https://api.wxyc.org/auth. Local: http://localhost:8082/auth.
WXYC_AUTH_ISSUER=

# OIDC client id (must match Backend-Service's FLOWSHEET_OIDC_CLIENT_ID).
WXYC_OIDC_CLIENT_ID=

# OIDC client secret (must match Backend-Service's
# FLOWSHEET_OIDC_CLIENT_SECRET exactly). Treat as production secret;
# never commit a real value.
WXYC_OIDC_CLIENT_SECRET=

# Signing key for the verifier's own session cookie. 32+ random bytes;
# rotating it invalidates every active reviewer session. Generate with
# `python -c "import secrets; print(secrets.token_urlsafe(48))"`. The
# cookie's TTL is hard-coded to 12 hours in core/auth.py; expect a
# re-login at that boundary mid-session (no refresh-token plumbing
# yet — by design, see plans/oidc-auth.md "Deliberately out of scope").
WXYC_SESSION_SECRET=

# Public origin where this verifier is reachable. Determines the
# `redirect_uri` sent to the auth server and gates the `secure` flag
# on session cookies (Secure only when the value starts with `https`).
# Production: https://flowsheet.wxyc.org. Local: http://localhost:8765.
FLOWSHEET_PUBLIC_URL=
19 changes: 17 additions & 2 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,23 @@ core/
schema.py Pydantic models. GeminiPageResult is what
the model returns (used as response_schema);
PageResult adds caller-set model_version +
extracted_at and is what lands on disk. Entry
and Quadrant are shared between both.
extracted_at + verified_by and is what lands
on disk. Entry and Quadrant are shared
between both.
auth.py OIDC client (against Better Auth at
api.wxyc.org/auth), signed session-cookie
codec (itsdangerous), and the ReviewerSession
dataclass. Consumed by verifier/serve.py
(middleware + /api/save). Module-level cached
discovery doc + JWKS — configuration, not
per-request state; _reset_metadata_cache()
is the test seam. Lazy-loaded (not eager at
boot) so a transient auth-server slowdown
doesn't take the verifier offline; first
caller after a deploy pays ~50-150ms cold
start. Deviation from the DI pattern in
core/gemini.py is intentional; don't "fix"
it back — see the module docstring.
prompts.py Extraction prompt (separate module so prompt
changes are reviewable independently).
gemini.py Async wrapper around google-genai. Dependency
Expand Down
21 changes: 19 additions & 2 deletions core/jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ class Job:
verified_at: str | None
verified_path: str | None
corrections_path: str | None
reviewer_id: str | None
created_at: str
updated_at: str

Expand All @@ -73,6 +74,7 @@ def from_row(cls, row: aiosqlite.Row) -> Self:
verified_at=row["verified_at"] if "verified_at" in keys else None,
verified_path=row["verified_path"] if "verified_path" in keys else None,
corrections_path=(row["corrections_path"] if "corrections_path" in keys else None),
reviewer_id=row["reviewer_id"] if "reviewer_id" in keys else None,
created_at=row["created_at"],
updated_at=row["updated_at"],
)
Expand All @@ -91,6 +93,7 @@ def from_row(cls, row: aiosqlite.Row) -> Self:
verified_at TEXT,
verified_path TEXT,
corrections_path TEXT,
reviewer_id TEXT,
created_at TEXT NOT NULL,
updated_at TEXT NOT NULL,
PRIMARY KEY (pdf_path, page_number)
Expand All @@ -106,6 +109,11 @@ def from_row(cls, row: aiosqlite.Row) -> Self:
("verified_at", "TEXT"),
("verified_path", "TEXT"),
("corrections_path", "TEXT"),
# Denormalized OIDC user_id of the volunteer who last verified the
# page. Duplicates the user_id in `verified_by.user_id` inside the
# verified.json file, but lets per-reviewer queries run without
# parsing every JSON on disk.
("reviewer_id", "TEXT"),
)

# Indexes that depend on late-added columns and therefore must be created
Expand Down Expand Up @@ -283,6 +291,7 @@ async def mark_verified(
*,
verified_path: Path,
corrections_path: Path,
reviewer_id: str | None = None,
) -> bool:
"""Record that a page has been hand-verified via the verifier UI.

Expand All @@ -292,26 +301,34 @@ async def mark_verified(
clear the verification record by default — that's a separate
decision a human makes via `retry`).

`reviewer_id` is the OIDC user_id of the volunteer who saved the
page (`ReviewerSession.user_id`). Optional so the BasicAuth and
no-auth deployments — and the existing test fixtures — keep
compiling without a reviewer to credit.

Returns True if a job row matched, False otherwise. Callers
(e.g. the verifier server) may want to write files even when no
job row exists for the page (test fixtures), so a False return
is not an error.
"""
now = _now()
async with self._connect() as db:
cursor = await db.execute(
"""
UPDATE jobs
SET verified_at = ?,
verified_path = ?,
corrections_path = ?,
reviewer_id = ?,
updated_at = ?
WHERE pdf_path = ? AND page_number = ?
""",
(
_now(),
now,
str(verified_path),
str(corrections_path),
_now(),
reviewer_id,
now,
pdf_path,
page_number,
),
Expand Down
50 changes: 49 additions & 1 deletion core/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,13 +170,61 @@ def _check_quadrant_order(self) -> Self:
return self


class VerifiedBy(BaseModel):
"""Identity of the reviewer who saved this verified page.

Populated only by `verifier/serve.py`'s POST /api/save handler when
an authenticated reviewer session is present. Pipeline-written
PageResults leave `PageResult.verified_by` as None.

`user_id` is deliberately denormalized to `jobs.reviewer_id` (see
`core/jobs.py`) so per-reviewer queries don't have to parse every
JSON file on disk. The two values are always equal for the same
save call; both are written inside `/api/save` under server
authority — the client never sets either. A client-supplied
`verified_by` block on a save POST is overwritten with the
authenticated reviewer's values before persistence (see the
handler).
"""

user_id: str = Field(description="Better Auth user.id (the OIDC `sub` claim).")
username: str | None = Field(
default=None,
description="Better Auth username, if set.",
)
real_name: str | None = Field(
default=None,
description="Reviewer's real name from the WXYC user record.",
)
dj_name: str | None = Field(
default=None,
description="Reviewer's on-air DJ name, if set.",
)
verified_at: datetime = Field(
description="When the verifier UI saved this page (UTC).",
)


class PageResult(GeminiPageResult):
"""On-disk shape: `GeminiPageResult` plus the two fields the caller owns.
"""On-disk shape: `GeminiPageResult` plus the fields the caller owns.

`model_version` and `extracted_at` are filled by the pipeline (or by
each calibration adapter) at write-time. They are NOT part of the
Gemini response_schema — see the module docstring.

`verified_by` is populated only by `verifier/serve.py` when a
reviewer saves a page through the verifier UI. Defaulting to None
keeps every pre-OIDC `verified.json` parseable (the field is
silently absent on old files); new saves write the block and old
files are upgraded on the next save.
"""

model_version: str = Field(description="Model id that produced this result.")
extracted_at: datetime = Field(description="When the extraction completed (UTC).")
verified_by: VerifiedBy | None = Field(
default=None,
description=(
"Reviewer who saved this corrected page via the verifier UI. "
"None for results that have only been through the automatic pipeline."
),
)
42 changes: 42 additions & 0 deletions tests/unit/conftest.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
"""Shared fixtures + helpers for `tests/unit/`.

Created in the OIDC PR for `_reset_serve_module_state`, which the
parametrized middleware-precedence and Secure-flag tests need to keep
test cases from leaking module-level state into each other.
"""

from __future__ import annotations

from types import ModuleType


def _reset_serve_module_state(serve_mod: ModuleType) -> None:
"""Clear module-level caches that survive `importlib.reload()`.

`importlib.reload(serve_mod)` rebinds the *module object*, but two
pieces of process-wide state can still leak from a previous test:

* `serve_mod._initialized_jobs_dbs` — set[Path] tracking which
jobs.db files have had their schema bootstrap run. A test that
flips DATA_ROOT between parametrizations needs this cleared,
or the new path inherits the old "already bootstrapped"
assumption and the schema migration is silently skipped.
* `core.auth._metadata` and `core.auth._jwks` — cached OIDC
discovery doc and JWKS. A test that flips `WXYC_AUTH_ISSUER`
between parametrizations needs these cleared, or the new
issuer's auth calls run against stale metadata.

REQUIRED: call this unconditionally after every
`importlib.reload(serve_mod)` and every `importlib.reload(auth_mod)`
— including when the test under construction looks orthogonal to
these caches. Whether a future parametrization touches DATA_ROOT or
WXYC_AUTH_ISSUER is not predictable from inside one test case;
skipping the reset produces flakes that depend on test execution
order, which is the failure mode you do not want. The cost of
calling it when unneeded is two `set.clear()` calls.
"""
import core.auth as auth_mod

if hasattr(serve_mod, "_initialized_jobs_dbs"):
serve_mod._initialized_jobs_dbs.clear()
auth_mod._reset_metadata_cache()
75 changes: 75 additions & 0 deletions tests/unit/test_jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -285,3 +285,78 @@ async def test_init_adds_late_columns_to_existing_db(tmp_path: Path) -> None:
assert "verified_at" in cols
assert "verified_path" in cols
assert "corrections_path" in cols
# reviewer_id is the late column added by the OIDC PR. Without this
# branch the migration on an existing prod jobs.db would silently
# leave the column off and every mark_verified(reviewer_id=...) call
# would persist no reviewer at all.
assert "reviewer_id" in cols


# -- reviewer_id (OIDC PR) -------------------------------------------------


async def test_mark_verified_round_trips_reviewer_id(store: JobStore, tmp_path: Path) -> None:
"""`mark_verified(..., reviewer_id="u-123")` persists to `jobs.reviewer_id`.

The verifier server denormalizes the OIDC user_id onto the row so
per-reviewer queries don't have to parse every verified.json on disk.
"""
await store.register("scans/a.pdf", 1)
await store.mark_rendered("scans/a.pdf", 1, image_path=tmp_path / "a.png")
await store.mark_completed("scans/a.pdf", 1, result_path=tmp_path / "a.json", model_version="m")

matched = await store.mark_verified(
"scans/a.pdf",
1,
verified_path=tmp_path / "a.verified.json",
corrections_path=tmp_path / "a.corrections.json",
reviewer_id="u-123",
)
assert matched is True

job = await store.get("scans/a.pdf", 1)
assert job is not None
assert job.reviewer_id == "u-123"


async def test_mark_verified_accepts_none_reviewer_id(store: JobStore, tmp_path: Path) -> None:
"""`reviewer_id=None` (the default) writes NULL — the BasicAuth and
no-auth deployments don't have a reviewer to credit and the column
should accept that without a NOT NULL violation."""
await store.register("scans/a.pdf", 1)
await store.mark_rendered("scans/a.pdf", 1, image_path=tmp_path / "a.png")

matched = await store.mark_verified(
"scans/a.pdf",
1,
verified_path=tmp_path / "a.verified.json",
corrections_path=tmp_path / "a.corrections.json",
# reviewer_id omitted; defaults to None.
)
assert matched is True

job = await store.get("scans/a.pdf", 1)
assert job is not None
assert job.reviewer_id is None


async def test_mark_verified_overwrites_reviewer_id_on_re_save(
store: JobStore, tmp_path: Path
) -> None:
"""A second `mark_verified` call updates `reviewer_id` to the latest
reviewer. Verification is "who last saved this page", not an
append-only log — matches the file-side semantics where verified.json
is rewritten by every save."""
await store.register("scans/a.pdf", 1)
await store.mark_rendered("scans/a.pdf", 1, image_path=tmp_path / "a.png")

paths = {
"verified_path": tmp_path / "a.verified.json",
"corrections_path": tmp_path / "a.corrections.json",
}
await store.mark_verified("scans/a.pdf", 1, **paths, reviewer_id="first")
await store.mark_verified("scans/a.pdf", 1, **paths, reviewer_id="second")

job = await store.get("scans/a.pdf", 1)
assert job is not None
assert job.reviewer_id == "second"
Loading