ux(coding-agent): combined ttsr rule notifications into one block

2026-06-10 23:15:40 +02:00
parent 6836dc1193
commit ad1c17fffd
3 changed files with 176 additions and 34 deletions
@@ -2,16 +2,24 @@ import { Box, Container, Spacer, Text } from "@oh-my-pi/pi-tui";
 import type { Rule } from "../../capability/rule";
 import { theme } from "../../modes/theme/theme";

+/** Collapsed view shows at most this many rules before eliding the rest. */
+const MAX_COLLAPSED_RULES = 4;
+
 /**
 * Component that renders a TTSR (Time Traveling Stream Rules) notification.
 * Shows when a rule violation is detected and the stream is being rewound.
+ * One block can carry several rules: a single event may match multiple rules,
+ * and consecutive notifications merge into the previous block via
+ * {@link addRules} while it is still the live transcript tail.
 */
 export class TtsrNotificationComponent extends Container {
 	#box: Box;
 	#expanded = false;
+	#rules: Rule[];

-	constructor(private readonly rules: Rule[]) {
+	constructor(rules: Rule[]) {
 		super();
+		this.#rules = [...rules];

 		this.addChild(new Spacer(1));

@@ -22,6 +30,17 @@ export class TtsrNotificationComponent extends Container {
 		this.#rebuild();
 	}

+	/** Merge additional rules into this block (deduped by rule name). */
+	addRules(rules: Rule[]): void {
+		let changed = false;
+		for (const rule of rules) {
+			if (this.#rules.some(existing => existing.name === rule.name)) continue;
+			this.#rules.push(rule);
+			changed = true;
+		}
+		if (changed) this.#rebuild();
+	}
+
 	setExpanded(expanded: boolean): void {
 		if (this.#expanded !== expanded) {
 			this.#expanded = expanded;
@@ -35,46 +54,69 @@ export class TtsrNotificationComponent extends Container {

 	#rebuild(): void {
 		this.#box.clear();
+		// fg colors conflict with inverse, so styling inside the block is limited
+		// to bold (names) and italic (descriptions).
+		if (this.#rules.length === 1) {
+			this.#rebuildSingle(this.#rules[0]!);
+		} else {
+			this.#rebuildMulti();
+		}
+	}

-		// Build header: warning symbol + rule name + rewind icon
-		const ruleNames = this.rules.map(r => theme.bold(r.name)).join(", ");
-		const label = this.rules.length === 1 ? "rule" : "rules";
-		const header = `${theme.icon.warning} Injecting ${label}: ${ruleNames}`;
+	#rebuildSingle(rule: Rule): void {
+		const header = `${theme.icon.warning} Injecting rule: ${theme.bold(rule.name)}  ${theme.icon.rewind}`;
+		this.#box.addChild(new Text(header, 0, 0));

-		// Create header with rewind icon on the right
-		const rewindIcon = theme.icon.rewind;
+		const desc = (rule.description || rule.content)?.trim();
+		if (!desc) return;

-		this.#box.addChild(new Text(`${header}  ${rewindIcon}`, 0, 0));
-
-		// Show description(s) - italic and truncated
-		for (const rule of this.rules) {
-			const desc = rule.description || rule.content;
-			if (desc) {
-				this.#box.addChild(new Spacer(1));
-
-				let displayText = desc.trim();
-				if (!this.#expanded) {
-					// Truncate to first 2 lines
-					const lines = displayText.split("\n");
-					if (lines.length > 2) {
-						displayText = `${lines.slice(0, 2).join("\n")}…`;
-					}
-				}
-
-				// Use italic for subtle distinction (fg colors conflict with inverse)
-				this.#box.addChild(new Text(theme.italic(displayText), 0, 0));
+		let displayText = desc;
+		let truncated = false;
+		if (!this.#expanded) {
+			const lines = desc.split("\n");
+			if (lines.length > 2) {
+				displayText = `${lines.slice(0, 2).join("\n")}…`;
+				truncated = true;
 			}
 		}

-		// Show expand hint if collapsed and there's more content
-		if (!this.#expanded) {
-			const hasMoreContent = this.rules.some(r => {
-				const desc = r.description || r.content;
-				return desc && desc.split("\n").length > 2;
-			});
-			if (hasMoreContent) {
-				this.#box.addChild(new Text(theme.italic(" (ctrl+o to expand)"), 0, 0));
+		this.#box.addChild(new Spacer(1));
+		this.#box.addChild(new Text(theme.italic(displayText), 0, 0));
+		if (truncated) {
+			this.#box.addChild(new Text(theme.italic(" (ctrl+o to expand)"), 0, 0));
+		}
+	}
+
+	#rebuildMulti(): void {
+		const header = `${theme.icon.warning} Injecting ${this.#rules.length} rules:  ${theme.icon.rewind}`;
+		this.#box.addChild(new Text(header, 0, 0));
+		this.#box.addChild(new Spacer(1));
+
+		const visible = this.#expanded ? this.#rules : this.#rules.slice(0, MAX_COLLAPSED_RULES);
+		let elidedDetail = false;
+		for (const rule of visible) {
+			const desc = (rule.description || rule.content)?.trim();
+			let line = theme.bold(rule.name);
+			if (desc) {
+				let displayText = desc;
+				if (!this.#expanded) {
+					// One line per rule when collapsed; full description when expanded.
+					const newline = desc.indexOf("\n");
+					if (newline !== -1) {
+						displayText = `${desc.slice(0, newline).trimEnd()}…`;
+						elidedDetail = true;
+					}
+				}
+				line += `: ${theme.italic(displayText)}`;
 			}
+			this.#box.addChild(new Text(line, 0, 0));
+		}
+
+		const hidden = this.#rules.length - visible.length;
+		if (hidden > 0) {
+			this.#box.addChild(new Text(theme.italic(`… +${hidden} more (ctrl+o to expand)`), 0, 0));
+		} else if (elidedDetail) {
+			this.#box.addChild(new Text(theme.italic(" (ctrl+o to expand)"), 0, 0));
 		}
 	}
 }
@@ -82,6 +82,9 @@ export class EventController {
 	// one persistent poll instead of a stack of "waiting on N jobs" frames —
 	// and sealed in place the moment anything else lands below it.
 	#displaceablePollComponent: ToolExecutionComponent | undefined = undefined;
+	// Most recent TTSR notification block. A new ttsr_triggered event merges its
+	// rules into this block while it is still the (live-region) transcript tail.
+	#lastTtsrNotification: TtsrNotificationComponent | undefined = undefined;
 	#streamingReveal: StreamingRevealController;
 	#handlers: AgentSessionEventHandlers;

@@ -953,9 +956,26 @@ export class EventController {
 	}

 	async #handleTtsrTriggered(event: Extract<AgentSessionEvent, { type: "ttsr_triggered" }>): Promise<void> {
+		// Consecutive notifications (e.g. per-tool matches from one assistant
+		// message) merge into the previous block instead of stacking. Mutating an
+		// existing block is only safe while it sits inside the live region — a
+		// still-mutating block above it means none of its rows have been committed
+		// to native scrollback yet (commits are prefix-only and stop at the first
+		// live block), so the grown block still repaints.
+		const previous = this.#lastTtsrNotification;
+		if (
+			previous &&
+			this.ctx.chatContainer.children.at(-1) === previous &&
+			this.ctx.chatContainer.isWithinLiveRegion(previous)
+		) {
+			previous.addRules(event.rules);
+			this.ctx.ui.requestRender();
+			return;
+		}
 		const component = new TtsrNotificationComponent(event.rules);
 		component.setExpanded(this.ctx.toolOutputExpanded);
 		this.ctx.present(component);
+		this.#lastTtsrNotification = component;
 	}

 	async #handleTodoReminder(event: Extract<AgentSessionEvent, { type: "todo_reminder" }>): Promise<void> {
@@ -0,0 +1,80 @@
+import { beforeAll, describe, expect, it } from "bun:test";
+import type { Rule } from "@oh-my-pi/pi-coding-agent/capability/rule";
+import { TtsrNotificationComponent } from "@oh-my-pi/pi-coding-agent/modes/components/ttsr-notification";
+import { initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme";
+
+beforeAll(async () => {
+	await initTheme(false);
+});
+
+function makeRule(name: string, description: string): Rule {
+	return {
+		name,
+		path: `/tmp/${name}.md`,
+		content: `${description}\nlong form guidance for ${name}`,
+		description,
+		condition: ["forbidden"],
+		_source: {
+			provider: "test",
+			providerName: "test",
+			path: `/tmp/${name}.md`,
+			level: "project",
+		},
+	};
+}
+
+function renderText(component: TtsrNotificationComponent, width = 100): string {
+	return Bun.stripANSI(component.render(width).join("\n"));
+}
+
+describe("TtsrNotificationComponent", () => {
+	it("renders multiple rules as one block with name: description rows", () => {
+		const component = new TtsrNotificationComponent([
+			makeRule("ts-no-tiny-functions", "Do not extract 1-2 line functions"),
+			makeRule("ts-set-map", "Prefer Record<K, V> for small static literals"),
+		]);
+		const text = renderText(component);
+
+		expect(text).toContain("Injecting 2 rules");
+		expect(text).toContain("ts-no-tiny-functions: Do not extract 1-2 line functions");
+		expect(text).toContain("ts-set-map: Prefer Record<K, V> for small static literals");
+	});
+
+	it("collapses to 4 rules with a +N more hint, expanded shows all", () => {
+		const rules = Array.from({ length: 6 }, (_, i) => makeRule(`rule-${i}`, `description ${i}`));
+		const component = new TtsrNotificationComponent(rules);
+
+		const collapsed = renderText(component);
+		expect(collapsed).toContain("Injecting 6 rules");
+		expect(collapsed).toContain("rule-3");
+		expect(collapsed).not.toContain("rule-4");
+		expect(collapsed).toContain("+2 more");
+
+		component.setExpanded(true);
+		const expanded = renderText(component);
+		expect(expanded).toContain("rule-4");
+		expect(expanded).toContain("rule-5");
+		expect(expanded).not.toContain("+2 more");
+	});
+
+	it("addRules merges new rules and dedupes by name", () => {
+		const component = new TtsrNotificationComponent([makeRule("ts-set-map", "Prefer Record<K, V>")]);
+		component.addRules([
+			makeRule("ts-set-map", "Prefer Record<K, V>"),
+			makeRule("ts-no-tiny-functions", "Do not extract tiny functions"),
+		]);
+
+		const text = renderText(component);
+		expect(text).toContain("Injecting 2 rules");
+		expect(text.match(/ts-set-map/g)).toHaveLength(1);
+		expect(text).toContain("ts-no-tiny-functions");
+	});
+
+	it("single rule keeps the dedicated header with description below", () => {
+		const component = new TtsrNotificationComponent([makeRule("ts-set-map", "Prefer Record<K, V>")]);
+		const text = renderText(component);
+
+		expect(text).toContain("Injecting rule: ts-set-map");
+		expect(text).toContain("Prefer Record<K, V>");
+	});
+});