feat: basic user profiles with customisation - #9
Merged
Conversation
|
Warning Review limit reached
More reviews will be available in 37 minutes and 17 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds “list user repositories by name” API with sorting/visibility/limit; introduces nullable user description across API/DB/UI; validates issues endpoint limit; implements profile README and repositories views with Suspense; updates styles/utilities; adds extensive agent skills and React best-practices docs; updates configs and dependencies. ChangesUser repositories API, bio field, and profile UI
Agent skills and React best-practices content
Config and project docs updates
Sequence Diagram(s)sequenceDiagram
participant ComponentA
participant ComponentB
ComponentA->>ComponentB: observable interaction
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (16)
.agents/skills/web-design-guidelines/SKILL.md-25-27 (1)
25-27:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a fence language to satisfy markdownlint on Line 25.
Nice addition overall — this fence should declare a language to avoid MD040 warnings in CI/docs checks.
Suggested patch
-``` +```text https://raw.githubusercontent.com/vercel-labs/web-interface-guidelines/main/command.md</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In @.agents/skills/web-design-guidelines/SKILL.md around lines 25 - 27, The
fenced code block containing the URL currently has no language and triggers
markdownlint MD040; update the triple-backtick fence that wraps
"https://raw.githubusercontent.com/vercel-labs/web-interface-guidelines/main/command.md"
to declare a language (e.g., changetotext) so the block becomes fenced
with a language identifier and satisfies MD040 while preserving the URL content.</details> </blockquote></details> <details> <summary>.agents/skills/vercel-react-best-practices/SKILL.md-136-139 (1)</summary><blockquote> `136-139`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Declare the code-fence language for the sample block on Line 136.** Small tidy-up: adding a language avoids markdownlint MD040 noise. <details> <summary>Suggested patch</summary> ```diff -``` +```text rules/async-parallel.md rules/bundle-barrel-imports.md ``` ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In @.agents/skills/vercel-react-best-practices/SKILL.md around lines 136 - 139,
In .agents/skills/vercel-react-best-practices/SKILL.md update the fenced code
block that currently contains the two lines "rules/async-parallel.md" and
"rules/bundle-barrel-imports.md" so the opening fence declares a language (e.g.,
changetotext) to satisfy markdownlint MD040 and avoid the warning;
locate the block by searching for those rule filenames and replace the opening
fence accordingly.</details> </blockquote></details> <details> <summary>AGENTS.md-57-57 (1)</summary><blockquote> `57-57`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Please fix the article in this guidance sentence.** Line 57 should read “a sqlc” rather than “an sqlc” to keep the guidance crisp and professional. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@AGENTS.md` at line 57, Update the guidance sentence that currently reads "an sqlc" to use the correct article "a sqlc"; locate the sentence containing the phrase "an sqlc" (the sentence mentioning CodeGraph not indexing OpenAPI schemas or SQL(c) files and referencing sqlc/openapi-managed files) and change only the article so it reads "a sqlc" while keeping the rest of the sentence unchanged. ``` </details> </blockquote></details> <details> <summary>.agents/skills/vercel-react-best-practices/rules/client-event-listeners.md-57-65 (1)</summary><blockquote> `57-65`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Align `useSWRSubscription` example with SWR’s subscribe callback signature (Lines 57–65)** SWR’s `subscribe` callback is typed as `(key, { next }) => unsubscribe`; this example uses `() => { ...; return () => ... }`, which diverges from the documented TypeScript shape and can cause copy-paste friction. Consider using `(key, { next }) => { ...; return () => window.removeEventListener('keydown', handler) }` (even if `next` is unused). <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/skills/vercel-react-best-practices/rules/client-event-listeners.md around lines 57 - 65, Update the useSWRSubscription example to match SWR’s subscribe signature by changing the subscribe callback to the (key, { next }) => unsubscribe shape; inside that callback define the handler (KeyboardEvent) => { if (e.metaKey && keyCallbacks.has(e.key)) keyCallbacks.get(e.key)!.forEach(cb => cb()) }, register it with window.addEventListener('keydown', handler), and return () => window.removeEventListener('keydown', handler) so the function signature (key, { next }) and the handler/window.removeEventListener cleanup are correctly aligned with SWR expectations. ``` </details> </blockquote></details> <details> <summary>.agents/skills/vercel-react-best-practices/rules/bundle-preload.md-50-50 (1)</summary><blockquote> `50-50`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Reword the SSR/bundling claim for the `typeof window` guard** - In `.agents/skills/vercel-react-best-practices/rules/bundle-preload.md` (line 50), `typeof window !== 'undefined'` only guards runtime execution; it doesn’t guarantee Next.js/webpack will omit the dynamically imported `./monaco-editor` from SSR bundles (or ensure build-size/build-speed gains). - In this snippet the import is inside `useEffect`, so it won’t run during SSR anyway—so the text should focus on runtime safety, not bundling behaviour. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/skills/vercel-react-best-practices/rules/bundle-preload.md at line 50, The sentence claiming that the `typeof window !== 'undefined'` guard causes bundling/prevents SSR inclusion is inaccurate; update the text in bundle-preload.md to state that `typeof window !== 'undefined'` only prevents runtime execution in non-browser environments (runtime safety), and that in this snippet the import of `./monaco-editor` inside `useEffect` already avoids SSR execution—do not claim webpack/Next.js will omit the module from server bundles or guarantee build-size/build-speed improvements. Reference the `typeof window !== 'undefined'` guard, the dynamic import of `./monaco-editor`, and the `useEffect` placement when you reword the paragraph. ``` </details> </blockquote></details> <details> <summary>.agents/skills/vercel-react-best-practices/rules/bundle-conditional.md-31-31 (1)</summary><blockquote> `31-31`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Fix the SSR/bundling claim around `typeof window` (line 31)** In this snippet, the import is only attempted client-side because it’s inside `useEffect`; the `typeof window !== 'undefined'` check mainly guards runtime, not whether `./animation-frames.js` is included/excluded from SSR bundles. Rephrase the sentence to avoid implying `typeof window` controls bundling/server bundle size. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/skills/vercel-react-best-practices/rules/bundle-conditional.md at line 31, The sentence claiming `typeof window !== 'undefined'` prevents bundling for SSR is inaccurate; update the text to state that the check only guards runtime execution and that placing the import inside React's useEffect ensures the module is only requested at client runtime, while noting that bundlers may still include the file unless explicitly code-split or dynamically imported; reference the `typeof window !== 'undefined'` check, the `useEffect` placement, and `./animation-frames.js` so the wording clarifies runtime guarding vs. bundler behavior. ``` </details> </blockquote></details> <details> <summary>.agents/skills/vercel-react-best-practices/rules/js-cache-storage.md-49-50 (1)</summary><blockquote> `49-50`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Make cookie parsing robust for `=` inside values** Line 49’s `c.split('=')` can mangle valid cookie values that include `=` characters. Split on the first `=` (name/value boundary) instead—for example: ```js document.cookie .split('; ') .map(c => { const eq = c.indexOf('='); return eq === -1 ? [c.trim(), ''] : [c.slice(0, eq).trim(), c.slice(eq + 1)]; }); ``` <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/skills/vercel-react-best-practices/rules/js-cache-storage.md around lines 49 - 50, The cookie parsing using document.cookie.split('; ').map(c => c.split('=')) is unsafe for values containing '='; update the logic in that map to find the first '=' (use c.indexOf('=')), and then produce [name, value] by slicing: name = c.slice(0, eq).trim() and value = eq === -1 ? '' : c.slice(eq + 1); ensure you handle the no-equals case and trim the name/value to preserve correctness. ``` </details> </blockquote></details> <details> <summary>.agents/skills/vercel-react-best-practices/rules/js-set-map-lookups.md-15-22 (1)</summary><blockquote> `15-22`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Example snippet currently uses invalid syntax** `['a', 'b', 'c', ...]` and `new Set(['a', 'b', 'c', ...])` are not valid code. Please replace with concrete values (or a named spread source) so readers can copy/paste safely. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/skills/vercel-react-best-practices/rules/js-set-map-lookups.md around lines 15 - 22, The example uses invalid literal ellipses; update the snippets so allowedIds and its Set use concrete values or a named spread source instead of "..." — e.g., replace "const allowedIds = ['a', 'b', 'c', ...]" and "const allowedIds = new Set(['a', 'b', 'c', ...])" with either an explicit array of values or a reference like "const moreIds = [...]; const allowedIds = ['a','b','c', ...moreIds]" (or the Set equivalent) so items.filter(item => allowedIds.includes(item.id)) and Set construction are valid copy/paste-ready code. ``` </details> </blockquote></details> <details> <summary>.agents/skills/vercel-react-best-practices/rules/js-length-check-first.md-23-23 (1)</summary><blockquote> `23-23`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Tiny wording polish for readability** Please add the missing article: “There is also **the** overhead of joining the arrays…”. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/skills/vercel-react-best-practices/rules/js-length-check-first.md at line 23, Edit the sentence that currently reads "There is also overhead of joining the arrays and comparing the strings." and insert the missing article so it reads "There is also the overhead of joining the arrays and comparing the strings."; locate and update that exact sentence in js-length-check-first.md (the line containing "overhead of joining the arrays and comparing the strings") to improve readability. ``` </details> </blockquote></details> <details> <summary>.agents/skills/vercel-react-best-practices/rules/js-request-idle-callback.md-86-86 (1)</summary><blockquote> `86-86`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Fallback API shape is narrower than `requestIdleCallback`** That fallback helper only accepts a `() => void` callback (`cb` is invoked via `setTimeout`) and doesn’t support the native `(deadline: IdleDeadline)` parameter or the `{ timeout }` option. Either label it explicitly as a simplified fallback, or adjust the helper’s signature/types to match `requestIdleCallback` and clarify what deadline/timeout behaviour it cannot emulate. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/skills/vercel-react-best-practices/rules/js-request-idle-callback.md at line 86, The fallback assigned to scheduleIdleWork narrows the API to () => void and omits the IdleDeadline parameter and options { timeout } that window.requestIdleCallback supports; update the helper so its TypeScript signature matches requestIdleCallback (accept (deadline: IdleDeadline) => void and optional options?: { timeout?: number }) and implement a minimal shim that calls the callback with a mock deadline object (e.g., { didTimeout: false, timeRemaining: () => 0 }) and accepts a timeout option (handle by setTimeout with the provided timeout), or explicitly rename/document it as a simplified fallback; locate scheduleIdleWork and adjust its type/signature and runtime wrapper accordingly. ``` </details> </blockquote></details> <details> <summary>.agents/skills/vercel-react-best-practices/rules/js-min-max-loop.md-82-82 (1)</summary><blockquote> `82-82`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Avoid hard-coding engine-specific spread limits in guidance** In `.agents/skills/vercel-react-best-practices/rules/js-min-max-loop.md` (line 82) the “Chrome 143 / Safari 18 max length” figures read like stable constants, but `Math.max(...arr)` via spread/apply is governed by each engine’s internal max-arguments cap (and MDN warns spread/apply will “fail or return the wrong result” for arrays that are too large, without a fixed cross-browser threshold). Rephrase the guidance as environment-dependent and keep the loop/reduce recommendation, with a link to MDN. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/skills/vercel-react-best-practices/rules/js-min-max-loop.md at line 82, Update the guidance that currently states the “Chrome 143 / Safari 18 max length” numbers and the spread/apply limitation to avoid presenting engine-specific limits as fixed constants; replace that sentence with a concise, environment-dependent warning that Math.max(...arr) (or Function.prototype.apply with a large array) can fail or behave incorrectly when an engine’s max-arguments cap is exceeded, cite MDN for the limitation, keep the recommendation to use an explicit loop or Array.prototype.reduce for reliability, and optionally keep the existing jsfiddle link as an example of variability across engines. ``` </details> </blockquote></details> <details> <summary>.agents/skills/vercel-react-best-practices/rules/rendering-activity.md-10-20 (1)</summary><blockquote> `10-20`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Document that `<Activity>` requires React 19.2+ (and what to do on older React)** - Add a clear “Minimum React version: 19.2” prerequisite for `<Activity>` (it’s a React 19.2 API). - For React <19.2, recommend the fallback already in use: conditional rendering (`{isOpen && <ExpensiveMenu />}` / ternary), noting it will unmount/remount and therefore won’t preserve DOM/state the way `<Activity mode="hidden">` does. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/skills/vercel-react-best-practices/rules/rendering-activity.md around lines 10 - 20, Add a "Minimum React version: 19.2" prerequisite to the <Activity> usage doc and document a fallback for older React: state that <Activity> is a React 19.2+ API, reference the example components (Dropdown, ExpensiveMenu) and the <Activity mode="hidden"> behavior, and explicitly instruct readers using React <19.2 to use conditional rendering ({isOpen && <ExpensiveMenu />} or a ternary) while noting that this fallback will unmount/remount and therefore will not preserve DOM/state like <Activity>. ``` </details> </blockquote></details> <details> <summary>api/api.v1.openapi.yaml-1498-1499 (1)</summary><blockquote> `1498-1499`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Document how clients clear `description`.** The backend accepts a blank string to remove the bio, but the request schema currently just advertises an arbitrary string. That leaves generated docs/clients without any discoverable way to clear an existing description. <details> <summary>Suggested contract fix</summary> ```diff UpdateAuthenticatedUserRequest: type: object properties: displayName: type: string description: type: string + description: Send an empty string to clear the profile description. ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api/api.v1.openapi.yaml` around lines 1498 - 1499, The OpenAPI schema's description property currently only declares type: string but doesn't document that sending an empty string will clear the existing bio; update the schema for the description field (the "description" property) to include a human-readable description stating that an empty string clears the existing bio and, if desired, add an explicit example such as "" to make the behavior discoverable to generated clients. Ensure the change targets the same "description" property in the request schema so docs/clients show the clear-via-empty-string contract. ``` </details> </blockquote></details> <details> <summary>web/src/app/(site)/[ownerName]/page.tsx-39-40 (1)</summary><blockquote> `39-40`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Use a truthy fallback for the profile title.** `??` only falls back on `null`/`undefined`, so a saved empty string here would render a blank `<h1>`. `user.displayName || user.name` is safer for the intended UI fallback. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/app/`(site)/[ownerName]/page.tsx around lines 39 - 40, The profile title currently uses the nullish coalescing operator with user.displayName ?? user.name which will still render an empty string; change the fallback to a truthy check so empty displayName falls back to user.name—update the <h1> rendering expression to use a logical-or style fallback (e.g., user.displayName || user.name) wherever the profile title is rendered in page.tsx to ensure a non-empty string is shown. ``` </details> </blockquote></details> <details> <summary>web/src/components/views/ProfileRepositories.tsx-60-62 (1)</summary><blockquote> `60-62`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Avoid calling this list “featured” until the ordering is real.** Right now the backing sort is only mirroring “updated”, so this subtitle overpromises what the section represents. A neutral label like “Recent repositories” would be less misleading for users. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/components/views/ProfileRepositories.tsx` around lines 60 - 62, In the ProfileRepositories component update the subtitle string that currently reads "Featured work from @{ownerName}." to a neutral label such as "Recent repositories from @{ownerName}." (or "Recent repositories." if you prefer) to avoid implying curated ordering; locate the JSX in ProfileRepositories (the paragraph element using ownerName) and replace the text accordingly so the UI copy matches the current sort behavior. ``` </details> </blockquote></details> <details> <summary>web/src/components/ExpandableContent.tsx-4-4 (1)</summary><blockquote> `4-4`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Drop the unused `motion` import in `web/src/components/ExpandableContent.tsx`.** `motion/react` is imported on line 4 but never used in this file, triggering `@typescript-eslint/no-unused-vars` and adding an unnecessary dependency. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/src/components/ExpandableContent.tsx` at line 4, Remove the unused import of motion from "motion/react" in ExpandableContent.tsx: locate the import statement that brings in the motion symbol and delete it (or replace it with a used import if motion was intended), ensuring no other code references motion; then run TypeScript/lint to confirm `@typescript-eslint/no-unused-vars` is resolved. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (3)</summary><blockquote> <details> <summary>.agents/skills/vercel-react-best-practices/rules/_template.md (1)</summary><blockquote> `5-5`: _⚡ Quick win_ **Use YAML list syntax for `tags` to avoid parser ambiguity.** Nice template overall 👍 — I’d suggest changing `tags` to an explicit YAML array so downstream tooling doesn’t treat it as one string. <details> <summary>Proposed tweak</summary> ```diff -tags: tag1, tag2 +tags: + - tag1 + - tag2 ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/skills/vercel-react-best-practices/rules/_template.md at line 5, The frontmatter line "tags: tag1, tag2" should be converted to an explicit YAML sequence to avoid it being parsed as a single string; update the template's YAML frontmatter by replacing the comma-separated value for the tags field with a proper YAML list (either a block list using "- tag1" / "- tag2" under the tags key or an inline array "[tag1, tag2]") so downstream tooling reliably recognizes each tag. ``` </details> </blockquote></details> <details> <summary>.agents/skills/vercel-react-best-practices/rules/rendering-animate-svg-wrapper.md (1)</summary><blockquote> `10-10`: _⚡ Quick win_ **Tone down absolute browser-performance claim** Please avoid categorical wording here; suggest “can be less performant depending on browser/workload” unless you can cite current engine-specific evidence. Also applies to: 47-47 <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/skills/vercel-react-best-practices/rules/rendering-animate-svg-wrapper.md at line 10, Replace the categorical sentence "Many browsers don't have hardware acceleration for CSS3 animations on SVG elements." with a softer, evidence-scoped suggestion such as "In some browsers or workloads, SVG animations can be less performant; consider wrapping the SVG in a <div> and animating the wrapper instead." Update the identical claim at the other location mentioned (the repeated occurrence) so both instances use the toned-down phrasing and keep the recommendation to animate a wrapper rather than the SVG itself. ``` </details> </blockquote></details> <details> <summary>.agents/skills/vercel-react-best-practices/rules/advanced-event-handler-refs.md (1)</summary><blockquote> `55-55`: _⚡ Quick win_ **Tighten wording around `useEffectEvent` semantics.** Small wording tweak: “stable function reference” is a bit risky here. Prefer wording that it always reads the latest handler and should be used as a non-reactive Effect Event, to stay aligned with the other rule docs. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/skills/vercel-react-best-practices/rules/advanced-event-handler-refs.md at line 55, Update the sentence describing useEffectEvent to avoid "stable function reference" and instead state that useEffectEvent returns a non-reactive Effect Event callback that always reads/uses the latest handler; specifically replace the phrase "`creates a stable function reference that always calls the latest version of the handler`" with wording like "`returns a non-reactive Effect Event callback which always reads the latest handler`" so the doc aligns with other rule docs and semantics for useEffectEvent. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>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
@.agents/skills/vercel-react-best-practices/rules/bundle-defer-third-party.md:
- Around line 31-37: The dynamic import using next/dynamic with { ssr: false }
(the Analytics constant created via dynamic(() =>
import('@vercel/analytics/react').then(m => m.Analytics), { ssr: false })) must
be moved into a Client Component instead of being declared in a Server
RootLayout; create a small wrapper component file or inline component that
begins with 'use client', define the Analytics dynamic import there (keep the
same dynamic(...) call and name), and then import and render that client wrapper
from your RootLayout (or other server component) so the { ssr: false } usage is
only inside a client component.In
@.agents/skills/vercel-react-best-practices/rules/js-cache-function-results.md:
- Line 10: Update the rule text that advises using a "module-level Map to cache
function results" to explicitly constrain this pattern to client/runtime-local
contexts (mention "client components" or "browser/runtime only") and warn that
module-level mutable state is shared in server environments like Next.js App
Router/React Server Components; reference the phrase "module-level Map" and the
example using "document.cookie" so maintainers locate affected wording, and add
alternative guidance for server contexts (recommend React's cache utility or
request-scoped caching patterns) to prevent cross-request leakage.In
@.agents/skills/vercel-react-best-practices/rules/rendering-hydration-no-flicker.md:
- Around line 53-75: The example ThemeWrapper uses an inline
dangerouslySetInnerHTML script (targeting#theme-wrapper) which encourages
unsafe CSP; change the guidance to use a per-request nonce: generate a unique
nonce for each response, emit a Content-Security-Policy header with script-src
'nonce-' (avoiding 'unsafe-inline'), and include that same nonce value as
the nonce attribute on the pre-hydration <script> element in ThemeWrapper so the
script is allowed without permitting unsafe-inline; also note this requires
dynamic rendering (nonce must be unique per request) and update the text to
document that requirement.In
@.agents/skills/vercel-react-best-practices/rules/rendering-script-defer-async.md:
- Around line 55-63: The example incorrectly uses strategy="beforeInteractive"
inside the Page component; update the snippet by either moving the <Script src="/scripts/utils.js" strategy="beforeInteractive" /> usage out of the Page
component into a router/document-level file (e.g., the root layout where
beforeInteractive is allowed) or change the strategy to a page-safe option such
as afterInteractive or lazyOnload; locate the Page component and the Script
element using the symbol Page and the Script tag and adjust the strategy or
relocation accordingly.In
@.agents/skills/vercel-react-best-practices/rules/rendering-usetransition-loading.md:
- Around line 73-74: Update the sentence that currently reads “New transitions
automatically cancel pending ones” to clarify that useTransition affects
rendering priority and may interrupt/restart renders but does not cancel
in-flight async side effects; mention that fetches or other async work are not
automatically aborted and recommend using AbortController (or explicit
cancellation) to cancel async operations if needed, and replace the overclaim
with a succinct line referencing useTransition and AbortController.In @.agents/skills/vercel-react-best-practices/rules/server-cache-lru.md:
- Around line 22-29: The getUser function caches db.user.findUnique results but
incorrectly stores null/undefined and uses a truthy check for cache hits; change
getUser to use cache.has(id) to detect presence, read via cache.get(id) and
return a NOT_FOUND sentinel (e.g., Symbol('NOT_FOUND')) mapped back to null when
returning; when populating the cache set id to NOT_FOUND if user === null else
the user object so lru-cache never stores null/undefined and cache hits are
reliable (update references in getUser, cache.has, cache.get, cache.set, and
introduce the NOT_FOUND symbol).In
@api/api.v1.openapi.yaml:
- Around line 188-193: The visibility query parameter is inheriting
RepositoryVisibility's default of "private", which contradicts the comment "When
omitted" and wrongly filters results; fix the contract by overriding the
parameter's schema so it does not include a default—e.g., reference
RepositoryVisibility but explicitly remove or set default to null/omit default
on the parameter named "visibility" (or declare an inline schema without a
default) so SDKs/docs won't advertise a default filter; update the parameter
definition for "visibility" accordingly.In
@internal/app/api/repositories.go:
- Around line 205-220: The code currently returns a 500 when auth lookup fails,
but this endpoint should gracefully fall back to public-only results; modify the
block using s.auth.GetAuthFromContext and s.auth.GetUserFromAuth so that on any
error you do not return a 500 but instead log or ignore the error and leave
includePrivate = false, then continue execution; keep the existing check that
sets includePrivate = true only when user.ID.Valid && user.ID == owner.ID (so
preserve the user/owner comparison), and only return errors for genuine failures
unrelated to optional auth resolution.In
@web/src/components/ReadmeMarkdown.tsx:
- Around line 7-25: ReadmeMarkdown currently renders Markdown with remarkGfm
only, so relative links/images resolve to the site route; update the
ReadmeMarkdown component signature to accept repository context props (e.g.,
repo, ref, path) and implement custom renderers for links and images passed into
the Markdown component (or a urlTransform) that detect relative URLs (starting
with ./, ../ or no scheme) and rewrite them to the correct repo blob/raw URLs
using the provided repo/ref/path; normalize relative paths against the current
file path before joining, and keep absolute/remote URLs unchanged; reference the
ReadmeMarkdown component, its Markdown usage, and the ExpandableContent wrapper
when making the change.
Minor comments:
In @.agents/skills/vercel-react-best-practices/rules/bundle-conditional.md:
- Line 31: The sentence claiming
typeof window !== 'undefined'prevents
bundling for SSR is inaccurate; update the text to state that the check only
guards runtime execution and that placing the import inside React's useEffect
ensures the module is only requested at client runtime, while noting that
bundlers may still include the file unless explicitly code-split or dynamically
imported; reference thetypeof window !== 'undefined'check, theuseEffect
placement, and./animation-frames.jsso the wording clarifies runtime guarding
vs. bundler behavior.In @.agents/skills/vercel-react-best-practices/rules/bundle-preload.md:
- Line 50: The sentence claiming that the
typeof window !== 'undefined'guard
causes bundling/prevents SSR inclusion is inaccurate; update the text in
bundle-preload.md to state thattypeof window !== 'undefined'only prevents
runtime execution in non-browser environments (runtime safety), and that in this
snippet the import of./monaco-editorinsideuseEffectalready avoids SSR
execution—do not claim webpack/Next.js will omit the module from server bundles
or guarantee build-size/build-speed improvements. Reference thetypeof window !== 'undefined'guard, the dynamic import of./monaco-editor, and the
useEffectplacement when you reword the paragraph.In @.agents/skills/vercel-react-best-practices/rules/client-event-listeners.md:
- Around line 57-65: Update the useSWRSubscription example to match SWR’s
subscribe signature by changing the subscribe callback to the (key, { next }) =>
unsubscribe shape; inside that callback define the handler (KeyboardEvent) => {
if (e.metaKey && keyCallbacks.has(e.key)) keyCallbacks.get(e.key)!.forEach(cb =>
cb()) }, register it with window.addEventListener('keydown', handler), and
return () => window.removeEventListener('keydown', handler) so the function
signature (key, { next }) and the handler/window.removeEventListener cleanup are
correctly aligned with SWR expectations.In @.agents/skills/vercel-react-best-practices/rules/js-cache-storage.md:
- Around line 49-50: The cookie parsing using document.cookie.split('; ').map(c
=> c.split('=')) is unsafe for values containing '='; update the logic in that
map to find the first '=' (use c.indexOf('=')), and then produce [name, value]
by slicing: name = c.slice(0, eq).trim() and value = eq === -1 ? '' : c.slice(eq
- 1); ensure you handle the no-equals case and trim the name/value to preserve
correctness.In @.agents/skills/vercel-react-best-practices/rules/js-length-check-first.md:
- Line 23: Edit the sentence that currently reads "There is also overhead of
joining the arrays and comparing the strings." and insert the missing article so
it reads "There is also the overhead of joining the arrays and comparing the
strings."; locate and update that exact sentence in js-length-check-first.md
(the line containing "overhead of joining the arrays and comparing the strings")
to improve readability.In @.agents/skills/vercel-react-best-practices/rules/js-min-max-loop.md:
- Line 82: Update the guidance that currently states the “Chrome 143 / Safari 18
max length” numbers and the spread/apply limitation to avoid presenting
engine-specific limits as fixed constants; replace that sentence with a concise,
environment-dependent warning that Math.max(...arr) (or Function.prototype.apply
with a large array) can fail or behave incorrectly when an engine’s
max-arguments cap is exceeded, cite MDN for the limitation, keep the
recommendation to use an explicit loop or Array.prototype.reduce for
reliability, and optionally keep the existing jsfiddle link as an example of
variability across engines.In
@.agents/skills/vercel-react-best-practices/rules/js-request-idle-callback.md:
- Line 86: The fallback assigned to scheduleIdleWork narrows the API to () =>
void and omits the IdleDeadline parameter and options { timeout } that
window.requestIdleCallback supports; update the helper so its TypeScript
signature matches requestIdleCallback (accept (deadline: IdleDeadline) => void
and optional options?: { timeout?: number }) and implement a minimal shim that
calls the callback with a mock deadline object (e.g., { didTimeout: false,
timeRemaining: () => 0 }) and accepts a timeout option (handle by setTimeout
with the provided timeout), or explicitly rename/document it as a simplified
fallback; locate scheduleIdleWork and adjust its type/signature and runtime
wrapper accordingly.In @.agents/skills/vercel-react-best-practices/rules/js-set-map-lookups.md:
- Around line 15-22: The example uses invalid literal ellipses; update the
snippets so allowedIds and its Set use concrete values or a named spread source
instead of "..." — e.g., replace "const allowedIds = ['a', 'b', 'c', ...]" and
"const allowedIds = new Set(['a', 'b', 'c', ...])" with either an explicit array
of values or a reference like "const moreIds = [...]; const allowedIds =
['a','b','c', ...moreIds]" (or the Set equivalent) so items.filter(item =>
allowedIds.includes(item.id)) and Set construction are valid copy/paste-ready
code.In @.agents/skills/vercel-react-best-practices/rules/rendering-activity.md:
- Around line 10-20: Add a "Minimum React version: 19.2" prerequisite to the
usage doc and document a fallback for older React: state that
is a React 19.2+ API, reference the example components (Dropdown,
ExpensiveMenu) and the behavior, and explicitly
instruct readers using React <19.2 to use conditional rendering ({isOpen &&
} or a ternary) while noting that this fallback will
unmount/remount and therefore will not preserve DOM/state like .In @.agents/skills/vercel-react-best-practices/SKILL.md:
- Around line 136-139: In .agents/skills/vercel-react-best-practices/SKILL.md
update the fenced code block that currently contains the two lines
"rules/async-parallel.md" and "rules/bundle-barrel-imports.md" so the opening
fence declares a language (e.g., changetotext) to satisfy markdownlint
MD040 and avoid the warning; locate the block by searching for those rule
filenames and replace the opening fence accordingly.In @.agents/skills/web-design-guidelines/SKILL.md:
- Around line 25-27: The fenced code block containing the URL currently has no
language and triggers markdownlint MD040; update the triple-backtick fence that
wraps
"https://raw.githubusercontent.com/vercel-labs/web-interface-guidelines/main/command.md"
to declare a language (e.g., changetotext) so the block becomes fenced
with a language identifier and satisfies MD040 while preserving the URL content.In
@AGENTS.md:
- Line 57: Update the guidance sentence that currently reads "an sqlc" to use
the correct article "a sqlc"; locate the sentence containing the phrase "an
sqlc" (the sentence mentioning CodeGraph not indexing OpenAPI schemas or SQL(c)
files and referencing sqlc/openapi-managed files) and change only the article so
it reads "a sqlc" while keeping the rest of the sentence unchanged.In
@api/api.v1.openapi.yaml:
- Around line 1498-1499: The OpenAPI schema's description property currently
only declares type: string but doesn't document that sending an empty string
will clear the existing bio; update the schema for the description field (the
"description" property) to include a human-readable description stating that an
empty string clears the existing bio and, if desired, add an explicit example
such as "" to make the behavior discoverable to generated clients. Ensure the
change targets the same "description" property in the request schema so
docs/clients show the clear-via-empty-string contract.In
@web/src/app/(site)/[ownerName]/page.tsx:
- Around line 39-40: The profile title currently uses the nullish coalescing
operator with user.displayName ?? user.name which will still render an empty
string; change the fallback to a truthy check so empty displayName falls back to
user.name—update therendering expression to use a logical-or style
fallback (e.g., user.displayName || user.name) wherever the profile title is
rendered in page.tsx to ensure a non-empty string is shown.In
@web/src/components/ExpandableContent.tsx:
- Line 4: Remove the unused import of motion from "motion/react" in
ExpandableContent.tsx: locate the import statement that brings in the motion
symbol and delete it (or replace it with a used import if motion was intended),
ensuring no other code references motion; then run TypeScript/lint to confirm
@typescript-eslint/no-unused-varsis resolved.In
@web/src/components/views/ProfileRepositories.tsx:
- Around line 60-62: In the ProfileRepositories component update the subtitle
string that currently reads "Featured work from @{ownerName}." to a neutral
label such as "Recent repositories from @{ownerName}." (or "Recent
repositories." if you prefer) to avoid implying curated ordering; locate the JSX
in ProfileRepositories (the paragraph element using ownerName) and replace the
text accordingly so the UI copy matches the current sort behavior.
Nitpick comments:
In @.agents/skills/vercel-react-best-practices/rules/_template.md:
- Line 5: The frontmatter line "tags: tag1, tag2" should be converted to an
explicit YAML sequence to avoid it being parsed as a single string; update the
template's YAML frontmatter by replacing the comma-separated value for the tags
field with a proper YAML list (either a block list using "- tag1" / "- tag2"
under the tags key or an inline array "[tag1, tag2]") so downstream tooling
reliably recognizes each tag.In
@.agents/skills/vercel-react-best-practices/rules/advanced-event-handler-refs.md:
- Line 55: Update the sentence describing useEffectEvent to avoid "stable
function reference" and instead state that useEffectEvent returns a non-reactive
Effect Event callback that always reads/uses the latest handler; specifically
replace the phrase "creates a stable function reference that always calls the latest version of the handler" with wording like "returns a non-reactive Effect Event callback which always reads the latest handler" so the doc aligns
with other rule docs and semantics for useEffectEvent.In
@.agents/skills/vercel-react-best-practices/rules/rendering-animate-svg-wrapper.md:
- Line 10: Replace the categorical sentence "Many browsers don't have hardware
acceleration for CSS3 animations on SVG elements." with a softer,
evidence-scoped suggestion such as "In some browsers or workloads, SVG
animations can be less performant; consider wrapping the SVG in aand
animating the wrapper instead." Update the identical claim at the other location
mentioned (the repeated occurrence) so both instances use the toned-down
phrasing and keep the recommendation to animate a wrapper rather than the SVG
itself.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Organization UI **Review profile**: CHILL **Plan**: Pro **Run ID**: `769d3687-7e44-433a-9d37-3e62650dadd7` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 0e6f5f4ab0c0e4ba170b51edd4c4f75a843c1d2c and f3094152c5e5b2d11cb09d2558315a1d9db66957. </details> <details> <summary>⛔ Files ignored due to path filters (7)</summary> * `bun.lock` is excluded by `!**/*.lock` * `go.sum` is excluded by `!**/*.sum` * `internal/app/api/api.gen.go` is excluded by `!**/*.gen.go` * `internal/pkg/db/issues.sql.go` is excluded by `!**/*.sql.go` * `internal/pkg/db/repositories.sql.go` is excluded by `!**/*.sql.go` * `internal/pkg/db/users.sql.go` is excluded by `!**/*.sql.go` * `web/types/api.d.ts` is excluded by `!**/*.d.ts` </details> <details> <summary>📒 Files selected for processing (110)</summary> * `.agents/skills/frontend-design/LICENSE.txt` * `.agents/skills/frontend-design/SKILL.md` * `.agents/skills/vercel-react-best-practices/AGENTS.md` * `.agents/skills/vercel-react-best-practices/README.md` * `.agents/skills/vercel-react-best-practices/SKILL.md` * `.agents/skills/vercel-react-best-practices/metadata.json` * `.agents/skills/vercel-react-best-practices/rules/_sections.md` * `.agents/skills/vercel-react-best-practices/rules/_template.md` * `.agents/skills/vercel-react-best-practices/rules/advanced-effect-event-deps.md` * `.agents/skills/vercel-react-best-practices/rules/advanced-event-handler-refs.md` * `.agents/skills/vercel-react-best-practices/rules/advanced-init-once.md` * `.agents/skills/vercel-react-best-practices/rules/advanced-use-latest.md` * `.agents/skills/vercel-react-best-practices/rules/async-api-routes.md` * `.agents/skills/vercel-react-best-practices/rules/async-cheap-condition-before-await.md` * `.agents/skills/vercel-react-best-practices/rules/async-defer-await.md` * `.agents/skills/vercel-react-best-practices/rules/async-dependencies.md` * `.agents/skills/vercel-react-best-practices/rules/async-parallel.md` * `.agents/skills/vercel-react-best-practices/rules/async-suspense-boundaries.md` * `.agents/skills/vercel-react-best-practices/rules/bundle-analyzable-paths.md` * `.agents/skills/vercel-react-best-practices/rules/bundle-barrel-imports.md` * `.agents/skills/vercel-react-best-practices/rules/bundle-conditional.md` * `.agents/skills/vercel-react-best-practices/rules/bundle-defer-third-party.md` * `.agents/skills/vercel-react-best-practices/rules/bundle-dynamic-imports.md` * `.agents/skills/vercel-react-best-practices/rules/bundle-preload.md` * `.agents/skills/vercel-react-best-practices/rules/client-event-listeners.md` * `.agents/skills/vercel-react-best-practices/rules/client-localstorage-schema.md` * `.agents/skills/vercel-react-best-practices/rules/client-passive-event-listeners.md` * `.agents/skills/vercel-react-best-practices/rules/client-swr-dedup.md` * `.agents/skills/vercel-react-best-practices/rules/js-batch-dom-css.md` * `.agents/skills/vercel-react-best-practices/rules/js-cache-function-results.md` * `.agents/skills/vercel-react-best-practices/rules/js-cache-property-access.md` * `.agents/skills/vercel-react-best-practices/rules/js-cache-storage.md` * `.agents/skills/vercel-react-best-practices/rules/js-combine-iterations.md` * `.agents/skills/vercel-react-best-practices/rules/js-early-exit.md` * `.agents/skills/vercel-react-best-practices/rules/js-flatmap-filter.md` * `.agents/skills/vercel-react-best-practices/rules/js-hoist-regexp.md` * `.agents/skills/vercel-react-best-practices/rules/js-index-maps.md` * `.agents/skills/vercel-react-best-practices/rules/js-length-check-first.md` * `.agents/skills/vercel-react-best-practices/rules/js-min-max-loop.md` * `.agents/skills/vercel-react-best-practices/rules/js-request-idle-callback.md` * `.agents/skills/vercel-react-best-practices/rules/js-set-map-lookups.md` * `.agents/skills/vercel-react-best-practices/rules/js-tosorted-immutable.md` * `.agents/skills/vercel-react-best-practices/rules/rendering-activity.md` * `.agents/skills/vercel-react-best-practices/rules/rendering-animate-svg-wrapper.md` * `.agents/skills/vercel-react-best-practices/rules/rendering-conditional-render.md` * `.agents/skills/vercel-react-best-practices/rules/rendering-content-visibility.md` * `.agents/skills/vercel-react-best-practices/rules/rendering-hoist-jsx.md` * `.agents/skills/vercel-react-best-practices/rules/rendering-hydration-no-flicker.md` * `.agents/skills/vercel-react-best-practices/rules/rendering-hydration-suppress-warning.md` * `.agents/skills/vercel-react-best-practices/rules/rendering-resource-hints.md` * `.agents/skills/vercel-react-best-practices/rules/rendering-script-defer-async.md` * `.agents/skills/vercel-react-best-practices/rules/rendering-svg-precision.md` * `.agents/skills/vercel-react-best-practices/rules/rendering-usetransition-loading.md` * `.agents/skills/vercel-react-best-practices/rules/rerender-defer-reads.md` * `.agents/skills/vercel-react-best-practices/rules/rerender-dependencies.md` * `.agents/skills/vercel-react-best-practices/rules/rerender-derived-state-no-effect.md` * `.agents/skills/vercel-react-best-practices/rules/rerender-derived-state.md` * `.agents/skills/vercel-react-best-practices/rules/rerender-functional-setstate.md` * `.agents/skills/vercel-react-best-practices/rules/rerender-lazy-state-init.md` * `.agents/skills/vercel-react-best-practices/rules/rerender-memo-with-default-value.md` * `.agents/skills/vercel-react-best-practices/rules/rerender-memo.md` * `.agents/skills/vercel-react-best-practices/rules/rerender-move-effect-to-event.md` * `.agents/skills/vercel-react-best-practices/rules/rerender-no-inline-components.md` * `.agents/skills/vercel-react-best-practices/rules/rerender-simple-expression-in-memo.md` * `.agents/skills/vercel-react-best-practices/rules/rerender-split-combined-hooks.md` * `.agents/skills/vercel-react-best-practices/rules/rerender-transitions.md` * `.agents/skills/vercel-react-best-practices/rules/rerender-use-deferred-value.md` * `.agents/skills/vercel-react-best-practices/rules/rerender-use-ref-transient-values.md` * `.agents/skills/vercel-react-best-practices/rules/server-after-nonblocking.md` * `.agents/skills/vercel-react-best-practices/rules/server-auth-actions.md` * `.agents/skills/vercel-react-best-practices/rules/server-cache-lru.md` * `.agents/skills/vercel-react-best-practices/rules/server-cache-react.md` * `.agents/skills/vercel-react-best-practices/rules/server-dedup-props.md` * `.agents/skills/vercel-react-best-practices/rules/server-hoist-static-io.md` * `.agents/skills/vercel-react-best-practices/rules/server-no-shared-module-state.md` * `.agents/skills/vercel-react-best-practices/rules/server-parallel-fetching.md` * `.agents/skills/vercel-react-best-practices/rules/server-parallel-nested-fetching.md` * `.agents/skills/vercel-react-best-practices/rules/server-serialization.md` * `.agents/skills/web-design-guidelines/SKILL.md` * `.codegraph/config.json` * `AGENTS.md` * `api/api.v1.openapi.yaml` * `data/migrations/003_user_description.sql` * `data/queries/issues.sql` * `data/queries/repositories.sql` * `data/queries/users.sql` * `go.mod` * `internal/app/api/issues.go` * `internal/app/api/issues_test.go` * `internal/app/api/repositories.go` * `internal/app/api/repositories_test.go` * `internal/app/api/users.go` * `internal/app/api/users_test.go` * `internal/app/api/util.go` * `internal/app/api/util_test.go` * `internal/pkg/db/models.go` * `package.json` * `skills-lock.json` * `web/package.json` * `web/src/app/(site)/[ownerName]/page.tsx` * `web/src/app/globals.css` * `web/src/components/ExpandableContent.tsx` * `web/src/components/ReadmeMarkdown.tsx` * `web/src/components/views/ProfileReadme.tsx` * `web/src/components/views/ProfileRepositories.tsx` * `web/src/components/views/ProfileSettings.tsx` * `web/src/components/views/RepositoryFileViewer.tsx` * `web/src/components/views/RepositoryHomepage/RepositoryReadme.tsx` * `web/src/lib/server/users.ts` * `web/tsconfig.json` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
How I tested this
Updated unit tests
Checklist