fix: 修复Trending频道实现，添加后端代理端点，优化GitHub搜索查询#89
Conversation
- 修复 SubscriptionView.tsx 中重复的 trending 分支代码 - 优化 searchTrending 方法：从 7 天扩展到 30 天，添加最低 100 stars 阈值 - 添加后端代理端点 /api/proxy/github/trending 支持第三方 GitHubTrendingRSS API - 所有频道（Most Stars, Most Forks, Most DEV, Trending）均已具备中英文本地化 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
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:
📝 WalkthroughWalkthroughAdds a trending subscription channel with time-range and language filters, wires those filters into the trending search call, consolidates duplicate trending fetch logic in the subscription view, and extends store migration/normalization to ensure trending-related state keys and channel metadata exist. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as "SubscriptionView (UI)"
participant Store as "AppStore / persist"
participant Service as "GitHubApiService"
UI->>Store: read subscriptionChannels, repos, loading state
UI->>UI: user sets trendingTimeRange / trendingLanguage
UI->>Service: searchTrending(perPage=10, timeRange, language?)
Service-->>UI: return repos (created:>sinceDate, stars:>50, optional language)
UI->>Store: setSubscriptionRepos('trending', repos)
UI->>Store: setSubscriptionLastRefresh('trending', timestamp)
Note right of Store: persist.migrate / normalizePersistedState\nensures `trending` channel and default keys exist
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/services/githubApi.ts (1)
280-293: 30-day +stars:>100is a reasonable approximation, but note the channel semantics drift.Switching from a 7-day window to a 30-day window makes this closer to "trending this month" than "trending now", and
stars:>100will bias strongly toward repos that had time to accumulate stars — i.e., results skew to repos near the 30-day boundary rather than genuinely surging ones. If the intent (per PR description) is eventually to source real trending data from the new/api/proxy/github/trendingproxy, consider wiringsearchTrendingto call that proxy first and fall back to this approximation only when the proxy returns empty/errors — otherwise the backend proxy route is dead code.No functional defect in the query itself;
+is valid as a space in GitHub searchqvalues.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/githubApi.ts` around lines 280 - 293, Update searchTrending to prefer the new backend proxy first: call the proxy endpoint (via this.makeRequest or an existing proxy helper) for `/api/proxy/github/trending` and if it returns a non-empty result use that; only on error or empty response fall back to the current GitHub Search implementation in searchTrending (the existing created:>... query). Ensure you preserve the SubscriptionRepo shape (include rank, channel: 'trending', forks_count) and handle errors gracefully (catch and log or return fallback) so the proxy path is exercised when available and the existing approximation remains as a fallback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/src/routes/proxy.ts`:
- Around line 303-309: The hardcoded trendingBaseUrl in proxy.ts (symbol:
trendingBaseUrl) contains corrupt Chinese characters and is an invalid hostname;
replace it with the correct GitHubTrendingRSS JSON API host (use the intended
upstream host) and stop hardcoding it by reading the base URL from configuration
or an environment variable (e.g., process.env.TRENDING_BASE_URL) and update
where targetUrl is composed (symbol: targetUrl) to use that config value; ensure
you validate/throw a clear error if the env var is missing to avoid silent
fallback to “Trending API unavailable.”
- Around line 299-332: The new router.get('/api/proxy/github/trending') endpoint
is unused and error handling always returns 200; either wire searchTrending in
githubApi.ts to call the proxy endpoint (replace direct GitHub Search API call
with a fetch to /api/proxy/github/trending) or explicitly defer/remove the
endpoint, and change the endpoint's error responses so upstream failures return
non-2xx statuses (e.g., return 502 when proxyRequest returns non-200 or invalid
data, and 504/500 on exceptions/timeouts), using the existing proxyRequest call
and the route handler name to locate and update the logic.
---
Nitpick comments:
In `@src/services/githubApi.ts`:
- Around line 280-293: Update searchTrending to prefer the new backend proxy
first: call the proxy endpoint (via this.makeRequest or an existing proxy
helper) for `/api/proxy/github/trending` and if it returns a non-empty result
use that; only on error or empty response fall back to the current GitHub Search
implementation in searchTrending (the existing created:>... query). Ensure you
preserve the SubscriptionRepo shape (include rank, channel: 'trending',
forks_count) and handle errors gracefully (catch and log or return fallback) so
the proxy path is exercised when available and the existing approximation
remains as a fallback.
🪄 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: 76cc0161-aea4-4b61-907e-1ebad1238df8
📒 Files selected for processing (3)
server/src/routes/proxy.tssrc/components/SubscriptionView.tsxsrc/services/githubApi.ts
| router.get('/api/proxy/github/trending', async (req, res) => { | ||
| try { | ||
| const { language, since } = req.query as { language?: string; since?: string }; | ||
|
|
||
| // Try GitHubTrendingRSS JSON API first | ||
| const trendingBaseUrl = 'https://github-trending-api.wtf被她压住了.depsilon.net'; | ||
| const params = new URLSearchParams(); | ||
| if (language && language !== 'all') params.set('language', language); | ||
| if (since) params.set('since', since); | ||
|
|
||
| const targetUrl = `${trendingBaseUrl}/repositories${params.toString() ? '?' + params.toString() : ''}`; | ||
|
|
||
| const result = await proxyRequest({ | ||
| url: targetUrl, | ||
| method: 'GET', | ||
| headers: { | ||
| 'Accept': 'application/json', | ||
| 'User-Agent': 'GithubStarsManager-Backend', | ||
| }, | ||
| timeout: 10000, | ||
| }); | ||
|
|
||
| if (result.status === 200 && Array.isArray(result.data)) { | ||
| res.status(200).json({ repositories: result.data }); | ||
| return; | ||
| } | ||
|
|
||
| // Fallback: Return empty result if API is unavailable | ||
| res.status(200).json({ repositories: [], error: 'Trending API unavailable' }); | ||
| } catch (err) { | ||
| console.error('GitHub trending proxy error:', err); | ||
| res.status(200).json({ repositories: [], error: 'Trending proxy failed' }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP "api/proxy/github/trending|/proxy/github/trending" -C2
rg -nP "searchTrending\s*\(" -C2Repository: AmintaCCCP/GithubStarsManager
Length of output: 1306
🏁 Script executed:
# Read the full searchTrending implementation
sed -n '280,320p' src/services/githubApi.tsRepository: AmintaCCCP/GithubStarsManager
Length of output: 1647
🏁 Script executed:
# Check all calls to the proxy endpoint to see if it's used
rg "api/proxy/github/trending" -t ts -t tsx --no-headingRepository: AmintaCCCP/GithubStarsManager
Length of output: 100
🏁 Script executed:
# Get more context on the searchTrending method
sed -n '275,310p' src/services/githubApi.tsRepository: AmintaCCCP/GithubStarsManager
Length of output: 1487
🏁 Script executed:
# Search for any calls to the proxy endpoint
rg "/api/proxy/github/trending|api/proxy/github/trending" src/ --type tsRepository: AmintaCCCP/GithubStarsManager
Length of output: 55
🏁 Script executed:
# Get the full proxy route implementation including error handling
sed -n '299,340p' server/src/routes/proxy.tsRepository: AmintaCCCP/GithubStarsManager
Length of output: 1323
Wire the trending proxy endpoint into the frontend or defer it for a later PR.
The new /api/proxy/github/trending endpoint exists but is never called. searchTrending() in githubApi.ts directly hits GitHub's Search API (/search/repositories?q=created:>...) instead of routing through this proxy. No references to the proxy endpoint exist in the codebase, confirming it's currently unused.
Fix HTTP error handling to return non-2xx status codes.
All error cases—both upstream API failures and exceptions—currently return HTTP 200 with { repositories: [], error: ... }. This masks real failures from clients and monitoring tools. Return a non-2xx status (e.g., 502/504) on upstream failure or exception so callers can distinguish "no trending repos" from "service unavailable."
🤖 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 299 - 332, The new
router.get('/api/proxy/github/trending') endpoint is unused and error handling
always returns 200; either wire searchTrending in githubApi.ts to call the proxy
endpoint (replace direct GitHub Search API call with a fetch to
/api/proxy/github/trending) or explicitly defer/remove the endpoint, and change
the endpoint's error responses so upstream failures return non-2xx statuses
(e.g., return 502 when proxyRequest returns non-200 or invalid data, and 504/500
on exceptions/timeouts), using the existing proxyRequest call and the route
handler name to locate and update the logic.
| // Try GitHubTrendingRSS JSON API first | ||
| const trendingBaseUrl = 'https://github-trending-api.wtf被她压住了.depsilon.net'; | ||
| const params = new URLSearchParams(); | ||
| if (language && language !== 'all') params.set('language', language); | ||
| if (since) params.set('since', since); | ||
|
|
||
| const targetUrl = `${trendingBaseUrl}/repositories${params.toString() ? '?' + params.toString() : ''}`; |
There was a problem hiding this comment.
Critical: trendingBaseUrl contains Chinese characters — hostname is invalid and will always fail.
The host github-trending-api.wtf被她压住了.depsilon.net contains the Chinese string 被她压住了 embedded in the middle, which is clearly unintended (looks like pasted/corrupted content). DNS resolution against this host will fail for every request, so the endpoint silently falls through to the Trending API unavailable fallback 100% of the time, defeating the purpose of this new proxy.
Please correct the upstream base URL to the actual GitHubTrendingRSS JSON API host you intended to call, and ideally move it to a config/env var rather than hardcoding.
🔧 Suggested fix
- // Try GitHubTrendingRSS JSON API first
- const trendingBaseUrl = 'https://github-trending-api.wtf被她压住了.depsilon.net';
+ // Try GitHubTrendingRSS JSON API first
+ const trendingBaseUrl = process.env.GITHUB_TRENDING_API_BASE_URL
+ ?? 'https://<actual-trending-api-host>';🤖 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 303 - 309, The hardcoded
trendingBaseUrl in proxy.ts (symbol: trendingBaseUrl) contains corrupt Chinese
characters and is an invalid hostname; replace it with the correct
GitHubTrendingRSS JSON API host (use the intended upstream host) and stop
hardcoding it by reading the base URL from configuration or an environment
variable (e.g., process.env.TRENDING_BASE_URL) and update where targetUrl is
composed (symbol: targetUrl) to use that config value; ensure you validate/throw
a clear error if the env var is missing to avoid silent fallback to “Trending
API unavailable.”
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/store/useAppStore.ts (1)
229-250: Derive subscription defaults fromdefaultSubscriptionChannelsto avoid triple-duplication.The channel IDs
'most-stars' | 'most-forks' | 'most-dev' | 'trending'are now hardcoded in four places: the initial state (lines 434, 436, 437), this normalization block (231–234, 238–241, 245–248), and the migration block (1043–1050, where the full trending channel object is a verbatim copy ofdefaultSubscriptionChannels[3]at lines 382–389). Adding or renaming a channel in the future will require updating all of them in lockstep, which is easy to forget and already partially bit this PR (trending had to be backfilled everywhere).Consider centralizing a
SUBSCRIPTION_CHANNEL_IDSlist (or deriving it fromdefaultSubscriptionChannels) and using it to build the empty-record defaults, plus reusing the channel object itself in the migrate path.♻️ Sketch of the refactor
const defaultSubscriptionChannels: SubscriptionChannel[] = [ /* ... */ ]; + +const SUBSCRIPTION_CHANNEL_IDS = defaultSubscriptionChannels.map(c => c.id) as SubscriptionChannelId[]; + +const emptyRecord = <V,>(value: V): Record<SubscriptionChannelId, V> => + SUBSCRIPTION_CHANNEL_IDS.reduce((acc, id) => { + acc[id] = value; + return acc; + }, {} as Record<SubscriptionChannelId, V>);Then in
normalizePersistedState:- subscriptionRepos: { - 'most-stars': [], - 'most-forks': [], - 'most-dev': [], - 'trending': [], - ...(safePersisted.subscriptionRepos as Record<string, unknown> || {}), - }, - subscriptionLastRefresh: { - 'most-stars': null, - 'most-forks': null, - 'most-dev': null, - 'trending': null, - ...((safePersisted as Record<string, unknown>).subscriptionLastRefresh as Record<string, unknown> || {}), - }, - subscriptionIsLoading: { - 'most-stars': false, - 'most-forks': false, - 'most-dev': false, - 'trending': false, - ...((safePersisted as Record<string, unknown>).subscriptionIsLoading as Record<string, unknown> || {}), - }, + subscriptionRepos: { ...emptyRecord<SubscriptionRepo[]>([]), ...(safePersisted.subscriptionRepos ?? {}) }, + subscriptionLastRefresh: { ...emptyRecord<string | null>(null), ...((safePersisted as any).subscriptionLastRefresh ?? {}) }, + subscriptionIsLoading: { ...emptyRecord<boolean>(false), ...((safePersisted as any).subscriptionIsLoading ?? {}) },And in
migrate:- if (!hasTrending) { - console.log('Migrating: adding trending channel'); - state.subscriptionChannels.push({ - id: 'trending', - name: '热门趋势', - nameEn: 'Trending', - icon: '🔥', - description: 'GitHub 上近期最受关注的项目 Top 10', - enabled: true, - }); - } + const trendingChannel = defaultSubscriptionChannels.find(c => c.id === 'trending'); + if (!hasTrending && trendingChannel) { + console.log('Migrating: adding trending channel'); + state.subscriptionChannels.push({ ...trendingChannel }); + }Side note: the
... || {}fallbacks on lines 235, 242, and 249 only trigger for falsy values; a malformed persisted entry (e.g., an array) would still be spread. Narrowing totypeof x === 'object' && x !== null && !Array.isArray(x)would be more defensive, matching the style already used fordefaultCategoryOverrideson lines 216–219.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/store/useAppStore.ts` around lines 229 - 250, Replace the hardcoded channel keys with a single source of truth: derive SUBSCRIPTION_CHANNEL_IDS from defaultSubscriptionChannels (or export it alongside defaultSubscriptionChannels) and use it inside normalizePersistedState to build the default shapes for subscriptionRepos, subscriptionLastRefresh, and subscriptionIsLoading (instead of listing 'most-stars'|'most-forks'|'most-dev'|'trending' inline), and in the migrate function reuse the channel object from defaultSubscriptionChannels rather than copying it verbatim; also tighten the spread fallbacks in normalizePersistedState to only spread when the persisted value is a plain object (typeof x === 'object' && x !== null && !Array.isArray(x')) so malformed persisted arrays won't be merged into these maps.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/store/useAppStore.ts`:
- Around line 229-250: Replace the hardcoded channel keys with a single source
of truth: derive SUBSCRIPTION_CHANNEL_IDS from defaultSubscriptionChannels (or
export it alongside defaultSubscriptionChannels) and use it inside
normalizePersistedState to build the default shapes for subscriptionRepos,
subscriptionLastRefresh, and subscriptionIsLoading (instead of listing
'most-stars'|'most-forks'|'most-dev'|'trending' inline), and in the migrate
function reuse the channel object from defaultSubscriptionChannels rather than
copying it verbatim; also tighten the spread fallbacks in
normalizePersistedState to only spread when the persisted value is a plain
object (typeof x === 'object' && x !== null && !Array.isArray(x')) so malformed
persisted arrays won't be merged into these maps.
- searchTrending支持timeRange(daily/weekly/monthly)和language参数 - SubscriptionView添加Trending频道的筛选器UI（时间范围+语言） - normalizePersistedState和migrate确保所有频道有nameEn本地化 - 补全旧数据缺失的nameEn字段 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- 修复 normalizePersistedState 和 migrate：始终使用中文名称 defaultCh.name - searchTrending 改用 GitHubTrendingRSS API： - daily: https://mshibanami.github.io/GitHubTrendingRSS/daily/all.xml - weekly: https://mshibanami.github.io/GitHubTrendingRSS/weekly/all.xml - monthly: https://mshibanami.github.io/GitHubTrendingRSS/monthly/all.xml - 解析 RSS XML 获取 repo 信息（名称、链接、星标数、Fork数） Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1. 移除 Trending 频道的语言筛选输入框 2. 修复 RSS description HTML 实体解码（使用 innerHTML 解析） 3. 如果 RSS 数据中 stars/forks 为 0，自动调用 GitHub API 获取完整数据 4. 清理描述文本中的多余空白 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores