Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughEnable TypeScript strict mode and apply widespread null-safety, stronger typings, and defensive guards across pages, components, hooks, server utilities, types, and tests to reduce runtime null/undefined errors. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
5 issues found across 76 files
Confidence score: 2/5
- High-risk issue:
frontend/src/wrappers/provider.tsxreturns only{children}whenapolloClientis missing, bypassing all providers and leading to runtime crashes in components that rely on those contexts. - Multiple concrete functional regressions are possible, including null GitHub URL construction in
frontend/src/types/release.ts/frontend/src/components/Release.tsxand invalid empty-date submissions infrontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx, which lowers merge confidence. - Pay close attention to
frontend/src/wrappers/provider.tsx,frontend/src/types/release.ts,frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx,frontend/src/utils/structuredData.ts- context bypass, null URL fields, empty date normalization, and structured data validity.
Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="frontend/src/types/release.ts">
<violation number="1" location="frontend/src/types/release.ts:13">
P1: Changing `repositoryName` and `organizationName` to allow `null` exposes a functional bug in `frontend/src/components/Release.tsx`. The component uses these fields in a template literal to construct a GitHub URL (`.../${release.repositoryName}/...`). Since TypeScript allows `null` in template literals (producing the string "null"), this code compiles but will generate broken links at runtime when the data is null.
You must update `frontend/src/components/Release.tsx` (and any other consumers) to handle these null values, either by conditionally rendering the link or providing a valid fallback.</violation>
</file>
<file name="frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx">
<violation number="1" location="frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx:102">
P2: Submitting an empty string for endedAt can violate the GraphQL date scalar when no end date is set. Normalize empty strings back to null as before.</violation>
<violation number="2" location="frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx:111">
P2: Submitting an empty string for startedAt can violate the GraphQL date scalar when no start date is set. Normalize empty strings back to null as before.</violation>
</file>
<file name="frontend/src/utils/structuredData.ts">
<violation number="1" location="frontend/src/utils/structuredData.ts:66">
P2: Avoid emitting `sameAs` with an empty string; omit the property when `user.url` is missing to prevent invalid structured data URLs.</violation>
</file>
<file name="frontend/src/wrappers/provider.tsx">
<violation number="1" location="frontend/src/wrappers/provider.tsx:25">
P0: Returning `<>{children}</>` when `apolloClient` is missing bypasses all context providers (`SessionProvider`, `HeroUIProvider`, `NextThemesProvider`, etc.). This causes runtime crashes in components (like `Header` and `UserMenu`) that rely on these contexts (e.g., `useSession`, `useTheme`, `useMutation`).
If the Apollo client fails to initialize, the application is misconfigured and should fail gracefully rather than rendering the app tree without its required environment.
(Based on your team's feedback about ensuring AI-generated changes are reviewed and tested.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx
Outdated
Show resolved
Hide resolved
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
frontend/src/wrappers/provider.tsx (1)
25-37:⚠️ Potential issue | 🟠 MajorKeep non-Apollo providers mounted even when
apolloClientis falsy.The early return drops
SessionProvider, theming, toasts, andAppInitializerentirely. IfapolloClientis missing, downstream components can lose session context, theme defaults, and Django session sync. Safer: keep all providers and only conditionally wrap the Apollo layer.🔧 Suggested adjustment
- if (!apolloClient) { - return <>{children}</> - } - return ( <Suspense> <SessionProvider> <HeroUIProvider> <NextThemesProvider attribute="class" defaultTheme="dark"> <ToastProvider /> - <ApolloProvider client={apolloClient}> - <AppInitializer /> - {children} - </ApolloProvider> + {apolloClient ? ( + <ApolloProvider client={apolloClient}> + <AppInitializer /> + {children} + </ApolloProvider> + ) : ( + <> + <AppInitializer /> + {children} + </> + )} </NextThemesProvider> </HeroUIProvider> </SessionProvider> </Suspense> )frontend/src/server/fetchCsrfTokenServer.ts (1)
13-15:⚠️ Potential issue | 🟡 MinorMissing validation for
csrftokenin response.The client-side counterpart (
fetchCsrfToken.ts) validates thatcsrftokenexists in the response before returning it. This server version lacks that check, which could cause the function to returnundefinedwhile the return type promisesstring.For strict mode type safety, add validation similar to the client version:
🛡️ Proposed fix
const data = await response.json() + if (!data?.csrftoken) { + throw new Error('CSRF token missing in response') + } + return data.csrftokenfrontend/src/components/ModuleForm.tsx (1)
401-415:⚠️ Potential issue | 🟠 MajorDead code: the "Selection cleared" branch is unreachable.
The early return on Line 402 when
keys === nullprevents the else branch (Lines 410-414) from ever executing. When a user clears the selection in the Autocomplete,onSelectionChangeis called withnull, which triggers the early return instead of reaching the clearing logic. This means the input value and project change handler are never reset when the selection is cleared.To fix this, remove the early return and handle
nullin the else branch:🐛 Proposed fix
const handleSelectionChange = (keys: React.Key | null) => { - if (keys === null) return - const selectedKey = keys as string - if (selectedKey) { + if (keys !== null) { + const selectedKey = keys as string const selectedProject = items.find((item) => item.id === selectedKey) if (selectedProject) { setInputValue(selectedProject.name) onProjectChange(selectedProject.id, selectedProject.name) } } else { // Selection cleared setInputValue('') onProjectChange(null, '') } }frontend/src/app/board/[year]/candidates/page.tsx (2)
147-164:⚠️ Potential issue | 🟡 MinorURL may contain
undefinedifmemberis null.The
candidate.member?.loginexpression returnsundefinedwhenmemberis null, resulting in a malformed URL like?author=undefined. While the data flow suggestssnapshotonly exists whenmember.loginis truthy (query skipped at line 232), TypeScript can't verify this invariant.Consider adding a fallback or guard:
Suggested fix
const renderRepositoryLink = (repoName: string, count: number) => { const commitCount = Number(count) + const authorLogin = candidate.member?.login return ( <a key={repoName} - href={`https://github.com/${repoName}/commits?author=${candidate.member?.login}`} + href={`https://github.com/${repoName}/commits${authorLogin ? `?author=${authorLogin}` : ''}`} target="_blank"
595-606:⚠️ Potential issue | 🟡 MinorSame issue: URL may contain
undefinedfor author parameter.This instance has the same issue as
renderRepositoryLink—ifcandidate.memberis null, the URL will include?author=undefined.Suggested fix
<a - href={`https://github.com/${topRepoName}/commits?author=${candidate.member?.login}`} + href={`https://github.com/${topRepoName}/commits${candidate.member?.login ? `?author=${candidate.member.login}` : ''}`} target="_blank"frontend/src/components/Release.tsx (1)
71-78:⚠️ Potential issue | 🟡 MinorPotential malformed URL when
organizationNameorrepositoryNameis undefined.Line 73 constructs a GitHub URL using
release.organizationNameandrelease.repositoryNamewithout guards. If either isundefined, the URL becomes malformed (e.g.,https://github.com/undefined/undefined/releases/tag/v1.0).Consider conditionally rendering this link or providing fallback behavior:
🛡️ Suggested guard
- <Link - className="text-blue-400 hover:underline" - href={`https://github.com/${release.organizationName}/${release.repositoryName}/releases/tag/${release.tagName}`} - target="_blank" - rel="noopener noreferrer" - > - <TruncatedText text={release.name || release.tagName} /> - </Link> + {release.organizationName && release.repositoryName ? ( + <Link + className="text-blue-400 hover:underline" + href={`https://github.com/${release.organizationName}/${release.repositoryName}/releases/tag/${release.tagName}`} + target="_blank" + rel="noopener noreferrer" + > + <TruncatedText text={release.name || release.tagName} /> + </Link> + ) : ( + <TruncatedText text={release.name || release.tagName} /> + )}frontend/src/components/RepositoryCard.tsx (1)
43-47:⚠️ Potential issue | 🟡 Minor**Navigation to invalid route when
organizationis null/undefined.**Iforganization?.loginisundefined, the URL becomes/organizations/undefined/repositories/..., which will navigate to an invalid route. Consider adding a guard or returning early when organization data is missing.🛡️ Suggested guard
const RepositoryItem = ({ details }: { details: RepositoryCardProps }) => { const router = useRouter() const handleClick = () => { + if (!details.organization?.login || !details.key) return router.push(`/organizations/${details.organization?.login}/repositories/${details.key}`) }frontend/src/app/my/mentorship/programs/[programKey]/modules/create/page.tsx (1)
98-111:⚠️ Potential issue | 🟠 MajorAvoid sending empty string for optional
endedAt.If
endedAtis left blank, sending an empty string may fail GraphQL input validation. Prefer omitting or nulling the field when empty to preserve previous behavior.✅ Suggested fix
- endedAt: formData.endedAt, + endedAt: formData.endedAt || null,frontend/src/components/MultiSearch.tsx (1)
108-109:⚠️ Potential issue | 🟠 MajorUse
logininstead ofkeyfor user routing.Line 109 uses
suggestion.key, but all other components in the codebase route users via/members/${login}(Release.tsx, ModuleCard.tsx, Leaders.tsx, ItemCardList.tsx, ContributorAvatar.tsx, CardDetailsPage.tsx). Additionally, theUsertype hasloginas a required property andkeyas optional, so usingkeyrisks routing to/members/undefined. Change torouter.push(/members/${suggestion.login}).
🤖 Fix all issues with AI agents
In `@frontend/src/app/api/auth/`[...nextauth]/route.ts:
- Around line 90-92: The current assignment in the update branch uses a non-null
assertion on (session as ExtendedSession).user! which can throw if session.user
is absent; change it to safely read the optional user with optional chaining and
a fallback (e.g., use (session as ExtendedSession).user?.isOwaspStaff ?? false)
so token.isOwaspStaff is always a boolean; update the assignment inside the if
(trigger === 'update' && session) block referencing token, session and
ExtendedSession accordingly.
- Around line 47-52: The code uses process.env with non-null assertions inside
the IS_GITHUB_AUTH_ENABLED block; replace those with the pre-validated constants
exported from env.server.ts (GITHUB_CLIENT_ID and GITHUB_CLIENT_SECRET) when
constructing the GitHubProvider in the GitHubProvider({...}) call, or
alternatively read and validate them once locally before pushing the provider;
update the import to bring in GITHUB_CLIENT_ID and GITHUB_CLIENT_SECRET and
remove the process.env...! usages to ensure type-safe, pre-validated values with
the existing IS_GITHUB_AUTH_ENABLED guard.
In `@frontend/src/app/committees/page.tsx`:
- Around line 40-41: The code unsafely uses the optional property committee.key
with non-null assertions in key={committee.objectID ?? committee.key!},
cardKey={committee.objectID ?? committee.key!},
url={`/committees/${committee.key}`} and
router.push(`/committees/${committee.key}`); fix by either making Committee.key
required in the type (update the Committee interface/type to remove the ?), or
add runtime guards: compute a safeId = committee.objectID ?? committee.key ??
/*fallback e.g. throw or skip render*/ and use safeId for key/cardKey, url and
router.push, and ensure components skip rendering or show an error if safeId is
missing; update references to use safeId instead of committee.key and remove
non-null assertions.
In `@frontend/src/app/my/mentorship/programs/`[programKey]/page.tsx:
- Line 116: The current cast on setStatus loses the ProgramStatusEnum constraint
and may send invalid statuses; update the code so setStatus is strongly typed to
ProgramStatusEnum instead of casting: change DetailsCard's prop type (or make it
generic) so its setStatus signature accepts (newStatus: ProgramStatusEnum) =>
void and remove the unsafe cast where setStatus={updateStatus as (newStatus:
string) => void}; alternatively (or additionally) add runtime validation inside
updateStatus (the function that calls updateProgram) to verify the incoming
value is a member of ProgramStatusEnum before calling the updateProgram mutation
and handle invalid values (throw/return/log) so the backend never receives
arbitrary strings.
In `@frontend/src/app/organizations/`[organizationKey]/page.tsx:
- Around line 118-122: The current casts like recentReleases={recentReleases as
unknown as Release[]} and similar for recentIssues, recentMilestones,
recentPullRequests, and repositories hide GraphQL/local shape mismatches;
replace these casts with explicit mapping/validation functions that convert the
GraphQL result shapes into your local types (e.g., mapRelease/releaseFromGraphQL
to convert publishedAt from any|null to a safe number or undefined/default,
mapAuthor to transform GraphQL UserNode into your local User shape, and
mapRepository to ensure organization has {id, login} mapped into your local
Organization type), and use type guards to assert validity before passing props
to the component so runtime errors are avoided instead of using as unknown as.
In `@frontend/src/app/projects/page.tsx`:
- Line 52: The React key currently uses project.key which is optional on the
Project type and may be undefined; update the key prop at the location with
key={project.key} to use a stable non-optional fallback (for example:
key={project.key ?? project.id ?? project.slug}) or coerce/assert that
project.key is present (and update the Project type if you choose to make it
required); alternatively, enforce upstream filtering so only projects with a
defined key are passed into the list. Ensure the chosen fallback is stable and
unique across projects to avoid React list reconciliation issues.
In `@frontend/src/components/InfoBlock.tsx`:
- Around line 13-14: The prop type for value in the InfoBlock component is
inconsistent with its default — either make value optional in the props type
(change value: number to value?: number) if you intend to rely on the default
value = 0, or remove the default assignment (value = 0) so value remains
required; update the Props/interface (where value is declared) and any other
default destructuring occurrences (also referenced around the other instance at
the second occurrence you flagged) so the type and default behavior match.
In `@frontend/src/components/ItemCardList.tsx`:
- Line 145: The double-cast bypasses TypeScript strictness; update the
ItemCardList component props so the renderDetails callback accepts ItemCardData
(the union type of Issue | Milestone | PullRequest | Release) instead of using a
Parameters<> hack, then call renderDetails(item) directly (no "as unknown as
...") inside ItemCardList; adjust the prop/interface that declares renderDetails
(e.g., ItemCardListProps or the renderDetails prop) to use (item: ItemCardData)
=> JSX.Element so consumers narrow the union within their implementations.
- Around line 95-107: The key-generation inside ItemCardList.tsx (the getItemKey
function used in data.map) can return duplicates or empty strings for
Milestone/Release-like items; update getItemKey (or its callers) to produce a
guaranteed non-empty, unique key by including a stable fallback such as a
type-specific prefix and the map index or an internal unique id (e.g.,
`${item.type || 'item'}-${index}`) when objectID, repositoryName, title, name,
and url are missing; ensure the returned key is always a string and never empty
to avoid React key collisions.
- Around line 130-141: The Link wrapper currently receives an empty href when
item.url is missing, causing Next.js router errors; update the JSX around the
Link/TruncatedText so that you only render the Next.js Link component when a
truthy URL exists (check ('url' in item ? item.url : '') or item.url), otherwise
render the TruncatedText directly (no Link) and keep the same
className/target/rel behavior only inside the Link branch; locate the block
using Link and TruncatedText in ItemCardList and make the conditional render
based on the URL presence.
In `@frontend/src/types/release.ts`:
- Line 13: The field repositoryName was made optional/nullable, so update all
unsafe accesses to guard against null/undefined: in places rendering
`{issue.organizationName}/{issue.repositoryName}` (where issue.repositoryName
may be null) and where template literals use `${release.repositoryName}`, add
null checks or provide a safe fallback (e.g., conditional rendering, optional
chaining with a default like '' or 'Unknown repository') so the UI doesn't
attempt to render null; locate usages by the symbols issue.repositoryName and
release.repositoryName and wrap them in a null-safe expression or conditional
before rendering.
In `@frontend/src/utils/structuredData.ts`:
- Line 66: The structuredData generation currently sets sameAs to [''] when
user.url is falsy (sameAs: [user.url ?? '']), which produces an invalid
empty-string URL; change the logic in the structured data builder (the sameAs
assignment that references user.url) to only include the URL when present (e.g.
use a conditional to set sameAs to [user.url] when user.url is truthy, otherwise
set sameAs to an empty array or omit the property entirely) so no empty-string
values are emitted.
🧹 Nitpick comments (16)
frontend/src/utils/metaconfig.ts (1)
75-83: Avoid theascast onconfig.typeto preserve strict safety.The cast on Line 82 bypasses compiler guarantees; a typo or undefined
typeinMETADATA_CONFIGwould still compile. Consider a small guard to keep strictness intact.♻️ Suggested refactor (narrow with a guard)
const config = METADATA_CONFIG[pageKey] + const ogType: SeoMetadataProps['type'] = + config.type === 'website' || config.type === 'article' || config.type === 'profile' + ? config.type + : 'website' return generateSeoMetadata({ canonicalPath: canonicalPath || `/${pageKey.toLowerCase()}`, description: config.description, keywords: config.keywords, title: config.pageTitle, - type: config.type as 'website' | 'article' | 'profile', + type: ogType, })frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx (1)
149-154: Consider widening ModuleForm'ssetFormDataprop type to accept nullable state setter.The
formData!non-null assertion is justified by the guard on line 135, which ensures formData is non-null before rendering. However, the cast on line 153 (setFormData as React.Dispatch<...>) unnecessarily bypasses type safety. ModuleForm's prop type forsetFormData(line 38 of ModuleForm.tsx) could be changed to:setFormData: React.Dispatch<React.SetStateAction<ModuleFormProps['formData'] | null>>This would eliminate the assertion while maintaining type correctness, allowing the component to accept nullable state setters without casting.
frontend/src/app/about/page.tsx (1)
81-88: Improve type safety by explicitly constructing thememberobject.The code currently assigns the full GraphQL user object to the
memberfield, which includes extra properties (id,badgeCount,badges) that aren't part of theLeader.membertype. While this works due to TypeScript's structural typing, explicitly constructing only the required fields makes the intent clearer and reduces unnecessary data carried through the component.♻️ Suggested improvement
const leadersData = [data?.leader1, data?.leader2, data?.leader3] .filter(Boolean) .map((user) => ({ description: user?.login ? leaders[user.login as keyof typeof leaders] : '', memberName: user?.name || user?.login || '', - member: user, + member: user ? { + login: user.login, + name: user.name, + avatarUrl: user.avatarUrl, + } : null, })) - .filter((leader) => leader.memberName) as Leader[] + .filter((leader): leader is Leader => Boolean(leader.memberName))frontend/src/components/ChapterMap.tsx (1)
107-109: Type assertions are safe but redundant given prior filtering.The
validGeoLocDatais already filtered at lines 145-151 to ensurelatandlngare numbers, making theseas numberassertions technically redundant at runtime. However, they satisfy TypeScript's strict null checks since the type system doesn't track the filter's effect.For consistency, lines 93-94 in the
userLocationbranch also use the same coordinate pattern but lack theas numberassertions. Consider applying the same pattern there or extracting a helper function.♻️ Optional: Extract coordinate helper for consistency
const getChapterCoords = (chapter: Chapter): [number, number] => [ (chapter._geoloc?.lat ?? chapter.geoLocation?.lat) as number, (chapter._geoloc?.lng ?? chapter.geoLocation?.lng) as number, ]Then use it consistently across all coordinate access points.
frontend/src/app/mentorship/programs/[programKey]/page.tsx (1)
74-76: The double cast reflects a real type mismatch between the GraphQL-generatedMilestoneNode(which usesUserNodefor the author field) and the localMilestonetype (which usesUser). While this works, consider aligning theMilestonetype definition to match the GraphQL schema if this pattern becomes widespread.frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx (1)
71-79: The double assertion is a necessary workaround during API migration.The generated GraphQL type for
recentPullRequestsincludes additional fields (state,mergedAt,author, etc.) that differ from thePullRequesttype shape, necessitating the double assertion. The current approach works correctly, though aligning the types or creating a mapping layer would improve type safety in future refactors.frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (1)
110-110: Redundant?? undefinedpatterns.When
programis nullish,program?.adminsalready evaluates toundefined. The?? undefinedfallback is unnecessary.✨ Suggested simplification
- admins={program?.admins ?? undefined} + admins={program?.admins} ... - domains={program?.domains ?? undefined} + domains={program?.domains} ... - tags={program?.tags ?? undefined} + tags={program?.tags}Also applies to: 113-113, 119-119
frontend/src/components/MultiSearch.tsx (1)
265-265: Consider a type guard helper to reduce verbose casting.The repeated
as unknown as Record<string, string | undefined>pattern is hard to read and maintain. A discriminated union or type guard function would improve clarity.✨ Suggested helper approach
// Add near the SearchHit type definition const getHitIdentifier = (hit: SearchHit): string => { if ('key' in hit && hit.key) return hit.key if ('login' in hit && hit.login) return hit.login if ('url' in hit && hit.url) return hit.url return '' } const getHitDisplayName = (hit: SearchHit): string => { return hit.name || ('login' in hit ? hit.login : '') || '' }Then use in the JSX:
- key={`multi-search-${suggestion.indexName}-${(hit as unknown as Record<string, string | undefined>).key || (hit as unknown as Record<string, string | undefined>).login || (hit as unknown as Record<string, string | undefined>).url}`} + key={`multi-search-${suggestion.indexName}-${getHitIdentifier(hit)}`}Also applies to: 279-282
frontend/src/app/projects/dashboard/metrics/page.tsx (1)
159-160: Consider addressing the type mismatch in a dedicated type safety PR rather than this one.The double assertion exists because the generated GraphQL type differs from
HealthMetricsProps. While this pattern works, it's also pervasive throughout the codebase (15+ files). Since this PR is focused on enabling strict mode, addressing this comprehensively would be better suited to a separate type alignment or adapter refactoring effort.frontend/src/app/chapters/[chapterKey]/page.tsx (1)
87-87: Use a typed mapper function to handle the GraphQL-to-domain type conversion.Line 87 uses
as unknown as Chapterto force the GraphQL-generated chapter type into the domainChaptertype. While structurally compatible, this double assertion bypasses type safety. Consider extracting a mapper function (e.g.,mapChapterNodeToChapter) to handle the conversion explicitly, improving type safety and maintainability. This is especially useful if the types diverge further over time.frontend/src/app/community/snapshots/[id]/page.tsx (3)
148-148: Avoidas unknown as Typedouble casting; align types instead.Double casting (
as unknown as Chapter[]) completely bypasses TypeScript's type safety. This pattern suggests the GraphQL-generated types don't match your localChaptertype. Consider either:
- Using the generated type directly from the GraphQL query
- Ensuring local types match the generated schema types
- Using a type guard or mapping function to safely convert between types
Example: Create a mapping function
// If the types are structurally compatible but named differently: const toChapter = (data: SnapshotChapterType): Chapter => ({ key: data.key, name: data.name, // ... map other fields explicitly })
158-160: Same double-casting concern applies here.This
as unknown as Chapterfollows the same problematic pattern. If the GraphQL query returns compatible data, consider using a shared type or explicit mapping.
191-191: Same double-casting concern for Release.Consider aligning the generated GraphQL type with
ReleaseTypeor using explicit mapping.frontend/src/app/projects/dashboard/metrics/[projectKey]/page.tsx (1)
43-43: Avoid double casting; consider type alignment.The
as unknown as HealthMetricsPropspattern bypasses type safety. IfhealthMetricsLatestfrom the GraphQL query has a compatible structure, consider using the generated type directly or ensuringHealthMetricsPropsmatches the schema.frontend/src/components/CardDetailsPage.tsx (2)
407-425: IIFE adds unnecessary complexity; consider simplifying.The immediately-invoked function expression (IIFE) here is more complex than needed. The variable
modulesListjust aliasesmodules, and the logic could be expressed more directly.Simplified version without IIFE
- {type === 'program' && - modules && - modules.length > 0 && - (() => { - const modulesList = modules - return ( - <> - {modulesList.length === 1 ? ( - <div className="mb-8"> - <ModuleCard modules={modulesList} accessLevel={accessLevel} admins={admins} /> - </div> - ) : ( - <SecondaryCard icon={FaFolderOpen} title={<AnchorTitle title="Modules" />}> - <ModuleCard modules={modulesList} accessLevel={accessLevel} admins={admins} /> - </SecondaryCard> - )} - </> - ) - })()} + {type === 'program' && modules && modules.length > 0 && ( + modules.length === 1 ? ( + <div className="mb-8"> + <ModuleCard modules={modules} accessLevel={accessLevel} admins={admins} /> + </div> + ) : ( + <SecondaryCard icon={FaFolderOpen} title={<AnchorTitle title="Modules" />}> + <ModuleCard modules={modules} accessLevel={accessLevel} admins={admins} /> + </SecondaryCard> + ) + )}
521-533: IIFE for SponsorCard can be simplified.Similar to the modules section, this IIFE can be refactored for clarity.
Simplified version
- {entityKey && - ['chapter', 'project', 'repository'].includes(type) && - (projectName || title) && - (() => { - const cardTitle = (projectName || title) as string - return ( - <SponsorCard - target={entityKey} - title={cardTitle} - type={type === 'chapter' ? 'chapter' : 'project'} - /> - ) - })()} + {entityKey && + ['chapter', 'project', 'repository'].includes(type) && + (projectName || title) && ( + <SponsorCard + target={entityKey} + title={(projectName || title) as string} + type={type === 'chapter' ? 'chapter' : 'project'} + /> + )}
There was a problem hiding this comment.
2 issues found across 17 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="frontend/src/app/organizations/[organizationKey]/page.tsx">
<violation number="1" location="frontend/src/app/organizations/[organizationKey]/page.tsx:118">
P1: The `Number()` conversion for `createdAt` will return `NaN` if the backend provides an ISO date string (which is standard for GraphQL), defaulting the date to `0` (Jan 1, 1970).
Since the `Issue` type accepts `createdAt` as `string | number` and the `formatDate` utility handles strings correctly, this conversion is unnecessary and causes a functional regression. Removing the map preserves the original data and functionality.</violation>
<violation number="2" location="frontend/src/app/organizations/[organizationKey]/page.tsx:125">
P1: The `Number()` conversion for `publishedAt` will return `NaN` if the backend provides an ISO date string, defaulting the date to `0` (Jan 1, 1970).
While the `Release` type defines `publishedAt` as `number`, the runtime `formatDate` utility supports both strings and numbers. Casting directly allows the existing string data to flow through to the formatter, preventing the 1970 date bug. If a number conversion were strictly required, it would need to parse the date string (e.g., `new Date(s).getTime() / 1000`), but since the formatter handles strings, removing the map is the safest fix.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx (1)
100-114:⚠️ Potential issue | 🟡 MinorEmpty string fallback for date fields can bypass validation if schema becomes optional.
The
_validate_module_datesfunction inbackend/apps/mentorship/api/internal/mutations/module.pychecks explicitly forNonevalues but not for empty strings. If the GraphQL schema ever allows null values forstarted_atandended_atfields, sending empty strings ('') instead ofnullwould bypass this validation and cause anAttributeErrorattimezone.is_naive('')since strings don't have that method.The current schema requires these fields as non-optional
datetimetypes, which prevents empty strings from reaching the mutation. However, if the schema is updated to make these fields optional (e.g.,datetime | None), the validation logic needs to be updated to handle empty strings explicitly, or the frontend should consistently sendnullinstead of empty strings for missing dates.
🤖 Fix all issues with AI agents
In `@frontend/src/app/organizations/`[organizationKey]/page.tsx:
- Around line 118-141: The current mapping for recentIssues (createdAt) and
recentReleases (publishedAt) can produce NaN when coercing ISO strings with
Number(...); update the mapping in recentIssues?.map(...) and
recentReleases?.map(...) to guard against NaN: attempt Number(value) and if that
yields NaN, try Date.parse(value) and convert to seconds (Date.parse(...) /
1000) and finally fallback to 0; ensure createdAt and publishedAt are normalized
to Unix seconds (number) or preserve the original string if you prefer letting
formatDate accept string input, but do not use Number(value) || 0 without an
explicit isNaN check.
In `@frontend/src/components/Release.tsx`:
- Around line 98-101: The button currently passes an empty string to
TruncatedText causing a blank visible label when release.repositoryName is
missing; update the Release.tsx usage so TruncatedText receives a visible
fallback (e.g., pass release.repositoryName ?? 'unknown' or conditionally render
a placeholder string) and ensure the aria-label still uses the same fallback
(aria-label={`View repository ${release.repositoryName ?? 'unknown'}`}) so the
visible text and accessible label match; target the TruncatedText call and the
aria-label in Release.tsx when making this change.
In `@frontend/src/types/committee.ts`:
- Line 8: The Committee type was changed to require a key property, but the test
fixture mockCommitteeDetailsData is missing it; open
frontend/__tests__/mockData/mockCommitteeDetailsData.ts and add a string key
(e.g., 'test_committee') to the top-level committee object so
mockCommitteeDetailsData.committee satisfies the Committee type (ensure the
property name is exactly key and its value is a string).
🧹 Nitpick comments (5)
frontend/__tests__/unit/components/ToggleableList.test.tsx (1)
219-224: Test title is misleading - it doesn't verifypreventDefaultwas called.The test description says "prevents default behavior on keyboard event" but the test only verifies that
mockPushwas called. To truly test that default behavior is prevented, you would need to check thatevent.preventDefault()was invoked.Consider either:
- Renaming the test to reflect what it actually tests (e.g., "navigates on Enter key press")
- Or verifying
preventDefaultwas called using a spy on the event♻️ Suggested fix to properly test preventDefault
it('prevents default behavior on keyboard event', () => { render(<ToggleableList entityKey="test" items={['React']} label="Tags" />) const button = screen.getByText('React') - fireEvent.keyDown(button, { key: 'Enter' }) - expect(mockPush).toHaveBeenCalled() + const event = { key: 'Enter', preventDefault: jest.fn() } + fireEvent.keyDown(button, event) + expect(event.preventDefault).toHaveBeenCalled() })frontend/src/components/ItemCardList.tsx (1)
96-105: Minor optimization: hoistgetItemKeyoutside the map callback.The
getItemKeyfunction is recreated on every iteration ofdata.map. While this is functionally correct, hoisting it outside the map (or outside the component) would avoid unnecessary function allocations.♻️ Suggested refactor
+const getItemKey = (i: ItemCardData, idx: number): string => { + if ('objectID' in i && i.objectID) return i.objectID + if ('id' in i && i.id) return i.id + const repoName = 'repositoryName' in i ? i.repositoryName : '' + const title = 'title' in i ? i.title : '' + const name = 'name' in i ? i.name : '' + const url = 'url' in i ? i.url : '' + const key = `${repoName || ''}-${title || name || ''}-${url || ''}` + return key || `item-${idx}` +} + const ItemCardList = ({ // ...props }) => ( <SecondaryCard icon={icon} title={title}> {data && data.length > 0 ? ( <div className={...}> {data.map((item, index) => { - const getItemKey = (i: ItemCardData, idx: number): string => { - if ('objectID' in i && i.objectID) return i.objectID - if ('id' in i && i.id) return i.id - const repoName = 'repositoryName' in i ? i.repositoryName : '' - const title = 'title' in i ? i.title : '' - const name = 'name' in i ? i.name : '' - const url = 'url' in i ? i.url : '' - const key = `${repoName || ''}-${title || name || ''}-${url || ''}` - return key || `item-${idx}` - } return ( <div key={getItemKey(item, index)}frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/edit/page.tsx (1)
154-154: Type cast is safe here due to the null guard, but consider narrowing instead.The state is
ModuleFormData | null, but by line 154, the guard at lines 136-138 ensuresformDatais non-null. The cast toReact.Dispatch<React.SetStateAction<ModuleFormData>>works at runtime.However, if
ModuleFormever attempts to callsetFormData(null), TypeScript won't catch it. A cleaner pattern would be to define a separate narrowed state after the guard or use a type predicate. That said, for a strict-mode enablement PR, this pragmatic approach is acceptable.frontend/src/app/my/mentorship/programs/[programKey]/page.tsx (1)
108-111: Prefer explicit “N/A” for missing dates instead of formatting empty strings.If
startedAt/endedAtare absent, formatting''can render awkward output (e.g., “Invalid Date”). A small guard keeps the UI clearer.♻️ Suggested change
- { label: 'Start Date', value: formatDate(program?.startedAt ?? '') }, - { label: 'End Date', value: formatDate(program?.endedAt ?? '') }, + { label: 'Start Date', value: program?.startedAt ? formatDate(program.startedAt) : 'N/A' }, + { label: 'End Date', value: program?.endedAt ? formatDate(program.endedAt) : 'N/A' },frontend/src/app/community/snapshots/[id]/page.tsx (1)
148-149: Remove unnecessaryas unknown astype casts. The GraphQL-generatedGetSnapshotDetailsDocumentalready provides proper typing forsnapshot.newChaptersandsnapshot.newReleases. The intermediateas unknown ascasts bypass TypeScript's strict checks and are redundant.This applies to:
- Line 148:
snapshot.newChapters as unknown as Chapter[]in ChapterMapWrapper props- Line 159:
chapter as unknown as Chapterin renderChapterCard call- Line 194:
release as unknown as ReleaseTypein Release component propsRemove these casts—the inferred types from the GraphQL query are already correct.
rudransh-shrivastava
left a comment
There was a problem hiding this comment.
Hey, the PR looks pretty good. Please address the CI checks and some suggestions:
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/app/organizations/[organizationKey]/repositories/[repositoryKey]/layout.tsx (1)
33-53:⚠️ Potential issue | 🔴 CriticalUnresolved merge conflict markers must be removed.
The file contains Git merge conflict markers (
<<<<<<<,=======,>>>>>>>) that were not resolved before committing. This will cause parse errors and prevent the application from compiling.Based on the PR objective to enable strict mode and the AI summary noting that similar layout files return
undefinedfor non-existent entities, theenable-strict-modebranch version appears to be the intended implementation.🔧 Proposed fix to resolve the conflict
-<<<<<<< enable-strict-mode - return repository - ? generateSeoMetadata({ - canonicalPath: `/organizations/${organizationKey}/repositories/${repositoryKey}`, - description: repository.description ?? `${repository.name} repository details`, - keywords: ['owasp', 'repository', repositoryKey, repository.name], - title: repository.name, - }) - : undefined -======= - if (!repository) { - return {} - } - - return generateSeoMetadata({ - canonicalPath: `/organizations/${organizationKey}/repositories/${repositoryKey}`, - description: repository.description ?? `${repository.name} repository details`, - keywords: ['owasp', 'repository', repositoryKey, repository.name], - title: repository.name, - }) ->>>>>>> main + return repository + ? generateSeoMetadata({ + canonicalPath: `/organizations/${organizationKey}/repositories/${repositoryKey}`, + description: repository.description ?? `${repository.name} repository details`, + keywords: ['owasp', 'repository', repositoryKey, repository.name], + title: repository.name, + }) + : undefined
🤖 Fix all issues with AI agents
In
`@frontend/src/app/organizations/`[organizationKey]/repositories/[repositoryKey]/page.tsx:
- Around line 26-33: Remove the Git conflict markers and the unused state-based
approach: delete the entire conflicted block containing <<<<<<<, =======, and
>>>>>>> as well as the declarations repository, topContributors, and
recentPullRequests that use useState; also remove the unused useState import
from React so only useEffect remains. Keep the current pattern that derives
values directly from the query result (the existing derived values used later in
the file) rather than the removed local state, ensuring no leftover references
to repository/topContributors/recentPullRequests or useState remain.
🧹 Nitpick comments (1)
frontend/src/app/organizations/[organizationKey]/repositories/[repositoryKey]/page.tsx (1)
123-126: Double-casting throughunknownbypasses type safety.The
as unknown as Type[]pattern completely disables TypeScript's type checking between the source and target types. While this resolves compilation errors, it undermines the goal of strict mode by hiding potential type mismatches at runtime.The GraphQL-generated types (e.g.,
PullRequestNode) have structural differences from the domain types (e.g.,PullRequest). Consider one of these approaches:
- Align the domain types with the generated GraphQL types to enable direct usage.
- Create explicit mapper functions that transform GraphQL types to domain types with proper null handling.
- Use the GraphQL types directly in
DetailsCardif feasible.For now, this may be acceptable as a transitional approach, but tracking this as technical debt is recommended.
♻️ Example mapper approach for better type safety
// Utility to map GraphQL PullRequestNode to domain PullRequest const mapToPullRequests = ( nodes: GetRepositoryDataQuery['recentPullRequests'] ): PullRequest[] => nodes.map((pr) => ({ id: pr.id, createdAt: pr.createdAt, organizationName: pr.organizationName, repositoryName: pr.repositoryName, title: pr.title, url: pr.url, author: pr.author, }))Then use:
- pullRequests={recentPullRequests as unknown as PullRequest[]} + pullRequests={mapToPullRequests(recentPullRequests ?? [])}
frontend/src/app/organizations/[organizationKey]/repositories/[repositoryKey]/page.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@frontend/src/app/organizations/`[organizationKey]/repositories/[repositoryKey]/page.tsx:
- Around line 113-116: The props currently using double casts
(pullRequests={recentPullRequests as unknown as PullRequest[]},
recentIssues={repository.issues as unknown as Issue[]},
recentMilestones={repository.recentMilestones as unknown as Milestone[]},
recentReleases={repository.releases as unknown as Release[]}) should be changed
to use nullish coalescing so you don't bypass TypeScript: replace each
double-cast reference (recentPullRequests, repository.issues,
repository.recentMilestones, repository.releases) with a nullish-coalesced value
(e.g., recentPullRequests ?? [] ) so the types align with
DetailsCard/RecentIssues and preserve defensive rendering without using as
unknown as T.
frontend/src/app/organizations/[organizationKey]/repositories/[repositoryKey]/page.tsx
Outdated
Show resolved
Hide resolved
|
There was a problem hiding this comment.
1 issue found across 14 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="frontend/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/issues/[issueId]/page.tsx">
<violation number="1">
P2: repositoryName is nullable; removing the fallback can render an incomplete header when it is null. Keep a fallback or conditional rendering to avoid blank repository names.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
...nd/src/app/my/mentorship/programs/[programKey]/modules/[moduleKey]/issues/[issueId]/page.tsx
Show resolved
Hide resolved
arkid15r
left a comment
There was a problem hiding this comment.
It looks quite solid but still leaves a room for improvements (Sonar highlights some warnings for me locally).
Thank you 👍
* Run make update * Clean up snapshot generated videos * Update backend/data/nest.dump * feat(ui): revamp corporate supporters carousel (Infinite Marquee + Dark Mode fix) (#3837) * feat(ui): revamp corporate supporters carousel (Infinite Marquee + Dark Mode fix) * fix: resolve failing test case * fix: add fallback text for unnamed sponsors * docs: add docstrings to satisfy coverage requirements * Run make check and fix tests. --------- Co-authored-by: Kate <kate@kgthreads.com> * Fix/redundant typescript assertion (#3834) * Fix Sonar S4325 by narrowing session user fields instead of casting * Fix unused ExtendedSession in mentorship page * fix: redundant-typescript-assertion * Fix stale latest date displayed in Project Health Dashboard metrics (#3842) * Fixed latest date in proejct health dashboard * updated order * Update code * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> * feat: improve backend test coverage to 96% (#3840) * feat: improve backend test coverage to 96% * fix comments * fix issues * fix issue * fix cubic-dev-ai comments * Update code * Fix tests --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> * Fix: merge consecutive RUN instructions in frontend Dockerfile (#3644) * Fix: merge consecutive RUN instructions in frontend Dockerfile * fix: comment Dockerfile note to prevent syntax error * Update code * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> * Fix 'is_merged' not being available on the Issue (#3843) * Fix 'is_merged' not being available on the Issue * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> * CI: Add ansible-lint workflow for Ansible playbooks (#3796) * ci: add ansible-lint workflow Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> * Update .github/workflows/lint-ansible.yaml Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * ci: add ansible-lint make target and workflow Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> * ci: add ansible-lint pre-commit hook Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> * fix: whitespace & version Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> * Update Makefile Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> * ci: enable ansible-lint scanning and add requirements.yml Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> * chore(ansible):align linting and module usage Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> * ci(ansible): install collections before deploy playbooks Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> * Update code * Update code * Update .github/workflows/run-ci-cd.yaml --------- Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> * Fix ElevenLabs API error (#3861) * use default liam voice * bump speed by 0.10 --------- Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> * Add Ime Iyonsi to MENTORS.md (#3866) * Add mentor profile for Ime Iyonsi Added Ime Iyonsi's mentor profile. * Fix GitHub link for Ime Iyonsi Corrected GitHub link for Ime Iyonsi. * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> * Update MENTORS.md * Enabled Strict Mode (#3776) * Enabled Strict Mode * fixed ai review * fix * fixed review * fix * update test * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> * Resolve case-sensitivity in QueryParser to support Chapters/Members search (#3844) * resolve query parser blocker * use case_sensitive flag in QueryParser * feat: add case_sensitive option to QueryParser and update tests * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> * Update dependencies (#3874) * Update dependencies * Bump django-ninja version * fix(proxy): pin nginx and certbot images (#3848) * fix(proxy): pin nginx and certbot images Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> * fix stable verssions Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> --------- Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> * Update docker-compose/proxy/compose.yaml * Update backend/pyproject.toml * Update ansible lint configuration (#3880) * Update .github/ansible/.ansible-lint.yaml * Improve frontend test coverage above 80% and add missing test files (#3864) * Imrove test coverage to 80% and added test * Fixed coderabbit review * update code * fixed coderabbit ai * fixed soanrqube warning * fixed review * update * fixed aloglia cache_key (#3825) * fixed aloglia cache_key * change separator val to be semicolon (;) * Update code * add tests + use json filters * add trailing newline * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> * fix: remove unused className prop from AnchorTitle component (#3822) * fix: remove unused className prop from AnchorTitle component Fixes #3805 The className prop was defined in AnchorTitleProps but never used in the component implementation. Removing it resolves Sonar rule typescript:S6767 and improves code maintainability. * fix: use className prop instead of removing it - Added className back to AnchorTitleProps interface - Accept className parameter in component - Apply className to root div element - Resolves reviewer feedback on PR #3822 * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> --------- Signed-off-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org> Co-authored-by: Yashraj Pahuja <yashrajpahuja9999@gmail.com> Co-authored-by: Kate <kate@kgthreads.com> Co-authored-by: CodeAritraDhank <aritradhank21@gmail.com> Co-authored-by: Anurag Yadav <143180737+anurag2787@users.noreply.github.com> Co-authored-by: Harshit Verma <harshit1092004@gmail.com> Co-authored-by: Arkadii Yakovets <2201626+arkid15r@users.noreply.github.com> Co-authored-by: Shuban Mutagi <shubanmutagi55@gmail.com> Co-authored-by: Muhammad Hassaan Saleem <iamhassaans@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com> Co-authored-by: emaybu <152900874+emaybu@users.noreply.github.com> Co-authored-by: sai chethana <saichethanavesireddy@gmail.com> Co-authored-by: Rahul Paul <179798584+Mr-Rahul-Paul@users.noreply.github.com> Co-authored-by: Lavanya <lavanyayadawad30@gmail.com>



Proposed change
This PR enables full TypeScript strict mode on the frontend and resolves the type errors that were surfaced as a result.
Resolves #3407
Checklist
make check-testlocally: all warnings addressed, tests passed