style: reading width, shell layout, and prose typography#1337
Merged
Conversation
- Add CLAUDE.md orienting contributors; flags the repo as public and sets a high bar for code that must pass open-source review. - Remove docs/plans/* internal design/planning notes (not appropriate for a public repo) and drop their references from code comments. - gitignore .claude/ and other local AI assistant config.
Adds opt-in feed personalization built on the topic vocabulary: - Schema (migration 0039, additive): user_topic_pref (follow/mute) and user_topic_affinity (implicit interest, time-decayed). - feedRanking: pure, unit-tested scoring — a transparent weighted blend of recency, quality, and topic affinity, with muted topics filtered out. - topicAffinity: derive per-user affinity from votes/bookmarks/comments through post_topic edges with decay; recomputed for active users by the nightly cron. - profile.getTopicPrefs / setTopicPref: manage follows and mutes. - content.getForYouFeed: re-rank a recent candidate window for the user; cold start (no signal) falls back to recency, so the existing feed is untouched. Also trims verbose comments across the content-pipeline modules.
- Widen app shell to 1300px (right rail 300→280px); fold breakpoint moved to 1300px so the center column holds its ~672px reading measure at all sizes - Narrow article/discussion/link readers to max-w-prose (672px), unifying all content surfaces and matching the readability sweet spot (~70 chars/line) - Remove double-up side padding from content wrappers; shell grid gap and mobile shell gutter (bumped to 1rem) now own all horizontal breathing room - Re-key prose overrides onto design tokens (bg-inset, border-hairline, text-accent-soft, color-muted/fg) so styles flip correctly in dark mode; inline code selector narrowed to :not(pre)>code to avoid touching fenced blocks
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Warning Review limit reached
More reviews will be available in 11 minutes and 24 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ 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. WalkthroughThis PR introduces a complete feed personalization system: new DB tables for explicit topic preferences and implicit affinity scores, a time-decayed affinity computation engine, a deterministic scoring/ranking library, two new tRPC endpoints, and nightly cron integration. It also refactors the OG image template stack for Satori safety, updates global CSS breakpoints and prose typography, removes horizontal padding from article readers, and adds housekeeping files ( ChangesFeed Personalization System
OG Image Template Refactor
UI Layout, Styling & Housekeeping
Sequence Diagram(s)sequenceDiagram
participant Client
participant getForYouFeed as contentRouter.getForYouFeed
participant loadUserProfile
participant DB as Database
participant rankCandidates
Client->>getForYouFeed: { limit, offset }
getForYouFeed->>loadUserProfile: userId
loadUserProfile->>DB: query user_topic_pref + user_topic_affinity
DB-->>loadUserProfile: pref rows + affinity scores
loadUserProfile-->>getForYouFeed: UserProfile { follows, mutes, affinity }
getForYouFeed->>DB: fetch published posts + post_metadata.qualityScore + votes/bookmarks
DB-->>getForYouFeed: raw candidates
getForYouFeed->>rankCandidates: candidates + UserProfile (when hasProfileSignal)
rankCandidates-->>getForYouFeed: scored + sorted items
getForYouFeed-->>Client: { items, nextOffset, personalized }
sequenceDiagram
participant Cron as daily-review cron
participant reviewAffinity
participant findRecentlyActiveUsers
participant recomputeUserAffinity
participant DB as user_topic_affinity (DB)
Cron->>reviewAffinity: invoke
reviewAffinity->>findRecentlyActiveUsers: sinceIso, AFFINITY_USER_CAP
findRecentlyActiveUsers-->>reviewAffinity: activeUserIds[]
loop per userId
reviewAffinity->>recomputeUserAffinity: db, userId, nowMs
recomputeUserAffinity->>DB: delete existing rows, insert new affinity scores
DB-->>recomputeUserAffinity: inserted count
recomputeUserAffinity-->>reviewAffinity: topicsInserted
end
reviewAffinity-->>Cron: { usersUpdated }
Cron-->>Cron: summary.affinityUsersUpdated = usersUpdated
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
styles/globals.css (1)
389-407:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd empty lines before declarations to fix Stylelint warnings.
The inline code selector narrowing to
:not(pre) > codecorrectly excludes fenced code blocks. However, Stylelint requires empty lines before certain declarations.🎨 Proposed formatting fix
.prose :not(pre) > code { `@apply` rounded-sm border border-hairline bg-inset px-1 py-0.5 font-mono text-accent-soft; + font-size: 0.86em; }Also add an empty line before the color declaration at line 345 in the
.prose :where(h1, h2, h3, h4)block:.prose :where(h1, h2, h3, h4) { `@apply` font-display tracking-tight; + color: rgb(var(--color-fg)); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@styles/globals.css` around lines 389 - 407, Add empty lines before property declarations in the CSS rules to satisfy Stylelint formatting requirements. In the styles shown in the diff (the .prose :not(pre) > code block, .prose code:after, .prose code:before, and .prose pre code rules), insert blank lines before declarations to improve readability and meet linting standards. Additionally, add an empty line before the color declaration in the .prose :where(h1, h2, h3, h4) block at line 345.Source: Linters/SAST tools
🧹 Nitpick comments (2)
app/(app)/s/[sourceSlug]/_sourceProfileClient.tsx (1)
68-68: ⚡ Quick winInconsistent padding between error and main states.
The error state wrapper uses
px-0 ... sm:px-4while the main profile wrapper at line 118 usespx-4. For consistency, the error state should match the main state's padding approach.🔄 Suggested consistency fix
- <div className="mx-auto max-w-2xl px-0 py-4 text-fg sm:px-4 sm:py-8"> + <div className="mx-auto max-w-2xl px-4 py-4 text-fg sm:py-8">🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/`(app)/s/[sourceSlug]/_sourceProfileClient.tsx at line 68, The error state wrapper div uses responsive padding with px-0 and sm:px-4 while the main profile wrapper uses consistent px-4 padding. To maintain consistency across both states, update the className on the error state wrapper to use the same padding approach as the main profile wrapper. Replace the responsive padding classes (px-0 and sm:px-4) with the matching px-4 padding used in the main state wrapper.server/lib/topicAffinity.test.ts (1)
4-21: 🏗️ Heavy liftExpand tests to cover the DB-backed affinity paths.
This suite validates decay math, but not
recomputeUserAffinity/findRecentlyActiveUsersbehavior (e.g., delete-on-empty, positive-score filtering, and limit/capping behavior). Add focused unit/integration tests for those paths.Based on learnings: “Code must clear public code review: ensure clear naming, no dead code, no debug logging, tests for new logic, and changes scoped to one concern.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@server/lib/topicAffinity.test.ts` around lines 4 - 21, The current test file only validates the decay math for the decayedWeight function but does not cover the DB-backed affinity logic. Add focused unit/integration tests to the topicAffinity.test.ts file that cover the behavior of recomputeUserAffinity and findRecentlyActiveUsers functions, specifically testing: delete-on-empty behavior when affinity scores become empty, positive-score filtering to ensure only positive scores are retained, and limit/capping behavior to verify results respect any configured limits. These tests should be separate test suites or describe blocks from the existing decayedWeight tests to keep concerns scoped and clear.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@server/api/router/content.ts`:
- Around line 1690-1717: The database query uses only publishedAt for ordering,
which causes non-deterministic ordering when multiple posts share the same
publication timestamp. This creates pagination issues where the same items can
appear on different pages or be skipped. Add a secondary sort key (such as the
post id) to the orderBy clause alongside the publishedAt descending order to
ensure stable, deterministic ordering across offset-based pagination requests.
In `@server/api/router/profile.ts`:
- Around line 105-133: The setTopicPref mutation validates that topicId is a
positive integer but does not verify the topic actually exists in the database.
If a non-existent topicId is provided, the insert operation on line 126 will
fail at the foreign key constraint boundary, returning a 500 internal error. Add
a database query after extracting userId to verify that the topic with the given
topicId exists. If the topic does not exist, throw a validation error before
proceeding to the delete or insert operations. This ensures client-facing
validation errors instead of internal database constraint failures.
In `@server/lib/feedRanking.ts`:
- Around line 99-105: The sort comparator in the ranked.sort() function has a
bug in its final tie-breaker logic. When comparing ids with the expression
`a.item.id < b.item.id ? 1 : -1`, it returns -1 for both the case where
`a.item.id > b.item.id` and when `a.item.id === b.item.id`. According to the
comparator contract, when two elements are equal, the comparator must return 0.
Fix this by checking if the ids are equal first and returning 0 in that case,
otherwise apply the ascending sort logic: return 1 if `a.item.id < b.item.id`,
else return -1.
In `@server/lib/topicAffinity.ts`:
- Around line 95-99: The delete-then-insert replacement pattern for user topic
affinity is non-atomic, which means if the insert fails after the delete
succeeds, the user loses all affinity rows. Wrap both the delete operation (in
the interactions.length === 0 block) and the insert operation in a single
database transaction using db.transaction() to ensure atomicity. This ensures
that if either operation fails, all changes are rolled back and the data remains
consistent. Apply this transaction wrapping to all locations where the affinity
replacement occurs to make the entire operation atomic.
- Around line 149-168: The `findRecentlyActiveUsers` function loads all distinct
users from three database queries (post_votes, bookmarks, and comments) then
applies limit in memory via slice(limit). To improve performance, add a
`.limit(limit)` clause directly to each of the three database queries for
voters, markers, and commenters to bound the result set at the database level
rather than loading all rows into memory. Then adjust the final return statement
to account for the fact that results are already bounded, removing the slice
operation since each query now respects the limit constraint.
In `@styles/globals.css`:
- Around line 366-374: Add empty lines before declarations to satisfy Stylelint
formatting requirements. Insert a blank line before the `.prose blockquote` rule
and before the `@apply border-accent` declaration within that rule to maintain
consistent spacing and meet Stylelint's style guidelines for CSS declarations.
---
Outside diff comments:
In `@styles/globals.css`:
- Around line 389-407: Add empty lines before property declarations in the CSS
rules to satisfy Stylelint formatting requirements. In the styles shown in the
diff (the .prose :not(pre) > code block, .prose code:after, .prose code:before,
and .prose pre code rules), insert blank lines before declarations to improve
readability and meet linting standards. Additionally, add an empty line before
the color declaration in the .prose :where(h1, h2, h3, h4) block at line 345.
---
Nitpick comments:
In `@app/`(app)/s/[sourceSlug]/_sourceProfileClient.tsx:
- Line 68: The error state wrapper div uses responsive padding with px-0 and
sm:px-4 while the main profile wrapper uses consistent px-4 padding. To maintain
consistency across both states, update the className on the error state wrapper
to use the same padding approach as the main profile wrapper. Replace the
responsive padding classes (px-0 and sm:px-4) with the matching px-4 padding
used in the main state wrapper.
In `@server/lib/topicAffinity.test.ts`:
- Around line 4-21: The current test file only validates the decay math for the
decayedWeight function but does not cover the DB-backed affinity logic. Add
focused unit/integration tests to the topicAffinity.test.ts file that cover the
behavior of recomputeUserAffinity and findRecentlyActiveUsers functions,
specifically testing: delete-on-empty behavior when affinity scores become
empty, positive-score filtering to ensure only positive scores are retained, and
limit/capping behavior to verify results respect any configured limits. These
tests should be separate test suites or describe blocks from the existing
decayedWeight tests to keep concerns scoped and clear.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 330fa14c-646d-4f1b-9cd7-709e29fa42be
⛔ Files ignored due to path filters (2)
drizzle/meta/0039_snapshot.jsonis excluded by!**/*.jsondrizzle/meta/_journal.jsonis excluded by!**/*.json
📒 Files selected for processing (28)
.gitignoreCLAUDE.mdapp/(app)/[username]/[slug]/_userLinkDetail.tsxapp/(app)/s/[sourceSlug]/[slug]/_feedArticleContent.tsxapp/(app)/s/[sourceSlug]/_sourceProfileClient.tsxapp/api/cron/daily-review/route.tsapp/og/route.tsxcomponents/Admin/AdminShell.tsxcomponents/ContentDetail/PostReader.tsxdocs/plans/2026-06-09-moderation-overhaul-design.mddocs/plans/2026-06-09-moderation-overhaul-plan.mddocs/plans/2026-06-10-content-url-restructure-seo-aeo.mddocs/plans/2026-06-14-admin-shell-and-ai-content-design.mddrizzle/0039_feed_personalization.sqllib/og/fonts.tslib/og/templates.tsxlib/og/tokens.tslib/og/url.tsserver/api/router/content.tsserver/api/router/profile.tsserver/db/schema.tsserver/lib/contentAnalysis.tsserver/lib/feedPersonalization.tsserver/lib/feedRanking.test.tsserver/lib/feedRanking.tsserver/lib/topicAffinity.test.tsserver/lib/topicAffinity.tsstyles/globals.css
💤 Files with no reviewable changes (4)
- docs/plans/2026-06-10-content-url-restructure-seo-aeo.md
- docs/plans/2026-06-14-admin-shell-and-ai-content-design.md
- docs/plans/2026-06-09-moderation-overhaul-design.md
- docs/plans/2026-06-09-moderation-overhaul-plan.md
Comment on lines
+1690
to
+1717
| .orderBy(desc(posts.publishedAt)) | ||
| .limit(WINDOW); | ||
|
|
||
| const personalized = hasProfileSignal(profile); | ||
| let ordered = rows; | ||
| if (personalized) { | ||
| const topicsByPost = await loadTopicsByPost( | ||
| ctx.db, | ||
| rows.map((r) => r.id), | ||
| ); | ||
| const candidates = rows.map((r) => ({ | ||
| id: r.id, | ||
| publishedAt: r.publishedAt, | ||
| score: r.upvotes - r.downvotes, | ||
| qualityScore: r.qualityScore, | ||
| topicIds: topicsByPost.get(r.id) ?? [], | ||
| row: r, | ||
| })); | ||
| ordered = rankCandidates(candidates, profile, Date.now()).map( | ||
| (c) => c.item.row, | ||
| ); | ||
| } | ||
|
|
||
| const items = ordered | ||
| .slice(offset, offset + limit) | ||
| .map((item) => ({ ...item, type: toFrontendType(item.type) })); | ||
| const nextOffset = | ||
| ordered.length > offset + limit ? offset + limit : undefined; |
Contributor
There was a problem hiding this comment.
Stabilize candidate ordering to prevent duplicate/missing items across offset pages.
Line 1690 orders only by publishedAt; ties on the same timestamp are non-deterministic, so page boundaries can shift between requests and cause duplicates/skips with offset.
Suggested fix
- .orderBy(desc(posts.publishedAt))
+ .orderBy(desc(posts.publishedAt), desc(posts.id))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/api/router/content.ts` around lines 1690 - 1717, The database query
uses only publishedAt for ordering, which causes non-deterministic ordering when
multiple posts share the same publication timestamp. This creates pagination
issues where the same items can appear on different pages or be skipped. Add a
secondary sort key (such as the post id) to the orderBy clause alongside the
publishedAt descending order to ensure stable, deterministic ordering across
offset-based pagination requests.
Comment on lines
+105
to
+133
| setTopicPref: protectedProcedure | ||
| .input( | ||
| z.object({ | ||
| topicId: z.number().int().positive(), | ||
| pref: z.enum(["follow", "mute", "none"]), | ||
| }), | ||
| ) | ||
| .mutation(async ({ ctx, input }) => { | ||
| const userId = ctx.session.user.id; | ||
| if (input.pref === "none") { | ||
| await ctx.db | ||
| .delete(user_topic_pref) | ||
| .where( | ||
| and( | ||
| eq(user_topic_pref.userId, userId), | ||
| eq(user_topic_pref.topicId, input.topicId), | ||
| ), | ||
| ); | ||
| return { topicId: input.topicId, pref: null }; | ||
| } | ||
| await ctx.db | ||
| .insert(user_topic_pref) | ||
| .values({ userId, topicId: input.topicId, pref: input.pref }) | ||
| .onConflictDoUpdate({ | ||
| target: [user_topic_pref.userId, user_topic_pref.topicId], | ||
| set: { pref: input.pref }, | ||
| }); | ||
| return { topicId: input.topicId, pref: input.pref }; | ||
| }), |
Contributor
There was a problem hiding this comment.
Validate topicId existence before upsert to avoid FK-driven 500s.
Line 108 only validates positivity; with a non-existent topicId, Line 126 can fail at the FK boundary and return an internal error instead of a clean client-facing validation error.
Suggested fix
.mutation(async ({ ctx, input }) => {
const userId = ctx.session.user.id;
+ const [topicRow] = await ctx.db
+ .select({ id: topic.id })
+ .from(topic)
+ .where(eq(topic.id, input.topicId))
+ .limit(1);
+
+ if (!topicRow) {
+ throw new TRPCError({
+ code: "BAD_REQUEST",
+ message: "Invalid topic",
+ });
+ }
+
if (input.pref === "none") {
await ctx.db
.delete(user_topic_pref)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/api/router/profile.ts` around lines 105 - 133, The setTopicPref
mutation validates that topicId is a positive integer but does not verify the
topic actually exists in the database. If a non-existent topicId is provided,
the insert operation on line 126 will fail at the foreign key constraint
boundary, returning a 500 internal error. Add a database query after extracting
userId to verify that the topic with the given topicId exists. If the topic does
not exist, throw a validation error before proceeding to the delete or insert
operations. This ensures client-facing validation errors instead of internal
database constraint failures.
Comment on lines
+99
to
+105
| ranked.sort((a, b) => { | ||
| if (b.score !== a.score) return b.score - a.score; | ||
| const at = a.item.publishedAt ? Date.parse(a.item.publishedAt) : 0; | ||
| const bt = b.item.publishedAt ? Date.parse(b.item.publishedAt) : 0; | ||
| if (bt !== at) return bt - at; | ||
| return a.item.id < b.item.id ? 1 : -1; | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd feedRanking.tsRepository: codu-code/codu
Length of output: 84
🏁 Script executed:
cat -n server/lib/feedRanking.ts | head -150Repository: codu-code/codu
Length of output: 4276
🏁 Script executed:
rg -i "rankCandidates" --type ts --type tsx -B 2 -A 5Repository: codu-code/codu
Length of output: 86
🏁 Script executed:
fd -e test.ts -e spec.ts -e test.tsx -e spec.tsx | xargs grep -l "rankCandidates" 2>/dev/null || echo "No direct test files found"Repository: codu-code/codu
Length of output: 89
🏁 Script executed:
rg "rankCandidates" --type ts --type tsx -lRepository: codu-code/codu
Length of output: 86
🏁 Script executed:
rg -i "rankCandidates" --type ts -B 2 -A 5Repository: codu-code/codu
Length of output: 3434
🏁 Script executed:
cat -n server/lib/feedRanking.test.tsRepository: codu-code/codu
Length of output: 5161
Handle equal IDs in the tie-break comparator.
When score, publishedAt, and id are all equal, the comparator should return 0; current logic returns -1, which violates the comparator contract and can cause non-deterministic sort behavior.
Suggested fix
ranked.sort((a, b) => {
if (b.score !== a.score) return b.score - a.score;
const at = a.item.publishedAt ? Date.parse(a.item.publishedAt) : 0;
const bt = b.item.publishedAt ? Date.parse(b.item.publishedAt) : 0;
if (bt !== at) return bt - at;
+ if (a.item.id === b.item.id) return 0;
return a.item.id < b.item.id ? 1 : -1;
});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/lib/feedRanking.ts` around lines 99 - 105, The sort comparator in the
ranked.sort() function has a bug in its final tie-breaker logic. When comparing
ids with the expression `a.item.id < b.item.id ? 1 : -1`, it returns -1 for both
the case where `a.item.id > b.item.id` and when `a.item.id === b.item.id`.
According to the comparator contract, when two elements are equal, the
comparator must return 0. Fix this by checking if the ids are equal first and
returning 0 in that case, otherwise apply the ascending sort logic: return 1 if
`a.item.id < b.item.id`, else return -1.
Comment on lines
+95
to
+99
| if (interactions.length === 0) { | ||
| await db | ||
| .delete(user_topic_affinity) | ||
| .where(eq(user_topic_affinity.userId, userId)); | ||
| return 0; |
Contributor
There was a problem hiding this comment.
Make affinity replacement atomic.
The current delete-then-insert replacement is non-atomic; if insert fails after delete, the user loses all affinity rows until the next recompute. Wrap replacement in one transaction.
Suggested fix
export async function recomputeUserAffinity(
db: Db,
userId: string,
nowMs: number,
): Promise<number> {
@@
- if (interactions.length === 0) {
- await db
- .delete(user_topic_affinity)
- .where(eq(user_topic_affinity.userId, userId));
- return 0;
- }
+ if (interactions.length === 0) {
+ return db.transaction(async (tx) => {
+ await tx
+ .delete(user_topic_affinity)
+ .where(eq(user_topic_affinity.userId, userId));
+ return 0;
+ });
+ }
@@
- await db
- .delete(user_topic_affinity)
- .where(eq(user_topic_affinity.userId, userId));
- if (rows.length > 0) {
- await db.insert(user_topic_affinity).values(rows);
- }
- return rows.length;
+ return db.transaction(async (tx) => {
+ await tx
+ .delete(user_topic_affinity)
+ .where(eq(user_topic_affinity.userId, userId));
+ if (rows.length > 0) {
+ await tx.insert(user_topic_affinity).values(rows);
+ }
+ return rows.length;
+ });
}Also applies to: 134-139
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/lib/topicAffinity.ts` around lines 95 - 99, The delete-then-insert
replacement pattern for user topic affinity is non-atomic, which means if the
insert fails after the delete succeeds, the user loses all affinity rows. Wrap
both the delete operation (in the interactions.length === 0 block) and the
insert operation in a single database transaction using db.transaction() to
ensure atomicity. This ensures that if either operation fails, all changes are
rolled back and the data remains consistent. Apply this transaction wrapping to
all locations where the affinity replacement occurs to make the entire operation
atomic.
Comment on lines
+149
to
+168
| const [voters, markers, commenters] = await Promise.all([ | ||
| db | ||
| .selectDistinct({ userId: post_votes.userId }) | ||
| .from(post_votes) | ||
| .where(gt(post_votes.createdAt, sinceIso)), | ||
| db | ||
| .selectDistinct({ userId: bookmarks.userId }) | ||
| .from(bookmarks) | ||
| .where(gt(bookmarks.createdAt, sinceIso)), | ||
| db | ||
| .selectDistinct({ userId: comments.authorId }) | ||
| .from(comments) | ||
| .where(gt(comments.createdAt, sinceIso)), | ||
| ]); | ||
|
|
||
| const ids = new Set<string>(); | ||
| for (const row of voters) ids.add(row.userId); | ||
| for (const row of markers) ids.add(row.userId); | ||
| for (const row of commenters) ids.add(row.userId); | ||
| return Array.from(ids).slice(0, limit); |
Contributor
There was a problem hiding this comment.
Bound active-user discovery at query time.
findRecentlyActiveUsers loads all distinct users since sinceIso from three tables, then applies slice(limit) in memory. On busy days, this can scan and materialize far more rows than needed on the cron path.
Suggested fix
export async function findRecentlyActiveUsers(
db: Db,
sinceIso: string,
limit: number,
): Promise<string[]> {
+ const sourceCap = Math.max(limit * 3, limit);
const [voters, markers, commenters] = await Promise.all([
db
.selectDistinct({ userId: post_votes.userId })
.from(post_votes)
- .where(gt(post_votes.createdAt, sinceIso)),
+ .where(gt(post_votes.createdAt, sinceIso))
+ .orderBy(desc(post_votes.createdAt))
+ .limit(sourceCap),
db
.selectDistinct({ userId: bookmarks.userId })
.from(bookmarks)
- .where(gt(bookmarks.createdAt, sinceIso)),
+ .where(gt(bookmarks.createdAt, sinceIso))
+ .orderBy(desc(bookmarks.createdAt))
+ .limit(sourceCap),
db
.selectDistinct({ userId: comments.authorId })
.from(comments)
- .where(gt(comments.createdAt, sinceIso)),
+ .where(gt(comments.createdAt, sinceIso))
+ .orderBy(desc(comments.createdAt))
+ .limit(sourceCap),
]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@server/lib/topicAffinity.ts` around lines 149 - 168, The
`findRecentlyActiveUsers` function loads all distinct users from three database
queries (post_votes, bookmarks, and comments) then applies limit in memory via
slice(limit). To improve performance, add a `.limit(limit)` clause directly to
each of the three database queries for voters, markers, and commenters to bound
the result set at the database level rather than loading all rows into memory.
Then adjust the final return statement to account for the fact that results are
already bounded, removing the slice operation since each query now respects the
limit constraint.
Comment on lines
+366
to
374
| @apply text-accent-soft underline transition-all; | ||
| text-underline-offset: 2px; | ||
| text-decoration-thickness: 1px; | ||
| } | ||
| .prose blockquote { | ||
| @apply border-accent; | ||
| border-left-width: 3px; | ||
| color: rgb(var(--color-muted)); | ||
| } |
Contributor
There was a problem hiding this comment.
Add empty lines before declarations to fix Stylelint warnings.
Stylelint requires empty lines before certain declarations for consistency.
🎨 Proposed formatting fix
.prose a {
`@apply` text-accent-soft underline transition-all;
+
text-underline-offset: 2px;
text-decoration-thickness: 1px;
}
.prose blockquote {
`@apply` border-accent;
+
border-left-width: 3px;
color: rgb(var(--color-muted));
}🧰 Tools
🪛 Stylelint (17.12.0)
[error] 367-367: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
[error] 372-372: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@styles/globals.css` around lines 366 - 374, Add empty lines before
declarations to satisfy Stylelint formatting requirements. Insert a blank line
before the `.prose blockquote` rule and before the `@apply border-accent`
declaration within that rule to maintain consistent spacing and meet Stylelint's
style guidelines for CSS declarations.
Source: Linters/SAST tools
- Restore line-height: 1.62 on .prose :where(p,ul,ol) so it explicitly
overrides prose-lg's own paragraph rule at lg+ (inherited value was silently
losing to the plugin's direct p selector)
- Add .tiptap :where(p,ul,ol) { color: inherit } so the muted body-copy color
does not bleed into the article editor's contenteditable
- Switch inline code from text-accent-soft to text-fg: accent-soft on bg-inset
in light mode was 2.30:1 contrast, failing WCAG AA
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.
Summary
max-w-prose(672px) — unifies the three surfaces and lands at the ~70 chars/line readability sweet spotpx-0 sm:px-4) from content page wrappers; the shell grid gap and the mobile shell gutter (bumped 0.75rem→1rem) own all horizontal breathing roombg-inset,border-hairline,text-accent-soft,color-muted/color-fg) so code blocks, inline code, links, and blockquotes flip correctly in dark mode.prose codeto.prose :not(pre) > codeto avoid touching fenced code blocks