feat: kitchen view command and parser skeleton (task 0003) #2

Merged
alexion merged 1 commits from task-0003-view-skeleton into main 2026-07-29 07:24:42 -04:00
Owner

Task: .claude/tasks/0003-view-skeleton.md

Summary

The walking skeleton — a runnable kitchen view <file> cutting the first complete thread through both layers with the smallest set of node types.

  • @kitchen-md/core: a pure, total parse(input): DocumentAST ({ frontmatter, blocks, diagnostics }) built on a minimal remark pipeline (remark-parse + remark-frontmatter) with an internal translation layer to core's own types. This slice models frontmatter passthrough, HeadingBlock, ParagraphBlock, and TextNode. Blocks are flat and in document order; remark's mdast does not appear in the public API. Types live in a dedicated types.ts, re-exported from the index.ts barrel alongside parse.
  • @kitchen-md/bin: the view subcommand (commander) reads the file, calls parse, and passes the AST to a pure render(ast): string — an ANSI-styled string via chalk (auto-suppressed off a TTY): frontmatter as raw YAML followed by a separator, headings styled distinctly by level, paragraphs as prose with blank-line spacing.

Errors as values

Fallible file I/O is modelled as a neverthrow Result<string, ViewError> rather than exceptions (see ADR 0008). The error is a plain-data tagged union, and formatError matches over a CliError union that new fallible commands extend. The CLI entry matches the Result at the boundary: stdout on success; stderr plus exit 1 on failure. A missing argument prints commander usage and exits 1; a missing/unreadable file prints kitchen: cannot read '<path>': <cause> and exits 1. The command functions (viewFile, formatError) sit beside the import.meta.main-guarded CLI entry in index.ts, so they are exercised in-process without spawning the binary.

Tests

Structured on ADR 0009's three tiers — 23 tests pass, nix flake check (checks.tests + treefmt) is green, and biome/typecheck are clean:

  • 7 core unit (packages/core/src/parse_test.ts) — pure parse over inline strings, through the package barrel: frontmatter (arbitrary/empty/absent), headings at every level 1–6, paragraph TextNode content, flat document order, empty diagnostics.
  • 6 renderer unit (packages/bin/src/render_test.ts) — pure render over inline ASTs, ANSI stripped: headings at every level, paragraph blank-line spacing, frontmatter YAML + separator, and their absence when frontmatter is empty.
  • 4 integration (packages/bin/src/index_test.ts) — the view command function asserted through its returned Result in-process: ok render, frontmatter passthrough, a read-failed error for a missing path, and formatError's message.
  • 6 e2e (packages/bin/src/end_to_end_test.ts) — the kitchen binary as a black box (subprocess): render + exit 0, document order, ANSI suppression on a pipe, the missing-file and missing-argument errors, and kitchen --help.

The value-returning seams (parse, render, viewFile, formatError) are 100% covered in-process; the thin .action() dispatch that writes to the streams and calls process.exit is reachable only through the e2e subprocess and so is invisible to in-process coverage — the split ADR 0009 predicts. Supporting tooling: coverage via Bun, a path-scoped test-report generator (scripts/test-report.ts), and the committed fixtures/prose.md.

fixtures/prose.md is a slice-scoped recipe (frontmatter + headings + plain paragraphs only) so it renders losslessly today; the full fixtures/basic.md end-to-end smoke stays in task 0007, which needs 0004's richer nodes first.

Deviations / scope boundaries

  • Non-heading/paragraph blocks and non-text inline nodes are not modelled yet; translateInline keeps only TextNodes and drops other inline nodes by whole node (so emphasised text is currently lost). Lossless raw fallbacks and typed richer nodes are task 0004.
  • Malformed-frontmatter handling is task 0007; this slice parses well-formed frontmatter and returns {} for empty/absent. Diagnostic is defined minimally as { severity, code, message } — the element type of the currently-empty diagnostics array; 0007 emits the first invalid-frontmatter diagnostic and adds any locator fields additively.

Key decisions

  • render stays its own module; the view command logic was folded into index.ts beside the guarded CLI entry (it is the only caller of render).
  • Diagnostic trimmed to the fields this slice produces — dropped the unused Point/Position types and source?/position? fields.
  • Heading-level styling is a monotonic bold→dim taper (colour/weight are visual, not asserted).
  • Recorded as ADRs: 0008 (errors as values at the CLI boundary) and 0009 (testing tiers and boundaries).

Review

Risk

Overall: Low

  • Blast radius: Low — new files plus a small additive index.ts barrel; no existing callers touched.
  • Reversibility: Low — all net-new/additive code; no migrations or published schema.
  • Test coverage: Low — unit, integration, and e2e tiers cover the parser, renderer, the command's Result, and the CLI as a subprocess.
  • Sensitive domain: Low — no auth, payments, permissions, concurrency, or data migration; only reads a file and prints.
  • Size & complexity: Low — small, linear translate/render functions.
  • Runtime criticality: Low — a dev-facing CLI, not a production hot path.

Standards findings left unaddressed

  • render.ts"─".repeat(40) hardcodes the separator width (baseline magic-number, note-only). Left inline: the SEPARATOR name already conveys intent and a fixed display width is reasonable for a pure function with no terminal-width access.

Spec findings left unaddressed

  • None. The diff faithfully implements task 0003; the translateInline whole-node drop is within this slice's stated scope (only TextNode is modelled) and its lossless successor is explicitly owned by task 0004.
Task: `.claude/tasks/0003-view-skeleton.md` ## Summary The walking skeleton — a runnable `kitchen view <file>` cutting the first complete thread through both layers with the smallest set of node types. - **`@kitchen-md/core`**: a pure, total `parse(input): DocumentAST` (`{ frontmatter, blocks, diagnostics }`) built on a minimal remark pipeline (`remark-parse` + `remark-frontmatter`) with an internal translation layer to core's own types. This slice models frontmatter passthrough, `HeadingBlock`, `ParagraphBlock`, and `TextNode`. Blocks are flat and in document order; remark's mdast does not appear in the public API. Types live in a dedicated `types.ts`, re-exported from the `index.ts` barrel alongside `parse`. - **`@kitchen-md/bin`**: the `view` subcommand (commander) reads the file, calls `parse`, and passes the AST to a pure `render(ast): string` — an ANSI-styled string via chalk (auto-suppressed off a TTY): frontmatter as raw YAML followed by a separator, headings styled distinctly by level, paragraphs as prose with blank-line spacing. ### Errors as values Fallible file I/O is modelled as a neverthrow `Result<string, ViewError>` rather than exceptions (see **ADR 0008**). The error is a plain-data tagged union, and `formatError` matches over a `CliError` union that new fallible commands extend. The CLI entry matches the `Result` at the boundary: stdout on success; stderr plus exit 1 on failure. A missing argument prints commander usage and exits 1; a missing/unreadable file prints `kitchen: cannot read '<path>': <cause>` and exits 1. The command functions (`viewFile`, `formatError`) sit beside the `import.meta.main`-guarded CLI entry in `index.ts`, so they are exercised in-process without spawning the binary. ## Tests Structured on **ADR 0009**'s three tiers — **23 tests pass**, `nix flake check` (`checks.tests` + treefmt) is green, and biome/typecheck are clean: - **7 core unit** (`packages/core/src/parse_test.ts`) — pure `parse` over inline strings, through the package barrel: frontmatter (arbitrary/empty/absent), headings at every level 1–6, paragraph `TextNode` content, flat document order, empty diagnostics. - **6 renderer unit** (`packages/bin/src/render_test.ts`) — pure `render` over inline ASTs, ANSI stripped: headings at every level, paragraph blank-line spacing, frontmatter YAML + separator, and their absence when frontmatter is empty. - **4 integration** (`packages/bin/src/index_test.ts`) — the `view` command function asserted through its returned `Result` in-process: `ok` render, frontmatter passthrough, a `read-failed` error for a missing path, and `formatError`'s message. - **6 e2e** (`packages/bin/src/end_to_end_test.ts`) — the `kitchen` binary as a black box (subprocess): render + exit 0, document order, ANSI suppression on a pipe, the missing-file and missing-argument errors, and `kitchen --help`. The value-returning seams (`parse`, `render`, `viewFile`, `formatError`) are 100% covered in-process; the thin `.action()` dispatch that writes to the streams and calls `process.exit` is reachable only through the e2e subprocess and so is invisible to in-process coverage — the split ADR 0009 predicts. Supporting tooling: coverage via Bun, a path-scoped test-report generator (`scripts/test-report.ts`), and the committed `fixtures/prose.md`. `fixtures/prose.md` is a slice-scoped recipe (frontmatter + headings + plain paragraphs only) so it renders losslessly today; the full `fixtures/basic.md` end-to-end smoke stays in task 0007, which needs 0004's richer nodes first. ## Deviations / scope boundaries - Non-heading/paragraph blocks and non-text inline nodes are not modelled yet; `translateInline` keeps only `TextNode`s and drops other inline nodes by whole node (so emphasised text is currently lost). Lossless raw fallbacks and typed richer nodes are **task 0004**. - Malformed-frontmatter handling is **task 0007**; this slice parses well-formed frontmatter and returns `{}` for empty/absent. `Diagnostic` is defined minimally as `{ severity, code, message }` — the element type of the currently-empty `diagnostics` array; 0007 emits the first `invalid-frontmatter` diagnostic and adds any locator fields additively. ## Key decisions - **`render` stays its own module**; the `view` command logic was folded into `index.ts` beside the guarded CLI entry (it is the only caller of `render`). - **`Diagnostic` trimmed** to the fields this slice produces — dropped the unused `Point`/`Position` types and `source?`/`position?` fields. - **Heading-level styling** is a monotonic bold→dim taper (colour/weight are visual, not asserted). - Recorded as ADRs: **0008** (errors as values at the CLI boundary) and **0009** (testing tiers and boundaries). ## Review ### Risk **Overall: Low** - Blast radius: Low — new files plus a small additive `index.ts` barrel; no existing callers touched. - Reversibility: Low — all net-new/additive code; no migrations or published schema. - Test coverage: Low — unit, integration, and e2e tiers cover the parser, renderer, the command's `Result`, and the CLI as a subprocess. - Sensitive domain: Low — no auth, payments, permissions, concurrency, or data migration; only reads a file and prints. - Size & complexity: Low — small, linear translate/render functions. - Runtime criticality: Low — a dev-facing CLI, not a production hot path. ### Standards findings left unaddressed - `render.ts` — `"─".repeat(40)` hardcodes the separator width (baseline magic-number, note-only). Left inline: the `SEPARATOR` name already conveys intent and a fixed display width is reasonable for a pure function with no terminal-width access. ### Spec findings left unaddressed - None. The diff faithfully implements task 0003; the `translateInline` whole-node drop is within this slice's stated scope (only `TextNode` is modelled) and its lossless successor is explicitly owned by task 0004.
alexion reviewed 2026-07-26 21:10:22 -04:00
@@ -0,0 +10,4 @@
export function parse(input: string): DocumentAST {
const tree = processor.parse(input);
const frontmatter = extractFrontmatter(tree);
const blocks = tree.children.flatMap(translateBlock);
Author
Owner

Why use flatMap if the array returned is always of size 0 or 1?

I suspect it's to deal with empty files.

Why use `flatMap` if the array returned is always of size 0 or 1? I suspect it's to deal with empty files.
alexion marked this conversation as resolved
@@ -0,0 +20,4 @@
return {};
}
const data = parseYaml(yamlNode.value);
if (data !== null && typeof data === "object" && !Array.isArray(data)) {
Author
Owner

Why are we being so restrictive with what types of yaml parsed?

Why are we being so restrictive with what types of yaml parsed?
alexion marked this conversation as resolved
@@ -0,0 +19,4 @@
message: string;
source?: string;
position?: Position;
}
Author
Owner

Is this ever created anywhere? The only diagnostics I see is always an empty array.

Is this ever created anywhere? The only diagnostics I see is always an empty array.
alexion marked this conversation as resolved
alexion reviewed 2026-07-27 19:23:19 -04:00
@@ -0,0 +10,4 @@
export function parse(input: string): DocumentAST {
const tree = processor.parse(input);
const frontmatter = extractFrontmatter(tree);
const blocks = tree.children.flatMap(translateBlock);
Author
Owner

It is the map-or-drop idiom: translateBlock returns [] to skip an unmodelled node or [block] to keep one, and flatMap flattens that in one step, with no nulls or holes. It also generalises unchanged to task 0004, where a single mdast node can translate to several core blocks (and containers recurse into their children). Not about empty files — an empty input is just tree.children === [], handled the same way regardless. translateInline uses it for the same reason. Happy to switch to an explicit push-loop if you find that reads clearer.

It is the map-or-drop idiom: `translateBlock` returns `[]` to skip an unmodelled node or `[block]` to keep one, and `flatMap` flattens that in one step, with no nulls or holes. It also generalises unchanged to task 0004, where a single mdast node can translate to several core blocks (and containers recurse into their children). Not about empty files — an empty input is just `tree.children === []`, handled the same way regardless. `translateInline` uses it for the same reason. Happy to switch to an explicit push-loop if you find that reads clearer.
alexion marked this conversation as resolved
@@ -0,0 +20,4 @@
return {};
}
const data = parseYaml(yamlNode.value);
if (data !== null && typeof data === "object" && !Array.isArray(data)) {
Author
Owner

Frontmatter is a mapping type (Record<string, unknown>), but YAML can also parse to a bare scalar (e.g. 42), a top-level sequence, or null/~. The three conditions narrow to exactly a plain mapping — both null and arrays report a typeof of object, which is why the explicit !== null and !Array.isArray are there. Everything else normalises to {}, matching the spec: empty when the block is present but invalid. If we later want 'present but not a mapping' to be visible rather than silently {}, that is a natural fit for an invalid-frontmatter-style diagnostic in task 0007.

`Frontmatter` is a mapping type (`Record<string, unknown>`), but YAML can also parse to a bare scalar (e.g. `42`), a top-level sequence, or `null`/`~`. The three conditions narrow to exactly a plain mapping — both `null` and arrays report a `typeof` of `object`, which is why the explicit `!== null` and `!Array.isArray` are there. Everything else normalises to `{}`, matching the spec: empty when the block is present but invalid. If we later want 'present but not a mapping' to be visible rather than silently `{}`, that is a natural fit for an `invalid-frontmatter`-style diagnostic in task 0007.
alexion marked this conversation as resolved
@@ -0,0 +19,4 @@
type: "heading";
level: 1 | 2 | 3 | 4 | 5 | 6;
children: InlineNode[];
}
Author
Owner

Correct — nothing constructs one yet; diagnostics is always [] in this slice. Trimmed in 8687b75: dropped Point, Position, and the source?/position? fields, leaving Diagnostic as { severity, code, message } — just the element type of the returned (empty) array. Task 0007 emits the first real invalid-frontmatter diagnostic and re-adds the locator fields additively, with its own tests.

Correct — nothing constructs one yet; `diagnostics` is always `[]` in this slice. Trimmed in 8687b75: dropped `Point`, `Position`, and the `source?`/`position?` fields, leaving `Diagnostic` as `{ severity, code, message }` — just the element type of the returned (empty) array. Task 0007 emits the first real `invalid-frontmatter` diagnostic and re-adds the locator fields additively, with its own tests.
alexion marked this conversation as resolved
alexion reviewed 2026-07-27 22:08:30 -04:00
@@ -1,18 +1,93 @@
import { describe, test } from "bun:test";
Author
Owner

I don't think this is a useful test.

Please add unit tests for parse.ts instead to test the functions on an individual level.

I don't think this is a useful test. Please add unit tests for parse.ts instead to test the functions on an individual level.
alexion marked this conversation as resolved
alexion reviewed 2026-07-27 22:15:58 -04:00
@@ -1,86 +0,0 @@
import { describe, test } from "bun:test";
Author
Owner

Agreed — index.ts here is a pure re-export barrel (export { parse } from "./parse.ts"; export type * from "./types.ts";), so these tests were stranded at the entrypoint. Moved them (real tests + roadmap todos) into a co-located parse_test.ts importing parse from ./parse.ts, and deleted index_test.ts — commit e32b71e. A broken re-export is still caught by typecheck and by the bin package importing parse through the public entrypoint in its E2E test. (Note the same doesn't apply to bin/index.ts, which is the actual CLI with logic, so its index_test.ts stays.)

On testing the functions individually: parse_test.ts is now the right home for finer-grained cases. The one pure, mdast-free piece — the frontmatter YAML string -> object normalisation (the guard you flagged) — I can pull into its own unit and test directly (empty/scalar/sequence/mapping). The translateBlock/translateInline helpers take mdast nodes, so unit-testing them in isolation would pull mdast types into the test suite, which the spec's Testing Decisions rule out; I'd keep those covered through parse(). Say the word if you'd rather I export and test them directly anyway.

Agreed — `index.ts` here is a pure re-export barrel (`export { parse } from "./parse.ts"; export type * from "./types.ts";`), so these tests were stranded at the entrypoint. Moved them (real tests + roadmap todos) into a co-located `parse_test.ts` importing `parse` from `./parse.ts`, and deleted `index_test.ts` — commit e32b71e. A broken re-export is still caught by typecheck and by the bin package importing `parse` through the public entrypoint in its E2E test. (Note the same doesn't apply to `bin/index.ts`, which is the actual CLI with logic, so its `index_test.ts` stays.) On testing the functions individually: `parse_test.ts` is now the right home for finer-grained cases. The one pure, mdast-free piece — the frontmatter YAML string -> object normalisation (the guard you flagged) — I can pull into its own unit and test directly (empty/scalar/sequence/mapping). The `translateBlock`/`translateInline` helpers take mdast nodes, so unit-testing them in isolation would pull mdast types into the test suite, which the spec's Testing Decisions rule out; I'd keep those covered through `parse()`. Say the word if you'd rather I export and test them directly anyway.
alexion marked this conversation as resolved
alexion added 1 commit 2026-07-28 23:31:24 -04:00
Cut the first vertical slice through both layers: a runnable
`kitchen view <file>` that reads a Recipe File, parses it, and renders
it styled to the terminal.

@kitchen-md/core exposes a pure, total `parse(input): DocumentAST` built
on a minimal remark pipeline (parse + frontmatter) with a translation
layer to core's own AST types — frontmatter passthrough, HeadingBlock,
ParagraphBlock, and TextNode. remark types never leak into the public
API.

@kitchen-md/bin's `view` subcommand (commander) reads the file and hands
the DocumentAST to a pure `render(ast): string` (chalk, ANSI
auto-suppressed off a TTY). Fallible file I/O is modelled as a neverthrow
Result over a tagged-union CliError, matched at the boundary: stdout on
success, stderr and exit 1 on failure. The command functions sit beside
the import.meta.main-guarded CLI entry so they are testable in-process.

Tests follow ADR 0009's tiers — unit (parse, render), integration (the
view command's Result), and e2e (the binary as a subprocess) — with
coverage, a path-scoped test-report generator, and a prose fixture
rounding out the tooling. ADR 0008 records errors-as-values at the CLI
boundary; ADR 0009 records the testing tiers.
alexion force-pushed task-0003-view-skeleton from 4ac11865c5 to ab309d3226 2026-07-28 23:31:24 -04:00 Compare
alexion merged commit ab309d3226 into main 2026-07-29 07:24:42 -04:00
alexion deleted branch task-0003-view-skeleton 2026-07-29 07:24:42 -04:00
This repo is archived. You cannot comment on pull requests.
No Reviewers
No Label
1 Participants
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: alexion/kitchen-md#2