The engine renders: an aligned card, aligned tables, drawn trees, a palette, and the width to fit them in - #153
Conversation
…s, width The ruled spec plus its dispatch plan. Four amendments came out of reading what prisma/prisma and prisma/composer already ship, rather than deciding by taste: six indexed colours instead of eight (the ORM's lane rotation is six and deliberately excludes red, and basic ANSI has no two further hues that stay distinguishable); status split out of tone, so a tree node can be a failure painted in its branch lane's colour; the fields card restored, because it is the most-used block and lost the most; and an explicit --color beating NO_COLOR. Every tone's rendering is taken from a shipping implementation, and the colour and width libraries are the ones both repos already depend on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
The engine shipped a common renderer that emitted exactly two escape sequences, bold and dim, and could not reach the terminal's width at all: OutputStream was write-only, structurally, even from the bin. Ui now carries tone(), which turns one of the contract's tones into bytes, and width, which is stderr's columns — unbounded when stderr is not a terminal, so the arithmetic a renderer does needs no special case. Every rendering in the palette is taken from a shipping CLI rather than chosen fresh: the platform shell's semantic colours and the ORM CLI's formatters, whose six-colour lane rotation becomes color-1..color-6 and keeps its exclusion of red. Colour off makes every verb the identity function, so a renderer has one code path. Text, Span, Tone and Status are exported for the block grammar to take up next; no block renders differently yet. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
Blocks, diagnostics and commentary all render to stderr, but the colour decision was keyed on stdout. So `cmd > file` stripped colour from output a human was still watching on the terminal, and `cmd 2> file` left escape sequences in a file nobody reads. Colour now follows stderr, and the precedence is stated explicitly: --color or --no-color decides whatever else is set, otherwise NO_COLOR disables it, otherwise stderr's being a terminal decides. An explicit flag on the invocation beats anything in the environment. sniffFormat and resolveAutoFormat still read stdout, which is right: format asks whether stdout is a pipe, which is a different question. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
The bin copied process.stderr.columns once, when it assembled the runtime, so ui.width was frozen at whatever the terminal was when the process started. Node updates that property on SIGWINCH, so reading it through a getter costs nothing and means a session command drawing minutes into a run sees the width the terminal has now. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
summary.tone was doing two jobs: it picked the glyph AND was the only colour word a block had. Fusing them means a block cannot say "this failed" and "paint it in this hue" at once — which the tree needs, so a failing node can be drawn in its branch lane's colour. The field becomes summary.status, typed Status, and selects the glyph alone. An optional tone overrides the colour the status implies and never touches the glyph. 57 call sites renamed with it, compiler-driven. The error glyph moves from ✖ to ✘, the character the contract names for Status and the one the style guide's tree example and the legacy shell both use. No summary in the CLI is an error today, so no output changes; a diagnostic's severity symbol is a different map and keeps its ✖. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
renderBlock was five one-liners. A table was columns.join(" "), so
nothing lined up; a card was `label: value`, which is the alignment and
the accent colour the platform port recorded as an accepted loss; a tree
was two-space indents with no connectors at all.
Every block text field now takes Text — a string or spans — and the
engine turns it into bytes. A table sizes each column to its widest
cell, measured on the text rather than the escape sequences, so a
coloured cell aligns exactly as the same words uncoloured; the last
column is never padded, so no line trails whitespace. A card pads
`label:` to a common width with the label in the accent colour, and
draws the dim rail when a command asks for one. A tree draws ├─, └─ and
the │ continuation in dim, with the status glyph before the label.
A new drawing block writes lines of spans verbatim, for output whose
two-dimensional structure the engine cannot derive — a migration graph's
lane gutter, where one hue has to reach the gutter cell, the node glyph
and the label alike.
The card and rail are asserted against the commander shell's own
renderFieldRows and renderVerboseBlock rather than a copied byte string,
so "the card came back" is checked, not claimed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: willbot <w.a.madden+machine@gmail.com>
Signed-off-by: Will Madden <madden@prisma.io>
Three maps in the engine pick the character that marks a failure: a block's status, a step's outcome, and a diagnostic's severity. D2 moved the first to ✘, the character the style guide, the contract and the legacy shell all name, and left the other two on ✖. A run could report a failed step with one mark, the summary with another, and the diagnostic explaining it with a third. All three now draw ✘, and the internal-error note in reporting.ts with them. ✔, ⚠ and ℹ were already one character each across the maps; two tests hold all of it in place by running one command that reports a failure through every surface at once, and one that does the same for every non-failure outcome. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
The command the colour-resolution tests run spliced ui.tone(...) into a summary's text string. That is the pattern Text and Span exist to replace: a handler that builds bytes is a handler whose width the engine cannot measure. The probe now hands over a span and lets the engine paint it, so the test demonstrates the path commands are meant to take rather than the one being retired. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
The contract requires the width be read per render rather than cached, and nothing covered it — the field went from a frozen snapshot to a getter inside this branch and the suite was green either way. The test harness cannot express this: it reads the caller's columns once while assembling the runtime, so the value can never change under a running command. The check is therefore a direct one against makeUi with a stub stream whose columns the test mutates, which is also what lets it cover the Ui's other verbs — that emphasize and dim still emit the exact bytes they emitted before colorette replaced the hand-rolled sequences. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
Two getters carry the terminal's width to a command: the Ui's, and the bin's forwarding of process.stderr.columns. Only the first was covered, and the second is the one that regressed inside this branch — it shipped as a copy taken while assembling the runtime, and was replaced by a getter, with the suite green both times. The engine-side stub cannot see past itself, so a copy reintroduced in the bin would freeze ui.width at process start with every existing test still passing. This asserts against the bin adapter directly: the fake process moves its stderr width after assembleRuntime has returned, and the runtime reports the new value. Reverting the getter to a copy fails it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
reporting.ts writes its mark as a literal instead of reading the diagnostic-severity map, so the map-level pins do not reach it. The only test that renders that line matched the message text and stopped short of the mark, which left the one error glyph in the engine that nothing held in place. The assertion now covers the whole line — mark, code and message. Changing the mark in reporting.ts fails it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
Three parity entries recorded the flattened rendering as accepted. The card's alignment and its accent colour are back and byte-equal to the legacy renderer, so those entries now say what returned and what did not — the rail, which is a per-command opt-in, and the header framing, which has no engine counterpart. The interface draft's presentation section matches what shipped, since the exports barrel cites it as normative. Two files argued for deferring glyph mode on the grounds that the engine already emits a character set it no longer draws; the failure mark is now the style guide's throughout, so the premise is corrected and the deferral stands on the true one. Also recorded: a test that reports green against a stale engine build when run as a lone pnpm filter, and a second spawn test that fails under load. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
|
Warning Review limit reached
Next review available in: 51 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (86)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
commit: |
|
@coderabbitai review this |
|
|
Two modify/delete conflicts, both on files this branch removed in D4: packages/cli/src/v8/telemetry/consent.ts and status.ts. The deletions stand — the engine ships those commands now — but #153 edited them as part of the rendering rework, so its change had to land on the engine's copies or the engine's telemetry commands would render unlike every other command in the CLI. What #153 changed there was summary blocks: tone: became status:, since Status (what happened, which picks the glyph) is now separate from Tone (what colour to paint). Carried onto src/telemetry/commands.ts for both the status summary and the enable/disable confirmation, and onto the four test command definitions on this branch that build summary blocks. Nothing else needed carrying, and I checked rather than assumed: the fields card takes Text where it took string, so its plain rows still typecheck; human: () => [...] is still assignable now that the presenter receives a Ui; and no v8 command sets fields.rail or emits spans — the PR says so and the tree agrees — so telemetry adopting either would have made it the odd one out rather than consistent. Verified by running them, not by compiling them. telemetry status draws a blue info glyph and a cyan-keyed card whose values align in one column — the same shapes auth whoami draws — and telemetry disable draws the bright green tick every ok summary draws. stdout still carries only the machine lines. Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io> #144 touches project commands and merged without interaction.
The new findings tests byte-assert stderr, and #153 changed the error marker from U+2716 to U+2718 while this branch was open. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: willbot <w.a.madden+machine@gmail.com> Signed-off-by: Will Madden <madden@prisma.io>
What this looks like
prisma-cli auth workspace list, before and after:prisma-cli auth whoami, whose card is the shape 36 commands use:And the tree the style guide has specified since it was written, which until now no block could express:
Headers and keys cyan, connectors dim, glyph and label coloured by the node's status. None of that was reachable before:
makeUiemitted two escape sequences, bold and dim, and nothing else in the engine emitted colour at all.What was broken
The engine shipped a common renderer that did almost no rendering.
renderBlockwascolumns.join(" ")— no sizing, no padding. The key/value card printedlabel: value, having lost the aligned, accent-coloured column the commander shell drew. The tree emitted two-space indents and had zero users. There was no way to draw a migration graph's lane gutter at all, andOutputStreamwas{ write(text) }, so the engine could not learn the terminal's width even from the bin.Both consumers had already lost rendering to it. The platform port flattened
auth workspace list,project listandagent status, recorded as accepted divergences. The ORM's renderers ship colourless, and two of them callcreateColors({ useColor: true })to force colour pastNO_COLORbecause there is no other way to get any.The decision
The engine renders; a command describes. So the fix is to the renderer, not to give commands a canvas.
tablealigns andfieldsdraws the card again — keys padded to one column, toned, with the dim│rail available asfields.railfor the commands that want it.treedraws connectors and status glyphs, matching the style guide verbatim.drawingis added as the escape hatch for layout the engine cannot derive: the migration DAG's lane gutter, where lane assignment comes from a BFS over the graph and one hue must reach the gutter cell, the node glyph and the label alike.Text = string | Span[]wherever a block takes display text. Spans carry tone, never escape sequences, so the engine measures on text and colour cannot break alignment. The pad-versus-colour bug that exists in shipped code becomes unrepresentable.Statusis separate fromTone. One says what happened and picks the glyph; the other says what colour to paint. They were fused insummary.tone, which meant a tree node could not be a failure painted in its branch lane's colour.ui.widthis stderr's width, unbounded when stderr is not a terminal. Only the command knows what to sacrifice, so it is told the room it has; if it overruns anyway the engine prints it unmodified.Two corrections ride along. Colour resolution keyed off stdout while blocks render to stderr, so
cmd > filedisabled colour for output a human was watching — it now follows the stream that is actually printed to. And every one of the CLI's four failure surfaces drew✖, while the style guide anddocs/product/output-conventions.mdhave both specified✘all along; all four now draw✘.Nothing here was invented
The palette is not a set of tastes. Every tone is the colour a shipping Prisma CLI already uses for that meaning, checked at source during review:
heading, and the eight other platform semanticspackages/cli/src/shell/ui.tsidentifier,ref,highlightcolor-1…color-6LANE_COLORIZERS, in orderThe indexed colours are six rather than the eight first sketched, because six is what the ORM's lane rotation is, and it excludes red deliberately so no lane can read as an error. Basic ANSI has no two further hues that stay distinguishable from those six; reaching eight would mean a 256-colour palette neither CLI uses.
coloretteandstring-widthare the colour and measurement libraries both repos already depend on.How to believe the card came back
packages/cli/tests/v8-golden-rendering.test.tsrenders afieldsblock through the engine and asserts it equals what the commander shell's ownrenderFieldRowsproduces. It is a comparison against the original code in another package, building its own colour instance and sharing nothing with the engine — not a byte string someone typed into a test.Alignment is proven not to depend on colour: a table whose cells carry spans is byte-identical to the same table in plain text once escapes are stripped, which a colour-first-then-pad implementation would fail. A CJK cell is measured in terminal columns rather than code units.
What this does not do
No command binds
uiyet — all 54 platform presenters are still writtenhuman: () => [...], so the engine colours what it draws itself and nothing more. Spans,drawingandui.widthget their first callers when each family converts its renderers, which is the next step for both. Next actions and diagnostics still render as plain strings, so half of stderr is coloured and half is not; that is the natural follow-on slice and it is recorded as such.The rail is available but no command sets it. Alignment and colour return to all 36 cards at once; which of them want the rail is a per-command judgement the engine cannot make.
Alternatives considered
A separate
graphicpresenter alongsidehuman— rejected: a drawing is human output, so it belongs in the human channel's grammar, and a parallel presenter would need its own channel and colour handling.Routing rich renderings through
Presentations.stdout— rejected: that is the machine channel, and a drawing's consumer is a human.Letting commands draw tables and trees themselves — rejected: if every command reimplements every form of rendering on an open canvas, the common renderer has no purpose.
Keeping
NO_COLORabsolute over an explicit--color— rejected: a flag on the invocation beats a standing environment default.NO_COLORstill beats the terminal.Supersedes #148, which carried this spec alone. The spec is in this PR at
.drive/projects/prisma-cli-v8/specs/engine-colour.md, with the four amendments the implementation survey produced recorded in its §8.🤖 Generated with Claude Code