refactor(core,cli): rename generic Gemini identifiers to Llm - #10124
Conversation
PR 1 of #4063 item 6 (de-Google naming). Renames three independent families plus the leaf LLM types: - Memory filename: GeminiMd* -> Memory* (project memory file, not an LLM client) - UI spinners: GeminiRespondingSpinner/GeminiSpinner -> RespondingSpinner/Spinner - Leaf types: GeminiCodeRequest/GeminiChatSendOptions/GeminiErrorEventValue/GeminiFinishedEventValue -> Llm* - geminiRequest.ts -> llm-request.ts (and its collocated test) No behavior change. Renamed symbols typecheck clean in core+cli; eslint clean on renamed files. Refs #4063
Assert that the primary daemon workspace service receives the hard-coded 'QWEN.md' context filename when boot settings carry no context.fileName. Previously only the secondary workspace's explicit SECONDARY.md resolution was asserted, so swapping the fallback literal at the createDaemonWorkspaceService call site survived the suite.
Complete the rename flagged in review: GeminiFileContent -> MemoryFileContent (module-local interface), includeDirectoriesToReadGemini -> includeDirectoriesToReadMemory (parameter only; all call sites are positional, zero cross-package impact), plus test-local variable names and the stale ORIGINAL_GEMINI_MD_FILENAME test title.
Per exception #1 the Llm prefix is reserved for the generic LLM-client surface; the symbol is a memory-file loader (thin wrapper around core's loadServerHierarchicalMemory), so the PR-2 symbol map targets loadHierarchicalMemory instead of loadHierarchicalLlmMemory. Doc-only: the code symbol is not renamed by this PR.
The undefined fallback first inherits the primary workspace's configured context.fileName snapshot (contextFilenameForInit) at the secondary startup and dynamically added workspace call sites, before the hard-coded QWEN.md. Describe the actual chain instead of the hard-coded default only. Comment-only: the inheritance behavior predates this PR and is unchanged.
…lMemory The design doc's symbol map routes the memory loader to loadHierarchicalMemory (memory family, exception #1), but no phasing bullet performed the rename and a prior round left the mixed signature. Complete the rename across the definition (config.ts), the AppContainer call site, and the AppContainer test mocks, and update the design doc's exception #1, symbol map, and PR-1 phasing bullet so the map row is no longer orphaned.
# Conflicts: # packages/core/src/memory/memoryDiscovery.ts
…4063-rename-core-llm # Conflicts: # packages/cli/src/acp-integration/session/Session.review-lease.test.ts # packages/cli/src/acp-integration/session/Session.test.ts # packages/cli/src/acp-integration/session/Session.ts # packages/cli/src/acp-integration/session/Session.worktree.test.ts # packages/cli/src/ui/hooks/useBranchCommand.test.ts # packages/cli/src/ui/hooks/useResumeCommand.test.ts # packages/core/src/agents/forkedAgent.ts # packages/core/src/core/geminiChat.ts # packages/core/src/core/llm-chat.test.ts # packages/core/src/core/openaiContentGenerator/converter.test.ts
|
Template looks good ✓ — all required sections, N/A Before/After (refactor), bilingual summary. Problem: not a speculative cleanup — this is Phase 2 of the tracked fork-residue work in #4063 (item 6), governed by Direction: aligned. The generic client/chat/event layer serves every supported provider but still carries names inherited from the Gemini CLI fork, while the repo already uses the Size: 13,509 production-logic lines (129 files) + 6,582 test lines (105 files), 234 files total. The Tier-1 refactor hard block doesn't apply — maintainer-authored PR. The 1000+ line advisory fires, but the design doc's coupling argument ( Approach: scope matches the design doc's PR-2 symbol map exactly. Aliases are same-binding re-exports; the old chat deep-import path is retained as a deprecated shim; true Gemini names (extension converter, Risk: Stage 1e high-risk paths matched — Moving on to code review. 🔍 中文说明模板完整 ✓——各必填部分齐全,Before/After 为 N/A(重构),含中文说明。 **问题:**不是投机性清理——这是 #4063(第 6 项)追踪的 fork 遗留清理的第二阶段,由 **方向:**对齐。通用 client/chat/event 层服务于所有 provider,却仍沿用 Gemini CLI fork 遗留命名,而仓库其他地方已经在用 **规模:**13,509 行生产逻辑(129 个文件)+ 6,582 行测试(105 个文件),共 234 个文件。Tier-1 重构硬拦截不适用——维护者自己的 PR。1000+ 行大 PR 提示触发,但设计文档的耦合论证( **方案:**范围与设计文档的 PR-2 符号表完全一致。别名是同绑定 re-export;旧 chat 深层导入路径保留为废弃 shim;真正的 Gemini 名称(extension converter、 **风险:**Stage 1e 高风险路径命中—— 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code review (static — the reviewed head includes today's restack merge)No critical findings. This is a provably mechanical rename, and I verified it rather than assumed it:
One non-blocking follow-up: this gate's own high-risk path list ( Files changed (25 of 234 shown)
Test evidence — the PR's own CI
To be plain about what this section does and does not carry: there is no CI signal on this commit, structurally — the repo's CI only fires for PRs based on Sandboxed verification would settle this: 中文说明代码评审(静态)无阻断性发现。这是可证明的机械重命名,且是验证过的,不是假设:
一条非阻断跟进:本门禁自身的高风险路径列表仍在 grep 测试证据这个提交上没有 CI 信号,且是结构性的:仓库 CI 只对基于 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean, provably mechanical, design-faithful rename with the compatibility surface pinned by tests; the withheld fifth is the structural absence of any execution evidence on a stacked base, not doubt about the code. Stepping back: my independent read of "how would I do this cleanup" lands exactly where this PR sits — same-binding aliases for the published names, a shim for the old deep-import path, one atomic move for the coupled layer, and nothing renamed that describes the real Gemini provider or a wire/persisted format. The diff does precisely that and nothing more: across 42.7k diff lines the minimality scan surfaced not one logic change, and the file-level comparison of the moved chat module and stream hook against their base versions showed pure renames. The mid-review restack was handled the right way — the merge carried the rename into the files that had landed on The honest reservation: nobody — including CI — has compiled or run this exact tree, because the repo's CI doesn't fire for stacked bases. My approval therefore rests on static verification plus the pinned alias-identity tests, with the merge queue as the structural backstop when this lands on Verdict: approve — same-repo maintainer PR, Phase 2 of an approved design whose Phase 1 was approved at 5/5, every stage clean. Approving pinned to the reviewed commit. ✅ 中文说明置信度:4/5 ——干净、可证明的机械性、忠实于设计的重命名,兼容面由测试钉住;扣掉的一分来自 stacked base 上结构性缺失的执行证据,而非对代码本身的疑虑。 退一步看:我对"这个清理该怎么做"的独立判断与这个 PR 完全一致——已发布名称用同绑定别名、旧深层导入路径用 shim、耦合层一次性原子迁移、描述真实 Gemini provider 或线协议/持久化格式的名称一律不改。diff 恰好做到这些,没有更多:4.27 万行 diff 的最小性扫描没有发现任何一处逻辑改动,移动后的 chat 模块与 stream hook 和 base 版本做文件级对比,确认是纯改名。评审中途的 restack 处理方式正确——合并把改名带进了分支分歧后落到 诚实的保留意见:包括 CI 在内,没有人编译或运行过这棵确切的树,因为仓库 CI 不对 stacked base 触发。因此我的批准基于静态验证加别名身份测试,落地 结论:批准——同仓维护者 PR,已批准设计的第二阶段(第一阶段以 5/5 批准),各阶段干净。批准锚定在被审提交上。✅ — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
The base branch was changed.
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterFull-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 12 scenario(s). — Qwen Code · serve A/B |
chiga0
left a comment
There was a problem hiding this comment.
Review — refactor(core,cli): rename generic Gemini identifiers to Llm
Tier: Standard — broad mechanical rename across core/cli, no new logic. 100 files in the original scope, plus a handful of follow-up commits fixing test expectations and snapshots after the base merge.
What I checked
Wire-format discipline (class 1 — contract asymmetry)
LlmEventTypeenum string values (content,tool_call_request, …) are unchanged — only the TypeScript name changed. ✓HistoryItemLlm*type discriminant strings remain"gemini","gemini_content","gemini_thought"— persisted session history deserializes correctly. ✓- HTTP header
"x-gemini-api-privileged-user-id"inllm-content-generator/index.tsunchanged. ✓ - Startup-profiler telemetry event keys
"geminiImportStart"/"geminiImportEnd"intentionally left incli.ts(consistent with PR scope: persisted/observable string discriminants not renamed).
Public API backward-compat (class 2 — API/compatibility contract)
All published aliases verified present and identity-tested in packages/core/src/index.test.ts:
client.ts→export { LlmClient as GeminiClient }✓llm-chat.ts→export { LlmChat as GeminiChat }✓turn.ts→export { LlmEventType as GeminiEventType }+ allServerGemini*type aliases ✓geminiChat.tsmodule shim →export * from "./llm-chat.js"✓geminiContentGenerator/paths → re-exportLlmContentGenerator as GeminiContentGeneratorandcreateLlmContentGenerator as createGeminiContentGenerator✓Config.getGeminiClient()→ deprecated wrapper callinggetLlmClient()✓skipGeminiInitializationoption backed by!(skipLlmInitialization ?? skipGeminiInitialization)✓
Identity-equality (toBe) tests in index.test.ts confirm every alias resolves to the same runtime object as the new name.
Follow-up commits (44352b9 → 7ac8e06)
Two files changed: IdeTrustChangeDialog.test.tsx updated test assertions to match the corrected user-visible string "Press r to restart Qwen Code"; HistoryItemDisplay.test.tsx.snap snapshot key renamed to match the renamed test. Both are correct and consistent with the component.
CI at review time
- Classify PR, Dependency CVE audit, Desktop Shell (ubuntu/windows), Dependency CVE, Secret scan — pass ✓
- Test (ubuntu-latest, Node 22.x), Capture web-shell visuals, Real daemon E2E, Serve A/B — pending (new CI run triggered by latest commit)
- Test (macos-latest), Test (windows-latest), Integration Tests (CLI, No Sandbox) — SKIPPING
The skipped platform unit-test runs are worth noting; for a pure rename with no new platform-specific branches the risk is low, but macOS and Windows test coverage is unexercised by CI on this PR.
Verdict
No blockers. Wire-format discipline is sound, deprecated alias coverage is comprehensive, and the identity-equality tests provide direct verification. The change achieves its stated goal — a single consistent Llm vocabulary for generic components without breaking existing consumers.
Reviewed with AI assistance.
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。移除 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| skipFileCheckpointing: true, | ||
| }); | ||
|
|
||
| expect(config.getGeminiClient()).toBe(config.getLlmClient()); |
There was a problem hiding this comment.
[Suggestion] The new 'accepts the deprecated Gemini initialization option' test only asserts getGeminiClient() === getLlmClient() — true by construction of the alias — and a clean shutdown. It never asserts that skipGeminiInitialization actually skipped LLM-client initialization, so the ?? options?.skipGeminiInitialization fallback in Config.initialize() goes unpinned. If a follow-up completes the rename and drops that fallback, this test still passes green (../core/client.js is mocked, so initialize() resolves harmlessly), and external consumers still passing the deprecated option would silently get full chat initialization on bootstrap paths that must not initialize before a session exists. Capture the mocked client instance and assert its initialize was not called; ideally also add a case passing both { skipLlmInitialization: false, skipGeminiInitialization: true } to pin that the explicit new flag wins. The strengthened test must go red if the ?? options?.skipGeminiInitialization fallback is removed.
| expect(config.getGeminiClient()).toBe(config.getLlmClient()); | |
| expect(config.getGeminiClient()).toBe(config.getLlmClient()); | |
| const llmClient = vi.mocked(LlmClient).mock.results.at(-1)?.value as | |
| | { initialize: Mock } | |
| | undefined; | |
| expect(llmClient?.initialize).not.toHaveBeenCalled(); |
中文说明
新增的 'accepts the deprecated Gemini initialization option' 测试只断言 getGeminiClient() === getLlmClient()(由别名构造方式决定必然为真)和正常 shutdown,从未断言 skipGeminiInitialization 确实跳过了 LLM client 初始化,因此 Config.initialize() 中的 ?? options?.skipGeminiInitialization 兜底没有被测试钉住。若后续完成改名时删掉该兜底,此测试仍会通过(../core/client.js 被 mock,initialize() 无害地 resolve),而仍在使用废弃选项的外部调用方会在那些"会话建立前不应初始化"的启动路径上悄悄执行完整的聊天初始化。请捕获被 mock 的 client 实例并断言其 initialize 未被调用;最好再补一个同时传入 { skipLlmInitialization: false, skipGeminiInitialization: true } 的用例,钉住"显式新标志优先"。加强后的测试必须在删除 ?? options?.skipGeminiInitialization 兜底时变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
|
|
||
| // Limit Gemini messages to a very high number of lines to mitigate performance | ||
| // issues in the worst case if we somehow get an enormous response from Gemini. | ||
| // Limit LLM messages to a very high number of lines to mitigate performance |
There was a problem hiding this comment.
[Suggestion] This diff rewords the comment from 'Limit Gemini messages' to 'Limit LLM messages' and renames the prop availableTerminalHeightGemini → availableTerminalHeightLlm, but leaves the constant MAX_GEMINI_MESSAGE_LINES unchanged, so the comment and the identifier now contradict each other. The constant is generic (it caps any model response, not a Gemini-provider-specific surface), so a reader following the new comment or grepping for the LLM message-line cap misses it, and the leftover GEMINI name falsely implies provider-specific behavior — the exact confusion this rename exists to remove. Rename MAX_GEMINI_MESSAGE_LINES → MAX_LLM_MESSAGE_LINES at its declaration just below and at both use sites.
const MAX_LLM_MESSAGE_LINES = 65536;中文说明
本次改动把注释从 'Limit Gemini messages' 改写为 'Limit LLM messages',并把 prop availableTerminalHeightGemini 改名为 availableTerminalHeightLlm,却保留了常量 MAX_GEMINI_MESSAGE_LINES 未改名,导致注释与标识符相互矛盾。该常量是通用的(它限制任意模型响应的行数,并非 Gemini provider 特有的边界),因此顺着新注释阅读、或按 "LLM message 行数上限" 去 grep 的读者会找不到它,而残留的 GEMINI 命名会错误地暗示这是 provider 特有的行为——正是本次改名要消除的混淆。请把紧随其下的声明处以及两处使用点的 MAX_GEMINI_MESSAGE_LINES 改名为 MAX_LLM_MESSAGE_LINES。
— qwen3.8-max via Qwen Code /review (v0.22.2)
|
|
||
| if (added.length > 0) { | ||
| const gemini = config.getGeminiClient(); | ||
| const gemini = config.getLlmClient(); |
There was a problem hiding this comment.
[Suggestion] The sweep renamed the generic client accessor but left this local variable named gemini while it now holds the generic LlmClient from config.getLlmClient(). That re-conflates the generic client with the Gemini provider — the exact confusion this rename eliminates — and, since getGeminiClient survives as a deprecated alias, also masks that the file is fully migrated. The same residue appears at packages/core/src/services/toolUseSummary.ts (debug log 'No gemini client available — skipping' beside the renamed getLlmClient() guard), packages/cli/src/nonInteractive/session.ts (debug 'gemini client is not ready'), and test titles such as 'rejects when the gemini client is not initialized'. Rename the local to llmClient (updating the if guard and the addDirectoryContext() call just below), and reword the remaining generic debug strings/test titles to 'LLM client'.
中文说明
本次批量改名重命名了通用的 client 访问器,却把这个局部变量仍命名为 gemini,而它现在持有的是来自 config.getLlmClient() 的通用 LlmClient。这又把通用 client 与 Gemini provider 混为一谈——正是本次改名要消除的混淆——而且由于 getGeminiClient 作为废弃别名仍然存在,也掩盖了该文件其实已完整迁移的事实。同样的残留还出现在 packages/core/src/services/toolUseSummary.ts(重命名后的 getLlmClient() 守卫旁的调试日志 'No gemini client available — skipping')、packages/cli/src/nonInteractive/session.ts(调试信息 'gemini client is not ready'),以及诸如 'rejects when the gemini client is not initialized' 的测试标题。请把该局部变量改名为 llmClient(同时更新紧随其下的 if 守卫和 addDirectoryContext() 调用),并把其余描述通用 client 的调试字符串/测试标题改为 'LLM client'。
— qwen3.8-max via Qwen Code /review (v0.22.2)
|
🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind Autofix round summary — PR #10124 (same-run verification repair)The previous commit The rejection and its diagnosisThe gate's
Two root causes, both reproduced and both fixed:
Why it was not pushed: Note: the base has since been auto-updated; the verdict below predates that update, and the next round's re-measurement may charge the round. tests failed in packages/cli 中文说明🤖 AutoFix 更新了一个过期的 base —— 修复未通过验证,但本 PR 落后于 验证门的拒绝原因与日志证据见上方英文部分(gate-rejection 不翻译)。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/33095705793 🧠 Handled by Qwen Code · model/模型 |
|
Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with |
qqqys
left a comment
There was a problem hiding this comment.
Approved after re-checking the approval gate at head 7ac8e060:
- Maintainer approval at current head: present.
- CI: all test/lint/build lanes green at this head (the only pending check is the automated review lane itself; platform lanes skipped as usual).
- Critical sweep on my side: verified the net diff (238 files) against the last upstream sync point — a mechanical
Gemini*→Llm*rename sweep plus@deprecatedcompatibility aliases. Hand-checked every structurally changed file (turn.ts,config.ts,AppContainer.tsx, the renamedllm-chat.ts/llm-content-generatorblobs, and the test updates): renames and compat aliases only, no behavioral changes. Enum values are unchanged, so runtime event shapes are unaffected, and the deprecated alias surface (old class names and legacy module paths) is pinned by the new identity tests inindex.test.ts. TheskipLlmInitialization ?? skipGeminiInitializationfallback inconfig.tspreserves the old option with correct precedence.
No Critical issues found.
Resolve conflicts against the Gemini->Llm identifier rename (QwenLM#10124) and utils/ leaf-layer move (QwenLM#9778): keep the attempt-state centralization and adopt the new Llm* names/paths (llm-chat.js, LlmEventType, ServerLlmStreamEvent, LlmChat) throughout, including PR-authored tests. forkedAgent changes follow the file move to agents/forkedAgent.ts.
|
Released in v0.22.3. |
What this PR does
This is the second, stacked part of the
Geminifork-residue cleanup from #4063. It renames the generic core LLM client, chat, event, content-generator, stream-hook, protocol-converter, and CLI entry-point identifiers toLlm, and updates their repository consumers together.Published core API names remain available as deprecated aliases until a future major release. The existing chat and content-generator deep-import paths are also retained as deprecated re-exports. Names that describe the real Gemini provider, API, or extension format, plus persisted and wire-format string discriminants, are intentionally unchanged.
This PR is based on #9900 and should be reviewed and merged after it.
Why it's needed
These generic components still carry names inherited from Gemini CLI even though they serve every supported provider. Moving the coupled core layer atomically gives contributors one consistent
Llmvocabulary without forcing downstream package consumers to migrate immediately.Reviewer Test Plan
How to verify
Llm*names and deprecated runtime-identicalGemini*aliases, and that the legacy chat and content-generator deep-import paths resolve.skipLlmInitializationtakes precedence when both initialization options are supplied, whileskipGeminiInitializationandgetGeminiClient()continue to work.Commands run locally:
All checks passed: full build and bundle through
npm ciprepare, repository lint, repository typecheck, and 2,821 focused tests (1 skipped).Evidence (Before & After)
N/A — naming and API-compatibility refactor only; no user-visible UI behavior changed.
Tested on
Environment (optional)
Node.js 22 workspace install and local package tests.
Risk & Scope
@google/genaitype system remain unchanged; Windows and Linux were not tested locally.Gemini*aliases and the old chat and content-generator module paths remain until a future major release.Linked Issues
Refs #4063
Depends on #9900
中文说明
这个 PR 做了什么
这是 #4063 中清理
Geminifork 遗留命名的第二个 stacked PR。它将通用 core LLM client、chat、event、content generator、stream hook、协议转换器和 CLI 入口相关标识统一改为Llm,并原子更新仓库内调用方。已发布的 core API 旧名称继续以废弃别名保留到后续 major release,原 chat 和 content-generator 深层导入路径也保留为废弃 re-export。真正表示 Gemini provider、API 或 extension format 的名称,以及持久化和线协议字符串 discriminant,均有意保持不变。
本 PR 基于 #9900,应在 #9900 之后 review 和合并。
为什么需要
这些通用组件仍沿用 Gemini CLI 上游名称,但它们实际服务于所有支持的 provider。将强耦合的 core 层一次性迁移,可以让贡献者统一使用
Llm语义,同时避免要求下游 package consumer 立即迁移。Reviewer Test Plan
如何验证
Llm*名称和运行时指向相同实现的废弃Gemini*别名,并确认旧 chat 和 content-generator 深层导入路径可解析。skipLlmInitialization优先,同时skipGeminiInitialization和getGeminiClient()仍可用。本地执行了上方列出的
npm ci、lint、typecheck 和定向测试命令。npm ciprepare 阶段的全量 build 与 bundle、仓库 lint、仓库 typecheck 均通过;2,821 个定向测试通过,1 个跳过。Evidence(Before & After)
N/A——仅命名和 API 兼容性重构,没有用户可见 UI 行为变化。
测试环境
macOS 已测试;Windows 和 Linux 未在本地测试。环境为 Node.js 22 workspace install 和本地 package tests。
风险与范围
@google/genai类型系统保持不变;Windows 与 Linux 未在本地测试。Gemini*别名以及旧 chat 和 content-generator module path 将保留到后续 major release。关联 Issue
Refs #4063
Depends on #9900