Add new ReportBoutList in the candidate report page#909
Add new ReportBoutList in the candidate report page#909
Conversation
WalkthroughConsolidates duration formatting into a shared util, widens DetectionsTable props, introduces ReportBoutList to fetch and render bouts for a candidate’s category/time window, adds enum conversion helpers, and makes a whitespace-only change in a page. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant DT as DetectionsTable
participant RBL as ReportBoutList
participant Q as useBoutsQuery
participant GQL as GraphQL API
participant BI as BoutItem
participant T as Timer
U->>DT: Open candidate report
DT->>RBL: Render when candidate.category exists (feed,id/name,min/max,category)
RBL->>Q: useBoutsQuery({ feedId, category, minTime, maxTime })
Q->>GQL: Fetch bouts with filters
GQL-->>Q: Bouts data
Q-->>RBL: {loading|data}
RBL->>BI: Render BoutItem per bout
loop Ongoing bouts
T-->>BI: tick every 1s
BI->>BI: Recompute duration
end
U-->>RBL: View table and actions
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (11)
ui/src/utils/enum.ts (1)
6-10: Consider adding runtime validation for unmapped enum values.While the current implementation works for known enum values, it could benefit from handling unexpected values more gracefully.
Consider adding runtime validation to handle unmapped enum values:
export function detectionCategoryToAudioCategory( detectionCategory: DetectionCategory, ): AudioCategory { - return { + const mapping = { WHALE: AudioCategory.Biophony, VESSEL: AudioCategory.Anthrophony, OTHER: AudioCategory.Geophony, - }[detectionCategory]; + }; + + const result = mapping[detectionCategory as keyof typeof mapping]; + if (result === undefined) { + throw new Error(`Unknown DetectionCategory: ${detectionCategory}`); + } + return result; } export function audioCategoryToDetectionCategory( audioCategory: AudioCategory, ): DetectionCategory { - return { + const mapping = { BIOPHONY: DetectionCategory.Whale, ANTHROPHONY: DetectionCategory.Vessel, GEOPHONY: DetectionCategory.Other, - }[audioCategory]; + }; + + const result = mapping[audioCategory as keyof typeof mapping]; + if (result === undefined) { + throw new Error(`Unknown AudioCategory: ${audioCategory}`); + } + return result; }Also applies to: 16-20
ui/src/modules/reports/ReportBoutList.tsx (10)
50-52: Potentially excludes live bouts (null endTime) from results.The filter
endTime >= minTimewill typically exclude ongoing bouts whereendTimeis null. If you intend to include bouts that started beforemaxTimeand are still ongoing, add an OR branch that acceptsendTime == null.If your GraphQL filter supports boolean logic, consider something like:
- startTime <= maxTime AND (endTime >= minTime OR endTime IS NULL)
Please confirm the supported filter syntax (e.g.,
or: [...],OR: [...],endTime: { isNull: true }) and adjust accordingly.
45-45: Remove leftover debug log.Console logging feed IDs in production is noisy and can leak implementation details.
Apply this diff to remove it:
- console.log("feed id?", feed.id);
59-59: Prefer MUI Typography over raw h3 for consistency and theming.Apply this diff:
- <h3>Bouts</h3> + <Typography variant="h6" component="h3">Bouts</Typography>
20-20: Avoid importing the entire lodash bundle for a simple range.Reduces bundle size and improves tree-shaking. Replace with native Array.from.
Apply this diff to drop lodash:
-import _ from "lodash";
76-81: Use native Array.from instead of lodash.range.Removes lodash dependency for this case.
Apply this diff:
- {_.range(8).map((_, idx) => ( + {Array.from({ length: 8 }, (_, idx) => ( <TableCell key={idx}> <Skeleton animation="wave" variant="text" /> </TableCell> ))}
83-94: Consider rendering an error state.If useBoutsQuery exposes error or isError, surface a brief message instead of falling back to “No bouts” when the query fails.
Example (non-diff, adjust to your hook API):
const { data: boutData, isLoading, error } = useBoutsQuery(/* ... */); // ... {!isLoading && error && ( <TableRow> <TableCell colSpan={8} sx={{ textAlign: "center", color: "error.main" }}> Failed to load bouts </TableCell> </TableRow> )}
133-133: Redundant key on TableRow inside a keyed list item.The key on BoutItem at the call site is sufficient. Remove the inner key to avoid confusion.
Apply this diff:
- <TableRow key={bout.id}> + <TableRow>
115-115: Timer typing and cleanup guard for the interval.Using NodeJS.Timeout in the browser can cause type friction; guard clearInterval against undefined.
Apply this diff:
- let interval: NodeJS.Timeout | undefined; + let interval: ReturnType<typeof setInterval> | undefined; @@ - return () => { - clearInterval(interval); - }; + return () => { + if (interval) { + clearInterval(interval); + } + };Also applies to: 127-129
1-13: Use Next.js Link for client-side navigation and prefetch.Switching to next/link avoids full page reloads and improves UX.
Apply these diffs:
} from "@mui/material"; +import NextLink from "next/link";- <Link href={`/bouts/${bout.id}`} title={`/bouts/${bout.id}`}> + <Link component={NextLink} href={`/bouts/${bout.id}`} title={`/bouts/${bout.id}`}> <Button size="small">View</Button> </Link>Also applies to: 171-174
101-109: Tighten BoutItem props typing to match list query results.Using BoutQuery["bout"] couples this to a different query shape. Prefer the generated Bout type or derive the item type from useBoutsQuery results for better resilience.
For example (adjust to your generated types):
- If exported:
import type { Bout } from "@/graphql/generated";thenbout: Pick<Bout, "id" | "name" | "category" | "startTime" | "endTime" | "duration">- Or derive:
type BoutListItem = NonNullable<NonNullable<ReturnType<typeof useBoutsQuery>["data"]>["bouts"]["results"]>[number];
Would you like me to push a typed version aligned to your codegen?
📜 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 (6)
ui/src/components/Bouts/BoutItem.tsx(1 hunks)ui/src/components/DetectionsTable.tsx(3 hunks)ui/src/modules/reports/ReportBoutList.tsx(1 hunks)ui/src/pages/reports/[candidateId].tsx(1 hunks)ui/src/utils/enum.ts(1 hunks)ui/src/utils/time.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-05T23:46:05.615Z
Learnt from: paulcretu
PR: orcasound/orcasite#694
File: ui/src/components/Bouts/FeedItem.tsx:36-36
Timestamp: 2024-11-05T23:46:05.615Z
Learning: DetectionCategory is a union type, not an enum; initializing arrays of DetectionCategory with string literals is valid.
Applied to files:
ui/src/utils/enum.ts
🧬 Code Graph Analysis (1)
ui/src/modules/reports/ReportBoutList.tsx (3)
ui/src/graphql/generated/index.ts (2)
BoutQuery(3308-3332)Feed(879-902)ui/src/utils/enum.ts (1)
detectionCategoryToAudioCategory(3-11)ui/src/utils/time.ts (1)
durationString(40-51)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (11)
ui/src/utils/time.ts (2)
1-1: LGTM! Clean import addition for duration formatting.The import of
intervalToDurationfrom date-fns is correctly added to support the new shared utility function.
40-51: Well-implemented shared duration utility with proper null handling.The
durationStringfunction is well-structured with:
- Robust null/undefined handling
- Proper hour calculation using Math.floor
- Consistent zero-padding for HH:MM:SS format
- Good use of date-fns for minutes/seconds extraction
This consolidation eliminates code duplication and provides a consistent duration formatting approach across components.
ui/src/components/Bouts/BoutItem.tsx (2)
16-16: Good refactoring to use shared duration utility.The import of the shared
durationStringfunction from@/utils/timesuccessfully eliminates the local duplicate implementation, promoting code reuse and maintainability.
139-139: Please verify the importeddurationStringmatches the original formattingI wasn’t able to locate a prior, in-file definition of
durationStringinBoutItem.tsx, nor compare it against the shared version inui/src/utils/time.ts. To be safe, please:
- Manually review any existing examples where
durationStringwas used locally and compare outputs against the shared util.- Add or update unit tests (if not already in place) to cover key duration cases (e.g., null/undefined, sub-hour, multi-hour spans).
- Confirm there are no subtle formatting differences introduced by
intervalToDuration.ui/src/pages/reports/[candidateId].tsx (1)
81-81: Minor formatting improvement.The added blank line improves code readability by creating better visual separation between the main content box and the conditional DetectionsTable rendering.
ui/src/components/DetectionsTable.tsx (3)
31-31: LGTM! Clean import for the new ReportBoutList component.The import statement correctly references the new ReportBoutList component from the modules directory.
57-61: Well-designed props expansion to support bout rendering.The expansion of the props interface is thoughtfully designed:
feednow includesidandnameneeded by ReportBoutList for bout queries and displaycandidatenow includesminTime,maxTime, andcategoryrequired for time-window filtering and category mappingThe type constraints using
Pickmaintain good encapsulation by only exposing the necessary properties.
189-197: Appropriate conditional rendering of bout information.The conditional rendering of
ReportBoutListwhencandidate.categoryexists is well-implemented:
- Properly guards against undefined category values
- Passes all required props (feed, time window, category)
- Maintains the existing table functionality while adding new bout visualization
The integration follows React patterns well and provides additional context for candidates with categorized detections.
ui/src/utils/enum.ts (2)
3-11: Well-structured enum conversion with clear mapping.The
detectionCategoryToAudioCategoryfunction provides a clean mapping:
- WHALE → AudioCategory.Biophony
- VESSEL → AudioCategory.Anthrophony
- OTHER → AudioCategory.Geophony
The implementation is concise and uses proper TypeScript enum references.
13-21: Symmetric conversion function with correct reverse mapping.The
audioCategoryToDetectionCategoryfunction correctly provides the inverse mapping:
- BIOPHONY → DetectionCategory.Whale
- ANTHROPHONY → DetectionCategory.Vessel
- GEOPHONY → DetectionCategory.Other
Both functions together provide bidirectional conversion between the enum types, which is useful for data transformation between different parts of the system.
ui/src/modules/reports/ReportBoutList.tsx (1)
49-49: Good use of enum mapping for query filter.Mapping DetectionCategory to AudioCategory via detectionCategoryToAudioCategory keeps the filter consistent with backend expectations.
| <TableCell> | ||
| {duration && ( | ||
| <Box | ||
| sx={{ mt: { sm: "auto" }, ml: { xs: "auto", sm: 0 } }} | ||
| title={formatDuration( | ||
| intervalToDuration({ start: 0, end: duration }), | ||
| )} | ||
| > | ||
| <Typography variant="monospace" textAlign="right"> | ||
| {durationString(duration)} | ||
| </Typography> | ||
| </Box> | ||
| )} | ||
| </TableCell> |
There was a problem hiding this comment.
Duration not shown for 0ms and invalid Typography variant.
- The truthy check hides “00:00:00” when duration is 0.
- Typography variant "monospace" is not a valid MUI variant; use a standard variant and set fontFamily.
Apply this diff:
- <TableCell>
- {duration && (
+ <TableCell>
+ {duration != null && (
<Box
sx={{ mt: { sm: "auto" }, ml: { xs: "auto", sm: 0 } }}
title={formatDuration(
intervalToDuration({ start: 0, end: duration }),
)}
>
- <Typography variant="monospace" textAlign="right">
+ <Typography variant="body2" sx={{ fontFamily: "monospace" }} textAlign="right">
{durationString(duration)}
</Typography>
</Box>
)}
</TableCell>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <TableCell> | |
| {duration && ( | |
| <Box | |
| sx={{ mt: { sm: "auto" }, ml: { xs: "auto", sm: 0 } }} | |
| title={formatDuration( | |
| intervalToDuration({ start: 0, end: duration }), | |
| )} | |
| > | |
| <Typography variant="monospace" textAlign="right"> | |
| {durationString(duration)} | |
| </Typography> | |
| </Box> | |
| )} | |
| </TableCell> | |
| <TableCell> | |
| {duration != null && ( | |
| <Box | |
| sx={{ mt: { sm: "auto" }, ml: { xs: "auto", sm: 0 } }} | |
| title={formatDuration( | |
| intervalToDuration({ start: 0, end: duration }), | |
| )} | |
| > | |
| <Typography | |
| variant="body2" | |
| sx={{ fontFamily: "monospace" }} | |
| textAlign="right" | |
| > | |
| {durationString(duration)} | |
| </Typography> | |
| </Box> | |
| )} | |
| </TableCell> |
🤖 Prompt for AI Agents
In ui/src/modules/reports/ReportBoutList.tsx around lines 156 to 169, the
current truthy check hides a 0ms duration and the Typography uses an invalid MUI
variant "monospace"; change the conditional to explicitly check for
null/undefined (e.g., duration !== null && duration !== undefined) so 0 is
rendered, and replace the Typography variant with a valid variant such as
"body2" (or "caption") while applying sx={{ fontFamily: 'monospace' }} (or
theme.typography.monospace) to get monospaced styling; keep the existing title
formatting logic.
2 participants
Summary by CodeRabbit
New Features
Enhancements