diff --git a/docs/check-plan.md b/docs/check-plan.md new file mode 100644 index 0000000..684fe7b --- /dev/null +++ b/docs/check-plan.md @@ -0,0 +1,390 @@ +# `tttk check` — implementation plan (validator-first) + +Status: **draft / not implemented.** Supersedes `docs/compile-plan.md`. + +## 1. Goal + +`tttk check [dir]` statically validates a directory of tttk markdown and +reports **structured diagnostics** with file, line, severity, and a stable +code. It exits non-zero when the content is broken. + +The user is an **authoring agent**. The tool's job is to answer, in one +command: *"Does this document set render the way I intend, and if not, exactly +where and why?"* HTML rendering is not the deliverable — a fix list is. + +Success criteria: + +- Every authoring mistake that would silently drop, mangle, or fail to + resolve content is reported with `file:line:col`. +- Output is machine-readable (`--json`) so an agent can parse and act on it + without scraping human prose. +- Exit code distinguishes clean / warnings-only / errors, so the command is + usable directly in a loop: write → check → fix → repeat. +- Checks run over the **same registry the app uses**, so a clean check implies + the app resolves the content too. + +Non-goals (v1): rendering HTML, a site build, hydrating custom elements, +journal/multiplayer, MCP, live reload, and automatic fixes. `check` reports +problems; it never edits content. + +## 2. Why not reuse `compile` + +The previous plan framed this as `tttk compile` producing HTML. That optimizes +for a human browsing output. An agent cannot easily tell "correct" from +"plausible but inert" by reading `./npcs.csv` — an +unresolved reference looks identical to a resolved one. + +The failure modes that actually hurt authors are invisible in inert HTML and +are currently logged to `console.warn` or thrown: + +| Failure | Current behavior | Location | +|---|---|---| +| Unknown `role=` | `console.warn`, block stripped → content vanishes | `src/cli/content-registry.ts:174-179` | +| Missing `md`/`csv`/`yarn`/`svg` asset ref | `resolveContent` returns `null`; fails at runtime only | `src/cli/content-registry.ts:340-372` | +| `role=declare` missing `key`/`expr` | **throws** (uncaught) | `src/cli/completions/declare-parser.ts:44-54` | +| `role=declare` bad row | `console.warn`, silently skipped | `src/cli/content-registry.ts:473-479` | +| spark-table first column not dice | `console.warn`, still stored | `src/cli/content-registry.ts:285-288` | +| Unknown element name (`:md-card`, `:dice`) | renders an inert unknown tag | none | + +A validator turns all of these into diagnostics. That is the product. +Rendering, if ever built, is a separate `tttk render` command (see §11). + +## 3. Design decisions + +### 3.1 Whole-directory registry pass first + +References only resolve against a full `ContentRegistry`: inline ids are +doc-scoped (`docContent[docPath]`), and file refs resolve relative to the doc +directory via `posixJoin(posixDir(docPath), id)` +(`src/cli/content-registry.ts:340-372`). So the pipeline is: + +``` +buildRegistry(dir) // reuse src/cli/commands/serve.ts:88 + → { pathIndex, docContent } +for each raw .md on disk: + → run checks(source, registry) // §3.2 + → collect Diagnostics +sort by (file, line, col) → report → exit code +``` + +`buildRegistry` already walks the tree and collects `.md`, `.csv`, `.yarn`, +`.svg` — exactly what reference resolution needs. Reuse it; do not re-walk. +Only `.md` files are validated; non-md files are inputs to resolution. + +### 3.2 Validate the **raw** source, resolve with the registry + +Do not run checks over `registry.pathIndex`. That string has fences already +replaced/stripped (`scanDoc`), so offsets no longer map to the author's file +and line numbers would be wrong. + +Instead: run checks over the **raw file contents**, tracking byte offsets, and +use the registry purely to *resolve* refs. This works because: + +- Directives (`:md-table[./npcs.csv]`, `:md-embed[./x.md#sec]`) appear + literally in raw source — that is where we locate and validate them. +- `role=spark-table` blocks are validated from their raw fence; the generated + `:md-table[./csv_]` never appears in the author's source, so it needs + no validation (the generator is trusted). +- Inline ids referenced by hand (`:md-embed[./text_]`) resolve through + `docContent`. + +Offsets are cheap: the `String.replace` callback in `scanDoc` already receives +the match offset as a trailing argument, and the directive/fence scanners we +add can capture `RegExp.exec` index directly. Convert offset → line/col with a +line-start table built once per source (`src/cli/validate/source.ts`). + +### 3.3 Diagnostics on raw source must skip fenced examples + +Docs legitimately contain example directives inside fenced blocks +(`reference.md` does). The scanner must track fence state (like +`stripFencedBlocks`, `content-registry.ts:494-510`) and mark matches as +`inFence`; refs/roles inside fences are examples, not live content. Roles are +only live on the fence *info line*; directive refs are only live outside +fences. + +## 4. Diagnostic model + +```ts +export type Severity = "error" | "warning" | "info"; + +export interface Diagnostic { + code: string; // stable, e.g. "TTTK-R001" + severity: Severity; + message: string; // one line, actionable + file: string; // project-relative, posix + line: number; // 1-based + column: number; // 1-based + hint?: string; // concrete fix + snippet?: string; // offending source line +} +``` + +Rules: + +- Codes are stable and documented; agents may key off them. +- Messages name the offending value (`unknown role "spark-tabel"`), not just + the category. +- `hint` is imperative and concrete (`did you mean \`spark-table\`?`). + +## 5. Rule catalog + +Priority: **P0** ships in v1, **P1** follows, **P2** experimental. + +### Structural + +| Code | Sev | Rule | Prio | +|---|---|---|---| +| `TTTK-S001` | error | Markdown pipeline throws while parsing the doc | P1 | +| `TTTK-S002` | error | Unterminated container directive (`:::` with no close) | P1 | + +### Directives + +| Code | Sev | Rule | Prio | +|---|---|---|---| +| `TTTK-D001` | error | Unknown `:md-*` element name | **P0** | +| `TTTK-D002` | error | Malformed attribute block (`{pins=A:30,40}` unquoted with spaces) | P1 | +| `TTTK-D003` | info | Legacy `:md-deck[./x.csv]` directive — prefer `yaml role=tag` | P2 | + +`D001` uses an explicit allowlist of the **registered** custom elements. From +`customElement(...)` call sites today (verified): + +``` +md-bg md-border md-commander md-deck md-dice md-embed +md-font md-link md-pins md-table md-yarn-spinner +``` + +Free-form names are allowed for **container** directives (`::: name`) — those +are CSS classes, not elements. Inline `:[name]` is the icon form and is not +governed by this list. + +> **Already-real finding:** `:md-card` is matched by the completion scanner +> (`content-registry.ts:442`) but is registered nowhere, so it renders as an +> inert tag. `D001` would flag it. This is the class of bug the tool exists for. + +### Code-block roles + +| Code | Sev | Rule | Prio | +|---|---|---|---| +| `TTTK-R001` | error | Unknown `role=` value (block is silently stripped) | **P0** | +| `TTTK-R002` | warning | `role=spark-table` first column header is not a dice formula | P1 | +| `TTTK-R003` | error | `role=declare` missing required `key`/`expr` columns | **P0** | +| `TTTK-R004` | warning | `role=declare` row skipped by the parser | P1 | +| `TTTK-R005` | error | `role=tag` body is not valid YAML | **P0** | +| `TTTK-R006` | error | `role=tag` missing `tag:` or names an unknown element | **P0** | + +`R003`/`R004` come from `parseDeclareCsv` (`declare-parser.ts:44-54`) — it +throws today; the check must catch and localize it rather than crash. +`R005`/`R006` parse with `js-yaml` (already a dependency) and reuse the `D001` +allowlist. + +### Content references + +| Code | Sev | Rule | Prio | +|---|---|---|---| +| `TTTK-C001` | error | Reference does not resolve (missing file or undefined inline id) | **P0** | +| `TTTK-C002` | warning | Reference resolves to an unexpected file type | P1 | +| `TTTK-C003` | info | Same CSV referenced by two `:md-table` in one doc | P1 | +| `TTTK-C004` | error | `:md-embed` cycle | P1 | + +`C001` covers every ref-bearing directive — `:md-table`, `:md-commander`, +`:md-pins`, `:md-bg`, `:md-border`(image), `:md-yarn-spinner`, +`:md-link`/`:md-embed` path part, and `role=tag` `body:` — by calling +`resolveContentEntry` (`content-registry.ts:340`) and flagging `null`. This is +the single highest-value check. + +### Links and anchors + +| Code | Sev | Rule | Prio | +|---|---|---|---| +| `TTTK-L001` | error | `:md-link`/`:md-embed`/markdown link target file missing | **P0** | +| `TTTK-L002` | warning | `#section` anchor matches no heading slug | P1 | + +Slugs come from the same `Slugger` used by `deriveLinks` +(`content-registry.ts:523-536`) so the validator and the app agree. + +### Assets + +| Code | Sev | Rule | Prio | +|---|---|---|---| +| `TTTK-A001` | warning | `:[icon]` not found under any `assets/` dir within the check root | P1 | +| `TTTK-A002` | warning | `:md-bg`/`:md-pins`/`:md-border` image path missing | P1 | + +`A001` mirrors `buildIconSrc` (`src/markdown/icon.ts:23-40`): search +`//assets/`, then each ancestor down to `/assets/`. We +**assume no `assets/` directory exists above the check root**, which makes the +lookup definitive rather than a guess — hence a `warning`, not `info`. + +### CSV data files + +| Code | Sev | Rule | Prio | +|---|---|---|---| +| `TTTK-V001` | error | CSV fails to parse (bad quoting) | P1 | +| `TTTK-V002` | warning | Row column count ≠ header column count | P1 | +| `TTTK-V003` | warning | Duplicate `label` values | P1 | +| `TTTK-V004` | warning | `{{prop}}` key is not a CSV column or front-matter key | P1 | +| `TTTK-V005` | warning | spark-table row roll value is not a number/range | P2 | + +`V004` mirrors `processVariables` (`src/components/utils/csv-loader.ts:86-99`): +a `{{key}}` resolves when `key` is a CSV header or a front-matter key (front +matter is merged into every row, `csv-loader.ts:56-61`). Unresolved keys are +left verbatim in the output, so the check is definitive. Honor the parser's +options: BOM stripped, `#` lines are comments, headers/cells trimmed, empty +lines skipped (`csv-loader.ts:45-51`). + +## 6. CLI surface + +``` +tttk check [dir] [options] + +Arguments: + [dir] directory to validate (default ".") + +Options: + --json emit a JSON report (see §6.1) + --strict treat warnings as errors + --quiet report errors only + --no-color disable ANSI color + -h, --help +``` + +Wire in `src/cli/index.ts` alongside `serve` (commander pattern at +`src/cli/index.ts:17-30`). + +### 6.1 Output + +Human (default), grouped by file: + +``` +content/adventures/hollow-mill/adventure.md + 12:1 error TTTK-C001 :md-table reference "./npcs.csv" does not resolve + hint: expected content/adventures/hollow-mill/npcs.csv + 40:8 warning TTTK-R002 spark-table first header "Name" is not a dice formula + hint: use d6, 2d6+1, … + +2 errors, 1 warning +``` + +`--json`: + +```json +{ + "root": "content", + "filesChecked": 12, + "diagnostics": [ { "code": "TTTK-C001", "severity": "error", "message": "…", + "file": "adventures/hollow-mill/adventure.md", + "line": 12, "column": 1, "hint": "…" } ], + "summary": { "errors": 2, "warnings": 1, "infos": 0 } +} +``` + +### 6.2 Exit codes + +| Code | Meaning | +|---|---| +| `0` | No errors (warnings allowed unless `--strict`) | +| `1` | At least one error, or any warning under `--strict` | +| `2` | Usage / IO / internal failure | + +Exit code is the contract that makes `check` usable in an agent loop. + +## 7. Implementation steps + +1. **`src/cli/validate/source.ts`** — line-start table; `offsetToPosition`. +2. **`src/cli/validate/diagnostics.ts`** — `Diagnostic`, `Sink` (collector), + sort, human + JSON formatters. +3. **`src/cli/validate/known.ts`** — the catalog is the single source of + truth: `KNOWN_ELEMENTS` (the 11 verified names, with body/attrs/description/ + examples), `KNOWN_ROLES = { spark-table, declare, file, tag }`, expected + file-type map for `C002`. `docs/reference.md` is generated from it (§7.11). +4. **`src/cli/validate/scan.ts`** — fence-aware scanner yielding + `{ name, body, attrs, offset, inFence }` for directives, plus role info + lines. Reuse `parseBlockAttrs` (`src/markdown/block-attrs.ts`); do not + reimplement attribute parsing. +5. **Checks** — `checks/roles.ts`, `checks/directives.ts`, `checks/refs.ts`, + `checks/links.ts`, `checks/csv.ts`, `checks/parse.ts`. Pure functions + `(source, registry, docPath, sink) => void`. +6. **`src/cli/validate/index.ts`** — `runCheck(dir)`: `buildRegistry`, iterate + raw `.md` sorted, run checks, sort diagnostics, compute summary. +7. **`src/cli/commands/check.ts`** — commander handler: parse options, run, + print, `process.exitCode = …`. Add `CheckCommandHandler` to + `src/cli/types.ts`. +8. **`src/cli/index.ts`** — register `check` (alias `lint`). +9. **Refactor `scanDoc` to report into a sink** (optional but removes drift): + give `scanDoc(content, docPath, sink?)` an optional sink, replace + `console.warn` at `content-registry.ts:175-179` and `:285-288` with + `sink.report(...)`, and pass the match offset through. Default no-op keeps + existing callers and tests unchanged. This lets the CLI share one code path + for role diagnostics instead of duplicating the switch. +10. **Prereq for `S001`** — make `src/markdown/mermaid.ts` browser-safe so + `parseMarkdown` is importable from the CLI: **delete** `import mermaid` + and the top-level `mermaid.initialize` (the extension only emits + `
`), then call `mermaid.initialize({ theme: "dark" })`
+    in `src/main.tsx` before the existing `mermaid.run()` calls
+    (`Article.tsx:89`, `md-embed.tsx:40`). Required for v1.
+11. **Reference generator** — `scripts/generate-reference.ts` renders the
+    `known.ts` catalog to markdown. Canonical output `docs/reference.md`
+    (versioned, testable); `--out` can target the skill copy at
+    `~/.agents/skills/tttk-authoring/reference.md`. Add `npm run
+    gen:reference`.
+12. **Drift guard** — a test that greps `customElement("md-*")` call sites
+    under `src/components` and asserts each appears in `known.ts` (and vice
+    versa). This is what would have caught `:md-card`.
+
+## 8. Tests
+
+`src/cli/validate/*.test.ts`, fixture-driven (temp dirs), covering:
+
+- **Positive:** a valid multi-file fixture produces zero diagnostics; exit 0.
+- **Negative, one per P0 code:** unknown role; missing declare columns;
+  invalid tag YAML; unknown tag element; unresolved `:md-table`; missing
+  `:md-link` target; unknown `:md-*` element. Assert `code`, `file`, `line`.
+- **Fence awareness:** an example directive inside a fenced block is *not*
+  reported; the same directive outside a fence is.
+- **No false positives:** a doc that round-trips cleanly, including
+  `:md-card`-free legacy content.
+- **Exit codes:** 0 / 1 / 1-under-`--strict` / 2 on a nonexistent dir.
+- **JSON shape:** stable, sorted, one entry per diagnostic.
+
+## 9. Docs and skill
+
+- Update `docs/cli.md`: replace the `compile` section with `check`; document
+  codes, `--json`, exit codes.
+- Update `README.md`.
+- Update `~/.agents/skills/tttk-authoring/SKILL.md`: the verification step
+  becomes **`npx tttk check ./content`**, and the workflow becomes
+  write → check → fix. Remove the "compile is a no-op stub" note. Cross-link
+  the code catalog (`reference.md`) with the diagnostic codes.
+- `reference.md` is generated from `known.ts` (§7.11) — do not hand-edit. The
+  skill's copy is refreshed by `npm run gen:reference`.
+
+## 10. Resolved decisions
+
+1. **`known.ts` is the source of truth; `reference.md` is generated from it**
+   (§7.11). A drift test (§7.12) keeps both honest.
+2. **`S001` is in scope.** Do the mermaid refactor (§7.10).
+3. **Icon existence is checked** (`A001`), assuming no `assets/` directory
+   above the check root, which makes the lookup definitive.
+4. **`V004` (`{{prop}}` keys) is in scope for v1.**
+5. **Report only — no `--fix`.** `check` never edits content.
+6. **`compile` stays a stub,** and its `--help` text now says so explicitly
+   (`src/cli/index.ts:27`).
+
+## 11. Risks
+
+- **Icon check assumes no parent assets.** `A001` treats `` as the top
+  of the icon search. If a project keeps shared `assets/` above the check root,
+  `A001` will false-positive; the assumption is documented in §5.
+- **False positives are fatal to adoption.** An agent that gets spurious
+  errors stops trusting the tool. Prefer P0 precision over coverage; gate
+  noisy checks behind `--strict` or `info`.
+- **Divergence from the app.** Do not reimplement attribute parsing, dice
+  detection, slugging, or role dispatch — import `parseBlockAttrs`,
+  `resolveContentEntry`, `parseDeclareCsv`, `inspectSparkTableCsv`,
+  `Slugger`. Reuse `buildRegistry` for the walk.
+- **Offsets vs. stripped content.** Validate raw source only (§3.2); mixing in
+  `pathIndex` will produce wrong line numbers.
+- **Windows paths.** Use `node:path`; normalize to posix keys via
+  `normalizePathKey` for all registry lookups. Repo targets Windows.
+- **Encoding.** Read UTF-8 explicitly.
+- **Declare parser throws.** Wrap `parseDeclareCsv`; a throw must become a
+  localized diagnostic, never a stack trace.