Conversation
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (6)
📒 Files selected for processing (9)
📝 WalkthroughWalkthroughAdds non‑optimistic verification polling for missing entries, a new verifyPostOnAlternateNode utility (exported from bridge) with tests to query up to two alternate Hive nodes, tweaks SDK Hive client defaults, and wires alternate-node verification into post query flows and related tests. Changes
Sequence DiagramsequenceDiagram
participant Client
participant QueryCache as Query Cache
participant Bridge
participant PrimaryNode as Primary Hive Node
participant AlternateNodes as Alternate Hive Nodes
Client->>Bridge: getPost(author, permlink, observer)
Bridge->>PrimaryNode: bridge.get_post(...)
alt Primary returns Entry
PrimaryNode-->>Bridge: Entry
Bridge->>QueryCache: update cache
Bridge-->>Client: return Entry
else Primary returns null
PrimaryNode-->>Bridge: null
Bridge->>AlternateNodes: verifyPostOnAlternateNode(author, permlink, observer, primaryNode)
alt Alternate returns Entry
AlternateNodes-->>Bridge: Entry
Bridge->>QueryCache: update cache
Bridge-->>Client: return verified Entry
else All alternates null/failed
AlternateNodes-->>Bridge: null
Bridge-->>Client: return null
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/sdk/src/modules/posts/queries/get-posts-ranked-query-options.spec.ts (1)
72-91: Strengthen the success-case assertion to actually prove sorting/filtering.At Line 72, the test says “sorted and filtered,” but the fixture is already in descending order and no filtering effect is asserted, so this can pass even if sorting/filtering regresses.
Suggested test adjustment
- it('should return sorted and filtered entries on success', async () => { + it('should return entries sorted by created desc on success', async () => { const mockEntries = [ - { author: 'a', permlink: 'p1', created: '2026-01-02T00:00:00', stats: null }, - { author: 'b', permlink: 'p2', created: '2026-01-01T00:00:00', stats: null } + { author: 'b', permlink: 'p2', created: '2026-01-01T00:00:00', stats: null }, + { author: 'a', permlink: 'p1', created: '2026-01-02T00:00:00', stats: null } ] vi.mocked(CONFIG.hiveClient.call).mockResolvedValue(mockEntries) const options = getPostsRankedInfiniteQueryOptions('created', 'hive') const result = await options.queryFn({ @@ - expect(result).toHaveLength(2) - // Sorted by created desc - expect(result[0].author).toBe('a') + expect(result.map((e) => e.author)).toEqual(['a', 'b']) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk/src/modules/posts/queries/get-posts-ranked-query-options.spec.ts` around lines 72 - 91, The test currently uses mockEntries already in descending order so it doesn't prove sorting/filtering; modify the spec for getPostsRankedInfiniteQueryOptions by providing mockEntries in unsorted order and/or adding an entry that should be filtered out, then call options.queryFn (the existing call that uses CONFIG.hiveClient.call) and assert the returned array is sorted descending by created (e.g., check that result[0] has the newest created timestamp) and that any filtered entries are absent (e.g., assert result does not contain the filtered permlink/author); this ensures the test fails if getPostsRankedInfiniteQueryOptions' sorting or filtering regresses.packages/sdk/src/modules/posts/queries/get-account-posts-query-options.spec.ts (1)
24-107: Reduce duplicated infinite-query context setup in tests.The repeated
options.queryFn({...})payload makes this suite harder to maintain. Extract a small helper for the infinite query context.♻️ Suggested refactor
+const buildInfiniteQueryContext = (queryKey: unknown, pageParam: { author?: string; permlink?: string; hasNextPage: boolean }) => ({ + pageParam, + meta: undefined as any, + direction: "forward" as const, + queryKey, + signal: new AbortController().signal, +}); + it('should return [] when pageParam.hasNextPage is false', async () => { const options = getAccountPostsInfiniteQueryOptions('testuser') - const result = await options.queryFn({ - pageParam: { author: undefined, permlink: undefined, hasNextPage: false }, - meta: undefined as any, - direction: 'forward', - queryKey: options.queryKey, - signal: new AbortController().signal - }) + const result = await options.queryFn( + buildInfiniteQueryContext(options.queryKey, { author: undefined, permlink: undefined, hasNextPage: false }) + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk/src/modules/posts/queries/get-account-posts-query-options.spec.ts` around lines 24 - 107, Tests repeat the same infinite-query context payload for options.queryFn; extract a small helper to build that context to reduce duplication. Add a function (e.g., makeInfiniteQueryContext) used by all cases that returns an object with keys pageParam, meta, direction, queryKey: options.queryKey, and signal so each test calls options.queryFn(makeInfiniteQueryContext({ pageParam: {...} })) instead of inlining the whole payload; update references in tests that call getAccountPostsInfiniteQueryOptions and ensure mockGetAccountPosts usage remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/web/src/app/`(dynamicPages)/entry/[category]/[author]/[permlink]/_components/entry-not-found-fallback.tsx:
- Around line 41-42: The non-optimistic "deleted" fallback can fire after
handleSuccess flips transition state because current guards use isVerifying =
!isOptimistic && verifyPollCount < VERIFY_MAX_POLLS without checking
hasTransitioned; update all places that decide non-optimistic polling/fallback
(the isVerifying computation and the conditional checks around
verifyPollCount/VERIFY_MAX_POLLS at the other occurrences) to include
hasTransitioned (e.g., require !isOptimistic && hasTransitioned &&
verifyPollCount < VERIFY_MAX_POLLS) so the component stops treating the post as
deleted once a transition has occurred (affecting the logic that renders
DeletedPostScreen and any code paths that call router.refresh()).
In `@packages/sdk/src/modules/bridge/verify-on-alternate-node.ts`:
- Around line 41-49: The code currently returns any truthy response from
client.call("bridge","get_post") as an Entry; instead, validate the identity
before accepting it by checking that the returned object's author and permlink
match the requested author and permlink. In the verify-on-alternate-node flow
(look for the client.call("bridge", "get_post", { author, permlink, observer })
call and the response variable), ensure response is an object with
response.author === author and response.permlink === permlink (and optionally
has required Entry properties) before returning it as an Entry; if the check
fails, treat it as an invalid result (e.g., reject/skip/return null) so
malformed or mismatched payloads are not accepted.
---
Nitpick comments:
In
`@packages/sdk/src/modules/posts/queries/get-account-posts-query-options.spec.ts`:
- Around line 24-107: Tests repeat the same infinite-query context payload for
options.queryFn; extract a small helper to build that context to reduce
duplication. Add a function (e.g., makeInfiniteQueryContext) used by all cases
that returns an object with keys pageParam, meta, direction, queryKey:
options.queryKey, and signal so each test calls
options.queryFn(makeInfiniteQueryContext({ pageParam: {...} })) instead of
inlining the whole payload; update references in tests that call
getAccountPostsInfiniteQueryOptions and ensure mockGetAccountPosts usage remains
unchanged.
In
`@packages/sdk/src/modules/posts/queries/get-posts-ranked-query-options.spec.ts`:
- Around line 72-91: The test currently uses mockEntries already in descending
order so it doesn't prove sorting/filtering; modify the spec for
getPostsRankedInfiniteQueryOptions by providing mockEntries in unsorted order
and/or adding an entry that should be filtered out, then call options.queryFn
(the existing call that uses CONFIG.hiveClient.call) and assert the returned
array is sorted descending by created (e.g., check that result[0] has the newest
created timestamp) and that any filtered entries are absent (e.g., assert result
does not contain the filtered permlink/author); this ensures the test fails if
getPostsRankedInfiniteQueryOptions' sorting or filtering regresses.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4fae5d25-d64f-40d1-afc8-263bde91708a
⛔ Files ignored due to path filters (6)
packages/sdk/dist/browser/index.jsis excluded by!**/dist/**packages/sdk/dist/browser/index.js.mapis excluded by!**/dist/**,!**/*.mappackages/sdk/dist/node/index.cjsis excluded by!**/dist/**packages/sdk/dist/node/index.cjs.mapis excluded by!**/dist/**,!**/*.mappackages/sdk/dist/node/index.mjsis excluded by!**/dist/**packages/sdk/dist/node/index.mjs.mapis excluded by!**/dist/**,!**/*.map
📒 Files selected for processing (5)
apps/web/src/app/(dynamicPages)/entry/[category]/[author]/[permlink]/_components/entry-not-found-fallback.tsxpackages/sdk/src/modules/bridge/verify-on-alternate-node.spec.tspackages/sdk/src/modules/bridge/verify-on-alternate-node.tspackages/sdk/src/modules/posts/queries/get-account-posts-query-options.spec.tspackages/sdk/src/modules/posts/queries/get-posts-ranked-query-options.spec.ts
feruzm
added
the
patch
Merged
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 by CodeRabbit
New Features
Bug Fixes
Tests
Documentation
Chores