From ad1c17fffddbbfcd8f6fee04425cf1440771c7bc Mon Sep 17 00:00:00 2001 From: can1357 Date: Wed, 10 Jun 2026 23:15:40 +0200 Subject: [PATCH] ux(coding-agent): combined ttsr rule notifications into one block --- .../src/modes/components/ttsr-notification.ts | 110 ++++++++++++------ .../src/modes/controllers/event-controller.ts | 20 ++++ .../components/ttsr-notification.test.ts | 80 +++++++++++++ 3 files changed, 176 insertions(+), 34 deletions(-) create mode 100644 packages/coding-agent/test/modes/components/ttsr-notification.test.ts diff --git a/packages/coding-agent/src/modes/components/ttsr-notification.ts b/packages/coding-agent/src/modes/components/ttsr-notification.ts index 6734bac37..f7ce8eccf 100644 --- a/packages/coding-agent/src/modes/components/ttsr-notification.ts +++ b/packages/coding-agent/src/modes/components/ttsr-notification.ts @@ -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)); } } } diff --git a/packages/coding-agent/src/modes/controllers/event-controller.ts b/packages/coding-agent/src/modes/controllers/event-controller.ts index 2c12e1d45..0e69a4220 100644 --- a/packages/coding-agent/src/modes/controllers/event-controller.ts +++ b/packages/coding-agent/src/modes/controllers/event-controller.ts @@ -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): Promise { + // 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): Promise { diff --git a/packages/coding-agent/test/modes/components/ttsr-notification.test.ts b/packages/coding-agent/test/modes/components/ttsr-notification.test.ts new file mode 100644 index 000000000..b4a1a29f3 --- /dev/null +++ b/packages/coding-agent/test/modes/components/ttsr-notification.test.ts @@ -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 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 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")]); + component.addRules([ + makeRule("ts-set-map", "Prefer Record"), + 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")]); + const text = renderText(component); + + expect(text).toContain("Injecting rule: ts-set-map"); + expect(text).toContain("Prefer Record"); + }); +});