Skip to content

Fix/pr90 whitescreen v2#94

Merged
AmintaCCCP merged 25 commits intomainfrom
fix/pr90-whitescreen-v2
Apr 22, 2026
Merged

Fix/pr90 whitescreen v2#94
AmintaCCCP merged 25 commits intomainfrom
fix/pr90-whitescreen-v2

Conversation

@AmintaCCCP
Copy link
Copy Markdown
Owner

@AmintaCCCP AmintaCCCP commented Apr 22, 2026

Summary by CodeRabbit

  • New Features

    • Discovery view (trending/topics/search/RSS) with filters, pagination and sidebar; virtualized lists and scroll-to-bottom
    • Per-repo AI analysis controls, star/unstar, richer repo cards; README modal with TOC, font sizing, image zoom/download, optimized image loading
    • Data export/import (backup/restore)
  • Bug Fixes

    • Improved error UI with “Report Issue” and copyable details; safer clipboard read/write; cancelable analysis requests
  • Style

    • Inter font added; enhanced dark-mode code highlighting and markdown styling; UI responsiveness tweaks

HappySummer and others added 22 commits April 20, 2026 15:23
- 在tailwind配置中添加sm-md-lg-xl-2xl屏幕断点
- 更新Header组件导航栏的响应式断点从lg改为xl
- 升级vite及相关插件版本
调整SubscriptionView组件中侧边栏的布局样式,添加sticky定位
移除构建过程中意外提交的dist/index.html文件
- 将订阅侧边栏、仓库卡片和开发者卡片中的emoji图标替换为lucide-react图标
- 为移动端添加可滑动的标签导航组件
- 优化平台图标的显示效果和间距
- 添加底部活动指示器和渐变遮罩效果
- 将订阅功能重构为发现频道,新增多种筛选和排序选项
- 添加AI分析辅助工具类,优化仓库分析逻辑
- 实现安全的剪贴板读写工具函数,增强兼容性
- 改进Markdown渲染器,添加代码复制和目录功能
- 优化仓库卡片组件,增加Star和AI分析操作
- 更新README模态框,支持字体大小调整和目录导航
- 移除旧的订阅相关组件和类型
- 在类型定义中添加 SubscriptionRepo 和 SubscriptionDev 等订阅相关类型
- 在应用状态中增加订阅相关的状态字段
- 实现订阅频道的初始化和迁移逻辑
- 移除未使用的 Category 类型导入
feat(ErrorBoundary): 增强错误边界组件功能
feat(DataManagementPanel): 扩展数据导入导出功能
style(index.css): 添加代码高亮和终端样式增强
feat(SubscriptionRepoCard): 改进Star操作逻辑和UI
feat(ScrollToBottom): 新增滚动到底部组件
build: 添加highlight.js依赖
添加Inter字体文件,包括多种字重和字符集支持
在tailwind配置中设置Inter为默认无衬线字体
在index.html中引入字体样式文件
使用动态分块函数替代静态配置,提高代码灵活性
移除过时注释以保持配置简洁
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
refactor(DiscoveryView): 使用ref优化滚动位置存储
fix(DataManagementPanel): 修复主题和视图模式类型检查
refactor(pagination): 重构分页组件,支持服务端分页和本地分页
feat(store): 添加discoveryCurrentPage状态管理
style(tooltip): 改进排序算法提示框的定位和响应式设计
fix(scroll): 修复页面切换时的滚动位置问题
perf(discovery): 优化数据加载逻辑和性能
- 添加性能优化相关代码,包括虚拟列表、图片懒加载和性能监控
- 实现RSS趋势功能,支持从第三方源获取GitHub趋势数据
- 重构发现频道相关代码,优化类型定义和状态管理
- 添加代码分割和懒加载以提升首屏性能
- 优化排序算法提示弹窗的交互和样式
- 更新依赖项,添加esbuild用于构建优化
- 在RSS服务中添加基础URL常量并重构URL配置
- 优化RepositoryList组件的暂停/恢复和停止逻辑,使用useCallback提升性能
- 在应用状态管理中新增rssTimeRange字段并实现版本迁移
保留切换频道时的当前页码,而不是总是重置为1
- 重构项目发现模块,合并项目类型和时间范围为场景化时间范围
- 新增模态框可见性钩子,优化滚动按钮在模态框打开时的显示逻辑
- 修复暗黑模式下Markdown文本颜色问题
- 更新数据管理面板,支持更多状态的导入导出
- 调整发现页筛选器UI,优化用户体验
在 persist store 的迁移逻辑中添加了对 discovery 相关状态的完整性检查和修复:
- discoveryRepos
- discoveryLastRefresh
- discoveryIsLoading
- discoveryHasMore
- discoveryTotalCount
- discoveryCurrentPage

确保所有频道键(trending、topic、search、rss-trending)都存在,防止因持久化数据不完整导致的白屏。

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 22, 2026

📝 Walkthrough

Walkthrough

Replaces the prior Subscription domain with a new Discovery domain (trending/topic/search/RSS), adds many new components/services (DiscoveryView, VirtualList, RSSTrendingService, aiAnalysisHelper), migrates store/types for discovery, introduces lazy-loaded views with ErrorBoundary/Suspense, adds modal open/close CustomEvents with a visibility hook, self-hosted Inter fonts, tooling upgrades, and extensive CSS/UX improvements.

Changes

Cohort / File(s) Summary
Font & Tailwind
index.html, public/fonts/inter.css, tailwind.config.js
Added Google preconnect + Inter stylesheet and a self-hosted inter.css with woff2 faces; added Inter to Tailwind font stack and explicit breakpoints.
Build / Dependencies & Vite
package.json, vite.config.ts
Added highlight.js, esbuild; upgraded Vite and plugins; added /api/rss dev proxy; changed build target/minify, chunking function, output names and prebundle includes/excludes.
App Shell & Lazy Loading
src/App.tsx
Converted several views to React.lazy + Suspense + ErrorBoundary, added full-app initialization gating (isInitialized), view prefetching, and a VIEW_COMPONENTS lookup.
Types & Project Constants
src/types/index.ts, src/constants/project.ts
Introduced Discovery types (channels, params, DiscoveryRepo), fork fields on Repository, discovery state in AppState, and PROJECT_REPO_URL / PROJECT_ISSUES_URL.
Store & Selectors
src/store/useAppStore.ts, src/hooks/useStoreSelectors.ts
Reworked Zustand store to add discovery domain, analyzingRepositoryIds, upsert repository logic, migrations/version bump, and many selector hooks.
Discovery Core UI
src/components/DiscoveryView.tsx, src/components/DiscoverySidebar.tsx
New DiscoveryView: multi-channel browsing, pagination, scroll restoration, centralized refresh, AI analysis workflow with aborting and progress; new DiscoverySidebar for channel list/refresh.
Virtualization & Scrolling Helpers
src/components/VirtualList.tsx, src/components/ScrollToBottom.tsx
Added generic VirtualList with imperative handle and infinite-load points; ScrollToBottom component respects modal visibility via hook.
Image & Markdown UX
src/components/OptimizedImage.tsx, src/components/MarkdownRenderer.tsx, src/components/ReadmeModal.tsx, src/index.css
Added OptimizedImage/AvatarImage and preloader hooks; new CodeBlock with highlight.js and copy, rich image zoom/download overlay, ReadmeModal TOC/scroll progress/font-size controls; extensive dark-mode & prose CSS and highlight styles.
Repository Cards & Analysis
src/components/RepositoryCard.tsx, src/components/SubscriptionRepoCard.tsx, src/components/RepositoryList.tsx
Per-repo analyze lifecycle moved to analyzingRepositoryIds with AbortController and analyzeRepository helper; SubscriptionRepoCard adapted to DiscoveryRepo with star/unstar and memoization; repository list updates and optimizer control changes.
New Services
src/services/aiAnalysisHelper.ts, src/services/aiService.ts, src/services/githubApi.ts, src/services/rssTrendingService.ts
Added analyzeRepository helper and failure result factory; AIService analysis supports AbortSignal; GitHubApiService extended for discovery endpoints and starring; new RSSTrendingService parses RSS and resolves repos with bounded concurrency.
Utilities & Hooks
src/utils/clipboardUtils.ts, src/utils/pagination.ts, src/utils/usePagination.ts, src/utils/performanceMonitor.ts, src/hooks/useModalVisibility.ts
Safe clipboard read/write helpers, pagination utilities and hook, performanceMonitor singleton + measure helpers, and useModalVisibility subscribing to gsm:modal-open/close.
Modal / Error UX
src/components/Modal.tsx, src/components/ErrorBoundary.tsx
Modal dispatches gsm:modal-open/gsm:modal-close; ErrorBoundary removed destructive storage-clear flow and added issue reporting, copy-error, toggle details UI and related handlers.
Settings & Data Management
src/components/settings/DataManagementPanel.tsx, src/components/settings/GeneralPanel.tsx
Added JSON export/import with preview/merge/replace, expanded deletion options and cleanup suggestions; GeneralPanel uses PROJECT_REPO_URL.
Removed / Replaced Subscription UI
src/components/SubscriptionView.tsx, src/components/SubscriptionSidebar.tsx, src/components/SubscriptionDevCard.tsx
Removed legacy Subscription view/sidebar/dev card files; functionality replaced by Discovery equivalents and updated types.
Other UI Additions
src/components/SortAlgorithmTooltip.tsx, src/components/BackToTop.tsx, src/components/ReadmeModal.tsx
Added SortAlgorithmTooltip with portal positioning; BackToTop and ScrollToBottom respect modal visibility hook; ReadmeModal UI updates (TOC, progress, font sizing).

Sequence Diagram(s)

sequenceDiagram
    participant User as User
    participant DiscoveryView as DiscoveryView
    participant GitHubApi as GitHubApiService
    participant RSSTrend as RSSTrendingService
    participant AIHelper as AIAnalysisHelper
    participant AppStore as AppStore

    User->>DiscoveryView: open discovery / select channel
    DiscoveryView->>AppStore: read/set discovery params & selectedChannel
    alt channel == rss-trending
        DiscoveryView->>RSSTrend: fetchRSSTrending(timeRange, onProgress)
        RSSTrend->>GitHubApi: getRepository(owner, repo) [concurrent]
        GitHubApi-->>RSSTrend: repository details
        RSSTrend-->>DiscoveryView: paginated repos
    else
        DiscoveryView->>GitHubApi: getTrending/search/topic(params)
        GitHubApi-->>DiscoveryView: paginated repos
    end
    DiscoveryView->>AppStore: setDiscoveryRepos(channel, repos)
    User->>DiscoveryView: click "Analyze page"
    DiscoveryView->>AIHelper: analyzeRepository(repo, token, aiConfig, signal)
    AIHelper->>GitHubApi: fetch README (if needed)
    AIHelper-->>DiscoveryView: AIAnalysisResult
    DiscoveryView->>AppStore: updateDiscoveryRepo(repo with AI fields)
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

Poem

🐰 I hopped through channels, sniffed each trending trail,
Fonts dressed the meadow, lazy views wagged their tail.
RSS and AI hummed like carrots in the sun,
Modals chimed their signals — the discovery's begun.
I nibbled codeblocks, zoomed images — oh what fun!

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title 'Fix/pr90 whitescreen v2' refers to fixing a whitescreen issue (PR #90), but the changeset is a massive refactor introducing discovery/exploration features with 6000+ lines of changes. The title doesn't reflect the primary scope of work. Use a more descriptive title reflecting the main changes, such as 'Implement discovery view with trending, topic, and search channels' or 'Add exploration features with multiple discovery channels and enhanced UI components'.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/pr90-whitescreen-v2

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

用于排查切换标签时的白屏问题:
1. 添加全局日志存储 __discoveryLogs 和 __discoveryErrors
2. 添加 dl() 和 de() 函数记录关键节点
3. 添加 DiscoveryViewErrorBoundary 错误边界组件
4. 导出 DiscoveryViewWithErrorBoundary 包装组件
5. App.tsx 使用新的包装组件

测试时可在控制台执行以下命令导出日志:
  - __discoveryLogs // 查看所有日志
  - __discoveryErrors // 查看所有错误
  - (window).__discoveryLogs?.slice(-20) // 最近20条日志

🤖 Generated with [Claude Code](https://claude.com/claude-code)
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 19

Note

Due to the large number of review comments, Critical, Major 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)
src/components/Modal.tsx (1)

27-37: ⚠️ Potential issue | 🟡 Minor

Only dispatch close/restore scroll for an opened modal instance.

The cleanup currently fires gsm:modal-close and resets body.style.overflow even when this effect ran with isOpen === false, which can desynchronize useModalVisibility consumers and re-enable background scroll unexpectedly.

Proposed fix
-    if (isOpen) {
-      document.addEventListener('keydown', handleEscape);
-      // Prevent body scroll when modal is open
-      document.body.style.overflow = 'hidden';
-      window.dispatchEvent(new CustomEvent('gsm:modal-open'));
-    }
-
-    return () => {
+    if (!isOpen) return;
+
+    const previousOverflow = document.body.style.overflow;
+    document.addEventListener('keydown', handleEscape);
+    // Prevent body scroll when modal is open
+    document.body.style.overflow = 'hidden';
+    window.dispatchEvent(new CustomEvent('gsm:modal-open'));
+
+    return () => {
       document.removeEventListener('keydown', handleEscape);
-      document.body.style.overflow = 'unset';
+      document.body.style.overflow = previousOverflow;
       window.dispatchEvent(new CustomEvent('gsm:modal-close'));
     };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/Modal.tsx` around lines 27 - 37, The effect in Modal.tsx
currently restores body scroll and dispatches 'gsm:modal-close' on every cleanup
even when isOpen was false; update the cleanup returned by the useEffect that
references isOpen and handleEscape so that it always removes the keydown
listener but only resets document.body.style.overflow and dispatches the
'gsm:modal-close' CustomEvent when isOpen was true (i.e., guard the overflow
restore and close dispatch with if (isOpen) or similar) to avoid desynchronizing
useModalVisibility consumers.
src/services/aiService.ts (1)

125-137: ⚠️ Potential issue | 🟠 Major

Forward the abort signal through the backend proxy path.

The new signal reaches direct fetch calls, but backend-backed AI requests drop it via backend.proxyAIRequest(...). Currently, proxyAIRequest accepts only configId and body and passes neither to its internal fetchWithTimeout. In desktop/backend mode, stop/cancel can leave the in-flight AI call running.

This affects three call sites:

  • Line 126 (OpenAI responses)
  • Line 183 (Claude)
  • Line 238 (Gemini)

Update backend.proxyAIRequest to accept an optional signal parameter and forward it to the underlying fetchWithTimeout call. Then pass options.signal from each of the three proxyAIRequest call sites.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/services/aiService.ts` around lines 125 - 137, The backend proxy path
drops abort signals: update backend.proxyAIRequest to accept an optional signal
parameter (e.g., proxyAIRequest(configId: string, body: unknown, signal?:
AbortSignal)) and forward that signal into the underlying fetchWithTimeout call
used inside proxyAIRequest; then update all proxy call sites (the calls that
pass this.config.id and requestBody — the three uses for OpenAI responses,
Claude and Gemini) to pass options.signal when invoking backend.proxyAIRequest
so aborts propagate to the in-flight fetch.
src/components/ReadmeModal.tsx (1)

189-214: ⚠️ Potential issue | 🟠 Major

Asymmetric modal-open/modal-close dispatching can desync useModalVisibility.

Line 200 dispatches gsm:modal-open only when isOpen is true, but line 211 dispatches gsm:modal-close unconditionally in cleanup. That means:

  1. Mount with isOpen=false (common — modal renders before being opened). Effect runs, no open dispatched. If this effect ever re-runs (e.g., onClose prop identity changes in the parent), cleanup fires gsm:modal-close without a preceding open.
  2. Same asymmetry on unmount while isOpen was false.

If useModalVisibility uses a counter, it can drift negative; if it uses a boolean set, stray closes may hide other open modals' state. Gate the close dispatch on the same condition as the open dispatch.

🛠️ Proposed fix
     if (isOpen) {
       previousFocusRef.current = document.activeElement as HTMLElement;
       document.addEventListener('keydown', handleEscape);
       document.body.style.overflow = 'hidden';
       window.dispatchEvent(new CustomEvent('gsm:modal-open'));
       setTimeout(() => {
         modalRef.current?.focus();
       }, 0);
     }

     return () => {
       document.removeEventListener('keydown', handleEscape);
       if (document.body.style.overflow === 'hidden') {
         document.body.style.overflow = 'unset';
       }
-      window.dispatchEvent(new CustomEvent('gsm:modal-close'));
-      previousFocusRef.current?.focus();
+      if (isOpen) {
+        window.dispatchEvent(new CustomEvent('gsm:modal-close'));
+        previousFocusRef.current?.focus();
+      }
     };
   }, [isOpen, onClose]);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/ReadmeModal.tsx` around lines 189 - 214, The effect dispatches
gsm:modal-open only when isOpen is true but always dispatches gsm:modal-close in
the cleanup, causing desync; update the useEffect in ReadmeModal (the effect
that references modalRef, previousFocusRef, handleEscape) to conditionally
dispatch the close and only restore body overflow/focus if you previously opened
the modal — e.g., capture a local flag (opened) or check isOpen at effect start
and use that in the cleanup so gsm:modal-close, document.body.style changes, and
previousFocusRef.current?.focus() only run when the effect actually performed
the open actions.
src/store/useAppStore.ts (2)

1165-1243: ⚠️ Potential issue | 🔴 Critical

Update persist version from 6 to match the current store version of 8.

The persist config writes version: 6, but migrations expect version 8. This causes users with any stored state to re-apply the version 7 and 8 migrations (including the non-trivial trendingParams transformation) on every app startup instead of just once.

Extract CURRENT_STORE_VERSION = 8 as a module-level constant and use it in both the persist config and the migrate function to maintain a single source of truth.

Proposed fix
+const CURRENT_STORE_VERSION = 8;
+
 const store = create<AppState & AppActions>()(
   persist(
     (set) => ({
     name: 'github-stars-manager',
-      version: 6,
+      version: CURRENT_STORE_VERSION,
       migrate: (persistedState, fromVersion) => {
-        const CURRENT_STORE_VERSION = 8;
         const state = persistedState as PersistedAppState | undefined;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/store/useAppStore.ts` around lines 1165 - 1243, The persist config
currently hardcodes version: 6 while migrate uses CURRENT_STORE_VERSION = 8;
extract a single module-level constant (e.g., const CURRENT_STORE_VERSION = 8)
and replace the hardcoded numeric literal in the persist config's version field
with that constant, and also use the same constant inside the migrate function
(instead of the local const), so the persist config and migrate share one source
of truth; ensure the constant is declared at top-level in the same module and
referenced by the persist config object (version) and by the migrate logic that
compares persistedState versions.

177-224: ⚠️ Potential issue | 🔴 Critical

Add discoveryIsLoading to the persisted-state type.

PersistedAppState does not include discoveryIsLoading, but the migration code at lines 1458–1465 reads and writes this property. This creates a type inconsistency; accessing discoveryIsLoading from a PersistedAppState-typed variable requires type assertions and fails to provide proper normalization during rehydration. Additionally, normalizePersistedState explicitly handles similar discovery fields (discoveryRepos, discoveryLastRefresh, discoveryTotalCount, discoveryCurrentPage) and other loading flags (subscriptionIsLoading) but omits discoveryIsLoading entirely.

Proposed fix
     | 'discoveryRepos'
     | 'discoveryLastRefresh'
+    | 'discoveryIsLoading'
     | 'discoveryTotalCount'

Also applies to: 1429-1468

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/store/useAppStore.ts` around lines 177 - 224, PersistedAppState is
missing the discoveryIsLoading property while migration and normalization logic
(see normalizePersistedState and the migration that reads/writes
discoveryIsLoading) expect it; add discoveryIsLoading?: boolean to the
PersistedAppState type union alongside other discovery fields, and update
normalizePersistedState/migration mappings if necessary to include
discoveryIsLoading so rehydration and type checks handle this flag without
assertions.
🟡 Minor comments (25)
src/components/settings/DataManagementPanel.tsx-388-434 (1)

388-434: ⚠️ Potential issue | 🟡 Minor

Reset discovery cache to the same defaults as the store.

discoveryHasMore is reset to true for several channels here, but the store defaults in src/store/useAppStore.ts:570-596 initialize every discovery channel to false. Keeping true after a delete/all-reset can trigger stale “load more” behavior on an empty cache.

Proposed fix
         discoveryHasMore: {
-          'trending': true,
-          'topic': true,
-          'search': true,
+          'trending': false,
+          'topic': false,
+          'search': false,
           'rss-trending': false
         },

Apply this in both reset blocks.

Also applies to: 1043-1085

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/settings/DataManagementPanel.tsx` around lines 388 - 434, The
reset in deleteDiscoveryData currently sets discoveryHasMore to true for several
channels causing stale "load more" behavior; update the useAppStore.setState
calls in deleteDiscoveryData (and the other reset block at lines 1043-1085) so
discoveryHasMore for all channels ('trending','topic','search','rss-trending')
is set to false to match the defaults in useAppStore, and ensure discoveryRepos,
discoveryLastRefresh, discoveryTotalCount, discoveryNextPage,
discoveryCurrentPage, and discoveryScrollPositions remain consistent with the
store defaults.
src/components/settings/DataManagementPanel.tsx-941-954 (1)

941-954: ⚠️ Potential issue | 🟡 Minor

Remove or implement the unanalyzedRepos cleanup action.

The suggestion is rendered, but handleCleanup has no case 'unanalyzedRepos', so clicking “Clean” reports success without doing anything.

Conservative fix: remove the unsupported suggestion
-    const unanalyzedRepos = repositories.filter(r => !r.analyzed_at).length;
-    if (unanalyzedRepos > 10) {
-      suggestions.push({
-        key: 'unanalyzedRepos',
-        label: '未分析的仓库',
-        labelEn: 'Unanalyzed Repositories',
-        description: '尚未进行AI分析的仓库数量',
-        descriptionEn: 'Repositories that have not been analyzed by AI',
-        count: unanalyzedRepos,
-        icon: <Bot className="w-4 h-4" />,
-        color: 'text-blue-600 dark:text-blue-400',
-        bgColor: 'bg-blue-50 dark:bg-blue-900/20'
-      });
-    }
-

Also applies to: 974-992

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/settings/DataManagementPanel.tsx` around lines 941 - 954, The
suggestion item with key 'unanalyzedRepos' is rendered in DataManagementPanel
but handleCleanup lacks a case for 'unanalyzedRepos', so clicking "Clean" does
nothing; either remove this suggestion creation or implement the cleanup path:
add a 'case "unanalyzedRepos"' branch inside handleCleanup (in
DataManagementPanel) that performs the appropriate cleanup logic (e.g., call the
existing repo cleanup helper or API, update state, await completion and push
success/error via the same notification flow used by other cases), and ensure
the suggestion's count/label keys remain consistent with the implemented action.
src/components/settings/DataManagementPanel.tsx-81-125 (1)

81-125: ⚠️ Potential issue | 🟡 Minor

Include all persisted discovery preferences in backup/restore.

The store persists selectedTrendingLanguage and selectedRSSLanguage in src/store/useAppStore.ts:1220-1240, but this export/import schema does not include them, so a discovery backup silently loses those user preferences.

Also applies to: 558-575, 704-752

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/settings/DataManagementPanel.tsx` around lines 81 - 125, The
ExportData schema in DataManagementPanel.tsx is missing the persisted discovery
preferences selectedTrendingLanguage and selectedRSSLanguage; update the
ExportData.data type to include these two fields (with the correct types used in
useAppStore, e.g., ProgrammingLanguage/RSSTimeRange or the exact types from
useAppStore.ts), and then update the export serialization and the import/restore
logic that reads ExportData (the functions that assemble and apply the backup in
DataManagementPanel) to write/read these keys so restoring a backup re-applies
selectedTrendingLanguage and selectedRSSLanguage from the store.
src/utils/performanceMonitor.ts-151-153 (1)

151-153: ⚠️ Potential issue | 🟡 Minor

Record render measurements as render, not custom.

The exported measureRender() helper currently delegates to startMeasure(), so its metrics are stored under custom and getSummary().render remains inaccurate.

Proposed fix
 export const measureRender = (componentName: string) => {
-  return performanceMonitor.startMeasure(`render:${componentName}`);
+  const startTime = performance.now();
+  return () => {
+    const duration = performance.now() - startTime;
+    performanceMonitor.measureRender(componentName, duration);
+    return duration;
+  };
 };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/performanceMonitor.ts` around lines 151 - 153, measureRender
currently calls performanceMonitor.startMeasure in a way that tags measurements
as custom; update measureRender to call startMeasure with the render kind so
measurements are recorded under getSummary().render (e.g., change
performanceMonitor.startMeasure(`render:${componentName}`) to
performanceMonitor.startMeasure(`render:${componentName}`, 'render') or
otherwise invoke the monitor's render-specific API), ensuring measureRender,
startMeasure, and getSummary().render consistently use the 'render' type.
public/fonts/inter.css-1-53 (1)

1-53: ⚠️ Potential issue | 🟡 Minor

Fix the stylelint violations and wire up the intended font source.

Stylelint expects font-family: Inter without quotes here. Also, this new local stylesheet is not loaded in the provided app path while index.html loads Inter from Google Fonts, so either import this CSS and remove the remote Inter link, or drop the unused local font file.

Stylelint fix
-  font-family: 'Inter';
+  font-family: Inter;

Apply the same change to all six @font-face declarations.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@public/fonts/inter.css` around lines 1 - 53, Change all six `@font-face`
declarations to use font-family: Inter (no quotes) to satisfy stylelint, and
wire the local stylesheet into the app by removing the remote Google Fonts
<link> in index.html and importing or linking this local inter.css instead (or
if you prefer to keep Google Fonts, delete the unused local inter.css); ensure
you update the six `@font-face` blocks (the src/unicode-range entries) and the
HTML head where Inter is loaded so only one source is used.
src/components/SortAlgorithmTooltip.tsx-72-86 (1)

72-86: ⚠️ Potential issue | 🟡 Minor

Initial placement can be measured with a zero-height tooltip.

On first open, the portal renders with width: 0 / opacity: 0 (lines 187–189), then requestAnimationFrame runs calculatePosition. At that point tooltipRect.height may still be ~0 because the tooltip has no intrinsic width to lay out its text, so spaceBelow >= tooltipRect.height at line 61 is almost always true — placement is effectively locked to 'bottom' until the next scroll/resize event, even when 'top' would fit better.

Consider rendering at maxWidth/width immediately (e.g., keep final width but toggle only opacity) so the first measurement reflects the laid-out size, or use a two-pass rAF.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/SortAlgorithmTooltip.tsx` around lines 72 - 86, The tooltip's
initial measurement happens while it's rendered with width:0/opacity:0 so
calculatePosition (used in SortAlgorithmTooltip via the calculatePosition
function called in the useEffect) reads a near-zero tooltipRect.height and
forces placement to 'bottom'; fix by ensuring the tooltip is laid out with its
final width before measuring: when isVisible is true render the portal tooltip
with the final width/maxWidth (or apply a "measure" class) but keep it invisible
only via opacity/visibility so calculatePosition reads the true dimensions, or
perform a two-pass requestAnimationFrame in the useEffect (first rAF to force
layout, second rAF to call calculatePosition) to guarantee
tooltipRef.getBoundingClientRect returns the laid-out size for correct
top/bottom placement.
src/App.tsx-40-46 (1)

40-46: ⚠️ Potential issue | 🟡 Minor

measureRender is started on every render but ended only once — measurement leak.

measureRender('RepositoriesView') runs inside the component body, so it starts a new measurement on every render. The returned endMeasure is captured by the first render's closure and invoked once from the mount-only useEffect([]). All subsequent renders start measurements that are never ended — depending on performanceMonitor's implementation, this can leak performance marks / timers and skew any aggregated stats.

🛠️ Proposed fix
-}) => {
-  const endMeasure = measureRender('RepositoriesView');
-  useEffect(() => {
-    endMeasure();
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, []);
+}) => {
+  useEffect(() => {
+    const endMeasure = measureRender('RepositoriesView');
+    endMeasure();
+  }, []);

Or, if you want to measure every render's duration, end the measurement in a per-render effect (no deps array).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/App.tsx` around lines 40 - 46, measureRender('RepositoriesView') is being
invoked on every render while endMeasure is only called once in a mount-only
useEffect, causing measurement leaks; fix by starting the measurement inside a
mount effect (so call measureRender() within the same useEffect that currently
calls endMeasure) or, if the intention is to measure every render, call
measureRender() in the component body but then call the returned endMeasure
inside a per-render useEffect (i.e., useEffect with no deps array) so each
started measurement is ended; reference the measureRender function, the
endMeasure variable, the RepositoriesView component body and the existing
useEffect to locate where to change.
src/components/SortAlgorithmTooltip.tsx-180-236 (1)

180-236: ⚠️ Potential issue | 🟡 Minor

role="dialog" + aria-modal="true" is incorrect for this popover.

This is a click-toggled informational popover that dismisses on outside-click and does not trap focus. Marking it aria-modal="true" tells assistive tech that the rest of the page is inert, which is untrue and degrades screen-reader UX. Use either:

  • role="tooltip" (pair with aria-describedby on the trigger, drop aria-modal), or
  • Keep role="dialog" without aria-modal, and implement a proper focus trap + label (aria-labelledby pointing to the <h4> title).
🛠️ Minimal fix (tooltip semantics)
-          role="dialog"
-          aria-modal="true"
+          role="tooltip"
+          id={`sort-algo-tooltip-${channelId}`}

And on the trigger button:

           aria-label={t('排序算法说明', 'Sorting algorithm info')}
           aria-expanded={isVisible}
+          aria-describedby={isVisible ? `sort-algo-tooltip-${channelId}` : undefined}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/SortAlgorithmTooltip.tsx` around lines 180 - 236, The popover
currently uses role="dialog" with aria-modal="true" which falsely makes the page
inert; change semantics to a non-modal tooltip: replace role="dialog" and remove
aria-modal="true" on the container rendered by createPortal in
SortAlgorithmTooltip (the element using tooltipRef), set role="tooltip" instead,
give the title element (the <h4> rendering info.title) a stable id (e.g.,
titleId) and add aria-describedby="<that id>" on the trigger that calls
setIsVisible (the button that opens the popover), and keep the outside-click
dismissal behavior without adding a focus trap.
src/App.tsx-21-28 (1)

21-28: ⚠️ Potential issue | 🟡 Minor

Hardcoded Chinese loading text ignores the user's language.

LoadingSpinner is the shared fallback for every lazy-loaded view (Repositories, Releases, Settings, Discovery) and the initial app boot (line 198), but it always renders "加载中…" regardless of the language in the store. Read language from the store (or accept it as a prop) and localize:

-const LoadingSpinner = () => (
+const LoadingSpinner = () => {
+  const language = useAppStore(state => state.language);
+  return (
   <div className="flex items-center justify-center min-h-[400px]">
     <div className="flex flex-col items-center gap-3">
       <div className="w-8 h-8 border-3 border-blue-500 border-t-transparent rounded-full animate-spin" />
-      <span className="text-sm text-gray-500 dark:text-gray-400">加载中...</span>
+      <span className="text-sm text-gray-500 dark:text-gray-400">
+        {language === 'zh' ? '加载中...' : 'Loading...'}
+      </span>
     </div>
   </div>
-);
+  );
+};

Note: during the pre-auth / pre-init render (line 195-201) the store may still have its default language — that's fine; the default is what the user configured previously via persisted state.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/App.tsx` around lines 21 - 28, LoadingSpinner currently hardcodes the
Chinese "加载中…" text; update LoadingSpinner to read the app language (either via
the store hook, e.g., useSelector/useStore to get language, or by accepting a
language prop) and render a localized string (use your existing
i18n/localization utility or a small map like { en: "Loading...", zh: "加载中…" })
instead of the hardcoded text; ensure it falls back to the store default
language when the store isn't fully initialized so the same component works as
the shared fallback for lazy-loaded views and initial app boot.
vite.config.ts-26-32 (1)

26-32: ⚠️ Potential issue | 🟡 Minor

Remove unused react-router-dom from vite.config.ts.

The library is never imported anywhere in the codebase. View switching is entirely driven by the Zustand store (currentView) and the VIEW_COMPONENTS lookup in App.tsx. Both the optimizeDeps.include entry and the manualChunks condition for react-router-dom are dead code and should be removed to keep the configuration clean.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@vite.config.ts` around lines 26 - 32, Remove the unused react-router-dom
references from Vite config: delete 'react-router-dom' from the
optimizeDeps.include array and remove the manualChunks branch that splits out
'react-router-dom' (the condition referencing that string). Confirm there are no
actual imports of react-router-dom in the codebase (App.tsx uses
VIEW_COMPONENTS/currentView via Zustand) before removing any dependency entries,
and update any comments or docs referencing react-router-dom to keep config
consistent.
src/index.css-819-831 (1)

819-831: ⚠️ Potential issue | 🟡 Minor

Use kebab-case keyframe names to satisfy Stylelint.

fadeIn and expandFade violate the configured keyframes-name-pattern.

🧹 Proposed fix
-@keyframes fadeIn {
+@keyframes fade-in {
@@
 .fade-in {
-  animation: fadeIn 0.2s ease-out forwards;
+  animation: fade-in 0.2s ease-out forwards;
 }
@@
 .stagger-fade-in > * {
-  animation: fadeIn 0.2s ease-out forwards;
+  animation: fade-in 0.2s ease-out forwards;
@@
-@keyframes expandFade {
+@keyframes expand-fade {
@@
 .animate-expand-fade {
-  animation: expandFade 0.2s ease-out forwards;
+  animation: expand-fade 0.2s ease-out forwards;

Also applies to: 835-849, 852-866

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/index.css` around lines 819 - 831, Rename the camelCase keyframe
identifiers to kebab-case to satisfy Stylelint: change `@keyframes` fadeIn to
`@keyframes` fade-in and update any animation declarations that reference "fadeIn"
(for example the .fade-in rule) to use "fade-in"; likewise change `@keyframes`
expandFade to `@keyframes` expand-fade and update all animation references (e.g.,
any .expand-... rules) to use "expand-fade". Ensure you update every occurrence
of the old names across the file so keyframe definitions and animation
properties match.
src/utils/clipboardUtils.ts-76-114 (1)

76-114: ⚠️ Potential issue | 🟡 Minor

Pass UI language into clipboard error messages.

safeWriteText() and safeReadText() call getClipboardErrorMessage() without a language, so English UI callers can receive Chinese fallback errors.

🌐 Proposed fix
-export const safeWriteText = async (text: string): Promise<{ success: boolean; error?: string }> => {
+export const safeWriteText = async (
+  text: string,
+  language: 'zh' | 'en' = 'zh'
+): Promise<{ success: boolean; error?: string }> => {
@@
     return {
       success: false,
-      error: getClipboardErrorMessage('write'),
+      error: getClipboardErrorMessage('write', language),
     };
@@
     return {
       success: false,
-      error: getClipboardErrorMessage('write'),
+      error: getClipboardErrorMessage('write', language),
     };
   }
 };
@@
-export const safeReadText = async (): Promise<{ success: boolean; text?: string; error?: string }> => {
+export const safeReadText = async (
+  language: 'zh' | 'en' = 'zh'
+): Promise<{ success: boolean; text?: string; error?: string }> => {
@@
     return {
       success: false,
-      error: getClipboardErrorMessage('read'),
+      error: getClipboardErrorMessage('read', language),
     };
@@
     return {
       success: false,
-      error: getClipboardErrorMessage('read'),
+      error: getClipboardErrorMessage('read', language),
     };

Then pass the current UI language at call sites.

Also applies to: 122-139

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/clipboardUtils.ts` around lines 76 - 114, safeWriteText and
safeReadText call getClipboardErrorMessage without a language, causing
non-localized (Chinese) fallbacks to appear for users; update both functions
(safeWriteText and safeReadText) to pass the current UI language into
getClipboardErrorMessage (e.g., getClipboardErrorMessage('write', uiLang) and
getClipboardErrorMessage('read', uiLang)). Obtain uiLang from the app's existing
i18n/context (or accept it as a parameter to these functions if not accessible),
and replace the two error-call sites so the error messages use the current UI
language.
src/index.css-412-412 (1)

412-412: ⚠️ Potential issue | 🟡 Minor

Fix Stylelint font-family quoting errors.

Consolas should be unquoted per the configured font-family-name-quotes rule.

🧹 Proposed fix
-  font-family: 'Cascadia Code', 'Fira Code', 'JetBrains Mono', 'Consolas', monospace;
+  font-family: 'Cascadia Code', 'Fira Code', 'JetBrains Mono', Consolas, monospace;

Apply the same change at each reported Consolas occurrence.

Also applies to: 432-432, 450-450, 617-617

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/index.css` at line 412, The font-family declarations currently quote
"Consolas", which violates the stylelint font-family-name-quotes rule; update
each font-family line that lists 'Consolas' (e.g., the declaration containing
'Cascadia Code', 'Fira Code', 'JetBrains Mono', 'Consolas', monospace) to remove
the quotes around Consolas (make it Consolas unquoted) and apply the same change
to the other reported occurrences (lines referenced in the review).
src/index.css-909-909 (1)

909-909: ⚠️ Potential issue | 🟡 Minor

Normalize the text-rendering keyword casing.

Stylelint expects the value to be lowercase.

🧹 Proposed fix
-  text-rendering: optimizeSpeed;
+  text-rendering: optimizespeed;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/index.css` at line 909, Update the text-rendering declaration to use a
lowercase keyword: replace the current property value "optimizeSpeed" with the
lowercase form "optimizespeed" in src/index.css (look for the text-rendering:
optimizeSpeed; rule) so it conforms to stylelint's lowercase keyword rule.
src/components/RepositoryCard.tsx-333-338 (1)

333-338: ⚠️ Potential issue | 🟡 Minor

Clear stale AI fields when marking analysis as failed.

createFailedAnalysisResult() includes empty summary/tags/platforms, but this failure path only writes analyzed_at and analysis_failed. A failed re-analysis can leave old ai_summary, ai_tags, or ai_platforms visible/persisted.

🐛 Proposed fix
 const failedResult = createFailedAnalysisResult();
 const failedRepo = {
   ...repository,
+  ai_summary: failedResult.summary,
+  ai_tags: failedResult.tags,
+  ai_platforms: failedResult.platforms,
   analyzed_at: failedResult.analyzed_at,
   analysis_failed: failedResult.analysis_failed
 };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/RepositoryCard.tsx` around lines 333 - 338, When constructing
failedRepo in the failure path, also clear any stale AI fields so old
ai_summary, ai_tags, and ai_platforms aren't preserved; update the object built
from repository (the failedRepo created alongside createFailedAnalysisResult())
to explicitly set ai_summary: '', ai_tags: [], and ai_platforms: [] in addition
to using failedResult.analyzed_at and failedResult.analysis_failed so the
UI/database doesn't retain prior AI data after a failed analysis.
src/components/SubscriptionRepoCard.tsx-390-398 (1)

390-398: ⚠️ Potential issue | 🟡 Minor

Stop GitHub-link clicks from opening the README modal too.

The surrounding card has onClick={handleCardClick}; this anchor currently bubbles, so clicking “Open on GitHub” can also open the README modal.

🐛 Proposed fix
 <a
   href={repo.html_url}
   target="_blank"
   rel="noopener noreferrer"
+  onClick={(e) => e.stopPropagation()}
   className="flex items-center justify-center w-8 h-8 rounded-lg bg-gray-100 text-gray-500 dark:bg-gray-700 dark:text-gray-400 hover:bg-gray-200 dark:hover:bg-gray-600 transition-colors"
   title={t('在GitHub打开', 'Open on GitHub')}
 >
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/SubscriptionRepoCard.tsx` around lines 390 - 398, The anchor
that opens GitHub is bubbling and triggering the card's onClick handler
(handleCardClick), so add an event handler to the anchor to stop propagation
(e.g., onClick that calls event.stopPropagation()) so clicks on the ExternalLink
do not trigger the README modal; update the <a ...> element in
SubscriptionRepoCard (the anchor with href={repo.html_url} and
title={t('在GitHub打开', 'Open on GitHub')}) to call stopPropagation on click (and
optionally onKeyDown for keyboard activation) to prevent handleCardClick from
running.
src/components/SubscriptionRepoCard.tsx-239-242 (1)

239-242: ⚠️ Potential issue | 🟡 Minor

Add noopener,noreferrer when opening ZRead.

This external window.open call should not give the new tab access to window.opener.

🛡️ Proposed fix
-    window.open(zreadUrl, '_blank');
+    window.open(zreadUrl, '_blank', 'noopener,noreferrer');
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/SubscriptionRepoCard.tsx` around lines 239 - 242, The
window.open call in handleOpenInZRead currently opens an external URL using
'_blank' without preventing access to window.opener; update the call to include
the noopener and noreferrer flags (pass 'noopener,noreferrer' as the third
argument to window.open) and, for extra compatibility, capture the returned
window and set its opener to null (e.g., const win = window.open(..., '_blank',
'noopener,noreferrer'); if (win) win.opener = null) so repo.full_name navigation
cannot access the opener.
src/components/SubscriptionRepoCard.tsx-298-304 (1)

298-304: ⚠️ Potential issue | 🟡 Minor

Clear stale AI fields on failed re-analysis.

The failed result includes empty summary/tags/platforms, but this path only persists the timestamp and failure flag. Discovery cards can keep showing stale AI content after a retry fails.

🐛 Proposed fix
 const failedRepo: DiscoveryRepo = {
   ...repo,
+  ai_summary: failedResult.summary,
+  ai_tags: failedResult.tags,
+  ai_platforms: failedResult.platforms,
   analyzed_at: failedResult.analyzed_at,
   analysis_failed: failedResult.analysis_failed,
 };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/SubscriptionRepoCard.tsx` around lines 298 - 304, The failure
path currently builds failedRepo using createFailedAnalysisResult but only
updates analyzed_at and analysis_failed, leaving stale AI fields (e.g., summary,
tags, platforms, maybe ai_generated_fields) intact; modify the failedRepo
creation used by updateDiscoveryRepo to explicitly clear/zero out AI-derived
fields (set summary to empty string, tags and platforms to empty arrays and any
other AI fields on DiscoveryRepo) before calling updateDiscoveryRepo so the UI
won't display stale analysis content after a failed re-analysis.
src/utils/clipboardUtils.ts-81-99 (1)

81-99: ⚠️ Potential issue | 🟡 Minor

Always clean up the fallback textarea.

If focus, select, or execCommand throws after appendChild, the hidden textarea remains in the DOM.

🐛 Proposed fix
     // 尝试使用降级方案(execCommand)
+    let textarea: HTMLTextAreaElement | null = null;
     try {
-      const textarea = document.createElement('textarea');
+      textarea = document.createElement('textarea');
       textarea.value = text;
       textarea.style.position = 'fixed';
       textarea.style.left = '-999999px';
       textarea.style.top = '-999999px';
       document.body.appendChild(textarea);
@@
 
       const success = document.execCommand('copy');
-      document.body.removeChild(textarea);
 
       if (success) {
         return { success: true };
       }
     } catch {
       // 降级方案失败,返回错误
+    } finally {
+      textarea?.parentNode?.removeChild(textarea);
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/clipboardUtils.ts` around lines 81 - 99, Ensure the fallback
textarea is always removed from the DOM if appended: after creating and
appending the textarea in src/utils/clipboardUtils.ts (the textarea variable and
the current document.body.appendChild(textarea) call), wrap the operations that
can throw (focus, select, execCommand) in a try/finally and in finally check
textarea && textarea.parentNode and call document.body.removeChild(textarea) so
the element is cleaned up even on errors; keep existing success handling but
ensure removal happens before returning or rethrowing.
src/hooks/useStoreSelectors.ts-259-281 (1)

259-281: ⚠️ Potential issue | 🟡 Minor

Use customized metadata when filtering repositories.

This filter ignores custom_tags and custom_description, so a user-edited card can display custom metadata while category filtering still uses stale AI/original metadata.

🐛 Proposed fix
-      if (repo.ai_tags && repo.ai_tags.length > 0) {
-        return repo.ai_tags.some(
+      const categoryTags = repo.custom_tags !== undefined ? repo.custom_tags : repo.ai_tags;
+      if (categoryTags && categoryTags.length > 0) {
+        return categoryTags.some(
           (tag) =>
             selectedCategoryObj.keywords.some(
               (keyword) =>
@@
       const repoText = [
         repo.name,
-        repo.description || '',
+        repo.custom_description !== undefined ? repo.custom_description : repo.description || '',
         repo.language || '',
         ...(repo.topics || []),
         repo.ai_summary || '',
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/hooks/useStoreSelectors.ts` around lines 259 - 281, The category filter
ignores user-edited metadata (custom_tags and custom_description), causing
filtering to use stale AI/original fields; update the filtering logic around
repo.ai_tags and repoText to include repo.custom_tags (merge with
ai_tags/topics) and repo.custom_description (include in repoText) so
selectedCategoryObj.keyword checks consider user-provided custom_tags and
custom_description; ensure you handle missing fields (fallback to empty
array/string) and retain the current case-insensitive matching used with
selectedCategoryObj.keywords.
src/components/DiscoverySidebar.tsx-62-87 (1)

62-87: ⚠️ Potential issue | 🟡 Minor

Expose refresh and selected-channel state to assistive tech.

The icon-only refresh button needs an explicit accessible name, and channel selection should be reflected semantically.

♿ Proposed fix
 <button
   onClick={onRefreshAll}
   disabled={anyLoading || isAnalyzing}
+  aria-label={t('刷新全部', 'Refresh All')}
+  aria-busy={anyLoading}
   className="p-1.5 rounded-lg bg-blue-100 text-blue-600 dark:bg-blue-900 dark:text-blue-400 hover:bg-blue-200 dark:hover:bg-blue-800 transition-colors disabled:opacity-50 disabled:cursor-not-allowed"
   title={t('刷新全部', 'Refresh All')}
 >
@@
 <button
   key={channel.id}
   onClick={() => onChannelSelect(channel.id)}
+  aria-current={isSelected ? 'true' : undefined}
   className={`flex w-full items-center justify-between px-3 py-2.5 rounded-lg text-left transition-all duration-200 ${
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/DiscoverySidebar.tsx` around lines 62 - 87, Add an accessible
name to the icon-only refresh button by setting an explicit aria-label (e.g.,
aria-label={t('刷新全部','Refresh All')}) on the button that calls onRefreshAll and
keep the disabled handling; for each channel button rendered in enabledChannels
(the element that calls onChannelSelect(channel.id) and uses selectedChannel,
channelLoading, channelLastRefresh), expose selection state to assistive tech by
adding aria-pressed={isSelected} (or aria-current="true" / aria-selected as
appropriate for your semantics) and include a descriptive aria-label if the
visible text isn’t sufficient; ensure these attributes are added to the same
button element that currently uses the className conditional and
key={channel.id}.
src/components/MarkdownRenderer.tsx-59-67 (1)

59-67: ⚠️ Potential issue | 🟡 Minor

Reset highlight.js state before re-highlighting.

highlightElement sets dataset.highlighted = "true" on the <code> element after highlighting. When children or normalizedLanguage changes, calling it again on the same node triggers a warning and skips highlighting. Remove the attribute before re-highlighting to ensure the highlight updates.

🐛 Proposed fix
   useEffect(() => {
     if (codeRef.current) {
       try {
+        delete codeRef.current.dataset.highlighted;
         hljs.highlightElement(codeRef.current);
       } catch (error) {
         console.warn('highlight.js failed:', error);
       }
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/MarkdownRenderer.tsx` around lines 59 - 67, Before calling
hljs.highlightElement in the useEffect, clear any previous highlight marker on
the node: check codeRef.current and delete codeRef.current.dataset.highlighted
(or removeAttribute('data-highlighted')) so highlightElement can run again;
update the useEffect block surrounding codeRef.current, hljs.highlightElement,
and the catch to remove the attribute before calling hljs.highlightElement.
src/components/MarkdownRenderer.tsx-69-89 (1)

69-89: ⚠️ Potential issue | 🟡 Minor

Prevent code corruption when children contain arrays or elements.

When children is an array (from markdown parsing), String(children) joins with commas. When it's a JSX element, it produces [object Object]. Use React.Children.toArray() to normalize properly.

Proposed fix
+  const codeText = useMemo(() => {
+    if (typeof children === 'string') return children;
+    return React.Children.toArray(children).join('');
+  }, [children]);
+
   const handleCopy = useCallback(async () => {
-    const codeText = typeof children === 'string'
-      ? children
-      : String(children);
-
     setCopyError(null);
 
     const result = await safeWriteText(codeText);
@@
-  }, [children, uiLanguage]);
+  }, [codeText, uiLanguage]);
 
-  const codeLines = typeof children === 'string' ? children.split('\n') : [];
+  const codeLines = codeText.split('\n');
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/MarkdownRenderer.tsx` around lines 69 - 89, The copy and
line-number logic fails when children is an array or React element because
String(children) joins with commas or yields “[object Object]”; update
handleCopy and the codeLines/showLineNumbers logic to normalize children using
React.Children.toArray(children), map each entry to its textual content (e.g.,
keep strings, extract child.props.children or fallback to empty string), then
join the pieces with '' (or '\n' when building codeLines) and pass that joined
string to safeWriteText and to split('\n'); update references in handleCopy,
codeLines, lineCount and showLineNumbers to use this normalized string instead
of String(children).
src/utils/pagination.ts-25-31 (1)

25-31: ⚠️ Potential issue | 🟡 Minor

Compute navigation flags from the clamped page.

currentPage is normalized in the return object, but hasNextPage / hasPreviousPage still use the raw input. For currentPage = 2, totalPages = 0, this returns currentPage: 1 with hasPreviousPage: true.

Proposed fix
 export function calculatePaginationState(
   currentPage: number,
   totalPages: number,
   totalCount: number
 ): PaginationState {
+  const normalizedTotalPages = Math.max(0, totalPages);
+  const normalizedCurrentPage = Math.max(1, Math.min(currentPage, normalizedTotalPages || 1));
+
   return {
-    currentPage: Math.max(1, Math.min(currentPage, totalPages || 1)),
-    totalPages: Math.max(0, totalPages),
+    currentPage: normalizedCurrentPage,
+    totalPages: normalizedTotalPages,
     totalCount: Math.max(0, totalCount),
-    hasNextPage: currentPage < totalPages,
-    hasPreviousPage: currentPage > 1,
+    hasNextPage: normalizedCurrentPage < normalizedTotalPages,
+    hasPreviousPage: normalizedTotalPages > 0 && normalizedCurrentPage > 1,
   };
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/pagination.ts` around lines 25 - 31, The navigation flags are
computed from the raw inputs so hasNextPage/hasPreviousPage can be inconsistent
with the returned currentPage; change the return to compute a clampedCurrentPage
(derived from currentPage, totalPages) and use that clamped value when
evaluating hasNextPage and hasPreviousPage (while still clamping totalPages and
totalCount); update references to currentPage in the return object to use
clampedCurrentPage so functions/methods that read currentPage, totalPages,
totalCount, hasNextPage, and hasPreviousPage are consistent.
src/utils/usePagination.ts-53-57 (1)

53-57: ⚠️ Potential issue | 🟡 Minor

Clamp pageRange the same way as pageItems.

For an empty result set this returns { start: 1, end: 0 }, and out-of-range pages can produce bounds that don’t match the sliced items.

Proposed fix
   const pageRange = useMemo(() => {
-    const start = (currentPage - 1) * ITEMS_PER_PAGE + 1;
-    const end = Math.min(currentPage * ITEMS_PER_PAGE, totalCount);
+    if (totalCount <= 0 || totalPages <= 0) {
+      return { start: 0, end: 0 };
+    }
+
+    const safePage = Math.max(1, Math.min(currentPage, totalPages));
+    const start = (safePage - 1) * ITEMS_PER_PAGE + 1;
+    const end = Math.min(safePage * ITEMS_PER_PAGE, totalCount);
     return { start, end };
-  }, [currentPage, totalCount]);
+  }, [currentPage, totalCount, totalPages]);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/utils/usePagination.ts` around lines 53 - 57, The pageRange calculation
in usePagination returns invalid bounds for empty or out-of-range pages (e.g.,
{start:1,end:0}); update the pageRange logic in usePagination so start and end
are clamped to the actual available items like pageItems does: compute start and
end as before but then set end = Math.max(0, end) and set start =
Math.min(Math.max(0, start), end) (or set start to 0 when totalCount === 0) so
start is never greater than end and both are non-negative; ensure you update the
pageRange useMemo dependencies and keep variable names pageRange, currentPage,
totalCount, ITEMS_PER_PAGE and pageItems for clarity.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ea072668-9122-41a2-bfce-dc727a854534

📥 Commits

Reviewing files that changed from the base of the PR and between bae04bc and 839fbf3.

⛔ Files ignored due to path filters (4)
  • dist/index.html is excluded by !**/dist/**
  • package-lock.json is excluded by !**/package-lock.json
  • public/fonts/inter-latin-ext.woff2 is excluded by !**/*.woff2
  • public/fonts/inter-latin.woff2 is excluded by !**/*.woff2
📒 Files selected for processing (42)
  • index.html
  • package.json
  • public/fonts/inter.css
  • src/App.tsx
  • src/components/BackToTop.tsx
  • src/components/DiscoverySidebar.tsx
  • src/components/DiscoveryView.tsx
  • src/components/ErrorBoundary.tsx
  • src/components/Header.tsx
  • src/components/LoginScreen.tsx
  • src/components/MarkdownRenderer.tsx
  • src/components/Modal.tsx
  • src/components/OptimizedImage.tsx
  • src/components/ReadmeModal.tsx
  • src/components/RepositoryCard.tsx
  • src/components/RepositoryList.tsx
  • src/components/ScrollToBottom.tsx
  • src/components/SortAlgorithmTooltip.tsx
  • src/components/SubscriptionDevCard.tsx
  • src/components/SubscriptionRepoCard.tsx
  • src/components/SubscriptionSidebar.tsx
  • src/components/SubscriptionView.tsx
  • src/components/VirtualList.tsx
  • src/components/settings/DataManagementPanel.tsx
  • src/components/settings/GeneralPanel.tsx
  • src/constants/project.ts
  • src/hooks/useModalVisibility.ts
  • src/hooks/useStoreSelectors.ts
  • src/index.css
  • src/services/aiAnalysisHelper.ts
  • src/services/aiService.ts
  • src/services/githubApi.ts
  • src/services/rssTrendingService.ts
  • src/services/updateService.ts
  • src/store/useAppStore.ts
  • src/types/index.ts
  • src/utils/clipboardUtils.ts
  • src/utils/pagination.ts
  • src/utils/performanceMonitor.ts
  • src/utils/usePagination.ts
  • tailwind.config.js
  • vite.config.ts
💤 Files with no reviewable changes (3)
  • src/components/SubscriptionView.tsx
  • src/components/SubscriptionDevCard.tsx
  • src/components/SubscriptionSidebar.tsx

Comment thread package.json
Comment on lines +39 to +43
"@vitejs/plugin-legacy": "^8.0.1",
"@vitejs/plugin-react": "^6.0.0",
"autoprefixer": "^10.4.18",
"concurrently": "^8.2.0",
"esbuild": "^0.28.0",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Verify that the requested versions exist and inspect engines / peer deps.
npm view vite@8.0.0 version engines peerDependencies dependencies
npm view `@vitejs/plugin-react`@6.0.0 version engines peerDependencies dependencies
npm view `@vitejs/plugin-legacy`@8.0.1 version engines peerDependencies dependencies
npm view esbuild@0.28.0 version engines

# Check whether legacy plugin is actually configured in the repo.
rg -nP -C4 "from ['\"]@vitejs/plugin-legacy['\"]|legacy\\s*\\(" --type ts --type js

# Confirm whether terser is declared anywhere.
node - <<'NODE'
const pkg = require('./package.json');
const all = { ...pkg.dependencies, ...pkg.devDependencies, ...pkg.optionalDependencies };
console.log(JSON.stringify({
  terserDeclared: Boolean(all.terser),
  terserVersion: all.terser || null,
}, null, 2));
NODE

Repository: AmintaCCCP/GithubStarsManager

Length of output: 2190


Add terser to devDependencies before merging.

@vitejs/plugin-legacy declares terser as a required peer dependency, which is not listed in package.json. Since the plugin is actively configured in vite.config.ts, production builds will fail without it. Install terser@^5.16.0 to satisfy the peer dependency.

Vite 8.0.0, @vitejs/plugin-react 6.0.0, @vitejs/plugin-legacy 8.0.1, and esbuild 0.28.0 are compatible as a set.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@package.json` around lines 39 - 43, The package.json is missing the peer
dependency "terser" required by "@vitejs/plugin-legacy"; add "terser": "^5.16.0"
to devDependencies in package.json (alongside existing dev deps like
"@vitejs/plugin-legacy" and "esbuild"), then run your package manager to update
the lockfile (npm install / yarn install / pnpm install) so production builds
that use vite.config.ts succeed.

Comment on lines +1089 to +1098
const refreshChannel = useCallback(async (channelId: DiscoveryChannelId, page: number = 1, append: boolean = false) => {
console.log(`[Discovery] Refreshing channel ${channelId}, page: ${page}, append: ${append}`);

if (!githubToken) {
alert(t('GitHub Token 未找到,请重新登录。', 'GitHub token not found. Please login again.'));
return;
}

setDiscoveryLoading(channelId, true);
try {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Synchronize the stored current page when refreshing a specific page.

Several callers refresh page 1 directly, but discoveryCurrentPage is not reset. If the user is on page 3 and changes filters or clicks refresh, page-1 data can be rendered while the UI still says page 3.

Proposed fix
   const refreshChannel = useCallback(async (channelId: DiscoveryChannelId, page: number = 1, append: boolean = false) => {
     console.log(`[Discovery] Refreshing channel ${channelId}, page: ${page}, append: ${append}`);
     
     if (!githubToken) {
       alert(t('GitHub Token 未找到,请重新登录。', 'GitHub token not found. Please login again.'));
       return;
     }
 
+    if (!append) {
+      setDiscoveryCurrentPage(channelId, Math.max(1, page));
+    }
+
     setDiscoveryLoading(channelId, true);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const refreshChannel = useCallback(async (channelId: DiscoveryChannelId, page: number = 1, append: boolean = false) => {
console.log(`[Discovery] Refreshing channel ${channelId}, page: ${page}, append: ${append}`);
if (!githubToken) {
alert(t('GitHub Token 未找到,请重新登录。', 'GitHub token not found. Please login again.'));
return;
}
setDiscoveryLoading(channelId, true);
try {
const refreshChannel = useCallback(async (channelId: DiscoveryChannelId, page: number = 1, append: boolean = false) => {
console.log(`[Discovery] Refreshing channel ${channelId}, page: ${page}, append: ${append}`);
if (!githubToken) {
alert(t('GitHub Token 未找到,请重新登录。', 'GitHub token not found. Please login again.'));
return;
}
if (!append) {
setDiscoveryCurrentPage(channelId, Math.max(1, page));
}
setDiscoveryLoading(channelId, true);
try {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/DiscoveryView.tsx` around lines 1089 - 1098, The
refreshChannel function updates data but doesn't update the stored
discoveryCurrentPage state, so callers that refresh page 1 can leave the UI
showing the old page number; inside refreshChannel (the function declared as
refreshChannel) after validating githubToken and before/after
setDiscoveryLoading, update the discoveryCurrentPage state to the page argument
(e.g., call setDiscoveryCurrentPage(channelId, page) or the appropriate setter
for discoveryCurrentPage) unless append is true if you deliberately want to
preserve the current page when appending; ensure the state update runs whenever
a non-append refresh is requested so the UI and data stay in sync.

Comment on lines +1102 to +1121
switch (channelId) {
case 'trending':
result = await githubApi.getTrendingRepositories(discoveryPlatform, page, 20, trendingParams);
break;
case 'topic':
if (discoverySelectedTopic) {
result = await githubApi.getTopicRepositories(discoverySelectedTopic, discoveryPlatform, page, 20, topicParams);
} else {
result = await githubApi.getTrendingRepositories(discoveryPlatform, page, 20, trendingParams);
}
break;
case 'search':
if (discoverySearchQuery.trim()) {
result = await githubApi.searchRepositories(
discoverySearchQuery,
discoveryPlatform,
discoveryLanguage,
searchParams,
page
);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Read fresh discovery params before fetching.

These callers set Zustand state and immediately call refreshChannel, but refreshChannel uses render-closure values. The first search/sort/filter request can therefore fetch with the previous query or params.

Proposed direction
       const githubApi = new GitHubApiService(githubToken);
+      const latestState = useAppStore.getState();
+      const activePlatform = latestState.discoveryPlatform;
+      const activeLanguage = latestState.discoveryLanguage;
+      const activeSearchQuery = latestState.discoverySearchQuery;
+      const activeSelectedTopic = latestState.discoverySelectedTopic;
+      const activeTrendingParams = latestState.trendingParams;
+      const activeTopicParams = latestState.topicParams;
+      const activeSearchParams = latestState.searchParams;
       let result;
 
       switch (channelId) {
         case 'trending':
-          result = await githubApi.getTrendingRepositories(discoveryPlatform, page, 20, trendingParams);
+          result = await githubApi.getTrendingRepositories(activePlatform, page, 20, activeTrendingParams);
           break;
         case 'topic':
-          if (discoverySelectedTopic) {
-            result = await githubApi.getTopicRepositories(discoverySelectedTopic, discoveryPlatform, page, 20, topicParams);
+          if (activeSelectedTopic) {
+            result = await githubApi.getTopicRepositories(activeSelectedTopic, activePlatform, page, 20, activeTopicParams);
           } else {
-            result = await githubApi.getTrendingRepositories(discoveryPlatform, page, 20, trendingParams);
+            result = await githubApi.getTrendingRepositories(activePlatform, page, 20, activeTrendingParams);
           }
           break;
         case 'search':
-          if (discoverySearchQuery.trim()) {
+          if (activeSearchQuery.trim()) {
             result = await githubApi.searchRepositories(
-              discoverySearchQuery,
-              discoveryPlatform,
-              discoveryLanguage,
-              searchParams,
+              activeSearchQuery,
+              activePlatform,
+              activeLanguage,
+              activeSearchParams,
               page
             );

Also applies to: 1222-1225, 1393-1398, 1550-1554, 1684-1688

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/DiscoveryView.tsx` around lines 1102 - 1121, refreshChannel is
using stale render-closure values (e.g. discoverySearchQuery,
discoverySelectedTopic, discoveryPlatform, discoveryLanguage, trendingParams,
topicParams, searchParams), causing the first fetch after updating Zustand state
to use previous params; update refreshChannel to read the latest values directly
from the Zustand store (e.g. call the store getter or getState() inside
refreshChannel) before executing the switch cases so each branch (cases
'trending', 'topic', 'search' and the other similar sites noted) uses the fresh
discovery params rather than the closed-over variables.

Comment on lines +1400 to +1432
const handlePageChange = useCallback((page: number) => {
console.log(`[Discovery] Page change requested: ${selectedDiscoveryChannel} -> page ${page}`);

const validPage = Math.max(1, page);

if (validPage === currentPage) {
console.log(`[Discovery] Same page, skipping`);
return;
}

setDiscoveryCurrentPage(selectedDiscoveryChannel, validPage);

if (currentIsLoading) {
console.log(`[Discovery] Currently loading, page will refresh after current load`);
return;
}

if (scrollContainerRef.current) {
scrollContainerRef.current.scrollTop = 0;
}
window.scrollTo(0, 0);
discoveryScrollPositionsRef.current[selectedDiscoveryChannel] = 0;
setDiscoveryScrollPosition(selectedDiscoveryChannel, 0);
setIsToolbarVisible(true);

if (selectedDiscoveryChannel === 'rss-trending') {
console.log(`[Discovery] RSS client pagination: switching to page ${validPage}, no API call needed`);
return;
}

console.log(`[Discovery] Triggering refresh for page ${validPage}`);
refreshChannel(selectedDiscoveryChannel, validPage, false);
}, [currentPage, currentIsLoading, selectedDiscoveryChannel, setDiscoveryCurrentPage, refreshChannel, setDiscoveryScrollPosition]);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Check loading before mutating the current page.

setDiscoveryCurrentPage runs before the loading guard. If a page jump slips through while loading, the page changes but no fetch is scheduled, leaving the UI on a new page with stale data.

Proposed fix
     if (validPage === currentPage) {
       console.log(`[Discovery] Same page, skipping`);
       return;
     }
 
-    setDiscoveryCurrentPage(selectedDiscoveryChannel, validPage);
-
     if (currentIsLoading) {
-      console.log(`[Discovery] Currently loading, page will refresh after current load`);
+      console.log(`[Discovery] Currently loading, ignoring page change`);
       return;
     }
+
+    setDiscoveryCurrentPage(selectedDiscoveryChannel, validPage);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const handlePageChange = useCallback((page: number) => {
console.log(`[Discovery] Page change requested: ${selectedDiscoveryChannel} -> page ${page}`);
const validPage = Math.max(1, page);
if (validPage === currentPage) {
console.log(`[Discovery] Same page, skipping`);
return;
}
setDiscoveryCurrentPage(selectedDiscoveryChannel, validPage);
if (currentIsLoading) {
console.log(`[Discovery] Currently loading, page will refresh after current load`);
return;
}
if (scrollContainerRef.current) {
scrollContainerRef.current.scrollTop = 0;
}
window.scrollTo(0, 0);
discoveryScrollPositionsRef.current[selectedDiscoveryChannel] = 0;
setDiscoveryScrollPosition(selectedDiscoveryChannel, 0);
setIsToolbarVisible(true);
if (selectedDiscoveryChannel === 'rss-trending') {
console.log(`[Discovery] RSS client pagination: switching to page ${validPage}, no API call needed`);
return;
}
console.log(`[Discovery] Triggering refresh for page ${validPage}`);
refreshChannel(selectedDiscoveryChannel, validPage, false);
}, [currentPage, currentIsLoading, selectedDiscoveryChannel, setDiscoveryCurrentPage, refreshChannel, setDiscoveryScrollPosition]);
const handlePageChange = useCallback((page: number) => {
console.log(`[Discovery] Page change requested: ${selectedDiscoveryChannel} -> page ${page}`);
const validPage = Math.max(1, page);
if (validPage === currentPage) {
console.log(`[Discovery] Same page, skipping`);
return;
}
if (currentIsLoading) {
console.log(`[Discovery] Currently loading, ignoring page change`);
return;
}
setDiscoveryCurrentPage(selectedDiscoveryChannel, validPage);
if (scrollContainerRef.current) {
scrollContainerRef.current.scrollTop = 0;
}
window.scrollTo(0, 0);
discoveryScrollPositionsRef.current[selectedDiscoveryChannel] = 0;
setDiscoveryScrollPosition(selectedDiscoveryChannel, 0);
setIsToolbarVisible(true);
if (selectedDiscoveryChannel === 'rss-trending') {
console.log(`[Discovery] RSS client pagination: switching to page ${validPage}, no API call needed`);
return;
}
console.log(`[Discovery] Triggering refresh for page ${validPage}`);
refreshChannel(selectedDiscoveryChannel, validPage, false);
}, [currentPage, currentIsLoading, selectedDiscoveryChannel, setDiscoveryCurrentPage, refreshChannel, setDiscoveryScrollPosition]);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/DiscoveryView.tsx` around lines 1400 - 1432, handlePageChange
mutates the current page via setDiscoveryCurrentPage before checking
currentIsLoading, which can change the UI page without scheduling a fetch; move
the loading guard so you check currentIsLoading first and return early if true
(i.e., perform the currentIsLoading check before calling
setDiscoveryCurrentPage), or defer calling setDiscoveryCurrentPage until after
you confirm not loading and will trigger refreshChannel; ensure scroll/reset
logic (scrollContainerRef, discoveryScrollPositionsRef,
setDiscoveryScrollPosition, setIsToolbarVisible) also only runs when you
actually schedule a refresh (and keep the rss-trending early-return behavior
intact).

Comment on lines +1560 to +1564
<PlatformFilter
platform={discoveryPlatform}
onPlatformChange={setDiscoveryPlatform}
language={language}
/>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Apply the platform filter immediately.

Changing the toolbar platform only updates state; it does not reset pagination or refresh the selected channel, so the visible list remains unfiltered until another manual fetch happens.

Proposed fix
                   <PlatformFilter 
                     platform={discoveryPlatform} 
-                    onPlatformChange={setDiscoveryPlatform} 
+                    onPlatformChange={(platform) => {
+                      setDiscoveryPlatform(platform);
+                      refreshChannel(selectedDiscoveryChannel, 1, false);
+                    }}
                     language={language}
                   />
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/DiscoveryView.tsx` around lines 1560 - 1564, The
PlatformFilter change currently only calls setDiscoveryPlatform so update the
onPlatformChange handler to also reset pagination and refresh the
selection/fetch: replace direct use of setDiscoveryPlatform with a wrapper that
calls setDiscoveryPlatform(newPlatform), resets pagination (e.g., setPage(1) or
resetPagination()), clears or updates the selected channel (e.g.,
setSelectedChannel(null) or selectFirstChannel()), and triggers the channel list
reload (invoke fetchChannels() or ensure the effect that depends on
discoveryPlatform runs immediately). Ensure you reference the existing symbols
PlatformFilter, discoveryPlatform, setDiscoveryPlatform,
selectedChannel/setSelectedChannel, and your pagination state setter
(setPage/resetPagination) when making the change.

Comment on lines +3 to +18
export function useModalVisibility() {
const [isModalOpen, setIsModalOpen] = useState(false);

useEffect(() => {
const handleModalOpen = () => setIsModalOpen(true);
const handleModalClose = () => setIsModalOpen(false);

window.addEventListener('gsm:modal-open', handleModalOpen);
window.addEventListener('gsm:modal-close', handleModalClose);
return () => {
window.removeEventListener('gsm:modal-open', handleModalOpen);
window.removeEventListener('gsm:modal-close', handleModalClose);
};
}, []);

return isModalOpen;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Track open modal count instead of a single boolean.

With multiple/nested modals, one gsm:modal-close sets isModalOpen to false even if another modal is still open, re-enabling background floating controls over the active modal.

Proposed fix
 export function useModalVisibility() {
-  const [isModalOpen, setIsModalOpen] = useState(false);
+  const [openModalCount, setOpenModalCount] = useState(0);
 
   useEffect(() => {
-    const handleModalOpen = () => setIsModalOpen(true);
-    const handleModalClose = () => setIsModalOpen(false);
+    const handleModalOpen = () => setOpenModalCount((count) => count + 1);
+    const handleModalClose = () => setOpenModalCount((count) => Math.max(0, count - 1));
 
     window.addEventListener('gsm:modal-open', handleModalOpen);
     window.addEventListener('gsm:modal-close', handleModalClose);
@@
-  return isModalOpen;
+  return openModalCount > 0;
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/hooks/useModalVisibility.ts` around lines 3 - 18, The hook
useModalVisibility uses a single boolean isModalOpen so a gsm:modal-close can
hide the UI even when other modals remain open; change it to track an open-modal
counter (e.g., modalCount state or ref) instead: increment the counter in
handleModalOpen, decrement (but never below zero) in handleModalClose, derive
and return the boolean as modalCount > 0; update the cleanup/addEventListener
logic around the same handlers (handleModalOpen and handleModalClose) and
replace setIsModalOpen usage with counter updates and a derived isModalOpen
boolean.

Comment on lines +37 to +40
onProgress?.('Fetching README...');
const readmeContent = backend.isAvailable
? await backend.getRepositoryReadme(owner, name)
: await githubApi.getRepositoryReadme(owner, name, signal);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Don’t drop cancellation on the backend README path.

The backend branch ignores signal, so an aborted repository analysis can continue through README fetching and then proceed unless a later call observes the abort. Add abort guards here, and ideally thread signal into backend.getRepositoryReadme as well.

Proposed guard
   onProgress?.('Fetching README...');
+  if (signal?.aborted) {
+    throw new DOMException('Analysis aborted', 'AbortError');
+  }
+
   const readmeContent = backend.isAvailable
     ? await backend.getRepositoryReadme(owner, name)
     : await githubApi.getRepositoryReadme(owner, name, signal);
+
+  if (signal?.aborted) {
+    throw new DOMException('Analysis aborted', 'AbortError');
+  }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
onProgress?.('Fetching README...');
const readmeContent = backend.isAvailable
? await backend.getRepositoryReadme(owner, name)
: await githubApi.getRepositoryReadme(owner, name, signal);
onProgress?.('Fetching README...');
if (signal?.aborted) {
throw new DOMException('Analysis aborted', 'AbortError');
}
const readmeContent = backend.isAvailable
? await backend.getRepositoryReadme(owner, name)
: await githubApi.getRepositoryReadme(owner, name, signal);
if (signal?.aborted) {
throw new DOMException('Analysis aborted', 'AbortError');
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/services/aiAnalysisHelper.ts` around lines 37 - 40, The backend README
fetch path currently ignores the AbortSignal, so aborts can be missed; update
aiAnalysisHelper.ts to (1) pass the existing signal into
backend.getRepositoryReadme(owner, name, signal) and (2) check signal.aborted
both immediately before calling and immediately after awaiting
getRepositoryReadme (throw or return early if aborted) so the repository
analysis will stop promptly; reference the functions/variables onProgress,
backend.getRepositoryReadme, githubApi.getRepositoryReadme, owner, name, and
signal to locate the change.

Comment thread src/services/githubApi.ts
Comment on lines +492 to +509
private buildDiscoverySortParams(sortBy: DiscoverySortBy, sortOrder: DiscoverySortOrder): { sort: string; order: string } {
const sortMap: Record<DiscoverySortBy, string> = {
'BestMatch': 'best-match',
'Stars': 'stars',
'Forks': 'forks',
'Updated': 'updated',
'Created': 'created',
'HelpWanted': 'help-wanted-issues',
};
const orderMap: Record<DiscoverySortOrder, string> = {
'Desc': 'desc',
'Asc': 'asc',
};
return {
sort: sortMap[sortBy],
order: orderMap[sortOrder],
};
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect the mapping and all UI/API references to Created/BestMatch sort options.
rg -n -C3 "BestMatch|Created|buildDiscoverySortParams|sort=\\$\\{sort\\}" --type=ts

Repository: AmintaCCCP/GithubStarsManager

Length of output: 10304


🌐 Web query:

GitHub Search Repositories API sort parameter valid values documentation

💡 Result:

The valid values for the sort parameter in the GitHub Search Repositories API (GET /search/repositories) are: stars, forks, help-wanted-issues, updated. Default is best match (no sort specified). These sort results by: - stars: number of stars - forks: number of forks - help-wanted-issues: number of help-wanted issues - updated: how recently the repository was updated Use with order parameter (asc or desc, default desc). Note: Older docs mentioned only stars, forks, updated; current includes help-wanted-issues. No created sort available (confirmed by secondary sources).

Citations:


Remove invalid GitHub Search API sort values best-match and created.

GitHub's Search Repositories API only accepts stars, forks, help-wanted-issues, and updated for the sort parameter. Best match is the default when sort is omitted. The current mapping sends sort=best-match (default search fails) and sort=created (created is not a valid value), causing API errors when users select these options.

Update buildDiscoverySortParams to omit sort for BestMatch and remove Created from the mapping. Change the return type to { sort?: string; order: string } and use Partial<Record<DiscoverySortBy, string>> for sortMap.

Proposed fix
-  private buildDiscoverySortParams(sortBy: DiscoverySortBy, sortOrder: DiscoverySortOrder): { sort: string; order: string } {
-    const sortMap: Record<DiscoverySortBy, string> = {
-      'BestMatch': 'best-match',
+  private buildDiscoverySortParams(sortBy: DiscoverySortBy, sortOrder: DiscoverySortOrder): { sort?: string; order: string } {
+    const sortMap: Partial<Record<DiscoverySortBy, string>> = {
       'Stars': 'stars',
       'Forks': 'forks',
       'Updated': 'updated',
-      'Created': 'created',
       'HelpWanted': 'help-wanted-issues',
     };
     const orderMap: Record<DiscoverySortOrder, string> = {
       'Desc': 'desc',
       'Asc': 'asc',
     };
     return {
       sort: sortMap[sortBy],
       order: orderMap[sortOrder],
     };
   }

Also remove or remap the Created option in UI components that target the Search Repositories endpoint (lines 512 in DiscoveryView.tsx).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/services/githubApi.ts` around lines 492 - 509, Update the
buildDiscoverySortParams function to avoid sending invalid GitHub Search API
sort values: change its return type to { sort?: string; order: string }, use a
Partial<Record<DiscoverySortBy,string>> for sortMap, remove the 'Created' entry
from the map, and make 'BestMatch' map to undefined (i.e., omit the sort field)
so that when sortBy === 'BestMatch' you return only order; ensure callers of
buildDiscoverySortParams handle the optional sort and also remove or remap the
'Created' option in the DiscoveryView UI component that targets the Search
Repositories endpoint.

Comment thread src/store/useAppStore.ts
Comment on lines +648 to +656
// 如果不存在,添加新仓库(生成新ID)
// 使用 timestamp + random 确保唯一性,避免并发时的竞态条件
const timestamp = Date.now();
const random = Math.floor(Math.random() * 10000);
const maxExistingId = state.repositories.length > 0
? Math.max(...state.repositories.map(r => r.id))
: 0;
const newId = Math.max(timestamp, maxExistingId + 1) + random;
updatedRepositories = [...state.repositories, { ...repo, id: newId }];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Preserve GitHub’s stable repository ID on insert.

New repositories already come with GitHub’s globally stable id. Replacing it with a timestamp/random ID breaks ID-keyed state such as release subscriptions, analysis tracking, and future updates by GitHub ID.

Proposed fix
         } else {
-          // 如果不存在,添加新仓库(生成新ID)
-          // 使用 timestamp + random 确保唯一性,避免并发时的竞态条件
-          const timestamp = Date.now();
-          const random = Math.floor(Math.random() * 10000);
-          const maxExistingId = state.repositories.length > 0
-            ? Math.max(...state.repositories.map(r => r.id))
-            : 0;
-          const newId = Math.max(timestamp, maxExistingId + 1) + random;
-          updatedRepositories = [...state.repositories, { ...repo, id: newId }];
+          updatedRepositories = [...state.repositories, repo];
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// 如果不存在,添加新仓库(生成新ID)
// 使用 timestamp + random 确保唯一性,避免并发时的竞态条件
const timestamp = Date.now();
const random = Math.floor(Math.random() * 10000);
const maxExistingId = state.repositories.length > 0
? Math.max(...state.repositories.map(r => r.id))
: 0;
const newId = Math.max(timestamp, maxExistingId + 1) + random;
updatedRepositories = [...state.repositories, { ...repo, id: newId }];
updatedRepositories = [...state.repositories, repo];
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/store/useAppStore.ts` around lines 648 - 656, The code is overwriting
incoming GitHub-stable IDs by always generating a timestamp/random newId; update
the insert logic in useAppStore so it preserves an existing repo.id from the
incoming repo object (use that as the id when present) and only generate a
fallback unique id when repo.id is null/undefined (e.g., const assignedId =
repo.id ?? computedFallbackId). Keep the rest of the append to
state.repositories the same (use updatedRepositories = [...state.repositories, {
...repo, id: assignedId }]) and ensure computedFallbackId still avoids
collisions by comparing with maxExistingId as currently implemented.

Comment thread vite.config.ts
Comment on lines +15 to +23
server: {
proxy: {
'/api/rss': {
target: 'https://mshibanami.github.io/GitHubTrendingRSS',
changeOrigin: true,
rewrite: (path) => path.replace(/^\/api\/rss/, ''),
},
},
},
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Dev-only proxy is not used by the RSS service and has no production equivalent.

This proxy at /api/rss is only active under vite dev server. However, src/services/rssTrendingService.ts fetches https://mshibanami.github.io/GitHubTrendingRSS/... directly (absolute URL), so the proxy is bypassed even in dev, and there is no production equivalent. Either:

  • Change the service to fetch /api/rss/<daily|weekly|monthly>/all.xml (so dev uses this proxy) and configure a production proxy/CORS-aware backend, or
  • Drop this proxy block entirely if the target actually serves CORS headers and direct fetching is intended.

See the companion comment on src/services/rssTrendingService.ts line 11–17.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@vite.config.ts` around lines 15 - 23, The dev-only proxy in vite.config.ts is
bypassed because src/services/rssTrendingService.ts fetches the absolute URL;
update rssTrendingService (the fetch call around lines 11–17) to request the
relative path /api/rss/<daily|weekly|monthly>/all.xml so the dev proxy (the
'/api/rss' entry in vite.config.ts) is used, or alternatively remove the proxy
block from vite.config.ts if you intend to fetch the absolute URL directly;
ensure the rewrite in vite.config.ts remains compatible with the chosen approach
and adjust any environment/config variables or server-side CORS/proxy setup
accordingly.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (2)
src/components/DiscoveryView.tsx (1)

1064-1067: Dead state: analysisState value is discarded and never read.

const [, setAnalysisState] = useState<{ paused: boolean; aborted: boolean }>(...) uses only the setter. Calls at lines 1341 and 1423 (setAnalysisState(prev => …)) therefore update a state no component ever renders from, and the paused/aborted intent is never consulted — the real abort is handled by analysisOptimizer?.abort(). Either wire the state into rendering/logic (e.g., gate UI or the effect loop on aborted), or drop it.

♻️ Proposed fix (if not needed)
-  const [, setAnalysisState] = useState<{ paused: boolean; aborted: boolean }>({ paused: false, aborted: false });

Then remove the setAnalysisState(...) calls at lines ~1341 and ~1423.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/DiscoveryView.tsx` around lines 1064 - 1067, The component
defines analysisState but never reads it (const [, setAnalysisState] =
useState(...)), so setAnalysisState calls (setAnalysisState(prev => ...)) update
unused state; either remove this dead state and all setAnalysisState(...) calls,
or wire the state into rendering/logic: keep the useState as const
[analysisState, setAnalysisState] = useState<{ paused: boolean; aborted: boolean
}>(...), replace direct abort logic where appropriate to consult
analysisState.aborted (e.g., in the analyzer loop/effects) and use
analysisState.paused to gate UI/processing, and ensure
analysisOptimizer?.abort() remains in sync with setting analysisState.aborted;
choose one approach and update references to analysisState, setAnalysisState,
and analysisOptimizer?.abort() accordingly.
src/App.tsx (1)

40-67: measureRender timer is started every render but captured only once.

measureRender('RepositoriesView') runs on every render and returns a fresh timer-closure, but endMeasure inside useEffect(() => { … }, []) captures only the closure from the first render. Every subsequent re-render starts a new timer that is never stopped, and the recorded metric reflects "first render body → first commit", not actual render duration. Either move measureRender into a useRef/mount-only effect, or call endMeasure after every render:

♻️ Proposed fix
 }) => {
-  const endMeasure = measureRender('RepositoriesView');
-  useEffect(() => {
-    endMeasure();
-    // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, []);
+  const endMeasure = measureRender('RepositoriesView');
+  useEffect(() => {
+    endMeasure();
+  });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/App.tsx` around lines 40 - 67, measureRender('RepositoriesView') is being
called on every render but endMeasure is only invoked once because useEffect has
an empty deps array; either call endMeasure after every render by removing the
empty deps (change useEffect(() => { endMeasure(); }) so it runs after each
render) or start the timer only on mount by storing the closure in a ref (useRef
to hold the result of measureRender in the component, then call ref.current()
inside the mount-only effect). Update the code around measureRender and
endMeasure in this component (the RepositoriesView render body and the useEffect
that currently calls endMeasure) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/App.tsx`:
- Around line 21-28: LoadingSpinner currently hardcodes the Chinese text
"加载中..."; update LoadingSpinner to produce a locale-aware label by using the
same locale-detection pattern as ErrorBoundary (prefer useAppStore().language
when available and fall back to navigator.language or the existing
locale-detection helper), then choose the localized string (e.g., English
"Loading..." vs Chinese "加载中...") accordingly so the spinner is store-agnostic
and safe at init time; locate the LoadingSpinner component in src/App.tsx and
implement the conditional selection of the label using useAppStore().language or
the ErrorBoundary locale helper.

In `@src/components/DiscoveryView.tsx`:
- Around line 1221-1244: The two effects can both call refreshChannel for
'topic' on mount causing duplicate fetches; in the second effect (the one
watching discoverySelectedTopic/selectedDiscoveryChannel) only call
refreshChannel('topic', 1, false) after checking the current channel's loading
and repo cache via useAppStore (e.g.
useAppStore.getState().discoveryIsLoading['topic'] and
useAppStore.getState().discoveryRepos['topic']) and bail out if loading is true
or repos exist; this ensures refreshChannel (the function referenced) runs only
when topic is selected AND there is no cached repos AND no in-flight load,
preventing the duplicate concurrent calls and races on setDiscoveryRepos.
- Around line 982-1016: Module-level debug arrays __discoveryLogs and
__discoveryErrors plus the helper functions dl/de are unbounded and get pushed
on every render and pinned to window, causing memory growth; remove or restrict
them: remove per-render dl(...) calls inside DiscoveryView (including the
initial dl('DiscoveryView render start') and the store-snapshot dl), and stop
exporting the live arrays to window, or gate all logging behind a dev-only flag
(e.g., process.env.NODE_ENV === 'development') and implement a bounded ring
buffer for __discoveryLogs/__discoveryErrors if you need to keep them; update
references in DiscoveryView and any handlers (useAppStore snapshot block,
handleScroll, pagination/fetch callbacks) to either not call dl/de or call the
gated/bounded logger instead.

---

Nitpick comments:
In `@src/App.tsx`:
- Around line 40-67: measureRender('RepositoriesView') is being called on every
render but endMeasure is only invoked once because useEffect has an empty deps
array; either call endMeasure after every render by removing the empty deps
(change useEffect(() => { endMeasure(); }) so it runs after each render) or
start the timer only on mount by storing the closure in a ref (useRef to hold
the result of measureRender in the component, then call ref.current() inside the
mount-only effect). Update the code around measureRender and endMeasure in this
component (the RepositoriesView render body and the useEffect that currently
calls endMeasure) accordingly.

In `@src/components/DiscoveryView.tsx`:
- Around line 1064-1067: The component defines analysisState but never reads it
(const [, setAnalysisState] = useState(...)), so setAnalysisState calls
(setAnalysisState(prev => ...)) update unused state; either remove this dead
state and all setAnalysisState(...) calls, or wire the state into
rendering/logic: keep the useState as const [analysisState, setAnalysisState] =
useState<{ paused: boolean; aborted: boolean }>(...), replace direct abort logic
where appropriate to consult analysisState.aborted (e.g., in the analyzer
loop/effects) and use analysisState.paused to gate UI/processing, and ensure
analysisOptimizer?.abort() remains in sync with setting analysisState.aborted;
choose one approach and update references to analysisState, setAnalysisState,
and analysisOptimizer?.abort() accordingly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a3bea1aa-239d-49a4-972b-a0a3601cdf5d

📥 Commits

Reviewing files that changed from the base of the PR and between 839fbf3 and ca9cf0f.

📒 Files selected for processing (2)
  • src/App.tsx
  • src/components/DiscoveryView.tsx

Comment thread src/App.tsx
Comment on lines +21 to +28
const LoadingSpinner = () => (
<div className="flex items-center justify-center min-h-[400px]">
<div className="flex flex-col items-center gap-3">
<div className="w-8 h-8 border-3 border-blue-500 border-t-transparent rounded-full animate-spin" />
<span className="text-sm text-gray-500 dark:text-gray-400">加载中...</span>
</div>
</div>
);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

LoadingSpinner hardcodes Chinese "加载中..." .

This fallback is used app-wide (including the full-screen !isInitialized gate), but the rest of the app localizes via useAppStore().language / the locale detection pattern used in ErrorBoundary.tsx. English users will see Chinese text during every lazy-view load.

🌐 Proposed fix (store-agnostic, safe at init time)
-const LoadingSpinner = () => (
+const LoadingSpinner = () => {
+  const label = navigator.language?.startsWith('zh') ? '加载中...' : 'Loading...';
+  return (
     <div className="flex items-center justify-center min-h-[400px]">
       <div className="flex flex-col items-center gap-3">
         <div className="w-8 h-8 border-3 border-blue-500 border-t-transparent rounded-full animate-spin" />
-        <span className="text-sm text-gray-500 dark:text-gray-400">加载中...</span>
+      <span className="text-sm text-gray-500 dark:text-gray-400">{label}</span>
       </div>
     </div>
-);
+  );
+};
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const LoadingSpinner = () => (
<div className="flex items-center justify-center min-h-[400px]">
<div className="flex flex-col items-center gap-3">
<div className="w-8 h-8 border-3 border-blue-500 border-t-transparent rounded-full animate-spin" />
<span className="text-sm text-gray-500 dark:text-gray-400">加载中...</span>
</div>
</div>
);
const LoadingSpinner = () => {
const label = navigator.language?.startsWith('zh') ? '加载中...' : 'Loading...';
return (
<div className="flex items-center justify-center min-h-[400px]">
<div className="flex flex-col items-center gap-3">
<div className="w-8 h-8 border-3 border-blue-500 border-t-transparent rounded-full animate-spin" />
<span className="text-sm text-gray-500 dark:text-gray-400">{label}</span>
</div>
</div>
);
};
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/App.tsx` around lines 21 - 28, LoadingSpinner currently hardcodes the
Chinese text "加载中..."; update LoadingSpinner to produce a locale-aware label by
using the same locale-detection pattern as ErrorBoundary (prefer
useAppStore().language when available and fall back to navigator.language or the
existing locale-detection helper), then choose the localized string (e.g.,
English "Loading..." vs Chinese "加载中...") accordingly so the spinner is
store-agnostic and safe at init time; locate the LoadingSpinner component in
src/App.tsx and implement the conditional selection of the label using
useAppStore().language or the ErrorBoundary locale helper.

Comment on lines +982 to +1016
// 全局错误日志存储
const __discoveryLogs: { ts: string; msg: string; data?: unknown }[] = [];
const __discoveryErrors: { ts: string; msg: string; err?: unknown; stack?: string }[] = [];

try {
(window as unknown as Record<string, unknown>).__discoveryLogs = __discoveryLogs;
(window as unknown as Record<string, unknown>).__discoveryErrors = __discoveryErrors;
} catch (_) {}

const dl = (msg: string, data?: unknown) => {
const entry = { ts: new Date().toISOString(), msg, data };
__discoveryLogs.push(entry);
console.log(`[DV] ${msg}`, data || '');
};
const de = (msg: string, err?: unknown) => {
const entry = { ts: new Date().toISOString(), msg, err, stack: err instanceof Error ? err.stack : undefined };
__discoveryErrors.push(entry);
console.error(`[DV ERROR] ${msg}`, err || '');
};

export const DiscoveryView: React.FC = React.memo(() => {
dl('DiscoveryView render start');

let storeSnapshot: Record<string, unknown> = {};
try {
const s = useAppStore.getState();
storeSnapshot = {
selChannel: s.selectedDiscoveryChannel,
chanKeys: s.discoveryChannels ? Object.keys(s.discoveryChannels) : 'N/A',
reposKeys: s.discoveryRepos ? Object.keys(s.discoveryRepos) : 'N/A',
currPageKeys: s.discoveryCurrentPage ? Object.keys(s.discoveryCurrentPage) : 'N/A',
isLoadingKeys: s.discoveryIsLoading ? Object.keys(s.discoveryIsLoading) : 'N/A',
};
dl('Store state snapshot', storeSnapshot);
} catch (e) { de('Failed to get store snapshot', e); }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Unbounded debug log buffers grow every render — memory leak in long sessions.

__discoveryLogs/__discoveryErrors are module-level arrays that dl()/de() push into with no cap, and they are also pinned on window. dl('DiscoveryView render start'), the store-snapshot block, and dl('After store destructuring', …) all fire on every render (plus every handleScroll, fetch, pagination update, etc. will push more through the flow). Users who keep the Discovery tab open for extended periods will accumulate thousands of entries and their referenced data/err payloads, which then can't be GC'd because window holds them.

This looks like diagnostic scaffolding from chasing the whitescreen — recommend removing before release, or at minimum: (1) gate behind a dev flag, (2) cap with a ring buffer, and (3) drop the per-render dl() calls. The store-snapshot in particular serializes object keys on every render.

🛡️ Minimum-viable fix
-const __discoveryLogs: { ts: string; msg: string; data?: unknown }[] = [];
-const __discoveryErrors: { ts: string; msg: string; err?: unknown; stack?: string }[] = [];
-
-try {
-  (window as unknown as Record<string, unknown>).__discoveryLogs = __discoveryLogs;
-  (window as unknown as Record<string, unknown>).__discoveryErrors = __discoveryErrors;
-} catch (_) {}
-
-const dl = (msg: string, data?: unknown) => {
-  const entry = { ts: new Date().toISOString(), msg, data };
-  __discoveryLogs.push(entry);
-  console.log(`[DV] ${msg}`, data || '');
-};
-const de = (msg: string, err?: unknown) => {
-  const entry = { ts: new Date().toISOString(), msg, err, stack: err instanceof Error ? err.stack : undefined };
-  __discoveryErrors.push(entry);
-  console.error(`[DV ERROR] ${msg}`, err || '');
-};
+const MAX_LOG_ENTRIES = 200;
+const __discoveryLogs: { ts: string; msg: string; data?: unknown }[] = [];
+const __discoveryErrors: { ts: string; msg: string; err?: unknown; stack?: string }[] = [];
+const DEBUG = import.meta.env.DEV;
+
+if (DEBUG) {
+  try {
+    (window as unknown as Record<string, unknown>).__discoveryLogs = __discoveryLogs;
+    (window as unknown as Record<string, unknown>).__discoveryErrors = __discoveryErrors;
+  } catch { /* noop */ }
+}
+
+const dl = (msg: string, data?: unknown) => {
+  if (!DEBUG) return;
+  if (__discoveryLogs.length >= MAX_LOG_ENTRIES) __discoveryLogs.shift();
+  __discoveryLogs.push({ ts: new Date().toISOString(), msg, data });
+};
+const de = (msg: string, err?: unknown) => {
+  if (__discoveryErrors.length >= MAX_LOG_ENTRIES) __discoveryErrors.shift();
+  __discoveryErrors.push({ ts: new Date().toISOString(), msg, err, stack: err instanceof Error ? err.stack : undefined });
+  console.error(`[DV ERROR] ${msg}`, err || '');
+};

Also remove the per-render dl(...) calls at lines 1003, 1015, and 1062.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/DiscoveryView.tsx` around lines 982 - 1016, Module-level debug
arrays __discoveryLogs and __discoveryErrors plus the helper functions dl/de are
unbounded and get pushed on every render and pinned to window, causing memory
growth; remove or restrict them: remove per-render dl(...) calls inside
DiscoveryView (including the initial dl('DiscoveryView render start') and the
store-snapshot dl), and stop exporting the live arrays to window, or gate all
logging behind a dev-only flag (e.g., process.env.NODE_ENV === 'development')
and implement a bounded ring buffer for __discoveryLogs/__discoveryErrors if you
need to keep them; update references in DiscoveryView and any handlers
(useAppStore snapshot block, handleScroll, pagination/fetch callbacks) to either
not call dl/de or call the gated/bounded logger instead.

Comment on lines +1221 to +1244
useEffect(() => {
const savedPage = useAppStore.getState().discoveryCurrentPage[selectedDiscoveryChannel] || 1;
console.log(`[Discovery] Channel changed to ${selectedDiscoveryChannel}, current page: ${savedPage}`);
if (scrollContainerRef.current) {
const savedPosition = discoveryScrollPositionsRef.current[selectedDiscoveryChannel] || 0;
scrollContainerRef.current.scrollTop = savedPosition;
}

const currentRepos = useAppStore.getState().discoveryRepos[selectedDiscoveryChannel];
if ((!currentRepos || currentRepos.length === 0) && githubToken) {
const channelLoading = useAppStore.getState().discoveryIsLoading[selectedDiscoveryChannel];
if (!channelLoading) {
console.log(`[Discovery] Auto-loading empty channel: ${selectedDiscoveryChannel}, page: ${savedPage}`);
refreshChannel(selectedDiscoveryChannel, savedPage, false);
}
}
}, [selectedDiscoveryChannel, githubToken, refreshChannel]);

// 主题改变时刷新数据
useEffect(() => {
if (selectedDiscoveryChannel === 'topic' && discoverySelectedTopic) {
refreshChannel('topic', 1, false);
}
}, [discoverySelectedTopic, selectedDiscoveryChannel, refreshChannel]);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Duplicate fetch on mount for the topic channel.

The effect at lines 1221–1237 auto-loads the current channel when its repo list is empty, and the effect at 1240–1244 additionally fires refreshChannel('topic', 1, false) whenever discoverySelectedTopic/selectedDiscoveryChannel changes — including initial mount. When the user lands on the topic channel with an already-selected topic and no cached repos, both effects fire synchronously at mount: the channelLoading guard in the first effect has not yet seen loading=true (the second effect's setDiscoveryLoading is async via set()), so two concurrent refreshChannel('topic', 1, false) calls can go out, doubling API usage and causing races on setDiscoveryRepos.

🛡️ Proposed fix
-  // 主题改变时刷新数据
-  useEffect(() => {
-    if (selectedDiscoveryChannel === 'topic' && discoverySelectedTopic) {
-      refreshChannel('topic', 1, false);
-    }
-  }, [discoverySelectedTopic, selectedDiscoveryChannel, refreshChannel]);
+  // 主题改变时刷新(跳过首次 mount,由频道切换 effect 负责初次加载)
+  const isInitialTopicMount = useRef(true);
+  useEffect(() => {
+    if (isInitialTopicMount.current) {
+      isInitialTopicMount.current = false;
+      return;
+    }
+    if (selectedDiscoveryChannel === 'topic' && discoverySelectedTopic) {
+      refreshChannel('topic', 1, false);
+    }
+  }, [discoverySelectedTopic, selectedDiscoveryChannel, refreshChannel]);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/DiscoveryView.tsx` around lines 1221 - 1244, The two effects
can both call refreshChannel for 'topic' on mount causing duplicate fetches; in
the second effect (the one watching
discoverySelectedTopic/selectedDiscoveryChannel) only call
refreshChannel('topic', 1, false) after checking the current channel's loading
and repo cache via useAppStore (e.g.
useAppStore.getState().discoveryIsLoading['topic'] and
useAppStore.getState().discoveryRepos['topic']) and bail out if loading is true
or repos exist; this ensures refreshChannel (the function referenced) runs only
when topic is selected AND there is no cached repos AND no in-flight load,
preventing the duplicate concurrent calls and races on setDiscoveryRepos.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (4)
src/components/DiscoveryView.tsx (4)

298-298: Missing displayName on memoized components.

Only CompactPagination gets displayName (line 980). TrendingFilter, RSSFilter, SortSelector, and Pagination are all React.memo(...) wrappers with no displayName, so they show up as Memo / Anonymous in React DevTools and error stacks — which is especially painful given the DiscoveryViewErrorBoundary at lines 1866–1868 logs info.componentStack. Add XYZ.displayName = 'XYZ' for each.

Also applies to: 471-471, 506-506, 563-563, 806-806

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/DiscoveryView.tsx` at line 298, Memoized components
TrendingFilter, RSSFilter, SortSelector, and Pagination lack displayName which
causes anonymous entries in DevTools and stack traces; for each memoized wrapper
(the React.memo(...) exports) add a displayName assignment like
TrendingFilter.displayName = 'TrendingFilter' (and similarly for RSSFilter,
SortSelector, Pagination) after their declarations so they appear with
meaningful names in React DevTools and in DiscoveryViewErrorBoundary logs; note
CompactPagination already has a displayName and use that same pattern for the
other memoized components.

1471-1477: refreshAll serializes every channel — parallelize it.

Each await refreshChannel(...) blocks on the full round trip before the next channel starts, so refreshing 4 enabled channels costs the sum of all latencies and the user stares at partial loading indicators that light up one-by-one. refreshChannel already manages its own per-channel loading state, so Promise.allSettled is safe and preserves the "one channel failing doesn't abort the rest" behavior (the current for..of already short-circuits on throw — which is arguably a bug too).

♻️ Proposed fix
   const refreshAll = useCallback(async () => {
     const channels = discoveryChannels || [];
     const enabledChannels = channels.filter(ch => ch && ch.enabled);
-    for (const channel of enabledChannels) {
-      await refreshChannel(channel.id, 1, false);
-    }
+    await Promise.allSettled(
+      enabledChannels.map(channel => refreshChannel(channel.id, 1, false))
+    );
   }, [discoveryChannels, refreshChannel]);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/DiscoveryView.tsx` around lines 1471 - 1477, refreshAll
currently serializes calls to refreshChannel causing slow cumulative latency;
change it to run all refreshes in parallel by mapping enabledChannels to calls
to refreshChannel(channel.id, 1, false) and await Promise.allSettled(...) on
that array so each channel refresh runs concurrently and failures don't abort
others; keep references to discoveryChannels, refreshChannel and enabledChannels
intact so per-channel loading state already handled by refreshChannel.

554-978: Consolidate Pagination and CompactPagination — ~400 lines of duplicated logic.

The two components share essentially identical input-edit state, sync effect, focus effect, handleInputChange/handleInputSubmit/handleInputKeyDown/handleInputBlur, and prev/next handlers. They only differ in the rendered shell (full page-number list vs. single editable page indicator) and minor copy. Extract a shared usePageInput({ currentPage, totalPages, onPageChange, t }) hook returning { inputPage, isEditing, error, inputRef, startEdit, handlers }, then render two thin presentational components on top. This eliminates drift risk (e.g., handlePreviousClick/handleNextClick in CompactPagination guard on !isLoading while Pagination does not — easy to miss).

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/DiscoveryView.tsx` around lines 554 - 978, Pagination and
CompactPagination duplicate ~400 lines of input/edit state and handlers; extract
that shared logic into a new custom hook usePageInput({ currentPage, totalPages,
onPageChange, t, isLoading? }) which returns { inputPage, isEditing, error,
inputRef, startEdit, handleInputChange, handleInputSubmit, handleInputKeyDown,
handleInputBlur, handlePreviousClick, handleNextClick } and update Pagination
and CompactPagination to be thin presenters that consume the hook (ensure the
hook accepts isLoading so both components use the same prev/next guards and
error messages and keep t for localized messages); move console.log calls into
the hook and keep render differences (page list vs compact display) only in the
two components.

1066-1066: Dead analysisState — setter has no readers.

const [, setAnalysisState] discards the value, and the two setAnalysisState(...) calls at lines 1341 and 1423 only trigger re-renders without being consumed anywhere. Either wire the paused/aborted flags into the UI (e.g., to gate handleAbortAnalysis or show a "stopping…" indicator) or drop the state entirely. Right now handleAbortAnalysis calls optimizer?.abort() but the UI cannot distinguish "aborting" from "still running" because isAnalyzing stays true until finally.

♻️ Minimal cleanup
-  const [, setAnalysisState] = useState<{ paused: boolean; aborted: boolean }>({ paused: false, aborted: false });
@@
-    setIsAnalyzing(true);
-    setAnalysisState({ paused: false, aborted: false });
+    setIsAnalyzing(true);
@@
   const handleAbortAnalysis = useCallback(() => {
     analysisOptimizer?.abort();
-    setAnalysisState(prev => ({ ...prev, aborted: true }));
   }, [analysisOptimizer]);

Also applies to: 1340-1341, 1421-1424

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/DiscoveryView.tsx` at line 1066, The analysisState setter is
unused (const [, setAnalysisState]) yet setAnalysisState(...) is called in
handleAbortAnalysis and other places, so either restore and consume the state or
remove it and its calls: either change to const [analysisState,
setAnalysisState] = useState<{ paused:boolean; aborted:boolean }>(...) and wire
analysisState.paused/aborted into the UI and logic (e.g., make
handleAbortAnalysis check or display a "stopping…" indicator and use isAnalyzing
together with analysisState.aborted to control UI), or delete
analysisState/setAnalysisState and replace setAnalysisState(...) calls (e.g., at
handleAbortAnalysis where optimizer?.abort() is called) with appropriate UI
updates (setIsAnalyzing(false) or a dedicated flag that is actually read).
Ensure references to analysisState, setAnalysisState, handleAbortAnalysis,
optimizer?.abort(), and isAnalyzing are updated consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/components/DiscoveryView.tsx`:
- Around line 1212-1217: Replace the per-channel native alert in the catch block
(where channelId is used, inside the refreshAll/refreshChannel flow) with a
non-blocking UI notification: call a toast/batch-notification helper (e.g.,
showToast or enqueueBanner) that accepts a short user message plus the
underlying error details (status code/message) so users can distinguish
401/404/network errors; additionally implement deduplication or rate-limiting in
that helper (suppress identical error messages for N seconds or aggregate
multiple failures into a single summary) and keep the existing
setDiscoveryLoading(channelId, false) in the finally block.

---

Nitpick comments:
In `@src/components/DiscoveryView.tsx`:
- Line 298: Memoized components TrendingFilter, RSSFilter, SortSelector, and
Pagination lack displayName which causes anonymous entries in DevTools and stack
traces; for each memoized wrapper (the React.memo(...) exports) add a
displayName assignment like TrendingFilter.displayName = 'TrendingFilter' (and
similarly for RSSFilter, SortSelector, Pagination) after their declarations so
they appear with meaningful names in React DevTools and in
DiscoveryViewErrorBoundary logs; note CompactPagination already has a
displayName and use that same pattern for the other memoized components.
- Around line 1471-1477: refreshAll currently serializes calls to refreshChannel
causing slow cumulative latency; change it to run all refreshes in parallel by
mapping enabledChannels to calls to refreshChannel(channel.id, 1, false) and
await Promise.allSettled(...) on that array so each channel refresh runs
concurrently and failures don't abort others; keep references to
discoveryChannels, refreshChannel and enabledChannels intact so per-channel
loading state already handled by refreshChannel.
- Around line 554-978: Pagination and CompactPagination duplicate ~400 lines of
input/edit state and handlers; extract that shared logic into a new custom hook
usePageInput({ currentPage, totalPages, onPageChange, t, isLoading? }) which
returns { inputPage, isEditing, error, inputRef, startEdit, handleInputChange,
handleInputSubmit, handleInputKeyDown, handleInputBlur, handlePreviousClick,
handleNextClick } and update Pagination and CompactPagination to be thin
presenters that consume the hook (ensure the hook accepts isLoading so both
components use the same prev/next guards and error messages and keep t for
localized messages); move console.log calls into the hook and keep render
differences (page list vs compact display) only in the two components.
- Line 1066: The analysisState setter is unused (const [, setAnalysisState]) yet
setAnalysisState(...) is called in handleAbortAnalysis and other places, so
either restore and consume the state or remove it and its calls: either change
to const [analysisState, setAnalysisState] = useState<{ paused:boolean;
aborted:boolean }>(...) and wire analysisState.paused/aborted into the UI and
logic (e.g., make handleAbortAnalysis check or display a "stopping…" indicator
and use isAnalyzing together with analysisState.aborted to control UI), or
delete analysisState/setAnalysisState and replace setAnalysisState(...) calls
(e.g., at handleAbortAnalysis where optimizer?.abort() is called) with
appropriate UI updates (setIsAnalyzing(false) or a dedicated flag that is
actually read). Ensure references to analysisState, setAnalysisState,
handleAbortAnalysis, optimizer?.abort(), and isAnalyzing are updated
consistently.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f7beda8a-4f71-47fc-82ee-b545eb5bdeca

📥 Commits

Reviewing files that changed from the base of the PR and between ca9cf0f and d8d3669.

📒 Files selected for processing (2)
  • .claude/settings.local.json
  • src/components/DiscoveryView.tsx
✅ Files skipped from review due to trivial changes (1)
  • .claude/settings.local.json

Comment on lines +1212 to +1217
} catch (error) {
console.error(`[Discovery] Failed to refresh channel ${channelId}:`, error);
alert(t('获取数据失败,请检查网络连接或GitHub Token。', 'Failed to fetch data. Please check your network connection or GitHub Token.'));
} finally {
setDiscoveryLoading(channelId, false);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

alert() on every channel failure — refreshAll can spawn a storm of modal alerts.

When several channels fail (e.g., offline / token revoked), the user has to dismiss one native alert per channel, and the thread is blocked between each. Additionally, the error path swallows the underlying error (only console.error'd) so the user cannot tell 404 from 401 from a network failure. Consider surfacing a single toast/inline banner per channel or deduplicating "same error within N seconds".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/DiscoveryView.tsx` around lines 1212 - 1217, Replace the
per-channel native alert in the catch block (where channelId is used, inside the
refreshAll/refreshChannel flow) with a non-blocking UI notification: call a
toast/batch-notification helper (e.g., showToast or enqueueBanner) that accepts
a short user message plus the underlying error details (status code/message) so
users can distinguish 401/404/network errors; additionally implement
deduplication or rate-limiting in that helper (suppress identical error messages
for N seconds or aggregate multiple failures into a single summary) and keep the
existing setDiscoveryLoading(channelId, false) in the finally block.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (3)
src/components/DiscoveryView.tsx (3)

1064-1066: Remove the unused analysisState hook — it's write-only.

The getter is discarded ([, setAnalysisState]) and setAnalysisState({ paused: false, aborted: false }) (Line 1341) and setAnalysisState(prev => ({ ...prev, aborted: true })) (Line 1423) only write state that is never read. isAnalyzing and analysisOptimizer?.isAborted() already cover the same semantics, so this is dead state triggering needless re-renders on abort and at the start of each analysis pass.

♻️ Proposed cleanup
-  const [isAnalyzing, setIsAnalyzing] = useState(false);
-  const [analysisOptimizer, setAnalysisOptimizer] = useState<AIAnalysisOptimizer | null>(null);
-  const [, setAnalysisState] = useState<{ paused: boolean; aborted: boolean }>({ paused: false, aborted: false });
+  const [isAnalyzing, setIsAnalyzing] = useState(false);
+  const [analysisOptimizer, setAnalysisOptimizer] = useState<AIAnalysisOptimizer | null>(null);

…and drop the setAnalysisState(...) calls at Lines 1341 and 1423.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/DiscoveryView.tsx` around lines 1064 - 1066, Remove the
write-only analysisState hook and its associated updates: delete the const [,
setAnalysisState] = useState<{ paused: boolean; aborted: boolean }>({ paused:
false, aborted: false }) declaration and remove the two calls to
setAnalysisState(...) (the initial set and the abort set). Rely on isAnalyzing
and analysisOptimizer?.isAborted() for the same semantics (do not add any
replacement state), ensuring no other code references analysisState or
setAnalysisState before committing.

1471-1477: refreshAll runs channels sequentially — slow and fails closed on the first error.

await refreshChannel(...) in a loop means N channels take N× the slowest one end-to-end, and since refreshChannel's catch block only logs/alerts (it doesn't rethrow), you're mostly serializing for no reason. Consider Promise.allSettled(enabledChannels.map(...)) so refreshes run in parallel. Combined with the past comment about one alert() per failure, this will also let you aggregate errors into a single surface.

♻️ Suggested change
-  const refreshAll = useCallback(async () => {
-    const channels = discoveryChannels || [];
-    const enabledChannels = channels.filter(ch => ch && ch.enabled);
-    for (const channel of enabledChannels) {
-      await refreshChannel(channel.id, 1, false);
-    }
-  }, [discoveryChannels, refreshChannel]);
+  const refreshAll = useCallback(async () => {
+    const enabledChannels = (discoveryChannels || []).filter(ch => ch && ch.enabled);
+    await Promise.allSettled(
+      enabledChannels.map(channel => refreshChannel(channel.id, 1, false))
+    );
+  }, [discoveryChannels, refreshChannel]);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/DiscoveryView.tsx` around lines 1471 - 1477, The refreshAll
function currently calls await refreshChannel(...) inside a for-loop which
serializes refreshes and fails closed; change refreshAll to run refreshes in
parallel by mapping enabledChannels to calls to refreshChannel(channel.id, 1,
false) and using Promise.allSettled(...) to await them; after allSettled
resolves, aggregate any rejected results (or results with status 'rejected') and
surface a single consolidated alert/log instead of one alert per failure;
reference the refreshAll function, discoveryChannels variable, and
refreshChannel(channel.id, 1, false) when making the change.

1058-1060: Destructured rssTimeRange is unused in the render path.

refreshChannel reads the fresh value via useAppStore.getState().rssTimeRange (Line 1164), and the JSX only passes rssTimeRange to RSSFilter. Destructuring it here still subscribes the component to updates on this slice, so changes to rssTimeRange (even when not on the rss-trending channel) force a re-render of the whole DiscoveryView. Since the store slice is already consumed where needed via getState(), you can drop it from the subscription — or conversely, remove the getState() call and rely solely on the destructured value for consistency. The current mix of both patterns makes the data flow harder to reason about.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/DiscoveryView.tsx` around lines 1058 - 1060, The component is
subscribing to rssTimeRange unnecessarily; remove rssTimeRange from the
useAppStore() destructuring in DiscoveryView and instead read the current value
directly where needed via useAppStore.getState().rssTimeRange to avoid extra
re-renders: update the JSX that currently passes rssTimeRange to RSSFilter to
pass useAppStore.getState().rssTimeRange, and keep the existing refreshChannel
usage of useAppStore.getState().rssTimeRange; leave setRssTimeRange in the
destructure if it's used elsewhere.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/components/DiscoveryView.tsx`:
- Line 1079: discoveryScrollPositionsRef is never populated from the persisted
Zustand store, so hydrated positions are lost after reload; on mount (inside the
existing effect that restores scroll positions) seed discoveryScrollPositionsRef
from the persisted state by reading
useAppStore.getState().discoveryScrollPositions (or the store selector) before
applying positions, then continue to use handleScroll and channel-select
handlers to update the ref and call setDiscoveryScrollPosition; ensure the mount
effect uses the seeded ref rather than the initial empty object so restored
scrollTop values are applied.
- Around line 1426-1428: The variable isAnalyzingThisChannel is misleading and
not scoped to a specific channel; update the logic so the UI and controls
reflect which channel started analysis by tracking the initiating channel (e.g.,
introduce channelUnderAnalysis or reuse an existing selectedDiscoveryChannel)
and change the condition used by the progress bar and controls from isAnalyzing
&& analysisProgress.total > 0 to something like isAnalyzing &&
analysisProgress.total > 0 && channelUnderAnalysis === selectedDiscoveryChannel;
update places referencing isAnalyzingThisChannel, handleAnalyzePage, and the
Abort control to consult channelUnderAnalysis (set when analysis starts and
cleared/updated on abort/finish) or rename the flag to indicate global scope if
you intentionally want global behavior and document it.

---

Nitpick comments:
In `@src/components/DiscoveryView.tsx`:
- Around line 1064-1066: Remove the write-only analysisState hook and its
associated updates: delete the const [, setAnalysisState] = useState<{ paused:
boolean; aborted: boolean }>({ paused: false, aborted: false }) declaration and
remove the two calls to setAnalysisState(...) (the initial set and the abort
set). Rely on isAnalyzing and analysisOptimizer?.isAborted() for the same
semantics (do not add any replacement state), ensuring no other code references
analysisState or setAnalysisState before committing.
- Around line 1471-1477: The refreshAll function currently calls await
refreshChannel(...) inside a for-loop which serializes refreshes and fails
closed; change refreshAll to run refreshes in parallel by mapping
enabledChannels to calls to refreshChannel(channel.id, 1, false) and using
Promise.allSettled(...) to await them; after allSettled resolves, aggregate any
rejected results (or results with status 'rejected') and surface a single
consolidated alert/log instead of one alert per failure; reference the
refreshAll function, discoveryChannels variable, and refreshChannel(channel.id,
1, false) when making the change.
- Around line 1058-1060: The component is subscribing to rssTimeRange
unnecessarily; remove rssTimeRange from the useAppStore() destructuring in
DiscoveryView and instead read the current value directly where needed via
useAppStore.getState().rssTimeRange to avoid extra re-renders: update the JSX
that currently passes rssTimeRange to RSSFilter to pass
useAppStore.getState().rssTimeRange, and keep the existing refreshChannel usage
of useAppStore.getState().rssTimeRange; leave setRssTimeRange in the destructure
if it's used elsewhere.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4118514a-576f-477d-beea-404f87ea90c3

📥 Commits

Reviewing files that changed from the base of the PR and between d8d3669 and 99096a7.

📒 Files selected for processing (1)
  • src/components/DiscoveryView.tsx

const lastScrollY = useRef(0);
const scrollTimeoutRef = useRef<ReturnType<typeof setTimeout> | null>(null);
// 用于在频道切换时直接读取最新滚动位置,避免订阅整个 map 导致 effect 重跑
const discoveryScrollPositionsRef = useRef<Record<string, number>>({});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Persisted scroll positions are never hydrated into discoveryScrollPositionsRef.

discoveryScrollPositionsRef is initialized to {} (Line 1079) and only written from handleScroll and the two channel-select handlers. The Zustand store, however, persists discoveryScrollPositions per channel (see setDiscoveryScrollPosition in src/store/useAppStore.ts). After a page reload, the effect at Lines 1221–1227 reads from the empty ref, so every channel is restored to scrollTop = 0 even though the store has the old position. Seed the ref from the store on mount (or read from useAppStore.getState().discoveryScrollPositions in the effect).

🛠 Proposed fix
-    if (scrollContainerRef.current) {
-      const savedPosition = discoveryScrollPositionsRef.current[selectedDiscoveryChannel] || 0;
-      scrollContainerRef.current.scrollTop = savedPosition;
-    }
+    if (scrollContainerRef.current) {
+      const storedPositions = useAppStore.getState().discoveryScrollPositions || {};
+      const savedPosition =
+        discoveryScrollPositionsRef.current[selectedDiscoveryChannel] ??
+        storedPositions[selectedDiscoveryChannel] ??
+        0;
+      discoveryScrollPositionsRef.current[selectedDiscoveryChannel] = savedPosition;
+      scrollContainerRef.current.scrollTop = savedPosition;
+    }

Also applies to: 1220-1237

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/DiscoveryView.tsx` at line 1079, discoveryScrollPositionsRef
is never populated from the persisted Zustand store, so hydrated positions are
lost after reload; on mount (inside the existing effect that restores scroll
positions) seed discoveryScrollPositionsRef from the persisted state by reading
useAppStore.getState().discoveryScrollPositions (or the store selector) before
applying positions, then continue to use handleScroll and channel-select
handlers to update the ref and call setDiscoveryScrollPosition; ensure the mount
effect uses the seeded ref rather than the initial empty object so restored
scrollTop values are applied.

Comment on lines +1426 to +1428
const isAnalyzingThisChannel = isAnalyzing && (
analysisProgress.total > 0
);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

isAnalyzingThisChannel is not scoped to a channel.

Despite the name, this flag is just isAnalyzing && analysisProgress.total > 0, with no reference to selectedDiscoveryChannel. If a user starts AI analysis on the trending channel and then switches to topic mid-run, the progress bar (Lines 1615–1642) renders on topic as if that channel were being analyzed, and the handleAnalyzePage/Abort controls manipulate the other channel's analysis. Either rename the flag to reflect global scope and document it, or track the channel that initiated the analysis and gate this on channelUnderAnalysis === selectedDiscoveryChannel.

🛠 Sketch
+  const [analyzingChannel, setAnalyzingChannel] = useState<DiscoveryChannelId | null>(null);
   ...
   const handleAnalyzePage = useCallback(async () => {
     ...
+    setAnalyzingChannel(selectedDiscoveryChannel);
     setIsAnalyzing(true);
     ...
     } finally {
       setIsAnalyzing(false);
+      setAnalyzingChannel(null);
       ...
     }
-  }, [githubToken, aiConfigs, activeAIConfig, language, currentPageRepos, t, updateDiscoveryRepo, setAnalysisProgress]);
+  }, [githubToken, aiConfigs, activeAIConfig, language, currentPageRepos, selectedDiscoveryChannel, t, updateDiscoveryRepo, setAnalysisProgress]);
   ...
-  const isAnalyzingThisChannel = isAnalyzing && (
-    analysisProgress.total > 0
-  );
+  const isAnalyzingThisChannel =
+    isAnalyzing && analysisProgress.total > 0 && analyzingChannel === selectedDiscoveryChannel;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/DiscoveryView.tsx` around lines 1426 - 1428, The variable
isAnalyzingThisChannel is misleading and not scoped to a specific channel;
update the logic so the UI and controls reflect which channel started analysis
by tracking the initiating channel (e.g., introduce channelUnderAnalysis or
reuse an existing selectedDiscoveryChannel) and change the condition used by the
progress bar and controls from isAnalyzing && analysisProgress.total > 0 to
something like isAnalyzing && analysisProgress.total > 0 && channelUnderAnalysis
=== selectedDiscoveryChannel; update places referencing isAnalyzingThisChannel,
handleAnalyzePage, and the Abort control to consult channelUnderAnalysis (set
when analysis starts and cleared/updated on abort/finish) or rename the flag to
indicate global scope if you intentionally want global behavior and document it.

@AmintaCCCP AmintaCCCP merged commit 99096a7 into main Apr 22, 2026
5 checks passed
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.

2 participants