Make the common single-repo config minimal with derived defaults - #160
Make the common single-repo config minimal with derived defaults#160KayleeWilliams wants to merge 3 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 |
KayleeWilliams
changed the title
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Two reproduced crashes on the exact path this PR exists to enable — an identity-only config with no navigation. An i18n project fails with Nav page "index" under "home-zh" did not match a documentation page., and any section whose index page carries a non-Latin title fails with Invalid group slug "". Both are flagged inline, along with a third bug that makes createDocsSource and generate disagree whenever openapi is configured.
Reviewed changes
Adds a config/infer.ts module that derives navigation from the content tree and the llms.txt body from resolved navigation whenever neither was authored, and relaxes config validation so an identity-only config is legal.
packages/leadtype/src/config/infer.ts(new):inferNavigationFromContentbuckets root pages and every top-level directory into one nav node, deterministically ordered (indexfirst, then frontmatterorder, then path — with the glob sorted before the batched frontmatter reads, so nothing depends on FS enumeration order);inferLlmsBlocksbuilds a capped "Best Starting Points" links block;InferenceReportrecords what was derived and what was ambiguous.packages/leadtype/src/cli/generate.ts: derives nav only when there are no path filters, nonav, and nogroups; derivesproduct.blocksfrom the default locale's resolved navigation; adds a--explainflag; drops themust export groups or navigationthrow.packages/leadtype/src/source/index.ts: lazily memoizedresolveNav()socreateDocsSourcederives the same tree asgenerate.packages/leadtype/src/cli/init-templates.tsanddocs/quickstart.mdx: scaffold and starter config reduced to identity only, plus a "grow into it" progression table.- Tests: new
config/infer.test.ts(16 cases), newcreateDocsSourcederivation cases, andcli.test.tsflipped to assert the scaffolded identity-only config now generates successfully.
⚠️ Two parallel mechanisms now record "this value was derived"
InferenceReport / InferredValue sits alongside the FieldProvenance model (origin: "inferred" + inferredFrom) that the immediately preceding stacked PR (dx/151-canonical-config) introduced. docs/concepts/config-model.mdx:119 already documents inferred as "Derived by Leadtype from content, package.json, or another config value" — precisely what this PR derives — yet nothing here populates it; today only collection.routePrefix sets that origin.
Are these meant to converge, with --explain rendering resolved provenance? Worth settling before --explain's output shape becomes something users depend on. Flagging in the body rather than inline because the tension spans both branches, not any one line.
ℹ️ Nitpicks
--explainis undocumented: thegenerateflag table indocs/reference/cli.mdx(~L61) is untouched, and no test exercises the flag at all — including the JSON interaction flagged inline.- With
collections, derived sections come out ordered alphabetically by mount path rather than incollectionsdeclaration order, silently. Reasonable as a default, but declaration order is the likelier expectation for a multi-repo setup, and nothing surfaces the difference. init-templates.ts:42points readers at/docs/pipeline/configure-sources"for the progression", but the progression table this PR adds lives indocs/quickstart.mdx.
Verified and not flagged, for the record: mounted pages still resolve (nav entries match on relativePath, which survives urlPath remapping); pages absent from a derived nav land in ungrouped rather than being dropped (llm/llm.ts:3521-3524); the OpenAPI exclusion in generate strips the extension on both sides correctly; deprecated bullets/bestStartingPoints/agentGuidance are unreachable from docs.config.*, so deriving blocks cannot clobber authored legacy fields; both new comparators guard against NaN; and the derivedNavPromise memoization matches the file's existing cachedFiles/cachedMetas convention. infer.test.ts (16) and source.test.ts (23) pass; the cli.test.ts lint-this-repo failure reproduces as 51 pre-existing snippet:types errors needing a built dist/, unrelated to this PR.
Claude Opus ｜ 𝕏
| if ( | ||
| !hasExplicitPathFilters && | ||
| (nav === undefined || nav.length === 0) && | ||
| groups.length === 0 | ||
| ) { | ||
| const inferred = await inferNavigationFromContent(sourceMirror.docsDir, { | ||
| // Generated OpenAPI pages already contribute their own nav node. | ||
| exclude: [ | ||
| ...generatedOpenApi.pages.map((page) => page.relativePath), | ||
| ...generatedOpenApi.indexPages.map((page) => page.relativePath), | ||
| ], | ||
| }); | ||
| if (inferred.navigation.length > 0) { | ||
| derivedNav = inferred.navigation; | ||
| inference = mergeInferenceReports(inference, inferred.report); | ||
| } | ||
| } |
There was a problem hiding this comment.
For an i18n project this guard fires and derives a nav tree that can never resolve, so generate fails outright.
inferNavigationFromContent walks sourceMirror.docsDir raw and keys sections off the first path segment (config/infer.ts:220), so docs/en/… and docs/zh/… become two sections named after the locales. But resolveDocsNavigation runs once per locale over locale-stripped doc keys, so no section's page refs ever match — and resolveNavEntryPages (llm/llm.ts:1943-1991) throws on an unmatched bare-string entry (only include entries warn).
Reproduced on an i18n project with the identity-only config leadtype init now scaffolds:
leadtype generate: Nav page "index" under "home-zh" did not match a documentation page.
Nothing covers this: the existing i18n test at cli.test.ts:922 declares groups, so derivation never fires there.
Technical details
Two viable shapes:
- Make derivation locale-aware — derive from the default locale's content subtree and strip the locale prefix, so section keys match the logical paths
resolveDocsNavigationbuilds per locale. This is the behavior the feature implies. - Skip derivation when
metadata.i18nis configured, and emit anInferenceWarningtelling the author to declarenavigation. Smaller, but it leaves i18n users on the old "config must declare structure" footing while the docs say otherwise.
Either way, source/index.ts:602 resolveNav() has the same defect — it forwards config.i18n to resolveDocsNavigation but derives from an un-stripped sourceContentDir — so the fix needs to land on both paths to keep the sidebar and the artifacts in agreement.
| const node: DocsNavNode = { | ||
| title: sectionIndex?.title ?? titleize(sectionKey), | ||
| base: sectionKey, |
There was a problem hiding this comment.
The derived section's slug comes from the frontmatter title, which crashes for any title with no URL-safe characters. resolveNavGroups does inferNavSlug(node.slug ?? node.title) and hands the result to assertValidGroupSlug (llm/llm.ts:1109-1110 and 80-88), which throws on an empty slug.
Reproduced with no i18n involved — a plain docs/guides/index.mdx whose frontmatter title is 指南:
leadtype generate: Invalid group slug "" under "root". Slugs must be URL-safe (alphanumerics and dashes).
A display title never had to be URL-safe before; inference is what quietly makes it load-bearing.
Setting slug from the directory name fixes the common case. It isn't complete — a non-Latin directory name slugifies to "" as well — so inferNavigationFromContent should also verify its derived slug is non-empty and surface an InferenceWarning (or fall back) rather than letting nav resolution throw from several layers down, where the message gives no hint that the cause was inference.
| const node: DocsNavNode = { | |
| title: sectionIndex?.title ?? titleize(sectionKey), | |
| base: sectionKey, | |
| const node: DocsNavNode = { | |
| title: sectionIndex?.title ?? titleize(sectionKey), | |
| slug: sectionKey, | |
| base: sectionKey, |
|
|
||
| - **Navigation** comes from the content tree. Root pages stay at the root; each top-level directory becomes a section titled from its `index` page. Ordering is `index` first, then frontmatter `order`, then path — never filesystem order, so every machine derives the same tree. | ||
| - **The `llms.txt` body** becomes a "Best Starting Points" block built from that same resolved navigation, so an agent's entry points cannot drift from a reader's. | ||
| - **Product name and tagline** fall back to `package.json`'s `name` and `description` when you have not set `product` at all. |
There was a problem hiding this comment.
This fallback does not exist when a config file is present. readPackageProduct (packages/leadtype/src/cli/generate.ts:1841) is only reached on the no-config path (L2117); with a config loaded, L2085 uses loaded.config.product and validateDocsConfig (L1448-1453) still hard-throws.
Reproduced — an identity-less docs.config.ts beside a package.json with both name and description:
leadtype generate: docs config at "…/docs.config.ts" must export product.name and product.tagline
The same claim appears in .changeset/derived-defaults.md, so the release notes will ship it too.
inferProductFromPackageJson (packages/leadtype/src/config/infer.ts:368) looks like the intended implementation — exported and tested, but never called by any production code. Either wire it into the config-loading path so product becomes genuinely optional, or drop the claim from the docs and the changeset.
bd565ab to
8fb0aa8
Compare
8fb0aa8 to
08ec15a
Compare
The quickstart and `leadtype init` scaffolded a config that made you learn the `navigation` tree and the `llms.sections` block model before writing a page. Those controls exist for c15t-scale multi-repo docs; a one-page local site does not need them, and that is the wrong point on the adoption curve to meet them. Identity is the one thing leadtype cannot guess. Everything else is now derived from a source of truth that already exists: - Navigation from the content tree — root pages at the root, one section per top-level directory titled from its `index` page. Ordering is `index` first, then frontmatter `order`, then path, so it never depends on filesystem enumeration order and two machines derive the same tree. - The `llms.txt` body from that same resolved navigation, so an agent's entry points cannot drift from a reader's. - Product name and tagline from `package.json`. `createDocsSource()` derives the same tree as `generate`, lazily on first `getNavigation()`. Without that the rendered sidebar would stay flat while generated artifacts gained sections — one content graph is the point, so both sides infer or neither does. Three rules hold every derivation together. Explicit always wins: authoring `navigation`, `groups`, or `llms.sections` turns inference off for that field, and inference never merges with, reorders, or rewrites an authored value. Deterministic: ordering comes from frontmatter and paths only. Explainable: `generate --explain` names every derived value, what it came from, and the field that takes control of it, and genuinely ambiguous cases — pages with no frontmatter `title`, more pages than the starting-points block lists — warn with the field to set. `leadtype init` scaffolds identity only, and the quickstart teaches the progression instead of the whole schema.
`leadtype init` now scaffolds `product` and nothing else, because navigation and the llms.txt body are derived until authored. Config validation still required `groups`, `navigation`, `collections`, or `openapi`, so the scaffold it wrote could not be loaded. Found by running `leadtype nav` against a fresh scaffold. The zero-config test missed it because it exercised the no-config-file path, not the small-config one — the two take different branches through the loader.
`--explain` corrupted `--format json`. It wrote a prose report to stdout unconditionally, ahead of the result object, breaking the invariant the line below it states — JSON output is a clean machine record. Text mode only now; `leadtype generate --format json --explain` parses again. Derivation broke localized projects outright. It keys sections off the first path segment, which for `docs/en/…` is the locale, while navigation resolves per locale over locale-stripped paths — so no derived section could ever match and generate failed. Skipped when `i18n` is configured. A derived section's slug came from its title, so any title with no URL-safe characters slugified to nothing and failed group validation — a plain `docs/guides/index.mdx` titled `指南` took the build down, no i18n involved. The directory name is URL-safe by construction, so the slug pins to it and the title stays whatever the author wrote. `createDocsSource` skipped derivation for any openapi-configured source, because it tested `nav` — which is reassigned above to include generated OpenAPI nodes — rather than the authored one. Generate derived on the same config, which is precisely the sidebar-versus-artifacts drift this branch exists to prevent, inside the code preventing it. `inferLlmsBlocks` took a `product` it never read, so both its docblock and the user-visible `--explain` line credited product identity for a value built entirely from navigation. Parameter dropped, wording corrected. The quickstart claimed `package.json` fills in product identity; that fallback only exists on the no-config path — with a config present, `product` is required and validation throws.
08ec15a to
86259a2
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Four of the six prior threads are cleanly fixed, but the two remaining ones still crash on the exact path this PR exists to enable. createDocsSource().getNavigation() still throws for an i18n project with the scaffolded identity-only config — generate got the guard, the sidebar path did not, so the two now disagree. And pinning the derived section slug to the directory name fixed the non-Latin title case while introducing a new duplicate-slug crash for sibling directories that collide after slugification. Both reproduced.
Reviewed changes
86259a2 "Address review on the derived-defaults branch" — the first synchronize since bd565ab that changes PR content (the two before it were pure rebases). The range-diff also carries SKILL.md, ci.yml, docs/pipeline/collections.mdx, docs/reference/cli.mdx, EVAL-AREAS.md, normalize.test.ts, skills.test.ts, and a generate.ts collections re-normalization — none of those are in this PR's diff, so they are out of scope here.
- Gated
--explainon text format —generate.ts:3249now checksargs.format !== "json", so JSON stdout stays parseable. - Skipped derivation for localized projects — added
metadata.i18n === undefinedto thegenerate.ts:3174guard, so a locale directory can no longer become a content section. - Pinned derived section slugs to the directory name —
infer.ts:255setsslug: sectionKeyinstead of letting the frontmatter title drive slug derivation. - Fixed the openapi derivation guard in
createDocsSource—source/index.ts:608testsconfig.navrather than the OpenAPI-augmentednav. - Dropped the unused
productparam frominferLlmsBlocks— parameter, JSDoc, and the user-visiblederivedFromstring all corrected. - Corrected the
package.jsonfallback claim in the quickstart — now scoped to the no-config path.
⚠️ Three one-line guards landed with one test between them
The delta adds exactly one test — the title-slug case in infer.test.ts. Nothing covers the i18n skip at generate.ts:3176, the config.nav fix at source/index.ts:608 (source.test.ts was not touched this round), or --explain in either format. Each is a single boolean that silently changes what gets derived, and each was found by review rather than by the suite — the openapi one in particular produced wrong output with no error, so a regression would ship unnoticed.
Technical details
# Regression tests for the three derivation guards
## Affected sites
- `packages/leadtype/src/cli/generate.ts:3176` — `metadata.i18n === undefined` clause has no test; the existing i18n test (`cli.test.ts:922`) declares `groups`, so derivation never fires there.
- `packages/leadtype/src/source/index.ts:608` — `config.nav` fix has no test; `source/source.test.ts` is unchanged in this commit.
- `packages/leadtype/src/cli/generate.ts:3249` — `--explain` has zero coverage in either output format.
## Required outcome
- A test that fails if derivation fires for an i18n-configured project.
- A `createDocsSource` test with `openapi` configured that asserts derived sections are still present — this is the case that reverting to `nav` would silently break.
- A test that `--format json --explain` produces parseable JSON, and that `--explain` in text mode names at least one derived field.ℹ️ i18n is now silently excluded from derivation rather than told why
Skipping derivation when metadata.i18n is set is the smaller of the two shapes the prior thread offered, and it is a reasonable call — but no InferenceWarning accompanies it. A localized project using the config leadtype init now scaffolds gets a flat sidebar with every page in ungrouped and no signal that navigation has to be authored, while the quickstart tells them navigation comes from the content tree. Worth deciding whether i18n is a permanent second-class citizen for derivation or a follow-up; either way the author deserves a warning naming the field to set.
ℹ️ Nitpicks
--explainis still undocumented: thegenerateflag table indocs/reference/cli.mdx(~L61) is untouched by this PR.inferProductFromPackageJson(infer.ts:372) is now definitively dead — exported, tested, and reachable from no production code. With the docs claim withdrawn rather than implemented, it should either be wired into the config-loading path or deleted.infer.test.ts:215and:253still declare and pass aproductargument thatinferLlmsBlocksno longer accepts.tsconfig.jsonexcludes**/*.test.ts, so nothing catches it.
Claude Opus ｜ 𝕏
| // A derived section's slug comes from its title, and a title with no | ||
| // URL-safe characters — `指南`, say — slugifies to nothing, which fails | ||
| // group validation and takes the whole build down. The directory name is | ||
| // already URL-safe by construction, so pin the slug to it and let the | ||
| // title stay whatever the author wrote. | ||
| const node: DocsNavNode = { | ||
| title: sectionIndex?.title ?? titleize(sectionKey), | ||
| slug: sectionKey, |
There was a problem hiding this comment.
Pinning the slug to the directory name trades one crash for another, and the comment's premise is wrong — a directory name is not URL-safe by construction. Two failures reproduced through createDocsSource().getNavigation(), both surfacing from resolveNavGroups with no hint that inference caused them:
docs/指南/→Invalid group slug "" under "root". Slugs must be URL-safe (alphanumerics and dashes).— the incompleteness the prior thread named, still live.docs/api-v2/+docs/api_v2/→Duplicate nav slug "api-v2" under "root". Nav slugs must be unique among siblings.— new in this commit:resolveNavGroupsre-slugifiesnode.slug ?? node.title, so before pinning these two directories got distinct title-derived slugs and did not collide.
Technical details
# Derived section slugs are unvalidated
## Affected sites
- `packages/leadtype/src/config/infer.ts:248-256` — `slug: sectionKey` is emitted unchecked, and the comment asserts an invariant (`already URL-safe by construction`) that does not hold.
- `packages/leadtype/src/config/infer.test.ts:136-137` — the new test's comment repeats the same false claim.
- `packages/leadtype/src/llm/llm.ts:1047-1052` — `inferNavSlug` lowercases and collapses every non-alphanumeric run to `-`, so `api-v2` and `api_v2` both normalize to `api-v2`, and an all-non-alphanumeric name normalizes to `""`.
- `packages/leadtype/src/llm/llm.ts:80-88` — `assertValidGroupSlug` throws on the empty slug.
- `packages/leadtype/src/llm/llm.ts:1109-1116` — the sibling `seen` set throws `Duplicate nav slug`.
## Required outcome
- `inferNavigationFromContent` validates the slugs it derives — non-empty after normalization, and unique among siblings — before returning them.
- A tree that cannot yield valid slugs degrades with an `InferenceWarning` naming `navigation` as the field to set, rather than failing several layers down with a message that reads like an authoring mistake.
## Suggested approach (optional)
Normalize with the same rules `inferNavSlug` applies, then de-duplicate (suffix, or drop the section back to root pages) and warn. Both failure modes then collapse into one check in the module that owns the derivation.| async function resolveNav(): Promise<DocsNavEntry[] | undefined> { | ||
| // Test the *authored* nav, not `nav`: `nav` is reassigned above to include | ||
| // generated OpenAPI nodes whenever `openapi` is set, so checking it meant | ||
| // any openapi-configured source skipped derivation entirely while | ||
| // `generate` derived on the same config — sidebar-versus-artifacts drift, | ||
| // inside the code meant to prevent it. | ||
| if (config.nav && config.nav.length > 0) { | ||
| return nav; | ||
| } | ||
| if (config.groups && config.groups.length > 0) { | ||
| return nav; | ||
| } | ||
| derivedNavPromise ??= inferNavigationFromContent(sourceContentDir).then( |
There was a problem hiding this comment.
The i18n guard landed on generate but not here, so the two paths now disagree instead of both being broken. resolveNav() derives regardless of config.i18n, keying sections off the first path segment while resolveDocsNavigation resolves per locale over locale-stripped paths.
Reproduced — await createDocsSource({ contentDir, i18n: { defaultLocale: "en", locales: [{ code: "en" }, { code: "zh" }] } }).getNavigation() over a docs/en/… + docs/zh/… tree:
Nav page "guides/index" under "en" did not match a documentation page.
Technical details
# `resolveNav()` derives for localized projects and throws
## Affected sites
- `packages/leadtype/src/source/index.ts:602-614` — `resolveNav()` guards on `config.nav` and `config.groups` only; no `config.i18n` clause.
- `packages/leadtype/src/cli/generate.ts:3176` — the `generate` path does guard on `metadata.i18n === undefined`.
## Required outcome
- The two derivation call sites agree on when derivation fires. Whichever shape is chosen for i18n — skip, or make derivation locale-aware — both paths adopt it, since a sidebar that throws while `generate` succeeds is the drift this PR exists to eliminate.
- Coverage that fails if only one of the two call sites is guarded.| // Text mode only: JSON output is a machine record on stdout, and a prose | ||
| // report written before it makes the whole stream unparseable. In JSON | ||
| // mode the same information rides on the result object instead. | ||
| if (args.explain && args.format !== "json") { |
There was a problem hiding this comment.
The gate itself is right, but the comment describes behavior that does not exist: GenerateResult (generate.ts:241-278) has no inference field and nothing in this commit adds one. Under --format json, --explain is a silent no-op — only inference warnings escape, via the logger.warn loop just above. Either carry the report on the result object as the comment promises, or say plainly that --explain is text-only (and qualify the unconditional claim in .changeset/derived-defaults.md L11).
|
|
||
| Derive navigation and the `llms.txt` body when they aren't authored, so a new project reaches useful output from identity alone. | ||
|
|
||
| Navigation is derived from the content tree — root pages at the root, one section per top-level directory titled from its `index` page, ordered `index` first then frontmatter `order` then path, so it never depends on filesystem enumeration order. The `llms.txt` body becomes a "Best Starting Points" block built from that same resolved navigation, which keeps an agent's entry points from drifting from a reader's. Product name and tagline fall back to `package.json`. |
There was a problem hiding this comment.
The quickstart was corrected for this in 86259a2, but the changeset still carries the claim, so the release notes will ship it. readPackageProduct (generate.ts:1841) is only reached on the no-config path (L2117); with a config present, validateDocsConfig throws must export product.name and product.tagline.
Suggest matching the corrected quickstart wording — the fallback applies when there is no docs config at all.
1 participant
Closes #152. Stacked on #159, part of #157.
The quickstart and
leadtype initscaffolded a config that made you learn thenavigationtree and thellms.sectionsblock model before writing a page. Those controls exist for c15t-scale multi-repo docs; a one-page local site does not need them, and that is the wrong point on the adoption curve to meet them.Identity is the one thing Leadtype cannot guess. Everything else is now derived from a source of truth that already exists:
indexpage. Ordering isindexfirst, then frontmatterorder, then path, so it never depends on filesystem enumeration order and two machines derive the same tree.llms.txtbody from that same resolved navigation, so an agent's entry points cannot drift from a reader's.package.json.createDocsSource()derives the same tree asgenerate, lazily on firstgetNavigation(). Without that the rendered sidebar would stay flat while generated artifacts gained sections — one content graph is the point, so both sides infer or neither does.Three rules
navigation,groups, orllms.sectionsturns inference off for that field, and inference never merges with, reorders, or rewrites an authored value.leadtype generate --explainnames every derived value, what it came from, and the field that takes control of it. Genuinely ambiguous cases — pages with no frontmattertitle, more pages than the starting-points block lists — warn with the field to set.leadtype initscaffolds identity only, and the quickstart teaches the progression (zero config → identity → navigation → agent surfaces → multi-repo) instead of the whole schema.The last commit fixes a bug this surfaced: config validation still required
groups/navigation/collections/openapi, so the identity-only config the scaffold writes could not be loaded. Found by runningleadtype nav(added later in the stack) against a fresh scaffold — the zero-config test missed it because it exercised the no-config-file path, which is a different branch through the loader.