Skip to content

Commit 4f43615

Browse files
jackfranklindevtools-frontend-scoped@luci-project-accounts.iam.gserviceaccount.com
authored andcommitted
AI: improve link handling for LH report
I noticed that, despite instructions, the AI often responds with: ``` [foo](#1,HTML,DIV) ``` Rather than: ``` [foo](#path-1,HTML,DIV) ``` This CL deals with that case by extending the parsing to support it. R=kimanh@chromium.org Bug: 489620101 Change-Id: I1ff87385004065711ed1f7bab98c3556b2cf1f87 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/7762035 Reviewed-by: Kim-Anh Tran <kimanh@chromium.org> Auto-Submit: Jack Franklin <jacktfranklin@chromium.org> Commit-Queue: Jack Franklin <jacktfranklin@chromium.org>
1 parent 9c972c7 commit 4f43615

File tree

2 files changed

+114
-14
lines changed

2 files changed

+114
-14
lines changed

front_end/panels/ai_assistance/components/AccessibilityAgentMarkdownRenderer.test.ts

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,5 +118,64 @@ describeWithEnvironment('AccessibilityAgentMarkdownRenderer', () => {
118118
sinon.assert.calledOnce(linkifyStub);
119119
assert.include(el.textContent, 'LINKIFIED_PATH');
120120
});
121+
122+
it('linkifies paths using #1,HTML (without #path- prefix)', async () => {
123+
const targetManager = SDK.TargetManager.TargetManager.instance();
124+
const mockDomModel = sinon.createStubInstance(SDK.DOMModel.DOMModel);
125+
126+
const mockTarget = {
127+
model: (modelClass: unknown) => {
128+
if (modelClass === SDK.DOMModel.DOMModel) {
129+
return mockDomModel;
130+
}
131+
return null;
132+
},
133+
} as unknown as SDK.Target.Target;
134+
135+
sinon.stub(targetManager, 'primaryPageTarget').returns(mockTarget);
136+
137+
const mockNode = {
138+
frameId: () => '',
139+
} as SDK.DOMModel.DOMNode;
140+
141+
mockDomModel.pushNodeByPathToFrontend.resolves(42 as Protocol.DOM.NodeId);
142+
mockDomModel.nodeForId.returns(mockNode);
143+
144+
const linkifyStub = sinon.stub(PanelsCommon.DOMLinkifier.Linkifier.instance(), 'linkify')
145+
.returns(html`<span>LINKIFIED_PATH</span>`);
146+
147+
const el = renderToElem('[text](#1,HTML,1,BODY)');
148+
149+
await new Promise(resolve => setTimeout(resolve, 0));
150+
151+
sinon.assert.calledOnce(mockDomModel.pushNodeByPathToFrontend);
152+
sinon.assert.calledWith(mockDomModel.pushNodeByPathToFrontend, '1,HTML,1,BODY');
153+
sinon.assert.calledOnce(linkifyStub);
154+
assert.include(el.textContent, 'LINKIFIED_PATH');
155+
});
156+
157+
it('does not linkify non-integer node IDs', async () => {
158+
const targetManager = SDK.TargetManager.TargetManager.instance();
159+
const mockDomModel = sinon.createStubInstance(SDK.DOMModel.DOMModel);
160+
161+
const mockTarget = {
162+
model: (modelClass: unknown) => {
163+
if (modelClass === SDK.DOMModel.DOMModel) {
164+
return mockDomModel;
165+
}
166+
return null;
167+
},
168+
} as unknown as SDK.Target.Target;
169+
170+
sinon.stub(targetManager, 'primaryPageTarget').returns(mockTarget);
171+
172+
const el = renderToElem('[text](#node-1.5)');
173+
174+
await new Promise(resolve => setTimeout(resolve, 0));
175+
176+
sinon.assert.notCalled(mockDomModel.pushNodesByBackendIdsToFrontend);
177+
assert.include(el.textContent, 'text');
178+
assert.notInclude(el.textContent, 'LINKIFIED');
179+
});
121180
});
122181
});

front_end/panels/ai_assistance/components/AccessibilityAgentMarkdownRenderer.ts

Lines changed: 55 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,18 @@ import {MarkdownRendererWithCodeBlock} from './MarkdownRendererWithCodeBlock.js'
1313
const {html} = Lit.StaticHtml;
1414
const {until} = Lit.Directives;
1515

16+
/**
17+
* Represents the different types of links that can be parsed from the AI agent's response.
18+
* The agent can linkify a node either by its backend node ID or by its full DOM path.
19+
*/
20+
type ParsedLink = {
21+
type: 'path',
22+
path: string,
23+
}|{
24+
type: 'node',
25+
nodeId: Protocol.DOM.BackendNodeId,
26+
};
27+
1628
export class AccessibilityAgentMarkdownRenderer extends MarkdownRendererWithCodeBlock {
1729
constructor(
1830
private mainFrameId = '',
@@ -22,28 +34,54 @@ export class AccessibilityAgentMarkdownRenderer extends MarkdownRendererWithCode
2234

2335
override templateForToken(token: Marked.Marked.MarkedToken): Lit.LitTemplate|null {
2436
if (token.type === 'link' && token.href.startsWith('#')) {
25-
if (token.href.startsWith('#path-')) {
26-
const path = token.href.replace('#path-', '');
27-
return html`<span>${
28-
until(this.#linkifyPath(path, token.text).then(node => node || token.text), token.text)}</span>`;
29-
}
37+
const parsed = this.#parseLink(token.href);
38+
if (parsed) {
39+
const resultPromise = parsed.type === 'path' ? this.#linkifyPath(parsed.path, token.text) :
40+
this.#linkifyNode(parsed.nodeId, token.text);
3041

31-
let nodeId = undefined;
32-
if (token.href.startsWith('#node-')) {
33-
nodeId = Number(token.href.replace('#node-', '')) as Protocol.DOM.BackendNodeId;
34-
} else if (token.href.startsWith('#')) {
35-
nodeId = Number(token.href.replace('#', '')) as Protocol.DOM.BackendNodeId;
42+
return html`<span>${until(resultPromise.then(node => node || token.text), token.text)}</span>`;
3643
}
44+
}
45+
46+
return super.templateForToken(token);
47+
}
3748

38-
if (nodeId) {
39-
return html`<span>${
40-
until(this.#linkifyNode(nodeId, token.text).then(node => node || token.text), token.text)}</span>`;
49+
/**
50+
* Parses a link href to determine if it's a node ID or a DOM path.
51+
*
52+
* The AI agent is instructed to use #node-ID or #path-PATH, but
53+
* sometimes it omits the prefixes, in which case we try to detect
54+
* paths by looking for `#1,HTML` which is often how paths in LH
55+
* start.
56+
*/
57+
#parseLink(href: string): ParsedLink|null {
58+
if (href.startsWith('#path-')) {
59+
return {type: 'path', path: href.replace('#path-', '')};
60+
}
61+
if (href.startsWith('#1,HTML')) {
62+
return {type: 'path', path: href.slice(1)};
63+
}
64+
65+
let nodeIdStr = '';
66+
if (href.startsWith('#node-')) {
67+
nodeIdStr = href.replace('#node-', '');
68+
} else if (href.startsWith('#')) {
69+
nodeIdStr = href.slice(1);
70+
}
71+
72+
if (nodeIdStr.trim() !== '') {
73+
const nodeId = Number(nodeIdStr);
74+
if (Number.isInteger(nodeId)) {
75+
return {type: 'node', nodeId: nodeId as Protocol.DOM.BackendNodeId};
4176
}
4277
}
4378

44-
return super.templateForToken(token);
79+
return null;
4580
}
4681

82+
/**
83+
* Linkifies a node using its backend node ID.
84+
*/
4785
async #linkifyNode(backendNodeId: Protocol.DOM.BackendNodeId, label: string): Promise<Lit.LitTemplate|undefined> {
4886
if (backendNodeId === undefined) {
4987
return;
@@ -68,6 +106,9 @@ export class AccessibilityAgentMarkdownRenderer extends MarkdownRendererWithCode
68106
return linkedNode;
69107
}
70108

109+
/**
110+
* Linkifies a node using its full DOM path (e.g. "1,HTML,1,BODY,...").
111+
*/
71112
async #linkifyPath(path: string, label: string): Promise<Lit.LitTemplate|undefined> {
72113
const target = SDK.TargetManager.TargetManager.instance().primaryPageTarget();
73114
const domModel = target?.model(SDK.DOMModel.DOMModel);

0 commit comments

Comments
 (0)