Open
Conversation
✅ Deploy Preview for antenna-ssec ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for antenna-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
📝 WalkthroughWalkthroughThis PR performs a comprehensive UI component library migration from custom design-system components (Icon, IconButton, Button) to lucide-react icons and nova-ui-kit components, alongside refactoring data models to return Date objects instead of formatted timestamp strings. The changes span dependency updates, component removals, and updates across 80+ files. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 14
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
ui/src/data-services/models/pipeline.ts (1)
63-71:⚠️ Potential issue | 🟠 MajorBug:
versionLabeldisplays string length instead of version name.Line 69 uses
this._pipeline.version_name?.lengthinside the template string, which will display the length of the version name (a number) rather than the actual version name string.🐛 Proposed fix
get versionLabel(): string | undefined { if (this._pipeline.version == undefined) { return undefined } return this._pipeline.version_name?.length - ? `${this._pipeline.version} "${this._pipeline.version_name?.length}"` + ? `${this._pipeline.version} "${this._pipeline.version_name}"` : `${this._pipeline.version}` }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/data-services/models/pipeline.ts` around lines 63 - 71, The versionLabel getter is accidentally interpolating the length of the version name instead of the version name itself; update the getter (versionLabel) so that when this._pipeline.version_name exists you return `${this._pipeline.version} "${this._pipeline.version_name}"` (not using .length), otherwise return `${this._pipeline.version}`; ensure you still handle undefined this._pipeline.version by returning undefined as currently implemented.ui/src/components/terms-of-service-info/terms-of-service-info.tsx (1)
12-17:⚠️ Potential issue | 🟡 MinorStale closure in cleanup effect.
The cleanup function references
userPreferencesbut it's not included in the dependency array. This means ifuserPreferenceschanges before unmount, the cleanup will spread the stale object.🛠️ Proposed fix
useEffect(() => { return () => { // Mark message as seen when component unmounts setUserPreferences({ ...userPreferences, termsMessageSeen: true }) } -}, []) +}, [userPreferences, setUserPreferences])Alternatively, use a functional update if
setUserPreferencessupports it:useEffect(() => { return () => { // Mark message as seen when component unmounts - setUserPreferences({ ...userPreferences, termsMessageSeen: true }) + setUserPreferences((prev) => ({ ...prev, termsMessageSeen: true })) } }, [setUserPreferences])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/terms-of-service-info/terms-of-service-info.tsx` around lines 12 - 17, The cleanup effect in useEffect references userPreferences but doesn't list it as a dependency, causing a stale closure; update the cleanup to use a functional state update so it doesn't depend on the closed-over userPreferences (e.g., call setUserPreferences(prev => ({ ...prev, termsMessageSeen: true })) inside the cleanup of the useEffect in terms-of-service-info.tsx), and ensure setUserPreferences is stable or included in the dependency array if it's not memoized.
🟡 Minor comments (11)
ui/src/utils/constants.ts-117-119 (1)
117-119:⚠️ Potential issue | 🟡 MinorUpdate both URLs to use consistent domain format (with
www).
LANDING_PAGE_URLusesinsectai.orgwhileLANDING_PAGE_CONTACT_URLuseswww.insectai.org. The non-www domain redirects (301) to the www version, so both constants should use the www format for consistency and to avoid unnecessary redirects.🔧 Suggested fix
-export const LANDING_PAGE_URL = 'https://insectai.org/' +export const LANDING_PAGE_URL = 'https://www.insectai.org/'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/utils/constants.ts` around lines 117 - 119, Update the inconsistent domain in the exported constants so both use the www form: change LANDING_PAGE_URL to 'https://www.insectai.org/' to match LANDING_PAGE_CONTACT_URL; locate the constants LANDING_PAGE_URL and LANDING_PAGE_CONTACT_URL in the file and make the LANDING_PAGE_URL value use the www subdomain to avoid the 301 redirect and keep formats consistent.ui/src/pages/deployment-details/deployment-details-form/section-location/section-location.tsx-119-124 (1)
119-124:⚠️ Potential issue | 🟡 MinorAdd explicit
typeattribute to form navigation buttons.Both buttons lack a
typeattribute. In HTML forms, buttons default totype="submit", causing the "Back" button to trigger form submission before callingonBack. Addtype="button"to prevent unintended submission:Suggested fix
- <Button onClick={onBack} size="small" variant="outline"> + <Button onClick={onBack} size="small" variant="outline" type="button"> <span>{translate(STRING.BACK)}</span> </Button> - <Button onClick={onNext} size="small" variant="success"> + <Button onClick={onNext} size="small" variant="success" type="button"> <span>{translate(STRING.NEXT)}</span> </Button>The same pattern exists in
section-source-images.tsx. Codebase convention shows explicittype="button"for non-submit buttons (e.g.,deployment-details-form.tsxCancel button,image-upload.tsx).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/deployment-details/deployment-details-form/section-location/section-location.tsx` around lines 119 - 124, The Back and Next navigation Buttons in section-location.tsx (handlers onBack and onNext) are missing an explicit type and therefore default to type="submit"; update both Button usages to include type="button" to prevent unintended form submission when clicking Back (and for consistency for Next if it should not submit). Also apply the same change to the corresponding Buttons in section-source-images.tsx, following the existing convention used for non-submit buttons like the Cancel button in deployment-details-form.tsx and the button in image-upload.tsx.ui/src/design-system/components/table/basic-table-cell/basic-table-cell.module.scss-11-11 (1)
11-11:⚠️ Potential issue | 🟡 MinorUse
overflow-wrap: break-wordinstead of the deprecatedword-break: break-word.The value
break-wordfor theword-breakproperty is deprecated. The CSS standard recommends usingoverflow-wrap: break-wordfor this behavior.Proposed fix
.label { display: block; `@include` paragraph-medium(); color: $color-neutral-700; - word-break: break-word; + overflow-wrap: break-word; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/design-system/components/table/basic-table-cell/basic-table-cell.module.scss` at line 11, Replace the deprecated CSS declaration "word-break: break-word" in basic-table-cell.module.scss with the modern equivalent by removing or replacing that property and adding "overflow-wrap: break-word" on the same selector (the basic table cell rule) so long words wrap correctly; update any related rules that rely on word-break in the same module to use overflow-wrap instead to maintain behavior across browsers.ui/src/data-services/models/occurrence.ts-23-25 (1)
23-25:⚠️ Potential issue | 🟡 Minor
createdAtshould not return an invalidDateinstance on bad/missing data.
Line 24can produceInvalid Datewhile the getter is typed as always validDate. Please align with a guardedDate | undefinedreturn (and validate parse result).Suggested fix
- get createdAt(): Date { - return new Date(this._occurrence.created_at) + get createdAt(): Date | undefined { + if (!this._occurrence.created_at) { + return undefined + } + const date = new Date(this._occurrence.created_at) + return Number.isNaN(date.getTime()) ? undefined : date } @@ get updatedAt(): Date | undefined { if (!this._occurrence.updated_at) { return undefined } - - return new Date(this._occurrence.updated_at) + const date = new Date(this._occurrence.updated_at) + return Number.isNaN(date.getTime()) ? undefined : date }Also applies to: 27-33
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/data-services/models/occurrence.ts` around lines 23 - 25, The createdAt getter currently returns new Date(this._occurrence.created_at) which can yield an invalid Date; change the getter signature to return Date | undefined, validate input (ensure this._occurrence.created_at exists and that constructed Date is valid via !isNaN(date.getTime())), and return undefined for missing/invalid values; apply the same pattern to the other similar getters (e.g., updatedAt/resolvedAt or any getters around lines 27-33) and update any callers to handle the optional Date return.ui/src/data-services/models/entity.ts-21-27 (1)
21-27:⚠️ Potential issue | 🟡 MinorGuard against
Invalid Datevalues before exposing timestamps.
Line 26andLine 46return aDateeven when parsing fails, which can propagate"Invalid Date"into table cells.Suggested fix
get createdAt(): Date | undefined { if (!this._data.created_at) { return undefined } - - return new Date(this._data.created_at) + const date = new Date(this._data.created_at) + return Number.isNaN(date.getTime()) ? undefined : date } @@ get updatedAt(): Date | undefined { if (!this._data.updated_at) { return undefined } - - return new Date(this._data.updated_at) + const date = new Date(this._data.updated_at) + return Number.isNaN(date.getTime()) ? undefined : date }Also applies to: 41-47
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/data-services/models/entity.ts` around lines 21 - 27, The createdAt (and similarly updatedAt) getters currently return new Date(this._data.created_at) even when parsing fails; update get createdAt and get updatedAt to parse the timestamp into a Date, test validity via isNaN(date.getTime()) (or date instanceof Date && !isNaN(...)) and return undefined when invalid, otherwise return the Date; reference the getters named createdAt and updatedAt and the backing field this._data.created_at / this._data.updated_at to locate the changes.ui/src/data-services/models/species.ts-39-41 (1)
39-41:⚠️ Potential issue | 🟡 MinorApply the same invalid-date guard to species timestamps.
Line 40andLine 113can returnInvalid Datewhen backend values are malformed, which leaks to UI formatting.Suggested fix
- get createdAt(): Date { - return new Date(this._species.created_at) + get createdAt(): Date | undefined { + if (!this._species.created_at) { + return undefined + } + const date = new Date(this._species.created_at) + return Number.isNaN(date.getTime()) ? undefined : date } @@ get updatedAt(): Date | undefined { if (!this._species.updated_at) { return undefined } - return new Date(this._species.updated_at) + const date = new Date(this._species.updated_at) + return Number.isNaN(date.getTime()) ? undefined : date }Also applies to: 109-114
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/data-services/models/species.ts` around lines 39 - 41, The createdAt getter (returning new Date(this._species.created_at)) and the other timestamp getters around lines 109-114 should guard against malformed backend values; parse the date into a Date object, check isNaN(date.getTime()) and if invalid return null (or undefined) instead of the Date object so the UI won't receive "Invalid Date". Update the createdAt getter and the corresponding timestamp getters (e.g., updatedAt/observedAt that reference this._species.*_at) to perform this validation and return a safe null/undefined when the parsed date is invalid.ui/src/pages/auth/login.tsx-96-103 (1)
96-103:⚠️ Potential issue | 🟡 MinorHardcoded copy here bypasses localization.
Line 96andLine 101-103introduce raw English strings, which breaks consistency for translated locales. Please move both strings toSTRINGkeys and render viatranslate(...).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/auth/login.tsx` around lines 96 - 103, The span text "Get in touch" and the trailing sentence "Please reach out and we will help you." are hardcoded; add two new keys to the STRING enum/lookup (e.g., GET_IN_TOUCH and FORGOT_PASSWORD_HELP), add their localized values, and replace the raw strings in the JSX with translate(STRING.GET_IN_TOUCH) for the <span> and translate(STRING.FORGOT_PASSWORD_HELP) for the paragraph (instead of the current inline English text after translate(STRING.FORGOT_PASSWORD)). Ensure you update any i18n resource files so translations are available.ui/src/pages/auth/sign-up.tsx-79-82 (1)
79-82:⚠️ Potential issue | 🟡 MinorRemove unused
loadingprop from Button.The
loading={isLoading}prop is not processed by nova-ui-kit's Button component (it doesn't support built-in loading state), making it unnecessary. The manualLoader2Iconrender is the correct approach and should be retained. Remove the unused prop to match the pattern used elsewhere in the codebase (e.g.,taxon-tags/tags-form.tsx).🔍 Proposed fix
-<Button type="submit" variant="success" loading={isLoading}> +<Button type="submit" variant="success"> <span>{translate(STRING.SIGN_UP)}</span> {isLoading ? <Loader2Icon className="w-4 h-4 animate-spin" /> : null} </Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/auth/sign-up.tsx` around lines 79 - 82, The Button in the sign-up form is being passed an unsupported loading prop; remove the unused loading={isLoading} from the Button element in sign-up.tsx and keep the existing manual spinner render (the Loader2Icon conditional) so the submit button uses <Button type="submit" variant="success"> with the inner <span>{translate(STRING.SIGN_UP)}</span> and the existing {isLoading ? <Loader2Icon ... /> : null} logic unchanged.ui/src/data-services/models/deployment.ts-37-42 (1)
37-42:⚠️ Potential issue | 🟡 MinorType assertion may cause runtime error if
updatedAtis undefined.The
as Dateassertion assumesupdatedAtis always defined, but if any job in_jobshas an undefinedupdatedAt, callinggetTime()will throw a runtime error.Additionally,
Array.sort()mutates the original_jobsarray. If this is unintended, consider usingtoSorted()or spreading before sorting.🔧 Proposed fix with null safety
- return this._jobs.sort((j1: Job, j2: Job) => { - const date1 = j1.updatedAt as Date - const date2 = j2.updatedAt as Date + return [...this._jobs].sort((j1: Job, j2: Job) => { + const date1 = j1.updatedAt + const date2 = j2.updatedAt - return date2.getTime() - date1.getTime() + return (date2?.getTime() ?? 0) - (date1?.getTime() ?? 0) })[0]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/data-services/models/deployment.ts` around lines 37 - 42, The sorting logic on this._jobs assumes each Job.updatedAt is a Date and mutates the array; update the code that computes the latest job so it first works on a copy (e.g., spread or toSorted) to avoid mutating this._jobs and guard against undefined updatedAt by using safe null checks or fallbacks (e.g., compute timestamps with (job.updatedAt?.getTime() ?? 0) or filter out jobs without updatedAt) before comparing; ensure the method using Job and updatedAt returns undefined or handles empty arrays if there are no valid timestamps.ui/src/pages/project/pipelines/pipelines-columns.tsx-40-43 (1)
40-43:⚠️ Potential issue | 🟡 MinorLong unbroken description text can still overflow.
whiteSpace: 'normal'won’t break long tokens. AddoverflowWrap(orwordBreak) so very long strings don’t blow out table layout.💡 Suggested tweak
style={{ width: '320px', whiteSpace: 'normal', + overflowWrap: 'anywhere', }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/project/pipelines/pipelines-columns.tsx` around lines 40 - 43, The inline style object currently set as style={{ width: '320px', whiteSpace: 'normal' }} can still allow long unbroken tokens to overflow; update that style in pipelines-columns.tsx (the same style object) to include overflowWrap: 'anywhere' or overflowWrap: 'break-word' (or alternatively wordBreak: 'break-word' / wordBreak: 'break-all' for more aggressive breaking) so very long strings wrap instead of blowing out the column layout.ui/src/design-system/components/wizard/status-bullet/status-bullet.tsx-30-30 (1)
30-30:⚠️ Potential issue | 🟡 MinorDon’t hide
0when renderingvalue.Using a truthy check drops valid numeric
0values.Suggested fix
- {value ? <span>{value}</span> : null} + {value != null ? <span>{value}</span> : null}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/design-system/components/wizard/status-bullet/status-bullet.tsx` at line 30, The current conditional in status-bullet.tsx uses a truthy check on value which hides valid numeric 0; change the check to explicitly test for null/undefined (e.g., use value != null) before rendering the <span> so that 0 is rendered. Locate the conditional that references value in the StatusBullet component and replace the truthy check with an explicit null/undefined guard.
🧹 Nitpick comments (5)
ui/src/components/cookie-dialog/cookie-dialog.module.scss (1)
48-64: Consider removing redundant mobile override.After setting the base
.actionsgap to8px(line 50), the mobile media query override (lines 62-64) is now redundant since both specify the same value. You could remove the.actionsblock from the media query for cleaner CSS.🧹 Proposed cleanup
`@media` only screen and (max-width: $small-screen-breakpoint) { .dialog { left: 0; bottom: 0; max-width: 100%; padding: 16px; border-radius: 0; } - - .actions { - gap: 8px; - } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/components/cookie-dialog/cookie-dialog.module.scss` around lines 48 - 64, Remove the redundant `.actions` rule inside the media query in cookie-dialog.module.scss: the base `.actions { gap: 8px; }` already applies the desired spacing, so delete the `.actions { gap: 8px; }` block within the `@media only screen and (max-width: $small-screen-breakpoint)` section to keep the CSS concise while leaving the `.dialog` overrides intact.ui/src/pages/deployment-details/deployment-details-form/deployment-details-form.tsx (1)
126-134: Set explicittype="button"on Save action for safety.This action is click-driven (
onClick), so settingtype="button"avoids implicit submit behavior if surrounding markup changes later.Suggested fix
<Button + type="button" disabled={!allValid || !someDirty || isLoading} onClick={onSaveClick} size="small" variant="success" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/deployment-details/deployment-details-form/deployment-details-form.tsx` around lines 126 - 134, The Save Button currently relies on onClick (onSaveClick) but lacks an explicit type, which can cause an implicit form submit if markup changes; update the Button JSX (the element using props disabled={!allValid || !someDirty || isLoading}, onClick={onSaveClick}, size="small", variant="success") to include type="button" so the click only triggers onSaveClick and never accidentally submits a surrounding form.ui/src/pages/occurrence-details/id-quick-actions/id-button.tsx (1)
38-60: Consider disabling button during loading to prevent duplicate submissions.The button is disabled only when
isSuccessbut not whenisLoading. This allows users to click repeatedly during the loading state, potentially triggering multiplecreateIdentificationscalls. If the previous implementation'sloadingprop also disabled the button, this could be a behavioral regression.🔧 Proposed fix
<Button className={classNames('justify-between', { 'text-destructive': error })} - disabled={isSuccess} + disabled={isLoading || isSuccess} onClick={() => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/occurrence-details/id-quick-actions/id-button.tsx` around lines 38 - 60, The Button is only disabled when isSuccess, so users can click while isLoading and trigger duplicate createIdentifications calls; update the disabled logic on the Button (the JSX using Button, classNames, and the onClick that calls addRecentIdentification and createIdentifications) to disable the control when either isLoading or isSuccess is true (e.g., disabled={isSuccess || isLoading}) so the button is inert during the loading state.ui/src/pages/deployment-details/deployment-details-form/section-location/location-map/location-map.tsx (1)
29-43: Consider disabling button while loading location.The button allows repeated clicks while
loadingLocationis true, which could trigger multiplemapRef.current.locate()calls. If the previous design-system button had aloadingprop that also disabled the button, this might be a behavioral regression.🔧 Proposed fix to prevent multiple clicks
<Button + disabled={loadingLocation} onClick={() => { if (mapRef.current) { mapRef.current.locate() setLoadingLocation(true) } }} size="small" variant="outline" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/deployment-details/deployment-details-form/section-location/location-map/location-map.tsx` around lines 29 - 43, The Button currently still accepts clicks while loadingLocation is true which lets mapRef.current.locate() be called multiple times; update the Button usage in location-map.tsx (the Button component where onClick calls mapRef.current.locate() and setLoadingLocation(true)) to prevent repeated clicks by passing the appropriate prop when loadingLocation is true (prefer the Button's loading prop if it exists, otherwise set disabled={loadingLocation}); ensure the UI still shows the Loader2Icon when loadingLocation is true and that onClick exits early if mapRef.current is missing.ui/src/pages/deployment-details/deployment-details-form/section-source-images/actions/sync-source-images.tsx (1)
25-37: Button should be disabled during loading to prevent duplicate sync operations.The button is not disabled while
isLoadingis true, allowing users to potentially trigger multiple sync operations by clicking repeatedly. This could cause unintended duplicate syncs.🔧 Proposed fix
<Button - disabled={!isConnected || isSuccess} + disabled={!isConnected || isLoading || isSuccess} onClick={() => syncDeploymentSourceImages(deploymentId)} size="small" variant="success" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/src/pages/deployment-details/deployment-details-form/section-source-images/actions/sync-source-images.tsx` around lines 25 - 37, The Sync button can be clicked while a sync is in progress; update the Button in sync-source-images.tsx (the JSX using <Button ... onClick={() => syncDeploymentSourceImages(deploymentId)} ...> and the isLoading / isSuccess / isConnected flags) to also disable when isLoading is true (i.e., include isLoading in the disabled condition) so users can't trigger duplicate syncs; optionally guard the click handler to no-op when isLoading to be extra safe.
🤖 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/data-services/models/capture-details.ts`:
- Around line 28-31: The comparator in currentJob unsafely casts j1.updatedAt
and j2.updatedAt to Date and calls getTime(), which can throw if updatedAt is
undefined; update the comparator used to sort currentJob so it safely handles
undefined updatedAt (e.g., compute timestamps via a guarded expression or
nullish-coalescing fallback when reading j1.updatedAt and j2.updatedAt before
calling getTime()), and apply the same defensive fix to the equivalent
comparators in capture-set.ts and deployment.ts; reference the updatedAt
property and the comparator used in currentJob to locate and patch the code.
In `@ui/src/data-services/models/capture-set.ts`:
- Around line 32-35: The comparator uses unsafe casts (j1.updatedAt as Date /
j2.updatedAt as Date) and calls getTime() which will throw if updatedAt is
undefined; change the comparator to safely get timestamps using optional
chaining and a fallback (e.g., const t1 = j1.updatedAt?.getTime() ?? 0 and const
t2 = j2.updatedAt?.getTime() ?? 0) and return t2 - t1; apply the same fix
pattern to the analogous usages in deployment.ts and capture-details.ts (look
for the comparator or any direct getTime() calls on updatedAt).
In `@ui/src/data-services/models/session.ts`:
- Around line 103-105: The createdAt getter currently returns the wrong
timestamp; update the get createdAt() accessor to return this._event.created_at
instead of this._event.updated_at so it reflects the session creation time;
locate the createdAt getter on the Session model (the get createdAt() method
that references this._event) and change the returned field to created_at.
In `@ui/src/design-system/components/image-carousel/image-carousel.tsx`:
- Around line 174-180: The carousel's icon-only navigation buttons (the Button
elements that call showPrev(slideIndex) and showNext(slideIndex) and render
ChevronLeftIcon/ChevronRightIcon) lack accessible names; add an accessible label
to each Button (e.g., aria-label="Previous slide" and aria-label="Next slide" or
include visually hidden text) so screen readers can distinguish the controls
while keeping the icons for sighted users.
In `@ui/src/design-system/components/pagination-bar/pagination-bar.tsx`:
- Around line 41-48: The icon-only pagination Buttons in PaginationBar render
without accessible names; update the Button components (the one with
<ChevronLeft /> and the one with <ChevronRight />) to include descriptive
accessible labels (e.g., aria-label or aria-labelledby like "Previous page" and
"Next page") so assistive tech can announce their purpose, keeping the existing
disabled logic (disabled={currentPage <= firstPage} / disabled={currentPage >=
lastPage}) and onClick handlers (setPage(currentPage - 1) / setPage(currentPage
+ 1}) intact.
In `@ui/src/design-system/components/popover/popover.module.scss`:
- Line 9: The popover's z-index is set to 50 in popover.module.scss which
conflicts with the dialog/modal and page-footer stacks; lower the popover
stacking context to a value below the modal (e.g., change the popover z-index
from 50 to 40 in the popover z-index rule), then ensure the dialog/modal overlay
and content (the dialog component) and page-footer use higher, distinct z-index
values (e.g., 60+ for modal overlay/content and 50 for footer) so modals always
layer above popovers.
In `@ui/src/pages/auth/reset-password.tsx`:
- Around line 72-77: The submit Button in reset-password.tsx (component Button
rendering the submit with {translate(STRING.SEND_INSTRUCTIONS)} and Loader2Icon)
is not disabled during isLoading, allowing duplicate submissions; update the
Button props to set disabled={isLoading} (and/or add aria-disabled when
appropriate) so the button becomes non-interactive while isLoading is true to
prevent repeated reset requests and preserve the existing submit behavior.
In
`@ui/src/pages/captures/upload-images-dialog/select-images-section/select-images-section.tsx`:
- Around line 49-59: The icon-only Button rendering the XIcon (and the other
icon-only Button in the same component) lacks an accessible name and an explicit
button type; update the Button elements in select-images-section.tsx (the ones
using XIcon and the other icon-only control) to include type="button" and an
accessible name (e.g., aria-label="Remove image" or aria-label appropriate for
the action) so screen readers can identify them; optionally add a title
attribute for hover tooltips, and keep the existing onClick handler
(setImages(images.filter(...))) unchanged.
In
`@ui/src/pages/deployment-details/deployment-details-form/section-general/section-general.tsx`:
- Around line 130-132: The Button inside the form lacks an explicit type which
can cause accidental form submissions; update the Button element used with
onClick={onNext} (the Button rendering the
<span>{translate(STRING.NEXT)}</span>) to include type="button" so the click
invokes onNext without submitting the enclosing form, matching other components
like processing-form.tsx.
In `@ui/src/pages/job-details/job-actions/cancel-job.tsx`:
- Around line 11-12: The cancel button currently only disables when isSuccess,
allowing multiple rapid clicks to call cancelJob(jobId) while isLoading is true;
update the button to also disable during loading (e.g., disabled={isSuccess ||
isLoading}) or add a guard inside the onClick to no-op when isLoading is true so
cancelJob(jobId) cannot be invoked while the request is pending (refer to the
isLoading flag and the cancelJob function used in the onClick handler).
In `@ui/src/pages/job-details/job-actions/queue-job.tsx`:
- Around line 11-12: The button currently only disables when isSuccess, allowing
repeated clicks while the request is in progress; change the disable logic to
include the loading state (use isLoading || isSuccess) and/or guard the click
handler by returning early if isLoading is true. Update the Button's disabled
prop and the onClick wrapper around queueJob(jobId) (referencing queueJob,
isLoading, isSuccess) so the action cannot be invoked while the call is in
flight.
In `@ui/src/pages/job-details/job-actions/retry-job.tsx`:
- Around line 10-21: The Retry button allows duplicate requests because it only
disables when isSuccess; update the click guard to also prevent clicks while
isLoading by disabling the Button when isLoading (e.g., disabled={isSuccess ||
isLoading}) and/or make the onClick handler no-op when isLoading (wrap
retryJob(jobId) behind a guard using isLoading). Modify the UI that renders
Button (the Button component and its onClick) so retryJob is not called while
isLoading to avoid duplicate retry mutations.
In `@ui/src/pages/occurrence-details/id-quick-actions/id-quick-actions.tsx`:
- Around line 71-77: The dynamic Tailwind interpolation using className={zIndex
? `z-${zIndex}` : undefined} will not be compiled by Tailwind; instead remove
the runtime class construction and apply the stacking context via an inline
style on the same element (use the existing zIndex variable to set style={{
zIndex: Number(zIndex) }} or style={{ zIndex }} ensuring correct type), keep any
other static className values separate, and remove the `z-${...}` usage so the
component (the element receiving the className prop in id-quick-actions.tsx)
uses inline styles for z-index.
In
`@ui/src/pages/session-details/playback/capture-navigation/capture-navigation.tsx`:
- Around line 89-96: The prev/next icon-only Buttons (the Button components
wrapping ChevronLeftIcon and ChevronRightIcon used with goToPrev and goToNext)
lack accessible names; add aria-label attributes (e.g., aria-label="Previous
capture" on the Button that checks activeCapture?.prevCaptureId and
aria-label="Next capture" on the Button that checks
activeCapture?.nextCaptureId) so screen readers announce the action even when
the button is icon-only; keep the existing disabled props and handlers
unchanged.
---
Outside diff comments:
In `@ui/src/components/terms-of-service-info/terms-of-service-info.tsx`:
- Around line 12-17: The cleanup effect in useEffect references userPreferences
but doesn't list it as a dependency, causing a stale closure; update the cleanup
to use a functional state update so it doesn't depend on the closed-over
userPreferences (e.g., call setUserPreferences(prev => ({ ...prev,
termsMessageSeen: true })) inside the cleanup of the useEffect in
terms-of-service-info.tsx), and ensure setUserPreferences is stable or included
in the dependency array if it's not memoized.
In `@ui/src/data-services/models/pipeline.ts`:
- Around line 63-71: The versionLabel getter is accidentally interpolating the
length of the version name instead of the version name itself; update the getter
(versionLabel) so that when this._pipeline.version_name exists you return
`${this._pipeline.version} "${this._pipeline.version_name}"` (not using
.length), otherwise return `${this._pipeline.version}`; ensure you still handle
undefined this._pipeline.version by returning undefined as currently
implemented.
---
Minor comments:
In `@ui/src/data-services/models/deployment.ts`:
- Around line 37-42: The sorting logic on this._jobs assumes each Job.updatedAt
is a Date and mutates the array; update the code that computes the latest job so
it first works on a copy (e.g., spread or toSorted) to avoid mutating this._jobs
and guard against undefined updatedAt by using safe null checks or fallbacks
(e.g., compute timestamps with (job.updatedAt?.getTime() ?? 0) or filter out
jobs without updatedAt) before comparing; ensure the method using Job and
updatedAt returns undefined or handles empty arrays if there are no valid
timestamps.
In `@ui/src/data-services/models/entity.ts`:
- Around line 21-27: The createdAt (and similarly updatedAt) getters currently
return new Date(this._data.created_at) even when parsing fails; update get
createdAt and get updatedAt to parse the timestamp into a Date, test validity
via isNaN(date.getTime()) (or date instanceof Date && !isNaN(...)) and return
undefined when invalid, otherwise return the Date; reference the getters named
createdAt and updatedAt and the backing field this._data.created_at /
this._data.updated_at to locate the changes.
In `@ui/src/data-services/models/occurrence.ts`:
- Around line 23-25: The createdAt getter currently returns new
Date(this._occurrence.created_at) which can yield an invalid Date; change the
getter signature to return Date | undefined, validate input (ensure
this._occurrence.created_at exists and that constructed Date is valid via
!isNaN(date.getTime())), and return undefined for missing/invalid values; apply
the same pattern to the other similar getters (e.g., updatedAt/resolvedAt or any
getters around lines 27-33) and update any callers to handle the optional Date
return.
In `@ui/src/data-services/models/species.ts`:
- Around line 39-41: The createdAt getter (returning new
Date(this._species.created_at)) and the other timestamp getters around lines
109-114 should guard against malformed backend values; parse the date into a
Date object, check isNaN(date.getTime()) and if invalid return null (or
undefined) instead of the Date object so the UI won't receive "Invalid Date".
Update the createdAt getter and the corresponding timestamp getters (e.g.,
updatedAt/observedAt that reference this._species.*_at) to perform this
validation and return a safe null/undefined when the parsed date is invalid.
In
`@ui/src/design-system/components/table/basic-table-cell/basic-table-cell.module.scss`:
- Line 11: Replace the deprecated CSS declaration "word-break: break-word" in
basic-table-cell.module.scss with the modern equivalent by removing or replacing
that property and adding "overflow-wrap: break-word" on the same selector (the
basic table cell rule) so long words wrap correctly; update any related rules
that rely on word-break in the same module to use overflow-wrap instead to
maintain behavior across browsers.
In `@ui/src/design-system/components/wizard/status-bullet/status-bullet.tsx`:
- Line 30: The current conditional in status-bullet.tsx uses a truthy check on
value which hides valid numeric 0; change the check to explicitly test for
null/undefined (e.g., use value != null) before rendering the <span> so that 0
is rendered. Locate the conditional that references value in the StatusBullet
component and replace the truthy check with an explicit null/undefined guard.
In `@ui/src/pages/auth/login.tsx`:
- Around line 96-103: The span text "Get in touch" and the trailing sentence
"Please reach out and we will help you." are hardcoded; add two new keys to the
STRING enum/lookup (e.g., GET_IN_TOUCH and FORGOT_PASSWORD_HELP), add their
localized values, and replace the raw strings in the JSX with
translate(STRING.GET_IN_TOUCH) for the <span> and
translate(STRING.FORGOT_PASSWORD_HELP) for the paragraph (instead of the current
inline English text after translate(STRING.FORGOT_PASSWORD)). Ensure you update
any i18n resource files so translations are available.
In `@ui/src/pages/auth/sign-up.tsx`:
- Around line 79-82: The Button in the sign-up form is being passed an
unsupported loading prop; remove the unused loading={isLoading} from the Button
element in sign-up.tsx and keep the existing manual spinner render (the
Loader2Icon conditional) so the submit button uses <Button type="submit"
variant="success"> with the inner <span>{translate(STRING.SIGN_UP)}</span> and
the existing {isLoading ? <Loader2Icon ... /> : null} logic unchanged.
In
`@ui/src/pages/deployment-details/deployment-details-form/section-location/section-location.tsx`:
- Around line 119-124: The Back and Next navigation Buttons in
section-location.tsx (handlers onBack and onNext) are missing an explicit type
and therefore default to type="submit"; update both Button usages to include
type="button" to prevent unintended form submission when clicking Back (and for
consistency for Next if it should not submit). Also apply the same change to the
corresponding Buttons in section-source-images.tsx, following the existing
convention used for non-submit buttons like the Cancel button in
deployment-details-form.tsx and the button in image-upload.tsx.
In `@ui/src/pages/project/pipelines/pipelines-columns.tsx`:
- Around line 40-43: The inline style object currently set as style={{ width:
'320px', whiteSpace: 'normal' }} can still allow long unbroken tokens to
overflow; update that style in pipelines-columns.tsx (the same style object) to
include overflowWrap: 'anywhere' or overflowWrap: 'break-word' (or alternatively
wordBreak: 'break-word' / wordBreak: 'break-all' for more aggressive breaking)
so very long strings wrap instead of blowing out the column layout.
In `@ui/src/utils/constants.ts`:
- Around line 117-119: Update the inconsistent domain in the exported constants
so both use the www form: change LANDING_PAGE_URL to 'https://www.insectai.org/'
to match LANDING_PAGE_CONTACT_URL; locate the constants LANDING_PAGE_URL and
LANDING_PAGE_CONTACT_URL in the file and make the LANDING_PAGE_URL value use the
www subdomain to avoid the 301 redirect and keep formats consistent.
---
Nitpick comments:
In `@ui/src/components/cookie-dialog/cookie-dialog.module.scss`:
- Around line 48-64: Remove the redundant `.actions` rule inside the media query
in cookie-dialog.module.scss: the base `.actions { gap: 8px; }` already applies
the desired spacing, so delete the `.actions { gap: 8px; }` block within the
`@media only screen and (max-width: $small-screen-breakpoint)` section to keep
the CSS concise while leaving the `.dialog` overrides intact.
In
`@ui/src/pages/deployment-details/deployment-details-form/deployment-details-form.tsx`:
- Around line 126-134: The Save Button currently relies on onClick (onSaveClick)
but lacks an explicit type, which can cause an implicit form submit if markup
changes; update the Button JSX (the element using props disabled={!allValid ||
!someDirty || isLoading}, onClick={onSaveClick}, size="small",
variant="success") to include type="button" so the click only triggers
onSaveClick and never accidentally submits a surrounding form.
In
`@ui/src/pages/deployment-details/deployment-details-form/section-location/location-map/location-map.tsx`:
- Around line 29-43: The Button currently still accepts clicks while
loadingLocation is true which lets mapRef.current.locate() be called multiple
times; update the Button usage in location-map.tsx (the Button component where
onClick calls mapRef.current.locate() and setLoadingLocation(true)) to prevent
repeated clicks by passing the appropriate prop when loadingLocation is true
(prefer the Button's loading prop if it exists, otherwise set
disabled={loadingLocation}); ensure the UI still shows the Loader2Icon when
loadingLocation is true and that onClick exits early if mapRef.current is
missing.
In
`@ui/src/pages/deployment-details/deployment-details-form/section-source-images/actions/sync-source-images.tsx`:
- Around line 25-37: The Sync button can be clicked while a sync is in progress;
update the Button in sync-source-images.tsx (the JSX using <Button ...
onClick={() => syncDeploymentSourceImages(deploymentId)} ...> and the isLoading
/ isSuccess / isConnected flags) to also disable when isLoading is true (i.e.,
include isLoading in the disabled condition) so users can't trigger duplicate
syncs; optionally guard the click handler to no-op when isLoading to be extra
safe.
In `@ui/src/pages/occurrence-details/id-quick-actions/id-button.tsx`:
- Around line 38-60: The Button is only disabled when isSuccess, so users can
click while isLoading and trigger duplicate createIdentifications calls; update
the disabled logic on the Button (the JSX using Button, classNames, and the
onClick that calls addRecentIdentification and createIdentifications) to disable
the control when either isLoading or isSuccess is true (e.g.,
disabled={isSuccess || isLoading}) so the button is inert during the loading
state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: aee8d2dc-92ed-40f2-834d-9f3587051e0b
⛔ Files ignored due to path filters (42)
ui/src/design-system/components/icon/assets/checkmark.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/close.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/detections.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/download.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/filters.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/gallery-view.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/identifiers.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/info.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/members.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/photograph.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/play-button.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/check.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/circle-backslash.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/clock.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/cross.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/error.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/external-link.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/heart-filled.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/heart.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/minus.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/options.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/pencil.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/plus.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/question-mark.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/search.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/toggle-down.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/toggle-left.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/toggle-right.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/trash.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/radix/update.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/settings.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/shield-check.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/sort.svgis excluded by!**/*.svgui/src/design-system/components/icon/assets/table-view.svgis excluded by!**/*.svgui/src/design-system/components/navigation/assets/captures.svgis excluded by!**/*.svgui/src/design-system/components/navigation/assets/deployments.svgis excluded by!**/*.svgui/src/design-system/components/navigation/assets/jobs.svgis excluded by!**/*.svgui/src/design-system/components/navigation/assets/occurrences.svgis excluded by!**/*.svgui/src/design-system/components/navigation/assets/project.svgis excluded by!**/*.svgui/src/design-system/components/navigation/assets/sessions.svgis excluded by!**/*.svgui/src/design-system/components/navigation/assets/taxa.svgis excluded by!**/*.svgui/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (98)
ui/package.jsonui/src/components/blueprint-collection/blueprint-collection.tsxui/src/components/cookie-dialog/cookie-dialog.module.scssui/src/components/cookie-dialog/cookie-dialog.tsxui/src/components/filtering/default-filter-control.tsxui/src/components/terms-of-service-info/terms-of-service-info.tsxui/src/data-services/models/algorithm.tsui/src/data-services/models/capture-details.tsui/src/data-services/models/capture-set.tsui/src/data-services/models/deployment.tsui/src/data-services/models/entity.tsui/src/data-services/models/job.tsui/src/data-services/models/occurrence.tsui/src/data-services/models/pipeline.tsui/src/data-services/models/processing-service.tsui/src/data-services/models/session.tsui/src/data-services/models/species.tsui/src/design-system/components/button/button.module.scssui/src/design-system/components/button/button.tsxui/src/design-system/components/card/card.tsxui/src/design-system/components/checkbox/checkbox.tsxui/src/design-system/components/combo-box/combo-box-simple/combo-box-simple.tsxui/src/design-system/components/dialog/dialog.tsxui/src/design-system/components/icon-button/icon-button.module.scssui/src/design-system/components/icon-button/icon-button.tsxui/src/design-system/components/icon/icon.module.scssui/src/design-system/components/icon/icon.tsxui/src/design-system/components/image-carousel/image-carousel.tsxui/src/design-system/components/input/input.tsxui/src/design-system/components/navigation/navigation-bar-icon.tsxui/src/design-system/components/navigation/navigation-bar.module.scssui/src/design-system/components/navigation/navigation-bar.tsxui/src/design-system/components/pagination-bar/page-button/page-button.tsxui/src/design-system/components/pagination-bar/pagination-bar.tsxui/src/design-system/components/popover/popover.module.scssui/src/design-system/components/popover/popover.tsxui/src/design-system/components/status-info/status-info.tsxui/src/design-system/components/status-info/types.tsui/src/design-system/components/table/basic-table-cell/basic-table-cell.module.scssui/src/design-system/components/table/column-settings/column-settings.tsxui/src/design-system/components/table/date-table-cell/date-table-cell.tsxui/src/design-system/components/table/table-header/table-header.tsxui/src/design-system/components/tabs/tabs.tsxui/src/design-system/components/toggle-group/toggle-group.tsxui/src/design-system/components/wizard/status-bullet/status-bullet.tsxui/src/design-system/components/wizard/wizard.tsxui/src/pages/auth/login.tsxui/src/pages/auth/reset-password.tsxui/src/pages/auth/sign-up.tsxui/src/pages/captures/captures.tsxui/src/pages/captures/upload-images-dialog/select-images-section/select-images-section.tsxui/src/pages/deployment-details/delete-deployment-dialog.tsxui/src/pages/deployment-details/deployment-details-form/deployment-details-form.tsxui/src/pages/deployment-details/deployment-details-form/section-general/section-general.tsxui/src/pages/deployment-details/deployment-details-form/section-location/location-map/location-map.tsxui/src/pages/deployment-details/deployment-details-form/section-location/section-location.tsxui/src/pages/deployment-details/deployment-details-form/section-source-images/actions/sync-source-images.tsxui/src/pages/deployment-details/deployment-details-form/section-source-images/section-source-images.tsxui/src/pages/deployment-details/deployment-details-info.tsxui/src/pages/deployments/deployment-columns.tsxui/src/pages/job-details/job-actions/cancel-job.tsxui/src/pages/job-details/job-actions/queue-job.tsxui/src/pages/job-details/job-actions/retry-job.tsxui/src/pages/job-details/job-details.tsxui/src/pages/jobs/delete-jobs-dialog.tsxui/src/pages/jobs/jobs-columns.tsxui/src/pages/jobs/jobs.tsxui/src/pages/occurrence-details/id-quick-actions/id-button.tsxui/src/pages/occurrence-details/id-quick-actions/id-quick-actions.tsxui/src/pages/occurrences/occurrence-columns.tsxui/src/pages/occurrences/occurrence-gallery.tsxui/src/pages/occurrences/occurrences.tsxui/src/pages/project/algorithms/algorithms-columns.tsxui/src/pages/project/capture-sets/capture-set-columns.tsxui/src/pages/project/entities/details-form/export-details-form.tsxui/src/pages/project/entities/entities-columns.tsxui/src/pages/project/exports/exports-columns.tsxui/src/pages/project/pipelines/pipelines-columns.tsxui/src/pages/project/processing-services/connection-status.tsxui/src/pages/project/processing-services/processing-services-columns.tsxui/src/pages/project/processing-services/status-info/status-info.module.scssui/src/pages/project/processing-services/status-info/status-info.tsxui/src/pages/project/storage/connection-status.tsxui/src/pages/project/storage/status-info/status-info.module.scssui/src/pages/project/storage/status-info/status-info.tsxui/src/pages/project/storage/status-info/types.tsui/src/pages/project/storage/storage-columns.tsxui/src/pages/project/team/team-columns.tsxui/src/pages/session-details/playback/capture-details/capture-job/capture-job-dialog.tsxui/src/pages/session-details/playback/capture-navigation/capture-navigation.tsxui/src/pages/session-details/playback/playback.tsxui/src/pages/sessions/session-columns.tsxui/src/pages/sessions/sessions.tsxui/src/pages/species/species-columns.tsxui/src/pages/species/species.tsxui/src/pages/taxa-lists/taxa-list-columns.tsxui/src/utils/constants.tsui/src/utils/useNavItems.ts
💤 Files with no reviewable changes (17)
- ui/src/pages/project/storage/status-info/types.ts
- ui/src/pages/jobs/jobs.tsx
- ui/src/design-system/components/icon-button/icon-button.module.scss
- ui/src/pages/project/storage/status-info/status-info.module.scss
- ui/src/pages/deployment-details/delete-deployment-dialog.tsx
- ui/src/pages/project/processing-services/status-info/status-info.module.scss
- ui/src/design-system/components/icon/icon.module.scss
- ui/src/components/blueprint-collection/blueprint-collection.tsx
- ui/src/pages/jobs/delete-jobs-dialog.tsx
- ui/src/utils/useNavItems.ts
- ui/src/data-services/models/processing-service.ts
- ui/src/design-system/components/button/button.module.scss
- ui/src/design-system/components/button/button.tsx
- ui/src/pages/project/processing-services/status-info/status-info.tsx
- ui/src/pages/project/storage/status-info/status-info.tsx
- ui/src/design-system/components/icon-button/icon-button.tsx
- ui/src/design-system/components/icon/icon.tsx
Comment on lines
+28
to
31
| const date1 = j1.updatedAt as Date | ||
| const date2 = j2.updatedAt as Date | ||
|
|
||
| return date2.getTime() - date1.getTime() |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify updatedAt contract and locate similar unsafe casts.
rg -nP --type=ts -C2 '\bget\s+updatedAt\(\)\s*:\s*Date\s*\|\s*undefined\b'
rg -nP --type=ts -C2 '\bupdatedAt\s+as\s+Date\b'Repository: RolnickLab/antenna
Length of output: 2738
Fix unsafe updatedAt as Date cast in currentJob sort comparator.
The updatedAt property returns Date | undefined per the model contract, but lines 28–29 unsafely cast it to Date. Calling .getTime() on undefined throws a runtime error and breaks currentJob.
This same pattern appears in capture-set.ts (lines 32–33) and deployment.ts (lines 38–39); all three require the same fix.
💡 Suggested fix
- return this._jobs.sort((j1: Job, j2: Job) => {
- const date1 = j1.updatedAt as Date
- const date2 = j2.updatedAt as Date
-
- return date2.getTime() - date1.getTime()
- })[0]
+ return [...this._jobs].sort((j1: Job, j2: Job) => {
+ const t1 = j1.updatedAt?.getTime() ?? -Infinity
+ const t2 = j2.updatedAt?.getTime() ?? -Infinity
+ return t2 - t1
+ })[0]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/data-services/models/capture-details.ts` around lines 28 - 31, The
comparator in currentJob unsafely casts j1.updatedAt and j2.updatedAt to Date
and calls getTime(), which can throw if updatedAt is undefined; update the
comparator used to sort currentJob so it safely handles undefined updatedAt
(e.g., compute timestamps via a guarded expression or nullish-coalescing
fallback when reading j1.updatedAt and j2.updatedAt before calling getTime()),
and apply the same defensive fix to the equivalent comparators in capture-set.ts
and deployment.ts; reference the updatedAt property and the comparator used in
currentJob to locate and patch the code.
Comment on lines
+103
to
105
| get createdAt(): Date { | ||
| return new Date(this._event.updated_at) | ||
| } |
Contributor
There was a problem hiding this comment.
Bug: createdAt getter uses wrong field.
The createdAt getter returns this._event.updated_at instead of this._event.created_at. This will return the wrong timestamp for when the session was created.
🐛 Proposed fix
get createdAt(): Date {
- return new Date(this._event.updated_at)
+ return new Date(this._event.created_at)
}📝 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
| get createdAt(): Date { | |
| return new Date(this._event.updated_at) | |
| } | |
| get createdAt(): Date { | |
| return new Date(this._event.created_at) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/data-services/models/session.ts` around lines 103 - 105, The createdAt
getter currently returns the wrong timestamp; update the get createdAt()
accessor to return this._event.created_at instead of this._event.updated_at so
it reflects the session creation time; locate the createdAt getter on the
Session model (the get createdAt() method that references this._event) and
change the returned field to created_at.
Comment on lines
+174
to
+180
| <Button | ||
| onClick={() => showPrev(slideIndex)} | ||
| /> | ||
| size="icon" | ||
| variant="success" | ||
| > | ||
| <ChevronLeftIcon className="w-4 h-4" /> | ||
| </Button> |
Contributor
There was a problem hiding this comment.
Add accessible labels to icon-only navigation buttons.
Both carousel controls are icon-only and currently have no accessible name, so assistive tech users cannot identify “previous” vs “next”.
Suggested fix
<Button
+ aria-label="Previous image"
onClick={() => showPrev(slideIndex)}
size="icon"
variant="success"
>
<ChevronLeftIcon className="w-4 h-4" />
</Button>
@@
<Button
+ aria-label="Next image"
onClick={() => showNext(slideIndex)}
size="icon"
variant="success"
>
<ChevronRightIcon className="w-4 h-4" />
</Button>Also applies to: 218-224
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/design-system/components/image-carousel/image-carousel.tsx` around
lines 174 - 180, The carousel's icon-only navigation buttons (the Button
elements that call showPrev(slideIndex) and showNext(slideIndex) and render
ChevronLeftIcon/ChevronRightIcon) lack accessible names; add an accessible label
to each Button (e.g., aria-label="Previous slide" and aria-label="Next slide" or
include visually hidden text) so screen readers can distinguish the controls
while keeping the icons for sighted users.
Comment on lines
+41
to
+48
| <Button | ||
| disabled={currentPage <= firstPage} | ||
| icon={IconType.ToggleLeft} | ||
| shape={IconButtonShape.RoundLarge} | ||
| onClick={() => setPage(currentPage - 1)} | ||
| /> | ||
| size="icon" | ||
| variant="outline" | ||
| > | ||
| <ChevronLeft className="w-4 h-4" /> | ||
| </Button> |
Contributor
There was a problem hiding this comment.
Icon-only pagination buttons need accessible names.
Line 41 and Line 75 render controls without visible text or aria-label, so assistive technologies cannot announce previous/next page actions.
💡 Suggested fix
<Button
+ aria-label="Previous page"
disabled={currentPage <= firstPage}
onClick={() => setPage(currentPage - 1)}
size="icon"
variant="outline"
>
- <ChevronLeft className="w-4 h-4" />
+ <ChevronLeft aria-hidden className="w-4 h-4" />
</Button>
@@
<Button
+ aria-label="Next page"
disabled={currentPage >= lastPage}
onClick={() => setPage(currentPage + 1)}
size="icon"
variant="outline"
>
- <ChevronRight className="w-4 h-4" />
+ <ChevronRight aria-hidden className="w-4 h-4" />
</Button>Also applies to: 75-82
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/design-system/components/pagination-bar/pagination-bar.tsx` around
lines 41 - 48, The icon-only pagination Buttons in PaginationBar render without
accessible names; update the Button components (the one with <ChevronLeft /> and
the one with <ChevronRight />) to include descriptive accessible labels (e.g.,
aria-label or aria-labelledby like "Previous page" and "Next page") so assistive
tech can announce their purpose, keeping the existing disabled logic
(disabled={currentPage <= firstPage} / disabled={currentPage >= lastPage}) and
onClick handlers (setPage(currentPage - 1) / setPage(currentPage + 1}) intact.
Comment on lines
+11
to
12
| disabled={isSuccess} | ||
| onClick={() => cancelJob(jobId)} |
Contributor
There was a problem hiding this comment.
Disable during loading to prevent duplicate cancel requests.
The button stays clickable while isLoading is true, so cancelJob(jobId) can be fired multiple times from rapid clicks.
Suggested fix
- disabled={isSuccess}
+ disabled={isSuccess || isLoading}📝 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
| disabled={isSuccess} | |
| onClick={() => cancelJob(jobId)} | |
| disabled={isSuccess || isLoading} | |
| onClick={() => cancelJob(jobId)} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/pages/job-details/job-actions/cancel-job.tsx` around lines 11 - 12,
The cancel button currently only disables when isSuccess, allowing multiple
rapid clicks to call cancelJob(jobId) while isLoading is true; update the button
to also disable during loading (e.g., disabled={isSuccess || isLoading}) or add
a guard inside the onClick to no-op when isLoading is true so cancelJob(jobId)
cannot be invoked while the request is pending (refer to the isLoading flag and
the cancelJob function used in the onClick handler).
Comment on lines
+11
to
12
| disabled={isSuccess} | ||
| onClick={() => queueJob(jobId)} |
Contributor
There was a problem hiding this comment.
Disable during loading to avoid repeated queue actions.
queueJob(jobId) can be invoked multiple times while isLoading is true because the button is only disabled on success.
Suggested fix
- disabled={isSuccess}
+ disabled={isSuccess || isLoading}📝 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
| disabled={isSuccess} | |
| onClick={() => queueJob(jobId)} | |
| disabled={isSuccess || isLoading} | |
| onClick={() => queueJob(jobId)} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/pages/job-details/job-actions/queue-job.tsx` around lines 11 - 12, The
button currently only disables when isSuccess, allowing repeated clicks while
the request is in progress; change the disable logic to include the loading
state (use isLoading || isSuccess) and/or guard the click handler by returning
early if isLoading is true. Update the Button's disabled prop and the onClick
wrapper around queueJob(jobId) (referencing queueJob, isLoading, isSuccess) so
the action cannot be invoked while the call is in flight.
Comment on lines
10
to
+21
| <Button | ||
| label={translate(STRING.RETRY)} | ||
| loading={isLoading} | ||
| theme={ButtonTheme.Neutral} | ||
| disabled={isSuccess} | ||
| onClick={() => retryJob(jobId)} | ||
| /> | ||
| size="small" | ||
| variant="outline" | ||
| > | ||
| <span>{translate(STRING.RETRY)}</span> | ||
| {isSuccess ? ( | ||
| <CheckIcon className="w-4 h-4" /> | ||
| ) : isLoading ? ( | ||
| <Loader2Icon className="w-4 h-4 animate-spin" /> | ||
| ) : null} |
Contributor
There was a problem hiding this comment.
Prevent duplicate retry requests while loading.
The button remains clickable during isLoading, so rapid clicks can trigger repeated retry mutations.
🔒 Suggested fix
<Button
- disabled={isSuccess}
+ disabled={isSuccess || isLoading}
onClick={() => retryJob(jobId)}
size="small"
variant="outline"
>📝 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
| <Button | |
| label={translate(STRING.RETRY)} | |
| loading={isLoading} | |
| theme={ButtonTheme.Neutral} | |
| disabled={isSuccess} | |
| onClick={() => retryJob(jobId)} | |
| /> | |
| size="small" | |
| variant="outline" | |
| > | |
| <span>{translate(STRING.RETRY)}</span> | |
| {isSuccess ? ( | |
| <CheckIcon className="w-4 h-4" /> | |
| ) : isLoading ? ( | |
| <Loader2Icon className="w-4 h-4 animate-spin" /> | |
| ) : null} | |
| <Button | |
| disabled={isSuccess || isLoading} | |
| onClick={() => retryJob(jobId)} | |
| size="small" | |
| variant="outline" | |
| > | |
| <span>{translate(STRING.RETRY)}</span> | |
| {isSuccess ? ( | |
| <CheckIcon className="w-4 h-4" /> | |
| ) : isLoading ? ( | |
| <Loader2Icon className="w-4 h-4 animate-spin" /> | |
| ) : null} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/pages/job-details/job-actions/retry-job.tsx` around lines 10 - 21, The
Retry button allows duplicate requests because it only disables when isSuccess;
update the click guard to also prevent clicks while isLoading by disabling the
Button when isLoading (e.g., disabled={isSuccess || isLoading}) and/or make the
onClick handler no-op when isLoading (wrap retryJob(jobId) behind a guard using
isLoading). Modify the UI that renders Button (the Button component and its
onClick) so retryJob is not called while isLoading to avoid duplicate retry
mutations.
Comment on lines
71
to
77
| align="start" | ||
| side="right" | ||
| ariaCloselabel={translate(STRING.CLOSE)} | ||
| className={zIndex ? `z-${zIndex}` : undefined} | ||
| container={containerRef?.current ?? undefined} | ||
| disableOutsideClose | ||
| style={{ zIndex }} | ||
| side="right" | ||
| > |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Find dynamic className interpolations:"
rg -n --type=tsx 'className=\{[^}]*`[^`]*\$\{[^}]+\}[^`]*`' ui/src
echo
echo "2) Find Tailwind config and check safelist/z-index hints:"
fd -HI 'tailwind.config.*' . --exec sh -c '
echo "== $1 ==";
rg -n "safelist|z-" "$1" || true
' sh {}Repository: RolnickLab/antenna
Length of output: 148
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== 1) Inspect the actual file at Lines 71-77 ==="
if [ -f "ui/src/pages/occurrence-details/id-quick-actions/id-quick-actions.tsx" ]; then
echo "File exists. Reading lines 65-85:"
sed -n '65,85p' ui/src/pages/occurrence-details/id-quick-actions/id-quick-actions.tsx | cat -n
else
echo "File not found at specified path. Searching for similar files:"
find . -name "*quick-actions*" -type f 2>/dev/null | head -20
fi
echo
echo "=== 2) Find Tailwind config in the repository ==="
find . -maxdepth 3 -name "tailwind.config.*" -o -name "tailwind.config.ts" -o -name "tailwind.config.js" 2>/dev/null
echo
echo "=== 3) Search for dynamic className patterns with z- in TSX/TS files ==="
rg -n 'className.*\$\{.*z-' --type ts --type js ui/src 2>/dev/null || echo "No matches found"
echo
echo "=== 4) Check for zIndex prop usage patterns ==="
rg -n 'zIndex\s*\?' --type ts --type js ui/src | head -30Repository: RolnickLab/antenna
Length of output: 1549
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Tailwind config content ==="
cat ./ui/tailwind.config.js
echo
echo "=== Check for safelist configuration ==="
rg -n "safelist|z-" ./ui/tailwind.config.js || echo "No safelist or z- config found"
echo
echo "=== Check Tailwind version and JIT mode ==="
if [ -f "ui/package.json" ]; then
echo "Looking for tailwindcss in package.json:"
rg '"tailwindcss"' ui/package.json
fiRepository: RolnickLab/antenna
Length of output: 836
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check if z-index utilities are defined in theme ==="
cat ./ui/tailwind.config.js | grep -A 20 "extend:"
echo
echo "=== Check node_modules for Tailwind defaults ==="
if [ -d "ui/node_modules/tailwindcss" ]; then
echo "Checking Tailwind's default z-index configuration:"
find ui/node_modules/tailwindcss -name "*.js" -o -name "*.ts" | head -1 | xargs grep -l "zIndex" 2>/dev/null || echo "Not found in quick search"
fi
echo
echo "=== Verify the issue: show zIndex prop usage context ==="
sed -n '15,25p' ui/src/pages/occurrence-details/id-quick-actions/id-quick-actions.tsx | cat -n
</parameter>Repository: RolnickLab/antenna
Length of output: 718
Avoid dynamic Tailwind z-index class interpolation on line 73.
className={zIndex ? \z-${zIndex}` : undefined}` relies on runtime class interpolation that Tailwind's JIT compiler cannot extract, causing z-index styles to silently fail in production builds. Use an inline style instead.
Suggested fix
<Popover.Content
align="start"
ariaCloselabel={translate(STRING.CLOSE)}
- className={zIndex ? `z-${zIndex}` : undefined}
+ style={zIndex ? { zIndex } : undefined}
container={containerRef?.current ?? undefined}
disableOutsideClose
side="right"
>📝 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
| align="start" | |
| side="right" | |
| ariaCloselabel={translate(STRING.CLOSE)} | |
| className={zIndex ? `z-${zIndex}` : undefined} | |
| container={containerRef?.current ?? undefined} | |
| disableOutsideClose | |
| style={{ zIndex }} | |
| side="right" | |
| > | |
| <Popover.Content | |
| align="start" | |
| ariaCloselabel={translate(STRING.CLOSE)} | |
| style={zIndex ? { zIndex } : undefined} | |
| container={containerRef?.current ?? undefined} | |
| disableOutsideClose | |
| side="right" | |
| > |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@ui/src/pages/occurrence-details/id-quick-actions/id-quick-actions.tsx` around
lines 71 - 77, The dynamic Tailwind interpolation using className={zIndex ?
`z-${zIndex}` : undefined} will not be compiled by Tailwind; instead remove the
runtime class construction and apply the stacking context via an inline style on
the same element (use the existing zIndex variable to set style={{ zIndex:
Number(zIndex) }} or style={{ zIndex }} ensuring correct type), keep any other
static className values separate, and remove the `z-${...}` usage so the
component (the element receiving the className prop in id-quick-actions.tsx)
uses inline styles for z-index.
Comment on lines
+89
to
+96
| <Button | ||
| disabled={!activeCapture?.prevCaptureId} | ||
| onClick={goToPrev} | ||
| /> | ||
| size="icon" | ||
| variant="outline" | ||
| > | ||
| <ChevronLeftIcon className="w-4 h-4" /> | ||
| </Button> |
Contributor
There was a problem hiding this comment.
Add accessible labels to icon-only navigation buttons.
Line 89 and Line 102 render icon-only controls without an accessible name. Add aria-label so assistive tech can announce previous/next actions.
💡 Suggested fix
<Button
+ aria-label="Previous capture"
disabled={!activeCapture?.prevCaptureId}
onClick={goToPrev}
size="icon"
variant="outline"
>
- <ChevronLeftIcon className="w-4 h-4" />
+ <ChevronLeftIcon aria-hidden className="w-4 h-4" />
</Button>
@@
<Button
+ aria-label="Next capture"
disabled={!activeCapture?.nextCaptureId}
onClick={goToNext}
size="icon"
variant="outline"
>
- <ChevronRightIcon className="w-4 h-4" />
+ <ChevronRightIcon aria-hidden className="w-4 h-4" />
</Button>Also applies to: 102-109
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@ui/src/pages/session-details/playback/capture-navigation/capture-navigation.tsx`
around lines 89 - 96, The prev/next icon-only Buttons (the Button components
wrapping ChevronLeftIcon and ChevronRightIcon used with goToPrev and goToNext)
lack accessible names; add aria-label attributes (e.g., aria-label="Previous
capture" on the Button that checks activeCapture?.prevCaptureId and
aria-label="Next capture" on the Button that checks
activeCapture?.nextCaptureId) so screen readers announce the action even when
the button is icon-only; keep the existing disabled props and handlers
unchanged.
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
1 participant
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 of changes
Summary by CodeRabbit
New Features
Refactor
Chores