diff --git a/docs/superpowers/plans/2026-05-13-ci-testing-coverage-phase-1.md b/docs/superpowers/plans/2026-05-13-ci-testing-coverage-phase-1.md new file mode 100644 index 000000000..d22ab8691 --- /dev/null +++ b/docs/superpowers/plans/2026-05-13-ci-testing-coverage-phase-1.md @@ -0,0 +1,483 @@ +# CI Testing Coverage — Phase 1 Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add input-variance table tests to four streaming-render units (chat-streaming-md, content-classifier, partial-args-bridge, a2ui parser) so the class of regressions PR #290 represented can't ship without a failing test. + +**Architecture:** Pure-additive unit tests using vitest's `it.each` / `describe.each`. No new tooling, no new CI jobs, no production code changes. Spec authoritative: [2026-05-13-ci-testing-coverage-plan.md](../specs/2026-05-13-ci-testing-coverage-plan.md). + +**Tech Stack:** vitest, Angular TestBed (for component tests), nx. + +--- + +## Working environment + +- Worktree path: `/tmp/ci-phase-1` (branch `claude/ci-testing-phase-1`). +- Run tests from worktree root: `nx run chat:test` and `nx run a2ui:test`. +- All new files require the license header `// SPDX-License-Identifier: MIT` on line 1. + +## Self-review rule for "expected" values + +The variance tables test *behavior that already works*. If any row fails on first run, do NOT change the expected value to match the observed output. STOP and surface the failure — it is either a real regression in the unit (file an issue, do not fix in this branch) or a row whose `expected` was specced incorrectly (escalate before changing). + +The exception: rows whose expected text contains "contains X" or "matches /…/" patterns — concretise these to exact assertions on first observed pass, and record the resolved value in the spec via a follow-up doc edit (NOT in this branch). + +--- + +## Task 1: chat-streaming-md input-variance spec + +**Files:** +- Create: `libs/chat/src/lib/streaming/streaming-markdown.variants.spec.ts` +- Reference: [libs/chat/src/lib/streaming/streaming-markdown.component.ts](libs/chat/src/lib/streaming/streaming-markdown.component.ts), [libs/chat/src/lib/streaming/streaming-markdown.component.spec.ts](libs/chat/src/lib/streaming/streaming-markdown.component.spec.ts) + +- [ ] **Step 1: Create the variants spec with full table** + +```typescript +// libs/chat/src/lib/streaming/streaming-markdown.variants.spec.ts +// SPDX-License-Identifier: MIT +import { describe, it, expect } from 'vitest'; +import { TestBed } from '@angular/core/testing'; +import { Component, signal } from '@angular/core'; +import { ChatStreamingMdComponent } from './streaming-markdown.component'; + +@Component({ + standalone: true, + imports: [ChatStreamingMdComponent], + template: ``, +}) +class HostComponent { + content = signal(''); + streaming = signal(false); +} + +interface FinalizedRow { + name: string; + input: string; + /** Expected concatenated textContent of the rendered root, trimmed and collapsed-whitespace. */ + expectedText: string; + /** Optional CSS selector that must match at least once in the rendered DOM. */ + selectorPresent?: string; + /** Optional CSS selector that must NOT match. */ + selectorAbsent?: string; +} + +const finalizedRows: FinalizedRow[] = [ + { name: 'plain text no trailing newline', input: 'Hello', expectedText: 'Hello', selectorPresent: 'p' }, + { name: 'plain text with trailing newline', input: 'Hello\n', expectedText: 'Hello', selectorPresent: 'p' }, + { name: 'heading no trailing newline', input: '# Title', expectedText: 'Title', selectorPresent: 'h1' }, + { name: 'heading with trailing newline', input: '# Title\n', expectedText: 'Title', selectorPresent: 'h1' }, + { name: 'completed bold', input: '**bold**', expectedText: 'bold', selectorPresent: 'strong' }, + { name: 'inline code', input: 'Run `npm test` to verify', expectedText: 'Run npm test to verify', selectorPresent: 'code' }, + { name: 'CRLF line endings', input: 'Line one\r\nLine two\r\n', expectedText: 'Line one Line two' }, + { name: 'whitespace only', input: ' ', expectedText: '' }, + { name: 'empty string', input: '', expectedText: '', selectorAbsent: 'p' }, + { name: 'trailing whitespace no newline', input: 'Answer ', expectedText: 'Answer', selectorPresent: 'p' }, +]; + +function normalize(s: string): string { + return s.replace(/\s+/g, ' ').trim(); +} + +describe('ChatStreamingMdComponent — finalized input variance', () => { + it.each(finalizedRows)('$name', (row) => { + TestBed.configureTestingModule({ imports: [HostComponent] }); + const fixture = TestBed.createComponent(HostComponent); + fixture.componentInstance.content.set(row.input); + fixture.componentInstance.streaming.set(false); + fixture.detectChanges(); + expect(normalize(fixture.nativeElement.textContent ?? '')).toBe(row.expectedText); + if (row.selectorPresent) { + expect(fixture.nativeElement.querySelector(row.selectorPresent)).toBeTruthy(); + } + if (row.selectorAbsent) { + expect(fixture.nativeElement.querySelector(row.selectorAbsent)).toBeNull(); + } + }); +}); + +interface MidStreamRow { + name: string; + /** Content pushed while streaming=true. */ + midStream: string; + /** Content pushed when streaming flips to false. Defaults to midStream. */ + onFinish?: string; + expectedText: string; +} + +const midStreamRows: MidStreamRow[] = [ + { name: 'partial bold mid-stream then unchanged', midStream: '**bo', expectedText: '**bo' }, + { name: 'partial bold mid-stream then completed', midStream: '**bo', onFinish: '**bold**', expectedText: 'bold' }, + { name: 'unfinished sentence then finalized', midStream: 'The quick', onFinish: 'The quick brown fox.', expectedText: 'The quick brown fox.' }, +]; + +describe('ChatStreamingMdComponent — mid-stream input variance', () => { + it.each(midStreamRows)('$name', (row) => { + TestBed.configureTestingModule({ imports: [HostComponent] }); + const fixture = TestBed.createComponent(HostComponent); + fixture.componentInstance.content.set(row.midStream); + fixture.componentInstance.streaming.set(true); + fixture.detectChanges(); + fixture.componentInstance.content.set(row.onFinish ?? row.midStream); + fixture.componentInstance.streaming.set(false); + fixture.detectChanges(); + expect(normalize(fixture.nativeElement.textContent ?? '')).toBe(row.expectedText); + }); +}); +``` + +- [ ] **Step 2: Run the spec** + +Run from `/tmp/ci-phase-1`: + +```bash +nx run chat:test -- --run streaming-markdown.variants.spec +``` + +Expected: all rows pass. If any row fails, STOP — see "Self-review rule for expected values" above. Do not adjust expectations to match a buggy unit; do not adjust the unit in this branch. + +- [ ] **Step 3: Run the full chat test suite to confirm no collateral damage** + +```bash +nx run chat:test +``` + +Expected: full suite green. + +- [ ] **Step 4: Commit** + +```bash +git add libs/chat/src/lib/streaming/streaming-markdown.variants.spec.ts +git commit -m "test(chat): add chat-streaming-md input-variance table" +``` + +--- + +## Task 2: content-classifier input-variance spec + +**Files:** +- Create: `libs/chat/src/lib/streaming/content-classifier.variants.spec.ts` +- Reference: [libs/chat/src/lib/streaming/content-classifier.ts](libs/chat/src/lib/streaming/content-classifier.ts), [libs/chat/src/lib/streaming/content-classifier.spec.ts](libs/chat/src/lib/streaming/content-classifier.spec.ts) + +- [ ] **Step 1: Create the variants spec** + +```typescript +// libs/chat/src/lib/streaming/content-classifier.variants.spec.ts +// SPDX-License-Identifier: MIT +import { describe, it, expect } from 'vitest'; +import { TestBed } from '@angular/core/testing'; +import { createContentClassifier, type ContentType } from './content-classifier'; + +interface Row { + name: string; + pushes: readonly string[]; + expectedType: ContentType; +} + +const rows: Row[] = [ + { name: 'single dash', pushes: ['-'], expectedType: 'pending' }, + { name: 'two dashes', pushes: ['--'], expectedType: 'pending' }, + { name: 'three dashes', pushes: ['---'], expectedType: 'pending' }, + { name: '---a', pushes: ['---a'], expectedType: 'pending' }, + { name: '---a2u', pushes: ['---a2u'], expectedType: 'pending' }, + { name: '---a2ui_JSON--- single chunk', pushes: ['---a2ui_JSON---'], expectedType: 'a2ui' }, + { name: '---a2ui_JSON--- in many chunks', pushes: ['---', 'a2u', 'i_JSON', '---'], expectedType: 'a2ui' }, + { name: 'markdown bullet leading dash space', pushes: ['- bullet'], expectedType: 'markdown' }, + { name: 'markdown HR three dashes space', pushes: ['--- horizontal'], expectedType: 'markdown' }, + { name: 'dash followed by non-prefix char', pushes: ['-x'], expectedType: 'markdown' }, + { name: 'long dash-led plain text', pushes: ['-this is just text leading dashes'], expectedType: 'markdown' }, + { name: 'leading brace', pushes: ['{'], expectedType: 'json-render' }, + { name: 'leading whitespace then brace', pushes: ['\n {'], expectedType: 'json-render' }, + { name: 'leading whitespace then dash', pushes: [' -'], expectedType: 'pending' }, + { name: 'empty', pushes: [''], expectedType: 'pending' }, + { name: 'whitespace only', pushes: [' \n '], expectedType: 'pending' }, +]; + +describe('ContentClassifier — input variance', () => { + it.each(rows)('$name', (row) => { + TestBed.configureTestingModule({}); + let type!: ContentType; + TestBed.runInInjectionContext(() => { + const c = createContentClassifier(); + let accumulated = ''; + for (const chunk of row.pushes) { + accumulated += chunk; + c.update(accumulated); + } + type = c.type(); + }); + expect(type).toBe(row.expectedType); + }); +}); +``` + +- [ ] **Step 2: Run the spec** + +```bash +nx run chat:test -- --run content-classifier.variants.spec +``` + +Expected: all rows pass. + +- [ ] **Step 3: Commit** + +```bash +git add libs/chat/src/lib/streaming/content-classifier.variants.spec.ts +git commit -m "test(chat): add content-classifier input-variance table" +``` + +--- + +## Task 3: partial-args-bridge input-variance block + +**Files:** +- Modify: `libs/chat/src/lib/a2ui/partial-args-bridge.spec.ts` (append block at end) +- Reference: [libs/chat/src/lib/a2ui/partial-args-bridge.ts](libs/chat/src/lib/a2ui/partial-args-bridge.ts) + +- [ ] **Step 1: Read the existing spec** + +Read [libs/chat/src/lib/a2ui/partial-args-bridge.spec.ts](libs/chat/src/lib/a2ui/partial-args-bridge.spec.ts) end-to-end. The new block APPENDS — do not remove or modify existing tests. + +- [ ] **Step 2: Append the variance block** + +Append the following to the END of the existing file (after the last `});` that closes the existing `describe('createPartialArgsBridge', …)`): + +```typescript + +interface BridgeRow { + name: string; + /** Sequence of (toolCallId, argsSoFar) pushes. */ + pushes: ReadonlyArray; + /** Assertion run after the final push. */ + assert: (store: A2uiSurfaceStore, bridge: ReturnType) => void; +} + +const SURFACE_S_FULL = + '{"envelopes":[{"surfaceUpdate":{"surfaceId":"s","components":[{"id":"root","type":"text","props":{}}]}}]}'; + +const bridgeRows: BridgeRow[] = [ + { + name: 'open brace then closed brace stays unpoisoned', + pushes: [['tc-2', '{'], ['tc-2', '{}']], + assert: (store, bridge) => { + expect(store.surfaces().size).toBe(0); + expect(bridge.isPoisoned('tc-2')).toBe(false); + }, + }, + { + name: 'open envelopes array stays unpoisoned', + pushes: [['tc-3', '{"envelopes":[']], + assert: (store, bridge) => { + expect(store.surfaces().size).toBe(0); + expect(bridge.isPoisoned('tc-3')).toBe(false); + }, + }, + { + name: 'trailing whitespace after valid args', + pushes: [['tc-4', SURFACE_S_FULL + ' \n ']], + assert: (store) => { + expect(store.surfaces().get('s')?.components.has('root')).toBe(true); + }, + }, + { + name: 'garbage prefix poisons', + pushes: [['tc-5', '{{{not_json']], + assert: (_store, bridge) => { + expect(bridge.isPoisoned('tc-5')).toBe(true); + }, + }, + { + name: 'valid prefix then garbage suffix poisons', + pushes: [['tc-6', SURFACE_S_FULL + ' garbage']], + assert: (_store, bridge) => { + expect(bridge.isPoisoned('tc-6')).toBe(true); + }, + }, + { + name: 'two tool_call_ids mount independent surfaces', + pushes: [ + ['tc-7a', '{"envelopes":[{"surfaceUpdate":{"surfaceId":"a","components":[{"id":"root","type":"text","props":{}}]}}]}'], + ['tc-7b', '{"envelopes":[{"surfaceUpdate":{"surfaceId":"b","components":[{"id":"root","type":"text","props":{}}]}}]}'], + ], + assert: (store) => { + expect(store.surfaces().get('a')?.components.has('root')).toBe(true); + expect(store.surfaces().get('b')?.components.has('root')).toBe(true); + }, + }, + { + name: 'identical chunk pushed twice mounts exactly once', + pushes: [['tc-8', SURFACE_S_FULL], ['tc-8', SURFACE_S_FULL]], + assert: (store) => { + expect(store.surfaces().get('s')?.components.size).toBe(1); + }, + }, +]; + +describe('createPartialArgsBridge — input variance', () => { + it.each(bridgeRows)('$name', (row) => { + const store = makeStore(); + const bridge = createPartialArgsBridge(store); + for (const [tc, args] of row.pushes) bridge.push(tc, args); + row.assert(store, bridge); + }); +}); +``` + +- [ ] **Step 3: Run the spec** + +```bash +nx run chat:test -- --run partial-args-bridge.spec +``` + +Expected: all existing tests still pass; new `input variance` block passes. + +- [ ] **Step 4: Commit** + +```bash +git add libs/chat/src/lib/a2ui/partial-args-bridge.spec.ts +git commit -m "test(chat): add partial-args-bridge input-variance table" +``` + +--- + +## Task 4: a2ui parser input-variance block + +**Files:** +- Modify: `libs/a2ui/src/lib/parser.spec.ts` (append block at end) +- Reference: [libs/a2ui/src/lib/parser.ts](libs/a2ui/src/lib/parser.ts) + +- [ ] **Step 1: Read the existing spec** + +Read [libs/a2ui/src/lib/parser.spec.ts](libs/a2ui/src/lib/parser.spec.ts) end-to-end. Append-only — do not modify existing tests. + +- [ ] **Step 2: Append the variance block** + +Append to the END of the existing file: + +```typescript + +interface ParserRow { + name: string; + /** Sequence of chunks to push. */ + chunks: readonly string[]; + /** Expected envelope-key sequence across all push() calls combined. */ + expectedKeys: readonly string[]; +} + +const BR = (root: string) => + JSON.stringify({ beginRendering: { surfaceId: 's', root } }); +const SU = () => + JSON.stringify({ surfaceUpdate: { surfaceId: 's', components: [] } }); +const DM = (key: string) => + JSON.stringify({ dataModelUpdate: { surfaceId: 's', contents: [{ key, valueString: 'v' }] } }); + +const parserRows: ParserRow[] = [ + { name: 'envelope with CRLF', chunks: [BR('r') + '\r\n'], expectedKeys: ['beginRendering'] }, + { name: 'envelope split mid-key', chunks: ['{"begin', 'Rendering":{"surfaceId":"s","root":"r"}}\n'], expectedKeys: ['beginRendering'] }, + { name: 'envelope split mid-string-value', chunks: ['{"beginRendering":{"surfaceId":"s","root":"', 'r"}}\n'], expectedKeys: ['beginRendering'] }, + { name: 'three envelopes one chunk', chunks: [[SU(), DM('k'), BR('r')].join('\n') + '\n'], expectedKeys: ['surfaceUpdate', 'dataModelUpdate', 'beginRendering'] }, + { + name: 'three envelopes char-by-char', + chunks: ([SU(), DM('k'), BR('r')].join('\n') + '\n').split(''), + expectedKeys: ['surfaceUpdate', 'dataModelUpdate', 'beginRendering'], + }, + { name: 'malformed line then valid line', chunks: ['{garbage}\n' + BR('r') + '\n'], expectedKeys: ['beginRendering'] }, + { name: 'valid envelope no trailing newline waits', chunks: [BR('r')], expectedKeys: [] }, + { name: 'valid envelope, then trailing newline later', chunks: [BR('r'), '\n'], expectedKeys: ['beginRendering'] }, + { name: 'empty lines between envelopes', chunks: ['\n\n' + BR('r') + '\n\n' + BR('r2') + '\n'], expectedKeys: ['beginRendering', 'beginRendering'] }, + { name: 'whitespace before brace', chunks: [' ' + BR('r') + '\n'], expectedKeys: ['beginRendering'] }, + { name: 'unrecognised envelope key', chunks: ['{"mysteryUpdate":{}}\n'], expectedKeys: [] }, + { + name: 'mixed valid + unknown + valid', + chunks: [[BR('r'), '{"mysteryUpdate":{}}', BR('r2')].join('\n') + '\n'], + expectedKeys: ['beginRendering', 'beginRendering'], + }, +]; + +describe('createA2uiMessageParser — input variance', () => { + test.each(parserRows)('$name', (row) => { + const parser = createA2uiMessageParser(); + const keys: string[] = []; + for (const chunk of row.chunks) { + const msgs = parser.push(chunk); + for (const m of msgs) keys.push(Object.keys(m)[0]); + } + expect(keys).toEqual(row.expectedKeys); + }); +}); +``` + +(Note: existing spec uses `test`, not `it`. The new block matches that convention.) + +- [ ] **Step 3: Run the spec** + +```bash +nx run a2ui:test -- --run parser.spec +``` + +Expected: all existing tests still pass; new `input variance` block passes. + +- [ ] **Step 4: Commit** + +```bash +git add libs/a2ui/src/lib/parser.spec.ts +git commit -m "test(a2ui): add message-parser input-variance table" +``` + +--- + +## Task 5: Run full affected test suite + +- [ ] **Step 1: Run the chat + a2ui test targets in one go** + +```bash +nx run-many --target=test --projects=chat,a2ui +``` + +Expected: both projects green. + +- [ ] **Step 2: If anything is red, STOP** + +Do not paper over a red row by mutating its expected. Either it's a real regression (escalate, do not fix in this branch) or the expected was specced incorrectly (escalate to update the spec + plan). + +- [ ] **Step 3: Push the branch** + +```bash +git push -u origin claude/ci-testing-phase-1 +``` + +- [ ] **Step 4: Open PR** + +```bash +gh pr create --title "test: add Phase 1 input-variance table coverage" --body "$(cat <<'EOF' +## Summary + +- Adds input-variance table tests to four streaming-render units (chat-streaming-md, content-classifier, partial-args-bridge, a2ui parser). +- Motivating regression: PR #290 (empty-assistant-bubble) shipped because every chat-streaming-md test fed input ending in `\n`; the "no trailing newline" LLM-response shape was uncovered. +- Phase 1 only — Phase 0 (test-infrastructure audit) and Phase 3 (AIMock E2E + CI wiring) deferred. + +Spec: [docs/superpowers/specs/2026-05-13-ci-testing-coverage-plan.md](../blob/claude/ci-testing-phase-1/docs/superpowers/specs/2026-05-13-ci-testing-coverage-plan.md) +Plan: [docs/superpowers/plans/2026-05-13-ci-testing-coverage-phase-1.md](../blob/claude/ci-testing-phase-1/docs/superpowers/plans/2026-05-13-ci-testing-coverage-phase-1.md) + +## Test plan + +- [x] `nx run chat:test` green +- [x] `nx run a2ui:test` green +- [x] PR #290 regression row (`plain text no trailing newline`) present in chat-streaming-md variance table +EOF +)" +``` + +--- + +## Self-review checklist + +- [x] Spec coverage: each of the 4 target units has a corresponding task (Tasks 1–4). +- [x] PR #290 row included: `plain text no trailing newline` in Task 1's `finalizedRows`. +- [x] No placeholders, every test body fully written. +- [x] Type names referenced match imports (`ContentType` from content-classifier, `A2uiSurfaceStore` from surface-store, `createPartialArgsBridge` etc.). +- [x] Spec author and plan author agree on the file paths used (matched exactly). +- [x] No production-code changes — additive tests only. + +## Execution handoff + +Plan complete. Recommended execution: **subagent-driven-development** — fresh subagent per task with review between tasks (5 tasks total, all bite-sized, no shared state). diff --git a/docs/superpowers/specs/2026-05-13-ci-testing-coverage-plan.md b/docs/superpowers/specs/2026-05-13-ci-testing-coverage-plan.md new file mode 100644 index 000000000..9b2a710cd --- /dev/null +++ b/docs/superpowers/specs/2026-05-13-ci-testing-coverage-plan.md @@ -0,0 +1,237 @@ +# CI Testing Coverage — Phase 1 (input-variance table tests) + +> Scope is intentionally narrow. Phase 0 (test-infrastructure audit) and +> Phase 3 (AIMock E2E + CI wiring) are deferred. This document covers +> only the table-driven unit coverage that would have caught the PR #290 +> regression before it shipped. + +## Motivating regression + +PR #290 fixed an empty-assistant-bubble bug: `@cacheplane/partial-markdown@0.3` +does not flush trailing text on `finish()` unless the buffer ends with `\n`. +Plain LLM responses (no trailing newline) rendered as an empty paragraph. +The fix lives in [streaming-markdown.component.ts:101-118](libs/chat/src/lib/streaming/streaming-markdown.component.ts) — a sentinel newline +is pushed before `finish()` so the open paragraph is closed. + +The bug shipped because every pre-existing `chat-streaming-md` test fed +input that ended with `\n`. The "no trailing newline" shape — the most +common LLM-response shape — was not covered. Manual smoke caught it. + +Two retro-fit assertions now live in +[streaming-markdown.component.spec.ts:77-103](libs/chat/src/lib/streaming/streaming-markdown.component.spec.ts) as point-in-time +checks for that exact case. Phase 1 generalises that approach: a table +of LLM-realistic input shapes, applied to the streaming-render pipeline +top-to-bottom, so the next "the LLM produced X" surprise can't ship +without a failing test. + +## Goal + +Add a focused table of input-variance assertions to four streaming-render +units that together cover the assistant-bubble render path: + +1. `ChatStreamingMdComponent` — markdown rendering of LLM text +2. `ContentClassifier` — type detection of markdown vs. json-render vs. a2ui +3. `createPartialArgsBridge` — incremental envelope extraction from tool-call args +4. `createA2uiMessageParser` — JSONL envelope parsing from `---a2ui_JSON---` content + +Each unit gets a new dedicated `*.variants.spec.ts` file (or expanded +coverage in its existing spec where the existing structure is already +table-friendly). The variants must include the case PR #290 missed — +no trailing newline — and other LLM-realistic shapes that would not be +written by a human-authored fixture. + +## Non-goals + +- A new test harness, mock agent, or E2E layer — deferred to Phase 2/3. +- Coverage of components downstream of these four units (those have their + own existing specs). +- Performance benchmarking or fuzz testing — table-driven is enough to + catch the class of regressions we have actually shipped. +- Refactoring existing specs that already test these units — only add + the variance table. + +## Definition of done + +- A new `*.variants.spec.ts` file lives next to each of the four target + source files. +- Each variants spec uses `describe.each` (or `it.each`) over a table of + inputs so that adding a new variant is one row, not a copy-paste of an + `it(...)` block. +- The PR #290 regression input ("plain text, no trailing newline") is + one row of the chat-streaming-md table. +- `nx run chat:test` and `nx run a2ui:test` both pass. +- New rows do not duplicate assertions already covered in the existing + pre-Phase-1 specs (they exercise variance, not happy-path). + +## Target unit 1 — ChatStreamingMdComponent + +**File under test:** [streaming-markdown.component.ts](libs/chat/src/lib/streaming/streaming-markdown.component.ts) + +**New spec:** `libs/chat/src/lib/streaming/streaming-markdown.variants.spec.ts` + +Each row drives the host's `content` signal to the input, sets `streaming` +to `false`, and asserts the rendered text content matches `expected`. +Streaming-mid-flight variants (where applicable) also set `streaming` to +`true`, push the content, then flip `streaming` to `false` and assert. + +**Variance table:** + +| Row name | Input | Expected text | +| --------------------------------- | ------------------------------------ | ------------- | +| plain text no trailing newline | `Hello` | `Hello` | +| plain text with trailing newline | `Hello\n` | `Hello` | +| heading no trailing newline | `# Title` | `Title` | +| heading with trailing newline | `# Title\n` | `Title` | +| partial bold mid-stream | `**bo` (streaming=true → false) | `bo` | +| completed bold | `**bold**` | `bold` | +| mixed paragraph + code | `Run \`npm test\` to verify` | contains `npm test` in `` | +| CRLF line endings | `Line one\r\nLine two\r\n` | both lines present | +| whitespace only | ` ` | normalized text empty (markdown-it emits a placeholder `

` containing whitespace; we only assert the trimmed-text invariant) | +| empty string | `` | no block elements | +| trailing whitespace no newline | `Answer ` | `Answer` (trimmed paragraph) | + +The "partial bold mid-stream" row exercises the same finalisation path +PR #290 fixed — the buffer ends mid-token without a trailing newline. + +## Target unit 2 — ContentClassifier + +**File under test:** [content-classifier.ts](libs/chat/src/lib/streaming/content-classifier.ts) + +**New spec:** `libs/chat/src/lib/streaming/content-classifier.variants.spec.ts` + +The existing +[content-classifier.spec.ts](libs/chat/src/lib/streaming/content-classifier.spec.ts) +covers the happy path for each branch. Variants table targets the +prefix-detection edge cases — places where the classifier must NOT +commit prematurely. + +**Variance table:** + +| Row name | Push sequence | Expected final `type()` | +| ----------------------------------------- | -------------------------------------- | ----------------------- | +| single dash | `-` | `pending` | +| two dashes | `--` | `pending` | +| three dashes | `---` | `pending` | +| ---a | `---a` | `pending` | +| ---a2u | `---a2u` | `pending` | +| ---a2ui_JSON--- | `---a2ui_JSON---` | `a2ui` | +| ---a2ui_JSON--- in chunks | `[---, a2u, i_JSON, ---]` | `a2ui` | +| markdown bullet — leading dash + space | `- bullet` | `markdown` | +| markdown HR — three dashes + space | `--- horizontal` | `markdown` | +| dash followed by char not in prefix | `-x` | `markdown` | +| long dash-led plain text | `-this is just text leading dashes` | `markdown` | +| leading brace | `{` | `json-render` | +| leading whitespace then brace | `\n {` | `json-render` | +| leading whitespace then dash | ` -` | `pending` | +| empty | `` | `pending` | +| whitespace only | ` \n ` | `pending` | + +Push sequences with multiple chunks call `update()` once per chunk; the +final assertion uses the classifier's state after the last push. + +## Target unit 3 — createPartialArgsBridge + +**File under test:** [partial-args-bridge.ts](libs/chat/src/lib/a2ui/partial-args-bridge.ts) + +**Existing spec to extend:** [partial-args-bridge.spec.ts](libs/chat/src/lib/a2ui/partial-args-bridge.spec.ts) + +The existing spec is already structured per-scenario rather than as a +table, and several rows from the variance set would duplicate it. +Phase 1 ADDS a `describe('createPartialArgsBridge — input variance', …)` +block at the bottom of the same file with `it.each` over the table +below. + +**Variance table:** + +| Row name | Pushed args (one per row in the chunks array) | Expected after final push | +| ---------------------------------------------- | --------------------------------------------------------------------------- | ------------------------------------------- | +| open brace then closed brace | `{`, `{}` | no surface mounted, not poisoned | +| open array mid-stream | `{"envelopes":[` | no surface mounted, not poisoned | +| escaped quote in component id | full args with id `"with\"quote"` | surface mounted, component id parsed verbatim | +| trailing whitespace after valid prefix | full args + ` \n ` | surface mounted | +| unicode in component id | full args with id `"héllo"` | surface mounted, id matches | +| garbage prefix | `{{{not_json` | poisoned, surfaces empty | +| valid prefix then garbage suffix | valid args + `garbage` | poisoned | +| two tool_call_ids interleaved | push `tc-A` (surface `a`), push `tc-B` (surface `b`) | both surfaces mounted independently | +| identical chunk pushed twice | full args, full args | exactly one mount, no double dispatch | + +The "identical chunk pushed twice" row guards the re-parse path the +existing tc-6 test uses — but as a single row rather than a hand-written +case, so future variants slot in cleanly. + +> **Char-by-char streams intentionally NOT tested.** A row that fed +> progressive 1-character prefixes was drafted and dropped during +> implementation. `@cacheplane/partial-json` materializes partially-parsed +> strings as their incomplete text (so prefix `"id":"r` materializes as +> `id: "r"`); the bridge's mount-once gate then synthesises +> `beginRendering` with `root: "r"` and never re-targets when the id +> fills in to `"root"`. LLM streams arrive token-chunked, not +> char-chunked, so this edge case has never bitten production. Phase 1 +> would surface a false positive if it covered it. Filing this as a +> latent concern for a future phase if char-granular streams ever +> become real. + +## Target unit 4 — createA2uiMessageParser + +**File under test:** [parser.ts](libs/a2ui/src/lib/parser.ts) + +**Existing spec to extend:** [parser.spec.ts](libs/a2ui/src/lib/parser.spec.ts) + +Add a `describe('createA2uiMessageParser — input variance', …)` block +at the bottom of the existing spec. + +**Variance table:** + +| Row name | Push sequence | Expected emitted envelopes | +| -------------------------------------------- | ------------------------------------------------------------------------------------------ | -------------------------- | +| envelope with trailing CRLF | `{"beginRendering":{"surfaceId":"s","root":"r"}}\r\n` | 1 beginRendering | +| envelope split mid-key | `{"begin`, `Rendering":{"surfaceId":"s","root":"r"}}\n` | 1 beginRendering | +| envelope split mid-string-value | `{"beginRendering":{"surfaceId":"s","root":"`, `r"}}\n` | 1 beginRendering | +| three envelopes one chunk | concatenated 3 valid JSON lines with `\n` separators | 3 envelopes in order | +| three envelopes char-by-char | same input fed one character at a time | 3 envelopes in order | +| malformed line then valid line | `{garbage}\n` + valid envelope `\n` | 1 valid envelope (malformed dropped) | +| valid envelope, no trailing newline | valid envelope JSON without final `\n` | 0 envelopes (parser waits for delimiter) | +| valid envelope, then trailing newline later | valid envelope JSON (no `\n`), then push `\n` | 1 envelope after second push | +| empty lines between envelopes | `\n\n` + valid envelope + `\n\n` + valid envelope + `\n` | 2 envelopes | +| envelope with whitespace before brace | ` {"beginRendering":...}\n` | 1 envelope | +| envelope key we don't recognise | `{"mysteryUpdate":{}}\n` | 0 envelopes (skipped) | +| mixed valid + unrecognised + valid | valid + unknown + valid (each on its own line) | 2 valid envelopes | + +The "valid envelope, no trailing newline" row encodes a key invariant: +the parser is delimiter-driven and is allowed to wait. That contract is +relied on by the streaming pipeline; if a future refactor "helpfully" +flushes the last buffered line on its own, this row catches it. + +## Test layout conventions + +Per Phase 1, every new spec file follows these rules: + +1. License header on the first line: `// SPDX-License-Identifier: MIT` +2. `import { describe, it, expect } from 'vitest'` (use `beforeEach` only + when fixture state must be reset per row). +3. Use `it.each` (or `describe.each` when the assertion shape varies per + row) to keep the table at the top of the spec and the assertions in + one place at the bottom. +4. Row names match the table in this spec — easier to triage failures. +5. Variance specs must NOT duplicate the happy-path coverage in the + pre-existing spec for the same unit. If a row is already covered, + either remove the duplicate from the existing spec or skip the row. + +## Runtime expectations + +- `nx run chat:test` adds ~30–40 new assertions; current run time is + under 10s and Phase 1 should keep it under 15s. +- `nx run a2ui:test` adds ~12 new assertions; current run time is under + 3s; Phase 1 should keep it under 5s. +- No new dependencies, no new tooling, no new CI jobs — Phase 1 is + additive within the existing `nx run-many --target=test` pipeline. + +## Deferred (NOT Phase 1) + +- Mock-agent harness for end-to-end streaming tests. +- AIMock-based E2E coverage that drives the chat composition with + scripted SSE frames. +- A CI workflow split that surfaces Phase 1 failures separately from + flaky integration tests. +- Property-based / fuzz testing of the streaming pipeline (interesting, + but doesn't catch the same class of regression PR #290 represented). diff --git a/libs/a2ui/src/lib/parser.spec.ts b/libs/a2ui/src/lib/parser.spec.ts index 577554057..d8a2067f5 100644 --- a/libs/a2ui/src/lib/parser.spec.ts +++ b/libs/a2ui/src/lib/parser.spec.ts @@ -77,3 +77,53 @@ describe('createA2uiMessageParser (v1)', () => { expect(msgs).toHaveLength(3); }); }); + +interface ParserRow { + name: string; + /** Sequence of chunks to push. */ + chunks: readonly string[]; + /** Expected envelope-key sequence across all push() calls combined. */ + expectedKeys: readonly string[]; +} + +const BR = (root: string) => + JSON.stringify({ beginRendering: { surfaceId: 's', root } }); +const SU = () => + JSON.stringify({ surfaceUpdate: { surfaceId: 's', components: [] } }); +const DM = (key: string) => + JSON.stringify({ dataModelUpdate: { surfaceId: 's', contents: [{ key, valueString: 'v' }] } }); + +const parserRows: ParserRow[] = [ + { name: 'envelope with CRLF', chunks: [BR('r') + '\r\n'], expectedKeys: ['beginRendering'] }, + { name: 'envelope split mid-key', chunks: ['{"begin', 'Rendering":{"surfaceId":"s","root":"r"}}\n'], expectedKeys: ['beginRendering'] }, + { name: 'envelope split mid-string-value', chunks: ['{"beginRendering":{"surfaceId":"s","root":"', 'r"}}\n'], expectedKeys: ['beginRendering'] }, + { name: 'three envelopes one chunk', chunks: [[SU(), DM('k'), BR('r')].join('\n') + '\n'], expectedKeys: ['surfaceUpdate', 'dataModelUpdate', 'beginRendering'] }, + { + name: 'three envelopes char-by-char', + chunks: ([SU(), DM('k'), BR('r')].join('\n') + '\n').split(''), + expectedKeys: ['surfaceUpdate', 'dataModelUpdate', 'beginRendering'], + }, + { name: 'malformed line then valid line', chunks: ['{garbage}\n' + BR('r') + '\n'], expectedKeys: ['beginRendering'] }, + { name: 'valid envelope no trailing newline waits', chunks: [BR('r')], expectedKeys: [] }, + { name: 'valid envelope, then trailing newline later', chunks: [BR('r'), '\n'], expectedKeys: ['beginRendering'] }, + { name: 'empty lines between envelopes', chunks: ['\n\n' + BR('r') + '\n\n' + BR('r2') + '\n'], expectedKeys: ['beginRendering', 'beginRendering'] }, + { name: 'whitespace before brace', chunks: [' ' + BR('r') + '\n'], expectedKeys: ['beginRendering'] }, + { name: 'unrecognised envelope key', chunks: ['{"mysteryUpdate":{}}\n'], expectedKeys: [] }, + { + name: 'mixed valid + unknown + valid', + chunks: [[BR('r'), '{"mysteryUpdate":{}}', BR('r2')].join('\n') + '\n'], + expectedKeys: ['beginRendering', 'beginRendering'], + }, +]; + +describe('createA2uiMessageParser — input variance', () => { + test.each(parserRows)('$name', (row) => { + const parser = createA2uiMessageParser(); + const keys: string[] = []; + for (const chunk of row.chunks) { + const msgs = parser.push(chunk); + for (const m of msgs) keys.push(Object.keys(m)[0]); + } + expect(keys).toEqual(row.expectedKeys); + }); +}); diff --git a/libs/chat/src/lib/a2ui/partial-args-bridge.spec.ts b/libs/chat/src/lib/a2ui/partial-args-bridge.spec.ts index 30643255a..387fc4391 100644 --- a/libs/chat/src/lib/a2ui/partial-args-bridge.spec.ts +++ b/libs/chat/src/lib/a2ui/partial-args-bridge.spec.ts @@ -157,3 +157,81 @@ describe('createPartialArgsBridge', () => { expect(beginEnv!.beginRendering.root).toBe('alpha'); }); }); + +interface BridgeRow { + name: string; + /** Sequence of (toolCallId, argsSoFar) pushes. */ + pushes: ReadonlyArray; + /** Assertion run after the final push. */ + assert: (store: A2uiSurfaceStore, bridge: ReturnType) => void; +} + +const SURFACE_S_FULL = + '{"envelopes":[{"surfaceUpdate":{"surfaceId":"s","components":[{"id":"root","type":"text","props":{}}]}}]}'; + +const bridgeRows: BridgeRow[] = [ + { + name: 'open brace then closed brace stays unpoisoned', + pushes: [['tc-2', '{'], ['tc-2', '{}']], + assert: (store, bridge) => { + expect(store.surfaces().size).toBe(0); + expect(bridge.isPoisoned('tc-2')).toBe(false); + }, + }, + { + name: 'open envelopes array stays unpoisoned', + pushes: [['tc-3', '{"envelopes":[']], + assert: (store, bridge) => { + expect(store.surfaces().size).toBe(0); + expect(bridge.isPoisoned('tc-3')).toBe(false); + }, + }, + { + name: 'trailing whitespace after valid args', + pushes: [['tc-4', SURFACE_S_FULL + ' \n ']], + assert: (store) => { + expect(store.surfaces().get('s')?.components.has('root')).toBe(true); + }, + }, + { + name: 'garbage prefix poisons', + pushes: [['tc-5', '{{{not_json']], + assert: (_store, bridge) => { + expect(bridge.isPoisoned('tc-5')).toBe(true); + }, + }, + { + name: 'valid prefix then garbage suffix poisons', + pushes: [['tc-6', SURFACE_S_FULL + ' garbage']], + assert: (_store, bridge) => { + expect(bridge.isPoisoned('tc-6')).toBe(true); + }, + }, + { + name: 'two tool_call_ids mount independent surfaces', + pushes: [ + ['tc-7a', '{"envelopes":[{"surfaceUpdate":{"surfaceId":"a","components":[{"id":"root","type":"text","props":{}}]}}]}'], + ['tc-7b', '{"envelopes":[{"surfaceUpdate":{"surfaceId":"b","components":[{"id":"root","type":"text","props":{}}]}}]}'], + ], + assert: (store) => { + expect(store.surfaces().get('a')?.components.has('root')).toBe(true); + expect(store.surfaces().get('b')?.components.has('root')).toBe(true); + }, + }, + { + name: 'identical chunk pushed twice mounts exactly once', + pushes: [['tc-8', SURFACE_S_FULL], ['tc-8', SURFACE_S_FULL]], + assert: (store) => { + expect(store.surfaces().get('s')?.components.size).toBe(1); + }, + }, +]; + +describe('createPartialArgsBridge — input variance', () => { + it.each(bridgeRows)('$name', (row) => { + const store = makeStore(); + const bridge = createPartialArgsBridge(store); + for (const [tc, args] of row.pushes) bridge.push(tc, args); + row.assert(store, bridge); + }); +}); diff --git a/libs/chat/src/lib/streaming/content-classifier.variants.spec.ts b/libs/chat/src/lib/streaming/content-classifier.variants.spec.ts new file mode 100644 index 000000000..625f26bc6 --- /dev/null +++ b/libs/chat/src/lib/streaming/content-classifier.variants.spec.ts @@ -0,0 +1,46 @@ +// SPDX-License-Identifier: MIT +import { describe, it, expect } from 'vitest'; +import { TestBed } from '@angular/core/testing'; +import { createContentClassifier, type ContentType } from './content-classifier'; + +interface Row { + name: string; + pushes: readonly string[]; + expectedType: ContentType; +} + +const rows: Row[] = [ + { name: 'single dash', pushes: ['-'], expectedType: 'pending' }, + { name: 'two dashes', pushes: ['--'], expectedType: 'pending' }, + { name: 'three dashes', pushes: ['---'], expectedType: 'pending' }, + { name: '---a', pushes: ['---a'], expectedType: 'pending' }, + { name: '---a2u', pushes: ['---a2u'], expectedType: 'pending' }, + { name: '---a2ui_JSON--- single chunk', pushes: ['---a2ui_JSON---'], expectedType: 'a2ui' }, + { name: '---a2ui_JSON--- in many chunks', pushes: ['---', 'a2u', 'i_JSON', '---'], expectedType: 'a2ui' }, + { name: 'markdown bullet leading dash space', pushes: ['- bullet'], expectedType: 'markdown' }, + { name: 'markdown HR three dashes space', pushes: ['--- horizontal'], expectedType: 'markdown' }, + { name: 'dash followed by non-prefix char', pushes: ['-x'], expectedType: 'markdown' }, + { name: 'long dash-led plain text', pushes: ['-this is just text leading dashes'], expectedType: 'markdown' }, + { name: 'leading brace', pushes: ['{'], expectedType: 'json-render' }, + { name: 'leading whitespace then brace', pushes: ['\n {'], expectedType: 'json-render' }, + { name: 'leading whitespace then dash', pushes: [' -'], expectedType: 'pending' }, + { name: 'empty', pushes: [''], expectedType: 'pending' }, + { name: 'whitespace only', pushes: [' \n '], expectedType: 'pending' }, +]; + +describe('ContentClassifier — input variance', () => { + it.each(rows)('$name', (row) => { + TestBed.configureTestingModule({}); + let type!: ContentType; + TestBed.runInInjectionContext(() => { + const c = createContentClassifier(); + let accumulated = ''; + for (const chunk of row.pushes) { + accumulated += chunk; + c.update(accumulated); + } + type = c.type(); + }); + expect(type).toBe(row.expectedType); + }); +}); diff --git a/libs/chat/src/lib/streaming/streaming-markdown.variants.spec.ts b/libs/chat/src/lib/streaming/streaming-markdown.variants.spec.ts new file mode 100644 index 000000000..a1b82604b --- /dev/null +++ b/libs/chat/src/lib/streaming/streaming-markdown.variants.spec.ts @@ -0,0 +1,89 @@ +// SPDX-License-Identifier: MIT +import { describe, it, expect } from 'vitest'; +import { TestBed } from '@angular/core/testing'; +import { Component, signal } from '@angular/core'; +import { ChatStreamingMdComponent } from './streaming-markdown.component'; + +@Component({ + standalone: true, + imports: [ChatStreamingMdComponent], + template: ``, +}) +class HostComponent { + content = signal(''); + streaming = signal(false); +} + +interface FinalizedRow { + name: string; + input: string; + /** Expected concatenated textContent of the rendered root, trimmed and collapsed-whitespace. */ + expectedText: string; + /** Optional CSS selector that must match at least once in the rendered DOM. */ + selectorPresent?: string; + /** Optional CSS selector that must NOT match. */ + selectorAbsent?: string; +} + +const finalizedRows: FinalizedRow[] = [ + { name: 'plain text no trailing newline', input: 'Hello', expectedText: 'Hello', selectorPresent: 'p' }, + { name: 'plain text with trailing newline', input: 'Hello\n', expectedText: 'Hello', selectorPresent: 'p' }, + { name: 'heading no trailing newline', input: '# Title', expectedText: 'Title', selectorPresent: 'h1' }, + { name: 'heading with trailing newline', input: '# Title\n', expectedText: 'Title', selectorPresent: 'h1' }, + { name: 'completed bold', input: '**bold**', expectedText: 'bold', selectorPresent: 'strong' }, + { name: 'inline code', input: 'Run `npm test` to verify', expectedText: 'Run npm test to verify', selectorPresent: 'code' }, + { name: 'CRLF line endings', input: 'Line one\r\nLine two\r\n', expectedText: 'Line one Line two' }, + { name: 'whitespace only', input: ' ', expectedText: '' }, + { name: 'empty string', input: '', expectedText: '', selectorAbsent: 'p' }, + { name: 'trailing whitespace no newline', input: 'Answer ', expectedText: 'Answer', selectorPresent: 'p' }, +]; + +function normalize(s: string): string { + return s.replace(/\s+/g, ' ').trim(); +} + +describe('ChatStreamingMdComponent — finalized input variance', () => { + it.each(finalizedRows)('$name', (row) => { + TestBed.configureTestingModule({ imports: [HostComponent] }); + const fixture = TestBed.createComponent(HostComponent); + fixture.componentInstance.content.set(row.input); + fixture.componentInstance.streaming.set(false); + fixture.detectChanges(); + expect(normalize(fixture.nativeElement.textContent ?? '')).toBe(row.expectedText); + if (row.selectorPresent) { + expect(fixture.nativeElement.querySelector(row.selectorPresent)).toBeTruthy(); + } + if (row.selectorAbsent) { + expect(fixture.nativeElement.querySelector(row.selectorAbsent)).toBeNull(); + } + }); +}); + +interface MidStreamRow { + name: string; + /** Content pushed while streaming=true. */ + midStream: string; + /** Content pushed when streaming flips to false. Defaults to midStream. */ + onFinish?: string; + expectedText: string; +} + +const midStreamRows: MidStreamRow[] = [ + { name: 'partial bold mid-stream then unchanged', midStream: '**bo', expectedText: '**bo' }, + { name: 'partial bold mid-stream then completed', midStream: '**bo', onFinish: '**bold**', expectedText: 'bold' }, + { name: 'unfinished sentence then finalized', midStream: 'The quick', onFinish: 'The quick brown fox.', expectedText: 'The quick brown fox.' }, +]; + +describe('ChatStreamingMdComponent — mid-stream input variance', () => { + it.each(midStreamRows)('$name', (row) => { + TestBed.configureTestingModule({ imports: [HostComponent] }); + const fixture = TestBed.createComponent(HostComponent); + fixture.componentInstance.content.set(row.midStream); + fixture.componentInstance.streaming.set(true); + fixture.detectChanges(); + fixture.componentInstance.content.set(row.onFinish ?? row.midStream); + fixture.componentInstance.streaming.set(false); + fixture.detectChanges(); + expect(normalize(fixture.nativeElement.textContent ?? '')).toBe(row.expectedText); + }); +});