-
-
Notifications
You must be signed in to change notification settings - Fork 204
Fix homepage container flexibility #2100
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix homepage container flexibility #2100
Conversation
Summary by CodeRabbit
WalkthroughRootLayout restructured (skip-link, single main landmark, footer after main with mt-auto); Home/About simplified to static pages; global Loading component added; many Tailwind/tooling upgrades (Tailwind v4, PostCSS change, package/devDeps/scripts); Issue type extended + IssueMetadata added; ApolloProvider removed; JSON Schema validators and CI/LHCI changes added. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested labels
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
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/globals.css (1)
203-211
: Duplicate @layer base block detected.This block duplicates the earlier one at lines 88-95. Since this appears later in the file, it will override the earlier one, but having both creates confusion and maintenance issues.
Remove the earlier duplicate block (lines 88-95) and keep this one to maintain the intended styling order.
🧹 Nitpick comments (6)
frontend/src/components/TestCard.tsx (1)
1-13
: Consider removing test component before production deploymentThis appears to be a development/testing component used to verify Tailwind theme functionality. While the implementation is correct and uses appropriate semantic classes, consider removing this component before merging to production or moving it to a development-only directory.
If this component is intended for ongoing theme testing, consider moving it to a dedicated testing/development directory:
-frontend/src/components/TestCard.tsx +frontend/src/components/dev/TestCard.tsxOr add a comment indicating its purpose and lifecycle:
+// Development component for testing Tailwind theme configuration +// TODO: Remove before production deployment export default function TestCard() {frontend/tailwind.config.cjs (1)
68-68
: Review the animation duration for the scroll effect.The scroll animation is set to
0.5s
duration, which seems unusually fast for a scrolling effect that translates by 500%. This could result in jarring or difficult-to-read content.Consider adjusting the animation duration to be more user-friendly:
- scroll: "scroll 0.5s linear infinite", + scroll: "scroll 20s linear infinite",frontend/src/app/loading.tsx (1)
1-31
: Consider enhancing the loading component for better accessibility and theming.The loading component implementation is clean and functional. However, it could benefit from accessibility improvements and theme integration.
Consider these enhancements:
// src/app/loading.tsx export default function Loading() { return ( - <div className="flex h-screen w-full items-center justify-center bg-white"> + <div className="flex h-screen w-full items-center justify-center bg-background"> <div className="flex flex-col items-center space-y-4"> <svg className="h-12 w-12 animate-spin text-blue-500" xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" + role="status" + aria-label="Loading" > <circle className="opacity-25" cx="12" cy="12" r="10" stroke="currentColor" strokeWidth="4" ></circle> <path className="opacity-75" fill="currentColor" d="M4 12a8 8 0 018-8v4a4 4 0 00-4 4H4z" ></path> </svg> - <p className="text-gray-600 text-sm">Loading...</p> + <p className="text-muted-foreground text-sm">Loading...</p> </div> </div> ); }This change:
- Uses the theme's
bg-background
class for consistent theming- Adds ARIA attributes for better screen reader support
- Uses
text-muted-foreground
for consistent text coloringfrontend/src/app/about/page.tsx (1)
3-9
: Consider using consistent layout structure for better footer positioning.Similar to the home page, this component should support the footer positioning fix mentioned in the PR objectives.
Consider wrapping the content to match the homepage structure:
export default function AboutPage() { return ( - <div className="p-8 font-sans"> + <div className="flex-1 bg-background text-foreground p-8"> <h1 className="text-3xl font-bold mb-4">About OWASP Nest</h1> - <p> + <p className="text-muted-foreground"> OWASP Nest is a platform to discover, engage, and contribute to OWASP projects and initiatives. It serves as a central hub for the community. </p> </div> ); }frontend/src/app/layout.tsx (1)
76-77
: Clean up trailing whitespace.There's unnecessary trailing whitespace after
{children}
that should be removed for consistency.- {children } - + {children}frontend/src/app/globals.css (1)
97-99
: Remove excessive blank lines.These empty lines create unnecessary whitespace in the CSS file.
- - -
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
frontend/pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
frontend/package.json
(3 hunks)frontend/postcss.config.js
(1 hunks)frontend/src/app/about/layout.tsx
(1 hunks)frontend/src/app/about/page.tsx
(1 hunks)frontend/src/app/globals.css
(1 hunks)frontend/src/app/layout.tsx
(1 hunks)frontend/src/app/loading.tsx
(1 hunks)frontend/src/app/page.tsx
(1 hunks)frontend/src/components/TestCard.tsx
(1 hunks)frontend/src/sentry.server.config.ts
(1 hunks)frontend/src/wrappers/provider.tsx
(2 hunks)frontend/tailwind.config.cjs
(1 hunks)frontend/tailwind.config.js
(0 hunks)
💤 Files with no reviewable changes (1)
- frontend/tailwind.config.js
🧰 Additional context used
🧬 Code Graph Analysis (3)
frontend/src/sentry.server.config.ts (1)
frontend/src/utils/credentials.ts (1)
ENVIRONMENT
(3-3)
frontend/src/app/about/layout.tsx (1)
frontend/src/app/layout.tsx (1)
RootLayout
(59-87)
frontend/src/app/layout.tsx (5)
frontend/src/wrappers/provider.tsx (1)
Providers
(19-34)frontend/src/components/AutoScrollToTop.tsx (1)
AutoScrollToTop
(6-14)frontend/src/components/Header.tsx (1)
Header
(20-248)frontend/src/utils/credentials.ts (1)
IS_GITHUB_AUTH_ENABLED
(9-11)frontend/src/components/BreadCrumbs.tsx (1)
BreadCrumbs
(10-70)
🔇 Additional comments (11)
frontend/package.json (2)
60-60
: GraphQL dependencies successfully removedThe removal of
@apollo/client
andgraphql
dependencies aligns with the broader architectural shift observed across the codebase, as confirmed by changes infrontend/src/wrappers/provider.tsx
where theApolloProvider
has been disabled.
69-69
: Tailwind CSS configuration updatedThe updates to
@tailwindcss/postcss
,tailwindcss
, and addition oftailwindcss-animate
support the enhanced theming and styling improvements mentioned in the PR objectives for improved homepage container flexibility.Also applies to: 103-104
frontend/src/wrappers/provider.tsx (2)
9-11
: Temporary GraphQL removal is well-documentedThe comments clearly indicate that Apollo/GraphQL removal is temporary, which is good for maintainability. The
AppInitializer
remains functional for Django session management.Also applies to: 26-26
27-28
: Critical GraphQL dependencies still actively used throughout the applicationThe verification reveals that GraphQL data fetching has NOT been disabled as the original review comment suggested. Extensive GraphQL usage remains throughout the codebase:
• Active GraphQL queries in 30+ components including snapshots, projects dashboard, organization details, user profiles, mentorship programs, and API key management
• Active GraphQL mutations for logout, session sync, program management, and API key operations
• Apollo Client hooks still functioning withuseQuery
anduseMutation
actively used across the application
• GraphQL query definitions still present and imported in all relevant filesThe components are functioning correctly with their GraphQL data dependencies intact. The
AppInitializer
and{children}
components in the provider are receiving proper GraphQL context through the Apollo Client setup that remains active.No action required - the GraphQL infrastructure is operational and components have their expected data sources available.
Likely an incorrect or invalid review comment.
frontend/src/sentry.server.config.ts (1)
7-7
: Good fallback handling for development environmentsThe fallback to 'development' when
ENVIRONMENT
is undefined is a sensible improvement that prevents Sentry initialization failures in local development setups.frontend/tailwind.config.cjs (1)
1-77
: LGTM! Well-structured Tailwind configuration with comprehensive theming support.The configuration provides a solid foundation for the application's styling system with proper theme variable integration, custom colors, and animation support. The use of CSS variables for theming enables smooth light/dark mode transitions.
frontend/postcss.config.js (1)
3-3
: Change is correct and follows Tailwind CSS v4 migration requirements.The change from
tailwindcss
to@tailwindcss/postcss
is the proper way to configure PostCSS with Tailwind CSS v4. According to the official Tailwind CSS v4 upgrade guide, the PostCSS plugin was moved out of the core package into a dedicated@tailwindcss/postcss
package.The verification confirms:
- Both
@tailwindcss/postcss
(v4.1.12) andtailwindcss
(v4.1.12) are properly installed in package.json- The
@tailwindcss/postcss
package exists and is the current stable version- This is the recommended configuration for Tailwind CSS v4 with PostCSS
The configuration is correctly implemented using the object syntax format that works with most build tools.
frontend/src/app/layout.tsx (2)
67-67
: LGTM! Layout changes implement the sticky footer solution.The addition of
min-h-screen flex flex-col
to the body element ensures the page takes up the full viewport height and establishes a flex container for proper layout distribution.
74-78
: LGTM! Main content wrapper correctly implements flexible layout.The
<main>
element withflex-1 flex flex-col
classes properly:
- Takes up all available vertical space between header and footer (
flex-1
)- Establishes a flex container for child components (
flex flex-col
)- Ensures the footer is pushed to the bottom regardless of content height
This directly addresses issue #2033 regarding the footer not sticking to the bottom on screens over 1100px height.
frontend/src/app/globals.css (2)
5-44
: LGTM! Well-structured CSS custom properties for theming.The root variables are properly organized with:
- Clear commenting and structure
- Comprehensive color scheme coverage (background, foreground, card, popover, etc.)
- HSL color format for better manipulation
- Chart colors for data visualization
- Standard border radius variable
49-83
: LGTM! Dark theme implementation follows consistent structure.The dark theme variables properly override the light theme with:
- Appropriate color inversions (dark backgrounds, light text)
- Consistent variable naming with the root theme
- Proper contrast ratios for accessibility
- Chart colors adapted for dark backgrounds
…om/OUMIMANDAL/Nest-milestones into fix-homepage-container-flexibility # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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/tailwind.config.cjs (1)
1-77
: This .cjs file uses TypeScript types and ESM syntax — it will not parse; keep one config or convert to valid CJSStatic analysis is correct:
.cjs
cannot useimport
/export
or TS type annotations (import type
,: Config
). You currently have both a TS config and this CJS one with duplicate content. Prefer one source of truth.Options:
- Recommended: delete
frontend/tailwind.config.cjs
and keepfrontend/tailwind.config.ts
.- Or convert this file to actual CommonJS.
If you choose CJS, here’s the minimal conversion:
-import type { Config } from "tailwindcss"; -import { heroui } from "@heroui/react"; -import animate from "tailwindcss-animate"; +const { heroui } = require("@heroui/react"); +const animate = require("tailwindcss-animate"); @@ -const config: Config = { +/** @type {import('tailwindcss').Config} */ +const config = { @@ -export default config; +module.exports = config;Also, ensure the
text
color either maps to an existing CSS var or is removed here as well, mirroring the fix in the TS config.
🧹 Nitpick comments (6)
frontend/src/app/about/page.tsx (1)
wrapper and constrain content width (optional).
3-9
: Use a semanticImproves semantics (a11y/landmarks) and readability on wide screens by centering and limiting line length.
Apply this diff:
- <div className="p-8 font-sans"> + <main className="mx-auto max-w-3xl p-8 font-sans"> ... - </div> + </main>frontend/tailwind.config.ts (1)
13-19
:text
color references an undefined CSS variable (--text
)In
globals.css
, there’s no--text
token defined. Usingtext: "hsl(var(--text))"
will yield invalid CSS when referenced. Either remove this alias or map it to--foreground
, or add--text
to the CSS tokens.Proposed quick fix (map to foreground):
colors: { background: "hsl(var(--background))", foreground: "hsl(var(--foreground))", border: "hsl(var(--border))", - text: "hsl(var(--text))", + text: "hsl(var(--foreground))", "owasp-blue": "#98AFC7",frontend/src/app/globals.css (1)
8-44
: Define--text
to match Tailwind config expectations (or remove the Tailwindtext
color)Tailwind config exposes a
text
color tied to--text
, which is missing here. If you keeptext
in the Tailwind config, add--text
tokens for both light and dark to avoid invalid CSS.Add a
--text
token to both themes::root { --background: 0 0% 100%; --foreground: 222.2 84% 4.9%; + --text: 222.2 47.4% 11.2%; @@ .dark { --background: 222.2 84% 4.9%; --foreground: 210 40% 98%; + --text: 210 40% 98%;frontend/src/app/layout.tsx (3)
1-1
: Analytics mismatch: importing GoogleAnalytics but providing a GTM IDYou’re importing
GoogleAnalytics
while usingGTM_ID
(fromNEXT_PUBLIC_GTM_ID
). If this is a Google Tag Manager container ID (GTM-xxxx), you should useGoogleTagManager
withgtmId
instead. If you intend GA4 (G-xxxx), exposeNEXT_PUBLIC_GA_ID
and pass that toGoogleAnalytics
.Suggested import change if using GTM:
-import { GoogleAnalytics } from '@next/third-parties/google'; +import { GoogleTagManager } from '@next/third-parties/google';
96-97
: Guard third-party script and use the correct component for GTMAlso guard against undefined envs to avoid type errors. If you’re sticking with GTM, render
GoogleTagManager
only whenGTM_ID
is present.- <GoogleAnalytics gaId={GTM_ID} /> + {GTM_ID ? <GoogleTagManager gtmId={GTM_ID} /> : null}If the intention is GA4 instead, define
NEXT_PUBLIC_GA_ID
and use:{GA_ID ? <GoogleAnalytics gaId={GA_ID} /> : null}
81-91
: Fixed header may overlap page content on routes without breadcrumbs
Header
isfixed
withh-16
, andBreadCrumbs
returnsnull
on/
. In that case,main
has no top offset, so top content can sit under the header. Confirm the home loading state and simple pages don’t render under the header. If they do, add a consistent top padding (e.g.,pt-16
) at the layout level and remove ad-hoc top margins elsewhere as needed.Optional one-line change here:
- <main + <main id="main-content" - className="flex-1 flex flex-col min-w-0" + className="flex-1 flex flex-col min-w-0 pt-16" >Note: if you adopt this, you may want to remove the
mt-16
fromBreadCrumbs
to avoid double spacing.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
frontend/pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
frontend/package.json
(5 hunks)frontend/pnpm-workspace.yaml
(1 hunks)frontend/src/app/about/layout.tsx
(1 hunks)frontend/src/app/about/page.tsx
(1 hunks)frontend/src/app/globals.css
(1 hunks)frontend/src/app/layout.tsx
(2 hunks)frontend/src/app/page.tsx
(1 hunks)frontend/tailwind.config.cjs
(1 hunks)frontend/tailwind.config.ts
(1 hunks)package.json
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/app/about/layout.tsx
- frontend/src/app/page.tsx
- frontend/package.json
🧰 Additional context used
🧬 Code Graph Analysis (2)
frontend/tailwind.config.ts (1)
frontend/tailwind.config.cjs (1)
config
(5-74)
frontend/src/app/layout.tsx (7)
frontend/src/wrappers/provider.tsx (1)
Providers
(19-34)frontend/src/components/AutoScrollToTop.tsx (1)
AutoScrollToTop
(6-14)frontend/src/components/Header.tsx (1)
Header
(20-248)frontend/src/utils/credentials.ts (2)
IS_GITHUB_AUTH_ENABLED
(9-11)GTM_ID
(7-7)frontend/src/components/BreadCrumbs.tsx (1)
BreadCrumbs
(10-70)frontend/src/components/Footer.tsx (1)
Footer
(12-113)frontend/src/components/ScrollToTop.tsx (1)
ScrollToTop
(7-43)
🪛 Biome (2.1.2)
frontend/tailwind.config.cjs
[error] 1-1: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 1-1: Illegal use of an import declaration outside of a module
not allowed inside scripts
(parse)
[error] 1-2: Illegal use of an import declaration outside of a module
not allowed inside scripts
(parse)
[error] 2-3: Illegal use of an import declaration outside of a module
not allowed inside scripts
(parse)
[error] 5-5: type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 75-76: Illegal use of an export declaration outside of a module
not allowed inside scripts
(parse)
🔇 Additional comments (6)
frontend/src/app/about/page.tsx (1)
3-8
: Good switch to Tailwind; resolves prior inline-style feedback.Using Tailwind classes here brings the About page in line with the project’s styling approach and addresses the previous review suggestion about inline styles.
frontend/tailwind.config.ts (1)
5-74
: Typed Tailwind config looks solid and aligns with the new token systemContent globs, dark mode strategy, CSS-var-based palette, radii, and plugins are all consistent with globals.css and the stated objectives. Once the
text
token is resolved, this should be good to go.frontend/src/app/globals.css (1)
193-201
: LGTM: Base layer applies theme tokens correctlyApplying
border-border
on all elements andbg-background text-foreground
on body is consistent with the CSS-var palette and Tailwind config.frontend/src/app/layout.tsx (3)
66-73
: Footer stickiness fix via flex + dvh/min-h-screen looks solidUsing a flex column on body with min-h-screen plus dvh support will keep the footer at the bottom even during loading on tall screens. This aligns with the PR objective.
74-80
: Nice a11y improvement with the skip linkThe skip link is correctly implemented and points to the single main landmark. Good job.
85-91
: Single document-wide main landmark and min-w-0 are on pointGood semantics and overflow resilience here. This prevents nested mains and helps with long inline content.
> | ||
{children} | ||
</main> | ||
<Footer className="mt-auto" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Passing className to Footer breaks types and is redundant (Footer already has mt-auto)
Footer
does not accept props and already applies mt-auto
internally (see frontend/src/components/Footer.tsx). Passing className
here will cause a TS error and is unnecessary.
Apply this diff:
- <Footer className="mt-auto" />
+ <Footer />
📝 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.
<Footer className="mt-auto" /> | |
<Footer /> |
🤖 Prompt for AI Agents
In frontend/src/app/layout.tsx around line 92, the Footer component is being
passed a className prop which Footer does not accept and already applies mt-auto
internally; remove the className attribute from the <Footer /> invocation so it
is rendered as <Footer /> with no props, and update any surrounding
code/comments if necessary to reflect that Footer controls its own spacing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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/globals.css (1)
180-187
: Don’t gate.dark
behavior behindprefers-color-scheme: dark
.Requiring both
.dark
and theprefers-color-scheme: dark
media query prevents the filter from applying when the app is toggled to dark mode but the system is light. This is a functional bug for explicit theme toggling.Apply this diff:
-@media (prefers-color-scheme: dark) { - .dark .map-tiles { - filter: var(--map-tiles-filter, none); - } -} +.dark .map-tiles { + filter: var(--map-tiles-filter, none); +}
♻️ Duplicate comments (1)
frontend/src/app/globals.css (1)
171-178
: Duplicate @layer base block from prior review is resolved.There is now a single
@layer base
block. Thanks for cleaning that up.
🧹 Nitpick comments (4)
frontend/src/app/globals.css (4)
91-99
: Avoid redefining Tailwind’s built-in@keyframes spin
; rely on Tailwind defaults.Tailwind already provides a
spin
keyframes block andanimate-spin
utility. Redefining@keyframes spin
risks subtle conflicts and is redundant.Apply this diff to remove the custom keyframes and rely on Tailwind’s:
-@keyframes spin { - 0% { transform: rotate(0deg); } - 100% { transform: rotate(360deg); } -}
149-151
: Remove custom.animate-spin
class to avoid overriding Tailwind’s utility.Tailwind ships
.animate-spin
with the same behavior. Keeping a duplicate definition increases maintenance and can accidentally override Tailwind defaults if they change.-.animate-spin { - animation: spin 1s linear infinite; -}
221-248
: Add Firefox scrollbar styles for parity with WebKit.You’ve styled WebKit scrollbars; Firefox still uses
scrollbar-width
andscrollbar-color
. Add equivalents for consistent theming./* ========================= Scrollbar Styles ========================= */ +/* Firefox */ +html { + scrollbar-width: thin; + scrollbar-color: #3d4554 #7689ad; /* thumb track */ +} + +.dark html { + scrollbar-color: #4a5568 #2d3748; +}
329-333
: Provide a light-mode ::selection style as well.Currently only dark mode selection is customized. Add a default selection style for light mode for a consistent experience.
@layer utilities { + ::selection { + @apply bg-blue-200 text-black; + } .dark ::selection { @apply bg-blue-500 text-white; } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
frontend/src/app/globals.css
(8 hunks)
🔇 Additional comments (1)
frontend/src/app/globals.css (1)
130-147
: LGTM on added keyframes.The additional fade/scale keyframes are concise and useful for subtle UI transitions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
frontend/src/app/globals.css (1)
184-188
: Remove the media-query guard around .dark map-tiles filterGating the
.dark .map-tiles
rule behind@media (prefers-color-scheme: dark)
prevents the filter from applying when users explicitly toggle.dark
while their OS is set to light. Make the selector unconditional on.dark
.-@media (prefers-color-scheme: dark) { - .dark .map-tiles { - filter: var(--map-tiles-filter, none); - } -} +.dark .map-tiles { + filter: var(--map-tiles-filter, none); +}
🧹 Nitpick comments (5)
frontend/src/app/globals.css (5)
150-152
: Avoid redefining Tailwind’s animate-spin utilityTailwind already provides
.animate-spin
; redefining it can cause drift or conflicts. Prefer using Tailwind’s built-in class. If you keep custom usage (e.g.,.heatmap-loader
), you can keep the@keyframes spin
but drop the duplicate class.-.animate-spin { - animation: spin 1s linear infinite; -}
119-121
: Align link color with theme tokensHardcoding
#1d7bd7
bypasses your CSS-variable palette. Usetext-primary
so links adapt to light/dark palettes.-a { - color: #1d7bd7; -} +a { + @apply text-primary; +}
320-324
: Use token-based colors for dropdown menuReplace hard-coded light/dark colors with
popover
tokens to keep the component themed consistently.-.dropdown-menu { - @apply absolute left-0 top-full mt-2 w-48 rounded-lg bg-white p-3 shadow-lg dark:bg-gray-800; - @apply invisible opacity-0 transition-all duration-200 ease-in-out; - @apply flex flex-col space-y-2; -} +.dropdown-menu { + @apply absolute left-0 top-full mt-2 w-48 rounded-lg bg-popover text-popover-foreground p-3 shadow-lg; + @apply invisible opacity-0 transition-all duration-200 ease-in-out; + @apply flex flex-col space-y-2; +}
254-261
: Theme popup text via variables rather than hard-coded black/whiteThis keeps tooltip/popup text consistent with foreground tokens.
-.popup-content { - cursor: pointer; - color: black; -} - -.dark .popup-content { - color: white; -} +.popup-content { + cursor: pointer; + @apply text-foreground; +}
190-197
: Leaflet popup theming: prefer CSS variablesReplace fixed hex colors with
popover
tokens so Leaflet popups match the theme and future palette tweaks.-.dark .leaflet-popup-content-wrapper { - background-color: #2c3e50; - color: white; -} +.dark .leaflet-popup-content-wrapper { + background-color: hsl(var(--popover)); + color: hsl(var(--popover-foreground)); +} @@ -.dark .leaflet-popup-tip { - background-color: #2c3e50; -} +.dark .leaflet-popup-tip { + background-color: hsl(var(--popover)); +}Also applies to: 267-269
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
frontend/src/app/globals.css
(8 hunks)
🔇 Additional comments (3)
frontend/src/app/globals.css (3)
79-86
: Good move: map tiles filter token is now dark-onlyDefining
--map-tiles-filter
inside.dark
prevents leakage into light mode and keeps the theme surface clean. Nice improvement.
172-179
: Base layer consolidation looks goodThe consolidated
@layer base
applyingborder-border
and body theme colors is clean and aligns with the Tailwind variable palette.
106-113
: Footer stick-to-bottom objective: verify layout classes are in placeThe globals look fine, but the sticky-footer behavior depends on layout structure. Ensure RootLayout applies
min-h-screen flex flex-col
to the outer container andflex-1
to<main>
, so the footer naturally sticks to the bottom even during loading.Would you like me to scan the repo for
layout.tsx
andRootLayout
usage to confirm these classes are present and consistent?Also applies to: 172-179
…om/OUMIMANDAL/Nest-milestones into fix-homepage-container-flexibility # Please enter a commit message to explain why this merge is necessary, # especially if it merges an updated upstream into a topic branch. # # Lines starting with '#' will be ignored, and an empty message aborts # the commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
frontend/src/app/globals.css (2)
50-87
: Dark theme tokens and map-tiles filter placement are correct (previous concern resolved)Defining --map-tiles-filter inside .dark and removing the media-query dependency addresses the earlier dark-mode mismatch. Good improvement.
172-179
: Duplicate @layer base block has been removedThere’s now a single @layer base block configuring border color and body theme tokens. This resolves the earlier duplication.
🧹 Nitpick comments (2)
frontend/src/app/globals.css (2)
185-187
: Smooth the dark-mode map-tiles transitionWhen toggling dark mode, the filter applies abruptly. Add a small transition for a nicer UX.
Apply this diff:
- .dark .map-tiles { - filter: var(--map-tiles-filter, none); - } + .dark .map-tiles { + filter: var(--map-tiles-filter, none); + transition: filter 200ms ease; + }
331-334
: Add a light-mode ::selection style for consistencyYou’ve styled selection in dark mode only. Provide a light-mode counterpart to avoid jarring contrast when not in dark mode.
Apply this diff:
@layer utilities { + ::selection { + @apply bg-blue-200 text-blue-900; + } .dark ::selection { @apply bg-blue-500 text-white; } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
frontend/src/app/globals.css
(8 hunks)
🔇 Additional comments (2)
frontend/src/app/globals.css (2)
5-45
: Light theme tokens look solid and align with Tailwind’s CSS-variable paletteThe expanded light token surface under :root is consistent and should work well with hsl(var(--token) / ) mapping in tailwind.config. No issues spotted.
5-45
: Verify sticky‐footer support in your layout and loading UIThe new CSS variables don’t block sticky‐footer behavior, but it relies on your root layout and loading fallbacks spanning the viewport and flexing correctly. Please confirm:
• frontend/src/app/layout.tsx
– The top‐level wrapper (<html>
,<body>
or a div) usesmin-h-screen
(ormin-h-dvh
),flex
andflex-col
.
– The primary content container (<main>
or equivalent) includesflex-1
.• frontend/src/app/**/loading.tsx (all loading files)
– Each loading fallback spans the viewport withmin-h-screen
,min-h-dvh
,h-screen
orh-dvh
.Ensuring these classes are in place (including during loading) will guarantee the footer sticks at the bottom as intended.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (2)
frontend/src/components/RecentIssues.tsx (1)
20-20
: Mark the header icon as decorative for accessibility.Add aria-hidden to avoid redundant announcements by screen readers.
- <FontAwesomeIcon icon={faCircleExclamation} className="h-5 w-5 text-red-500" /> + <FontAwesomeIcon + icon={faCircleExclamation} + className="h-5 w-5 text-red-500" + aria-hidden="true" + />frontend/src/components/IssueMetadata.tsx (1)
24-26
: Treat calendar icon as decorative.Screen readers will get the date text; the icon should be hidden from a11y trees.
- <FontAwesomeIcon icon={faCalendar} className="mr-2 h-4 w-4" /> + <FontAwesomeIcon icon={faCalendar} className="mr-2 h-4 w-4" aria-hidden="true" />
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
frontend/src/components/IssueMetadata.tsx
(1 hunks)frontend/src/components/RecentIssues.tsx
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
frontend/src/components/IssueMetadata.tsx (2)
frontend/src/utils/dateFormatter.ts (1)
formatDate
(1-20)frontend/src/components/TruncatedText.tsx (1)
TruncatedText
(3-45)
frontend/src/components/RecentIssues.tsx (2)
frontend/src/types/issue.ts (1)
Issue
(3-19)frontend/src/components/IssueMetadata.tsx (1)
IssueMetadata
(17-46)
🔇 Additional comments (2)
frontend/src/components/RecentIssues.tsx (2)
7-7
: Nice extraction to a reusable metadata component.Delegating details to IssueMetadata improves cohesion and reduces duplication in RecentIssues.
26-26
: Ensure IssueMetadata is a Client Component (uses router/navigation).IssueMetadata uses client-only behavior (router navigation and likely hooks via TruncatedText). Make sure it’s marked as a client component. See my comment in IssueMetadata.tsx with the concrete fix.
import { faCalendar, faFolderOpen } from '@fortawesome/free-solid-svg-icons'; | ||
import { FontAwesomeIcon } from '@fortawesome/react-fontawesome'; | ||
import { useRouter } from 'next/navigation'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add 'use client' and prefer Link over imperative router.push.
This component uses client-only APIs. Without 'use client', Next.js App Router will throw "useRouter only works in a Client Component." Also, navigation is better handled with a semantic Link, which supports open-in-new-tab, context menus, and default browser behaviors.
+ 'use client';
-import { useRouter } from 'next/navigation';
+import Link from 'next/link';
🤖 Prompt for AI Agents
In frontend/src/components/IssueMetadata.tsx around lines 1 to 3, the component
uses client-only APIs but lacks a "use client" directive and uses an imperative
useRouter(). Replace the file top by adding the "use client" directive, remove
useRouter import and any router.push calls, and instead import Link from
'next/link' and use a semantic <Link href="..."> wrapper for navigable elements
so browser behaviors (open-in-new-tab, context menu) work correctly; update
imports to include Link and remove unused useRouter.
const router = useRouter(); | ||
|
||
return ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Remove unused router and prepare safe route inputs.
Avoid using router directly; compute org/repo once and reuse. This also sets you up for Link-based navigation.
-export const IssueMetadata: React.FC<IssueMetadataProps> = ({ issue }) => {
- const router = useRouter();
+export const IssueMetadata: React.FC<IssueMetadataProps> = ({ issue }) => {
+ // Prefer derived values; tolerate partial data
+ const repoName =
+ (issue as any)?.repository?.name ?? (issue as any)?.repositoryName ?? issue.projectName;
+ const orgName =
+ (issue as any)?.repository?.organizationName ?? issue.organizationName;
+ const href =
+ orgName && repoName
+ ? `/organizations/${orgName}/repositories/${repoName}`
+ : undefined;
Note:
- repositoryName is not present in the Issue type you import; see separate comment below.
- If RepositoryDetails has different field names, adjust the derivation accordingly.
📝 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.
const router = useRouter(); | |
return ( | |
export const IssueMetadata: React.FC<IssueMetadataProps> = ({ issue }) => { | |
// Prefer derived values; tolerate partial data | |
const repoName = | |
(issue as any)?.repository?.name ?? | |
(issue as any)?.repositoryName ?? | |
issue.projectName; | |
const orgName = | |
(issue as any)?.repository?.organizationName ?? issue.organizationName; | |
const href = | |
orgName && repoName | |
? `/organizations/${orgName}/repositories/${repoName}` | |
: undefined; | |
return ( |
🤖 Prompt for AI Agents
In frontend/src/components/IssueMetadata.tsx around lines 18 to 20, remove the
unused useRouter() call and instead compute and memoize the organization and
repository strings once (e.g., derive org and repo from the Issue props or
nested RepositoryDetails fields, handling possible undefineds), then reuse those
computed safe route inputs wherever navigation or Link targets are needed; also
replace any direct router pushes/usages with Link components or functions that
accept the precomputed org/repo, and if RepositoryDetails uses different field
names than repositoryName, adjust the derivation to the actual field names
(guard against null/undefined and normalize strings).
{issue.repositoryName && ( | ||
<div className="flex flex-1 items-center overflow-hidden"> | ||
<FontAwesomeIcon icon={faFolderOpen} className="mr-2 h-5 w-4" /> | ||
<button | ||
className="cursor-pointer overflow-hidden text-ellipsis whitespace-nowrap text-gray-600 hover:underline dark:text-gray-400" | ||
onClick={() => | ||
router.push( | ||
`/organizations/${issue.organizationName}/repositories/${issue.repositoryName}` | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify the correct source of organization and repository names.
The current code references issue.repositoryName and issue.organizationName, but Issue (types/issue.ts) doesn’t declare repositoryName. Confirm the actual location:
- If names live under issue.repository, use repository.name and repository.organizationName.
- If repositoryName is intended, update the Issue type accordingly or adjust this component to use projectName/projectUrl.
Run this script to locate the RepositoryDetails shape and any existing repositoryName usage:
Expected:
- Find RepositoryDetails with fields for name/org if available, or surface missing fields.
- Either zero hits for repositoryName (indicating mismatch) or locations to align with.
- Validate that the repositories route exists and matches the intended params.
---
🏁 Script executed:
```shell
#!/bin/bash
set -euo pipefail
echo "Searching for RepositoryDetails definition and fields..."
rg -n -C3 -g '!**/node_modules/**' -P 'export\s+(type|interface)\s+RepositoryDetails\b|^type\s+RepositoryDetails\b|^interface\s+RepositoryDetails\b'
echo
echo "Any references to repositoryName on Issue objects?"
rg -n -C2 -g '!**/node_modules/**' -P '\brepositoryName\b'
echo
echo "Check routes that match /organizations/:org/repositories/:repo"
rg -n -C3 -g '!**/node_modules/**' -P '(?i)/organizations/.+/repositories/.+'
Length of output: 51941
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for Issue type definition..."
rg -n -C3 -P 'export\s+(type|interface)\s+Issue\b' -g '!**/node_modules/**'
echo
echo "Check for repositoryName in types/issue.ts"
rg -n "repositoryName" -g "frontend/src/types/issue.ts" || echo "No references to repositoryName in types/issue.ts"
echo
echo "Print first 200 lines of types/issue.ts"
if [ -f frontend/src/types/issue.ts ]; then
sed -n '1,200p' frontend/src/types/issue.ts
else
echo "frontend/src/types/issue.ts not found"
fi
Length of output: 1337
Use the correct Issue
fields instead of the non-existent repositoryName
The Issue
type (frontend/src/types/issue.ts) doesn’t declare repositoryName
. It provides:
projectName: string
projectUrl: string
repository?: { key: string; ownerKey: string }
organizationName?: string
Update IssueMetadata.tsx to use those fields and guard against a missing repository
:
• Replace the top-level check:
- {issue.repositoryName && (
+ {issue.repository?.key && (
• Update your route to use repository.key
and repository.ownerKey
(or fall back to organizationName
):
- router.push(
- `/organizations/${issue.organizationName}/repositories/${issue.repositoryName}`
- )
+ router.push(
+ `/organizations/${
+ issue.repository?.ownerKey || issue.organizationName || ''
+ }/repositories/${issue.repository.key}`
+ )
• Swap out the displayed text (or link externally via projectUrl
):
- <TruncatedText text={issue.repositoryName} />
+ <TruncatedText text={issue.projectName} />
• Ensure similar fixes in RecentPullRequests, RecentReleases, Milestones, etc., wherever repositoryName
was used.
📝 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.
{issue.repositoryName && ( | |
<div className="flex flex-1 items-center overflow-hidden"> | |
<FontAwesomeIcon icon={faFolderOpen} className="mr-2 h-5 w-4" /> | |
<button | |
className="cursor-pointer overflow-hidden text-ellipsis whitespace-nowrap text-gray-600 hover:underline dark:text-gray-400" | |
onClick={() => | |
router.push( | |
`/organizations/${issue.organizationName}/repositories/${issue.repositoryName}` | |
) | |
{issue.repository?.key && ( | |
<div className="flex flex-1 items-center overflow-hidden"> | |
<FontAwesomeIcon icon={faFolderOpen} className="mr-2 h-5 w-4" /> | |
<button | |
className="cursor-pointer overflow-hidden text-ellipsis whitespace-nowrap text-gray-600 hover:underline dark:text-gray-400" | |
onClick={() => | |
router.push( | |
`/organizations/${ | |
issue.repository?.ownerKey || issue.organizationName || '' | |
}/repositories/${issue.repository.key}` | |
) | |
} | |
> | |
<TruncatedText text={issue.projectName} /> | |
</button> | |
</div> | |
)} |
🤖 Prompt for AI Agents
In frontend/src/components/IssueMetadata.tsx around lines 29-37, the component
checks and uses a non-existent top-level field repositoryName; update the logic
to check for issue.repository (guarding for undefined) and use
issue.repository.key and issue.repository.ownerKey when building the internal
route (fall back to issue.organizationName if ownerKey is missing), replace the
displayed repository text/link with issue.projectName or link externally to
issue.projectUrl, and ensure all uses are null-checked (e.g., render nothing or
the external projectUrl when repository is absent); also propagate the same
replacements in RecentPullRequests, RecentReleases, Milestones, etc., where
repositoryName was used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
🧹 Nitpick comments (3)
frontend/package.json (2)
62-67
: Duplicate dependency: tailwindcss-animate appears in both dependencies and devDependencies.Keep it only in devDependencies (used at build time via Tailwind config).
- "tailwindcss-animate": "^1.0.7" + "tailwindcss-animate": "^1.0.7"And remove from dependencies:
- "tailwindcss-animate": "^1.0.7"
48-49
: ESLint plugin should be a devDependency, not a runtime dependency.
eslint-plugin-import
is a lint-time dependency; move it under devDependencies to keep the runtime surface minimal.frontend/tailwind.config.js (1)
62-70
: Nit: animation speed likely too fast for continuous scroll.
scroll: "scroll 0.5s linear infinite"
will be extremely fast. If this is meant for marquee-like UI, consider 5–20s depending on content width.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
frontend/pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
frontend/package.json
(6 hunks)frontend/scripts/generate-tailwind-config.ts
(1 hunks)frontend/tailwind.config.cjs
(1 hunks)frontend/tailwind.config.js
(1 hunks)frontend/tsconfig.tsnode.json
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- frontend/tsconfig.tsnode.json
🧰 Additional context used
🪛 Biome (2.1.2)
frontend/tailwind.config.js
[error] 1-1: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 5-5: type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
🔇 Additional comments (5)
frontend/scripts/generate-tailwind-config.ts (1)
20-22
: Potential ESM/CJS __dirname pitfall.This script uses
__dirname
, which is only defined when the script runs under CommonJS. Verifyts-node
compiles this script to CJS (e.g., tsconfig.tsnode.json sets"module": "CommonJS"
). If it’s ESM, switch toimport.meta.url
.Suggested ESM-safe alternative:
const here = typeof __dirname !== 'undefined' ? __dirname : path.dirname(new URL(import.meta.url).pathname) const outputPath = path.resolve(here, '../tailwind.config.generated.js')frontend/package.json (1)
123-125
: Node 22 with Jest 29: check compatibility.Jest 29’s official Node support predates Node 22. It may work but is not guaranteed. If you observe odd test failures, consider either downgrading Node for tests (e.g., 18/20) or upgrading Jest to a version that supports Node 22.
frontend/tailwind.config.js (3)
6-11
: LGTM on content globs and plugin registration.Content globs include src and heroui theme paths; plugin list correctly registers
heroui()
andtailwindcss-animate
. Once the export/import issues above are addressed, this config should behave as intended.
14-61
: Colors/radius tokens migration looks correct.Switch to
hsl(var(--token))
is consistent with modern theming; customowasp-blue
preserved. Border radii mapping to CSS variables is fine.
7-9
: Tailwind v4 note: content key optional, but harmless.Tailwind v4 can work without a config and without explicit
content
in many setups. Keeping explicit globs is fine and sometimes necessary with UI libraries.
// scripts/generate-tailwind-config.ts | ||
import fs from 'fs'; | ||
import path from 'path'; | ||
import tailwindConfig from '../tailwind.config.js'; // ← now .js |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Incorrect import target: this script imports .js while the config uses TypeScript-only syntax.
../tailwind.config.js
contains TS-only syntax and ESM export; importing it will throw under ts-node/CommonJS. If you keep the config in TS, import the TS file instead.
-import tailwindConfig from '../tailwind.config.js'; // ← now .js
+// Import the TS config directly; ts-node project should enable allowImportingTsExtensions
+import tailwindConfig from '../tailwind.config.ts';
📝 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.
import tailwindConfig from '../tailwind.config.js'; // ← now .js | |
// Import the TS config directly; ts-node project should enable allowImportingTsExtensions | |
import tailwindConfig from '../tailwind.config.ts'; |
🤖 Prompt for AI Agents
In frontend/scripts/generate-tailwind-config.ts around line 5, the script
imports ../tailwind.config.js but that file contains TypeScript-only syntax and
ESM export which will throw under ts-node/CommonJS; update the import to point
to the TypeScript source (../tailwind.config.ts) or otherwise load the config
via a TS-aware loader (e.g., register ts-node or compile the config to JS) so
the runtime can parse the TS/ESM syntax — ensure the import path, runtime
loader, and any build scripts are adjusted consistently.
frontend/tailwind.config.js
Outdated
import type { Config } from "tailwindcss"; | ||
import { heroui } from "@heroui/react"; | ||
import animate from "tailwindcss-animate"; | ||
|
||
const config: Config = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Address static analysis errors (Biome).
Biome errors are accurate: TS-only syntax in .js
. Fix via one of the refactors above (convert to .ts
or to valid CommonJS/JS).
🧰 Tools
🪛 Biome (2.1.2)
[error] 1-1: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 5-5: type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
🤖 Prompt for AI Agents
In frontend/tailwind.config.js around lines 1 to 5, the file uses
TypeScript-only syntax (import type and type annotation) in a .js file causing
Biome/TS static analysis errors; fix by either renaming the file to
tailwind.config.ts and keep the current ES module imports plus change the final
export to an "export default config" with the appropriate Config type, or keep
the .js extension and convert to valid JS/CommonJS by removing "import type",
replacing type imports with regular imports or omitting types, change "import"
to require (or ensure ESM is enabled), and replace the TypeScript type
annotation on config with plain object and export using module.exports (or
export default if ESM). Apply only one of these refactors consistently and
update any build config if needed.
frontend/tailwind.config.js
Outdated
import type { Config } from "tailwindcss"; | ||
import { heroui } from "@heroui/react"; | ||
import animate from "tailwindcss-animate"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
TS-only syntax in a .js file breaks parsing.
import type
and ESM import
in a .js
file (with no "type": "module"
) will fail for Node/tools. Either convert this file to .ts
or make it valid CommonJS/JS.
Two safe options:
- Option A (recommended for broad tooling): Convert to CommonJS with JSDoc typing.
-import type { Config } from "tailwindcss";
-import { heroui } from "@heroui/react";
-import animate from "tailwindcss-animate";
+// @ts-check
+const { heroui } = require("@heroui/react");
+const animate = require("tailwindcss-animate");
📝 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.
import type { Config } from "tailwindcss"; | |
import { heroui } from "@heroui/react"; | |
import animate from "tailwindcss-animate"; | |
// @ts-check | |
const { heroui } = require("@heroui/react"); | |
const animate = require("tailwindcss-animate"); |
🧰 Tools
🪛 Biome (2.1.2)
[error] 1-1: 'import type' are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
🤖 Prompt for AI Agents
In frontend/tailwind.config.js around lines 1 to 3, the file uses
TypeScript-only "import type" and ESM imports which break parsing for a .js
file; either rename the file to .ts or make it valid CommonJS. Recommended fix:
convert imports to CommonJS requires and add JSDoc @type annotation for the
Tailwind Config (e.g., require("tailwindcss-animate") and
require("@heroui/react")), replace "import type { Config }" with a JSDoc comment
specifying the Config type, and export the config with module.exports so
tooling/node can parse it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
frontend/tailwind.config.ts (4)
5-5
: Prefer “satisfies Config” over a type annotation for better inferenceUsing “satisfies” preserves literal types and catches shape errors without losing inference on nested objects. It’s the recommended pattern for Tailwind TS configs.
Apply this minimal diff:
-const config: Config = { +const config = { @@ -}; +} satisfies Config;Also applies to: 74-74
6-9
: Content globs: scope is fine; consider the trade-off of scanning node_modulesIncluding only “@heroui/theme/dist” is reasonable, but scanning any node_modules subtree can slow JIT. If builds get sluggish, consider narrowing the pattern further or rely on HeroUI’s documented content path exactly (and keep everything else under ./src).
Example alternative (equivalent but a bit tighter):
- "./node_modules/@heroui/theme/dist/**/*.{js,ts,jsx,tsx}", + "node_modules/@heroui/theme/dist/**/*.{js,ts,jsx,tsx}",
14-18
: “text” color token may lead to awkward class namesDefining a color key named “text” creates classes like “text-text”, which is easy to misread and inconsistent with common schemes (“foreground”, “muted-foreground”, etc.). If this is intentional, ignore. Otherwise, consider removing or renaming.
Two options:
- Keep “foreground” only and drop “text”:
- text: "hsl(var(--text))",
- Or rename for clarity:
- text: "hsl(var(--text))", + body: "hsl(var(--text))",If “text-text” is already used in src, keep as-is.
Could you confirm whether “text-text” is used in the codebase? If not, I’d drop/rename it to avoid confusion.
62-70
: Animation “scroll” is extremely fast and distance is brittle
- 0.5s linear infinite will look like a strobe on most screens and can be a11y-hostile.
- Hard-coding translateX(-500%) tightly couples the keyframe to content width; it’ll tear if the content doesn’t match that width assumption.
Minimal, non-breaking tweak (keep the “scroll” name):
- scroll: { - "0%": { transform: "translateX(0)" }, - "100%": { transform: "translateX(-500%)" }, - }, + scroll: { + "0%": { transform: "translateX(0)" }, + "100%": { transform: "translateX(-100%)" }, + }, @@ - scroll: "scroll 0.5s linear infinite", + // Respect OS/user prefs by allowing components to add `motion-reduce:animate-none`. + // Make speed configurable via CSS var with a sane default. + scroll: "scroll var(--scroll-duration, 24s) linear infinite",If you can accommodate a rename, “marquee” or “ticker” better conveys the intent and avoids potential naming ambiguity with “scroll” utilities.
Please confirm whether any components already use “animate-scroll”; if so, keep the name and just adopt the speed/distance tweaks.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
frontend/tailwind.config.cjs
(1 hunks)frontend/tailwind.config.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/tailwind.config.cjs
🔇 Additional comments (6)
frontend/tailwind.config.ts (6)
1-3
: Imports and plugin setup look correctTyped import for Config, HeroUI plugin, and tailwindcss-animate are all good and idiomatic for a TS Tailwind config. No action needed.
18-18
: Brand color check for “owasp-blue”Hex “#98AFC7” reads as a light steel/air force blue. Please verify this matches OWASP brand guidance and passes contrast where used (especially on light backgrounds).
If helpful, I can script a quick WCAG contrast check against your typical backgrounds once you share the intended usage contexts.
19-56
: Color token set is comprehensive and consistentThe token structure mirrors common UI kits (card/popover/primary/secondary/muted/accent/destructive/input/ring/chart). Good alignment with CSS vars for runtime theming.
57-61
: Border radius via CSS vars is clean and flexibleThe lg/md/sm mapping to --radius is a good choice for themeability.
73-73
: Plugin order and selection look goodHeroUI first, then tailwindcss-animate is fine. No conflicts spotted.
76-76
: Build usage: ensure Tailwind actually consumes this TS configGiven you’re exporting a TS default, confirm your Tailwind CLI/build points at a CJS/JS artifact (or uses your generator) rather than importing “.ts” directly. Otherwise, Tailwind may fail to load the config in non-ts-node environments.
If you want, I can outline a small check to ensure a utility like “bg-primary” compiles to the expected HSL at build time.
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 3.29.9 to 3.29.10. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@df55935...96f518a) --- updated-dependencies: - dependency-name: github/codeql-action dependency-version: 3.29.10 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Added test for RecentIssues Component * Updated code format * update missing title test --------- Co-authored-by: Kate Golovanova <kate@kgthreads.com>
* Integrate lighthouseci * use nest.owasp.dev * run lighthouse after staging deployment * Update code * Update code temporarily * run lighthouse after staging deployment * remove github app environment variable and test * run lighthouse after staging deployment * Update code --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
frontend/package.json (1)
73-80
: Remove unused ts-jest dependency
jest.config.ts is configured to use only the @swc/jest transformer (no ts-jest preset or transform), so drop ts-jest from devDependencies in frontend/package.json (remove the"ts-jest": "^29.4.1"
entry)..pre-commit-config.yaml (1)
12-19
: Align Ruff versions between pre-commit and backend
Pre-commit hook rev is v0.12.9 while backend’s pyproject.toml pins ruff to ^0.12.8; update one to match the other (e.g. bump backend ruff to ^0.12.9 or pin hook rev to v0.12.8).backend/pyproject.toml (4)
125-182
: Second [tool.ruff] block duplicates the first. Keep only one unified block.Pick this block or the earlier one, not both. Prefer keeping a single block here and deleting the earlier one.
272-279
: Corrupted block: mypy overrides lines ended up under [tool.djlint].Lines 277–279 belong to mypy, not djlint. Move or delete; currently this makes config inconsistent.
[tool.djlint] format = true format_css = true format_js = true indent = 4 - disable_error_code = [ "return-value", "attr-defined", "misc" ] - module = [ "apps.*.api.internal.queries.*", "apps.*.api.internal.nodes.*" ]
280-285
: Duplicate [tool.djlint] table.Keep only one djlint table with the desired options.
-[tool.djlint] -format = true -format_css = true -format_js = true -indent = 4
1-285
: Fix duplicate keys in pyproject.toml (causes parse errors)
- Remove repeated definitions (e.g.
authors
,packages
,django-storages
,[tool.ruff]
,[tool.coverage.run]
,[[tool.mypy.overrides]]
) so the file parses successfully- Add a CI validation step—e.g. run a short Python script using
tomllib.loads()
—to catch duplicate-key errors on push
♻️ Duplicate comments (3)
frontend/package.json (3)
5-10
: Run Tailwind config generator before dev/build.Add pre-scripts so the generated CJS exists when starting dev/build.
"scripts": { "build:tailwind": "ts-node --project tsconfig.tsnode.json scripts/generate-tailwind-config.ts", + "predev": "pnpm build:tailwind", + "prebuild": "pnpm build:tailwind", "build": "next build", "build:turbo": "next build --turbo", "dev": "next dev --port 3000",
18-18
: cross-env used but not declared — tests will fail in CI.Add cross-env to devDependencies.
"devDependencies": { + "cross-env": "^7.0.3",
Also applies to: 69-118
62-68
: Duplicate tailwindcss-animate in deps and devDeps. Keep only in devDeps.Remove runtime copy.
"dependencies": { @@ - "tailwindcss-animate": "^1.0.7" + },Also applies to: 111-112
🧹 Nitpick comments (7)
.github/workflows/run-code-ql.yaml (1)
36-41
: Scope pnpm installation to JS/TS matrix to save time.On the Python leg we still install pnpm unnecessarily. Gate this step like Node setup.
- - name: Install pnpm + - name: Install pnpm + if: matrix.language == 'javascript-typescript' uses: pnpm/action-setup@a7487c7e89a18df4991f7f222e4898a00d66ddda with: version: 10 run_install: falsefrontend/lighthouserc.json (1)
1-19
: Solid baseline; consider flake controls and upload policy.
- To reduce flakiness, pin throttling and form factor.
- Confirm public upload is acceptable for this project.
Proposed tweak:
"ci": { "assert": { "assertions": { "categories:performance": ["warn", { "minScore": 0.9 }], "categories:accessibility": ["warn", { "minScore": 0.9 }], "categories:best-practices": ["warn", { "minScore": 0.9 }], "categories:seo": ["warn", { "minScore": 0.9 }] } }, "collect": { - "url": ["https://nest.owasp.dev/"], - "numberOfRuns": 3 + "url": ["https://nest.owasp.dev/"], + "numberOfRuns": 3, + "settings": { + "formFactor": "desktop", + "screenEmulation": { "mobile": false, "width": 1366, "height": 768, "deviceScaleFactor": 1, "disabled": false }, + "throttlingMethod": "devtools" + } }, "upload": { "target": "temporary-public-storage" } }frontend/Makefile (1)
30-32
: Parametrize and preflight LHCI target.Make the target reusable and fail fast if the dev server isn’t up.
-lighthouse-ci: - @cd frontend && npx -y @lhci/cli@0.15.1 autorun --collect.url=http://localhost:3000 +LHCI_URL ?= http://localhost:3000 +lighthouse-ci: + @curl -sSf -o /dev/null $(LHCI_URL) || (echo "ERROR: $(LHCI_URL) is not reachable"; exit 1) + @cd frontend && npx -y @lhci/cli@0.15.1 autorun --collect.url=$(LHCI_URL).github/workflows/run-ci-cd.yaml (1)
500-504
: LGTM: switch to pinned LHCI CLI via npx.Consistent with frontend config and Makefile. Consider adding a retry for transient network issues if LHCI proves flaky on runners.
Example:
- - name: Run Lighthouse CI + - name: Run Lighthouse CI working-directory: frontend - run: | - npx -y @lhci/cli@0.15.1 autorun + run: | + npx -y @lhci/cli@0.15.1 autorun || npx -y @lhci/cli@0.15.1 autorunfrontend/package.json (2)
119-121
: Engines too strict: "node": "22" may block installs on Node 20 LTS.Unless CI enforces Node 22 everywhere, relax to a range.
- "engines": { - "node": "22" - }, + "engines": { + "node": ">=20" + },
90-90
: autoprefixer likely redundant with @tailwindcss/postcss.Tailwind v4 pipeline bundles Autoprefixer via @tailwindcss/postcss. Consider removing to reduce noise.
- "autoprefixer": "^10.4.21",
Also applies to: 107-111
schema/utils/schema_validators.py (1)
9-9
: Dependency check: ensurevalidators
is declared and pinned.Add
validators
to your Python dependency manifest (requirements*.txt
orpyproject.toml
) with an appropriate version pin to avoid runtime import errors and drift.Would you like a script to scan the repo for dependency manifests and verify the pin?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (4)
backend/poetry.lock
is excluded by!**/*.lock
cspell/pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
docs/poetry.lock
is excluded by!**/*.lock
frontend/pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (9)
.github/workflows/run-ci-cd.yaml
(1 hunks).github/workflows/run-code-ql.yaml
(2 hunks).pre-commit-config.yaml
(1 hunks)backend/pyproject.toml
(7 hunks)frontend/Makefile
(1 hunks)frontend/lighthouserc.json
(1 hunks)frontend/package.json
(6 hunks)frontend/src/app/layout.tsx
(2 hunks)schema/utils/schema_validators.py
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/src/app/layout.tsx
🔇 Additional comments (6)
.github/workflows/run-code-ql.yaml (2)
31-36
: Good: CodeQL actions pinned to commit SHAs.Pinning
init
to a specific SHA is great for supply-chain security. Keepinit
andanalyze
in lockstep when bumping.
55-58
: Good: Analyze action pinned; category wiring is correct.
category: /language:${{ matrix.language }}
matches the matrix; no issues.frontend/package.json (2)
103-106
: Jest/jsdom versions aligned. LGTM.
65-65
: Remove unused react-router-dom dependency from package.json
No imports or requires of react-router-dom were found in the codebase—safely drop it to trim dependencies.backend/pyproject.toml (1)
50-52
: Version downgrade: strawberry-graphql ^0.278.1 — verify compatibility. Scanned for strawberry.{type,field,permission,experimental} usages; none appear to require APIs introduced after 0.278.1. Confirm schema/types and mypy plugin behavior.schema/utils/schema_validators.py (1)
1-52
: Scope sanity check: Python backend utility in a frontend layout PR.This module appears unrelated to the PR’s stated scope (homepage layout/tailwind). Confirm inclusion was intentional; otherwise, move it to a separate PR to keep changes focused.
authors = [ "Arkadii Yakovets <arkadii.yakovets@owasp.org>" ] | ||
readme = "README.md" | ||
|
||
packages = [ { include = "apps" } ] | ||
packages = [ { include = "apps" } ] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate keys in [tool.poetry]: authors and packages are repeated (invalid TOML).
TOML forbids redefining the same key/table; Poetry will fail to parse. Remove the duplicates.
Apply:
- authors = [ "Arkadii Yakovets <arkadii.yakovets@owasp.org>" ]
- packages = [ { include = "apps" } ]
🤖 Prompt for AI Agents
In backend/pyproject.toml around lines 10 to 15 there are duplicate keys
(authors and packages) which is invalid TOML; remove the repeated entries so
each key appears only once — keep the correct single authors line and a single
packages table (or merge their contents if they differ), and ensure the
resulting TOML is syntactically valid (no duplicate keys).
django-storages = { extras = [ "s3" ], version = "^1.14.4" } | ||
django-storages = { extras = [ "s3" ], version = "^1.14.4" } | ||
emoji = "^2.14.1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate dependency entry: django-storages repeated.
Keep a single line for the package.
- django-storages = { extras = [ "s3" ], version = "^1.14.4" }
📝 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.
django-storages = { extras = [ "s3" ], version = "^1.14.4" } | |
django-storages = { extras = [ "s3" ], version = "^1.14.4" } | |
emoji = "^2.14.1" | |
django-storages = { extras = [ "s3" ], version = "^1.14.4" } | |
emoji = "^2.14.1" |
🤖 Prompt for AI Agents
In backend/pyproject.toml around lines 24 to 26 there is a duplicate dependency
entry for django-storages; remove the redundant line so only a single
django-storages = { extras = ["s3"], version = "^1.14.4" } remains (preserve the
extras and version exactly) and keep the subsequent emoji = "^2.14.1" line
unchanged.
[tool.ruff] | ||
target-version = "py313" | ||
|
||
line-length = 99 | ||
lint.select = [ "ALL" ] | ||
lint.extend-select = [ "I" ] | ||
lint.ignore = [ | ||
"ANN", # https://docs.astral.sh/ruff/rules/#flake8-annotations-ann/ | ||
"ARG002", # https://docs.astral.sh/ruff/rules/unused-method-argument/ | ||
"C901", # https://docs.astral.sh/ruff/rules/complex-structure/ | ||
"COM812", # https://docs.astral.sh/ruff/rules/missing-trailing-comma/ | ||
"D407", # https://docs.astral.sh/ruff/rules/missing-dashed-underline-after-section/ | ||
"DJ012", # https://docs.astral.sh/ruff/rules/django-unordered-body-content-in-model/ | ||
"FIX002", # https://docs.astral.sh/ruff/rules/line-contains-todo/ | ||
"PLC0415", # https://docs.astral.sh/ruff/rules/import-outside-top-level/ | ||
"PLR0912", # https://docs.astral.sh/ruff/rules/too-many-branches/ | ||
"PLR0913", # https://docs.astral.sh/ruff/rules/too-many-arguments/ | ||
"PLR0915", # https://docs.astral.sh/ruff/rules/too-many-statements/ | ||
"RUF012", # https://docs.astral.sh/ruff/rules/mutable-class-default/ | ||
"TD003", # https://docs.astral.sh/ruff/rules/missing-todo-link/ | ||
] | ||
lint.per-file-ignores."**/__init__.py" = [ | ||
"D104", # https://docs.astral.sh/ruff/rules/undocumented-public-package/ | ||
"F401", # https://docs.astral.sh/ruff/rules/unused-import/ | ||
] | ||
lint.per-file-ignores."**/management/commands/*.py" = [ | ||
"D101", # https://docs.astral.sh/ruff/rules/undocumented-public-class/ | ||
"D102", # https://docs.astral.sh/ruff/rules/undocumented-public-method/ | ||
"T201", # https://docs.astral.sh/ruff/rules/print/ | ||
] | ||
lint.per-file-ignores."**/migrations/*.py" = [ | ||
"D100", # https://docs.astral.sh/ruff/rules/undocumented-public-module/ | ||
"D101", # https://docs.astral.sh/ruff/rules/undocumented-public-class/ | ||
"D104", # https://docs.astral.sh/ruff/rules/undocumented-public-package/ | ||
"E501", # https://docs.astral.sh/ruff/rules/line-too-long/ | ||
"RUF012", # https://docs.astral.sh/ruff/rules/mutable-class-default/ | ||
] | ||
lint.per-file-ignores."**/models/*.py" = [ | ||
"D106", # https://docs.astral.sh/ruff/rules/undocumented-public-nested-class/ | ||
] | ||
lint.per-file-ignores."**/rest/v1/*.py" = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate [tool.ruff] block begins here; another appears below — this is invalid TOML.
Consolidate to a single [tool.ruff] table. Decide on one source of truth.
-[tool.ruff]
-target-version = "py313"
-
-line-length = 99
-lint.select = [ "ALL" ]
-lint.extend-select = [ "I" ]
-lint.ignore = [
- "ANN",
- "ARG002",
- "C901",
- "COM812",
- "D407",
- "DJ012",
- "FIX002",
- "PLC0415",
- "PLR0912",
- "PLR0913",
- "PLR0915",
- "RUF012",
- "TD003",
-]
-lint.per-file-ignores."**/__init__.py" = [ "D104", "F401" ]
-lint.per-file-ignores."**/management/commands/*.py" = [ "D101", "D102", "T201" ]
-lint.per-file-ignores."**/migrations/*.py" = [ "D100", "D101", "D104", "E501", "RUF012" ]
-lint.per-file-ignores."**/models/*.py" = [ "D106" ]
-lint.per-file-ignores."**/rest/v1/*.py" = [ "ARG001", "B008" ]
-lint.per-file-ignores."**/settings/*.py" = [ "RUF012" ]
-lint.per-file-ignores."**/tests/**/*.py" = [
- "D100","D101","D102","D103","D107","PLR2004","RUF012","S101","SLF001",
-]
📝 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.
[tool.ruff] | |
target-version = "py313" | |
line-length = 99 | |
lint.select = [ "ALL" ] | |
lint.extend-select = [ "I" ] | |
lint.ignore = [ | |
"ANN", # https://docs.astral.sh/ruff/rules/#flake8-annotations-ann/ | |
"ARG002", # https://docs.astral.sh/ruff/rules/unused-method-argument/ | |
"C901", # https://docs.astral.sh/ruff/rules/complex-structure/ | |
"COM812", # https://docs.astral.sh/ruff/rules/missing-trailing-comma/ | |
"D407", # https://docs.astral.sh/ruff/rules/missing-dashed-underline-after-section/ | |
"DJ012", # https://docs.astral.sh/ruff/rules/django-unordered-body-content-in-model/ | |
"FIX002", # https://docs.astral.sh/ruff/rules/line-contains-todo/ | |
"PLC0415", # https://docs.astral.sh/ruff/rules/import-outside-top-level/ | |
"PLR0912", # https://docs.astral.sh/ruff/rules/too-many-branches/ | |
"PLR0913", # https://docs.astral.sh/ruff/rules/too-many-arguments/ | |
"PLR0915", # https://docs.astral.sh/ruff/rules/too-many-statements/ | |
"RUF012", # https://docs.astral.sh/ruff/rules/mutable-class-default/ | |
"TD003", # https://docs.astral.sh/ruff/rules/missing-todo-link/ | |
] | |
lint.per-file-ignores."**/__init__.py" = [ | |
"D104", # https://docs.astral.sh/ruff/rules/undocumented-public-package/ | |
"F401", # https://docs.astral.sh/ruff/rules/unused-import/ | |
] | |
lint.per-file-ignores."**/management/commands/*.py" = [ | |
"D101", # https://docs.astral.sh/ruff/rules/undocumented-public-class/ | |
"D102", # https://docs.astral.sh/ruff/rules/undocumented-public-method/ | |
"T201", # https://docs.astral.sh/ruff/rules/print/ | |
] | |
lint.per-file-ignores."**/migrations/*.py" = [ | |
"D100", # https://docs.astral.sh/ruff/rules/undocumented-public-module/ | |
"D101", # https://docs.astral.sh/ruff/rules/undocumented-public-class/ | |
"D104", # https://docs.astral.sh/ruff/rules/undocumented-public-package/ | |
"E501", # https://docs.astral.sh/ruff/rules/line-too-long/ | |
"RUF012", # https://docs.astral.sh/ruff/rules/mutable-class-default/ | |
] | |
lint.per-file-ignores."**/models/*.py" = [ | |
"D106", # https://docs.astral.sh/ruff/rules/undocumented-public-nested-class/ | |
] | |
lint.per-file-ignores."**/rest/v1/*.py" = [ |
🤖 Prompt for AI Agents
In backend/pyproject.toml around lines 67-107 there is a duplicate [tool.ruff]
table which makes the TOML invalid; consolidate into a single [tool.ruff] table
by merging rules, options, and all lint.per-file-ignores entries under one
header, remove the redundant [tool.ruff] block, ensure no conflicting keys
remain (resolve by choosing the intended value) and keep the combined
configuration syntactically valid TOML.
"--cov-config=pyproject.toml", | ||
"--cov-fail-under=80", | ||
"--cov-precision=2", | ||
"--cov-report=term-missing", | ||
"--cov=.", | ||
"--dist=loadscope", | ||
"--durations-min=1", | ||
"--durations=5", | ||
"--no-cov-on-fail", | ||
"--numprocesses=auto", | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pytest addopts duplicated verbatim.
Remove the second copy to avoid confusing downstream tooling that parses arrays.
- "--cov-config=pyproject.toml",
- "--cov-fail-under=80",
- "--cov-precision=2",
- "--cov-report=term-missing",
- "--cov=.",
- "--dist=loadscope",
- "--durations-min=1",
- "--durations=5",
- "--no-cov-on-fail",
- "--numprocesses=auto",
📝 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.
"--cov-config=pyproject.toml", | |
"--cov-fail-under=80", | |
"--cov-precision=2", | |
"--cov-report=term-missing", | |
"--cov=.", | |
"--dist=loadscope", | |
"--durations-min=1", | |
"--durations=5", | |
"--no-cov-on-fail", | |
"--numprocesses=auto", | |
] | |
] |
🤖 Prompt for AI Agents
In backend/pyproject.toml around lines 198 to 208, the pytest addopts array is
duplicated verbatim; remove the second copy of the "--cov-..."/related entries
so the addopts array contains each option only once, ensuring downstream tooling
parses a single, non-duplicated list.
"ignore::DeprecationWarning:pytest_cov", | ||
"ignore::DeprecationWarning:xdist", | ||
"ignore::django.conf.RemovedInDjango60Warning:django", | ||
"ignore::pydantic.warnings.PydanticDeprecatedSince20", | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Duplicate filterwarnings entries.
Delete the repeated block to keep settings DRY.
- "ignore::DeprecationWarning:pytest_cov",
- "ignore::DeprecationWarning:xdist",
- "ignore::django.conf.RemovedInDjango60Warning:django",
- "ignore::pydantic.warnings.PydanticDeprecatedSince20",
📝 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.
"ignore::DeprecationWarning:pytest_cov", | |
"ignore::DeprecationWarning:xdist", | |
"ignore::django.conf.RemovedInDjango60Warning:django", | |
"ignore::pydantic.warnings.PydanticDeprecatedSince20", | |
] | |
] |
🤖 Prompt for AI Agents
In backend/pyproject.toml around lines 214 to 218 there are duplicate
filterwarnings entries; remove the repeated block so each warning filter appears
only once (keep a single canonical filterwarnings list with unique entries), and
verify the TOML array syntax remains valid after deletion.
@@ -3,6 +3,7 @@ | |||
"version": "0.1.0", | |||
"private": true, | |||
"scripts": { | |||
"build:tailwind": "ts-node --project tsconfig.tsnode.json scripts/generate-tailwind-config.ts", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify referenced generator files exist.
Ensure tsconfig.tsnode.json and scripts/generate-tailwind-config.ts are present.
🏁 Script executed:
#!/bin/bash
fd -a 'tsconfig.tsnode.json' frontend || true
fd -a 'generate-tailwind-config.ts' frontend || true
Length of output: 131
Add or fix generate-tailwind-config.ts path The file frontend/scripts/generate-tailwind-config.ts
is missing; ensure it’s added or update the "build:tailwind"
command to point to the correct script path.
🤖 Prompt for AI Agents
In frontend/package.json around line 6 the "build:tailwind" script references
frontend/scripts/generate-tailwind-config.ts which is missing; either add the
script file at that exact path (frontend/scripts/generate-tailwind-config.ts) or
update the package.json entry to point to the real location of the generator
(e.g., where the existing generate-tailwind-config.ts lives), and verify the
ts-node project config (tsconfig.tsnode.json) and file extension are correct so
the script executes successfully.
@format_checker.checks("email") | ||
def check_email_format(value): | ||
return validators.email(value) | ||
|
||
|
||
@format_checker.checks("uri") | ||
def check_uri_format(value): | ||
return validators.url(value) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Return explicit booleans from format checks; reconsider overriding uri
semantics.
validators.email/url
return True
or ValidationFailure
(falsy). Cast to bool
for clarity. Also, validators.url
enforces “URL”, which is stricter/different than JSON Schema’s RFC 3986 “uri” (e.g., may reject urn:
, mailto:
). Prefer the default FormatChecker
“uri” or implement with rfc3986
.
@format_checker.checks("email")
def check_email_format(value):
- return validators.email(value)
+ return bool(validators.email(value))
@@
@format_checker.checks("uri")
def check_uri_format(value):
- return validators.url(value)
+ # Consider removing this override to use the default 'uri' checker.
+ return bool(validators.url(value))
If you choose to rely on the default uri
checker, remove the entire check_uri_format
function and its decorator.
📝 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.
@format_checker.checks("email") | |
def check_email_format(value): | |
return validators.email(value) | |
@format_checker.checks("uri") | |
def check_uri_format(value): | |
return validators.url(value) | |
@format_checker.checks("email") | |
def check_email_format(value): | |
return bool(validators.email(value)) | |
@format_checker.checks("uri") | |
def check_uri_format(value): | |
# Consider removing this override to use the default 'uri' checker. | |
return bool(validators.url(value)) |
🤖 Prompt for AI Agents
In schema/utils/schema_validators.py around lines 19 to 27, the format check
functions currently return validators.email/validators.url values which can be
ValidationFailure objects (falsy) instead of explicit booleans; change
check_email_format to return bool(validators.email(value)). For the URI check,
either remove the entire check_uri_format function and its decorator to rely on
jsonschema.FormatChecker's default RFC3986 "uri" behavior, or replace the
implementation to validate against RFC3986 explicitly (e.g., using rfc3986
parser/validator) and return a boolean; do not leave validators.url as-is.
@lru_cache | ||
def get_registry(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Decorator misuse: @lru_cache must be called; current code will crash at runtime.
Using @lru_cache
without parentheses wraps get_registry
in the undecorated factory, leading to a TypeError
when called. Use @lru_cache(maxsize=1)
(or @lru_cache()
).
-@lru_cache
-def get_registry():
+@lru_cache(maxsize=1)
+def get_registry():
📝 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.
@lru_cache | |
def get_registry(): | |
@lru_cache(maxsize=1) | |
def get_registry(): |
🤖 Prompt for AI Agents
In schema/utils/schema_validators.py around lines 29-30, the function is
decorated with @lru_cache without calling it which will wrap the function in the
decorator factory and cause a TypeError at runtime; update the decorator to call
lru_cache (e.g. change to @lru_cache() or @lru_cache(maxsize=1)) so the function
is properly cached, and ensure the functools.lru_cache import is present.
return Registry().with_resource( | ||
COMMON_JSON, | ||
Resource.from_contents( | ||
json.load( | ||
Path.open(f"{Path(__file__).parent.parent.resolve()}/{COMMON_JSON}"), | ||
), | ||
), | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Fix path handling and file I/O; current code calls Path.open incorrectly and leaks the file handle.
Path.open(...)
is an instance method; passing a string path to the class method is invalid. Also, use a context manager and explicit encoding. While here, simplify path building.
Ensure schema/common.json
exists relative to this file and that its $id
(if present) matches "common.json"
to align with the registry key.
- return Registry().with_resource(
- COMMON_JSON,
- Resource.from_contents(
- json.load(
- Path.open(f"{Path(__file__).parent.parent.resolve()}/{COMMON_JSON}"),
- ),
- ),
- )
+ common_path = (Path(__file__).parent.parent / COMMON_JSON)
+ with common_path.open("r", encoding="utf-8") as f:
+ contents = json.load(f)
+ return Registry().with_resource(COMMON_JSON, Resource.from_contents(contents))
Optional (packaging-friendly): use importlib.resources
to read common.json
from the installed package instead of filesystem paths.
🏁 Script executed:
#!/bin/bash
# Locate schema_validators.py and print lines 1-100
file=$(fd --max-depth 4 schema_validators.py)
if [ -z "$file" ]; then
echo "File not found"
exit 1
fi
sed -n '1,100p' "$file"
Length of output: 1258
🏁 Script executed:
#!/bin/bash
# Find common.json and inspect for $id field
paths=$(fd --max-depth 3 -t f common.json)
echo "common.json paths:"
echo "$paths"
for p in $paths; do
echo "-----"
echo "Contents of $p (first 20 lines):"
head -n 20 "$p"
echo "-----"
echo "Searching for \"$id\" in $p:"
grep -n '"\$id"' "$p" || echo "No $id field found"
done
Length of output: 103
Fix resource loading for common.json
and ensure file presence
Refactor get_registry
to use a Path
instance with a context manager and explicit encoding:
- return Registry().with_resource(
- COMMON_JSON,
- Resource.from_contents(
- json.load(
- Path.open(f"{Path(__file__).parent.parent.resolve()}/{COMMON_JSON}"),
- ),
- ),
- )
+ common_path = Path(__file__).parent.parent / COMMON_JSON
+ with common_path.open("r", encoding="utf-8") as f:
+ contents = json.load(f)
+ return Registry().with_resource(COMMON_JSON, Resource.from_contents(contents))
common.json
is not present in schema/
or at the repo root—add it at schema/common.json
(or adjust the path), and if it defines a $id
, ensure it matches "common.json"
.
📝 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.
return Registry().with_resource( | |
COMMON_JSON, | |
Resource.from_contents( | |
json.load( | |
Path.open(f"{Path(__file__).parent.parent.resolve()}/{COMMON_JSON}"), | |
), | |
), | |
) | |
common_path = Path(__file__).parent.parent / COMMON_JSON | |
with common_path.open("r", encoding="utf-8") as f: | |
contents = json.load(f) | |
return Registry().with_resource(COMMON_JSON, Resource.from_contents(contents)) |
🤖 Prompt for AI Agents
In schema/utils/schema_validators.py around lines 31 to 38, the current resource
loading builds a string path and uses json.load on Path.open incorrectly; change
it to open the file via a Path object with a context manager and explicit
encoding (e.g., Path(...).open("r", encoding="utf-8")) and pass the parsed JSON
to Resource.from_contents; also ensure the file exists at schema/common.json (or
update the Path to the actual location) and, if common.json declares a $id, set
it to "common.json" so the registry resource id matches.
def validate_data(schema, data): | ||
"""Validate data against schema.""" | ||
try: | ||
validate( | ||
format_checker=format_checker, | ||
instance=data, | ||
registry=get_registry(), | ||
schema=schema, | ||
) | ||
except ValidationError as e: | ||
return e.message |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Broaden error handling and make the API contract explicit.
Catching only ValidationError
misses invalid-schema cases (SchemaError
). Also, explicitly return None
on success and consider adding type hints.
-from jsonschema.exceptions import ValidationError
+from jsonschema.exceptions import ValidationError, SchemaError
@@
-def validate_data(schema, data):
+def validate_data(schema, data):
"""Validate data against schema."""
try:
validate(
format_checker=format_checker,
instance=data,
registry=get_registry(),
schema=schema,
)
- except ValidationError as e:
- return e.message
+ except ValidationError as e:
+ return e.message
+ except SchemaError as e:
+ return f"Invalid schema: {e.message}"
+ return None
📝 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.
def validate_data(schema, data): | |
"""Validate data against schema.""" | |
try: | |
validate( | |
format_checker=format_checker, | |
instance=data, | |
registry=get_registry(), | |
schema=schema, | |
) | |
except ValidationError as e: | |
return e.message | |
from jsonschema.exceptions import ValidationError, SchemaError | |
def validate_data(schema, data): | |
"""Validate data against schema.""" | |
try: | |
validate( | |
format_checker=format_checker, | |
instance=data, | |
registry=get_registry(), | |
schema=schema, | |
) | |
except ValidationError as e: | |
return e.message | |
except SchemaError as e: | |
return f"Invalid schema: {e.message}" | |
return None |
🤖 Prompt for AI Agents
In schema/utils/schema_validators.py around lines 41 to 51, expand the error
handling and make the function signature explicit: update validate_data to
include type hints for parameters and return type, catch both
jsonschema.ValidationError and jsonschema.SchemaError (returning the SchemaError
message for schema problems and the ValidationError message for data problems),
and ensure the function returns None when validation succeeds; keep the existing
registry/format_checker usage unchanged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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/package.json (1)
23-24
: @apollo/client requires a graphql peer — add it back to avoid peer resolution errors (pnpm).Install graphql matching Apollo’s peer range.
Apply:
"dependencies": { "@apollo/client": "^3.14.0", + "graphql": "^17.0.0",
If your toolchain prefers v16, use ^16.9.0 instead.
♻️ Duplicate comments (3)
frontend/package.json (3)
6-6
: Tailwind build generator path likely missing (confirm).Script references scripts/generate-tailwind-config.ts; ensure the file exists and tsconfig.tsnode.json is correctly placed.
Run:
#!/bin/bash fd -a 'tsconfig.tsnode.json' frontend || true fd -a 'generate-tailwind-config.ts' frontend || true
17-20
: cross-env is used but not declared — tests will fail on clean installs.Add cross-env to devDependencies.
Apply:
"devDependencies": { + "cross-env": "^7.0.3",
68-70
: Duplicate tailwindcss-animate in dependencies and devDependencies. Keep only in devDependencies.It’s a build-time plugin; remove the runtime copy to prevent duplication/bloat.
Apply:
- "tailwind-merge": "^3.3.1", - "tailwindcss-animate": "^1.0.7" + "tailwind-merge": "^3.3.1"Also applies to: 115-116
🧹 Nitpick comments (2)
frontend/package.json (2)
92-93
: Redundant typescript-eslint meta package (likely unused with flat config).If eslint.config.mjs uses @typescript-eslint/parser and plugin directly, drop the standalone "typescript-eslint" package.
Apply:
- "typescript-eslint": "^8.40.0",
Also applies to: 120-120
79-80
: PostCSS stack: consider removing explicit autoprefixer if using @tailwindcss/postcss only.Tailwind v4’s @tailwindcss/postcss includes the typical pipeline; explicit autoprefixer may be redundant unless you configure it separately.
Check postcss.config.js; if it’s just ['@tailwindcss/postcss'], you can remove:
- "autoprefixer": "^10.4.21",
Also applies to: 94-94, 111-115
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
backend/poetry.lock
is excluded by!**/*.lock
frontend/pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
.github/workflows/run-ci-cd.yaml
(1 hunks).pre-commit-config.yaml
(1 hunks)frontend/package.json
(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .pre-commit-config.yaml
🔇 Additional comments (3)
frontend/package.json (3)
107-110
: Jest/jsdom versions now aligned.jest and jest-environment-jsdom are both 29.x — mismatch resolved.
123-125
: Node engine pinned to 22 only — verify CI/hosting runtime.If you support Node 20 LTS (common on CI/Vercel images), widen the range; otherwise ensure CI/hosting use Node 22.
Suggestion:
- "engines": { "node": "22" }, + "engines": { "node": ">=20 <23" },
60-60
: Confirm downgrade intent and validate compatibility
- Verify the downgrade from Next.js 15.5.x to 15.4.7 (in frontend/package.json) is intentional.
- Test in a feature branch for known Next 15 + React 19 peer-dependency/runtime edge cases (install errors, precompiled libs, Emotion hydration) and add package overrides or use
--legacy-peer-deps
if necessary.
- name: Run Lighthouse CI | ||
working-directory: frontend | ||
run: | | ||
npx -y @lhci/cli@0.15.1 autorun |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Prefer pinned, lockfile-backed execution over npx; also pass config explicitly and retain artifacts.
npx fetches code at runtime and bypasses your lockfile. Use pnpm dlx (via Corepack) to keep supply-chain controls tighter and speed up repeat runs, and pass the config path explicitly to avoid accidental defaults. Also upload LHCI artifacts for debugging.
Apply this diff within the step:
- - name: Run Lighthouse CI
+ - name: Run Lighthouse CI
working-directory: frontend
- run: |
- npx -y @lhci/cli@0.15.1 autorun
+ run: |
+ corepack enable
+ corepack prepare pnpm@10 --activate
+ pnpm dlx @lhci/cli@0.15.1 autorun --config=./lighthouserc.json
And add a follow-up step to persist reports:
- name: Upload LHCI reports
if: always()
uses: actions/upload-artifact@v4
with:
name: lhci-report
path: frontend/.lighthouseci
if-no-files-found: ignore
retention-days: 7
🤖 Prompt for AI Agents
In .github/workflows/run-ci-cd.yaml around lines 500-503, replace the npx
invocation with a lockfile-backed pnpm dlx (via Corepack) call and pass the LHCI
config path explicitly so the runner uses the pinned package and the intended
config (e.g., run pnpm dlx @lhci/cli@0.15.1 autorun
--config=frontend/.lighthouseci/lhci.yml from the frontend working-directory),
and add a new follow-up step to always upload the LHCI artifacts at
frontend/.lighthouseci using actions/upload-artifact@v4 with name lhci-report,
if-no-files-found: ignore, and a retention-days of 7.
💡 Verification agent
🧩 Analysis chain
Confirm LHCI 0.15.1 compatibility with Node 22 (or pin Node 20 LTS).
Node 22 is new; some CLIs lag support. If incompatibilities exist, pin Node to 20.x here or bump LHCI to a version that officially supports Node 22.
🌐 Web query:
Does @lhci/cli@0.15.1 officially support Node.js 22? If not, what is the earliest LHCI version that supports Node 22?
💡 Result:
Short answer: No — @lhci/cli@0.15.1 does not officially state Node.js 22 support. The Lighthouse CI docs and repo examples recommend Node 16 (and their quick-start shows Node 18 for CI), and the project’s server docs require Node 16 LTS. I could not find any LHCI release that explicitly declares official Node 22 support. [1][2][3]
Sources:
- @lhci/cli v0.15.1 on npm (package info). [1]
- Lighthouse CI docs / quick-start (shows Node 16 / Node 18 in examples). [2]
- LHCI server requirements (Node v16 LTS). [3]
Pin Node to 20.x in CI before running LHCI
LHCI v0.15.1 doesn’t support Node 22 (docs/examples only reference Node 16/18). Add an actions/setup-node@v3 step with node-version: '20.x'
immediately before the “Run Lighthouse CI” step in .github/workflows/run-ci-cd.yaml.
🤖 Prompt for AI Agents
.github/workflows/run-ci-cd.yaml around lines 500 to 503: LHCI v0.15.1 is
incompatible with Node 22 in the runner; add an actions/setup-node@v3 step with
node-version: '20.x' immediately before the "Run Lighthouse CI" step so the
frontend working-directory uses Node 20.x when executing npx @lhci/cli autorun.
Include the setup-node step as a separate job step just above the existing "Run
Lighthouse CI" step and ensure it applies to the same job/runner.
"@swc/jest": "^0.2.39", | ||
"@tailwindcss/postcss": "^4.1.12", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Pick a single Jest transformer.
Both @swc/jest and ts-jest are present; using both can conflict. Prefer @swc/jest for speed, or keep ts-jest — but not both in config.
Run to see what your config uses:
If only one is referenced, remove the unused package.
Also applies to: 117-117
🏁 Script executed:
#!/bin/bash
rg -nP -C2 '(jest\.config|setupFiles|setupFilesAfterEnv|jest":\s*{)'
rg -nP -C3 '"transform"\s*:\s*{[^}]*}'
rg -nP -C2 '(ts-jest|@swc/jest)'
Length of output: 5414
Remove unused ts-jest dependency
transform in frontend/jest.config.ts only references @swc/jest, so ts-jest in package.json (line 117) isn’t used and should be removed.
🤖 Prompt for AI Agents
In frontend/package.json (noting the current diff at lines 78-79 and the unused
"ts-jest" entry around line 117), remove the "ts-jest" dependency entry from the
dependencies/devDependencies block so only @swc/jest remains; after editing, run
the package manager (npm/yarn/pnpm) to update node_modules and the lockfile to
keep manifests in sync.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (5)
frontend/package.json (5)
6-6
: Verify Tailwind config generator path (script likely to fail if file missing).Ensure
scripts/generate-tailwind-config.ts
andtsconfig.tsnode.json
exist at the referenced paths; otherwisepnpm run build:tailwind
will fail.Run:
#!/bin/bash fd -a 'tsconfig.tsnode.json' frontend || true fd -a 'scripts/generate-tailwind-config.ts' frontend || true
110-110
: Jest/jsdom versions aligned — previous mismatch resolved.
121-121
: Redundant typescript-eslint aggregate (see earlier note).Remove the
typescript-eslint
meta package or drop the individual parser/plugin and use only the meta — don’t keep both.
116-117
: Duplicate tailwindcss-animate in deps and devDeps — keep only dev-time.
tailwindcss-animate
is a build-time plugin; remove it from"dependencies"
to avoid shipping it in prod.@@ "dependencies": { - "tailwindcss-animate": "^1.0.7" }
17-20
: frontend/package.json: add missing cross-env devDependency
test:unit
invokescross-env
but it’s not declared in devDependencies, causing CI/tests to fail. Add the following underdevDependencies
:"devDependencies": { + "cross-env": "^7.0.3",
🧹 Nitpick comments (1)
frontend/package.json (1)
93-94
: Avoid duplicating typescript-eslint meta and plugin packages.You have both the meta package
typescript-eslint
and the individual@typescript-eslint/{parser,eslint-plugin}
. Keep one approach to prevent version skew. Recommend removing the meta package.- "typescript-eslint": "^8.40.0",
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
backend/poetry.lock
is excluded by!**/*.lock
frontend/pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
.github/workflows/run-ci-cd.yaml
(1 hunks)frontend/package.json
(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/run-ci-cd.yaml
🔇 Additional comments (6)
frontend/package.json (6)
61-61
: LGTM on Next upgrade (version alignment).
next@^15.4.7
matcheseslint-config-next@^15.4.7
.
81-81
: LGTM: jest-dom version is compatible with Jest 29.Matches
jest@^29.7.0
.
87-87
: No actionable change (blank line).
91-91
: No actionable change (blank line).
96-98
: LGTM: ESLint stack versions are coherent.
68-68
: Ignore React Router SSR check: noreact-router-dom
imports or usage detected infrontend/src
, so SSR/hydration concerns don’t apply.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@OUMIMANDAL hi! There are a lot of conflicts after most recent main update. Please work on resolving those.
Also, your changes should not include package.json file unless your fix actually requires some new packages to be added, which I would surprised to see 👀
Also, if this is not addressed today or tomorrow - I will reassign this issue.
It's been taking too long to fix. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is going on here?! 👀
please clean this up and revert any unnecessary changes for any of these file.
I would not expect this fix to require sooo many changes.
Closing this PR as it has been replaced by a cleaner version with only the necessary changes. |
Title: Fix homepage container flexibility
Description / Proposed changes:
loading.tsx
(app-level loading UI)TestCard.tsx
(frontend component)tailwind.config.cjs
)Issue reference:
Resolves #2033
Notes for reviewers:
pnpm-lock.yaml
file was restored from upstream/main to keep dependencies consistent.src/
andtailwind.config.cjs
.