Repository navigation
feat(devvit): add pending post reconciliation operations - #70
Conversation
|
@codex review Please focus on the atomic state/index transaction boundary, scope-bound cursor semantics, normal concurrent transitions, and the invariant that pending discovery never restores submit permission. |
📝 WalkthroughWalkthroughAdds indexed durable Devvit post-operation storage, scope-bound cursor pagination through ChangesDevvit pending-operation recovery
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant RecoveryWorker
participant Coordinator
participant IndexedStore
participant Redis
RecoveryWorker->>Coordinator: listPending(scope, cursor, limit)
Coordinator->>IndexedStore: listIndex(scope/type index, cursor, limit)
IndexedStore->>Redis: zRange lex range
Redis-->>IndexedStore: indexed members
Coordinator->>IndexedStore: read referenced durable records
Coordinator-->>RecoveryWorker: pending page and nextCursor
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![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: 286b1ab8a6
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
adapters/devvit/src/server/post-operation.ts (1)
6-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared page-limit cap instead of duplicating it.
maximumPendingPageLimit = 100here has an implicit "+1" contract with the hardcoded101cap inassertIndexPageLimit(redis-post-operation-store.ts). Since this constant isn't exported, the Redis store can't reference it, so the two values must be kept in sync by hand. See consolidated comment for both sites.🤖 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 `@adapters/devvit/src/server/post-operation.ts` around lines 6 - 7, Extract the shared pending-page limit cap into an exported common constant and update both maximumPendingPageLimit in post-operation handling and the hardcoded 101 bound in assertIndexPageLimit to reference it, preserving the existing implicit +1 contract without duplicated numeric literals.adapters/devvit/src/server/redis-post-operation-store.ts (1)
291-296: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHardcoded
101duplicates the coordinator's page-limit contract.This cap is
maximumPendingPageLimit + 1frompost-operation.ts, but that constant isn't exported/shared, so the two values can silently drift. See consolidated comment.🤖 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 `@adapters/devvit/src/server/redis-post-operation-store.ts` around lines 291 - 296, The hardcoded 101 in assertIndexPageLimit duplicates the coordinator page-limit contract and can drift. Export or otherwise share the maximumPendingPageLimit-based limit from post-operation.ts, then reuse that symbol in assertIndexPageLimit instead of embedding 101, preserving the existing validation and error behavior.
🤖 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 `@adapters/devvit/src/server/redis-post-operation-store.ts`:
- Around line 15-34: Update DevvitRedisTransactionLike to match the Devvit Redis
SDK: define zAdd as accepting a single sorted-set member object or array, zRem
as accepting a string or string array, and zRange with positional start/stop
arguments plus options supporting by 'rank' or 'score'. Replace the incompatible
DevvitRedisRangeOptions shape and adjust the related method signatures without
changing unrelated transaction methods.
---
Nitpick comments:
In `@adapters/devvit/src/server/post-operation.ts`:
- Around line 6-7: Extract the shared pending-page limit cap into an exported
common constant and update both maximumPendingPageLimit in post-operation
handling and the hardcoded 101 bound in assertIndexPageLimit to reference it,
preserving the existing implicit +1 contract without duplicated numeric
literals.
In `@adapters/devvit/src/server/redis-post-operation-store.ts`:
- Around line 291-296: The hardcoded 101 in assertIndexPageLimit duplicates the
coordinator page-limit contract and can drift. Export or otherwise share the
maximumPendingPageLimit-based limit from post-operation.ts, then reuse that
symbol in assertIndexPageLimit instead of embedding 101, preserving the existing
validation and error behavior.
🪄 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: 9f9f0475-b54b-40fb-aef3-da12742e7583
📒 Files selected for processing (11)
.sampo/changesets/devvit-post-reconciliation-operations.mdadapters/devvit/src/server/post-operation.test.tsadapters/devvit/src/server/post-operation.tsadapters/devvit/src/server/redis-post-operation-store.test.tsadapters/devvit/src/server/redis-post-operation-store.tsapps/target-devvit/README.mdapps/target-devvit/src/server/postOperationStore.tsdocs/DEVVIT_DURABLE_POST_OPERATIONS.mdpackages/cli/templates/phaser-game/apps/target-devvit/README.mdpackages/cli/templates/phaser-game/apps/target-devvit/src/server/postOperationStore.tstools/validate-starter-workflow.ts
|
@codex review Please re-review latest head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c18ae98663
ℹ️ 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".
|
@codex review Please review latest head |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f6be3cace
ℹ️ 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".
|
@codex review Please re-review latest head |
|
Codex Review: Didn't find any major issues. 🎉 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". |
Summary
listPending()cursor pagination for Devvit post operationsSafety contract
Validation
mise exec -- pnpm checkmise exec -- pnpm testmise exec -- pnpm graph:preflightmise exec -- pnpm --filter @mpgd/adapter-devvit test(78 tests on latest head)mise exec -- pnpm build:devvitgit diff --checkpnpm sampo:release:dry-runparsed the workspace and then stopped because releases are restricted tomain, as expected on a feature branch.Open Code Review was invoked for all four commits. The configured external
glm-5.2endpoint timed out or failed before completing any file reviews: five files on the initial commit, two files on the stable-registry patch, two files on the initial backfill patch (session60a00399-7b3d-4e87-8963-179eaaac9dc2), and one file on the exact-read availability patch (session4d5ed4d3-3ba3-4572-9357-d77ae8828311). No OCR code comments were produced; GitHub Codex and CodeRabbit reviews remain required before merge.Release
Adds a Sampo changeset for:
@mpgd/adapter-devvit: minor@mpgd/cli: patch@mpgd/create-game: patch