feat: kitchen view command and parser skeleton (task 0003) #2
Reference in New Issue
Block a user
Delete Branch "task-0003-view-skeleton"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Task:
.claude/tasks/0003-view-skeleton.mdSummary
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, totalparse(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, andTextNode. Blocks are flat and in document order; remark's mdast does not appear in the public API. Types live in a dedicatedtypes.ts, re-exported from theindex.tsbarrel alongsideparse.@kitchen-md/bin: theviewsubcommand (commander) reads the file, callsparse, and passes the AST to a purerender(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, andformatErrormatches over aCliErrorunion that new fallible commands extend. The CLI entry matches theResultat the boundary: stdout on success; stderr plus exit 1 on failure. A missing argument prints commander usage and exits 1; a missing/unreadable file printskitchen: cannot read '<path>': <cause>and exits 1. The command functions (viewFile,formatError) sit beside theimport.meta.main-guarded CLI entry inindex.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:packages/core/src/parse_test.ts) — pureparseover inline strings, through the package barrel: frontmatter (arbitrary/empty/absent), headings at every level 1–6, paragraphTextNodecontent, flat document order, empty diagnostics.packages/bin/src/render_test.ts) — purerenderover inline ASTs, ANSI stripped: headings at every level, paragraph blank-line spacing, frontmatter YAML + separator, and their absence when frontmatter is empty.packages/bin/src/index_test.ts) — theviewcommand function asserted through its returnedResultin-process:okrender, frontmatter passthrough, aread-failederror for a missing path, andformatError's message.packages/bin/src/end_to_end_test.ts) — thekitchenbinary as a black box (subprocess): render + exit 0, document order, ANSI suppression on a pipe, the missing-file and missing-argument errors, andkitchen --help.The value-returning seams (
parse,render,viewFile,formatError) are 100% covered in-process; the thin.action()dispatch that writes to the streams and callsprocess.exitis 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 committedfixtures/prose.md.fixtures/prose.mdis a slice-scoped recipe (frontmatter + headings + plain paragraphs only) so it renders losslessly today; the fullfixtures/basic.mdend-to-end smoke stays in task 0007, which needs 0004's richer nodes first.Deviations / scope boundaries
translateInlinekeeps onlyTextNodes and drops other inline nodes by whole node (so emphasised text is currently lost). Lossless raw fallbacks and typed richer nodes are task 0004.{}for empty/absent.Diagnosticis defined minimally as{ severity, code, message }— the element type of the currently-emptydiagnosticsarray; 0007 emits the firstinvalid-frontmatterdiagnostic and adds any locator fields additively.Key decisions
renderstays its own module; theviewcommand logic was folded intoindex.tsbeside the guarded CLI entry (it is the only caller ofrender).Diagnostictrimmed to the fields this slice produces — dropped the unusedPoint/Positiontypes andsource?/position?fields.Review
Risk
Overall: Low
index.tsbarrel; no existing callers touched.Result, and the CLI as a subprocess.Standards findings left unaddressed
render.ts—"─".repeat(40)hardcodes the separator width (baseline magic-number, note-only). Left inline: theSEPARATORname already conveys intent and a fixed display width is reasonable for a pure function with no terminal-width access.Spec findings left unaddressed
translateInlinewhole-node drop is within this slice's stated scope (onlyTextNodeis modelled) and its lossless successor is explicitly owned by task 0004.@@ -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);Why use
flatMapif the array returned is always of size 0 or 1?I suspect it's to deal with empty files.
@@ -0,0 +20,4 @@return {};}const data = parseYaml(yamlNode.value);if (data !== null && typeof data === "object" && !Array.isArray(data)) {Why are we being so restrictive with what types of yaml parsed?
@@ -0,0 +19,4 @@message: string;source?: string;position?: Position;}Is this ever created anywhere? The only diagnostics I see is always an empty array.
@@ -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);It is the map-or-drop idiom:
translateBlockreturns[]to skip an unmodelled node or[block]to keep one, andflatMapflattens 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 justtree.children === [], handled the same way regardless.translateInlineuses it for the same reason. Happy to switch to an explicit push-loop if you find that reads clearer.@@ -0,0 +20,4 @@return {};}const data = parseYaml(yamlNode.value);if (data !== null && typeof data === "object" && !Array.isArray(data)) {Frontmatteris a mapping type (Record<string, unknown>), but YAML can also parse to a bare scalar (e.g.42), a top-level sequence, ornull/~. The three conditions narrow to exactly a plain mapping — bothnulland arrays report atypeofofobject, which is why the explicit!== nulland!Array.isArrayare 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 aninvalid-frontmatter-style diagnostic in task 0007.@@ -0,0 +19,4 @@type: "heading";level: 1 | 2 | 3 | 4 | 5 | 6;children: InlineNode[];}Correct — nothing constructs one yet;
diagnosticsis always[]in this slice. Trimmed in8687b75: droppedPoint,Position, and thesource?/position?fields, leavingDiagnosticas{ severity, code, message }— just the element type of the returned (empty) array. Task 0007 emits the first realinvalid-frontmatterdiagnostic and re-adds the locator fields additively, with its own tests.@@ -1,18 +1,93 @@import { describe, test } from "bun:test";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.
@@ -1,86 +0,0 @@import { describe, test } from "bun:test";Agreed —
index.tshere 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-locatedparse_test.tsimportingparsefrom./parse.ts, and deletedindex_test.ts— commite32b71e. A broken re-export is still caught by typecheck and by the bin package importingparsethrough the public entrypoint in its E2E test. (Note the same doesn't apply tobin/index.ts, which is the actual CLI with logic, so itsindex_test.tsstays.)On testing the functions individually:
parse_test.tsis 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). ThetranslateBlock/translateInlinehelpers 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 throughparse(). Say the word if you'd rather I export and test them directly anyway.4ac11865c5toab309d3226