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..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) │ │ │ └───────────────┴──────────────────────────────┴───────────────────────┘ ``` @@ -58,6 +58,82 @@ 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. + +**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 +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 @@ -124,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 @@ -562,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/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..b849553 --- /dev/null +++ b/source/packages/layout/src/layout.test.ts @@ -0,0 +1,270 @@ +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, + 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)] + +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('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) + + 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..4d42714 --- /dev/null +++ b/source/packages/layout/src/layout.ts @@ -0,0 +1,330 @@ +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, and the verb is never consulted + * about whether a region exists. No verb owns both `branches:` and a `steps:` + * body, and a `handler:` outside a `core.try` is meaningless — but nothing + * refuses a document that writes them, because the schema's step keys are all + * optional and no rule reads them together. Such a region is still walked: + * `walkSteps` yields the Steps inside it, so the generic rules report against + * them by name. A card no surface draws is a diagnostic the user cannot act on, + * so drawing every region in hand is what keeps a hand-edited region reachable + * enough to delete. `` 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) + 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. + * + * 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/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..4277d32 100644 --- a/source/packages/react/src/layouts/README.md +++ b/source/packages/react/src/layouts/README.md @@ -109,6 +109,35 @@ 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. + +**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 They are not redundant, and neither replaces the other. The list is scannable at 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