bugfix[#10]:修复ai分析过程中,切换Header导致分析进度变为0的问题#14
Conversation
WalkthroughCentralizes AI analysis progress tracking by moving progress state from RepositoryList local state to the global AppStore. Introduces AnalysisProgress type, adds analysisProgress state and setAnalysisProgress action to the store, and updates RepositoryList to consume and update this shared state. Persistence excludes analysisProgress. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant RepoList as RepositoryList
participant Store as AppStore
participant AI as AI Analyzer
User->>RepoList: Trigger Analyze
RepoList->>Store: setAnalysisProgress({current:0,total:N})
loop For each repository
RepoList->>AI: Analyze repo (async)
AI-->>RepoList: Result/err
RepoList->>Store: setAnalysisProgress({current:+1,total:N})
end
RepoList->>Store: setAnalysisProgress({current:0,total:0})
Note over Store,RepoList: RepositoryList renders progress from Store
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/RepositoryList.tsx (2)
264-265: Fix: React hook useAppStore called conditionally (violates Rules of Hooks)Calling useAppStore inside a conditional render path will trip the hooks linter and can lead to runtime issues. Lift the access to searchFilters to the top-level store destructure (or use a selector) and remove the conditional hook call.
- const selectedCategoryObj = allCategories.find(cat => cat.id === selectedCategory); - const categoryName = selectedCategoryObj?.name || selectedCategory; - const { searchFilters } = useAppStore(); + const selectedCategoryObj = allCategories.find(cat => cat.id === selectedCategory); + const categoryName = selectedCategoryObj?.name || selectedCategory;And include searchFilters in the top-level destructure:
const { githubToken, aiConfigs, activeAIConfig, isLoading, setLoading, updateRepository, language, customCategories, analysisProgress, - setAnalysisProgress + setAnalysisProgress, + searchFilters } = useAppStore();
146-148: Pause/resume is ineffective due to stale state captured in async loopisPaused is a React state value captured by analyzeRepository’s closure when handleAIAnalyze starts. Subsequent updates to isPaused won’t be observed inside the while loop, so pausing won’t actually pause running tasks.
Use a ref for the “live” paused state and read it inside the loop. Also keep the UI state in sync.
- import React, { useState, useRef } from 'react'; + import React, { useState, useRef, useEffect } from 'react'; @@ - const shouldStopRef = useRef(false); - const isAnalyzingRef = useRef(false); + const shouldStopRef = useRef(false); + const isAnalyzingRef = useRef(false); + const isPausedRef = useRef(false); + + useEffect(() => { + isPausedRef.current = isPaused; + }, [isPaused]); @@ - while (isPaused && !shouldStopRef.current) { + while (isPausedRef.current && !shouldStopRef.current) { await new Promise(resolve => setTimeout(resolve, 1000)); } @@ - const handlePauseResume = () => { + const handlePauseResume = () => { if (!isAnalyzingRef.current) return; - setIsPaused(!isPaused); + setIsPaused(prev => { + const next = !prev; + isPausedRef.current = next; + return next; + }); console.log(isPaused ? 'Analysis resumed' : 'Analysis paused'); };Additionally, ensure isPausedRef is reset when starting/finishing an analysis:
- setIsPaused(false); + setIsPaused(false); + isPausedRef.current = false; @@ - setIsPaused(false); + setIsPaused(false); + isPausedRef.current = false;Also applies to: 241-245
🧹 Nitpick comments (4)
src/types/index.ts (1)
162-162: Minor consistency nit: add a trailing semicolonAppState properties all end with semicolons; analysisProgress lacks one. Not functionally wrong, but it’s a small consistency improvement.
- analysisProgress: AnalysisProgress + analysisProgress: AnalysisProgress;src/store/useAppStore.ts (2)
199-209: Reset analysis state on logout to avoid stale UI after sign-outIf a user logs out while analysis is running (or has recently run), isLoading and analysisProgress are left as-is. This can surface stale progress UI on the sign-in screen. Reset both on logout.
logout: () => set({ user: null, githubToken: null, isAuthenticated: false, repositories: [], releases: [], releaseSubscriptions: new Set(), readReleases: new Set(), searchResults: [], lastSync: null, + isLoading: false, + analysisProgress: { current: 0, total: 0 }, }),To verify in-app: start an analysis, trigger logout, confirm that the header/button text and progress bar are cleared.
323-324: Optional: guard against progress regress due to out-of-order updatesGiven concurrent updates, a late-arriving setAnalysisProgress with a smaller current could briefly regress the UI. Not likely in single-threaded JS, but easy to guard.
- setAnalysisProgress: (newProgress) => set({ analysisProgress: newProgress }) + setAnalysisProgress: (newProgress) => + set((state) => { + const total = Math.max(state.analysisProgress.total, newProgress.total); + const current = Math.max(state.analysisProgress.current, newProgress.current); + return { analysisProgress: { current, total } }; + })src/components/RepositoryList.tsx (1)
20-30: Prefer a single store subscription and avoid getState for reactive propsYou already subscribe to the store; pass searchFilters.query from that subscription rather than useAppStore.getState(), which is non-reactive by itself.
const { githubToken, aiConfigs, activeAIConfig, isLoading, setLoading, updateRepository, language, customCategories, analysisProgress, - setAnalysisProgress + setAnalysisProgress, + searchFilters } = useAppStore(); @@ - searchQuery={useAppStore.getState().searchFilters.query} + searchQuery={searchFilters.query}Also applies to: 472-473
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
src/components/RepositoryList.tsx(1 hunks)src/store/useAppStore.ts(4 hunks)src/types/index.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/store/useAppStore.ts (1)
src/types/index.ts (1)
AnalysisProgress(173-176)
🔇 Additional comments (3)
src/types/index.ts (1)
161-163: Types for global analysis progress: solid, matches store usageAdding AnalysisProgress and wiring it into AppState is the right direction for preventing progress reset on header navigation. The shape is minimal and sufficient for current UI needs.
Also applies to: 173-176
src/store/useAppStore.ts (1)
188-189: Global progress state and setter look goodDefaulting analysisProgress to { current: 0, total: 0 } and updating it via setAnalysisProgress is straightforward and aligns with the PR objective. Leaving it out of persistence is also appropriate since it’s ephemeral.
Also applies to: 323-324
src/components/RepositoryList.tsx (1)
120-127: Good: progress lifecycle updates moved to the storeReset at start, incremental updates per repo, and final reset are now centralized via setAnalysisProgress. This addresses the original bug (progress reset on header changes) by decoupling the progress state from component lifecycle.
Also applies to: 173-176, 188-191, 231-238
…tures) Upstream merge: - Fix releases SQL placeholder (AmintaCCCP#140) - Fix settings serialization (AmintaCCCP#140) - Gemini thinking model support (AmintaCCCP#140) - Persist GitHub star sync immediately (AmintaCCCP#142) - Disable thinking chain for Xiaomi MiMo models (AmintaCCCP#143) - Fix sync ERR_CONNECTION_RESET (AmintaCCCP#144) - Upgrade GitHub Actions to Node 24 (AmintaCCCP#145) New optimizations: - AmintaCCCP#1 Security: clear local token when backend stores it - AmintaCCCP#2 Security: API_SECRET auto-generated on first start - AmintaCCCP#3 Security: key files use 0o600 permissions - AmintaCCCP#6 Performance: GraphQL batch proxy endpoint - AmintaCCCP#7 Architecture: last-write-wins sync timestamps - AmintaCCCP#8 Architecture: DB migration v2 (sync_updated_at) - AmintaCCCP#9 Architecture: remove Electron build (web-only) - AmintaCCCP#10 Feature: offline keyword search fallback - AmintaCCCP#11 Feature: export stars as Markdown/CSV - AmintaCCCP#12 Feature: rule-based auto-categorization - AmintaCCCP#13 Feature: unstar detection notifications - AmintaCCCP#14 Engineering: unit tests for crypto and search - AmintaCCCP#15 Engineering: CI workflow with tests - AmintaCCCP#16 Engineering: README updated with new features
bug:#10
将analysisProgress、setAnalysisProgress 改为全局store,通过app store更新
Summary by CodeRabbit
New Features
Refactor