Page the per-tenant SEO feed at the bridge's own limit - #1464
Conversation
The bridge asserts limit into [1:20] and answers an error rather than a shorter list, so the single limit=100 call failed every tenant's SEO pass in production and no robots.txt, sitemap.xml or rss.xml was ever written. Walk pages of 20 with an exclusive cursor up to the same wanted depth, under one budget for the whole walk as well as the per-call timeout, and terminate on a short page, a repeated page or an unusable record.
Code Review by Qodo
1.
|
|
Warning Review limit reached
Next review available in: 13 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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 Plus Run ID: 📒 Files selected for processing (2)
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 |
PR Summary by QodoPage per-tenant SEO feed at bridge limit (20) with cursor + global budget
AI Description
Diagram
High-Level Assessment
Files changed (2)
|
![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: d11729a0cc
ℹ️ 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".
| while (posts.length < POST_LIMIT) { | ||
| const raw = await boundedCall<unknown>(method, { | ||
| ...feedParams, | ||
| limit: Math.min(BRIDGE_PAGE_LIMIT, POST_LIMIT - posts.length), |
There was a problem hiding this comment.
When a bridge node includes the cursor post in the next page, this limit counts that duplicate against the remaining post count. After collecting 96 unique posts, for example, the request for 4 returns the cursor plus 3 new posts; the short-page check then stops with only 99 posts. The repository's scan-post-corpus.mjs paginator explicitly handles this API behavior by requesting one extra slot on follow-up pages, so this walk should do the same while retaining the identity deduplication.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid, fixed in e0f97ee. Follow-up pages now ask for one extra slot (Math.min(20, cursor ? need + 1 : need)), the short-page check compares against what was ASKED rather than the constant, and the result is trimmed to the wanted depth since the reserved slot can overshoot by one on an exclusive node. The identity set still does the actual de-duplication, so both node behaviours are covered. Worth noting for the record: I checked api.hive.blog directly and BOTH feeds are exclusive today (page 2 starts after the cursor post), while scan-post-corpus.mjs documents the inclusive behaviour, so the reserved slot is the right defence against the variance rather than a fix for a known-inclusive node. A regression test drives an inclusive-cursor mock and asserts the walk still reaches exactly 100 unique posts; it returns 99 without the reserved slot.
| // A short page is the end of the feed; no new posts or no usable record | ||
| // means paging further cannot help. Either way, stop. | ||
| if (page.length < BRIDGE_PAGE_LIMIT || added === 0 || !last) break; | ||
| if (Date.now() >= deadline) break; |
There was a problem hiding this comment.
If several pages each complete just under the 10-second per-call timeout, this check can observe 29 seconds elapsed and start another full 10-second call, allowing the advertised 30-second fetch budget to take nearly 40 seconds. Because stale tenants are processed by a bounded worker pool, repeated overruns can substantially extend or skip SEO sync passes; pass the remaining overall budget into the next call (and its abort timer) rather than checking only after it finishes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid, fixed in e0f97ee. The remaining budget is now computed BEFORE each page and passed into the call as its timeout (Math.min(RPC_TIMEOUT_MS, remaining), which also drives the abort timer), and a page is not started at all when less than a second of budget is left. The walk can no longer exceed its advertised 30s. Covered by a test that spends 9s per page against a stubbed clock: four pages run, the fifth is never started, and the final call's timeout is the 3s that were left rather than the full 10s.
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
|
All four findings are addressed at 5fb63d3. Short-page check uses 20 (High, already marked resolved): the check now compares against what the page was ASKED for, not the constant, so a deliberately short final request is no longer read as end-of-feed. Budget checked after call (already marked resolved): the remaining budget is computed BEFORE each page and passed in as that call's timeout (which also drives its abort timer); a page is not started at all with under a second left. The walk can no longer run past its advertised 30s.
Two more regression tests came out of the Codex round on the same head: one drives an inclusive-cursor node and asserts the walk still reaches exactly 100 unique posts (it returns 99 without the reserved slot), the other spends 9s per page against a stubbed clock and asserts the fifth page is never started and the final call is bounded by the 3s left rather than the full 10s. 455 tests, typecheck clean, and re-verified against the live chain after each change: a blog and a community tenant each collect 100 unique posts, a 102-URL sitemap and a 100-item feed. |
1 participant
The per-tenant SEO files shipped in #1463 were never written in production:
bridge.get_account_postsandbridge.get_ranked_postsassertlimitinto [1:20] and answer an error rather than a shorter list, so the singlelimit=100call failed every tenant's pass.The feed is now walked in pages of 20 (the bridge's own page size) with an exclusive
start_author/start_permlinkcursor up to the same wanted depth of 100. The whole walk carries a single budget alongside the existing per-call timeout, so a slow chain costs a bounded pass rather than page count times the per-call timeout. The walk stops on a short page, on a page that adds nothing new (an inclusive-cursor node would otherwise repeat its last page forever) and on an unusable record.Verified against the live chain with the real function, not mocks: a blog tenant and a community tenant each collect 100 unique posts and produce a 102-URL sitemap and a 100-item feed. Tests pin the per-page limit ceiling, the cursor walk, the short-page stop and the repeated-page termination.