fix: address i18n review issues from PR #718#758
Merged
igorls merged 1 commit intoMemPalace:developfrom Apr 15, 2026
Merged
Conversation
0df7313 to
4b7c551
Compare
4b7c551 to
f40aac4
Compare
Three issues flagged by bensig on the i18n PR before merge:
1. ko.json: status_drawers used {drawers} instead of {count}, causing
the Korean UI to show the raw template string instead of the actual
drawer count. All other 7 languages use {count}.
2. Test file was shipped inside the package at mempalace/i18n/test_i18n.py
with a sys.path.insert hack. Moved to tests/test_i18n.py per the
project convention in AGENTS.md.
3. Dialect.from_config() passed lang=config.get("lang") which defaults
to None, causing __init__ to inherit whatever language was loaded
earlier via module-level state. Now defaults to "en" explicitly so
from_config is deterministic regardless of prior load_lang() calls.
Added two regression tests for the ko.json fix and the state leak.
f40aac4 to
d565718
Compare
This was referenced Apr 15, 2026
7 tasks
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses three issues bensig flagged on PR #718 before merge.
1. ko.json variable name mismatch (line 28)
status_drawersused{drawers}while all other 7 languages use{count}. Thet()call passescount=N, so Korean showed the raw template string instead of the number. One-character fix.2. Test file shipped inside the installed package
mempalace/i18n/test_i18n.pyused asys.path.inserthack to work around its wrong location. Moved totests/test_i18n.pyper AGENTS.md convention. Removed the sys.path hack and the__name__runner block.3. Dialect.from_config() state leak
from_configpassedlang=config.get("lang")which defaults to None. When no"lang"key is in the config,__init__fell through tocurrent_lang()-- a module-level global set by whicheverload_lang()call ran most recently. Now defaults to"en"explicitly sofrom_configis deterministic.Bug 1 from bensig's review (compress() never uses self.aaak_instruction or self.lang_regex) is intentionally not addressed here -- bensig suggested deferring the regex wiring until alignment with #488/#507 on extraction approach.
Changes (3 files, +19/-17):
mempalace/i18n/ko.json--{drawers}->{count}mempalace/dialect.py--config.get("lang")->config.get("lang", "en")tests/test_i18n.py-- moved frommempalace/i18n/, added 2 regression testsReferences: bensig's review comments on PR #718 (2026-04-12T21:54Z and 22:01Z)