Add Table of Contents extraction, sidebar, and authoring docs#27
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds heading slugification and build-time TOC extraction, extends navigation manifests with per-page TOC data, exposes TOC APIs and types, provides a client TableOfContents component wired into DocsShell, enhances MDX headings with copy-link behavior, and updates tests and documentation. ChangesTable of Contents Feature
Sequence Diagram(s)sequenceDiagram
participant Router
participant DocsShell
participant DocsLib
participant TableOfContents
Router->>DocsShell: current pathname
DocsShell->>DocsLib: findDocsNavigationPage(pathname)
DocsLib-->>DocsShell: DocsNavigationPage { toc }
DocsShell->>TableOfContents: render(items = toc)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
KayleeWilliams
force-pushed
the
KayleeWilliams/toc
branch
from
96a9197 to
dfd763e
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96a9197b1e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const headingText = textFromChildren(children); | ||
| const headingId = id ?? slugifyDocsHeading(headingText); |
There was a problem hiding this comment.
When a heading contains inline MDX/markdown markup (for example the existing docs/reference/lint.mdx heading ### meta.json``), the new TOC extractor records an entry like #meta-json, but this renderer derives `headingText` via `textFromChildren`, which returns an empty string for React elements such as `` or ``. The rendered heading therefore gets no matching `id`, so the new "On this page" link points to an anchor that is not present on the page.
Useful? React with 👍 / 👎.
| if (!(event.metaKey || event.ctrlKey || event.shiftKey || event.altKey)) { | ||
| event.preventDefault(); | ||
| history.replaceState(null, "", hash); | ||
| document.querySelector(hash)?.scrollIntoView(); |
There was a problem hiding this comment.
For headings whose generated slug starts with a digit (the docs already have numbered headings such as ## 1. Generate the artifacts), hash becomes something like #1-generate-the-artifacts, and passing that raw string to querySelector throws because it is not a valid CSS id selector. Since the handler calls preventDefault() before this line, clicking the heading anchor in those cases does not perform the intended scroll/copy behavior.
Useful? React with 👍 / 👎.
Ships table-of-contents support so docs sites built on leadtype can
render an "On this page" sidebar. Leadtype owns the contract — slug-
stable heading IDs plus a `toc` array on every page in the navigation
manifest — while the user's app owns the React rendering, matching the
framework-neutral stance documented in docs/authoring/components.mdx.
- leadtype/llm: resolveDocsTableOfContents, extractDocsTableOfContents,
DocsTableOfContentsItem, plus a `toc: { minLevel, maxLevel }` option
on resolveDocsNavigation (defaults h2-h3).
- leadtype/llm/readability: slugifyDocsHeading helper (Unicode-safe,
fs-free) so renderers can match extracted slugs.
- apps/example: TableOfContents reference component, MDX heading wiring
via slugifyDocsHeading, smoke test covering the slug contract.
- docs/authoring/components.mdx: new "Headings and table of contents"
section explaining the slug contract and pointing at the reference
component; docs/reference/llm.mdx trimmed to API-only with a cross-
link.
KayleeWilliams
force-pushed
the
KayleeWilliams/toc
branch
from
dfd763e to
3ab1a45
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/example/src/mdx-components.tsx (1)
9-17:⚠️ Potential issue | 🟠 Major | ⚡ Quick winExtract text from React elements in headings to preserve anchor IDs.
When MDX processes formatted headings like
# \npm init` quickstartor##API, inline formatting becomes React elements. The currenttextFromChildrenfunction (line 9) only handles strings, numbers, and arrays—returning""for React elements. This causes the generatedheadingId` (line 38) to be empty or incorrect, breaking anchor links between build-time TOC slugs and runtime heading IDs.Add an
isValidElementcheck to recursively extract text from React element children. The fix also applies to line 35–39 wheretextFromChildrenis called.Proposed fix
-import type { ComponentPropsWithoutRef, MouseEvent } from "react"; +import { + isValidElement, + type ComponentPropsWithoutRef, + type MouseEvent, + type ReactNode, +} from "react"; @@ -function textFromChildren(children: HeadingProps["children"]): string { +function textFromChildren(children: ReactNode): string { if (typeof children === "string" || typeof children === "number") { return String(children); } if (Array.isArray(children)) { return children.map(textFromChildren).join(" "); } + if (isValidElement<{ children?: ReactNode }>(children)) { + return textFromChildren(children.props.children); + } return ""; }🤖 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 `@apps/example/src/mdx-components.tsx` around lines 9 - 17, The textFromChildren function fails to extract text from React elements, causing empty/incorrect heading IDs; update textFromChildren to import and use React.isValidElement and, when isValidElement(children) is true, recursively call textFromChildren on children.props.children (handling string/number/array/element cases) so formatted inline elements (e.g., <Code />) return their text; also ensure the code that computes headingId (where textFromChildren is invoked) will receive the extracted string.
🤖 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 `@apps/example/server/utils/agent-readability.ts`:
- Around line 17-20: The object currently assigned to agentReadabilityManifest
uses a double-cast ("as unknown as AgentReadabilityManifest") which masks shape
errors; replace that double-cast with the TypeScript satisfies operator so the
manifest is validated structurally (e.g., export const agentReadabilityManifest
= { ...manifestJson, version: 1 } as const satisfies AgentReadabilityManifest)
and apply the identical change in apps/example/src/lib/docs-head.ts for the
other manifest export; keep the spread of manifestJson and add as const where
appropriate to preserve literal types while using satisfies for compile-time
validation.
In `@apps/example/src/components/table-of-contents.tsx`:
- Around line 131-133: The decodeURIComponent call inside updateFromHash can
throw on malformed percent-encodings; wrap the decode step in a try/catch and
fall back to a safe value (e.g., empty string or raw sliced hash) so the
function never throws — capture const raw = window.location.hash.slice(1); then
try { hashId = decodeURIComponent(raw) } catch { hashId = raw || '' } before
using headings.find(...). Keep updateFromHash and its hashchange listener
behavior unchanged otherwise.
In `@apps/example/src/lib/docs-head.ts`:
- Around line 8-11: Replace the unsafe double-cast on the manifest object:
instead of "const manifest = { ...agentReadability, version: 1 } as unknown as
AgentReadabilityManifest", use TypeScript's "satisfies" operator to validate the
shape against AgentReadabilityManifest and make the version a literal by
asserting it as a const literal (e.g., version: 1 as const); update the
declaration for manifest and keep agentReadability and version fields unchanged
so the compiler enforces the manifest structure while preserving version as the
literal 1.
In `@apps/example/src/styles.css`:
- Around line 122-124: The existing prose link rule selector `.docs-prose
a:not([data-leadtype-card]):not([data-leadtype-topic-switcher-link]):not([data-leadtype-prompt-content]
*)` is more specific than `.docs-prose [data-docs-heading-anchor]`, so heading
anchors still get underlined; update that original selector (the `.docs-prose
a:not(... )` rule) to also exclude heading anchors by adding
`:not([data-docs-heading-anchor])` to it so the `.docs-prose
[data-docs-heading-anchor] { `@apply` no-underline; }` rule takes effect.
In `@packages/leadtype/src/llm/llm.ts`:
- Around line 419-440: The TOC extractor uses slugifyDocsHeading in
extractDocsTableOfContents to generate non-unique IDs for duplicate headings;
add a per-page Map<string,number> seenIds inside extractDocsTableOfContents,
increment the count for each slug from slugifyDocsHeading(title) and if count>1
suffix the id with `-${count}` (e.g., examples-2) before creating the
DocsTableOfContentsItem (the id used for urlWithHash/absoluteUrlWithHash and
pushing to items/stack). Apply the identical per-page seenIds logic in the
heading renderer in apps/example/src/mdx-components.tsx so the DOM id produced
by the renderer matches the slug used by extractDocsTableOfContents, ensuring
consistent deduped IDs across extractor and renderer.
---
Outside diff comments:
In `@apps/example/src/mdx-components.tsx`:
- Around line 9-17: The textFromChildren function fails to extract text from
React elements, causing empty/incorrect heading IDs; update textFromChildren to
import and use React.isValidElement and, when isValidElement(children) is true,
recursively call textFromChildren on children.props.children (handling
string/number/array/element cases) so formatted inline elements (e.g., <Code />)
return their text; also ensure the code that computes headingId (where
textFromChildren is invoked) will receive the extracted string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4de748f5-a5f4-455c-8f40-9550e4647488
⛔ Files ignored due to path filters (4)
apps/example/src/generated/agent-readability.jsonis excluded by!**/generated/**apps/example/src/generated/docs-nav.jsonis excluded by!**/generated/**apps/example/src/generated/docs-search-content.jsonis excluded by!**/generated/**apps/example/src/generated/docs-search-index.jsonis excluded by!**/generated/**
📒 Files selected for processing (17)
apps/example/server/utils/agent-readability.tsapps/example/src/components/docs-shell.tsxapps/example/src/components/table-of-contents.tsxapps/example/src/lib/docs-head.tsapps/example/src/lib/docs.tsapps/example/src/mdx-components.tsxapps/example/src/styles.cssapps/example/tests/e2e/smoke.e2e.tsdocs/authoring/components.mdxdocs/reference/llm.mdxpackages/leadtype/src/index.tspackages/leadtype/src/internal/docs-heading.tspackages/leadtype/src/llm/index.tspackages/leadtype/src/llm/llm.test.tspackages/leadtype/src/llm/llm.tspackages/leadtype/src/llm/readability.tspackages/leadtype/src/search/search.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use explicit types for function parameters and return values when they enhance clarity
Preferunknownoveranywhen the type is genuinely unknown
Use const assertions (as const) for immutable values and literal types
Leverage TypeScript's type narrowing instead of type assertions
Files:
packages/leadtype/src/internal/docs-heading.tsapps/example/src/lib/docs-head.tsapps/example/tests/e2e/smoke.e2e.tsapps/example/src/components/table-of-contents.tsxapps/example/server/utils/agent-readability.tsapps/example/src/mdx-components.tsxpackages/leadtype/src/llm/readability.tspackages/leadtype/src/index.tsapps/example/src/lib/docs.tspackages/leadtype/src/llm/index.tspackages/leadtype/src/search/search.tspackages/leadtype/src/llm/llm.test.tspackages/leadtype/src/llm/llm.tsapps/example/src/components/docs-shell.tsx
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use meaningful variable names instead of magic numbers - extract constants with descriptive names
Use arrow functions for callbacks and short functions
Preferfor...ofloops over.forEach()and indexedforloops
Use optional chaining (?.) and nullish coalescing (??) for safer property access
Prefer template literals over string concatenation
Use destructuring for object and array assignments
Useconstby default,letonly when reassignment is needed, nevervar
Alwaysawaitpromises in async functions - don't forget to use the return value
Useasync/awaitsyntax instead of promise chains for better readability
Handle errors appropriately in async code with try-catch blocks
Don't use async functions as Promise executors
Removeconsole.log,debugger, andalertstatements from production code
ThrowErrorobjects with descriptive messages, not strings or other values
Usetry-catchblocks meaningfully - don't catch errors just to rethrow them
Prefer early returns over nested conditionals for error cases
Extract complex conditions into well-named boolean variables
Use early returns to reduce nesting
Prefer simple conditionals over nested ternary operators
Don't useeval()or assign directly todocument.cookie
Avoid spread syntax in accumulators within loops
Use top-level regex literals instead of creating them in loops
Prefer specific imports over namespace imports
Use descriptive names for functions, variables, and types for meaningful naming
Add comments for complex logic, but prefer self-documenting code
Files:
packages/leadtype/src/internal/docs-heading.tsapps/example/src/lib/docs-head.tsapps/example/tests/e2e/smoke.e2e.tsapps/example/src/components/table-of-contents.tsxapps/example/server/utils/agent-readability.tsapps/example/src/mdx-components.tsxpackages/leadtype/src/llm/readability.tspackages/leadtype/src/index.tsapps/example/src/lib/docs.tspackages/leadtype/src/llm/index.tspackages/leadtype/src/search/search.tspackages/leadtype/src/llm/llm.test.tspackages/leadtype/src/llm/llm.tsapps/example/src/components/docs-shell.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{jsx,tsx}: Use function components over class components in React
Call hooks at the top level only, never conditionally
Specify all dependencies in hook dependency arrays correctly
Use thekeyprop for elements in iterables (prefer unique IDs over array indices)
Nest children between opening and closing tags instead of passing as props
Don't define components inside other components
AvoiddangerouslySetInnerHTMLunless absolutely necessary
Use proper image components (e.g., Next.js<Image>) over<img>tags
Use Next.js<Image>component for images
Usenext/heador App Router metadata API for head elements in Next.js
Use Server Components for async data fetching instead of async Client Components in Next.js
Use ref as a prop instead ofReact.forwardRefin React 19+
Files:
apps/example/src/components/table-of-contents.tsxapps/example/src/mdx-components.tsxapps/example/src/components/docs-shell.tsx
**/*.{jsx,tsx,html}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{jsx,tsx,html}: Use semantic HTML and ARIA attributes for accessibility: provide meaningful alt text for images, use proper heading hierarchy, add labels for form inputs, include keyboard event handlers alongside mouse events, use semantic elements instead of divs with roles
Addrel="noopener"when usingtarget="_blank"on links
Files:
apps/example/src/components/table-of-contents.tsxapps/example/src/mdx-components.tsxapps/example/src/components/docs-shell.tsx
**/index.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid barrel files (index files that re-export everything)
Files:
packages/leadtype/src/index.tspackages/leadtype/src/llm/index.ts
**/*.{test,spec}.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{test,spec}.{js,ts,jsx,tsx}: Write assertions insideit()ortest()blocks
Avoid done callbacks in async tests - use async/await instead
Don't use.onlyor.skipin committed code
Keep test suites reasonably flat - avoid excessivedescribenesting
Files:
packages/leadtype/src/llm/llm.test.ts
🪛 OpenGrep (1.20.0)
packages/leadtype/src/llm/llm.ts
[ERROR] 398-398: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🔍 Remote MCP Context7
Summary of Additional Context for PR Review
MDX Component Patterns
The PR's MDX heading component changes align with MDX best practices for custom component overrides. According to Context7 documentation, MDX allows passing a components object to override default HTML element rendering. The PR uses the useMDXComponents() hook pattern, which is the recommended approach for globally injecting components through the providerImportSource configuration. This pattern enables:
- Defining custom heading components (
h1–h6) that wrap default behavior - Returning component mappings from
useMDXComponents()that MDX will use automatically - Merging caller-provided component overrides with built-in defaults
The PR's implementation of adding anchor links to headings with copy-to-clipboard functionality and scroll ID tracking fits within this MDX component override pattern.
React Event Listener and State Management Patterns
The TableOfContents component uses several React patterns documented in the React reference:
-
useEffect with Event Listeners: The component's scroll/resize/hashchange listener implementation follows the recommended pattern where event listeners are added in
useEffectand removed in the cleanup function (return statement). This prevents memory leaks. -
State Management: The use of
useStatefor trackingactiveIdstate and updating it based on scroll position is the standard React approach for managing component state in functional components. -
Dependency Arrays: The component should properly declare dependencies in
useEffecthooks to ensure listeners are re-attached when needed. Dependencies likeitemswould trigger re-registration of scroll handlers. -
requestAnimationFrame for Throttling: The pattern of throttling scroll updates via
requestAnimationFrameis a performance optimization technique mentioned in React documentation for handling frequent events like scroll.
Key Review Considerations
Based on the context and PR summary:
-
Slug Generation Contract: The new
slugifyDocsHeadingutility (diacritic-removal, Unicode normalization, hyphen conversion) needs to match exactly how the MDX heading renderer computes heading IDs to ensure extracted TOC links point to actual DOM elements. -
TOC Extraction Edge Cases: The
extractDocsTableOfContentshelper correctly skips frontmatter and code fences when parsing markdown headings—this prevents false positives in TOC generation. -
Scroll Position Detection: The
TableOfContentscomponent's scroll listener uses an "active line" threshold approach to determine which heading is currently visible, with synchronization to URL hash changes. -
Type Safety: The casting pattern in
agent-readability.tsanddocs-head.ts(object spread withas unknown as Type) bypasses type checking—this should be reviewed to ensure the runtime JSON structure matches the expected types.
🔇 Additional comments (1)
packages/leadtype/src/llm/llm.test.ts (1)
997-1066: Strong TOC coverage across parsing and integration paths.Nice work covering frontmatter/code-fence exclusion, nested heading extraction, navigation embedding, and standalone TOC resolution in one suite.
Also applies to: 1118-1129, 1140-1158, 1161-1192
| export const agentReadabilityManifest = { | ||
| ...manifestJson, | ||
| version: 1, | ||
| }; | ||
| } as unknown as AgentReadabilityManifest; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify all forced casts to AgentReadabilityManifest so they can be replaced consistently.
rg -nP 'as\s+unknown\s+as\s+AgentReadabilityManifest'Repository: inthhq/leadtype
Length of output: 230
🏁 Script executed:
#!/bin/bash
# Check context around line 20 in agent-readability.ts
echo "=== apps/example/server/utils/agent-readability.ts context ==="
sed -n '10,25p' apps/example/server/utils/agent-readability.ts
echo -e "\n=== apps/example/src/lib/docs-head.ts context ==="
# Check context around line 11 in docs-head.ts
sed -n '5,20p' apps/example/src/lib/docs-head.ts
echo -e "\n=== TypeScript configuration ==="
# Check tsconfig to verify satisfies operator support
if [ -f "tsconfig.json" ]; then
cat tsconfig.json | jq -r '.compilerOptions.target' 2>/dev/null || cat tsconfig.json
fiRepository: inthhq/leadtype
Length of output: 963
🏁 Script executed:
#!/bin/bash
# Find AgentReadabilityManifest type definition
echo "=== Finding AgentReadabilityManifest type definition ==="
rg -n 'type AgentReadabilityManifest|interface AgentReadabilityManifest' --type ts --type tsx
echo -e "\n=== Check for any other variations of this problematic pattern ==="
# Check for any other double-cast patterns in the codebase
rg -n 'as unknown as' --type ts --type tsx | head -20Repository: inthhq/leadtype
Length of output: 258
🏁 Script executed:
#!/bin/bash
# Search for AgentReadabilityManifest more broadly
echo "=== Searching for AgentReadabilityManifest definition ==="
rg 'AgentReadabilityManifest' | head -30
echo -e "\n=== Check for any other variations of double-cast patterns ==="
rg 'as unknown as' | head -20
echo -e "\n=== Find files importing AgentReadabilityManifest ==="
rg "import.*AgentReadabilityManifest"Repository: inthhq/leadtype
Length of output: 3822
🏁 Script executed:
#!/bin/bash
# Get the AgentReadabilityManifest type definition
echo "=== AgentReadabilityManifest type definition ==="
sed -n '/export type AgentReadabilityManifest/,/^}/p' packages/leadtype/src/llm/readability.ts | head -20Repository: inthhq/leadtype
Length of output: 403
Fix double-casts to AgentReadabilityManifest in both files using satisfies operator
Two instances of the problematic as unknown as AgentReadabilityManifest pattern exist in the codebase (not just one). Both should be replaced consistently using satisfies for proper structural validation:
apps/example/server/utils/agent-readability.ts:20apps/example/src/lib/docs-head.ts:11
The satisfies operator validates the object shape at compile time without forcing a type assertion, catching schema drift that double-casts would mask. Apply the same fix to both:
Proposed fix for both files
export const agentReadabilityManifest = {
...manifestJson,
- version: 1,
-} as unknown as AgentReadabilityManifest;
+ version: 1 as const,
+} satisfies AgentReadabilityManifest;(Same pattern applies to docs-head.ts)
Per coding guidelines: "Leverage TypeScript's type narrowing instead of type assertions" and "Use const assertions (as const) for immutable values and literal types."
📝 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 const agentReadabilityManifest = { | |
| ...manifestJson, | |
| version: 1, | |
| }; | |
| } as unknown as AgentReadabilityManifest; | |
| export const agentReadabilityManifest = { | |
| ...manifestJson, | |
| version: 1 as const, | |
| } satisfies AgentReadabilityManifest; |
🤖 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 `@apps/example/server/utils/agent-readability.ts` around lines 17 - 20, The
object currently assigned to agentReadabilityManifest uses a double-cast ("as
unknown as AgentReadabilityManifest") which masks shape errors; replace that
double-cast with the TypeScript satisfies operator so the manifest is validated
structurally (e.g., export const agentReadabilityManifest = { ...manifestJson,
version: 1 } as const satisfies AgentReadabilityManifest) and apply the
identical change in apps/example/src/lib/docs-head.ts for the other manifest
export; keep the spread of manifestJson and add as const where appropriate to
preserve literal types while using satisfies for compile-time validation.
| const manifest = { | ||
| ...agentReadability, | ||
| version: 1, | ||
| }; | ||
| } as unknown as AgentReadabilityManifest; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
fd -t f "docs-head.ts"Repository: inthhq/leadtype
Length of output: 93
🏁 Script executed:
cat -n apps/example/src/lib/docs-head.tsRepository: inthhq/leadtype
Length of output: 579
🏁 Script executed:
rg "type AgentReadabilityManifest" -A 10Repository: inthhq/leadtype
Length of output: 3508
🏁 Script executed:
cat -n packages/leadtype/src/llm/readability.ts | head -40Repository: inthhq/leadtype
Length of output: 2021
🏁 Script executed:
rg "export type AgentReadabilityManifest" -A 15 packages/leadtype/src/llm/readability.tsRepository: inthhq/leadtype
Length of output: 448
🏁 Script executed:
rg "agent-readability.json" -B 2 -A 2 apps/example/src/libRepository: inthhq/leadtype
Length of output: 368
🏁 Script executed:
fd -t f "agent-readability.json" apps/example/src/generated/Repository: inthhq/leadtype
Length of output: 109
🏁 Script executed:
cat apps/example/src/generated/agent-readability.json | head -50Repository: inthhq/leadtype
Length of output: 1994
🏁 Script executed:
cat apps/example/src/generated/agent-readability.json | jq 'keys'Repository: inthhq/leadtype
Length of output: 156
Replace double type cast with satisfies and literal type assertion
The double cast as unknown as AgentReadabilityManifest bypasses TypeScript's type checking and removes compile-time guarantees for the manifest structure. Use satisfies AgentReadabilityManifest with a literal type assertion on version instead:
Proposed fix
-const manifest = {
- ...agentReadability,
- version: 1,
-} as unknown as AgentReadabilityManifest;
+const manifest = {
+ ...agentReadability,
+ version: 1 as const,
+} satisfies AgentReadabilityManifest;This preserves compile-time type safety while correctly typing version as the literal 1 rather than number, aligning with the guidelines on avoiding type assertions and using const assertions for immutable literal types.
📝 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 manifest = { | |
| ...agentReadability, | |
| version: 1, | |
| }; | |
| } as unknown as AgentReadabilityManifest; | |
| const manifest = { | |
| ...agentReadability, | |
| version: 1 as const, | |
| } satisfies AgentReadabilityManifest; |
🤖 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 `@apps/example/src/lib/docs-head.ts` around lines 8 - 11, Replace the unsafe
double-cast on the manifest object: instead of "const manifest = {
...agentReadability, version: 1 } as unknown as AgentReadabilityManifest", use
TypeScript's "satisfies" operator to validate the shape against
AgentReadabilityManifest and make the version a literal by asserting it as a
const literal (e.g., version: 1 as const); update the declaration for manifest
and keep agentReadability and version fields unchanged so the compiler enforces
the manifest structure while preserving version as the literal 1.
| const id = slugifyDocsHeading(title); | ||
| const item: DocsTableOfContentsItem = { | ||
| id, | ||
| title, | ||
| level, | ||
| urlPath: page.urlPath, | ||
| urlWithHash: withHash(page.urlPath, id), | ||
| absoluteUrlWithHash: withHash(page.absoluteUrl, id), | ||
| children: [], | ||
| }; | ||
|
|
||
| while (stack.length > 0 && (stack.at(-1)?.level ?? 0) >= level) { | ||
| stack.pop(); | ||
| } | ||
|
|
||
| const parent = stack.at(-1); | ||
| if (parent) { | ||
| parent.children.push(item); | ||
| } else { | ||
| items.push(item); | ||
| } | ||
| stack.push(item); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python - <<'PY'
import pathlib
import re
import unicodedata
heading_re = re.compile(r'^(#{2,6})\s+(.+)$')
def clean(text: str) -> str:
text = re.sub(r'\s+#+\s*$', '', text)
text = re.sub(r'\[([^\]]+)\]\(([^)]+)\)', r'\1', text)
text = re.sub(r'<[^>]+>', ' ', text)
text = re.sub(r'[`*_~>#:[\](){}|]', ' ', text)
text = re.sub(r'\s+', ' ', text).strip()
return text
def slugify(text: str) -> str:
text = unicodedata.normalize("NFKD", text)
text = ''.join(ch for ch in text if not 0x0300 <= ord(ch) <= 0x036F)
text = text.lower()
text = re.sub(r'[^\w]+', '-', text, flags=re.UNICODE)
return text.strip('-')
docs_root = pathlib.Path("docs")
if not docs_root.exists():
raise SystemExit(0)
for path in sorted(docs_root.rglob("*")):
if path.suffix not in {".md", ".mdx"}:
continue
seen = {}
in_fence = False
for raw_line in path.read_text(encoding="utf-8").splitlines():
line = raw_line.strip()
if line.startswith("```"):
in_fence = not in_fence
continue
if in_fence:
continue
match = heading_re.match(line)
if not match:
continue
slug = slugify(clean(match.group(2)))
if not slug:
continue
seen[slug] = seen.get(slug, 0) + 1
duplicates = [slug for slug, count in seen.items() if count > 1]
if duplicates:
print(f"{path}: {', '.join(duplicates)}")
PYRepository: inthhq/leadtype
Length of output: 41
🏁 Script executed:
# First, locate the relevant files
fd -type f -name "*.ts" -o -name "*.tsx" | grep -E "(llm|mdx-components)" | head -20Repository: inthhq/leadtype
Length of output: 230
🏁 Script executed:
# Search for slugifyDocsHeading function definition
rg "slugifyDocsHeading" -B 2 -A 5 --type ts --type tsxRepository: inthhq/leadtype
Length of output: 87
🏁 Script executed:
# Get the full context of extractDocsTableOfContents function
rg "extractDocsTableOfContents" -B 5 -A 50 --type ts --type tsxRepository: inthhq/leadtype
Length of output: 87
🏁 Script executed:
# Check mdx-components.tsx for heading ID generation
rg "heading|h[1-6]|id\s*=" apps/example/src/mdx-components.tsx -B 2 -A 2Repository: inthhq/leadtype
Length of output: 1137
🏁 Script executed:
# Find the files first
find . -name "llm.ts" -o -name "mdx-components.tsx" | head -10Repository: inthhq/leadtype
Length of output: 132
🏁 Script executed:
# Search for slugifyDocsHeading without using --type
rg "slugifyDocsHeading" -B 2 -A 10Repository: inthhq/leadtype
Length of output: 50374
🏁 Script executed:
# Get full extractDocsTableOfContents function
rg "export function extractDocsTableOfContents" -A 70Repository: inthhq/leadtype
Length of output: 4133
🏁 Script executed:
# Check if there's any deduplication mechanism already in place
rg "seenIds|duplicat|Counter" packages/leadtype/src/llm/llm.tsRepository: inthhq/leadtype
Length of output: 41
🏁 Script executed:
# Search for tests related to extractDocsTableOfContents
rg "extractDocsTableOfContents" -A 15 -B 2 | grep -E "(test|spec|expect|describe)"Repository: inthhq/leadtype
Length of output: 50374
🏁 Script executed:
# Check the full llm.ts file around the function to see if seenIds or any dedup exists
sed -n '378,450p' packages/leadtype/src/llm/llm.tsRepository: inthhq/leadtype
Length of output: 1880
🏁 Script executed:
# Look for any test files for llm.ts
find . -name "*llm*.test.ts" -o -name "*llm*.spec.ts" 2>/dev/nullRepository: inthhq/leadtype
Length of output: 99
🏁 Script executed:
# Check if there's any deduplication logic in docs heading or elsewhere
rg "seenIds|occurrence|duplicate.*slug|slug.*duplicate" packages/leadtype/src/Repository: inthhq/leadtype
Length of output: 294
Deduplicate heading IDs per page in both extractor and renderer.
Lines 419 and throughout extractDocsTableOfContents use slugifyDocsHeading(title) without per-page deduplication. When a page has two identical headings (e.g., two ## Examples sections), both will generate the same ID (e.g., examples), causing TOC links to collide and always navigate to the first occurrence. The same issue exists in apps/example/src/mdx-components.tsx at the heading renderer.
Add a per-page seenIds counter to both the extractor and renderer:
- In
extractDocsTableOfContents: track slug occurrences and suffix subsequent duplicates as${slug}-2,${slug}-3, etc. - In the renderer: apply identical logic so DOM IDs match extracted TOC slugs exactly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/leadtype/src/llm/llm.ts` around lines 419 - 440, The TOC extractor
uses slugifyDocsHeading in extractDocsTableOfContents to generate non-unique IDs
for duplicate headings; add a per-page Map<string,number> seenIds inside
extractDocsTableOfContents, increment the count for each slug from
slugifyDocsHeading(title) and if count>1 suffix the id with `-${count}` (e.g.,
examples-2) before creating the DocsTableOfContentsItem (the id used for
urlWithHash/absoluteUrlWithHash and pushing to items/stack). Apply the identical
per-page seenIds logic in the heading renderer in
apps/example/src/mdx-components.tsx so the DOM id produced by the renderer
matches the slug used by extractDocsTableOfContents, ensuring consistent deduped
IDs across extractor and renderer.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/example/src/mdx-components.tsx (1)
9-16:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle nested React children when deriving heading slugs.
textFromChildrendrops text inside React elements and injects separator spaces between array items. This breaks the slug contract for common MDX headings like inline code, emphasis, or links — extracted TOC hashes stop matching renderedids.💡 Proposed fix
-import type { ComponentPropsWithoutRef, MouseEvent } from "react"; +import { + Children, + isValidElement, + type ComponentPropsWithoutRef, + type MouseEvent, +} from "react"; @@ function textFromChildren(children: HeadingProps["children"]): string { - if (typeof children === "string" || typeof children === "number") { - return String(children); - } - if (Array.isArray(children)) { - return children.map(textFromChildren).join(" "); - } - return ""; + return Children.toArray(children) + .map((child) => { + if (typeof child === "string" || typeof child === "number") { + return String(child); + } + if (isValidElement<{ children?: HeadingProps["children"] }>(child)) { + return textFromChildren(child.props.children); + } + return ""; + }) + .join(""); }🤖 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 `@apps/example/src/mdx-components.tsx` around lines 9 - 16, textFromChildren currently only handles strings/numbers and arrays (joining with spaces) and therefore drops text inside React elements and inserts unwanted spaces; update textFromChildren (used for deriving heading slugs) to recursively unwrap React elements by checking React.isValidElement(children) and recursing into children.props.children, handle fragments and arrays by mapping and joining with an empty string (""), and ignore null/boolean/undefined values so inline code, emphasis, and links produce the correct continuous text for slugs.
🤖 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 `@apps/example/src/mdx-components.tsx`:
- Around line 19-31: In copyHeadingUrl, replace the
document.querySelector(hash)?.scrollIntoView() call with
document.getElementById(...) to avoid CSS-selector parsing issues for IDs
containing characters like ':' or '.'; derive the plain id by stripping a
leading '#' from the hash (e.g., if hash starts with '#' use hash.slice(1)) and
call document.getElementById(id)?.scrollIntoView() so the element lookup treats
the value as an identifier rather than a selector.
In `@packages/leadtype/src/index.ts`:
- Around line 12-20: The root barrel is exporting TOC and docs utilities that
should remain on the specialized subpath; remove these exports
(DocsTableOfContentsItem, DocsTableOfContentsOptions, DocsTableOfContentsPage,
defineDocsConfig, extractDocsTableOfContents, ProductInfo,
ResolveDocsTableOfContentsConfig, resolveDocsTableOfContents,
slugifyDocsHeading) from the root index.ts and re-export or keep them only from
the dedicated leadtype/llm entrypoint; update any internal imports or tests to
import from the leadtype/llm module instead of the root so the top-level public
surface remains minimal and stable.
In `@packages/leadtype/src/llm/llm.ts`:
- Around line 333-340: cleanHeadingText currently uses MARKDOWN_INLINE_PATTERN
which strips punctuation like ":" and "#" and causes display titles to diverge
from rendered headings; update cleanHeadingText to retain literal punctuation by
removing the call to .replace(MARKDOWN_INLINE_PATTERN, " ") and instead strip
only markdown formatting wrappers (e.g., emphasis markers and code ticks) while
preserving punctuation — keep the existing .replace(MARKDOWN_LINK_PATTERN,
"$1"), .replace(/<[^>]+>/g, " "), .replace(WHITESPACE_PATTERN, " "), and
.trim(), but replace MARKDOWN_INLINE_PATTERN with a narrower regex or logic that
removes only formatting characters (e.g., *, _, ~, backticks) so titles like
"API: Quick Start" and "C#" remain unchanged.
In `@packages/leadtype/src/llm/readability.ts`:
- Around line 79-86: The public type DocsNavigationPage currently makes toc
required which is a breaking change; change its declaration to make toc optional
(toc?: DocsTableOfContentsItem[]) so existing consumers constructing
DocsNavigationPage or AgentReadabilityManifest.navigation object literals still
type-check, and keep generators emitting an empty array ([]) at runtime where
appropriate to preserve behavior.
---
Outside diff comments:
In `@apps/example/src/mdx-components.tsx`:
- Around line 9-16: textFromChildren currently only handles strings/numbers and
arrays (joining with spaces) and therefore drops text inside React elements and
inserts unwanted spaces; update textFromChildren (used for deriving heading
slugs) to recursively unwrap React elements by checking
React.isValidElement(children) and recursing into children.props.children,
handle fragments and arrays by mapping and joining with an empty string (""),
and ignore null/boolean/undefined values so inline code, emphasis, and links
produce the correct continuous text for slugs.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: de88e58f-0f1e-405b-9554-c09644006e5d
⛔ Files ignored due to path filters (4)
apps/example/src/generated/agent-readability.jsonis excluded by!**/generated/**apps/example/src/generated/docs-nav.jsonis excluded by!**/generated/**apps/example/src/generated/docs-search-content.jsonis excluded by!**/generated/**apps/example/src/generated/docs-search-index.jsonis excluded by!**/generated/**
📒 Files selected for processing (17)
apps/example/server/utils/agent-readability.tsapps/example/src/components/docs-shell.tsxapps/example/src/components/table-of-contents.tsxapps/example/src/lib/docs-head.tsapps/example/src/lib/docs.tsapps/example/src/mdx-components.tsxapps/example/src/styles.cssapps/example/tests/e2e/smoke.e2e.tsdocs/authoring/components.mdxdocs/reference/llm.mdxpackages/leadtype/src/index.tspackages/leadtype/src/internal/docs-heading.tspackages/leadtype/src/llm/index.tspackages/leadtype/src/llm/llm.test.tspackages/leadtype/src/llm/llm.tspackages/leadtype/src/llm/readability.tspackages/leadtype/src/search/search.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use explicit types for function parameters and return values when they enhance clarity
Preferunknownoveranywhen the type is genuinely unknown
Use const assertions (as const) for immutable values and literal types
Leverage TypeScript's type narrowing instead of type assertions
Files:
apps/example/server/utils/agent-readability.tsapps/example/src/lib/docs-head.tspackages/leadtype/src/llm/llm.test.tspackages/leadtype/src/internal/docs-heading.tspackages/leadtype/src/llm/readability.tspackages/leadtype/src/llm/index.tsapps/example/src/components/docs-shell.tsxapps/example/src/components/table-of-contents.tsxapps/example/tests/e2e/smoke.e2e.tsapps/example/src/mdx-components.tsxpackages/leadtype/src/index.tsapps/example/src/lib/docs.tspackages/leadtype/src/search/search.tspackages/leadtype/src/llm/llm.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use meaningful variable names instead of magic numbers - extract constants with descriptive names
Use arrow functions for callbacks and short functions
Preferfor...ofloops over.forEach()and indexedforloops
Use optional chaining (?.) and nullish coalescing (??) for safer property access
Prefer template literals over string concatenation
Use destructuring for object and array assignments
Useconstby default,letonly when reassignment is needed, nevervar
Alwaysawaitpromises in async functions - don't forget to use the return value
Useasync/awaitsyntax instead of promise chains for better readability
Handle errors appropriately in async code with try-catch blocks
Don't use async functions as Promise executors
Removeconsole.log,debugger, andalertstatements from production code
ThrowErrorobjects with descriptive messages, not strings or other values
Usetry-catchblocks meaningfully - don't catch errors just to rethrow them
Prefer early returns over nested conditionals for error cases
Extract complex conditions into well-named boolean variables
Use early returns to reduce nesting
Prefer simple conditionals over nested ternary operators
Don't useeval()or assign directly todocument.cookie
Avoid spread syntax in accumulators within loops
Use top-level regex literals instead of creating them in loops
Prefer specific imports over namespace imports
Use descriptive names for functions, variables, and types for meaningful naming
Add comments for complex logic, but prefer self-documenting code
Files:
apps/example/server/utils/agent-readability.tsapps/example/src/lib/docs-head.tspackages/leadtype/src/llm/llm.test.tspackages/leadtype/src/internal/docs-heading.tspackages/leadtype/src/llm/readability.tspackages/leadtype/src/llm/index.tsapps/example/src/components/docs-shell.tsxapps/example/src/components/table-of-contents.tsxapps/example/tests/e2e/smoke.e2e.tsapps/example/src/mdx-components.tsxpackages/leadtype/src/index.tsapps/example/src/lib/docs.tspackages/leadtype/src/search/search.tspackages/leadtype/src/llm/llm.ts
**/*.{test,spec}.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{test,spec}.{js,ts,jsx,tsx}: Write assertions insideit()ortest()blocks
Avoid done callbacks in async tests - use async/await instead
Don't use.onlyor.skipin committed code
Keep test suites reasonably flat - avoid excessivedescribenesting
Files:
packages/leadtype/src/llm/llm.test.ts
**/index.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid barrel files (index files that re-export everything)
Files:
packages/leadtype/src/llm/index.tspackages/leadtype/src/index.ts
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{jsx,tsx}: Use function components over class components in React
Call hooks at the top level only, never conditionally
Specify all dependencies in hook dependency arrays correctly
Use thekeyprop for elements in iterables (prefer unique IDs over array indices)
Nest children between opening and closing tags instead of passing as props
Don't define components inside other components
AvoiddangerouslySetInnerHTMLunless absolutely necessary
Use proper image components (e.g., Next.js<Image>) over<img>tags
Use Next.js<Image>component for images
Usenext/heador App Router metadata API for head elements in Next.js
Use Server Components for async data fetching instead of async Client Components in Next.js
Use ref as a prop instead ofReact.forwardRefin React 19+
Files:
apps/example/src/components/docs-shell.tsxapps/example/src/components/table-of-contents.tsxapps/example/src/mdx-components.tsx
**/*.{jsx,tsx,html}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{jsx,tsx,html}: Use semantic HTML and ARIA attributes for accessibility: provide meaningful alt text for images, use proper heading hierarchy, add labels for form inputs, include keyboard event handlers alongside mouse events, use semantic elements instead of divs with roles
Addrel="noopener"when usingtarget="_blank"on links
Files:
apps/example/src/components/docs-shell.tsxapps/example/src/components/table-of-contents.tsxapps/example/src/mdx-components.tsx
🪛 OpenGrep (1.20.0)
packages/leadtype/src/llm/llm.ts
[ERROR] 394-394: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🔍 Remote MCP Context7
Additional context found (concise, review-focused)
-
MDX: providerImportSource + useMDXComponents
- MDX supports a providerImportSource that makes a module-exported useMDXComponents() available to all MDX files at compile time; MDX-generated code merges defaults, provider components, and props.components (provider components are applied before per-file props) — confirms the PR’s use of a central useMDXComponents provider for heading overrides is the supported pattern.
-
MDX: useMDXComponents shape example
- Example shows useMDXComponents returning an MDXComponents map (e.g., h1..h6) which MDX will merge into rendered output — relevant to verifying heading override correctness.
-
React: useEffect event listeners & cleanup
- React docs show the recommended pattern: add window event listeners inside useEffect and remove them in the cleanup return to avoid leaks and duplicate listeners (also highlights dev double-invocation behavior). This applies directly to the TableOfContents component’s scroll/resize/hashchange listeners and their cleanup.
-
React: requestAnimationFrame throttling pattern / hook example
- React guidance includes a requestAnimationFrame-based loop with proper cancelation via cancelAnimationFrame in cleanup (example useAnimationLoop). This validates the PR’s use of requestAnimationFrame to throttle scroll-driven TOC updates, but also implies reviewers should confirm cancelAnimationFrame is called on unmount and when listeners re-register.
Cited lookups:
- Context7 resolve-library-id (MDX): /mdx-js/mdx
- Context7 resolve-library-id (React): /reactjs/react.dev
- MDX docs (useMDXComponents / providerImportSource):
- React docs (useEffect cleanup, requestAnimationFrame hook):
🔇 Additional comments (5)
apps/example/src/lib/docs-head.ts (1)
8-11: Replace the unsafe manifest double-cast withsatisfies.Line 11 still uses
as unknown as AgentReadabilityManifest, which bypasses structural checks.Proposed fix
const manifest = { ...agentReadability, - version: 1, -} as unknown as AgentReadabilityManifest; + version: 1 as const, +} satisfies AgentReadabilityManifest;As per coding guidelines, "Leverage TypeScript's type narrowing instead of type assertions" and "Use const assertions (
as const) for immutable values and literal types."#!/bin/bash # Verify whether unsafe AgentReadabilityManifest double-casts still exist. rg -nP 'as\s+unknown\s+as\s+AgentReadabilityManifest' --type ts --type tsxapps/example/server/utils/agent-readability.ts (1)
17-20: Apply the samesatisfiesfix here for manifest typing consistency.Line 20 repeats the same unsafe double-cast and should use compile-time structural validation instead.
Proposed fix
export const agentReadabilityManifest = { ...manifestJson, - version: 1, -} as unknown as AgentReadabilityManifest; + version: 1 as const, +} satisfies AgentReadabilityManifest;As per coding guidelines, "Leverage TypeScript's type narrowing instead of type assertions" and "Use const assertions (
as const) for immutable values and literal types."packages/leadtype/src/llm/llm.ts (1)
415-423: Duplicate headings still collapse onto the same hash.This is still generating identical
idvalues for repeated headings on the same page, so later TOC links resolve to the first occurrence instead of the matching section.apps/example/src/styles.css (1)
115-124: This still loses the specificity fight.The existing
.docs-prose a:not(...)rule is still more specific than.docs-prose [data-docs-heading-anchor], so the heading copy-link remains underlined unless the original prose-link selector also excludes[data-docs-heading-anchor]or this override gets comparable specificity.apps/example/src/components/table-of-contents.tsx (1)
131-133: Guard malformed hash decoding.
decodeURIComponent(window.location.hash.slice(1))still throws on invalid percent-encoding, so a bad hash can break TOC state syncing.
| type DocsTableOfContentsItem, | ||
| type DocsTableOfContentsOptions, | ||
| type DocsTableOfContentsPage, | ||
| defineDocsConfig, | ||
| extractDocsTableOfContents, | ||
| type ProductInfo, | ||
| type ResolveDocsTableOfContentsConfig, | ||
| resolveDocsTableOfContents, | ||
| slugifyDocsHeading, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Keep these TOC exports on leadtype/llm instead of widening the root entrypoint.
The root file explicitly says specialized surfaces stay on dedicated subpaths, but these additions expose more build-time/readability APIs from the top-level package anyway. That expands the general public surface for APIs consumers can already import from leadtype/llm, and it makes the root barrel harder to keep stable over time. As per coding guidelines, **/index.{js,ts,jsx,tsx}: Avoid barrel files (index files that re-export everything).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/leadtype/src/index.ts` around lines 12 - 20, The root barrel is
exporting TOC and docs utilities that should remain on the specialized subpath;
remove these exports (DocsTableOfContentsItem, DocsTableOfContentsOptions,
DocsTableOfContentsPage, defineDocsConfig, extractDocsTableOfContents,
ProductInfo, ResolveDocsTableOfContentsConfig, resolveDocsTableOfContents,
slugifyDocsHeading) from the root index.ts and re-export or keep them only from
the dedicated leadtype/llm entrypoint; update any internal imports or tests to
import from the leadtype/llm module instead of the root so the top-level public
surface remains minimal and stable.
| export type DocsNavigationPage = { | ||
| urlPath: string; | ||
| title: string; | ||
| description: string; | ||
| /** All group slugs the page declared (normalized). */ | ||
| groups: string[]; | ||
| toc: DocsTableOfContentsItem[]; | ||
| }; |
There was a problem hiding this comment.
Keep DocsNavigationPage.toc backward-compatible.
Making toc required here is a source-breaking change for any consumer that constructs DocsNavigationPage or AgentReadabilityManifest.navigation objects manually. Their existing object literals now fail type-checking even if they do not use TOC yet. Prefer toc?: DocsTableOfContentsItem[] in the public type and keep emitting [] from the generators, or treat this as a major-version change.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/leadtype/src/llm/readability.ts` around lines 79 - 86, The public
type DocsNavigationPage currently makes toc required which is a breaking change;
change its declaration to make toc optional (toc?: DocsTableOfContentsItem[]) so
existing consumers constructing DocsNavigationPage or
AgentReadabilityManifest.navigation object literals still type-check, and keep
generators emitting an empty array ([]) at runtime where appropriate to preserve
behavior.
Review surfaced four real bugs in the TOC slug contract; this addresses
the ones that affect users today.
- mdx-components.tsx: textFromChildren returned "" for ReactElement
children, so headings like `### `meta.json`` got no id while the TOC
extractor still emitted #meta-json — anchor links silently missed.
Now recurses through isValidElement(children).props.children.
- mdx-components.tsx: copyHeadingUrl called document.querySelector(hash),
which throws SyntaxError on digit-leading ids ("## 1. Generate the
artifacts" → "#1-..."). The throw was swallowed by the async catch so
scrollIntoView never ran. Switched to document.getElementById.
- styles.css: the new .docs-prose [data-docs-heading-anchor] no-underline
rule (specificity 0,2,0) was overridden by the existing prose link rule
(0,4,1), so heading anchors still rendered underlined. Added
:not([data-docs-heading-anchor]) to the underline selector.
- table-of-contents.tsx: decodeURIComponent on a malformed % sequence in
the URL hash throws URIError; wrapped in try/catch with a raw-hash
fallback.
- Updated the documented textFromChildren snippet in
docs/authoring/components.mdx to match the corrected version (so
copy-paste users don't inherit the bug).
- Added an e2e smoke test that asserts the inline-code heading id is
attached AND that clicking a digit-leading heading anchor scrolls
the heading into view.
Skipped two findings:
- Per-page dedup of duplicate heading slugs: no current docs page has
duplicate headings (scan returned only matches inside a fenced code
block in llm.mdx). Fix requires coordinated state in extractor and
every renderer, materially out of scope here.
- Replacing `as unknown as AgentReadabilityManifest` with `satisfies`:
the JSON import widens TS literal types (level: number vs 1|2|...|6,
version: number vs 1), so satisfies fails. The double-cast is the
pragmatic workaround for that gap; a proper fix needs a typed JSON
declaration.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@apps/example/src/mdx-components.tsx`:
- Around line 13-24: The helper textFromChildren incorrectly inserts synthetic
spaces when flattening arrays (Array.isArray(children) uses join(" ")), which
alters heading text and breaks slug/TOC alignment; update the array-flattening
logic in textFromChildren to concatenate elements without adding spaces (use
join("") or equivalent) so inline markup adjacent to text remains unchanged, and
ensure the isValidElement branch still recurses into children to preserve
existing behavior.
In `@docs/authoring/components.mdx`:
- Around line 52-63: The helper textFromChildren injects extra spaces because
Array.isArray branch uses children.map(...).join(" "), so change it to join("")
to preserve original adjacency; keep the existing handling for string/number and
isValidElement (children.props) but ensure you return empty string for
null/undefined/boolean to avoid introducing characters—update textFromChildren
accordingly (reference textFromChildren and isValidElement).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 072d1b4d-1bf2-40d3-a80c-cc88541892ce
⛔ Files ignored due to path filters (3)
apps/example/src/generated/agent-readability.jsonis excluded by!**/generated/**apps/example/src/generated/docs-search-content.jsonis excluded by!**/generated/**apps/example/src/generated/docs-search-index.jsonis excluded by!**/generated/**
📒 Files selected for processing (5)
apps/example/src/components/table-of-contents.tsxapps/example/src/mdx-components.tsxapps/example/src/styles.cssapps/example/tests/e2e/smoke.e2e.tsdocs/authoring/components.mdx
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use explicit types for function parameters and return values when they enhance clarity
Preferunknownoveranywhen the type is genuinely unknown
Use const assertions (as const) for immutable values and literal types
Leverage TypeScript's type narrowing instead of type assertions
Files:
apps/example/tests/e2e/smoke.e2e.tsapps/example/src/components/table-of-contents.tsxapps/example/src/mdx-components.tsx
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use meaningful variable names instead of magic numbers - extract constants with descriptive names
Use arrow functions for callbacks and short functions
Preferfor...ofloops over.forEach()and indexedforloops
Use optional chaining (?.) and nullish coalescing (??) for safer property access
Prefer template literals over string concatenation
Use destructuring for object and array assignments
Useconstby default,letonly when reassignment is needed, nevervar
Alwaysawaitpromises in async functions - don't forget to use the return value
Useasync/awaitsyntax instead of promise chains for better readability
Handle errors appropriately in async code with try-catch blocks
Don't use async functions as Promise executors
Removeconsole.log,debugger, andalertstatements from production code
ThrowErrorobjects with descriptive messages, not strings or other values
Usetry-catchblocks meaningfully - don't catch errors just to rethrow them
Prefer early returns over nested conditionals for error cases
Extract complex conditions into well-named boolean variables
Use early returns to reduce nesting
Prefer simple conditionals over nested ternary operators
Don't useeval()or assign directly todocument.cookie
Avoid spread syntax in accumulators within loops
Use top-level regex literals instead of creating them in loops
Prefer specific imports over namespace imports
Use descriptive names for functions, variables, and types for meaningful naming
Add comments for complex logic, but prefer self-documenting code
Files:
apps/example/tests/e2e/smoke.e2e.tsapps/example/src/components/table-of-contents.tsxapps/example/src/mdx-components.tsx
**/*.{jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{jsx,tsx}: Use function components over class components in React
Call hooks at the top level only, never conditionally
Specify all dependencies in hook dependency arrays correctly
Use thekeyprop for elements in iterables (prefer unique IDs over array indices)
Nest children between opening and closing tags instead of passing as props
Don't define components inside other components
AvoiddangerouslySetInnerHTMLunless absolutely necessary
Use proper image components (e.g., Next.js<Image>) over<img>tags
Use Next.js<Image>component for images
Usenext/heador App Router metadata API for head elements in Next.js
Use Server Components for async data fetching instead of async Client Components in Next.js
Use ref as a prop instead ofReact.forwardRefin React 19+
Files:
apps/example/src/components/table-of-contents.tsxapps/example/src/mdx-components.tsx
**/*.{jsx,tsx,html}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{jsx,tsx,html}: Use semantic HTML and ARIA attributes for accessibility: provide meaningful alt text for images, use proper heading hierarchy, add labels for form inputs, include keyboard event handlers alongside mouse events, use semantic elements instead of divs with roles
Addrel="noopener"when usingtarget="_blank"on links
Files:
apps/example/src/components/table-of-contents.tsxapps/example/src/mdx-components.tsx
🔍 Remote MCP Context7
Relevant facts found for reviewing this PR
-
MDX providerImportSource / useMDXComponents:
- MDX supports providerImportSource: compiled MDX will call an exported useMDXComponents() from that provider to inject/merge global components used by MDX pages. Confirmation and example provider pattern shown (useMDXComponents returns MDXComponents; providerImportSource compiles code that calls it). This validates the PR’s central useMDXComponents approach for heading overrides and that MDX merges provider components with per-file props.components.
-
useMDXComponents behavior:
- useMDXComponents(components?) returns the current MDX components and can merge additional components or a function that creates them — relevant for ensuring heading overrides in the PR will be applied in rendered MDX output.
-
React useEffect + event listener lifecycle:
- React docs recommend adding global listeners (scroll/resize/hashchange) inside useEffect and removing them in the cleanup return to avoid leaks and duplicate listeners (dev StrictMode can double-invoke effects). Reviewers should confirm the TableOfContents component registers listeners inside useEffect and removes them on cleanup.
-
requestAnimationFrame throttling and cleanup:
- React examples show using requestAnimationFrame for animations and cancelAnimationFrame in cleanup; confirm the TOC’s rAF-based throttling cancels pending frames on unmount and when listeners are re-registered.
Notes for focused review actions
- Verify MDX providerImportSource is configured such that useMDXComponents from the project’s provider is actually used at MDX compile time so slugifyDocsHeading-based heading IDs match extracted TOC slugs.
- Verify TableOfContents useEffect:
- listeners (scroll/resize/hashchange) are added in useEffect and removed in cleanup,
- any requestAnimationFrame is cancelled with cancelAnimationFrame on cleanup,
- handlers tolerate React dev double-invocation without leaking duplicate listeners.
Tools used
- Context7 resolve-library-id and query-docs (MDX) — for providerImportSource and useMDXComponents details.,
- Context7 resolve-library-id and query-docs (React) — for useEffect listener and requestAnimationFrame cleanup guidance.,
Two valid review findings on the TOC PR; three skipped (stale or wrong).
- packages/leadtype/src/index.ts: drop the seven TOC-related exports
this PR added to the root barrel (DocsTableOfContents{Item,Options,Page},
ResolveDocsTableOfContentsConfig, extractDocsTableOfContents,
resolveDocsTableOfContents, slugifyDocsHeading). They remain on the
dedicated `leadtype/llm` subpath. Keeps the top-level surface minimal
and consistent with the rest of the package layout. All existing
consumers (apps/example, tests) already import from `leadtype/llm`
or `leadtype/llm/readability`, so no internal updates needed.
- packages/leadtype/src/llm/llm.ts: drop `:` and `#` from
MARKDOWN_INLINE_PATTERN so titles like "API: Quick Start" and "C#"
survive `cleanHeadingText` for TOC display. Slugs are unchanged
because `slugifyDocsHeading` strips this punctuation when forming
the id. Updated the matching test expectation; no real docs page
currently has `:` or `#` in a heading, so generated artifacts are
effectively unchanged.
Skipped:
- copyHeadingUrl querySelector→getElementById: already fixed in 541d0fd.
- DocsNavigationPage.toc → optional: the field is always emitted by
the generators (no internal site constructs the type without it).
Package is pre-release (0.0.0); making it optional would force
defensive `?? []` at every consumer with no gain.
- textFromChildren `.join(" ")` → `.join("")`: would break slug parity.
The extractor's `cleanHeadingText` replaces inline markers with " ",
so `### Hello*world*` extracts to "Hello world". Joining renderer
children with "" would produce "Helloworld" and mismatch the slug.
Switch `textFromChildren` array branch from `.join(" ")` to `.join("")`
in both the example renderer and the documented snippet so adjacent
inline elements don't get a synthetic space inserted between them.
Verified slug parity with a probe against `extractDocsTableOfContents`:
for `## Hello *world*` the extractor produces "Hello world" (single
space, after WHITESPACE_PATTERN normalization). With either join
character the renderer's text is normalized by `slugifyDocsHeading`
to the same `hello-world` slug, so realistic headings are unaffected.
Empty-string join is the more honest semantic — MDX text nodes already
carry their own source spacing, the renderer shouldn't add more.
null/undefined/boolean already fall through to the catch-all `return ""`
branch, so no separate handling needed.
E2e smoke test still passes (`heading anchors handle inline markup and
digit-leading slugs`).
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/leadtype/src/llm/llm.ts (1)
415-423:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDeduplicate repeated heading anchors within a page.
Line 415 derives every anchor from
slugifyDocsHeading(title)without tracking prior occurrences. Two identical headings on the same page will both become the same hash, so later TOC entries link back to the first section instead of their own anchor.Suggested fix
export function extractDocsTableOfContents( content: string, page: Pick<SourceDoc, "absoluteUrl" | "urlPath">, options?: DocsTableOfContentsOptions ): DocsTableOfContentsItem[] { const { minLevel, maxLevel } = resolveTocOptions(options); const items: DocsTableOfContentsItem[] = []; const stack: DocsTableOfContentsItem[] = []; + const seenIds = new Map<string, number>(); let inCodeFence = false; @@ - const id = slugifyDocsHeading(title); + const baseId = slugifyDocsHeading(title); + const occurrence = (seenIds.get(baseId) ?? 0) + 1; + seenIds.set(baseId, occurrence); + const id = occurrence === 1 ? baseId : `${baseId}-${occurrence}`; const item: DocsTableOfContentsItem = { id, title, level,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/leadtype/src/llm/llm.ts` around lines 415 - 423, The TOC generation uses slugifyDocsHeading(title) to build anchors but does not ensure uniqueness, so duplicate headings get the same id; update the logic around slugifyDocsHeading and creation of DocsTableOfContentsItem to track per-page slug counts (e.g., a local Map keyed by page.urlPath or a current page slug map) and, when a slug already exists, append a numeric suffix (like -1, -2) to produce a unique id before calling withHash; ensure the unique id is stored in the item.id and used for urlWithHash/absoluteUrlWithHash so each repeated heading links to its own anchor.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/leadtype/src/llm/llm.ts`:
- Around line 385-387: The TOC extractor currently only toggles code-fence state
for backticks using FENCE_PATTERN and inCodeFence, so headings inside
triple-tilde fences leak through; update FENCE_PATTERN to match either backticks
or tildes (e.g., /^(`{3,}|~{3,})/), replace the boolean inCodeFence with a
tracked activeFence variable (type "`" | "~" | null), and in the loop use
trimmedLine.match(FENCE_PATTERN) to determine the fence character, set
activeFence = fenceChar when entering and null only when closing the same
fenceChar, then skip lines while activeFence is non-null; keep stripFrontmatter
and the rest of the heading logic unchanged so only matching fence types toggle
the fence state.
---
Duplicate comments:
In `@packages/leadtype/src/llm/llm.ts`:
- Around line 415-423: The TOC generation uses slugifyDocsHeading(title) to
build anchors but does not ensure uniqueness, so duplicate headings get the same
id; update the logic around slugifyDocsHeading and creation of
DocsTableOfContentsItem to track per-page slug counts (e.g., a local Map keyed
by page.urlPath or a current page slug map) and, when a slug already exists,
append a numeric suffix (like -1, -2) to produce a unique id before calling
withHash; ensure the unique id is stored in the item.id and used for
urlWithHash/absoluteUrlWithHash so each repeated heading links to its own
anchor.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5ebad3a1-2ee3-4f68-b180-bda4d841f037
⛔ Files ignored due to path filters (3)
apps/example/src/generated/agent-readability.jsonis excluded by!**/generated/**apps/example/src/generated/docs-search-content.jsonis excluded by!**/generated/**apps/example/src/generated/docs-search-index.jsonis excluded by!**/generated/**
📒 Files selected for processing (3)
packages/leadtype/src/index.tspackages/leadtype/src/llm/llm.test.tspackages/leadtype/src/llm/llm.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use explicit types for function parameters and return values when they enhance clarity
Preferunknownoveranywhen the type is genuinely unknown
Use const assertions (as const) for immutable values and literal types
Leverage TypeScript's type narrowing instead of type assertions
Files:
packages/leadtype/src/index.tspackages/leadtype/src/llm/llm.test.tspackages/leadtype/src/llm/llm.ts
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use meaningful variable names instead of magic numbers - extract constants with descriptive names
Use arrow functions for callbacks and short functions
Preferfor...ofloops over.forEach()and indexedforloops
Use optional chaining (?.) and nullish coalescing (??) for safer property access
Prefer template literals over string concatenation
Use destructuring for object and array assignments
Useconstby default,letonly when reassignment is needed, nevervar
Alwaysawaitpromises in async functions - don't forget to use the return value
Useasync/awaitsyntax instead of promise chains for better readability
Handle errors appropriately in async code with try-catch blocks
Don't use async functions as Promise executors
Removeconsole.log,debugger, andalertstatements from production code
ThrowErrorobjects with descriptive messages, not strings or other values
Usetry-catchblocks meaningfully - don't catch errors just to rethrow them
Prefer early returns over nested conditionals for error cases
Extract complex conditions into well-named boolean variables
Use early returns to reduce nesting
Prefer simple conditionals over nested ternary operators
Don't useeval()or assign directly todocument.cookie
Avoid spread syntax in accumulators within loops
Use top-level regex literals instead of creating them in loops
Prefer specific imports over namespace imports
Use descriptive names for functions, variables, and types for meaningful naming
Add comments for complex logic, but prefer self-documenting code
Files:
packages/leadtype/src/index.tspackages/leadtype/src/llm/llm.test.tspackages/leadtype/src/llm/llm.ts
**/index.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid barrel files (index files that re-export everything)
Files:
packages/leadtype/src/index.ts
**/*.{test,spec}.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{test,spec}.{js,ts,jsx,tsx}: Write assertions insideit()ortest()blocks
Avoid done callbacks in async tests - use async/await instead
Don't use.onlyor.skipin committed code
Keep test suites reasonably flat - avoid excessivedescribenesting
Files:
packages/leadtype/src/llm/llm.test.ts
🪛 OpenGrep (1.20.0)
packages/leadtype/src/llm/llm.ts
[ERROR] 394-394: Dynamic command passed to child_process.exec/execSync. Use child_process.execFile or spawn with an argument array instead.
(coderabbit.command-injection.exec-js)
🔍 Remote MCP Context7
Summary of additional facts relevant to reviewing the PR
-
MDX providerImportSource / useMDXComponents
- When MDX is compiled with providerImportSource, the compiled output imports and calls useMDXComponents() from that provider and merges its returned components with props.components (i.e., ..._provideComponents(), ...props.components). The provider module must export useMDXComponents for this to work.
-
React useEffect and global event listeners
- React guidance shows global listeners (scroll/resize/hashchange) should be added inside useEffect and removed in the returned cleanup to avoid leaks and duplicate subscriptions (important under Strict Mode which may double-invoke effects). Examples show addEventListener in the effect and removeEventListener in cleanup.
Notes for reviewers (grounded in the retrieved docs)
- Verify the repo’s MDX compilation is configured with providerImportSource that points to the module exporting useMDXComponents (so slugifyDocsHeading used by the MDX heading renderer is actually applied at compile/runtime). Evidence: compiled MDX merges provider components before props.components.
- Confirm TableOfContents registers scroll/resize/hashchange listeners inside useEffect and removes them in cleanup to avoid leaks and duplicate listeners in dev Strict Mode.
Sources used
- MDX docs: providerImportSource and compiled output / useMDXComponents examples.
- React docs: useEffect examples demonstrating adding/removing global event listeners and cleanup guidance.
🔇 Additional comments (1)
packages/leadtype/src/index.ts (1)
4-4: Good API-boundary clarification on root entrypoint.This comment clearly documents that TOC and slug helpers remain scoped to
leadtype/llm, which keeps the top-level surface minimal and stable.
1 participant
Summary
leadtype/llm:resolveDocsTableOfContents,extractDocsTableOfContents,DocsTableOfContentsItem, plus atoc: { minLevel, maxLevel }option onresolveDocsNavigationthat populates atocarray on every page in the manifest (defaults h2-h3).slugifyDocsHeadinginleadtype/llm/readability(Unicode-safe, fs-free) so renderers can keep HTML heading IDs in sync with extracted slugs.apps/examplegets aTableOfContentsreference component, MDX heading wiring viaslugifyDocsHeading, and a smoke test asserting the slug contract holds end-to-end.docs/authoring/components.mdxunder a new "Headings and table of contents" H2 (sibling to "The naming contract").docs/reference/llm.mdxis trimmed to API-only with a cross-link.The framing keeps leadtype framework-neutral: the package owns slug stability + manifest data; the user's app owns the React rendering, same stance as the existing MDX-component contract.
Test plan
bun testinpackages/leadtype(140 new lines of TOC extraction tests inllm.test.ts)bun run test:e2einapps/example(smoke test verifies the "On this page" sidebar links resolve to rendered heading IDs)/docs/authoring/componentslocally and confirm the new "Headings and table of contents" section appears in the page's own right-rail TOC — dogfood verification that the slug contract works on the page that documents it/docs/reference/llmand confirm the cross-link tocomponents#headings-and-table-of-contentsresolves🤖 Generated with Claude Code