feat: colour output tags by status, not mode — readable error bodies (#10)
The output tag was tinted by submission mode for every line kind, so a [system] line and an [error] line rendered with an identical leftmost tag — distinguishable only by body colour. And flooding the whole error body in red made long messages hard to read. Colour the tag by message status instead (its OutputKind): [system] → green, [error] → red; the echo tag keeps the mode tint (ADR-0037's actual purpose — per-command success rides the ✓/✗ marker). Bodies go neutral; the error body stays bold for weight (rustc-style: severity- coloured label, readable bold message). Yields a status traffic-light matching the ✓/✗ palette. Narrows ADR-0037's mode side-channel to the echo line it was always for. ADR-0037 Amendment 1; closes the tag-colour gap ADR-0040 flagged as OOS.
This commit is contained in:
claude@clouddev1
@@ -203,8 +203,74 @@ Two refinements found when building, recorded so the ADR matches reality:
|
||||
`EffectiveMode`) is Tier-1 testable, and the worker-side threading
|
||||
becomes live + end-to-end testable the moment the echo reads it.
|
||||
|
||||
## Amendment 1 — Output tag is colour-coded by status, not mode (2026-05-31, issue #10)
|
||||
|
||||
The original side-channel (§ above) exists "purely to label the
|
||||
**input-echo line**" — the `[simple]`/`[advanced]` tag whose colour
|
||||
tells the learner which mode their command ran under. In
|
||||
implementation, however, the tag-colour rule in `render_output_line`
|
||||
(`src/ui.rs`) was applied to **every** output kind, keyed on
|
||||
`mode_at_submission` regardless of whether the line was an echo. That
|
||||
over-applied the channel: a `[system]` line and an `[error]` line —
|
||||
neither of which is an input echo — both picked up the same mode tint
|
||||
(blue in simple, orange in advanced). The only thing distinguishing a
|
||||
routine `[system]` message from an `[error]` was the **body** colour
|
||||
(green vs red), while the tag — the leftmost glyph the eye lands on —
|
||||
was identical (issue #10).
|
||||
|
||||
That is backwards for the line a learner most needs to spot fastest.
|
||||
The mode has limited value on an error line; "this is an error" has
|
||||
high value. And flooding the whole error **body** in red makes a long
|
||||
message *harder* to read, not easier.
|
||||
|
||||
**Change — the status-coloured-tag model.** The output tag is
|
||||
colour-coded by the message's **status** (its `OutputKind`), and the
|
||||
**body** is neutral so the message text stays readable:
|
||||
|
||||
| Kind | Tag colour | Body |
|
||||
| --- | --- | --- |
|
||||
| `Echo` | **mode tint** (`mode_simple`/`mode_advanced`) — *the sole exception* | `theme.fg` / lexed (unchanged); per-command success rides the trailing ✓/✗ (ADR-0040) |
|
||||
| `System` | `theme.system` (green) | `theme.fg` (was green) |
|
||||
| `TeachingEcho` | `theme.system` (green — it is a `[system]`-tagged line) | dim prefix + lexed SQL (unchanged) |
|
||||
| `Error` | `theme.error` (red) | `theme.fg` **+ BOLD** (was red) |
|
||||
|
||||
This **narrows** the side-channel to its stated purpose rather than
|
||||
contradicting it: the mode tint now lives **only** on the echo tag,
|
||||
where ADR-0037 always said it belonged. Everything else reads as a
|
||||
status traffic-light — **green tag = ok/info, red tag = error** —
|
||||
which is the same palette as the ✓/✗ echo markers (ADR-0040), so the
|
||||
whole output surface speaks one colour vocabulary.
|
||||
|
||||
**Why bold-neutral for the error body** (not plain, not red). This is
|
||||
the established diagnostic-rendering convention — `rustc`, `clang`,
|
||||
`tsc`, and most linters colour the **severity label** and render the
|
||||
**message** in the default foreground (bold), not a wall of severity
|
||||
colour. The red moves to the tag (the scan target); the body keeps
|
||||
weight via BOLD without the readability cost of coloured prose.
|
||||
|
||||
**Scope / non-changes.**
|
||||
|
||||
- `OutputLine.mode_at_submission` is **unchanged** — still carries the
|
||||
mode for the echo tag. Only *which kinds consult it for colour* changed.
|
||||
- The ✓/✗ completion markers (ADR-0040) are untouched — they already
|
||||
use `theme.system`/`theme.error` directly, and now visually rhyme
|
||||
with the new tag colours.
|
||||
- This supersedes the three options sketched in issue #10 (red tag /
|
||||
amber attention tag / glyph) with a cleaner fourth model that also
|
||||
fixes body readability and the `[system]` tag in one rule. ADR-0040
|
||||
had flagged the `[error]`/`[system]` tag colours as orthogonal and
|
||||
out of its scope (issue #10) — this amendment closes that gap.
|
||||
|
||||
**Coverage** (`src/ui.rs` tests): `system_line_renders_green_tag_and_neutral_body`,
|
||||
`error_line_renders_red_tag_and_bold_neutral_body`,
|
||||
`echo_tag_keeps_the_mode_tint_not_a_status_colour` (locks the sole
|
||||
exception across both modes), `teaching_echo_tag_is_green_like_other_system_lines`.
|
||||
|
||||
## See also
|
||||
|
||||
- ADR-0040 — the ✓/✗ completion markers whose green/red palette the
|
||||
status tag now matches; it deferred these tag colours as orthogonal
|
||||
(issue #10), closed by Amendment 1.
|
||||
- ADR-0033 Amendment 3 — deferred this side-channel; defines the
|
||||
intrinsic command-identity model this ADR must not disturb.
|
||||
- ADR-0030 §10 — the DSL → SQL teaching bridge (the motivating consumer).
|
||||
|
||||
+1
-1
File diff suppressed because one or more lines are too long
@@ -760,9 +760,20 @@ const fn output_span_style(class: OutputStyleClass, theme: &Theme) -> Style {
|
||||
}
|
||||
|
||||
fn render_output_line<'a>(line: &'a OutputLine, theme: &Theme) -> Line<'a> {
|
||||
let tag_style = match line.mode_at_submission {
|
||||
Mode::Simple => Style::default().fg(theme.mode_simple),
|
||||
Mode::Advanced => Style::default().fg(theme.mode_advanced),
|
||||
// ADR-0037 Amendment (issue #10): the output tag is colour-coded by
|
||||
// the message's STATUS (its kind), not the submission mode — so the
|
||||
// leftmost glyph the eye lands on says "ok" vs "error" at a glance,
|
||||
// and a routine `[system]` line never looks identical to an `[error]`.
|
||||
// The echo line is the sole exception: its tag's whole job is to label
|
||||
// the submission mode (ADR-0037's stated purpose), so it keeps the
|
||||
// mode tint (per-command success rides the trailing ✓/✗ — ADR-0040).
|
||||
let tag_style = match line.kind {
|
||||
OutputKind::Echo => match line.mode_at_submission {
|
||||
Mode::Simple => Style::default().fg(theme.mode_simple),
|
||||
Mode::Advanced => Style::default().fg(theme.mode_advanced),
|
||||
},
|
||||
OutputKind::System | OutputKind::TeachingEcho => Style::default().fg(theme.system),
|
||||
OutputKind::Error => Style::default().fg(theme.error),
|
||||
};
|
||||
let tag = match line.kind {
|
||||
OutputKind::Echo => format!("[{}] ", line.mode_at_submission.label().to_lowercase()),
|
||||
@@ -862,14 +873,19 @@ fn render_output_line<'a>(line: &'a OutputLine, theme: &Theme) -> Line<'a> {
|
||||
return Line::from(spans);
|
||||
}
|
||||
|
||||
// ADR-0037 Amendment (issue #10): bodies are neutral — the status
|
||||
// colour lives on the tag, not flooded across the message text. An
|
||||
// error body keeps weight via BOLD (rustc-style: severity-coloured
|
||||
// label, readable bold message) rather than a hard-to-read wall of
|
||||
// red; a system body is plain `theme.fg`.
|
||||
let body_style = match line.kind {
|
||||
OutputKind::Echo => Style::default().fg(theme.fg),
|
||||
// TeachingEcho without a prefix falls back to `system` styling
|
||||
// — the kind itself only signals "render with the dim+lex
|
||||
// custom path above"; reaching here means the builder produced
|
||||
// a malformed line, so degrade gracefully rather than crash.
|
||||
OutputKind::System | OutputKind::TeachingEcho => Style::default().fg(theme.system),
|
||||
OutputKind::Error => Style::default().fg(theme.error),
|
||||
// TeachingEcho without a prefix reaches here only as a builder
|
||||
// bug; it shares the neutral system body so it degrades
|
||||
// gracefully rather than crashing.
|
||||
OutputKind::Echo | OutputKind::System | OutputKind::TeachingEcho => {
|
||||
Style::default().fg(theme.fg)
|
||||
}
|
||||
OutputKind::Error => Style::default().fg(theme.fg).add_modifier(Modifier::BOLD),
|
||||
};
|
||||
Line::from(vec![
|
||||
Span::styled(tag, tag_style),
|
||||
@@ -1317,10 +1333,11 @@ mod tests {
|
||||
#[test]
|
||||
fn teaching_echo_line_renders_dim_prefix_and_lexed_sql() {
|
||||
// ADR-0038 §4 styled-runs polish: a TeachingEcho line is laid
|
||||
// out as [tag][dim prefix][lexed SQL spans]. The tag stays the
|
||||
// mode tint; the `Executing SQL: ` prefix is `theme.muted`;
|
||||
// the SQL portion is re-lexed in advanced mode so it picks up
|
||||
// keyword / identifier / literal colours.
|
||||
// out as [tag][dim prefix][lexed SQL spans]. The tag is the green
|
||||
// status colour (a `[system]` line — ADR-0037 Amendment, issue
|
||||
// #10); the `Executing SQL: ` prefix is `theme.muted`; the SQL
|
||||
// portion is re-lexed in advanced mode so it picks up keyword /
|
||||
// identifier / literal colours.
|
||||
let theme = Theme::dark();
|
||||
let line = OutputLine {
|
||||
text: format!(
|
||||
@@ -1429,20 +1446,171 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_line_without_styled_runs_keeps_whole_line_kind_styling() {
|
||||
fn system_line_renders_green_tag_and_neutral_body() {
|
||||
// ADR-0037 Amendment (issue #10): the status-coloured-tag model.
|
||||
// A `[system]` line's TAG carries the green status colour; its
|
||||
// BODY is neutral `theme.fg`, not flooded green. The mode tint
|
||||
// no longer leaks onto system lines (it belongs to the echo line
|
||||
// alone — ADR-0037's stated purpose). `mode_at_submission` is
|
||||
// Advanced here precisely to prove the tag is NOT the mode tint.
|
||||
let theme = Theme::dark();
|
||||
let line = OutputLine {
|
||||
text: "plain system line".to_string(),
|
||||
kind: OutputKind::System,
|
||||
mode_at_submission: Mode::Simple,
|
||||
mode_at_submission: Mode::Advanced,
|
||||
styled_runs: None,
|
||||
status: None,
|
||||
};
|
||||
let rendered = render_output_line(&line, &theme);
|
||||
// tag span + single whole-line body span.
|
||||
assert_eq!(rendered.spans.len(), 2);
|
||||
assert_eq!(rendered.spans[0].content.as_ref(), "[system] ");
|
||||
assert_eq!(
|
||||
rendered.spans[0].style.fg,
|
||||
Some(theme.system),
|
||||
"the [system] tag is green (status), not the mode tint",
|
||||
);
|
||||
assert_eq!(rendered.spans[1].content.as_ref(), "plain system line");
|
||||
assert_eq!(rendered.spans[1].style.fg, Some(theme.system));
|
||||
assert_eq!(
|
||||
rendered.spans[1].style.fg,
|
||||
Some(theme.fg),
|
||||
"the system body is neutral, not flooded green",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn error_line_renders_red_tag_and_bold_neutral_body() {
|
||||
// ADR-0037 Amendment (issue #10): the `[error]` TAG carries the
|
||||
// red status colour (the leftmost glyph the eye lands on), while
|
||||
// the BODY renders in neutral `theme.fg` + BOLD (rustc-style:
|
||||
// severity-coloured label, readable bold message). A wall of red
|
||||
// prose is hard to read; the red lives on the tag instead. The
|
||||
// mode tint does not leak onto error lines.
|
||||
let theme = Theme::dark();
|
||||
let line = OutputLine {
|
||||
text: "no such column: agx".to_string(),
|
||||
kind: OutputKind::Error,
|
||||
mode_at_submission: Mode::Advanced,
|
||||
styled_runs: None,
|
||||
status: None,
|
||||
};
|
||||
let rendered = render_output_line(&line, &theme);
|
||||
assert_eq!(rendered.spans.len(), 2);
|
||||
assert_eq!(rendered.spans[0].content.as_ref(), "[error] ");
|
||||
assert_eq!(
|
||||
rendered.spans[0].style.fg,
|
||||
Some(theme.error),
|
||||
"the [error] tag is red (status), not the mode tint",
|
||||
);
|
||||
assert_eq!(rendered.spans[1].content.as_ref(), "no such column: agx");
|
||||
assert_eq!(
|
||||
rendered.spans[1].style.fg,
|
||||
Some(theme.fg),
|
||||
"the error body is neutral fg, not flooded red",
|
||||
);
|
||||
assert!(
|
||||
rendered.spans[1].style.add_modifier.contains(Modifier::BOLD),
|
||||
"the error body is bold for weight without the red-wall readability cost",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn echo_tag_keeps_the_mode_tint_not_a_status_colour() {
|
||||
// The echo line is the sole exception to the status-tag model:
|
||||
// its tag's whole job is to label the submission mode (ADR-0037),
|
||||
// so it keeps the mode tint. Per-command success rides the trailing
|
||||
// ✓/✗ marker (ADR-0040), not the tag. Locked for both modes so a
|
||||
// future refactor of `tag_style` cannot regress the echo.
|
||||
let theme = Theme::dark();
|
||||
for (mode, want) in [
|
||||
(Mode::Simple, theme.mode_simple),
|
||||
(Mode::Advanced, theme.mode_advanced),
|
||||
] {
|
||||
let line = OutputLine {
|
||||
text: format!("{}create table T", crate::dsl::ECHO_PREFIX),
|
||||
kind: OutputKind::Echo,
|
||||
mode_at_submission: mode,
|
||||
styled_runs: None,
|
||||
status: None,
|
||||
};
|
||||
let rendered = render_output_line(&line, &theme);
|
||||
assert_eq!(
|
||||
rendered.spans[0].style.fg,
|
||||
Some(want),
|
||||
"echo tag must stay the {mode:?} mode tint",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn teaching_echo_tag_is_green_like_other_system_lines() {
|
||||
// A TeachingEcho is a `[system]`-tagged line, so under the
|
||||
// status-tag model its tag is green, not the mode tint. The dim
|
||||
// prefix + lexed-SQL body are unchanged (covered separately).
|
||||
let theme = Theme::dark();
|
||||
let line = OutputLine {
|
||||
text: format!(
|
||||
"{}{}",
|
||||
crate::echo::TEACHING_ECHO_LABEL,
|
||||
"CREATE TABLE T (id serial PRIMARY KEY)"
|
||||
),
|
||||
kind: OutputKind::TeachingEcho,
|
||||
mode_at_submission: Mode::Advanced,
|
||||
styled_runs: None,
|
||||
status: None,
|
||||
};
|
||||
let rendered = render_output_line(&line, &theme);
|
||||
assert_eq!(rendered.spans[0].content.as_ref(), "[system] ");
|
||||
assert_eq!(
|
||||
rendered.spans[0].style.fg,
|
||||
Some(theme.system),
|
||||
"the teaching-echo tag is green (a [system] line), not the mode tint",
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn error_and_system_tags_are_distinguishable_in_both_themes() {
|
||||
// Issue #10 regression guard, stated directly: the `[error]` and
|
||||
// `[system]` tags must NOT render in the same colour, and neither
|
||||
// may collapse to the mode tint. Asserted on both palettes — the
|
||||
// render logic is theme-agnostic, but locking both proves the
|
||||
// colours themselves stay distinct end to end.
|
||||
for theme in [Theme::dark(), Theme::light()] {
|
||||
let tag_fg = |kind| {
|
||||
render_output_line(
|
||||
&OutputLine {
|
||||
text: "x".to_string(),
|
||||
kind,
|
||||
mode_at_submission: Mode::Advanced,
|
||||
styled_runs: None,
|
||||
status: None,
|
||||
},
|
||||
&theme,
|
||||
)
|
||||
.spans[0]
|
||||
.style
|
||||
.fg
|
||||
};
|
||||
let error_tag = tag_fg(OutputKind::Error);
|
||||
let system_tag = tag_fg(OutputKind::System);
|
||||
assert_ne!(
|
||||
error_tag, system_tag,
|
||||
"[error] and [system] tags must differ ({:?})",
|
||||
theme.background,
|
||||
);
|
||||
assert_ne!(
|
||||
error_tag,
|
||||
Some(theme.mode_advanced),
|
||||
"the error tag must not be the mode tint ({:?})",
|
||||
theme.background,
|
||||
);
|
||||
assert_ne!(
|
||||
system_tag,
|
||||
Some(theme.mode_advanced),
|
||||
"the system tag must not be the mode tint ({:?})",
|
||||
theme.background,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
fn render_to_string(app: &mut App, theme: &Theme, width: u16, height: u16) -> String {
|
||||
|
||||
Reference in New Issue
Block a user