Add leadtype doctor to explain resolved config and project health - #163
Add leadtype doctor to explain resolved config and project health#163KayleeWilliams 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 |
There was a problem hiding this comment.
Important
Two findings worth fixing before merge: the sources loop recomputes a default cacheDir differently than sync does, so doctor reports "not synced" for a synced source (reproduced by running it); and inspectNavigation resolves navigation from inputs that diverge from what generate stages, which can produce a false nav.unknown-group error and exit 1 on a project that generates cleanly. Both undercut the headline claim that a clean doctor run and a clean generate run cannot disagree.
Reviewed changes — the whole PR at 7e8909b: the new command, its tests, the CLI wiring, and the docs/nav/changeset that ship with it.
- New
leadtype doctorcommand —packages/leadtype/src/cli/doctor.tsloads the config throughloadDocsConfig, then inspects collections, the deduped source graph, navigation, output artifacts, and agent surfaces, emitting findings that each carry a stableid,level,ownerconfig field, and afixcommand. - Read-only by construction — no clone, refresh, write, or generate; an unsynced remote becomes a
source.not-syncedfinding namingleadtype sync. Exit0unless some finding islevel: "error",2on usage error. - CLI wiring —
cli.tsgains thedoctordispatch branch, thecommandUsagebranch forleadtype help doctor, and a line in the command list. - Tests — 18 tests over the healthy path, provenance, config discovery/deprecation, remote sync states, shared acquisitions, glob misses, unrepresented pages, artifacts, and framework/surface detection. All pass locally (
npx vitest run src/cli/doctor.test.ts). - Docs — new
docs/reference/doctor.mdx(finding-id table,--jsonshape), adoctorsection incli.mdx, nav +paths.lock.jsonentries, and SKILL.md step 1 now tells agents to run doctor first. - Pre-existing nav gap fixed —
pipeline/redirectswas reachable and listed in llms.txt starting points but absent fromnavigation;docs/docs.config.tsnow places it. Confirmed: running doctor on this repo at head reports nonav.unrepresented-pagefinding.
⚠️ The error-level findings that gate exit 1 are the least tested
source.not-synced and source.cache-stale have tests, but source.cache-unverifiable, source.dir-missing, and nav.unknown-group have none — and those three are the ones that flip the exit code that CI is invited to gate on. The cacheDir bug below is a direct consequence of the same gap: every git fixture in doctor.test.ts sets an explicit cacheDir, so the default-path branch is never executed.
Technical details
# Cover the exit-1 paths
## Affected sites
- `packages/leadtype/src/cli/doctor.test.ts` — no fixture omits `cacheDir`; no fixture reaches `source.cache-unverifiable` (checkout present, manifest absent), `source.dir-missing` (collection `dir` pointing nowhere), or `nav.unknown-group` (page `group:` absent from a collection's declared `groups`).
## Required outcome
- Each finding id that can produce exit `1` is exercised by at least one test that asserts the id, its `level`, and the exit code.
- At least one remote-collection fixture relies on the default `cacheDir` rather than authoring one, so the `defaultCacheDir` path is covered.
## Suggested approach
- The existing `fixture()` + `runJson()` helpers are enough; `writeSyncManifest` is already imported for seeding cache state, and omitting it is what produces `source.cache-unverifiable`.ℹ️ Nitpicks
packages/leadtype/README.md:72still reads "Theleadtypebinary wrapsinit,generate,sync,lint,mcp, andscore" —doctoris missing, and that README is the npm landing page.EXPECTED_SITE_ARTIFACTSdescribes site mode only, so after a successfulleadtype generate --bundledoctor reportsoutput.not-generatedand suggestsleadtype generate. Bundle mode is a CLI flag with no config trace, so detecting it is awkward — a one-line caveat indoctor.mdxmay be the honest fix.inspectNavigationcollapses disagreeing per-collection origins to"explicit"(origins.size !== 1), which reports a specific provenance for a project that doesn't have one — in a command whose purpose is explaining where values came from.renderHuman'ssurfaces: (defaults)branch is unreachable:enabledSurfacespushesskillsandagent-cardon!== false, which holds whenagentsisundefined, so the list is never empty.
Claude Opus | 𝕏
| const cacheDir = path.resolve( | ||
| configDir, | ||
| source.cacheDir ?? | ||
| path.join(".leadtype", "sources", `${source.repository}@${source.ref}`) | ||
| ); |
There was a problem hiding this comment.
This recomputes the default cache path with the raw repository URL, but sync derives it through repositorySlug() in defaultCacheDir() — .leadtype/sources/acme-acme@main, not .leadtype/sources/https:/github.com/acme/acme.git@main. For any git source without an authored cacheDir the reported cacheDir is a path that can never exist and syncedCommit is always null, so the human report prints "not synced" while the collection-level checks (which go through resolveCollection, hence the correct path) emit no source.not-synced finding — one report contradicting itself.
Technical details
# Use the shared default cache path
## Reproduced
A fixture with `collections.docs = { repository: "https://github.com/acme/acme.git", ref: "main", dir: "docs" }` (no `cacheDir`) and a real checkout + sync manifest at `.leadtype/sources/acme-acme@main` reports:
```json
{ "cacheDir": "…/.leadtype/sources/https:/github.com/acme/acme.git@main", "syncedCommit": null }
```
with no `source.not-synced` finding — `Sources` says not synced, `Findings` says nothing is wrong.
## Affected sites
- `packages/leadtype/src/cli/doctor.ts:460-464` — the `resolved.sources` loop's `cacheDir` fallback.
- Downstream: `syncedCommit` (JSON) and the `not synced` column in `renderHuman`.
## Required outcome
- Doctor resolves a source's cache directory to exactly the path `sync` would use, so `cacheDir` and `syncedCommit` agree with the collection-level `source.*` findings in the same report.
## Suggested approach
- `defaultCacheDir(repository, ref)` is already exported from `../sync/sync` (it is what `resolveCollection` uses); import it alongside `readSyncManifest` and drop the hand-rolled `path.join`.| const cacheDir = path.resolve( | |
| configDir, | |
| source.cacheDir ?? | |
| path.join(".leadtype", "sources", `${source.repository}@${source.ref}`) | |
| ); | |
| const cacheDir = path.resolve( | |
| configDir, | |
| source.cacheDir ?? defaultCacheDir(source.repository, source.ref) | |
| ); |
| mounts, | ||
| }; | ||
|
|
||
| const manifest = await resolveDocsNavigation({ |
There was a problem hiding this comment.
This call reuses resolveDocsNavigation, but it assembles the inputs itself rather than the way generate does, and three inputs go missing: collection include/exclude (generate applies them while staging the source mirror), i18n/locale (without them every translation file becomes its own page), and globally merged groups (generate merges across collections via mergeCollectionGroups; here each collection sees only its own). The result is false nav.unrepresented-page warnings on filtered and localized projects, and a false nav.unknown-group — which is level: "error", so doctor exits 1 on a project that generates cleanly.
Technical details
# Resolve navigation from the same inputs `generate` resolves it from
## Affected sites
- `packages/leadtype/src/cli/doctor.ts:580-590` — `navigationOptions` + the `resolveDocsNavigation` call; no `include`/`exclude`, no `i18n`/`locale`, `groups: authoredGroups ?? []`.
- `packages/leadtype/src/cli/doctor.ts:592-600` — `nav.unknown-group`, `level: "error"`, so these divergences change the exit code.
- `packages/leadtype/src/cli/doctor.ts:631-641` — `unrepresentedPages` compares `manifest.ungrouped` against literal root entries.
## The three divergences
- **Filters.** `resolveDocsSources` in `cli/generate.ts` turns a collection's `include`/`exclude` into `source.filters`, and `copySourceFiles` stages with `ignore: exclude` before nav is resolved over the mirror. Doctor's own `inspectCollection` honors them for `pageCount` (`ignore: authored?.exclude ?? []`), so `pageCount` and `routedPages` in one report are computed over different file sets, and an excluded page is reported as absent from the curated tree. (The single-default-`docs`-collection shape takes `createSourceMirror`'s no-staging fast path, so that one shape agrees by accident.)
- **i18n.** `generate` passes `i18n` and loops locales; `readSourceDocs` then collapses translations to one page per logical path. With neither, `docs/es/guide.mdx` resolves as its own page `/docs/es/guide`, lands in `manifest.ungrouped`, and is reported unrepresented on every curated i18n site. `routedPages` is inflated the same way.
- **Groups.** `mergeCollectionGroups` builds one globally-unique group list before nav; doctor passes each collection only its own `groups`, so a page whose `group:` is declared by a sibling collection errors here and resolves there.
## Required outcome
- Either doctor resolves navigation over the same file set and group list `generate` does, or the checks that depend on the difference (`nav.unrepresented-page`, `nav.unknown-group`) are skipped for the configs where doctor cannot reproduce it — the way the `include`-glob root-entry case is already deliberately skipped just below.
- No `level: "error"` finding on a config that `generate` resolves without complaint.
## Open questions for the human
- Is mirroring generate's input assembly the intended long-term shape, or should the staging/filter/locale resolution move behind a shared helper both commands call? The stated contract ("a clean doctor run and a clean generate run cannot disagree") is a standing invariant, and every future generate-side change to staging will silently re-break it as long as the inputs are assembled twice.7e8909b to
2ab82ba
Compare
There was a problem hiding this comment.
Important
The --out fix is right (it matches generate, verified at generate.ts:2630). The inheritance fix is right in direction but only half-plumbed: navigation and groups now come from the inherited collection while the mounts used to compute every page's urlPath still come from the un-inherited one, which re-opens the same class of false nav.* finding the commit set out to close. Separately, source.inherit-failed is warn, so doctor exits 0 on the exact fixture the new test builds — a synced source whose config can't be read — while generate hard-fails on it.
Reviewed changes — the delta since the prior pullfrog review at 7e8909b. The branch was rebased, so the original commit is now 3683f11; the substantive new work is 2ab82ba. The sparse git-source support that appears in a raw commit-range diff arrived from the base branch and is out of scope here.
- Fixed
--outresolution —doctor.ts:374now resolves the flag against the current directory rather than--src, matchingleadtype generate; the artifacts test passes an absolute path accordingly. - Applied source-owned inheritance before reporting — a new
applySourceInheritancecalls the sharedinheritCollectionSourceConfigson the normalized collections map (correct input: it is what expandssources: {}and folds thesourceConfigalias), threads the merged collections intoinspectNavigation, and adds"inherited"tonavigation.origin. - Degraded rather than thrown — a source config that can't be read becomes a
source.inherit-failedfinding, keeping the report intact. - Tests — two new cases covering the inherited-navigation and unreadable-source-config paths; 20 tests pass (
npx vitest run src/cli/doctor.test.ts). - Docs —
doctor.mdxgains theinheritedorigin, thesource.inherit-failedrow, and the--outresolution note;paths.lock.jsonhash refreshed.
ℹ️ Nitpicks
docs/reference/cli.mdx:51andDOCTOR_USAGE(doctor.ts:119) both still describe--outwithout the cwd-resolution note thatdoctor.mdxnow carries, and they sit directly beneath--docs-dir, which is--src-relative — the two flags differing is exactly what needs saying in the place a reader hits first.source.inherit-failedusesowner: "inheritConfig"where every other finding names a keyed path (collections.<key>.inheritConfig), and becauseinheritCollectionSourceConfigsthrows on the first failing collection, one unreadable source silently drops inherited navigation for every healthy collection in the same project too.
Claude Opus | 𝕏
| // resolved with its own mount — the same mapping the runtime project uses. | ||
| const mounts = [ | ||
| { pathPrefix: "", urlPrefix: collection?.routePrefix ?? "/docs" }, | ||
| ...(collection?.mounts ?? []), |
There was a problem hiding this comment.
mounts is in DEFAULT_SOURCE_CONFIG_INHERIT, so a source repo's mounts are inherited exactly like its navigation — but this reads the resolved (un-inherited) collection while line 635 now reads the merged one. resolveDocsNavigation computes every page's urlPath from these mounts, so for a pinned source that declares mounts doctor resolves the inherited tree against un-mounted paths: the curated entries miss, the pages land in ungrouped, and doctor emits nav.unrepresented-page on a project generate routes cleanly.
Technical details
# Resolve navigation with the inherited mounts too
## Affected sites
- `packages/leadtype/src/cli/doctor.ts:653` — `...(collection?.mounts ?? [])` reads `loaded.resolved.collections`, which `normalizeDocsConfig` built before inheritance ran.
- `packages/leadtype/src/cli/doctor.ts:710` — `toDocsUrlPath(entry, mounts)` uses the same list to compare root entries, so the `unrepresented` comparison is wrong on both sides.
## Why generate disagrees
- `generate` replaces `loadedConfig.config.collections` with the inherited map (`cli/generate.ts:2751`) *before* `resolveDocsSources`, so `entry.collection.mounts` at `cli/generate.ts:2013` is post-inheritance and composes into the mount list at `cli/generate.ts:1955`.
## Required outcome
- The mount list doctor resolves navigation against is the one generate would use for the same collection, including mounts inherited from the source repo.
- A fixture whose source `docs.config.ts` declares `mounts` (and whose project config does not) reports no `nav.unrepresented-page` — no test currently exercises inherited mounts at all.| ...(collection?.mounts ?? []), | |
| ...(merged?.mounts ?? collection?.mounts ?? []), |
| } catch (error) { | ||
| issues.push({ | ||
| id: "source.inherit-failed", | ||
| level: "warn", |
There was a problem hiding this comment.
warn means exit 0, and the new test above builds the case where that is wrong: a checkout that is present and manifest-verified but has no readable source config. generate throws there (nothing catches inheritCollectionSourceConfigs at generate.ts:2751), so CI gating on doctor passes and the build then fails — while doctor.mdx promises exit 1 when "a required input is missing or invalid". The unsynced case doesn't need warn to stay reportable; it already exits 1 through source.not-synced.
Technical details
# Level `source.inherit-failed` by what it actually blocks
## Affected sites
- `packages/leadtype/src/cli/doctor.ts:596-602` — the finding, `level: "warn"`.
- `packages/leadtype/src/cli/doctor.test.ts` — "keeps reporting when the source config cannot be read" asserts `code` is `0` for a fixture with a valid `.git`, a matching sync manifest, and no `docs/docs.config.ts` in the cache. That is a synced source, and it is a hard `generate` failure.
## Required outcome
- When a collection declares `inheritConfig` and the checkout exists but its config cannot be read or throws, doctor reports `level: "error"` and exits `1`, because `generate` cannot run on that project.
- Keeping the report itself intact (no throw) is right and should not change.
- A test asserts the exit code for both shapes: cache absent (already `error` via `source.not-synced`) and cache present but config unreadable.
## Open questions for the human
- Is there a case where a synced source legitimately ships no config while `inheritConfig` is on? If so it wants a distinct id, not a softer level on this one — `inheritConfig` is opt-in, so "enabled and unsatisfiable" reads as a config error either way.| // pinned-source project, so doctor has to apply it before reporting — the | ||
| // un-inherited config would say "inferred" for a project whose tree comes | ||
| // from the source repo. Uses the same shared implementation generation does. | ||
| const inherited = await applySourceInheritance({ |
There was a problem hiding this comment.
Inheritance runs here, after loadDocsConfig already normalized, so resolveCollectionEntry's recordExplicit never saw the inherited fields — report.collections[].provenance.navigation is absent for a project whose navigation did come from a source repo, in the same report where navigation.origin says "inherited". generate (generate.ts:2751) and project/index.ts:207 both inherit before normalizing, which is what keeps their provenance honest.
Technical details
# Make provenance see what inheritance supplied
## Affected sites
- `packages/leadtype/src/cli/doctor.ts:508-512` — inheritance applied after `loadDocsConfig`, and its result is threaded only into `inspectNavigation`.
- `packages/leadtype/src/cli/doctor.ts:449` — `provenance: collection.provenance`, computed by `config/normalize.ts:151-163` over the un-inherited collection.
- Inheritable fields with no provenance signal at all today: `groups`, `mounts`, `frontmatterSchema`, `flatteners`.
## Required outcome
- For a collection whose `navigation`/`groups`/`mounts`/`frontmatterSchema` came from its source repository, the report says so — `provenance` and `navigation.origin` cannot contradict each other in one run. Explaining where values came from is the command's stated purpose.
## Suggested approach
- Inherit first, then normalize, the way `project/index.ts:204-219` does it, and let `recordExplicit` record the merged values. That also removes the standing hazard of two orderings drifting.
## Open questions for the human
- Is "inherited" meant to be a `FieldProvenance` origin of its own rather than folded into `explicit`? A reader debugging a pinned-source project wants "authored here" and "came from the source repo" distinguishable per field, not only for the navigation tree as a whole.2ab82ba to
80f4df4
Compare
There was a problem hiding this comment.
Important
The gate itself is sound — I ran leadtype doctor --src . --docs-dir docs at head and it exits 0 (output.not-generated is warn, so the absent public/ doesn't fail the job). What needs fixing is the guidance shipped alongside it: both the new doc section and the ci.yml comment explain the multi-repo hazard through the wrong mechanism, and they tell a reader that pinning a commit makes the gate safe when it doesn't.
Reviewed changes — the delta since the prior pullfrog review at 2ab82ba. The branch was rebased again, so that commit is now 827e697; the only new work is 80f4df4, which touches three files. doctor.ts is unchanged, so all five prior inline threads remain open.
- Gated CI on doctor —
.github/workflows/ci.ymlgains aDiagnose project (leadtype doctor)step runningbun x leadtype doctor --src . --docs-dir docs, placed after the existingleadtype lintstep, with a comment explaining why the command is safe to gate on and why the example apps are excluded. - Documented gating for users —
docs/pipeline/validate-in-ci.mdxgains a "Gate on the resolved project too" section, and its "Run before generate" snippet is reordered to doctor → lint → generate with a new rationale for that order. - Refreshed the lockfile hash —
docs/paths.lock.jsonpicks up the newvalidate-in-cibody.
ℹ️ Gating on a command whose two known divergence-from-generate findings are still open
nav.unknown-group is error-level (doctor.ts:667-675), and the still-open thread on inspectNavigation's inputs is precisely a claim that doctor can emit it for a project generate routes cleanly. Until that closes, this step converts a latent false positive into a red build on main. This repo doesn't trip it today — one default collection, no include/exclude, no i18n, no mounts — so this is a sequencing question, not a present breakage.
Technical details
# Decide whether the gate lands before or after the open divergence threads
## Affected sites
- `.github/workflows/ci.yml:55-56` — the new gate.
- `packages/leadtype/src/cli/doctor.ts:655-665` — nav resolution inputs (open thread on review 4851890790's predecessor).
- `packages/leadtype/src/cli/doctor.ts:653` — un-inherited `mounts` (open thread).
- `packages/leadtype/src/cli/doctor.ts:667-675` — `nav.unknown-group`, `level: "error"`.
## Required outcome
- Either the two nav-input threads are closed before this gate merges, or there is a deliberate decision that the gate's blast radius on this repo's single-collection shape is acceptable for now.
## Open questions for the human
- Is the intent to gate every repo in this monorepo eventually, or only the root docs permanently? The `ci.yml` comment reads as the latter, but the doc section invites users to do the former.
- `source.cache-unverifiable`, `source.dir-missing`, and `nav.unknown-group` are the three findings that flip the exit code and none has a test (raised in review 1, still open). Gating CI on them raises the cost of that gap.ℹ️ Nitpicks
ci.ymlruns doctor afterleadtype lint, while the doc it dogfoods now says to order these "cheapest-and-most-specific first" and puts doctor above lint in its snippet, on the rationale that "Doctor fails on a project that can't resolve at all, before anything reads content." Swapping the two steps would make the repo's own CI match the sequence it recommends.docs/reference/doctor.mdx:156describes--jsonas "Machine-readable report on stdout. Warnings stay on stderr", butfinish(doctor.ts:902-914) writes only to stdout in both modes and nothing reaches stderr except the usage error — findings atlevel: "warn"live in the JSONissuesarray. Pre-existing to this delta, but it is the one claim in the flag table that a--jsonconsumer would act on.
Claude Opus | 𝕏
|
|
||
| It is read-only by construction: it never clones, refreshes, writes, or generates, which is what makes it safe to run first in a job. Exit `1` means a required input is missing or invalid; warnings still print but do not fail the build. | ||
|
|
||
| Worth knowing before you gate a multi-repo project on it: a config that pins a **mutable ref** — a branch rather than a commit — has a cache that is stale whenever CI hasn't synced, and doctor reports that. On a pinned commit it is a real signal; on a branch it mostly reports the CI environment. Either run `leadtype sync` first, or gate only on the repo whose docs you own. |
There was a problem hiding this comment.
Doctor's exit code does not depend on ref kind, so this tells a reader the opposite of what will happen. On a fresh CI checkout the cache is absent, not stale, which is source.not-synced at level: "error" (doctor.ts:276-284) — for a pinned SHA exactly as much as for a branch. source.mutable-ref is only warn (doctor.ts:471-483), so the ref kind never moves the exit code on its own, and source.cache-stale needs an existing manifest to compare against. Someone who follows this, pins a commit, and gates gets a red build.
Technical details
# Describe the gate hazard as "no cache", not "mutable ref"
## Affected sites
- `docs/pipeline/validate-in-ci.mdx:138` — attributes gate failure to mutable refs and cache staleness, and asserts a pinned commit makes it "a real signal".
- `.github/workflows/ci.yml:51-54` — the same misconception in the comment justifying the example-app exclusion.
## Why the code disagrees
- `packages/leadtype/src/cli/doctor.ts:276` tests `existsSync(path.join(cacheDir, ".git"))` with no reference to `refKind`, pushes `source.not-synced` at `level: "error"`, and returns before any manifest read.
- `packages/leadtype/src/cli/doctor.ts:297-305` (`source.cache-stale`) is only reachable once that check passes and a manifest exists, so it cannot fire on a fresh checkout.
- `packages/leadtype/src/cli/doctor.ts:471-483` (`source.mutable-ref`) is `level: "warn"`; `report.ok` is `!issues.some(i => i.level === "error")` (`:525`) and `finish` returns `report.ok ? 0 : 1` (`:913`).
- Source caches are gitignored by this project's own convention — root `.gitignore:38` (`apps/*/.leadtype/`) and `apps/c15t-example/.gitignore:3` — so "no cache in CI" is the normal state for every remote-source project.
## Required outcome
- The paragraph says that any project with a remote source exits `1` in CI until `leadtype sync` has run, regardless of whether `ref` is a branch or a commit, and names `source.not-synced` as what fires.
- The two remedies already given (`leadtype sync` first, or gate only on the repo whose docs you own) stay — they are correct. Only the mechanism needs rewriting.
- Pinning a commit is still worth recommending, but for reproducibility, not for making this gate pass.| # declares, a collection pointing at a directory that isn't there. | ||
| # | ||
| # Only this repo's own docs are gated. The example apps pin the current | ||
| # git branch as their source ref, so their caches are stale by design on |
There was a problem hiding this comment.
This is right to exclude the example apps but wrong about why, in a way that would mislead whoever revisits the decision. apps/next-example does track the current branch (resolveExampleSourceRef() shells out to git branch --show-current), but apps/c15t-example/leadtype.config.ts:51 pins a commit SHA and its own comment says it does so deliberately. And neither would report a stale cache: both cache dirs are gitignored, so in CI they have no checkout at all and produce source.not-synced at level: "error". The accurate reason is "no source is synced in CI, so any remote collection is an error until leadtype sync runs".
80f4df4 to
f5dac6e
Compare
generate, sync, lint, and score each answer a question about a project by doing something to it. None answered the one you need first: what *is* this project, and why? Which config was discovered, whether it resolved single-source or multi-repo, which values were authored versus inherited from a source repo versus inferred, which collections share one clone, what routes will exist, and which command fixes what is currently wrong. Read-only by construction. Doctor never clones, refreshes, writes, or generates, so an unsynced remote is a finding naming `leadtype sync` rather than a fetch — which is what makes it safe to run first, in CI, or against a production config you are debugging. Everything it reports comes from the same config loader and resolvers the other commands use, so a clean doctor run and a clean generate run cannot disagree about the project. Every finding carries a stable id, the config field or file that owns it, and a concrete next command. `--json` keeps those ids and adds provenance so an agent can act without parsing prose; warnings stay on stderr so the report stays a clean machine record. Exit 0 when nothing is an error, 1 when a required input is missing or invalid. Dogfooding it on this repo immediately found a real bug: `/docs/pipeline/redirects` was listed in llms.txt's starting points but absent from `navigation`, so the page agents were pointed at had no place in the sidebar. Fixed here. One premise had to be corrected while building it: pages missing from a curated tree are not unreachable — they fall back to the root of `ungrouped`. The finding reports that accurately (present, but placed by default rather than by decision) and skips configs whose root entries are include globs rather than guessing at an expansion it would have to re-derive.
Pointing doctor at a real pinned-source project — the migrated c15t example — surfaced two things the fixture tests could not. `--out` resolved against `--src`, while `leadtype generate` resolves it against the current directory. Running `doctor --src apps/c15t-example --out apps/c15t-example/public` therefore looked for artifacts in `apps/c15t-example/apps/c15t-example/public` and reported the output as missing. Two commands reading the same flag differently is its own bug, so doctor now matches generate. More seriously, doctor reported navigation as `inferred` for a project whose navigation is inherited from its source repository. Config loading does not apply source-owned inheritance — generation does, later in its pipeline — so doctor was describing a pre-inheritance project and showing a filesystem-derived tree that the real build never uses. For a pinned-source project that is precisely the question doctor exists to answer, answered wrongly. It now applies inheritance through the same shared implementation generation uses, and reports `inherited` as a distinct origin, which is the more useful answer anyway: "this tree belongs to the source repo" is different from "you wrote this tree". Because doctor is read-only and must keep reporting when a source is unsynced, a failure to read source config degrades to a `source.inherit-failed` finding rather than throwing.
CI already dogfoods `leadtype lint` on this repo's docs. Doctor is the same argument one level up: lint checks content, doctor checks that the project resolves at all — config loads, every collection's directory exists, no page declares a group nothing declares. It is read-only by construction, so gating on it is safe: it never clones, refreshes, writes, or generates. Only this repo's own docs are gated. The example apps pin the current git branch as their source ref, so their caches are stale by design on any branch that hasn't synced — gating there reports the CI environment rather than the config. The docs page says so, because anyone with a branch-pinned multi-repo config will hit exactly that.
f5dac6e to
271420f
Compare
There was a problem hiding this comment.
ℹ️
This push is a rebase onto dx/154-docs-project-runtime at 0d78b7c — doctor.ts and doctor.test.ts are byte-identical to the last review, and all five open threads on them still describe the code as written. The rebase absorbed four base-branch files correctly and mis-resolved one: two hashes in docs/paths.lock.json are the pre-rebase values for content the base branch has since edited. Separately, the base moved generate's side of three seams doctor is supposed to agree with — worth reading before addressing the open threads, because one of them is now a working in-repo reference implementation.
Reviewed changes — the delta since the prior pullfrog review at 80f4df4. git diff-tree -r --name-status 0d30b51 0d78b7c (old base → new base) returns the same 16 files as the head-to-head delta, so there is no new hand-written work in this push.
SKILL.md— thewriting.md→write-for-agents.mdreference rename absorbed from the base; the doctor-specific content is unchanged..github/workflows/ci.yml— the base'sTest evalsstep absorbed and the PR'sDiagnose project (leadtype doctor)gate survived intact at L55-56. Verified the gate is green at head: runningrunDoctorCommand(["--src", ".", "--docs-dir", "docs"])from workspace source exits0(52 pages, one local collection, onlyoutput.not-generatedatwarn), so the base's config-loading changes did not break it.docs/reference/cli.mdx— the base'sroutePrefix→prefixprose correction absorbed alongside the PR'sdoctorsection.docs/paths.lock.json— mis-resolved (inline, two comments). The base-to-base diff changes three hashes; head-to-head only one landed.
The base branch moved generate's side of three seams
The whole argument for doctor is that it reports what generate will do. Three of the five open threads are about doctor and generate disagreeing — and on this rebase the base branch changed generate at all three sites while doctor.ts stood still. None of this is a new defect in the PR; it changes what "fix the open thread" means.
generate.ts:2765-2788now re-normalizes after inheritance. That is precisely what the thread atdoctor.ts:508asks for — inherit, thennormalizeDocsConfigover the merged collections sorecordExplicitsees the inherited fields andprovenancestops contradictingnavigation.origin. There is now a concrete in-repo implementation to mirror rather than a design to invent, including the detail of preservingresolved.sourcesandresolved.deprecationsacross the re-normalize.generate.ts:2939-2951gained an i18n guard on content-derived navigation (metadata.i18n === undefinedbefore deriving), with a comment spelling out why: derivation keys sections off the first path segment, which fordocs/en/…is the locale, while navigation resolves per locale over locale-stripped paths — so no derived section can ever match.doctor.ts:639-647derives unconditionally. The thread atdoctor.ts:661already namesi18n/localeas one of the missing inputs, so this is the same concern with a sharper counterexample now checked in, not a separate finding.generate.ts:2765-2788also widens the inherited-mountsgap flagged atdoctor.ts:653: because generate re-normalizes over the merged collections,entry.collection.mountsis post-inheritance everywhere downstream, so the distance between the two mount lists grew rather than shrank.
ℹ️ Nitpicks
- A stale hash on a stable path is inert today —
computeDocsRedirectsconsults hashes only for paths that disappeared or appeared, and any regenerate silently rewrites the file. The cost is deferred: the next rename of/docs/pipeline/collectionsor/docs/reference/clifails rename detection and generate throwsdocs page(s) disappeared without a redirect. CI has no lockfile-drift check to catch the divergence in between, which is what makes a rebase conflict here easy to miss.
Claude Opus | 𝕏
| { | ||
| "path": "/docs/pipeline/collections", | ||
| "hash": "8114273c620aa826" | ||
| "hash": "c0385584dc467a04" |
There was a problem hiding this comment.
Rebase conflict mis-resolution: this is the pre-rebase hash for /docs/pipeline/collections, whose content the base branch edited. Regenerating on this branch (runGenerateCommand with --base-url) produces 8114273c620aa826 — the value this line held before the push. Every other entry in the file matches a fresh regenerate.
hashRedirectContent (redirects/node.ts:21-27) hashes the trimmed body of the generated <outDir>/docs/<page>.md mirror, so this drifts whenever the page content changes and the lockfile isn't refreshed.
| "hash": "c0385584dc467a04" | |
| "hash": "8114273c620aa826" |
| { | ||
| "path": "/docs/reference/cli", | ||
| "hash": "7dcd173e5a973493" | ||
| "hash": "73e07d8bd071d8d0" |
There was a problem hiding this comment.
Same mis-resolution for /docs/reference/cli: a fresh regenerate yields ce55b7b6c5c42916, not 73e07d8bd071d8d0. This one is expected to move on this branch — the PR adds a doctor section to cli.mdx and the base branch corrected the routePrefix → prefix prose in the same file — but the recorded value is neither the old content's hash nor the new one's.
The adjacent /docs/pipeline/validate-in-ci hash (d85e9261d6493e0a) and the new /docs/reference/doctor entry (f1ca98d7b80d093e) both verified correct, so regenerating and committing the lockfile is a two-line change.
| "hash": "73e07d8bd071d8d0" | |
| "hash": "ce55b7b6c5c42916" |

Closes #155. Stacked on #162, part of #157.
generate,sync,lint, andscoreeach answer a question about a project by doing something to it. None answered the one you need first: what is this project, and why? Which config was discovered, whether it resolved single-source or multi-repo, which values were authored versus inherited from a source repo versus inferred, which collections share one clone, what routes will exist, and which command fixes what is currently wrong.Read-only by construction
Doctor never clones, refreshes, writes, or generates, so an unsynced remote is a finding naming
leadtype syncrather than a fetch — which is what makes it safe to run first, in CI, or against a production config you are debugging. Everything it reports comes from the same config loader and resolvers the other commands use, so a clean doctor run and a clean generate run cannot disagree about the project.Every finding carries a stable id, the config field or file that owns it, and a concrete next command.
--jsonkeeps those ids and adds provenance so an agent can act without parsing prose; warnings stay on stderr so the report stays a clean machine record. Exit0when nothing is an error,1when a required input is missing or invalid.It found a real bug on its first run
/docs/pipeline/redirectswas listed inllms.txt's starting points but absent fromnavigation, so the page agents were pointed at had no place in the sidebar. Fixed in this PR.One premise corrected while building it
Pages missing from a curated tree are not unreachable — they fall back to the root of
ungrouped. The finding reports that accurately (present, but placed by default rather than by decision) and skips configs whose root entries are include globs rather than guessing at an expansion it would have to re-derive.Two bugs the c15t migration then exposed
Fixture tests could not catch either; pointing doctor at a real pinned-source project did.
--outresolved against--src, whileleadtype generateresolves it against the current directory.doctor --src apps/c15t-example --out apps/c15t-example/publictherefore looked inapps/c15t-example/apps/c15t-example/publicand reported the output missing. Two commands reading the same flag differently is its own bug.Navigation reported as
inferredfor a project whose navigation is inherited. Config loading does not apply source-owned inheritance — generation does, later in its pipeline — so doctor was describing a pre-inheritance project and showing a filesystem-derived tree the real build never uses. For a pinned-source project that is precisely the question doctor exists to answer, answered wrongly.It now applies inheritance through the same shared implementation generation uses, and reports
inheritedas a distinct origin, which is the more useful answer anyway: "this tree belongs to the source repo" is not the same as "you wrote this tree". A source that cannot be read degrades to asource.inherit-failedfinding rather than throwing, because doctor must keep reporting when a source is unsynced.Against real c15t content it now reports
inherited, the seven sections c15t actually publishes, and 63 pages absent from that curated tree.