Adding last 7 days of sightings + detections to MapLayout and Feed page#995
Open
Adding last 7 days of sightings + detections to MapLayout and Feed page#995
Conversation
…ined-data hook Add getDateMsAgo utility and use it in sightings query start date Fix useCombinedData memo dependency warning by memoizing detections results Refine data transform naming/defaults (toNewCategory, optional sightings radius) Add /json debug page to inspect combined transformed data
…uding hot-reload safety guards to avoid calling setView on a stale Leaflet instance
…rning for default export
Contributor
📝 WalkthroughWalkthroughThis pull request introduces maintainer_emails field to the Feed resource, refactors seeding logic with a new helper function, enhances attribute validation, and significantly expands the UI with sightings data integration, new data hooks, utility functions, and an enriched map component that displays audible radius circles, detection counts, and sighting markers with tooltips. Changes
Sequence Diagram(s)sequenceDiagram
participant MapLayout as MapLayout Component
participant useDetectionsQuery as useDetectionsQuery Hook
participant useSightings as useSightings Hook
participant useCombinedData as useCombinedData Hook
participant Map as Map Component
participant Leaflet as Leaflet Map Instance
MapLayout->>useDetectionsQuery: Fetch detections
activate useDetectionsQuery
useDetectionsQuery-->>MapLayout: Return detections data
deactivate useDetectionsQuery
MapLayout->>useSightings: Fetch sightings (7-day range)
activate useSightings
useSightings-->>MapLayout: Return sightings data
deactivate useSightings
MapLayout->>useCombinedData: Aggregate all data
activate useCombinedData
useCombinedData->>useCombinedData: transformAudioDetections()
useCombinedData->>useCombinedData: transformSightings()
useCombinedData->>useCombinedData: Combine arrays
useCombinedData-->>MapLayout: Return combined dataset
deactivate useCombinedData
MapLayout->>Map: Pass sightings, detections props
activate Map
Map->>Leaflet: Render base layers
Map->>Leaflet: Add AudibleRadiusCircles
Map->>Leaflet: Add ReportCount badges
Map->>Leaflet: Add sighting markers with Tooltips
Leaflet-->>Map: Rendered map instance
Map-->>MapLayout: Ready
deactivate Map
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ui/src/pages/seed.tsx (1)
124-124:⚠️ Potential issue | 🟡 MinorRemove debug
console.log.This debug statement will fire on every successful "Seed All" operation in production, polluting the browser console.
🐛 Proposed fix
.map(([resource, count]) => { - console.log(resource, count); return `${count} ${lowerCaseResource(resource)}${count === 1 ? "" : "s"}`; })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/seed.tsx` at line 124, Remove the debug console.log call that prints resource and count in the seeding logic (the console.log(resource, count) in ui/src/pages/seed.tsx) so it doesn't spam the browser console in production; locate the function handling the "Seed All" operation and delete that console.log line or replace it with a conditional debug logger tied to an environment flag if you need retained debug output.
🧹 Nitpick comments (12)
ui/src/types/DataTypes.ts (1)
19-24:newCategorycasing is inconsistent with consumer comparisons.
AudioDetection.newCategoryuses ALL-CAPS values ("WHALE (HUMAN)","VESSEL", etc.) whileSighting.newCategoryuses"SIGHTING", butReportCount.tsxstores the category keys in lowercase ("whale (human)","sighting", etc.). The match works today because both sides are.toLowerCase()-d at comparison time, but the type definition advertising all-caps values while consumers use lowercase creates unnecessary mental overhead. Consider aligning the literal union with the lowercase strings used in consumers, or vice-versa.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/types/DataTypes.ts` around lines 19 - 24, The type union for newCategory is inconsistent with consumers: change the literal union in DataTypes (the newCategory type used by AudioDetection.newCategory and Sighting.newCategory) to use the lowercase strings that ReportCount.tsx and other consumers store/compare (e.g., "whale (human)", "vessel", "other", "whale (ai)", "uncategorized") so the type reflects actual runtime values, or alternatively update the consumers to use the ALL-CAPS literals consistently; pick the lowercase option for minimal churn and update the union accordingly and any related usages that assume uppercase.ui/src/components/ReportCount.tsx (1)
71-75:<Button href="/reports">triggers a full-page navigation in Next.js.Using
hrefdirectly on an MUIButtonrenders an<a>tag that bypasses Next.js's SPA router, causing a full page reload. Use thecomponentprop with Next.jsLinkfor client-side routing:♻️ Proposed fix
+import Link from "next/link"; ... -<Button href={`/reports`} variant="contained"> +<Button component={Link} href="/reports" variant="contained"> View all reports </Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/ReportCount.tsx` around lines 71 - 75, The Button currently uses href which causes a full-page reload; import Next.js Link (e.g. Link from 'next/link') and change the MUI Button inside Breadcrumbs to use client-side routing by setting component={Link} and keeping href="/reports" (or alternatively wrap the Button with Next.js Link and give the Button component="a"); update the Button in the Breadcrumbs block so it uses Link for SPA navigation (refer to Breadcrumbs and Button in this diff).ui/vitest.config.ts (1)
5-7:environment: "node"is fine for the current utility-only tests — just be aware for future component tests.If React component tests are added later, each test file will need
@vitest-environment jsdom(orhappy-dom) via a doc-block comment, or the globalenvironmenthere will need changing tojsdom/happy-domwith@types/jsdominstalled.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/vitest.config.ts` around lines 5 - 7, The Vitest config currently sets test: { environment: "node" } which is fine for utility-only tests but will break React component tests; either (A) update future component test files to include a top-of-file doc-block like /** `@vitest-environment` jsdom */ (or happy-dom) so those tests run under jsdom, or (B) change the global test.environment from "node" to "jsdom" (or "happy-dom") in the vitest config and install `@types/jsdom` to provide DOM typings for TypeScript; locate the test config object (test.environment) and apply one of these two options depending on whether you want per-file overrides or a global jsdom environment.ui/src/hooks/useSightings.ts (1)
11-18: Avoid reassigning function parameters.Reassigning
startDateandendDateparameters is a minor code smell. Consider using local constants instead:Proposed fix
export function useSightings(startDate?: string, endDate?: string) { const endpoint = "https://maplify.com/waseak/php/search-all-sightings.php"; - if (startDate === undefined) - startDate = getDateMsAgo(rangeOptions.sevenDays) - .toISOString() - .split("T")[0]; // e.g. "2025-01-01" - if (endDate === undefined) endDate = apiTodayUTC; + const start = + startDate ?? + getDateMsAgo(rangeOptions.sevenDays).toISOString().split("T")[0]; + const end = endDate ?? apiTodayUTC;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/hooks/useSightings.ts` around lines 11 - 18, The function useSightings is reassigning its parameters startDate and endDate; replace those mutations by creating local constants (e.g., resolvedStartDate and resolvedEndDate) that compute fallback values using getDateMsAgo(rangeOptions.sevenDays).toISOString().split("T")[0] and apiTodayUTC respectively, and then update all subsequent uses in useSightings (including any request construction or hooks) to reference the new constants instead of mutating startDate/endDate.ui/src/utils/mapHelpers.tsx (1)
69-80: Parameter reassignment ofcount— consider a default parameter instead.Line 80 reassigns the
countparameter. A default value in the destructured props or a local const is cleaner:Proposed fix
export function ReportCount({ center, - count, + count = 0, onClick, }: { center: LatLngExpression; count?: number; onClick?: () => void; }) { const map = useMap(); - - if (!count) count = 0;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/utils/mapHelpers.tsx` around lines 69 - 80, The ReportCount component reassigns the incoming prop count (if (!count) count = 0), which is bad practice; change the destructured props to give count a default (e.g., { center, count = 0, onClick }) or create a new local constant (e.g., const safeCount = count ?? 0) and use that throughout; update references in ReportCount to use the defaulted/destructured count or the new safeCount and remove the reassignment.ui/src/pages/listen/[feed].tsx (1)
23-25: Naming:detectionsThisFeedincludes sightings too.Since
combinedDatais a union ofAudioDetection | Sighting,detectionsThisFeedmay contain sightings as well. Consider renaming toreportsThisFeedorcombinedDataThisFeedto avoid confusion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/listen/`[feed].tsx around lines 23 - 25, The variable detectionsThisFeed is misleading because useCombinedData().combined contains both AudioDetection and Sighting; rename detectionsThisFeed to reportsThisFeed (or combinedDataThisFeed) and update all usages accordingly (wherever detectionsThisFeed is referenced) so the filter remains the same: const reportsThisFeed = combinedData?.filter((d) => d.feedSlug === slug); keep the source hook useCombinedData and the filter predicate unchanged, only rename the identifier throughout the file to avoid implying the array contains only detections.ui/src/components/layouts/MapLayout.tsx (2)
64-68: IncludingcurrentFeedin the dependency array causes a redundant effect re-run.When
setCurrentFeed(feed)fires,currentFeedchanges, re-triggering this effect. The condition on line 65 prevents an infinite loop, but the extra invocation is unnecessary. A functional update avoids the dependency:Proposed fix
useEffect(() => { - if (feed && feed.slug !== currentFeed?.slug) { - setCurrentFeed(feed); - } - }, [feed, currentFeed]); + if (feed) { + setCurrentFeed((prev) => (prev?.slug !== feed.slug ? feed : prev)); + } + }, [feed]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/layouts/MapLayout.tsx` around lines 64 - 68, The useEffect in MapLayout re-runs unnecessarily because currentFeed is included in the dependency array; change the effect to depend only on feed and perform a functional state update with setCurrentFeed(prev => ...) so the setter compares prev?.slug to feed?.slug and only returns feed when different, thereby removing currentFeed from the deps and avoiding redundant re-renders; locate the useEffect, setCurrentFeed, currentFeed and feed symbols in MapLayout and apply this change.
74-76: Replace private_mapPanewith public Leaflet API for forward-compatibility.
_mapPaneis a private internal property (not part of Leaflet's public API) and could be removed or renamed in future versions, particularly in the upcoming Leaflet 2.0. Since this only affects dev hot-reload, production impact is minimal, but consider switching to the stable public API:map.getPane('mapPane')ormap.getPanes().mapPanewill safely check if the map pane is initialized.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/layouts/MapLayout.tsx` around lines 74 - 76, The check uses the private field `_mapPane` on the `mapWithPane` cast; replace that with Leaflet's public API by checking `map.getPane('mapPane')` (or `map.getPanes().mapPane`) before returning so the hot-reload guard relies on a stable public method; update the type cast on `map` (in the `MapLayout` component where `mapWithPane` is created) if needed to satisfy TypeScript, and change the early-return condition to use the public pane lookup instead of `_mapPane`.ui/src/components/Map.tsx (1)
45-52:L.iconinstances are recreated on every render.These icon objects are static and don't depend on props or state. Defining them inside the component means new allocations on each render cycle. Move them to module scope or wrap in
useMemo.Proposed fix: hoist to module scope
+const hydrophoneDefaultIcon = L.icon({ + iconUrl: hydrophoneDefaultIconImage.src, + iconSize: [30, 30], +}); +const hydrophoneActiveIcon = L.icon({ + iconUrl: hydrophoneActiveIconImage.src, + iconSize: [30, 30], +}); + export default function Map({ ... }) { const router = useRouter(); - - const hydrophoneDefaultIcon = L.icon({ - iconUrl: hydrophoneDefaultIconImage.src, - iconSize: [30, 30], - }); - const hydrophoneActiveIcon = L.icon({ - iconUrl: hydrophoneActiveIconImage.src, - iconSize: [30, 30], - });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/Map.tsx` around lines 45 - 52, The L.icon creations (hydrophoneDefaultIcon and hydrophoneActiveIcon) are being recreated on every render; move these static icon objects out of the React component into module scope (or wrap their creation in useMemo referencing no deps) so they are instantiated once. Locate the hydrophoneDefaultIcon and hydrophoneActiveIcon definitions (the L.icon(...) calls) and either hoist them above the component or wrap them with useMemo() inside the component to prevent reallocation on each render.ui/src/utils/dataTransforms.ts (2)
100-101: Duplicate ISO-string construction.
el.created.replace(" ", "T") + "Z"is computed twice — once fortimestampStringand once for theDateconstructor. Extract to a local variable.Proposed fix
+ const isoString = el.created.replace(" ", "T") + "Z"; return { ...el, type: "sightings", newCategory: "SIGHTING", standardizedFeedName: feedName, feedSlug: feedSlug, feedId: feedId, - timestampString: el.created.replace(" ", "T") + "Z", - timestamp: new Date(el.created.replace(" ", "T") + "Z"), + timestampString: isoString, + timestamp: new Date(isoString), };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/utils/dataTransforms.ts` around lines 100 - 101, Extract the duplicated ISO string construction into a single local variable and reuse it for both fields: compute a local const (e.g., iso = el.created.replace(" ", "T") + "Z") before returning the object and then set timestampString to that iso and timestamp to new Date(iso); update the mapping where timestampString and timestamp are created (look for the lines referencing el.created.replace(" ", "T") + "Z") so the expression is no longer duplicated.
71-84: Last-match-wins when bounding boxes overlap; redundantArray.isArrayguard.Two minor points:
Overlap:
forEachon line 73 never breaks, so if a sighting falls inside two overlapping feed bounding boxes the last feed silently wins. If feeds are guaranteed to have non-overlapping radii this is fine — otherwise consider returning the closest feed.Dead guard: The
if (!Array.isArray(sightings)) return [];on line 86 is unreachable — line 54 already returns early whensightingsis falsy or empty, and the type guarantees it's an array by this point. Safe to remove for clarity.Also applies to: 86-86
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/utils/dataTransforms.ts` around lines 71 - 84, The assignSightingsToHydrophones function currently iterates feedBoundingBoxes with forEach which causes a last-match-wins when bounding boxes overlap; change logic in assignSightingsToHydrophones to compute and return the nearest matching feed (e.g., track minimum distance from sighting to feed center or centroid) instead of overwriting hydrophone for each match, and short-circuit/return once the nearest is determined; use feedBoundingBoxes and standardizeFeedName to pick and normalize the chosen feed. Also remove the redundant runtime guard that checks Array.isArray(sightings) (the earlier falsy/empty check and types already guarantee an array) to avoid dead code.ui/src/utils/dataHelpers.ts (1)
50-95: Lookup helpers scan the full array and silently return sentinel strings on miss.All three
lookup*functions iterate the entire feed list (no early exit) and return placeholders like"feed id not found"on miss. Callers currently treat the return as a valid value without checking for the sentinel — meaning a missed lookup propagates a garbage string into downstream UI or data. For small feed lists the performance is fine, but consider returningundefined(or using.find()) so callers can handle the miss explicitly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/utils/dataHelpers.ts` around lines 50 - 95, The three helpers lookupFeedId, lookupFeedName, and lookupFeedSlug should stop scanning the whole feedList and stop returning sentinel strings; replace the manual forEach scans with a find-based lookup (or early return when matched) and return undefined on miss instead of "feed id not found"/"feed name not found"/"feed slug not found" so callers can handle misses explicitly; preserve the standardizeFeedName call in lookupFeedId and lookupFeedName but have lookupFeedName return standardizeFeedName(found.name) or undefined when no feed is found, and have lookupFeedId and lookupFeedSlug return the matched feed.id/feed.slug or undefined.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ui/src/components/Map.tsx`:
- Around line 138-147: The popup HTML uses dangerouslySetInnerHTML with external
data (sighting.name, sighting.comments) which is an XSS risk; fix by either
(preferred) replacing the dangerouslySetInnerHTML usage in Map.tsx with JSX text
nodes (render <strong>{sighting.name}</strong>, <div>{timeAgo} ago</div>,
<div>{sighting.created}</div>, and a sanitized comment block) or sanitize the
concatenated HTML with a trusted sanitizer (import DOMPurify and call
DOMPurify.sanitize(...) on the string returned/constructed for
cleanSightingsDescription) before passing it to dangerouslySetInnerHTML; update
or wrap the existing cleanSightingsDescription so it returns fully sanitized
safe HTML (no scripts or event handlers) if you keep innerHTML, and ensure all
references to sighting.name are escaped/sanitized as well.
- Around line 114-122: sighting.created is being parsed directly with new
Date(...) inside the sightings.map in Map.tsx which is unreliable for the
"YYYY-MM-DD HH:mm:ss" format; normalize the timestamp before parsing (e.g.
replace(" ", "T") + "Z") or, better, reuse the same normalization helper from
dataTransforms.ts, then convert to seconds and pass to formatDuration as you
currently do (update the code paths around sightings.map, sighting.created, and
the call to formatDuration to use the normalized/ISO string before creating the
Date).
- Around line 80-111: The AudibleRadiusCircles component is being rendered
inside the feeds.map loop causing N identical sets of circles (N²); compute
centers once (e.g., const centers = feeds.map(f => f.latLng)) and move
<AudibleRadiusCircles centers={centers} /> out of the feeds.map iteration so it
renders a single set of circles (render it once, before or after mapping over
feeds), and replace the current in-loop conditional (feeds?.length && ...) with
a single check (e.g., centers.length > 0) to avoid duplicate renders; update
references to AudibleRadiusCircles and feeds.map accordingly.
In `@ui/src/components/ReportCount.tsx`:
- Line 49: The code mutates the items array with items.unshift(<strong>...) and
inserts a React element without a key, causing a key warning; instead, prepend a
keyed element and avoid mutation by creating a new array (e.g., create a
<strong> element with a unique key like "last-7-days" and build a new array via
[thatElement, ...items]) and use that new array where interleaved is rendered
(refer to the items variable and the interleaved rendering in ReportCount
component).
- Line 47: In the ReportCount component remove the dead .filter((c) => c) call
(and its comment) that follows the .map over items because the .map always
returns a JSX element and the filter is a no-op; update the expression that
consumes the mapped array (e.g., the code that computes count or renders the
nodes) to use the result of .map(...) directly (refer to the .map callback
variable c and the ReportCount component) so no unnecessary filtering remains.
In `@ui/src/hooks/useCombinedData.ts`:
- Around line 12-17: The CombinedDataObject type is missing the
isSuccessSightings property while the useCombinedData hook returns it; update
the CombinedDataObject type declaration to include isSuccessSightings: boolean
(or remove isSuccessSightings from the returned object if that was intentional)
so the runtime shape matches the TypeScript type—specifically modify the
CombinedDataObject type (the type defined at top of useCombinedData.ts) to add
the isSuccessSightings field.
- Around line 28-29: The fallback creates a new empty array each render causing
unstable references and unnecessary recomputations; fix useCombinedData by
introducing a stable empty array constant (e.g. EMPTY_FEEDS: ReadonlyArray<Feed>
= []) and use it as the nullish fallback for useFeedsQuery().data?.feeds (const
seedFeeds = useFeedsQuery().data?.feeds ?? EMPTY_FEEDS), remove the redundant
`feeds` alias and the `as Feed[]` cast, and update downstream code to use
seedFeeds so the references used by the useMemo transforms are stable.
In `@ui/src/hooks/useSightings.ts`:
- Around line 34-37: The queryKey passed to useQuery is static ("sightings") and
will return stale cached results when callers change the date range; update the
queryKey in the useSightings hook to include the date parameters (e.g.,
["sightings", startDate, endDate] or normalized ISO strings) so React Query
treats different ranges as distinct keys, and ensure the fetchSightings call
uses the same startDate/endDate values used in the key.
In `@ui/src/utils/dataHelpers.ts`:
- Around line 97-103: The top-level apiTodayUTC constant is computed once at
module load and will become stale; change it into a function apiTodayUTC() that
computes a fresh Date inside the function (replace the module-level now and
todayUTC usage with local variables inside apiTodayUTC), return the same
yyyy-mm-dd string, and update callers to invoke apiTodayUTC() instead of reading
the constant so the value reflects the current UTC date on each call.
- Around line 13-34: formatDuration currently computes seconds = endOffset -
startOffset and returns undefined when seconds is negative; update the function
(formatDuration) to handle negative or invalid intervals by adding an early
guard that normalizes or clamps negative seconds (e.g., treat as 0 or return a
clear string like "audio unavailable" or "0 seconds") or add a fallback return
at the end that returns a safe string; ensure branches that reference seconds,
minutesRound, minutesDown, hoursDown, daysDown and remainder still work after
the change so Map.tsx won't interpolate "undefined ago".
- Around line 3-11: constructUrl currently concatenates params from paramsObj
without URL-encoding and leaves a trailing ampersand; update constructUrl to
build the query string with URLSearchParams (or encodeURIComponent) to properly
encode keys and values, remove the trailing '&' by using
URLSearchParams.prototype.toString(), and return endpoint + (queryString ? "?" +
queryString : "") so empty params don't add a '?'. Locate the function
constructUrl and replace the manual loop over entries/Object.entries(paramsObj)
with creation of new URLSearchParams(paramsObj) or iterative
URLSearchParams.append for each [key, value], then use params.toString() for the
final query string.
In `@ui/src/utils/dataTransforms.ts`:
- Around line 38-46: The code uses non-null assertions (el.feedId!) inside the
detections.map which lets undefined/null feedId values silently become sentinel
strings via lookupFeedName/lookupFeedSlug; instead, filter out detections with
missing feedId before mapping (e.g., detections.filter(d => d.feedId !=
null).map(...)) and remove the ! usage so lookupFeedName and lookupFeedSlug are
always called with a defined feedId; update the mapping in dataTransforms.ts
(the detections.map block and any callers relying on AudioDetection shape)
accordingly.
In `@ui/src/utils/mapHelpers.tsx`:
- Around line 86-101: In the HTML badge template string in mapHelpers.tsx (the
html property building the count badge), fix the two typos: replace the
malformed CSS rule "align-items; center;" with the correct "align-items:
center;" inside the span's style so vertical centering works, and correct the
closing tag sequence from "</span><div>" to "</span></div>" so the outer div is
properly closed and the fragment produces valid HTML.
---
Outside diff comments:
In `@ui/src/pages/seed.tsx`:
- Line 124: Remove the debug console.log call that prints resource and count in
the seeding logic (the console.log(resource, count) in ui/src/pages/seed.tsx) so
it doesn't spam the browser console in production; locate the function handling
the "Seed All" operation and delete that console.log line or replace it with a
conditional debug logger tied to an environment flag if you need retained debug
output.
---
Nitpick comments:
In `@ui/src/components/layouts/MapLayout.tsx`:
- Around line 64-68: The useEffect in MapLayout re-runs unnecessarily because
currentFeed is included in the dependency array; change the effect to depend
only on feed and perform a functional state update with setCurrentFeed(prev =>
...) so the setter compares prev?.slug to feed?.slug and only returns feed when
different, thereby removing currentFeed from the deps and avoiding redundant
re-renders; locate the useEffect, setCurrentFeed, currentFeed and feed symbols
in MapLayout and apply this change.
- Around line 74-76: The check uses the private field `_mapPane` on the
`mapWithPane` cast; replace that with Leaflet's public API by checking
`map.getPane('mapPane')` (or `map.getPanes().mapPane`) before returning so the
hot-reload guard relies on a stable public method; update the type cast on `map`
(in the `MapLayout` component where `mapWithPane` is created) if needed to
satisfy TypeScript, and change the early-return condition to use the public pane
lookup instead of `_mapPane`.
In `@ui/src/components/Map.tsx`:
- Around line 45-52: The L.icon creations (hydrophoneDefaultIcon and
hydrophoneActiveIcon) are being recreated on every render; move these static
icon objects out of the React component into module scope (or wrap their
creation in useMemo referencing no deps) so they are instantiated once. Locate
the hydrophoneDefaultIcon and hydrophoneActiveIcon definitions (the L.icon(...)
calls) and either hoist them above the component or wrap them with useMemo()
inside the component to prevent reallocation on each render.
In `@ui/src/components/ReportCount.tsx`:
- Around line 71-75: The Button currently uses href which causes a full-page
reload; import Next.js Link (e.g. Link from 'next/link') and change the MUI
Button inside Breadcrumbs to use client-side routing by setting component={Link}
and keeping href="/reports" (or alternatively wrap the Button with Next.js Link
and give the Button component="a"); update the Button in the Breadcrumbs block
so it uses Link for SPA navigation (refer to Breadcrumbs and Button in this
diff).
In `@ui/src/hooks/useSightings.ts`:
- Around line 11-18: The function useSightings is reassigning its parameters
startDate and endDate; replace those mutations by creating local constants
(e.g., resolvedStartDate and resolvedEndDate) that compute fallback values using
getDateMsAgo(rangeOptions.sevenDays).toISOString().split("T")[0] and apiTodayUTC
respectively, and then update all subsequent uses in useSightings (including any
request construction or hooks) to reference the new constants instead of
mutating startDate/endDate.
In `@ui/src/pages/listen/`[feed].tsx:
- Around line 23-25: The variable detectionsThisFeed is misleading because
useCombinedData().combined contains both AudioDetection and Sighting; rename
detectionsThisFeed to reportsThisFeed (or combinedDataThisFeed) and update all
usages accordingly (wherever detectionsThisFeed is referenced) so the filter
remains the same: const reportsThisFeed = combinedData?.filter((d) => d.feedSlug
=== slug); keep the source hook useCombinedData and the filter predicate
unchanged, only rename the identifier throughout the file to avoid implying the
array contains only detections.
In `@ui/src/types/DataTypes.ts`:
- Around line 19-24: The type union for newCategory is inconsistent with
consumers: change the literal union in DataTypes (the newCategory type used by
AudioDetection.newCategory and Sighting.newCategory) to use the lowercase
strings that ReportCount.tsx and other consumers store/compare (e.g., "whale
(human)", "vessel", "other", "whale (ai)", "uncategorized") so the type reflects
actual runtime values, or alternatively update the consumers to use the ALL-CAPS
literals consistently; pick the lowercase option for minimal churn and update
the union accordingly and any related usages that assume uppercase.
In `@ui/src/utils/dataHelpers.ts`:
- Around line 50-95: The three helpers lookupFeedId, lookupFeedName, and
lookupFeedSlug should stop scanning the whole feedList and stop returning
sentinel strings; replace the manual forEach scans with a find-based lookup (or
early return when matched) and return undefined on miss instead of "feed id not
found"/"feed name not found"/"feed slug not found" so callers can handle misses
explicitly; preserve the standardizeFeedName call in lookupFeedId and
lookupFeedName but have lookupFeedName return standardizeFeedName(found.name) or
undefined when no feed is found, and have lookupFeedId and lookupFeedSlug return
the matched feed.id/feed.slug or undefined.
In `@ui/src/utils/dataTransforms.ts`:
- Around line 100-101: Extract the duplicated ISO string construction into a
single local variable and reuse it for both fields: compute a local const (e.g.,
iso = el.created.replace(" ", "T") + "Z") before returning the object and then
set timestampString to that iso and timestamp to new Date(iso); update the
mapping where timestampString and timestamp are created (look for the lines
referencing el.created.replace(" ", "T") + "Z") so the expression is no longer
duplicated.
- Around line 71-84: The assignSightingsToHydrophones function currently
iterates feedBoundingBoxes with forEach which causes a last-match-wins when
bounding boxes overlap; change logic in assignSightingsToHydrophones to compute
and return the nearest matching feed (e.g., track minimum distance from sighting
to feed center or centroid) instead of overwriting hydrophone for each match,
and short-circuit/return once the nearest is determined; use feedBoundingBoxes
and standardizeFeedName to pick and normalize the chosen feed. Also remove the
redundant runtime guard that checks Array.isArray(sightings) (the earlier
falsy/empty check and types already guarantee an array) to avoid dead code.
In `@ui/src/utils/mapHelpers.tsx`:
- Around line 69-80: The ReportCount component reassigns the incoming prop count
(if (!count) count = 0), which is bad practice; change the destructured props to
give count a default (e.g., { center, count = 0, onClick }) or create a new
local constant (e.g., const safeCount = count ?? 0) and use that throughout;
update references in ReportCount to use the defaulted/destructured count or the
new safeCount and remove the reassignment.
In `@ui/vitest.config.ts`:
- Around line 5-7: The Vitest config currently sets test: { environment: "node"
} which is fine for utility-only tests but will break React component tests;
either (A) update future component test files to include a top-of-file doc-block
like /** `@vitest-environment` jsdom */ (or happy-dom) so those tests run under
jsdom, or (B) change the global test.environment from "node" to "jsdom" (or
"happy-dom") in the vitest config and install `@types/jsdom` to provide DOM
typings for TypeScript; locate the test config object (test.environment) and
apply one of these two options depending on whether you want per-file overrides
or a global jsdom environment.
Comment on lines
+80
to
+111
| {feeds.map((feed) => { | ||
| const audioDetectionsThisFeed = detections?.filter( | ||
| (d) => d.feedId === feed?.id, | ||
| ).length; | ||
|
|
||
| return ( | ||
| <Fragment key={feed.slug}> | ||
| {feeds?.length && ( | ||
| <AudibleRadiusCircles centers={feeds.map((f) => f.latLng)} /> | ||
| )} | ||
|
|
||
| <Marker | ||
| key={feed.slug} | ||
| position={feed.latLng} | ||
| icon={ | ||
| feed.slug === currentFeed?.slug | ||
| ? hydrophoneActiveIcon | ||
| : hydrophoneDefaultIcon | ||
| } | ||
| zIndexOffset={100} | ||
| /> | ||
|
|
||
| <ReportCount | ||
| center={feed.latLng} | ||
| count={audioDetectionsThisFeed} | ||
| onClick={() => { | ||
| router.push(`/listen/${feed.slug}`); | ||
| }} | ||
| /> | ||
| </Fragment> | ||
| ); | ||
| })} |
Contributor
There was a problem hiding this comment.
AudibleRadiusCircles is rendered inside the feeds.map loop, creating N² circles.
Each iteration of the loop passes all feed centers (feeds.map((f) => f.latLng)) to AudibleRadiusCircles, so for N feeds you get N identical sets of N circles (N² total). This stacks the semi-transparent red fills, making the visual progressively more opaque, and wastes DOM/canvas resources.
Move it outside the loop so it renders once.
Proposed fix
+ {feeds?.length > 0 && (
+ <AudibleRadiusCircles centers={feeds.map((f) => f.latLng)} />
+ )}
+
{feeds.map((feed) => {
const audioDetectionsThisFeed = detections?.filter(
(d) => d.feedId === feed?.id,
).length;
return (
<Fragment key={feed.slug}>
- {feeds?.length && (
- <AudibleRadiusCircles centers={feeds.map((f) => f.latLng)} />
- )}
-
<Marker
key={feed.slug}
position={feed.latLng}📝 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.
Suggested change
| {feeds.map((feed) => { | |
| const audioDetectionsThisFeed = detections?.filter( | |
| (d) => d.feedId === feed?.id, | |
| ).length; | |
| return ( | |
| <Fragment key={feed.slug}> | |
| {feeds?.length && ( | |
| <AudibleRadiusCircles centers={feeds.map((f) => f.latLng)} /> | |
| )} | |
| <Marker | |
| key={feed.slug} | |
| position={feed.latLng} | |
| icon={ | |
| feed.slug === currentFeed?.slug | |
| ? hydrophoneActiveIcon | |
| : hydrophoneDefaultIcon | |
| } | |
| zIndexOffset={100} | |
| /> | |
| <ReportCount | |
| center={feed.latLng} | |
| count={audioDetectionsThisFeed} | |
| onClick={() => { | |
| router.push(`/listen/${feed.slug}`); | |
| }} | |
| /> | |
| </Fragment> | |
| ); | |
| })} | |
| {feeds?.length > 0 && ( | |
| <AudibleRadiusCircles centers={feeds.map((f) => f.latLng)} /> | |
| )} | |
| {feeds.map((feed) => { | |
| const audioDetectionsThisFeed = detections?.filter( | |
| (d) => d.feedId === feed?.id, | |
| ).length; | |
| return ( | |
| <Fragment key={feed.slug}> | |
| <Marker | |
| key={feed.slug} | |
| position={feed.latLng} | |
| icon={ | |
| feed.slug === currentFeed?.slug | |
| ? hydrophoneActiveIcon | |
| : hydrophoneDefaultIcon | |
| } | |
| zIndexOffset={100} | |
| /> | |
| <ReportCount | |
| center={feed.latLng} | |
| count={audioDetectionsThisFeed} | |
| onClick={() => { | |
| router.push(`/listen/${feed.slug}`); | |
| }} | |
| /> | |
| </Fragment> | |
| ); | |
| })} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/components/Map.tsx` around lines 80 - 111, The AudibleRadiusCircles
component is being rendered inside the feeds.map loop causing N identical sets
of circles (N²); compute centers once (e.g., const centers = feeds.map(f =>
f.latLng)) and move <AudibleRadiusCircles centers={centers} /> out of the
feeds.map iteration so it renders a single set of circles (render it once,
before or after mapping over feeds), and replace the current in-loop conditional
(feeds?.length && ...) with a single check (e.g., centers.length > 0) to avoid
duplicate renders; update references to AudibleRadiusCircles and feeds.map
accordingly.
Comment on lines
+114
to
+122
| {sightings?.map((sighting) => { | ||
| const sightingTimeSeconds = | ||
| new Date(sighting.created).getTime() / 1000; | ||
| const currentTimeSeconds = new Date().getTime() / 1000; | ||
|
|
||
| const timeAgo = formatDuration( | ||
| sightingTimeSeconds, | ||
| currentTimeSeconds, | ||
| ); |
Contributor
There was a problem hiding this comment.
sighting.created parsed with new Date() without ISO normalization.
CascadiaSighting.created uses the format "2025-01-01 17:25:00" (space-separated). The new Date(string) constructor behaviour for non-ISO strings is implementation-defined — some browsers may parse it as local time, others as UTC, and some may return NaN. In dataTransforms.ts this is correctly normalized with .replace(" ", "T") + "Z", but here the raw value is passed directly.
Proposed fix
- const sightingTimeSeconds =
- new Date(sighting.created).getTime() / 1000;
+ const sightingTimeSeconds =
+ new Date(sighting.created.replace(" ", "T") + "Z").getTime() / 1000;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/components/Map.tsx` around lines 114 - 122, sighting.created is being
parsed directly with new Date(...) inside the sightings.map in Map.tsx which is
unreliable for the "YYYY-MM-DD HH:mm:ss" format; normalize the timestamp before
parsing (e.g. replace(" ", "T") + "Z") or, better, reuse the same normalization
helper from dataTransforms.ts, then convert to seconds and pass to
formatDuration as you currently do (update the code paths around sightings.map,
sighting.created, and the call to formatDuration to use the normalized/ISO
string before creating the Date).
Comment on lines
+138
to
+147
| <div | ||
| dangerouslySetInnerHTML={{ | ||
| __html: ` | ||
| <strong>${sighting.name}</strong><br /> | ||
| ${timeAgo} ago<br /> | ||
| ${sighting.created}<br /> | ||
| ${cleanSightingsDescription(sighting.comments)} | ||
| `, | ||
| }} | ||
| /> |
Contributor
There was a problem hiding this comment.
XSS risk: dangerouslySetInnerHTML with external sighting data.
sighting.name and sighting.comments originate from an external API (Acartia/Cascadia). cleanSightingsDescription strips URLs and <br> tags but does not neutralize <script>, onerror, or other injection vectors. This is a real XSS surface.
Sanitize with a library like DOMPurify before injection, or avoid dangerouslySetInnerHTML entirely by rendering the content as text nodes with structural JSX.
🧰 Tools
🪛 ast-grep (0.40.5)
[warning] 138-138: Usage of dangerouslySetInnerHTML detected. This bypasses React's built-in XSS protection. Always sanitize HTML content using libraries like DOMPurify before injecting it into the DOM to prevent XSS attacks.
Context: dangerouslySetInnerHTML
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml
- https://cwe.mitre.org/data/definitions/79.html
(react-unsafe-html-injection)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/components/Map.tsx` around lines 138 - 147, The popup HTML uses
dangerouslySetInnerHTML with external data (sighting.name, sighting.comments)
which is an XSS risk; fix by either (preferred) replacing the
dangerouslySetInnerHTML usage in Map.tsx with JSX text nodes (render
<strong>{sighting.name}</strong>, <div>{timeAgo} ago</div>,
<div>{sighting.created}</div>, and a sanitized comment block) or sanitize the
concatenated HTML with a trusted sanitizer (import DOMPurify and call
DOMPurify.sanitize(...) on the string returned/constructed for
cleanSightingsDescription) before passing it to dangerouslySetInnerHTML; update
or wrap the existing cleanSightingsDescription so it returns fully sanitized
safe HTML (no scripts or event handlers) if you keep innerHTML, and ensure all
references to sighting.name are escaped/sanitized as well.
| </div> | ||
| ); | ||
| }) | ||
| .filter((c) => c); // filters out the null items |
Contributor
There was a problem hiding this comment.
Dead .filter() call with misleading comment.
.map() always returns a JSX element here — no item is ever null or falsy — so this filter is a no-op. Remove it (and the comment) to avoid confusion.
♻️ Proposed fix
- })
- .filter((c) => c); // filters out the null items
+ });📝 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.
Suggested change
| .filter((c) => c); // filters out the null items | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/components/ReportCount.tsx` at line 47, In the ReportCount component
remove the dead .filter((c) => c) call (and its comment) that follows the .map
over items because the .map always returns a JSX element and the filter is a
no-op; update the expression that consumes the mapped array (e.g., the code that
computes count or renders the nodes) to use the result of .map(...) directly
(refer to the .map callback variable c and the ReportCount component) so no
unnecessary filtering remains.
| }) | ||
| .filter((c) => c); // filters out the null items | ||
|
|
||
| items.unshift(<strong>Last 7 days</strong>); |
Contributor
There was a problem hiding this comment.
Missing key prop on the <strong> item causes a React key warning.
items is rendered as part of the interleaved array inside JSX. React requires a key on every element in such arrays; the <strong> item added via unshift has none. The fix also avoids mutating the local array:
🐛 Proposed fix
-items.unshift(<strong>Last 7 days</strong>);
+const allItems = [<strong key="label">Last 7 days</strong>, ...items];
// Interleave with separators
-const interleaved = items.flatMap((item, index) =>
- index < items.length - 1
+const interleaved = allItems.flatMap((item, index) =>
+ index < allItems.length - 1
? [item, <span key={`dot-${index}`}> • </span>]
: [item],
);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/components/ReportCount.tsx` at line 49, The code mutates the items
array with items.unshift(<strong>...) and inserts a React element without a key,
causing a key warning; instead, prepend a keyed element and avoid mutation by
creating a new array (e.g., create a <strong> element with a unique key like
"last-7-days" and build a new array via [thatElement, ...items]) and use that
new array where interleaved is rendered (refer to the items variable and the
interleaved rendering in ReportCount component).
Comment on lines
+3
to
+11
| export function constructUrl(endpoint: string, paramsObj: object) { | ||
| let params = ""; | ||
| const entries = Object.entries(paramsObj); | ||
| for (const [key, value] of entries) { | ||
| const str = [key, value].join("=") + "&"; | ||
| params += str; | ||
| } | ||
| return endpoint + "?" + params; | ||
| } |
Contributor
There was a problem hiding this comment.
constructUrl doesn't URL-encode values and leaves a trailing &.
Special characters in parameter values (spaces, &, =, etc.) will break the URL. The trailing & is cosmetically wrong. Consider using URLSearchParams which handles both issues.
Proposed fix
export function constructUrl(endpoint: string, paramsObj: object) {
- let params = "";
- const entries = Object.entries(paramsObj);
- for (const [key, value] of entries) {
- const str = [key, value].join("=") + "&";
- params += str;
- }
- return endpoint + "?" + params;
+ const params = new URLSearchParams(
+ Object.entries(paramsObj).map(([k, v]) => [k, String(v)]),
+ );
+ return `${endpoint}?${params.toString()}`;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/utils/dataHelpers.ts` around lines 3 - 11, constructUrl currently
concatenates params from paramsObj without URL-encoding and leaves a trailing
ampersand; update constructUrl to build the query string with URLSearchParams
(or encodeURIComponent) to properly encode keys and values, remove the trailing
'&' by using URLSearchParams.prototype.toString(), and return endpoint +
(queryString ? "?" + queryString : "") so empty params don't add a '?'. Locate
the function constructUrl and replace the manual loop over
entries/Object.entries(paramsObj) with creation of new
URLSearchParams(paramsObj) or iterative URLSearchParams.append for each [key,
value], then use params.toString() for the final query string.
Comment on lines
+13
to
+34
| export default function formatDuration(startOffset: number, endOffset: number) { | ||
| const seconds = endOffset - startOffset; | ||
| const minutesRound = Math.round(seconds / 60); | ||
| const minutesDown = Math.floor(seconds / 60); | ||
| const hoursDown = Math.floor(seconds / 60 / 60); | ||
| const daysDown = Math.floor(seconds / 60 / 60 / 24); | ||
| const remainder = Math.round(seconds % 60); | ||
|
|
||
| if (seconds === 0) { | ||
| return "audio unavailable"; | ||
| } else if (seconds < 60) { | ||
| return `${seconds} second${seconds === 1 ? "" : "s"}`; | ||
| } else if (seconds < 600) { | ||
| return `${minutesDown} minute${minutesDown === 1 ? "" : "s"} ${remainder} second${remainder === 1 ? "" : "s"}`; | ||
| } else if (seconds >= 600 && seconds < 3600) { | ||
| return `${minutesRound} minute${minutesRound === 1 ? "" : "s"}`; | ||
| } else if (seconds >= 3600 && seconds < 86400) { | ||
| return `${hoursDown} hour${hoursDown === 1 ? "" : "s"}`; | ||
| } else if (seconds >= 86400) { | ||
| return `${daysDown} day${daysDown === 1 ? "" : "s"}`; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
formatDuration returns undefined for negative inputs.
If endOffset < startOffset (e.g., a future-dated sighting timestamp), seconds is negative and no branch matches — the function silently returns undefined. This is consumed in Map.tsx line 119 and interpolated as "undefined ago" in the tooltip.
At minimum, add a fallback return at the end of the function.
Proposed fix
} else if (seconds >= 86400) {
return `${daysDown} day${daysDown === 1 ? "" : "s"}`;
}
+ return "unknown duration";
}📝 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.
Suggested change
| export default function formatDuration(startOffset: number, endOffset: number) { | |
| const seconds = endOffset - startOffset; | |
| const minutesRound = Math.round(seconds / 60); | |
| const minutesDown = Math.floor(seconds / 60); | |
| const hoursDown = Math.floor(seconds / 60 / 60); | |
| const daysDown = Math.floor(seconds / 60 / 60 / 24); | |
| const remainder = Math.round(seconds % 60); | |
| if (seconds === 0) { | |
| return "audio unavailable"; | |
| } else if (seconds < 60) { | |
| return `${seconds} second${seconds === 1 ? "" : "s"}`; | |
| } else if (seconds < 600) { | |
| return `${minutesDown} minute${minutesDown === 1 ? "" : "s"} ${remainder} second${remainder === 1 ? "" : "s"}`; | |
| } else if (seconds >= 600 && seconds < 3600) { | |
| return `${minutesRound} minute${minutesRound === 1 ? "" : "s"}`; | |
| } else if (seconds >= 3600 && seconds < 86400) { | |
| return `${hoursDown} hour${hoursDown === 1 ? "" : "s"}`; | |
| } else if (seconds >= 86400) { | |
| return `${daysDown} day${daysDown === 1 ? "" : "s"}`; | |
| } | |
| } | |
| export default function formatDuration(startOffset: number, endOffset: number) { | |
| const seconds = endOffset - startOffset; | |
| const minutesRound = Math.round(seconds / 60); | |
| const minutesDown = Math.floor(seconds / 60); | |
| const hoursDown = Math.floor(seconds / 60 / 60); | |
| const daysDown = Math.floor(seconds / 60 / 60 / 24); | |
| const remainder = Math.round(seconds % 60); | |
| if (seconds === 0) { | |
| return "audio unavailable"; | |
| } else if (seconds < 60) { | |
| return `${seconds} second${seconds === 1 ? "" : "s"}`; | |
| } else if (seconds < 600) { | |
| return `${minutesDown} minute${minutesDown === 1 ? "" : "s"} ${remainder} second${remainder === 1 ? "" : "s"}`; | |
| } else if (seconds >= 600 && seconds < 3600) { | |
| return `${minutesRound} minute${minutesRound === 1 ? "" : "s"}`; | |
| } else if (seconds >= 3600 && seconds < 86400) { | |
| return `${hoursDown} hour${hoursDown === 1 ? "" : "s"}`; | |
| } else if (seconds >= 86400) { | |
| return `${daysDown} day${daysDown === 1 ? "" : "s"}`; | |
| } | |
| return "unknown duration"; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/utils/dataHelpers.ts` around lines 13 - 34, formatDuration currently
computes seconds = endOffset - startOffset and returns undefined when seconds is
negative; update the function (formatDuration) to handle negative or invalid
intervals by adding an early guard that normalizes or clamps negative seconds
(e.g., treat as 0 or return a clear string like "audio unavailable" or "0
seconds") or add a fallback return at the end that returns a safe string; ensure
branches that reference seconds, minutesRound, minutesDown, hoursDown, daysDown
and remainder still work after the change so Map.tsx won't interpolate
"undefined ago".
Comment on lines
+97
to
+103
| const now = new Date(); | ||
| const todayUTC = { | ||
| yyyy: now.getUTCFullYear(), | ||
| mm: String(now.getUTCMonth() + 1).padStart(2, "0"), // e.g. month as "05" | ||
| dd: String(now.getUTCDate()).padStart(2, "0"), | ||
| }; | ||
| export const apiTodayUTC = `${todayUTC.yyyy}-${todayUTC.mm}-${todayUTC.dd}`; |
Contributor
There was a problem hiding this comment.
apiTodayUTC is computed once at module load and never refreshes.
This top-level const captures the date at import time. In a long-lived client session (or a Next.js server that caches modules across requests), it will go stale after midnight UTC. If callers expect "today," make it a function instead.
Proposed fix
-const now = new Date();
-const todayUTC = {
- yyyy: now.getUTCFullYear(),
- mm: String(now.getUTCMonth() + 1).padStart(2, "0"),
- dd: String(now.getUTCDate()).padStart(2, "0"),
-};
-export const apiTodayUTC = `${todayUTC.yyyy}-${todayUTC.mm}-${todayUTC.dd}`;
+export const apiTodayUTC = () => {
+ const now = new Date();
+ const yyyy = now.getUTCFullYear();
+ const mm = String(now.getUTCMonth() + 1).padStart(2, "0");
+ const dd = String(now.getUTCDate()).padStart(2, "0");
+ return `${yyyy}-${mm}-${dd}`;
+};Note: this changes the call-site from apiTodayUTC to apiTodayUTC() — all consumers would need to be updated.
📝 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.
Suggested change
| const now = new Date(); | |
| const todayUTC = { | |
| yyyy: now.getUTCFullYear(), | |
| mm: String(now.getUTCMonth() + 1).padStart(2, "0"), // e.g. month as "05" | |
| dd: String(now.getUTCDate()).padStart(2, "0"), | |
| }; | |
| export const apiTodayUTC = `${todayUTC.yyyy}-${todayUTC.mm}-${todayUTC.dd}`; | |
| export const apiTodayUTC = () => { | |
| const now = new Date(); | |
| const yyyy = now.getUTCFullYear(); | |
| const mm = String(now.getUTCMonth() + 1).padStart(2, "0"); | |
| const dd = String(now.getUTCDate()).padStart(2, "0"); | |
| return `${yyyy}-${mm}-${dd}`; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/utils/dataHelpers.ts` around lines 97 - 103, The top-level apiTodayUTC
constant is computed once at module load and will become stale; change it into a
function apiTodayUTC() that computes a fresh Date inside the function (replace
the module-level now and todayUTC usage with local variables inside
apiTodayUTC), return the same yyyy-mm-dd string, and update callers to invoke
apiTodayUTC() instead of reading the constant so the value reflects the current
UTC date on each call.
Comment on lines
+38
to
+46
| return detections.map((el) => ({ | ||
| ...el, | ||
| type: "audio", | ||
| standardizedFeedName: lookupFeedName(el.feedId!, feeds), | ||
| feedSlug: lookupFeedSlug(el.feedId!, feeds), | ||
| comments: el.description, | ||
| newCategory: toNewCategory(el), | ||
| timestampString: el.timestamp.toString(), | ||
| })); |
Contributor
There was a problem hiding this comment.
Non-null assertions on el.feedId! risk silent data corruption.
If any DetectionsResult has a null/undefined feedId, the ! operator will pass undefined to the lookup helpers, which will silently return sentinel strings like "feed name not found" and "feed slug not found" — producing an AudioDetection that looks valid but carries garbage metadata. Prefer a guard or filter.
Proposed fix: filter out detections with missing feedId
- return detections.map((el) => ({
+ return detections.filter((el) => el.feedId != null).map((el) => ({
...el,
type: "audio",
- standardizedFeedName: lookupFeedName(el.feedId!, feeds),
- feedSlug: lookupFeedSlug(el.feedId!, feeds),
+ standardizedFeedName: lookupFeedName(el.feedId, feeds),
+ feedSlug: lookupFeedSlug(el.feedId, feeds),
comments: el.description,
newCategory: toNewCategory(el),
timestampString: el.timestamp.toString(),
}));📝 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.
Suggested change
| return detections.map((el) => ({ | |
| ...el, | |
| type: "audio", | |
| standardizedFeedName: lookupFeedName(el.feedId!, feeds), | |
| feedSlug: lookupFeedSlug(el.feedId!, feeds), | |
| comments: el.description, | |
| newCategory: toNewCategory(el), | |
| timestampString: el.timestamp.toString(), | |
| })); | |
| return detections.filter((el) => el.feedId != null).map((el) => ({ | |
| ...el, | |
| type: "audio", | |
| standardizedFeedName: lookupFeedName(el.feedId, feeds), | |
| feedSlug: lookupFeedSlug(el.feedId, feeds), | |
| comments: el.description, | |
| newCategory: toNewCategory(el), | |
| timestampString: el.timestamp.toString(), | |
| })); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/utils/dataTransforms.ts` around lines 38 - 46, The code uses non-null
assertions (el.feedId!) inside the detections.map which lets undefined/null
feedId values silently become sentinel strings via
lookupFeedName/lookupFeedSlug; instead, filter out detections with missing
feedId before mapping (e.g., detections.filter(d => d.feedId != null).map(...))
and remove the ! usage so lookupFeedName and lookupFeedSlug are always called
with a defined feedId; update the mapping in dataTransforms.ts (the
detections.map block and any callers relying on AudioDetection shape)
accordingly.
Comment on lines
+86
to
+101
| html: `<div style="position: relative; z-index: 1001;"> | ||
| <span style=" | ||
| position: absolute; | ||
| top: -10px; | ||
| right: -10px; | ||
| background: red; | ||
| color: white; | ||
| border-radius: 100%; | ||
| padding: 2px 5px; | ||
| font-size: 10px; | ||
| min-width: 20px; | ||
| min-height: 20px; | ||
| display: flex; | ||
| justify-content: center; | ||
| align-items; center; | ||
| ">${count}</span><div>`, |
Contributor
There was a problem hiding this comment.
Two bugs in the count badge HTML template.
- Line 100:
align-items; center;uses a semicolon instead of a colon — the vertical centering rule is broken (CSS parse error, property is silently ignored). - Line 101:
</span><div>should be</span></div>— the outer<div>is never closed, producing malformed HTML.
Proposed fix
- align-items; center;
- ">${count}</span><div>`,
+ align-items: center;
+ ">${count}</span></div>`,📝 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.
Suggested change
| html: `<div style="position: relative; z-index: 1001;"> | |
| <span style=" | |
| position: absolute; | |
| top: -10px; | |
| right: -10px; | |
| background: red; | |
| color: white; | |
| border-radius: 100%; | |
| padding: 2px 5px; | |
| font-size: 10px; | |
| min-width: 20px; | |
| min-height: 20px; | |
| display: flex; | |
| justify-content: center; | |
| align-items; center; | |
| ">${count}</span><div>`, | |
| html: `<div style="position: relative; z-index: 1001;"> | |
| <span style=" | |
| position: absolute; | |
| top: -10px; | |
| right: -10px; | |
| background: red; | |
| color: white; | |
| border-radius: 100%; | |
| padding: 2px 5px; | |
| font-size: 10px; | |
| min-width: 20px; | |
| min-height: 20px; | |
| display: flex; | |
| justify-content: center; | |
| align-items: center; | |
| ">${count}</span></div>`, |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/utils/mapHelpers.tsx` around lines 86 - 101, In the HTML badge
template string in mapHelpers.tsx (the html property building the count badge),
fix the two typos: replace the malformed CSS rule "align-items; center;" with
the correct "align-items: center;" inside the span's style so vertical centering
works, and correct the closing tag sequence from "</span><div>" to
"</span></div>" so the outer div is properly closed and the fragment produces
valid HTML.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds map markers and report counts from the last 7 days of sightings (Acartia) and detections (Orcasound human+machine) to the MapLayout/Map component and listen/[feed] pages:
useSightings) for data calls to Acartia endpointuseCombinedData) for mapping sightings to hydrophones by 3-mile bounding box, and aligning fieldsDependency Context
This branch includes/depends on work from two unmerged PRs:
adrian/fix-seeding(/server + seed.tsx page behavior)adrian/add-test-runner(Vitest runner setup)What Changed
1) Combined data layer (DTO -> domain adapter)
useSightingshook for Acartia sightings API.useCombinedDatahook to composeuseDetectionsQuery+useSightingsinto unified frontend shapeFiles:
ui/src/hooks/useCombinedData.tsui/src/hooks/useSightings.tsui/src/types/DataTypes.tsui/src/utils/dataHelpers.tsui/src/utils/dataTransforms.ts2) Map data visualization upgrades
Files:
ui/src/components/Map.tsxui/src/utils/mapHelpers.tsx3) Hydrophone page report count and Reports link
ReportCountcomponent (useCombinedData)./listen/[feed]./reportsFiles:
ui/src/components/ReportCount.tsxui/src/pages/listen/[feed].tsx4) Map transition behavior
/listen/[feed].setViewcrashes in dev.Files:
ui/src/components/layouts/MapLayout.tsxui/src/components/Map.tsx5) Test runner + initial unit tests
Files:
ui/package.jsonui/package-lock.jsonui/vitest.config.tsui/src/utils/dataTransforms.test.tsTesting Performed
cd ui && npx tsc --noEmitcd ui && npm run test4 passedReview Guidance
Review order (recommended by Codex):
ui/src/types/DataTypes.tsui/src/utils/dataHelpers.tsui/src/utils/dataTransforms.tsui/src/hooks/useSightings.tsui/src/hooks/useCombinedData.tsui/src/components/Map.tsxui/src/components/layouts/MapLayout.tsxui/src/pages/listen/[feed].tsxui/src/components/ReportCount.tsxFollow-ups (optional)
orcasite.Up next
Summary by CodeRabbit
Release Notes
New Features
Chores