Skip to content

Commit 6dca59e

Browse files
authored
Grammar partial improvements (#1749)
- Capture completion terms after wildcard - Add grammar rule for PlayTrack action - Shell live test: show all the agent message so far if there is an error.
1 parent 043cf7d commit 6dca59e

File tree

3 files changed

+63
-28
lines changed

3 files changed

+63
-28
lines changed

ts/packages/actionGrammar/src/grammarMatcher.ts

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -256,19 +256,17 @@ function nextNonSeparatorIndex(request: string, index: number) {
256256
return match === null ? index : index + match[0].length;
257257
}
258258

259-
function finalizeMatch(
260-
state: MatchState,
261-
request: string,
262-
results: GrammarMatchResult[],
263-
) {
259+
// Finalize the state to make capture the last wildcard if any
260+
// and make sure there are any trailing un-matched non-separator characters.
261+
function finalizeState(state: MatchState, request: string) {
264262
if (state.pendingWildcard !== undefined) {
265263
const value = getWildcardStr(
266264
request,
267265
state.pendingWildcard.start,
268266
request.length,
269267
);
270268
if (value === undefined) {
271-
return;
269+
return false;
272270
}
273271
state.index = request.length;
274272
addValueWithId(state, state.pendingWildcard.valueId, value, true);
@@ -282,13 +280,24 @@ function finalizeMatch(
282280
nonSepIndex,
283281
)}`,
284282
);
285-
return;
283+
return false;
286284
}
287285

288286
debugMatch(
289287
`Consume trailing separators at ${state.index} to ${request.length}}`,
290288
);
291289
}
290+
return true;
291+
}
292+
293+
function finalizeMatch(
294+
state: MatchState,
295+
request: string,
296+
results: GrammarMatchResult[],
297+
) {
298+
if (!finalizeState(state, request)) {
299+
return;
300+
}
292301
debugMatch(
293302
`Matched at end of input. Matched ids: ${JSON.stringify(state.valueIds)}, values: ${JSON.stringify(state.values)}'`,
294303
);
@@ -640,12 +649,7 @@ function partialMatchRules(
640649
`Start state ${state.name}{${state.partIndex}}: @${state.index}`,
641650
);
642651
if (!matchState(state, request, pending)) {
643-
const nonSepIndex = nextNonSeparatorIndex(request, state.index);
644-
if (nonSepIndex !== request.length) {
645-
// There are not matched non-separator characters left
646-
debugCompletion(
647-
` Rejecting completion at ${state.name} with non-separator text at ${nonSepIndex}`,
648-
);
652+
if (!finalizeState(state, request)) {
649653
continue;
650654
}
651655
const nextPart = state.rule.parts[state.partIndex];

ts/packages/agents/player/src/agent/playerGrammar.agr

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
| <PlayCommand>
77
@ <Pause> = pause ((the)? music)? -> { actionName: "pause" }
88
@ <Resume> = resume ((the)? music)? -> { actionName: "resume" }
9-
@ <PlayCommand> = <PlayTrackNumberCommand>
9+
@ <PlayCommand> = <PlayTrackNumberCommand> | <PlaySpecificTrack>
1010
@ <PlayTrackNumberCommand> =
1111
play (the)? $(n:<Ordinal>) (<Item>)? -> {
1212
actionName: "playFromCurrentTrackList",
@@ -26,16 +26,35 @@
2626
trackNumber: $(n)
2727
}
2828
}
29-
@ <Item> = one | track | song | cut
29+
30+
@ <Item> = one | cut | <TrackTerm>
31+
@ <TrackTerm> = track | song
32+
3033
@ <Cardinal> =
31-
one -> 1
34+
$(x:number)
35+
| one -> 1
3236
| two -> 2
3337
| three -> 3
3438
| four -> 4
35-
@ <Ordinal> =
39+
40+
41+
@ <Ordinal> =
3642
first -> 1
3743
| second -> 2
3844
| third -> 3
3945
| troisiemme -> 3
40-
| fourth -> 4
41-
46+
| fourth -> 4
47+
48+
@ <TrackPhrase> = ((the)? <TrackTerm>)? $(trackName:<TrackName>)
49+
| $(trackName:<TrackName>) <TrackTerm>
50+
@ <PlaySpecificTrack> = play $(trackName:<TrackPhrase>) -> { actionName: "playTrack", parameters: { trackName: $(trackName) } }
51+
| play $(trackName:<TrackPhrase>) by $(artist:<ArtistName>) -> { actionName: "playTrack", parameters: { trackName: $(trackName), artists: [$(artist)] } }
52+
| play $(trackName:<TrackPhrase>) from (the)? album $(albumName:<AlbumName>) -> { actionName: "playTrack", parameters: { trackName: $(trackName), albumName: $(albumName) } }
53+
| play $(trackName:<TrackPhrase>) by $(artist:<ArtistName>) from (the)? album $(albumName:<AlbumName>) -> { actionName: "playTrack", parameters: { trackName: $(trackName), artists: [$(artist)], albumName: $(albumName) } }
54+
55+
// TODO: wire up entity names
56+
@ <TrackName> = $(x:string)
57+
@ <ArtistName> = $(x:string)
58+
@ <AlbumName> = $(x:string)
59+
60+

ts/packages/shell/test/testHelper.ts

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,10 @@ export async function sendUserRequestFast(prompt: string, page: Page) {
227227
page.keyboard.down("Enter");
228228
}
229229

230+
async function getAgentMessageLocators(page: Page): Promise<Locator[]> {
231+
return page.locator(".chat-message-agent .chat-message-content").all();
232+
}
233+
230234
/**
231235
* Submits a user request to the system via the chat input box and then waits for the first available response
232236
* NOTE: If your expected response changes or you invoke multi-action flow you should be calling
@@ -241,9 +245,7 @@ export async function sendUserRequestAndWaitForResponse(
241245
prompt: string,
242246
page: Page,
243247
): Promise<string> {
244-
const locators: Locator[] = await page
245-
.locator(".chat-message-agent .chat-message-content")
246-
.all();
248+
const locators = await getAgentMessageLocators(page);
247249

248250
// send the user request
249251
await sendUserRequest(prompt, page);
@@ -264,9 +266,7 @@ export async function sendUserRequestAndWaitForCompletion(
264266
prompt: string,
265267
page: Page,
266268
): Promise<string> {
267-
const locators: Locator[] = await page
268-
.locator(".chat-message-agent .chat-message-content")
269-
.all();
269+
const locators = await getAgentMessageLocators(page);
270270

271271
// send the user request
272272
await sendUserRequest(prompt, page);
@@ -415,8 +415,17 @@ export async function runTestCallback(
415415
}
416416
}
417417

418+
export async function getAllAgentMessages(page: Page): Promise<string[]> {
419+
const locators = await getAgentMessageLocators(page);
420+
return (
421+
await Promise.all(
422+
locators.map((locator) => locator.innerText()).reverse(),
423+
)
424+
).filter((msg) => msg.length > 0);
425+
}
426+
418427
/**
419-
* Encapsulates the supplied method within a startup and shutdown of teh
428+
* Encapsulates the supplied method within a startup and shutdown of the
420429
* shell. Test code executes between them.
421430
*/
422431
export async function testUserRequest(
@@ -434,11 +443,14 @@ export async function testUserRequest(
434443
userRequests[i],
435444
mainWindow,
436445
);
437-
438446
// verify expected result
439447
expect(
440448
msg,
441-
`Chat agent didn't respond with the expected message. Request: '${userRequests[i]}', Response: '${expectedResponses[i]}'`,
449+
[
450+
`Chat agent didn't respond with the expected message. Request: '${userRequests[i]}', Response: '${expectedResponses[i]}'`,
451+
`All response so far:`,
452+
...(await getAllAgentMessages(mainWindow)),
453+
].join("\n"),
442454
).toBe(expectedResponses[i]);
443455
}
444456
});

0 commit comments

Comments
 (0)