feat: SSR redirects for index pages and folder navigation#69
Merged
Conversation
- Move redirect logic to entry-server.tsx for SSR 307 redirects - Shared tree-utils.ts used by both SSR and client-side DocsPage - /apis → first API endpoint - /docs (no index) → first page or index_page from config - /docs/folder → first child page (derived from child URLs) - Add http-status-codes for readable status constants - 16 tests for tree-utils (getFirstPageUrl, findFolderFirstPage, resolveDocsRedirect) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds tree traversal utilities and tests to compute docs redirect targets, uses them in DocsPage 404 handling, and performs early SSR redirects for unresolved docs and API index routes; also adds an http-status-codes dependency. ChangesDocs and API Index Redirect Refactor
Sequence DiagramsequenceDiagram
participant Client
participant EntryServer as entry-server.fetch
participant getFirstApiUrl as getFirstApiUrl(apiSpecs)
participant resolveDocsRedirect as resolveDocsRedirect(slug, tree, contentConfig)
Client->>EntryServer: request (route)
EntryServer->>EntryServer: detect RouteType
EntryServer->>getFirstApiUrl: if ApiIndex -> getFirstApiUrl(apiSpecs)
getFirstApiUrl-->>EntryServer: apiUrl?
EntryServer->>Client: HTTP 307 Location: apiUrl (if present)
EntryServer->>resolveDocsRedirect: if DocsPage and no page -> compute redirect
resolveDocsRedirect-->>EntryServer: redirectUrl?
EntryServer->>Client: HTTP 307 Location: versionPrefix+redirectUrl (if present)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/chronicle/src/lib/tree-utils.test.ts (2)
85-113: ⚡ Quick winAdd a regression test for
index_pageon non-root slugs.Please add a case asserting that
index_pagedoes not override folder/non-root redirects (or 404 paths).💡 Proposed test case
describe('resolveDocsRedirect', () => { @@ test('index_page takes priority over first page', () => { expect(resolveDocsRedirect(['docs'], tree, { dir: 'docs', index_page: 'custom' })) .toBe('/docs/custom') }) + + test('does not apply index_page for nested slugs', () => { + expect(resolveDocsRedirect(['docs', 'guides'], tree, { dir: 'docs', index_page: 'custom' })) + .toBe('/docs/guides/install') + }) })🤖 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 `@packages/chronicle/src/lib/tree-utils.test.ts` around lines 85 - 113, Add a regression test in tree-utils.test.ts for resolveDocsRedirect to ensure index_page only applies to the content root: call resolveDocsRedirect with a non-root slug (e.g., ['docs', 'guides']) and content config that contains dir: 'docs' plus index_page: 'custom' and assert it still redirects to the folder's first child (e.g., '/docs/guides/install') rather than '/docs/custom'; use the existing test pattern and the same tree fixture so the test name and expectations match the other cases.
3-3: ⚡ Quick winUse project path alias instead of relative import.
Switch this test import to
@/lib/tree-utilsfor consistency with the repo alias policy.💡 Proposed fix
-import { getFirstPageUrl, findFolderFirstPage, resolveDocsRedirect } from './tree-utils' +import { getFirstPageUrl, findFolderFirstPage, resolveDocsRedirect } from '@/lib/tree-utils'As per coding guidelines
**/*.{ts,tsx}: Use path alias@/*→./src/*configured in tsconfig and vite.🤖 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 `@packages/chronicle/src/lib/tree-utils.test.ts` at line 3, Update the test import to use the project path alias: replace the relative import of the module used by tests (import { getFirstPageUrl, findFolderFirstPage, resolveDocsRedirect } from './tree-utils') with the aliased path import from '@/lib/tree-utils' so the tests follow the repo's tsconfig/vite alias policy and resolve the same exported symbols (getFirstPageUrl, findFolderFirstPage, resolveDocsRedirect).packages/chronicle/src/server/entry-server.tsx (1)
47-57: ⚡ Quick winMove API index redirect before page-tree fetch.
For
RouteType.ApiIndex, lines [47-50] still load docs tree/page before returning the 307. Reordering this keeps the redirect path truly early-exit and reduces SSR work.💡 Suggested structure
- const [tree, page] = await Promise.all([ - getPageTree(), - route.type === RouteType.DocsPage ? getPage(route.slug) : Promise.resolve(null), - ]); - // SSR redirects for index pages if (route.type === RouteType.ApiIndex) { const firstUrl = getFirstApiUrl(apiSpecs); if (firstUrl) { return new Response(null, { status: StatusCodes.TEMPORARY_REDIRECT, headers: { Location: firstUrl } }); } } + + const [tree, page] = await Promise.all([ + getPageTree(), + route.type === RouteType.DocsPage ? getPage(route.slug) : Promise.resolve(null), + ]);🤖 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 `@packages/chronicle/src/server/entry-server.tsx` around lines 47 - 57, The API index redirect runs after expensive page-tree/page fetches; move the RouteType.ApiIndex early-exit before calling getPageTree() and getPage() so SSR work is avoided when redirecting. Specifically, check if route.type === RouteType.ApiIndex, call getFirstApiUrl(apiSpecs) and return the TEMPORARY_REDIRECT response immediately if a URL exists, and only then perform the Promise.all([getPageTree(), ...]) calls for non-ApiIndex routes (functions: getPageTree, getPage, getFirstApiUrl; symbol: RouteType.ApiIndex).
🤖 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 `@packages/chronicle/src/lib/tree-utils.ts`:
- Around line 43-45: The current code always redirects missing doc slugs to
contentConfig?.index_page; change it so the redirect to
`/${contentConfig.dir}/${contentConfig.index_page}` only happens when the
requested slug is the content root (e.g., empty or "/") rather than any
nonexistent slug. Update the conditional around contentConfig?.index_page to
also check the incoming slug parameter (or request path) is empty/root (not a
non-empty missing slug) so that non-root missing slugs fall through to 404; keep
references to contentConfig and index_page when locating the change in
tree-utils.ts.
---
Nitpick comments:
In `@packages/chronicle/src/lib/tree-utils.test.ts`:
- Around line 85-113: Add a regression test in tree-utils.test.ts for
resolveDocsRedirect to ensure index_page only applies to the content root: call
resolveDocsRedirect with a non-root slug (e.g., ['docs', 'guides']) and content
config that contains dir: 'docs' plus index_page: 'custom' and assert it still
redirects to the folder's first child (e.g., '/docs/guides/install') rather than
'/docs/custom'; use the existing test pattern and the same tree fixture so the
test name and expectations match the other cases.
- Line 3: Update the test import to use the project path alias: replace the
relative import of the module used by tests (import { getFirstPageUrl,
findFolderFirstPage, resolveDocsRedirect } from './tree-utils') with the aliased
path import from '@/lib/tree-utils' so the tests follow the repo's tsconfig/vite
alias policy and resolve the same exported symbols (getFirstPageUrl,
findFolderFirstPage, resolveDocsRedirect).
In `@packages/chronicle/src/server/entry-server.tsx`:
- Around line 47-57: The API index redirect runs after expensive page-tree/page
fetches; move the RouteType.ApiIndex early-exit before calling getPageTree() and
getPage() so SSR work is avoided when redirecting. Specifically, check if
route.type === RouteType.ApiIndex, call getFirstApiUrl(apiSpecs) and return the
TEMPORARY_REDIRECT response immediately if a URL exists, and only then perform
the Promise.all([getPageTree(), ...]) calls for non-ApiIndex routes (functions:
getPageTree, getPage, getFirstApiUrl; symbol: RouteType.ApiIndex).
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a4e191d1-de0f-4901-9202-59730ee4a07d
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
packages/chronicle/package.jsonpackages/chronicle/src/lib/tree-utils.test.tspackages/chronicle/src/lib/tree-utils.tspackages/chronicle/src/pages/DocsPage.tsxpackages/chronicle/src/server/entry-server.tsx
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
rohanchkrabrty
previously requested changes
May 14, 2026
Strip version prefix from slug before resolving redirect, use version-specific content config, prepend version prefix to redirect URL. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
rohilsurana
approved these changes
May 14, 2026
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
3 participants
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
tree-utils.tswithgetFirstPageUrl,findFolderFirstPage,resolveDocsRedirectentry-server.tsx(SSR) andDocsPage.tsx(client)http-status-codesfor readable status constantsRedirects
/apis/docs(no index)index_pageor first page/docs/guides(folder)/docs/nonexistentTests
16 tests covering all redirect scenarios.
Test plan
curl -I /apis→ 307curl -I /docs/guides→ 307 to first child/docs/nonexistent→ 404bun test src/lib/tree-utils.test.ts→ 16 pass🤖 Generated with Claude Code