feat: add author_roles table, sync from author frontmatter (#177)#186
Conversation
Tracks author roles in a real table (mirroring playfulprogramming#81's tags pattern) instead of embedding them in profiles.meta, so /content/profiles can eventually filter by role.
|
Warning Review limit reached
Next review available in: 19 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a new ChangesAuthor Roles Persistence
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Processor
participant DB
participant S3
Processor->>S3: processProfileImg (resize + upload profiles/authorId.jpeg)
Processor->>DB: upsert profiles (meta without roles)
Processor->>DB: delete authorRoles where profileSlug matches
Processor->>DB: insert authorRoles (if roles list non-empty)
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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.
🧹 Nitpick comments (1)
apps/worker/src/tasks/sync-author/processor.ts (1)
20-38: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winForward source stream errors to the sharp pipeline.
Readable.fromWeb(stream).pipe(pipeline)does not propagate source errors to the sharp destination. If the GitHub content stream errors mid-transfer, the sharp pipeline will hang waiting for data, ands3.uploadwill block indefinitely — potentially stalling the worker job until a timeout fires.Attach an error listener to forward failures:
♻️ Proposed fix
async function processProfileImg( stream: ReadableStream<Uint8Array>, uploadKey: string, ) { const pipeline = sharp() .resize({ width: PROFILE_IMAGE_SIZE_MAX, height: PROFILE_IMAGE_SIZE_MAX, fit: "inside", }) .jpeg({ mozjpeg: true }); - Readable.fromWeb(stream as never).pipe(pipeline); + const source = Readable.fromWeb(stream as never); + source.on("error", (err) => pipeline.destroy(err)); + source.pipe(pipeline); const bucket = await s3.ensureBucket(env.S3_BUCKET); await s3.upload(bucket, uploadKey, undefined, pipeline, "image/jpeg"); }🤖 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 `@apps/worker/src/tasks/sync-author/processor.ts` around lines 20 - 38, The source stream errors are not being forwarded into the sharp pipeline in processProfileImg, so a mid-transfer failure can leave the upload hanging. Update the Readable.fromWeb(stream) handling to attach an error listener that destroys or rejects the sharp pipeline when the source ReadableStream fails, and make sure the async flow around s3.upload cannot wait indefinitely after a stream error. Use processProfileImg and the Readable.fromWeb(...).pipe(pipeline) section as the fix point.
🤖 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.
Nitpick comments:
In `@apps/worker/src/tasks/sync-author/processor.ts`:
- Around line 20-38: The source stream errors are not being forwarded into the
sharp pipeline in processProfileImg, so a mid-transfer failure can leave the
upload hanging. Update the Readable.fromWeb(stream) handling to attach an error
listener that destroys or rejects the sharp pipeline when the source
ReadableStream fails, and make sure the async flow around s3.upload cannot wait
indefinitely after a stream error. Use processProfileImg and the
Readable.fromWeb(...).pipe(pipeline) section as the fix point.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4e56d0b2-7631-4d20-b6d5-c2578ca07c95
📒 Files selected for processing (6)
apps/worker/src/tasks/sync-author/processor.test.tsapps/worker/src/tasks/sync-author/processor.tsapps/worker/test-utils/setup.tspackages/db/drizzle/20260709150348_shocking_expediter/migration.sqlpackages/db/drizzle/20260709150348_shocking_expediter/snapshot.jsonpackages/db/src/schema/profiles.ts
…essProfileImg Readable.fromWeb(stream).pipe(pipeline) doesn't forward source errors to the destination — a mid-transfer failure from GitHub's content stream left the pipeline hanging and s3.upload blocking indefinitely instead of failing fast. Per CodeRabbit feedback on PR playfulprogramming#186.
|
|
||
| // Placed here for now since it's an owner-scoped role list like profileAchievements, | ||
| // but it's structurally identical to the post_tags/collection_tags (ownerSlug, value) | ||
| // composite-PK pattern in tags.ts, so this may belong there instead — open question for James. | ||
| export const authorRoles = pgTable( |
There was a problem hiding this comment.
| // Placed here for now since it's an owner-scoped role list like profileAchievements, | |
| // but it's structurally identical to the post_tags/collection_tags (ownerSlug, value) | |
| // composite-PK pattern in tags.ts, so this may belong there instead — open question for James. | |
| export const authorRoles = pgTable( | |
| export const authorRoles = pgTable( |
LGTM, just want to remove this comment.
TBH I'd rather have the tables in tags.ts moved into the post/collection files rather than their own file, since there's no central "tags" table.
James's review closed the open question about whether author_roles belongs with the post_tags/collection_tags pattern in tags.ts instead - it's staying in profiles.ts, so the comment flagging that as unresolved no longer applies. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2 participants
Summary
Closes #177. Adds an
author_rolestable populated fromsync-author, tracking author roles in a real table instead ofprofiles.meta.roles, so/content/profilescan eventually filter by role. This follows the same JSONB-to-join-table pattern as #81'spost_tags/collection_tagswork.author_rolestable: composite PK on(profileSlug, role), FK toprofiles.slugwith cascade delete, no surrogate id — mirroring the shape ofpost_tags/collection_tags.sync-author's processor now deletes all existingauthor_rolesrows for the profile slug and re-inserts the current roles (if any) inside the existingdb.transactionblock, following the same delete-then-insert patternsync-postuses forpostTags. Authors aren't locale-scoped like posts, so there's no multi-locale aggregation concern here.profiles.metano longer includesroles— it now only containssocials.GET /content/profilesdoesn't readmeta.rolesanywhere in this repo today, and sincesync-authordoes a fullonConflictDoUpdateon the wholemetaobject every sync, oldmeta.rolesdata on existing rows will self-heal away automatically the next time each profile is synced. No backfill needed.drizzle-kit generate.Open question: table placement
I placed
author_rolesinprofiles.ts(alongside the existingprofileAchievementstable) rather than intags.ts, since it's an owner-scoped role list similar toprofileAchievements. That said, it's structurally identical to thepost_tags/collection_tags(ownerSlug, value)composite-PK pattern intags.ts— so James may prefer it live there instead for consistency. Left a comment in the code noting this; happy to move it if that's the preferred convention.Heads up for James
meta.rolesis no longer populated going forward. Nothing in this repo's API currently reads it, so this isn't blocking — but if the frontend repo reads raw profilemetadirectly anywhere, it would need to switch to the newauthor_rolestable.Test plan
apps/worker/src/tasks/sync-author/processor.test.ts:author_roleson syncpnpm test:unitpasses (lint, knip, publint, sherif, vitest across all NX projects)Paste that into the compare URL, double-check for truncation, then create the PR yourself.
Summary by CodeRabbit
New Features
Bug Fixes