Skip to content

feat: UI customization - OpenAPI branding & Apple-inspired design - #3491

Closed
sz-wuyanzu wants to merge 6 commits into
QuantumNous:mainfrom
sz-wuyanzu:m-chen
Closed

feat: UI customization - OpenAPI branding & Apple-inspired design#3491
sz-wuyanzu wants to merge 6 commits into
QuantumNous:mainfrom
sz-wuyanzu:m-chen

Conversation

@sz-wuyanzu

@sz-wuyanzu sz-wuyanzu commented Mar 29, 2026

Copy link
Copy Markdown
  • Rebrand: SystemName -> OpenAPI, new SVG logo, default values updated
  • Home: new hero with flowing gradient, 'no VPN needed' badge, gradient title, remove provider icons
  • Nav: sliding capsule indicator with spring animation for active route
  • CSS: Apple-inspired design tokens (glass system, radius, transitions, font stack)
  • Header/Sidebar: glassmorphism with backdrop-blur
  • Fix: VChart black blocks when trend data is empty/all-zeros
  • Fix: CTA button click blocked by overlay layers (pointer-events-none)
  • Cleanup: remove unused imports (icons, Link, docsLink)

Summary by CodeRabbit

  • New Features

    • Animated character visuals added to auth and password flows
    • Navigation gains active-link sliding indicator
  • Enhancements

    • Rebranded system name and logo to "OpenAPI"
    • Homepage redesigned with new hero, compact stats, and updated CTAs
    • Glassmorphism design system and global style refresh
    • Login/register/reset forms: responsive two-column layout and password visibility toggles
    • Pricing page now lazy-loaded for faster initial load
  • Bug Fixes

    • Trend charts render only when containing non-zero data points
  • Documentation

    • Added project conventions and performance-optimization guidance
  • Infrastructure

    • New reverse-proxy configuration and streaming-friendly proxy rules

- Rebrand: SystemName -> OpenAPI, new SVG logo, default values updated
- Home: new hero with flowing gradient, 'no VPN needed' badge, gradient title, remove provider icons
- Nav: sliding capsule indicator with spring animation for active route
- CSS: Apple-inspired design tokens (glass system, radius, transitions, font stack)
- Header/Sidebar: glassmorphism with backdrop-blur
- Fix: VChart black blocks when trend data is empty/all-zeros
- Fix: CTA button click blocked by overlay layers (pointer-events-none)
- Cleanup: remove unused imports (icons, Link, docsLink)
@coderabbitai

coderabbitai Bot commented Mar 29, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds a project conventions doc; renames branding to "OpenAPI"; introduces glassmorphism design tokens and UI restyling; replaces nav hover with a measured sliding indicator; lazy-loads Mermaid and Pricing; adds AnimatedCharacters and two-column auth layouts; adjusts chart rendering; adds Nginx config and gzip middleware for relay model routes.

Changes

Cohort / File(s) Summary
Project Conventions (docs)
.kiro/steering/project.md
New conventions doc describing tech stack, layered architecture, i18n, DB/JSON rules, relay DTO pointer requirement, CLI workflows, and protected identifier policies.
Branding & Helper Defaults
common/constants.go, web/src/helpers/utils.jsx
System name changed to OpenAPI and default logo updated to /openapi-logo.svg.
Global Design & Tokens
web/src/index.css, web/src/components/layout/headerbar/index.jsx
Added glassmorphism design tokens, radii, transitions, animations, Sem UI overrides; header styling moved to CSS-variable inline style.
Header Navigation & Headerbar Components
web/src/components/layout/headerbar/Navigation.jsx, web/src/components/layout/headerbar/HeaderLogo.jsx, web/src/components/layout/headerbar/UserArea.jsx, web/src/components/layout/headerbar/index.jsx
Navigation now computes active item from route, measures refs, and renders a sliding indicator; removed skeleton/loading branches; Navigation prop isLoading removed.
Charts & Dashboard
web/src/components/dashboard/StatsCards.jsx, web/src/helpers/dashboard.jsx
Trend chart now renders only when trendData includes a non-zero value; chart spec background changed from object to string.
Home Page
web/src/pages/Home/index.jsx
Hero/CTA, backgrounds and layout redesigned (new gradient/circuit layers, OpenAPI title); provider grid removed; CTA uses programmatic navigation.
Auth UI & Animated Visuals
web/src/components/auth/AnimatedCharacters.jsx, web/src/components/auth/LoginForm.jsx, web/src/components/auth/PasswordResetForm.jsx, web/src/components/auth/PasswordResetConfirm.jsx, web/src/components/auth/RegisterForm.jsx
Added AnimatedCharacters (mouse-following, blinking/peeking eyes) and integrated into two-column auth layouts; added password visibility toggles and layout updates.
Markdown / Mermaid & Lazy Routes
web/src/components/common/markdown/MarkdownRenderer.jsx, web/src/App.jsx
Replaced eager Mermaid import with a lazy singleton; Pricing route component changed to lazy-loaded dynamic import.
Relay Router & Proxy
router/relay-router.go, deploy/nginx.conf
Added gzip middleware for relay model-list endpoints; new Nginx reverse-proxy config with SSE/WebSocket handling, gzip, timeouts, and caching rules.
Documentation (performance)
docs/performance-optimization.md
New doc describing mermaid lazy-load, relay gzip guidance, and Nginx recommendations.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Suggested reviewers

  • creamlike1024
  • Calcium-Ion

Poem

🐰 I hopped through styles and left a glossy trail,
OpenAPI now shimmers with gradient and veil.
Eyes blink and follow, nav slides smooth and wide,
Servers stream, charts stay quiet when zeros hide.
A rabbit's tiny cheer — the UI takes a glide!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: UI customization with OpenAPI branding and Apple-inspired design system, which directly aligns with the extensive CSS tokens, glassmorphism styling, branding updates, and component visual overhauls present throughout the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sz-wuyanzu

Copy link
Copy Markdown
Author

qq

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (4)
web/src/pages/Home/index.jsx (1)

166-176: Consider using getSystemName() for the "OpenAPI" title.

The hero title hardcodes "OpenAPI" (line 168), but elsewhere the system name is configurable via getSystemName(). If administrators should be able to customize the branding displayed on the home page, consider using the dynamic helper instead.

If "OpenAPI" is intentionally fixed as a product label distinct from the configurable system name, this is acceptable—just ensure this is the intended behavior.

♻️ Optional: Use dynamic system name
+import { getSystemName } from '../../helpers/utils';
+// ... in component:
+const systemName = getSystemName();
+
 <h1 className='text-5xl md:text-6xl lg:text-7xl xl:text-8xl font-bold leading-[1.05] tracking-tight mb-6'>
-  <span className='text-semi-color-text-0'>OpenAPI</span>
+  <span className='text-semi-color-text-0'>{systemName}</span>
   <br />
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/pages/Home/index.jsx` around lines 166 - 176, The hero title
currently hardcodes "OpenAPI" in the h1 block; replace that literal with the
dynamic system name by calling getSystemName() (ensure getSystemName is imported
where used) so the span that now contains 'OpenAPI' renders getSystemName()
instead; update any necessary JSX to render the function result (e.g., inside
the span in the h1 with className 'text-semi-color-text-0') and keep the styling
and structure unchanged.
web/src/components/layout/headerbar/index.jsx (1)

68-68: Consider extracting glassmorphism styles to a reusable CSS class.

The inline style object is verbose and mixes paradigms with adjacent Tailwind classes. Since these glass effect styles appear to be part of a design system (using --glass-* CSS variables), consider defining a .glass-header or similar utility class in your CSS to improve readability and reusability.

♻️ Example CSS class approach

In your CSS file (e.g., index.css):

.glass-header {
  background: var(--glass-bg-thick);
  backdrop-filter: blur(var(--glass-blur-heavy));
  -webkit-backdrop-filter: blur(var(--glass-blur-heavy));
  box-shadow: 0 1px 0 var(--glass-border);
  border-bottom: 1px solid var(--glass-border);
}

Then in JSX:

-<header className='text-semi-color-text-0 sticky top-0 z-50 transition-all duration-300' style={{ background: 'var(--glass-bg-thick)', backdropFilter: 'blur(var(--glass-blur-heavy))', WebkitBackdropFilter: 'blur(var(--glass-blur-heavy))', boxShadow: '0 1px 0 var(--glass-border)', borderBottom: '1px solid var(--glass-border)' }}>
+<header className='glass-header text-semi-color-text-0 sticky top-0 z-50 transition-all duration-300'>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/components/layout/headerbar/index.jsx` at line 68, Extract the inline
glassmorphism styles from the header element (the <header> with className
'text-semi-color-text-0 sticky top-0 z-50 ...') into a reusable CSS utility
(e.g., .glass-header) that sets background: var(--glass-bg-thick),
backdrop-filter and -webkit-backdrop-filter using var(--glass-blur-heavy),
box-shadow: 0 1px 0 var(--glass-border), and border-bottom: 1px solid
var(--glass-border); then remove the style prop from the header and add the new
.glass-header class alongside the existing Tailwind classes so the header uses
className="text-semi-color-text-0 sticky top-0 z-50 glass-header".
web/src/components/layout/headerbar/Navigation.jsx (1)

59-63: Add cleanup safety check for resize listener.

The resize listener is properly cleaned up, but if the component unmounts during a resize event, updateIndicator could potentially access unmounted refs. This is low risk, but consider adding a mounted check or using useLayoutEffect for DOM measurements to ensure synchronous updates before paint.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/components/layout/headerbar/Navigation.jsx` around lines 59 - 63, The
resize listener may call updateIndicator after unmount; add a mounted guard:
create an isMounted ref (e.g., const isMounted = useRef(false)) set
isMounted.current = true at start of the effect and set false in the cleanup,
then wrap the resize handler and/or the start of updateIndicator with if
(!isMounted.current) return; so it never touches unmounted refs; alternatively
replace useEffect with useLayoutEffect for synchronous DOM measurements and
still include the mounted guard; ensure you update the bindings around
updateIndicator, window.addEventListener('resize', updateIndicator) and the
return cleanup to use the same guarded handler.
web/src/index.css (1)

1163-1174: Consolidate the duplicated sidebar nav state rules.

Lines 184-200 already define .sidebar-nav-item, its hover state, and its selected state. Re-declaring the same selectors here splits the source of truth and makes future nav-state tweaks easy to miss.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/index.css` around lines 1163 - 1174, The CSS repeats the same rules
for .sidebar-nav-item, .sidebar-nav-item:hover and .sidebar-nav-item-selected;
remove this duplicate block and keep a single source-of-truth definition (the
earlier declaration) so nav-state styling isn't split across the file—ensure the
earlier .sidebar-nav-item block contains the border-radius and transition, the
:hover rule uses background-color: rgba(var(--semi-blue-0), 0.06), and
.sidebar-nav-item-selected uses background-color: rgba(var(--semi-blue-0), 0.1).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.kiro/steering/project.md:
- Around line 24-42: The fenced code block containing the project directory
listing (starting with "router/        — HTTP routing (API, relay, dashboard,
web)") is missing a language specifier; update the opening triple-backtick to
include a plain text language (e.g., change ``` to ```text or ```plaintext) so
linters and renderers correctly recognize it.

In `@web/src/components/layout/headerbar/Navigation.jsx`:
- Around line 42-57: updateIndicator references isActive but doesn't list it in
the dependency array, risking a stale closure; either memoize isActive with
useCallback (e.g., const isActive = useCallback(..., [location.pathname]) ) and
then add isActive to updateIndicator's deps, or inline isActive's logic directly
inside updateIndicator and keep deps as [location.pathname, mainNavLinks];
target symbols: updateIndicator, isActive, mainNavLinks, navRef, itemRefs.

In `@web/src/index.css`:
- Around line 1177-1221: Replace the camelCase `@keyframes` name gradientFlow with
a kebab-case name (e.g., hero-gradient-flow) and update both animation
declarations that reference it in .hero-gradient-flow and html.dark
.hero-gradient-flow to use the new keyframe name; additionally add a
prefers-reduced-motion media query that disables the animation for
.hero-gradient-flow (set animation: none or animation-duration: 0s and
optionally reset background-position/background-size) so users who opt into
reduced motion won't see the infinite animation.

---

Nitpick comments:
In `@web/src/components/layout/headerbar/index.jsx`:
- Line 68: Extract the inline glassmorphism styles from the header element (the
<header> with className 'text-semi-color-text-0 sticky top-0 z-50 ...') into a
reusable CSS utility (e.g., .glass-header) that sets background:
var(--glass-bg-thick), backdrop-filter and -webkit-backdrop-filter using
var(--glass-blur-heavy), box-shadow: 0 1px 0 var(--glass-border), and
border-bottom: 1px solid var(--glass-border); then remove the style prop from
the header and add the new .glass-header class alongside the existing Tailwind
classes so the header uses className="text-semi-color-text-0 sticky top-0 z-50
glass-header".

In `@web/src/components/layout/headerbar/Navigation.jsx`:
- Around line 59-63: The resize listener may call updateIndicator after unmount;
add a mounted guard: create an isMounted ref (e.g., const isMounted =
useRef(false)) set isMounted.current = true at start of the effect and set false
in the cleanup, then wrap the resize handler and/or the start of updateIndicator
with if (!isMounted.current) return; so it never touches unmounted refs;
alternatively replace useEffect with useLayoutEffect for synchronous DOM
measurements and still include the mounted guard; ensure you update the bindings
around updateIndicator, window.addEventListener('resize', updateIndicator) and
the return cleanup to use the same guarded handler.

In `@web/src/index.css`:
- Around line 1163-1174: The CSS repeats the same rules for .sidebar-nav-item,
.sidebar-nav-item:hover and .sidebar-nav-item-selected; remove this duplicate
block and keep a single source-of-truth definition (the earlier declaration) so
nav-state styling isn't split across the file—ensure the earlier
.sidebar-nav-item block contains the border-radius and transition, the :hover
rule uses background-color: rgba(var(--semi-blue-0), 0.06), and
.sidebar-nav-item-selected uses background-color: rgba(var(--semi-blue-0), 0.1).

In `@web/src/pages/Home/index.jsx`:
- Around line 166-176: The hero title currently hardcodes "OpenAPI" in the h1
block; replace that literal with the dynamic system name by calling
getSystemName() (ensure getSystemName is imported where used) so the span that
now contains 'OpenAPI' renders getSystemName() instead; update any necessary JSX
to render the function result (e.g., inside the span in the h1 with className
'text-semi-color-text-0') and keep the styling and structure unchanged.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: e167f19b-4ca4-4ed8-84d1-3d58b2412fc7

📥 Commits

Reviewing files that changed from the base of the PR and between fbf235d and 41e2059.

⛔ Files ignored due to path filters (2)
  • web/bun.lock is excluded by !**/*.lock
  • web/public/openapi-logo.svg is excluded by !**/*.svg
📒 Files selected for processing (9)
  • .kiro/steering/project.md
  • common/constants.go
  • web/src/components/dashboard/StatsCards.jsx
  • web/src/components/layout/headerbar/Navigation.jsx
  • web/src/components/layout/headerbar/index.jsx
  • web/src/helpers/dashboard.jsx
  • web/src/helpers/utils.jsx
  • web/src/index.css
  • web/src/pages/Home/index.jsx

Comment thread .kiro/steering/project.md
Comment on lines +24 to +42
```
router/ — HTTP routing (API, relay, dashboard, web)
controller/ — Request handlers
service/ — Business logic
model/ — Data models and DB access (GORM)
relay/ — AI API relay/proxy with provider adapters
relay/channel/ — Provider-specific adapters (openai/, claude/, gemini/, aws/, etc.)
middleware/ — Auth, rate limiting, CORS, logging, distribution
setting/ — Configuration management (ratio, model, operation, system, performance)
common/ — Shared utilities (JSON, crypto, Redis, env, rate-limit, etc.)
dto/ — Data transfer objects (request/response structs)
constant/ — Constants (API types, channel types, context keys)
types/ — Type definitions (relay formats, file sources, errors)
i18n/ — Backend internationalization (go-i18n, en/zh)
oauth/ — OAuth provider implementations
pkg/ — Internal packages (cachex, ionet)
web/ — React frontend
web/src/i18n/ — Frontend internationalization (i18next, zh/en/fr/ru/ja/vi)
```

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add a language specifier to the fenced code block.

The code block at line 24 is missing a language identifier. Adding text or plaintext would satisfy linters and improve rendering consistency.

📝 Proposed fix
-```
+```text
 router/        — HTTP routing (API, relay, dashboard, web)
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)

[warning] 24-24: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.kiro/steering/project.md around lines 24 - 42, The fenced code block
containing the project directory listing (starting with "router/        — HTTP
routing (API, relay, dashboard, web)") is missing a language specifier; update
the opening triple-backtick to include a plain text language (e.g., change ```
to ```text or ```plaintext) so linters and renderers correctly recognize it.

Comment on lines +42 to +57
const updateIndicator = useCallback(() => {
const activeLink = mainNavLinks.find((l) => isActive(l));
if (!activeLink || !navRef.current) {
setIndicator((prev) => ({ ...prev, opacity: 0 }));
return;
}
const el = itemRefs.current[activeLink.itemKey];
if (!el) return;
const navRect = navRef.current.getBoundingClientRect();
const elRect = el.getBoundingClientRect();
setIndicator({
left: elRect.left - navRect.left,
width: elRect.width,
opacity: 1,
});
}, [location.pathname, mainNavLinks]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Potential stale closure: isActive referenced but not in dependencies.

The updateIndicator callback references isActive (line 43), but isActive is not included in the dependency array. Since isActive is redefined on every render and captures location.pathname, the current dependency on location.pathname happens to work, but the pattern is fragile.

Consider memoizing isActive with useCallback or inlining its logic within updateIndicator to make the dependencies explicit and avoid potential issues if the code evolves.

♻️ Option: Inline isActive logic
 const updateIndicator = useCallback(() => {
-  const activeLink = mainNavLinks.find((l) => isActive(l));
+  const activeLink = mainNavLinks.find((link) => {
+    if (link.isExternal) return false;
+    if (link.to === '/') return location.pathname === '/';
+    return location.pathname.startsWith(link.to);
+  });
   if (!activeLink || !navRef.current) {
     setIndicator((prev) => ({ ...prev, opacity: 0 }));
     return;
   }
   // ...rest unchanged
 }, [location.pathname, mainNavLinks]);
📝 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 updateIndicator = useCallback(() => {
const activeLink = mainNavLinks.find((l) => isActive(l));
if (!activeLink || !navRef.current) {
setIndicator((prev) => ({ ...prev, opacity: 0 }));
return;
}
const el = itemRefs.current[activeLink.itemKey];
if (!el) return;
const navRect = navRef.current.getBoundingClientRect();
const elRect = el.getBoundingClientRect();
setIndicator({
left: elRect.left - navRect.left,
width: elRect.width,
opacity: 1,
});
}, [location.pathname, mainNavLinks]);
const updateIndicator = useCallback(() => {
const activeLink = mainNavLinks.find((link) => {
if (link.isExternal) return false;
if (link.to === '/') return location.pathname === '/';
return location.pathname.startsWith(link.to);
});
if (!activeLink || !navRef.current) {
setIndicator((prev) => ({ ...prev, opacity: 0 }));
return;
}
const el = itemRefs.current[activeLink.itemKey];
if (!el) return;
const navRect = navRef.current.getBoundingClientRect();
const elRect = el.getBoundingClientRect();
setIndicator({
left: elRect.left - navRect.left,
width: elRect.width,
opacity: 1,
});
}, [location.pathname, mainNavLinks]);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/components/layout/headerbar/Navigation.jsx` around lines 42 - 57,
updateIndicator references isActive but doesn't list it in the dependency array,
risking a stale closure; either memoize isActive with useCallback (e.g., const
isActive = useCallback(..., [location.pathname]) ) and then add isActive to
updateIndicator's deps, or inline isActive's logic directly inside
updateIndicator and keep deps as [location.pathname, mainNavLinks]; target
symbols: updateIndicator, isActive, mainNavLinks, navRef, itemRefs.

Comment thread web/src/index.css
Comment on lines +1177 to +1221
@keyframes gradientFlow {
0% {
background-position: 0% 50%;
}
25% {
background-position: 50% 0%;
}
50% {
background-position: 100% 50%;
}
75% {
background-position: 50% 100%;
}
100% {
background-position: 0% 50%;
}
}

.hero-gradient-flow {
background: linear-gradient(
-45deg,
rgba(99, 102, 241, 0.12),
rgba(6, 182, 212, 0.08),
rgba(168, 85, 247, 0.1),
rgba(59, 130, 246, 0.08),
rgba(139, 92, 246, 0.12),
rgba(6, 182, 212, 0.06)
);
background-size: 300% 300%;
animation: gradientFlow 12s ease infinite;
}

html.dark .hero-gradient-flow {
background: linear-gradient(
-45deg,
rgba(99, 102, 241, 0.15),
rgba(6, 182, 212, 0.1),
rgba(168, 85, 247, 0.12),
rgba(59, 130, 246, 0.1),
rgba(139, 92, 246, 0.15),
rgba(6, 182, 212, 0.08)
);
background-size: 300% 300%;
animation: gradientFlow 12s ease infinite;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix the keyframe name and add a reduced-motion fallback.

gradientFlow currently violates the configured keyframes-name-pattern, and the new infinite hero animation still runs for users who opt into reduced motion.

🛠️ Suggested patch
-@keyframes gradientFlow {
+@keyframes gradient-flow {
   0% {
     background-position: 0% 50%;
   }
   25% {
     background-position: 50% 0%;
   }
   50% {
     background-position: 100% 50%;
   }
   75% {
     background-position: 50% 100%;
   }
   100% {
     background-position: 0% 50%;
   }
 }

 .hero-gradient-flow {
   background: linear-gradient(
     -45deg,
     rgba(99, 102, 241, 0.12),
     rgba(6, 182, 212, 0.08),
     rgba(168, 85, 247, 0.1),
     rgba(59, 130, 246, 0.08),
     rgba(139, 92, 246, 0.12),
     rgba(6, 182, 212, 0.06)
   );
   background-size: 300% 300%;
-  animation: gradientFlow 12s ease infinite;
+  animation: gradient-flow 12s ease infinite;
 }

 html.dark .hero-gradient-flow {
   background: linear-gradient(
     -45deg,
     rgba(99, 102, 241, 0.15),
     rgba(6, 182, 212, 0.1),
     rgba(168, 85, 247, 0.12),
     rgba(59, 130, 246, 0.1),
     rgba(139, 92, 246, 0.15),
     rgba(6, 182, 212, 0.08)
   );
   background-size: 300% 300%;
-  animation: gradientFlow 12s ease infinite;
+  animation: gradient-flow 12s ease infinite;
+}
+
+@media (prefers-reduced-motion: reduce) {
+  .hero-gradient-flow,
+  html.dark .hero-gradient-flow {
+    animation: 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.

Suggested change
@keyframes gradientFlow {
0% {
background-position: 0% 50%;
}
25% {
background-position: 50% 0%;
}
50% {
background-position: 100% 50%;
}
75% {
background-position: 50% 100%;
}
100% {
background-position: 0% 50%;
}
}
.hero-gradient-flow {
background: linear-gradient(
-45deg,
rgba(99, 102, 241, 0.12),
rgba(6, 182, 212, 0.08),
rgba(168, 85, 247, 0.1),
rgba(59, 130, 246, 0.08),
rgba(139, 92, 246, 0.12),
rgba(6, 182, 212, 0.06)
);
background-size: 300% 300%;
animation: gradientFlow 12s ease infinite;
}
html.dark .hero-gradient-flow {
background: linear-gradient(
-45deg,
rgba(99, 102, 241, 0.15),
rgba(6, 182, 212, 0.1),
rgba(168, 85, 247, 0.12),
rgba(59, 130, 246, 0.1),
rgba(139, 92, 246, 0.15),
rgba(6, 182, 212, 0.08)
);
background-size: 300% 300%;
animation: gradientFlow 12s ease infinite;
}
`@keyframes` gradient-flow {
0% {
background-position: 0% 50%;
}
25% {
background-position: 50% 0%;
}
50% {
background-position: 100% 50%;
}
75% {
background-position: 50% 100%;
}
100% {
background-position: 0% 50%;
}
}
.hero-gradient-flow {
background: linear-gradient(
-45deg,
rgba(99, 102, 241, 0.12),
rgba(6, 182, 212, 0.08),
rgba(168, 85, 247, 0.1),
rgba(59, 130, 246, 0.08),
rgba(139, 92, 246, 0.12),
rgba(6, 182, 212, 0.06)
);
background-size: 300% 300%;
animation: gradient-flow 12s ease infinite;
}
html.dark .hero-gradient-flow {
background: linear-gradient(
-45deg,
rgba(99, 102, 241, 0.15),
rgba(6, 182, 212, 0.1),
rgba(168, 85, 247, 0.12),
rgba(59, 130, 246, 0.1),
rgba(139, 92, 246, 0.15),
rgba(6, 182, 212, 0.08)
);
background-size: 300% 300%;
animation: gradient-flow 12s ease infinite;
}
`@media` (prefers-reduced-motion: reduce) {
.hero-gradient-flow,
html.dark .hero-gradient-flow {
animation: none;
}
}
🧰 Tools
🪛 Stylelint (17.5.0)

[error] 1177-1177: Expected keyframe name "gradientFlow" to be kebab-case (keyframes-name-pattern)

(keyframes-name-pattern)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/index.css` around lines 1177 - 1221, Replace the camelCase `@keyframes`
name gradientFlow with a kebab-case name (e.g., hero-gradient-flow) and update
both animation declarations that reference it in .hero-gradient-flow and
html.dark .hero-gradient-flow to use the new keyframe name; additionally add a
prefers-reduced-motion media query that disables the animation for
.hero-gradient-flow (set animation: none or animation-duration: 0s and
optionally reset background-position/background-size) so users who opt into
reduced motion won't see the infinite animation.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

♻️ Duplicate comments (1)
web/src/index.css (1)

1188-1258: ⚠️ Potential issue | 🟡 Minor

Rename these keyframes and add a reduced-motion escape hatch.

This still violates the configured keyframes-name-pattern, and the hero animations continue to run for users who opt into reduced motion.

🛠️ Suggested patch
-@keyframes gradientFlow {
+@keyframes gradient-flow {
@@
-  animation: gradientFlow 12s ease infinite;
+  animation: gradient-flow 12s ease infinite;
@@
-  animation: gradientFlow 12s ease infinite;
+  animation: gradient-flow 12s ease infinite;
@@
-@keyframes fadeInUp {
+@keyframes fade-in-up {
@@
-  animation: fadeInUp 0.6s var(--ease-out-expo) both;
+  animation: fade-in-up 0.6s var(--ease-out-expo) both;
@@
-@keyframes circuitPulse {
+@keyframes circuit-pulse {
@@
-  animation: circuitPulse 6s ease-in-out infinite;
+  animation: circuit-pulse 6s ease-in-out infinite;
+}
+
+@media (prefers-reduced-motion: reduce) {
+  .hero-gradient-flow,
+  html.dark .hero-gradient-flow,
+  .animate-fade-in-up,
+  .hero-circuit-bg {
+    animation: none !important;
+  }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/index.css` around lines 1188 - 1258, Rename the keyframes to match
the project's keyframes-name-pattern (e.g., gradientFlow -> hero-gradient-flow,
fadeInUp -> hero-fade-in-up, circuitPulse -> hero-circuit-pulse) and update the
animation declarations on .hero-gradient-flow, .animate-fade-in-up and
.hero-circuit-bg to reference the new keyframe names; additionally add a
reduced-motion escape hatch using `@media` (prefers-reduced-motion: reduce) {
.hero-gradient-flow, .animate-fade-in-up, .hero-circuit-bg { animation: none
!important; } } so animations are disabled for users who prefer reduced motion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@web/src/components/auth/AnimatedCharacters.jsx`:
- Around line 97-129: The nested setTimeouts in the blinking and peeking effects
(the schedule function and the inner timeouts that call
setIsPurpleBlinking/setIsBlackBlinking and the inner timeout that sets/clears
setIsPurplePeeking) are never stored or cleared; change these effects to capture
every timeout ID (both outer and inner) in refs (e.g., useRef arrays or a single
Map) and clear them in each effect cleanup so no inner callback runs after
unmount; alternatively replace schedule with a single cancellable loop that
checks a mounted/ref flag before rescheduling so setIsPurpleBlinking,
setIsBlackBlinking and setIsPurplePeeking no longer run against an unmounted
component.
- Around line 8-17: Pupils/EyeBalls each add their own window.mousemove
listener; move pointer tracking into the parent AnimatedCharacters and pass the
pointer coords down as props to avoid N global listeners and repeated state/rect
reads. Remove local useState/useEffect mouse handling from Pupil and EyeBall
(delete their handleMouseMove, setMouseX/setMouseY, and
addEventListener/removeEventListener logic) and change their signatures to
accept mouseX/mouseY (or a single pointer object) as props (keep existing
forceLookX/forceLookY handling intact). In AnimatedCharacters create a single
window mousemove handler (or rAF-throttled handler) that updates one pointer
state, compute any necessary getBoundingClientRect calls inside EyeBall once via
ref when needed, and pass the pointer coordinates into each EyeBall/Pupil
instance so they compute pupil offsets locally without their own global
listeners.

In `@web/src/components/auth/LoginForm.jsx`:
- Line 108: The showPassword state is never toggled so AnimatedCharacters always
receives false; wire the state to the password-visibility control or remove the
prop. Add a toggle handler (using setShowPassword) tied to the password reveal
UI in LoginForm (e.g., the eye icon/button next to the password input) that
flips showPassword (setShowPassword(prev => !prev)) and ensure
AnimatedCharacters receives the updated showPassword prop, or if you don't
implement the toggle yet, remove the showPassword prop usage from
AnimatedCharacters to avoid dead state.
- Around line 952-977: The root wrapper currently uses "overflow-hidden" which
prevents the right login column from scrolling when OAuth
buttons/agreements/Turnstile expand; remove "overflow-hidden" from the root
div's className (the div with class 'min-h-screen grid lg:grid-cols-2') and
instead enable scrolling on the right pane by adding an overflow class (e.g.,
add "overflow-auto" and "min-h-screen" or "max-h-screen") to the right column
div with class 'flex items-center justify-center p-8 bg-semi-color-bg-0' (and,
if needed, to the inner container 'w-full max-w-sm') so AnimatedCharacters,
inputs, and expanded provider/agreement/Turnstile UI remain reachable.

In `@web/src/components/layout/headerbar/Navigation.jsx`:
- Around line 101-106: The Link elements in Navigation.jsx need an accessible
indicator for the active route; update the link rendering to add
aria-current="page" only when that Link is the active route—either switch Link
-> NavLink and use NavLink's isActive to set aria-current={isActive ? "page" :
undefined}, or compute active by comparing the current location
(useLocation().pathname) to targetPath and add aria-current={isActive ? "page" :
undefined} to the Link; reference the Link element, targetPath, and
itemRefs/itemKey to locate where to add this attribute.

In `@web/src/pages/Home/index.jsx`:
- Around line 230-235: The icon-only copy Button (the Button using IconCopy and
onClick={handleCopyBaseURL}) is currently unnamed for assistive tech; add an
accessible name by passing an aria-label prop (e.g., aria-label="Copy base URL"
or similar) to that Button component so screen readers can announce its purpose
while keeping the icon-only appearance.
- Around line 258-260: The onClick handler for the demo-site button uses
window.open('https://github.com/QuantumNous/new-api', '_blank') which leaves
window.opener set; update the onClick in the component that renders when
isDemoSiteMode && statusState?.status?.version to open the tab with noreferrer
and noopener protection (e.g., pass "noopener,noreferrer" as the third argument
to window.open or use an anchor with rel="noopener noreferrer" and
target="_blank") so reverse-tabnabbing is prevented.

---

Duplicate comments:
In `@web/src/index.css`:
- Around line 1188-1258: Rename the keyframes to match the project's
keyframes-name-pattern (e.g., gradientFlow -> hero-gradient-flow, fadeInUp ->
hero-fade-in-up, circuitPulse -> hero-circuit-pulse) and update the animation
declarations on .hero-gradient-flow, .animate-fade-in-up and .hero-circuit-bg to
reference the new keyframe names; additionally add a reduced-motion escape hatch
using `@media` (prefers-reduced-motion: reduce) { .hero-gradient-flow,
.animate-fade-in-up, .hero-circuit-bg { animation: none !important; } } so
animations are disabled for users who prefer reduced motion.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8d146b04-e38c-4173-a206-14cb8b56d01e

📥 Commits

Reviewing files that changed from the base of the PR and between 41e2059 and 68788ec.

⛔ Files ignored due to path filters (1)
  • web/public/hero-bg.svg is excluded by !**/*.svg
📒 Files selected for processing (8)
  • web/src/components/auth/AnimatedCharacters.jsx
  • web/src/components/auth/LoginForm.jsx
  • web/src/components/layout/headerbar/HeaderLogo.jsx
  • web/src/components/layout/headerbar/Navigation.jsx
  • web/src/components/layout/headerbar/UserArea.jsx
  • web/src/components/layout/headerbar/index.jsx
  • web/src/index.css
  • web/src/pages/Home/index.jsx
💤 Files with no reviewable changes (1)
  • web/src/components/layout/headerbar/UserArea.jsx
✅ Files skipped from review due to trivial changes (1)
  • web/src/components/layout/headerbar/index.jsx

Comment on lines +8 to +17
const Pupil = ({ size = 12, maxDistance = 5, pupilColor = 'black', forceLookX, forceLookY }) => {
const [mouseX, setMouseX] = useState(0);
const [mouseY, setMouseY] = useState(0);
const pupilRef = useRef(null);

useEffect(() => {
const handleMouseMove = (e) => { setMouseX(e.clientX); setMouseY(e.clientY); };
window.addEventListener('mousemove', handleMouseMove);
return () => window.removeEventListener('mousemove', handleMouseMove);
}, []);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid per-eye mousemove subscriptions.

Each Pupil/EyeBall instance subscribes to window.mousemove and keeps its own pointer state. With the current tree that's nine global listeners, nine state updates, and repeated getBoundingClientRect() reads for every pointer move on the login page. Track the pointer once in AnimatedCharacters and pass the coordinates down instead.

Also applies to: 39-48, 79-95

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/components/auth/AnimatedCharacters.jsx` around lines 8 - 17,
Pupils/EyeBalls each add their own window.mousemove listener; move pointer
tracking into the parent AnimatedCharacters and pass the pointer coords down as
props to avoid N global listeners and repeated state/rect reads. Remove local
useState/useEffect mouse handling from Pupil and EyeBall (delete their
handleMouseMove, setMouseX/setMouseY, and addEventListener/removeEventListener
logic) and change their signatures to accept mouseX/mouseY (or a single pointer
object) as props (keep existing forceLookX/forceLookY handling intact). In
AnimatedCharacters create a single window mousemove handler (or rAF-throttled
handler) that updates one pointer state, compute any necessary
getBoundingClientRect calls inside EyeBall once via ref when needed, and pass
the pointer coordinates into each EyeBall/Pupil instance so they compute pupil
offsets locally without their own global listeners.

Comment on lines +97 to +129
// Blinking
useEffect(() => {
const schedule = (setter) => {
const t = setTimeout(() => {
setter(true);
setTimeout(() => { setter(false); schedule(setter); }, 150);
}, Math.random() * 4000 + 3000);
return t;
};
const t1 = schedule(setIsPurpleBlinking);
const t2 = schedule(setIsBlackBlinking);
return () => { clearTimeout(t1); clearTimeout(t2); };
}, []);

useEffect(() => {
if (isTyping) {
setIsLookingAtEachOther(true);
const t = setTimeout(() => setIsLookingAtEachOther(false), 800);
return () => clearTimeout(t);
}
setIsLookingAtEachOther(false);
}, [isTyping]);

useEffect(() => {
if (passwordLength > 0 && showPassword) {
const t = setTimeout(() => {
setIsPurplePeeking(true);
setTimeout(() => setIsPurplePeeking(false), 800);
}, Math.random() * 3000 + 2000);
return () => clearTimeout(t);
}
setIsPurplePeeking(false);
}, [passwordLength, showPassword, isPurplePeeking]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Track and clear the nested timers.

Both animation effects create inner setTimeout calls that are never stored anywhere. After the component unmounts, those callbacks can still fire and keep rescheduling blink/peek work against an unmounted tree. Store every timeout ID in refs, or switch to a single cancellable loop.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/components/auth/AnimatedCharacters.jsx` around lines 97 - 129, The
nested setTimeouts in the blinking and peeking effects (the schedule function
and the inner timeouts that call setIsPurpleBlinking/setIsBlackBlinking and the
inner timeout that sets/clears setIsPurplePeeking) are never stored or cleared;
change these effects to capture every timeout ID (both outer and inner) in refs
(e.g., useRef arrays or a single Map) and clear them in each effect cleanup so
no inner callback runs after unmount; alternatively replace schedule with a
single cancellable loop that checks a mounted/ref flag before rescheduling so
setIsPurpleBlinking, setIsBlackBlinking and setIsPurplePeeking no longer run
against an unmounted component.

Comment thread web/src/components/auth/LoginForm.jsx
Comment on lines +952 to +977
<div className='min-h-screen grid lg:grid-cols-2 overflow-hidden'>
{/* Left: Animated Characters */}
<div className='relative hidden lg:flex flex-col justify-between p-12' style={{
background: 'linear-gradient(135deg, rgba(99,102,241,0.15), rgba(6,182,212,0.1), rgba(168,85,247,0.12))',
}}>
<div className='absolute inset-0 opacity-[0.03] pointer-events-none' style={{
backgroundImage: 'radial-gradient(circle, rgba(255,255,255,0.15) 1px, transparent 1px)',
backgroundSize: '24px 24px',
}} />
<div className='relative z-10 flex items-center gap-2'>
<img src={getLogo()} alt='logo' className='w-8 h-8 rounded-full' />
<span className='text-lg font-semibold text-semi-color-text-0'>{getSystemName()}</span>
</div>
<div className='relative z-10 flex items-end justify-center flex-1'>
<AnimatedCharacters
isTyping={inputs.username.length > 0 && inputs.password.length === 0}
showPassword={showPassword}
passwordLength={inputs.password.length}
/>
</div>
<div className='relative z-10' />
</div>

{/* Right: Login Form */}
<div className='flex items-center justify-center p-8 bg-semi-color-bg-0'>
<div className='w-full max-w-sm'>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Allow the login pane to scroll.

The new overflow-hidden root plus the centered right column can make part of the auth UI unreachable once OAuth providers, agreements, and Turnstile push the content past the viewport height. That blocks login on short screens and high zoom levels.

🩹 Suggested patch
-    <div className='min-h-screen grid lg:grid-cols-2 overflow-hidden'>
+    <div className='min-h-screen grid lg:grid-cols-2 overflow-x-hidden'>

-      <div className='flex items-center justify-center p-8 bg-semi-color-bg-0'>
+      <div className='flex items-start lg:items-center justify-center p-8 bg-semi-color-bg-0 overflow-y-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.

Suggested change
<div className='min-h-screen grid lg:grid-cols-2 overflow-hidden'>
{/* Left: Animated Characters */}
<div className='relative hidden lg:flex flex-col justify-between p-12' style={{
background: 'linear-gradient(135deg, rgba(99,102,241,0.15), rgba(6,182,212,0.1), rgba(168,85,247,0.12))',
}}>
<div className='absolute inset-0 opacity-[0.03] pointer-events-none' style={{
backgroundImage: 'radial-gradient(circle, rgba(255,255,255,0.15) 1px, transparent 1px)',
backgroundSize: '24px 24px',
}} />
<div className='relative z-10 flex items-center gap-2'>
<img src={getLogo()} alt='logo' className='w-8 h-8 rounded-full' />
<span className='text-lg font-semibold text-semi-color-text-0'>{getSystemName()}</span>
</div>
<div className='relative z-10 flex items-end justify-center flex-1'>
<AnimatedCharacters
isTyping={inputs.username.length > 0 && inputs.password.length === 0}
showPassword={showPassword}
passwordLength={inputs.password.length}
/>
</div>
<div className='relative z-10' />
</div>
{/* Right: Login Form */}
<div className='flex items-center justify-center p-8 bg-semi-color-bg-0'>
<div className='w-full max-w-sm'>
<div className='min-h-screen grid lg:grid-cols-2 overflow-x-hidden'>
{/* Left: Animated Characters */}
<div className='relative hidden lg:flex flex-col justify-between p-12' style={{
background: 'linear-gradient(135deg, rgba(99,102,241,0.15), rgba(6,182,212,0.1), rgba(168,85,247,0.12))',
}}>
<div className='absolute inset-0 opacity-[0.03] pointer-events-none' style={{
backgroundImage: 'radial-gradient(circle, rgba(255,255,255,0.15) 1px, transparent 1px)',
backgroundSize: '24px 24px',
}} />
<div className='relative z-10 flex items-center gap-2'>
<img src={getLogo()} alt='logo' className='w-8 h-8 rounded-full' />
<span className='text-lg font-semibold text-semi-color-text-0'>{getSystemName()}</span>
</div>
<div className='relative z-10 flex items-end justify-center flex-1'>
<AnimatedCharacters
isTyping={inputs.username.length > 0 && inputs.password.length === 0}
showPassword={showPassword}
passwordLength={inputs.password.length}
/>
</div>
<div className='relative z-10' />
</div>
{/* Right: Login Form */}
<div className='flex items-start lg:items-center justify-center p-8 bg-semi-color-bg-0 overflow-y-auto'>
<div className='w-full max-w-sm'>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/components/auth/LoginForm.jsx` around lines 952 - 977, The root
wrapper currently uses "overflow-hidden" which prevents the right login column
from scrolling when OAuth buttons/agreements/Turnstile expand; remove
"overflow-hidden" from the root div's className (the div with class
'min-h-screen grid lg:grid-cols-2') and instead enable scrolling on the right
pane by adding an overflow class (e.g., add "overflow-auto" and "min-h-screen"
or "max-h-screen") to the right column div with class 'flex items-center
justify-center p-8 bg-semi-color-bg-0' (and, if needed, to the inner container
'w-full max-w-sm') so AnimatedCharacters, inputs, and expanded
provider/agreement/Turnstile UI remain reachable.

Comment on lines +101 to +106
<Link
key={link.itemKey}
ref={(el) => { itemRefs.current[link.itemKey] = el; }}
to={targetPath}
className={`${baseClasses} ${colorClasses}`}
>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Expose the active route semantically.

The new capsule indicator is purely visual right now. Adding aria-current="page" to the active Link lets screen readers announce which section is selected.

♿ Suggested patch
         <Link
           key={link.itemKey}
           ref={(el) => { itemRefs.current[link.itemKey] = el; }}
           to={targetPath}
+          aria-current={active ? 'page' : undefined}
           className={`${baseClasses} ${colorClasses}`}
         >
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/components/layout/headerbar/Navigation.jsx` around lines 101 - 106,
The Link elements in Navigation.jsx need an accessible indicator for the active
route; update the link rendering to add aria-current="page" only when that Link
is the active route—either switch Link -> NavLink and use NavLink's isActive to
set aria-current={isActive ? "page" : undefined}, or compute active by comparing
the current location (useLocation().pathname) to targetPath and add
aria-current={isActive ? "page" : undefined} to the Link; reference the Link
element, targetPath, and itemRefs/itemKey to locate where to add this attribute.

Comment on lines +230 to +235
<Button
type='primary'
onClick={handleCopyBaseURL}
icon={<IconCopy />}
className='!rounded-full'
/>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Give the icon-only copy button an accessible name.

This renders as an unnamed button to assistive tech today. Adding an aria-label is enough.

♿ Suggested patch
                         <Button
                           type='primary'
                           onClick={handleCopyBaseURL}
                           icon={<IconCopy />}
                           className='!rounded-full'
+                          aria-label={t('复制基址')}
                         />
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<Button
type='primary'
onClick={handleCopyBaseURL}
icon={<IconCopy />}
className='!rounded-full'
/>
<Button
type='primary'
onClick={handleCopyBaseURL}
icon={<IconCopy />}
className='!rounded-full'
aria-label={t('复制基址')}
/>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/pages/Home/index.jsx` around lines 230 - 235, The icon-only copy
Button (the Button using IconCopy and onClick={handleCopyBaseURL}) is currently
unnamed for assistive tech; add an accessible name by passing an aria-label prop
(e.g., aria-label="Copy base URL" or similar) to that Button component so screen
readers can announce its purpose while keeping the icon-only appearance.

Comment on lines +258 to +260
{isDemoSiteMode && statusState?.status?.version && (
<button
onClick={() => window.open('https://github.com/QuantumNous/new-api', '_blank')}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Open the GitHub tab without an opener.

window.open(..., '_blank') keeps window.opener by default. Adding noopener,noreferrer closes that reverse-tabnabbing path.

🔐 Suggested patch
-                    onClick={() => window.open('https://github.com/QuantumNous/new-api', '_blank')}
+                    onClick={() =>
+                      window.open(
+                        'https://github.com/QuantumNous/new-api',
+                        '_blank',
+                        'noopener,noreferrer',
+                      )
+                    }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/pages/Home/index.jsx` around lines 258 - 260, The onClick handler for
the demo-site button uses window.open('https://github.com/QuantumNous/new-api',
'_blank') which leaves window.opener set; update the onClick in the component
that renders when isDemoSiteMode && statusState?.status?.version to open the tab
with noreferrer and noopener protection (e.g., pass "noopener,noreferrer" as the
third argument to window.open or use an anchor with rel="noopener noreferrer"
and target="_blank") so reverse-tabnabbing is prevented.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (3)
web/src/components/auth/AnimatedCharacters.jsx (2)

97-109: ⚠️ Potential issue | 🟠 Major

Track every nested timeout.

Cleanup only owns the first timeout ID. Once a blink or peek timer fires, the inner setTimeout callbacks can still update state after unmount, and the blink loop keeps rescheduling work that the cleanup never clears. Store all timeout IDs in refs, or replace these effects with a single cancellable loop.

Also applies to: 120-129

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/components/auth/AnimatedCharacters.jsx` around lines 97 - 109, The
cleanup currently only clears the outer timeout IDs (t1/t2) leaving the inner
timeouts and continued rescheduling alive; update the blinking effect (useEffect
and the inner schedule function that uses setTimeout and calls
setIsPurpleBlinking / setIsBlackBlinking) to track every timeout ID (e.g., store
IDs in a ref array or Set) or replace schedule with a single cancellable loop,
and in the effect’s return handler iterate over and clear all stored timeout IDs
(and stop rescheduling) so no inner timers or reschedules run after unmount.

8-29: ⚠️ Potential issue | 🟠 Major

Consolidate pointer tracking into AnimatedCharacters.

The parent already tracks pointer state, but every Pupil and EyeBall still adds its own window.mousemove listener and does its own getBoundingClientRect() read. That multiplies global subscriptions and layout work on every move, and the (0,0) default makes the characters snap toward the top-left before the first real pointer event. Keep one pointer source in the parent and pass it down as props instead.

Also applies to: 39-60, 91-95

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/components/auth/AnimatedCharacters.jsx` around lines 8 - 29, The
Pupil/EyeBall components currently each add window.mousemove listeners and call
getBoundingClientRect() on every render causing duplicated subscriptions and
layout thrash; move pointer tracking into AnimatedCharacters (keep a single
mousemove there, store pointerX/pointerY or null), remove the
window.addEventListener logic from Pupil and EyeBall, accept pointerX/pointerY
(and existing forceLookX/forceLookY) as props, and change calc() to use the
passed pointer values (treat null/undefined pointer as "no pointer" to avoid the
initial (0,0) snap); also stop repeated getBoundingClientRect() reads in calc()
by either computing the element center once on mount (store in a ref) or by
having AnimatedCharacters compute and pass down the element center/offsets so
Pupil/EyeBall only do cheap math with supplied coordinates.
web/src/components/auth/LoginForm.jsx (1)

960-1003: ⚠️ Potential issue | 🟠 Major

Don't lock the auth page viewport.

overflow-hidden on the root plus a vertically centered right pane can still make the lower part of the form unreachable once OAuth providers, agreements, and Turnstile push the content past the viewport height. Let the page overflow on Y and make the right pane scroll instead.

🩹 Suggested patch
-    <div className='min-h-screen grid lg:grid-cols-2 overflow-hidden'>
+    <div className='min-h-screen grid lg:grid-cols-2 overflow-x-hidden'>
@@
-      <div className='flex items-center justify-center p-8 bg-semi-color-bg-0'>
+      <div className='flex items-start lg:items-center justify-center p-8 bg-semi-color-bg-0 overflow-y-auto'>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/components/auth/LoginForm.jsx` around lines 960 - 1003, The page is
currently locked by the root div class "min-h-screen grid lg:grid-cols-2
overflow-hidden" which prevents vertical scrolling when content (OAuth options,
agreements, Turnstile) exceeds the viewport; remove the "overflow-hidden" from
that root container and make the right pane (the div with class 'flex
items-center justify-center p-8 bg-semi-color-bg-0') scrollable by adding
"overflow-y-auto" and switching vertical centering to start (replace
"items-center" with "items-start" and add appropriate vertical padding like
"py-8") so the login form and modals (renderEmailLoginForm, renderOAuthOptions,
renderWeChatLoginModal, render2FAModal) can be scrolled into view on small
viewports.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@web/src/components/auth/LoginForm.jsx`:
- Around line 767-774: Replace the non-focusable span suffix with a proper
<button> element for the password toggle: use the existing showPassword state
and setShowPassword handler in the onClick, add type="button" so it won't submit
forms, include aria-pressed={showPassword} and an accessible name via
aria-label={t('password.toggleVisibility')} (use useTranslation() / t('...') per
i18n rules), and keep the existing emoji visuals; ensure the component imports
useTranslation and uses t(...) for any visible text/labels.

---

Duplicate comments:
In `@web/src/components/auth/AnimatedCharacters.jsx`:
- Around line 97-109: The cleanup currently only clears the outer timeout IDs
(t1/t2) leaving the inner timeouts and continued rescheduling alive; update the
blinking effect (useEffect and the inner schedule function that uses setTimeout
and calls setIsPurpleBlinking / setIsBlackBlinking) to track every timeout ID
(e.g., store IDs in a ref array or Set) or replace schedule with a single
cancellable loop, and in the effect’s return handler iterate over and clear all
stored timeout IDs (and stop rescheduling) so no inner timers or reschedules run
after unmount.
- Around line 8-29: The Pupil/EyeBall components currently each add
window.mousemove listeners and call getBoundingClientRect() on every render
causing duplicated subscriptions and layout thrash; move pointer tracking into
AnimatedCharacters (keep a single mousemove there, store pointerX/pointerY or
null), remove the window.addEventListener logic from Pupil and EyeBall, accept
pointerX/pointerY (and existing forceLookX/forceLookY) as props, and change
calc() to use the passed pointer values (treat null/undefined pointer as "no
pointer" to avoid the initial (0,0) snap); also stop repeated
getBoundingClientRect() reads in calc() by either computing the element center
once on mount (store in a ref) or by having AnimatedCharacters compute and pass
down the element center/offsets so Pupil/EyeBall only do cheap math with
supplied coordinates.

In `@web/src/components/auth/LoginForm.jsx`:
- Around line 960-1003: The page is currently locked by the root div class
"min-h-screen grid lg:grid-cols-2 overflow-hidden" which prevents vertical
scrolling when content (OAuth options, agreements, Turnstile) exceeds the
viewport; remove the "overflow-hidden" from that root container and make the
right pane (the div with class 'flex items-center justify-center p-8
bg-semi-color-bg-0') scrollable by adding "overflow-y-auto" and switching
vertical centering to start (replace "items-center" with "items-start" and add
appropriate vertical padding like "py-8") so the login form and modals
(renderEmailLoginForm, renderOAuthOptions, renderWeChatLoginModal,
render2FAModal) can be scrolled into view on small viewports.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8fde5399-b13f-46dd-b9c7-b50cb97c21e1

📥 Commits

Reviewing files that changed from the base of the PR and between 68788ec and fb04176.

📒 Files selected for processing (2)
  • web/src/components/auth/AnimatedCharacters.jsx
  • web/src/components/auth/LoginForm.jsx

Comment on lines +767 to +774
suffix={
<span
style={{ cursor: 'pointer', display: 'flex', alignItems: 'center', padding: '0 4px' }}
onClick={() => setShowPassword(!showPassword)}
>
{showPassword ? '🙈' : '👁️'}
</span>
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, locate the LoginForm.jsx file
fd -type f "LoginForm.jsx" web/

Repository: QuantumNous/new-api

Length of output: 233


🏁 Script executed:

# Check the file size and read the password toggle area around lines 767-774
wc -l web/src/components/auth/LoginForm.jsx

Repository: QuantumNous/new-api

Length of output: 105


🏁 Script executed:

# Read the specific lines mentioned in the review
sed -n '760,780p' web/src/components/auth/LoginForm.jsx

Repository: QuantumNous/new-api

Length of output: 933


🏁 Script executed:

# Check imports at the top of the file to see if useTranslation is imported
head -50 web/src/components/auth/LoginForm.jsx

Repository: QuantumNous/new-api

Length of output: 1518


🏁 Script executed:

# Verify i18n translation files exist and check their structure
fd -type f "*.json" web/src/i18n/locales/

Repository: QuantumNous/new-api

Length of output: 233


🏁 Script executed:

# Check one translation file to see the flat JSON structure with Chinese keys
cat web/src/i18n/locales/zh.json | head -20

Repository: QuantumNous/new-api

Length of output: 123


🏁 Script executed:

# Search for useTranslation import in LoginForm.jsx
rg "useTranslation" web/src/components/auth/LoginForm.jsx

Repository: QuantumNous/new-api

Length of output: 144


🏁 Script executed:

# Search for where i18n locales are actually stored
find web -type d -name "locales" 2>/dev/null | head -10

Repository: QuantumNous/new-api

Length of output: 83


🏁 Script executed:

# Also check for i18n configuration
fd -type f "i18n" web/ 2>/dev/null | head -20

Repository: QuantumNous/new-api

Length of output: 45


🏁 Script executed:

# Look for any translation file patterns
find web -type f -name "*.json" | grep -i "i18n\|locale\|lang" | head -20

Repository: QuantumNous/new-api

Length of output: 271


🏁 Script executed:

# Check the rest of imports in LoginForm to find useTranslation
sed -n '40,100p' web/src/components/auth/LoginForm.jsx

Repository: QuantumNous/new-api

Length of output: 2290


🏁 Script executed:

# Check the Chinese translation file for any existing password-related strings
cat web/src/i18n/locales/zh-CN.json | head -50

Repository: QuantumNous/new-api

Length of output: 3817


🏁 Script executed:

# Search for password-related translations that might already exist
rg "密码|显示|隐藏" web/src/i18n/locales/

Repository: QuantumNous/new-api

Length of output: 50375


🏁 Script executed:

# Verify the exact lines around the password toggle to confirm no aria attributes exist
sed -n '765,775p' web/src/components/auth/LoginForm.jsx

Repository: QuantumNous/new-api

Length of output: 546


🏁 Script executed:

# Check if there are any other interactive span elements in the codebase that use buttons instead
rg "type=['\"]button['\"]" web/src/components/auth/LoginForm.jsx -A 2 -B 2 | head -30

Repository: QuantumNous/new-api

Length of output: 45


Make the password reveal control keyboard-accessible.

The clickable span is not keyboard-focusable and lacks an accessible name, preventing keyboard and screen-reader users from toggling password visibility. Convert to a button with aria-label, aria-pressed, and translation support via t().

Suggested implementation
                  suffix={
                    <button
                      type='button'
                      aria-label={showPassword ? t('隐藏密码') : t('显示密码')}
                      aria-pressed={showPassword}
                      className='flex items-center px-1 bg-transparent border-0 cursor-pointer'
                      onMouseDown={(e) => e.preventDefault()}
                      onClick={() => setShowPassword((prev) => !prev)}
                    >
                      {showPassword ? '🙈' : '👁️'}
                    </button>
                  }

Per coding guidelines, all user-facing text in web/src/**/*.{ts,tsx,js,jsx} must be internationalized using useTranslation() and t('中文key').

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/components/auth/LoginForm.jsx` around lines 767 - 774, Replace the
non-focusable span suffix with a proper <button> element for the password
toggle: use the existing showPassword state and setShowPassword handler in the
onClick, add type="button" so it won't submit forms, include
aria-pressed={showPassword} and an accessible name via
aria-label={t('password.toggleVisibility')} (use useTranslation() / t('...') per
i18n rules), and keep the existing emoji visuals; ensure the component imports
useTranslation and uses t(...) for any visible text/labels.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
web/src/components/auth/RegisterForm.jsx (2)

805-808: Use existing logo and systemName variables.

getLogo() and getSystemName() are already called and stored in variables at lines 116-117. Reusing them avoids redundant function calls.

♻️ Suggested fix
 <div className='lg:hidden flex items-center justify-center gap-2 mb-8'>
-  <img src={getLogo()} alt='logo' className='w-8 h-8 rounded-full' />
-  <span className='text-lg font-semibold'>{getSystemName()}</span>
+  <img src={logo} alt='logo' className='w-8 h-8 rounded-full' />
+  <span className='text-lg font-semibold'>{systemName}</span>
 </div>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/components/auth/RegisterForm.jsx` around lines 805 - 808, The JSX is
redundantly calling getLogo() and getSystemName() instead of using the already
stored variables; update the component to use the existing logo and systemName
variables (replace getLogo() with logo and getSystemName() with systemName) in
the img src and span text so you reuse the values computed earlier (referencing
getLogo(), getSystemName(), logo, and systemName to locate and change the code).

595-602: Consider using Semi UI icons for password visibility toggle.

The emoji icons (👁️/🙈) are inconsistent with the rest of the UI which uses @douyinfe/semi-icons. Semi UI provides IconEyeOpened and IconEyeClosed (or similar) for this purpose, which would maintain visual consistency and provide better accessibility.

♻️ Suggested refactor using Semi icons
+import { IconEyeOpened, IconEyeClosedSolid } from '@douyinfe/semi-icons';
 suffix={
   <span
-    style={{ cursor: 'pointer', display: 'flex', alignItems: 'center', padding: '0 4px' }}
+    style={{ cursor: 'pointer', display: 'flex', alignItems: 'center', padding: '0 8px' }}
     onClick={() => setShowPassword(!showPassword)}
+    role="button"
+    aria-label={showPassword ? t('隐藏密码') : t('显示密码')}
   >
-    {showPassword ? '🙈' : '👁️'}
+    {showPassword ? <IconEyeClosedSolid /> : <IconEyeOpened />}
   </span>
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/components/auth/RegisterForm.jsx` around lines 595 - 602, Replace the
emoji visibility toggle in the suffix with Semi UI icons: import IconEyeOpened
and IconEyeClosed from `@douyinfe/semi-icons`, render IconEyeOpened when
showPassword is true and IconEyeClosed when false (keeping the existing onClick
that calls setShowPassword(!showPassword)), and replace the raw <span> with the
Semi Icon (or a Button/Icon wrapper) so styling and sizing match the rest of the
UI; also add an accessible aria-label (e.g., "Toggle password visibility") and
keep the existing inline styles or move them to className for consistent layout.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@web/src/components/auth/RegisterForm.jsx`:
- Around line 805-808: The JSX is redundantly calling getLogo() and
getSystemName() instead of using the already stored variables; update the
component to use the existing logo and systemName variables (replace getLogo()
with logo and getSystemName() with systemName) in the img src and span text so
you reuse the values computed earlier (referencing getLogo(), getSystemName(),
logo, and systemName to locate and change the code).
- Around line 595-602: Replace the emoji visibility toggle in the suffix with
Semi UI icons: import IconEyeOpened and IconEyeClosed from `@douyinfe/semi-icons`,
render IconEyeOpened when showPassword is true and IconEyeClosed when false
(keeping the existing onClick that calls setShowPassword(!showPassword)), and
replace the raw <span> with the Semi Icon (or a Button/Icon wrapper) so styling
and sizing match the rest of the UI; also add an accessible aria-label (e.g.,
"Toggle password visibility") and keep the existing inline styles or move them
to className for consistent layout.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1f28c36c-ddec-4e2f-89b2-565a12ff85ff

📥 Commits

Reviewing files that changed from the base of the PR and between fb04176 and d961cf8.

📒 Files selected for processing (3)
  • web/src/components/auth/PasswordResetConfirm.jsx
  • web/src/components/auth/PasswordResetForm.jsx
  • web/src/components/auth/RegisterForm.jsx

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (2)
docs/performance-optimization.md (1)

29-37: Update code sample if singleton pattern is fixed.

If the race condition fix for getMermaid() is applied in the actual code, update this documentation sample to reflect the promise-based singleton pattern for consistency.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/performance-optimization.md` around lines 29 - 37, The docs snippet
currently shows a naive singleton using mermaidInstance and getMermaid; update
it to match the promise-based singleton used in code by replacing
mermaidInstance with a single shared promise (e.g., mermaidPromise) and have
getMermaid return/await that promise so concurrent callers share the same
import/initialization sequence; reference mermaidInstance and getMermaid in the
text and ensure the example demonstrates storing the import/initialization as a
Promise so subsequent calls await the same promise rather than re-entering
initialization.
deploy/nginx.conf (1)

52-69: Location order note: regex may partially overlap with /assets/.

Nginx processes locations in a specific order: exact match → longest prefix → regex (in order). The /assets/ prefix location (line 52) will match first for /assets/* paths, but the regex at line 62 could theoretically match if nginx's order processing differs in edge cases.

Current config should work correctly, but for clarity, consider adding ^~ modifier to the /assets/ location to ensure it takes precedence:

♻️ Suggested improvement
-    location /assets/ {
+    location ^~ /assets/ {
         proxy_pass http://openapi_backend;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@deploy/nginx.conf` around lines 52 - 69, The /assets/ prefix location may
overlap with the regex location and to guarantee nginx chooses it before regex
matches, change the prefix location declaration for /assets/ (the "location
/assets/" block) to use the ^~ modifier so it takes precedence over regex
locations; update the "location /assets/" directive to "location ^~ /assets/"
and leave the rest of that block (proxy_pass, headers, expires, add_header,
access_log) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@deploy/nginx.conf`:
- Around line 11-14: The nginx upstream block "upstream openapi_backend"
currently hardcodes "server 127.0.0.1:3000" which will mismatch if the
application sets PORT via the PORT env var in main.go; update the nginx config
to either (a) add a clear comment above upstream openapi_backend explaining that
the backend port must match the application's PORT and that you should use
envsubst during deployment to substitute the port, or (b) implement envsubst
substitution so the server line uses a placeholder like ${PORT} at deploy time;
reference the upstream name openapi_backend and the PORT env var mentioned in
main.go when making the change.

In `@web/src/components/common/markdown/MarkdownRenderer.jsx`:
- Around line 38-51: The lazy-load singleton has a race where multiple callers
of getMermaid can trigger duplicate imports because they all check
mermaidInstance===null; change getMermaid to use a promise-based singleton by
introducing a shared mermaidPromise (in addition to mermaidInstance) that is
assigned to the import() promise the first time getMermaid runs, await that
promise for subsequent callers, then set and initialize mermaidInstance only
once (use mermaidPromise to coalesce concurrent calls and perform initialization
after the import resolves in the same promise chain).

---

Nitpick comments:
In `@deploy/nginx.conf`:
- Around line 52-69: The /assets/ prefix location may overlap with the regex
location and to guarantee nginx chooses it before regex matches, change the
prefix location declaration for /assets/ (the "location /assets/" block) to use
the ^~ modifier so it takes precedence over regex locations; update the
"location /assets/" directive to "location ^~ /assets/" and leave the rest of
that block (proxy_pass, headers, expires, add_header, access_log) unchanged.

In `@docs/performance-optimization.md`:
- Around line 29-37: The docs snippet currently shows a naive singleton using
mermaidInstance and getMermaid; update it to match the promise-based singleton
used in code by replacing mermaidInstance with a single shared promise (e.g.,
mermaidPromise) and have getMermaid return/await that promise so concurrent
callers share the same import/initialization sequence; reference mermaidInstance
and getMermaid in the text and ensure the example demonstrates storing the
import/initialization as a Promise so subsequent calls await the same promise
rather than re-entering initialization.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ad98f2e4-e986-4908-a460-8f7ebaa698ef

📥 Commits

Reviewing files that changed from the base of the PR and between d961cf8 and 3cc1b9d.

📒 Files selected for processing (5)
  • deploy/nginx.conf
  • docs/performance-optimization.md
  • router/relay-router.go
  • web/src/App.jsx
  • web/src/components/common/markdown/MarkdownRenderer.jsx

Comment thread deploy/nginx.conf
Comment on lines +11 to +14
upstream openapi_backend {
server 127.0.0.1:3000;
keepalive 64;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Add comment about configurable backend port.

The upstream hardcodes port 3000, but per main.go the application respects the PORT environment variable. Operators setting PORT=8080 would have nginx pointing to a dead backend.

Consider adding a comment or using an environment variable substitution (via envsubst at deploy time):

📝 Suggested documentation addition
 upstream openapi_backend {
-    server 127.0.0.1:3000;
+    # 确保此端口与应用的 PORT 环境变量一致(默认 3000)
+    # If using a different port, update this value or use envsubst:
+    #   envsubst '$BACKEND_PORT' < nginx.conf.template > nginx.conf
+    server 127.0.0.1:3000;
     keepalive 64;
 }
📝 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
upstream openapi_backend {
server 127.0.0.1:3000;
keepalive 64;
}
upstream openapi_backend {
# 确保此端口与应用的 PORT 环境变量一致(默认 3000)
# If using a different port, update this value or use envsubst:
# envsubst '$BACKEND_PORT' < nginx.conf.template > nginx.conf
server 127.0.0.1:3000;
keepalive 64;
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@deploy/nginx.conf` around lines 11 - 14, The nginx upstream block "upstream
openapi_backend" currently hardcodes "server 127.0.0.1:3000" which will mismatch
if the application sets PORT via the PORT env var in main.go; update the nginx
config to either (a) add a clear comment above upstream openapi_backend
explaining that the backend port must match the application's PORT and that you
should use envsubst during deployment to substitute the port, or (b) implement
envsubst substitution so the server line uses a placeholder like ${PORT} at
deploy time; reference the upstream name openapi_backend and the PORT env var
mentioned in main.go when making the change.

Comment on lines +38 to +51
// Mermaid lazy-loaded to reduce initial bundle by ~5MB
let mermaidInstance = null;
const getMermaid = async () => {
if (!mermaidInstance) {
const m = await import('mermaid');
mermaidInstance = m.default;
mermaidInstance.initialize({
startOnLoad: false,
theme: 'default',
securityLevel: 'loose',
});
}
return mermaidInstance;
};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Race condition in lazy-load singleton when multiple Mermaid components mount concurrently.

If two Mermaid components mount simultaneously (e.g., multiple code blocks in the same message as seen in MessageContent.jsx), both may see mermaidInstance === null before the first import resolves, causing duplicate imports and double initialization.

Use a promise-based singleton to coalesce concurrent requests:

🛠️ Proposed fix
-let mermaidInstance = null;
-const getMermaid = async () => {
-  if (!mermaidInstance) {
-    const m = await import('mermaid');
-    mermaidInstance = m.default;
-    mermaidInstance.initialize({
-      startOnLoad: false,
-      theme: 'default',
-      securityLevel: 'loose',
-    });
-  }
-  return mermaidInstance;
-};
+let mermaidPromise = null;
+const getMermaid = () => {
+  if (!mermaidPromise) {
+    mermaidPromise = import('mermaid').then((m) => {
+      m.default.initialize({
+        startOnLoad: false,
+        theme: 'default',
+        securityLevel: 'loose',
+      });
+      return m.default;
+    });
+  }
+  return mermaidPromise;
+};
📝 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
// Mermaid lazy-loaded to reduce initial bundle by ~5MB
let mermaidInstance = null;
const getMermaid = async () => {
if (!mermaidInstance) {
const m = await import('mermaid');
mermaidInstance = m.default;
mermaidInstance.initialize({
startOnLoad: false,
theme: 'default',
securityLevel: 'loose',
});
}
return mermaidInstance;
};
// Mermaid lazy-loaded to reduce initial bundle by ~5MB
let mermaidPromise = null;
const getMermaid = () => {
if (!mermaidPromise) {
mermaidPromise = import('mermaid').then((m) => {
m.default.initialize({
startOnLoad: false,
theme: 'default',
securityLevel: 'loose',
});
return m.default;
});
}
return mermaidPromise;
};
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/components/common/markdown/MarkdownRenderer.jsx` around lines 38 -
51, The lazy-load singleton has a race where multiple callers of getMermaid can
trigger duplicate imports because they all check mermaidInstance===null; change
getMermaid to use a promise-based singleton by introducing a shared
mermaidPromise (in addition to mermaidInstance) that is assigned to the import()
promise the first time getMermaid runs, await that promise for subsequent
callers, then set and initialize mermaidInstance only once (use mermaidPromise
to coalesce concurrent calls and perform initialization after the import
resolves in the same promise chain).

@Calcium-Ion

Copy link
Copy Markdown
Member

@Calcium-Ion

Copy link
Copy Markdown
Member

logo和系统名称都改了提交上来干嘛?把这里当你家吗

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants