Introduce a canonical config API with backward-compatible normalization - #159
Introduce a canonical config API with backward-compatible normalization#159KayleeWilliams 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 |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
The alias-folding half of this PR is solid and I could not find a path where a legacy or canonical field name is silently dropped. The resolved model half is not wired up: nothing in the codebase reads ResolvedDocsConfig, and the new public docs page tells users to read it themselves — where it returns incomplete data for the flagship inheritConfig: true shape.
Reviewed changes — full diff of the single commit f08dcca, plus the surrounding config-load, sync, lint, and generate paths that consume the renamed fields.
- Three per-collection field renames —
prefix→routePrefix,sourceConfig→inheritConfig,schema→frontmatterSchema, with the old names kept as@deprecatedoptional properties onDocsCollection. - New normalizer —
config/normalize.tsfolds each alias onto its canonical key,deletes the alias, records aConfigDeprecation, and throws when both spellings are set. - New resolved project model —
config/types.tsaddsResolvedDocsConfigwith a deduped(repository, ref)source graph,refKind, per-fieldFieldProvenance, andserializeResolvedConfig. - Single wiring point —
loadDocsConfigFromDirnormalizes, sogenerate,sync, andlintall inherit it; every reader was switched to canonical names only (sync.ts resolveCollection,lint/cli.tsschemas,generate.tsinheritance). - Deprecation warning at load —
warnConfigDeprecationsemits onelogger.warnper config path, deduped through a module-globalSet. - New public exports —
defineLeadtypeConfig/LeadtypeConfig(identity aliases ofdefineDocsConfig/DocsConfig),normalizeDocsConfig,serializeResolvedConfig, and the resolved-model types. - Docs — new
concepts/config-model.mdxpage wired into nav, and every example across five existing pages moved to canonical names. - Tests — a new
normalize.test.tssuite (aliases, ambiguity, source graph, prefixes, single-source, serialization) and twocli.test.tscases for the legacy and canonical end-to-end runs.
⚠️ The resolved config is documented as the model every subsystem reads, but nothing reads it
The back-compat mechanism that actually works here is the canonicalized config object — aliases get folded and deleted, and downstream readers were switched to canonical names. ResolvedDocsConfig is a parallel artifact whose only consumer is warnConfigDeprecations, which touches resolved.deprecations and nothing else. Three places state otherwise, and one of them is a shipped docs page: docs/concepts/config-model.mdx line 127 ("the project model that generate, sync, lint, score, and the runtime source all read"), the types.ts header JSDoc lines 7-9, and the changeset. That claim is what makes the model's gaps user-visible rather than internal.
Technical details
# Resolved-config docs overstate what consumes the model
## Affected sites
- `docs/concepts/config-model.mdx:127` — "Normalization turns it into the project model that generate, sync, lint, score, and the runtime source all read. There is one normalizer, so there is one answer to 'what is this project?'". No subsystem reads it; grep for `resolved.collections` / `resolved.sources` / `resolved.provenance` returns only `normalize.ts`, `types.ts`, and the test file.
- `docs/concepts/config-model.mdx:118` — documents `inherited` as an observable origin. No code path ever produces `origin: "inherited"`.
- `packages/leadtype/src/config/types.ts:7-9` — same claim in the header JSDoc.
- `packages/leadtype/src/config/types.ts:103-127` — `ResolvedDocsCollection` also omits `flatteners`, which `lint/cli.ts:333` reads off the raw collection, so the resolved shape is not yet a superset of what consumers need.
- `.changeset/canonical-config-api.md:9` — "generate, sync, lint, score, and the runtime source all read".
## Required outcome
- The docs and JSDoc describe what is true at merge time. Either the subsystems are switched over to read `resolved`, or the prose is scoped down to what the model is today: an exported, inspectable derivation plus the source of the deprecation warning.
- `ConfigValueOrigin`'s `"inherited"` member and `FieldProvenance.inheritedFrom` are either produced somewhere or dropped until they are.
## Open questions for the human
- Is switching subsystems onto `resolved` intended for this PR or a follow-up? If it is a follow-up, saying so in the changeset costs nothing and stops the docs from promising a guarantee the next reader will try to rely on.ℹ️ Nitpicks
apps/next-example/leadtype.config.ts:56-57is still authored withprefix/sourceConfig, so the repo's own flagship example now prints the new deprecation warning on every build. Either update it or keep it deliberately as the in-repo legacy fixture — worth a one-line comment saying which.
Claude Opus ｜ 𝕏
| config: DocsConfig; | ||
| path: string; | ||
| /** The resolved project: collections, source graph, provenance, deprecations. */ | ||
| resolved: ResolvedDocsConfig; |
There was a problem hiding this comment.
resolved is snapshotted at config load, but runGenerateCommand (generate.ts:2983-2993) rebuilds loadedConfig after inheritCollectionSourceConfigs and spreads this field through unchanged. For the inheritConfig: true shape the resolved collections therefore omit every inherited navigation, frontmatterSchema, groups, and mounts — the exact shape docs/concepts/config-model.mdx recommends and then tells users to inspect via the newly exported normalizeDocsConfig.
Technical details
# `LoadedDocsConfig.resolved` goes stale after source-config inheritance
## Affected sites
- `packages/leadtype/src/cli/generate.ts:310` — `resolved` is a required field on `LoadedDocsConfig`, so every holder is entitled to assume it matches `config`.
- `packages/leadtype/src/cli/generate.ts:2983-2993` — `loadedConfig = { ...loadedConfig, config: { ...loadedConfig.config, collections } }`. `config.collections` is replaced with the post-inheritance map; `resolved` is carried over from before the merge.
- `packages/leadtype/src/config/types.ts:36` and `:53` — `ConfigValueOrigin`'s `"inherited"` member and `FieldProvenance.inheritedFrom` have no producer anywhere in the codebase, which is the same gap seen from the type side.
## Required outcome
- A `ResolvedDocsConfig` handed to any consumer describes the same collections as the `config` it travels with, including values that arrived by inheritance.
- Values sourced from a source repo's `docs.config.*` resolve with `origin: "inherited"` and an `inheritedFrom` key, or those two declarations go away until something produces them.
## Suggested approach (optional)
- Re-run `normalizeDocsConfig` on the post-inheritance config at the point where `loadedConfig` is rebuilt, and have `mergeInheritedSourceConfig` report which fields it supplied so the second pass can mark them `inherited` rather than `explicit`.| }; | ||
| } | ||
|
|
||
| function assertUniqueRoutePrefixes( |
There was a problem hiding this comment.
These two rules already exist downstream: resolveDocsSourcesFromCollections (generate.ts:2210-2226) rejects the site root and duplicate URL prefixes with its own wording, and resolveSources' cacheDir check duplicates resolveRemoteSources (sync.ts:161-183) — with different semantics, since this copy compares authored strings and treats undefined as no-opinion while sync compares resolved absolute paths, so an explicit-vs-default pair conflicts there and not here. Given the PR's "one normalizer, one answer" framing, collapsing to one owner per rule is the point.
Technical details
# Route-prefix and cacheDir validation now lives in two places with divergent semantics
## Affected sites
- `packages/leadtype/src/config/normalize.ts:404-423` (`assertUniqueRoutePrefixes`) vs `packages/leadtype/src/cli/generate.ts:2210-2226` — same two rules, two error messages for the same misconfiguration depending on which layer trips first.
- `packages/leadtype/src/config/normalize.ts:221-233` vs `packages/leadtype/src/sync/sync.ts:161-183` — the `cacheDir` conflict rule. normalize compares the authored `cacheDir` strings and does `existing.cacheDir ??= collection.cacheDir`; sync compares the *resolved* cacheDir (with its `.leadtype/sources/<slug>@<ref>` default applied). A config where one collection sets `cacheDir` and a sibling on the same `(repository, ref)` does not passes normalize and is then rejected by sync — and in the meantime `resolved.sources[*].cacheDir` advertises a value sync will not accept.
## Required outcome
- One owner per rule, so a given misconfiguration produces one message and `resolved.sources` cannot describe an acquisition that the sync layer rejects.
## Open questions for the human
- Is the downstream copy meant to stay as a defence for callers that build collections without going through `loadDocsConfigFromDir`? If there are no such callers, the generate-side check is now unreachable and can go.
There was a problem hiding this comment.
Important
The config migration in this commit is clean, but it also carries six generated artifact files into apps/sveltekit-example/static/ with http://localhost:5173 baked into them, because the root .gitignore's sveltekit list is missing six of the entries its apps/*/public/* twin has.
Reviewed changes — f08dcca..b93ddd0:
apps/next-example/leadtype.config.tsmoved ontodefineLeadtypeConfig/routePrefix/inheritConfig, which retires the only nitpick from the previous review.docs/docs.config.tsswitched fromimport type { DocsConfig }+ object literal todefineDocsConfig(...).docs/concepts/config-modelwired into the Concepts nav and added todocs/paths.lock.json; the five refreshed hashes match exactly the mdx pagesf08dccaedited.- Six generated artifacts committed under
apps/sveltekit-example/static/— see the inline comment.
All six inline threads from the previous review were re-checked against the working tree, not just by anchor position; b93ddd0 touches none of those files, so they all stay open.
ℹ️ Nitpicks
docs/docs.config.ts:3-4— the new comment is missing its verb: "This repo's docs are source-owned content, sodefineDocsConfig— the same helper the docs tell everyone else to use." Adding "so it usesdefineDocsConfig" closes the sentence.
Claude Opus ｜ 𝕏
| Allow: /sitemap.xml | ||
| Allow: /sitemap.md | ||
|
|
||
| Sitemap: http://localhost:5173/sitemap.xml |
There was a problem hiding this comment.
These six files are build output, not source. apps/sveltekit-example/package.json runs leadtype generate --src ../.. --docs-dir docs --out static --base-url http://localhost:5173, and dev, build, and check-types all invoke it — so every one of these files is rewritten (with fresh dateModified timestamps) the moment anyone runs the example, and every URL inside them now has http://localhost:5173 committed into git.
Root cause is the root .gitignore: it maintains two parallel generated-output lists, but the apps/sveltekit-example/static/* block has only 5 entries where the apps/*/public/* block has 11. git check-ignore --no-index confirms the asymmetry — apps/next-example/public/robots.txt is ignored via .gitignore:46, and none of the six sveltekit files are ignored at all. Nothing else has ever been tracked in that directory, and CI never builds the example apps, so this drift cannot be caught automatically.
Technical details
apps/sveltekit-example/static/robots.txt (Sitemap: http://localhost:5173/sitemap.xml)
apps/sveltekit-example/static/sitemap.xml (223 lines of http://localhost:5173/... locs)
apps/sveltekit-example/static/sitemap.md
apps/sveltekit-example/static/schema-map.xml
apps/sveltekit-example/static/mcp.json
apps/sveltekit-example/static/feeds/schema.jsonl
Affected sites: .gitignore:39-54, apps/sveltekit-example/package.json
Required outcome: nothing generated is tracked under apps/sveltekit-example/static/, and git status stays clean after running the example.
Suggested approach: git rm --cached the six files and add the six missing entries (feeds/, mcp.json, robots.txt, schema-map.xml, sitemap.md, sitemap.xml) to the sveltekit block — or drop both hand-maintained lists in favour of an app-local .gitignore, which is what apps/c15t-example and apps/tanstack already do.
Open question: were these swept in accidentally while regenerating docs for the new config-model page?
b93ddd0 to
cd5cb75
Compare
DocsConfig grew from a description of one MDX folder into the project model — identity, source acquisition, navigation, feeds, redirects, agent surfaces, multi-repo orchestration. Growth like that left field names from the job they used to do, and left each subsystem re-deriving the project from raw config. Canonical vocabulary: - `defineLeadtypeConfig` for a project/site `leadtype.config.ts`, alongside `defineDocsConfig` for a source repo's content-owned `docs.config.ts`. The two shapes are identical; the names mark ownership, which is what decides what may be inherited across repos. - `prefix` → `routePrefix` (three different prefixes appear in one multi-collection config; only one reaches a URL), `sourceConfig` → `inheritConfig` (it declares inheritance, not a config object), `schema` → `frontmatterSchema` (matching the top-level field of the same purpose). One normalizer, one resolved project. `normalizeDocsConfig` folds deprecated aliases onto canonical names and derives `ResolvedDocsConfig`; generate, sync, lint, score, and the runtime source read that and nothing else. Beyond canonical names it adds: - A source graph. Collections sharing a `(repository, ref)` resolve to one acquisition listing both keys — what sync already did internally, now visible — with `refKind` separating a pinned commit from a mutable branch or tag. - Provenance. Every value records whether it was authored here, inherited from a source repo, inferred, or defaulted, and a value written under a deprecated name records the name the author actually typed. Compatibility is the point, not an afterthought: existing configs stay type-valid and byte-for-byte identical in output (asserted by test), deprecated fields carry IDE-visible `@deprecated` guidance and warn once per config file, and setting an old name beside its replacement fails naming both rather than applying an invisible precedence rule. Nothing is removed before 1.0.
The PR claimed docs and examples use only the canonical syntax. They did not:
`apps/next-example/leadtype.config.ts` — the flagship pinned-source dogfood —
still used `prefix` and `sourceConfig`, so running `doctor` or `generate`
against our own example printed the deprecation warning this branch adds. A
deprecation whose reference implementation triggers it is not a migration
path, it is a warning users learn to ignore.
It is a site config pinning a source repo, so it is `defineLeadtypeConfig` too.
`docs/docs.config.ts` also declared `const config: DocsConfig = {…}` rather
than calling `defineDocsConfig()` — the helper every page of our own
documentation tells readers to use.
Committed build output. Six generated files under
`apps/sveltekit-example/static/` went in with the config migration — rewritten
with fresh timestamps whenever anyone runs the example, and carrying
`http://localhost:5173` into the repo. Removed. The ignore rules that stop it
recurring are in the example-app hygiene PR, which is off `main`.
The resolved config went stale after inheritance. `runGenerateCommand` swapped
the inherited collections into `loadedConfig.config` and spread `resolved`
through untouched, so for `inheritConfig: true` — the shape the config-model
page recommends — the resolved model omitted every inherited `navigation`,
`frontmatterSchema`, `groups`, and `mounts`. It re-normalizes now, keeping
deprecations and the acquisition graph from the first pass, which is the only
one that saw the authored aliases and source names.
A test that could not fail: both fixtures in the alias-equivalence case used
prefixes identical to their collection keys, which is exactly what the
`/${key}` fallback produces — so deleting the `prefix` fold entirely left it
green. The legacy fixture now uses a prefix that differs from its key.
Find-and-replace damage in `cli.mdx` ("its own public URL routePrefix" —
`routePrefix` is the field name, not a noun), a `schema:` left unrenamed in the
per-collection frontmatter example on the page most likely to be copied, and a
`resetConfigDeprecationWarnings` export described as a test seam that no test
uses.
Verified with the hook's own command run standalone: 743 pass, 0 fail. The
in-hook run flaked on 5s timeouts under contention, failing a different test
each attempt.
cd5cb75 to
0f7ede9
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
0f7ede9 closes four of the seven open threads, and the re-normalization fix is the right call — the old object spread really did leave inherited navigation/groups/frontmatterSchema out of the resolved model. But re-running normalizeDocsConfig on an already-normalized config rebuilds provenance from a config whose alias keys pass 1 already deleted, so the resolved model now contradicts two of the contracts docs/concepts/config-model.mdx introduces in this same PR. Separately, switching docs/docs.config.ts from import type to a value import makes a leadtype build a hard prerequisite for bun test.
Reviewed changes — new commit 0f7ede9 (5 files in PR scope):
packages/leadtype/src/cli/generate.ts—resetConfigDeprecationWarningsdeleted (grep confirms zero remaining references), andexecuteGeneratenow re-normalizes after inheritance instead of spreading collections in. See the inline comment on the provenance side effect.packages/leadtype/src/config/normalize.test.ts— thechangelogfixture moved to/releases, so the headline "produces the same collections and sources" test can now actually fail if the alias fold regresses. This was the strongest of the previous review's findings and it is properly fixed.docs/pipeline/collections.mdx:256—schema:→frontmatterSchema:.docs/reference/cli.mdx:127— "public URL routePrefix" → "public URL prefix".docs/docs.config.ts— moved ontodefineDocsConfig(...). See the inline comment.
docs/paths.lock.json's two refreshed hashes match exactly the two mdx files this commit edited.
The branch was rebased again (base tip 7967d82 → 5227964), so four files in the 26-file total — .agents/skills/leadtype/SKILL.md, .github/workflows/ci.yml, evals/EVAL-AREAS.md, packages/leadtype/src/llm/skills.test.ts — are #158's scope, not this PR's. git diff-tree 7967d82 5227964 confirms.
I re-checked all seven prior threads against the working tree rather than by anchor position. Four are now addressed and have been replied to and resolved. Three stay open:
packages/leadtype/src/cli/generate.ts:310— theinheritedprovenance origin andinheritedFromstill have no producer anywhere in the codebase.packages/leadtype/src/config/normalize.ts:404— the duplicated route-prefix validation is untouched.apps/sveltekit-example/static/robots.txt:210— the six generated artifacts are still tracked and.gitignore:50-54still has 5 entries where itsapps/*/public/*twin has 11. Verified on disk, not just from the diff.
Also still open from the previous review body, with no line in this delta to anchor to: docs/concepts/config-model.mdx:86, packages/leadtype/src/config/types.ts:7-9, and .changeset/canonical-config-api.md:9 all say the resolved model is what "generate, sync, lint, score, and the runtime source all read". Grep says only two call sites read it at all — warnConfigDeprecations reads resolved.deprecations, and generate.ts:2998 reads resolved.sources. That is a description of where this is heading, not where it is.
ℹ️ Nitpicks
docs/docs.config.ts:3-4— the comment is still missing its verb: "This repo's docs are source-owned content, sodefineDocsConfig— the same helper the docs tell everyone else to use." Carried from the previous review.
Claude Opus ｜ 𝕏
| resolved: { | ||
| ...renormalized.resolved, | ||
| sources: loadedConfig.resolved.sources, | ||
| deprecations: loadedConfig.resolved.deprecations, |
There was a problem hiding this comment.
The comment reasons carefully about carrying sources and deprecations forward from pass 1 because "the first pass ... is the only one that saw the authored aliases and source names" — but provenance has exactly the same property and is not carried forward, so pass 2's version wins.
Pass 1's applyCollectionAliases does delete mutable[alias.deprecated] after folding, so by the time pass 2 runs, prefix/sourceConfig/schema are gone from the config. Pass 2 therefore takes the authored === undefined path and records routePrefix via recordExplicit, i.e. a bare { origin: "explicit", configPath } with no authoredAs. Two documented contracts break, both introduced by this same PR:
docs/concepts/config-model.mdx:126documents the output as{ origin: "explicit", authoredAs: "prefix", configPath: "/repo/leadtype.config.ts" }. After inheritance runs,authoredAsis absent.docs/concepts/config-model.mdx:118documents aninheritedorigin. Every field that inheritance just supplied —navigation,groups,frontmatterSchema,flatteners,mounts— is now recorded asexplicitwith the project'sconfigPath, attributing source-repo values to a file that never authored them. That is the one placeinheritedcould plausibly have been produced, so this also makes thegenerate.ts:310thread strictly harder to close.
To be clear about blast radius: nothing reads resolved.provenance yet, and I confirmed independently that this call changes no output file, warning, thrown error, or exit code versus the spread it replaced — inheritance can only ever write the five SOURCE_CONFIG_INHERIT_FIELDS, none of which can trip pass 2's routePrefix, cacheDir, or both-spellings-set checks, and normalizeDocsConfig doesn't mutate its input. So this is a latent correctness problem in the model, not a live bug.
Required outcome: after inheritance, resolved.provenance still reflects what the user actually authored, and inherited fields are distinguishable from project-authored ones.
Suggested approach: merge pass 1's provenance over pass 2's the same way sources and deprecations are already merged, keeping pass 2's entries only for keys pass 1 had none for. Or, if the inherited origin is meant to become real here, have mergeInheritedSourceConfig record { origin: "inherited", inheritedFrom: <source config path> } per field it writes, which would close generate.ts:310 at the same time. Either way a test asserting authoredAs survives inheritance would pin it.
| import { defineDocsConfig } from "leadtype"; | ||
|
|
||
| const config: DocsConfig = { | ||
| // This repo's docs are source-owned content, so `defineDocsConfig` — the same | ||
| // helper the docs tell everyone else to use. | ||
| const config = defineDocsConfig({ |
There was a problem hiding this comment.
This turns leadtype from a types-only dependency of this file into a runtime one. packages/leadtype/package.json's exports all point at ./dist/*, so on a fresh clone with no build, loading this config fails with No "exports" main defined in .../node_modules/leadtype/package.json — I hit exactly that: 10 failures in packages/leadtype/src/cli.test.ts (lines 224, 249, 622, 2049, 2090, 2125, 2157, 2264, 2333, 2363), all reported as failed to load docs config at ".../docs/docs.config.ts". bun run --filter leadtype build fixes it and the suite goes green (50 files / 697 tests). CI stays green because its build step precedes test, so this won't be caught there — it lands on whoever clones and runs bun test first, and the error message points at the config file rather than at the missing build.
The import type version had no such ordering requirement, because the type import erased.
Required outcome: bun test passes on a clean clone without a prior package build, while this file still gets full type checking against DocsConfig.
Suggested approach: satisfies gives identical checking with no runtime import. Applying the suggestion below also requires changing the closing }); on line 340 to } satisfies DocsConfig;. (If the value import is deliberate — so this repo dogfoods the exact helper the docs recommend — then the ordering needs to be enforced somewhere a contributor will see it, e.g. a pretest that builds the package, rather than left implicit in CI's step order.)
| import { defineDocsConfig } from "leadtype"; | |
| const config: DocsConfig = { | |
| // This repo's docs are source-owned content, so `defineDocsConfig` — the same | |
| // helper the docs tell everyone else to use. | |
| const config = defineDocsConfig({ | |
| import type { DocsConfig } from "leadtype"; | |
| // This repo's docs are source-owned content, so this config is checked against | |
| // the same `DocsConfig` shape the docs tell everyone else to use. | |
| const config = { |
1 participant
Closes #151. Stacked on #158, part of #157.
DocsConfiggrew from a description of one MDX folder into the project model — identity, source acquisition, navigation, feeds, redirects, agent surfaces, multi-repo orchestration. Growth like that left field names from the job they used to do, and left each subsystem re-deriving the project from raw config.Canonical vocabulary
defineLeadtypeConfignames a project/siteleadtype.config.tsalongsidedefineDocsConfigfor a source repo's content-owneddocs.config.ts. The two shapes are identical; the names mark ownership, which is what decides what may be inherited across repos.routePrefixprefixinheritConfigsourceConfigfrontmatterSchemaschemafrontmatterSchema.One normalizer, one resolved project
normalizeDocsConfigfolds deprecated aliases onto canonical names and derivesResolvedDocsConfig. Generate, sync, lint, score, and the runtime source read that and nothing else. Beyond canonical names it adds:(repository, ref)resolve to one acquisition listing both keys — what sync already did internally, now visible — withrefKindseparating a pinned commit from a mutable branch or tag.Compatibility
Existing configs stay type-valid and produce byte-identical output — there's a test asserting equivalent legacy and canonical configs resolve identically. Deprecated fields carry IDE-visible
@deprecatedguidance and warn once per config file, not per command. Setting an old name beside its replacement fails naming both rather than applying an invisible precedence rule. Nothing is removed before 1.0; the policy is written down in the newdocs/concepts/config-model.mdx.