Skip to content

♻️ refactor(i18n): replace kebab-case with snake_case in FTL keys#72

Merged
itisnotyourenv merged 6 commits intomainfrom
refactor/i18n-snake-case-keys
Feb 24, 2026
Merged

♻️ refactor(i18n): replace kebab-case with snake_case in FTL keys#72
itisnotyourenv merged 6 commits intomainfrom
refactor/i18n-snake-case-keys

Conversation

@itisnotyourenv
Copy link
Copy Markdown
Owner

@itisnotyourenv itisnotyourenv commented Feb 24, 2026

Summary

  • Rename all Fluent (.ftl) locale keys from kebab-case to snake_case across en/ and ru/ locales
  • Add separator="_" to TranslatorHub so typed method calls match the new FTL key format
  • Introduce types.py with a TYPE_CHECKING-guarded TranslatorRunner class providing IDE autocomplete for all translation keys
  • Migrate all routers to typed i18n.method() calls and unify TranslatorRunner imports to src.infrastructure.i18n
  • Remove obsolete stubs.pyi (replaced by types.py)
  • Update generate_i18n_stubs.py to target types.py with TYPE_CHECKING guard pattern
  • Add generate-i18n justfile command
  • Add logging to admin stats handlers
  • Extract stats keyboard markup to utils/markups/admin.py
  • Add check_alive_btn locale key for en/ru

Why

Fluent's recommended key format uses hyphens, but fluentogram's TranslatorRunner maps keys to Python method calls using a configurable separator. With underscore-based keys and separator="_", the FTL keys directly match Python method names — eliminating the need for kebab-to-snake conversion and enabling type-safe i18n.key_name() calls with full IDE support.

Changes

Area Files What changed
Locales 8 .ftl files All keys: -_, added check_alive_btn
Infrastructure hub.py, types.py, __init__.py Separator config, typed TranslatorRunner, exports
Routers 7 Python files Typed i18n method calls, middleware injection, unified imports, logging
Markups settings.py, admin.py (new) Typed i18n calls, extracted stats keyboard
Scripts generate_i18n_stubs.py Now generates types.py with TYPE_CHECKING guard
Tooling justfile Added generate-i18n command
Cleanup stubs.pyi Deleted (replaced by types.py)

Checklist

  • All FTL keys migrated (verified with grep — zero remaining hyphenated keys)
  • All Python references updated to typed method calls
  • TranslatorRunner imports unified to src.infrastructure.i18n
  • Logging added to admin stats handlers
  • Ruff format + lint pass
  • Pre-commit hooks pass

🤖 Generated with Claude Code

- Rename all locale keys from kebab-case to snake_case across en/ and ru/
- Add separator="_" to TranslatorHub so typed method calls match FTL keys
- Introduce types.py with TYPE_CHECKING guard for typed TranslatorRunner
- Migrate routers to use typed i18n method calls where applicable
- Unify TranslatorRunner imports to src.infrastructure.i18n
- Remove obsolete stubs.pyi (replaced by types.py)
- Update generate_i18n_stubs.py to target types.py
- Add generate-i18n justfile command

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

This comment was marked as outdated.

itisnotyourenv and others added 2 commits February 24, 2026 03:27
…i18n

- Add logger.info calls to all three admin stats handlers
- Switch from TranslatorHub to i18n middleware injection
- Use typed i18n method calls instead of i18n.get() strings
- Extract stats keyboard markup to utils/markups/admin.py
- Add check_alive_btn locale key for en/ and ru/

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace i18n.get() string calls with typed method calls
in _start_onboarding and command_start_handler.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@claude
Copy link
Copy Markdown

claude bot commented Feb 24, 2026

Supplementary Review Notes

The previous review comment (by an earlier Claude session) covers the major issues well. A few additional observations not mentioned there:


Critical Issue

check_alive button has no registered callback handler

stats_main_markup adds an InlineKeyboardButton with callback_data="check_alive", but no handler for this callback exists in stats.py (or anywhere in the admin router). Clicking the button will silently do nothing or produce an unhandled update error.

This button was present before the PR as a hardcoded string ("Check Alive"), so it is a pre-existing gap — but this PR promotes it to a properly localized key (check_alive_btn) and moves it into the shared stats_main_markup helper, implying it is ready for use. Either add a handler or leave it as a clearly marked stub (e.g., callback_data="noop" with a placeholder handler that just calls callback.answer()).


Minor Additional Suggestions

write_text in generator missing encoding="utf-8"

The script was correctly updated to read with explicit encoding:

content = ftl_file.read_text(encoding="utf-8")

But the write still uses the platform default:

output_file.write_text(types_content)

Should be output_file.write_text(types_content, encoding="utf-8") for consistency and cross-platform safety.

cb_back_to_stats is inconsistent within its own file

The existing review table marks all of admin/stats.py as untyped, but stats_handler actually does use typed calls (i18n.stats_overview(...)). The remaining inconsistency is specifically in cb_back_to_stats, which still uses i18n.get("stats_overview", ...) while the identical call in stats_handler is typed. These two handlers render the same view — they should use the same pattern.


These are additive to the prior review's "Request Changes" verdict. The check_alive handler gap is the only new blocker.

@claude

This comment was marked as outdated.

itisnotyourenv and others added 2 commits February 24, 2026 03:34
- Switch from TranslatorHub to i18n middleware injection
- Replace i18n.get() string calls with typed method calls
- Remove duplicate referral_info call

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace i18n.get() string calls with typed method calls
for btn_settings, btn_language, and btn_back buttons.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@claude
Copy link
Copy Markdown

claude bot commented Feb 24, 2026

PR Review: ♻️ refactor(i18n): replace kebab-case with snake_case in FTL keys

Overall Assessment: Request Changes

The core idea is solid — snake_case keys with separator=\"_\" make FTL keys map directly to Python method names, and the TYPE_CHECKING-guarded types.py is a clean pattern. However, there are a few bugs and inconsistencies that need addressing before merging.


🔴 Critical Issues

1. Incomplete migration in cb_back_to_statsstats.py:82

stats_handler correctly uses the typed call, but cb_back_to_stats was not fully migrated:

# stats_handler (correct):
text=i18n.stats_overview(total=..., referred=..., ...)

# cb_back_to_stats (missed):
text=i18n.get(
    "stats_overview",   # ← still using string-based .get()
    total=stats.total_users,
    ...
)

This is a missed migration in the same file. Functionally it works (the key name is correct), but it's inconsistent with the entire purpose of this refactor and defeats the type-safety goal.

2. _I18nArg TypeVar with constraints is overly restrictive — types.py:10

_I18nArg = TypeVar("_I18nArg", str, int)

A TypeVar with constraints (not bound) means all usages within a single call must resolve to the same concrete type. So this call would fail type checking:

i18n.referral_info(link="https://t.me/...", count=42)
# link: str, count: int → TypeVar can't be both simultaneously

The fix for Python 3.13 is to use str | int directly in the signatures, since constrained TypeVars are not the right tool here:

def referral_info(self, *, link: str | int, count: str | int) -> str: ...

Or, if a shared alias is preferred, use type _I18nArg = str | int (PEP 695) or _I18nArg = str | int as a plain alias.


🟡 Significant Issues

3. main.py:397 — startup notification not migrated

# Still using string-based .get():
await bot.send_message(chat_id=admin_id, text=i18n.get("bot_started"))

Should be i18n.bot_started(). Minor inconsistency, but the checklist in the PR description claims all Python references are updated.

4. check_alive.py not migrated to i18n

check_alive.py was added to the router (and a check_alive_btn locale key was introduced for the entry-point button), but all button labels inside the check_alive flow remain hardcoded English strings:

InlineKeyboardButton(text="All Users", ...)
InlineKeyboardButton(text="30 Days", ...)
InlineKeyboardButton(text="Back", ...)
InlineKeyboardButton(text="Back to Stats", ...)
# Also: "Select users to check:", "Starting alive check for...", etc.

This seems out of scope for the PR but is worth flagging — the PR added a locale key for the entry button while leaving the rest of the feature untranslated.

5. TypeVar imported at module level but only used inside TYPE_CHECKINGtypes.py:7

from typing import TYPE_CHECKING, TypeVar   # TypeVar imported unconditionally

if TYPE_CHECKING:
    _I18nArg = TypeVar(...)                 # only used here

At runtime TypeVar is imported but never used. Move it inside the if TYPE_CHECKING: block:

from typing import TYPE_CHECKING

if TYPE_CHECKING:
    from typing import TypeVar
    _I18nArg = ...

🟢 Suggestions / Minor

6. Misleading comment in generate_i18n_stubs.py:231

# Convert kebab-case to snake_case for method name
method_name = key.replace("-", "_")

Since all FTL keys are now snake_case, this conversion is a no-op. The comment should be removed or updated to reflect that it's a harmless guard for any hypothetical hyphens.

7. justfile missing trailing newline

Both lint and generate-i18n targets end without a newline (\ No newline at end of file). Consistent newlines at EOF are a standard POSIX convention and avoid noisy diffs.

8. Type annotation cast pattern

i18n: TranslatorRunner = hub.get_translator_by_locale(new_language.value)

This is a type hint, not a cast — the runtime type is unchanged. For clarity and correctness, use typing.cast:

from typing import cast
i18n = cast(TranslatorRunner, hub.get_translator_by_locale(new_language.value))

This makes the intent explicit. (Applies to onboarding.py:616 and settings.py:719.)

9. No tests added

The refactor touches 7 routers and 2 markup utilities. No tests were added or updated. Given the project enforces 90% coverage, it's worth verifying the existing test suite still meets that threshold and consider adding smoke-tests for the new stats_main_markup function and the typed i18n wiring.


What's Good

  • The TYPE_CHECKING guard pattern in types.py is the right approach — zero runtime overhead, full IDE support.
  • Migrating from hub.get_translator_by_locale(locale) per-handler to middleware injection is a clean improvement.
  • Extracting stats_main_markup to utils/markups/admin.py removes duplication correctly.
  • The generate-i18n justfile command and encoding fix (read_text(encoding="utf-8")) are good additions.
  • Commit history is well-organized with clear, atomic messages.

@itisnotyourenv itisnotyourenv merged commit 2d845ef into main Feb 24, 2026
8 of 9 checks passed
@itisnotyourenv itisnotyourenv deleted the refactor/i18n-snake-case-keys branch February 24, 2026 00:47
@claude
Copy link
Copy Markdown

claude bot commented Feb 24, 2026

PR Review: refactor(i18n): replace kebab-case with snake_case in FTL keys

Overall Assessment: Request Changes — the refactor direction is solid, but there's one functional bug and several issues worth addressing before merge.


Critical Issues

1. Missing callback.answer() in unified stats_handler (stats.py)

When update is a CallbackQuery (the admin:back_to_stats path), the handler calls update.message.edit_text() but never calls update.answer(). Telegram requires every callback query to be acknowledged within 10 seconds, otherwise the button shows a perpetual loading spinner.

# Current - missing callback.answer() for CallbackQuery branch
if isinstance(update, CallbackQuery):
    await update.message.edit_text(**kwargs)  # ← no await update.answer()
else:
    await update.answer(**kwargs)

The old cb_back_to_stats handler had await callback.answer() at the end. That got lost during the deduplication.

Fix: Add await update.answer() after the edit_text call for the CallbackQuery branch.


Bugs

2. ref_top_callback leaves users stranded — no back button

After viewing the top referrers list, callback.message.edit_text(text=text) is called without a reply_markup. The user sees the list with no way to navigate back to stats. This was broken before this PR too (the old handler also had no back button), but the deduplication of cb_back_to_stats makes it more visible now that the admin:back_to_stats callback is wired up.


Suggestions

3. make_label in settings.py still uses i18n.get() (bypasses typed API)

def make_label(key: str, code: str) -> str:
    label = i18n.get(key)  # string-based, defeats the purpose

The keys passed in ("lang_en", "lang_ru") are now typed methods. The get() escape hatch is still valid since you added it to the TranslatorRunner stub, but the intent of the PR is typed calls. Consider rewriting this inline without make_label, using i18n.lang_en() / i18n.lang_ru() directly and adding the marker via a helper that takes the pre-fetched string.

4. check_alive.py not localized — consistency gap

The check_alive router hardcodes English for all its strings ("All Users", "30 Days", "Back", "Check Complete!", etc.). While this is pre-existing and out of scope for this PR, it's inconsistent with the typed i18n system being established here. Worth a follow-up issue.

5. Last commit message doesn't follow project conventions

remove duplicated code doesn't match the type(scope): description pattern used by all other commits in this PR and the project history. Should be something like refactor(bot): deduplicate stats and back_to_stats handlers.

6. Justfile still missing trailing newline

The generate-i18n recipe ends without a trailing newline (\ No newline at end of file). This matches the pre-existing lint issue which was fixed in this PR — would be good to fix both.


What's Done Well

  • The TYPE_CHECKING guard pattern for types.py is clean: zero runtime overhead, full IDE support, and the else branch transparently re-exports the real fluentogram.TranslatorRunner.
  • Unifying stats_handler + cb_back_to_stats into a single handler with Message | CallbackQuery is a nice DRY improvement.
  • Extracting stats_main_markup to utils/markups/admin.py follows the established pattern in markups/settings.py.
  • Adding encoding="utf-8" to ftl_file.read_text() in the generator is a correctness fix.
  • Adding separator="_" to TranslatorHub is the correct approach; the keys now map 1:1 to Python method names.

Questions

  • Is there a plan to add the check_alive_btn and the back-navigation strings (Back, Back to Stats) to the FTL files for check_alive.py in a follow-up?
  • The generate-i18n justfile command currently runs ruff format on just types.py. Should it also run ruff check --fix to ensure lint consistency?

Summary: Fix the missing callback.answer() (#1) and the missing back button for ref_top_callback (#2) before merging. The rest are suggestions or follow-up candidates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant