Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 5 minutes and 45 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds two new routing areas: DOI-based citations at /citations/[...doi] and entity-based citations at /citationsByEntity/[id]. Introduces layouts, pages, a Header, DoiRecordList and EventFeed UI components, and four data-fetching functions that call DataCite endpoints. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
src/components/EventFeed.tsx (1)
48-54: Removeshallowfrom these App Router links.The App Router
LinkAPI does not exposeshallow, and Next.js calls that prop Pages Router-only. Leaving it here suggests behavior that App Router navigation won't provide. (nextjs.org)Also applies to: 73-79
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/EventFeed.tsx` around lines 48 - 54, In EventFeed.tsx remove the invalid shallow prop from the App Router Link components (the Link instance that builds the /citations/... href using event.attributes["subj-id"] and the other Link around lines 73-79); locate the Link elements in the component and delete the shallow attribute so only supported props (href, className, title, scroll, etc.) remain to avoid passing Pages-router-only props to the App Router Link.src/components/DoiRecordList.tsx (1)
47-52: Dropshallowfrom App Router links.App Router
<Link>does not supportshallow; the App Router API only listshref,replace,scroll,prefetch,onNavigate, andtransitionTypes, and Next.js explicitly documentsshallowas Pages Router-only. (nextjs.org)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/DoiRecordList.tsx` around lines 47 - 52, The Link in the DoiRecordList component is using the Pages-only prop shallow; remove the shallow prop from the Link JSX (the Link element that has href={`/citations/${record.attributes.doi}`} and scroll={false}) so the App Router Link uses only supported props (href, scroll, etc.). Ensure no other Link usages in this component pass shallow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/citations/`[...doi]/page.tsx:
- Around line 27-31: The chart is receiving Facet[] (objects with id, title,
count) but ResourceTypesChart expects an array of { type, count }; locate where
doisRecords.meta.resourceTypes is passed (e.g., the call site that renders
<ResourceTypesChart ... /> and the Promise.all that fetches fetchDoisRecords)
and map the facets before passing them — transform each facet { id, title, count
} into { type: facet.id or facet.title (choose the chart's key), count:
facet.count }; apply the same mapping where other facet charts are fed (the
second occurrence noted around the later render) so all chart props match the
expected shape.
- Around line 18-24: The Page component incorrectly types and awaits params:
change the component signature to use Next.js route-aware typing
PageProps<'/citations/[...doi]'> so params.doi is typed as string[], stop using
await on params (params is not a promise), and destructure/use params.doi as an
array before calling join (e.g., const { doi } = params; const doi_id =
doi.join("/")). Update the Page function signature and references to params/doi
accordingly (look for PageProps, Page, params, doi.join).
In `@src/app/citationsByEntity/`[id]/page.tsx:
- Around line 16-18: Move the normalization/redirect logic to before any data
fetching so uppercase IDs don't hit DataCite; compute a normalizedId (e.g.,
id.toLowerCase()) at the top of the page component, and if id !== normalizedId
perform a server redirect back into the same route tree using the normalized
path (for example redirect(`/citationsByEntity/${normalizedId}`) or the
equivalent in your routing API) instead of redirecting to `/${normalizedId}`;
then call fetchEntityCitations with the normalizedId (e.g.,
fetchEntityCitations("provider.id:" + normalizedId + " OR client_id:" +
normalizedId)); apply the same change to the other normalization block that
affects the logic around the code handling lines 43-55 so all fetches use the
normalized value.
In `@src/components/DoiRecordList.tsx`:
- Around line 73-76: The UI renders undefined when a record lacks metrics;
update the rendering in DoiRecordList (the JSX that references
record.attributes.citationCount, record.attributes.viewCount, and
record.attributes.downloadCount) to default missing values (e.g., 0 or "-" as
your design) before rendering; either inline with a nullish-coalescing fallback
like record.attributes.citationCount ?? 0 or extract a small helper (e.g.,
formatMetric(record, 'citationCount')) and use that for all three fields so
undefined values never appear in the badge.
In `@src/components/EventFeed.tsx`:
- Around line 55-56: Replace substring checks using
event.attributes["subj-id"].includes(doi) with an equality comparison of
normalized DOIs: normalize both event.attributes["subj-id"] and the page doi by
stripping any "https://doi.org/" prefix (and normalizing case) then compare for
equality; update all occurrences in EventFeed.tsx where includes(doi) is used
(the instances around the current snippet and the other mentions at the blocks
corresponding to lines 63-64, 80-81, 88-89) so the check shows the checkmark
only when the normalized DOIs are exactly equal.
In `@src/data/fetch.ts`:
- Around line 501-503: The DataCite query is interpolated raw into the URL in
fetchDoisRecords which allows `&`, `=` or `#` to break the request; update
fetchDoisRecords to URL-encode the user-provided query (use encodeURIComponent
or equivalent) before inserting it into the DataCite URL and adjust the
console.log to show the encoded URL or the original plus encoded query for
debugging, and apply the same fix to the other helper that builds a DataCite URL
with a query parameter (the other function around the similar DataCite URL
construction).
---
Nitpick comments:
In `@src/components/DoiRecordList.tsx`:
- Around line 47-52: The Link in the DoiRecordList component is using the
Pages-only prop shallow; remove the shallow prop from the Link JSX (the Link
element that has href={`/citations/${record.attributes.doi}`} and
scroll={false}) so the App Router Link uses only supported props (href, scroll,
etc.). Ensure no other Link usages in this component pass shallow.
In `@src/components/EventFeed.tsx`:
- Around line 48-54: In EventFeed.tsx remove the invalid shallow prop from the
App Router Link components (the Link instance that builds the /citations/...
href using event.attributes["subj-id"] and the other Link around lines 73-79);
locate the Link elements in the component and delete the shallow attribute so
only supported props (href, className, title, scroll, etc.) remain to avoid
passing Pages-router-only props to the App Router Link.
🪄 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: ab311e30-c5d7-4447-9883-1b412bf18201
📒 Files selected for processing (8)
src/app/citations/[...doi]/layout.tsxsrc/app/citations/[...doi]/page.tsxsrc/app/citationsByEntity/[id]/Header.tsxsrc/app/citationsByEntity/[id]/layout.tsxsrc/app/citationsByEntity/[id]/page.tsxsrc/components/DoiRecordList.tsxsrc/components/EventFeed.tsxsrc/data/fetch.ts
src/app/citations/[...doi]/page.tsx
Outdated
| interface PageProps { | ||
| params: { doi: string }; | ||
| } | ||
|
|
||
| export default async function Page({ params }: PageProps) { | ||
| const { doi } = await params; | ||
| const doi_id = doi.join("/"); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and read the file
find . -name "page.tsx" -path "*citations*[...doi]*" 2>/dev/null | head -20Repository: datacite/puli
Length of output: 136
🏁 Script executed:
# Try alternative search
fd -t f "page.tsx" | grep -i citationsRepository: datacite/puli
Length of output: 132
🏁 Script executed:
# Broader search for the citations directory
fd -t d "citations" | head -20Repository: datacite/puli
Length of output: 102
🏁 Script executed:
# Read the actual file
cat -n src/app/citations/[...doi]/page.tsxRepository: datacite/puli
Length of output: 4004
🏁 Script executed:
# Check if there are type definitions or imports that show the proper PageProps typing
rg -t ts -t tsx "PageProps" --context 3 | head -50Repository: datacite/puli
Length of output: 84
🏁 Script executed:
# Check tsconfig or next.config to understand the project setup
cat tsconfig.json 2>/dev/null | head -50Repository: datacite/puli
Length of output: 710
🏁 Script executed:
# Look for other route handlers to see how they type params
fd -t f "page.tsx" | xargs rg "interface.*PageProps" -B2 -A2 2>/dev/null | head -100Repository: datacite/puli
Length of output: 341
🏁 Script executed:
# Search for Next.js PageProps type definitions in node_modules or types
find . -name "*.d.ts" -type f | xargs rg "PageProps" 2>/dev/null | head -20Repository: datacite/puli
Length of output: 410
🏁 Script executed:
# Check package.json for Next.js version
cat package.json | rg '"next"|"version"' -A 1 -B 1Repository: datacite/puli
Length of output: 199
🏁 Script executed:
# Check if PageProps is imported from somewhere or if it's just using a standard Next.js pattern
rg -t typescript "import.*PageProps|from.*PageProps" 2>/dev/nullRepository: datacite/puli
Length of output: 39
🏁 Script executed:
# Look at how other dynamic routes are typed
cat -n src/app/citationsByEntity/[id]/page.tsx | head -30Repository: datacite/puli
Length of output: 1486
🏁 Script executed:
# Also check the root page to see the pattern
cat -n src/app/page.tsx | head -30Repository: datacite/puli
Length of output: 1394
🏁 Script executed:
# Check if PageProps is globally defined or imported from somewhere
rg -l "PageProps" --type ts --type tsx 2>/dev/null | xargs grep -l "^import.*PageProps\|^export.*PageProps\|^type PageProps\|^interface PageProps" 2>/dev/nullRepository: datacite/puli
Length of output: 41
🏁 Script executed:
# Look for global type definitions in a types file or d.ts
fd -t f "\.d\.ts$|types\.ts" | xargs cat 2>/dev/nullRepository: datacite/puli
Length of output: 3203
🏁 Script executed:
# Check for Next.js global types or augmentations
rg "declare global|PageProps" -t typescript 2>/dev/null | head -20Repository: datacite/puli
Length of output: 39
🏁 Script executed:
# Check if PageProps is coming from a global augmentation or middleware file
rg "PageProps" -B 5 -A 5 | head -80Repository: datacite/puli
Length of output: 3064
🏁 Script executed:
# Look for any explicit imports of PageProps from next or types
rg "import.*PageProps" 2>/dev/nullRepository: datacite/puli
Length of output: 39
🏁 Script executed:
# Check if there's any global d.ts file that defines PageProps
fd "global\.d\.ts" -o fd "\.d\.ts$" | xargs cat 2>/dev/null | rg "PageProps" -B 2 -A 2Repository: datacite/puli
Length of output: 180
🏁 Script executed:
# Check if there's a next-env.d.ts or similar that might declare PageProps globally
cat -n .next/types/app/page.ts 2>/dev/null || echo "File not found"Repository: datacite/puli
Length of output: 71
🏁 Script executed:
# Search for any .d.ts files that might declare PageProps
find . -name "*.d.ts" -type f -exec cat {} \; 2>/dev/null | rg "PageProps" -B 2 -A 2Repository: datacite/puli
Length of output: 39
🏁 Script executed:
# Check the package.json more thoroughly to understand the setup
cat package.jsonRepository: datacite/puli
Length of output: 1408
🏁 Script executed:
# Check Next.js version and see if PageProps is a known type in Next.js 16
# Let's search the web for Next.js 16.0.10 PageProps typing
echo "Checking the actual Next.js typing pattern in 16.0.10..."Repository: datacite/puli
Length of output: 113
🌐 Web query:
Next.js 16.0.10 PageProps type catch-all route params typing
💡 Result:
In Next.js 16.0.10 (App Router), PageProps is a global TypeScript helper type for typing page components with route-aware params and searchParams. For catch-all routes like app/shop/[...slug]/page.tsx, use PageProps with the route literal path. Example for catch-all route app/shop/[...slug]/page.tsx: tsx export default async function Page(props: PageProps<'/shop/[...slug]'>) { const { slug } = await props.params; // slug: string[] const searchParams = await props.searchParams; return
Citations:
- 1: https://dev.to/bharathkumar28/nextjs-16-type-safety-async-pageprops-typed-routes-3ilc
- 2: https://nextjs.org/docs/app/building-your-application/routing/dynamic-routes
- 3: https://nextjs.org/docs/app/api-reference/file-conventions/page
- 4: https://nextjs.org/docs/basic-features/pages
- 5: https://stackoverflow.com/questions/79119608/pageprops-type-resolution-with-basic-example
Fix the catch-all route typing before using join().
This is a [...doi] route, so doi is an array, not a string. The current interface makes doi.join("/") invalid against the page's own declared props, and Next.js 16 requires using PageProps<'/citations/[...doi]'> for route-aware params with proper typing.
🛠️ Suggested fix
-interface PageProps {
- params: { doi: string };
-}
-
-export default async function Page({ params }: PageProps) {
+export default async function Page({
+ params,
+}: PageProps<"/citations/[...doi]">) {
const { doi } = await params;
const doi_id = Array.isArray(doi) ? doi.join("/") : doi;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/citations/`[...doi]/page.tsx around lines 18 - 24, The Page component
incorrectly types and awaits params: change the component signature to use
Next.js route-aware typing PageProps<'/citations/[...doi]'> so params.doi is
typed as string[], stop using await on params (params is not a promise), and
destructure/use params.doi as an array before calling join (e.g., const { doi }
= params; const doi_id = doi.join("/")). Update the Page function signature and
references to params/doi accordingly (look for PageProps, Page, params,
doi.join).
| const [record, eventsResult, doisRecords] = await Promise.all([ | ||
| fetchDoiRecord(doi_id), | ||
| fetchEvents(doi_id), | ||
| fetchDoisRecords("reference_ids:" + doi_id), | ||
| ]); |
There was a problem hiding this comment.
Map facet data into the chart's expected shape first.
ResourceTypesChart expects { type, count }[], but doisRecords.meta.resourceTypes comes through as Facet[] with { id, title, count }. Passing it straight through leaves the chart without the field it renders against.
📊 Suggested fix
const events = eventsResult?.data || [];
+ const resourceTypeData = (doisRecords.meta.resourceTypes ?? []).map(
+ ({ title, count }: { title: string; count: number }) => ({
+ type: title,
+ count,
+ }),
+ );
return (
@@
- {doisRecords.meta.resourceTypes && <ResourceTypesChart data={doisRecords.meta.resourceTypes} />}
+ {resourceTypeData.length > 0 && (
+ <ResourceTypesChart data={resourceTypeData} />
+ )}Also applies to: 85-85
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/citations/`[...doi]/page.tsx around lines 27 - 31, The chart is
receiving Facet[] (objects with id, title, count) but ResourceTypesChart expects
an array of { type, count }; locate where doisRecords.meta.resourceTypes is
passed (e.g., the call site that renders <ResourceTypesChart ... /> and the
Promise.all that fetches fetchDoisRecords) and map the facets before passing
them — transform each facet { id, title, count } into { type: facet.id or
facet.title (choose the chart's key), count: facet.count }; apply the same
mapping where other facet charts are fed (the second occurrence noted around the
later render) so all chart props match the expected shape.
| const [doisRecords] = await Promise.all([ | ||
| fetchEntityCitations("provider.id:" + id + " OR client_id:" + id), | ||
| ]); |
There was a problem hiding this comment.
Normalize before fetching, and redirect back into the same route tree.
The redirect target drops the /citationsByEntity/ prefix, so an uppercase request like /citationsByEntity/FOO gets sent to /foo instead of /citationsByEntity/foo. I'd also move this normalization block ahead of the fetch on Lines 16-18 so uppercase URLs don't hit DataCite first.
↪️ Suggested fix
- redirect(`/${id.toLowerCase()}?${urlSearchParams.toString()}`);
+ const query = urlSearchParams.toString();
+ redirect(
+ `/citationsByEntity/${id.toLowerCase()}${query ? `?${query}` : ""}`,
+ );Also applies to: 43-55
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/citationsByEntity/`[id]/page.tsx around lines 16 - 18, Move the
normalization/redirect logic to before any data fetching so uppercase IDs don't
hit DataCite; compute a normalizedId (e.g., id.toLowerCase()) at the top of the
page component, and if id !== normalizedId perform a server redirect back into
the same route tree using the normalized path (for example
redirect(`/citationsByEntity/${normalizedId}`) or the equivalent in your routing
API) instead of redirecting to `/${normalizedId}`; then call
fetchEntityCitations with the normalizedId (e.g.,
fetchEntityCitations("provider.id:" + normalizedId + " OR client_id:" +
normalizedId)); apply the same change to the other normalization block that
affects the logic around the code handling lines 43-55 so all fetches use the
normalized value.
| <span className="align-right font-semibold mt-2 bg-gray-100 text-[#003366] rounded-sm px-3 py-1 text-xs inline-block"> | ||
| Citations: <span className="font-bold mr-2">{record.attributes.citationCount} </span> | ||
| Views: <span className="font-bold mr-2">{record.attributes.viewCount} </span> | ||
| Downloads: <span className="font-bold mr-2">{record.attributes.downloadCount} </span> |
There was a problem hiding this comment.
Default missing metrics before rendering the badge.
These fields are optional in the local type, so records without metrics will render undefined in the UI.
🪄 Suggested fix
- Citations: <span className="font-bold mr-2">{record.attributes.citationCount} </span>
- Views: <span className="font-bold mr-2">{record.attributes.viewCount} </span>
- Downloads: <span className="font-bold mr-2">{record.attributes.downloadCount} </span>
+ Citations: <span className="font-bold mr-2">{record.attributes.citationCount ?? 0} </span>
+ Views: <span className="font-bold mr-2">{record.attributes.viewCount ?? 0} </span>
+ Downloads: <span className="font-bold mr-2">{record.attributes.downloadCount ?? 0} </span>📝 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.
| <span className="align-right font-semibold mt-2 bg-gray-100 text-[#003366] rounded-sm px-3 py-1 text-xs inline-block"> | |
| Citations: <span className="font-bold mr-2">{record.attributes.citationCount} </span> | |
| Views: <span className="font-bold mr-2">{record.attributes.viewCount} </span> | |
| Downloads: <span className="font-bold mr-2">{record.attributes.downloadCount} </span> | |
| <span className="align-right font-semibold mt-2 bg-gray-100 text-[`#003366`] rounded-sm px-3 py-1 text-xs inline-block"> | |
| Citations: <span className="font-bold mr-2">{record.attributes.citationCount ?? 0} </span> | |
| Views: <span className="font-bold mr-2">{record.attributes.viewCount ?? 0} </span> | |
| Downloads: <span className="font-bold mr-2">{record.attributes.downloadCount ?? 0} </span> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/DoiRecordList.tsx` around lines 73 - 76, The UI renders
undefined when a record lacks metrics; update the rendering in DoiRecordList
(the JSX that references record.attributes.citationCount,
record.attributes.viewCount, and record.attributes.downloadCount) to default
missing values (e.g., 0 or "-" as your design) before rendering; either inline
with a nullish-coalescing fallback like record.attributes.citationCount ?? 0 or
extract a small helper (e.g., formatMetric(record, 'citationCount')) and use
that for all three fields so undefined values never appear in the badge.
| {event.attributes["subj-id"].includes(doi) ? <span className="mr-1">✔</span> : ""} | ||
| {event.attributes["subj-id"].replace("https://doi.org/", "")} |
There was a problem hiding this comment.
Use DOI equality for the checkmark, not substring matching.
includes(doi) will also mark nearby DOIs like 10.123/abcd as the current record when the page DOI is 10.123/abc. Compare normalized DOI values for equality instead.
Also applies to: 63-64, 80-81, 88-89
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/EventFeed.tsx` around lines 55 - 56, Replace substring checks
using event.attributes["subj-id"].includes(doi) with an equality comparison of
normalized DOIs: normalize both event.attributes["subj-id"] and the page doi by
stripping any "https://doi.org/" prefix (and normalizing case) then compare for
equality; update all occurrences in EventFeed.tsx where includes(doi) is used
(the instances around the current snippet and the other mentions at the blocks
corresponding to lines 63-64, 80-81, 88-89) so the check shows the checkmark
only when the normalized DOIs are exactly equal.
| export async function fetchDoisRecords(query: string) { | ||
| const url = `https://api.datacite.org/dois?query=${query}&page[size]=25&include_other_registration_agencies=true&disable-facets=false&facets=resourceTypes`; | ||
| console.log("Fetching DOIs with query:", url); |
There was a problem hiding this comment.
Encode the DataCite query value before sending it upstream.
Both helpers splice a user-controlled query straight into the URL. Any &, = or # inside the DOI/search text will mutate the request instead of staying inside the query term.
🔧 Suggested fix
export async function fetchDoisRecords(query: string) {
- const url = `https://api.datacite.org/dois?query=${query}&page[size]=25&include_other_registration_agencies=true&disable-facets=false&facets=resourceTypes`;
+ const url = `https://api.datacite.org/dois?${new URLSearchParams({
+ query,
+ "page[size]": "25",
+ include_other_registration_agencies: "true",
+ "disable-facets": "false",
+ facets: "resourceTypes",
+ }).toString()}`;
console.log("Fetching DOIs with query:", url);
const response = await fetch(url, {
method: "GET",
headers: { accept: "application/vnd.api+json" },
});
@@
export async function fetchEntityCitations(query: string) {
- const url = `https://api.datacite.org/dois?query=${query}&page[size]=25&disable-facets=false&facets=citations&sort=-citation-count`;
+ const url = `https://api.datacite.org/dois?${new URLSearchParams({
+ query,
+ "page[size]": "25",
+ "disable-facets": "false",
+ facets: "citations",
+ sort: "-citation-count",
+ }).toString()}`;
console.log("Fetching DOIs with query:", url);
const response = await fetch(url, {
method: "GET",
headers: { accept: "application/vnd.api+json" },
});Also applies to: 516-518
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/data/fetch.ts` around lines 501 - 503, The DataCite query is interpolated
raw into the URL in fetchDoisRecords which allows `&`, `=` or `#` to break the
request; update fetchDoisRecords to URL-encode the user-provided query (use
encodeURIComponent or equivalent) before inserting it into the DataCite URL and
adjust the console.log to show the encoded URL or the original plus encoded
query for debugging, and apply the same fix to the other helper that builds a
DataCite URL with a query parameter (the other function around the similar
DataCite URL construction).
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
src/app/citationsByEntity/[id]/page.tsx (1)
57-58:⚠️ Potential issue | 🟡 MinorUse
notFound()instead of returningnullfor missing entities.Returning
nullrenders a blank page. The layout already usesnotFound()for this case — keep the 404 handling consistent.Proposed fix
+import { notFound, redirect } from "next/navigation"; -import { redirect } from "next/navigation"; const entity = await fetchEntity(id); - if (!entity) return null; + if (!entity) notFound();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/citationsByEntity/`[id]/page.tsx around lines 57 - 58, The code currently returns null when fetchEntity(id) yields no result (const entity = await fetchEntity(id); if (!entity) return null;); replace this with calling Next's notFound() to trigger a 404 (if (!entity) notFound();) and ensure notFound is imported from 'next/navigation' at the top of the file; keep the rest of the component unchanged so 404 handling is consistent with the layout.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/citationsByEntity/`[id]/page.tsx:
- Around line 60-62: The JSX in the page component contains nested <main>
elements which breaks HTML semantics; locate the returned markup in
src/app/citationsByEntity/[id]/page.tsx (the page component render/return) and
replace the inner <main className="w-full mx-auto flex flex-row items-start
gap-8"> with a semantically appropriate container such as a <div> or <section>,
preserving the existing className and structure so only the tag changes.
- Around line 28-30: The code computing yearNums and minYear can produce NaN
when parseInt returns NaN for non-numeric mapped entries; update the logic that
builds yearNums (used in mapped.map and variable yearNums) to parse each
item.year safely (e.g., Number or parseInt) and filter out non-finite results
before calling Math.min, then compute minYear from the filtered array and fall
back to a safe default (e.g., maxYear or new Date().getFullYear()) if the
filtered array is empty; ensure references to yearNums, mapped, minYear, and
maxYear are adjusted accordingly so the subsequent year loop never receives NaN
values.
In `@src/components/DoiRecordList.tsx`:
- Around line 57-59: The JSX directly reads record.attributes.titles[0].title
which will throw if titles is empty; in DoiRecordList update the render to guard
against empty or missing titles (e.g., use optional chaining and a safe fallback
like record.attributes.titles?.[0]?.title || 'Untitled' or fall back to another
attribute) so the <div className="font-bold text-[`#243B54`]"> never tries to
access index 0 of an empty array; keep the change local to the component where
record.attributes.titles is referenced.
- Around line 63-65: In DoiRecordList.tsx fix the metadata line rendering by
building the pieces from record.attributes (publicationYear, publisher) into an
array, filtering out falsy values, and joining them with ' · ' so orphan
separators don't appear; then separately render the agency only when present
(e.g., compute agencyLabel = agency ? 'via ' +
capitalize(record.attributes.agency) : undefined) and append it with a preceding
separator if needed—update the JSX that currently uses
{record.attributes.publicationYear} · {record.attributes.publisher} · via
{record.attributes.agency && ...} to use these conditional pieces so missing
fields produce no extraneous separators or trailing "via ".
- Around line 50-55: The Link usage in DoiRecordList.tsx passes a removed prop
`shallow` — remove the `shallow` attribute from the <Link> element that
navigates to `/citations/${record.attributes.doi}`; simply delete the `shallow`
prop (keeping existing href, className, and scroll as-is) and, if you need App
Router-specific client-side behavior later, use next/navigation hooks (e.g.,
useRouter/push or router.replace) instead of `shallow`.
---
Duplicate comments:
In `@src/app/citationsByEntity/`[id]/page.tsx:
- Around line 57-58: The code currently returns null when fetchEntity(id) yields
no result (const entity = await fetchEntity(id); if (!entity) return null;);
replace this with calling Next's notFound() to trigger a 404 (if (!entity)
notFound();) and ensure notFound is imported from 'next/navigation' at the top
of the file; keep the rest of the component unchanged so 404 handling is
consistent with the layout.
🪄 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: f0f30762-4026-4a13-86bb-a078ea3624c6
📒 Files selected for processing (4)
src/app/citations/[...doi]/layout.tsxsrc/app/citations/[...doi]/page.tsxsrc/app/citationsByEntity/[id]/page.tsxsrc/components/DoiRecordList.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/app/citations/[...doi]/layout.tsx
- src/app/citations/[...doi]/page.tsx
| const yearNums = mapped.map((item: { year: string; count: number }) => parseInt(item.year, 10)); | ||
| const minYear = Math.min(...yearNums); | ||
| const maxYear = new Date().getFullYear(); |
There was a problem hiding this comment.
Guard against invalid year values that produce NaN.
If the API returns a non-numeric year string, parseInt returns NaN, causing Math.min(...yearNums) to return NaN and the subsequent loop to silently fail.
Proposed fix
- const yearNums = mapped.map((item: { year: string; count: number }) => parseInt(item.year, 10));
- const minYear = Math.min(...yearNums);
- const maxYear = new Date().getFullYear();
+ const yearNums = mapped
+ .map((item: { year: string; count: number }) => parseInt(item.year, 10))
+ .filter((y) => !Number.isNaN(y));
+ if (yearNums.length === 0) {
+ // No valid years, leave chartData empty
+ } else {
+ const minYear = Math.min(...yearNums);
+ const maxYear = new Date().getFullYear();
+ const yearToCount: Record<string, number> = {};
+ mapped.forEach((item: { year: string; count: number }) => {
+ yearToCount[item.year] = item.count;
+ });
+ for (let y = minYear; y <= maxYear; y++) {
+ chartData.push({
+ year: y.toString(),
+ count: yearToCount[y.toString()] ?? 0,
+ });
+ }
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/citationsByEntity/`[id]/page.tsx around lines 28 - 30, The code
computing yearNums and minYear can produce NaN when parseInt returns NaN for
non-numeric mapped entries; update the logic that builds yearNums (used in
mapped.map and variable yearNums) to parse each item.year safely (e.g., Number
or parseInt) and filter out non-finite results before calling Math.min, then
compute minYear from the filtered array and fall back to a safe default (e.g.,
maxYear or new Date().getFullYear()) if the filtered array is empty; ensure
references to yearNums, mapped, minYear, and maxYear are adjusted accordingly so
the subsequent year loop never receives NaN values.
| return ( | ||
| <main className=""> | ||
| <main className="w-full mx-auto flex flex-row items-start gap-8"> |
There was a problem hiding this comment.
Avoid nested <main> elements — use a single <main> landmark per page.
Nested <main> elements violate HTML semantics and can confuse assistive technologies. Replace the inner <main> with a <div> or <section>.
Proposed fix
return (
- <main className="">
- <main className="w-full mx-auto flex flex-row items-start gap-8">
+ <main className="w-full mx-auto flex flex-row items-start gap-8">
<div className="w-1/2">
...
</div>
<div className="w-1/2">
...
</div>
- </main>
</main>
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return ( | |
| <main className=""> | |
| <main className="w-full mx-auto flex flex-row items-start gap-8"> | |
| return ( | |
| <main className="w-full mx-auto flex flex-row items-start gap-8"> | |
| <div className="w-1/2"> | |
| ... | |
| </div> | |
| <div className="w-1/2"> | |
| ... | |
| </div> | |
| </main> | |
| ); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/citationsByEntity/`[id]/page.tsx around lines 60 - 62, The JSX in the
page component contains nested <main> elements which breaks HTML semantics;
locate the returned markup in src/app/citationsByEntity/[id]/page.tsx (the page
component render/return) and replace the inner <main className="w-full mx-auto
flex flex-row items-start gap-8"> with a semantically appropriate container such
as a <div> or <section>, preserving the existing className and structure so only
the tag changes.
| <Link | ||
| href={`/citations/${record.attributes.doi}`} | ||
| className="cursor-pointer" | ||
| scroll={false} | ||
| shallow | ||
| > |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does Next.js 16 Link component support the shallow prop?
💡 Result:
No, the Next.js 16 Link component does not support the shallow prop in the App Router (the default and recommended router). The shallow prop was specific to the Pages Router, where it enabled changing the URL without re-running data fetching methods like getStaticProps or getServerSideProps. In Next.js 16, official App Router Link documentation lists props like href, replace, scroll, prefetch, and onNavigate, but not shallow. GitHub discussions confirm shallow routing is unavailable in the App Router since Next.js 13, with no restoration in version 16 release notes. The Pages Router documentation still mentions shallow, but Next.js 16 emphasizes the App Router, and Pages Router is legacy.
Citations:
- 1: Shallow prop with link component redirect to as prop value with app directory. vercel/next.js#50274
- 2: https://nextjs.org/docs/pages/api-reference/components/link
- 3: https://beta.nextjs.org/docs/api-reference/components/link
- 4: Shallow routing on Next.js 13's next/navigation vercel/next.js#48110
- 5: https://nextjs.org/blog/next-16
- 6: https://nextjs.org/docs/app/guides/upgrading/version-16
- 7: https://nextjs.org/docs/app/getting-started/linking-and-navigating
- 8: https://nextjs.org/docs/13/app/building-your-application/routing/linking-and-navigating
- 9: https://nextjs.org/docs/app/building-your-application/routing/linking-and-navigating
🏁 Script executed:
# Check Next.js version in package.json
cat package.json | grep -A 2 '"next"'Repository: datacite/puli
Length of output: 128
🏁 Script executed:
# Check directory structure to determine routing system
git ls-files | head -20
find . -type d -name "app" -o -type d -name "pages" | head -10Repository: datacite/puli
Length of output: 515
🏁 Script executed:
# Check the actual file to see routing context
cat -n src/components/DoiRecordList.tsx | head -60Repository: datacite/puli
Length of output: 1987
Remove the shallow prop — it was removed from next/link in Next.js 13+.
The shallow prop no longer exists on the Link component in the App Router. For client-side navigation without refetching server data, the App Router handles this differently through its caching mechanisms.
Proposed fix
<Link
href={`/citations/${record.attributes.doi}`}
className="cursor-pointer"
scroll={false}
- shallow
>📝 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.
| <Link | |
| href={`/citations/${record.attributes.doi}`} | |
| className="cursor-pointer" | |
| scroll={false} | |
| shallow | |
| > | |
| <Link | |
| href={`/citations/${record.attributes.doi}`} | |
| className="cursor-pointer" | |
| scroll={false} | |
| > |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/DoiRecordList.tsx` around lines 50 - 55, The Link usage in
DoiRecordList.tsx passes a removed prop `shallow` — remove the `shallow`
attribute from the <Link> element that navigates to
`/citations/${record.attributes.doi}`; simply delete the `shallow` prop (keeping
existing href, className, and scroll as-is) and, if you need App Router-specific
client-side behavior later, use next/navigation hooks (e.g., useRouter/push or
router.replace) instead of `shallow`.
| <div className="font-bold text-[#243B54]"> | ||
| {record.attributes.titles[0].title} | ||
| </div> |
There was a problem hiding this comment.
Guard against empty titles array to prevent runtime crash.
titles is a required array in the type, but the API could return an empty array. Accessing titles[0].title without a check will throw a TypeError.
Proposed fix
<div className="font-bold text-[`#243B54`]">
- {record.attributes.titles[0].title}
+ {record.attributes.titles[0]?.title ?? "Untitled"}
</div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className="font-bold text-[#243B54]"> | |
| {record.attributes.titles[0].title} | |
| </div> | |
| <div className="font-bold text-[`#243B54`]"> | |
| {record.attributes.titles[0]?.title ?? "Untitled"} | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/DoiRecordList.tsx` around lines 57 - 59, The JSX directly
reads record.attributes.titles[0].title which will throw if titles is empty; in
DoiRecordList update the render to guard against empty or missing titles (e.g.,
use optional chaining and a safe fallback like
record.attributes.titles?.[0]?.title || 'Untitled' or fall back to another
attribute) so the <div className="font-bold text-[`#243B54`]"> never tries to
access index 0 of an empty array; keep the change local to the component where
record.attributes.titles is referenced.
| <div className="text-gray-500 text-sm mt-2"> | ||
| {record.attributes.publicationYear} · {record.attributes.publisher} · via {record.attributes.agency && record.attributes.agency.charAt(0).toUpperCase() + record.attributes.agency.slice(1)} | ||
| </div> |
There was a problem hiding this comment.
Handle missing metadata fields to avoid rendering artifacts like "· via ".
When publicationYear, publisher, or agency are undefined, the current template renders orphaned separators or "via " with nothing following. Use conditional rendering or template only the fields that exist.
Proposed fix
- <div className="text-gray-500 text-sm mt-2">
- {record.attributes.publicationYear} · {record.attributes.publisher} · via {record.attributes.agency && record.attributes.agency.charAt(0).toUpperCase() + record.attributes.agency.slice(1)}
- </div>
+ <div className="text-gray-500 text-sm mt-2">
+ {[
+ record.attributes.publicationYear,
+ record.attributes.publisher,
+ record.attributes.agency &&
+ `via ${record.attributes.agency.charAt(0).toUpperCase() + record.attributes.agency.slice(1)}`,
+ ]
+ .filter(Boolean)
+ .join(" · ")}
+ </div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className="text-gray-500 text-sm mt-2"> | |
| {record.attributes.publicationYear} · {record.attributes.publisher} · via {record.attributes.agency && record.attributes.agency.charAt(0).toUpperCase() + record.attributes.agency.slice(1)} | |
| </div> | |
| <div className="text-gray-500 text-sm mt-2"> | |
| {[ | |
| record.attributes.publicationYear, | |
| record.attributes.publisher, | |
| record.attributes.agency && | |
| `via ${record.attributes.agency.charAt(0).toUpperCase() + record.attributes.agency.slice(1)}`, | |
| ] | |
| .filter(Boolean) | |
| .join(" · ")} | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/DoiRecordList.tsx` around lines 63 - 65, In DoiRecordList.tsx
fix the metadata line rendering by building the pieces from record.attributes
(publicationYear, publisher) into an array, filtering out falsy values, and
joining them with ' · ' so orphan separators don't appear; then separately
render the agency only when present (e.g., compute agencyLabel = agency ? 'via '
+ capitalize(record.attributes.agency) : undefined) and append it with a
preceding separator if needed—update the JSX that currently uses
{record.attributes.publicationYear} · {record.attributes.publisher} · via
{record.attributes.agency && ...} to use these conditional pieces so missing
fields produce no extraneous separators or trailing "via ".
1 participant
Purpose
closes: Add github issue that originated this PR
Approach
Open Questions and Pre-Merge TODOs
Learning
Types of changes
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Reviewer, please remember our guidelines:
Summary by CodeRabbit