Skip to content

perf(i18n): 缓存 macOS locale 探测结果 + 清 #2519 遗留的中文 docstring - #2523

Merged
wehos merged 2 commits into
mainfrom
chore/locale-detection-followup
Jul 28, 2026
Merged

perf(i18n): 缓存 macOS locale 探测结果 + 清 #2519 遗留的中文 docstring#2523
wehos merged 2 commits into
mainfrom
chore/locale-detection-followup

Conversation

@wehos

@wehos wehos commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

#1542 收尾,两件小事。

1. _get_macos_locale() 加进程级缓存

每次未命中都要 subprocess.run(['/usr/bin/defaults', 'read', '-g', key]),超时 1s,原实现无缓存。而 initialize_global_language() 会经 _is_china_region_get_system_language 各调它一次,每次最多试 AppleLocale / AppleLanguages 两个 key —— 最坏一次冷启动 spawn 4 次、约 4 秒,且整个初始化持 _global_language_lock

get_global_language() / get_global_language_full() 又大量出现在 async 请求路径上(app/memory_server/routes.py 等),首次触发若落在事件循环里就会把它阻塞住 —— 与 #2466「async 路径同步配置读挪出事件循环」的方向相悖。

改成双检 + 锁的进程级缓存(OS locale 在进程运行期间不会变),真正的查询逻辑拆到 _read_macos_locale_uncached()

顺带修了一个实测出来的测试顺序污染:加缓存后 test_macos_locale_reads_apple_locale 会读到别的用例留下的缓存值(非 macOS CI 上是 None),断言就永远看不到 fake_run 的结果 —— 先跑 test_language_region_override.py 再跑它就会红。已加 monkeypatch.setattr(language_utils, "_macos_locale_cache", None) 隔离。

2. 清掉 #2519 遗留的 4 处中文 docstring

DOCSTRING_CJK 门要求 docstring 用英文。#2519 合并时带进去 4 处中文的,这里一并清掉,背景说明移到 # 注释(注释不受该门限制)。

关于 changelog:本 PR 不做,已转 #2522

原计划在这个 PR 里补 #1542 那批 locale 变更的 changelog(台港用户升级后区域选源与界面语言会变),查完之后判断不该在功能 PR 里加

  • main_routers/system_router/changelog_survey.pyget_changelog 只有 if file_ver > since_ver 一个条件,没有 <= APP_VERSION 的上界。现在建 config/changelog/0.8.4.md,跑在 0.8.3 的用户下次打开就会看到尚未发布的内容。
  • v0.8.3 tag 已存在、APP_VERSION 仍是 0.8.3,说明 0.8.4 还没开题。
  • 仓库惯例是发版时统一写全语言(参见 1f3188ca0 release: 0.8.3 全语言更新日志 + 问卷从 0.8.2 迁到 0.8.3)。

已开 #2522 挂到发版流程上。

回归报告 / Regression Report

  • 改动了什么_get_macos_locale() 加进程级缓存并拆出未缓存实现;补两条测试;清 4 处中文 docstring。
  • 理由 / 必要性:冷启动最坏 4 次 1s 超时的 subprocess 且全程持锁,而调用方在 async 路径上。
  • 改动前后的表现对比:返回值不变,只是同一进程内第二次起直接命中缓存。副作用:运行中修改系统语言需重启才生效 —— 与既有的 _global_language_initialized 缓存行为一致。
  • 潜在回归点:(a) 测试若在缓存被填充后 monkeypatch subprocess.run,会读到旧值 —— 已由 _reset_macos_locale_cache() 与 monkeypatch 清缓存覆盖,并新增 test_macos_locale_is_read_once_per_process 守住缓存本身;(b) 非 macOS 平台 _read_macos_locale_uncached 仍在第一行返回 None,行为不变。
  • 验证:变异验证 —— 去掉写缓存那行后 test_macos_locale_is_read_once_per_process 变红,还原后 13 passed;check_docstring_no_cjk --base origin/main 退出码 0;uv run pytest tests/unit -q6812 passed, 26 skipped, 0 failed

不拆分理由 / Why Not Split

两处都在 locale 探测这条线的收尾上,共 4 个文件、+79/-23,且共享同一次全量验证。


Review 后的修正(64520104f

Greptile 两条 P1 都成立,已处理:

1. 瞬时失败被永久缓存 —— 原实现无论成功失败都写缓存,macOS 上首次 defaults 超时就会把 None 钉死一整个进程周期,而那恰恰是这个 helper 要兜底的场景。改为只缓存确定性结论:非 Darwin(缓存 None)、探测到 locale(缓存该值);超时 / 非零退出 / 空输出一律不写缓存。顺带把哨兵从 (bool, value) 元组换成独立的 _MACOS_LOCALE_UNSET(原来的 bool 恒为 True,是冗余状态)。新增两条测试,变异验证:改回「失败也缓存」后 test_macos_locale_probe_failure_is_retried_not_cached 立刻变红。

2. 上层初始化仍会钉死语言 —— 这条我在本 PR 里没能解决,已开 #2525 跟踪。核实下来是既有设计92ed2818b~1#1542 合入前)的 initialize_global_language() 已经是无条件 _global_language_initialized = True,而 _get_system_language() 自带兜底(最终 return 'en'),所以「探测失败」表现为回落值被当成探测结果存下。

同时修正本 PR 上文一处过强的说法:「失败不缓存 → 下次调用重试」只在没走上层初始化缓存时成立。它仍有两处实际价值 —— 同一次 initialize_global_language()_is_china_region()_get_system_language() 会各调一次 _get_macos_locale(),前者瞬时失败后者仍可能成功;以及直接调用该 helper 的路径。但对「进程已完成初始化」这条主链路无效。

3. Unused global variable ×3 —— 误报,未改。_macos_locale_cache 读 2 次写 3 次,三条都报在 global 声明行上,分析器应该是没跟踪 global 造成的模块级绑定。

最终验证:uv run pytest tests/unit -q6814 passed, 26 skipped, 0 failed

_get_macos_locale() 每次未命中都要 subprocess.run(['/usr/bin/defaults', ...]),
超时 1s,且原实现无缓存。initialize_global_language() 会经 _is_china_region 和
_get_system_language 各调它一次,每次最多试 AppleLocale / AppleLanguages 两个
key——最坏一次冷启动 spawn 4 次、约 4 秒,而整个初始化过程持 _global_language_lock。
get_global_language() / get_global_language_full() 又大量出现在 async 请求路径上,
首次触发若落在事件循环里就会把它阻塞住,与 #2466「async 路径同步配置读挪出事件
循环」的方向相悖。

改为进程级缓存(双检 + 锁),OS locale 在进程运行期间不会变。真正的查询逻辑拆到
_read_macos_locale_uncached(),另提供 _reset_macos_locale_cache() 供测试使用。

测试:
- test_macos_locale_is_read_once_per_process:连续三次调用只 spawn 一次
- test_macos_locale_reads_apple_locale 补 monkeypatch 清缓存——否则它会读到别的
  用例留下的缓存值(非 macOS CI 上是 None),断言永远看不到 fake_run 的结果。
  这个顺序污染是实测出来的:先跑 test_language_region_override.py 再跑它就红。

顺带清掉 #2519 遗留的 4 处中文 docstring(DOCSTRING_CJK 门要求英文),背景说明
移到注释里。

验证:变异验证——去掉写缓存那行后 test_macos_locale_is_read_once_per_process 变红,
还原后 13 passed;check_docstring_no_cjk --base origin/main 退出码 0;
uv run pytest tests/unit -q → 6812 passed, 26 skipped, 0 failed。

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 2 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 490357f3-34e9-4e1a-babd-98712022927e

📥 Commits

Reviewing files that changed from the base of the PR and between 6852ea8 and 6452010.

📒 Files selected for processing (4)
  • tests/unit/test_memory_language_resolution.py
  • tests/unit/test_memory_request_language_context.py
  • tests/unit/test_outbox_wiring.py
  • utils/language_utils.py

Comment @coderabbitai help to get the list of available commands.

Comment thread utils/language_utils.py Outdated
@greptile-apps

greptile-apps Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR refines macOS locale probing and cleans test docstrings 喵

  • Adds a locked process-level cache for conclusive macOS locale results while leaving failed probes uncached 喵
  • Splits the subprocess-based lookup into _read_macos_locale_uncached() and adds cache/reset coverage 喵
  • Converts four CJK docstrings to English while retaining detailed rationale in comments 喵

Confidence Score: 2/5

This PR is not yet safe to merge because a transient first macOS locale-probe failure still pins the process to the wrong global language and region until restart 喵

Leaving _macos_locale_cache unset does not provide recovery because initialize_global_language() commits the fallback and all ordinary getters subsequently bypass locale probing, so the previously reported failure remains reachable 喵

Files Needing Attention: utils/language_utils.py and tests/unit/test_memory_language_resolution.py 喵

Important Files Changed

Filename Overview
utils/language_utils.py Adds synchronized locale caching, but the uncached-failure retry is bypassed once global initialization persists the fallback 喵
tests/unit/test_memory_language_resolution.py Adds direct helper-cache tests, but the retry test does not exercise the global initialization path that permanently retains the fallback 喵
tests/unit/test_memory_request_language_context.py Rewords test docstrings and preserves the existing request-language assertions unchanged 喵
tests/unit/test_outbox_wiring.py Rewords test docstrings while leaving outbox behavior and assertions unchanged 喵

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[First global-language lookup] --> B[macOS defaults probe]
  B -->|Success| C[Cache resolved locale]
  B -->|Transient failure| D[Return None]
  D --> E[Store fallback global language]
  E --> F[Mark global language initialized]
  F --> G[Later getters return fallback without retry]
Loading

Reviews (2): Last reviewed commit: "fix(review): macOS locale 探测失败不再被永久缓存" | Re-trigger Greptile

Comment thread utils/language_utils.py Fixed
Greptile P1 指出得对:原实现无论成功失败都写缓存,于是 macOS 上首次 defaults
超时(或非零退出 / 空输出)会把 None 钉死一整个进程周期——区域判定掉到
non-china、语言判定掉到英文,而「launcher 子进程继承中性 locale、其它信号都不
可靠」恰恰是这个 helper 要兜的场景。

改为只缓存确定性结论:
- 非 Darwin:确定性,缓存(省掉后续每次的 platform 判断)
- Darwin 且探测到 locale:缓存
- Darwin 但探测失败:不写缓存,下次调用重试

顺带把哨兵从 (bool, value) 元组换成独立的 _MACOS_LOCALE_UNSET 对象——原来的
bool 恒为 True,是冗余状态。

测试:
- test_macos_locale_probe_failure_is_retried_not_cached:前两次 defaults 抛
  TimeoutExpired → 返回 None 且不缓存;下一次调用重新探测拿到真实 locale
- test_macos_locale_non_darwin_is_cached:非 macOS 只判一次 platform.system()
- 既有两条的 monkeypatch 从 None 改为 _MACOS_LOCALE_UNSET(None 现在表示
  「已确认非 macOS」,不再是「未缓存」)

验证:变异验证——改回「失败也缓存」后 probe_failure 用例变红,去掉非 macOS 缓存后
non_darwin 用例变红;uv run pytest tests/unit -q → 6813 passed, 26 skipped, 0 failed。

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Comment thread utils/language_utils.py
Comment thread utils/language_utils.py
Comment thread utils/language_utils.py
@wehos
wehos merged commit cae254e into main Jul 28, 2026
12 checks passed
@wehos
wehos deleted the chore/locale-detection-followup branch July 28, 2026 07:21
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