Skip to content

Commit 11cdf33

Browse files
authored
fix(chat): dispatch table rows through row component (0.0.24) (#197)
* fix(chat): dispatch table rows through component, not children MarkdownTableComponent template used <chat-md-children [parent]="row"> which walked the row's children (cells) directly under <thead>/<tbody>, skipping the <chat-md-table-row> wrapper entirely. Result: bare cells appeared under thead/tbody and the row component (with its IS_HEADER_ROW DI plumbing) was never instantiated, so cells couldn't tell whether to render <th> or <td>. Caught by live browser smoke testing in ~/tmp/ngaf — Chrome inspection showed the parser produced 3 row nodes but the rendered DOM had zero <chat-md-table-row> elements. Fixed by importing MarkdownTableRowComponent and dispatching each row through it directly. Synchronized version bump to 0.0.24 across all 16 @Ngaf libs. * fix(chat): citation marker without URL renders span, not broken anchor Live Chrome smoke caught: when a Pandoc citation def has a bare URL (no <autolink> brackets), mdDefToCitation returns Citation { url: undefined }. Prior template used [href]="r.citation.url ?? null" which Angular serialized as href="" — a broken anchor that renders as a clickable link that goes nowhere. Fix: split the resolved branch into "with URL" (renders <a>) and "without URL" (renders <span class="chat-citation-marker--no-url">). The span shows the bracket marker but is non-interactive, matching the unresolved fallback's contract. Also switched from [href] property binding to [attr.href] for explicit attribute removal when null. Regression test pins both branches. * fix(chat): chat-citations panel surfaces markdown-sidecar citations too Live Chrome smoke caught: when citations come from Pandoc-formatted [^id]: defs in message content (no provider metadata in additional_kwargs.citations), inline markers resolved correctly via the markdown sidecar but the sources panel under the message never rendered — defeating the click-to-source affordance. Fix: ChatCitationsComponent now optionally injects CitationsResolverService and merges markdown-sidecar defs (via mdDefToCitation) into its citations list, with Message.citations taking precedence by id. Behavior matches the same precedence as inline-marker resolution. Two new regression tests pin both behaviors: markdown-only citations appear in the panel; Message.citations wins on id overlap. * fix(chat): task-list checkbox aligns inline with first-paragraph text Live Chrome smoke caught: checkbox rendered on a separate line above the task text because <chat-md-paragraph> > <p> is block-level, pushing the content below the input. Now uses flexbox on the .chat-md-list-item--task container to align checkbox + content baseline, and collapses the inner <p> margin so tight tasks read as a single line. Multi-paragraph task items (sub-lists, additional paragraphs) still flow correctly because flex-wrap is enabled and only direct children flex. * fix(chat): only collapse first-paragraph margin in task items Live Chrome smoke caught: prior fix collapsed margin on ALL paragraphs inside task items, so multi-paragraph content ran together with no visible break (e.g. "write reportCoral reefs are diverse..." running into the next block when the model emitted prose immediately after a task item without a blank line). Refined: only the FIRST paragraph of a task item collapses its margin (so it aligns inline with the checkbox). Subsequent blocks keep normal vertical spacing.
1 parent 6881b2b commit 11cdf33

23 files changed

Lines changed: 188 additions & 31 deletions

File tree

libs/a2ui/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@ngaf/a2ui",
3-
"version": "0.0.23",
3+
"version": "0.0.24",
44
"license": "MIT",
55
"repository": {
66
"type": "git",

libs/ag-ui/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@ngaf/ag-ui",
3-
"version": "0.0.23",
3+
"version": "0.0.24",
44
"peerDependencies": {
55
"@ngaf/chat": "*",
66
"@ngaf/licensing": "*",

libs/chat/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@ngaf/chat",
3-
"version": "0.0.23",
3+
"version": "0.0.24",
44
"exports": {
55
".": {
66
"types": "./index.d.ts",

libs/chat/src/lib/markdown/views/markdown-citation-reference.component.spec.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,4 +47,22 @@ describe('MarkdownCitationReferenceComponent', () => {
4747
expect(a.getAttribute('href')).toBe('https://example.com');
4848
expect(a.textContent).toContain('1');
4949
});
50+
51+
it('renders <span> (not <a>) when citation has no URL — bug #197 regression', () => {
52+
// Live Chrome smoke caught: a Pandoc def with bare URL (no <autolink> brackets)
53+
// produces a Citation with url === undefined. Prior template rendered <a href="">
54+
// which is a broken link. Fix renders <span class="chat-citation-marker--no-url">.
55+
const fixture = TestBed.createComponent(HostComponent);
56+
const svc = fixture.debugElement.injector.get(CitationsResolverService);
57+
svc.message.set({
58+
id: 'm1', role: 'assistant', content: 'x',
59+
citations: [{ id: 'src1', index: 1, title: 'Source title only, no URL' }],
60+
});
61+
fixture.componentInstance.node.set(makeNode('src1', 1, true));
62+
fixture.detectChanges();
63+
expect(fixture.nativeElement.querySelector('a.chat-citation-marker')).toBeNull();
64+
const span = fixture.nativeElement.querySelector('span.chat-citation-marker--no-url');
65+
expect(span).toBeTruthy();
66+
expect(span.textContent).toContain('1');
67+
});
5068
});

libs/chat/src/lib/markdown/views/markdown-citation-reference.component.ts

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,19 @@ import { CitationsResolverService } from '../citations-resolver.service';
1010
changeDetection: ChangeDetectionStrategy.OnPush,
1111
template: `
1212
@if (resolved(); as r) {
13-
<a class="chat-citation-marker"
14-
[href]="r.citation.url ?? null"
15-
[attr.title]="r.citation.snippet ?? r.citation.url ?? null"
16-
target="_blank" rel="noopener noreferrer">
17-
<sup>[{{ node().index }}]</sup>
18-
</a>
13+
@if (r.citation.url; as href) {
14+
<a class="chat-citation-marker"
15+
[attr.href]="href"
16+
[attr.title]="r.citation.snippet ?? href"
17+
target="_blank" rel="noopener noreferrer">
18+
<sup>[{{ node().index }}]</sup>
19+
</a>
20+
} @else {
21+
<span class="chat-citation-marker chat-citation-marker--no-url"
22+
[attr.title]="r.citation.snippet ?? r.citation.title ?? null">
23+
<sup>[{{ node().index }}]</sup>
24+
</span>
25+
}
1926
} @else {
2027
<span class="chat-citation-marker chat-citation-marker--unresolved"
2128
[attr.title]="'No source available'">

libs/chat/src/lib/markdown/views/markdown-table.component.spec.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,4 +61,29 @@ describe('MarkdownTableComponent', () => {
6161
expect(fixture.nativeElement.querySelector('thead')).toBeTruthy();
6262
expect(fixture.nativeElement.querySelector('tbody')).toBeTruthy();
6363
});
64+
65+
it('dispatches each row through chat-md-table-row component', () => {
66+
// Regression: prior impl used <chat-md-children [parent]="row"> which
67+
// walked row.children (cells) directly and skipped the row wrapper. Cells
68+
// appeared bare under <thead>/<tbody>, no <chat-md-table-row> elements
69+
// existed. Live browser smoke caught this; the test below pins the fix.
70+
const fixture = TestBed.createComponent(HostComponent);
71+
fixture.componentInstance.node.set(makeTableNode({
72+
alignments: [null, null],
73+
children: [
74+
{ id: 2, type: 'table-row', status: 'complete', parent: null, index: 0,
75+
isHeader: true, children: [] } as never,
76+
{ id: 3, type: 'table-row', status: 'complete', parent: null, index: 1,
77+
isHeader: false, children: [] } as never,
78+
{ id: 4, type: 'table-row', status: 'complete', parent: null, index: 2,
79+
isHeader: false, children: [] } as never,
80+
],
81+
}));
82+
fixture.detectChanges();
83+
const rows = fixture.nativeElement.querySelectorAll('chat-md-table-row');
84+
expect(rows.length).toBe(3);
85+
// Header row goes in <thead>; body rows in <tbody>.
86+
expect(fixture.nativeElement.querySelectorAll('thead chat-md-table-row').length).toBe(1);
87+
expect(fixture.nativeElement.querySelectorAll('tbody chat-md-table-row').length).toBe(2);
88+
});
6489
});

libs/chat/src/lib/markdown/views/markdown-table.component.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,23 +2,23 @@
22
// SPDX-License-Identifier: MIT
33
import { Component, ChangeDetectionStrategy, input, computed } from '@angular/core';
44
import type { MarkdownTableNode, MarkdownTableRowNode } from '@cacheplane/partial-markdown';
5-
import { MarkdownChildrenComponent } from '../markdown-children.component';
5+
import { MarkdownTableRowComponent } from './markdown-table-row.component';
66

77
@Component({
88
selector: 'chat-md-table',
99
standalone: true,
10-
imports: [MarkdownChildrenComponent],
10+
imports: [MarkdownTableRowComponent],
1111
changeDetection: ChangeDetectionStrategy.OnPush,
1212
template: `
1313
<table class="chat-md-table">
1414
<thead>
1515
@if (headerRow(); as row) {
16-
<chat-md-children [parent]="row" />
16+
<chat-md-table-row [node]="row" />
1717
}
1818
</thead>
1919
<tbody>
2020
@for (row of bodyRows(); track $any(row)) {
21-
<chat-md-children [parent]="row" />
21+
<chat-md-table-row [node]="row" />
2222
}
2323
</tbody>
2424
</table>

libs/chat/src/lib/primitives/chat-citations/chat-citations.component.spec.ts

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,4 +64,62 @@ describe('ChatCitationsComponent', () => {
6464
expect(fixture.nativeElement.querySelector('.custom-card')?.textContent.trim()).toBe('Custom');
6565
expect(fixture.nativeElement.querySelector('.chat-citations-card')).toBeNull();
6666
});
67+
68+
it('merges markdown sidecar citations when resolver is available — bug #197 regression', async () => {
69+
// Live Chrome smoke caught: when citations come from Pandoc-formatted
70+
// [^id]: defs in content (no provider metadata), inline markers resolved
71+
// correctly via the markdown sidecar but the sources panel never rendered.
72+
const { CitationsResolverService } = await import('../../markdown/citations-resolver.service');
73+
@Component({
74+
standalone: true,
75+
imports: [ChatCitationsComponent],
76+
providers: [CitationsResolverService],
77+
template: `<chat-citations [message]="message" />`,
78+
})
79+
class ResolverHost {
80+
message: Message = msg(undefined); // no provider citations
81+
}
82+
const fixture = TestBed.createComponent(ResolverHost);
83+
const resolver = fixture.debugElement.injector.get(CitationsResolverService);
84+
resolver.markdownDefs.set(new Map([
85+
['src1', {
86+
id: 'src1', index: 1, status: 'complete',
87+
children: [
88+
{ id: 1, type: 'text', status: 'complete', parent: null, index: 0, text: 'Wikipedia ' },
89+
{ id: 2, type: 'autolink', status: 'complete', parent: null, index: 1,
90+
url: 'https://en.wikipedia.org/wiki/Coral_reef',
91+
text: 'https://en.wikipedia.org/wiki/Coral_reef' },
92+
],
93+
} as never],
94+
]));
95+
fixture.detectChanges();
96+
const cards = fixture.nativeElement.querySelectorAll('.chat-citations-card');
97+
expect(cards.length).toBe(1);
98+
expect(fixture.nativeElement.textContent).toContain('Wikipedia');
99+
});
100+
101+
it('Message.citations takes precedence over markdown sidecar when ids overlap', async () => {
102+
const { CitationsResolverService } = await import('../../markdown/citations-resolver.service');
103+
@Component({
104+
standalone: true,
105+
imports: [ChatCitationsComponent],
106+
providers: [CitationsResolverService],
107+
template: `<chat-citations [message]="message" />`,
108+
})
109+
class PrecedenceHost {
110+
message: Message = msg([{ id: 'src1', index: 1, title: 'From message' }]);
111+
}
112+
const fixture = TestBed.createComponent(PrecedenceHost);
113+
const resolver = fixture.debugElement.injector.get(CitationsResolverService);
114+
resolver.markdownDefs.set(new Map([
115+
['src1', { id: 'src1', index: 1, status: 'complete',
116+
children: [{ id: 1, type: 'text', status: 'complete', parent: null, index: 0, text: 'From markdown' }],
117+
} as never],
118+
]));
119+
fixture.detectChanges();
120+
const cards = fixture.nativeElement.querySelectorAll('.chat-citations-card');
121+
expect(cards.length).toBe(1);
122+
expect(fixture.nativeElement.textContent).toContain('From message');
123+
expect(fixture.nativeElement.textContent).not.toContain('From markdown');
124+
});
67125
});

libs/chat/src/lib/primitives/chat-citations/chat-citations.component.ts

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { NgTemplateOutlet } from '@angular/common';
88
import type { Message } from '../../agent/message';
99
import type { Citation } from '../../agent/citation';
1010
import { ChatCitationsCardComponent } from './chat-citations-card.component';
11+
import { CitationsResolverService, mdDefToCitation } from '../../markdown/citations-resolver.service';
1112

1213
/**
1314
* ContentChild template directive for custom citation card rendering.
@@ -48,8 +49,33 @@ export class ChatCitationsComponent {
4849

4950
@ContentChild(ChatCitationCardTemplateDirective) cardTpl: ChatCitationCardTemplateDirective | null = null;
5051

52+
/**
53+
* Optional resolver — present when chat-citations is rendered inside a
54+
* chat-message that provides CitationsResolverService (the standard
55+
* placement). When absent, the panel reads only Message.citations.
56+
*/
57+
private readonly resolver = inject(CitationsResolverService, { optional: true });
58+
59+
/**
60+
* Combined citation list:
61+
* 1. Message.citations (provider-populated, takes precedence by id)
62+
* 2. Markdown sidecar defs (Pandoc-formatted [^id]: lines), merged in
63+
* for any id not already present.
64+
*
65+
* Sorted by index ascending. This guarantees the sources panel surfaces
66+
* citations whether they come from message metadata, content syntax, or
67+
* both — matching the same precedence as inline-marker resolution.
68+
*/
5169
protected readonly citations = computed<Citation[]>(() => {
52-
const list = this.message().citations ?? [];
53-
return [...list].sort((a, b) => a.index - b.index);
70+
const fromMessage = this.message().citations ?? [];
71+
const seenIds = new Set(fromMessage.map(c => c.id));
72+
const fromMarkdown: Citation[] = [];
73+
const mdDefs = this.resolver?.markdownDefs();
74+
if (mdDefs) {
75+
for (const def of mdDefs.values()) {
76+
if (!seenIds.has(def.id)) fromMarkdown.push(mdDefToCitation(def));
77+
}
78+
}
79+
return [...fromMessage, ...fromMarkdown].sort((a, b) => a.index - b.index);
5480
});
5581
}

libs/chat/src/lib/styles/chat-markdown.styles.ts

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,9 +114,32 @@ export const CHAT_MARKDOWN_STYLES = `
114114
chat-streaming-md chat-md-table { display: contents; }
115115
chat-streaming-md chat-md-table-row { display: contents; }
116116
chat-streaming-md chat-md-table-cell { display: contents; }
117-
/* Task-list items */
118-
chat-streaming-md li.chat-md-list-item--task { list-style: none; margin-left: -1.25rem; }
119-
chat-streaming-md li.chat-md-list-item--task > input[type="checkbox"] { margin-right: 0.5rem; vertical-align: middle; }
117+
/* Task-list items: checkbox + first paragraph render inline; subsequent
118+
blocks (sub-lists, multi-paragraph items) flow normally below. */
119+
chat-streaming-md li.chat-md-list-item--task {
120+
list-style: none;
121+
margin-left: -1.25rem;
122+
display: flex;
123+
flex-wrap: wrap;
124+
align-items: baseline;
125+
gap: 0.5rem;
126+
}
127+
chat-streaming-md li.chat-md-list-item--task > input[type="checkbox"] {
128+
margin: 0;
129+
flex: 0 0 auto;
130+
transform: translateY(2px);
131+
}
132+
/* The chat-md-children wrapper around list-item content takes remaining width */
133+
chat-streaming-md li.chat-md-list-item--task > chat-md-children {
134+
flex: 1 1 auto;
135+
min-width: 0;
136+
}
137+
/* Tight task items: only the FIRST paragraph aligns inline with the
138+
checkbox (margin collapsed). Subsequent paragraphs/blocks keep their
139+
normal vertical spacing so multi-block items render readably. */
140+
chat-streaming-md li.chat-md-list-item--task > chat-md-children > chat-md-paragraph:first-child > p {
141+
margin: 0;
142+
}
120143
121144
/* Media */
122145
chat-streaming-md img { max-width: 100%; height: auto; border-radius: 6px; }

0 commit comments

Comments
 (0)