From 8786d87d31ba271ede67c9ef1ff3442329869b0e Mon Sep 17 00:00:00 2001 From: Pedro Gomes Date: Mon, 24 Aug 2026 14:22:54 +0100 Subject: [PATCH 1/2] feat(layout): one Board's geometry, and the regions enumerated once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Board's Step tree in, flow-map geometry out. Nothing draws. `layout(board)` rather than `layout(doc)`: the canvas draws one Board at a time with a call as a doorway into another, so a function over the document computes four screens to draw one. A Placement is keyed by StepRef. Step ids are Board-local, so a bare id says two Blocks' `ret` share a position. `regionsOf` in @hatua/model is the single enumeration of what a container nests. walkSteps, stepLists and callsOf all read it, and so does the layout — so a region is covered by construction rather than by a fourth private walk with no corpus to disagree with it. That is why this package depends on the model; the model does not depend on it. A Fork's Branches are columns that converge. Every other region — a loop's body, a core.try's body and handler — is stacked under the card that owns it, each under a label band. A handler is not an alternative chosen between: it runs because the body failed, and part of the body has already run. The card height is answered here, from isContainer. Collapse is an input, and the only one that is not a function of the document; a collapsed container's children get no geometry, so the totals describe the map on screen. No Go mirror: a Host runner never lays anything out. Said in the doc comment so it is a decision rather than an omission. `--passWithNoTests` is gone from the package, so an empty suite fails. --- CONTEXT.md | 12 +- docs/handoff.md | 69 +++++ source/packages/layout/package.json | 5 +- source/packages/layout/src/fixtures.ts | 200 ++++++++++++ source/packages/layout/src/index.ts | 33 +- source/packages/layout/src/layout.test.ts | 249 +++++++++++++++ source/packages/layout/src/layout.ts | 326 ++++++++++++++++++++ source/packages/model/src/blocks.ts | 6 +- source/packages/model/src/tree.ts | 61 +++- source/packages/model/src/validity.ts | 15 +- source/packages/react/src/layouts/README.md | 20 ++ source/pnpm-lock.yaml | 3 + 12 files changed, 947 insertions(+), 52 deletions(-) create mode 100644 source/packages/layout/src/fixtures.ts create mode 100644 source/packages/layout/src/layout.test.ts create mode 100644 source/packages/layout/src/layout.ts diff --git a/CONTEXT.md b/CONTEXT.md index 7bceb21..9c68b83 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -185,7 +185,17 @@ _Avoid_: element, current, each, loop var The rule that a **Step**'s position on the flow map is computed from the tree on every render and never persisted. This is what guarantees a hand-edited **Workflow Definition** and the map can never disagree. -_Avoid_: auto-layout as a mere feature name — it is a constraint, not a convenience + +A **Placement** is one Step's box on the map, named by the **Board** it is on and its id together — +never by a bare id, which two **Blocks** may share. The map is laid out one **Board** at a time +(ADR-0013), and what it takes besides the Board is which containers are drawn collapsed: chrome, the +one input that is not a function of the document, and the reason the totals describe the map that is +actually on screen rather than one with folded regions counted into it. + +Positions are the builder's and nobody else's. A **Host** runner never lays anything out, so this is +the one cross-cutting rule in the repo implemented once rather than in both languages. +_Avoid_: auto-layout as a mere feature name — it is a constraint, not a convenience; node position +as something a document could carry **Published Version**: An immutable, numbered snapshot of a **Workflow Definition**, carrying `status: published`. Exactly diff --git a/docs/handoff.md b/docs/handoff.md index ac7e9a4..7ba853b 100644 --- a/docs/handoff.md +++ b/docs/handoff.md @@ -58,6 +58,75 @@ decision. A Host mounting one region gets no tab strip at all. --- +## Flow map geometry + +The numbers `@hatua/layout`'s `LAYOUT` carries, and what each one has to satisfy. The package cites +this section; change one in either place and the other is wrong. + +| Constant | Pixels | What it measures | +| --- | --- | --- | +| `nodeWidth` | 236 | A card's width. Every card is the same width, on every Board and at every depth. | +| `nodeHeight` | 64 | A card with a name and nothing else. | +| `nodeHeightWithMeta` | 100 | A card that also carries the meta row. | +| `verticalGap` | 96 | Between one card's bottom and the next card's top, down a column. | +| `branchGap` | 44 | Between two Branch columns. | +| `regionLabel` | 28 | Reserved above a child region for the label naming it. | +| `joinMarker` | 26 | Reserved below a Fork's Branches for the mark where they converge. | + +**Cards are a fixed size, and only two heights exist.** A card sized to its content makes a column's +rhythm a function of how long somebody's Step names are, and makes the map reflow when one is +renamed. The meta row is the container summary — how many Branches, how many Steps, whether there is +a handler — so a card is the taller one exactly when the Step owns child regions. That question is +`isContainer`, answered where the regions are enumerated rather than by whatever is drawing. + +**What the gaps have to satisfy.** `verticalGap` exceeds `nodeHeight`, so the space between two cards +reads as a run of the flow and not as a crack between two cards that nearly touch. `branchGap` is +much smaller than `nodeWidth`, so two columns read as siblings under one Fork rather than as two +separate maps; it is what stops adjacent columns touching and is the only thing keeping them apart, +so it cannot be zero. `regionLabel` fits one chip's line box, and every child region gets one. + +### Regions + +**A Fork's Branches are columns, and they converge.** They are alternatives chosen between, and +*which one* is the reader's question — so they sit side by side, separated by `branchGap`, over a +`joinMarker`'s worth of room for the mark where they come back together. + +**Every other region is stacked under the card that owns it.** A `core.try`'s body and its handler sit +one above the other, in document order, not side by side. They are not alternatives chosen between: +the handler runs *because* the body failed, and part of the body has already run by then. Columns +would make left-to-right mean "later" in the one place on the map where it means nothing else, and +would put a third thing on screen that reads as a Fork. + +**The label band is what tells regions apart** — `try` and `on failure` over a `core.try`'s two, +`loop` over a loop's one, `if` / `else if` / `and` over a Branch's. Every child region gets a band, so +the second region of a `core.try` costs no shape the first did not already have, and a loop body and +a protected body are told apart by the word over them rather than by their geometry. `` +gives the same answer with the chip over each region; the two surfaces draw differently but they do +not disagree about which regions there are or what they are called. + +### The root node + +The canvas draws one node above the first Step: the Triggers on the root Board, the Block's contract +inside one. It is **chrome, not a `steps[]` entry**, which is what makes `once:`/`fixed:` unnecessary +and keeps `removeStep`, `walkSteps` and `unknownComponents` from needing a case for it. + +So it names no Step, and `FlowMap` carries it as a plain `Rect` beside the `Placement[]` rather than +as a `Placement` with no id. A `Placement` whose Step reference were optional would push "sometimes +there is no Step here" into every consumer's type to spare exactly one field here. + +### Collapse + +**Collapse is an input to the layout, and the only one that is not a function of the document.** It is +chrome — the document has no key for it, because a view state in the file is a diff in the Host's +repository every time somebody folds a loop shut. + +A collapsed container's children get **no geometry at all**, rather than geometry the canvas then +hides. Laying them out anyway would leave the map's total width and height describing a map nobody is +looking at, and everything reading a total — the scroll extent, fit-to-screen, a minimap — would be +reading a number that is wrong whenever anything is folded. + +--- + ## TopBar Left cluster carries identity: `workflows /`, the workflow's name, its slug, then the version diff --git a/source/packages/layout/package.json b/source/packages/layout/package.json index c86df76..d435425 100644 --- a/source/packages/layout/package.json +++ b/source/packages/layout/package.json @@ -20,10 +20,11 @@ "scripts": { "build": "vite build", "typecheck": "tsc --noEmit", - "test": "vitest run --passWithNoTests", - "test:coverage": "vitest run --coverage --coverage.reporter=text-summary --coverage.reporter=json-summary --coverage.reporter=lcovonly --passWithNoTests" + "test": "vitest run", + "test:coverage": "vitest run --coverage --coverage.reporter=text-summary --coverage.reporter=json-summary --coverage.reporter=lcovonly" }, "dependencies": { + "@hatua/model": "workspace:*", "@hatua/schema": "workspace:*" } } diff --git a/source/packages/layout/src/fixtures.ts b/source/packages/layout/src/fixtures.ts new file mode 100644 index 0000000..0221c3d --- /dev/null +++ b/source/packages/layout/src/fixtures.ts @@ -0,0 +1,200 @@ +import type { WorkflowDefinition } from '@hatua/schema' + +/** The shell every fixture below fills in, so each one is only its Step tree. */ +const workflow = (name: string, rest: Partial): WorkflowDefinition => ({ + id: `wf_${name}`, + name, + version: 1, + status: 'draft', + triggers: [{ id: 'nightly', use: 'core.schedule', with: {} }], + steps: [], + ...rest, +}) + +/** + * Every region a container owns, on one Board: two Branches, a loop body, and a + * `core.try`'s body and handler. + * + * The document the coverage test is worth running against. A fixture missing a + * region would let a layout that forgets that region pass, which is the failure + * the test exists to make impossible rather than unlikely. + */ +export const ALL_REGIONS = workflow('all regions', { + steps: [ + { id: 'fetch', use: 'component.email.fetch', with: {} }, + { + id: 'sort', + use: 'core.fork', + with: { mode: 'condition' }, + branches: [ + { + label: 'Has new mail', + when: '{{steps.fetch.count}} > 0', + steps: [ + { + id: 'each', + use: 'core.for_each', + with: { list: '{{steps.fetch.messages}}' }, + steps: [ + { + id: 'guarded', + use: 'core.try', + with: { attempts: 3 }, + steps: [{ id: 'triage', use: 'component.agent.act', with: {} }], + handler: [{ id: 'shelve', use: 'component.email.send', with: {} }], + }, + ], + }, + ], + }, + { label: 'Otherwise', steps: [{ id: 'quiet', use: 'core.end' }] }, + ], + }, + { id: 'digest', use: 'component.email.send', with: {} }, + ], +}) + +/** + * Two Blocks, each holding a Step called `ret`. + * + * Step ids are Board-local (ADR-0013), so this is the document that tells a + * Placement keyed by a `StepRef` from one keyed by a bare id: under a bare id + * the two `ret`s are one entry, and one of the two Blocks draws the other's map. + */ +export const TWO_RETS = workflow('two rets', { + steps: [ + { id: 'first', use: 'block.alpha', with: {} }, + { id: 'second', use: 'block.beta', with: {} }, + ], + blocks: [ + { + id: 'alpha', + outputs: [{ k: 'out', label: 'Out', t: 'text' }], + steps: [{ id: 'ret', use: 'core.return', with: { out: 'a' } }], + }, + { + id: 'beta', + outputs: [{ k: 'out', label: 'Out', t: 'text' }], + steps: [{ id: 'ret', use: 'core.return', with: { out: 'b' } }], + }, + ], +}) + +/** A Board with no Steps at all: the root node and nothing under it. */ +export const EMPTY_BOARD = workflow('empty', { steps: [] }) + +/** A straight run of leaves, the shape a layout is easiest to get wrong on last. */ +export const STRAIGHT = workflow('straight', { + steps: [ + { id: 's1', use: 'component.email.fetch', with: {} }, + { id: 's2', use: 'component.agent.act', with: {} }, + { id: 's3', use: 'component.email.send', with: {} }, + ], +}) + +/** Containers holding regions with nothing in them, and a Fork of three. */ +export const EMPTY_REGIONS = workflow('empty regions', { + steps: [ + { id: 'try_nothing', use: 'core.try', with: {}, steps: [], handler: [] }, + { + id: 'wide', + use: 'core.fork', + with: { mode: 'parallel' }, + branches: [ + { label: 'One', steps: [{ id: 'a', use: 'core.end' }] }, + { label: 'Two', steps: [] }, + { + label: 'Three', + steps: [ + { id: 'b', use: 'core.repeat', until: '{{var.done}}', steps: [] }, + { id: 'c', use: 'core.end' }, + ], + }, + ], + }, + ], +}) + +/** Deep nesting: a loop inside a handler inside a branch inside a loop. */ +export const DEEP = workflow('deep', { + steps: [ + { + id: 'outer', + use: 'core.repeat', + until: '{{var.done}}', + steps: [ + { + id: 'pick', + use: 'core.fork', + with: { mode: 'condition' }, + branches: [ + { + label: 'Risky', + when: '{{var.risky}}', + steps: [ + { + id: 'attempt', + use: 'core.try', + with: { attempts: 2 }, + steps: [{ id: 'call', use: 'block.alpha', with: {} }], + handler: [ + { + id: 'sweep', + use: 'core.for_each', + with: { list: '{{steps.attempt.error.items}}' }, + steps: [{ id: 'note', use: 'component.email.send', with: {} }], + }, + ], + }, + ], + }, + { label: 'Otherwise', steps: [] }, + ], + }, + ], + }, + ], + blocks: [{ id: 'alpha', steps: [{ id: 'ret', use: 'core.return', with: {} }] }], +}) + +/** + * One Step carrying Branches, a body and a handler at once. + * + * No verb owns all three, and nothing refuses a document that writes them — the + * schema's step keys are all optional and no rule reads them together. So this + * is the shape that separates "lay out the regions in hand" from "lay out the + * regions a Fork has": the second drops two of these three, silently. + */ +export const MIXED_REGIONS = workflow('mixed regions', { + steps: [ + { + id: 'confused', + use: 'core.fork', + with: { mode: 'condition' }, + branches: [ + { label: 'One', when: '{{var.go}}', steps: [{ id: 'in_branch', use: 'core.end' }] }, + { label: 'Two', steps: [{ id: 'in_other', use: 'core.end' }] }, + ], + steps: [{ id: 'in_body', use: 'core.end' }], + handler: [{ id: 'in_handler', use: 'core.end' }], + }, + ], +}) + +/** + * Every shape above, for the properties that must hold of all of them. + * + * A property checked against one hand-written example is a property checked + * against one hand-written example. Iterated rather than repeated, and asserted + * non-empty where it is iterated, because a list that shrank to nothing makes + * every property over it pass while checking none. + */ +export const SHAPES: readonly { name: string; doc: WorkflowDefinition }[] = [ + { name: 'all regions', doc: ALL_REGIONS }, + { name: 'two rets', doc: TWO_RETS }, + { name: 'empty board', doc: EMPTY_BOARD }, + { name: 'straight', doc: STRAIGHT }, + { name: 'empty regions', doc: EMPTY_REGIONS }, + { name: 'deep', doc: DEEP }, + { name: 'mixed regions', doc: MIXED_REGIONS }, +] diff --git a/source/packages/layout/src/index.ts b/source/packages/layout/src/index.ts index c1643d3..626835b 100644 --- a/source/packages/layout/src/index.ts +++ b/source/packages/layout/src/index.ts @@ -1,32 +1 @@ -/** - * Derived layout: step tree in, flow-map geometry out. - * - * Positions are computed on every render and never stored (ADR-0001) — the map - * is a reading of the tree, so a hand-edited Workflow Definition cannot - * disagree with it. Constants come from the design handoff. - */ - -export const LAYOUT = { - nodeWidth: 236, - nodeHeight: 64, - /** Node height when the card shows a meta row. */ - nodeHeightWithMeta: 100, - verticalGap: 96, - /** Horizontal gap between branch columns. */ - branchGap: 44, - joinMarker: 26, -} as const - -export interface Placement { - stepId: string - x: number - y: number - width: number - height: number -} - -export interface FlowMap { - placements: Placement[] - width: number - height: number -} +export * from './layout' diff --git a/source/packages/layout/src/layout.test.ts b/source/packages/layout/src/layout.test.ts new file mode 100644 index 0000000..694c1c2 --- /dev/null +++ b/source/packages/layout/src/layout.test.ts @@ -0,0 +1,249 @@ +import { type Board, boards, stepKey, walkSteps } from '@hatua/model' +import type { WorkflowDefinition } from '@hatua/schema' +import { describe, expect, it } from 'vitest' +import { ALL_REGIONS, DEEP, EMPTY_BOARD, MIXED_REGIONS, SHAPES, TWO_RETS } from './fixtures' +import { LAYOUT, layout, type Placement, placementOf, type Rect } from './layout' + +const boardsOf = (doc: WorkflowDefinition): Board[] => [...boards(doc)] + +const at = (map: ReturnType, id: string): Placement => { + const found = placementOf(map, { board: map.board, id }) + if (!found) throw new Error(`No Placement for "${id}"`) + return found +} + +const overlaps = (a: Rect, b: Rect) => + a.x < b.x + b.width && b.x < a.x + a.width && a.y < b.y + b.height && b.y < a.y + a.height + +describe('layout', () => { + // A glob or a list that quietly matched nothing makes every property below + // pass while checking none of them. + it('has shapes to lay out', () => { + expect(SHAPES.length).toBeGreaterThan(0) + expect(SHAPES.flatMap(({ doc }) => boardsOf(doc)).length).toBeGreaterThan(SHAPES.length) + }) + + describe('every Step is placed exactly once', () => { + for (const { name, doc } of SHAPES) { + it(name, () => { + for (const board of boardsOf(doc)) { + const map = layout(board) + const walked = [...walkSteps(board.steps)].map((step) => + stepKey({ board: board.id, id: step.id }), + ) + const placed = map.placements.map((placement) => stepKey(placement.ref)) + + // Sorted sets first: when a region is forgotten, that is the readable + // failure. Then the counts, which is what catches a Step placed twice. + expect([...placed].sort()).toEqual([...walked].sort()) + expect(placed.length).toBe(walked.length) + } + }) + } + }) + + it('covers all four regions in the fixture that cross-checks the walk', () => { + const ids = [...walkSteps(ALL_REGIONS.steps)].map((step) => step.id) + // A Step from each region: a Branch's, the other Branch's, a loop body's, a + // try's body and a try's handler. + expect(ids).toEqual(expect.arrayContaining(['each', 'quiet', 'guarded', 'triage', 'shelve'])) + }) + + it('gives two Blocks that each hold a `ret` two addressable Placements', () => { + const [, alpha, beta] = boardsOf(TWO_RETS) + if (!alpha || !beta) throw new Error('fixture lost a Block') + + const one = at(layout(alpha), 'ret') + const other = at(layout(beta), 'ret') + + expect(stepKey(one.ref)).toBe('alpha/ret') + expect(stepKey(other.ref)).toBe('beta/ret') + expect(stepKey(one.ref)).not.toBe(stepKey(other.ref)) + }) + + describe('the same Board laid out twice is the same map', () => { + for (const { name, doc } of SHAPES) { + it(name, () => { + for (const board of boardsOf(doc)) { + // Serialised rather than field-checked: ADR-0001's promise is that the + // map is a function of the document, and spot-checking a few numbers + // is not that promise. + expect(JSON.stringify(layout(board))).toBe(JSON.stringify(layout(board))) + } + }) + } + }) + + describe('no two cards overlap', () => { + for (const { name, doc } of SHAPES) { + it(name, () => { + for (const board of boardsOf(doc)) { + const map = layout(board) + for (const [i, a] of map.placements.entries()) { + expect(overlaps(a, map.root)).toBe(false) + for (const b of map.placements.slice(i + 1)) { + expect({ a: stepKey(a.ref), b: stepKey(b.ref), hit: overlaps(a, b) }).toEqual({ + a: stepKey(a.ref), + b: stepKey(b.ref), + hit: false, + }) + } + } + } + }) + } + }) + + describe('the totals describe the map that was drawn', () => { + for (const { name, doc } of SHAPES) { + it(name, () => { + for (const board of boardsOf(doc)) { + const map = layout(board) + for (const box of [map.root, ...map.placements]) { + expect(box.x).toBeGreaterThanOrEqual(0) + expect(box.y).toBeGreaterThanOrEqual(0) + expect(box.x + box.width).toBeLessThanOrEqual(map.width) + expect(box.y + box.height).toBeLessThanOrEqual(map.height) + } + } + }) + } + }) + + it('draws the Board root above every Step on it', () => { + const [board] = boardsOf(ALL_REGIONS) + if (!board) throw new Error('fixture lost its root Board') + const map = layout(board) + + expect(map.root.y).toBe(0) + for (const placement of map.placements) { + expect(placement.y).toBeGreaterThanOrEqual(map.root.y + map.root.height) + } + }) + + it('is the root node and nothing else on an empty Board', () => { + const [board] = boardsOf(EMPTY_BOARD) + if (!board) throw new Error('fixture lost its root Board') + const map = layout(board) + + expect(map.placements).toEqual([]) + expect(map.height).toBe(LAYOUT.nodeHeight) + expect(map.width).toBe(LAYOUT.nodeWidth) + }) +}) + +describe('regions', () => { + const rootOf = (doc: WorkflowDefinition) => { + const [board] = boardsOf(doc) + if (!board) throw new Error('fixture lost its root Board') + return layout(board) + } + + it('lays a Fork out as columns', () => { + const map = rootOf(ALL_REGIONS) + const left = at(map, 'each') + const right = at(map, 'quiet') + + expect(left.x).not.toBe(right.x) + // Side by side means neither column's cards reach into the other's band. + expect(Math.abs(left.x - right.x)).toBeGreaterThanOrEqual(LAYOUT.branchGap) + }) + + it('stacks a `core.try`’s body above its handler on one spine', () => { + const map = rootOf(ALL_REGIONS) + const body = at(map, 'triage') + const handler = at(map, 'shelve') + + expect(handler.y).toBeGreaterThan(body.y + body.height) + expect(handler.x).toBe(body.x) + }) + + it('leaves each region room for the label that names it', () => { + const map = rootOf(ALL_REGIONS) + const container = at(map, 'guarded') + const body = at(map, 'triage') + + expect(body.y - (container.y + container.height)).toBe(LAYOUT.verticalGap + LAYOUT.regionLabel) + }) + + it('lays out every region a Step carries, not the ones its verb implies', () => { + const map = rootOf(MIXED_REGIONS) + const columns = [at(map, 'in_branch'), at(map, 'in_other')] + const stacked = [at(map, 'in_body'), at(map, 'in_handler')] + + expect(columns[0]?.x).not.toBe(columns[1]?.x) + // The stacked regions sit below the columns rather than in place of them. + for (const column of columns) { + for (const below of stacked) expect(below.y).toBeGreaterThan(column.y) + } + expect(stacked[1]?.y).toBeGreaterThan(stacked[0]?.y ?? 0) + }) + + it('gives a container the taller card and a leaf the shorter one', () => { + const map = rootOf(ALL_REGIONS) + + expect(at(map, 'guarded').height).toBe(LAYOUT.nodeHeightWithMeta) + expect(at(map, 'triage').height).toBe(LAYOUT.nodeHeight) + }) +}) + +describe('collapse', () => { + const boardOf = (doc: WorkflowDefinition): Board => { + const [board] = boardsOf(doc) + if (!board) throw new Error('fixture lost its root Board') + return board + } + + it('places nothing for a collapsed container’s children', () => { + const board = boardOf(ALL_REGIONS) + const map = layout(board, { collapsed: new Set(['guarded']) }) + const placed = map.placements.map((placement) => stepKey(placement.ref)) + + expect(placed).toContain('guarded') + expect(placed).not.toContain('triage') + expect(placed).not.toContain('shelve') + }) + + it('collapses every region of a container, not the first one', () => { + const board = boardOf(DEEP) + const open = layout(board) + const shut = layout(board, { collapsed: new Set(['attempt']) }) + + for (const id of ['call', 'sweep', 'note']) { + expect(open.placements.some((one) => one.ref.id === id)).toBe(true) + expect(shut.placements.some((one) => one.ref.id === id)).toBe(false) + } + }) + + it('shrinks the map rather than hiding cards inside it', () => { + const board = boardOf(ALL_REGIONS) + const open = layout(board) + const shut = layout(board, { collapsed: new Set(['sort']) }) + + expect(shut.height).toBeLessThan(open.height) + expect(shut.width).toBeLessThanOrEqual(open.width) + }) + + it('is a function of the collapsed set, and of nothing else', () => { + const board = boardOf(ALL_REGIONS) + const collapsed = new Set(['guarded']) + + expect(JSON.stringify(layout(board, { collapsed }))).toBe( + JSON.stringify(layout(board, { collapsed: new Set(['guarded']) })), + ) + }) +}) + +describe('nothing is persisted', () => { + for (const { name, doc } of SHAPES) { + it(name, () => { + const before = structuredClone(doc) + for (const board of boards(doc)) layout(board, { collapsed: new Set(['guarded', 'sort']) }) + // No position reaches the document, and no Step gains a key it did not + // have: the map is a reading of the tree (ADR-0001), so a layout call the + // document could notice is the whole promise gone. + expect(doc).toEqual(before) + expect(JSON.stringify(doc)).toBe(JSON.stringify(before)) + }) + } +}) diff --git a/source/packages/layout/src/layout.ts b/source/packages/layout/src/layout.ts new file mode 100644 index 0000000..b9e7eeb --- /dev/null +++ b/source/packages/layout/src/layout.ts @@ -0,0 +1,326 @@ +import { + type Board, + type BoardId, + isContainer, + type Region, + regionsOf, + type StepRef, + stepKey, +} from '@hatua/model' +import type { Step } from '@hatua/schema' + +/** + * Derived layout: one Board's Step tree in, flow-map geometry out. + * + * Positions are computed on every render and never stored (ADR-0001) — the map + * is a reading of the tree, so a hand-edited Workflow Definition cannot + * disagree with it. Nothing here draws: it answers where each card goes and how + * big the map is, and the canvas paints that. + * + * ## One Board at a time + * + * `layout` takes a Board rather than a document, because the canvas draws one + * Board at a time with a call as a doorway into another (ADR-0013). A function + * over the whole document would compute four screens to draw one, and would + * have to invent a second coordinate space to hold them apart. + * + * ## The walk is not restated here + * + * Which regions a Step nests is `regionsOf`, in @hatua/model. That is the whole + * reason this package depends on the model rather than on @hatua/schema alone: + * a container's regions are already enumerated three times over there — + * `walkSteps`, `stepLists`, `callsOf` — and a fourth copy inside a package with + * no corpus and no runner to disagree with it is the copy that would silently + * drop a region and be found by nobody. The edge is safe in the direction it + * points: the model does not import this package, and must not. + * + * ## There is no Go mirror, deliberately + * + * Every other cross-cutting rule in this repo is implemented twice and pinned by + * `conformance/`. This one is implemented once. Hatua does not execute, and a + * Host runner never lays anything out — geometry is the builder's, and the + * builder is TypeScript. A `sdk/go/layout.go` would be a second implementation + * of a question no runner asks, with no corpus to catch it drifting. + * + * ## Where the numbers come from + * + * `docs/handoff.md` § Flow map geometry. Change one there and here together. + */ + +export const LAYOUT = { + nodeWidth: 236, + nodeHeight: 64, + /** Node height when the card shows a meta row. */ + nodeHeightWithMeta: 100, + verticalGap: 96, + /** Horizontal gap between branch columns. */ + branchGap: 44, + /** + * Height reserved above a child region for the label that names it — `if` over + * a Branch, `try` and `on failure` over a `core.try`'s two. + */ + regionLabel: 28, + /** Height reserved below a Fork's branches for the mark where they converge. */ + joinMarker: 26, +} as const + +/** A box on the map, in flow-map coordinates with the origin at the top left. */ +export interface Rect { + readonly x: number + readonly y: number + readonly width: number + readonly height: number +} + +/** + * Where one Step's card goes. + * + * Keyed by `StepRef` and never by a bare id: Step ids are Board-local, so two + * Blocks may each hold a Step called `ret` and a bare `ret` would say the two + * share a position (ADR-0013). `stepKey` is the flat spelling where one is + * needed — a React key, a `data-` attribute — and it is minted in one place so + * two readers cannot pick two separators. + */ +export interface Placement extends Rect { + readonly ref: StepRef +} + +/** + * One Board's geometry. + * + * `root` is the node the canvas draws above the first Step: the Triggers on the + * root Board, the Block's contract inside one. It is a `Rect` and not a + * `Placement` because it names no Step — it is derived from `doc.triggers[]` as + * chrome rather than living in `steps[]`, which is what keeps `removeStep`, + * `walkSteps` and `unknownComponents` from needing a special case for it. A + * `Placement` with an optional `ref` would push "sometimes there is no Step + * here" into every consumer's type instead, to spare exactly one field here. + */ +export interface FlowMap { + readonly board: BoardId + readonly root: Rect + readonly placements: readonly Placement[] + readonly width: number + readonly height: number +} + +export interface LayoutOptions { + /** + * The ids of containers drawn collapsed, on this Board. + * + * The one input that is not a function of the document, and it is a parameter + * for exactly that reason. Collapse is chrome — `StepList` holds it in React + * state and the Workflow Definition has no key for it, because a view state + * in the document is a diff in the Host's repository every time someone folds + * a loop shut. + * + * A collapsed container's children get no Placement at all, rather than a + * Placement the canvas then hides. Laying them out anyway would make `height` + * and `width` describe a map nobody is looking at, and every consumer of a + * total — the scroll extent, fit-to-screen, a minimap — would be reading a + * number that is wrong whenever anything is folded. + * + * Bare ids rather than `StepRef`s: a Board is already the argument, and ids + * are unique on one. + */ + readonly collapsed?: ReadonlySet +} + +/** + * How tall one Step's card is. + * + * The layout package answers this, rather than taking a height from the caller. + * A caller-supplied height would leave `FlowMap.width` and `.height` as totals + * over numbers this package never saw, so it could not compute them honestly — + * and "how tall is a card" would have as many answers as there are callers. + * + * The meta row carries the container summary — how many branches, how many + * steps, whether there is a handler — so a card is the taller one exactly when + * the Step owns child regions. That is `isContainer`, asked of `regionsOf`: the + * same enumeration that decides what this recurses into, so a card cannot be + * the short one and still open into something. + */ +export const heightOf = (step: Step): number => + isContainer(step) ? LAYOUT.nodeHeightWithMeta : LAYOUT.nodeHeight + +/** + * One Board's Step tree as geometry. + * + * A pure function of `(board, collapsed)`: the same Board laid out twice is the + * same map, which is what ADR-0001's promise rests on. Nothing is written back — + * the Board and every Step in it are read and never touched. + */ +export function layout(board: Board, options: LayoutOptions = {}): FlowMap { + const collapsed = options.collapsed ?? NOTHING_COLLAPSED + const body = stack(board.steps, board.id, collapsed) + const width = Math.max(LAYOUT.nodeWidth, body.width) + + const root: Rect = { + x: centre(width, LAYOUT.nodeWidth), + y: 0, + width: LAYOUT.nodeWidth, + height: LAYOUT.nodeHeight, + } + + // An empty Board is the root node and nothing else. Reserving the gap below + // it would leave the map taller than everything drawn on it. + if (board.steps.length === 0) { + return { board: board.id, root, placements: [], width, height: LAYOUT.nodeHeight } + } + + const top = LAYOUT.nodeHeight + LAYOUT.verticalGap + return { + board: board.id, + root, + placements: shift(body.placements, centre(width, body.width), top), + width, + height: top + body.height, + } +} + +/** One Board's Placement for a Step, or undefined when that Step is collapsed away. */ +export const placementOf = (map: FlowMap, ref: StepRef): Placement | undefined => + map.placements.find((placement) => stepKey(placement.ref) === stepKey(ref)) + +const NOTHING_COLLAPSED: ReadonlySet = new Set() + +/** + * A laid-out fragment, positioned relative to its own top-left corner. + * + * Every combinator below takes boxes and returns a box, so a fragment is placed + * once, by whatever encloses it, and never has to know where it ended up. + */ +interface Box { + readonly width: number + readonly height: number + readonly placements: readonly Placement[] +} + +/** The empty region: no cards, but a card's width so its label has somewhere to sit. */ +const EMPTY: Box = { width: LAYOUT.nodeWidth, height: 0, placements: [] } + +/** + * A step list as a column: each Step under the last, centred on one spine. + * + * Time runs down the map, so the reading order of a list is its vertical order + * and nothing else carries it. + */ +function stack(steps: readonly Step[], board: BoardId, collapsed: ReadonlySet): Box { + if (steps.length === 0) return EMPTY + + const boxes = steps.map((step) => place(step, board, collapsed)) + const width = Math.max(...boxes.map((box) => box.width)) + const placements: Placement[] = [] + let y = 0 + + for (const box of boxes) { + if (y > 0) y += LAYOUT.verticalGap + placements.push(...shift(box.placements, centre(width, box.width), y)) + y += box.height + } + + return { width, height: y, placements } +} + +/** + * One Step's card and everything it nests. + * + * A Fork's Branches become columns side by side and converge on a join marker, + * because they are alternatives chosen between, and *which one* is the reader's + * question. + * + * Every other region is stacked below the card, in document order — so a + * `core.try`'s body and handler sit one above the other rather than beside each + * other. They are not alternatives chosen between: the handler runs *because* + * the body failed, and part of the body has already run by then. Drawing them + * as columns would make left-to-right mean "later" in the one place on the map + * where it means nothing else, and would put a third thing on screen that reads + * as a Fork. + * + * What tells the two regions apart, and tells either of them from a loop body, + * is the label band above each one — `try` and `on failure`, `loop` — which is + * the same answer `StepList` gives with the chip over each region. Every child + * region gets one, including a Branch's, so a second region costs no shape the + * first did not already have. + * + * Both kinds are laid out, not one or the other. No verb owns both `branches:` + * and a `steps:` body, but nothing refuses a document that writes both — the + * schema's step keys are all optional and no rule reads them together — so a + * Step that carries both is laid out with all of its regions rather than half of + * them. Branching on "is this a Fork" instead of on the regions in hand is how a + * reader silently drops one. + */ +function place(step: Step, board: BoardId, collapsed: ReadonlySet): Box { + const height = heightOf(step) + const regions = collapsed.has(step.id) ? [] : [...regionsOf(step)] + const bands: Box[] = [] + + const branches = regions.filter((region) => region.kind === 'branch') + if (branches.length > 0) bands.push(join(spread(labelledAll(branches, board, collapsed)))) + for (const region of regions.filter((one) => one.kind !== 'branch')) { + bands.push(labelled(stack(region.steps, board, collapsed))) + } + + const width = Math.max(LAYOUT.nodeWidth, ...bands.map((band) => band.width)) + const placements: Placement[] = [ + { + ref: { board, id: step.id }, + x: centre(width, LAYOUT.nodeWidth), + y: 0, + width: LAYOUT.nodeWidth, + height, + }, + ] + + let y = height + for (const band of bands) { + y += LAYOUT.verticalGap + placements.push(...shift(band.placements, centre(width, band.width), y)) + y += band.height + } + + return { width, height: y, placements } +} + +const labelledAll = ( + regions: readonly Region[], + board: BoardId, + collapsed: ReadonlySet, +): Box[] => regions.map((region) => labelled(stack(region.steps, board, collapsed))) + +/** A region under the band that names it. */ +const labelled = (box: Box): Box => ({ + width: box.width, + height: LAYOUT.regionLabel + box.height, + placements: shift(box.placements, 0, LAYOUT.regionLabel), +}) + +/** Boxes side by side, left to right, in document order. */ +function spread(boxes: readonly Box[]): Box { + const placements: Placement[] = [] + let x = 0 + + for (const box of boxes) { + if (x > 0) x += LAYOUT.branchGap + placements.push(...shift(box.placements, x, 0)) + x += box.width + } + + return { width: x, height: Math.max(...boxes.map((box) => box.height)), placements } +} + +/** Room below a spread for the mark where its columns converge. */ +const join = (box: Box): Box => ({ ...box, height: box.height + LAYOUT.joinMarker }) + +const shift = (placements: readonly Placement[], dx: number, dy: number): Placement[] => + placements.map((placement) => ({ ...placement, x: placement.x + dx, y: placement.y + dy })) + +/** + * Where a `width`-wide box sits inside an `outer`-wide one. + * + * Floored rather than rounded, so a centred fragment can never round its way + * past its parent's right edge and into the gap beside a sibling column. Every + * coordinate on the map is therefore a whole number of pixels, which is also + * what makes two layouts of one Board compare byte for byte. + */ +const centre = (outer: number, width: number): number => Math.floor((outer - width) / 2) diff --git a/source/packages/model/src/blocks.ts b/source/packages/model/src/blocks.ts index 014105e..5385c43 100644 --- a/source/packages/model/src/blocks.ts +++ b/source/packages/model/src/blocks.ts @@ -1,6 +1,6 @@ import type { Slot } from '@hatua/expressions' import type { Block, Declaration, Step, WorkflowDefinition } from '@hatua/schema' -import { own } from './tree' +import { own, regionsOf } from './tree' /** * Blocks: what a call takes, what a return publishes, and which Blocks reach @@ -86,9 +86,7 @@ export function callsOf(steps: readonly Step[]): string[] { for (const step of list) { const id = blockIdOf(step.use) if (id !== null) found.push(id) - for (const branch of step.branches ?? []) walk(branch.steps) - if (step.steps) walk(step.steps) - if (step.handler) walk(step.handler) + for (const region of regionsOf(step)) walk(region.steps) } } walk(steps) diff --git a/source/packages/model/src/tree.ts b/source/packages/model/src/tree.ts index aff8051..50cc5a4 100644 --- a/source/packages/model/src/tree.ts +++ b/source/packages/model/src/tree.ts @@ -1,4 +1,4 @@ -import type { Block, Step, Variable, WorkflowDefinition } from '@hatua/schema' +import type { Block, Branch, Step, Variable, WorkflowDefinition } from '@hatua/schema' /** * Pure domain rules over the step tree. No state, no I/O, no YAML — those live @@ -67,21 +67,64 @@ export function boardOf(doc: WorkflowDefinition, id: BoardId): Board | undefined return undefined } +/** Which of a container's child regions a step list is. */ +export type RegionKind = 'branch' | 'body' | 'handler' + +/** + * One child region a container Step owns, and which region it is. + * + * A Fork contributes one per Branch, a `core.for_each` and a `core.repeat` one + * body each, and a `core.try` two — a body under `steps:` and a handler under + * `handler:` (ADR-0013). + */ +export interface Region { + readonly kind: RegionKind + readonly steps: readonly Step[] + /** The Branch this region is. Absent on a body and on a handler. */ + readonly branch?: Branch +} + +/** + * The regions one Step owns, in document order. + * + * The single answer to "what does this Step nest". Every traversal of the tree + * asks that question, and one that answers it for itself is one that can forget + * a region — a region no rule then sees, reported by nothing, in silence. That + * has to be spelled out once so a reader gains coverage of a new region by + * construction rather than by remembering to ask for it. + * + * The region is named rather than yielded as a bare list because a reader that + * draws differently per region — branches side by side, a try's two regions + * stacked under their own labels — still has to get its regions from here. + */ +export function* regionsOf(step: Step): Generator { + for (const branch of step.branches ?? []) yield { kind: 'branch', branch, steps: branch.steps } + if (step.steps) yield { kind: 'body', steps: step.steps } + if (step.handler) yield { kind: 'handler', steps: step.handler } +} + +/** + * Whether a Step owns child regions at all. + * + * Asked of `regionsOf` rather than of the three keys, so "container" and "what a + * container nests" cannot come apart — a fourth region would otherwise be + * walked by every reader while still reading as a leaf to whichever surface + * decides how tall a card is or whether it collapses. + */ +export const isContainer = (step: Step): boolean => !regionsOf(step).next().done + /** * Depth-first walk of every step in one tree, parents before children. * - * Every region a container owns is walked here and nowhere else: a Fork's - * branches, a loop body, and a `core.try`'s handler. A region this forgets is a - * region no rule ever sees — the validator reports nothing about it, silently, - * which is the same failure as a validator that only ever looked at the root - * Board. + * Every region a container owns is walked here and nowhere else. A region + * `regionsOf` forgets is a region no rule ever sees — the validator reports + * nothing about it, silently, which is the same failure as a validator that + * only ever looked at the root Board. */ export function* walkSteps(steps: readonly Step[]): Generator { for (const step of steps) { yield step - for (const branch of step.branches ?? []) yield* walkSteps(branch.steps) - if (step.steps) yield* walkSteps(step.steps) - if (step.handler) yield* walkSteps(step.handler) + for (const region of regionsOf(step)) yield* walkSteps(region.steps) } } diff --git a/source/packages/model/src/validity.ts b/source/packages/model/src/validity.ts index a62df2b..c62b64c 100644 --- a/source/packages/model/src/validity.ts +++ b/source/packages/model/src/validity.ts @@ -12,7 +12,16 @@ import { SET_VAR_VERB, TRY_VERB, } from './slots' -import { type BoardId, boards, own, stepKey, varsOn, walkDocument, walkSteps } from './tree' +import { + type BoardId, + boards, + own, + regionsOf, + stepKey, + varsOn, + walkDocument, + walkSteps, +} from './tree' /** * Whether a Workflow Definition is filled in enough to run — the rules that read @@ -548,9 +557,7 @@ export function blockRules(doc: WorkflowDefinition): Diagnostic[] { function* stepLists(steps: readonly Step[]): Generator { yield steps for (const step of steps) { - for (const branch of step.branches ?? []) yield* stepLists(branch.steps) - if (step.steps) yield* stepLists(step.steps) - if (step.handler) yield* stepLists(step.handler) + for (const region of regionsOf(step)) yield* stepLists(region.steps) } } diff --git a/source/packages/react/src/layouts/README.md b/source/packages/react/src/layouts/README.md index 088df03..2bebf1b 100644 --- a/source/packages/react/src/layouts/README.md +++ b/source/packages/react/src/layouts/README.md @@ -109,6 +109,26 @@ which is what would otherwise force `once`/`fixed` back and a special case into A run drawer belongs to `views/Runs`, not to `Build`. A **Workflow Execution** is read-only history; nothing in the designer edits one. +### The shape of each region on the map + +`@hatua/layout` decides where every card goes; this tier draws what it is handed +and computes no geometry of its own. What each child region looks like is +settled in `docs/handoff.md` § Flow map geometry, and the two answers this tier +has to agree with are: a Fork's Branches are **columns** that converge, and every +other region — a loop's body, a `core.try`'s body and handler — is **stacked** +under the card that owns it, in document order, each under a band carrying the +label that names it. + +`` says the same thing in a list, with the chip over each region — the +two surfaces draw differently and must not disagree about which regions a +container has or what they are called. + +They agree by inspection and by nothing else. `@hatua/layout` asks `regionsOf` +in `@hatua/model`; this region reads `branches`, `steps` and `handler` itself, +both to decide what to render and to decide what gets a chevron. A region added +to one reading and not the other is a container the map draws and the list does +not, with nothing failing. + ### The list and the map are both on screen They are not redundant, and neither replaces the other. The list is scannable at diff --git a/source/pnpm-lock.yaml b/source/pnpm-lock.yaml index 65e2c08..fe835c4 100644 --- a/source/pnpm-lock.yaml +++ b/source/pnpm-lock.yaml @@ -91,6 +91,9 @@ importers: packages/layout: dependencies: + '@hatua/model': + specifier: workspace:* + version: link:../model '@hatua/schema': specifier: workspace:* version: link:../schema From 883fed2a3fbdccf26cf7ce70367be341b9cc5fa9 Mon Sep 17 00:00:00 2001 From: Pedro Gomes Date: Mon, 24 Aug 2026 16:02:39 +0100 Subject: [PATCH 2/2] fix(react,layout): a region is drawn because the document carries it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The map placed a handler region on any verb; the list drew one only on core.try. A `handler:` on a core.fork is meaningless and no runner reads it, but walkSteps still yields the Steps inside it, so the generic rules report COMPONENT_UNKNOWN against them by name — a diagnostic naming a Step no surface draws is one nobody can select or delete. Refusing to draw a region does not make it absent from the document; it makes it unreachable. So StepList draws a handler wherever the key is, the rule its branches and its body already followed. The verb decides the word over a region, never whether there is one. StepList.test.tsx holds the two surfaces together on a Step carrying all three region keys at once. Also: one name for the node above the first Step, where handoff.md had been saying both "start node" and "root node"; an assertion for what EMPTY_REGIONS is named for; and centre() states the property it keeps rather than a rounding failure its callers make unreachable. --- docs/handoff.md | 13 +++- source/packages/layout/src/layout.test.ts | 23 ++++++- source/packages/layout/src/layout.ts | 24 ++++--- source/packages/react/src/layouts/README.md | 19 ++++-- .../react/src/layouts/StepList.test.tsx | 68 +++++++++++++++++++ .../packages/react/src/layouts/StepList.tsx | 22 +++--- 6 files changed, 141 insertions(+), 28 deletions(-) diff --git a/docs/handoff.md b/docs/handoff.md index 7ba853b..fb6f2cf 100644 --- a/docs/handoff.md +++ b/docs/handoff.md @@ -30,7 +30,7 @@ file a user hand-edits must not gain keys about what a session had laid out wher │ 304px │ fills │ 372px │ │ │ │ ← expands into the │ │ Components │ the Step tree, laid out │ Data panel │ -│ Workflow │ the start node on top │ │ +│ Workflow │ the root node on top │ │ │ (Flow) │ │ │ └───────────────┴──────────────────────────────┴───────────────────────┘ ``` @@ -104,6 +104,13 @@ a protected body are told apart by the word over them rather than by their geome gives the same answer with the chip over each region; the two surfaces draw differently but they do not disagree about which regions there are or what they are called. +**Both draw every region the document carries, and neither reads the verb to decide.** A `handler:` +on a `core.fork` is meaningless and has no runner, but it is not invisible: `walkSteps` yields the +Steps inside it, so every generic rule reports against them by name — a `COMPONENT_UNKNOWN` naming a +Step no surface draws is a problem a user cannot go and fix, because they cannot reach the Step it +names. Refusing to draw a region does not make it not exist; it makes it undeletable. What the verb +decides is the *word* over the region, not whether there is one. + ### The root node The canvas draws one node above the first Step: the Triggers on the root Board, the Block's contract @@ -193,7 +200,7 @@ per trigger, plus a `TRIGGER` builtin when more than one exists so an Expression one fired. A single `core.start` Step cannot express two Triggers, and ADR-0006 makes the schema the source of truth, so `core.start` is retired. -The canvas still draws a **start node** above the first Step, derived from `doc.triggers[]`. Drawing +The canvas still draws a **root node** above the first Step, derived from `doc.triggers[]`. Drawing it as chrome rather than as a `steps[]` entry is what makes the original handoff's `once: true` and `fixed: true` flags unnecessary: `removeStep` cannot find it, `walkSteps` does not yield it, and `unknownComponents` does not flag it. The guarantees come from the model instead of from two @@ -631,7 +638,7 @@ Recorded here so the two documents cannot disagree quietly. ADR-0011 already lis | `inputs[]` on the document | Retired. A Trigger's declared outputs are the parameter contract | | `dirty` enables **Save changes** | Editing autosaves (ADR-0005). No Save button, no flag | | "Are manifests served by the Host?" — open | Answered: yes, through `ManifestSource` | -| Trigger is a Step: `core.start`, `once`, `fixed` | `doc.triggers[]` is top-level. The canvas draws a derived start node; `once`/`fixed` become unnecessary | +| Trigger is a Step: `core.start`, `once`, `fixed` | `doc.triggers[]` is top-level. The canvas draws a derived root node; `once`/`fixed` become unnecessary | | The **Data tab** — reference tree over a variables editor | Split. The tree moves beside the step editor; variables move to the Workflow tab. One panel held two scopes, and listed every variable twice | | The tab strip is **Flow / Library / Data** | **Components / Workflow**. Flow is optional; Data is not a tab | | "Library" | "Components" — the glossary term, and the region's name | diff --git a/source/packages/layout/src/layout.test.ts b/source/packages/layout/src/layout.test.ts index 694c1c2..b849553 100644 --- a/source/packages/layout/src/layout.test.ts +++ b/source/packages/layout/src/layout.test.ts @@ -1,7 +1,15 @@ import { type Board, boards, stepKey, walkSteps } from '@hatua/model' import type { WorkflowDefinition } from '@hatua/schema' import { describe, expect, it } from 'vitest' -import { ALL_REGIONS, DEEP, EMPTY_BOARD, MIXED_REGIONS, SHAPES, TWO_RETS } from './fixtures' +import { + ALL_REGIONS, + DEEP, + EMPTY_BOARD, + EMPTY_REGIONS, + MIXED_REGIONS, + SHAPES, + TWO_RETS, +} from './fixtures' import { LAYOUT, layout, type Placement, placementOf, type Rect } from './layout' const boardsOf = (doc: WorkflowDefinition): Board[] => [...boards(doc)] @@ -179,6 +187,19 @@ describe('regions', () => { expect(stacked[1]?.y).toBeGreaterThan(stacked[0]?.y ?? 0) }) + it('keeps a container with two empty regions a container, and reserves both bands', () => { + const map = rootOf(EMPTY_REGIONS) + const container = at(map, 'try_nothing') + const below = at(map, 'wide') + + // `steps: []` and `handler: []` are regions with nothing in them, not absent + // regions: the card stays the taller one and each band still takes its room, + // so an empty `handler:` is somewhere a Step can be dropped. + expect(container.height).toBe(LAYOUT.nodeHeightWithMeta) + const bands = 2 * (LAYOUT.verticalGap + LAYOUT.regionLabel) + expect(below.y - (container.y + container.height)).toBe(bands + LAYOUT.verticalGap) + }) + it('gives a container the taller card and a leaf the shorter one', () => { const map = rootOf(ALL_REGIONS) diff --git a/source/packages/layout/src/layout.ts b/source/packages/layout/src/layout.ts index b9e7eeb..4d42714 100644 --- a/source/packages/layout/src/layout.ts +++ b/source/packages/layout/src/layout.ts @@ -243,12 +243,16 @@ function stack(steps: readonly Step[], board: BoardId, collapsed: ReadonlySet` draws them on the same rule. The verb decides + * the *word* over a region, never whether there is one. */ function place(step: Step, board: BoardId, collapsed: ReadonlySet): Box { const height = heightOf(step) @@ -318,9 +322,9 @@ const shift = (placements: readonly Placement[], dx: number, dy: number): Placem /** * Where a `width`-wide box sits inside an `outer`-wide one. * - * Floored rather than rounded, so a centred fragment can never round its way - * past its parent's right edge and into the gap beside a sibling column. Every - * coordinate on the map is therefore a whole number of pixels, which is also - * what makes two layouts of one Board compare byte for byte. + * Integral, so every coordinate on the map is a whole number of pixels and two + * layouts of one Board compare byte for byte. Every caller passes an `outer` + * that is a `Math.max` including this `width`, so the offset is never negative + * and a centred fragment always lies inside its parent. */ const centre = (outer: number, width: number): number => Math.floor((outer - width) / 2) diff --git a/source/packages/react/src/layouts/README.md b/source/packages/react/src/layouts/README.md index 2bebf1b..4277d32 100644 --- a/source/packages/react/src/layouts/README.md +++ b/source/packages/react/src/layouts/README.md @@ -123,11 +123,20 @@ label that names it. two surfaces draw differently and must not disagree about which regions a container has or what they are called. -They agree by inspection and by nothing else. `@hatua/layout` asks `regionsOf` -in `@hatua/model`; this region reads `branches`, `steps` and `handler` itself, -both to decide what to render and to decide what gets a chevron. A region added -to one reading and not the other is a container the map draws and the list does -not, with nothing failing. +**Neither surface reads the verb to decide whether a region exists.** A +`handler:` on a `core.fork` is meaningless and no runner reads it, but it is not +invisible: `walkSteps` yields the Steps inside it, so the generic rules report +against them by name, and a `COMPONENT_UNKNOWN` naming a Step that nothing draws +is a problem the user cannot go and fix. Refusing to draw a region does not make +it absent from the document — it makes it unreachable. What the verb decides is +the *word* over a region, which is why `bodyKeywordFor` says `try` over a try's +body and `loop` over a loop's. + +`@hatua/layout` asks `regionsOf` in `@hatua/model`; this region reads `branches`, +`steps` and `handler` itself. `StepList.test.tsx` § "the list draws every region +the document carries" holds the two together on a Step that carries all three +keys at once, so a region added to one reading and not the other fails rather +than quietly making the map and the list disagree. ### The list and the map are both on screen diff --git a/source/packages/react/src/layouts/StepList.test.tsx b/source/packages/react/src/layouts/StepList.test.tsx index c9aa682..8fa45d0 100644 --- a/source/packages/react/src/layouts/StepList.test.tsx +++ b/source/packages/react/src/layouts/StepList.test.tsx @@ -622,6 +622,74 @@ steps: name: "Say it failed" ` +/* + * A Step carrying all three region keys at once. No verb owns that shape and no + * rule refuses it, so it is what a hand-edit reaches — and it is where the list + * and the map could disagree about what a region is. + */ +const MIXED = `id: wf_morning +name: "Morning inbox triage" +version: 4 +status: draft + +steps: + - id: s1 + use: core.fork + name: "Confused" + branches: + - label: One + steps: + - id: s2 + use: core.end + name: "In a branch" + - label: Two + steps: + - id: s3 + use: core.end + name: "In the other" + steps: + - id: s4 + use: core.end + name: "In the body" + handler: + - id: s5 + use: core.end + name: "In the handler" +` + +describe('the list draws every region the document carries', () => { + /* + * The list and the map must not disagree about which regions a container has. + * Neither reads the verb to decide: a `handler:` on a `core.fork` is + * meaningless and no runner reads it, but `walkSteps` yields the Steps inside + * it, so the generic rules report against them by name. A Step that is + * diagnosed and drawn nowhere is one nobody can select or delete. + */ + it('draws a Step from a branch, a body and a handler on one non-try Step', async () => { + mount(host(MIXED)) + expect(await screen.findByText('In a branch')).toBeDefined() + expect(screen.getByText('In the other')).toBeDefined() + expect(screen.getByText('In the body')).toBeDefined() + expect(screen.getByText('In the handler')).toBeDefined() + }) + + it('draws one row per Step the walk yields, and no more', async () => { + mount(host(MIXED)) + await screen.findByText('Confused') + + // `regionsOf` is what both surfaces enumerate regions with, so the walk over + // it is the list of cards the map places. The list has to match it name for + // name, or one surface is holding a Step the other cannot reach. + expect(rowNames()).toEqual([ + 'Confused', + 'In a branch', + 'In the other', + 'In the body', + 'In the handler', + ]) + }) +}) + describe('a core.try draws two regions', () => { /* * The one Step with two child regions, so the one place the tree has to say diff --git a/source/packages/react/src/layouts/StepList.tsx b/source/packages/react/src/layouts/StepList.tsx index f88b58c..09de910 100644 --- a/source/packages/react/src/layouts/StepList.tsx +++ b/source/packages/react/src/layouts/StepList.tsx @@ -458,16 +458,20 @@ function Sequence({ steps, scope, at, ...handlers }: SequenceProps) { {/* A `core.try`'s second region, drawn whenever the key is present — - the same rule the body above follows. Its own region rather than - a Branch: a Branch's identity is its label, which is free text a - user renames, and a region the user could rename out of existence - is not a region. - - Rendered only for a try. A `handler:` on any other verb means - nothing, and drawing one would put a region on screen that no - rule and no runner reads. + the same rule the body and the branches above follow. Its own + region rather than a Branch: a Branch's identity is its label, + which is free text a user renames, and a region the user could + rename out of existence is not a region. + + Drawn on any verb, not only on a try. A `handler:` elsewhere is + meaningless and no runner reads it — but `walkSteps` still yields + the Steps inside it, so every generic rule reports against them + by name. A diagnostic naming a Step no region draws is one the + user cannot act on, because there is nothing on screen to select + or delete. Hiding the region does not make it absent from the + document; it makes it unreachable. */} - {open && step.use === TRY_VERB && step.handler ? ( + {open && step.handler ? (
on failure