Cache include resolution per conversion run#111
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a scoped include-resolution cache to MDX include handling and threads it through conversion, public exports, tests, and release notes. ChangesInclude-resolution caching
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 markdownlint-cli2 (0.22.1).changeset/cache-include-resolution.mdmarkdownlint-cli2 wrapper config was not available before execution Comment |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
One regression to fix before merge: on the resolveMdxFrontmatter path (no cache), this PR makes file.data become undefined instead of the usual {}, which can break any remark plugin that reads file.data. Details inline.
Reviewed changes — adds a per-conversion-run cache so repeated <include> / <import> targets share one raw file read and one parsed markdown AST.
- Scoped include cache — new
IncludeResolutionCache+createIncludeResolutionCache()ininclude.remark.ts;readIncludedFile()caches raw reads asPromise<string>,parseIncludedMarkdown()caches ASTs in aWeakMap<parser, Map<path, Root>>and returnsstructuredClonecopies so callers mutate freely without corrupting the cache. - Pipeline threading —
convert.tsforwards the cache throughprocessor.runvia the VFiledata._leadtypeIncludeCachekey;convertAllMdxshares one cache per batch,convertMdxFilecreates its own when none is supplied,writeMdxFileAsMarkdownpasses a fresh one. - Wider
remarkIncludesignature — acceptsstring[] | { basePaths?, cache? }(array form unchanged); new symbols re-exported fromleadtype/remarkandleadtype/mdx. - Tests + changeset — covers raw-read reuse, parse reuse, and per-section extraction from a shared AST.
ℹ️ VFile cache-injection path is untested
The new tests exercise only the pluginCache path (remarkInclude({ cache })). The path the production pipeline actually uses — cache delivered through file.data._leadtypeIncludeCache and read back via the fileCache branch in remarkInclude — has no direct coverage. That gap is why the file.data regression flagged inline slips past the suite.
Technical details
# VFile cache-injection path is untested
## Affected sites
- `packages/leadtype/src/remark/plugins/include.remark.test.ts:165-215` — only covers `.use(remarkInclude, { cache })`.
- `packages/leadtype/src/remark/plugins/include.remark.ts:682-686` — the `fileData` / `fileCache` branch, exercised only indirectly.
## Required outcome
- A test that drives the cache through `file.data._leadtypeIncludeCache` (i.e. via `convertMdxFile` / `convertAllMdx`, or by setting `data` on the run options) and asserts include expansion still works and stats accumulate.
- That test should also cover the no-cache case so a regression that sets `file.data` to `undefined` is caught.ℹ️ Nitpicks
packages/leadtype/src/remark/plugins/include.remark.ts:5— module JSDoc still describesremarkInclude(basePaths?); the signature is nowremarkInclude(options: string[] | { basePaths?, cache? }).packages/leadtype/src/remark/plugins/include.remark.ts:279—rawFileHitsis incremented beforereturn await cached, so a cached read that rejects still counts as a hit. Instrumentation-only, but the increment belongs after a successful await.
Claude Opus ｜ 𝕏
KayleeWilliams
force-pushed
the
feat/include-resolution-cache
branch
from
01f76f9 to
5f8b082
Compare
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — re-reviewed the commits since the prior pullfrog review (01f76f99). All three earlier findings are now addressed, and the new test and nit fixes introduce no regressions.
- Fixed the
file.dataclobber —runRemarkAstnow builds run options as...(fileData ? { data: fileData } : {}), so the no-cacheresolveMdxFrontmatterpath no longer passesdata: undefinedandfile.datakeeps the vfile default{}. - Closed the VFile coverage gap —
convert.test.tsadds "preserves VFile data as an object when no include cache is supplied" (regression guard for the bug above) and "shares include cache through VFile data during conversion" (drives the cache viafile.data._leadtypeIncludeCacheand assertsrawFileReads/rawFileHits/markdownParses/markdownParseHits). - Resolved both nits — module JSDoc now reads
remarkInclude(options?)with the legacy-array note, andrawFileHitsis incremented afterawait cachedso a rejected cached read no longer counts as a hit.
Claude Opus ｜ 𝕏
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/leadtype/src/convert/convert.ts (1)
1334-1370: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy liftThe batch cache still reparses the same include once per file.
convertAllMdx()now shares oneincludeResolutionCache, butprepareMdxConversion()still creates a fresh remark processor per file. Becauseinclude.remark.tskeysmarkdownAstByParserby parser identity, identical includes used by different files cannot hit the same parsed-AST entry. That means the new batch cache deduplicates raw reads, but not markdown parsing, which misses the stated per-batch parser-cache objective. Reuse a single processor/parser for the batch, or change the parsed-AST cache to use a stable parser key instead of the per-file parser object.🤖 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/convert/convert.ts` around lines 1334 - 1370, The batch-level include cache in convertAllMdx still misses parsed-AST reuse because prepareMdxConversion creates a fresh remark processor/parser for each file, so include.remark.ts cannot share markdownAstByParser across files. Update the batch flow to reuse a single processor/parser for the whole conversion run, or change the parsed-AST cache keying to a stable parser identifier instead of the per-file parser object, and ensure the shared includeResolutionCache is used with that same batch-scoped parser in convertMdxToMarkdown/prepareMdxConversion.
🤖 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/remark/plugins/include.remark.ts`:
- Around line 657-663: The nested include error reporting is using only the
top-level file.path, so failures inside included partials lose the immediate
source context. Update includeContentAsMarkdown and the nested include metadata
handling to thread resolution.resolvedPath alongside baseDir, and prefer that
path when setting sourcePath for logs and failures. Make the same change in the
nested-include flow referenced by includeContentAsMarkdown so the reporting uses
the partial that contains the failing tag instead of always the outer page.
---
Outside diff comments:
In `@packages/leadtype/src/convert/convert.ts`:
- Around line 1334-1370: The batch-level include cache in convertAllMdx still
misses parsed-AST reuse because prepareMdxConversion creates a fresh remark
processor/parser for each file, so include.remark.ts cannot share
markdownAstByParser across files. Update the batch flow to reuse a single
processor/parser for the whole conversion run, or change the parsed-AST cache
keying to a stable parser identifier instead of the per-file parser object, and
ensure the shared includeResolutionCache is used with that same batch-scoped
parser in convertMdxToMarkdown/prepareMdxConversion.
🪄 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: 7229a7ae-51d1-419b-b244-6265c9e4d89c
📒 Files selected for processing (7)
.changeset/cache-include-resolution.mdpackages/leadtype/src/convert/convert.test.tspackages/leadtype/src/convert/convert.tspackages/leadtype/src/mdx/index.tspackages/leadtype/src/remark/index.tspackages/leadtype/src/remark/plugins/include.remark.test.tspackages/leadtype/src/remark/plugins/include.remark.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/remark/index.tspackages/leadtype/src/mdx/index.tspackages/leadtype/src/convert/convert.test.tspackages/leadtype/src/remark/plugins/include.remark.test.tspackages/leadtype/src/convert/convert.tspackages/leadtype/src/remark/plugins/include.remark.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/remark/index.tspackages/leadtype/src/mdx/index.tspackages/leadtype/src/convert/convert.test.tspackages/leadtype/src/remark/plugins/include.remark.test.tspackages/leadtype/src/convert/convert.tspackages/leadtype/src/remark/plugins/include.remark.ts
**/index.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid barrel files (index files that re-export everything)
Files:
packages/leadtype/src/remark/index.tspackages/leadtype/src/mdx/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/convert/convert.test.tspackages/leadtype/src/remark/plugins/include.remark.test.ts
🧠 Learnings (1)
📚 Learning: 2026-06-09T18:30:08.038Z
Learnt from: KayleeWilliams
Repo: inthhq/leadtype PR: 97
File: .changeset/search-prototype-safety-and-scaling.md:5-5
Timestamp: 2026-06-09T18:30:08.038Z
Learning: In this repo, `.changeset/*.md` files must not start the body with an H1/first-line heading (`#`) immediately after the YAML frontmatter. The changesets tool inlines the body as bullet entries into `CHANGELOG.md` during release, and a leading `#` heading would break the generated changelog format. As a result, MD041 (`first-line-heading`) warnings for files under `.changeset/` are expected false positives and should be ignored.
Applied to files:
.changeset/cache-include-resolution.md
🪛 ast-grep (0.44.0)
packages/leadtype/src/convert/convert.test.ts
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
packages/leadtype/src/convert/convert.ts
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🪛 markdownlint-cli2 (0.22.1)
.changeset/cache-include-resolution.md
[warning] 5-5: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🔇 Additional comments (8)
packages/leadtype/src/convert/convert.ts (1)
25-28: LGTM!Also applies to: 407-407, 494-504, 915-926, 981-988, 1052-1058, 1291-1291
packages/leadtype/src/remark/index.ts (1)
47-53: LGTM!packages/leadtype/src/mdx/index.ts (1)
33-39: LGTM!packages/leadtype/src/remark/plugins/include.remark.test.ts (1)
4-11: LGTM!Also applies to: 149-162, 165-216
packages/leadtype/src/convert/convert.test.ts (1)
9-17: LGTM!Also applies to: 352-393
.changeset/cache-include-resolution.md (1)
1-17: LGTM!packages/leadtype/src/remark/plugins/include.remark.ts (2)
5-8: LGTM!Also applies to: 32-59, 270-291, 298-323, 371-387, 481-482, 527-527, 566-568, 600-614, 666-688
294-295: 🩺 Stability & AvailabilityNo issue:
structuredCloneis available on the declared Node 22+ runtime.> Likely an incorrect or invalid review comment.
KayleeWilliams
force-pushed
the
feat/include-resolution-cache
branch
from
5f8b082 to
e02aa33
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/convert/convert.test.ts`:
- Around line 358-360: The assertion in the dataReaderPlugin callback is too
strict because resolveMdxFrontmatter forwards prepared.data and VFile.data may
contain unrelated plugin metadata; update the test to verify the shared
file.data bag is an object and that includeResolutionCache is not present,
rather than expecting exact emptiness. Keep the change localized to the
convert.test.ts case around dataReaderPlugin and the resolveMdxFrontmatter flow.
🪄 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: cae60748-e4e1-4e4d-b867-ec9b039097c4
📒 Files selected for processing (8)
.changeset/cache-include-resolution.mddocs/changelog/0-4.mdxpackages/leadtype/src/convert/convert.test.tspackages/leadtype/src/convert/convert.tspackages/leadtype/src/mdx/index.tspackages/leadtype/src/remark/index.tspackages/leadtype/src/remark/plugins/include.remark.test.tspackages/leadtype/src/remark/plugins/include.remark.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/remark/index.tspackages/leadtype/src/mdx/index.tspackages/leadtype/src/remark/plugins/include.remark.test.tspackages/leadtype/src/convert/convert.test.tspackages/leadtype/src/convert/convert.tspackages/leadtype/src/remark/plugins/include.remark.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/remark/index.tspackages/leadtype/src/mdx/index.tspackages/leadtype/src/remark/plugins/include.remark.test.tspackages/leadtype/src/convert/convert.test.tspackages/leadtype/src/convert/convert.tspackages/leadtype/src/remark/plugins/include.remark.ts
**/index.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Avoid barrel files (index files that re-export everything)
Files:
packages/leadtype/src/remark/index.tspackages/leadtype/src/mdx/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/remark/plugins/include.remark.test.tspackages/leadtype/src/convert/convert.test.ts
🧠 Learnings (1)
📚 Learning: 2026-06-09T18:30:08.038Z
Learnt from: KayleeWilliams
Repo: inthhq/leadtype PR: 97
File: .changeset/search-prototype-safety-and-scaling.md:5-5
Timestamp: 2026-06-09T18:30:08.038Z
Learning: In this repo, `.changeset/*.md` files must not start the body with an H1/first-line heading (`#`) immediately after the YAML frontmatter. The changesets tool inlines the body as bullet entries into `CHANGELOG.md` during release, and a leading `#` heading would break the generated changelog format. As a result, MD041 (`first-line-heading`) warnings for files under `.changeset/` are expected false positives and should be ignored.
Applied to files:
.changeset/cache-include-resolution.md
🪛 ast-grep (0.44.0)
packages/leadtype/src/convert/convert.test.ts
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
packages/leadtype/src/convert/convert.ts
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🔇 Additional comments (12)
packages/leadtype/src/remark/plugins/include.remark.test.ts (1)
4-11: LGTM!Also applies to: 149-216
packages/leadtype/src/convert/convert.test.ts (1)
9-17: LGTM!Also applies to: 369-393
.changeset/cache-include-resolution.md (1)
1-17: LGTM!docs/changelog/0-4.mdx (1)
16-16: LGTM!packages/leadtype/src/remark/plugins/include.remark.ts (4)
657-663: 🩺 Stability & AvailabilityNested include failures still report the top-level page, not the failing partial.
sourcePathis threaded asfile.path(Line 740) andincludeContentAsMarkdownonly annotates nested includes withbaseDir, notresolution.resolvedPath. So a failure inside a nested partial still logs the outer page assource. Threadresolution.resolvedPathinto the nested-include metadata (alongsidebaseDir) and prefer it when settingsourcePath.Also applies to: 738-740
32-59: LGTM!Also applies to: 270-292
666-688: LGTM!
294-324: 🩺 Stability & AvailabilityNo action needed for
structuredClonehere. The package targets Node 22+, so this global is available.> Likely an incorrect or invalid review comment.packages/leadtype/src/convert/convert.ts (2)
489-508: LGTM!
915-926: LGTM!Also applies to: 981-988, 1052-1058, 1291-1291, 1334-1334, 1370-1370
packages/leadtype/src/remark/index.ts (1)
47-53: LGTM!packages/leadtype/src/mdx/index.ts (1)
33-39: LGTM!
| const dataReaderPlugin = | ||
| () => (_tree: unknown, file: { data: unknown }) => { | ||
| expect(file.data).toEqual({}); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Assert the missing cache key, not exact empty file.data.
resolveMdxFrontmatter() forwards prepared.data, and VFile.data is a shared plugin bag. toEqual({}) will fail as soon as any unrelated metadata is added, even though the no-cache contract still holds. Assert that file.data is an object and that includeResolutionCache is absent instead.
Suggested assertion change
const dataReaderPlugin =
() => (_tree: unknown, file: { data: unknown }) => {
- expect(file.data).toEqual({});
+ expect(file.data).toEqual(expect.any(Object));
+ expect(file.data).not.toHaveProperty("includeResolutionCache");
sawVFileData = true;
};📝 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 dataReaderPlugin = | |
| () => (_tree: unknown, file: { data: unknown }) => { | |
| expect(file.data).toEqual({}); | |
| const dataReaderPlugin = | |
| () => (_tree: unknown, file: { data: unknown }) => { | |
| expect(file.data).toEqual(expect.any(Object)); | |
| expect(file.data).not.toHaveProperty("includeResolutionCache"); | |
| sawVFileData = true; | |
| }; |
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { execFile } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 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/convert/convert.test.ts` around lines 358 - 360, The
assertion in the dataReaderPlugin callback is too strict because
resolveMdxFrontmatter forwards prepared.data and VFile.data may contain
unrelated plugin metadata; update the test to verify the shared file.data bag is
an object and that includeResolutionCache is not present, rather than expecting
exact emptiness. Keep the change localized to the convert.test.ts case around
dataReaderPlugin and the resolveMdxFrontmatter flow.
1 participant
Summary
<include>/<import>targets.convertAllMdx()batch while keeping single-file conversions scoped to their own cache.createIncludeResolutionCache()and cache types fromleadtype/remarkandleadtype/mdxfor instrumentation.Notes
Current docs and the c15t fixture do not contain repeated real include nodes, so the current repo workload does not show a material production-docs speedup. A synthetic 200-page repeated-include benchmark showed one raw read, one markdown parse, and about a 5.9x speedup in include expansion time.
Fixes #109.
Validation
bun x ultracite fix packages/leadtype/src/remark/plugins/include.remark.ts packages/leadtype/src/remark/plugins/include.remark.test.ts packages/leadtype/src/convert/convert.ts packages/leadtype/src/remark/index.ts packages/leadtype/src/mdx/index.ts .changeset/cache-include-resolution.mdbun test packages/leadtype/src/remark/plugins/include.remark.test.ts packages/leadtype/src/convert/convert.test.tsbun run check-typesfrompackages/leadtypebun run testfrompackages/leadtype