Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@claude review |
dosubot
Bot
added
the
size:L
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| handbook, | ||
| marketing, | ||
| } from "../.source/server"; | ||
| import { CONTENT_DIR_TO_URL_PREFIX } from "./content-dir-map.js"; | ||
|
|
||
| function baseUrl(contentDir: string): string { | ||
| const prefix = CONTENT_DIR_TO_URL_PREFIX[contentDir]; | ||
| if (typeof prefix !== "string") { | ||
| throw new Error(`Missing content-dir-map entry for "${contentDir}"`); | ||
| } | ||
| return prefix === "" ? "" : `/${prefix}`; | ||
| } | ||
|
|
||
| export const source = loader({ | ||
| baseUrl: baseUrl("docs"), | ||
| baseUrl: "/docs", | ||
| source: docs.toFumadocsSource(), |
There was a problem hiding this comment.
🔴 This PR removes the baseUrl() helper from lib/source.ts and replaces all calls with hardcoded string literals, but does not update CLAUDE.md (which still instructs developers that lib/content-dir-map.js is the single source of truth and that updating it keeps lib/source.ts in sync) or fix the stale error message in scripts/copy_md_sources.js line 64 that references the now-deleted baseUrl() function. If a developer follows CLAUDE.md guidance and updates content-dir-map.js to rename a URL prefix, lib/source.ts will silently diverge — copy_md_sources.js will write files to the new path while the loader still serves pages at the old hardcoded URL. The fix is to either update CLAUDE.md to drop the single-source-of-truth claim and update the copy_md_sources.js error message, or restore the content-dir-map.js dependency.
Extended reasoning...
What the bug is and how it manifests
This PR performs a refactor in lib/source.ts that removes the baseUrl() helper function (lines 14–33 of the diff) and its import { CONTENT_DIR_TO_URL_PREFIX } from './content-dir-map.js' dependency, replacing every baseUrl("docs")-style call with a hardcoded string like "/docs". Separately, scripts/copy_md_sources.js still imports and reads content-dir-map.js to construct output paths for static markdown files. Additionally, the error message on line 64 of that script still reads: "add it to lib/content-dir-map.js, same as lib/source.ts baseUrl()" — but baseUrl() no longer exists in lib/source.ts.
The specific code path that triggers it
CLAUDE.md (line 67) states: "lib/content-dir-map.js — single source of truth for mapping content/ top-level directories to URL prefixes; keep this updated when adding/renaming content sections so lib/source.ts and scripts/copy_md_sources.js stay in sync." content-dir-map.js itself also carries a comment listing lib/source.ts as a consumer. After this PR, lib/source.ts no longer consumes content-dir-map.js. The documented sync mechanism is broken.
Why existing code doesn't prevent it
The current hardcoded values in lib/source.ts happen to match the entries in content-dir-map.js, so no immediate runtime error occurs. There is no automated check that validates the two are consistent. A developer following the CLAUDE.md contract — which has not been updated by this PR — will modify content-dir-map.js expecting both consumers to stay in sync. Only copy_md_sources.js will reflect the change; lib/source.ts will not.
What the impact would be
Concrete scenario: a developer renames the "users" section to "customers" in content-dir-map.js (as instructed by CLAUDE.md). copy_md_sources.js will copy markdown files into public/md-src/customers/..., while lib/source.ts still serves pages at /users/ (hardcoded). The /md-src/ paths embedded in pages will point to non-existent files, breaking any feature that serves static markdown. The divergence will be silent — no build error, no runtime exception.
Separately, any future developer who triggers the error at copy_md_sources.js:64 will be told to update lib/source.ts baseUrl() — a function that no longer exists — causing confusion and wasted debugging time.
How to fix it
Either: (a) Update CLAUDE.md to remove the single-source-of-truth claim about content-dir-map.js syncing lib/source.ts, and update the error message in scripts/copy_md_sources.js line 64 to say something like "update the hardcoded baseUrl in lib/source.ts"; or (b) restore lib/source.ts's import of content-dir-map.js and the baseUrl() helper so the documented invariant is preserved.
Step-by-step proof
- Developer reads CLAUDE.md: "keep content-dir-map.js updated so lib/source.ts and scripts/copy_md_sources.js stay in sync."
- Developer renames a section, e.g. changes
'users': 'users'to'users': 'customers'incontent-dir-map.js. copy_md_sources.jsreadscontent-dir-map.js, resolves prefix to"customers", copies files topublic/md-src/customers/....lib/source.tsstill hasbaseUrl: "/users"hardcoded — it does not readcontent-dir-map.js. Pages are served at/users/.- Any link from a page to
/md-src/customers/breaks (404), while/md-src/users/no longer exists. - No build-time warning is produced.
| --- | ||
| title: "Langfuse 在中国 - Langfuse in China" | ||
| --- | ||
|
|
||
| import { ChinaLanding } from "@/components/china"; | ||
|
|
||
| <ChinaLanding /> |
There was a problem hiding this comment.
🔴 The new /china page is missing from pathsWithoutFooterWidgets in MainContentWrapper.tsx, causing duplicate 'Was this page helpful?' feedback widgets and
separators to render at the bottom of the page. Add '/china' to the pathsWithoutFooterWidgets array, matching how /cn, /jp, and /kr are already handled.
Extended reasoning...
What the bug is: MainContentWrapper.tsx maintains a pathsWithoutFooterWidgets array (lines ~36-59) that lists paths where DocBodyChrome already renders footer widgets (hr, DocsFeedback, DocsSupport), so MainContentWrapper skips rendering them a second time. The array includes /cn, /jp, /kr, and other similar regional/marketing pages, but does NOT include /china despite it being the same type of page.
The specific code path: When a user visits /china, two rendering layers apply: (1) app/[section]/[[...slug]]/page.tsx wraps all MDX content in DocBodyChrome with withProse=true, which ALWAYS appends hr + DocsFeedback + DocsSupport at the bottom. (2) app/[section]/layout.tsx wraps marketing content in MainContentWrapper, which ALSO renders hr + DocsFeedback + DocsSupport — UNLESS the current path is in pathsWithoutFooterWidgets. Since /china is absent from that list, both layers render the widgets.
Why existing code doesn't prevent it: The pathsWithoutFooterWidgets exclusion list requires manual updates whenever a new marketing page that uses DocBodyChrome is added. This PR added china to MARKETING_SLUGS (lib/source.ts) and created content/marketing/china.mdx, but did not update MainContentWrapper.tsx to add /china to the exclusion list.
Step-by-step proof:
- User visits /china
- layout.tsx identifies 'china' as a marketing slug → wraps in MainContentWrapper
- page.tsx renders ChinaLanding inside DocBodyChrome (withProse=true) → DocBodyChrome appends:
<hr />+<DocsFeedback />+<DocsSupport /> - MainContentWrapper checks: is '/china' in pathsWithoutFooterWidgets? → NO
- MainContentWrapper appends its own:
<hr />+<DocsFeedback />+<DocsSupport /> - Result: both sets of widgets appear at the bottom of the page
Impact: Visitors to /china see duplicated UI elements — two 'Was this page helpful?' prompts, two horizontal rules, and two support buttons — creating a broken, unprofessional appearance on the new China landing page.
Fix: Add '/china' to the pathsWithoutFooterWidgets array in MainContentWrapper.tsx, exactly as /cn, /jp, and /kr are already listed. The comment on that line explicitly documents this purpose: 'Paths where the page already shows feedback (DocBodyChrome); skip duplicate here.'
| en: "GitHub Stars", | ||
| zh: "GitHub 星标", | ||
| }, |
There was a problem hiding this comment.
🟡 The GitHub stars stat in ChinaLanding.tsx displays '24K+' when the actual count is 23,619 (below 24,000), because toFixed(0) rounds 23.619 up to 24. Replace (getGitHubStars() / 1000).toFixed(0) with Math.floor(getGitHubStars() / 1000).toString() to correctly display '23K+'.
Extended reasoning...
The Bug
In components/china/ChinaLanding.tsx (lines 62–64), the GitHub stars stat is computed as:
getValue: () => (getGitHubStars() / 1000).toFixed(0) + "K+"With GITHUB_STARS = 23619, this evaluates to (23.619).toFixed(0) which is "24" — because JavaScript's toFixed uses standard rounding and 0.619 > 0.5. The displayed string becomes "24K+".
Why it's wrong
The + suffix conventionally means "at least this many". Displaying 24K+ implies the project has at least 24,000 stars, but the actual count is 23,619 — 381 short of that threshold. The claim is factually incorrect.
Why existing code doesn't prevent it
The other two stats happen to be safe by coincidence: SDK_INSTALLS_PER_MONTH = 23_100_000 gives 23.1, which rounds down to 23, and DOCKER_PULLS = 6_000_000 gives exactly 6.0. Only the GitHub stars value has a fractional part large enough (0.619) to round up past an integer boundary. The home page Usage.tsx sidesteps this entirely by displaying the raw integer 23,619 via NumberTicker with no K+ rounding.
Step-by-step proof
getGitHubStars()returns23619(fromsrc/github-stars.ts)23619 / 1000 = 23.619(23.619).toFixed(0)→"24"(standard half-up rounding, 0.619 ≥ 0.5)"24" + "K+"→"24K+"- But 23,619 < 24,000, so "at least 24,000" is false.
Fix
Use Math.floor to conservatively round down:
getValue: () => Math.floor(getGitHubStars() / 1000).toString() + "K+"Math.floor(23619 / 1000) = 23 → displays "23K+", which is truthful (23,619 ≥ 23,000).
| --- | ||
| title: "Langfuse 在中国 - Langfuse in China" | ||
| --- | ||
|
|
||
| import { ChinaLanding } from "@/components/china"; | ||
|
|
||
| <ChinaLanding /> |
There was a problem hiding this comment.
🟡 china.mdx is missing a description field in its frontmatter, so the page will have no <meta name='description'> tag. All other marketing pages (cn.mdx, jp.mdx, kr.mdx, about.mdx, startups.mdx, etc.) include a description; adding one to china.mdx would align with the established pattern and improve SEO for this new landing page.
Extended reasoning...
Missing description field in china.mdx frontmatter
The new china.mdx page has only a title in its frontmatter:
---
title: "Langfuse 在中国 - Langfuse in China"
---Every other marketing page consistently includes a description field alongside the title. For example, cn.mdx, jp.mdx, kr.mdx, about.mdx, startups.mdx, and wrapped.mdx all have a description.
How it manifests: The generateMetadata function in the marketing page route reads page.data.description ?? undefined to populate the HTML <meta name='description'> tag. With no description field present, the page will render without a meta description tag entirely.
Why existing code doesn't prevent it: The frontmatter schema doesn't require a description field—it's optional. Nothing enforces its presence, so the omission silently results in a missing SEO tag rather than a build error.
Impact: Search engines fall back to auto-generated snippets when no meta description is present. For a new market-entry landing page like /china, a crafted description would help control how the page appears in search results and improve click-through rates.
How to fix: Add a concise bilingual description to the frontmatter, e.g.:
---
title: "Langfuse 在中国 - Langfuse in China"
description: "Langfuse 在中国持续增长，已广泛用于生产环境 AI 应用。了解我们与 ClickHouse 合作加大中国市场投入的最新进展。"
---Proof: Open china.mdx frontmatter (lines 1–3) — only title is present. Compare with cn.mdx which has both title and description. The rendering pipeline at generateMetadata will produce <meta name='description' content='...'> for cn.mdx but nothing for china.mdx.
1 participant
No description provided.