Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughSummary by CodeRabbit
Walkthrough本PR新增“公开主页”功能(草稿/批准/回填工作流)、AI头像生成、公开站点API与前端页面,扩展了附件文本提取、agent 推理开关(thinking_enabled)、多处后端模型/迁移、前端组件/样式和大量测试与测试基础设施。 Changes
Sequence Diagram(s)sequenceDiagram
participant Client as 浏览器 (PublicHome UI)
participant WebAPI as Web 前端服务 (public-home-service)
participant API as 后端 API (/api/v1)
participant DB as 数据库 (PublicHomeState, Notebook, UserPortrait)
participant LLM as 外部 LLM/Image API
participant Storage as 对象存储
Client->>WebAPI: 点击生成草稿 (POST /public-home/generate)
WebAPI->>API: 调用 /api/v1/public-home/generate
API->>DB: 读取用户公开笔记与肖像数据
API->>LLM: 请求产生草稿 JSON (prompt 含笔记文本、肖像)
LLM-->>API: 返回草稿 JSON
API->>DB: 写入 PublicHomeState.draft_profile_json、draft_generated_at
API->>Storage: 若配置 image_gen_api_key => 调用 image-gen 生成头像并上传
Storage-->>API: 返回 avatar_url(写入 UserPortrait.avatar_url)
API-->>WebAPI: 返回已生成的 admin 状态
WebAPI-->>Client: 更新 UI(草稿存在,等待批准)
(色块在 Mermaid 中使用默认渲染;请求/返回序列展示核心交互) Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Actionable comments posted: 2
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
apps/web/src/features/notebook/notebooks-view.tsx (1)
87-112:⚠️ Potential issue | 🟠 Major给视图切换按钮补上可访问名称。
这两个按钮现在只有图标和
data-testid,读屏时会变成无名称按钮,也暴露不出当前选中态。这里至少要补aria-label和aria-pressed。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/features/notebook/notebooks-view.tsx` around lines 87 - 112, The two view-toggle buttons (using LayoutGrid and List icons, with onClick calling setView and testids "notebooks-grid-toggle" / "notebooks-list-toggle") lack accessible names and state; add aria-labels (e.g. "Grid view" and "List view" or localized equivalents) and set aria-pressed based on the view prop (aria-pressed={view === "grid"} and aria-pressed={view === "list"}) so screen readers announce the button purpose and current selection.apps/web/tests/unit/features/portrait/portrait-cards.test.tsx (1)
1-5:⚠️ Potential issue | 🟠 Major测试文件位置不符合仓库约定(需与被测组件同目录)。
当前测试位于
apps/web/tests/unit/features/portrait/,未与portrait-cards组件同目录放置。建议迁移到组件目录并改为相对导入,避免后续路径漂移和维护成本增加。📦 建议调整
- import { ThoughtStream } from "@/features/portrait/portrait-cards"; + import { ThoughtStream } from "./portrait-cards";并将文件移动到:
apps/web/src/features/portrait/portrait-cards.test.tsxAs per coding guidelines:
apps/web/**/*.test.tsx: Write frontend tests as*.test.tsxfiles in the same directory as the feature/component being tested.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/tests/unit/features/portrait/portrait-cards.test.tsx` around lines 1 - 5, Move the test file so it resides alongside the tested component (the portrait-cards component) and update all absolute aliased imports to relative imports; specifically replace the "@/features/portrait/portrait-cards" import of ThoughtStream with a relative import from the component file (e.g., "./portrait-cards") and change the "@/services/portrait-service" UserPortrait import to the correct relative path from the new test location, ensuring the test filename remains portrait-cards.test.tsx and lives in the same directory as the component.apps/api/app/domains/config/router.py (1)
291-326:⚠️ Potential issue | 🔴 Critical存在无法执行的死代码。
test_utility_llm_connection函数在第 288-289 行已经有except块并返回,因此第 291-326 行的代码永远不会被执行。这段代码看起来是从test_llm_connection函数意外复制过来的。🐛 建议删除死代码
except Exception as exc: return success(TestLlmResult(ok=False, model=utility_model, message=str(exc)[:200])) - - provider = rows.get("llm_provider") or app_settings.llm_provider or "openai" - api_key = rows.get("openai_api_key") or app_settings.openai_api_key - base_url = rows.get("openai_base_url") or app_settings.openai_base_url or None - model = rows.get("llm_model") or app_settings.llm_model - - if not api_key: - return success(TestLlmResult(ok=False, model=model, message="未设置 API Key")) - - try: - if provider == "litellm": - import litellm - call_kw: dict = dict( - model=model, - messages=[{"role": "user", "content": "Hi"}], - max_tokens=100, - api_key=api_key, - drop_params=True, - ) - if model.startswith("gemini/"): - call_kw["custom_llm_provider"] = "gemini" - if base_url: - call_kw["api_base"] = base_url - resp = await litellm.acompletion(**call_kw) - reply = (resp.choices[0].message.content or "").strip() - else: - from openai import AsyncOpenAI - client = AsyncOpenAI(api_key=api_key, base_url=base_url, timeout=15.0) - resp = await client.chat.completions.create( - model=model, - messages=[{"role": "user", "content": "Hi"}], - max_tokens=100, - ) - reply = (resp.choices[0].message.content or "").strip() - return success(TestLlmResult(ok=True, model=model, message=reply or "OK")) - except Exception as exc: - return success(TestLlmResult(ok=False, model=model, message=str(exc)[:200]))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/app/domains/config/router.py` around lines 291 - 326, The block starting with "provider = rows.get("llm_provider") ..." inside test_utility_llm_connection is dead/duplicated code copied from test_llm_connection and never executed because an earlier except already returns; remove this entire duplicated LLM-call block (the litellm/openai call-path and reply handling) from test_utility_llm_connection and ensure only the intended logic and return remain (also remove any now-unused imports or variables referenced only by that block).apps/web/src/features/notebook/notebook-header.tsx (1)
549-552:⚠️ Potential issue | 🟡 Minor移动端与桌面端
onBlur行为不一致移动端重命名输入框在
onBlur时调用commitRename()保存更改(第 456-458 行),而桌面端在onBlur时重置标题并关闭重命名状态(第 549-551 行)。这会导致用户在不同设备上体验不一致:移动端失焦保存,桌面端失焦丢弃。建议统一行为,推荐采用移动端的保存逻辑。
🔧 建议统一为保存行为
onBlur={() => { - setTitle(externalTitle); - setIsRenaming(false); + void commitRename(); }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/features/notebook/notebook-header.tsx` around lines 549 - 552, The onBlur handler for the desktop rename input currently resets the title (setTitle(externalTitle)) and cancels renaming (setIsRenaming(false)), causing inconsistent behavior vs mobile where onBlur calls commitRename(); update the desktop onBlur to call commitRename() and then setIsRenaming(false) (or let commitRename clear renaming state) so both platforms save edits on blur; locate the handlers referencing onBlur, setTitle, setIsRenaming, externalTitle and commitRename in notebook-header.tsx and replace the reset logic with a call to commitRename() (ensuring commitRename runs synchronously/asynchronously as appropriate and still clears renaming state).apps/api/app/agents/core/react_agent.py (1)
50-63:⚠️ Potential issue | 🟠 Major
thinking_enabled在 multi-agent 分支会被静默忽略。这个参数已经进入
run_agent()的公共签名,但当前只有 single-agent 路径会把它传到AgentEngine。一旦命中深度研究并走MULTI_AGENT_GRAPH,调用方设置的开关就不再生效,同一个 API 会根据路由路径表现不一致。建议把该值继续透传到 multi-agent 的 state/config,或者在 graph 还不支持前显式回退到 single-agent。Also applies to: 98-115, 122-135, 340-346
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/app/agents/core/react_agent.py` around lines 50 - 63, The public run_agent signature accepts thinking_enabled but the MULTI_AGENT_GRAPH path silently ignores it; update the multi-agent branch so that thinking_enabled is propagated into the multi-agent state/config (or explicitly fall back to the single-agent AgentEngine when graph doesn't support it). Locate run_agent and where MULTI_AGENT_GRAPH is selected (and where AgentEngine is constructed) and add passing of the thinking_enabled flag into the multi-agent graph builder/state or selection config so behavior matches the single-agent path; if the graph implementation cannot accept thinking_enabled yet, implement an explicit fallback to the single-agent flow when thinking_enabled is set.
🟠 Major comments (21)
scripts/generate_thesis_pdfs.py-253-259 (1)
253-259:⚠️ Potential issue | 🟠 Major元信息识别条件过于脆弱,正文可能被错误套用
meta样式。Line 253 用
len(story) <= 4推断“仍在元信息区”,这和内容结构耦合太强:例如文档开头先有一个#标题后,紧随其后的首段正文仍可能被当成meta(居中、小号)渲染。可选修复示例
def markdown_to_story(text: str, doc_title: str, styles) -> list: story: list = [] @@ paragraph_buffer: list[str] = [] list_buffer: list[str] = [] list_type: str | None = None + in_meta_block = True @@ if stripped.startswith("# "): + in_meta_block = False flush_paragraph(story, paragraph_buffer, styles["body"]) @@ if stripped.startswith("## "): + in_meta_block = False flush_paragraph(story, paragraph_buffer, styles["body"]) @@ if stripped.startswith("### "): + in_meta_block = False flush_paragraph(story, paragraph_buffer, styles["body"]) @@ if bullet_match: + in_meta_block = False flush_paragraph(story, paragraph_buffer, styles["body"]) @@ if ordered_match: + in_meta_block = False flush_paragraph(story, paragraph_buffer, styles["body"]) @@ - if not story or len(story) <= 4: + if in_meta_block: flush_paragraph(story, paragraph_buffer, styles["body"]) if list_type: flush_list(story, list_buffer, list_type, styles) list_type = None story.append(Paragraph(escape_text(stripped), styles["meta"])) continue + in_meta_block = False paragraph_buffer.append(stripped)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/generate_thesis_pdfs.py` around lines 253 - 259, The current metadata detection uses "len(story) <= 4" which is too brittle and causes real first-paragraph content to be rendered with styles["meta"]; replace this ad-hoc check with a proper metadata-detection heuristic (e.g., add an in_meta boolean flag initialized True and a helper is_meta_line(str_line) that returns True for short lines matching "Key: Value", dates, emails, single-word headings or very short centered lines and False for normal sentence-like paragraphs with multiple words/punctuation); in the block around flush_paragraph/flush_list and where Paragraph(escape_text(stripped), styles["meta"]) is appended, use in_meta and is_meta_line(stripped) to decide meta vs body, and flip in_meta = False when a non-meta paragraph is encountered so subsequent content uses body styles (reference symbols: story, stripped, flush_paragraph, flush_list, list_type, styles["meta"]).apps/web/tests/utils/render-with-providers.tsx-28-28 (1)
28-28: 🛠️ Refactor suggestion | 🟠 Major避免在 React 组件内进行模块级 i18n 状态写入,防止跨测试用例污染
Line 28 在
TestProviders组件渲染时调用setI18nMessages,这个函数直接修改模块顶部的全局变量_msgs,导致两个问题:
- 多次调用与组件重渲染:每次组件渲染都会执行该副作用(违反 React 最佳实践)
- 跨用例污染风险:多个测试顺序运行时,后续测试的
_msgs会被覆盖,且无 reset 机制,可能导致测试间污染建议:
- 将
setI18nMessages调用前移到renderWithProviders函数体(在render调用前执行一次)- 在
lib/i18n.ts中导出resetI18nMessages函数供测试清理使用- 在
tests/setup/vitest.setup.ts的afterEach中添加 i18n reset 调用♻️ 建议修改
function TestProviders({ children, locale, timeZone, defaultTheme, messages, queryClient, }: PropsWithChildren<Required<Omit<ExtendedRenderOptions, keyof RenderOptions>>>) { - setI18nMessages(messages as Record<string, unknown>); - return ( <NextIntlClientProvider locale={locale} timeZone={timeZone} messages={messages}export function renderWithProviders( ui: ReactElement, { locale = "zh", timeZone = "Asia/Shanghai", defaultTheme = "dark", messages = {}, queryClient = createTestQueryClient(), ...renderOptions }: ExtendedRenderOptions = {}, ) { + setI18nMessages(messages as Record<string, unknown>); + return render(ui, { wrapper: ({ children }) => (同时在
lib/i18n.ts中添加:+export function resetI18nMessages() { + _msgs = {}; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/tests/utils/render-with-providers.tsx` at line 28, TestProviders currently calls setI18nMessages during render, causing module-level i18n state writes and test pollution; move the setI18nMessages call out of the TestProviders component into the renderWithProviders function so it runs once before calling render, add a resetI18nMessages export in lib/i18n.ts to clear the module-level _msgs, and call resetI18nMessages in tests/setup/vitest.setup.ts afterEach to ensure test isolation; locate references by the TestProviders component name and the setI18nMessages symbol and update renderWithProviders and the global test teardown to use resetI18nMessages.apps/web/tests/unit/app/(workspace)/app/notebooks/[id]/loading.test.tsx-1-13 (1)
1-13:⚠️ Potential issue | 🟠 Major测试文件位置不符合仓库约定。
该测试应与被测组件放在同目录下,而不是
apps/web/tests/unit/...。建议移动到组件旁(例如apps/web/src/app/(workspace)/app/notebooks/[id]/loading.test.tsx),以保持维护与定位一致性。As per coding guidelines,
apps/web/**/*.test.tsx: Write frontend tests as*.test.tsxfiles in the same directory as the feature/component being tested.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/tests/unit/app/`(workspace)/app/notebooks/[id]/loading.test.tsx around lines 1 - 13, The test file testing the NotebookLoading component is placed in the centralized tests folder instead of next to the component; move loading.test.tsx into the same directory as the component that exports NotebookLoading, keep the filename as loading.test.tsx, update the import of NotebookLoading to the correct relative path (e.g., import NotebookLoading from "./loading"), and ensure the test runner pattern still picks up tests in that location.apps/web/src/features/knowledge/knowledge-view.tsx-416-437 (1)
416-437:⚠️ Potential issue | 🟠 Major给网格/列表切换按钮补上可访问名称。
这里的两个切换按钮同样只有 SVG,没有可读名称;
data-testid对辅助技术没有帮助,当前选中态也没有暴露出来。建议补aria-label和aria-pressed。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/features/knowledge/knowledge-view.tsx` around lines 416 - 437, The two toggle buttons ("knowledge-grid-toggle" and "knowledge-list-toggle") lack accessible names and state; update the button elements used with setViewMode and viewMode to include an appropriate aria-label (e.g., "Show as grid" / "Show as list") and an aria-pressed attribute that reflects the current state (aria-pressed={viewMode === "grid"} for the grid button and aria-pressed={viewMode === "list"} for the list button) so assistive technologies can announce purpose and selection.apps/web/src/features/public-home/public-home-styles.ts-37-66 (1)
37-66:⚠️ Potential issue | 🟠 Major持续动画需要
prefers-reduced-motion降级。公开页这次加了多组无限动画,但没有尊重系统的 reduced motion 偏好。对这类 public landing/page,这已经是实打实的可访问性问题,建议统一在一个 media query 里把持续动画关掉。
♿ 建议补一个统一降级块
+ `@media` (prefers-reduced-motion: reduce) { + .lp-orb-1, + .lp-orb-2, + .lp-orb-3, + .lp-avatar-float, + .lp-ai-badge, + .lp-cursor, + .lp-spin { + animation: none !important; + } + }Also applies to: 211-218, 274-282, 362-367, 519-520
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/features/public-home/public-home-styles.ts` around lines 37 - 66, The animated background orbs (.lp-orb, .lp-orb-1, .lp-orb-2, .lp-orb-3) use infinite keyframes (lp-orb-drift-1 and lp-orb-drift-2) and must respect prefers-reduced-motion; add a single media query `@media` (prefers-reduced-motion: reduce) that sets animation: none (or animation-duration: 0s / animation: none important) and removes ongoing transform animations for these selectors so the orbs stop animating for users who opt out; apply the same rule to any other animated selectors referenced (e.g., other .lp-orb-* usages mentioned) to ensure consistent reduced-motion fallback.apps/web/tests/unit/features/notebook/notebooks-view.test.tsx-1-96 (1)
1-96: 🛠️ Refactor suggestion | 🟠 Major把这个测试移回组件同目录。
这个新用例现在放在
apps/web/tests/unit/...,后续查找组件和测试会继续分叉,也和仓库现有前端测试布局不一致。 As per coding guidelines,apps/web/**/*.test.tsx: Write frontend tests as*.test.tsxfiles in the same directory as the feature/component being tested.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/tests/unit/features/notebook/notebooks-view.test.tsx` around lines 1 - 96, Move the test file from apps/web/tests/unit/... into the same directory as the NotebooksView component (where NotebooksView is defined) so it lives beside the component, update any relative imports/mocks to use local paths, and ensure the test filename follows *.test.tsx naming convention; specifically adjust imports referencing "@/features/notebook/notebooks-view", "@/hooks/use-media-query", "@/services/notebook-service", and any fixture imports (e.g., buildNotebook) to correct relative paths after relocating the test and run tests to verify mocks (useMediaQuery, NotebookCard/NotebookListRow) still resolve.apps/web/tests/unit/features/knowledge/knowledge-view.test.tsx-1-92 (1)
1-92: 🛠️ Refactor suggestion | 🟠 Major把这个测试移回组件同目录。
这个新用例现在放在
apps/web/tests/unit/...,后续查找组件和测试会继续分叉,也和仓库现有前端测试布局不一致。 As per coding guidelines,apps/web/**/*.test.tsx: Write frontend tests as*.test.tsxfiles in the same directory as the feature/component being tested.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/tests/unit/features/knowledge/knowledge-view.test.tsx` around lines 1 - 92, The test file knowledge-view.test.tsx was placed under apps/web/tests/unit/... instead of beside the component; move this test into the same directory as the KnowledgeView component, update its imports to use correct relative paths (keeping mocks for IntersectionObserver, next-intl, framer-motion, react-query and the mocked components), and ensure the test filename remains knowledge-view.test.tsx so Vitest picks it up with the project's frontend test convention; run the test suite to confirm everything still passes after moving and adjusting any import paths.apps/web/tsconfig.json-24-24 (1)
24-24:⚠️ Potential issue | 🟠 Major
@test/*别名与测试同目录约定冲突。新增
@test/* -> ./tests/*会固化集中式测试目录,不利于执行“测试与被测组件同目录”的仓库规范。建议移除该别名,并在测试迁移到组件目录后使用相对路径或@/*指向源码。As per coding guidelines:
apps/web/**/*.test.tsx: Write frontend tests as*.test.tsxfiles in the same directory as the feature/component being tested.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/tsconfig.json` at line 24, Remove the centralized test alias by deleting the "@test/*": ["./tests/*"] entry from tsconfig.json; update any imports that used "@test/..." to use relative paths or the existing "@/..." source alias, and run the test suite/TypeScript build to catch and fix remaining import errors (search for the string "@test/" to locate usages).apps/api/app/domains/notebook/router.py-141-141 (1)
141-141:⚠️ Potential issue | 🟠 Major考虑将
refresh_public_home_draft改为后台任务,避免阻塞用户请求。
refresh_public_home_draft触发的generate_public_home_draft涉及 LLM 推理调用(chat())和图像生成 API(SiliconFlow avatar 生成)。在路由处理函数中同步等待这些耗时操作会阻塞 HTTP 响应,延长用户请求的响应时间。建议将其改为 Celery 后台任务处理,保持请求响应的即时性。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/app/domains/notebook/router.py` at line 141, 当前路由在处理函数中直接 await refresh_public_home_draft(...),但其内部的 generate_public_home_draft 会调用耗时的 LLM 推理 chat() 和 SiliconFlow avatar 生成,导致阻塞请求;请把 refresh_public_home_draft 移成一个 Celery 后台任务(例如新增一个 celery task 包装 generate_public_home_draft 或 refresh_public_home_draft),在 router.py 中替换 await refresh_public_home_draft(db, current_user.id) 为非阻塞的任务触发(如 refresh_public_home_draft_task.delay(current_user.id) 或 apply_async),并确保在任务中正确重建/管理数据库会话、捕获并记录异常以及传递必要的上下文/参数(user id、任何必要的资源标识),以避免在 HTTP 请求路径上进行 LLM/图像生成阻塞调用。apps/api/app/domains/portrait/router.py-30-33 (1)
30-33:⚠️ Potential issue | 🟠 Major路由层直查数据库且查询“最新头像”不稳定。
这里建议把头像读取逻辑下沉到 service/loader,并在查询中显式按时间倒序 +
limit(1);否则在多版本数据下,scalar_one_or_none()可能报多行异常,且可能与load_latest_portrait版本不一致。As per coding guidelines
apps/api/app/domains/**/router.py: In backenddomains/routers, perform HTTP layer validation and return unifiedApiResponseenvelope - do not execute database queries directlyAlso applies to: 39-43
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/app/domains/portrait/router.py` around lines 30 - 33, The router is directly querying UserPortrait and fetching the "latest" avatar which can be unstable and violate domain rules; move the DB selection into the portrait service/loader (e.g. extend or modify load_latest_portrait) so the router no longer executes queries, and implement the query there using an explicit order_by(UserPortrait.updated_at.desc() or created_at.desc()) with limit(1) to avoid multiple-row errors from scalar_one_or_none; then have the router call load_latest_portrait and return the result wrapped in the unified ApiResponse envelope instead of performing DB access in the router.apps/web/tests/mocks/http-client.ts-11-22 (1)
11-22:⚠️ Potential issue | 🟠 Major
http.url的 mock 实现会泄漏到后续用例Line 22 只清空了调用记录,没有恢复默认实现。只要某个测试覆盖了
http.url.mockImplementation(...),后面的测试就会继承这个实现,导致跨用例污染和偶发失败。🛠 建议修改
import { vi } from "vitest"; +const defaultUrl = (path: string) => path; + export const http = { get: vi.fn(), post: vi.fn(), patch: vi.fn(), put: vi.fn(), delete: vi.fn(), stream: vi.fn(), fetchJson: vi.fn(), - url: vi.fn((path: string) => path), + url: vi.fn(defaultUrl), }; @@ export function resetHttpClientMocks() { http.get.mockReset(); http.post.mockReset(); http.patch.mockReset(); http.put.mockReset(); http.delete.mockReset(); http.stream.mockReset(); http.fetchJson.mockReset(); - http.url.mockClear(); + http.url.mockReset(); + http.url.mockImplementation(defaultUrl); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/tests/mocks/http-client.ts` around lines 11 - 22, The test helper resetHttpClientMocks currently only calls http.url.mockClear(), which leaves any custom mockImplementation set by a test in place and leaks into subsequent tests; update resetHttpClientMocks to fully reset http.url to its original default implementation (the vi.fn((path: string) => path) used in the initial mock) by either calling mockReset() on http.url or explicitly restoring its implementation inside resetHttpClientMocks so that http.url returns path by default and no custom implementations persist between tests.apps/web/messages/en.json-700-714 (1)
700-714:⚠️ Potential issue | 🟠 Major删除重复的
researchTrajectoryTitle键Line 700 的
researchTrajectoryTitle会被 Line 713 的同名键直接覆盖。虽然两个值现在一样,但前一个定义实际上不可达,而且这会持续触发 Biome 的noDuplicateObjectKeys。请删除其中一个,或把其中一个改成真正想暴露的新 key。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/messages/en.json` around lines 700 - 714, Remove the duplicate JSON key "researchTrajectoryTitle" (the earlier definition is shadowed by the later one and triggers noDuplicateObjectKeys). Edit the messages JSON so only one "researchTrajectoryTitle" remains (keep the intended value) or rename one occurrence to the correct new key name you intended to expose; ensure any code referencing the old/renamed key (e.g., localization lookups) is updated to match.apps/web/tests/unit/features/copilot/copilot-panel.test.tsx-1-142 (1)
1-142: 🛠️ Refactor suggestion | 🟠 Major把这个组件测试移到
CopilotPanel同目录这个文件现在放在
apps/web/tests/unit/features/...,但仓库规则要求前端组件测试以*.test.tsx的形式与被测组件同目录存放。请把它移动到apps/web/src/features/copilot/copilot-panel.test.tsx一类的共置位置,避免组件重构时测试继续漂在外面。As per coding guidelines:
apps/web/**/*.test.tsx: Write frontend tests as*.test.tsxfiles in the same directory as the feature/component being tested🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/tests/unit/features/copilot/copilot-panel.test.tsx` around lines 1 - 142, The test file for CopilotPanel is in the wrong location; move the test so it sits alongside the component (e.g., next to CopilotPanel in the same directory) and update any relative imports/paths accordingly so exports like CopilotPanel, mocks (e.g., "@/features/copilot/copilot-panel"), and test utilities still resolve; ensure the filename remains copilot-panel.test.tsx and run tests to confirm imports (useTranslations, ChatInput mock, etc.) still work after relocation.apps/api/tests/unit/test_public_home_service.py-235-237 (1)
235-237:⚠️ Potential issue | 🟠 MajorJSONB 字段修改需要 flag_modified
与集成测试中相同的问题:直接修改
approved_profile_json字典可能不会被 SQLAlchemy 检测到变化。🐛 建议添加 flag_modified
+from sqlalchemy.orm.attributes import flag_modified + raw_state = (await db_session.execute(select(PublicHomeState).where(PublicHomeState.user_id == user.id))).scalar_one() raw_state.approved_profile_json.pop("portrait_snapshot", None) +flag_modified(raw_state, "approved_profile_json") await db_session.flush()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/tests/unit/test_public_home_service.py` around lines 235 - 237, The test mutates the JSONB dict on the PublicHomeState instance (raw_state.approved_profile_json) but SQLAlchemy may not detect in-place dict changes; import and call flag_modified on the instance before flushing (e.g., flag_modified(raw_state, "approved_profile_json")) so the change to approved_profile_json is persisted; locate the modification in test_public_home_service.py where raw_state.approved_profile_json is popped and add the flag_modified call right after that mutation.apps/api/tests/integration/test_public_home_router.py-135-139 (1)
135-139:⚠️ Potential issue | 🟠 MajorJSONB 字段原地修改可能未被 SQLAlchemy 检测到
直接修改
approved_profile_json字典后调用commit(),SQLAlchemy 可能不会检测到 JSONB 字段的变化,因为字典对象引用未改变。这可能导致测试通过但实际数据库未更新。🐛 建议使用 flag_modified 或重新赋值
+from sqlalchemy.orm.attributes import flag_modified + raw_state = ( await db_session.execute(select(PublicHomeState).where(PublicHomeState.user_id == user.id)) ).scalar_one() raw_state.approved_profile_json.pop("portrait_snapshot", None) +flag_modified(raw_state, "approved_profile_json") await db_session.commit()或者通过重新赋值触发变更检测:
raw_state = ( await db_session.execute(select(PublicHomeState).where(PublicHomeState.user_id == user.id)) ).scalar_one() -raw_state.approved_profile_json.pop("portrait_snapshot", None) +updated_json = {**raw_state.approved_profile_json} +updated_json.pop("portrait_snapshot", None) +raw_state.approved_profile_json = updated_json await db_session.commit()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/tests/integration/test_public_home_router.py` around lines 135 - 139, You modified the JSONB in-place on PublicHomeState.approved_profile_json (raw_state) and then called await db_session.commit(), which SQLAlchemy may not detect; instead, either call sqlalchemy.orm.attributes.flag_modified(raw_state, "approved_profile_json") after mutating the dict or reassign the mutated dict back to raw_state.approved_profile_json (e.g., raw_state.approved_profile_json = modified_dict) before await db_session.commit() so the change on PublicHomeState is detected and persisted.apps/api/app/providers/reasoning.py-29-33 (1)
29-33:⚠️ Potential issue | 🟠 Major修复 Gemini 2.5 模型识别逻辑
第 30 行对
gemini-2.5的检查与实际 LiteLLM 使用不符。根据项目文档(litellm_provider.py 第 7-9 行),LiteLLM 模型名称必须包含提供商前缀,如gemini/gemini-2.5-pro、gemini/gemini-2.5-flash等。由于_normalize_model()仅执行.strip().lower(),规范化后的模型名称如gemini/gemini-2.5-pro不会匹配startswith("gemini-2.5")。建议改为
startswith("gemini/gemini-2.5"),或调整规范化逻辑以提取提供商前缀后的模型标识符。此外,第 32-33 行的
"reasoner" in normalized检查过于宽泛,可能会无意匹配到包含"reasoner"字符的其他模型名称。建议更明确指定要匹配的模型。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/app/providers/reasoning.py` around lines 29 - 33, The Gemini 2.5 check fails because _normalize_model only lowercases/strips and provider prefixes remain; update reasoning detection to split off any provider prefix (e.g., model_id = normalized.split("/", 1)[-1]) and then test model_id.startswith("gemini-2.5") to catch names like "gemini/gemini-2.5-pro"; similarly replace the broad '"reasoner" in normalized' test with a more specific check against the post-prefix identifier (e.g., model_id.startswith("reasoner") or model_id.endswith("-reasoner") or compare against a small whitelist) so only intended reasoner models cause {"reasoning_effort": "medium"} for the logic around normalized/_normalize_model and the reasoning_effort return.apps/web/src/features/public-home/public-home-page.tsx-34-43 (1)
34-43:⚠️ Potential issue | 🟠 Major补上请求失败分支,别把接口异常渲染成“空档案”。
useQuery失败后这里会直接落到正常渲染分支,并用空数组/零值默认值继续出页面。用户看到的会是“没有公开内容”,而不是真正的加载失败,也没有重试入口。建议至少显式处理isError或!data。Also applies to: 105-110
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/features/public-home/public-home-page.tsx` around lines 34 - 43, The code currently treats failed useQuery results as empty data; update the public-home page to explicitly handle the error branch from useQuery (check isError and/or !data) when calling getPublicSite: detect when isError or data is missing, surface an error state instead of falling back to empty profile/stats/notebooks, and render an error UI or retry button (or return early) so profile/portrait/stats/notebooks/featuredNotebooks are not rendered as “empty” on request failure; adjust the logic around the useQuery result handling (the const { data, isLoading } = useQuery(...) and the derived profile/portrait/stats/notebooks/featuredNotebooks) to short-circuit on error and include the error info for user feedback and retry.apps/web/src/app/(marketing)/notebooks/[id]/page.tsx-331-337 (1)
331-337:⚠️ Potential issue | 🟠 Major这里把 TOC 侧栏永久隐藏了。
外层已经
tocItems.length > 0才渲染,但aside又写死了display: "none",结果目录在任何断点下都不会出现,长笔记的导航功能直接失效。如果只是想做响应式控制,请把显示逻辑放到类名或媒体查询里,不要用内联样式把它彻底覆盖掉。📌 最小修正
- <aside style={{ width: 200, flexShrink: 0, display: "none" }} className="lp-toc-aside"> + <aside style={{ width: 200, flexShrink: 0 }} className="lp-toc-aside">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/app/`(marketing)/notebooks/[id]/page.tsx around lines 331 - 337, The TOC is permanently hidden because the aside element rendering the table of contents has an inline style display: "none"; remove that inline display override from the aside (the element that uses className="lp-toc-aside" and renders <TableOfContents items={tocItems} activeId={activeHeading} />) and move responsive hide/show logic into CSS (use the lp-toc-aside class with media queries or utility classes) so the TOC still renders when tocItems.length > 0 but is hidden only at the intended breakpoints.apps/web/src/features/portrait/portrait-cards.tsx-43-45 (1)
43-45:⚠️ Potential issue | 🟠 Major不要在前端直接拼第三方头像 API URL。
这里即便只是放在
<img src>里,本质上也是浏览器直连外部 API,而且还把identity.primary_role作为 seed 发给第三方。这样既增加了前端可用性依赖,也把画像信息暴露到外部服务。更稳妥的方案是改成本地静态兜底图,或者由后端/服务层生成并缓存默认头像地址。
As per coding guidelines:apps/web/**/*.{ts,tsx}: Do not hardcode API URLs in frontend code - uselib/axios.tsencapsulation.Also applies to: 70-75
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/features/portrait/portrait-cards.tsx` around lines 43 - 45, The code builds a third-party avatar URL directly (seed, fallbackUrl, src using portrait.avatar_url) which exposes identity.primary_role and makes the browser call an external API; replace this by not constructing the Dicebear URL in the frontend — instead use a local static fallback asset or fetch a backend/avatar service endpoint (via the project's axios wrapper in lib/axios.ts) that returns a cached avatar URL; update the logic around seed/fallbackUrl/src to use portrait.avatar_url if present else call the backend (or reference a static asset) so the frontend never hardcodes or calls the external API directly.apps/web/src/services/public-home-service.ts-15-129 (1)
15-129:⚠️ Potential issue | 🟠 Major这些 mapper 现在没有真正做运行时防御。
这里把
unknown响应直接当数组/对象用;只要后端某个字段临时变成null、字符串或别的对象,像这些.map()和属性访问就会直接抛异常,public home 页面会跟着挂掉。既然这一层的职责就是把原始响应收敛成类型安全对象,建议统一先做Array.isArray/isRecordguard,再映射。🛡️ 一个可复用的收敛方式
type Raw = Record<string, unknown>; + +function asRecord(value: unknown): Raw { + return value !== null && typeof value === "object" && !Array.isArray(value) + ? (value as Raw) + : {}; +} + +function asStringArray(value: unknown): string[] { + return Array.isArray(value) ? value.map((item) => String(item)) : []; +} ... - const identity = (raw.identity as Raw | undefined) ?? {}; + const identity = asRecord(raw.identity); ... - sourceNotebookIds: ((raw.source_notebook_ids as string[]) ?? []).map(String), + sourceNotebookIds: asStringArray(raw.source_notebook_ids),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/services/public-home-service.ts` around lines 15 - 129, The mappers (mapPublicNotebook, mapTimelineItem, mapPortraitSnapshot, mapProfile, mapDraftState, mapStats) assume raw fields are correctly typed and call .map()/property access directly; add runtime guards to prevent crashes by validating inputs with reusable helpers (e.g., ensureArray(value): returns [] if not Array.isArray(value) and casts items safely, and ensureRecord(value): returns {} if value is not a plain object) and use them before mapping (e.g., replace raw.some_array as Raw[] ?? [] with ensureArray(raw.some_array).map(...), and wrap nested objects with ensureRecord(raw.portrait_snapshot) before reading properties); ensure numeric/boolean fields are checked with typeof guards and string fields default to "" or undefined as currently intended so any malformed backend response yields safe defaults instead of throwing.apps/web/src/features/public-home/public-home-sections.tsx-427-446 (1)
427-446:⚠️ Potential issue | 🟠 Major
AnimatedCount存在内存泄漏:setInterval清理逻辑无效
setTimeout回调内部的return () => clearInterval(timer)不会作为清理函数执行——它只是setTimeout回调的返回值,会被忽略。如果组件在 timeout 触发后、interval 仍在运行时卸载,会导致内存泄漏和对已卸载组件的状态更新。🐛 建议修复
function AnimatedCount({ target, active, delay = 0, suffix = "" }: { target: number; active: boolean; delay?: number; suffix?: string }) { const [count, setCount] = useState(0) + const timerRef = useRef<number | null>(null) useEffect(() => { if (!active) return const timeout = setTimeout(() => { let current = 0 const duration = 900 const step = 16 const increment = target / (duration / step) - const timer = setInterval(() => { + timerRef.current = window.setInterval(() => { current += increment if (current >= target) { setCount(target) - clearInterval(timer) + if (timerRef.current) clearInterval(timerRef.current) } else { setCount(Math.round(current)) } }, step) - return () => clearInterval(timer) }, delay) - return () => clearTimeout(timeout) + return () => { + clearTimeout(timeout) + if (timerRef.current) clearInterval(timerRef.current) + } }, [target, active, delay])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/features/public-home/public-home-sections.tsx` around lines 427 - 446, The AnimatedCount useEffect creates an interval inside a setTimeout but returns the interval cleanup from the timeout callback (which is ignored), causing potential leaks; modify the effect so the interval ID is stored in an outer-scope ref/variable (e.g., intervalRef) and the effect's cleanup clears both the timeout and that interval; specifically, in the useEffect for AnimatedCount move declaration of timer (interval) to a scope accessible by the outer return, assign its id (from setInterval) to intervalRef.current, and ensure the return () => { clearTimeout(timeout); clearInterval(intervalRef.current); } also handles cases where interval never started, and keep using setCount as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7fb2854b-f53d-43d7-beed-81f3ed07fdab
📒 Files selected for processing (119)
CLAUDE.mdapps/api/alembic/versions/036_public_home_states.pyapps/api/alembic/versions/037_user_portrait_avatar_url.pyapps/api/app/agents/core/attachment_text.pyapps/api/app/agents/core/engine.pyapps/api/app/agents/core/react_agent.pyapps/api/app/config.pyapps/api/app/domains/config/router.pyapps/api/app/domains/conversation/router.pyapps/api/app/domains/conversation/schemas.pyapps/api/app/domains/notebook/router.pyapps/api/app/domains/portrait/router.pyapps/api/app/domains/public/router.pyapps/api/app/domains/public_home/router.pyapps/api/app/domains/public_home/schemas.pyapps/api/app/main.pyapps/api/app/models/__init__.pyapps/api/app/models/memory.pyapps/api/app/models/public_home.pyapps/api/app/models/user.pyapps/api/app/providers/anthropic_provider.pyapps/api/app/providers/base.pyapps/api/app/providers/image_gen_provider.pyapps/api/app/providers/litellm_provider.pyapps/api/app/providers/llm.pyapps/api/app/providers/openai_provider.pyapps/api/app/providers/reasoning.pyapps/api/app/services/conversation_service.pyapps/api/app/services/public_home_service.pyapps/api/tests/integration/test_public_home_router.pyapps/api/tests/unit/test_attachment_text_extraction.pyapps/api/tests/unit/test_public_home_service.pyapps/api/tests/unit/test_reasoning_options.pyapps/web/messages/en.jsonapps/web/messages/zh.jsonapps/web/src/app/(marketing)/notebooks/[id]/page.tsxapps/web/src/app/(marketing)/page.tsxapps/web/src/app/(workspace)/app/notebooks/[id]/loading.tsxapps/web/src/app/(workspace)/app/notebooks/loading.tsxapps/web/src/app/(workspace)/app/page.tsxapps/web/src/components/chat-input/chat-input.tsxapps/web/src/components/home/home-content.tsxapps/web/src/components/home/home-hero.tsxapps/web/src/components/home/home-qa.tsxapps/web/src/components/home/home-suggestions.tsxapps/web/src/components/layout/app-shell.tsxapps/web/src/components/layout/sidebar.tsxapps/web/src/components/settings/sections/ai-config-section.tsxapps/web/src/components/settings/sections/public-home-section.tsxapps/web/src/components/settings/settings-modal.tsxapps/web/src/components/settings/settings-sections.tsxapps/web/src/features/chat/chat-view.tsxapps/web/src/features/chat/use-chat-page.tsxapps/web/src/features/copilot/copilot-panel.tsxapps/web/src/features/editor/note-editor.tsxapps/web/src/features/knowledge/knowledge-view.tsxapps/web/src/features/notebook/mobile-workspace-sheet.tsxapps/web/src/features/notebook/note-picker-dropdown.tsxapps/web/src/features/notebook/notebook-card.tsxapps/web/src/features/notebook/notebook-header.tsxapps/web/src/features/notebook/notebook-toc.tsxapps/web/src/features/notebook/notebook-workspace.tsxapps/web/src/features/notebook/notebooks-view.tsxapps/web/src/features/portrait/portrait-cards.tsxapps/web/src/features/portrait/portrait-view.tsxapps/web/src/features/public-home/helpers.tsapps/web/src/features/public-home/public-home-page.tsxapps/web/src/features/public-home/public-home-sections.tsxapps/web/src/features/public-home/public-home-styles.tsapps/web/src/features/source/source-detail-drawer.tsxapps/web/src/features/source/sources-panel.tsxapps/web/src/features/tasks/tasks-view.tsxapps/web/src/hooks/use-chat-stream.tsapps/web/src/hooks/use-media-query.tsapps/web/src/lib/api-routes.tsapps/web/src/services/ai-service.tsapps/web/src/services/config-service.tsapps/web/src/services/portrait-service.tsapps/web/src/services/public-home-service.tsapps/web/src/store/use-ui-store.tsapps/web/src/test/example.test.tsapps/web/src/test/setup.tsapps/web/src/types/index.tsapps/web/tests/contracts/services/notebook-service.contract.test.tsapps/web/tests/contracts/services/source-service.contract.test.tsapps/web/tests/fixtures/notebook.factory.tsapps/web/tests/fixtures/source.factory.tsapps/web/tests/fixtures/task.factory.tsapps/web/tests/integration/components/home/home-suggestions.test.tsxapps/web/tests/integration/features/notebook/notebook-workspace.test.tsxapps/web/tests/mocks/http-client.tsapps/web/tests/setup/vitest.setup.tsapps/web/tests/unit/app/(workspace)/app/notebooks/[id]/loading.test.tsxapps/web/tests/unit/app/(workspace)/app/notebooks/loading.test.tsxapps/web/tests/unit/components/layout/sidebar.test.tsxapps/web/tests/unit/components/settings/public-home-section.test.tsxapps/web/tests/unit/features/copilot/copilot-panel.test.tsxapps/web/tests/unit/features/knowledge/knowledge-view.test.tsxapps/web/tests/unit/features/notebook/mobile-workspace-sheet.test.tsxapps/web/tests/unit/features/notebook/notebook-header.test.tsxapps/web/tests/unit/features/notebook/notebook-toc.test.tsxapps/web/tests/unit/features/notebook/notebooks-view.test.tsxapps/web/tests/unit/features/portrait/portrait-cards.test.tsxapps/web/tests/unit/features/portrait/portrait-view.test.tsxapps/web/tests/unit/features/public-home/public-home-page.test.tsxapps/web/tests/unit/features/source/source-detail-drawer.test.tsxapps/web/tests/unit/features/source/sources-panel.test.tsxapps/web/tests/unit/features/tasks/task-delivery.test.tsapps/web/tests/unit/features/tasks/tasks-view.test.tsxapps/web/tests/unit/hooks/use-media-query.test.tsxapps/web/tests/unit/services/ai-service.test.tsapps/web/tests/unit/services/task-service.test.tsapps/web/tests/utils/create-test-query-client.tsapps/web/tests/utils/render-with-providers.tsxapps/web/tsconfig.jsonapps/web/vitest.config.tspackage.jsonscripts/generate_thesis_pdfs.pyturbo.json
💤 Files with no reviewable changes (2)
- apps/web/src/test/setup.ts
- apps/web/src/test/example.test.ts
Summary
Type of change
Related issue
Closes #
Changes
How to test
Screenshots (if applicable)
Checklist
./lyra lintand there are no type errorsfeat:,fix:, etc.)