refactor: extract TIER_NAMES/TIER_COLORS to shared constants module (#146) #155

Claude · 2026-04-08T05:04:02Z

Claude commented

2026-04-08 05:04:02 +00:00

Closes #146

Summary

Creates helpers/refractor_constants.py as single source of truth for refractor tier constants
TIER_NAMES — shared by all three consumers (was triplicated)
STATUS_TIER_COLORS — status view colors (previously TIER_COLORS in cogs/refractor.py)
NOTIF_TIER_COLORS — notification embed colors (previously TIER_COLORS in helpers/refractor_notifs.py)
Documents why the two color dicts intentionally differ (muted/metallic for status display vs celebratory for tier-up notifications)

Files Changed

helpers/refractor_constants.py — new module (source of truth)
cogs/refractor.py — removed local TIER_NAMES/TIER_COLORS; imports TIER_NAMES, STATUS_TIER_COLORS as TIER_COLORS
helpers/refractor_notifs.py — removed local TIER_NAMES/TIER_COLORS; imports TIER_NAMES, NOTIF_TIER_COLORS as TIER_COLORS
cogs/players.py — removed local REFRACTOR_TIER_NAMES; imports TIER_NAMES as REFRACTOR_TIER_NAMES (call sites unchanged)
tests/test_refractor_commands.py — updated divergence test to cover all three consumers including cogs.players

Test Results

No test command configured. Ruff lint passed on commit hook. Changes verified by reading back modified files.

Closes #146 ## Summary - Creates `helpers/refractor_constants.py` as single source of truth for refractor tier constants - `TIER_NAMES` — shared by all three consumers (was triplicated) - `STATUS_TIER_COLORS` — status view colors (previously `TIER_COLORS` in `cogs/refractor.py`) - `NOTIF_TIER_COLORS` — notification embed colors (previously `TIER_COLORS` in `helpers/refractor_notifs.py`) - Documents why the two color dicts intentionally differ (muted/metallic for status display vs celebratory for tier-up notifications) ## Files Changed - `helpers/refractor_constants.py` — new module (source of truth) - `cogs/refractor.py` — removed local `TIER_NAMES`/`TIER_COLORS`; imports `TIER_NAMES, STATUS_TIER_COLORS as TIER_COLORS` - `helpers/refractor_notifs.py` — removed local `TIER_NAMES`/`TIER_COLORS`; imports `TIER_NAMES, NOTIF_TIER_COLORS as TIER_COLORS` - `cogs/players.py` — removed local `REFRACTOR_TIER_NAMES`; imports `TIER_NAMES as REFRACTOR_TIER_NAMES` (call sites unchanged) - `tests/test_refractor_commands.py` — updated divergence test to cover all three consumers including `cogs.players` ## Test Results No test command configured. Ruff lint passed on commit hook. Changes verified by reading back modified files.

Claude added 1 commit 2026-04-08 05:04:04 +00:00

refactor: extract TIER_NAMES/TIER_COLORS to shared constants module (#146 )

Ruff Lint / lint (pull_request) Successful in 12s

Details

21bad7af51

Closes #146

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Claude referenced this pull request

2026-04-08 05:04:12 +00:00

refactor: extract TIER_NAMES/TIER_COLORS to shared constants module #146

Claude added the

ai-reviewing

label 2026-04-08 05:15:50 +00:00

Claude reviewed 2026-04-08 05:18:12 +00:00

Claude left a comment

AI Code Review

Files Reviewed

helpers/refractor_constants.py (added)
cogs/refractor.py (modified)
helpers/refractor_notifs.py (modified)
cogs/players.py (modified)
tests/test_refractor_commands.py (modified)

Findings

Correctness

No issues found.

All three consumers (cogs/refractor.py, helpers/refractor_notifs.py, cogs/players.py) correctly import from helpers.refractor_constants. Values in the new module exactly match the removed local copies.
Backward compatibility maintained: external code that imports TIER_NAMES from cogs.refractor or helpers.refractor_notifs still works — Python re-exports imported names in module namespace. The top-level test import from cogs.refractor import TIER_NAMES (line 31) continues to resolve correctly.
NOTIF_TIER_COLORS intentionally omits T0 — matches original, correct since tier-up notifications are never sent for T0.
STATUS_TIER_COLORS as TIER_COLORS alias in cogs/refractor.py means all existing TIER_COLORS[...] call sites in that module continue to work without change.

Security

No issues found. Pure constant data reorganization with no user input, credentials, or auth logic.

Style & Conventions

Import placement in cogs/players.py: the new from helpers.refractor_constants import TIER_NAMES as REFRACTOR_TIER_NAMES is appended after the utilities.* imports rather than grouped with the other helpers.* imports. Minor ordering inconsistency, not a blocker.

Suggestions

The divergence tests in TestTierNamesDivergenceCheck now compare three references to the same underlying object, so x == x == x will always be true. The docstring correctly frames them as a "tripwire against accidental re-localization" — that's accurate and they earn their keep. No action needed.

Verdict: APPROVED

Clean, well-scoped refactor. A single source of truth is clearly better than three identical copies. The aliasing strategy (STATUS_TIER_COLORS as TIER_COLORS, NOTIF_TIER_COLORS as TIER_COLORS) keeps all call sites unchanged, and the distinction between the two color dicts is now documented where it matters. No correctness or security issues.

Automated review by Claude PR Reviewer (posted as COMMENT — Gitea blocks self-approval)

## AI Code Review ### Files Reviewed - `helpers/refractor_constants.py` (added) - `cogs/refractor.py` (modified) - `helpers/refractor_notifs.py` (modified) - `cogs/players.py` (modified) - `tests/test_refractor_commands.py` (modified) ### Findings #### Correctness No issues found. - All three consumers (`cogs/refractor.py`, `helpers/refractor_notifs.py`, `cogs/players.py`) correctly import from `helpers.refractor_constants`. Values in the new module exactly match the removed local copies. - Backward compatibility maintained: external code that imports `TIER_NAMES` from `cogs.refractor` or `helpers.refractor_notifs` still works — Python re-exports imported names in module namespace. The top-level test import `from cogs.refractor import TIER_NAMES` (line 31) continues to resolve correctly. - `NOTIF_TIER_COLORS` intentionally omits T0 — matches original, correct since tier-up notifications are never sent for T0. - `STATUS_TIER_COLORS as TIER_COLORS` alias in `cogs/refractor.py` means all existing `TIER_COLORS[...]` call sites in that module continue to work without change. #### Security No issues found. Pure constant data reorganization with no user input, credentials, or auth logic. #### Style & Conventions - Import placement in `cogs/players.py`: the new `from helpers.refractor_constants import TIER_NAMES as REFRACTOR_TIER_NAMES` is appended after the `utilities.*` imports rather than grouped with the other `helpers.*` imports. Minor ordering inconsistency, not a blocker. #### Suggestions - The divergence tests in `TestTierNamesDivergenceCheck` now compare three references to the same underlying object, so `x == x == x` will always be true. The docstring correctly frames them as a "tripwire against accidental re-localization" — that's accurate and they earn their keep. No action needed. ### Verdict: APPROVED Clean, well-scoped refactor. A single source of truth is clearly better than three identical copies. The aliasing strategy (`STATUS_TIER_COLORS as TIER_COLORS`, `NOTIF_TIER_COLORS as TIER_COLORS`) keeps all call sites unchanged, and the distinction between the two color dicts is now documented where it matters. No correctness or security issues. --- *Automated review by Claude PR Reviewer (posted as COMMENT — Gitea blocks self-approval)*

Claude added

ai-reviewed

and removed

ai-reviewing

labels 2026-04-08 05:18:27 +00:00

cal approved these changes 2026-04-08 05:25:28 +00:00

Dismissed

cal left a comment

Approved via pd-pr

cal approved these changes 2026-04-08 05:25:32 +00:00

cal left a comment

Approved via pd-pr