Skip to content

JMCP: Papers - jmcp/extend-home-page-dedup-to-full-visible-f-dyT2Pwp6#36

Merged
landigf merged 1 commit intomainfrom
jmcp/extend-home-page-dedup-to-full-visible-f-dyT2Pwp6
Mar 25, 2026
Merged

JMCP: Papers - jmcp/extend-home-page-dedup-to-full-visible-f-dyT2Pwp6#36
landigf merged 1 commit intomainfrom
jmcp/extend-home-page-dedup-to-full-visible-f-dyT2Pwp6

Conversation

@landigf
Copy link
Copy Markdown
Owner

@landigf landigf commented Mar 25, 2026

Review Summary

Change Summary

Three changes bundled in this PR:

  1. Feed/Home dedup (page.tsx x2): Both the home page and /feed page now fetch 6 trending papers instead of 3, filter out any that already appear in the user's main feed, then slice back to 3. This prevents duplicate paper cards showing in both the feed and trending sidebar.

  2. Groups sidebar on /feed (feed/page.tsx): Adds a "Your circles" section showing up to 2 groups in the feed sidebar, fetched via listGroups.

  3. ORCID profile linking (auth/index.ts): Adds a databaseHook on account creation that writes the ORCID ID to the user's profile row when they link an ORCID account.

Validation Confidence: Medium

  • The dedup logic is straightforward and correct — Set lookup + filter + slice.
  • The as never casts in the auth hook suggest a Drizzle type mismatch (profiles table shape vs. what update/where expects). It works at runtime but masks type errors.
  • No tests were added or modified for any of the three changes.

Risks

Risk Severity Detail
Bundled unrelated changes Low ORCID hook and groups sidebar are unrelated to the dedup objective. Makes revert granularity worse.
as never casts Medium If the profiles schema changes, the cast silences the compiler — a real column rename or type change would only surface at runtime.
No error handling on ORCID hook Medium If the db.update in the after hook throws (e.g. no matching profile row), it's unclear whether betterAuth swallows or propagates the error. An unhandled throw could fail the entire account-creation flow.
Over-fetch when feed is empty Low If the feed has 0 papers, you still fetch 6 trending and discard none — minor wasted work, not a real problem.
Missing loading/empty state for groups Low Groups section is conditionally rendered only when groups.length > 0, which is fine, but the GroupCard import is untested in this page context.

Recommendation

The dedup logic itself is clean. Consider:

  1. Splitting the ORCID hook into its own PR.
  2. Wrapping the ORCID db.update in a try/catch with logging so a profile-write failure doesn't break account creation.
  3. Replacing as never with proper typing or a targeted // @ts-expect-error with an explanation.

@landigf landigf merged commit 09aac4e into main Mar 25, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant