feat(route): redesign /resume-builder with Forge AI-inspired landing#4400
Open
anuragmishrainsights wants to merge 3 commits into
Open
feat(route): redesign /resume-builder with Forge AI-inspired landing#4400anuragmishrainsights wants to merge 3 commits into
anuragmishrainsights wants to merge 3 commits into
Conversation
- forge.css: dark/technical design tokens (status chips, monospace metrics, bento) - forge/ primitives: StatusBadge, MetricChip, ForgeButton, BentoCard - ForgeFeaturePage: data-driven landing rendering all 5 routes from FEATURES_BY_SLUG - 5 thin wrapper pages (ResumeBuilderForge, ResumeRoastForge, GithubPortfolioForge, ProjectVisualizerForge, MockInterviewForge)
|
CodeAnt AI is reviewing your PR. |
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
codeant-ai
Bot
added
the
size:XL
Contributor
📝 WalkthroughWalkthroughAdds a Forge dark UI system, reusable components, a data-driven feature landing page, five feature route adapters, a corrected resume-builder lazy import, and an animated tools section with responsive styling. ChangesForge feature experience
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Visitor
participant FeatureRoute
participant ForgeFeaturePage
participant FEATURES_BY_SLUG
participant useAuth
Visitor->>FeatureRoute: open feature route
FeatureRoute->>ForgeFeaturePage: pass feature slug
ForgeFeaturePage->>FEATURES_BY_SLUG: load feature configuration
ForgeFeaturePage->>useAuth: read authentication state
ForgeFeaturePage-->>Visitor: render configured Forge page
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| import { useAuth } from '../../hooks/useAuth'; | ||
| import { FEATURES_BY_SLUG } from '../../data/featuresConfig'; | ||
| import Seo from '../../components/Seo'; | ||
| import '../styles/forge.css'; |
There was a problem hiding this comment.
Suggestion: The stylesheet import path is off by one directory level, so it resolves to components/styles/forge.css instead of src/styles/forge.css. This will either fail the build or load no styles for the new page; update the relative path to the actual styles directory. [import error]
Severity Level: Major ⚠️
⚠️ Forge landing renders unstyled; Forge design not applied.
⚠️ Visual consistency of resume-builder marketing experience degraded.Steps of Reproduction ✅
1. `ForgeFeaturePage` is loaded for `/resume-builder` via `ResumeBuilderForge`
(`frontend/src/pages/features/ResumeBuilderForge.jsx:1-4`) and the route configuration in
`frontend/src/App.jsx:26,81`.
2. When `ForgeFeaturePage.jsx` is evaluated, it executes the stylesheet import at
`frontend/src/components/forge/ForgeFeaturePage.jsx:6`, which uses the relative path
`../styles/forge.css` from the `components/forge` directory.
3. From `frontend/src/components/forge`, the path `../styles/forge.css` resolves to
`frontend/src/components/styles/forge.css`, but a glob over the repository shows the
actual stylesheet at `frontend/src/styles/forge.css` (`functions.Glob` found
`/frontend/src/styles/forge.css`).
4. Because `frontend/src/components/styles/forge.css` does not exist, the build either
throws a "module not found" error or silently omits the stylesheet, resulting in the Forge
landing rendering without the intended Forge design-system styles.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** frontend/src/components/forge/ForgeFeaturePage.jsx
**Line:** 6:6
**Comment:**
*Import Error: The stylesheet import path is off by one directory level, so it resolves to `components/styles/forge.css` instead of `src/styles/forge.css`. This will either fail the build or load no styles for the new page; update the relative path to the actual styles directory.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+7
to
+10
| import StatusBadge from './forge/StatusBadge'; | ||
| import MetricChip from './forge/MetricChip'; | ||
| import ForgeButton from './forge/ForgeButton'; | ||
| import BentoCard from './forge/BentoCard'; |
There was a problem hiding this comment.
Suggestion: The relative imports for local forge components point to a non-existent nested forge/ subdirectory. Because this file is already inside components/forge, these paths will fail module resolution and break the page at build/runtime; import the sibling files directly from the current directory instead. [import error]
Severity Level: Critical 🚨
❌ /resume-builder Forge landing fails due to import error.
⚠️ Future Forge feature pages would also fail to load.Steps of Reproduction ✅
1. The route `/resume-builder` is wired in `frontend/src/App.jsx:81` to
`ResumeBuilderLanding`, which is lazily imported from
`./pages/features/ResumeBuilderForge` at `frontend/src/App.jsx:26`.
2. `frontend/src/pages/features/ResumeBuilderForge.jsx:1-4` renders `<ForgeFeaturePage
slug="resume-builder" />`, pulling in
`frontend/src/components/forge/ForgeFeaturePage.jsx`.
3. `ForgeFeaturePage.jsx:7-10` imports `StatusBadge`, `MetricChip`, `ForgeButton`, and
`BentoCard` from `./forge/...`, implying a nested `forge` subdirectory under
`frontend/src/components/forge`.
4. A directory listing of `frontend/src/components/forge` (`LS` output) shows only sibling
files `StatusBadge.jsx`, `MetricChip.jsx`, `ForgeButton.jsx`, and `BentoCard.jsx` directly
in that folder with no `forge/` subdirectory, so the bundler cannot resolve
`./forge/StatusBadge` etc., causing a module resolution error and preventing the Forge
feature page from loading.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** frontend/src/components/forge/ForgeFeaturePage.jsx
**Line:** 7:10
**Comment:**
*Import Error: The relative imports for local forge components point to a non-existent nested `forge/` subdirectory. Because this file is already inside `components/forge`, these paths will fail module resolution and break the page at build/runtime; import the sibling files directly from the current directory instead.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| <Link to="/mock-interview" className="forge-nav-link">Mock Interview</Link> | ||
| </div> | ||
| <div className="forge-nav-actions"> | ||
| {!user && <ForgeButton href="/login" variant="ghost">Sign in</ForgeButton>} |
There was a problem hiding this comment.
Suggestion: The sign-in action uses href for an internal route, which makes ForgeButton render a plain anchor and triggers a full page reload instead of client-side router navigation. This resets in-memory app state and creates inconsistent navigation behavior compared with other internal links; use router navigation (to) for internal paths. [api mismatch]
Severity Level: Major ⚠️
⚠️ Sign-in from Forge landing triggers full SPA reload.
⚠️ Navigation behavior inconsistent with other internal login links.Steps of Reproduction ✅
1. For unauthenticated visitors, the `/resume-builder` route renders `ForgeFeaturePage`
via `ResumeBuilderForge` (`frontend/src/pages/features/ResumeBuilderForge.jsx:1-4` and
`frontend/src/App.jsx:26,81`), with `useAuth` returning `user = null`.
2. In this state, the navbar actions at
`frontend/src/components/forge/ForgeFeaturePage.jsx:72-75` render the sign-in button as
`{!user && <ForgeButton href="/login" variant="ghost">Sign in</ForgeButton>}`, passing
`href` but no `to`.
3. The `ForgeButton` implementation at `frontend/src/components/forge/ForgeButton.jsx:4-8`
renders a `<Link>` only when `to` is provided, and otherwise falls back to `<a href={href
|| '#'} ...>`, so the Forge sign-in button becomes a plain anchor navigating to `/login`.
4. Clicking this anchor from the Forge landing triggers a full page reload to `/login`,
unlike other login entry points which use `react-router-dom`'s `<Link to="/login">` (e.g.
`frontend/src/components/ui/LandingNavbar.jsx:29`,
`frontend/src/components/Navbar.jsx:356-360`), leading to inconsistent navigation behavior
and resetting any in-memory state on the Forge page.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** frontend/src/components/forge/ForgeFeaturePage.jsx
**Line:** 73:73
**Comment:**
*Api Mismatch: The sign-in action uses `href` for an internal route, which makes `ForgeButton` render a plain anchor and triggers a full page reload instead of client-side router navigation. This resets in-memory app state and creates inconsistent navigation behavior compared with other internal links; use router navigation (`to`) for internal paths.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
| const ProjectVisualizerDashboard = lazy(() => import('./pages/ProjectVisualizer/Dashboard')); | ||
|
|
||
| const ResumeBuilderLanding = lazy(() => import('./pages/features/ResumeBuilderLanding')); | ||
| const ResumeBuilderLanding = lazy(() => import('./pages/features/ResumeBuilderForge')); |
There was a problem hiding this comment.
Suggestion: This route switch now lazy-loads the Forge resume page, but that page currently depends on ForgeFeaturePage imports that do not resolve (./forge/... and the wrong CSS relative path), so navigating to /resume-builder will fail to load the module at runtime. Fix the broken Forge imports first (or keep the previous landing component until they are corrected). [import error]
Severity Level: Critical 🚨
- ❌ /resume-builder landing route fails to render.
- ❌ Build or dev server fails resolving Forge imports.
- ⚠️ Resume hub links into landing break for users.Steps of Reproduction ✅
1. In `frontend/src/App.jsx` lines 51-66, `AppRoutes` defines the public route
`"/resume-builder"` with element `ResumeBuilderLanding` wrapped in `Suspense` (verified
via Read of App.jsx lines 52-66).
2. At the top of the same file, line 105 defines `const ResumeBuilderLanding = lazy(() =>
import('./pages/features/ResumeBuilderForge'));` (Read of App.jsx lines 20-33), so
navigating to `/resume-builder` triggers a dynamic import of
`frontend/src/pages/features/ResumeBuilderForge.jsx`.
3. `frontend/src/pages/features/ResumeBuilderForge.jsx` imports `ForgeFeaturePage`
(`import ForgeFeaturePage from '../../components/forge/ForgeFeaturePage';` at line 2) and
renders it (`return ForgeFeaturePage slug="resume-builder"` at line 4) as seen in the Read
of that file.
4. `frontend/src/components/forge/ForgeFeaturePage.jsx` (Read lines 1-10) imports
`../styles/forge.css` and `StatusBadge`, `MetricChip`, `ForgeButton`, `BentoCard` from
`./forge/...`, but LS shows `/frontend/src/components/forge` only contains
`BentoCard.jsx`, `ForgeButton.jsx`, `ForgeFeaturePage.jsx`, `MetricChip.jsx`,
`StatusBadge.jsx` and no `forge` subdirectory, and LS of `/frontend/src/styles` shows
`forge.css` under `src/styles`, not `components/styles`. These relative paths therefore
resolve to non-existent `components/styles/forge.css` and `components/forge/forge/*`,
causing module resolution errors when `ForgeFeaturePage` is bundled, so the lazy-loaded
`/resume-builder` landing fails to load.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** frontend/src/App.jsx
**Line:** 105:105
**Comment:**
*Import Error: This route switch now lazy-loads the Forge resume page, but that page currently depends on `ForgeFeaturePage` imports that do not resolve (`./forge/...` and the wrong CSS relative path), so navigating to `/resume-builder` will fail to load the module at runtime. Fix the broken Forge imports first (or keep the previous landing component until they are corrected).
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| import React from 'react'; | ||
| import { motion } from 'framer-motion'; | ||
| import { FEATURES } from '../../data/featuresConfig'; | ||
| import '../styles/forge.css'; |
There was a problem hiding this comment.
Suggestion: This relative stylesheet path is incorrect from components/landing and points to a non-existent components/styles/forge.css, which will cause a module resolution failure. Update it to the correct path under src/styles. [import error]
Severity Level: Major ⚠️
- ⚠️ Future Forge tools grid component fails bundling.
- ⚠️ CSS import mismatch complicates Forge components reuse.Steps of Reproduction ✅
1. `frontend/src/components/landing/ForgeToolsSection.jsx` imports the Forge stylesheet
with `import '../styles/forge.css';` at line 4 (verified via Read of that file).
2. LS of `/frontend/src/styles` shows `forge.css` located at
`frontend/src/styles/forge.css`, and LS of `/frontend/src/components` plus
`/frontend/src/components/styles` confirms there is no `components/styles/forge.css`, so
from `components/landing` the relative path `../styles/forge.css` actually targets a
non-existent `frontend/src/components/styles/forge.css`.
3. JavaScript module resolution for relative paths means any page that imports
`ForgeToolsSection` will cause the bundler to resolve `../styles/forge.css` from the
`components/landing` directory, which fails because that file does not exist, resulting in
a "module not found" error for the CSS import.
4. Currently `ForgeToolsSection` has no external callers (Grep for `ForgeToolsSection`
only finds its own file), so the issue is dormant today but will reliably surface as soon
as this component is wired into a landing page, breaking the page’s build or runtime load.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** frontend/src/components/landing/ForgeToolsSection.jsx
**Line:** 4:4
**Comment:**
*Import Error: This relative stylesheet path is incorrect from `components/landing` and points to a non-existent `components/styles/forge.css`, which will cause a module resolution failure. Update it to the correct path under `src/styles`.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
CodeAnt AI finished reviewing your PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
frontend/src/styles/forge.css (2)
21-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueQuote standard font names.
Optionally, you can quote font family names containing capital letters to ensure consistent parsing and satisfy CSS linters.
🎨 Proposed update
- --forge-mono: 'JetBrains Mono', 'SF Mono', 'Fira Code', ui-monospace, SFMono-Regular, Menlo, monospace; - --forge-sans: 'Inter', system-ui, -apple-system, 'Segoe UI', Roboto, sans-serif; + --forge-mono: 'JetBrains Mono', 'SF Mono', 'Fira Code', ui-monospace, 'SFMono-Regular', 'Menlo', monospace; + --forge-sans: 'Inter', system-ui, -apple-system, 'Segoe UI', 'Roboto', sans-serif;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/styles/forge.css` around lines 21 - 22, Update the --forge-mono and --forge-sans font stacks to quote font family names containing capital letters, including JetBrains Mono, SF Mono, Fira Code, SFMono-Regular, Menlo, and Inter, while preserving the existing fallback order and values.Source: Linters/SAST tools
68-72: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse kebab-case for keyframes.
Optionally switch to standard kebab-case naming for the keyframes to clear CSS linter warnings.
🎨 Proposed update
- box-shadow: 0 0 10px var(--forge-accent); animation: forgePulse 1.8s infinite; + box-shadow: 0 0 10px var(--forge-accent); animation: forge-pulse 1.8s infinite; } .forge-status.warn .pulse { background: `#ffcf5a`; box-shadow: 0 0 10px `#ffcf5a`; } .forge-status.danger .pulse { background: var(--forge-danger); box-shadow: 0 0 10px var(--forge-danger); } -@keyframes forgePulse { +@keyframes forge-pulse {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/styles/forge.css` around lines 68 - 72, Rename the forgePulse keyframe identifier used by the pulse animation to standard kebab-case, and update the corresponding animation reference so both names remain consistent.Source: Linters/SAST tools
frontend/src/components/forge/ForgeFeaturePage.jsx (1)
36-40: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUse a single SVG
<path>instead of splitting.Standard SVGs natively support multiple
M(move-to) commands inside a singledattribute to define complex compound paths. There is no need to split the string, map an array, or create multiple DOM elements.♻️ Proposed refactor
<svg width={size} height={size} viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth={strokeWidth} strokeLinecap="round" strokeLinejoin="round" aria-hidden="true"> - {d.split(' M').map((seg, i) => ( - <path key={i} d={(i === 0 ? seg : 'M' + seg)} /> - ))} + <path d={d} /> </svg>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/forge/ForgeFeaturePage.jsx` around lines 36 - 40, Update the SVG rendering in the Forge feature icon component to use one path element with the complete d string directly. Remove the d.split mapping and generated per-segment keys, while preserving the existing SVG attributes and path data.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/forge/ForgeButton.jsx`:
- Around line 5-8: Update ForgeButton to destructure the incoming className prop
separately from ...rest, then combine it with the generated forge-btn and
variant classes. Use the merged className on both the Link and anchor branches,
while ensuring ...rest no longer overwrites it.
In `@frontend/src/components/forge/ForgeFeaturePage.jsx`:
- Line 73: Update the unauthenticated login link in ForgeFeaturePage’s
ForgeButton to use the React Router `to` prop with `/login` instead of `href`,
preserving the existing ghost variant and sign-in label.
- Around line 5-10: Correct the relative imports in ForgeFeaturePage so they
resolve from src/components/forge: point forge.css to the styles directory at
the appropriate parent level, and import StatusBadge, MetricChip, ForgeButton,
and BentoCard from the local forge component directory without duplicating forge
in the path.
In `@frontend/src/components/landing/ForgeToolsSection.jsx`:
- Around line 70-72: Update the ForgeButton usage in ForgeToolsSection to pass
both destination props, using optional chaining for f.primaryAction.to and
f.primaryAction.href. Preserve the existing label and variant while allowing
ForgeButton’s fallback logic to handle internal and external links safely.
- Around line 49-64: Inline the SVG markup within the forge-tools map render
instead of defining the per-item Icon component. Preserve the existing
ICON_PATHS fallback, path-splitting logic, SVG attributes, and prop spreading
while removing the locally recreated Icon component and rendering the SVG
directly inside the ico container.
- Line 47: Convert the tool-card grid container in ForgeToolsSection from div to
motion.div, preserving its className and inline style, and apply the same
scroll/animation trigger props used by the surrounding motion section so its
child variants and stagger animation activate. Update the matching closing tag
while leaving the tool card markup unchanged.
---
Nitpick comments:
In `@frontend/src/components/forge/ForgeFeaturePage.jsx`:
- Around line 36-40: Update the SVG rendering in the Forge feature icon
component to use one path element with the complete d string directly. Remove
the d.split mapping and generated per-segment keys, while preserving the
existing SVG attributes and path data.
In `@frontend/src/styles/forge.css`:
- Around line 21-22: Update the --forge-mono and --forge-sans font stacks to
quote font family names containing capital letters, including JetBrains Mono, SF
Mono, Fira Code, SFMono-Regular, Menlo, and Inter, while preserving the existing
fallback order and values.
- Around line 68-72: Rename the forgePulse keyframe identifier used by the pulse
animation to standard kebab-case, and update the corresponding animation
reference so both names remain consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d65e81c3-a179-4dae-96cb-2513ca4d70a1
📒 Files selected for processing (13)
frontend/src/App.jsxfrontend/src/components/forge/BentoCard.jsxfrontend/src/components/forge/ForgeButton.jsxfrontend/src/components/forge/ForgeFeaturePage.jsxfrontend/src/components/forge/MetricChip.jsxfrontend/src/components/forge/StatusBadge.jsxfrontend/src/components/landing/ForgeToolsSection.jsxfrontend/src/pages/features/GithubPortfolioForge.jsxfrontend/src/pages/features/MockInterviewForge.jsxfrontend/src/pages/features/ProjectVisualizerForge.jsxfrontend/src/pages/features/ResumeBuilderForge.jsxfrontend/src/pages/features/ResumeRoastForge.jsxfrontend/src/styles/forge.css
Comment on lines
+5
to
+8
| export default function ForgeButton({ to, href, children, variant = 'primary', ...rest }) { | ||
| const cls = `forge-btn ${variant === 'primary' ? 'forge-btn-primary' : 'forge-btn-ghost'}`; | ||
| if (to) return <Link to={to} className={cls} {...rest}>{children}</Link>; | ||
| return <a href={href || '#'} className={cls} {...rest}>{children}</a>; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Merge custom className properly.
When a consumer passes a custom className to this component, it will completely overwrite the base forge-btn classes because {...rest} is spread after className={cls}. Extract className from the props to combine it correctly.
🐛 Proposed fix
-export default function ForgeButton({ to, href, children, variant = 'primary', ...rest }) {
- const cls = `forge-btn ${variant === 'primary' ? 'forge-btn-primary' : 'forge-btn-ghost'}`;
+export default function ForgeButton({ to, href, children, variant = 'primary', className = '', ...rest }) {
+ const cls = `forge-btn ${variant === 'primary' ? 'forge-btn-primary' : 'forge-btn-ghost'} ${className}`.trim();
if (to) return <Link to={to} className={cls} {...rest}>{children}</Link>;
return <a href={href || '#'} className={cls} {...rest}>{children}</a>;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export default function ForgeButton({ to, href, children, variant = 'primary', ...rest }) { | |
| const cls = `forge-btn ${variant === 'primary' ? 'forge-btn-primary' : 'forge-btn-ghost'}`; | |
| if (to) return <Link to={to} className={cls} {...rest}>{children}</Link>; | |
| return <a href={href || '#'} className={cls} {...rest}>{children}</a>; | |
| export default function ForgeButton({ to, href, children, variant = 'primary', className = '', ...rest }) { | |
| const cls = `forge-btn ${variant === 'primary' ? 'forge-btn-primary' : 'forge-btn-ghost'} ${className}`.trim(); | |
| if (to) return <Link to={to} className={cls} {...rest}>{children}</Link>; | |
| return <a href={href || '#'} className={cls} {...rest}>{children}</a>; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/forge/ForgeButton.jsx` around lines 5 - 8, Update
ForgeButton to destructure the incoming className prop separately from ...rest,
then combine it with the generated forge-btn and variant classes. Use the merged
className on both the Link and anchor branches, while ensuring ...rest no longer
overwrites it.
Comment on lines
+5
to
+10
| import Seo from '../../components/Seo'; | ||
| import '../styles/forge.css'; | ||
| import StatusBadge from './forge/StatusBadge'; | ||
| import MetricChip from './forge/MetricChip'; | ||
| import ForgeButton from './forge/ForgeButton'; | ||
| import BentoCard from './forge/BentoCard'; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Fix broken relative import paths.
The current paths for local components and the forge.css stylesheet are incorrect based on this file's location (src/components/forge/ForgeFeaturePage.jsx). These will trigger compilation failures because they evaluate to non-existent directories like src/components/styles/ and src/components/forge/forge/.
🛠️ Proposed fix
-import Seo from '../../components/Seo';
-import '../styles/forge.css';
-import StatusBadge from './forge/StatusBadge';
-import MetricChip from './forge/MetricChip';
-import ForgeButton from './forge/ForgeButton';
-import BentoCard from './forge/BentoCard';
+import Seo from '../Seo';
+import '../../styles/forge.css';
+import StatusBadge from './StatusBadge';
+import MetricChip from './MetricChip';
+import ForgeButton from './ForgeButton';
+import BentoCard from './BentoCard';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import Seo from '../../components/Seo'; | |
| import '../styles/forge.css'; | |
| import StatusBadge from './forge/StatusBadge'; | |
| import MetricChip from './forge/MetricChip'; | |
| import ForgeButton from './forge/ForgeButton'; | |
| import BentoCard from './forge/BentoCard'; | |
| import Seo from '../Seo'; | |
| import '../../styles/forge.css'; | |
| import StatusBadge from './StatusBadge'; | |
| import MetricChip from './MetricChip'; | |
| import ForgeButton from './ForgeButton'; | |
| import BentoCard from './BentoCard'; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/forge/ForgeFeaturePage.jsx` around lines 5 - 10,
Correct the relative imports in ForgeFeaturePage so they resolve from
src/components/forge: point forge.css to the styles directory at the appropriate
parent level, and import StatusBadge, MetricChip, ForgeButton, and BentoCard
from the local forge component directory without duplicating forge in the path.
| <Link to="/mock-interview" className="forge-nav-link">Mock Interview</Link> | ||
| </div> | ||
| <div className="forge-nav-actions"> | ||
| {!user && <ForgeButton href="/login" variant="ghost">Sign in</ForgeButton>} |
Contributor
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
Use client-side routing for the login link.
Using href here bypasses React Router and causes a full browser page reload. Switch to to="/login" to keep the SPA navigation fast and seamless.
🚀 Proposed fix
- {!user && <ForgeButton href="/login" variant="ghost">Sign in</ForgeButton>}
+ {!user && <ForgeButton to="/login" variant="ghost">Sign in</ForgeButton>}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {!user && <ForgeButton href="/login" variant="ghost">Sign in</ForgeButton>} | |
| {!user && <ForgeButton to="/login" variant="ghost">Sign in</ForgeButton>} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/forge/ForgeFeaturePage.jsx` at line 73, Update the
unauthenticated login link in ForgeFeaturePage’s ForgeButton to use the React
Router `to` prop with `/login` instead of `href`, preserving the existing ghost
variant and sign-in label.
| </motion.p> | ||
| </motion.div> | ||
|
|
||
| <div className="forge-tools-grid" style={{ marginTop: 44 }}> |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Fix broken Framer Motion variant propagation for tool cards.
The tool cards are wrapped in a standard <div> sibling to the header motion.div. Because they are not descendants of an active motion parent, the variants={item} prop on the tool cards will not be triggered when scrolled into view, causing the grid animation to fail entirely.
Convert the grid container to a motion.div and apply the same trigger props to restore the stagger animation. Remember to also change the closing </div> to </motion.div> on line 76.
🪄 Proposed fix
- <div className="forge-tools-grid" style={{ marginTop: 44 }}>
+ <motion.div
+ className="forge-tools-grid"
+ style={{ marginTop: 44 }}
+ variants={header}
+ initial="hidden"
+ whileInView="show"
+ viewport={{ once: true, margin: '-100px' }}
+ >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/landing/ForgeToolsSection.jsx` at line 47, Convert
the tool-card grid container in ForgeToolsSection from div to motion.div,
preserving its className and inline style, and apply the same scroll/animation
trigger props used by the surrounding motion section so its child variants and
stagger animation activate. Update the matching closing tag while leaving the
tool card markup unchanged.
Comment on lines
+49
to
+64
| const Icon = (p) => ( | ||
| <svg width={24} height={24} viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth={1.8} strokeLinecap="round" strokeLinejoin="round" {...p}> | ||
| {(ICON_PATHS[f.icon?.name] || ICON_PATHS.FileText).split(' M').map((seg, k) => ( | ||
| <path key={k} d={k === 0 ? seg : 'M' + seg} /> | ||
| ))} | ||
| </svg> | ||
| ); | ||
| const large = f.size === 'large'; | ||
| return ( | ||
| <motion.div | ||
| key={f.slug} | ||
| variants={item} | ||
| className={`forge-tool ${large ? 'lg' : ''}`} | ||
| style={large ? { display: 'flex', flexDirection: 'column', justifyContent: 'space-between' } : {}} | ||
| > | ||
| <div className="ico"><Icon /></div> |
Contributor
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Inline the SVG to prevent unmounting/remounting on every render.
Defining a React component (Icon) inside a .map loop creates a new component type on every render. This forces React to destroy and recreate the DOM nodes instead of cleanly updating them, degrading performance.
Inline the SVG directly to avoid this overhead.
⚡ Proposed fix
- const Icon = (p) => (
- <svg width={24} height={24} viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth={1.8} strokeLinecap="round" strokeLinejoin="round" {...p}>
- {(ICON_PATHS[f.icon?.name] || ICON_PATHS.FileText).split(' M').map((seg, k) => (
- <path key={k} d={k === 0 ? seg : 'M' + seg} />
- ))}
- </svg>
- );
const large = f.size === 'large';
return (
<motion.div
key={f.slug}
variants={item}
className={`forge-tool ${large ? 'lg' : ''}`}
style={large ? { display: 'flex', flexDirection: 'column', justifyContent: 'space-between' } : {}}
>
- <div className="ico"><Icon /></div>
+ <div className="ico">
+ <svg width={24} height={24} viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth={1.8} strokeLinecap="round" strokeLinejoin="round">
+ {(ICON_PATHS[f.icon?.name] || ICON_PATHS.FileText).split(' M').map((seg, k) => (
+ <path key={k} d={k === 0 ? seg : 'M' + seg} />
+ ))}
+ </svg>
+ </div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const Icon = (p) => ( | |
| <svg width={24} height={24} viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth={1.8} strokeLinecap="round" strokeLinejoin="round" {...p}> | |
| {(ICON_PATHS[f.icon?.name] || ICON_PATHS.FileText).split(' M').map((seg, k) => ( | |
| <path key={k} d={k === 0 ? seg : 'M' + seg} /> | |
| ))} | |
| </svg> | |
| ); | |
| const large = f.size === 'large'; | |
| return ( | |
| <motion.div | |
| key={f.slug} | |
| variants={item} | |
| className={`forge-tool ${large ? 'lg' : ''}`} | |
| style={large ? { display: 'flex', flexDirection: 'column', justifyContent: 'space-between' } : {}} | |
| > | |
| <div className="ico"><Icon /></div> | |
| const large = f.size === 'large'; | |
| return ( | |
| <motion.div | |
| key={f.slug} | |
| variants={item} | |
| className={`forge-tool ${large ? 'lg' : ''}`} | |
| style={large ? { display: 'flex', flexDirection: 'column', justifyContent: 'space-between' } : {}} | |
| > | |
| <div className="ico"> | |
| <svg width={24} height={24} viewBox="0 0 24 24" fill="none" stroke="currentColor" strokeWidth={1.8} strokeLinecap="round" strokeLinejoin="round"> | |
| {(ICON_PATHS[f.icon?.name] || ICON_PATHS.FileText).split(' M').map((seg, k) => ( | |
| <path key={k} d={k === 0 ? seg : 'M' + seg} /> | |
| ))} | |
| </svg> | |
| </div> |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 63-63: A list component should have a key to prevent re-rendering
Context:
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
[warning] 63-63: A list component should have a key to prevent re-rendering
Context:
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
[warning] 50-52: Do not use array indexes for a list component's key
Context: (ICON_PATHS[f.icon?.name] || ICON_PATHS.FileText).split(' M').map((seg, k) => (
<path key={k} d={k === 0 ? seg : 'M' + seg} />
))
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-no-index)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/landing/ForgeToolsSection.jsx` around lines 49 - 64,
Inline the SVG markup within the forge-tools map render instead of defining the
per-item Icon component. Preserve the existing ICON_PATHS fallback,
path-splitting logic, SVG attributes, and prop spreading while removing the
locally recreated Icon component and rendering the SVG directly inside the ico
container.
Comment on lines
+70
to
+72
| <div style={{ marginTop: 18 }}> | ||
| <ForgeButton to={f.primaryAction.to} variant="ghost">{f.primaryAction.label} →</ForgeButton> | ||
| </div> |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Safeguard action config and bind href for external links.
Currently, ForgeButton only receives the to prop. If a feature configures an href instead of to, the button's internal link logic will break or render empty. Additionally, accessing f.primaryAction.to without optional chaining risks a runtime crash if the configuration is incomplete.
Pass both destination props with optional chaining. As per Context snippet 1, ForgeButton gracefully handles the fallback logic between to and href.
🔗 Proposed fix
- <div style={{ marginTop: 18 }}>
- <ForgeButton to={f.primaryAction.to} variant="ghost">{f.primaryAction.label} →</ForgeButton>
- </div>
+ <div style={{ marginTop: 18 }}>
+ <ForgeButton to={f.primaryAction?.to} href={f.primaryAction?.href} variant="ghost">
+ {f.primaryAction?.label} →
+ </ForgeButton>
+ </div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <div style={{ marginTop: 18 }}> | |
| <ForgeButton to={f.primaryAction.to} variant="ghost">{f.primaryAction.label} →</ForgeButton> | |
| </div> | |
| <div style={{ marginTop: 18 }}> | |
| <ForgeButton to={f.primaryAction?.to} href={f.primaryAction?.href} variant="ghost"> | |
| {f.primaryAction?.label} → | |
| </ForgeButton> | |
| </div> |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 70-70: A list component should have a key to prevent re-rendering
Context: {f.primaryAction.label} →
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/landing/ForgeToolsSection.jsx` around lines 70 - 72,
Update the ForgeButton usage in ForgeToolsSection to pass both destination
props, using optional chaining for f.primaryAction.to and f.primaryAction.href.
Preserve the existing label and variant while allowing ForgeButton’s fallback
logic to handle internal and external links safely.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
User description
What
Redesigns the /resume-builder route with the new Forge design system (forge-ai.space-inspired: dark, technical, operational-status chips, monospace metrics, glassy bento).
ResumeBuilderForge(data-driven viaForgeFeaturePage+FEATURES_BY_SLUG['resume-builder']).Depends on
forge/design-system(design tokens + primitives).Verification
✅ No merge — raised for review only.
CodeAnt-AI Description
Redesign the resume builder and related feature pages with a Forge-style landing layout
What Changed
/resume-builderpage now opens with a dark Forge-themed landing page instead of the previous designImpact
✅ More polished resume-builder landing page✅ Consistent look across feature pages✅ Clearer entry point for signed-in users💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit