feat: add llms and sitemap support #40
Conversation
🦋 Changeset detectedLatest commit: a412d31 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Warning Rate limit exceeded@JounQin has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 14 minutes and 39 seconds before requesting another review. ⌛ 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. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (14)
WalkthroughThis update introduces support for sitemap generation and large language models (LLMs). It adds related CLI options, configuration fields, and a new sitemap plugin. Documentation is updated in both English and Chinese to reflect these changes, and dependencies are adjusted to include the LLM plugin and modify existing package versions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant ConfigLoader
participant Plugins
participant Builder
User->>CLI: Run build with -S/--site-url flag
CLI->>ConfigLoader: Load config (with siteUrl flag)
ConfigLoader->>Plugins: Enable sitemapPlugin (if siteUrl)
Plugins->>Builder: Register sitemapPlugin
Builder->>Plugins: Build process triggers sitemapPlugin
Plugins->>Builder: Generate sitemap.xml in output
Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This pull request introduces support for LLMS integration and sitemap generation. Key changes include adding a new sitemap plugin, integrating LLMS and sitemap options into the configuration and CLI, and updating the documentation and workflows accordingly.
Reviewed Changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types.ts | Added new "siteUrl" property to types with differing types. |
| src/plugins/sitemap/index.ts | Added new sitemap plugin for generating sitemap.xml. |
| src/plugins/index.ts | Exported the new sitemap plugin. |
| src/cli/translate.ts | Updated the temperature setting for the gpt-4o-mini model. |
| src/cli/load-config.ts | Integrated LLMS and sitemap plugin options into configuration logic. |
| src/cli/index.ts | Added CLI flag for enabling sitemap generation. |
| package.json | Added dependency for LLMS plugin and downgraded the commander version. |
| doom.config.yml | Added siteUrl configuration for sitemap generation. |
| docs/zh & docs/en usage/configuration.md | Updated documentation with new sitemap configuration details. |
| .github/workflows/gh-pages.yml | Updated docs build command to enable sitemap generation. |
Comments suppressed due to low confidence (2)
package.json:74
- The downgrade of commander from ^14.0.0 to ^13.1.0 may affect compatibility or available features; please confirm that this version change is intentional and aligns with the rest of the project dependencies.
"commander": "^13.1.0"
src/cli/translate.ts:170
- [nitpick] The temperature value was increased from 0.1 to 0.2; verify that this change is intentional and update relevant documentation or tests if necessary to reflect the revised behavior.
temperature: 0.2,
commit: |
JounQin
force-pushed
the
feat/llms_sitemap
branch
from
876bc9e to
eb996fa
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 3
♻️ Duplicate comments (2)
src/types.ts (1)
34-34: 🛠️ Refactor suggestionType mismatch between CLI flag and configuration value.
The
siteUrlproperty has different types in different contexts (boolean in CLI options, string in config). This could lead to confusion about the property's purpose and usage.Consider renaming one of these properties to clearly differentiate their purposes:
- siteUrl?: boolean + enableSitemap?: booleanOr alternatively, rename the config property:
- siteUrl?: string + siteMapUrl?: stringsrc/cli/load-config.ts (1)
121-121: Consider renaming the CLI flag to avoid confusion.The CLI flag
siteUrlis used as a boolean to enable/disable sitemap generation, but the name suggests it should be a URL value. The actual URL comes fromconfig.siteUrl. Consider renaming the CLI flag to something likeenableSitemaporsitemapfor clarity.Also applies to: 139-139, 366-366, 447-447
🧹 Nitpick comments (1)
src/plugins/sitemap/index.ts (1)
57-63: Consider using a proper logger instead of console.log.The XML generation logic is correct, but consider using a proper logging mechanism instead of
console.logfor better control over log levels and formatting.Example improvement:
- console.log(`Generate sitemap.xml for ${sitemaps.length} pages.`) + // Use config.logger if available, or a structured logging approach
🛑 Comments failed to post (3)
src/plugins/sitemap/index.ts (3)
65-76: 🛠️ Refactor suggestion
Add input validation for critical options.
Consider validating the
domainoption to ensure it's a valid URL format, especially since it's critical for sitemap generation.Example validation:
export function sitemapPlugin(options?: Options): RspressPlugin { + if (options?.domain && !options.domain.match(/^https?:\/\/.+/)) { + throw new Error('Invalid domain format. Domain must be a valid URL starting with http:// or https://') + } + options = { domain: 'https://rspress.dev', customMaps: {},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.export function sitemapPlugin(options?: Options): RspressPlugin { if (options?.domain && !options.domain.match(/^https?:\/\/.+/)) { throw new Error( 'Invalid domain format. Domain must be a valid URL starting with http:// or https://' ) } options = { domain: 'https://rspress.dev', customMaps: {}, defaultChangeFreq: 'monthly', defaultPriority: '0.5', ...options, } const sitemaps: Sitemap[] = [] const set = new Set() return { name: 'rspress-plugin-sitemap', // …🤖 Prompt for AI Agents
In src/plugins/sitemap/index.ts around lines 65 to 76, the domain option is used for sitemap generation but lacks validation. Add input validation to check if the domain string is a valid URL format before proceeding. You can use a try-catch block with the URL constructor or a regex to validate the domain, and throw an error or log a warning if the validation fails to prevent invalid sitemap generation.
77-98:
⚠️ Potential issueAdd error handling for file operations.
Multiple potential runtime errors due to missing error handling:
fs.statSync(pageData._filepath)can throw if the file doesn't exist or_filepathis undefined- File write operations should have error handling
- The hard-coded fallback
'doc_build'should match the actual default output directoryApply this diff to improve error handling:
extendPageData(pageData, isProd) { if (!isProd || set.has(pageData.id)) { return } set.add(pageData.id) + + let lastmod: string | undefined + try { + if (pageData._filepath) { + lastmod = fs.statSync(pageData._filepath).mtime.toISOString() + } + } catch (error) { + // File might not exist or be accessible, skip lastmod + } + sitemaps.push({ loc: `${options.domain}${pageData.routePath}`, - lastmod: fs.statSync(pageData._filepath).mtime.toISOString(), + lastmod, priority: pageData.routePath === '/' ? '1.0' : options.defaultPriority, changefreq: options.defaultChangeFreq, ...options.customMaps?.[pageData.routePath], }) }, afterBuild(config, isProd) { if (!isProd) { return } - const outputPath = `${config.outDir || 'doc_build'}/sitemap.xml` - fs.mkdirSync(dirname(outputPath), { recursive: true }) - fs.writeFileSync(outputPath, generateXml(sitemaps)) + try { + const outputPath = `${config.outDir || 'doc_build'}/sitemap.xml` + fs.mkdirSync(dirname(outputPath), { recursive: true }) + fs.writeFileSync(outputPath, generateXml(sitemaps)) + } catch (error) { + console.error('Failed to generate sitemap:', error) + throw error + } },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.extendPageData(pageData, isProd) { if (!isProd || set.has(pageData.id)) { return } set.add(pageData.id) let lastmod: string | undefined try { if (pageData._filepath) { lastmod = fs.statSync(pageData._filepath).mtime.toISOString() } } catch (error) { // File might not exist or be accessible, skip lastmod } sitemaps.push({ loc: `${options.domain}${pageData.routePath}`, lastmod, priority: pageData.routePath === '/' ? '1.0' : options.defaultPriority, changefreq: options.defaultChangeFreq, ...options.customMaps?.[pageData.routePath], }) }, afterBuild(config, isProd) { if (!isProd) { return } try { const outputPath = `${config.outDir || 'doc_build'}/sitemap.xml` fs.mkdirSync(dirname(outputPath), { recursive: true }) fs.writeFileSync(outputPath, generateXml(sitemaps)) } catch (error) { console.error('Failed to generate sitemap:', error) throw error } },🤖 Prompt for AI Agents
In src/plugins/sitemap/index.ts around lines 77 to 98, add try-catch blocks around fs.statSync and fs.writeFileSync calls to handle potential runtime errors from missing files or write failures. Also, verify that the fallback directory 'doc_build' matches the actual default output directory used elsewhere in the project and update it if necessary. This will prevent crashes and improve robustness during file operations.
48-55:
⚠️ Potential issueFix type safety issue causing pipeline failures.
The
Object.entries(sitemap)returns[string, unknown][], but the code assumesvalueis always a string. This could cause runtime errors if a value isundefinedor not a string.Apply this diff to add proper type guards:
const generateNode = (sitemap: Sitemap): string => { let result = '<url>' for (const [tag, value] of Object.entries(sitemap)) { - result += `<${tag}>${value}</${tag}>` + if (value !== undefined && value !== null) { + result += `<${tag}>${String(value)}</${tag}>` + } } result += '</url>' return result }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.const generateNode = (sitemap: Sitemap): string => { let result = '<url>' for (const [tag, value] of Object.entries(sitemap)) { if (value !== undefined && value !== null) { result += `<${tag}>${String(value)}</${tag}>` } } result += '</url>' return result }🧰 Tools
🪛 GitHub Actions: CI
[error] 50-50: Type coverage issue detected at line 50.
[error] 51-51: Type coverage issue detected at line 51.
🤖 Prompt for AI Agents
In src/plugins/sitemap/index.ts around lines 48 to 55, the function generateNode assumes all values from Object.entries(sitemap) are strings, which can cause runtime errors if values are undefined or not strings. To fix this, add a type guard inside the loop to check if the value is a string before appending it to the result. Skip or handle non-string values appropriately to ensure type safety and prevent pipeline failures.
JounQin
force-pushed
the
feat/llms_sitemap
branch
from
eb996fa to
c79331c
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (2)
src/types.ts (1)
34-34: Consider renaming the CLI flag to avoid confusion between enabling the feature and specifying the URL.The 'siteUrl' property is defined as a boolean in GlobalCliOptions but as a string in the shared module; consider aligning the types or renaming one of them to clearly differentiate between the flag and the actual URL value.
src/cli/load-config.ts (1)
121-121: The siteUrl parameter naming could be clearer.The 'siteUrl' parameter here is used as a boolean flag, while later it is used to provide the actual URL from config.siteUrl; consider renaming the CLI flag (e.g., 'enableSitemap') to avoid confusion between enabling the feature and specifying the URL.
Also applies to: 139-139
🧹 Nitpick comments (2)
src/cli/translate.ts (1)
170-170: Increased translation temperature
The temperature parameter was raised from0.1to0.2, making translations more creative. Consider exposing this as a configurable option via CLI or config to allow users to fine-tune translation randomness.src/plugins/sitemap/index.ts (1)
64-71: Consider making domain a required option.The hardcoded default domain 'https://rspress.dev' may not be appropriate for all users. Consider either making the domain required or using a more generic approach.
export function sitemapPlugin(options?: Options): RspressPlugin { + if (!options?.domain) { + throw new Error('Domain is required for sitemap generation') + } options = { - domain: 'https://rspress.dev', customMaps: {}, defaultChangeFreq: 'monthly', defaultPriority: '0.5', ...options, }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (15)
.changeset/smart-rivers-smash.md(1 hunks).github/workflows/gh-pages.yml(1 hunks)docs/en/start.mdx(1 hunks)docs/en/usage/configuration.md(3 hunks)docs/public/robots.txt(0 hunks)docs/zh/start.mdx(1 hunks)docs/zh/usage/configuration.md(1 hunks)doom.config.yml(1 hunks)package.json(2 hunks)src/cli/index.ts(1 hunks)src/cli/load-config.ts(10 hunks)src/cli/translate.ts(1 hunks)src/plugins/index.ts(1 hunks)src/plugins/sitemap/index.ts(1 hunks)src/types.ts(2 hunks)
💤 Files with no reviewable changes (1)
- docs/public/robots.txt
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/cli/index.ts (1)
src/cli/helpers.ts (1)
parseBoolean(10-11)
🪛 ESLint
src/cli/load-config.ts
[error] 15-15: Unable to resolve path to module '@rspress/plugin-algolia'.
(import-x/no-unresolved)
[error] 16-16: Unable to resolve path to module '@rspress/plugin-llms'.
(import-x/no-unresolved)
src/plugins/sitemap/index.ts
[error] 7-7: Unable to resolve path to module '@rspress/shared/logger'.
(import-x/no-unresolved)
🔇 Additional comments (22)
docs/en/start.mdx (1)
2-2: Metadata update is okay
ThesourceSHAvalue update aligns metadata across documentation. No functional impact.doom.config.yml (1)
38-38: AddsiteUrlfor sitemap generation
The newsiteUrlconfiguration key at the root level is required for the sitemap plugin to generate URLs correctly. Ensure that this key is referenced in documentation and loaded properly by the configuration loader..changeset/smart-rivers-smash.md (1)
1-6: Changeset for llms and sitemap feature
The new changeset correctly records a minor version bump for the feature addition. It will trigger a feature release of@alauda/doom.package.json (3)
69-69: Add@rspress/plugin-llmsdependency
Introducing the LLMS plugin dependency aligns with the new LLM integration feature. Ensure the version matches other@rspresspackages on2.0.0-beta.8.
74-74: Verify compatibility ofcommanderdowngrade
The CLI framework is downgraded from^14.0.0to^13.1.0to support the new boolean option-S, --site-url. Please verify that commander v13 handles boolean options and defaults as intended, and that no features in the CLI code require v14+.
116-116: Update@types/nodedev dependency
Bumping@types/nodeto^22.15.24is a minor maintenance update. No issues detected..github/workflows/gh-pages.yml (1)
40-40: LGTM! Proper integration of sitemap feature in CI/CD pipeline.The addition of the
-Sflag correctly enables the sitemap generation feature during documentation builds, which aligns with the new CLI option introduced in this PR.src/cli/index.ts (1)
113-118: LGTM! Well-implemented CLI option following established patterns.The new
-S, --site-urloption is properly implemented:
- Follows the same pattern as other boolean CLI options
- Uses the
parseBooleanhelper function consistently- Has clear description and appropriate default value
- Integrates seamlessly with the existing CLI structure
docs/zh/start.mdx (1)
101-101: LGTM! Accurate documentation of the new CLI option.The documentation correctly reflects the new
-S, --site-urlCLI option with proper description and default value, maintaining consistency with the existing documentation format.docs/zh/usage/configuration.md (1)
231-236: LGTM! Clear clarification of Algolia configuration usage.The update properly clarifies that Algolia configuration is optional and only takes effect when the
-a, --algoliaCLI flag is enabled, improving user understanding.src/plugins/index.ts (1)
3-3: LGTM! Clean plugin exports following established patterns.The new exports for
auto-tocandsitemapplugins follow the existing convention and properly extend the central plugin index.Also applies to: 10-10
docs/en/usage/configuration.md (3)
4-4: LGTM! Routine metadata update.The sourceSHA update reflects the latest changes in the documentation.
213-213: LGTM! Minor formatting improvement.Removing the trailing period improves consistency in the comment formatting.
237-242: LGTM! Clear documentation for the new sitemap feature.The sitemap configuration section is well-documented with a clear example and proper reference to the CLI flag. The documentation accurately describes that it requires the
-s, --sitemapflag to be effective.src/types.ts (1)
68-68: LGTM! Correct type definition for the configuration value.The string type for
siteUrlin UserConfig is appropriate as it represents the actual URL value used by the sitemap plugin.src/cli/load-config.ts (5)
271-276: LGTM! Proper conditional plugin integration.The Algolia and LLMs plugins are correctly integrated with appropriate conditional logic. The Algolia plugin includes a verification content and the LLMs plugin is added unconditionally as expected.
293-298: LGTM! Clean sitemap plugin integration with proper filtering.The sitemap plugin is correctly added conditionally when both the CLI flag and config URL are present. The
filter(Boolean)call appropriately removes any falsy plugins from the array.
299-299: LGTM! Proper search configuration based on Algolia usage.The search is correctly disabled when Algolia is enabled to avoid conflicts between different search implementations.
308-310: Clarify the purpose of disabling cleanDistPath.Setting
cleanDistPath: falseprevents the build system from cleaning the output directory. This might be intentional for preserving certain files across builds, but it could also lead to stale files accumulating.Could you clarify why
cleanDistPathis set tofalse? This setting prevents automatic cleanup of the output directory, which might be needed for sitemap persistence or other build artifacts.
15-16:Details
✅ Verification successful
Verify that the new plugin dependencies are properly installed.
The static analysis indicates these modules cannot be resolved. Ensure that
@rspress/plugin-algoliaand@rspress/plugin-llmsare properly added to the project dependencies.
🏁 Script executed:
#!/bin/bash # Description: Check if the required plugin dependencies are listed in package.json # Expected: Both plugins should be present in dependencies or devDependencies cat package.json | jq '.dependencies["@rspress/plugin-algolia"], .dependencies["@rspress/plugin-llms"], .devDependencies["@rspress/plugin-algolia"], .devDependencies["@rspress/plugin-llms"]'Length of output: 233
Dependency Check Passed for New RSPress Plugins
Both
@rspress/plugin-algolia(2.0.0-beta.8) and@rspress/plugin-llms(^2.0.0-beta.8) are declared under dependencies in your package.json. The unresolved import errors are likely IDE/TS false positives—no changes required here.🧰 Tools
🪛 ESLint
[error] 15-15: Unable to resolve path to module '@rspress/plugin-algolia'.
(import-x/no-unresolved)
[error] 16-16: Unable to resolve path to module '@rspress/plugin-llms'.
(import-x/no-unresolved)
src/plugins/sitemap/index.ts (2)
9-29: LGTM! Type definitions follow sitemap standards.The
ChangeFreqandPrioritytypes correctly implement the sitemap.xml standard values. Using string literals for priority is appropriate since XML attributes are strings.
31-43: LGTM! Well-structured interfaces.The interfaces properly define the sitemap entry structure and plugin options with appropriate optional fields and types.
JounQin
force-pushed
the
feat/llms_sitemap
branch
from
c79331c to
a412d31
Compare
Summary by CodeRabbit
New Features
Documentation
Chores