Split post modules along feature boundaries#357
Conversation
Separate post model behavior and GraphQL schema registration into focused modules while preserving the existing compatibility facades and generated schema. Move actor and account post fields to the composition root, and centralize shared persistence helpers so local and remote synchronization use the same implementation. Closes hackers-pub#340 Assisted-by: Codex:gpt-5.6-sol
dahlia
added
enhancement
|
@codex review |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe post domain and GraphQL implementations are split into feature-specific modules. Compatibility facades preserve existing exports, consumers use targeted subpaths, GraphQL fields are rewired, and visibility, persistence, sharing, article, media, and link-preview workflows are organized by responsibility. ChangesPost domain modularization
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant GraphQL
participant PostModels
participant Database
Client->>GraphQL: request post operation
GraphQL->>PostModels: invoke feature-specific workflow
PostModels->>Database: persist or query post data
Database-->>PostModels: return related records
PostModels-->>GraphQL: return hydrated schema data
GraphQL-->>Client: return GraphQL response
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the post-related modules by splitting the monolithic post files in both models/ and graphql/ into smaller, focused subpaths (such as core, remote, source, visibility, sharing, engagement, and lifecycle) to establish clearer architectural boundaries and prevent dependency cycles. The review feedback highlights a few important issues: first, the use of Response.bytes() in models/link-preview.ts will cause runtime errors in Node.js 20 or older, so it should be replaced with Response.arrayBuffer() for LTS compatibility; second, a loose nullish check (== null) should be used instead of === undefined when checking post.sharedPost in models/post/visibility.ts to correctly handle deleted relations; and finally, a redundant loop in models/post/lifecycle.ts that queries a deleted post should be removed.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Codex Review: Didn't find any major issues. Another round soon, please! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
federation/inbox/subscribe.ts (1)
39-41: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlphabetize the internal imports in both files.
As per coding guidelines, internal imports must be alphabetized within their group:
federation/inbox/subscribe.ts#L39-L41: order imports aspost/core,post/engagement, thenpost/remote.models/post.facade.test.ts#L1-L11: move./link-preview.tsbefore the./post/*imports.🤖 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 `@federation/inbox/subscribe.ts` around lines 39 - 41, Alphabetize the internal imports in federation/inbox/subscribe.ts by ordering post/core, post/engagement, then post/remote; also move ./link-preview.ts before the ./post/* imports in models/post.facade.test.ts.Source: Coding guidelines
models/post.facade.test.ts (1)
13-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover all runtime facade exports.
This test checks only a subset of the compatibility facade’s runtime re-exports, so a dropped export could regress without failing the test. Add assertions for every runtime export, with separate compile-time coverage for type-only exports.
🤖 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 `@models/post.facade.test.ts` around lines 13 - 28, Expand the “post compatibility facade re-exports each feature implementation” test to assert every runtime export exposed by the facade, including any currently omitted symbols from the feature modules. Add separate compile-time coverage for type-only exports, keeping type assertions distinct from runtime equality checks.graphql/post.facade.test.ts (1)
22-28: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd identity checks for remaining facade exports.
The test verifies identity for 7 of 11 exported keys but skips
PostLink,PostType,hidePostRelationWithoutActor, andisPostVisibleToViewer. Sincecoreis already imported,PostLinkandPostTypecan be checked with zero new imports. The other two would require importing the visibility module.♻️ Optional: add missing identity assertions
assert.equal(facade.Post, core.Post); assert.equal(facade.Medium, core.Medium); + assert.equal(facade.PostLink, core.PostLink); + assert.equal(facade.PostType, core.PostType); assert.equal(facade.Note, note.Note); assert.equal(facade.Question, note.Question); assert.equal(facade.Article, article.Article); assert.equal(facade.ArticleContent, article.ArticleContent); assert.equal(facade.ArticleDraft, article.ArticleDraft);🤖 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 `@graphql/post.facade.test.ts` around lines 22 - 28, Extend the facade export identity test to cover the remaining exports: add assertions for core.PostLink and core.PostType, and import the visibility module to assert hidePostRelationWithoutActor and isPostVisibleToViewer reference the same implementations exposed by the facade.graphql/post/actor-fields.ts (1)
108-135: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd
descriptiontonoteByUuidfield and itsuuidarg.
noteByUuidis the only field in this module without a fielddescription, and itsuuidargument also lacks one. Every other field/arg here documents intent. Note this may change the generated schema (which the PR aims to keep stable), so confirm against the facade/schema snapshot tests.As per coding guidelines: "Add
description:property to field definitions in GraphQL files" and "Adddescription:property tot.arg()for arguments in GraphQL files".📝 Proposed addition
noteByUuid: t.drizzleField({ type: Note, + description: + "Look up one of this actor's `Note`-type posts by either its " + + "`Post.uuid` (row PK) or `Note.sourceId`. Returns `null` if no " + + "match or the note is not visible to the current viewer.", select: { columns: { id: true } }, nullable: true, args: { - uuid: t.arg({ type: "UUID", required: true }), + uuid: t.arg({ + type: "UUID", + required: true, + description: "Either `Post.uuid` or `Note.sourceId`.", + }), },🤖 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 `@graphql/post/actor-fields.ts` around lines 108 - 135, Add descriptions to the noteByUuid field definition and its required uuid argument, following the module’s existing GraphQL documentation style. Keep the resolver behavior unchanged, then verify the generated facade/schema snapshot tests reflect the intended descriptions without unrelated schema changes.Source: Coding guidelines
🤖 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 `@models/link-preview.ts`:
- Around line 501-506: Update the cache lookup in scrapePostLink to use the same
URL identity persisted by persistPostLink: resolve and query using the stored
responseUrl rather than only the original scrapeUrl.href, while preserving hash
removal and the existing 24-hour cache behavior.
- Around line 379-457: Add SSRF-safe validation for image URLs in the preview
image metadata fetch flow, validating the initial URL and every redirect target
before following it rather than relying on redirect: "follow"; reuse the
existing URL validation utilities if available and preserve the current cleanup
behavior for invalid or failed fetches. In persistPostLink(), align the cache
lookup key using scrapeUrl.href with the URL used when storing scraped rows,
including redirected response URLs, so redirected entries are reused.
---
Nitpick comments:
In `@federation/inbox/subscribe.ts`:
- Around line 39-41: Alphabetize the internal imports in
federation/inbox/subscribe.ts by ordering post/core, post/engagement, then
post/remote; also move ./link-preview.ts before the ./post/* imports in
models/post.facade.test.ts.
In `@graphql/post.facade.test.ts`:
- Around line 22-28: Extend the facade export identity test to cover the
remaining exports: add assertions for core.PostLink and core.PostType, and
import the visibility module to assert hidePostRelationWithoutActor and
isPostVisibleToViewer reference the same implementations exposed by the facade.
In `@graphql/post/actor-fields.ts`:
- Around line 108-135: Add descriptions to the noteByUuid field definition and
its required uuid argument, following the module’s existing GraphQL
documentation style. Keep the resolver behavior unchanged, then verify the
generated facade/schema snapshot tests reflect the intended descriptions without
unrelated schema changes.
In `@models/post.facade.test.ts`:
- Around line 13-28: Expand the “post compatibility facade re-exports each
feature implementation” test to assert every runtime export exposed by the
facade, including any currently omitted symbols from the feature modules. Add
separate compile-time coverage for type-only exports, keeping type assertions
distinct from runtime equality checks.
🪄 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: e5413630-1f7f-404c-8087-73eae47794ac
📒 Files selected for processing (53)
ARCHITECTURE.mdfederation/collections.tsfederation/inbox/quote.tsfederation/inbox/subscribe.tsfederation/inbox/update.tsfederation/objects.tsgraphql/account.tsgraphql/actor.tsgraphql/builder.tsgraphql/lookup.tsgraphql/mod.tsgraphql/moderation.tsgraphql/news.tsgraphql/poll.tsgraphql/post.facade.test.tsgraphql/post.tsgraphql/post/actor-fields.tsgraphql/post/article-fields.tsgraphql/post/article.tsgraphql/post/core.tsgraphql/post/mutations.tsgraphql/post/note.tsgraphql/reactable.tsgraphql/refresh.tsgraphql/search.tsmodels/actor.tsmodels/article-source.tsmodels/article.tsmodels/deno.jsonmodels/link-preview.tsmodels/note.tsmodels/poll.tsmodels/post.delete.test.tsmodels/post.facade.test.tsmodels/post.lifecycle.test.tsmodels/post.remote.test.tsmodels/post.share.test.tsmodels/post.sync.test.tsmodels/post.test.tsmodels/post.tsmodels/post/core.tsmodels/post/engagement.tsmodels/post/lifecycle.tsmodels/post/persistence.tsmodels/post/remote-fetch.tsmodels/post/remote.tsmodels/post/sharing.tsmodels/post/source.tsmodels/post/visibility.tsmodels/profile-interactions.tsmodels/question.tsmodels/reaction.tsmodels/timeline.ts
💤 Files with no reviewable changes (1)
- graphql/account.ts
Validate every preview URL in a manually followed redirect chain before issuing the next request. Use portable response body APIs and reuse the redirect destination recorded by posts that share the same authored URL. hackers-pub#357 (comment) hackers-pub#357 (comment) hackers-pub#357 (comment) hackers-pub#357 (comment) Assisted-by: Codex:gpt-5.6-sol
Treat both unloaded and missing shared-post relations as insufficient to evaluate a boost wrapper. This prevents a dangling relation from bypassing the wrapper visibility guard. hackers-pub#357 (comment) Assisted-by: Codex:gpt-5.6-sol
Delete the query and update loop that ran after both the quoted post and its quoting rows had already been removed. The subsequent interaction processing already handles the affected news score inputs. hackers-pub#357 (comment) Assisted-by: Codex:gpt-5.6-sol
|
/gemini review |
|
@codex review |
There was a problem hiding this comment.
Code Review
This pull request reorganizes the post-related models and GraphQL schemas into focused sub-modules (such as core, remote, source, visibility, sharing, engagement, and lifecycle) to prevent dependency cycles and improve code architecture, updating imports across the codebase accordingly. It also introduces article source rendering helpers in a separate module and enhances link preview scraping with SSRF safety checks. The review feedback highlights several improvement opportunities: stripping surrounding quotes from parsed charsets to prevent encoding errors, replacing the non-standard Temporal API with standard Date arithmetic for better compatibility, and ensuring that async iterators are properly closed in try...finally blocks when breaking early to avoid resource leaks.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3dc3bee0c9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Strip matching quotes from Content-Type charset values before decoding so valid quoted UTF-8 declarations follow the normal HTML parsing path. Cover the case through the existing Open Graph scraping test. hackers-pub#357 (comment) Assisted-by: Codex:gpt-5.6-sol
Always return the manually consumed collection iterator when inline reply traversal stops. This releases underlying collection resources on reply limits, time budgets, and traversal failures. hackers-pub#357 (comment) Assisted-by: Codex:gpt-5.6-sol
Load the original post and its viewer-specific relations before checking a share wrapper as a reply target. This keeps visibility checks fail-closed without rejecting valid replies to public shares in either mutation. hackers-pub#357 (comment) Assisted-by: Codex:gpt-5.6-sol
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the post-related codebase by splitting monolithic files in both the database models and the GraphQL schema into focused subpath modules (such as core, remote, source, visibility, sharing, engagement, and lifecycle) to prevent dependency cycles and improve modularity. It also updates all corresponding imports and adds compatibility facades and tests to ensure public references are preserved. The review feedback correctly identifies style guide violations, specifically missing descriptions for GraphQL arguments and fields in graphql/post/article.ts and graphql/post/core.ts, as well as unsorted relative imports in models/post/sharing.ts.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
graphql/post/mutations.ts (1)
259-278: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated reply/quote-target hydration query into a shared helper.
This diff adds
sharedPosthydration to thereplyTargetIdlookup in bothcreateNoteandcreateQuestionto catch it up with thequotedPostIdlookup's pre-existing (identical) hydration shape. The exact same nestedactor/followers/blockees/blockers/mentions/sharedPostshape is now duplicated 4 times in this file (2× forreplyTargetId, 2× forquotedPostId). This diff is itself evidence of the drift risk: the reply-target query had fallen behind the quote-target query on this exact relation set until now. Extracting a small helper (e.g.loadVisibilityHydratedPost(db, postId, viewerActorId)) that returns the hydrated post would remove ~80 lines of duplication and prevent a future patch from missing one of the four call sites again.♻️ Sketch of a shared helper
function withVisibilityRelations(viewerActorId: Uuid) { return { actor: { with: { followers: { where: { followerId: viewerActorId } }, blockees: { where: { blockeeId: viewerActorId } }, blockers: { where: { blockerId: viewerActorId } }, }, }, mentions: { where: { actorId: viewerActorId } }, } as const; } async function loadVisibilityHydratedPost( db: typeof ctx.db, postId: Uuid, viewerActorId: Uuid, ) { return await db.query.postTable.findFirst({ with: { ...withVisibilityRelations(viewerActorId), sharedPost: { with: withVisibilityRelations(viewerActorId) }, }, where: { id: postId }, }); }Also applies to: 582-601
🤖 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 `@graphql/post/mutations.ts` around lines 259 - 278, The reply-target and quote-target lookups duplicate the same visibility hydration query across four call sites. Extract shared helpers near the existing mutation logic, such as a relation-builder and a post loader, then update both createNote and createQuestion lookups for replyTargetId and quotedPostId to use them while preserving the actor, follower, blockee, blocker, mention, and sharedPost hydration.
🤖 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 `@graphql/post/mutations.ts`:
- Around line 259-278: The reply-target and quote-target lookups duplicate the
same visibility hydration query across four call sites. Extract shared helpers
near the existing mutation logic, such as a relation-builder and a post loader,
then update both createNote and createQuestion lookups for replyTargetId and
quotedPostId to use them while preserving the actor, follower, blockee, blocker,
mention, and sharedPost hydration.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 2070b7d5-bd95-40a4-b588-55579ee269f3
📒 Files selected for processing (5)
graphql/post.more.test.tsgraphql/post/mutations.tsmodels/link-preview.tsmodels/post.test.tsmodels/post/remote.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- models/post.test.ts
- models/link-preview.ts
- models/post/remote.ts
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Explain locale filtering, translation progress, visibility, and quote state semantics in the public GraphQL contract. hackers-pub#357 (comment) hackers-pub#357 (comment) Assisted-by: Codex:gpt-5.6-sol
Keep internal relative imports ordered by module path so the extracted sharing module follows the repository import convention. hackers-pub#357 (comment) Assisted-by: Codex:gpt-5.6-sol
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the post feature boundaries by splitting monolithic post models and GraphQL schemas into focused subpaths (such as core, remote, source, visibility, sharing, engagement, and lifecycle) to prevent workspace dependency cycles and improve modularity. Compatibility facades and updated tests are introduced to preserve public references. The review feedback highlights several critical null-pointer dereference risks in the GraphQL resolvers where ctx.account is accessed without optional chaining, which could crash requests for unauthenticated viewers. Additionally, the reviewer pointed out a potential resource leak in the link preview scraper where unconsumed response bodies should be explicitly cancelled to prevent unhandled rejections in Deno.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Cancel unsuccessful image metadata response streams so failed probes do not leave network resources open. Cover the cleanup path with a stream whose cancellation is observable. hackers-pub#357 (comment) Assisted-by: Codex:gpt-5.6-sol
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the post feature by splitting the monolithic models/post.ts and graphql/post.ts files into focused submodules (such as core, visibility, remote, sharing, engagement, and lifecycle) to establish clear boundaries and prevent dependency cycles. Compatibility facades are maintained to preserve public references. Feedback on the changes highlights a critical runtime TypeError in models/post/remote.ts due to an incorrect lookupObject call on the context, a TypeScript compilation error with Drizzle's inArray subquery usage, and several import-ordering style guide violations across the newly created GraphQL files.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f5a18e1310
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Check both a share wrapper and its effective original before accepting it as a reply target. This prevents public wrappers from exposing replies to followers-only or direct posts while retaining replies to public shares. Add one regression test that covers both note and question creation paths. hackers-pub#357 (comment) Assisted-by: Codex:gpt-5.6-sol
Keep third-party dependencies ahead of workspace and relative imports in the split post resolver modules. Also order the moved imports within their local groups so each module follows the repository convention. hackers-pub#357 (comment) hackers-pub#357 (comment) hackers-pub#357 (comment) Assisted-by: Codex:gpt-5.6-sol
|
/gemini review |
|
@codex review |
There was a problem hiding this comment.
Code Review
This pull request refactors the post feature boundaries by splitting the monolithic post models and GraphQL schemas into focused sub-modules (such as core, remote, source, visibility, sharing, engagement, and lifecycle) and updating imports across the codebase. The review feedback highlights several critical areas in the GraphQL resolvers where optional chaining is needed to prevent potential runtime TypeError crashes when accessing ctx.account or ctx.account.actor. Additionally, the reviewer noted import sorting violations in models/link-preview.ts and models/post/source.ts that should be resolved to align with the repository's style guide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Place nested post imports alongside the other internal modules in lexical order. This keeps the extracted model files aligned with the repository's import grouping convention without changing runtime behavior. hackers-pub#357 (comment) hackers-pub#357 (comment) Assisted-by: Codex:gpt-5.6-sol
|
/gemini review |
|
@codex review |
There was a problem hiding this comment.
Code Review
This pull request refactors the post-related models and GraphQL schema by modularizing the post-related logic into sub-packages (core, engagement, lifecycle, remote, sharing, source, visibility). It also introduces a new scrapePostLink utility for link previews with improved security (SSRF protection) and robust error handling. The review comments correctly identify critical runtime risks where optional chaining was missing when accessing ctx.account?.actor.id, which would cause crashes for unauthenticated requests, and point out an import sorting violation in federation/inbox/subscribe.ts.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d81425dd99
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Load each boost's original post author before applying the shared visibility guard. The article itself is already constrained and visible, while the relation lets the guard evaluate its sanction state instead of failing closed. hackers-pub#357 (comment) Assisted-by: Codex:gpt-5.6-sol
Group third-party, workspace, and relative dependencies in lexical order. This makes the extracted post module imports follow the repository style without changing inbox behavior. hackers-pub#357 (comment) Assisted-by: Codex:gpt-5.6-sol
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request performs a significant refactoring of the post-related logic, modularizing it into separate files under models/post/ and graphql/post/ to improve maintainability. It also introduces new link scraping functionality and updates import paths across the codebase. Several potential null pointer exceptions were identified in the new resolvers, which should be addressed by using optional chaining as suggested.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Part of #336. Closes #340.
Why these boundaries
The old models/post.ts and graphql/post.ts grouped code because it worked on posts, even when that code changed for unrelated reasons. A visibility rule, an ActivityPub ingestion fix, and a GraphQL mutation all had to pass through the same modules. The problem was not file length by itself. The modules had too many reasons to change, which made their dependency direction hard to see and encouraged more code to accumulate there.
This PR uses those reasons to change as the boundary. It keeps the existing entry points intact, so callers do not have to migrate in one large step, but new code can depend on a narrower part of the post model.
Model boundaries
models/post.ts is now a compatibility facade with explicit re-exports. The public subpaths declared in models/deno.json separate core lookup, remote ingestion, local source synchronization, visibility policy, sharing, engagement counters, and deletion. This makes dependencies more honest. Code that only evaluates visibility no longer needs to load remote federation and link scraping code.
Remote ingestion and local source synchronization remain separate even though both persist posts. They start from different inputs and have different dependency needs: remote ingestion interprets ActivityPub objects, while local synchronization renders authored sources. The small amount of persistence logic they genuinely share lives in models/post/persistence.ts, rather than being copied into both modules.
Link preview scraping moved to models/link-preview.ts because its cache and repair behavior belong to link metadata, not to the post lifecycle. Article source helpers moved to models/article-source.ts so local synchronization can render article content without introducing a cycle through models/article.ts.
GraphQL composition
graphql/post.ts follows the same compatibility-facade approach. The schema implementation is divided between shared post fields, article types, note and question types, and mutation/query registration under graphql/post/.
Actor post fields and
Account.articleDraftsare registered from graphql/mod.ts after their base types are available. Keeping those registrations out of graphql/actor.ts and graphql/account.ts avoids making the base actor and account modules depend on the post facade. The composition root now shows when those cross-feature fields enter the schema.Compatibility
The facades preserve the existing model exports and GraphQL type references, while internal consumers use focused imports where the dependency is known. The generated GraphQL schema and Relay artifacts remain unchanged. The new boundaries and the rule for choosing an import path are documented in ARCHITECTURE.md.