feat: card-of-the-week endpoint for automated content pipeline #212

Open
cal wants to merge 1 commits from autonomous/feat-card-of-the-week-endpoint into main
Owner

Autonomous Pipeline Finding

Source: growth-po nightly sweep (2026-04-10)
Roadmap: Phase 2.6c
Category: feature / content pipeline

What

New endpoint GET /api/v2/cards/featured/card-of-the-week returning the highest-rated eligible card from the past 7 days in a single response shape ready for Discord embed consumption.

Why

Lowest-friction entry into the automated content pipeline — proves the pull-model content pattern before committing to webhooks (see roadmap 2.6a). Keeps Discord active between pack drops, showcases the new refractor art, and creates a reusable template for weekly stat leaders (2.6b).

Design Notes

  • Card "created at" is approximated by Pack.open_time (when the pack was opened = when the card entered a team's collection). The Card model has no created_at column; this is the correct semantic proxy.
  • Rating is Card.value. Tiebreak: value DESC, open_time DESC, card.id DESC — fully deterministic.
  • AI teams excluded by default (Team.is_ai != 0); opt-in via ?include_ai=true.
  • Query params: ?days=7 (1-90) and ?include_ai=false.

Test Plan

  • Unit tests cover happy path, empty window → 404, AI exclusion by default, include_ai=true opt-in, tiebreak on rating, tiebreak on same-rating newest pack
  • All 6 new tests pass (python -m pytest tests/test_card_of_the_week.py -v)
  • Manual curl pddev.manticorum.com:813/api/v2/cards/featured/card-of-the-week on dev (reviewer)

🤖 Generated via Paper Dynasty autonomous pipeline (2026-04-10)

## Autonomous Pipeline Finding **Source:** growth-po nightly sweep (2026-04-10) **Roadmap:** Phase 2.6c **Category:** feature / content pipeline ## What New endpoint `GET /api/v2/cards/featured/card-of-the-week` returning the highest-rated eligible card from the past 7 days in a single response shape ready for Discord embed consumption. ## Why Lowest-friction entry into the automated content pipeline — proves the pull-model content pattern before committing to webhooks (see roadmap 2.6a). Keeps Discord active between pack drops, showcases the new refractor art, and creates a reusable template for weekly stat leaders (2.6b). ## Design Notes - Card "created at" is approximated by `Pack.open_time` (when the pack was opened = when the card entered a team's collection). The `Card` model has no `created_at` column; this is the correct semantic proxy. - Rating is `Card.value`. Tiebreak: `value DESC`, `open_time DESC`, `card.id DESC` — fully deterministic. - AI teams excluded by default (`Team.is_ai != 0`); opt-in via `?include_ai=true`. - Query params: `?days=7` (1-90) and `?include_ai=false`. ## Test Plan - [x] Unit tests cover happy path, empty window → 404, AI exclusion by default, include_ai=true opt-in, tiebreak on rating, tiebreak on same-rating newest pack - [x] All 6 new tests pass (`python -m pytest tests/test_card_of_the_week.py -v`) - [ ] Manual `curl pddev.manticorum.com:813/api/v2/cards/featured/card-of-the-week` on dev (reviewer) 🤖 Generated via Paper Dynasty autonomous pipeline (2026-04-10)
cal added 1 commit 2026-04-10 15:32:10 +00:00
Exposes the highest-rated card from the past 7 days (configurable via
?days=) as a Discord-embed-ready payload.  AI teams are excluded by
default (?include_ai=true lifts the filter).  Deterministic tiebreak:
rating desc, pack open_time desc, card id desc.

Roadmap: Phase 2.6c — lowest-friction entry into the automated content
pipeline.  Single-call response includes player name, team, rarity,
rating, card image URL, and cardset name.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
cal added the
autonomous
size:S
type:feature
labels 2026-04-10 15:32:54 +00:00
cal reviewed 2026-04-10 15:38:42 +00:00
cal left a comment
Author
Owner

AI Code Review

Files Reviewed

  • app/routers_v2/featured.py (added)
  • app/main.py (modified — import + router registration)
  • tests/test_card_of_the_week.py (added)

Findings

Correctness

No issues found.

  • Joins: Card → Pack, Card → Player → Rarity, Player → Cardset, Card → Team chain is correct for the Peewee model structure. .switch(Card) and .switch(Player) are used correctly to navigate back up the join tree.
  • AI exclusion filter: (Team.is_ai == 0) | (Team.is_ai.is_null(True)) correctly keeps human teams. The schema confirms is_ai = IntegerField(null=True) where 1 = AI and 0 / NULL = human. Logic matches the docstring.
  • NULL pack open_time: Pack.open_time is null=True in the schema. Cards from unopened packs (open_time IS NULL) are silently excluded by the >= cutoff WHERE clause — NULL comparisons evaluate false in SQL. This is the correct semantic: only opened packs are meaningful for this endpoint.
  • Tiebreak: value DESC, open_time DESC, card.id DESC is fully deterministic; no random element.
  • 404: Correct contract — 404 on empty window, not a 200 with empty body.
  • Auth: GET endpoints in this project are unauthenticated by convention (verified against cards.py and other v2 routers). Only mutating endpoints require the token. Consistent.

Security

No issues found. No user-supplied data lands in raw queries; days is an integer bounded to [1, 90] via FastAPI Query(ge=1, le=90). No secrets or credentials introduced.

Style & Conventions

Two minor cosmetic items that do not block merge:

  1. Optional[str] without = None (featured.py line 53): In Pydantic v1 (confirmed pydantic==1.10.21), Optional[str] without a default means the field is technically required at construction time. The response construction always provides card_created_at=pack_open_time explicitly so there is no runtime failure, but the convention is Optional[str] = None to signal that None is a valid value. Safe to fix in a follow-up.

  2. _PACK_COUNTER is defined but never used (test_card_of_the_week.py line ~149): _make_pack does not increment or read _PACK_COUNTER. Dead code — harmless but could be removed.

Suggestions

  • The endpoint has no response caching headers (e.g., Cache-Control: max-age=3600). The PR description mentions "cache-friendly" and "deterministic within a calendar day" — consider adding a Cache-Control header in a follow-up so n8n/Cloudflare can actually cache the response.
  • datetime.utcnow() is deprecated in Python 3.12+. Pre-existing pattern across the codebase, so no action needed in this PR, but worth noting for a future cleanup pass.

Test Coverage

All 6 required scenarios covered:

  • test_happy_path_single_eligible_card — response shape + field values
  • test_empty_window_returns_404 — 404 contract
  • test_ai_team_excluded_by_default — default exclusion
  • test_include_ai_true_includes_ai_team_card — opt-in inclusion
  • test_tiebreak_highest_rating_wins — primary sort
  • test_tiebreak_same_rating_newest_pack_wins — secondary sort (determinism)

Factory data pattern follows CLAUDE.md conventions. Shared-memory SQLite + per-test table teardown provides proper isolation.

Verdict: APPROVED

Implementation is correct, joins and filters match the schema, AI exclusion logic is sound, tiebreak is deterministic, tests cover the full contract. The two style notes above are cosmetic and do not warrant blocking merge.

Note: posted as COMMENT state due to single-user repo restriction (Gitea blocks self-approval). Intended verdict is APPROVED — merge via pd-pr merge --no-approve.


Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/routers_v2/featured.py` (added) - `app/main.py` (modified — import + router registration) - `tests/test_card_of_the_week.py` (added) ### Findings #### Correctness No issues found. - **Joins**: Card → Pack, Card → Player → Rarity, Player → Cardset, Card → Team chain is correct for the Peewee model structure. `.switch(Card)` and `.switch(Player)` are used correctly to navigate back up the join tree. - **AI exclusion filter**: `(Team.is_ai == 0) | (Team.is_ai.is_null(True))` correctly keeps human teams. The schema confirms `is_ai = IntegerField(null=True)` where `1` = AI and `0 / NULL` = human. Logic matches the docstring. - **NULL pack open_time**: `Pack.open_time` is `null=True` in the schema. Cards from unopened packs (open_time IS NULL) are silently excluded by the `>= cutoff` WHERE clause — NULL comparisons evaluate false in SQL. This is the correct semantic: only opened packs are meaningful for this endpoint. - **Tiebreak**: `value DESC, open_time DESC, card.id DESC` is fully deterministic; no random element. - **404**: Correct contract — 404 on empty window, not a 200 with empty body. - **Auth**: GET endpoints in this project are unauthenticated by convention (verified against `cards.py` and other v2 routers). Only mutating endpoints require the token. Consistent. #### Security No issues found. No user-supplied data lands in raw queries; `days` is an integer bounded to `[1, 90]` via FastAPI `Query(ge=1, le=90)`. No secrets or credentials introduced. #### Style & Conventions Two minor cosmetic items that do not block merge: 1. **`Optional[str]` without `= None`** (`featured.py` line 53): In Pydantic v1 (confirmed `pydantic==1.10.21`), `Optional[str]` without a default means the field is technically *required* at construction time. The response construction always provides `card_created_at=pack_open_time` explicitly so there is no runtime failure, but the convention is `Optional[str] = None` to signal that None is a valid value. Safe to fix in a follow-up. 2. **`_PACK_COUNTER` is defined but never used** (`test_card_of_the_week.py` line ~149): `_make_pack` does not increment or read `_PACK_COUNTER`. Dead code — harmless but could be removed. #### Suggestions - The endpoint has no response caching headers (e.g., `Cache-Control: max-age=3600`). The PR description mentions "cache-friendly" and "deterministic within a calendar day" — consider adding a `Cache-Control` header in a follow-up so n8n/Cloudflare can actually cache the response. - `datetime.utcnow()` is deprecated in Python 3.12+. Pre-existing pattern across the codebase, so no action needed in this PR, but worth noting for a future cleanup pass. ### Test Coverage All 6 required scenarios covered: - `test_happy_path_single_eligible_card` — response shape + field values - `test_empty_window_returns_404` — 404 contract - `test_ai_team_excluded_by_default` — default exclusion - `test_include_ai_true_includes_ai_team_card` — opt-in inclusion - `test_tiebreak_highest_rating_wins` — primary sort - `test_tiebreak_same_rating_newest_pack_wins` — secondary sort (determinism) Factory data pattern follows CLAUDE.md conventions. Shared-memory SQLite + per-test table teardown provides proper isolation. ### Verdict: APPROVED Implementation is correct, joins and filters match the schema, AI exclusion logic is sound, tiebreak is deterministic, tests cover the full contract. The two style notes above are cosmetic and do not warrant blocking merge. Note: posted as COMMENT state due to single-user repo restriction (Gitea blocks self-approval). Intended verdict is APPROVED — merge via `pd-pr merge --no-approve`. --- *Automated review by Claude PR Reviewer*
Claude added the
ai-reviewing
label 2026-04-10 15:45:51 +00:00
Claude reviewed 2026-04-10 15:48:02 +00:00
Claude left a comment
Collaborator

AI Code Review

Files Reviewed

  • app/main.py (modified — router wiring)
  • app/routers_v2/featured.py (added, 127 lines)
  • tests/test_card_of_the_week.py (added, 470 lines)

Findings

Correctness

No blocking issues found.

  • Query join chain is valid Peewee: Card → Pack → Player → Rarity, .switch(Player) → Cardset, .switch(Card) → Team — all INNER JOINs with correct .switch() resets.
  • AI exclusion logic correct: (Team.is_ai == 0) | (Team.is_ai.is_null(True)) correctly keeps human teams (is_ai=0 or NULL) and excludes AI teams (is_ai=1+). Matches the documented behavior.
  • Null guards: .where(Card.player.is_null(False)), .where(Card.team.is_null(False)), .where(Card.pack.is_null(False)) are redundant given the INNER JOINs, but harmless.
  • Tiebreak order: Card.value DESC, Pack.open_time DESC, Card.id DESC — fully deterministic as documented.
  • card.pack.open_time access safe: Pack.open_time is DateTimeField(null=True) on the model; the if card.pack and card.pack.open_time: guard is correct.

Security

  • No raw SQL — all Peewee ORM with parameterized queries.
  • days (int, ge=1, le=90) and include_ai (bool) are validated by FastAPI before reaching the handler.
  • No auth guard — intentional for a read-only content pipeline endpoint; acceptable for this use case.

Style & Conventions

  • from ..db_engine import ... at module top-level — follows CLAUDE.md lazy-import rule.
  • Module docstring is clear and thorough.
  • Test docstrings explain "what" and "why" per project convention.
  • _PACK_COUNTER is defined at module level but never incremented or read — dead variable. Harmless.
  • _make_pack_type(cardset: Cardset) accepts a cardset argument but never uses it — PackType has no cardset FK. Dead parameter. Harmless, but misleading.

Suggestions

  • card_image_url field naming: The field is populated from card.player.image (Player.image, a CharField), which is the player's stock/template photo URL — not the Playwright-rendered card PNG. For Discord embeds that want to show the actual card art (including refractor shimmer), callers will need a follow-up call to /api/v2/players/{id}/...card/.../animated or the static card endpoint. Consider renaming to player_image_url in a follow-up to avoid misleading consumers, or noting the distinction in the endpoint description.
  • Variant cards are included without filter: Card.variant is not filtered, so refractor variant cards (variant > 0) can win card-of-the-week. Their card_image_url still points to the base Player.image, not the refractor-rendered art. If refractor cards are expected to appear with their shimmer art, a follow-up to include image_url from BattingCard/PitchingCard would be needed. Design call — not a blocker.
  • No ?days= boundary test: The test matrix doesn't include a custom ?days=14 or ?days=1 case. The 6 documented tests are all present and pass. Low priority.

Verdict: COMMENT

Functionally correct. Query logic, AI exclusion, tiebreak, and 404 handling are all sound. Tests cover the documented matrix. Two dead code nits (_PACK_COUNTER, unused cardset param in _make_pack_type) are harmless. Primary suggestion is a follow-up to clarify card_image_url vs the rendered card image — not a blocker for merge.

Self-review restriction applies (autonomous pipeline PR, same repo owner) — no blocking issues, ready to merge.


Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/main.py` (modified — router wiring) - `app/routers_v2/featured.py` (added, 127 lines) - `tests/test_card_of_the_week.py` (added, 470 lines) ### Findings #### Correctness No blocking issues found. - **Query join chain is valid Peewee**: `Card → Pack → Player → Rarity`, `.switch(Player) → Cardset`, `.switch(Card) → Team` — all INNER JOINs with correct `.switch()` resets. ✅ - **AI exclusion logic correct**: `(Team.is_ai == 0) | (Team.is_ai.is_null(True))` correctly keeps human teams (is_ai=0 or NULL) and excludes AI teams (is_ai=1+). Matches the documented behavior. ✅ - **Null guards**: `.where(Card.player.is_null(False))`, `.where(Card.team.is_null(False))`, `.where(Card.pack.is_null(False))` are redundant given the INNER JOINs, but harmless. ✅ - **Tiebreak order**: `Card.value DESC, Pack.open_time DESC, Card.id DESC` — fully deterministic as documented. ✅ - **`card.pack.open_time` access safe**: `Pack.open_time` is `DateTimeField(null=True)` on the model; the `if card.pack and card.pack.open_time:` guard is correct. ✅ #### Security - No raw SQL — all Peewee ORM with parameterized queries. ✅ - `days` (int, ge=1, le=90) and `include_ai` (bool) are validated by FastAPI before reaching the handler. ✅ - No auth guard — intentional for a read-only content pipeline endpoint; acceptable for this use case. #### Style & Conventions - `from ..db_engine import ...` at module top-level — follows CLAUDE.md lazy-import rule. ✅ - Module docstring is clear and thorough. ✅ - Test docstrings explain "what" and "why" per project convention. ✅ - `_PACK_COUNTER` is defined at module level but never incremented or read — dead variable. Harmless. - `_make_pack_type(cardset: Cardset)` accepts a `cardset` argument but never uses it — `PackType` has no `cardset` FK. Dead parameter. Harmless, but misleading. #### Suggestions - **`card_image_url` field naming**: The field is populated from `card.player.image` (`Player.image`, a CharField), which is the player's stock/template photo URL — not the Playwright-rendered card PNG. For Discord embeds that want to show the actual card art (including refractor shimmer), callers will need a follow-up call to `/api/v2/players/{id}/...card/.../animated` or the static card endpoint. Consider renaming to `player_image_url` in a follow-up to avoid misleading consumers, or noting the distinction in the endpoint description. - **Variant cards are included without filter**: `Card.variant` is not filtered, so refractor variant cards (variant > 0) can win card-of-the-week. Their `card_image_url` still points to the base `Player.image`, not the refractor-rendered art. If refractor cards are expected to appear with their shimmer art, a follow-up to include `image_url` from `BattingCard`/`PitchingCard` would be needed. Design call — not a blocker. - **No `?days=` boundary test**: The test matrix doesn't include a custom `?days=14` or `?days=1` case. The 6 documented tests are all present and pass. Low priority. ### Verdict: COMMENT Functionally correct. Query logic, AI exclusion, tiebreak, and 404 handling are all sound. Tests cover the documented matrix. Two dead code nits (`_PACK_COUNTER`, unused `cardset` param in `_make_pack_type`) are harmless. Primary suggestion is a follow-up to clarify `card_image_url` vs the rendered card image — not a blocker for merge. Self-review restriction applies (autonomous pipeline PR, same repo owner) — no blocking issues, ready to merge. --- *Automated review by Claude PR Reviewer*
Claude added
ai-reviewed
and removed
ai-reviewing
labels 2026-04-10 15:48:43 +00:00
cal removed the
ai-reviewed
label 2026-04-10 20:35:17 +00:00
Author
Owner

TODO: Let's rename this "Pull of the Week" instead of Card of the Week so we guarantee there is no confusion about what it is. Card of the Week is a bit ambiguous and could refer to statistics as well.

TODO: Let's rename this "Pull of the Week" instead of Card of the Week so we guarantee there is no confusion about what it is. Card of the Week is a bit ambiguous and could refer to statistics as well.
Claude added the
ai-reviewing
label 2026-04-10 20:49:07 +00:00
Claude reviewed 2026-04-10 20:50:53 +00:00
Claude left a comment
Collaborator

AI Code Review

Files Reviewed

  • app/main.py (modified — router import + registration)
  • app/routers_v2/featured.py (added — new endpoint)
  • tests/test_card_of_the_week.py (added — 6 tests)

Findings

Correctness

No issues found.

  • Peewee join chain (Card→Pack, .switch(Card)→Player→Rarity, .switch(Player)→Cardset, .switch(Card)→Team) is correct; .switch() resets are in the right places
  • AI exclusion filter (Team.is_ai == 0) | (Team.is_ai.is_null(True)) correctly matches the model (is_ai = IntegerField(null=True) where non-AI teams have 0 or NULL)
  • Tiebreak ORDER BY Card.value DESC, Pack.open_time DESC, Card.id DESC is deterministic
  • Pack.open_time >= cutoff WHERE clause correctly excludes NULL open_time rows at the SQL level (NULL comparisons always evaluate false)
  • 404 path correct; card.pack.open_time null guard in response assembly is redundant but harmless
  • Card.player == Player.player_id FK join condition is correct — Player uses player_id as primary key

Security

No issues found. days is bounded ge=1, le=90; include_ai is a bool — no unvalidated user input reaches the query.

Style & Conventions

  • card_image_url field naming: populates from card.player.image which is the stock/template headshot URL, not a Playwright-rendered card PNG. For refractor variant cards (Card.variant > 0), the shimmer art lives in BattingCard.image_url / PitchingCard.image_url, not Player.image. A variant card can win COTW but its image won't show the refractor art. Recommend a follow-up to either rename to player_image_url or add a rendered_card_url field that conditionally resolves variant art.
  • _PACK_COUNTER in tests: declared at module level (_PACK_COUNTER = [0]) but never incremented or referenced. Dead variable.
  • _make_pack_type(cardset) helper: accepts a cardset parameter that is never used — PackType has no cardset FK. Parameter can be removed.
  • datetime.utcnow(): deprecated since Python 3.12. Non-blocking; consistent with existing usage elsewhere in the codebase.

Suggestions

  • When a follow-up adds rendered card URL support, consider filtering out Card.variant > 0 from COTW results until the image URL is wired — otherwise refractor cards appear with a plain headshot instead of their animated art.

Verdict: COMMENT

Self-review restriction applies (PR author = repo owner). Code is functionally correct, follows CLAUDE.md import conventions, and is secure. No blocking issues. Ready to merge pending the card_image_url / variant image follow-up being tracked as a separate issue.


Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `app/main.py` (modified — router import + registration) - `app/routers_v2/featured.py` (added — new endpoint) - `tests/test_card_of_the_week.py` (added — 6 tests) ### Findings #### Correctness No issues found. - Peewee join chain (`Card→Pack`, `.switch(Card)→Player→Rarity`, `.switch(Player)→Cardset`, `.switch(Card)→Team`) is correct; `.switch()` resets are in the right places ✅ - AI exclusion filter `(Team.is_ai == 0) | (Team.is_ai.is_null(True))` correctly matches the model (`is_ai = IntegerField(null=True)` where non-AI teams have `0` or `NULL`) ✅ - Tiebreak `ORDER BY Card.value DESC, Pack.open_time DESC, Card.id DESC` is deterministic ✅ - `Pack.open_time >= cutoff` WHERE clause correctly excludes NULL `open_time` rows at the SQL level (NULL comparisons always evaluate false) ✅ - 404 path correct; `card.pack.open_time` null guard in response assembly is redundant but harmless ✅ - `Card.player == Player.player_id` FK join condition is correct — `Player` uses `player_id` as primary key ✅ #### Security No issues found. `days` is bounded `ge=1, le=90`; `include_ai` is a bool — no unvalidated user input reaches the query. #### Style & Conventions - **`card_image_url` field naming**: populates from `card.player.image` which is the stock/template headshot URL, not a Playwright-rendered card PNG. For refractor variant cards (`Card.variant > 0`), the shimmer art lives in `BattingCard.image_url` / `PitchingCard.image_url`, not `Player.image`. A variant card can win COTW but its image won't show the refractor art. Recommend a follow-up to either rename to `player_image_url` or add a `rendered_card_url` field that conditionally resolves variant art. - **`_PACK_COUNTER` in tests**: declared at module level (`_PACK_COUNTER = [0]`) but never incremented or referenced. Dead variable. - **`_make_pack_type(cardset)` helper**: accepts a `cardset` parameter that is never used — `PackType` has no `cardset` FK. Parameter can be removed. - **`datetime.utcnow()`**: deprecated since Python 3.12. Non-blocking; consistent with existing usage elsewhere in the codebase. #### Suggestions - When a follow-up adds rendered card URL support, consider filtering out `Card.variant > 0` from COTW results until the image URL is wired — otherwise refractor cards appear with a plain headshot instead of their animated art. ### Verdict: COMMENT Self-review restriction applies (PR author = repo owner). Code is functionally correct, follows CLAUDE.md import conventions, and is secure. No blocking issues. Ready to merge pending the `card_image_url` / variant image follow-up being tracked as a separate issue. --- *Automated review by Claude PR Reviewer*
Claude added
ai-reviewed
and removed
ai-reviewing
labels 2026-04-10 20:50:58 +00:00
This pull request is blocked because it's outdated.
This branch is out-of-date with the base branch
You are not authorized to merge this pull request.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin autonomous/feat-card-of-the-week-endpoint:autonomous/feat-card-of-the-week-endpoint
git checkout autonomous/feat-card-of-the-week-endpoint
Sign in to join this conversation.
No description provided.