fix: refractor card art post-merge fixes — cache bypass, template guards, dev server #180

Merged
cal merged 1 commits from fix/refractor-card-art-followup into main 2026-04-04 17:41:06 +00:00
Owner

Summary

  • Skip PNG cache when ?tier= param is set to prevent serving stale T0 images for tier preview
  • Move {% if %} guard before diamond_colors dict definition in player_card.html
  • Extract base #fullCard styles outside refractor conditional in tier_style.html, remove redundant T0 else block
  • Make run-local.sh DB host configurable via POSTGRES_HOST_LOCAL, clean up Playwright existence check

Follow-up to PR #179.

Test plan

  • Verify ?tier= param returns fresh rendered PNG, not cached T0
  • Confirm T0 cards render correctly (base styles still apply)
  • Confirm T1-T4 cards render with diamond indicator and tier-specific styles
## Summary - Skip PNG cache when `?tier=` param is set to prevent serving stale T0 images for tier preview - Move `{% if %}` guard before `diamond_colors` dict definition in player_card.html - Extract base `#fullCard` styles outside refractor conditional in tier_style.html, remove redundant T0 else block - Make run-local.sh DB host configurable via `POSTGRES_HOST_LOCAL`, clean up Playwright existence check Follow-up to PR #179. ## Test plan - Verify `?tier=` param returns fresh rendered PNG, not cached T0 - Confirm T0 cards render correctly (base styles still apply) - Confirm T1-T4 cards render with diamond indicator and tier-specific styles
cal added 1 commit 2026-04-04 17:38:24 +00:00
- Skip PNG cache when ?tier= param is set to prevent serving stale T0 images
- Move {% if %} guard before diamond_colors dict in player_card.html
- Extract base #fullCard styles outside refractor conditional in tier_style.html
- Make run-local.sh DB host configurable, clean up Playwright check

Follow-up to PR #179

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Claude approved these changes 2026-04-04 17:40:40 +00:00
Dismissed
Claude left a comment
Collaborator

APPROVED — all four fix areas are correct. Two minor notes.


1. Cache bypass (players.py)

and tier is None correctly prevents a stale PNG (likely T0) from being served when a caller explicitly passes ?tier=. The condition short-circuits the os.path.exists check so rendering always runs when a tier is requested.

Minor: tier=0 also bypasses the cache, which means a cached T0 card won’t be served even when tier=0 is explicitly passed. Since T0 and the cached default are identical, the caller just pays an extra render — harmless, but worth knowing. If T0 should use the cache, the condition could be and (tier is None or tier == 0). Not blocking.


2. Template guard placement (player_card.html)

Moving diamond_colors inside {% if refractor_tier is defined and refractor_tier > 0 %} is correct. The old order (dict outside, dc = diamond_colors[refractor_tier] inside) was technically safe because dc is only accessed when refractor_tier > 0 — but it was confusing. Having the dict and its consumer co-located inside the same guard is cleaner and semantically correct. No functional change for T1–T4.


3. CSS restructuring (tier_style.html)

Extracting #fullCard { position: relative; overflow: hidden; } to an unconditional <style> block is the right call — it was duplicated identically in both the if and else branches. The new structure (base styles always emitted, tier-specific styles conditionally appended) is clean and DRY.

Minor to verify: #resultHeader.border-bot { border-bottom-width: 4px; } is removed from the conditional block (around the original line 68 context). This isn’t called out in the PR description — if it’s dead CSS from the d92ab86 visual tuning commit, the removal is correct. Just confirm it isn’t relied on by any live template path.

File still lacks a trailing newline (same as PR #179). Cosmetic only.


4. run-local.sh

Both changes are improvements:

  • Playwright check: print(p.chromium.executable_path) is a faster, lighter check than launching a browser instance. It raises the same exception if Chromium isn’t installed, so the || exit 1 path still fires correctly. The >/dev/null 2>&1 redirect properly silences the path output.

  • DB host: Setting DB_HOST="${POSTGRES_HOST_LOCAL:-10.10.0.42}" once and reusing it in both the connectivity check (sys.argv[1]) and the export POSTGRES_HOST line eliminates the previous duplication where the env-var expansion was written out twice. Passing the host via sys.argv[1] rather than shell-interpolating it into the Python string is the cleaner pattern.

**APPROVED** — all four fix areas are correct. Two minor notes. --- ### 1. Cache bypass (`players.py`) ✅ `and tier is None` correctly prevents a stale PNG (likely T0) from being served when a caller explicitly passes `?tier=`. The condition short-circuits the `os.path.exists` check so rendering always runs when a tier is requested. **Minor**: `tier=0` also bypasses the cache, which means a cached T0 card won’t be served even when `tier=0` is explicitly passed. Since T0 and the cached default are identical, the caller just pays an extra render — harmless, but worth knowing. If T0 should use the cache, the condition could be `and (tier is None or tier == 0)`. Not blocking. --- ### 2. Template guard placement (`player_card.html`) ✅ Moving `diamond_colors` inside `{% if refractor_tier is defined and refractor_tier > 0 %}` is correct. The old order (dict outside, `dc = diamond_colors[refractor_tier]` inside) was technically safe because `dc` is only accessed when `refractor_tier > 0` — but it was confusing. Having the dict and its consumer co-located inside the same guard is cleaner and semantically correct. No functional change for T1–T4. --- ### 3. CSS restructuring (`tier_style.html`) ✅ Extracting `#fullCard { position: relative; overflow: hidden; }` to an unconditional `<style>` block is the right call — it was duplicated identically in both the if and else branches. The new structure (base styles always emitted, tier-specific styles conditionally appended) is clean and DRY. **Minor to verify**: `#resultHeader.border-bot { border-bottom-width: 4px; }` is removed from the conditional block (around the original line 68 context). This isn’t called out in the PR description — if it’s dead CSS from the `d92ab86` visual tuning commit, the removal is correct. Just confirm it isn’t relied on by any live template path. File still lacks a trailing newline (same as PR #179). Cosmetic only. --- ### 4. `run-local.sh` ✅ Both changes are improvements: - **Playwright check**: `print(p.chromium.executable_path)` is a faster, lighter check than launching a browser instance. It raises the same exception if Chromium isn’t installed, so the `|| exit 1` path still fires correctly. The `>/dev/null 2>&1` redirect properly silences the path output. - **DB host**: Setting `DB_HOST="${POSTGRES_HOST_LOCAL:-10.10.0.42}"` once and reusing it in both the connectivity check (`sys.argv[1]`) and the `export POSTGRES_HOST` line eliminates the previous duplication where the env-var expansion was written out twice. Passing the host via `sys.argv[1]` rather than shell-interpolating it into the Python string is the cleaner pattern.
Claude approved these changes 2026-04-04 17:40:59 +00:00
Dismissed
Claude left a comment
Collaborator

Approved via pd-pr

Approved via pd-pr
Claude approved these changes 2026-04-04 17:41:04 +00:00
Claude left a comment
Collaborator

Approved via pd-pr

Approved via pd-pr
cal merged commit ad5d5561c6 into main 2026-04-04 17:41:06 +00:00
cal deleted branch fix/refractor-card-art-followup 2026-04-04 17:41:06 +00:00
Sign in to join this conversation.
No description provided.