Upgrade to VFBQuery 0.5.1 #198
Conversation
There was a problem hiding this comment.
Pull request overview
This PR upgrades the VFBQuery library from version 0.4.0 to 0.5.1 and implements corresponding frontend changes to handle the new data structures and improve query organization.
Key changes:
- Upgraded vfbquery dependency to version 0.5.1
- Enhanced query organization by separating "Neurons with" queries from other queries
- Improved query state management with better tracking of open queries
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
applications/virtual-fly-brain/backend/requirements.txt |
Updated vfbquery from 0.4.0 to 0.5.1 |
applications/virtual-fly-brain/Dockerfile |
Removed debug echo statement |
applications/virtual-fly-brain/frontend/src/reducers/QueriesReducer.js |
Removed unnecessary conditional check when updating query state |
applications/virtual-fly-brain/frontend/src/components/queryBuilder/Card.jsx |
Added null check for thumbnail parameter |
applications/virtual-fly-brain/frontend/src/components/TermInfo.jsx |
Major refactoring: added query state tracking, improved query organization with separate sections, added metadata rendering support, enhanced toggle functionality |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {termInfoData?.metadata?.Queries?.filter(q => | ||
| q.label?.startsWith("Neurons with") && | ||
| q.output_format === "table" && | ||
| q?.preview_results?.rows?.length > 0 | ||
| ).length > 0 && ( |
There was a problem hiding this comment.
The filter condition is duplicated in multiple places (lines 1071-1074, 1084-1087, 1098). Consider extracting this filter logic into a named function like isNeuronsWithQuery(q) to reduce duplication and improve maintainability.
| // Move queries with count === 0 to the bottom | ||
| if ((a.count || 0) === 0 && (b.count || 0) !== 0) return 1; | ||
| if ((a.count || 0) !== 0 && (b.count || 0) === 0) return -1; | ||
| return 0; |
There was a problem hiding this comment.
The sorting logic for moving zero-count queries to the bottom is duplicated (lines 1099-1104 and 1339-1344). Consider extracting this into a reusable comparator function like sortByCountDescending to reduce duplication.
| <Typography sx={{ pr: 0.5 }}> | ||
| {termInfoData?.metadata?.Queries?.filter(q => | ||
| !q.label?.startsWith("Neurons with") | ||
| ).reduce((n, { count }) => n + (count || 0), 0)} |
There was a problem hiding this comment.
The reduce function safely handles missing count values with (count || 0) on line 1328, but the same operation on line 1088 does not. This inconsistency could lead to NaN if a query has an undefined count. Update line 1088 to use the same safe pattern: .reduce((n, { count }) => n + (count || 0), 0).
| pl={0.5} | ||
| > | ||
| <Typography sx={{ pr: 0.5 }}> | ||
| {query?.preview_results?.rows?.length} |
There was a problem hiding this comment.
Line 1482 displays query?.preview_results?.rows?.length but line 1122 in the similar context uses query?.count. This inconsistency means the count display differs between sections. Use query?.count for consistency, as it appears to be the intended data source based on the pattern established in the 'Neurons with' section.
| {query?.preview_results?.rows?.length} | |
| {query?.count} |
No description provided.