Codecov Report

❌ Patch coverage is 56.00000% with 11 lines in your changes missing coverage. Please review.

📢 Thoughts on this report? Let us know!

Code Review：fix(search): make Cmd+F work when chat is in a folder or on mobile (#1692)

变更概述

本 PR 包含两个独立部分：① 修复 Cmd+F 快捷键在 hasTabs / isMobile 模式下失效的问题，通过在 ChatLayout 中渲染一个隐藏的 ConversationTitleMinimap（hideTrigger=true）保持快捷键监听器存活；② 新增 PWA 支持（service worker、manifest、图标、registerPwa.ts 服务），并将 public/ 目录接入 electron-vite 和 web 构建的 publicDir。

方案评估

结论：✅ 方案合理

Cmd+F 修复思路正确——通过互斥条件（!isMobile && !hasTabs vs hasTabs || isMobile）确保同一时刻只有一个 minimap 实例存在，保证键盘监听器始终在 DOM 中，且不引入重复事件注册。hideTrigger 作为 prop 让触发按钮可选而不影响面板逻辑，是干净的单一职责扩展。PWA 部分实现标准（network-first 导航 + stale-while-revalidate 静态资源），并通过 Electron 环境检测避免在桌面端注册 SW。

唯一的设计层面疑问：Cmd+F 修复与 PWA 支持是两个完全独立的功能，合并在同一 PR 增加了回滚粒度，建议下次拆分提交。

问题清单

🟡 MEDIUM — useMinimapPanel.ts 新增定位逻辑覆盖率不足（42.85%）

文件：src/renderer/pages/conversation/components/ConversationTitleMinimap/useMinimapPanel.ts，第 152–179 行

问题代码：

const updatePanelLayout = useCallback((height = PANEL_HEIGHT) => {
  if (typeof window === 'undefined') return;
  const width = getPanelWidth();
  let left: number;
  let top: number;
  const isNarrow = window.innerWidth < 768;
  const header = document.querySelector('.chat-layout-header');
  if (isNarrow) {
    left = PANEL_MARGIN;
    top = header ? header.getBoundingClientRect().bottom + PANEL_OFFSET : 60;
  } else if (header) {
    const headerRect = header.getBoundingClientRect();
    left = headerRect.left + Math.round((headerRect.width - width) / 2);
    top = headerRect.bottom + PANEL_OFFSET;
  } else {
    left = Math.round((window.innerWidth - width) / 2);
    top = 60;
  }
  // ...
}, []);

问题说明：Codecov 报告此文件 patch 覆盖率仅 42.85%（5 missing + 3 partials），而 index.tsx patch 覆盖率为 57.14%。updatePanelLayout 有三条分支（narrow / header-present / fallback），均未被单元测试覆盖。若此函数计算出错（如 headerRect 边界判断），将导致面板定位偏移，且没有测试能够捕获回归。

修复建议：在 minimapPanelWidth.dom.test.ts 或新测试文件中补充 updatePanelLayout 的测试，利用 jsdom 的 getBoundingClientRect mock：

it('positions panel centered below header on wide viewport', () => {
  const mockHeader = document.createElement('div');
  mockHeader.className = 'chat-layout-header';
  mockHeader.getBoundingClientRect = () => ({
    left: 100, width: 800, bottom: 60,
    top: 0, right: 900, height: 60
  } as DOMRect);
  document.body.appendChild(mockHeader);
  // invoke updatePanelLayout via openSearchPanel and inspect panelPos
  // ...
  document.body.removeChild(mockHeader);
});

🔵 LOW — sw.js 硬编码缓存版本，部署新版本可能出现旧资源残留

文件：public/sw.js，第 1 行

问题代码：

const CACHE_NAME = 'aionui-webui-v1';

问题说明：缓存名硬编码为 v1。当后续部署更新 JS/CSS 资源时，若 CACHE_NAME 未同步更新，旧版缓存不会被清除（activate 阶段只删除非 CACHE_NAME 的旧 key），stale-while-revalidate 策略可能让用户继续命中旧缓存，直到后台 revalidate 完成。推荐在 CI 构建时注入构建版本号，或改用带内容哈希的文件名来管理缓存失效。

修复建议：在 vite.renderer.config.ts 中用 define 注入构建版本号：

// sw.js
const CACHE_NAME = `aionui-webui-${__BUILD_VERSION__}`;

汇总

结论

⚠️ 有条件批准 — 核心修复逻辑正确，PWA 实现规范，主要问题是新增的面板定位逻辑未被单元测试覆盖（patch coverage 42.85%），建议补充测试后合并。

本报告由本地 pr-review skill 生成，包含完整项目上下文，无截断限制。

Code Review：fix(search): make Cmd+F work when chat is in a folder or on mobile (#1692)

变更概述

本 PR 分为两部分：① Cmd+F 修复：通过新增 hideTrigger prop，在 hasTabs || isMobile 时在 ChatLayout 中渲染一个隐藏触发器的 ConversationTitleMinimap，保持快捷键 listener 始终存活；面板定位逻辑改为以 header 为中心，并针对窄视口（<768px）适配宽度。② PWA 支持：新增 sw.js（service worker）、manifest.webmanifest、PWA 图标、registerPwa.ts 服务，以及对构建配置（electron.vite.config.ts、vite.renderer.config.ts）和入口 HTML 的配套修改。

方案评估

结论：⚠️ 方案有缺陷

Cmd+F 修复方案本身逻辑清晰——将 shortcut listener 的生命周期与 trigger 按钮解耦，不影响现有 ChatTitleEditor 的渲染路径，两实例互斥，无副作用。PWA 实现也经过正确的平台守卫（isElectronDesktop() 检查、secure context 检查）。缺陷在于：这是两个独立功能被合并到一个 PR 中，且 PR 描述仅声称修改了 6 个文件，而实际 diff 包含 20 个文件（含完整 PWA 特性），造成 review 盲区——service worker 会影响所有 WebUI 用户的缓存行为，此类变更未在 PR 描述中体现是一个显著的信息缺失。

问题清单

🟡 MEDIUM — PR 描述与实际变更范围严重不符

文件：PR 描述

问题说明：PR 标题、描述、"Files changed" 一节均仅提及 6 个 minimap 相关文件，而实际 diff 包含 14 个 PWA 相关文件：sw.js、manifest.webmanifest、3 个 PWA 图标、registerPwa.ts、构建配置修改（electron.vite.config.ts 新增 publicDir、vite.renderer.config.ts）、src/renderer/index.html 新增 manifest/apple-touch-icon 标签、main.tsx 调用 registerPwa()、删除 public/index.html。Service worker 会在 WebUI 用户浏览器中拦截所有 GET 请求并影响缓存，属于高影响变更，review 方应明确知晓其存在。

修复建议：更新 PR 描述，增加 PWA 相关的 Summary / Files changed 说明，包括 SW 策略（navigate → networkFirst，assets → staleWhileRevalidate）、manifest 信息、以及对 Electron 运行时的豁免逻辑。

🟡 MEDIUM — 缺少 hideTrigger 路径的行为测试

文件：tests/ 目录

问题说明：本 PR 的核心修复是：当 hideTrigger=true 时，ConversationTitleMinimap 不渲染触发器按钮，但 useMinimapPanel 内的 Cmd+F keyboard listener 仍注册。目前没有任何测试覆盖这一场景：即验证 hideTrigger=true 的组件实例确实仍能响应 Cmd+F 事件（通过 openSearchPanel）。Codecov 也报告 useMinimapPanel.ts patch 覆盖率仅 42.85%，index.tsx 57.14%，均低于项目 80% 标准。

修复建议：在 tests/unit/ 中为 useMinimapPanel 增加覆盖 shortcut listener 的测试，例如：

it('registers Cmd+F listener when hideTrigger=true (no trigger DOM element)', () => {
  // render <ConversationTitleMinimap conversationId="c1" hideTrigger />
  // fire Cmd+F keyboard event
  // assert openSearchPanel was called (panel becomes visible)
});

🔵 LOW — sw.js 未使用 TypeScript / 缺少类型检查覆盖

文件：public/sw.js

问题说明：sw.js 为纯 JavaScript，oxlint 和 tsc 均不检查此文件。项目其他新增文件均有 TypeScript 类型保护，但 service worker 若出现运行时错误（如 API 变化），构建阶段无法发现。

修复建议：可在文件顶部添加 JSDoc 类型注释（如 /* @ts-check */ + /// <reference lib="WebWorker" />），使 TypeScript 以 JSDoc 模式检查此文件，或在 tsconfig.json 中将 public/sw.js 纳入 checkJs 范围。

汇总

结论

⚠️ 有条件批准 — Cmd+F 修复逻辑正确，PWA 实现质量良好，但 PR 描述与实际变更范围严重不符（14 个 PWA 文件未提及），且核心修复路径（hideTrigger）缺乏行为测试。建议在更新 PR 描述并补充测试后合并。

本报告由本地 pr-review skill 生成，包含完整项目上下文，无截断限制。

PR Fix 验证报告

原始 PR: #1692
修复方式: 直接推送到 fix/cmd-f-with-tabs（fork with maintainer access）

总结： ✅ 已修复 2 个 | ❌ 未能修复 0 个

🔵 LOW 级别问题（public/sw.js 无类型检查覆盖）已跳过，不阻塞合并。