fix: move static/llm files to src/routes/llm and +server.ts#1852
fix: move static/llm files to src/routes/llm and +server.ts#1852shinokada merged 10 commits intothemesberg:mainfrom
Conversation
|
@shinokada is attempting to deploy a commit to the Themesberg Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughReplaces runtime filesystem reads in the LLM route with an eager import.meta.glob cache, prevents path traversal, standardizes content-type and caching, removes the EnhancedImg component and its plugin/export, moves createCountdown into an internal util, updates LLM scripts/configs, and applies assorted doc/ui formatting edits. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server as LLM Route
participant Cache as import.meta.glob Cache
Client->>Server: GET /llm/<...slug>
Note right of Server: build filePath from [slug]
Server->>Server: validate path (reject ".." or leading "/")
alt invalid path
Server-->>Client: 400 Bad Request
else valid path
Server->>Cache: lookup `filePath.md` or `filePath.txt`
alt found
Cache-->>Server: raw content + extension
Server->>Server: set Content-Type (md/plain) and Cache-Control: max-age=3600
Server-->>Client: 200 + content
else not found
Server-->>Client: 404 Not Found
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas to focus review on:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/llm/[...slug]/+server.ts (1)
43-56: Guard against double extensions (.md.md/.txt.txt) by detecting existing extensionsThe code blindly appends
.mdand.txttoresolvedPathwithout checking if the slug already includes an extension. If a caller requests/llm/my-file.md, the code will search for/path/to/static/llm/my-file.md.mdinstead of/path/to/static/llm/my-file.md, resulting in an unexpected 404.Add a check using
path.extname()to skip the extension appending ifresolvedPathalready ends with.md,.txt, or other extensions. For example:const ext = path.extname(resolvedPath); if (!ext || !['.md', '.txt'].includes(ext)) { // only append extensions if missing const mdPath = `${resolvedPath}.md`; const txtPath = `${resolvedPath}.txt`; // ... rest of logic }Alternatively, document that slugs must be extensionless and validate them explicitly at the start.
🧹 Nitpick comments (2)
src/routes/llm/[...slug]/+server.ts (2)
13-33: Consider cachingstaticDirand avoiding per-request synchronousexistsSyncprobesThe multi-location search is reasonable, but doing four
fs.existsSyncchecks on every request is synchronous and repeated work. Consider resolvingstaticDironce at module load (or caching it lazily in a module-level variable) and reusing it across requests; if no directory is found, you can throw during initialization rather than on every call.
57-58: Downgrade 404 logging verbosity or gate it behind a debug flagLogging every 404 as
console.errorwith full paths is great for debugging, but in production it can generate noisy error logs for benign misses and may expose internal directory layout. Consider usingconsole.warn/console.infoor only emitting this detail when a debug flag/environment variable is enabled.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/llm/[...slug]/+server.ts(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Playwright and Lint
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/routes/docs-examples/typography/image/index.ts (1)
6-6: Consider removing commented code entirely.Commented-out exports should typically be removed rather than left in the codebase, as they can cause confusion and clutter. If this is temporary for debugging or a staged rollout, consider using feature flags instead.
Apply this diff to remove the commented line:
-// export { default as Enhanced } from "./Enhanced.svelte";src/routes/llm/[...slug]/+server.ts (1)
44-49: Consider differentiating Content-Type for .txt files.The response headers are well-configured with appropriate caching. However, Content-Type is always
text/markdowneven for.txtfiles. If this is intentional for your LLM use case, that's fine. Otherwise, consider settingtext/plainfor.txtfiles.If you want to differentiate, apply this diff:
- return new Response(content, { - headers: { - 'Content-Type': 'text/markdown; charset=utf-8', - 'Cache-Control': 'public, max-age=3600' // Cache for 1 hour - } - }); + const contentType = files[mdKey] ? 'text/markdown' : 'text/plain'; + return new Response(content, { + headers: { + 'Content-Type': `${contentType}; charset=utf-8`, + 'Cache-Control': 'public, max-age=3600' // Cache for 1 hour + } + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
package.json(0 hunks)src/lib/typography/img/EnhancedImg.svelte(0 hunks)src/lib/typography/img/index.ts(0 hunks)src/lib/utils/index.ts(0 hunks)src/routes/docs-examples/components/modal/Countdown.svelte(1 hunks)src/routes/docs-examples/typography/image/Enhanced.svelte(0 hunks)src/routes/docs-examples/typography/image/EnhancedEffect.svelte(0 hunks)src/routes/docs-examples/typography/image/index.ts(1 hunks)src/routes/docs/typography/image.md(0 hunks)src/routes/llm/[...slug]/+server.ts(1 hunks)src/routes/utils/countdown.svelte.ts(1 hunks)vite.config.ts(1 hunks)
💤 Files with no reviewable changes (7)
- src/lib/typography/img/index.ts
- package.json
- src/lib/utils/index.ts
- src/routes/docs/typography/image.md
- src/routes/docs-examples/typography/image/EnhancedEffect.svelte
- src/lib/typography/img/EnhancedImg.svelte
- src/routes/docs-examples/typography/image/Enhanced.svelte
🧰 Additional context used
🧬 Code graph analysis (1)
src/routes/llm/[...slug]/+server.ts (1)
src/hooks.server.js (1)
parts(24-26)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Playwright and Lint
🔇 Additional comments (13)
src/routes/docs-examples/typography/image/index.ts (1)
1-10: Inconsistency: AI summary doesn't match actual changes.The AI-generated summary describes changes to "LLM route with preloaded import.meta.glob cache, path-traversal checks, and Content-Type headers," but this file contains changes to image component exports. This mismatch suggests either the summary is incorrect or these files don't represent the main changes described in the PR objectives.
vite.config.ts (2)
14-19: LGTM! Clean plugin removal.The removal of the
enhancedImagesplugin from the array is straightforward and aligns with the broader cleanup of Enhanced image functionality across the codebase.
1-70: Inconsistency: Changes don't match PR description.The PR is titled "fix: debug for production server" and the AI summary describes changes to LLM routes, filesystem caching, and path-traversal security checks. However, this file only shows removal of the
enhancedImagesplugin, which is unrelated to server debugging or LLM functionality. Please verify that the correct files are included in this review.src/routes/llm/[...slug]/+server.ts (4)
4-10: Excellent serverless-friendly approach!The use of
import.meta.globwith eager loading and raw content is the right solution for serverless environments where filesystem access is unavailable. This preloads all LLM files at build time and bundles them into the deployment.
12-15: Development logging looks good.The development-only logging is helpful for debugging file loading without impacting production performance.
Note: The PR title mentions "debug for production server," but these logs only run in development. If production debugging is needed, consider adding optional production logging or request tracing.
24-26: Path traversal protection is appropriate.The checks for
".."and leading"/"provide basic but adequate protection against path traversal attempts, especially since the glob pattern already constrains available files to/static/llm/**.
29-42: File lookup logic is clean and well-debugged.The fallback from
.mdto.txtusing nullish coalescing is elegant, and the development-time error logging with available files will help troubleshoot path mismatches.src/routes/utils/countdown.svelte.ts (5)
1-6: LGTM: Clean initialization.The function signature and state initialization are well-structured, with appropriate use of Svelte 5
$staterunes for reactive values and standard variables for internal tracking.
7-28: Solid timing logic with requestAnimationFrame.The tick function correctly tracks elapsed time and decrements the countdown every second. The stop-at-zero guard (lines 18-21) and the isRunning check (line 25) work together to prevent runaway frames.
30-56: Well-structured control methods.The start, stop, reset, and destroy methods handle state transitions correctly:
startguards against invalid statesstopperforms complete cleanupresetcalls stop before updating timeLeftdestroyhandles animation frame cleanup
58-61: Excellent automatic cleanup pattern.The
$effectcleanup ensures the animation frame is cancelled when the component unmounts, preventing memory leaks. This is idiomatic Svelte 5.
63-74: Clean API with reactive getters.The return object provides reactive access to state through getters while exposing control methods. This is a good encapsulation pattern for Svelte 5 utilities.
src/routes/docs-examples/components/modal/Countdown.svelte (1)
2-3: <!-- [scratchpad_start]
[task review_file_2/2 final_verification_complete_3/3]
All verification checks passed[observations]
✓ File exists: src/routes/utils/countdown.svelte.ts
✓ Export verified: createCountdown function properly exported with correct signature
✓ $utils alias correctly configured in svelte.config.js to ./src/routes/utils
✓ tsconfig.json includes "allowImportingTsExtensions": true enabling .ts imports
✓ File uses Svelte 5 runes ($state, $effect) - .svelte.ts is the documented convention
✓ The import path in Countdown.svelte is valid and correctly resolves[analysis]
Original review comment concerns:
- "Verify the import alias configuration" - CONFIRMED: $utils alias is properly configured
- ".ts extension is unconventional" - INCORRECT: .svelte.ts is the documented Svelte 5 convention for TypeScript utility files with runes
All import changes are valid and follow best practices. No issues exist. The review comment should be RESOLVED.
[pending]
None[actions]
Generate resolved review comment with tag[done]
Verification complete. All configuration is correct. No actionable items remain.
Import path configuration is correct.
The
$utilsalias is properly configured insvelte.config.jsto resolve to./src/routes/utils, and thecountdown.svelte.tsfile exists at the expected location with a properly exportedcreateCountdownfunction. The.tsextension is the documented convention in Svelte 5 for utility files using runes (as seen in the$stateand$effectrunes in the file), and the project'stsconfig.jsonincludes"allowImportingTsExtensions": trueto support this pattern.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
static/llm/components/modal.md (1)
524-524: Import spacing inconsistency.The import statement uses
{createCountdown}without spaces around the braces. Follow the convention with consistent spacing:{ createCountdown }.- import {createCountdown} from "$utils/countdown.svelte.ts"; + import { createCountdown } from "$utils/countdown.svelte.ts";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/routes/docs-examples/typography/image/Enhanced.svelte(1 hunks)src/routes/docs/typography/image.md(1 hunks)static/llm/components/modal.md(1 hunks)static/llm/context-full.txt(3 hunks)static/llm/typography/image.md(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/routes/docs-examples/typography/image/Enhanced.svelte
🚧 Files skipped from review as they are similar to previous changes (1)
- src/routes/docs/typography/image.md
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-03-20T12:47:33.307Z
Learnt from: superstes
Repo: themesberg/flowbite-svelte PR: 0
File: :0-0
Timestamp: 2025-03-20T12:47:33.307Z
Learning: In the Flowbite Svelte library's composite form components like MultiInput, when implementing focus styles, ensure both the container (with focus-within) and the inner input element (with direct focus) have dynamic styling based on the validation state. The input element needs its own focus styles that change to red when inputInvalid=true.
Applied to files:
static/llm/context-full.txt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Playwright and Lint
🔇 Additional comments (5)
static/llm/typography/image.md (1)
1-207: [Inconsistent summary] — Documentation content contradicts removal claims.The enriched summary states that the changes "reflect removal of the entire block documenting effect, size, and align props for enhanced image," but these sections remain fully present in the provided code:
- "Image effects" section: lines 110–131
- "Alignment" section: lines 133–157
- "Sizes" section: lines 159–182
Additionally, line 188 is marked as changed, but the actual modification is unclear from the annotation. Without visibility into the previous version, it's difficult to assess what specifically changed on this line or whether the claimed removals occurred elsewhere in the file.
The PR's stated objective ("fix: debug for production server" with LLM route caching) also appears disconnected from this static markdown documentation file, raising questions about the scope and context of this change.
Please clarify:
- What specifically changed on line 188?
- Were blocks actually removed from this file (despite their presence in the provided code)?
- How does this documentation edit relate to the LLM server fix described in the PR?
static/llm/components/modal.md (1)
523-524: No issues found — countdown utility API is correctly implemented and properly imported.The verification confirms that:
- The
createCountdownutility exists atsrc/routes/utils/countdown.svelte.ts(resolves via the$utilsalias configured insvelte.config.js)- All expected API members are present and correctly exported:
.start(),.reset(),.isRunning, and.timeLeft- The import statement will resolve correctly
The code is ready as-is.
static/llm/context-full.txt (3)
11024-11025: Import path update accurately reflects code reorganization.The import statement correctly points to the relocated
createCountdownfunction at$utils/countdown.svelte.ts, aligning with the PR context mentioning that the countdown utility was moved from the public API. This is consistent with the codebase reorganization.
24529-24529: Documentation reference is appropriate.The updated reference to
https://svelte.dev/docs/kit/images#sveltejs-enhanced-imgcorrectly directs users to the official Svelte documentation for the Enhanced image feature, replacing any outdated local references.
1-2: The review comment is based on a misunderstanding of this file's purpose and can be resolved.Based on verification,
context-full.txtis an intentionally generated and versioned artifact, not accidentally committed build output:
- Auto-generated as intended: The file is generated by the
npm run llmscript (which runssvelte-doc-llm && ./scripts/update-llms.sh), and this script is explicitly part of thelib-helpersbuild pipeline- Properly versioned: The file is NOT in
.gitignoreand appears in git history with multiple commits per release (recent example: commit5d753c04 llm: update (#1851)), confirming it's an intentional artifact- Timestamp is appropriate: The "Generated on" timestamp indicates when the context was last synchronized with the codebase, which is useful information for LLM consumers
The three content changes in this PR are correct:
- Import path update to align with
createCountdownrelocation- Addition of
createCountdownimport- Documentation link update reflecting the
@sveltejs/enhanced-imgremovalNo clarification or refactoring needed.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
package.json (1)
1-906: Version bump violates semver and breaking change is not documented in CHANGELOG.The removal of
EnhancedImg.svelteexport and@sveltejs/enhanced-imgdependency is a confirmed breaking change (commits78d10b9aand985be929), but it has critical issues:
- Semver violation: Breaking changes require a major version bump (e.g., v2.0.0), not a patch bump. This change is currently in v1.28.1 (patch).
- Missing CHANGELOG entry: The v1.28.1 CHANGELOG entry only mentions "fix: DatePicker, Dropdown RTL, Button loading spinner" with no mention of the EnhancedImg removal or migration guidance.
- No changeset: No
.changesetentry documents this breaking change.Users upgrading to v1.28.1 will encounter breaking changes without warning. Update the CHANGELOG.md with a detailed breaking change entry, bump to v2.0.0, and add a changeset documenting the removal.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
package.json(1 hunks)src/routes/admin-dashboard/(sidebar)/crud/users/+page.svelte(1 hunks)src/routes/admin-dashboard/(sidebar)/quickstart/+layout.svelte(1 hunks)src/routes/admin-dashboard/(sidebar)/quickstart/+page.svelte(1 hunks)src/routes/docs-examples/components/device-mockups/Smartwatch.svelte(1 hunks)src/routes/docs-examples/components/modal/Countdown.svelte(1 hunks)src/routes/docs-examples/typography/image/index.ts(0 hunks)src/routes/llm/[...slug]/+server.ts(1 hunks)vite.config.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/routes/docs-examples/typography/image/index.ts
✅ Files skipped from review due to trivial changes (2)
- src/routes/docs-examples/components/device-mockups/Smartwatch.svelte
- src/routes/admin-dashboard/(sidebar)/crud/users/+page.svelte
🚧 Files skipped from review as they are similar to previous changes (2)
- vite.config.ts
- src/routes/docs-examples/components/modal/Countdown.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Playwright and Lint
🔇 Additional comments (4)
src/routes/admin-dashboard/(sidebar)/quickstart/+layout.svelte (1)
5-7: LGTM - CSS class reordering has no functional impact.The reordering of Tailwind utility classes from
max-w-7xl mx-autotomx-auto max-w-7xlproduces identical rendering results.src/routes/admin-dashboard/(sidebar)/quickstart/+page.svelte (1)
24-28: LGTM - Formatting improvement.Breaking the paragraph text across multiple lines improves code readability without affecting the rendered output.
src/routes/llm/[...slug]/+server.ts (2)
1-15: Excellent refactor for serverless compatibility.The migration from filesystem reads to
import.meta.globwith eager loading is a solid improvement. This bundles the content at build time, making it available in serverless environments where filesystem access may be restricted.
23-25: Good path traversal protection.The validation correctly prevents directory traversal attacks by blocking paths containing
..or starting with/.
src/routes/llm/[...slug]/+server.ts
Outdated
| export async function GET({ params }: RequestEvent) { | ||
| const parts = Array.isArray(params.slug) ? params.slug : [params.slug]; | ||
|
|
||
| const resolvedPath = path.resolve(staticDir, filePath); | ||
| const relative = path.relative(staticDir, resolvedPath); | ||
| const filePath = parts.join("/"); |
There was a problem hiding this comment.
🧩 Analysis chain
Verify params.slug type handling.
In SvelteKit catch-all routes [...slug], the params.slug is always a string (not an array), where segments are joined with slashes. The array check on line 18 appears unnecessary.
🏁 Script executed:
#!/bin/bash
# Verify how SvelteKit handles catch-all route parameters
# Search for other [...slug] route handlers to see how they handle params
rg -nP --type=ts -C3 'params\.slug' -g '**/+server.ts' -g '**/+page.ts' -g '**/+page.server.ts'Length of output: 8258
Remove unnecessary array check on params.slug.
In SvelteKit catch-all routes, params.slug is a string where segments are joined by slashes (e.g., 'use-cases/something' for /use-cases/something). To access individual segments, split the string by "/" rather than checking if it's an array.
Change line 18 from:
const parts = Array.isArray(params.slug) ? params.slug : [params.slug];
to:
const parts = params.slug.split("/");
🤖 Prompt for AI Agents
In src/routes/llm/[...slug]/+server.ts around lines 17 to 20, remove the
unnecessary Array.isArray check on params.slug and treat params.slug as a
string; replace the current conditional that wraps params.slug in an array with
splitting the string by "/" so parts = params.slug.split("/") to get individual
segments for building filePath.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.coderabbit.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Playwright and Lint
.coderabbit.yml
Outdated
| reviews: | ||
| timeout_ms: 900000 |
There was a problem hiding this comment.
🧩 Analysis chain
Fix the configuration structure to match the CodeRabbit schema.
The timeout_ms property is nested incorrectly. According to the CodeRabbit configuration schema, timeout_ms belongs under reviews.tools.github-checks, not directly under reviews. The current structure will not be recognized by CodeRabbit.
Apply this fix to correct the configuration structure:
-reviews:
- timeout_ms: 900000
+reviews:
+ tools:
+ github-checks:
+ timeout_ms: 900000Additionally, clarify why the timeout is set to the maximum value (900000 ms = 15 minutes). This is 10× the default (90000 ms).
🌐 Web query:
CodeRabbit configuration schema timeout_ms reviews tools github-checks
💡 Result:
Quick summary (fields live under reviews.tools in .coderabbit.yaml):
- reviews.tools.github-checks.enabled — enable GitHub Checks integration (boolean). Default: true. [1][2]
- reviews.tools.github-checks.timeout_ms — time to wait for all GitHub Checks to finish, in milliseconds. Default: 90000 (90s); max 900000 (15 min). [2]
- reviews.tools.* — each tool (eslint, flake8, gitleaks, etc.) has its own reviews.tools..* settings (typically enabled boolean and tool-specific options). Configure per-tool under reviews.tools in .coderabbit.yaml or via the Web UI. [1][2]
- reviews.* — general review settings (profile, auto_review, finishing_touches, path_instructions, etc.) live under reviews in the same schema. [1]
References: CodeRabbit configuration reference and tools reference. [1][2]
Fix the configuration structure to match the CodeRabbit schema.
The timeout_ms property is nested incorrectly. According to the CodeRabbit configuration schema, timeout_ms belongs under reviews.tools.github-checks, not directly under reviews. The current structure will not be recognized by CodeRabbit.
Apply this fix to correct the configuration structure:
-reviews:
- timeout_ms: 900000
+reviews:
+ tools:
+ github-checks:
+ timeout_ms: 900000Additionally, clarify why the timeout is set to the maximum value (900000 ms = 15 minutes). This is 10× the default (90000 ms).
📝 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.
| reviews: | |
| timeout_ms: 900000 | |
| reviews: | |
| tools: | |
| github-checks: | |
| timeout_ms: 900000 |
🤖 Prompt for AI Agents
In .coderabbit.yml around lines 1 to 2, the timeout_ms key is placed at the
top-level reviews object but must live under reviews.tools.github-checks per the
CodeRabbit schema; move timeout_ms into reviews.tools.github-checks (nest under
reviews -> tools -> github-checks) so the config is recognized, and add a brief
inline comment or adjacent comment explaining why the timeout is set to 900000
ms (15 minutes) — e.g., to allow long-running review tools/workflows and why it
intentionally exceeds the default (10× longer) — ensuring you keep YAML comment
syntax and no other structural changes.
There was a problem hiding this comment.
In the previous review, I found the following:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
There was a problem hiding this comment.
@shinokada, thank you for clarifying! That makes perfect sense—experiencing timeout issues with checks is a valid reason to increase the timeout to the maximum value.
However, the structural issue with the configuration still needs to be addressed. The timeout_ms property must be nested under reviews.tools.github-checks (not directly under reviews) to be recognized by CodeRabbit.
The corrected structure should be:
reviews:
tools:
github-checks:
timeout_ms: 900000 # Increased to 15 minutes to prevent checks from timing outYou might also want to add an inline comment (as shown above) explaining why you've set this to the maximum value, so it's clear for future reference.
Once you apply this fix, the timeout configuration will work as intended! 🎉
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
scripts/clean-llms-urls.sh (2)
7-13: Multiple sed invocations with-i.bakcreate unnecessary backup files.Both sed commands create
.bakbackups; the second overwrites the first before cleanup. Consider consolidating into a single sed invocation or using-i ''with a single backup at the end.# Example: Combine into single sed call sed -i.bak \ -e 's/\.md$//' \ -e 's|/context: https://flowbite-svelte.com/llm/context-full$|/context: https://flowbite-svelte.com/llm/context-full.txt|' \ ./static/llms.txt rm -f ./static/llms.txt.bak
1-16: Add input validation and document purpose.The script modifies llms.txt but doesn't validate that the file exists. Additionally, it's not immediately clear why this script is necessary or how it relates to
scripts/update-llms.sh. Adding a brief header comment explaining the relationship would improve maintainability.#!/usr/bin/env bash # Clean URL extensions in llms.txt after update-llms.sh inserts them. # Removes .md from most URLs but preserves .txt for context-full. set -e if [[ ! -f ./static/llms.txt ]]; then echo "Error: ./static/llms.txt not found" >&2 exit 1 fi echo "Updating llms.txt URLs to remove .md extensions..." # ... rest of scriptsrc/routes/llm/components/device-mockups.md (1)
118-119: Class reordering is fine—ensure consistency across file.Reordering Tailwind classes from
w-[188px] h-[193px]toh-[193px] w-[188px]is a safe, stylistic change. However, verify this ordering convention is consistent with other device mockup examples in the same file (e.g., lines 28–29, 43–44, 58–59). If other examples use different ordering, consider normalizing all of them for consistency.src/routes/llm/context-full.txt (1)
1-5: PR objective unclear: specify what "debug for production server" refers to.The PR title suggests fixing debug code or a production server issue, but the changes shown (timestamp update, import migration, class reordering) don't explicitly reveal what debugging concern is being addressed. Clarify:
- What debug code or configuration is being removed/fixed?
- How do these changes (especially the createCountdown internalization) relate to the production server fix?
- Are there related changes to server route handling mentioned in the AI summary ("refactors the LLM server route to use glob-based preloaded caching")?
Would you like me to search for related changes in the LLM server route or other configuration files to understand the full scope of this fix?
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
.coderabbit.yml(1 hunks)llm.config.js(1 hunks)package.json(2 hunks)scripts/clean-llms-urls.sh(1 hunks)scripts/update-llms.sh(1 hunks)src/routes/llm/[...slug]/+server.ts(1 hunks)src/routes/llm/components/device-mockups.md(1 hunks)src/routes/llm/components/modal.md(1 hunks)src/routes/llm/context-full.txt(4 hunks)src/routes/llm/typography/image.md(1 hunks)static/llms.txt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- package.json
- .coderabbit.yml
- src/routes/llm/[...slug]/+server.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-03-20T12:47:33.307Z
Learnt from: superstes
Repo: themesberg/flowbite-svelte PR: 0
File: :0-0
Timestamp: 2025-03-20T12:47:33.307Z
Learning: In the Flowbite Svelte library's composite form components like MultiInput, when implementing focus styles, ensure both the container (with focus-within) and the inner input element (with direct focus) have dynamic styling based on the validation state. The input element needs its own focus styles that change to red when inputInvalid=true.
Applied to files:
src/routes/llm/context-full.txt
🪛 markdownlint-cli2 (0.18.1)
src/routes/llm/typography/image.md
188-188: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (11)
llm.config.js (2)
26-28: LGTM! Good fix for preserving dynamic routes.The
ignoreDirsconfiguration correctly prevents the cleaning operation from deleting the[...slug]dynamic route directory, which aligns with the PR's objective to fix the production server issue.
6-44: Path configurations verified and correct.All configured directories exist and are properly referenced. The configuration is ready for production use.
static/llms.txt (1)
108-108: Fix looks correct: route key now matches endpoint intent.Changing
/icons:to/illustrations:correctly differentiates the illustrations section from the icons section (line 105). This aligns with the API contract expected by the llms.txt specification.scripts/update-llms.sh (2)
4-21: Verify thatsrc/routes/llm/is the correct destination for glob-based preloading.Per the PR summary, changes refactor the LLM server route to use glob-based preloaded caching. Confirm that copying files to
src/routes/llm/(instead of the previousstatic/llm/) aligns with how your SvelteKit config loads these resources via glob patterns (e.g.,import.meta.glob()in+server.tsor+page.server.ts).
4-6: No issues found.The source files
./scripts/icons.mdand./scripts/illustrations.mdboth exist and have been successfully copied to the destination directory./src/routes/llm/. The script includesset -eat the top, which ensures it will exit immediately if eithercpcommand fails, providing adequate error handling for the file copy operations.src/routes/llm/components/modal.md (2)
523-524: No issues found—import change is correctly implemented.Verification confirms:
- ✓
src/routes/utils/countdown.svelte.tsexists and exportscreateCountdown- ✓
$utilsalias insvelte.config.jscorrectly points tosrc/routes/utils- ✓ Function signature
createCountdown(initialSeconds = 60)matches usage- ✓ Return object provides all required properties/methods:
timeLeft,isRunning,start(),stop(),reset(seconds),destroy()- ✓ No broken imports from old
flowbite-sveltelocation- ✓ Implementation includes automatic cleanup via
$effecthookThe import change from
flowbite-svelteto$utils/countdown.svelte.tsis valid and properly implemented.
523-588: No stale imports of createCountdown from flowbite-svelte detected.Verified: The codebase contains no imports of
createCountdownfromflowbite-svelte. All instances correctly import from the custom utility path$utils/countdown.svelte.ts. The file under review and all other files properly reference the internal countdown utility function.src/routes/llm/context-full.txt (4)
7201-7203: The review comment is incorrect.The code does not reorder classes. Lines 7202–7203 contain two separate
<img>elements with different purposes (one hidden in dark mode, one shown in dark mode), not a reordering of the same classes. The class ordering is consistent across all instances in the codebase:
- Line 7202 appears identically in
device-mockups.md:118andSmartwatch.svelte:6- Line 7203 appears identically in
device-mockups.md:119andSmartwatch.svelte:7The different class sequences between these two lines reflect their distinct purposes, not a refactoring change.
Likely an incorrect or invalid review comment.
1-5: Based on my verification, thecontext-full.txtfile is a complete bundle of all documentation, combined into one large text file that includes full explanations, code examples, and details for LLMs. This is an auto-generated file, and the timestamp update in line 2 is expected behavior when the documentation is regenerated.Flowbite Svelte follows the llms.txt standard, which standardizes how websites provide LLM-friendly information. The timestamp reflects when the file was generated, which changes on each regeneration as part of the build process.
Verify the PR changes beyond the timestamp update.
The review comment provided shows only the first hunk's header. To properly verify the remaining hunks (class reordering, import path changes, and documentation updates), the actual diff content for hunks 2-4 needs to be examined. Confirm that:
- The Tailwind class reordering in device mockups is intentional and maintains functionality
- The import path change from
flowbite-svelteto$utils/countdown.svelte.tsis correctly implemented and the utility exists- The documentation updates for enhanced-img align with actual component changes
24527-24529: Verify the actual changes in commit 985be92 to clarify what was removed.The git log shows commit
985be929 fix: remove @sveltejs/enhanced-img, but the current code in lines 24527-24529 (and matching sections insrc/routes/llm/typography/image.mdandsrc/routes/docs/typography/image.md) still contains the warning and setup link for@sveltejs/enhanced-img. The EnhancedImg component metadata also remains active in the codebase. Without seeing the actual diff from that commit, it's unclear whether:
- The removal was incomplete or partially reverted
- The AI summary mischaracterized the changes
- These specific documentation sections were excluded from the removal
The review's concern is valid: clarify what was actually removed in that commit and whether the current documentation accurately reflects the support status.
Please check the diff for commit
985be929to confirm what@sveltejs/enhanced-imgcontent was actually removed and whether these documentation sections should be updated or removed as well.
11024-11025: Import path and API compatibility verified.All verification points confirm the migration is sound:
- ✓ File exists:
src/routes/utils/countdown.svelte.ts(resolved via$utilsalias in svelte.config.js)- ✓ Function signature matches:
createCountdown(initialSeconds = 60)accepts numeric parameter- ✓ Usage at lines 11027-11028 is compatible:
createCountdown(4)andcreateCountdown(5)pass values matching the expected parameter typeNo issues found.
| **Warning: ** Since `@sveltejs/enhanced-img` uses pre-1.0 versioning and may introduce breaking changes with every minor version release. | ||
|
|
||
| Please follow how to set up `@sveltejs/enhanced-img` at https://svelte.dev/docs/kit/images#sveltejs-enhanced-img and install `v0.6.1`. | ||
| Please follow how to set up `@sveltejs/enhanced-img` at https://svelte.dev/docs/kit/images#sveltejs-enhanced-img. |
There was a problem hiding this comment.
Wrap bare URL in markdown link syntax.
Line 188 contains a bare URL that violates markdown best practices (MD034). Wrap the URL in proper markdown link syntax.
Apply this diff to fix the bare URL:
-Please follow how to set up `@sveltejs/enhanced-img` at https://svelte.dev/docs/kit/images#sveltejs-enhanced-img.
+Please follow [how to set up `@sveltejs/enhanced-img`](https://svelte.dev/docs/kit/images#sveltejs-enhanced-img).📝 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.
| Please follow how to set up `@sveltejs/enhanced-img` at https://svelte.dev/docs/kit/images#sveltejs-enhanced-img. | |
| Please follow [how to set up `@sveltejs/enhanced-img`](https://svelte.dev/docs/kit/images#sveltejs-enhanced-img). |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
188-188: Bare URL used
(MD034, no-bare-urls)
🤖 Prompt for AI Agents
In src/routes/llm/typography/image.md around line 188 there's a bare URL causing
MD034; replace the bare URL with a proper Markdown link by wrapping the URL in
[link text](URL) form (e.g., use descriptive link text like "SvelteKit images
docs" or "svelte.dev docs" and put the URL in parentheses) so the URL is no
longer bare and the line complies with Markdown best practices.
shinokada
changed the title
| sed -i.bak \ | ||
| -e 's/\.md$//' \ | ||
| -e 's|/context: https://flowbite-svelte.com/llm/context-full$|/context: https://flowbite-svelte.com/llm/context-full.txt|' \ | ||
| ./static/llms.txt |
There was a problem hiding this comment.
🧩 Analysis chain
Brittle sed patterns—undocumented assumptions about file format.
The sed patterns assume a very specific llms.txt format (particularly line 16's exact URL match), but there's no validation that these patterns actually succeed. If the file format differs, the replacements fail silently—sed returns exit code 0 even when no substitutions occur, so set -e won't catch it.
Line 16's pattern is especially fragile: it assumes the context-full URL appears on its own line in the exact form /context: https://flowbite-svelte.com/llm/context-full$. If the actual format is different (e.g., JSON with quoted URLs, different key names, or extra whitespace), the pattern won't match.
To confirm this will work reliably, please:
- Share or verify the actual format of
./static/llms.txtto ensure the sed patterns match the real structure. - Consider adding a check after the sed operations to ensure replacements actually occurred:
# Remove .md extensions from all URLs and restore .txt for context-full
sed -i.bak \
-e 's/\.md$//' \
-e 's|/context: https://flowbite-svelte.com/llm/context-full$|/context: https://flowbite-svelte.com/llm/context-full.txt|' \
./static/llms.txt
+# Verify the context-full URL now ends with .txt
+if ! grep -q 'context-full\.txt' ./static/llms.txt; then
+ echo "Warning: context-full.txt pattern not found in llms.txt after update" >&2
+fiAlternatively, make the patterns more flexible to handle variations in whitespace or structure.
🏁 Script executed:
#!/bin/bash
# Check if ./static/llms.txt exists and inspect its format
if [ -f "./static/llms.txt" ]; then
echo "=== File exists, showing first 30 lines ==="
head -30 "./static/llms.txt"
echo ""
echo "=== Total line count ==="
wc -l "./static/llms.txt"
echo ""
echo "=== Searching for 'context-full' pattern ==="
grep -n "context-full" "./static/llms.txt" || echo "No 'context-full' found"
echo ""
echo "=== Searching for .md extensions ==="
grep -n "\.md" "./static/llms.txt" || echo "No .md extensions found"
else
echo "File ./static/llms.txt does not exist"
fiLength of output: 7989
🏁 Script executed:
#!/bin/bash
# Test the sed patterns against the actual file to see what would change
echo "=== Testing sed patterns on actual file ==="
if [ -f "./static/llms.txt" ]; then
echo "Pattern 1: Removing .md extensions"
sed 's/\.md$//' "./static/llms.txt" | head -5
echo ""
echo "Pattern 2: Restoring .txt for context-full (applied after pattern 1)"
sed -e 's/\.md$//' -e 's|/context: https://flowbite-svelte.com/llm/context-full$|/context: https://flowbite-svelte.com/llm/context-full.txt|' "./static/llms.txt" | head -10
echo ""
echo "=== Verifying if context-full URL would actually be replaced ==="
# Show lines that should match the pattern in line 16
grep "context-full" "./static/llms.txt" | head -5
else
echo "File not found"
fiLength of output: 689
🏁 Script executed:
#!/bin/bash
# Check the exact structure and format around context-full references
echo "=== Context around 'context-full' entries ==="
if [ -f "./static/llms.txt" ]; then
grep -B2 -A2 "context-full" "./static/llms.txt" 2>/dev/null | head -20 || echo "Pattern not found"
fiLength of output: 166
Script's sed pattern fails silently—context-full URL already has .txt extension.
The review comment correctly flagged brittle sed patterns. Verification confirms the second pattern will fail to match:
- Current file state (line 111):
/context: https://flowbite-svelte.com/llm/context-full.txt - Pattern 2 expects:
/context: https://flowbite-svelte.com/llm/context-full$(without .txt) - Result: No match, silent failure (sed exits 0 despite no substitution)
The script assumes the context-full URL lacks the .txt extension and needs it added, but the file already contains it. Pattern 1 (removing .md) works correctly on the /docs URLs, but pattern 2 will do nothing—though the desired state is already present.
Fix the logic to match actual file state or add validation:
# Remove .md extensions from all URLs and restore .txt for context-full
sed -i.bak \
-e 's/\.md$//' \
- -e 's|/context: https://flowbite-svelte.com/llm/context-full$|/context: https://flowbite-svelte.com/llm/context-full.txt|' \
+ -e 's|/context: https://flowbite-svelte.com/llm/context-full(?:\.md)?$|/context: https://flowbite-svelte.com/llm/context-full.txt|' \
./static/llms.txtOr verify the result post-execution to catch silent failures:
+# Verify context-full has .txt extension
+if ! grep -q 'context-full\.txt' ./static/llms.txt; then
+ echo "Error: context-full.txt not found after update" >&2
+ exit 1
+fiCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In scripts/clean-llms-urls.sh around lines 14 to 17, the second sed pattern
assumes the context-full URL lacks a .txt suffix so it never matches when the
file already contains /context:
https://flowbite-svelte.com/llm/context-full.txt; update the sed invocation to
robustly handle both variants by making the .txt part optional in the match (or
supply two complementary replacements), and add a post-sed validation step that
checks the file contains the desired /context:
https://flowbite-svelte.com/llm/context-full.txt line (exit non-zero and print
an error if validation fails) so silent failures are detected.
1 participant
🔗 Related issue (optional)
Closes #
📑 Description
🔍 PR Type
🚦 PR Status
✅ Checklist
pnpm check && pnpm test:e2eand all tests passmainbranch (not the published npm version)🧪 Screenshots / Demos (optional)
ℹ️ Additional Information
Summary by CodeRabbit
Chores
Improvements
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.