Skip to content

feat: 添加真正的 GitHub Trending 频道#88

Closed
AmintaCCCP wants to merge 5 commits intomainfrom
add-trending-channel
Closed

feat: 添加真正的 GitHub Trending 频道#88
AmintaCCCP wants to merge 5 commits intomainfrom
add-trending-channel

Conversation

@AmintaCCCP
Copy link
Copy Markdown
Owner

@AmintaCCCP AmintaCCCP commented Apr 20, 2026

概述

在趋势标签页内添加新的 Trending 订阅频道,展示 GitHub Trending 页面的真实数据。

修改内容

后端

  • 新增 /api/proxy/github/trending 路由
  • 爬取 GitHub Trending 页面并解析数据
  • 支持 daily/weekly/monthly 时间范围

前端

  • 修改 searchTrending 方法调用后端 API
  • 在 SubscriptionRepoCard 中显示「今日新增 star 数」
  • 修复重复的 trending 处理逻辑

类型

  • 添加 stars_today 字段到 SubscriptionRepo

效果

用户可以在趋势页面选择 Trending 频道,查看真实的 GitHub 热门项目列表,并看到每个项目今日新增的 star 数量。

注意事项

  • GitHub 没有官方 Trending API,此实现通过爬取页面获取数据
  • 需要启动后端服务才能正常工作

等待测试通过后再合并。

Summary by CodeRabbit

  • New Features

    • Added a "stars today" indicator to repository cards.
    • Added a trending channel and migration so it appears after upgrade.
  • Bug Fixes

    • Fixed trending refresh logic to fetch correct lists.
    • Improved trending data reliability via a backend proxy with RSS → HTML fallbacks and safer error handling.
  • Chores

    • Added XML parsing support and updated types to include daily star counts.

- 后端: 添加 /api/proxy/github/trending 路由,爬取 GitHub Trending 页面
- 前端: 修改 searchTrending 方法调用后端 API
- 类型: 添加 stars_today 字段到 SubscriptionRepo
- UI: 在 RepoCard 中显示今日新增 star 数

此实现使用真实的 GitHub Trending 数据,而非模拟数据。
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 20, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2c45bd9c-2966-462b-a287-f1a958adf554

📥 Commits

Reviewing files that changed from the base of the PR and between 510363a and d7212b5.

📒 Files selected for processing (1)
  • src/store/useAppStore.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/store/useAppStore.ts

📝 Walkthrough

Walkthrough

Added a backend POST proxy for GitHub Trending (RSS first, HTML fallback) and HTML parser; client now calls this proxy for trending results, subscription repo type gained optional stars_today, UI shows "stars today" badges, subscription channel refresh logic was simplified and persisted channels migrated/normalized, and xml2js was added.

Changes

Cohort / File(s) Summary
Backend Trending Proxy
server/src/routes/proxy.ts
New POST /api/proxy/github/trending route: attempts RSS fetch then falls back to HTML, added parseTrendingHtml, imports parseStringPromise, returns structured trendingRepos and a dedicated route error handler.
Service Integration
src/services/githubApi.ts
`searchTrending(perPage = 25, since = 'daily'
Types
src/types/index.ts
SubscriptionRepo extended with optional stars_today?: number.
UI — Card & View
src/components/SubscriptionRepoCard.tsx, src/components/SubscriptionView.tsx
SubscriptionRepoCard conditionally renders a green "stars today" indicator when stars_today > 0; SubscriptionView.refreshChannel() simplified and corrected (consolidated branches, fixed most-dev handling, removed duplicated/incorrect trending branches).
Packages
package.json, server/package.json
Added runtime xml2js (server) and dev @types/xml2js; adjusted dependency placements/ordering in server/package.json.
State Migration / Store
src/store/useAppStore.ts
Persisted subscriptionChannels migration: ensure trending channel exists, normalize legacy daily-devmost-dev, and normalize channel metadata (name, nameEn, icon).

Sequence Diagram

sequenceDiagram
    participant Client as Frontend Client
    participant Service as GitHubApiService
    participant Backend as Backend Proxy
    participant GitHub as GitHub
    participant Parser as Parser

    Client->>Service: searchTrending(perPage, since)
    Service->>Backend: POST /api/proxy/github/trending { since, per_page }
    Backend->>GitHub: GET Trending RSS (language path)
    GitHub-->>Backend: RSS (or non-200)
    alt RSS ok
        Backend->>Parser: parseTrendingRss(xml)
    else RSS failed
        Backend->>GitHub: GET Trending HTML page (language/since)
        GitHub-->>Backend: HTML
        Backend->>Parser: parseTrendingHtml(html)
    end
    Parser-->>Backend: trendingRepos[] (owner, name, description, language, color, stars, forks, stars_today)
    Backend-->>Service: { trendingRepos: [...] }
    Service-->>Client: SubscriptionRepo[] (mapped, includes stars_today)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 I hopped through RSS and HTML hay,
Pulled star-sprinkles from the trending way,
Tiny green badges show today's bright cheer,
I munched old branches and leapt a new tier,
Repos gleam — I twitch my whiskers, clear. ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: 添加真正的 GitHub Trending 频道' (feat: Add real GitHub Trending channel) clearly and specifically describes the main change in the pull request—adding a new Trending subscription channel with actual data from GitHub Trending pages.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ 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 add-trending-channel

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.

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: 5

🧹 Nitpick comments (4)
src/components/SubscriptionRepoCard.tsx (1)

150-156: Simplify guard and consider layout polish.

  • The repo.stars_today !== undefined check is redundant: undefined > 0 already evaluates to false, so repo.stars_today > 0 alone is sufficient (TypeScript also narrows number | undefined correctly against > 0 in strict mode via a simple !!repo.stars_today && repo.stars_today > 0 or just (repo.stars_today ?? 0) > 0 if you prefer explicitness).
  • Indentation of this block is inconsistent with the surrounding JSX (single leading space), and it has no top margin so it will visually crowd the stats row above it. Consider integrating the indicator inline into the stats row (line 131) or adding mt-2 to separate it.
✏️ Suggested refactor
-          </div>
- {repo.stars_today !== undefined && repo.stars_today > 0 && (
- <div className="flex items-center gap-1 text-green-600 dark:text-green-400">
- <span className="text-xs font-medium">+{formatNumber(repo.stars_today)}</span>
- <span className="text-xs">{t('今日', 'today')}</span>
- </div>
- )}
- </div>
+            {repo.stars_today !== undefined && repo.stars_today > 0 && (
+              <div className="flex items-center gap-1 text-green-600 dark:text-green-400">
+                <span className="text-xs font-medium">+{formatNumber(repo.stars_today)}</span>
+                <span className="text-xs">{t('今日', 'today')}</span>
+              </div>
+            )}
+          </div>
+        </div>

(Moving the indicator into the stats row so it appears inline with stars/forks.)

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

In `@src/components/SubscriptionRepoCard.tsx` around lines 150 - 156, In
SubscriptionRepoCard, simplify the conditional rendering for the today-stats
indicator by removing the redundant repo.stars_today !== undefined check and use
a single guard like (repo.stars_today ?? 0) > 0 or repo.stars_today > 0; then
fix JSX indentation to match surrounding markup and either move the indicator
inline into the existing stats row (where stars/forks are rendered) using
formatNumber(repo.stars_today) and t('今日','today') or add a small spacing class
(e.g. mt-2) to the container so it doesn’t crowd the row above.
src/services/githubApi.ts (1)

280-280: since parameter is accepted but not plumbed through from callers.

The new optional since: 'daily' | 'weekly' | 'monthly' is a nice addition, but the only caller — SubscriptionView.refreshChannel at line 62 — invokes searchTrending(25) with no second argument, so weekly/monthly is unreachable from the UI. If the PR's stated goal is to support all three ranges, consider either (a) exposing a range selector in the subscription sidebar/toolbar, or (b) trimming the parameter until the UI lands to avoid dead surface area.

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

In `@src/services/githubApi.ts` at line 280, The searchTrending method now accepts
a since: 'daily'|'weekly'|'monthly' parameter but callers never pass it; update
the call site in SubscriptionView.refreshChannel to forward the selected range
(e.g., call searchTrending(perPage, selectedRange)) so the UI can request
weekly/monthly trends, and add a UI control or existing state value (the
subscription sidebar/toolbar selection) as the source of selectedRange;
alternatively, if UI wiring isn't ready, remove the since parameter from
searchTrending to avoid dead surface area—look for the searchTrending function
and SubscriptionView.refreshChannel to implement either forwarding of the
selected range or removal of the unused parameter.
server/src/routes/proxy.ts (1)

292-375: Prefer cheerio over hand-rolled regex for HTML parsing.

The comment already acknowledges this. Each regex here is brittle to the smallest markup tweak by GitHub (class reordering, attribute quoting, whitespace changes inside <h2> / <svg>), and a single upstream change will silently drop fields to null/0 without any signal. Since this runs on the backend, pulling in cheerio (a very small, jsdom-free parser) would turn this into a handful of CSS selectors that match GitHub's documented Trending DOM structure (article.Box-row, h2 a, p.col-9, [itemprop="programmingLanguage"], a[href$="/stargazers"], span.d-inline-block.float-sm-right). Recommended as a follow-up given the backend already owns this endpoint.

src/components/SubscriptionView.tsx (1)

62-62: Collapse-to-one-line hurts readability; break this into a normal if/else chain or a switch.

The entire per-channel branching was compressed onto a single ~350-column line, which makes diff review, debugging (breakpoints/stack-traces mapping back to the right branch), and future additions (e.g., the planned weekly/monthly trending variants) noticeably harder. Please restore a multi-line form.

♻️ Suggested refactor
-      if (normalizedId === 'most-stars') { const repos = await githubApi.searchMostStars(10); setSubscriptionRepos('most-stars', repos); } else if (normalizedId === 'most-forks') { const repos = await githubApi.searchMostForks(10); setSubscriptionRepos('most-forks', repos); } else if (normalizedId === 'most-dev') { const devs = await githubApi.searchDailyDevs(10); setSubscriptionDevs(devs); } else if (normalizedId === 'trending') { const repos = await githubApi.searchTrending(25); setSubscriptionRepos('trending', repos); }
+      if (normalizedId === 'most-stars') {
+        const repos = await githubApi.searchMostStars(10);
+        setSubscriptionRepos('most-stars', repos);
+      } else if (normalizedId === 'most-forks') {
+        const repos = await githubApi.searchMostForks(10);
+        setSubscriptionRepos('most-forks', repos);
+      } else if (normalizedId === 'most-dev') {
+        const devs = await githubApi.searchDailyDevs(10);
+        setSubscriptionDevs(devs);
+      } else if (normalizedId === 'trending') {
+        const repos = await githubApi.searchTrending(25);
+        setSubscriptionRepos('trending', repos);
+      }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/SubscriptionView.tsx` at line 62, The current single-line
chained conditional using normalizedId compresses multiple branches
(normalizedId checks and calls to githubApi.searchMostStars, searchMostForks,
searchDailyDevs, searchTrending and mutators
setSubscriptionRepos/setSubscriptionDevs) into one unreadable line; expand this
into a multi-line if/else-if/else chain or a switch on normalizedId so each
branch is on its own lines, calling githubApi.searchMostStars(10) then
setSubscriptionRepos('most-stars', repos), githubApi.searchMostForks(10) then
setSubscriptionRepos('most-forks', repos), githubApi.searchDailyDevs(10) then
setSubscriptionDevs(devs), and githubApi.searchTrending(25) then
setSubscriptionRepos('trending', repos) respectively, preserving the same logic
and await handling.
🤖 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 261-268: Validate and reject invalid inputs before building the
upstream URL: ensure the extracted variable since from req.body (in the proxy
route where const { since = 'daily', language } is declared) is one of the
allowed values "daily", "weekly", or "monthly" and return a 400 for anything
else; for language, avoid accepting arbitrary strings by either checking against
a whitelist of supported language slugs used by GitHub trending or at minimum
restricting to a safe pattern (e.g., letters, digits, hyphens, underscores) and
then use encodeURIComponent when appending to url; if validation fails, respond
with a clear 400 error and do not interpolate the bad value into the url
variable used to fetch https://github.com/trending.
- Around line 258-289: The /api/proxy/github/trending POST route is being
shadowed by the generic /api/proxy/github/* wildcard handler — move/register the
trending route (router.post('/api/proxy/github/trending', ...)) before the
wildcard generic proxy handler (and ensure other specific routes like
/api/proxy/github/search/repositories and /api/proxy/github/search/users are
also registered before the wildcard) so it can be reached; validate the incoming
since param against allowed values ['daily','weekly','monthly'] and reject
invalid values; update parseTrendingHtml to accept the requested since value (do
not hardcode since: 'daily') and return correctly labeled counts, and
rename/replace the stars_today logic to stars_in_period (or equivalent) and
adjust its regex to match period phrasing (e.g., "stars today" / "stars this
week" / "stars this month") or derive the phrase from the since param so counts
reflect the requested period.
- Around line 356-370: The parser currently only recognizes "stars today" and
hardcodes since: 'daily'; update the regex in parseTrendingHtml to match all
variants (e.g., /([\d,]+)\s*stars\s+(?:today|this\s+week|this\s+month)/i) so
stars_today (or equivalent) captures daily/weekly/monthly counts, and thread the
since parameter through by changing parseTrendingHtml signature to
parseTrendingHtml(html: string, since: string) and passing the request's since
value from the route handler (where since is extracted) into parseTrendingHtml;
also replace the hard-coded since: 'daily' in the returned repo objects with the
passed since value.

In `@src/services/githubApi.ts`:
- Around line 305-318: The mapped trending-repo object violates
Repository/SubscriptionRepo: change id from repo.full_name to a numeric id
(e.g., compute a stable numeric hash from full_name or fetch the real repo id
via /repos/{owner}/{name}) so React keys and by-id lookups work; populate
owner.avatar_url (e.g., `https://github.com/${repo.owner}.png`) so
SubscriptionRepoCard's <img> is valid; and add required fields created_at,
updated_at, pushed_at (stub ISO timestamps) and topics (empty array) to the
mapped object to satisfy types and avoid runtime crashes in sorting/search/AI
flows; update the mapping code (the map callback that returns the object) and
ensure the result is typed/cast to Repository/SubscriptionRepo or validated
before returning so TypeScript checks pass.
- Around line 283-287: The code currently uses a hard-coded
fetch('/api/proxy/github/trending', ...) in searchTrending; replace that with a
call into the existing backend adapter: add a searchTrending(since: string,
language?: string) method on BackendAdapter that POSTs to
`${this._backendUrl}/proxy/github/trending` using the adapter's headers/error
handling, then in src/services/githubApi.ts call backend.searchTrending(since,
language) instead of fetch; also stop sending per_page (the server expects only
since and language) or, if paging is required, add server support and wire it
through consistently—prefer dropping per_page in the request body to match
server handler.

---

Nitpick comments:
In `@src/components/SubscriptionRepoCard.tsx`:
- Around line 150-156: In SubscriptionRepoCard, simplify the conditional
rendering for the today-stats indicator by removing the redundant
repo.stars_today !== undefined check and use a single guard like
(repo.stars_today ?? 0) > 0 or repo.stars_today > 0; then fix JSX indentation to
match surrounding markup and either move the indicator inline into the existing
stats row (where stars/forks are rendered) using formatNumber(repo.stars_today)
and t('今日','today') or add a small spacing class (e.g. mt-2) to the container so
it doesn’t crowd the row above.

In `@src/components/SubscriptionView.tsx`:
- Line 62: The current single-line chained conditional using normalizedId
compresses multiple branches (normalizedId checks and calls to
githubApi.searchMostStars, searchMostForks, searchDailyDevs, searchTrending and
mutators setSubscriptionRepos/setSubscriptionDevs) into one unreadable line;
expand this into a multi-line if/else-if/else chain or a switch on normalizedId
so each branch is on its own lines, calling githubApi.searchMostStars(10) then
setSubscriptionRepos('most-stars', repos), githubApi.searchMostForks(10) then
setSubscriptionRepos('most-forks', repos), githubApi.searchDailyDevs(10) then
setSubscriptionDevs(devs), and githubApi.searchTrending(25) then
setSubscriptionRepos('trending', repos) respectively, preserving the same logic
and await handling.

In `@src/services/githubApi.ts`:
- Line 280: The searchTrending method now accepts a since:
'daily'|'weekly'|'monthly' parameter but callers never pass it; update the call
site in SubscriptionView.refreshChannel to forward the selected range (e.g.,
call searchTrending(perPage, selectedRange)) so the UI can request
weekly/monthly trends, and add a UI control or existing state value (the
subscription sidebar/toolbar selection) as the source of selectedRange;
alternatively, if UI wiring isn't ready, remove the since parameter from
searchTrending to avoid dead surface area—look for the searchTrending function
and SubscriptionView.refreshChannel to implement either forwarding of the
selected range or removal of the unused parameter.
🪄 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: e442b3c3-f222-47ce-b146-d37e5f1a0c16

📥 Commits

Reviewing files that changed from the base of the PR and between be636f5 and 67a44c6.

⛔ Files ignored due to path filters (2)
  • dist/index.html is excluded by !**/dist/**
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (5)
  • server/src/routes/proxy.ts
  • src/components/SubscriptionRepoCard.tsx
  • src/components/SubscriptionView.tsx
  • src/services/githubApi.ts
  • src/types/index.ts

Comment on lines +258 to +289
// POST /api/proxy/github/trending
router.post('/api/proxy/github/trending', async (req, res) => {
try {
const { since = 'daily', language } = req.body as { since?: string; language?: string };

// 构建请求 URL
let url = 'https://github.com/trending';
if (language) {
url += `/${encodeURIComponent(language)}`;
}
url += `?since=${since}`;

const headers: Record<string, string> = {
'Accept': 'text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8',
'Accept-Language': 'en-US,en;q=0.5',
'User-Agent': 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36',
};

const result = await proxyRequest({ url, method: 'GET', headers });

// 解析 HTML 提取 trending 数据
if (result.status === 200 && typeof result.data === 'string') {
const repos = parseTrendingHtml(result.data);
res.json(repos);
} else {
res.status(result.status).json(result.data);
}
} catch (err) {
console.error('GitHub trending proxy error:', err);
res.status(500).json({ error: 'GitHub trending proxy failed', code: 'GITHUB_TRENDING_PROXY_FAILED' });
}
});
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
# Confirm registration order and route definitions
rg -nP "router\.(post|get|use|all)\s*\(\s*['\"]/api/proxy/github" server/src/routes/proxy.ts

Repository: AmintaCCCP/GithubStarsManager

Length of output: 351


Critical: this route is unreachable — it is shadowed by the /api/proxy/github/* wildcard handler registered at line 46.

In Express.js, routes are matched in registration order. Since /api/proxy/github/* is registered first (line 46), it matches and handles any POST request to /api/proxy/github/trending before the more specific handler at line 259 is ever evaluated. The trending handler is dead code.

A request to this endpoint will instead hit the wildcard handler, which will:

  1. Require a GitHub token (returns 400 if absent).
  2. Attempt https://api.github.com/trending with Bearer authentication — an endpoint that does not exist on the REST API, returning 404.

Move this handler above line 46, or reorder all specific routes (/api/proxy/github/search/repositories at line 220 and /api/proxy/github/search/users at line 378) above the wildcard to resolve the shadowing.

Additionally:

  • Validate since input against allowed values (['daily', 'weekly', 'monthly']) to prevent unintended URL injection.
  • parseTrendingHtml hardcodes since: 'daily' regardless of the request parameter — this will mislabel weekly/monthly trending data.
  • The stars_today regex only matches "X stars today" but GitHub pages for since=weekly show "X stars this week" and since=monthly show "X stars this month". Consider renaming to stars_in_period and adjust the regex accordingly.
🤖 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 258 - 289, The
/api/proxy/github/trending POST route is being shadowed by the generic
/api/proxy/github/* wildcard handler — move/register the trending route
(router.post('/api/proxy/github/trending', ...)) before the wildcard generic
proxy handler (and ensure other specific routes like
/api/proxy/github/search/repositories and /api/proxy/github/search/users are
also registered before the wildcard) so it can be reached; validate the incoming
since param against allowed values ['daily','weekly','monthly'] and reject
invalid values; update parseTrendingHtml to accept the requested since value (do
not hardcode since: 'daily') and return correctly labeled counts, and
rename/replace the stars_today logic to stars_in_period (or equivalent) and
adjust its regex to match period phrasing (e.g., "stars today" / "stars this
week" / "stars this month") or derive the phrase from the since param so counts
reflect the requested period.

Comment thread server/src/routes/proxy.ts Outdated
Comment on lines +261 to +268
const { since = 'daily', language } = req.body as { since?: string; language?: string };

// 构建请求 URL
let url = 'https://github.com/trending';
if (language) {
url += `/${encodeURIComponent(language)}`;
}
url += `?since=${since}`;
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

Validate since and sanitize language before interpolating into the upstream URL.

since is taken verbatim from the request body and appended to https://github.com/trending?since=${since}, so a caller can inject arbitrary query fragments (e.g. since=daily&foo=bar). GitHub only accepts daily | weekly | monthly; anything else silently falls back to daily on GitHub's side but still round-trips through your server. Restrict to the allowed set and reject the request otherwise.

language is encodeURIComponent'd, which is fine for path traversal, but also worth whitelisting to avoid cache-busting a fetch pool with arbitrary strings.

🛡️ Suggested fix
-    const { since = 'daily', language } = req.body as { since?: string; language?: string };
-    
-    // 构建请求 URL
-    let url = 'https://github.com/trending';
-    if (language) {
-      url += `/${encodeURIComponent(language)}`;
-    }
-    url += `?since=${since}`;
+    const { since = 'daily', language } = req.body as { since?: string; language?: string };
+    const allowedSince = ['daily', 'weekly', 'monthly'] as const;
+    if (!allowedSince.includes(since as typeof allowedSince[number])) {
+      res.status(400).json({ error: 'Invalid "since" value', code: 'INVALID_SINCE' });
+      return;
+    }
+    if (language && !/^[a-zA-Z0-9+#._-]{1,40}$/.test(language)) {
+      res.status(400).json({ error: 'Invalid "language" value', code: 'INVALID_LANGUAGE' });
+      return;
+    }
+
+    let url = 'https://github.com/trending';
+    if (language) url += `/${encodeURIComponent(language)}`;
+    url += `?since=${since}`;
🤖 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 261 - 268, Validate and reject
invalid inputs before building the upstream URL: ensure the extracted variable
since from req.body (in the proxy route where const { since = 'daily', language
} is declared) is one of the allowed values "daily", "weekly", or "monthly" and
return a 400 for anything else; for language, avoid accepting arbitrary strings
by either checking against a whitelist of supported language slugs used by
GitHub trending or at minimum restricting to a safe pattern (e.g., letters,
digits, hyphens, underscores) and then use encodeURIComponent when appending to
url; if validation fails, respond with a clear 400 error and do not interpolate
the bad value into the url variable used to fetch https://github.com/trending.

Comment on lines +356 to +370
const starsTodayMatch = article.match(/([\d,]+)\s*stars\s*today/i);
const stars_today = starsTodayMatch ? parseInt(starsTodayMatch[1].replace(/,/g, ''), 10) : 0;

repos.push({
name,
owner,
full_name,
html_url,
description,
language,
language_color,
stars,
forks,
stars_today,
since: 'daily',
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

🌐 Web query:

What markup does github.com/trending use for "stars today" / "stars this week" / "stars this month" labels?

💡 Result:

The "stars today", "stars this week", and "stars this month" labels on github.com/trending are plain text appended to repository entries in the HTML, such as "1,254 stars today". They are contained within <span class="d-inline-block float-sm-right"> elements, based on scrapers parsing the page. The repository rows use <article class="Box-row">. The exact markup varies by time period selected (today/this week/this month), but follows this pattern for the period-specific stars count.

Citations:


🏁 Script executed:

# First, let's examine the proxy.ts file around the identified lines and find the parseTrendingHtml function
cat -n server/src/routes/proxy.ts | head -400 | tail -100

Repository: AmintaCCCP/GithubStarsManager

Length of output: 4113


🏁 Script executed:

# Find the parseTrendingHtml function definition
rg -n "parseTrendingHtml" server/src/routes/proxy.ts -A 20 -B 5

Repository: AmintaCCCP/GithubStarsManager

Length of output: 1249


🏁 Script executed:

# Check the full context around lines 356-370 to see the entire function
sed -n '340,380p' server/src/routes/proxy.ts

Repository: AmintaCCCP/GithubStarsManager

Length of output: 1440


🏁 Script executed:

# Find where the route handler calls parseTrendingHtml to see if since is passed
rg -n "parseTrendingHtml" server/src/routes/proxy.ts -B 10

Repository: AmintaCCCP/GithubStarsManager

Length of output: 1060


🏁 Script executed:

# Find the route handler that calls parseTrendingHtml
sed -n '250,290p' server/src/routes/proxy.ts

Repository: AmintaCCCP/GithubStarsManager

Length of output: 1617


🏁 Script executed:

# Search for the full route definition to see where since parameter comes from
rg -n "trending" server/src/routes/proxy.ts -B 20 | head -60

Repository: AmintaCCCP/GithubStarsManager

Length of output: 2228


Fix regex pattern and thread since parameter through to parser.

Two confirmed correctness issues:

  1. The regex ([\d,]+)\s*stars\s*today only matches GitHub's daily wording. For since=weekly / since=monthly, GitHub renders "stars this week" / "stars this month", causing stars_today to always be 0. Expand the regex to match all three patterns: /([\d,]+)\s*stars\s+(?:today|this\s+week|this\s+month)/i.

  2. The since parameter is extracted from the request (line 261) but never passed to parseTrendingHtml(), causing it to hard-code since: 'daily' in every returned item (line 370). Update the function signature to parseTrendingHtml(html: string, since: string) and pass since from the route handler (line 280).

Suggested fix
-      const repos = parseTrendingHtml(result.data);
+      const repos = parseTrendingHtml(result.data, since);

-function parseTrendingHtml(html: string): Array<{
+function parseTrendingHtml(html: string, since: string): Array<{

-    const starsTodayMatch = article.match(/([\d,]+)\s*stars\s*today/i);
+    const starsPeriodMatch = article.match(/([\d,]+)\s*stars\s+(?:today|this\s+week|this\s+month)/i);
-    const stars_today = starsTodayMatch ? parseInt(starsTodayMatch[1].replace(/,/g, ''), 10) : 0;
+    const stars_today = starsPeriodMatch ? parseInt(starsPeriodMatch[1].replace(/,/g, ''), 10) : 0;

-      since: 'daily',
+      since,
🤖 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 356 - 370, The parser currently only
recognizes "stars today" and hardcodes since: 'daily'; update the regex in
parseTrendingHtml to match all variants (e.g.,
/([\d,]+)\s*stars\s+(?:today|this\s+week|this\s+month)/i) so stars_today (or
equivalent) captures daily/weekly/monthly counts, and thread the since parameter
through by changing parseTrendingHtml signature to parseTrendingHtml(html:
string, since: string) and passing the request's since value from the route
handler (where since is extracted) into parseTrendingHtml; also replace the
hard-coded since: 'daily' in the returned repo objects with the passed since
value.

Comment thread src/services/githubApi.ts
Comment on lines +283 to +287
const response = await fetch('/api/proxy/github/trending', {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ since, per_page: perPage }),
});
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
# Confirm backendAdapter shape / helper to use, and survey existing proxy callers.
fd -t f backendAdapter src
rg -nP "backend(Adapter)?\.(post|request|call)\(" src -C2

Repository: AmintaCCCP/GithubStarsManager

Length of output: 103


🏁 Script executed:

# Examine backendAdapter.ts to understand the API
head -100 src/services/backendAdapter.ts

# Also check githubApi.ts around the problem lines
sed -n '275,295p' src/services/githubApi.ts

# Find actual usages of backendAdapter
rg "backendAdapter\." src -A2 | head -50

Repository: AmintaCCCP/GithubStarsManager

Length of output: 3945


🏁 Script executed:

# Check if backendAdapter is imported in githubApi.ts
head -30 src/services/githubApi.ts

# Check the server proxy handler
grep -n "per_page\|since\|language" server/src/routes/proxy.ts | head -20

# Find the actual handler at line 261
sed -n '250,280p' server/src/routes/proxy.ts

# Check usages of searchTrending
rg "searchTrending" src -B2 -A2

Repository: AmintaCCCP/GithubStarsManager

Length of output: 3843


🏁 Script executed:

# Get the complete backendAdapter.ts to see all methods
wc -l src/services/backendAdapter.ts

# Check for trending-related methods
rg "trending|Trending" src/services/backendAdapter.ts

Repository: AmintaCCCP/GithubStarsManager

Length of output: 107


🏁 Script executed:

# Read the full backendAdapter.ts to see all available proxy methods
cat -n src/services/backendAdapter.ts | grep -A 5 "async.*Repo\|async.*proxy"

Repository: AmintaCCCP/GithubStarsManager

Length of output: 2933


🏁 Script executed:

# Check if backendAdapter is imported anywhere in githubApi.ts
grep "import.*backendAdapter\|from.*backendAdapter" src/services/githubApi.ts

# Check the end of backendAdapter.ts to see if there are any other methods
tail -50 src/services/backendAdapter.ts

# Check how githubApi.ts instantiates and uses things
grep -n "class\|export\|new " src/services/githubApi.ts | head -20

Repository: AmintaCCCP/GithubStarsManager

Length of output: 2553


🏁 Script executed:

# Double check: is backendAdapter imported/used anywhere in githubApi.ts
cat src/services/githubApi.ts | head -50

# Verify the exact lines of the problematic code
sed -n '280,295p' src/services/githubApi.ts

Repository: AmintaCCCP/GithubStarsManager

Length of output: 1781


Use the existing backend adapter instead of a hard-coded relative URL.

fetch('/api/proxy/github/trending', ...) assumes the frontend is served from the same origin as the backend, which breaks in Vite dev (frontend on :5173, backend elsewhere) and in any split-deploy setup — in those cases this call 404s and searchTrending silently returns []. The rest of the codebase goes through src/services/backendAdapter.ts (which exports a backend singleton), so this should too to pick up the configured base URL, auth headers, and error handling consistently. Add a searchTrending method to BackendAdapter that uses ${this._backendUrl}/proxy/github/trending.

Also, the payload includes per_page: perPage but the backend handler at server/src/routes/proxy.ts:261 only destructures since and languageper_page is unused on the server, so either wire it through or drop it from the request body.

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

In `@src/services/githubApi.ts` around lines 283 - 287, The code currently uses a
hard-coded fetch('/api/proxy/github/trending', ...) in searchTrending; replace
that with a call into the existing backend adapter: add a searchTrending(since:
string, language?: string) method on BackendAdapter that POSTs to
`${this._backendUrl}/proxy/github/trending` using the adapter's headers/error
handling, then in src/services/githubApi.ts call backend.searchTrending(since,
language) instead of fetch; also stop sending per_page (the server expects only
since and language) or, if paging is required, add server support and wire it
through consistently—prefer dropping per_page in the request body to match
server handler.

Comment thread src/services/githubApi.ts
Comment on lines +305 to +318
}, index: number) => ({
id: repo.full_name,
name: repo.name,
full_name: repo.full_name,
owner: { login: repo.owner },
html_url: repo.html_url,
description: repo.description || '',
language: repo.language,
stargazers_count: repo.stars,
forks_count: repo.forks,
stars_today: repo.stars_today,
rank: index + 1,
channel: 'trending' as const,
}));
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

Critical: the mapped object doesn't satisfy SubscriptionRepo / Repository — wrong id type and missing required fields needed by the UI.

Several issues in the mapping will either fail type-checking or break the UI at runtime:

  1. id: repo.full_nameRepository.id is declared as number in src/types/index.ts, not a string. This is a type violation, and it also silently breaks:

    • React reconciliation via key={repo.id} in SubscriptionView (line 370).
    • Any by-id lookups (e.g. updateSubscriptionRepo finding a trending item after AI analysis).
      Consider using a stable numeric hash of full_name, or having the backend proxy additionally call /repos/{owner}/{name} to enrich with the real numeric id.
  2. owner: { login: repo.owner }Repository.owner requires avatar_url: string. SubscriptionRepoCard unconditionally renders <img src={repo.owner.avatar_url}> (line 62), so every trending card will show a broken avatar. Populate it, e.g. avatar_url: \https://github.com/${repo.owner}.png\``, or parse the avatar out of the trending HTML on the server.

  3. Missing required fields: created_at, updated_at, pushed_at, and topics are all required on Repository. At minimum stub them to avoid downstream crashes in code paths that assume they exist (sorting, search filters, AI analysis flow).

🐛 Suggested mapping
-      return (trendingRepos || []).slice(0, perPage).map((repo: {
+      const now = new Date().toISOString();
+      return (trendingRepos || []).slice(0, perPage).map((repo: {
         name: string;
         owner: string;
         full_name: string;
         html_url: string;
         description: string;
         language: string | null;
         stars: number;
         forks: number;
         stars_today: number;
       }, index: number) => ({
-        id: repo.full_name,
+        // Stable numeric id derived from full_name (replace with real id from GitHub if backend is enriched)
+        id: Array.from(repo.full_name).reduce((h, c) => (h * 31 + c.charCodeAt(0)) | 0, 0),
         name: repo.name,
         full_name: repo.full_name,
-        owner: { login: repo.owner },
+        owner: {
+          login: repo.owner,
+          avatar_url: `https://github.com/${repo.owner}.png`,
+        },
         html_url: repo.html_url,
         description: repo.description || '',
         language: repo.language,
         stargazers_count: repo.stars,
         forks_count: repo.forks,
         stars_today: repo.stars_today,
+        topics: [],
+        created_at: now,
+        updated_at: now,
+        pushed_at: now,
         rank: index + 1,
         channel: 'trending' as const,
       }));
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/services/githubApi.ts` around lines 305 - 318, The mapped trending-repo
object violates Repository/SubscriptionRepo: change id from repo.full_name to a
numeric id (e.g., compute a stable numeric hash from full_name or fetch the real
repo id via /repos/{owner}/{name}) so React keys and by-id lookups work;
populate owner.avatar_url (e.g., `https://github.com/${repo.owner}.png`) so
SubscriptionRepoCard's <img> is valid; and add required fields created_at,
updated_at, pushed_at (stub ISO timestamps) and topics (empty array) to the
mapped object to satisfy types and avoid runtime crashes in sorting/search/AI
flows; update the mapping code (the map callback that returns the object) and
ensure the result is typed/cast to Repository/SubscriptionRepo or validated
before returning so TypeScript checks pass.

- 后端: 集成 xml2js,优先通过 GitHubTrendingRSS 获取数据
- 后端: 保留原网页爬取作为 RSS 失败时的回退方案
- 前端: 修复并优化相关逻辑
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 (1)
server/package.json (1)

14-14: Move @types/xml2js to devDependencies.

TypeScript @types/* packages are only needed at build/type-check time and should live in devDependencies (consistent with @types/better-sqlite3, @types/cors, @types/express, @types/morgan already placed there). Keeping them in runtime dependencies unnecessarily bloats production installs.

♻️ Proposed fix
   "dependencies": {
-    "@types/xml2js": "^0.4.14",
     "better-sqlite3": "^11.0.0",
     "cors": "^2.8.5",
     "express": "^4.21.0",
     "helmet": "^7.1.0",
     "morgan": "^1.10.0",
     "xml2js": "^0.6.2"
   },
   "devDependencies": {
     "@types/better-sqlite3": "^7.6.8",
     "@types/cors": "^2.8.17",
     "@types/express": "^4.17.21",
     "@types/morgan": "^1.9.9",
     "@types/supertest": "^6.0.2",
+    "@types/xml2js": "^0.4.14",
     "supertest": "^6.3.4",
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/package.json` at line 14, The package "@types/xml2js" is listed under
dependencies but should be a dev-only type dependency; update package.json by
removing "@types/xml2js" from "dependencies" and adding the same version string
to "devDependencies" so it matches the other `@types` entries (e.g., alongside
"@types/better-sqlite3", "@types/cors", "@types/express", "@types/morgan"); run
an install (npm/yarn/pnpm) to update lockfile afterwards.
🤖 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/package.json`:
- Line 20: The route calls an undefined parseTrendingRss; implement
parseTrendingRss to parse RSS XML via the already-imported parseStringPromise,
extract items (title, link, pubDate/iso date, description/summary, author if
present), normalize each item into the same shape parseTrendingHtml returns,
filter items by the provided since timestamp, and return the merged/normalized
list so the existing call to parseTrendingRss(result.data, since) succeeds;
ensure parseTrendingRss handles parse errors (fallback to empty array) and
matches the field names/structure expected by the rest of the code that
currently uses parseTrendingHtml.

---

Nitpick comments:
In `@server/package.json`:
- Line 14: The package "@types/xml2js" is listed under dependencies but should
be a dev-only type dependency; update package.json by removing "@types/xml2js"
from "dependencies" and adding the same version string to "devDependencies" so
it matches the other `@types` entries (e.g., alongside "@types/better-sqlite3",
"@types/cors", "@types/express", "@types/morgan"); run an install
(npm/yarn/pnpm) to update lockfile afterwards.
🪄 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: 94212c07-18a8-440a-ae86-9ac9e1d85b7c

📥 Commits

Reviewing files that changed from the base of the PR and between 67a44c6 and 343ee71.

⛔ Files ignored due to path filters (2)
  • package-lock.json is excluded by !**/package-lock.json
  • server/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (3)
  • package.json
  • server/package.json
  • server/src/routes/proxy.ts
✅ Files skipped from review due to trivial changes (1)
  • package.json
🚧 Files skipped from review as they are similar to previous changes (1)
  • server/src/routes/proxy.ts

Comment thread server/package.json
"helmet": "^7.1.0",
"morgan": "^1.10.0"
"morgan": "^1.10.0",
"xml2js": "^0.6.2"
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
# Confirm whether parseTrendingRss is defined and whether parseStringPromise is used.
echo "--- definitions of parseTrendingRss ---"
rg -nP '\b(function|const|let|var)\s+parseTrendingRss\b|\bparseTrendingRss\s*[:=]\s*(async\s*)?\(' -C2
echo "--- all references to parseTrendingRss ---"
rg -nP '\bparseTrendingRss\b' -C2
echo "--- uses of parseStringPromise ---"
rg -nP '\bparseStringPromise\s*\(' -C2
echo "--- xml2js imports ---"
rg -nP "from\s+['\"]xml2js['\"]" -C1

Repository: AmintaCCCP/GithubStarsManager

Length of output: 678


🏁 Script executed:

sed -n '265,280p' server/src/routes/proxy.ts

Repository: AmintaCCCP/GithubStarsManager

Length of output: 870


🏁 Script executed:

rg -n 'parseTrendingHtml' -C2

Repository: AmintaCCCP/GithubStarsManager

Length of output: 748


Implement missing parseTrendingRss function — RSS trending route will crash at runtime.

parseStringPromise is imported from xml2js but never used anywhere in the codebase. More critically, server/src/routes/proxy.ts:272 calls parseTrendingRss(result.data, since) but this function is not defined anywhere in the repository. When the /api/proxy/github/trending route is invoked, it will throw ReferenceError: parseTrendingRss is not defined at runtime.

The parseTrendingHtml fallback (line 279) is properly defined, but the primary RSS parsing path is completely broken. Implement parseTrendingRss to parse RSS XML data using parseStringPromise before merging.

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

In `@server/package.json` at line 20, The route calls an undefined
parseTrendingRss; implement parseTrendingRss to parse RSS XML via the
already-imported parseStringPromise, extract items (title, link, pubDate/iso
date, description/summary, author if present), normalize each item into the same
shape parseTrendingHtml returns, filter items by the provided since timestamp,
and return the merged/normalized list so the existing call to
parseTrendingRss(result.data, since) succeeds; ensure parseTrendingRss handles
parse errors (fallback to empty array) and matches the field names/structure
expected by the rest of the code that currently uses parseTrendingHtml.

- 修复版本迁移逻辑:检查并添加缺失的 trending 频道
- 用户即使有历史数据也会正确显示新增的 Trending 频道
- 确保频道名称和图标正确更新
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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/store/useAppStore.ts (1)

904-904: ⚠️ Potential issue | 🟠 Major

Bump persist version and complete the subscription state migration.

The migration at lines 1011–1016 only adds the trending channel but omits the companion data structures. More critically, PersistedAppState does not include the subscription fields (subscriptionChannels, selectedSubscriptionChannel, subscriptionRepos, subscriptionDevs, subscriptionLastRefresh) that partialize persists at lines 962–966, creating a type mismatch. Because the version remains 5, users with existing version-5 state will not trigger this migration.

Bump the persist version to 6, add the subscription fields to PersistedAppState, and initialize both subscriptionRepos['trending'] and subscriptionLastRefresh['trending'] during migration:

Suggested changes
 type PersistedAppState = Partial<
   Pick<
     AppState,
     | 'releaseViewMode'
     | 'releaseSelectedFilters'
     | 'releaseSearchQuery'
+    | 'subscriptionChannels'
+    | 'selectedSubscriptionChannel'
+    | 'subscriptionRepos'
+    | 'subscriptionDevs'
+    | 'subscriptionLastRefresh'
   >
 > & {
-      version: 5,
+      version: 6,
     if (!hasTrending) {
       console.log('Migrating: adding trending channel');
       state.subscriptionChannels = [...state.subscriptionChannels, defaultSubscriptionChannels[3]];
+      state.subscriptionRepos = {
+        ...state.subscriptionRepos,
+        trending: [],
+      };
+      state.subscriptionLastRefresh = {
+        ...state.subscriptionLastRefresh,
+        trending: null,
+      };
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/store/useAppStore.ts` at line 904, Bump the persist version to 6 and
complete the subscription-state migration: update the persisted version value
(currently version: 5 → 6), add the subscription fields to the PersistedAppState
type (subscriptionChannels, selectedSubscriptionChannel, subscriptionRepos,
subscriptionDevs, subscriptionLastRefresh) to match what partialize persists,
and update the migration block that adds the 'trending' channel (the migration
around lines 1011–1016) to also push 'trending' into subscriptionChannels (and
set selectedSubscriptionChannel if needed), initialize
subscriptionRepos['trending'] = [] and subscriptionLastRefresh['trending'] = 0
(or appropriate initial value), and ensure subscriptionDevs/subscriptionRepos
objects exist before assigning; this will resolve the type mismatch and ensure
users with version 5 state hit the migration.
🤖 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/store/useAppStore.ts`:
- Line 904: Bump the persist version to 6 and complete the subscription-state
migration: update the persisted version value (currently version: 5 → 6), add
the subscription fields to the PersistedAppState type (subscriptionChannels,
selectedSubscriptionChannel, subscriptionRepos, subscriptionDevs,
subscriptionLastRefresh) to match what partialize persists, and update the
migration block that adds the 'trending' channel (the migration around lines
1011–1016) to also push 'trending' into subscriptionChannels (and set
selectedSubscriptionChannel if needed), initialize subscriptionRepos['trending']
= [] and subscriptionLastRefresh['trending'] = 0 (or appropriate initial value),
and ensure subscriptionDevs/subscriptionRepos objects exist before assigning;
this will resolve the type mismatch and ensure users with version 5 state hit
the migration.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: cfab1a8c-58d4-495e-acac-602a9df398be

📥 Commits

Reviewing files that changed from the base of the PR and between 343ee71 and 8a51106.

📒 Files selected for processing (1)
  • src/store/useAppStore.ts

- 无论版本号如何,都会检查并添加缺失的频道
- 解决用户已有历史数据但不显示 Trending 的问题
- 确保所有默认频道都存在
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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/store/useAppStore.ts (1)

192-1149: ⚠️ Potential issue | 🔴 Critical

Critical: file is broken — stray subscriptionChannels, tokens injected throughout will not parse and, where parseable, will throw ReferenceError.

A mechanical edit appears to have injected a bare subscriptionChannels, line in ~25 locations across the file. Concretely:

  1. Duplicate object key in normalizePersistedState (Line 205 duplicates Line 210). Biome flags this as noDuplicateObjectKeys. Only Line 210 should remain.
  2. Parse errors after return { ... }; statements — a stray subscriptionChannels, appears as a dangling token after the function has already returned. This affects at least lines 533, 543, 556, 564, 575, 585, 590, 605, 628, 683, 709, 713, 723, 748, 752, 770, 795, 808, 857, 908, 1041, 1065, 1070. Biome reports parse errors at 557, 586, 711, 750, 809, 1066 (others only don't surface because the parser already bailed).
  3. ReferenceErrors where the token sits inside a returned object literal (shorthand property referencing an out-of-scope identifier): lines 458, 476, 615, 696, 735, 782, 816, 829, 929, 1099, 1149. None of these scopes declares a subscriptionChannels binding; the only one that legitimately needs it is normalizePersistedState (Line 210), and merge already spreads normalized which carries it through.

Net effect: the store module does not compile; the entire app is broken by this PR. This must be fixed before merge.

🔧 Representative fixes (apply the same removal pattern at every location listed above)

Remove the duplicate key in normalizePersistedState:

   return {
-    subscriptionChannels,
     ...currentState,
     ...safePersisted,
     repositories,
     releases,
     subscriptionChannels,

Remove the dangling tokens after returns — e.g. in setSearchFilters:

         return { searchFilters: newFilters };
-    subscriptionChannels,
       }),

Same pattern at lines 543, 556, 564, 575, 585, 590, 605, 628, 683, 709, 713, 723, 748, 752, 770, 795, 808, 857, 908, 1041, 1065, 1070.

Remove the shorthand property from returned objects where it's out of scope — e.g. in updateRepository:

         return {
-    subscriptionChannels,
           repositories: updatedRepositories,
           searchResults: state.searchResults.map(r => r.id === repo.id ? repo : r)
         };

Same pattern at lines 476, 615, 696, 735, 782, 816, 829, 929, 1149.

And in merge (Line 1099) — normalized already carries subscriptionChannels:

         return {
-    subscriptionChannels,
           ...currentState,
           ...normalized,
         };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/store/useAppStore.ts` around lines 192 - 1149, The file contains many
stray bare "subscriptionChannels," tokens injected across the module; remove
every stray occurrence so the file parses and no undefined identifier is
referenced — specifically: in normalizePersistedState remove the duplicate
subscriptionChannels key, in the merge handler remove the redundant
subscriptionChannels shorthand (normalized already includes it), and delete the
dangling "subscriptionChannels," tokens that appear immediately after return
statements in action setters (e.g., setSearchFilters, addReleases,
toggleReleaseSubscription, batchUnsubscribeReleases,
markReleaseAsRead/markAllReleasesAsRead, removeReleasesByRepoId,
toggleReleaseExpandedRepository, reorderCategories, and others), and remove the
out-of-scope shorthand from returned objects in action methods such as
updateRepository, deleteRepository, updateSubscriptionRepo,
updateCustomCategory, updateDefaultCategory, resetDefaultCategory,
deleteCustomCategory, etc.; search for the literal "subscriptionChannels," and
remove each instance so only the legitimate subscriptionChannels state in
useAppStore (initial state and persisted normalized object) remains.
🧹 Nitpick comments (1)
src/store/useAppStore.ts (1)

1055-1060: Avoid magic-index reference to defaultSubscriptionChannels[3] in migration.

Hard-coding index 3 couples migration correctness to the ordering of defaultSubscriptionChannels. If someone reorders or inserts a channel ahead of trending, the wrong channel gets appended silently during migration for historical users. Look up by id instead.

♻️ Proposed fix
-    if (!hasTrending) {
-      console.log('Migrating: adding trending channel');
-      state.subscriptionChannels = [...state.subscriptionChannels, defaultSubscriptionChannels[3]];
-    }
+    if (!hasTrending) {
+      const trendingDefault = defaultSubscriptionChannels.find(ch => ch.id === 'trending');
+      if (trendingDefault) {
+        console.log('Migrating: adding trending channel');
+        state.subscriptionChannels = [...state.subscriptionChannels, trendingDefault];
+      }
+    }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/store/useAppStore.ts` around lines 1055 - 1060, The migration currently
appends defaultSubscriptionChannels[3] which is a brittle magic-index; instead,
find the trending channel by id and append that. Change the logic around
hasTrending/state.subscriptionChannels to locate the channel from
defaultSubscriptionChannels using something like find(ch => ch.id ===
'trending') (referencing defaultSubscriptionChannels and
state.subscriptionChannels) and only push/concat the found channel; handle the
case where no matching default channel exists by skipping the append or logging
an error.
🤖 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/store/useAppStore.ts`:
- Around line 192-1149: The file contains many stray bare
"subscriptionChannels," tokens injected across the module; remove every stray
occurrence so the file parses and no undefined identifier is referenced —
specifically: in normalizePersistedState remove the duplicate
subscriptionChannels key, in the merge handler remove the redundant
subscriptionChannels shorthand (normalized already includes it), and delete the
dangling "subscriptionChannels," tokens that appear immediately after return
statements in action setters (e.g., setSearchFilters, addReleases,
toggleReleaseSubscription, batchUnsubscribeReleases,
markReleaseAsRead/markAllReleasesAsRead, removeReleasesByRepoId,
toggleReleaseExpandedRepository, reorderCategories, and others), and remove the
out-of-scope shorthand from returned objects in action methods such as
updateRepository, deleteRepository, updateSubscriptionRepo,
updateCustomCategory, updateDefaultCategory, resetDefaultCategory,
deleteCustomCategory, etc.; search for the literal "subscriptionChannels," and
remove each instance so only the legitimate subscriptionChannels state in
useAppStore (initial state and persisted normalized object) remains.

---

Nitpick comments:
In `@src/store/useAppStore.ts`:
- Around line 1055-1060: The migration currently appends
defaultSubscriptionChannels[3] which is a brittle magic-index; instead, find the
trending channel by id and append that. Change the logic around
hasTrending/state.subscriptionChannels to locate the channel from
defaultSubscriptionChannels using something like find(ch => ch.id ===
'trending') (referencing defaultSubscriptionChannels and
state.subscriptionChannels) and only push/concat the found channel; handle the
case where no matching default channel exists by skipping the append or logging
an error.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: c8afc6d7-6467-4b2e-8130-0be72c3ca526

📥 Commits

Reviewing files that changed from the base of the PR and between 8a51106 and 510363a.

📒 Files selected for processing (1)
  • src/store/useAppStore.ts

@AmintaCCCP AmintaCCCP closed this Apr 20, 2026
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.

1 participant