fix: 完善 Release 同步机制 (#119)#129
Conversation
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
主要改动: - 移除 Header 同步按钮的 Release 获取逻辑,仅同步仓库列表 - 重构 getMultipleRepositoryReleases 支持增量同步和 pre-release 过滤 - 新增 fetchAllReleasesForRepo 分页拉取最多30条 - 添加速率限制检测,剩余配额不足时自动等待重置 - 支持 includePreRelease 开关,默认开启,状态持久化 - 老用户迁移:已有 release 数据的仓库标记为已同步,走增量 - 刷新失败仓库汇总提示,不再静默跳过 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
📝 WalkthroughWalkthroughHeader sync no longer fetches or updates release data. ReleaseTimeline now performs bulk, concurrent release fetching with rate-limit handling, prerelease filtering, deduplication, and per-repo failure tracking. Store adds persisted Changes
Sequence DiagramsequenceDiagram
participant UI as ReleaseTimeline
participant Store as AppStore
participant API as GitHubApiService
participant GH as GitHub
UI->>Store: read repositories, includePreRelease
UI->>API: getMultipleRepositoryReleases(repos, { includePreRelease })
loop concurrent workers (3)
alt repo.has_fetched_releases = false
API->>GH: GET /repos/{owner}/{repo}/releases (paginated full)
else
API->>GH: GET /repos/{owner}/{repo}/releases (since last_release_fetch_time)
end
alt rate limit low
API->>API: wait until X-RateLimit-Reset
end
GH-->>API: releases (with prerelease flag)
API->>API: filter out prereleases if includePreRelease=false
API-->>API: collect releases or record failedRepo
end
API-->>UI: { releases, failedRepos }
UI->>Store: deduplicate & add new releases
UI->>Store: update repo.has_fetched_releases, last_release_fetch_time (skip failures)
UI->>UI: show toast (include failure count / use 'info' if no new releases)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/Header.tsx (1)
79-95:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve release sync metadata when rebuilding repositories.
Now that Lines 102-103 explicitly move release syncing out of the header, this merge path also needs to carry forward the existing release-sync fields. As written, a normal starred-repos sync drops
has_fetched_releasesandlast_release_fetch_time, so the next refresh falls back to a full sync instead of the intended incremental path.Suggested fix
if (existing) { return { ...newRepo, + has_fetched_releases: existing.has_fetched_releases, + last_release_fetch_time: existing.last_release_fetch_time, ai_summary: existing.ai_summary, ai_tags: existing.ai_tags, ai_platforms: existing.ai_platforms, analyzed_at: existing.analyzed_at, analysis_failed: existing.analysis_failed,Also applies to: 102-103
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Header.tsx` around lines 79 - 95, When merging newRepositories into existing data (see existingRepoMap, mergedRepositories, repositories, newRepositories), preserve the release-sync metadata by copying has_fetched_releases and last_release_fetch_time from the existing repo into the merged object; update the merge return block (the object built when existing is truthy) to include these two fields so a starred-repos sync does not drop them (also apply the same addition in the other merge path that mirrors this logic).src/services/githubApi.ts (1)
714-728:⚠️ Potential issue | 🔴 CriticalFix the
stargazers_countproperty typo.Line 727 reads
data.stargizers_count, but the response shape declared above on line 716 only hasstargazers_count(missing 'ar'). This TypeScript type mismatch will cause the enriched repo star count to remain unset and fail type-checking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/githubApi.ts` around lines 714 - 728, The assignment to r.stargazers_count uses a misspelled property (data.stargizers_count) — update the assignment in the enrichment block that calls this.makeRequest(`/repos/${owner}/${repo}`) so it reads data.stargazers_count ?? r.stargazers_count (matching the declared response shape), ensuring the property name matches the interface and TypeScript checks pass.src/store/useAppStore.ts (1)
210-259:⚠️ Potential issue | 🔴 CriticalPersist
includePreReleasein the hydrated state shape.Line 297 reads
safePersisted.includePreRelease, butPersistedAppStateand thepartializefunction both omit this field. This causes a type-safety violation in strict builds and prevents the toggle from persisting to localStorage, causing it to reset on every reload.Suggested fix
Add
includePreReleaseto thePersistedAppStatetype definition:type PersistedAppState = Partial< Pick< AppState, | 'releaseViewMode' | 'releaseSelectedFilters' | 'releaseSearchQuery' + | 'includePreRelease' | 'discoveryChannels'And to the
partializefunction:releaseViewMode: state.releaseViewMode, releaseSelectedFilters: state.releaseSelectedFilters, releaseSearchQuery: state.releaseSearchQuery, releaseExpandedRepositories: Array.from(state.releaseExpandedRepositories), + includePreRelease: state.includePreRelease,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/store/useAppStore.ts` around lines 210 - 259, PersistedAppState is missing the includePreRelease field which causes safePersisted.includePreRelease to be invalid and the toggle not to persist; add includePreRelease: boolean to the PersistedAppState type and include includePreRelease in the partialize function's returned object (ensure the returned key reads from safePersisted.includePreRelease) so the hydrated state contains and persists this boolean flag.
🤖 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/ReleaseTimeline.tsx`:
- Around line 320-327: The code currently updates every repo in subscribedRepos
via updateRepository(...) which advances
has_fetched_releases/last_release_fetch_time even for failed repos; change this
to only update repositories that succeeded by filtering subscribedRepos against
failedRepos (e.g., by repo id or full_name) and call updateRepository only for
those not in failedRepos; apply the same filtering fix to the other update block
referenced (the later block around where similar metadata is set).
- Around line 39-40: The UI currently toggles includePreRelease only for the
fetch but still renders from the full releases array; update ReleaseTimeline to
derive a filtered list (e.g., filteredReleases = includePreRelease ? releases :
releases.filter(r => !r.prerelease)) and use filteredReleases wherever the
component maps/renders releases (replace usages of releases in the render and in
the block referenced by the render logic around the mapping at the bottom of the
component); ensure the same filter logic is used for any other render paths that
currently use releases so toggling includePreRelease hides prerelease items
immediately.
- Around line 534-545: The prerelease toggle uses a non-semantic clickable div;
replace that div with a focusable <button> that implements the switch pattern:
keep using includePreRelease and setIncludePreRelease but render a button with
role="switch", aria-checked={includePreRelease}, tabIndex=0 (inherent on
button), and move the onClick handler onto the button; also handle keyboard
activation (Space/Enter) via the button's onKeyDown or rely on default button
behavior so keyboard users can toggle it, and preserve the existing className
logic and inner span for the knob/visual state; apply the same change to the
other toggle block referenced (the similar component using its corresponding
include state and setter).
In `@src/store/useAppStore.ts`:
- Around line 283-291: The migration currently sets last_release_fetch_time to
new Date().toISOString(), which can hide real releases; instead, in the
migratedRepositories mapping (the block that uses repositories, releases,
has_fetched_releases, last_release_fetch_time), compute the latest persisted
release timestamp for that repository by filtering releases where
r.repository?.id === repo.id and taking the max r.published_at (ISO string),
then set last_release_fetch_time to repo.last_release_fetch_time ||
latestPersistedReleaseTimestamp (and only fall back to existing value if no
persisted release exists); update the returned repo object accordingly so we
backfill from persisted release data rather than hydration time.
---
Outside diff comments:
In `@src/components/Header.tsx`:
- Around line 79-95: When merging newRepositories into existing data (see
existingRepoMap, mergedRepositories, repositories, newRepositories), preserve
the release-sync metadata by copying has_fetched_releases and
last_release_fetch_time from the existing repo into the merged object; update
the merge return block (the object built when existing is truthy) to include
these two fields so a starred-repos sync does not drop them (also apply the same
addition in the other merge path that mirrors this logic).
In `@src/services/githubApi.ts`:
- Around line 714-728: The assignment to r.stargazers_count uses a misspelled
property (data.stargizers_count) — update the assignment in the enrichment block
that calls this.makeRequest(`/repos/${owner}/${repo}`) so it reads
data.stargazers_count ?? r.stargazers_count (matching the declared response
shape), ensuring the property name matches the interface and TypeScript checks
pass.
In `@src/store/useAppStore.ts`:
- Around line 210-259: PersistedAppState is missing the includePreRelease field
which causes safePersisted.includePreRelease to be invalid and the toggle not to
persist; add includePreRelease: boolean to the PersistedAppState type and
include includePreRelease in the partialize function's returned object (ensure
the returned key reads from safePersisted.includePreRelease) so the hydrated
state contains and persists this boolean flag.
🪄 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: 71657d84-178a-4f54-8f8c-ee427e84af36
📒 Files selected for processing (5)
src/components/Header.tsxsrc/components/ReleaseTimeline.tsxsrc/services/githubApi.tssrc/store/useAppStore.tssrc/types/index.ts
| // Update repository sync metadata | ||
| const now = new Date().toISOString(); | ||
| for (const repo of subscribedRepos) { | ||
| const [owner, name] = repo.full_name.split('/'); | ||
|
|
||
| const hasExistingReleases = releases.some(r => r.repository.id === repo.id); | ||
|
|
||
| let repoReleases: Release[]; | ||
| if (!hasExistingReleases) { | ||
| repoReleases = await githubApi.getRepositoryReleases(owner, name, 1, 10); | ||
| } else { | ||
| repoReleases = await githubApi.getIncrementalRepositoryReleases(owner, name, sinceTimestamp, 10); | ||
| } | ||
|
|
||
| repoReleases.forEach(release => { | ||
| release.repository.id = repo.id; | ||
| updateRepository({ | ||
| ...repo, | ||
| has_fetched_releases: true, | ||
| last_release_fetch_time: now, | ||
| }); |
There was a problem hiding this comment.
Do not advance sync markers for failed repositories.
failedRepos is surfaced in the toast, but Lines 320-327 still stamp has_fetched_releases and last_release_fetch_time onto every subscribed repo. Any repo that failed here will be treated as incrementally synced on the next refresh, which can hide missed releases.
Suggested fix
// Update repository sync metadata
const now = new Date().toISOString();
+ const failedRepoIds = new Set(failedRepos.map(repo => repo.repoId));
for (const repo of subscribedRepos) {
+ if (failedRepoIds.has(repo.id)) {
+ continue;
+ }
updateRepository({
...repo,
has_fetched_releases: true,
last_release_fetch_time: now,
});Also applies to: 341-353
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ReleaseTimeline.tsx` around lines 320 - 327, The code
currently updates every repo in subscribedRepos via updateRepository(...) which
advances has_fetched_releases/last_release_fetch_time even for failed repos;
change this to only update repositories that succeeded by filtering
subscribedRepos against failedRepos (e.g., by repo id or full_name) and call
updateRepository only for those not in failedRepos; apply the same filtering fix
to the other update block referenced (the later block around where similar
metadata is set).
1. 修复 githubApi.ts 中 stargazers_count 拼写错误 (stargizers_count) 2. Header.tsx 合并仓库时保留 has_fetched_releases 和 last_release_fetch_time 3. useAppStore.ts 添加 includePreRelease 到 PersistedAppState 和 partialize 4. useAppStore.ts 迁移时从已有 release 数据反推 last_release_fetch_time 5. ReleaseTimeline.tsx subscribedReleases 根据 includePreRelease 过滤 prerelease 6. ReleaseTimeline.tsx 只为成功的仓库更新 sync metadata (跳过失败仓库) 7. ReleaseTimeline.tsx 改为语义化 button role="switch" 无障碍支持 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/services/githubApi.ts (1)
1-16:⚠️ Potential issue | 🔴 CriticalAdd missing type imports to fix TypeScript compilation failure.
TrendingTimeRangeandDiscoveryRepoare used in this file (lines 632–634 and 653) but are missing from the import statement, causing a compilation error.Suggested fix
import { Repository, Release, GitHubUser, + DiscoveryRepo, DiscoveryPlatform, ProgrammingLanguage, SortBy, SortOrder, + TrendingTimeRange, PaginatedDiscoveryRepositories, DiscoveryChannelId, TopicCategory, SubscriptionRepo, SubscriptionDev,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/githubApi.ts` around lines 1 - 16, The file is missing two type imports causing TS compile errors: add TrendingTimeRange and DiscoveryRepo to the existing import list at the top alongside Repository, Release, etc.; update the import statement that currently lists Repository, Release, GitHubUser, DiscoveryPlatform, ProgrammingLanguage, SortBy, SortOrder, PaginatedDiscoveryRepositories, DiscoveryChannelId, TopicCategory, SubscriptionRepo, SubscriptionDev, GitHubSearchUserResponse, GitHubUserDetail to also include TrendingTimeRange and DiscoveryRepo so usages around TrendingTimeRange (lines ~632–634) and DiscoveryRepo (line ~653) resolve.
🤖 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/services/githubApi.ts`:
- Around line 281-294: The incremental-release sync currently fetches only page
1 (10 items) via getRepositoryReleases and then filters by
repo.last_release_fetch_time, which can drop newer releases beyond that first
page; change the logic to paginate through getRepositoryReleases (e.g., loop
incrementing page) accumulating results until either no more releases are
returned or the oldest release in the accumulated page is older than
repo.last_release_fetch_time, then apply the client-side filter; update
references to releases, getRepositoryReleases, and repo.last_release_fetch_time
so you fetch subsequent pages (increase page param and aggregate) instead of
only calling page=1 once.
---
Outside diff comments:
In `@src/services/githubApi.ts`:
- Around line 1-16: The file is missing two type imports causing TS compile
errors: add TrendingTimeRange and DiscoveryRepo to the existing import list at
the top alongside Repository, Release, etc.; update the import statement that
currently lists Repository, Release, GitHubUser, DiscoveryPlatform,
ProgrammingLanguage, SortBy, SortOrder, PaginatedDiscoveryRepositories,
DiscoveryChannelId, TopicCategory, SubscriptionRepo, SubscriptionDev,
GitHubSearchUserResponse, GitHubUserDetail to also include TrendingTimeRange and
DiscoveryRepo so usages around TrendingTimeRange (lines ~632–634) and
DiscoveryRepo (line ~653) resolve.
🪄 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: d6ed3517-ec1b-4ef6-9cc4-e3752bbaab49
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
src/components/Header.tsxsrc/components/ReleaseTimeline.tsxsrc/services/githubApi.tssrc/store/useAppStore.ts
✅ Files skipped from review due to trivial changes (1)
- src/components/ReleaseTimeline.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Header.tsx
之前只拉取第一页10条,现在循环分页直到: 1. 遇到 last_release_fetch_time 之前发布的 release(越过水印) 2. 或者数据已取完(batch.length < 10) 这样避免漏掉两次刷新之间的新 release。 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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/services/githubApi.ts`:
- Around line 213-251: fetchAllReleasesForRepo currently treats perPage as a
total cap and stops fetching when allReleases.length >= perPage, which truncates
results (callers pass 30 as page size). Change the loop to paginate until no
more results: set a pageSize = Math.min(perPage, 100) and call makeRequest with
per_page=pageSize, loop while true, break only when batch.length === 0, push
mapped releases into allReleases and break when batch.length < pageSize (end of
pages), then return allReleases (or slice only if there is an explicit total cap
parameter); update the variables page, batch, and perPage usage in
fetchAllReleasesForRepo accordingly and apply the same fix to the other similar
pagination block that uses batch.length < perPage.
- Around line 57-65: The rate-limit sleep in makeRequest blocks without honoring
the provided AbortSignal (signal), so when rateLimitRemaining is low a caller
can be stuck until reset; replace the plain new Promise/setTimeout with an
abortable sleep that listens to the signal, clears the timer and rejects/throws
when aborted, and otherwise resolves after waitMs (plus padding). Modify the
rate-limit branch in makeRequest to create a timer id, attach
signal?.addEventListener('abort', ...) to cancel the timer and reject, and
ensure you remove the listener and clear the timer on both resolution and abort
so the function returns promptly when signal is aborted.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
1. fetchAllReleasesForRepo 移除 perPage 参数限制,改为循环分页直到 API 返回不足30条(取完为止),不再有30条上限 2. makeRequest 的 rate limit 等待支持 AbortSignal,用户取消时抛出错误 3. 更新调用处,移除多余的 30 参数 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/services/githubApi.ts (1)
363-406: Remove unused methodgetIncrementalRepositoryReleases.This method has no callers in the codebase. Since the incremental sync logic is now handled by
getMultipleRepositoryReleases, this standalone method is dead code and can be safely removed to reduce maintenance surface.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/githubApi.ts` around lines 363 - 406, Remove the dead method getIncrementalRepositoryReleases from the file: delete the entire async function definition (the method that begins with "async getIncrementalRepositoryReleases(owner: string, repo: string, since?: string, perPage = 10): Promise<Release[]> { ... }") since incremental sync is now handled by getMultipleRepositoryReleases; ensure no remaining references to getIncrementalRepositoryReleases remain and run tests/compile to confirm no usages exist.
🤖 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/services/githubApi.ts`:
- Around line 363-406: Remove the dead method getIncrementalRepositoryReleases
from the file: delete the entire async function definition (the method that
begins with "async getIncrementalRepositoryReleases(owner: string, repo: string,
since?: string, perPage = 10): Promise<Release[]> { ... }") since incremental
sync is now handled by getMultipleRepositoryReleases; ensure no remaining
references to getIncrementalRepositoryReleases remain and run tests/compile to
confirm no usages exist.
修复内容
问题背景
Issue #119 描述了 Release 同步的多个问题:首次同步只覆盖前20个仓库、增量更新机制有缺陷、刷新失败静默跳过等。
主要改动
Header 同步按钮重构 (
Header.tsx)增量同步机制 (
githubApi.ts)getMultipleRepositoryReleases支持增量同步fetchAllReleasesForRepo分页拉取最多30条includePreRelease选项控制是否包含预发布版速率限制处理 (
githubApi.ts)makeRequest检测X-RateLimit-*响应头Pre-release 开关 (
ReleaseTimeline.tsx)老数据迁移 (
useAppStore.ts)has_fetched_releases = trueincludePreRelease = true保持向后兼容失败汇总提示
测试建议
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Bug Fixes / Data Migration