Resolve the project once instead of in every command - #167
Resolve the project once instead of in every command#167KayleeWilliams wants to merge 5 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Three paths that worked before this PR now fail: createDocsProject({ configPath }) resolves a content root one directory too deep, a fresh leadtype init scaffold throws on first run because nothing installs jiti, and leadtype nav refuses a project that has no config file. None of the three has test coverage.
Reviewed changes
Reviewed the new resolveProject pipeline and every consumer it replaces, tracing the deleted per-command assembly to confirm behaviour was preserved.
-
New shared resolution pipeline —
config/project.tsdiscovers the config, applies source-owned inheritance, normalizes, derives what wasn't authored, and resolves each collection's content dir through the sync cache. -
Environmental failures become diagnostics —
ProjectDiagnosticcarrieslevel,collection,owner, andfix, sodoctorreports and continues whilecreateDocsProjectthrows on the firsterror. -
Normalization runs twice — inheritance changes what the collections are, so the config is re-normalized, with
sources,deprecations, and per-collectionsourceIdcarried forward from the first pass. -
createDocsProjectconfig is now optional — apps can omitconfigand let discovery findleadtype.config.*ordocs.config.*fromcwd. -
doctor,nav,sync, andlintread the resolved project —inspectNavigation,applySourceInheritance, andresolveCollectionDirare deleted in favour ofcollection.navigationOrigin/collection.contentDir. -
Scaffolds stop importing the config — all four
initplans now emit a barecreateDocsProject({ baseUrl })and rely on discovery.
⚠️ generate never reads the resolved project the changeset says it reads
generate.ts still hand-assembles syncCollections → inheritCollectionSourceConfigs → resolveDocsSourcesFromCollections → loadDocsConfig, and never re-normalizes after inheritance — the exact second pass resolveProject was built to add. Meanwhile the changeset and docs/concepts/config-model.mdx both state that generate now reads one resolved project. doctor's job is to predict what generate produces, so this is the one pairing where a divergent pipeline matters most, and it is the pairing that still diverges.
Technical details
# `generate` bypasses `resolveProject`
## Affected sites
- `packages/leadtype/src/cli/generate.ts` ~1689-1715 — `syncCollections(...)`,
`inheritCollectionSourceConfigs(loadedConfig.config.collections, configDir)`,
`resolveDocsSourcesFromCollections(...)`, then `loadDocsConfig({ docsDirs })`.
No call to `resolveProject`, and no re-normalization after inheritance.
- `.changeset/resolve-project.md` — claims `generate`, `doctor`, and `nav` "now
read one resolved project".
- `docs/concepts/config-model.mdx` — same claim, now published documentation.
## Required outcome
Either `generate` resolves through `resolveProject` like the other commands, or
the changeset and `config-model.mdx` are corrected so they do not promise a
convergence that has not happened. Shipping the doc claim without the code
change is the outcome to avoid: a reader who trusts it will assume `doctor` and
`generate` cannot disagree about inherited navigation, and they still can.
## Open questions for the human
- Is the `generate` migration a deliberate follow-up PR, or an oversight? If
deliberate, it would help to say so in the changeset rather than describing
the end state as already reached.
- `generate` skips the second normalization pass. Does any currently-shipping
config reach `generate` with post-inheritance collections that need
re-canonicalizing (deprecated field names inside a source-owned
`docs.config.*`, for instance)? That would be a real behavioural split between
`doctor` and `generate` today, not just a docs inaccuracy.ℹ️ Nitpicks
-
doctor.ts~611-626:inspectOutputsnow globs**/*.{md,mdx}per collection instead of reusing the already-filtered file list, soinclude/excludeare ignored when deciding freshness (editing an excluded file marks artifacts stale) and every collection is globbed twice per run. -
config/project.ts~331-341: the cherry-pick after re-normalization carriescollections,sources, anddeprecationsforward but not top-levelprovenance. For asources-authored config the second pass seessources: undefinedandcollections: {...}, soprovenance.sourcesdisappears andprovenance.collectionsis reported as explicitly authored. This is user-visible throughdoctor --json(doctor.ts:569). -
config/project.ts~384-386: whenoptions.infer === falseand nothing was authored,navigationOriginis still set to"inferred"— labelling "nothing was derived" as "derived". The test atproject.test.ts:122only assertsnavigationisundefined, so the label isn't pinned either way.
Claude Opus ｜ 𝕏
56aaa7f to
008150a
Compare
There was a problem hiding this comment.
ℹ️ The delta itself is clean — I verified the two behavioural claims it makes rather than taking them on faith. One inline note, and one sharpening of an already-open thread. All three prior threads remain open; nothing in these commits addressed them.
Reviewed changes
Since the prior pullfrog review the branch was rebased (56aaa7f → 65e0e0c, content-identical for packages/leadtype/src) and one new commit landed, so the reviewable delta is 008150a alone. In it you:
- made
createDocsProject's parameter optional (input: CreateDocsProjectConfig<TFrontmatter> = {}), socreateDocsProject()now takes no arguments at all; - migrated
apps/fumadocs-example/lib/source.tsoff its../../../docs/docs.configimport ontocwd: repoRootdiscovery — this removed the last first-party caller that passedconfigPath; - rewrote the published fumadocs snippet in
docs/integrations/integrate-with-fumadocs.mdxas a barecreateDocsProject()and dropped its// @noErrorstwoslash directive; - refreshed the
docs/paths.lock.jsonhashes for the pages you touched.
Two things I checked rather than assumed:
- Dropping
// @noErrorsis safe.docs.config.tssetslint: { snippets: { typecheck: true } }and CI gates onleadtype lint docs --error-unknown --max-warnings 0, so those fences are now really typechecked. I built the package and ran that exact command: all 53 files pass. The directive was only ever needed for the removed relative../docs/docs.configimport. cwddiscovery works in both examples. I executedcreateDocsProject({ cwd: repoRoot })fromapps/fumadocs-exampleunder plain node, andcreateDocsProject()from the repo root under bun. Both discoverdocs/docs.config.ts, report zero diagnostics,navigationOrigin: "explicit", and list 55 pages.
ℹ️ configPath is worse than the open thread describes, and now has no callers left to notice
The still-open thread on src/project/index.ts covers the content-dir regression: resolveProject never inspects the config basename, so configPath: "/repo/docs/docs.config.ts" resolves contentDir to /repo/docs/docs where the deleted configIsSourceOwned branch gave /repo/docs. Reading config/project.ts closely, the option is broken a second, more basic way — and 008150a removes the last caller who would have hit it.
config/project.ts branches on options.config, not on options.configPath. Pass configPath without config and the else branch runs loadDocsConfig({ cwd: rootDir }): the file you named is never opened. What you get is ordinary discovery rooted at dirname(configPath) — which usually finds the same file by luck, and silently finds a different one when it doesn't (e.g. a leadtype.config.ts sitting beside it wins the discovery order).
Not a request to fix it in this PR, but the option is now undocumented-by-divergence and untested: the JSDoc on createDocsProject still describes the pre-PR contract, docs/pipeline/use-the-source-primitive.mdx dropped its configPath paragraph entirely, and src/config/project.test.ts — thorough as it is on cwd discovery — has no case that passes configPath. Deleting the option outright would be a defensible call.
Technical details
packages/leadtype/src/config/project.ts256-266 —options.config ? { ...normalizeDocsConfig(options.config, …) } : await loadDocsConfig({ cwd: rootDir, docsDirs }).options.configPathis consumed only inside the then branch.packages/leadtype/src/project/index.ts156-162 — the forwarding block derivescwd: path.dirname(input.configPath)and passesconfigPaththrough, which is what makes the miss look like a working path.packages/leadtype/src/project/index.ts98-115 — JSDoc still promises "adocs.config.*sits inside the docs directory, aleadtype.config.*sits at the project root above it".docs/pipeline/use-the-source-primitive.mdx~50 — theconfigPathparagraph is removed in this PR, so prose and type no longer agree.
Claude Opus ｜ 𝕏
| configPath: "docs/docs.config.ts", | ||
| }); | ||
|
|
||
| const project = await createDocsProject(); |
There was a problem hiding this comment.
createDocsProject() with no arguments means discovery, and discovery of a .ts config goes through importConfigModule, which does a literal await import("jiti"). jiti is an optional peer of leadtype, and the install line for this page is bun add leadtype fumadocs-core fumadocs-ui — so someone following this guide verbatim gets a module-not-found at first render.
Worth noting this isn't new-in-kind: the CLI (generate, lint, doctor) already loaded .ts configs through the same helper, so jiti was already a de-facto requirement for any TS-config consumer. What changed is that it's now an app runtime dependency, and this snippet is a second published site of the gap (the leadtype init scaffolds are the first — that's the open thread on src/cli/init-templates.ts). Either add jiti to the install line here, or promote it to a real dependency and close both at once.
Technical details
docs/integrations/integrate-with-fumadocs.mdx13 / 57 — install line, nojiti.packages/leadtype/src/config/inherit.ts268-286 —importConfigModule; the literalawait import("jiti")survives intodist/_shared/project-*.jsverbatim.packages/leadtype/package.json—jitiis a devDependency (231), a peer>=2.0.0(252), andpeerDependenciesMeta.jiti.optional(278).
One thing I could not verify: whether next build --webpack for apps/fumadocs-example still compiles now that that literal dynamic import is reachable from a bundled server module. CI doesn't build the example apps, and my background build was killed before webpack ran — so treat this as an open question, not a claim.
008150a to
e8b81ad
Compare
Answering "what is this project?" takes five ordered steps: discover the
config, apply source-owned inheritance, normalize to canonical names, derive
what wasn't authored, and resolve each collection's content directory through
the sync cache. Every command needs all five, and each assembled them by hand.
The cost was not hypothetical. `doctor` and `nav` shipped with the *same two*
bugs: both skipped inheritance, so a project whose navigation lives in its
source repo reported as "inferred" with a filesystem-derived tree the real
build never uses; and both resolved a remote collection's `dir` against the
config directory instead of its checkout, so every page vanished. Two commands,
written days apart, same two omissions. That is a design problem, not two
mistakes.
`resolveProject()` runs the pipeline once and the commands read its result.
It throws only for a genuinely malformed config — there is no project to
describe. Everything environmental is a diagnostic on the result, carrying a
stable id, the owning config field, and the command that fixes it. That split
is what lets one function serve both kinds of caller: doctor reports an
unsynced source and keeps going, while `createDocsProject` refuses to hand a
renderer a source it cannot read. Only the caller knows which is right, so the
resolver doesn't decide.
Two details worth knowing, both found by running it against the real c15t
project rather than a fixture. Deprecations come from the load-time
normalization, because aliases are folded there and a second pass over the
canonical config correctly finds none — reporting none would tell a legacy
config it has nothing to migrate. And the acquisition graph comes from that
same first pass: normalization expands `sources` into `collections`, so only
the first pass ever sees authored source names, and re-deriving reported c15t's
source as `repo#ref` instead of `c15t`.
Config loading moves out of `cli/generate.ts` into the config module. The
runtime needs it — `createDocsProject` and `doctor` both ask which config
describes a project — and reaching through the generate pipeline to ask would
drag staging and conversion into an app bundle.
`createDocsProject()` now discovers the config itself, so an app that has one
doesn't import it just to hand it straight back:
export const source = await createDocsProject({ baseUrl });
The scaffolds and the Astro example use that shape. Docs make the single-repo
path unambiguous: use `defineDocsConfig`; ownership only becomes a question
once a second repo is involved.
`config` became optional when the project learned to discover it, but the options object itself stayed required, so the shortest correct call — `createDocsProject()` — did not typecheck. Our own snippet typechecking caught it in the fumadocs integration page before a user would have. Also simplifies the fumadocs example onto config discovery now that this branch provides it.
Seven findings, all of the same shape: a config field that generation honours
and the runtime silently ignored — which is the drift this primitive exists to
end, reappearing inside it.
The two that matter most:
`include`/`exclude` were never applied. They are a page-existence filter, not
a display filter, so an author excluding `drafts/**` got them kept out of the
build and served by the site — publishing exactly the content the field
withholds. `createDocsSource` now takes them and applies them in its glob.
Cross-collection slug lookup was first-declared-wins. Two collections each
holding `overview.mdx` both produce `["overview"]`, and `index.mdx` produces
`[]` in every collection, so `loadPage(["overview"])` silently returned
whichever was declared first — and adapters build static params from `slug`,
so a route resolved to another collection's content. Route paths are unique by
construction and are now tried first; an ambiguous local slug throws naming
both collections rather than guessing.
The rest: top-level `mounts` were dropped for multi-collection projects, so a
site-wide remap applied to the artifacts and not the site; `typeTableBasePath`
had no config fallback while its sibling `typeTableStrict` did, so
`<AutoTypeTable>` resolved correctly at build time and not at render time; a
`flatteners` spread evaluated to `{}` on both branches and did nothing; and a
cache checked out with fewer sparse paths than the config asks for passed every
validity check, degrading type tables to nothing with no error — `sync` already
rejects that cache, and now so does the runtime.
`openapi` alongside `collections` now throws. Generation emits those pages
regardless, so silently skipping them left the site serving fewer routes than
its own sitemap advertised — the incident this whole branch was written about.
`FumadocsSourceConfig` was a plain union, so TypeScript relaxed excess-property
checking across members and `{ source, typeTableBasePath }` compiled while
dropping the second key. The arms are now mutually exclusive, with a test that
fails to compile if that regresses.
**nav pins could be silently discarded.** Group assembly is first-entry-wins by urlPath, so a page an earlier entry already placed swallowed a later entry's pin: the pin resolved, reordered within its own expansion, and never reached the tree. That now throws naming the page and the conflict. `pin` also accepts a bare string like `exclude` — `pin: "setup"` used to iterate character by character and fail with `Nav pin "s" did not match` — and is validated, so a wrong-typed pin from a JS or inherited config can't reach the resolver. **The unplaced check silently switched itself off for inherited trees.** The `origin !== "explicit"` shortcut was safe while every other origin had empty root entries; `inherited` is the first one carrying real ones, so the guard short-circuited and every glob-placed page in an inherited tree read as unplaced. **Inheritance was all-or-nothing.** `inheritCollectionSourceConfigs` throws on the first unreadable source config, and the whole result was discarded — so one unsynced collection degraded every other collection's tree to a filesystem-derived one. It now runs per collection. And an unreadable source config is an `error`, not a `warn`: `generate` throws there, so exiting 0 let CI gate on doctor and pass a project the build then fails on. **Doctor reported a cache path that can never exist.** `sync` derives it through `repositorySlug`; doctor hand-joined the raw URL, so every unpinned source read "not synced" while the collection checks reported nothing wrong — one report contradicting itself. **Doctor resolved navigation from different inputs than generate.** Missing i18n made every translation its own page and report as unplaced; per-collection rather than globally merged groups made a page grouped by a sibling collection an `error`, which changes the exit code; and include/exclude meant doctor read a different file set than generate stages, so excluded pages read as unplaced. **Provenance contradicted itself.** A field taken from a source repo reported `origin: "explicit"` in the same report that called the tree `inherited`. **resolveProject regressions.** `configPath` stopped fixing the content root — `docs.config.ts` resolved to `<dir>/docs/docs` — because the root was derived as `dirname(configPath)` without consulting the basename. The scaffold needs `jiti` now that it discovers a `.ts` config at runtime rather than importing it through the app's bundler. And `nav` refused a project with no config, a state `doctor` supports and reports.
e8b81ad to
0d7ca46
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Two of the parity fixes added in these commits open fresh generate-vs-runtime splits — the exact failure class this PR exists to close. Both are inline. The three prior threads the branch addressed are now replied to and resolved; the jiti install-line thread stays open.
Reviewed changes
checkout_pr couldn't compute an incremental diff (e8b81ad was orphaned by the rebase and isn't reachable in the shallow checkout), so I derived the delta as 14bd791..0d7ca46. In it you:
- derived the project root from the config basename (
projectRootForConfig), fixing thecontentDirregression from the openproject/index.tsthread, with a test; - added
source.cache-narrowand promotedsource.inherit-failedtolevel: "error", moving inheritance failure to per-collection; - added
include/excludepass-through tocreateDocsSourceand todoctor's newcountCollectionPages; - rewrote
doctor'sinspectNavigationto pass every collection'sgroupsplus top-leveli18n, and to skip the unrepresented-page check for filtered collections; - added
reportInferredTreetonav,RUNTIME_CONFIG_DEPS = ["jiti"]to all fourinitplans, a pin-conflict throw pluspin: string | string[], anopenapi+collectionsrefusal, and a mutually-exclusiveFumadocsSourceConfigunion.
Verified rather than assumed — none of these are findings, listing them so they don't get re-litigated:
stampInherited'sinheritedFrom: collectionKeymatches the documented meaning inconfig/types.ts:52-53.defaultCacheDir(source.repository, source.ref)indoctor.ts:531matchessync.ts:139exactly.- The pin-conflict throw only fires where the pin was already a silent no-op pre-PR (
buildNavigationGroupFromNavdropped the duplicate atllm.ts:3661), so it converts a silent no-op into a loud error rather than rejecting configs that worked.authoring.test.ts(9),nav.test.ts(16) andllm.test.ts(86) pass;docs/docs.config.tsuses noinclude/pinat all. - The
pin: string | string[]widening is complete — the only readers arellm.tsandvalidateDocsNavPageEntry(config/inherit.ts:154-161), both updated; no array-only schema survives. - Removing
resetConfigDeprecationWarningsleaves zero references repo-wide, andconfig/load.tsis a faithful lift of the validators out ofcli/generate.ts.
generate still doesn't read the resolved project, and 0d7ca46 re-added a second copy of the assembly
The changeset says generate, doctor, nav and createDocsProject "now read one resolved project", and docs/concepts/config-model.mdx:139 says "resolveProject() is the one function that runs them". generate calls neither resolveProject nor anything in config/project.ts. This commit went the other way: generate.ts:1696-1720 now re-implements the inheritance→re-normalize→cherry-pick sequence inline.
That second copy already diverges from the original on three axes, so the prose describes a state the code isn't in:
resolveProject |
generate.ts:1696-1720 |
|
|---|---|---|
| inheritance | per collection, one failure isolated | whole-map, throws on the first |
| failure surface | source.inherit-failed diagnostic |
exception |
| provenance | stampInherited per field |
not stamped |
I checked whether the missing per-collection sourceId cherry-pick matters and it doesn't — generate reads only .resolved.sources (2362) and .deprecations. So this is a maintenance and accuracy problem, not a live bug. But it's the specific thing the changeset's own argument warns about ("two commands, written days apart, same two omissions"), and the two inline findings below are both instances of it. Either route generate through resolveProject, or soften the docs to say which commands actually share the pipeline.
nit: source.cache-narrow also rejects a cache that is a strict superset
sameSparse(undefined, ["docs"]) is false (sync/sync.ts:178-183), so a fully-cloned cache plus a newly-added sparse: list in config trips source.cache-narrow — and the message reads "was checked out with the full repository, but the config asks for [docs]", which contradicts the id and the helper's own doc comment ("a checkout narrower than the config asks for"). A full checkout contains everything the collection needs. sync --refresh clears it, so this is a nuisance rather than data loss; the new test only covers the true-positive direction (["docs"] vs ["docs","packages"]).
Claude Opus ｜ 𝕏
| : {}), | ||
| // Site-wide `mounts` apply to every collection; the collection's own | ||
| // come first, matching the order generation composes them in. | ||
| mounts: collectionMounts(collection.routePrefix, [ |
There was a problem hiding this comment.
Site-wide config.mounts get applied to every collection here, resolved against that collection's own content root. generate resolves the same mounts once, against the merged staged tree, where each non-default collection's files already sit under mountPath. The two agree only when routePrefix === "/docs" — which is exactly the shape the new test pins.
Worked example:
mounts: [{ pathPrefix: "legal", urlPrefix: "/legal" }],
collections: {
docs: { dir: "content/docs", routePrefix: "/docs" },
guides: { dir: "content/guides", routePrefix: "/guides" },
}For content/guides/legal/refund.mdx:
- runtime —
collectionMounts("/guides", [{pathPrefix:"legal",…}])yields[{"","/guides"},{"legal","/legal"}]; longest-prefix-wins pickslegal→ served at/legal/refund. - generate — the file stages to
guides/legal/refund.mdx, and the global mount list is[{pathPrefix:"guides",urlPrefix:"/guides"},{pathPrefix:"legal",urlPrefix:"/legal"}];guideswins → sitemap, llms.txt and the search index all advertise/guides/legal/refund.
Second consequence: because the site-wide mount is re-applied per collection, two collections that both contain legal/refund.mdx now resolve to the same runtime urlPath, a collision generate can't produce because its mount prefixes are disjoint by construction.
Applying config.mounts only to the default collection — or prefixing each one with the collection's mountPath the way sourceMounts does — would restore parity.
Technical details
packages/leadtype/src/project/index.ts129-142 —collectionMountsalways emits[{pathPrefix:"",urlPrefix:routePrefix}, ...extra], withextrarelative to that collection's content root.packages/leadtype/src/cli/generate.ts894-907 —sourceMountsbuilds one flat global list, each collection contributing{pathPrefix: source.mountPath, urlPrefix}.packages/leadtype/src/cli/generate.ts912-920 —pathPrefixForUrlPrefixreturns""only forurlPrefix === "/docs"; that's why the divergence is invisible in the single-default-collection case.packages/leadtype/src/cli/generate.ts1900 —const effectiveMounts = [...mounts, ...(metadata.mounts ?? [])], consumed at 1914 / 2015 / 2026.packages/leadtype/src/internal/docs-url.ts50-104 —resolveDocsPathMountsorts by descendingpathPrefix.length, so the longest match wins on both sides.packages/leadtype/src/project/project.test.ts— "applies site-wide mounts alongside a collection's own" uses a single default/docscollection, the one configuration where both paths coincide.
| onlyFiles: true, | ||
| }); | ||
| const matches = await fg( | ||
| config.include && config.include.length > 0 |
There was a problem hiding this comment.
This glob and generate's don't use the same options, so the same authored include selects different files at build time and at runtime.
tinyglobby defaults expandDirectories: true (dist/index.mjs:267), and normalizePattern appends /** to any pattern not ending in * (dist/index.mjs:137). copySourceFiles sets expandDirectories: false deliberately — its comment says so: "Match the staging-level expansion semantics so bare-directory include entries don't silently fan out to dir/**". This call omits the flag, so it gets the default.
For collections: { docs: { dir: "content/docs", routePrefix: "/docs", include: ["guides"] } } with content/docs/guides/intro.mdx:
- runtime —
"guides"expands to"guides/**"→ the page is listed and loadable. - generate —
"guides"stays literal, matching only a file namedguides→ nothing is staged, and the collection is empty in every artifact. - doctor —
countCollectionPages(doctor.ts:269) also omits the flag, so it agrees with the runtime and reports a healthy page countgeneratenever produces.
Same shape, opposite direction, for dot: copySourceFiles sets dot: true, this call leaves it default-off, so a page under .internal/ is built but never listed at runtime.
The new test can't catch either — exclude: ["drafts/**"] already ends in **, so expansion is a no-op for it.
Technical details
packages/leadtype/src/cli/generate.ts949-954 —filterscomes straight fromentry.collection.include/.exclude, so this is the same authored field on both sides.packages/leadtype/src/cli/generate.ts988-997 —copySourceFiles:dot: true,expandDirectories: false.packages/leadtype/src/cli/generate.ts723-730 —sourceStagedMdxPaths: same two settings.packages/leadtype/src/cli/doctor.ts269-274 — neither flag set.node_modules/.bun/tinyglobby@0.2.16/.../dist/index.mjs137, 267 — the expansion and thetruedefault.
| // Every collection's groups, not just this one's: `generate` merges them | ||
| // globally before resolving, so a page whose `group:` is declared by a | ||
| // sibling collection resolves there and would error here. | ||
| groups: allGroups, |
There was a problem hiding this comment.
Passing allGroups to every collection makes report.navigation.groups accumulate the same titles once per collection.
The groups branch of resolveDocsNavigation emits one manifest.groups entry per declared group whether or not it matched any page (llm.ts:3834 maps over resolved, and buildNavigationGroup at llm.ts:3619-3635 returns a node with pages: [] when membership is empty). So for N collections sharing M merged groups, line 353's groups.push(...manifest.groups.map(g => g.title)) produces up to N×M titles, where generate produces one deduplicated list of M. That surfaces directly in the human output — doctor.ts:732-733 prints sections: ${report.navigation.groups.join(", ")}, so a two-collection project reads sections: Guides, Reference, Guides, Reference.
The rationale in the comment is right (group membership is pure slug matching — DocsGroup has no directory scope), so the fix is on the collecting side: dedupe by slug when pushing, or keep only groups with pages.length > 0 for the per-collection pass.
Second, smaller point: mergeCollectionGroups (generate.ts:585-605) throws a clear "group slugs must be globally unique across the project" for a repeated slug, but inspectNavigation flatMaps without that check and isn't inside the try/catch that wraps resolveProject (doctor.ts:440-467). So the same config surfaces as an uncaught Duplicate group slug "x" under "root" from resolveGroups (llm.ts:1135-1140) — a stack trace where doctor's stated contract is a reported issue.
Technical details
- No test covers this:
doctor.test.tshas no multi-collection distinct-groups case, andgrep i18nin it returns nothing, so change 2 is untested too (it is correct —project.config?.i18nis the same valuegeneratepasses asmetadata.i18natgenerate.ts:1910-1917). - Existing assertions use exact matching (
doctor.test.ts:144,472usetoEqualonreport.navigation?.groups), so the duplication will bite whoever adds the first multi-collection fixture.
| if (!existsSync(contentDir)) { | ||
| // Environmental problems belong to the collection, not the tree: an | ||
| // unsynced source has no content to resolve navigation against. | ||
| const blocking = project.diagnostics.find( |
There was a problem hiding this comment.
blocking is computed for the whole collection but only read inside if (!collection.contentDir), so any error diagnostic on a collection whose content directory does resolve is silently discarded — nav prints { ok: true, … } and exits 0.
That's reachable with the newly-promoted diagnostic from this same commit: source.inherit-failed (config/project.ts:390-402) fires when inheritCollectionSourceConfigs throws for a collection, and it's independent of resolveContentDir. So an unreadable inheritConfig source means doctor exits non-zero (doctor.ts:471-478 loops over all diagnostics unconditionally) and createDocsProject throws (project/index.ts:193-201 finds the first error regardless of id), while nav prints a tree built from whatever navigationOrigin it fell back to — most likely inferred, i.e. precisely the wrong tree, presented as fine.
Hoisting the blocking check above the contentDir branch would line all three up.
Technical details
packages/leadtype/src/cli/nav.ts313-322 —blockingis not referenced anywhere else in the function.packages/leadtype/src/lint/cli.tsis outside this pipeline entirely — it still usesloadDocsConfigand never callsresolveProject, so it can't surface anyProjectDiagnostic. Noting that as context, not as part of this finding.
| // drift that passing a project exists to end. | ||
| const mixed: FumadocsSourceConfig = { | ||
| source: project, | ||
| // @ts-expect-error must not compile alongside `source` |
There was a problem hiding this comment.
This test can't fail. The @ts-expect-error is the entire assertion — expect(mixed).toBeDefined() holds whether or not the union is exclusive — and nothing typechecks the file:
packages/leadtype/tsconfig.jsonsets"exclude": ["node_modules", "dist", "**/*.test.ts", "**/*.test.tsx"], socheck-types(tsgo --noEmit) never reads it;packages/leadtype/vitest.config.tscontains onlytestTimeout: 30_000— notypecheckblock, so vitest doesn't either.
Revert the union to a plain intersection and both it() blocks still pass. Either enable typecheck.enabled in the vitest config (and add include/tsconfig for test files), or drop the file and assert the exclusivity at runtime — createDocsSource is skipped whenever config.source is present (fumadocs/index.ts), which is observable behaviour.
| // already placed silently swallows a later entry's pin — the pin resolves, | ||
| // reorders within its own expansion, and then never reaches the tree. A | ||
| // pin that cannot take effect is an authoring mistake worth naming. | ||
| for (const urlPath of pinnedUrlPaths(group, entry, docsByRelativePath)) { |
There was a problem hiding this comment.
nit: buildNavigationFromNav's root-entry loop (llm.ts:3712-3727) resolves rootPageEntries through the same resolveNavEntryPages and applies the same first-wins rootSeenUrlPaths skip, but never calls pinnedUrlPaths. So the identical authoring mistake at the top level of nav: [...], outside any titled group, still silently no-ops. Worth hoisting the check so both loops share it.
| // `resolveDocsNavigation` reads the whole directory, while `generate` | ||
| // stages a filtered mirror first — so with include/exclude in play the two | ||
| // see different file sets and every excluded page would read as unplaced. | ||
| const isFiltered = |
There was a problem hiding this comment.
nit: the justification is right — generate stages a filtered mirror before resolving nav, so reading the raw directory would report excluded files as unplaced — but disabling curatable for any collection with include/exclude trades false positives for guaranteed false negatives. A page that passes the filter and genuinely renders unrouted in generate's real output can no longer be flagged. countCollectionPages (doctor.ts:265-274) already computes the filtered set with the same globs; intersecting manifest.ungrouped against it would keep the check alive for filtered collections.
| } | ||
| if (!sameSparse(manifest.sparse, resolvedCollection.remote.sparse)) { | ||
| diagnostics.push({ | ||
| id: "source.cache-narrow", |
There was a problem hiding this comment.
nit: see the body note — this fires for a fully-cloned cache once sparse is added to config, and the message then reads "was checked out with the full repository, but the config asks for [docs]", which inverts what cache-narrow means. Gating on manifest.sparse !== undefined would keep the true-positive behaviour the new test covers while accepting a superset checkout.
1 participant
Stacked on #164, the follow-up layer for #157. Implements the four gaps flagged when that stack landed.
The evidence
Answering "what is this project?" takes five ordered steps — discover the config, apply source-owned inheritance, normalize to canonical names, derive what wasn't authored, resolve each collection's content directory through the sync cache. Every command needs all five, and each assembled them by hand.
That cost was not hypothetical.
doctorandnavshipped with the same two bugs:inferred, showing a filesystem-derived tree the real build never uses.diragainst the config directory instead of its checkout, so every page vanished.Two commands, written days apart, same two omissions. That is a design problem, not two mistakes.
resolveProject()One function runs the pipeline; the commands read its result.
Diagnostics, not exceptions. It throws only for a genuinely malformed config — there is no project to describe. Everything environmental carries a stable id, the owning config field, and the command that fixes it. That split is what lets one function serve both kinds of caller:
doctorreports an unsynced source and keeps going, whilecreateDocsProjectrefuses to hand a renderer a source it cannot read. Only the caller knows which is right, so the resolver doesn't decide.Two details found against real c15t, not fixtures
sourcesintocollections, so only the first pass ever sees authored source names; re-deriving reported c15t's source asrepo#refinstead ofc15t. Both have regression tests.The other three follow-ups
Config loading moves out of
cli/generate.tsinto the config module. The runtime needs it —createDocsProjectanddoctorboth ask which config describes a project — and reaching through the generate pipeline to ask would drag staging and conversion into an app bundle. This is the last piece of #151's "one normalizer shared by every command and runtime helper".createDocsProject()discovers its own config, so an app that has one doesn't import it just to hand it straight back:The scaffolds and the Astro example use that shape. It also collapses the
configDir/contentDir/configPathcluster into one rule, stated once: adocs.config.*sits inside the docs directory, aleadtype.config.*at the root above it.The single-repo path is unambiguous in the docs: use
defineDocsConfig; ownership only becomes a question once a second repo is involved.Verification
805 tests pass. Re-verified against the migrated c15t example:
doctorandnavreportinherited, 252/254 pages, the seven sections c15t publishes, 63 unplaced pages and one duplicate — identical to before the refactor, with the source namedc15tagain.bun run check-typesstill trips the parallel-build race on this branch; that fix is #166, offmain.