fix(DiscoveryView): 移除分页自动加载逻辑并优化滚动行为#97
Merged
AmintaCCCP merged 8 commits intoAmintaCCCP:mainfrom Apr 24, 2026
Merged
Conversation
refactor(RepositoryEditModal): 修复自定义描述为空时的处理逻辑
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReplaces page-based pagination in Discovery with an incremental "load more" flow and per-channel load-more loading/error state; makes RepositoryEditModal null-safe for Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DiscoveryView as DiscoveryView (UI)
participant ChannelSvc as ChannelService / refreshChannel
participant Store as App Store
rect rgba(135,206,250,0.5)
User->>DiscoveryView: Click "Load more"
DiscoveryView->>DiscoveryView: check discoveryIsLoadingMore / discoveryHasMore
DiscoveryView->>ChannelSvc: refreshChannel(channelId, nextPage, append=true)
ChannelSvc->>Store: request next page repos
Store-->>ChannelSvc: return repos + hasMore/nextPage
ChannelSvc-->>DiscoveryView: deliver repos, hasMore, nextPage
DiscoveryView->>DiscoveryView: append to allRepos, set discoveryIsLoadingMore/LoadMoreError
DiscoveryView->>DiscoveryView: locate repo at prev-count index and scroll into view
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/RepositoryEditModal.tsx (1)
748-748: Inconsistent null-safety in this branch.Every other
formData.descriptionaccess in this PR was hardened with(formData.description || '').trim(), but this condition still callsformData.description.trim()directly. SinceformData.descriptionis initialized as''and typed asstring, none of these guards are strictly required, but if you intend to keep them defensively, this branch should match for consistency (and to avoid a future TS-strict crash if the field type ever widens tostring | null).Proposed change
- ) : editIntent.description === 'keep-custom' && formData.description.trim() !== '' && !customStatus.description ? ( + ) : editIntent.description === 'keep-custom' && (formData.description || '').trim() !== '' && !customStatus.description ? (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/RepositoryEditModal.tsx` at line 748, The condition in RepositoryEditModal's JSX branch uses formData.description.trim() directly which is inconsistent with the defensive null-safety used elsewhere; update that check to use (formData.description || '').trim() so the condition reads editIntent.description === 'keep-custom' && (formData.description || '').trim() !== '' && !customStatus.description, matching the other guarded accesses and preventing future TS-strict issues.
🤖 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 938-950: The handlePageChange currently only calls setCurrentPage,
which can move to pages without loaded data because totalPages uses
currentTotalCount while currentPageRepos slices allRepos; update
handlePageChange to either (A) trigger loading the missing page: call the
existing fetch/load function (e.g., fetchReposForPage or loadMoreRepos) with the
target page when the requested slice is not present, then setCurrentPage after
the data resolves, or (B) constrain page selection to loaded data by checking
whether allRepos contains items for the requested page (compute expected
start/end index from PAGE_SIZE) and if not, prevent changing page (or clamp to
maxLoadedPage) so currentPageRepos never becomes empty; reference
handlePageChange, currentPageRepos, allRepos, totalPages, currentTotalCount, and
your repo-fetching function when implementing the fix.
---
Nitpick comments:
In `@src/components/RepositoryEditModal.tsx`:
- Line 748: The condition in RepositoryEditModal's JSX branch uses
formData.description.trim() directly which is inconsistent with the defensive
null-safety used elsewhere; update that check to use (formData.description ||
'').trim() so the condition reads editIntent.description === 'keep-custom' &&
(formData.description || '').trim() !== '' && !customStatus.description,
matching the other guarded accesses and preventing future TS-strict issues.
🪄 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: bfaa4876-ca4c-4e91-94cd-5c96f29361ae
📒 Files selected for processing (2)
src/components/DiscoveryView.tsxsrc/components/RepositoryEditModal.tsx
移除传统的分页组件，改为实现无限滚动加载模式 新增加载更多按钮组件，优化大数据量下的用户体验 简化页码管理逻辑，减少不必要的状态和计算
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/DiscoveryView.tsx (1)
638-652:⚠️ Potential issue | 🟡 MinorStale "current page" copy and scope change in
handleAnalyzePageafter pagination removal.Two concerns in this block:
- Line 650 still says
当前页面所有项目已完成AI分析/All projects on current page have been analyzed, but there is no "current page" anymore —pageReposis now the fullallReposset. Update the string to reflect "all loaded projects" (or equivalent).- Behavior change worth calling out: previously this button analyzed only the visible page slice; now it analyzes every loaded repo across all fetched pages. Once a user clicks "加载更多" several times, a single AI Analyze click can fan out into many concurrent API calls / much higher token spend than before. Consider either renaming/relabeling the button (line 940
AI分析/AI Analyze) to make the new scope explicit, or constraining analysis to a bounded batch.💡 Minimal copy fix
- if (unanalyzed.length === 0) { - alert(t('当前页面所有项目已完成AI分析。', 'All projects on current page have been analyzed.')); - return; - } + if (unanalyzed.length === 0) { + alert(t('已加载的所有项目均已完成AI分析。', 'All loaded projects have been analyzed.')); + return; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/DiscoveryView.tsx` around lines 638 - 652, The copy and scope in handleAnalyzePage are stale: update the user-facing string that currently reads "当前页面所有项目已完成AI分析" / "All projects on current page have been analyzed" to reflect the full loaded set (e.g., "所有已加载项目已完成AI分析" / "All loaded projects have been analyzed") because pageRepos now equals allRepos; then address the scope change by either (A) renaming the AI Analyze button label referenced as "AI分析" to clarify it runs across all loaded repos, or (B) constrain handleAnalyzePage to operate on a bounded batch (slice) instead of the entire allRepos/unanalyzed set to avoid unbounded concurrent API calls — update the related symbols pageRepos, allRepos, unanalyzed, and the handler handleAnalyzePage accordingly.
🧹 Nitpick comments (1)
src/components/DiscoveryView.tsx (1)
1101-1114: Remove the leftover "Page Info" block — it duplicatesDataStatsand has a dangling empty column.The counter on lines 1106-1111 (
共 N 个项目/Total N items) is already rendered byDataStatsin the toolbar (line 943) and again insideLoadMoreButton(line 351-358). On top of the duplication, the sibling<div>that used to hold the "more available" pill is now empty (lines 1112-1113), sojustify-betweenleaves an orphaned whitespace gap on the right. Drop the entire block.♻️ Proposed cleanup
- {/* Page Info */} - {allRepos.length > 0 && ( - <div className={isDesktopSafeMode - ? 'flex items-center justify-between py-3.5 px-5 bg-gray-50 dark:bg-gray-800 rounded-lg border border-gray-200 dark:border-gray-700 text-sm' - : 'flex items-center justify-between py-3.5 px-5 bg-gradient-to-r from-gray-50 to-slate-50 dark:from-gray-800/60 dark:to-slate-800/40 rounded-xl border border-gray-100 dark:border-gray-700/50 text-sm'}> - <div className="flex items-center gap-2 text-gray-600 dark:text-gray-400"> - <div className="w-1.5 h-1.5 rounded-full bg-blue-500" /> - <span> - {t('共', 'Total')} <strong className="text-gray-900 dark:text-white">{allRepos.length}</strong> {t('个项目', 'items')} - </span> - </div> - - </div> - )} - {/* Load More Button */}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/DiscoveryView.tsx` around lines 1101 - 1114, Remove the redundant "Page Info" JSX block that displays "共 N 个项目" — it duplicates DataStats (rendered in the toolbar) and LoadMoreButton counts; specifically delete the conditional block that checks allRepos.length > 0 and its inner div (the fragment with the two className variants and the span showing the total), which also contains the now-empty right-hand column causing layout gap; after removal ensure no remaining orphaned markup or styles referencing that block remain in DiscoveryView.tsx.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/components/DiscoveryView.tsx`:
- Around line 638-652: The copy and scope in handleAnalyzePage are stale: update
the user-facing string that currently reads "当前页面所有项目已完成AI分析" / "All projects on
current page have been analyzed" to reflect the full loaded set (e.g.,
"所有已加载项目已完成AI分析" / "All loaded projects have been analyzed") because pageRepos
now equals allRepos; then address the scope change by either (A) renaming the AI
Analyze button label referenced as "AI分析" to clarify it runs across all loaded
repos, or (B) constrain handleAnalyzePage to operate on a bounded batch (slice)
instead of the entire allRepos/unanalyzed set to avoid unbounded concurrent API
calls — update the related symbols pageRepos, allRepos, unanalyzed, and the
handler handleAnalyzePage accordingly.
---
Nitpick comments:
In `@src/components/DiscoveryView.tsx`:
- Around line 1101-1114: Remove the redundant "Page Info" JSX block that
displays "共 N 个项目" — it duplicates DataStats (rendered in the toolbar) and
LoadMoreButton counts; specifically delete the conditional block that checks
allRepos.length > 0 and its inner div (the fragment with the two className
variants and the span showing the total), which also contains the now-empty
right-hand column causing layout gap; after removal ensure no remaining orphaned
markup or styles referencing that block remain in DiscoveryView.tsx.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 347aafd6-f19f-491f-8c5c-c0cbf7208408
📒 Files selected for processing (1)
src/components/DiscoveryView.tsx
Owner
PR Review@SummerRay160 感谢修复 modal 的空值处理问题。不过 DiscoveryView 的改动有一个关键问题需要解决： 🔴 问题：分页切换后数据加载逻辑被移除PR #96 引入了 RSS 数据源的趋势页功能，该功能需要分页加载： // getTrendingRepositories (PR #96)
const startIndex = (page - 1) * perPage; // 按页切片
const endIndex = Math.min(startIndex + perPage, items.length);PR #97 删除了 // 删除的代码（导致分页后数据无法加载）
if (allRepos.length < requiredItems && currentHasMore && !currentIsLoading) {
refreshChannel(selectedDiscoveryChannel, currentNextPage, true);
}后果：趋势页初始只加载 20 条数据，用户切到第 2 页时列表为空（数据未加载）。 ✅ 建议修复方案 1：保留分页数据加载（推荐） const handlePageChange = useCallback((page: number) => {
setCurrentPage(page);
// 保留数据加载检查
const requiredItems = page * ITEMS_PER_PAGE;
if (allRepos.length < requiredItems && currentHasMore && !currentIsLoading) {
refreshChannel(selectedDiscoveryChannel, currentNextPage, true);
}
// 保留滚动到顶部
scrollContainerRef.current?.scrollTo({ top: 0, behavior: 'smooth' });
}, [allRepos.length, currentHasMore, currentIsLoading, currentNextPage,
refreshChannel, selectedDiscoveryChannel]);方案 2：RSS 数据量不大(~100条)，可一次性全部加载 如果 RSS 数据量可控，可以在 // 读取整个 RSS 而非分页
const repos = await githubApi.getTrendingRepositories(..., 1, 999);🟢 RepositoryEditModal 的修复 ✅
请确认下 DiscoveryView 的修复方案，我这边再跑一遍测试。 |
Contributor
Author
|
在新的更改中移除了分页功能 增加了增量加载功能 |
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/DiscoveryView.tsx (1)
648-662:⚠️ Potential issue | 🟡 MinorStale "current page" wording and silently broader analysis scope after the load-more refactor.
pageReposnow aliasesallRepos, so clicking AI Analyze will analyze every repo that has been accumulated via repeated Load More clicks (potentially hundreds), not a single page's 20. Two concrete issues:
- Line 660 still says
当前页面所有项目已完成AI分析/All projects on current page have been analyzed.— there is no "current page" anymore. Please reword to reflect "已加载的所有项目 / all loaded projects".- The scope change can meaningfully inflate AI API usage/cost/latency vs. the previous per-page behavior. Consider either documenting this in the button tooltip (line 961
title={t('AI分析', 'Analyze with AI')}is generic) or confirming with a count before kicking off analysis on a large accumulated list.✏️ Wording fix
- if (unanalyzed.length === 0) { - alert(t('当前页面所有项目已完成AI分析。', 'All projects on current page have been analyzed.')); - return; - } + if (unanalyzed.length === 0) { + alert(t('已加载的所有项目均已完成AI分析。', 'All loaded projects have been analyzed.')); + return; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/DiscoveryView.tsx` around lines 648 - 662, The UI currently aliases pageRepos = allRepos so the alerts and behavior reference a "current page" but actually operate on all loaded repositories; update the user-facing message in the unanalyzed-empty branch to say "已加载的所有项目已完成AI分析 / All loaded projects have been analyzed" instead of "当前页面...", and add a confirmation or explicit count before starting analysis when unanalyzed.length is large (e.g., show modal/confirm with unanalyzed.length) or update the AI Analyze button tooltip/title (the prop using title={t('AI分析', 'Analyze with AI')}) to indicate it will analyze all loaded projects; change references to pageRepos/unanalyzed in the Analyze trigger logic so messaging and confirmation reflect the actual scope.
🧹 Nitpick comments (3)
src/components/DiscoveryView.tsx (3)
761-782: Double-click race on Load More uses stalecurrentIsLoading.
currentIsLoadingis captured from the store snapshot at render time, so two rapid clicks on the button can both pass the gate before React re-renders with the updated loading state, triggering duplicaterefreshChannelcalls for the same page. Reading the latest state synchronously viauseAppStore.getState()(similarly to how the channel-switch effect at line 565-566 already does it) avoids this. The button'sdisabled={isLoading}already covers most UI cases, but the programmatic guard is still worth tightening.🔒 Safer gate
const handleLoadMore = useCallback(async () => { - if (!discoveryHasMore[selectedDiscoveryChannel]) { + const state = useAppStore.getState(); + if (!state.discoveryHasMore[selectedDiscoveryChannel]) { return; } - - if (currentIsLoading) { + + if (state.discoveryIsLoading[selectedDiscoveryChannel]) { return; } - - const nextPage = discoveryNextPage[selectedDiscoveryChannel]; + + const nextPage = state.discoveryNextPage[selectedDiscoveryChannel]; if (!nextPage) { return; } - + await refreshChannel(selectedDiscoveryChannel, nextPage, true); - }, [ - discoveryHasMore, - discoveryNextPage, - selectedDiscoveryChannel, - currentIsLoading, - refreshChannel - ]); + }, [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 761 - 782, The handleLoadMore callback uses the render-time snapshot currentIsLoading which can be stale on rapid double clicks; update the guard to read the latest loading state synchronously from the store (e.g. call useAppStore.getState().currentIsLoading) before returning, while keeping the existing checks for discoveryHasMore[selectedDiscoveryChannel], discoveryNextPage[selectedDiscoveryChannel], and invoking refreshChannel(selectedDiscoveryChannel, nextPage, true) if safe; ensure you reference currentIsLoading only via the store read inside handleLoadMore so the debounce gate is reliable even between renders.
1125-1138: Dead layout and duplicated count after removing pagination.This "Page Info" footer block now just repeats the
DataStatsalready rendered in the toolbar (lines 967–971), and the right-side<div>that used to host the pagination controls is gone, leaving thejustify-betweencontainer with a single left item and empty whitespace on the right. Either remove this block entirely or drop thejustify-betweenwrapper since there's no second child. TheLoadMoreButtonbelow also already shows已加载 currentCount / totalCount, so the three surfaces overlap.🧹 Proposed cleanup
- {/* Page Info */} - {allRepos.length > 0 && ( - <div className={isDesktopSafeMode - ? 'flex items-center justify-between py-3.5 px-5 bg-gray-50 dark:bg-gray-800 rounded-lg border border-gray-200 dark:border-gray-700 text-sm' - : 'flex items-center justify-between py-3.5 px-5 bg-gradient-to-r from-gray-50 to-slate-50 dark:from-gray-800/60 dark:to-slate-800/40 rounded-xl border border-gray-100 dark:border-gray-700/50 text-sm'}> - <div className="flex items-center gap-2 text-gray-600 dark:text-gray-400"> - <div className="w-1.5 h-1.5 rounded-full bg-blue-500" /> - <span> - {t('共', 'Total')} <strong className="text-gray-900 dark:text-white">{allRepos.length}</strong> {t('个项目', 'items')} - </span> - </div> - - </div> - )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/DiscoveryView.tsx` around lines 1125 - 1138, The "Page Info" footer block rendering the total count duplicates DataStats and leaves an empty right side after pagination removal; remove the entire conditional block that uses allRepos and isDesktopSafeMode (the div starting with the Page Info comment) so DataStats and LoadMoreButton remain the single sources of truth, or alternatively replace the outer container's 'justify-between' with a single-side layout and remove the duplicated count rendering to avoid overlap; locate references in DiscoveryView.tsx to allRepos, the Page Info JSX, and isDesktopSafeMode/DataStats/LoadMoreButton to apply the change.
1119-1123: Unbounded list rendering with the new infinite load-more.With pagination removed,
allReposgrows monotonically as users click Load More, and every item is mounted into the DOM. For channels with large total counts this can noticeably degrade scroll/analyze performance and memory over a session. SincescrollContainerRefis already a single scroll container, dropping in a windowing library likereact-windowor@tanstack/react-virtualwould be a good follow-up. Not blocking, but worth tracking as the feature sees real use.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/DiscoveryView.tsx` around lines 1119 - 1123, The allRepos list is rendered unbounded (mapping to SubscriptionRepoCard) and will grow as users click Load More, so replace the direct map with a virtualized list (e.g., react-window or `@tanstack/react-virtual`) to only mount visible items; keep using scrollContainerRef as the scrollParent, set itemCount to allRepos.length, render SubscriptionRepoCard inside the virtualized item renderer and pass desktopSafeMode (isDesktopSafeMode) and key via the renderer, and ensure spacing logic (isDesktopSafeMode ? 'space-y-3' : 'space-y-4') is preserved around the virtualized container in DiscoveryView.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/components/DiscoveryView.tsx`:
- Around line 648-662: The UI currently aliases pageRepos = allRepos so the
alerts and behavior reference a "current page" but actually operate on all
loaded repositories; update the user-facing message in the unanalyzed-empty
branch to say "已加载的所有项目已完成AI分析 / All loaded projects have been analyzed" instead
of "当前页面...", and add a confirmation or explicit count before starting analysis
when unanalyzed.length is large (e.g., show modal/confirm with
unanalyzed.length) or update the AI Analyze button tooltip/title (the prop using
title={t('AI分析', 'Analyze with AI')}) to indicate it will analyze all loaded
projects; change references to pageRepos/unanalyzed in the Analyze trigger logic
so messaging and confirmation reflect the actual scope.
---
Nitpick comments:
In `@src/components/DiscoveryView.tsx`:
- Around line 761-782: The handleLoadMore callback uses the render-time snapshot
currentIsLoading which can be stale on rapid double clicks; update the guard to
read the latest loading state synchronously from the store (e.g. call
useAppStore.getState().currentIsLoading) before returning, while keeping the
existing checks for discoveryHasMore[selectedDiscoveryChannel],
discoveryNextPage[selectedDiscoveryChannel], and invoking
refreshChannel(selectedDiscoveryChannel, nextPage, true) if safe; ensure you
reference currentIsLoading only via the store read inside handleLoadMore so the
debounce gate is reliable even between renders.
- Around line 1125-1138: The "Page Info" footer block rendering the total count
duplicates DataStats and leaves an empty right side after pagination removal;
remove the entire conditional block that uses allRepos and isDesktopSafeMode
(the div starting with the Page Info comment) so DataStats and LoadMoreButton
remain the single sources of truth, or alternatively replace the outer
container's 'justify-between' with a single-side layout and remove the
duplicated count rendering to avoid overlap; locate references in
DiscoveryView.tsx to allRepos, the Page Info JSX, and
isDesktopSafeMode/DataStats/LoadMoreButton to apply the change.
- Around line 1119-1123: The allRepos list is rendered unbounded (mapping to
SubscriptionRepoCard) and will grow as users click Load More, so replace the
direct map with a virtualized list (e.g., react-window or
`@tanstack/react-virtual`) to only mount visible items; keep using
scrollContainerRef as the scrollParent, set itemCount to allRepos.length, render
SubscriptionRepoCard inside the virtualized item renderer and pass
desktopSafeMode (isDesktopSafeMode) and key via the renderer, and ensure spacing
logic (isDesktopSafeMode ? 'space-y-3' : 'space-y-4') is preserved around the
virtualized container in DiscoveryView.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a343dd97-4905-49d8-98a8-a153776140f1
📒 Files selected for processing (1)
src/components/DiscoveryView.tsx
Contributor
Author
|
我检查下代码 先不合并 |
Owner
好的，注意身体啊兄弟，多休息。现在主分支还有几个小问题我刚测出来的:
|
添加 discoveryIsLoadingMore 和 discoveryLoadMoreError 状态用于管理加载更多操作 实现加载更多时的滚动定位和错误处理 优化频道切换时的状态重置逻辑
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/store/useAppStore.ts`:
- Around line 337-343: The persisted per-channel restoration in useAppStore
(e.g., the persistedRepos assignment and keys like 'trending', 'hot-release',
'most-popular', 'topic', 'search') blindly trusts the persisted shape; change it
to validate and normalize each value before restoring (ensure discoveryRepos
entries are arrays, discoveryNextPage values are finite non-negative numbers,
discoveryLoading values are booleans, and page indices are integers > 0) using
the provided type-guard patterns (e.g., Array.isArray(...) ? value : [], typeof
value === 'boolean' ? value : false, Number.isFinite(value) && value >= 0 ?
value : 0, Number.isInteger(value) && value > 0 ? value : 1) and apply the same
guards to the other maps referenced in this file (the blocks around the other
commented ranges) so consumers always get the expected array/boolean/number
shapes.
- Around line 1179-1205: The setSelectedDiscoveryChannel updater currently
resets per-channel state (discoveryRepos, discoveryNextPage, discoveryHasMore,
discoveryTotalCount, discoveryIsLoadingMore, discoveryLoadMoreError) whenever a
channel is selected; change it so setSelectedDiscoveryChannel only updates
selectedDiscoveryChannel and leaves all discovery* maps untouched to preserve
per-channel cache/scroll state. Move any logic that clears those maps into the
code paths that actually change queries/filters (where query/filter change
handlers live) so that only real query changes reset discoveryRepos,
discoveryNextPage, discoveryHasMore, discoveryTotalCount,
discoveryIsLoadingMore, and discoveryLoadMoreError.
🪄 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: 2d41dc9d-532a-4431-b505-5223c8813672
📒 Files selected for processing (3)
src/components/DiscoveryView.tsxsrc/store/useAppStore.tssrc/types/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/DiscoveryView.tsx
扩展 AI 服务类型以支持兼容 OpenAI API 的自定义端点，包括： 1. 在类型定义中添加 'openai-compatible' 选项 2. 修改请求处理逻辑以直接使用自定义端点的完整 URL 3. 在配置面板中增加相关说明和最终请求地址预览
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/src/routes/proxy.ts (1)
140-148:⚠️ Potential issue | 🟡 MinorFix misleading UI copy and document
reasoningfield compatibility across OpenAI-compatible endpoints.The code injects
reasoning: { effort: ... }on anyopenai-compatibleapiType, but this format is not universally supported:
- NVIDIA NIM (placeholder on line 380): Does not accept the
reasoningfield; uses special system prompts or customreasoning_effortparameter for specific models only.- vLLM: Does not accept input
reasoningparameters; reasoning is extracted server-side via CLI flags only.- llama.cpp: Uses
reasoning_contentfield (notreasoning); does not accept thereasoning: {effort: ...}format.- Ollama: Accepts
reasoning_effortparameter (notreasoningwith nestedeffort).- LocalAI: Supports
reasoning: {effort: ...}in/v1/responses(as of Jan 2026).The UI text at line 467 ("仅对 OpenAI 兼容接口生效" / "Only applies to OpenAI-compatible APIs") is misleading. Most widely-used compatible endpoints will reject the injected field as unknown. Update the UI text to clarify which endpoints actually accept this format, or restrict injection to explicitly supported backends (e.g., only
openai,openai-responses, andlocalai). At minimum, document the compatibility limitations for users configuring third-party endpoints.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/routes/proxy.ts` around lines 140 - 148, The current injection of reasoning: { effort: reasoningEffort } into effectiveRequestBody is too broad for the apiType 'openai-compatible' and will break backends that do not accept that field; change the condition that builds effectiveRequestBody to only inject the reasoning object for explicitly supported apiTypes (e.g., 'openai', 'openai-responses', and 'localai') instead of the generic 'openai-compatible' branch, and update the UI copy shown by the component that renders the "仅对 OpenAI 兼容接口生效" text to clearly list supported backends or say it only applies to OpenAI, OpenAI Responses, and LocalAI; also add a short comment near effectiveRequestBody and in relevant docs to enumerate incompatible backends (NVIDIA NIM, vLLM, llama.cpp, Ollama) and their alternative parameters (e.g., reasoning_effort, reasoning_content) so integrators know not to rely on the injected reasoning field.
🧹 Nitpick comments (2)
src/utils/apiUrlBuilder.ts (1)
3-36: Optional: de-duplicatebuildApiUrlwith the server implementation.
buildApiUrlhere is an almost verbatim copy of the one inserver/src/routes/proxy.ts(lines 10–43). If the two ever drift (e.g., new version pattern or host-specific rule), client UI preview and server-side routing will silently diverge. If you have a shared code directory (or can import fromsrc/utils/on the server via a path alias), consider hoisting this into a single module; otherwise at least add cross-reference comments so future edits stay in sync.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/apiUrlBuilder.ts` around lines 3 - 36, The buildApiUrl function is duplicated between client and server (see buildApiUrl) which risks drift; refactor by extracting the logic into a single shared module and import it from both places (or, if sharing is impossible, add a clear cross-reference comment pointing to the server implementation and ensure both copies reference a single source of truth), update all usages to import the shared buildApiUrl function, and run tests to verify behavior; if exporting/importing requires path aliasing, add the necessary tsconfig/webpack alias and update imports accordingly.server/src/routes/proxy.ts (1)
119-123: Optional: extract a server-sidebuildFinalApiUrl.The inline ternary here replicates the client-side
buildFinalApiUrllogic fromsrc/utils/apiUrlBuilder.tslines 38-45. Extracting a small local helper (e.g.,buildFinalApiUrl(baseUrl, apiType)next to the existingbuildApiUrlin this file) would keep the OpenAI-style URL composition in one place and reduce the risk of the server drifting from client behavior when new apiType values are added.♻️ Suggested helper
+function buildFinalApiUrl(baseUrl: string, apiType: string): string { + if (apiType === 'openai-compatible') { + return baseUrl.replace(/\/$/, ''); + } + const path = apiType === 'openai-responses' ? 'v1/responses' : 'v1/chat/completions'; + return buildApiUrl(baseUrl, path); +}if (apiType === 'openai' || apiType === 'openai-responses' || apiType === 'openai-compatible') { - // openai-compatible 类型直接使用 baseUrl 作为完整地址 - targetUrl = apiType === 'openai-compatible' - ? baseUrl.replace(/\/$/, '') - : buildApiUrl(baseUrl, apiType === 'openai-responses' ? 'v1/responses' : 'v1/chat/completions'); + targetUrl = buildFinalApiUrl(baseUrl, apiType); headers['Authorization'] = `Bearer ${apiKey}`;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/routes/proxy.ts` around lines 119 - 123, Extract the OpenAI-style URL selection into a local helper so server logic matches the client: add a buildFinalApiUrl(baseUrl, apiType) next to the existing buildApiUrl and replace the inline ternary that sets targetUrl for apiType === 'openai' | 'openai-responses' | 'openai-compatible' to call it; implement buildFinalApiUrl to return baseUrl.replace(/\/$/, '') for 'openai-compatible', and otherwise call buildApiUrl(baseUrl, apiType === 'openai-responses' ? 'v1/responses' : 'v1/chat/completions') so the same mapping is centralized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@server/src/routes/proxy.ts`:
- Around line 140-148: The current injection of reasoning: { effort:
reasoningEffort } into effectiveRequestBody is too broad for the apiType
'openai-compatible' and will break backends that do not accept that field;
change the condition that builds effectiveRequestBody to only inject the
reasoning object for explicitly supported apiTypes (e.g., 'openai',
'openai-responses', and 'localai') instead of the generic 'openai-compatible'
branch, and update the UI copy shown by the component that renders the "仅对
OpenAI 兼容接口生效" text to clearly list supported backends or say it only applies to
OpenAI, OpenAI Responses, and LocalAI; also add a short comment near
effectiveRequestBody and in relevant docs to enumerate incompatible backends
(NVIDIA NIM, vLLM, llama.cpp, Ollama) and their alternative parameters (e.g.,
reasoning_effort, reasoning_content) so integrators know not to rely on the
injected reasoning field.
---
Nitpick comments:
In `@server/src/routes/proxy.ts`:
- Around line 119-123: Extract the OpenAI-style URL selection into a local
helper so server logic matches the client: add a buildFinalApiUrl(baseUrl,
apiType) next to the existing buildApiUrl and replace the inline ternary that
sets targetUrl for apiType === 'openai' | 'openai-responses' |
'openai-compatible' to call it; implement buildFinalApiUrl to return
baseUrl.replace(/\/$/, '') for 'openai-compatible', and otherwise call
buildApiUrl(baseUrl, apiType === 'openai-responses' ? 'v1/responses' :
'v1/chat/completions') so the same mapping is centralized.
In `@src/utils/apiUrlBuilder.ts`:
- Around line 3-36: The buildApiUrl function is duplicated between client and
server (see buildApiUrl) which risks drift; refactor by extracting the logic
into a single shared module and import it from both places (or, if sharing is
impossible, add a clear cross-reference comment pointing to the server
implementation and ensure both copies reference a single source of truth),
update all usages to import the shared buildApiUrl function, and run tests to
verify behavior; if exporting/importing requires path aliasing, add the
necessary tsconfig/webpack alias and update imports accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 05e25803-bdce-4bed-b4a6-1f7166f93440
📒 Files selected for processing (5)
server/src/routes/proxy.tssrc/components/settings/AIConfigPanel.tsxsrc/services/aiService.tssrc/types/index.tssrc/utils/apiUrlBuilder.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/types/index.ts
added 3 commits
April 24, 2026 03:50
当选择搜索频道时，显示不同的空状态UI和提示信息，包括搜索图标和搜索引导文本，而非默认的刷新按钮和数据提示
实现使用IndexedDB存储和加载仓库AI分析结果，包括保存、加载、批量加载和删除功能。在DiscoveryView组件中集成该存储服务，确保分析结果在页面刷新后仍可保留。
调整卡片悬停时的阴影、边框和位移效果，统一桌面安全模式和非安全模式下的交互体验
Contributor
Author
|
可以合并 |
This was referenced Apr 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
refactor(RepositoryEditModal): 修复自定义描述为空时的处理逻辑
“趋势”页移除分页加载功能
Summary by CodeRabbit
New Features
Bug Fixes