feat: restructure detections view as run-based browser#2
Conversation
Separate analysis configuration from results display following a ChatGPT/Claude conversation-history pattern: - Analysis tab now always shows configuration (file/folder, model, coordinates, confidence) — never results - Detections tab shows a sidebar of past analysis runs with a detail view for the selected run's detections - After analysis completes, auto-navigate to Detections and select the new run New components: RunList sidebar. Removed: FilterPanel, DetectionsTable, catalog store (replaced by run-scoped queries). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
If the app was closed or crashed during an analysis, the run status remains 'running' in the database forever. On startup, mark any such stale runs as 'failed' so they display correctly in the run list. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughAdds a Runs list feature: backend DB queries to fetch runs with detection counts and mark stale runs failed, a new IPC channel, renderer IPC helper and UI components to list/select runs, and refactors detections UI/state to load detections per selected run. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant DetectionsPage as DetectionsPage (UI)
participant RunList as RunList (Component)
participant IPC as Preload / IPC
participant Main as Main Process
participant DB as Database
participant AnalysisTable as AnalysisTable (UI)
User->>DetectionsPage: Open Detections page
DetectionsPage->>IPC: getRuns()
IPC->>Main: invoke 'catalog:get-runs'
Main->>DB: query runs with detection counts & location
DB-->>Main: runs data
Main-->>IPC: RunWithStats[]
IPC-->>DetectionsPage: RunWithStats[]
DetectionsPage->>RunList: render runs
User->>RunList: select run
RunList->>DetectionsPage: onselect(runId)
DetectionsPage->>IPC: getDetections(filter runId, paging, sort)
IPC->>Main: invoke 'get-detections'
Main->>DB: query detections for run
DB-->>Main: detections
Main-->>IPC: EnrichedDetection[]
IPC-->>DetectionsPage: EnrichedDetection[]
DetectionsPage->>AnalysisTable: render detections
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 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 |
Summary of ChangesHello @tphakala, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant architectural and UI overhaul, transforming how users interact with analysis results. By creating a dedicated, run-based browser within the 'Detections' tab, it provides a more organized and intuitive way to manage and review historical analysis data. The 'Analysis' tab is now streamlined for initiating new analyses, enhancing the overall clarity and efficiency of the application's workflow. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request successfully restructures the application flow by separating the analysis configuration from the results view, introducing a run-based browser in the Detections tab. This is a significant improvement to the user experience. The implementation is clean, and the fix for stale 'running' statuses is a great addition. I've provided a couple of suggestions for the new DetectionsPage to improve efficiency and restore previous state management behavior for the confidence filter. Overall, this is a well-executed refactoring.
| const limit = 200; | ||
| let speciesQuery = $state(''); | ||
| let ignoreConfidence = $state(false); | ||
| let minConfidence = $state(0.5); |
There was a problem hiding this comment.
The confidence filter's state (minConfidence) is managed locally within this component. This is a change from the previous implementation where it was part of the global appState. As a result, the selected confidence value is not persisted when navigating between tabs. To restore the global nature of this filter, I suggest using appState.minConfidence instead of this local state. This would require removing this line and updating references to minConfidence throughout the component to use appState.minConfidence (e.g., in loadRunDetections at line 50, the confidence slider at line 198, and the $effect block for confidence changes at lines 115-125).
| // Refresh runs list when navigating here (e.g. after analysis completes) | ||
| void refreshRuns(); |
There was a problem hiding this comment.
The list of analysis runs is re-fetched from the database every time the user selects a different run from the list. This is inefficient as it causes an unnecessary database query and re-render. The list of runs should only be refreshed when it's likely to have changed, for example when a new run is created.
A better approach is to only call refreshRuns() if the newly selected run is not already present in the local runs array. This will handle the case of a new analysis completing and adding a run, without the overhead of re-fetching on every click between existing runs.
// Refresh runs list only if the selected run is not already in our list.
// This handles new runs after analysis without re-fetching on every selection.
if (appState.selectedRunId && !runs.some((r) => r.id === appState.selectedRunId)) {
void refreshRuns();
}
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| try { | ||
| const result = await getDetections({ | ||
| run_id: appState.selectedRunId, | ||
| min_confidence: ignoreConfidence && speciesQuery ? undefined : minConfidence, |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| let prevSelectedRunId: number | null = null; | ||
| $effect(() => { | ||
| if (appState.selectedRunId !== prevSelectedRunId) { | ||
| prevSelectedRunId = appState.selectedRunId; | ||
| offset = 0; | ||
| speciesQuery = ''; | ||
| ignoreConfidence = false; | ||
| void loadRunDetections(); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
- Remove unused AudioPlayer component (was only used by deleted DetectionsTable) - Remove unused formatTimeRange export from format.ts - Use appState.minConfidence instead of local state for cross-tab persistence of the confidence filter - Make ignoreConfidence checkbox work independently of species search - Only refresh runs list when selected run isn't already loaded Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![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
🤖 Fix all issues with AI agents
In `@src/renderer/src/pages/DetectionsPage.svelte`:
- Around line 116-126: The effect watching appState.minConfidence currently
calls loadRunDetections() on every intermediate slider change (variables:
prevConfidence, $effect, appState.minConfidence, loadRunDetections, offset),
causing many IPC calls while dragging; fix by debouncing or committing only on
change/end: either wrap the $effect handler so it delays calling
loadRunDetections() using a debounce timer (e.g., 200–300ms) and cancels/resets
the timer on subsequent changes, or change the slider binding so it updates a
transient value during input and only assigns to appState.minConfidence (or
triggers loadRunDetections()) on the slider's change/pointerup event; ensure
offset reset and selectedRunId check remain the same.
- Around line 100-133: The component is triggering loadRunDetections() and
refreshRuns() redundantly on mount; remove the explicit calls to
loadRunDetections() and refreshRuns() from the onMount handler and rely on the
existing $effect blocks that compare prevSelectedRunId and prevConfidence to
appState.selectedRunId and appState.minConfidence to perform the initial load
and refresh; ensure onMount keeps any other necessary initialization but does
not call loadRunDetections() or refreshRuns() to avoid the triple-fetch.
🧹 Nitpick comments (2)
src/renderer/src/pages/DetectionsPage.svelte (2)
86-92: Debounce timer not cleared on unmount.If the component unmounts within the 250 ms window, the pending
setTimeoutcallback will still fire and attempt to mutate destroyed state. Consider clearing it in anonDestroy(or returning a cleanup from an$effect) to be safe.
33-64: Silent error swallowing in both fetch functions.Both
refreshRunsandloadRunDetectionshave emptycatchblocks that discard errors without any user feedback. Consider showing a toast or inline error state so users know something went wrong, especially for the detections fetch where the UI will simply appear empty.
- Initialize prevConfidence to current value so the confidence effect doesn't fire redundantly on mount - Remove loadRunDetections() from onMount since the selectedRunId effect already handles the initial load - Debounce confidence slider changes (200ms) to avoid rapid IPC calls during drag Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Issue #1 (HIGH): Ensure CPU provider always available - Combine birda providers with default CPU and deduplicate - Prevents losing CPU fallback if birda doesn't include it Issue #2 (MEDIUM): Remove duplicate GpuCapabilities type - Import GpuCapabilities from shared/types.ts in ipc.ts - Eliminates inline type duplication
1 participant
Summary
runningstate (from a crash or force-quit) are automatically marked asfailedTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Chores