feat: add exact-locale Split guide foundation - #2657
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds a multilingual split-guide system with strict content and manifest validation, localized pages, metadata, locale switching, route ownership, document-language handling, and sitemap coverage. It also adds contract, verifier, routing, SEO, and content-check tests. ChangesSplit-guide publishing
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Visitor
participant SplitGuidePage
participant splitGuideData
participant MDXRenderer
Visitor->>SplitGuidePage: request localized guide path
SplitGuidePage->>splitGuideData: load published guide
splitGuideData-->>SplitGuidePage: return guide data
SplitGuidePage->>MDXRenderer: render MDX content
MDXRenderer-->>SplitGuidePage: return rendered content
SplitGuidePage-->>Visitor: return page, metadata, and locale links
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
Code-analysis diffPainscore total: 7157.18 → 7174.01 (+16.83) 🆕 New findings (24)
…and 4 more. ✅ Resolved (17)
📈 Painscore deltas (top movers)
|
🧪 UI test report — ✅ all greenSuites
📊 Coverage (unit)
⏱ 10 slowest test cases
|
![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 (5)
scripts/__tests__/verify-split-guides.test.ts (1)
122-135: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd one test that exercises the default MDX compiler.
runVerifieralways injects a no-opvalidateMdx. No test runscompileSplitGuideMdx. The production path performs dynamic ESM imports ofnext-mdx-remote/serializeandremark-gfmand wiresremarkRejectSplitGuideH1. A regression in that wiring, for example an import failure under the tsx entrypoint, would pass the whole suite.Add one case that omits
validateMdxand asserts that a body containing an H1 produces asplit-guide-mdxdiagnostic.🤖 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 `@scripts/__tests__/verify-split-guides.test.ts` around lines 122 - 135, Add a test case in the verifier tests that calls runVerifier without the validateMdx argument, exercising the default compileSplitGuideMdx path. Use content containing an H1 and assert that diagnostics include a split-guide-mdx check, while leaving the existing injected-validator tests unchanged.scripts/verify-content.ts (2)
729-731: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueExtract the split-guide slug derivation used in both passes.
Lines 200-202 and Lines 729-731 repeat the same
listDirsplushasPublishedLocalePage(..., 'en')filter, and the per-locale gate at Lines 310-314 and Lines 763-767 repeats too. A shared helper keeps the route pass and the sitemap pass from drifting.♻️ Proposed helper
+function publishedSplitGuideSlugs(): string[] { + return listDirs(path.join(CONTENT_DIR, 'split-guides')).filter((slug) => + hasPublishedLocalePage('split-guides', slug, 'en') + ) +}Then use it in both passes:
- const splitGuideSlugs = listDirs(path.join(CONTENT_DIR, 'split-guides')).filter((slug) => - hasPublishedLocalePage('split-guides', slug, 'en') - ) + const splitGuideSlugs = publishedSplitGuideSlugs()🤖 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 `@scripts/verify-content.ts` around lines 729 - 731, Extract the repeated split-guide slug derivation into a shared helper that lists split-guide directories and filters them through hasPublishedLocalePage('split-guides', slug, 'en'). Replace the duplicate logic in both the route pass and sitemap pass, and reuse the shared per-locale gate in each corresponding flow so both passes remain consistent.
307-315: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winKeep split-guide locale checks aligned.
The verifier and runtime use
['en', 'es-419', 'pt-br'], whilescripts/verify-content.tsstill derives split-guide routes from the broaderSUPPORTED_LOCALESlist that includeses-ar. Addes-arto the split-guide locale sources or derive this loop fromSPLIT_GUIDE_LOCALESso future locale additions do not accidentally manufacture supported split-guide routes without exact published files.🤖 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 `@scripts/verify-content.ts` around lines 307 - 315, Update the split-guide route generation in the hasRoute('split/guides/[slug]') block to iterate over the exact split-guide locale source, preferably SPLIT_GUIDE_LOCALES, instead of the broader SUPPORTED_LOCALES-derived values. Keep route creation gated by hasPublishedLocalePage so only locales with exact published files produce routes, and include es-ar through the shared split-guide locale definition.src/components/Marketing/__tests__/locale-switcher.test.ts (1)
52-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the two boundary cases for the guide index and deeper paths.
hasRouteScopedLocaleSwitcherrequiressegments.length > 3. The suite does not pin the guide index path or a deeper path./en/split/guidesmust return false, and/en/split/guides/a/bcurrently returns true. Pin both so a later change to the segment check fails loudly.♻️ Proposed additions
expect(hasRouteScopedLocaleSwitcher('/en/split')).toBe(false) + expect(hasRouteScopedLocaleSwitcher('/en/split/guides')).toBe(false) expect(hasRouteScopedLocaleSwitcher('/split/guides/group-trip')).toBe(false) expect(hasRouteScopedLocaleSwitcher('/en/blog/group-trip')).toBe(false)🤖 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 `@src/components/Marketing/__tests__/locale-switcher.test.ts` around lines 52 - 58, Add boundary assertions to the hasRouteScopedLocaleSwitcher test for /en/split/guides returning false and /en/split/guides/a/b returning true, preserving the existing Split guide route cases and explicitly pinning the segments.length > 3 behavior.scripts/lib/split-guide-contract.ts (1)
33-40: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider narrowing the setext scan to skip fenced code blocks.
Line 36 flags any nonblank line that is followed by a
=+line. A fenced code sample that contains such a pair produces a blockingsetext-h1diagnostic, even though remark parses it as code and the authoritative plugin accepts it. The same applies to<h1>inside a fenced block on Line 37.If fenced samples are expected in guide bodies, strip fenced regions before the scan.
♻️ Proposed refactor to skip fenced regions
export function findSplitGuideHeadingCollisions(body: string): SplitGuideHeadingCollision[] { const collisions: SplitGuideHeadingCollision[] = [] - if (/^[ \t]*#(?:[ \t]+|$)/m.test(body)) collisions.push('atx-h1') - if (/^(?![ \t]*$).+\r?\n[ \t]*=+[ \t]*$/m.test(body)) collisions.push('setext-h1') - if (/<h1(?:\s|>)/i.test(body)) collisions.push('html-h1') - if (/<Hero\b/.test(body)) collisions.push('hero') + // Fenced samples are code to remark; keep the cheap scan aligned with it. + const scannable = body.replace(/^[ \t]*(`{3,}|~{3,})[\s\S]*?^[ \t]*\1[ \t]*$/gm, '') + if (/^[ \t]*#(?:[ \t]+|$)/m.test(scannable)) collisions.push('atx-h1') + if (/^(?![ \t]*$).+\r?\n[ \t]*=+[ \t]*$/m.test(scannable)) collisions.push('setext-h1') + if (/<h1(?:\s|>)/i.test(scannable)) collisions.push('html-h1') + if (/<Hero\b/.test(scannable)) collisions.push('hero') return collisions }🤖 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 `@scripts/lib/split-guide-contract.ts` around lines 33 - 40, Update findSplitGuideHeadingCollisions to exclude fenced code-block regions before running the setext-h1 and html-h1 scans, while preserving detection of headings in normal guide content and the existing atx-h1/hero checks as appropriate.
🤖 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 `@scripts/lib/verify-split-guides.ts`:
- Around line 457-472: Update the title-length validation in the split-guide
contract check to use the effective authored-title budget of 51 characters,
remove the unreachable rawLength > 60 condition, and revise the error message to
state the 51-character limit clearly while preserving the existing raw/effective
length details.
- Around line 699-712: Update compileSplitGuideMdx to remove the unsupported
format: 'mdx' property from mdxOptions, leaving remarkPlugins unchanged.
---
Nitpick comments:
In `@scripts/__tests__/verify-split-guides.test.ts`:
- Around line 122-135: Add a test case in the verifier tests that calls
runVerifier without the validateMdx argument, exercising the default
compileSplitGuideMdx path. Use content containing an H1 and assert that
diagnostics include a split-guide-mdx check, while leaving the existing
injected-validator tests unchanged.
In `@scripts/lib/split-guide-contract.ts`:
- Around line 33-40: Update findSplitGuideHeadingCollisions to exclude fenced
code-block regions before running the setext-h1 and html-h1 scans, while
preserving detection of headings in normal guide content and the existing
atx-h1/hero checks as appropriate.
In `@scripts/verify-content.ts`:
- Around line 729-731: Extract the repeated split-guide slug derivation into a
shared helper that lists split-guide directories and filters them through
hasPublishedLocalePage('split-guides', slug, 'en'). Replace the duplicate logic
in both the route pass and sitemap pass, and reuse the shared per-locale gate in
each corresponding flow so both passes remain consistent.
- Around line 307-315: Update the split-guide route generation in the
hasRoute('split/guides/[slug]') block to iterate over the exact split-guide
locale source, preferably SPLIT_GUIDE_LOCALES, instead of the broader
SUPPORTED_LOCALES-derived values. Keep route creation gated by
hasPublishedLocalePage so only locales with exact published files produce
routes, and include es-ar through the shared split-guide locale definition.
In `@src/components/Marketing/__tests__/locale-switcher.test.ts`:
- Around line 52-58: Add boundary assertions to the hasRouteScopedLocaleSwitcher
test for /en/split/guides returning false and /en/split/guides/a/b returning
true, preserving the existing Split guide route cases and explicitly pinning the
segments.length > 3 behavior.
🪄 Autofix
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 Plus
Run ID: 848e7507-985b-4110-a04c-430b955c51e3
📒 Files selected for processing (17)
scripts/__tests__/split-guide-contract.test.tsscripts/__tests__/verify-split-guides.test.tsscripts/lib/split-guide-contract.tsscripts/lib/verify-split-guides.tsscripts/verify-content.tssrc/app/[locale]/(marketing)/split/[...path]/page.tsxsrc/app/[locale]/(marketing)/split/guides/[slug]/page.tsxsrc/app/[locale]/(marketing)/split/page.tsxsrc/app/__tests__/split-guides-route.test.tsxsrc/app/sitemap.tssrc/components/Marketing/LocaleSwitcher.tsxsrc/components/Marketing/__tests__/locale-switcher.test.tssrc/constants/__tests__/routes.test.tssrc/constants/routes.tssrc/i18n/config.tssrc/lib/__tests__/split-guides.test.tssrc/lib/split-guides.ts
|
CodeRabbit summary nitpick disposition (0e58e80):
Additional runtime/release hardening: removed empty generateStaticParams from the Split namespace catch-all (the Next production soft-404 trigger), verified real GET 404 statuses across missing locales/slugs/subtrees, and added required raw-source SHA-256 provenance plus optional source-root recomputation/frontmatter comparison for content/release audits. Mirrored UI CI remains source-checkout-independent. |
|
@coderabbitai review |
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/__tests__/verify-split-guides.test.ts (1)
181-202: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the subprocess collector parse only the verifier payload, with a clear timeout failure.
completed.stdoutis consumed directly inscripts/__tests__/verify-split-guides.test.ts:201, so any transitive stdout after a successful exit would breakJSON.parse. The MDX compile path also has no child timeout inscripts/__tests__/verify-split-guides.test.ts:194; add a boundedtimeouttospawnSyncand an explicit error when it times out.🤖 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 `@scripts/__tests__/verify-split-guides.test.ts` around lines 181 - 202, Update runDefaultCompilerVerifier to give spawnSync a bounded timeout and explicitly throw a timeout-specific error when the subprocess expires. Parse only the JSON verifier payload from completed.stdout, tolerating unrelated trailing stdout while preserving the existing diagnostics/result return shape; continue reporting nonzero subprocess failures with stderr/stdout context.
🤖 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 `@scripts/__tests__/verify-split-guides.test.ts`:
- Around line 181-202: Update runDefaultCompilerVerifier to give spawnSync a
bounded timeout and explicitly throw a timeout-specific error when the
subprocess expires. Parse only the JSON verifier payload from completed.stdout,
tolerating unrelated trailing stdout while preserving the existing
diagnostics/result return shape; continue reporting nonzero subprocess failures
with stderr/stdout context.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 997ffc55-a274-40b3-be65-9ba33c00f647
📒 Files selected for processing (14)
scripts/__tests__/split-guide-contract.test.tsscripts/__tests__/verify-split-guides.test.tsscripts/lib/split-guide-contract.tsscripts/lib/verify-split-guides.tsscripts/verify-content.tssrc/app/[locale]/(marketing)/layout.tsxsrc/app/[locale]/(marketing)/split/[...path]/page.tsxsrc/app/[locale]/(marketing)/split/guides/[slug]/page.tsxsrc/app/__tests__/split-guides-route.test.tsxsrc/components/Marketing/__tests__/html-lang.test.tsxsrc/components/Marketing/__tests__/locale-switcher.test.tssrc/components/Marketing/mdx/CTA.tsxsrc/components/Marketing/mdx/__tests__/CTA.test.tsxsrc/i18n/app/AppIntlProvider.tsx
🚧 Files skipped from review as they are similar to previous changes (4)
- src/components/Marketing/tests/locale-switcher.test.ts
- src/app/tests/split-guides-route.test.tsx
- src/app/[locale]/(marketing)/split/guides/[slug]/page.tsx
- scripts/verify-content.ts
|
CodeRabbit final nitpick fixed in |
|
@coderabbitai review |
|
|
Architecture update (2026-08-11): this native-renderer implementation is superseded and must not be merged as written. The locked seam is now:
Durable contract and recovery map: https://github.com/peanutprotocol/mono/pull/75 Keeping this PR draft temporarily so genuinely generic i18n fixes can be evaluated separately. The Split route/loader/verifier implementation here should not be carried forward. |
1 participant
Summary
/{locale}/split/guides/{slug}family foren,es-419, andpt-br.BlogPostingschema contract.Task
Intentional behavior and risk
/splitpaths return 404 instead of falling through to the generic recipient route.es-arandes-esfrom this canary.Verification
Screenshots
The orphan
pr-assets-2657branch is evidence-only and should be deleted after merge.Design notes
main[lang]plus a supported locale marker. The existing platform root still emits<html lang="en">in raw HTML; hydration promotes the verified marketing locale to the document root. Browser QA observed the localized value within 100–250ms and gates on the hydrated state.BlogPostingonly; the route emits that object and a separate breadcrumb. A redundant standaloneArticleobject is rejected.Rollout
main, rerun source/provenance gates, and publish the six exact-locale pages through the content pipeline.Docs and follow-up