From 28e2c9a4adc377e7e167ac426447d69e37c0e43c Mon Sep 17 00:00:00 2001 From: Dominic Nguyen Date: Thu, 30 Jul 2026 18:49:11 -0700 Subject: [PATCH 1/9] Reject placeholder list names from grammar matches Treat determiner-only listName captures (the/my/list) as invalid via checked_wildcard validation so "add ham to the list" falls through to the LLM instead of creating a list named "the". --- .../agents/list/src/listActionHandler.ts | 47 +++++++++++++----- ts/packages/agents/list/src/listNameUtils.ts | 48 +++++++++++++++++++ ts/packages/agents/list/src/listSchema.agr | 20 ++++---- ts/packages/agents/list/src/listSchema.json | 2 +- ts/packages/agents/list/src/listSchema.ts | 2 + .../list/test/listPlaceholderName.spec.ts | 44 +++++++++++++++++ 6 files changed, 138 insertions(+), 25 deletions(-) create mode 100644 ts/packages/agents/list/src/listNameUtils.ts create mode 100644 ts/packages/agents/list/test/listPlaceholderName.spec.ts diff --git a/ts/packages/agents/list/src/listActionHandler.ts b/ts/packages/agents/list/src/listActionHandler.ts index ac70e6e0dc..fb41e8169d 100644 --- a/ts/packages/agents/list/src/listActionHandler.ts +++ b/ts/packages/agents/list/src/listActionHandler.ts @@ -15,6 +15,9 @@ import { createStructuredResult, } from "@typeagent/agent-sdk/helpers/action"; import { ListAction, ListActivity } from "./listSchema.js"; +import { isPlaceholderListName } from "./listNameUtils.js"; + +export { isPlaceholderListName } from "./listNameUtils.js"; export function instantiate(): AppAgent { return { @@ -90,7 +93,7 @@ function simpleNoun(item: string) { function validateWildcardItems( items: string[], - context: SessionContext, + _context: SessionContext, ) { for (const item of items) { if (!simpleNoun(item)) { @@ -100,19 +103,35 @@ function validateWildcardItems( return true; } +function listNameFromAction( + action: ListAction | ListActivity, +): string | undefined { + if ( + action.actionName === "addItems" || + action.actionName === "removeItems" || + action.actionName === "createList" || + action.actionName === "getList" || + action.actionName === "clearList" || + action.actionName === "startEditList" + ) { + return action.parameters.listName; + } + return undefined; +} + async function listValidateWildcardMatch( action: ListAction | ListActivity, context: SessionContext, ) { + const listName = listNameFromAction(action); + if (listName !== undefined && isPlaceholderListName(listName)) { + return false; + } + if (action.actionName === "addItems") { - const addItemsAction = action; - return validateWildcardItems(addItemsAction.parameters.items, context); + return validateWildcardItems(action.parameters.items, context); } else if (action.actionName === "removeItems") { - const removeItemsAction = action; - return validateWildcardItems( - removeItemsAction.parameters.items, - context, - ); + return validateWildcardItems(action.parameters.items, context); } return true; } @@ -338,8 +357,10 @@ async function handleListAction( if (items.length === 0) { throw new Error("No items to add"); } - if (listName === "") { - throw new Error("List name is empty"); + if (listName === "" || isPlaceholderListName(listName)) { + throw new Error( + "List name is missing or only a reference phrase (e.g. \"the list\"); clarify which list", + ); } store.addItems(listName, items); @@ -361,8 +382,10 @@ async function handleListAction( if (items.length === 0) { throw new Error("No items to remove"); } - if (listName === "") { - throw new Error("List name is empty"); + if (listName === "" || isPlaceholderListName(listName)) { + throw new Error( + "List name is missing or only a reference phrase (e.g. \"the list\"); clarify which list", + ); } store.removeItems(listName, items); diff --git a/ts/packages/agents/list/src/listNameUtils.ts b/ts/packages/agents/list/src/listNameUtils.ts new file mode 100644 index 0000000000..1e93c52e53 --- /dev/null +++ b/ts/packages/agents/list/src/listNameUtils.ts @@ -0,0 +1,48 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +/** + * Determiners / bare "list" captured as listName by listSchema.agr + * ("add ham to the list" → listName="the", "put cheese on the list" → "list"). + * Not a real list identity — reject so grammar falls through to the LLM, + * which can clarify or resolve the active list from history. + */ +export function isPlaceholderListName(listName: string): boolean { + const normalized = listName.trim().toLowerCase(); + if (normalized.length === 0) { + return true; + } + + // Single-token placeholders (incl. possessive determiners + bare "list") + const placeholders = new Set([ + "the", + "a", + "an", + "this", + "that", + "these", + "those", + "my", + "your", + "our", + "his", + "her", + "their", + "list", + ]); + if (placeholders.has(normalized)) { + return true; + } + + // "the list", "my list", "that list", … + const words = normalized.split(/\s+/).filter((w) => w.length > 0); + if ( + words.length === 2 && + words[1] === "list" && + placeholders.has(words[0]!) + ) { + return true; + } + + return false; +} diff --git a/ts/packages/agents/list/src/listSchema.agr b/ts/packages/agents/list/src/listSchema.agr index ad91d44b3b..ffafeca2a8 100644 --- a/ts/packages/agents/list/src/listSchema.agr +++ b/ts/packages/agents/list/src/listSchema.agr @@ -7,18 +7,14 @@ // addItems - add one or more items to a list // -// TODO: these rules bind a bare determiner as the list name — "add ham to the -// list" / "add ham to that list" match here and yield listName="the" / "that" -// instead of letting the LLM clarify which list. As a stopgap the translate -// stability test marks that case with "skipGrammar" (see translate-e2e.json and -// translateTestCommon.ts) so it bypasses grammar matching. Proper fix: constrain -// $(listName:...) to reject bare determiners (the/a/an/this/that/these/those/ -// my/your/... + "list") via a registered entity type, then drop skipGrammar. -// -// Confirmed 2026-07-17: the `put ... on (the)? ... (list)?` alternative below has -// the same bug — "put cheese on the list" yields listName="the". With grammar -// off, the LLM resolves it to the active list from history 10/10. See the schema -// tuning guide (docs/guides/build-an-agent/schema-tuning.md, note #5). +// These rules can still *parse* a bare determiner as listName ("add ham to the +// list" → listName="the"; "put cheese on the list" → listName="list"). That is +// rejected at match time by listValidateWildcardMatch / isPlaceholderListName +// (addItems.listName is checked_wildcard in listSchema.json), so the request +// falls through to the LLM to clarify or resolve the active list from history. +// See docs/guides/build-an-agent/schema-tuning.md note #5. +// Translate e2e may still use skipGrammar for pure-LLM measurement; grammar +// path no longer accepts placeholder list names. = add $(item:wildcard) to (the)? (my)? $(listName:wildcard) list -> { actionName: "addItems", parameters: { diff --git a/ts/packages/agents/list/src/listSchema.json b/ts/packages/agents/list/src/listSchema.json index 4cda638cfe..ed0963120d 100644 --- a/ts/packages/agents/list/src/listSchema.json +++ b/ts/packages/agents/list/src/listSchema.json @@ -2,7 +2,7 @@ "paramSpec": { "addItems": { "items.*": "wildcard", - "listName": "wildcard" + "listName": "checked_wildcard" }, "removeItems": { "items.*": "wildcard", diff --git a/ts/packages/agents/list/src/listSchema.ts b/ts/packages/agents/list/src/listSchema.ts index 5c989c3986..c22e2917a8 100644 --- a/ts/packages/agents/list/src/listSchema.ts +++ b/ts/packages/agents/list/src/listSchema.ts @@ -21,6 +21,8 @@ export type AddItemsAction = { // IMPORTANT: Do not invent a list name. // If the user uses a reference phrase ("the list", "that list",etc.) we should clarify // with the user which list they meant unless it is obvious from the conversation history. + // Grammar may still capture a determiner as listName; the handler rejects those + // placeholders via validateWildcardMatch (checked_wildcard) so the LLM can clarify. listName: string; }; }; diff --git a/ts/packages/agents/list/test/listPlaceholderName.spec.ts b/ts/packages/agents/list/test/listPlaceholderName.spec.ts new file mode 100644 index 0000000000..4a41cb550f --- /dev/null +++ b/ts/packages/agents/list/test/listPlaceholderName.spec.ts @@ -0,0 +1,44 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +/** + * Placeholder listName rejection — grammar can bind "the"/"my"/"list" from + * phrases like "add ham to the list". Those must not be treated as real lists. + */ + +import { isPlaceholderListName } from "../src/listNameUtils.js"; + +describe("isPlaceholderListName", () => { + test.each([ + "the", + "that", + "my", + "a", + "an", + "this", + "these", + "those", + "your", + "list", + "the list", + "my list", + "that list", + "", + " ", + "THE", + "My", + ])("rejects placeholder %j", (name) => { + expect(isPlaceholderListName(name)).toBe(true); + }); + + test.each([ + "grocery", + "shopping", + "Contoso grocery", + "to do", + "q3 packing", + "gift", + ])("accepts real list name %j", (name) => { + expect(isPlaceholderListName(name)).toBe(false); + }); +}); From 49a9c6918bb75c217dd43c184a837197e89aee23 Mon Sep 17 00:00:00 2001 From: typeagent-bot Date: Fri, 31 Jul 2026 02:00:09 +0000 Subject: [PATCH 2/9] style: apply prettier formatting and policy fixes --- ts/packages/agents/list/src/listActionHandler.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ts/packages/agents/list/src/listActionHandler.ts b/ts/packages/agents/list/src/listActionHandler.ts index fb41e8169d..5c8239764b 100644 --- a/ts/packages/agents/list/src/listActionHandler.ts +++ b/ts/packages/agents/list/src/listActionHandler.ts @@ -359,7 +359,7 @@ async function handleListAction( } if (listName === "" || isPlaceholderListName(listName)) { throw new Error( - "List name is missing or only a reference phrase (e.g. \"the list\"); clarify which list", + 'List name is missing or only a reference phrase (e.g. "the list"); clarify which list', ); } @@ -384,7 +384,7 @@ async function handleListAction( } if (listName === "" || isPlaceholderListName(listName)) { throw new Error( - "List name is missing or only a reference phrase (e.g. \"the list\"); clarify which list", + 'List name is missing or only a reference phrase (e.g. "the list"); clarify which list', ); } From e6a85263b856bd48f5e45f3d403a271bf586051b Mon Sep 17 00:00:00 2001 From: Dominic Nguyen Date: Thu, 30 Jul 2026 21:26:59 -0700 Subject: [PATCH 3/9] Harden listName normalize/reject for determiner captures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Normalize: strip true determiners + trailing list(s) only (keep whole foods / active tasks / most wanted intact) - Reject bare closed-class, of-led junk, closed-class "… of them", lots/rest partial anaphora; accept open-class "photos of them" - Execute guards on all listName actions; removeItems throws if missing - Salvage key __recovered__; checked_wildcard on createList - Expand unit coverage for validator, coalesce, and edge names --- .../agents/list/src/listActionHandler.ts | 217 ++++- ts/packages/agents/list/src/listNameUtils.ts | 383 +++++++- ts/packages/agents/list/src/listSchema.json | 2 +- .../list/test/listPlaceholderName.spec.ts | 864 +++++++++++++++++- 4 files changed, 1403 insertions(+), 63 deletions(-) diff --git a/ts/packages/agents/list/src/listActionHandler.ts b/ts/packages/agents/list/src/listActionHandler.ts index 5c8239764b..ca2c44684e 100644 --- a/ts/packages/agents/list/src/listActionHandler.ts +++ b/ts/packages/agents/list/src/listActionHandler.ts @@ -15,10 +15,17 @@ import { createStructuredResult, } from "@typeagent/agent-sdk/helpers/action"; import { ListAction, ListActivity } from "./listSchema.js"; -import { isPlaceholderListName } from "./listNameUtils.js"; - -export { isPlaceholderListName } from "./listNameUtils.js"; - +import { + isPlaceholderListName, + normalizeListName, + RECOVERED_LIST_NAME, +} from "./listNameUtils.js"; + +export { + isPlaceholderListName, + normalizeListName, + RECOVERED_LIST_NAME, +} from "./listNameUtils.js"; export function instantiate(): AppAgent { return { initializeAgentContext: initializeListContext, @@ -119,7 +126,11 @@ function listNameFromAction( return undefined; } -async function listValidateWildcardMatch( +/** + * Reject grammar matches whose listName is only a determiner / "list" + * (after stripping leading dets). Exported for unit tests. + */ +export async function listValidateWildcardMatch( action: ListAction | ListActivity, context: SessionContext, ) { @@ -136,6 +147,17 @@ async function listValidateWildcardMatch( return true; } +/** Normalize listName and reject placeholders at execute time. */ +function requireListName(listName: string): string { + const normalized = normalizeListName(listName); + if (normalized === "" || isPlaceholderListName(normalized)) { + throw new Error( + 'List name is missing or only a reference phrase (e.g. "the list"); clarify which list', + ); + } + return normalized; +} + async function initializeListContext() { return { store: undefined }; } @@ -157,6 +179,134 @@ function createMemoryList(list: List): MemoryList { }; } +/** + * Collapse legacy/raw list records onto normalized keys. + * Placeholder keys ("the", "list", "my", "it", …) are not kept as identities, + * but any items under them are salvaged into RECOVERED_LIST_NAME so hydrate + * never permanently drops user data from the pre-fix failure mode. + * RECOVERED_LIST_NAME itself is a canonical, addressable store key (aliases + * "recovered" / "the recovered list" normalize to it) so a salvage-only store + * is steady-state and does not rewrite on every session load. + * Exported for unit tests. + */ +export function coalesceStoredLists(rawLists: List[]): List[] { + const map = new Map>(); + const salvaged = new Set(); + if (!Array.isArray(rawLists)) { + return []; + } + for (const list of rawLists) { + // Corrupted / hand-edited entries: null name, non-string name, or + // non-object rows. Salvage any items we can still read. + if (list == null || typeof list !== "object") { + continue; + } + const rawName = (list as List).name; + const rawItems = Array.isArray((list as List).items) + ? (list as List).items + : []; + if (typeof rawName !== "string") { + for (const item of rawItems) { + if (typeof item === "string") { + salvaged.add(item); + } + } + continue; + } + const name = normalizeListName(rawName); + if (isPlaceholderListName(name)) { + for (const item of rawItems) { + if (typeof item === "string") { + salvaged.add(item); + } + } + continue; + } + let items = map.get(name); + if (items === undefined) { + items = new Set(); + map.set(name, items); + } + for (const item of rawItems) { + if (typeof item === "string") { + items.add(item); + } + } + } + if (salvaged.size > 0) { + let items = map.get(RECOVERED_LIST_NAME); + if (items === undefined) { + items = new Set(); + map.set(RECOVERED_LIST_NAME, items); + } + for (const item of salvaged) { + items.add(item); + } + } + return Array.from(map.entries()).map(([name, itemsSet]) => ({ + name, + items: Array.from(itemsSet), + })); +} + +/** + * True when raw disk records would change under coalesce (dirty names, merges, + * or placeholder salvage). Used to decide whether to rewrite lists.json on load. + */ +export function storedListsNeedRewrite(rawLists: List[]): boolean { + if (!Array.isArray(rawLists)) { + return true; + } + const coalesced = coalesceStoredLists(rawLists); + if (coalesced.length !== rawLists.length) { + return true; + } + // Build multiset comparison on normalized shape (order-independent names). + const rawByName = new Map>(); + for (const list of rawLists) { + if (list == null || typeof list !== "object") { + return true; + } + if (typeof list.name !== "string") { + return true; + } + // Any non-canonical name on disk must be rewritten. + if (list.name !== normalizeListName(list.name)) { + return true; + } + if (isPlaceholderListName(list.name)) { + return true; + } + if (rawByName.has(list.name)) { + return true; // duplicate keys → merge + } + const itemSet = new Set(); + for (const item of list.items ?? []) { + if (typeof item === "string") { + itemSet.add(item); + } else { + return true; // non-string item → rewrite + } + } + rawByName.set(list.name, itemSet); + } + for (const list of coalesced) { + const rawItems = rawByName.get(list.name); + if (rawItems === undefined) { + return true; + } + if (rawItems.size !== list.items.length) { + return true; + } + for (const item of list.items) { + if (!rawItems.has(item)) { + return true; + } + } + } + return false; +} + class MemoryListCollection { private lists = new Map(); constructor( @@ -164,12 +314,9 @@ class MemoryListCollection { private storage: Storage, private listStoreName: string, ) { - rawLists.forEach((list) => { - const lookupName = list.name; - if (lookupName !== undefined) { - this.lists.set(lookupName, createMemoryList(list)); - } - }); + for (const list of coalesceStoredLists(rawLists)) { + this.lists.set(list.name, createMemoryList(list)); + } } createList(name: string) { @@ -193,10 +340,11 @@ class MemoryListCollection { removeItems(listName: string, items: string[]) { const list = this.getList(listName); - if (list !== undefined) { - for (const item of items) { - list.itemsSet.delete(item); - } + if (list === undefined) { + throw new Error(`List '${listName}' not found`); + } + for (const item of items) { + list.itemsSet.delete(item); } } @@ -234,14 +382,22 @@ async function createListStoreForSession( listStoreName: string, ) { let lists: List[] = []; + let existed = false; // check whether file exists if (await storage.exists(listStoreName)) { + existed = true; const data = await storage.read(listStoreName, "utf8"); lists = JSON.parse(data); } else { await storage.write(listStoreName, JSON.stringify(lists)); } - return new MemoryListCollection(lists, storage, listStoreName); + const store = new MemoryListCollection(lists, storage, listStoreName); + // Persist scrubbed/normalized keys immediately so read-only sessions and + // process exit before a mutate do not leave polluted keys on disk. + if (existed && storedListsNeedRewrite(lists)) { + await store.save(); + } + return store; } async function updateListContext( @@ -353,15 +509,11 @@ async function handleListAction( switch (action.actionName) { case "addItems": { const store = getStore(listContext); - const { items, listName } = action.parameters; + const { items } = action.parameters; + const listName = requireListName(action.parameters.listName); if (items.length === 0) { throw new Error("No items to add"); } - if (listName === "" || isPlaceholderListName(listName)) { - throw new Error( - 'List name is missing or only a reference phrase (e.g. "the list"); clarify which list', - ); - } store.addItems(listName, items); await store.save(); @@ -378,15 +530,11 @@ async function handleListAction( } case "removeItems": { const store = getStore(listContext); - const { items, listName } = action.parameters; + const { items } = action.parameters; + const listName = requireListName(action.parameters.listName); if (items.length === 0) { throw new Error("No items to remove"); } - if (listName === "" || isPlaceholderListName(listName)) { - throw new Error( - 'List name is missing or only a reference phrase (e.g. "the list"); clarify which list', - ); - } store.removeItems(listName, items); await store.save(); @@ -403,7 +551,7 @@ async function handleListAction( } case "createList": { const store = getStore(listContext); - const listName = action.parameters.listName; + const listName = requireListName(action.parameters.listName); if (store.createList(listName)) { displayText = `Created list: ${listName}`; @@ -423,7 +571,8 @@ async function handleListAction( break; } case "getList": { - result = getListDisplay(listContext, action.parameters.listName); + const listName = requireListName(action.parameters.listName); + result = getListDisplay(listContext, listName); break; } case "listLists": { @@ -463,8 +612,7 @@ async function handleListAction( } case "clearList": { const store = getStore(listContext); - const clearListAction = action; - const listName = clearListAction.parameters.listName; + const listName = requireListName(action.parameters.listName); const list = getList(listContext, listName); list.itemsSet.clear(); await store.save(); @@ -477,9 +625,10 @@ async function handleListAction( break; } case "startEditList": { + const listName = requireListName(action.parameters.listName); result = getListDisplay( listContext, - action.parameters.listName, + listName, "What do you want to add or remove from this list?", ); // TODO: formalize the schema for activityContext @@ -487,7 +636,7 @@ async function handleListAction( activityName: "edit", description: "editing list", state: { - listName: action.parameters.listName, + listName, }, }; break; diff --git a/ts/packages/agents/list/src/listNameUtils.ts b/ts/packages/agents/list/src/listNameUtils.ts index 1e93c52e53..a67bf3fcf4 100644 --- a/ts/packages/agents/list/src/listNameUtils.ts +++ b/ts/packages/agents/list/src/listNameUtils.ts @@ -2,44 +2,375 @@ // Licensed under the MIT License. /** - * Determiners / bare "list" captured as listName by listSchema.agr - * ("add ham to the list" → listName="the", "put cheese on the list" → "list"). - * Not a real list identity — reject so grammar falls through to the LLM, - * which can clarify or resolve the active list from history. + * Determiners / bare "list" / anaphora captured as listName by listSchema.agr + * ("add ham to the list" → listName="the", "put cheese on the list" → "list", + * "add milk to a grocery list" → listName="a grocery", + * "add milk to it" → listName="it", + * "create a new list" → listName="new", + * "add eggs to grocery list" → listName="grocery list", + * "add milk to this one" → listName="this one" / "one", + * "clear the other list" → listName="other", + * "add milk to the first one" → listName="first one", + * "clear both lists" → listName="both", + * "add milk to mine" → listName="mine"). + * + * Prefer normalize-then-validate: + * 1. strip token-edge punctuation (LLM/STT artifacts: "list.", "the,") + * and English possessives ("list's") while preserving Unicode letters + * 2. strip leading determiners/quantifiers only (NOT anaphoric pronouns + * me/us/it/you — those can be real multi-word names like "me time") + * 3. strip all trailing generic "list"/"lists" tokens when a real name remains + * 4. casefold so grammar/LLM casing does not split store keys + * 5. map the salvage identity "recovered" → RECOVERED_LIST_NAME + * 6. reject empty / bare-det / bare-"list(s)" / bare-"new" / bare pronoun / + * bare deictic (one/ones/other/same/another/current/…) / quantifiers / + * possessives / ordinal+one leftovers / all-closed-class leftovers / + * partial anaphora ("lots of them", "which of those") + * so real names like "grocery" keep working while "the list" falls through. + */ + +/** Closed-class determiners the grammar may glue onto a real list name. */ +const DETERMINERS = new Set([ + "the", + "a", + "an", + "this", + "that", + "these", + "those", + "my", + "your", + "our", + "his", + "her", + "its", + "their", + // anaphoric / deictic determiners ("another list", "the other list", "the same list") + "another", + "other", + "same", + // interrogative determiner ("which list", "which one") + "which", +]); + +/** + * Quantifiers that never name a list ("both lists", "every list", "any list", + * "all lists", "some list", "each list", "either list", "none of them"). + * Stripped when leading a multi-word capture; rejected when bare. + */ +const QUANTIFIERS = new Set([ + "any", + "all", + "both", + "every", + "each", + "some", + "either", + "neither", + "none", + // underspecified quantity words ("more lists", "many lists", …) + "more", + "many", + "several", + "few", + "most", +]); + +/** + * Light closed-class glue that appears in anaphora leftovers after quantifier + * strip ("all of them" → "of them"). Never a list identity alone or in an + * all-closed-class phrase. + */ +const CLOSED_GLUE = new Set(["of", "for", "to", "with", "from", "on", "in", "at"]); + +/** + * Anaphoric pronouns the grammar/LLM may bind as listName + * ("add milk to it", "put cheese on them", "make me a list" → "me"/"me a", + * subject forms "they"/"we"/"she" from STT/LLM slips). + * Rejected when bare (or all-closed-class), but NOT stripped from multi-word + * names so real lists like "me time" / "us travel" / "it projects" stay intact. + */ +const ANAPHORIC_PRONOUNS = new Set([ + "it", + "them", + "me", + "him", + "us", + "you", + "they", + "we", + "she", + "he", +]); + +/** + * Independent possessives ("add milk to mine", "what's on yours", "clear ours"). + * Never a list identity when bare. + */ +const INDEPENDENT_POSSESSIVES = new Set([ + "mine", + "yours", + "ours", + "theirs", + "hers", + "his", // already a determiner; listed for clarity when bare +]); + +/** + * Ordinal / sequential words used in deictic phrases + * ("first one", "last one", "next one", "the previous one"). + * Rejected when the whole name is closed-class; kept when paired with a real + * noun ("first aid", "next week"). + */ +const ORDINALS = new Set([ + "first", + "second", + "third", + "fourth", + "fifth", + "last", + "next", + "previous", + "former", + "latter", +]); + +/** + * Small cardinals used deictically ("the two", "those three lists"). + * Rejected when bare / all-closed-class; kept inside real names ("two trees"). + */ +const CARDINALS = new Set([ + "two", + "three", + "four", + "five", + "six", + "seven", + "eight", + "nine", + "ten", +]); + +/** + * Deictic leftovers after det strip ("this one", "that one", "those ones", + * "my own") — never a list id. + */ +const DEICTIC_PLACEHOLDERS = new Set(["one", "ones", "own"]); + +/** + * Session/selection deictics ("the current list", "my active list") — never a + * list id when bare / all-closed-class. NOT leading-stripped: real names like + * "whole foods", "active tasks", "current events" must stay intact. + */ +const SELECTION_DEICTICS = new Set([ + "current", + "active", + "default", + "whole", + "entire", +]); + +/** + * Underspecified content heads that only appear in partial anaphora + * ("lots of them", "the rest of those") — never a list identity alone. + */ +const PARTIAL_ANAPHORA_HEADS = new Set(["lots", "rest"]); + +/** + * Tokens stripped from the front of multi-word names when a real name remains. + * TRUE DETERMINERS ONLY. Quantifiers / selection deictics are rejected when + * bare but never peeled off open-class compounds ("most wanted", "whole foods", + * "active tasks", "many thanks"). + */ +const LEADING_STRIP = new Set([...DETERMINERS]); + +/** + * Single-token values that are never a real list identity after normalize. + * Includes bare "new" from CreateList's optional (new)? before the wildcard, + * and bare "list"/"lists" from underspecified "show the lists" paths. + * NOTE: "recovered" is intentionally NOT here — it is the user-facing alias + * of the salvage store key (see normalizeListName). + */ +const PLACEHOLDER_TOKENS = new Set([ + ...DETERMINERS, + ...QUANTIFIERS, + ...ANAPHORIC_PRONOUNS, + ...INDEPENDENT_POSSESSIVES, + ...ORDINALS, + ...CARDINALS, + ...DEICTIC_PLACEHOLDERS, + ...SELECTION_DEICTICS, + ...PARTIAL_ANAPHORA_HEADS, + ...CLOSED_GLUE, + "list", + "lists", + "new", +]); + +/** + * Strip leading/trailing non-letter/non-number junk from a token (keep internal), + * then an English possessive suffix. Unicode-aware so accented / non-Latin names + * (café, résumé, 買い物) survive. + */ +function cleanToken(token: string): string { + return ( + token + .toLowerCase() + .replace(/^[^\p{L}\p{N}]+/gu, "") + .replace(/[^\p{L}\p{N}]+$/gu, "") + // STT/LLM possessives: "list's", "grocery's", curly apostrophe + .replace(/['\u2019]s$/u, "") + ); +} + +function splitWords(listName: string): string[] { + return listName + .trim() + .split(/\s+/) + .map(cleanToken) + .filter((w) => w.length > 0); +} + +/** + * Internal fallback store key when legacy lists.json only has placeholder keys + * ("the", "list", "my", …) that still hold real items. Double-underscore form + * so it cannot collide with a normal user-typed list name. Addressable via + * normalize aliases "recovered" / "the recovered list" / "__recovered__". + */ +export const RECOVERED_LIST_NAME = "__recovered__"; + +/** User-facing / underscore-stripped form of the salvage key. */ +const RECOVERED_ALIAS = "recovered"; + +/** + * Normalize a captured/emitted listName into a stable store key: + * strip edge punctuation + possessives, strip leading true determiners only, + * strip all trailing generic "list"/"lists", casefold, + * map salvage alias → RECOVERED_LIST_NAME. + * Single-token input is not leading-stripped (placeholder check handles bare dets). + * Quantifiers / selection deictics / anaphora are not leading-stripped + * (preserve "whole foods", "active tasks", "most wanted", "me time"). + * Examples: "a grocery" → "grocery", "the grocery list" → "grocery", + * "Grocery Lists" → "grocery", "the." → "the", "grocery list!" → "grocery", + * "grocery list list" → "grocery", "both lists" → "both", "café list" → "café", + * "Contoso grocery" → "contoso grocery", "me time" → "me time", + * "whole foods list" → "whole foods", + * "recovered" / "the recovered list" / "__recovered__" → "__recovered__". + */ +export function normalizeListName(listName: string): string { + const words = splitWords(listName); + let start = 0; + // Only strip true determiners while a real name token would remain. + while (start < words.length - 1 && LEADING_STRIP.has(words[start]!)) { + start++; + } + let end = words.length; + // Trailing generic "list"/"lists" from optional (list)? / LLM phrasing — + // strip ALL of them so normalize is idempotent ("grocery list list"). + // Keep bare "list"/"lists" for the placeholder check. + while (end - start > 1) { + const last = words[end - 1]!; + if (last === "list" || last === "lists") { + end--; + } else { + break; + } + } + const normalized = words.slice(start, end).join(" "); + // Canonical salvage key (cleanToken strips underscores from "__recovered__"). + if (normalized === RECOVERED_ALIAS || normalized === RECOVERED_LIST_NAME) { + return RECOVERED_LIST_NAME; + } + return normalized; +} + +/** + * True when the last two tokens are "of" + anaphora/demonstrative + * ("of them", "of those", "of it"). + */ +function endsWithOfAnaphora(words: string[]): boolean { + if (words.length < 2) { + return false; + } + const last = words[words.length - 1]!; + const prev = words[words.length - 2]!; + if (prev !== "of") { + return false; + } + return ( + ANAPHORIC_PRONOUNS.has(last) || + last === "those" || + last === "these" || + last === "this" || + last === "that" + ); +} + +/** + * Partial anaphora like "all of them" / "lots of those" / "rest of it". + * Rejects only when every token before "of + anaphor" is closed-class, so + * real names like "photos of them" stay valid. + */ +function isClosedClassOfAnaphora(words: string[]): boolean { + if (!endsWithOfAnaphora(words)) { + return false; + } + const head = words.slice(0, -2); + return head.every((w) => PLACEHOLDER_TOKENS.has(w)); +} + +/** + * True when listName is not a usable list identity after normalization + * (empty, bare determiner/pronoun/deictic/quantifier/possessive/ordinal, + * bare "list(s)"/"new", all-closed-class phrases, glue-led leftovers like + * "of grocery", or closed-class "… of them/those"). + * The salvage key RECOVERED_LIST_NAME is a real store identity (addressable). */ export function isPlaceholderListName(listName: string): boolean { - const normalized = listName.trim().toLowerCase(); + const normalized = normalizeListName(listName); if (normalized.length === 0) { return true; } - // Single-token placeholders (incl. possessive determiners + bare "list") - const placeholders = new Set([ - "the", - "a", - "an", - "this", - "that", - "these", - "those", - "my", - "your", - "our", - "his", - "her", - "their", - "list", - ]); - if (placeholders.has(normalized)) { + // Salvage key is a real, addressable store identity — not a placeholder. + if (normalized === RECOVERED_LIST_NAME) { + return false; + } + + if (PLACEHOLDER_TOKENS.has(normalized)) { return true; } - // "the list", "my list", "that list", … const words = normalized.split(/\s+/).filter((w) => w.length > 0); + + // "me a", "first one", "the other", "those ones", "of them" (all closed-class) + if (words.length > 0 && words.every((w) => PLACEHOLDER_TOKENS.has(w))) { + return true; + } + + // Grammar/LLM junk that starts with "of" ("of grocery", "of them") after a + // quantifier peel. Only "of" — not other prepositions ("to do", "on call"). + if (words.length > 0 && words[0] === "of") { + return true; + } + + // "lots of …" / "rest of …" are partial anaphora heads, not list titles + // ("lots of groceries", "rest of the grocery"). + if (words.length > 0 && PARTIAL_ANAPHORA_HEADS.has(words[0]!)) { + return true; + } + + // "all of them", "none of it" — but NOT "photos of them" + if (isClosedClassOfAnaphora(words)) { + return true; + } + + // Defense in depth: "X list(s)" where X is still a closed-class token + // (should already have been reduced by normalize, but keep the check cheap). if ( words.length === 2 && - words[1] === "list" && - placeholders.has(words[0]!) + (words[1] === "list" || words[1] === "lists") && + PLACEHOLDER_TOKENS.has(words[0]!) ) { return true; } diff --git a/ts/packages/agents/list/src/listSchema.json b/ts/packages/agents/list/src/listSchema.json index ed0963120d..665d29d857 100644 --- a/ts/packages/agents/list/src/listSchema.json +++ b/ts/packages/agents/list/src/listSchema.json @@ -9,7 +9,7 @@ "listName": "checked_wildcard" }, "createList": { - "listName": "wildcard" + "listName": "checked_wildcard" }, "getList": { "listName": "checked_wildcard" diff --git a/ts/packages/agents/list/test/listPlaceholderName.spec.ts b/ts/packages/agents/list/test/listPlaceholderName.spec.ts index 4a41cb550f..9b9ec078c6 100644 --- a/ts/packages/agents/list/test/listPlaceholderName.spec.ts +++ b/ts/packages/agents/list/test/listPlaceholderName.spec.ts @@ -3,10 +3,189 @@ /** * Placeholder listName rejection — grammar can bind "the"/"my"/"list" from - * phrases like "add ham to the list". Those must not be treated as real lists. + * phrases like "add ham to the list", or "a grocery" from determiner+name + * compounds, "it"/"them"/"me"/"they"/"we"/"she" from anaphora, "new" from + * CreateList's optional adjective, "grocery list"/"grocery lists" when the + * trailing list token is eaten by the wildcard, deictics like "this one"/ + * "the other list"/"first one"/"those ones"/"the current list", quantifiers + * ("both lists", "every list", "none of them"), independent possessives + * ("mine"), interrogatives ("which list"), partial anaphora ("lots of them"), + * possessives ("list's"), or punctuated LLM/STT artifacts ("the list.", "list?"). + * Normalize strips edge punctuation (Unicode-safe), possessives, leading + * dets/quantifiers/selection deictics (not anaphoric pronouns), all trailing + * "list"/"lists", casefolds, and maps the salvage alias to RECOVERED_LIST_NAME. + * Leftovers that are still placeholders must not be real lists. Real multi-word + * names like "me time" and non-ASCII names like "café" stay intact. The salvage + * key is addressable so recovered items remain reachable. */ -import { isPlaceholderListName } from "../src/listNameUtils.js"; +import { + isPlaceholderListName, + normalizeListName, + RECOVERED_LIST_NAME, +} from "../src/listNameUtils.js"; +import { + coalesceStoredLists, + listValidateWildcardMatch, + storedListsNeedRewrite, +} from "../src/listActionHandler.js"; +import type { ListAction, ListActivity } from "../src/listSchema.js"; +import type { SessionContext } from "@typeagent/agent-sdk"; + +describe("normalizeListName", () => { + test.each([ + ["a grocery", "grocery"], + ["that shopping", "shopping"], + ["your packing", "packing"], + ["the list", "list"], + ["my list", "list"], + ["an idea", "idea"], + [" A Grocery ", "grocery"], + // trailing generic "list" / "lists" (including repeats — idempotent) + ["grocery list", "grocery"], + ["the grocery list", "grocery"], + ["Grocery List", "grocery"], + ["GROCERY LIST", "grocery"], + ["shopping list", "shopping"], + ["to do list", "to do"], + ["grocery lists", "grocery"], + ["Grocery Lists", "grocery"], + ["the grocery lists", "grocery"], + ["grocery list list", "grocery"], + ["shopping lists list", "shopping"], + ["grocery list lists", "grocery"], + ["the grocery list list", "grocery"], + // possessives + ["list's", "list"], + ["the list's", "list"], + ["my list's", "list"], + ["grocery list's", "grocery"], + ["grocery's", "grocery"], + // anaphoric dets + ["another grocery", "grocery"], + ["the other shopping", "shopping"], + ["the same packing", "packing"], + // all-det + trailing list collapses to bare "list" (still a placeholder) + ["another list", "list"], + ["the other list", "list"], + ["the same list", "list"], + ["this one", "one"], + ["that one", "one"], + // selection deictics are NOT leading-stripped (preserve "whole foods"); + // "the current list" → strip det + trailing list → bare "current" + ["the current list", "current"], + ["the active list", "active"], + ["the default list", "default"], + ["the whole list", "whole"], + ["the entire list", "entire"], + ["current", "current"], + ["active", "active"], + ["whole grocery list", "whole grocery"], + ["the whole grocery list", "whole grocery"], + ["whole foods list", "whole foods"], + ["active tasks", "active tasks"], + ["current events", "current events"], + // quantifiers are NOT leading-stripped (preserve "most wanted"); + // trailing list still peels: "both lists" → "both" + ["both lists", "both"], + ["every list", "every"], + ["all lists", "all"], + ["any list", "any"], + ["some list", "some"], + ["each list", "each"], + ["either list", "either"], + ["none of them", "none of them"], + ["both grocery", "both grocery"], + ["every shopping", "every shopping"], + ["most wanted", "most wanted"], + ["many thanks", "many thanks"], + // interrogative det still strips when leading + ["which", "which"], + ["which one", "one"], + ["which list", "list"], + ["which grocery", "grocery"], + // ordinals + one kept as multi-token for all-closed-class reject + ["first one", "first one"], + ["last one", "last one"], + ["next one", "next one"], + ["the previous one", "previous one"], + ["the first one", "first one"], + ["those ones", "ones"], + ["these ones", "ones"], + ["the ones", "ones"], + // leading anaphora is NOT stripped (preserve real names) + ["me a", "me a"], + ["me a new", "me a new"], + ["me time", "me time"], + ["us travel", "us travel"], + ["it projects", "it projects"], + ["you team", "you team"], + ["US travel", "us travel"], + // single-token input is not leading-stripped + ["the", "the"], + ["list", "list"], + ["lists", "lists"], + ["grocery", "grocery"], + ["Grocery", "grocery"], + ["GROCERY", "grocery"], + ["mine", "mine"], + ["both", "both"], + ["they", "they"], + // salvage alias → canonical store key + ["recovered", RECOVERED_LIST_NAME], + ["Recovered", RECOVERED_LIST_NAME], + ["__recovered__", RECOVERED_LIST_NAME], + ["the recovered list", RECOVERED_LIST_NAME], + ["recovered list", RECOVERED_LIST_NAME], + // punctuation (LLM/STT) + ["the.", "the"], + ["list,", "list"], + ["it!", "it"], + ["my?", "my"], + ["the list.", "list"], + ["list?", "list"], + ["my list,", "list"], + ["grocery list!", "grocery"], + ["Grocery Lists.", "grocery"], + // Unicode letters preserved (not ASCII-stripped) + ["café", "café"], + ["Café", "café"], + ["résumé", "résumé"], + ["café list", "café"], + ["the café list", "café"], + ["買い物", "買い物"], + ["買い物 list", "買い物"], + ["café!", "café"], + // non-determiner first token kept (casefolded) + ["Contoso grocery", "contoso grocery"], + ["to do", "to do"], + ["q3 packing", "q3 packing"], + ["New York", "new york"], + ["first aid", "first aid"], + ["next week", "next week"], + ["", ""], + [" ", ""], + ])("normalize %j → %j", (input, expected) => { + expect(normalizeListName(input)).toBe(expected); + }); + + test("normalize is idempotent (including repeated trailing list)", () => { + for (const input of [ + "grocery list list", + "shopping lists list", + "the grocery list", + "a grocery", + "recovered", + "__recovered__", + "the recovered list", + "grocery list's", + "the current list", + ]) { + const once = normalizeListName(input); + expect(normalizeListName(once)).toBe(once); + } + }); +}); describe("isPlaceholderListName", () => { test.each([ @@ -19,14 +198,168 @@ describe("isPlaceholderListName", () => { "these", "those", "your", + "our", + "his", + "her", + "its", + "their", "list", + "lists", "the list", "my list", "that list", + "its list", + "a list", + "the lists", + "my lists", + // possessives + "list's", + "the list's", + "my list's", + // anaphoric pronouns (object + subject forms) + "it", + "them", + "me", + "him", + "us", + "you", + "they", + "we", + "she", + "he", + "IT", + "Them", + "They", + "me a", + // independent possessives + "mine", + "yours", + "ours", + "theirs", + "hers", + "Mine", + "yours list", + // quantifiers + "any", + "all", + "both", + "every", + "each", + "some", + "either", + "neither", + "none", + "more", + "many", + "several", + "few", + "most", + "both lists", + "every list", + "all lists", + "any list", + "some list", + "each list", + "either list", + "more lists", + "many lists", + "none of them", + "none of those", + // quantifier + of + anaphora leftovers / glue-led junk + "of them", + "of those", + "of it", + "of grocery", + "all of them", + "both of them", + "some of it", + "any of these", + "lots of them", + "lots of groceries", + "rest of those", + "rest of the grocery", + "the rest of the grocery list", + // partial anaphora with content head + "lots of them", + "rest of them", + "rest of those", + "which of them", + "lots", + "rest", + // interrogative + "which", + "which one", + "which list", + // selection / session deictics + "current", + "active", + "default", + "whole", + "entire", + "the current list", + "the active list", + "the default list", + "the whole list", + "the entire list", + "current list", + "active list", + // deictic "own" leftover + "own", + "my own", + "my own list", + // small cardinals + "two", + "three", + "the two", + "the three lists", + "those two", + // bare CreateList adjective + "new", + "NEW", + "a new", + "me a new", + // deictic / anaphoric dets + "one", + "ones", + "this one", + "that one", + "those ones", + "these ones", + "the ones", + "other", + "another", + "same", + "the other", + "the other list", + "another list", + "the same list", + "another lists", + // ordinal + one deictics + "first one", + "last one", + "next one", + "previous one", + "the previous one", + "the first one", + "second one", + "the last one", + // punctuation-glued placeholders + "the.", + "list,", + "it!", + "my?", + "the list.", + "list?", + "my list,", + "lists!", + "mine!", + "both lists.", "", " ", "THE", "My", + "ITS", + "Lists", ])("rejects placeholder %j", (name) => { expect(isPlaceholderListName(name)).toBe(true); }); @@ -38,7 +371,534 @@ describe("isPlaceholderListName", () => { "to do", "q3 packing", "gift", + // determiner+name compounds normalize to a real name + "a grocery", + "that shopping", + "your packing", + "my grocery", + "an idea", + "its shopping", + "another grocery", + "the other shopping", + // trailing list(s) collapses to real name + "grocery list", + "the grocery list", + "Grocery List", + "shopping list", + "to do list", + "grocery lists", + "Grocery Lists", + "grocery list!", + "grocery list list", + "grocery list's", + // multi-word with "new" as part of a real name stays usable + "New York", + "new hires", + // ordinal + real noun stays usable + "first aid", + "next week", + "last christmas", + // anaphoric-pronoun-leading real names (not stripped) + "me time", + "us travel", + "it projects", + "you team", + "US travel", + // Unicode / non-ASCII real names + "café", + "résumé", + "café list", + "the café", + "買い物", + "買い物 list", + // selection deictic + real name (not stripped) + "whole grocery", + "the whole grocery list", + "whole foods", + "active tasks", + "current events", + "default settings", + "entire catalog", + // quantifier + open-class compound (not stripped) + "most wanted", + "many thanks", + "few ingredients", + "several projects", + "more errands", + // open-class head + of + anaphor is a real name + "photos of them", + "pictures of those", + // salvage key is addressable (not a placeholder) + "recovered", + "__recovered__", + "the recovered list", + "Recovered", ])("accepts real list name %j", (name) => { expect(isPlaceholderListName(name)).toBe(false); }); }); + +describe("listValidateWildcardMatch", () => { + const ctx = {} as SessionContext; + + function action( + actionName: ListAction["actionName"] | ListActivity["actionName"], + listName: string, + items?: string[], + ): ListAction | ListActivity { + if (actionName === "addItems" || actionName === "removeItems") { + return { + actionName, + parameters: { listName, items: items ?? ["milk"] }, + } as ListAction; + } + if (actionName === "listLists") { + return { actionName: "listLists", parameters: {} }; + } + return { + actionName, + parameters: { listName }, + } as ListAction | ListActivity; + } + + test.each([ + "the", + "my", + "list", + "lists", + "the list", + "the lists", + "its", + "it", + "them", + "me", + "they", + "we", + "she", + "mine", + "yours", + "ours", + "both", + "every", + "all", + "any", + "some", + "none", + "none of them", + "both lists", + "every list", + "new", + "a new", + "one", + "ones", + "this one", + "those ones", + "first one", + "last one", + "the previous one", + "other", + "another", + "same", + "the other list", + "which", + "which one", + "which list", + "lots of them", + "rest of those", + "the current list", + "the active list", + "the default list", + "the whole list", + "the entire list", + "list's", + "the list's", + "the.", + "list?", + "the list.", + "", + " ", + ])("rejects placeholder listName %j on createList", async (name) => { + expect( + await listValidateWildcardMatch(action("createList", name), ctx), + ).toBe(false); + }); + + test.each([ + "the", + "the list", + "my", + "list", + "lists", + "its", + "it", + "them", + "me", + "they", + "mine", + "both", + "any", + "none of them", + "first one", + "those ones", + "another", + "the other list", + "this one", + "which", + "which one", + "the current list", + "list!", + "list's", + ])("rejects placeholder listName %j on addItems", async (name) => { + expect( + await listValidateWildcardMatch(action("addItems", name), ctx), + ).toBe(false); + }); + + test.each(["getList", "clearList", "startEditList", "removeItems"] as const)( + "rejects bare 'the list' on %s", + async (actionName) => { + expect( + await listValidateWildcardMatch( + action(actionName, "the list"), + ctx, + ), + ).toBe(false); + }, + ); + + test.each([ + "grocery", + "a grocery", + "that shopping", + "your packing", + "Contoso grocery", + "to do", + "q3 packing", + "grocery list", + "the grocery list", + "grocery lists", + "grocery list list", + "Grocery", + "GROCERY LIST", + "grocery list!", + "grocery list's", + "me time", + "us travel", + "café", + "買い物", + "first aid", + // salvage aliases must pass wildcard so users can address recovered items + "recovered", + "__recovered__", + "the recovered list", + ])("accepts usable listName %j on createList", async (name) => { + expect( + await listValidateWildcardMatch(action("createList", name), ctx), + ).toBe(true); + }); + + test.each([ + "getList", + "clearList", + "startEditList", + "addItems", + "removeItems", + ] as const)( + "accepts salvage aliases on %s so recovered items are reachable", + async (actionName) => { + for (const name of [ + "recovered", + "__recovered__", + "the recovered list", + ]) { + expect( + await listValidateWildcardMatch( + action(actionName, name), + ctx, + ), + ).toBe(true); + } + }, + ); + + test("accepts determiner+name compound on addItems", async () => { + expect( + await listValidateWildcardMatch( + action("addItems", "a grocery", ["eggs"]), + ctx, + ), + ).toBe(true); + }); + + test("accepts trailing-list form on addItems (grammar without literal list)", async () => { + expect( + await listValidateWildcardMatch( + action("addItems", "grocery list", ["ham"]), + ctx, + ), + ).toBe(true); + expect( + await listValidateWildcardMatch( + action("addItems", "the grocery list", ["milk", "eggs"]), + ctx, + ), + ).toBe(true); + expect( + await listValidateWildcardMatch( + action("addItems", "grocery lists", ["bread"]), + ctx, + ), + ).toBe(true); + expect( + await listValidateWildcardMatch( + action("addItems", "grocery list list", ["butter"]), + ctx, + ), + ).toBe(true); + }); + + test("still rejects non-simple item nouns on addItems", async () => { + expect( + await listValidateWildcardMatch( + action("addItems", "grocery", ["the big red apple"]), + ctx, + ), + ).toBe(false); + }); +}); + +describe("coalesceStoredLists (session hydrate)", () => { + test("salvages items under placeholder keys into recovered list", () => { + expect( + coalesceStoredLists([ + { name: "the", items: ["x"] }, + { name: "list", items: ["y"] }, + { name: "my", items: ["z"] }, + { name: "new", items: ["n"] }, + { name: "it", items: ["i"] }, + { name: "the list", items: ["t"] }, + { name: "mine", items: ["m"] }, + { name: "both", items: ["b"] }, + ]), + ).toEqual([ + { + name: RECOVERED_LIST_NAME, + items: ["x", "y", "z", "n", "i", "t", "m", "b"], + }, + ]); + }); + + test("salvages placeholder items alongside real lists", () => { + const result = coalesceStoredLists([ + { name: "the", items: ["lost"] }, + { name: "grocery", items: ["milk"] }, + ]); + expect(result).toEqual( + expect.arrayContaining([ + { name: "grocery", items: ["milk"] }, + { name: RECOVERED_LIST_NAME, items: ["lost"] }, + ]), + ); + expect(result).toHaveLength(2); + }); + + test("does not create recovered list when no placeholder items", () => { + expect( + coalesceStoredLists([ + { name: "grocery", items: ["milk"] }, + ]), + ).toEqual([{ name: "grocery", items: ["milk"] }]); + }); + + test("keeps already-canonical salvage list stable (no re-salvage loop)", () => { + expect( + coalesceStoredLists([ + { name: RECOVERED_LIST_NAME, items: ["milk"] }, + ]), + ).toEqual([{ name: RECOVERED_LIST_NAME, items: ["milk"] }]); + // alias forms also land on the same key + expect( + coalesceStoredLists([{ name: "recovered", items: ["eggs"] }]), + ).toEqual([{ name: RECOVERED_LIST_NAME, items: ["eggs"] }]); + }); + + test("normalizes determiner-prefixed and trailing-list keys", () => { + expect( + coalesceStoredLists([ + { name: "a grocery", items: ["eggs"] }, + { name: "Grocery List", items: ["bread"] }, + { name: "the grocery list", items: ["milk"] }, + { name: "grocery lists", items: ["butter"] }, + { name: "grocery list list", items: ["cheese"] }, + ]), + ).toEqual([ + { + name: "grocery", + items: ["eggs", "bread", "milk", "butter", "cheese"], + }, + ]); + }); + + test("casefolds so mixed-case keys merge", () => { + expect( + coalesceStoredLists([ + { name: "Grocery", items: ["a"] }, + { name: "grocery", items: ["b"] }, + { name: "GROCERY", items: ["c"] }, + ]), + ).toEqual([{ name: "grocery", items: ["a", "b", "c"] }]); + }); + + test("keeps distinct real names including anaphora-leading and Unicode", () => { + expect( + coalesceStoredLists([ + { name: "shopping", items: ["soap"] }, + { name: "to do", items: ["call"] }, + { name: "Contoso grocery", items: ["badge"] }, + { name: "me time", items: ["read"] }, + { name: "US travel", items: ["visa"] }, + { name: "café", items: ["latte"] }, + { name: "買い物", items: ["tea"] }, + ]), + ).toEqual([ + { name: "shopping", items: ["soap"] }, + { name: "to do", items: ["call"] }, + { name: "contoso grocery", items: ["badge"] }, + { name: "me time", items: ["read"] }, + { name: "us travel", items: ["visa"] }, + { name: "café", items: ["latte"] }, + { name: "買い物", items: ["tea"] }, + ]); + }); + + test("merges salvaged items into existing recovered list", () => { + expect( + coalesceStoredLists([ + { name: "recovered", items: ["keep"] }, + { name: "the", items: ["from-the"] }, + ]), + ).toEqual([ + { name: RECOVERED_LIST_NAME, items: ["keep", "from-the"] }, + ]); + expect( + coalesceStoredLists([ + { name: RECOVERED_LIST_NAME, items: ["keep"] }, + { name: "list", items: ["from-list"] }, + ]), + ).toEqual([ + { name: RECOVERED_LIST_NAME, items: ["keep", "from-list"] }, + ]); + }); + + test("does not throw on null / non-string names; salvages items", () => { + const result = coalesceStoredLists([ + { name: null as unknown as string, items: ["from-null"] }, + { name: 42 as unknown as string, items: ["from-num"] }, + { name: undefined as unknown as string, items: ["from-undef"] }, + null as unknown as { name: string; items: string[] }, + { name: "grocery", items: ["milk"] }, + ]); + expect(result).toEqual( + expect.arrayContaining([ + { name: "grocery", items: ["milk"] }, + { + name: RECOVERED_LIST_NAME, + items: expect.arrayContaining([ + "from-null", + "from-num", + "from-undef", + ]), + }, + ]), + ); + expect(result).toHaveLength(2); + }); +}); + +describe("storedListsNeedRewrite", () => { + test("false for already-canonical store", () => { + expect( + storedListsNeedRewrite([ + { name: "grocery", items: ["milk"] }, + { name: "to do", items: ["call"] }, + { name: "café", items: ["latte"] }, + ]), + ).toBe(false); + }); + + test("false for steady-state salvage-only store (no rewrite loop)", () => { + expect( + storedListsNeedRewrite([ + { name: RECOVERED_LIST_NAME, items: ["milk"] }, + ]), + ).toBe(false); + expect( + storedListsNeedRewrite([ + { name: RECOVERED_LIST_NAME, items: ["a", "b"] }, + { name: "grocery", items: ["eggs"] }, + ]), + ).toBe(false); + }); + + test("true when salvage alias still uses non-canonical 'recovered' key", () => { + expect( + storedListsNeedRewrite([{ name: "recovered", items: ["milk"] }]), + ).toBe(true); + }); + + test("true for placeholder keys (even with items)", () => { + expect( + storedListsNeedRewrite([{ name: "the", items: ["x"] }]), + ).toBe(true); + expect( + storedListsNeedRewrite([{ name: "list", items: [] }]), + ).toBe(true); + expect( + storedListsNeedRewrite([{ name: "mine", items: ["x"] }]), + ).toBe(true); + }); + + test("true for unnormalized names", () => { + expect( + storedListsNeedRewrite([{ name: "a grocery", items: ["eggs"] }]), + ).toBe(true); + expect( + storedListsNeedRewrite([{ name: "Grocery List", items: ["bread"] }]), + ).toBe(true); + expect( + storedListsNeedRewrite([{ name: "grocery lists", items: ["x"] }]), + ).toBe(true); + expect( + storedListsNeedRewrite([ + { name: "grocery list list", items: ["x"] }, + ]), + ).toBe(true); + }); + + test("true when duplicate keys would merge", () => { + expect( + storedListsNeedRewrite([ + { name: "grocery", items: ["a"] }, + { name: "Grocery", items: ["b"] }, + ]), + ).toBe(true); + }); + + test("true for null / non-string names without throwing", () => { + expect( + storedListsNeedRewrite([ + { name: null as unknown as string, items: ["x"] }, + ]), + ).toBe(true); + expect( + storedListsNeedRewrite([ + { name: 1 as unknown as string, items: [] }, + ]), + ).toBe(true); + expect( + storedListsNeedRewrite([ + null as unknown as { name: string; items: string[] }, + ]), + ).toBe(true); + }); +}); From d2c93362b7c5dd611d43d2612d0955f6a44b19db Mon Sep 17 00:00:00 2001 From: typeagent-bot Date: Fri, 31 Jul 2026 04:30:18 +0000 Subject: [PATCH 4/9] style: apply prettier formatting and policy fixes --- ts/packages/agents/list/src/listNameUtils.ts | 11 +++- .../list/test/listPlaceholderName.spec.ts | 54 +++++++++---------- 2 files changed, 37 insertions(+), 28 deletions(-) diff --git a/ts/packages/agents/list/src/listNameUtils.ts b/ts/packages/agents/list/src/listNameUtils.ts index a67bf3fcf4..b40f704cfd 100644 --- a/ts/packages/agents/list/src/listNameUtils.ts +++ b/ts/packages/agents/list/src/listNameUtils.ts @@ -81,7 +81,16 @@ const QUANTIFIERS = new Set([ * strip ("all of them" → "of them"). Never a list identity alone or in an * all-closed-class phrase. */ -const CLOSED_GLUE = new Set(["of", "for", "to", "with", "from", "on", "in", "at"]); +const CLOSED_GLUE = new Set([ + "of", + "for", + "to", + "with", + "from", + "on", + "in", + "at", +]); /** * Anaphoric pronouns the grammar/LLM may bind as listName diff --git a/ts/packages/agents/list/test/listPlaceholderName.spec.ts b/ts/packages/agents/list/test/listPlaceholderName.spec.ts index 9b9ec078c6..43b15f3cc0 100644 --- a/ts/packages/agents/list/test/listPlaceholderName.spec.ts +++ b/ts/packages/agents/list/test/listPlaceholderName.spec.ts @@ -554,17 +554,19 @@ describe("listValidateWildcardMatch", () => { ).toBe(false); }); - test.each(["getList", "clearList", "startEditList", "removeItems"] as const)( - "rejects bare 'the list' on %s", - async (actionName) => { - expect( - await listValidateWildcardMatch( - action(actionName, "the list"), - ctx, - ), - ).toBe(false); - }, - ); + test.each([ + "getList", + "clearList", + "startEditList", + "removeItems", + ] as const)("rejects bare 'the list' on %s", async (actionName) => { + expect( + await listValidateWildcardMatch( + action(actionName, "the list"), + ctx, + ), + ).toBe(false); + }); test.each([ "grocery", @@ -704,9 +706,7 @@ describe("coalesceStoredLists (session hydrate)", () => { test("does not create recovered list when no placeholder items", () => { expect( - coalesceStoredLists([ - { name: "grocery", items: ["milk"] }, - ]), + coalesceStoredLists([{ name: "grocery", items: ["milk"] }]), ).toEqual([{ name: "grocery", items: ["milk"] }]); }); @@ -777,9 +777,7 @@ describe("coalesceStoredLists (session hydrate)", () => { { name: "recovered", items: ["keep"] }, { name: "the", items: ["from-the"] }, ]), - ).toEqual([ - { name: RECOVERED_LIST_NAME, items: ["keep", "from-the"] }, - ]); + ).toEqual([{ name: RECOVERED_LIST_NAME, items: ["keep", "from-the"] }]); expect( coalesceStoredLists([ { name: RECOVERED_LIST_NAME, items: ["keep"] }, @@ -847,15 +845,15 @@ describe("storedListsNeedRewrite", () => { }); test("true for placeholder keys (even with items)", () => { - expect( - storedListsNeedRewrite([{ name: "the", items: ["x"] }]), - ).toBe(true); - expect( - storedListsNeedRewrite([{ name: "list", items: [] }]), - ).toBe(true); - expect( - storedListsNeedRewrite([{ name: "mine", items: ["x"] }]), - ).toBe(true); + expect(storedListsNeedRewrite([{ name: "the", items: ["x"] }])).toBe( + true, + ); + expect(storedListsNeedRewrite([{ name: "list", items: [] }])).toBe( + true, + ); + expect(storedListsNeedRewrite([{ name: "mine", items: ["x"] }])).toBe( + true, + ); }); test("true for unnormalized names", () => { @@ -863,7 +861,9 @@ describe("storedListsNeedRewrite", () => { storedListsNeedRewrite([{ name: "a grocery", items: ["eggs"] }]), ).toBe(true); expect( - storedListsNeedRewrite([{ name: "Grocery List", items: ["bread"] }]), + storedListsNeedRewrite([ + { name: "Grocery List", items: ["bread"] }, + ]), ).toBe(true); expect( storedListsNeedRewrite([{ name: "grocery lists", items: ["x"] }]), From f108b60e631af4d0bfcac36cf138bf45c553d7ec Mon Sep 17 00:00:00 2001 From: Dominic Nguyen Date: Thu, 20 Aug 2026 17:10:35 -0700 Subject: [PATCH 5/9] Remove superseded list agent changes --- .../agents/list/src/listActionHandler.ts | 242 +---- ts/packages/agents/list/src/listNameUtils.ts | 388 -------- ts/packages/agents/list/src/listSchema.json | 4 +- .../list/test/listPlaceholderName.spec.ts | 904 ------------------ 4 files changed, 37 insertions(+), 1501 deletions(-) delete mode 100644 ts/packages/agents/list/src/listNameUtils.ts delete mode 100644 ts/packages/agents/list/test/listPlaceholderName.spec.ts diff --git a/ts/packages/agents/list/src/listActionHandler.ts b/ts/packages/agents/list/src/listActionHandler.ts index 0526efebe3..13effef5be 100644 --- a/ts/packages/agents/list/src/listActionHandler.ts +++ b/ts/packages/agents/list/src/listActionHandler.ts @@ -17,17 +17,7 @@ import { createYesNoChoiceResult, } from "@typeagent/agent-sdk/helpers/action"; import { ListAction, ListActivity } from "./listSchema.js"; -import { - isPlaceholderListName, - normalizeListName, - RECOVERED_LIST_NAME, -} from "./listNameUtils.js"; - -export { - isPlaceholderListName, - normalizeListName, - RECOVERED_LIST_NAME, -} from "./listNameUtils.js"; + export function instantiate(): AppAgent { return { initializeAgentContext: initializeListContext, @@ -111,7 +101,7 @@ function simpleNoun(item: string) { function validateWildcardItems( items: string[], - _context: SessionContext, + context: SessionContext, ) { for (const item of items) { if (!simpleNoun(item)) { @@ -121,52 +111,21 @@ function validateWildcardItems( return true; } -function listNameFromAction( - action: ListAction | ListActivity, -): string | undefined { - if ( - action.actionName === "addItems" || - action.actionName === "removeItems" || - action.actionName === "createList" || - action.actionName === "getList" || - action.actionName === "clearList" || - action.actionName === "startEditList" - ) { - return action.parameters.listName; - } - return undefined; -} - -/** - * Reject grammar matches whose listName is only a determiner / "list" - * (after stripping leading dets). Exported for unit tests. - */ -export async function listValidateWildcardMatch( +async function listValidateWildcardMatch( action: ListAction | ListActivity, context: SessionContext, ) { - const listName = listNameFromAction(action); - if (listName !== undefined && isPlaceholderListName(listName)) { - return false; - } - if (action.actionName === "addItems") { - return validateWildcardItems(action.parameters.items, context); + const addItemsAction = action; + return validateWildcardItems(addItemsAction.parameters.items, context); } else if (action.actionName === "removeItems") { - return validateWildcardItems(action.parameters.items, context); - } - return true; -} - -/** Normalize listName and reject placeholders at execute time. */ -function requireListName(listName: string): string { - const normalized = normalizeListName(listName); - if (normalized === "" || isPlaceholderListName(normalized)) { - throw new Error( - 'List name is missing or only a reference phrase (e.g. "the list"); clarify which list', + const removeItemsAction = action; + return validateWildcardItems( + removeItemsAction.parameters.items, + context, ); } - return normalized; + return true; } async function initializeListContext() { @@ -190,134 +149,6 @@ function createMemoryList(list: List): MemoryList { }; } -/** - * Collapse legacy/raw list records onto normalized keys. - * Placeholder keys ("the", "list", "my", "it", …) are not kept as identities, - * but any items under them are salvaged into RECOVERED_LIST_NAME so hydrate - * never permanently drops user data from the pre-fix failure mode. - * RECOVERED_LIST_NAME itself is a canonical, addressable store key (aliases - * "recovered" / "the recovered list" normalize to it) so a salvage-only store - * is steady-state and does not rewrite on every session load. - * Exported for unit tests. - */ -export function coalesceStoredLists(rawLists: List[]): List[] { - const map = new Map>(); - const salvaged = new Set(); - if (!Array.isArray(rawLists)) { - return []; - } - for (const list of rawLists) { - // Corrupted / hand-edited entries: null name, non-string name, or - // non-object rows. Salvage any items we can still read. - if (list == null || typeof list !== "object") { - continue; - } - const rawName = (list as List).name; - const rawItems = Array.isArray((list as List).items) - ? (list as List).items - : []; - if (typeof rawName !== "string") { - for (const item of rawItems) { - if (typeof item === "string") { - salvaged.add(item); - } - } - continue; - } - const name = normalizeListName(rawName); - if (isPlaceholderListName(name)) { - for (const item of rawItems) { - if (typeof item === "string") { - salvaged.add(item); - } - } - continue; - } - let items = map.get(name); - if (items === undefined) { - items = new Set(); - map.set(name, items); - } - for (const item of rawItems) { - if (typeof item === "string") { - items.add(item); - } - } - } - if (salvaged.size > 0) { - let items = map.get(RECOVERED_LIST_NAME); - if (items === undefined) { - items = new Set(); - map.set(RECOVERED_LIST_NAME, items); - } - for (const item of salvaged) { - items.add(item); - } - } - return Array.from(map.entries()).map(([name, itemsSet]) => ({ - name, - items: Array.from(itemsSet), - })); -} - -/** - * True when raw disk records would change under coalesce (dirty names, merges, - * or placeholder salvage). Used to decide whether to rewrite lists.json on load. - */ -export function storedListsNeedRewrite(rawLists: List[]): boolean { - if (!Array.isArray(rawLists)) { - return true; - } - const coalesced = coalesceStoredLists(rawLists); - if (coalesced.length !== rawLists.length) { - return true; - } - // Build multiset comparison on normalized shape (order-independent names). - const rawByName = new Map>(); - for (const list of rawLists) { - if (list == null || typeof list !== "object") { - return true; - } - if (typeof list.name !== "string") { - return true; - } - // Any non-canonical name on disk must be rewritten. - if (list.name !== normalizeListName(list.name)) { - return true; - } - if (isPlaceholderListName(list.name)) { - return true; - } - if (rawByName.has(list.name)) { - return true; // duplicate keys → merge - } - const itemSet = new Set(); - for (const item of list.items ?? []) { - if (typeof item === "string") { - itemSet.add(item); - } else { - return true; // non-string item → rewrite - } - } - rawByName.set(list.name, itemSet); - } - for (const list of coalesced) { - const rawItems = rawByName.get(list.name); - if (rawItems === undefined) { - return true; - } - if (rawItems.size !== list.items.length) { - return true; - } - for (const item of list.items) { - if (!rawItems.has(item)) { - return true; - } - } - } - return false; -} - class MemoryListCollection { private lists = new Map(); constructor( @@ -325,9 +156,12 @@ class MemoryListCollection { private storage: Storage, private listStoreName: string, ) { - for (const list of coalesceStoredLists(rawLists)) { - this.lists.set(list.name, createMemoryList(list)); - } + rawLists.forEach((list) => { + const lookupName = list.name; + if (lookupName !== undefined) { + this.lists.set(lookupName, createMemoryList(list)); + } + }); } createList(name: string) { @@ -351,11 +185,10 @@ class MemoryListCollection { removeItems(listName: string, items: string[]) { const list = this.getList(listName); - if (list === undefined) { - throw new Error(`List '${listName}' not found`); - } - for (const item of items) { - list.itemsSet.delete(item); + if (list !== undefined) { + for (const item of items) { + list.itemsSet.delete(item); + } } } @@ -397,22 +230,14 @@ async function createListStoreForSession( listStoreName: string, ) { let lists: List[] = []; - let existed = false; // check whether file exists if (await storage.exists(listStoreName)) { - existed = true; const data = await storage.read(listStoreName, "utf8"); lists = JSON.parse(data); } else { await storage.write(listStoreName, JSON.stringify(lists)); } - const store = new MemoryListCollection(lists, storage, listStoreName); - // Persist scrubbed/normalized keys immediately so read-only sessions and - // process exit before a mutate do not leave polluted keys on disk. - if (existed && storedListsNeedRewrite(lists)) { - await store.save(); - } - return store; + return new MemoryListCollection(lists, storage, listStoreName); } async function updateListContext( @@ -524,11 +349,13 @@ async function handleListAction( switch (action.actionName) { case "addItems": { const store = getStore(listContext); - const { items } = action.parameters; - const listName = requireListName(action.parameters.listName); + const { items, listName } = action.parameters; if (items.length === 0) { throw new Error("No items to add"); } + if (listName === "") { + throw new Error("List name is empty"); + } store.addItems(listName, items); await store.save(); @@ -545,11 +372,13 @@ async function handleListAction( } case "removeItems": { const store = getStore(listContext); - const { items } = action.parameters; - const listName = requireListName(action.parameters.listName); + const { items, listName } = action.parameters; if (items.length === 0) { throw new Error("No items to remove"); } + if (listName === "") { + throw new Error("List name is empty"); + } store.removeItems(listName, items); await store.save(); @@ -566,7 +395,7 @@ async function handleListAction( } case "createList": { const store = getStore(listContext); - const listName = requireListName(action.parameters.listName); + const listName = action.parameters.listName; if (store.createList(listName)) { displayText = `Created list: ${listName}`; @@ -586,8 +415,7 @@ async function handleListAction( break; } case "getList": { - const listName = requireListName(action.parameters.listName); - result = getListDisplay(listContext, listName); + result = getListDisplay(listContext, action.parameters.listName); break; } case "listLists": { @@ -627,7 +455,8 @@ async function handleListAction( } case "clearList": { const store = getStore(listContext); - const listName = requireListName(action.parameters.listName); + const clearListAction = action; + const listName = clearListAction.parameters.listName; const list = getList(listContext, listName); list.itemsSet.clear(); await store.save(); @@ -668,10 +497,9 @@ async function handleListAction( break; } case "startEditList": { - const listName = requireListName(action.parameters.listName); result = getListDisplay( listContext, - listName, + action.parameters.listName, "What do you want to add or remove from this list?", ); // TODO: formalize the schema for activityContext @@ -679,7 +507,7 @@ async function handleListAction( activityName: "edit", description: "editing list", state: { - listName, + listName: action.parameters.listName, }, }; break; diff --git a/ts/packages/agents/list/src/listNameUtils.ts b/ts/packages/agents/list/src/listNameUtils.ts deleted file mode 100644 index b40f704cfd..0000000000 --- a/ts/packages/agents/list/src/listNameUtils.ts +++ /dev/null @@ -1,388 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -/** - * Determiners / bare "list" / anaphora captured as listName by listSchema.agr - * ("add ham to the list" → listName="the", "put cheese on the list" → "list", - * "add milk to a grocery list" → listName="a grocery", - * "add milk to it" → listName="it", - * "create a new list" → listName="new", - * "add eggs to grocery list" → listName="grocery list", - * "add milk to this one" → listName="this one" / "one", - * "clear the other list" → listName="other", - * "add milk to the first one" → listName="first one", - * "clear both lists" → listName="both", - * "add milk to mine" → listName="mine"). - * - * Prefer normalize-then-validate: - * 1. strip token-edge punctuation (LLM/STT artifacts: "list.", "the,") - * and English possessives ("list's") while preserving Unicode letters - * 2. strip leading determiners/quantifiers only (NOT anaphoric pronouns - * me/us/it/you — those can be real multi-word names like "me time") - * 3. strip all trailing generic "list"/"lists" tokens when a real name remains - * 4. casefold so grammar/LLM casing does not split store keys - * 5. map the salvage identity "recovered" → RECOVERED_LIST_NAME - * 6. reject empty / bare-det / bare-"list(s)" / bare-"new" / bare pronoun / - * bare deictic (one/ones/other/same/another/current/…) / quantifiers / - * possessives / ordinal+one leftovers / all-closed-class leftovers / - * partial anaphora ("lots of them", "which of those") - * so real names like "grocery" keep working while "the list" falls through. - */ - -/** Closed-class determiners the grammar may glue onto a real list name. */ -const DETERMINERS = new Set([ - "the", - "a", - "an", - "this", - "that", - "these", - "those", - "my", - "your", - "our", - "his", - "her", - "its", - "their", - // anaphoric / deictic determiners ("another list", "the other list", "the same list") - "another", - "other", - "same", - // interrogative determiner ("which list", "which one") - "which", -]); - -/** - * Quantifiers that never name a list ("both lists", "every list", "any list", - * "all lists", "some list", "each list", "either list", "none of them"). - * Stripped when leading a multi-word capture; rejected when bare. - */ -const QUANTIFIERS = new Set([ - "any", - "all", - "both", - "every", - "each", - "some", - "either", - "neither", - "none", - // underspecified quantity words ("more lists", "many lists", …) - "more", - "many", - "several", - "few", - "most", -]); - -/** - * Light closed-class glue that appears in anaphora leftovers after quantifier - * strip ("all of them" → "of them"). Never a list identity alone or in an - * all-closed-class phrase. - */ -const CLOSED_GLUE = new Set([ - "of", - "for", - "to", - "with", - "from", - "on", - "in", - "at", -]); - -/** - * Anaphoric pronouns the grammar/LLM may bind as listName - * ("add milk to it", "put cheese on them", "make me a list" → "me"/"me a", - * subject forms "they"/"we"/"she" from STT/LLM slips). - * Rejected when bare (or all-closed-class), but NOT stripped from multi-word - * names so real lists like "me time" / "us travel" / "it projects" stay intact. - */ -const ANAPHORIC_PRONOUNS = new Set([ - "it", - "them", - "me", - "him", - "us", - "you", - "they", - "we", - "she", - "he", -]); - -/** - * Independent possessives ("add milk to mine", "what's on yours", "clear ours"). - * Never a list identity when bare. - */ -const INDEPENDENT_POSSESSIVES = new Set([ - "mine", - "yours", - "ours", - "theirs", - "hers", - "his", // already a determiner; listed for clarity when bare -]); - -/** - * Ordinal / sequential words used in deictic phrases - * ("first one", "last one", "next one", "the previous one"). - * Rejected when the whole name is closed-class; kept when paired with a real - * noun ("first aid", "next week"). - */ -const ORDINALS = new Set([ - "first", - "second", - "third", - "fourth", - "fifth", - "last", - "next", - "previous", - "former", - "latter", -]); - -/** - * Small cardinals used deictically ("the two", "those three lists"). - * Rejected when bare / all-closed-class; kept inside real names ("two trees"). - */ -const CARDINALS = new Set([ - "two", - "three", - "four", - "five", - "six", - "seven", - "eight", - "nine", - "ten", -]); - -/** - * Deictic leftovers after det strip ("this one", "that one", "those ones", - * "my own") — never a list id. - */ -const DEICTIC_PLACEHOLDERS = new Set(["one", "ones", "own"]); - -/** - * Session/selection deictics ("the current list", "my active list") — never a - * list id when bare / all-closed-class. NOT leading-stripped: real names like - * "whole foods", "active tasks", "current events" must stay intact. - */ -const SELECTION_DEICTICS = new Set([ - "current", - "active", - "default", - "whole", - "entire", -]); - -/** - * Underspecified content heads that only appear in partial anaphora - * ("lots of them", "the rest of those") — never a list identity alone. - */ -const PARTIAL_ANAPHORA_HEADS = new Set(["lots", "rest"]); - -/** - * Tokens stripped from the front of multi-word names when a real name remains. - * TRUE DETERMINERS ONLY. Quantifiers / selection deictics are rejected when - * bare but never peeled off open-class compounds ("most wanted", "whole foods", - * "active tasks", "many thanks"). - */ -const LEADING_STRIP = new Set([...DETERMINERS]); - -/** - * Single-token values that are never a real list identity after normalize. - * Includes bare "new" from CreateList's optional (new)? before the wildcard, - * and bare "list"/"lists" from underspecified "show the lists" paths. - * NOTE: "recovered" is intentionally NOT here — it is the user-facing alias - * of the salvage store key (see normalizeListName). - */ -const PLACEHOLDER_TOKENS = new Set([ - ...DETERMINERS, - ...QUANTIFIERS, - ...ANAPHORIC_PRONOUNS, - ...INDEPENDENT_POSSESSIVES, - ...ORDINALS, - ...CARDINALS, - ...DEICTIC_PLACEHOLDERS, - ...SELECTION_DEICTICS, - ...PARTIAL_ANAPHORA_HEADS, - ...CLOSED_GLUE, - "list", - "lists", - "new", -]); - -/** - * Strip leading/trailing non-letter/non-number junk from a token (keep internal), - * then an English possessive suffix. Unicode-aware so accented / non-Latin names - * (café, résumé, 買い物) survive. - */ -function cleanToken(token: string): string { - return ( - token - .toLowerCase() - .replace(/^[^\p{L}\p{N}]+/gu, "") - .replace(/[^\p{L}\p{N}]+$/gu, "") - // STT/LLM possessives: "list's", "grocery's", curly apostrophe - .replace(/['\u2019]s$/u, "") - ); -} - -function splitWords(listName: string): string[] { - return listName - .trim() - .split(/\s+/) - .map(cleanToken) - .filter((w) => w.length > 0); -} - -/** - * Internal fallback store key when legacy lists.json only has placeholder keys - * ("the", "list", "my", …) that still hold real items. Double-underscore form - * so it cannot collide with a normal user-typed list name. Addressable via - * normalize aliases "recovered" / "the recovered list" / "__recovered__". - */ -export const RECOVERED_LIST_NAME = "__recovered__"; - -/** User-facing / underscore-stripped form of the salvage key. */ -const RECOVERED_ALIAS = "recovered"; - -/** - * Normalize a captured/emitted listName into a stable store key: - * strip edge punctuation + possessives, strip leading true determiners only, - * strip all trailing generic "list"/"lists", casefold, - * map salvage alias → RECOVERED_LIST_NAME. - * Single-token input is not leading-stripped (placeholder check handles bare dets). - * Quantifiers / selection deictics / anaphora are not leading-stripped - * (preserve "whole foods", "active tasks", "most wanted", "me time"). - * Examples: "a grocery" → "grocery", "the grocery list" → "grocery", - * "Grocery Lists" → "grocery", "the." → "the", "grocery list!" → "grocery", - * "grocery list list" → "grocery", "both lists" → "both", "café list" → "café", - * "Contoso grocery" → "contoso grocery", "me time" → "me time", - * "whole foods list" → "whole foods", - * "recovered" / "the recovered list" / "__recovered__" → "__recovered__". - */ -export function normalizeListName(listName: string): string { - const words = splitWords(listName); - let start = 0; - // Only strip true determiners while a real name token would remain. - while (start < words.length - 1 && LEADING_STRIP.has(words[start]!)) { - start++; - } - let end = words.length; - // Trailing generic "list"/"lists" from optional (list)? / LLM phrasing — - // strip ALL of them so normalize is idempotent ("grocery list list"). - // Keep bare "list"/"lists" for the placeholder check. - while (end - start > 1) { - const last = words[end - 1]!; - if (last === "list" || last === "lists") { - end--; - } else { - break; - } - } - const normalized = words.slice(start, end).join(" "); - // Canonical salvage key (cleanToken strips underscores from "__recovered__"). - if (normalized === RECOVERED_ALIAS || normalized === RECOVERED_LIST_NAME) { - return RECOVERED_LIST_NAME; - } - return normalized; -} - -/** - * True when the last two tokens are "of" + anaphora/demonstrative - * ("of them", "of those", "of it"). - */ -function endsWithOfAnaphora(words: string[]): boolean { - if (words.length < 2) { - return false; - } - const last = words[words.length - 1]!; - const prev = words[words.length - 2]!; - if (prev !== "of") { - return false; - } - return ( - ANAPHORIC_PRONOUNS.has(last) || - last === "those" || - last === "these" || - last === "this" || - last === "that" - ); -} - -/** - * Partial anaphora like "all of them" / "lots of those" / "rest of it". - * Rejects only when every token before "of + anaphor" is closed-class, so - * real names like "photos of them" stay valid. - */ -function isClosedClassOfAnaphora(words: string[]): boolean { - if (!endsWithOfAnaphora(words)) { - return false; - } - const head = words.slice(0, -2); - return head.every((w) => PLACEHOLDER_TOKENS.has(w)); -} - -/** - * True when listName is not a usable list identity after normalization - * (empty, bare determiner/pronoun/deictic/quantifier/possessive/ordinal, - * bare "list(s)"/"new", all-closed-class phrases, glue-led leftovers like - * "of grocery", or closed-class "… of them/those"). - * The salvage key RECOVERED_LIST_NAME is a real store identity (addressable). - */ -export function isPlaceholderListName(listName: string): boolean { - const normalized = normalizeListName(listName); - if (normalized.length === 0) { - return true; - } - - // Salvage key is a real, addressable store identity — not a placeholder. - if (normalized === RECOVERED_LIST_NAME) { - return false; - } - - if (PLACEHOLDER_TOKENS.has(normalized)) { - return true; - } - - const words = normalized.split(/\s+/).filter((w) => w.length > 0); - - // "me a", "first one", "the other", "those ones", "of them" (all closed-class) - if (words.length > 0 && words.every((w) => PLACEHOLDER_TOKENS.has(w))) { - return true; - } - - // Grammar/LLM junk that starts with "of" ("of grocery", "of them") after a - // quantifier peel. Only "of" — not other prepositions ("to do", "on call"). - if (words.length > 0 && words[0] === "of") { - return true; - } - - // "lots of …" / "rest of …" are partial anaphora heads, not list titles - // ("lots of groceries", "rest of the grocery"). - if (words.length > 0 && PARTIAL_ANAPHORA_HEADS.has(words[0]!)) { - return true; - } - - // "all of them", "none of it" — but NOT "photos of them" - if (isClosedClassOfAnaphora(words)) { - return true; - } - - // Defense in depth: "X list(s)" where X is still a closed-class token - // (should already have been reduced by normalize, but keep the check cheap). - if ( - words.length === 2 && - (words[1] === "list" || words[1] === "lists") && - PLACEHOLDER_TOKENS.has(words[0]!) - ) { - return true; - } - - return false; -} diff --git a/ts/packages/agents/list/src/listSchema.json b/ts/packages/agents/list/src/listSchema.json index 665d29d857..4cda638cfe 100644 --- a/ts/packages/agents/list/src/listSchema.json +++ b/ts/packages/agents/list/src/listSchema.json @@ -2,14 +2,14 @@ "paramSpec": { "addItems": { "items.*": "wildcard", - "listName": "checked_wildcard" + "listName": "wildcard" }, "removeItems": { "items.*": "wildcard", "listName": "checked_wildcard" }, "createList": { - "listName": "checked_wildcard" + "listName": "wildcard" }, "getList": { "listName": "checked_wildcard" diff --git a/ts/packages/agents/list/test/listPlaceholderName.spec.ts b/ts/packages/agents/list/test/listPlaceholderName.spec.ts deleted file mode 100644 index 43b15f3cc0..0000000000 --- a/ts/packages/agents/list/test/listPlaceholderName.spec.ts +++ /dev/null @@ -1,904 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -/** - * Placeholder listName rejection — grammar can bind "the"/"my"/"list" from - * phrases like "add ham to the list", or "a grocery" from determiner+name - * compounds, "it"/"them"/"me"/"they"/"we"/"she" from anaphora, "new" from - * CreateList's optional adjective, "grocery list"/"grocery lists" when the - * trailing list token is eaten by the wildcard, deictics like "this one"/ - * "the other list"/"first one"/"those ones"/"the current list", quantifiers - * ("both lists", "every list", "none of them"), independent possessives - * ("mine"), interrogatives ("which list"), partial anaphora ("lots of them"), - * possessives ("list's"), or punctuated LLM/STT artifacts ("the list.", "list?"). - * Normalize strips edge punctuation (Unicode-safe), possessives, leading - * dets/quantifiers/selection deictics (not anaphoric pronouns), all trailing - * "list"/"lists", casefolds, and maps the salvage alias to RECOVERED_LIST_NAME. - * Leftovers that are still placeholders must not be real lists. Real multi-word - * names like "me time" and non-ASCII names like "café" stay intact. The salvage - * key is addressable so recovered items remain reachable. - */ - -import { - isPlaceholderListName, - normalizeListName, - RECOVERED_LIST_NAME, -} from "../src/listNameUtils.js"; -import { - coalesceStoredLists, - listValidateWildcardMatch, - storedListsNeedRewrite, -} from "../src/listActionHandler.js"; -import type { ListAction, ListActivity } from "../src/listSchema.js"; -import type { SessionContext } from "@typeagent/agent-sdk"; - -describe("normalizeListName", () => { - test.each([ - ["a grocery", "grocery"], - ["that shopping", "shopping"], - ["your packing", "packing"], - ["the list", "list"], - ["my list", "list"], - ["an idea", "idea"], - [" A Grocery ", "grocery"], - // trailing generic "list" / "lists" (including repeats — idempotent) - ["grocery list", "grocery"], - ["the grocery list", "grocery"], - ["Grocery List", "grocery"], - ["GROCERY LIST", "grocery"], - ["shopping list", "shopping"], - ["to do list", "to do"], - ["grocery lists", "grocery"], - ["Grocery Lists", "grocery"], - ["the grocery lists", "grocery"], - ["grocery list list", "grocery"], - ["shopping lists list", "shopping"], - ["grocery list lists", "grocery"], - ["the grocery list list", "grocery"], - // possessives - ["list's", "list"], - ["the list's", "list"], - ["my list's", "list"], - ["grocery list's", "grocery"], - ["grocery's", "grocery"], - // anaphoric dets - ["another grocery", "grocery"], - ["the other shopping", "shopping"], - ["the same packing", "packing"], - // all-det + trailing list collapses to bare "list" (still a placeholder) - ["another list", "list"], - ["the other list", "list"], - ["the same list", "list"], - ["this one", "one"], - ["that one", "one"], - // selection deictics are NOT leading-stripped (preserve "whole foods"); - // "the current list" → strip det + trailing list → bare "current" - ["the current list", "current"], - ["the active list", "active"], - ["the default list", "default"], - ["the whole list", "whole"], - ["the entire list", "entire"], - ["current", "current"], - ["active", "active"], - ["whole grocery list", "whole grocery"], - ["the whole grocery list", "whole grocery"], - ["whole foods list", "whole foods"], - ["active tasks", "active tasks"], - ["current events", "current events"], - // quantifiers are NOT leading-stripped (preserve "most wanted"); - // trailing list still peels: "both lists" → "both" - ["both lists", "both"], - ["every list", "every"], - ["all lists", "all"], - ["any list", "any"], - ["some list", "some"], - ["each list", "each"], - ["either list", "either"], - ["none of them", "none of them"], - ["both grocery", "both grocery"], - ["every shopping", "every shopping"], - ["most wanted", "most wanted"], - ["many thanks", "many thanks"], - // interrogative det still strips when leading - ["which", "which"], - ["which one", "one"], - ["which list", "list"], - ["which grocery", "grocery"], - // ordinals + one kept as multi-token for all-closed-class reject - ["first one", "first one"], - ["last one", "last one"], - ["next one", "next one"], - ["the previous one", "previous one"], - ["the first one", "first one"], - ["those ones", "ones"], - ["these ones", "ones"], - ["the ones", "ones"], - // leading anaphora is NOT stripped (preserve real names) - ["me a", "me a"], - ["me a new", "me a new"], - ["me time", "me time"], - ["us travel", "us travel"], - ["it projects", "it projects"], - ["you team", "you team"], - ["US travel", "us travel"], - // single-token input is not leading-stripped - ["the", "the"], - ["list", "list"], - ["lists", "lists"], - ["grocery", "grocery"], - ["Grocery", "grocery"], - ["GROCERY", "grocery"], - ["mine", "mine"], - ["both", "both"], - ["they", "they"], - // salvage alias → canonical store key - ["recovered", RECOVERED_LIST_NAME], - ["Recovered", RECOVERED_LIST_NAME], - ["__recovered__", RECOVERED_LIST_NAME], - ["the recovered list", RECOVERED_LIST_NAME], - ["recovered list", RECOVERED_LIST_NAME], - // punctuation (LLM/STT) - ["the.", "the"], - ["list,", "list"], - ["it!", "it"], - ["my?", "my"], - ["the list.", "list"], - ["list?", "list"], - ["my list,", "list"], - ["grocery list!", "grocery"], - ["Grocery Lists.", "grocery"], - // Unicode letters preserved (not ASCII-stripped) - ["café", "café"], - ["Café", "café"], - ["résumé", "résumé"], - ["café list", "café"], - ["the café list", "café"], - ["買い物", "買い物"], - ["買い物 list", "買い物"], - ["café!", "café"], - // non-determiner first token kept (casefolded) - ["Contoso grocery", "contoso grocery"], - ["to do", "to do"], - ["q3 packing", "q3 packing"], - ["New York", "new york"], - ["first aid", "first aid"], - ["next week", "next week"], - ["", ""], - [" ", ""], - ])("normalize %j → %j", (input, expected) => { - expect(normalizeListName(input)).toBe(expected); - }); - - test("normalize is idempotent (including repeated trailing list)", () => { - for (const input of [ - "grocery list list", - "shopping lists list", - "the grocery list", - "a grocery", - "recovered", - "__recovered__", - "the recovered list", - "grocery list's", - "the current list", - ]) { - const once = normalizeListName(input); - expect(normalizeListName(once)).toBe(once); - } - }); -}); - -describe("isPlaceholderListName", () => { - test.each([ - "the", - "that", - "my", - "a", - "an", - "this", - "these", - "those", - "your", - "our", - "his", - "her", - "its", - "their", - "list", - "lists", - "the list", - "my list", - "that list", - "its list", - "a list", - "the lists", - "my lists", - // possessives - "list's", - "the list's", - "my list's", - // anaphoric pronouns (object + subject forms) - "it", - "them", - "me", - "him", - "us", - "you", - "they", - "we", - "she", - "he", - "IT", - "Them", - "They", - "me a", - // independent possessives - "mine", - "yours", - "ours", - "theirs", - "hers", - "Mine", - "yours list", - // quantifiers - "any", - "all", - "both", - "every", - "each", - "some", - "either", - "neither", - "none", - "more", - "many", - "several", - "few", - "most", - "both lists", - "every list", - "all lists", - "any list", - "some list", - "each list", - "either list", - "more lists", - "many lists", - "none of them", - "none of those", - // quantifier + of + anaphora leftovers / glue-led junk - "of them", - "of those", - "of it", - "of grocery", - "all of them", - "both of them", - "some of it", - "any of these", - "lots of them", - "lots of groceries", - "rest of those", - "rest of the grocery", - "the rest of the grocery list", - // partial anaphora with content head - "lots of them", - "rest of them", - "rest of those", - "which of them", - "lots", - "rest", - // interrogative - "which", - "which one", - "which list", - // selection / session deictics - "current", - "active", - "default", - "whole", - "entire", - "the current list", - "the active list", - "the default list", - "the whole list", - "the entire list", - "current list", - "active list", - // deictic "own" leftover - "own", - "my own", - "my own list", - // small cardinals - "two", - "three", - "the two", - "the three lists", - "those two", - // bare CreateList adjective - "new", - "NEW", - "a new", - "me a new", - // deictic / anaphoric dets - "one", - "ones", - "this one", - "that one", - "those ones", - "these ones", - "the ones", - "other", - "another", - "same", - "the other", - "the other list", - "another list", - "the same list", - "another lists", - // ordinal + one deictics - "first one", - "last one", - "next one", - "previous one", - "the previous one", - "the first one", - "second one", - "the last one", - // punctuation-glued placeholders - "the.", - "list,", - "it!", - "my?", - "the list.", - "list?", - "my list,", - "lists!", - "mine!", - "both lists.", - "", - " ", - "THE", - "My", - "ITS", - "Lists", - ])("rejects placeholder %j", (name) => { - expect(isPlaceholderListName(name)).toBe(true); - }); - - test.each([ - "grocery", - "shopping", - "Contoso grocery", - "to do", - "q3 packing", - "gift", - // determiner+name compounds normalize to a real name - "a grocery", - "that shopping", - "your packing", - "my grocery", - "an idea", - "its shopping", - "another grocery", - "the other shopping", - // trailing list(s) collapses to real name - "grocery list", - "the grocery list", - "Grocery List", - "shopping list", - "to do list", - "grocery lists", - "Grocery Lists", - "grocery list!", - "grocery list list", - "grocery list's", - // multi-word with "new" as part of a real name stays usable - "New York", - "new hires", - // ordinal + real noun stays usable - "first aid", - "next week", - "last christmas", - // anaphoric-pronoun-leading real names (not stripped) - "me time", - "us travel", - "it projects", - "you team", - "US travel", - // Unicode / non-ASCII real names - "café", - "résumé", - "café list", - "the café", - "買い物", - "買い物 list", - // selection deictic + real name (not stripped) - "whole grocery", - "the whole grocery list", - "whole foods", - "active tasks", - "current events", - "default settings", - "entire catalog", - // quantifier + open-class compound (not stripped) - "most wanted", - "many thanks", - "few ingredients", - "several projects", - "more errands", - // open-class head + of + anaphor is a real name - "photos of them", - "pictures of those", - // salvage key is addressable (not a placeholder) - "recovered", - "__recovered__", - "the recovered list", - "Recovered", - ])("accepts real list name %j", (name) => { - expect(isPlaceholderListName(name)).toBe(false); - }); -}); - -describe("listValidateWildcardMatch", () => { - const ctx = {} as SessionContext; - - function action( - actionName: ListAction["actionName"] | ListActivity["actionName"], - listName: string, - items?: string[], - ): ListAction | ListActivity { - if (actionName === "addItems" || actionName === "removeItems") { - return { - actionName, - parameters: { listName, items: items ?? ["milk"] }, - } as ListAction; - } - if (actionName === "listLists") { - return { actionName: "listLists", parameters: {} }; - } - return { - actionName, - parameters: { listName }, - } as ListAction | ListActivity; - } - - test.each([ - "the", - "my", - "list", - "lists", - "the list", - "the lists", - "its", - "it", - "them", - "me", - "they", - "we", - "she", - "mine", - "yours", - "ours", - "both", - "every", - "all", - "any", - "some", - "none", - "none of them", - "both lists", - "every list", - "new", - "a new", - "one", - "ones", - "this one", - "those ones", - "first one", - "last one", - "the previous one", - "other", - "another", - "same", - "the other list", - "which", - "which one", - "which list", - "lots of them", - "rest of those", - "the current list", - "the active list", - "the default list", - "the whole list", - "the entire list", - "list's", - "the list's", - "the.", - "list?", - "the list.", - "", - " ", - ])("rejects placeholder listName %j on createList", async (name) => { - expect( - await listValidateWildcardMatch(action("createList", name), ctx), - ).toBe(false); - }); - - test.each([ - "the", - "the list", - "my", - "list", - "lists", - "its", - "it", - "them", - "me", - "they", - "mine", - "both", - "any", - "none of them", - "first one", - "those ones", - "another", - "the other list", - "this one", - "which", - "which one", - "the current list", - "list!", - "list's", - ])("rejects placeholder listName %j on addItems", async (name) => { - expect( - await listValidateWildcardMatch(action("addItems", name), ctx), - ).toBe(false); - }); - - test.each([ - "getList", - "clearList", - "startEditList", - "removeItems", - ] as const)("rejects bare 'the list' on %s", async (actionName) => { - expect( - await listValidateWildcardMatch( - action(actionName, "the list"), - ctx, - ), - ).toBe(false); - }); - - test.each([ - "grocery", - "a grocery", - "that shopping", - "your packing", - "Contoso grocery", - "to do", - "q3 packing", - "grocery list", - "the grocery list", - "grocery lists", - "grocery list list", - "Grocery", - "GROCERY LIST", - "grocery list!", - "grocery list's", - "me time", - "us travel", - "café", - "買い物", - "first aid", - // salvage aliases must pass wildcard so users can address recovered items - "recovered", - "__recovered__", - "the recovered list", - ])("accepts usable listName %j on createList", async (name) => { - expect( - await listValidateWildcardMatch(action("createList", name), ctx), - ).toBe(true); - }); - - test.each([ - "getList", - "clearList", - "startEditList", - "addItems", - "removeItems", - ] as const)( - "accepts salvage aliases on %s so recovered items are reachable", - async (actionName) => { - for (const name of [ - "recovered", - "__recovered__", - "the recovered list", - ]) { - expect( - await listValidateWildcardMatch( - action(actionName, name), - ctx, - ), - ).toBe(true); - } - }, - ); - - test("accepts determiner+name compound on addItems", async () => { - expect( - await listValidateWildcardMatch( - action("addItems", "a grocery", ["eggs"]), - ctx, - ), - ).toBe(true); - }); - - test("accepts trailing-list form on addItems (grammar without literal list)", async () => { - expect( - await listValidateWildcardMatch( - action("addItems", "grocery list", ["ham"]), - ctx, - ), - ).toBe(true); - expect( - await listValidateWildcardMatch( - action("addItems", "the grocery list", ["milk", "eggs"]), - ctx, - ), - ).toBe(true); - expect( - await listValidateWildcardMatch( - action("addItems", "grocery lists", ["bread"]), - ctx, - ), - ).toBe(true); - expect( - await listValidateWildcardMatch( - action("addItems", "grocery list list", ["butter"]), - ctx, - ), - ).toBe(true); - }); - - test("still rejects non-simple item nouns on addItems", async () => { - expect( - await listValidateWildcardMatch( - action("addItems", "grocery", ["the big red apple"]), - ctx, - ), - ).toBe(false); - }); -}); - -describe("coalesceStoredLists (session hydrate)", () => { - test("salvages items under placeholder keys into recovered list", () => { - expect( - coalesceStoredLists([ - { name: "the", items: ["x"] }, - { name: "list", items: ["y"] }, - { name: "my", items: ["z"] }, - { name: "new", items: ["n"] }, - { name: "it", items: ["i"] }, - { name: "the list", items: ["t"] }, - { name: "mine", items: ["m"] }, - { name: "both", items: ["b"] }, - ]), - ).toEqual([ - { - name: RECOVERED_LIST_NAME, - items: ["x", "y", "z", "n", "i", "t", "m", "b"], - }, - ]); - }); - - test("salvages placeholder items alongside real lists", () => { - const result = coalesceStoredLists([ - { name: "the", items: ["lost"] }, - { name: "grocery", items: ["milk"] }, - ]); - expect(result).toEqual( - expect.arrayContaining([ - { name: "grocery", items: ["milk"] }, - { name: RECOVERED_LIST_NAME, items: ["lost"] }, - ]), - ); - expect(result).toHaveLength(2); - }); - - test("does not create recovered list when no placeholder items", () => { - expect( - coalesceStoredLists([{ name: "grocery", items: ["milk"] }]), - ).toEqual([{ name: "grocery", items: ["milk"] }]); - }); - - test("keeps already-canonical salvage list stable (no re-salvage loop)", () => { - expect( - coalesceStoredLists([ - { name: RECOVERED_LIST_NAME, items: ["milk"] }, - ]), - ).toEqual([{ name: RECOVERED_LIST_NAME, items: ["milk"] }]); - // alias forms also land on the same key - expect( - coalesceStoredLists([{ name: "recovered", items: ["eggs"] }]), - ).toEqual([{ name: RECOVERED_LIST_NAME, items: ["eggs"] }]); - }); - - test("normalizes determiner-prefixed and trailing-list keys", () => { - expect( - coalesceStoredLists([ - { name: "a grocery", items: ["eggs"] }, - { name: "Grocery List", items: ["bread"] }, - { name: "the grocery list", items: ["milk"] }, - { name: "grocery lists", items: ["butter"] }, - { name: "grocery list list", items: ["cheese"] }, - ]), - ).toEqual([ - { - name: "grocery", - items: ["eggs", "bread", "milk", "butter", "cheese"], - }, - ]); - }); - - test("casefolds so mixed-case keys merge", () => { - expect( - coalesceStoredLists([ - { name: "Grocery", items: ["a"] }, - { name: "grocery", items: ["b"] }, - { name: "GROCERY", items: ["c"] }, - ]), - ).toEqual([{ name: "grocery", items: ["a", "b", "c"] }]); - }); - - test("keeps distinct real names including anaphora-leading and Unicode", () => { - expect( - coalesceStoredLists([ - { name: "shopping", items: ["soap"] }, - { name: "to do", items: ["call"] }, - { name: "Contoso grocery", items: ["badge"] }, - { name: "me time", items: ["read"] }, - { name: "US travel", items: ["visa"] }, - { name: "café", items: ["latte"] }, - { name: "買い物", items: ["tea"] }, - ]), - ).toEqual([ - { name: "shopping", items: ["soap"] }, - { name: "to do", items: ["call"] }, - { name: "contoso grocery", items: ["badge"] }, - { name: "me time", items: ["read"] }, - { name: "us travel", items: ["visa"] }, - { name: "café", items: ["latte"] }, - { name: "買い物", items: ["tea"] }, - ]); - }); - - test("merges salvaged items into existing recovered list", () => { - expect( - coalesceStoredLists([ - { name: "recovered", items: ["keep"] }, - { name: "the", items: ["from-the"] }, - ]), - ).toEqual([{ name: RECOVERED_LIST_NAME, items: ["keep", "from-the"] }]); - expect( - coalesceStoredLists([ - { name: RECOVERED_LIST_NAME, items: ["keep"] }, - { name: "list", items: ["from-list"] }, - ]), - ).toEqual([ - { name: RECOVERED_LIST_NAME, items: ["keep", "from-list"] }, - ]); - }); - - test("does not throw on null / non-string names; salvages items", () => { - const result = coalesceStoredLists([ - { name: null as unknown as string, items: ["from-null"] }, - { name: 42 as unknown as string, items: ["from-num"] }, - { name: undefined as unknown as string, items: ["from-undef"] }, - null as unknown as { name: string; items: string[] }, - { name: "grocery", items: ["milk"] }, - ]); - expect(result).toEqual( - expect.arrayContaining([ - { name: "grocery", items: ["milk"] }, - { - name: RECOVERED_LIST_NAME, - items: expect.arrayContaining([ - "from-null", - "from-num", - "from-undef", - ]), - }, - ]), - ); - expect(result).toHaveLength(2); - }); -}); - -describe("storedListsNeedRewrite", () => { - test("false for already-canonical store", () => { - expect( - storedListsNeedRewrite([ - { name: "grocery", items: ["milk"] }, - { name: "to do", items: ["call"] }, - { name: "café", items: ["latte"] }, - ]), - ).toBe(false); - }); - - test("false for steady-state salvage-only store (no rewrite loop)", () => { - expect( - storedListsNeedRewrite([ - { name: RECOVERED_LIST_NAME, items: ["milk"] }, - ]), - ).toBe(false); - expect( - storedListsNeedRewrite([ - { name: RECOVERED_LIST_NAME, items: ["a", "b"] }, - { name: "grocery", items: ["eggs"] }, - ]), - ).toBe(false); - }); - - test("true when salvage alias still uses non-canonical 'recovered' key", () => { - expect( - storedListsNeedRewrite([{ name: "recovered", items: ["milk"] }]), - ).toBe(true); - }); - - test("true for placeholder keys (even with items)", () => { - expect(storedListsNeedRewrite([{ name: "the", items: ["x"] }])).toBe( - true, - ); - expect(storedListsNeedRewrite([{ name: "list", items: [] }])).toBe( - true, - ); - expect(storedListsNeedRewrite([{ name: "mine", items: ["x"] }])).toBe( - true, - ); - }); - - test("true for unnormalized names", () => { - expect( - storedListsNeedRewrite([{ name: "a grocery", items: ["eggs"] }]), - ).toBe(true); - expect( - storedListsNeedRewrite([ - { name: "Grocery List", items: ["bread"] }, - ]), - ).toBe(true); - expect( - storedListsNeedRewrite([{ name: "grocery lists", items: ["x"] }]), - ).toBe(true); - expect( - storedListsNeedRewrite([ - { name: "grocery list list", items: ["x"] }, - ]), - ).toBe(true); - }); - - test("true when duplicate keys would merge", () => { - expect( - storedListsNeedRewrite([ - { name: "grocery", items: ["a"] }, - { name: "Grocery", items: ["b"] }, - ]), - ).toBe(true); - }); - - test("true for null / non-string names without throwing", () => { - expect( - storedListsNeedRewrite([ - { name: null as unknown as string, items: ["x"] }, - ]), - ).toBe(true); - expect( - storedListsNeedRewrite([ - { name: 1 as unknown as string, items: [] }, - ]), - ).toBe(true); - expect( - storedListsNeedRewrite([ - null as unknown as { name: string; items: string[] }, - ]), - ).toBe(true); - }); -}); From cdc601cd99a2f6dae1d83e6501d158c8cfcf1421 Mon Sep 17 00:00:00 2001 From: Dominic Nguyen Date: Thu, 20 Aug 2026 17:14:09 -0700 Subject: [PATCH 6/9] Make translation bench checkpoints crash-safe --- .../synthesizer/generationSupport.ts | 121 +++++++++++++----- .../synthesizer/jsonlCheckpoint.ts | 104 +++++++++++++++ ...nslationBench.checkpointPrimitives.spec.ts | 58 ++++++--- 3 files changed, 230 insertions(+), 53 deletions(-) create mode 100644 ts/packages/benchmarks/src/translationBench/synthesizer/jsonlCheckpoint.ts diff --git a/ts/packages/benchmarks/src/translationBench/synthesizer/generationSupport.ts b/ts/packages/benchmarks/src/translationBench/synthesizer/generationSupport.ts index 3ab3422564..23b91e6862 100644 --- a/ts/packages/benchmarks/src/translationBench/synthesizer/generationSupport.ts +++ b/ts/packages/benchmarks/src/translationBench/synthesizer/generationSupport.ts @@ -6,6 +6,11 @@ import fs from "node:fs"; import { z } from "zod"; import type { TranslationBenchBenchmarkSchema } from "./benchmark.js"; +import { + appendSyncedJsonlRecords, + initializeSyncedJsonlFile, + readRecoverableJsonlLines, +} from "./jsonlCheckpoint.js"; import { parseJsonText, parseVersionedWithZod, @@ -193,7 +198,7 @@ export function translationBenchResumeKey( ]); } -function validateRowShard( +function validateTranslationBenchCheckpointRowShard( row: TranslationBenchCheckpointRow, header: TranslationBenchCheckpointHeader, ): void { @@ -208,11 +213,14 @@ function validateRowShard( } } -function settingsEqual(left: unknown, right: unknown): boolean { +function translationBenchCheckpointSettingsEqual( + left: unknown, + right: unknown, +): boolean { return canonicalJson(left) === canonicalJson(right); } -function assertCompatibleHeaders( +function assertTranslationBenchCheckpointHeadersCompatible( actual: TranslationBenchCheckpointHeader, expected: TranslationBenchCheckpointHeader, ): void { @@ -221,7 +229,12 @@ function assertCompatibleHeaders( "Translation bench checkpoint run fingerprint is incompatible", ); } - if (!settingsEqual(actual.settings, expected.settings)) { + if ( + !translationBenchCheckpointSettingsEqual( + actual.settings, + expected.settings, + ) + ) { throw new Error( "Translation bench checkpoint settings are incompatible", ); @@ -245,19 +258,12 @@ export function createTranslationBenchRunFingerprint( export function readTranslationBenchCheckpoint( filePath: string, ): TranslationBenchCheckpoint { - const text = fs.readFileSync(filePath, "utf8"); - const lines = text.endsWith("\n") - ? text.slice(0, -1).split("\n") - : text.split("\n"); - if (lines.length === 0 || (lines.length === 1 && lines[0] === "")) { - throw new Error(`Translation bench checkpoint '${filePath}' is empty`); - } - if (lines.some((line) => line.trim().length === 0)) { + const lines = readRecoverableJsonlLines(filePath); + if (lines.length === 0 || lines.some((line) => line.trim().length === 0)) { throw new Error( - `Translation bench checkpoint '${filePath}' contains a blank line`, + `Translation bench checkpoint '${filePath}' is empty or contains a blank line`, ); } - const header = parseTranslationBenchCheckpointHeader( parseJsonText(lines[0]!, `checkpoint '${filePath}' line 1`), ); @@ -270,7 +276,7 @@ export function readTranslationBenchCheckpoint( `checkpoint '${filePath}' line ${index + 1}`, ), ); - validateRowShard(row, header); + validateTranslationBenchCheckpointRowShard(row, header); const key = translationBenchResumeKey(row); if (resumeKeys.has(key)) { throw new Error(`Duplicate translation bench resume key '${key}'`); @@ -281,6 +287,10 @@ export function readTranslationBenchCheckpoint( return { header, rows, resumeKeys }; } +/** + * Appends checkpoint rows for one owning writer. Concurrent writers are not + * supported; the caller must serialize all access to the checkpoint path. + */ export function appendTranslationBenchCheckpointRows( filePath: string, checkpointHeader: TranslationBenchCheckpointHeader, @@ -290,7 +300,7 @@ export function appendTranslationBenchCheckpointRows( const batchKeys = new Set(); const normalizedRows = rows.map((row) => { const parsed = parseTranslationBenchCheckpointRow(row); - validateRowShard(parsed, header); + validateTranslationBenchCheckpointRowShard(parsed, header); const key = translationBenchResumeKey(parsed); if (batchKeys.has(key)) { throw new Error(`Duplicate translation bench resume key '${key}'`); @@ -302,34 +312,23 @@ export function appendTranslationBenchCheckpointRows( let current: TranslationBenchCheckpoint; if (fs.existsSync(filePath)) { current = readTranslationBenchCheckpoint(filePath); - assertCompatibleHeaders(current.header, header); + assertTranslationBenchCheckpointHeadersCompatible( + current.header, + header, + ); } else { - try { - fs.writeFileSync(filePath, `${canonicalJson(header)}\n`, { - flag: "wx", - }); - current = { - header, - rows: [], - resumeKeys: new Set(), - }; - } catch (error) { - const code = (error as NodeJS.ErrnoException).code; - if (code !== "EEXIST") throw error; - current = readTranslationBenchCheckpoint(filePath); - assertCompatibleHeaders(current.header, header); - } + initializeSyncedJsonlFile(filePath, canonicalJson(header)); + current = { header, rows: [], resumeKeys: new Set() }; } - for (const key of batchKeys) { if (current.resumeKeys.has(key)) { throw new Error(`Duplicate translation bench resume key '${key}'`); } } if (normalizedRows.length > 0) { - fs.appendFileSync( + appendSyncedJsonlRecords( filePath, - normalizedRows.map((row) => `${canonicalJson(row)}\n`).join(""), + normalizedRows.map((row) => canonicalJson(row)), ); } return { @@ -339,6 +338,58 @@ export function appendTranslationBenchCheckpointRows( }; } +export function mergeTranslationBenchCheckpoints( + checkpoints: readonly TranslationBenchCheckpoint[], +): TranslationBenchCheckpoint { + if (checkpoints.length === 0) { + throw new Error("No translation bench checkpoints to merge"); + } + const first = checkpoints[0]!.header; + const byShard = new Map>(); + for (const checkpoint of checkpoints) { + assertTranslationBenchCheckpointHeadersCompatible(checkpoint.header, { + ...first, + shardIndex: checkpoint.header.shardIndex, + }); + if (byShard.has(checkpoint.header.shardIndex)) { + throw new Error( + `Duplicate translation bench checkpoint shard ${checkpoint.header.shardIndex}`, + ); + } + byShard.set(checkpoint.header.shardIndex, checkpoint); + } + const rows: TranslationBenchCheckpointRow[] = []; + const resumeKeys = new Set(); + for (let index = 0; index < first.shardCount; index++) { + const checkpoint = byShard.get(index); + if (checkpoint === undefined) { + throw new Error(`Missing checkpoint shard: ${index}`); + } + for (const row of checkpoint.rows) { + const normalized = parseTranslationBenchCheckpointRow(row); + validateTranslationBenchCheckpointRowShard( + normalized, + checkpoint.header, + ); + const key = translationBenchResumeKey(normalized); + if (resumeKeys.has(key)) { + throw new Error( + `Duplicate translation bench resume key '${key}'`, + ); + } + resumeKeys.add(key); + rows.push(normalized); + } + } + rows.sort((left, right) => + compareText( + translationBenchResumeKey(left), + translationBenchResumeKey(right), + ), + ); + return { header: byShard.get(0)!.header, rows, resumeKeys }; +} + export function getTranslationBenchCatalogCensus( schemas: readonly TranslationBenchBenchmarkSchema[], ): TranslationBenchCatalogCensus { diff --git a/ts/packages/benchmarks/src/translationBench/synthesizer/jsonlCheckpoint.ts b/ts/packages/benchmarks/src/translationBench/synthesizer/jsonlCheckpoint.ts new file mode 100644 index 0000000000..3459547ce1 --- /dev/null +++ b/ts/packages/benchmarks/src/translationBench/synthesizer/jsonlCheckpoint.ts @@ -0,0 +1,104 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import fs from "node:fs"; +import path from "node:path"; + +function fsyncDirectory(filePath: string): void { + if (process.platform === "win32") return; + const directory = fs.openSync(path.dirname(filePath), "r"); + try { + fs.fsyncSync(directory); + } finally { + fs.closeSync(directory); + } +} + +function writeAll(handle: number, buffer: Buffer, position: number): void { + let offset = 0; + while (offset < buffer.length) { + const written = fs.writeSync( + handle, + buffer, + offset, + buffer.length - offset, + position + offset, + ); + if (written === 0) throw new Error("Unable to complete JSONL write"); + offset += written; + } +} + +/** Initializes a JSONL file owned by a single writer. */ +export function initializeSyncedJsonlFile( + filePath: string, + firstRecord: string, +): void { + const temporaryPath = `${filePath}.tmp`; + let handle: number | undefined; + try { + handle = fs.openSync(temporaryPath, "w"); + fs.writeFileSync(handle, `${firstRecord}\n`, "utf8"); + fs.fsyncSync(handle); + } finally { + if (handle !== undefined) fs.closeSync(handle); + } + fs.renameSync(temporaryPath, filePath); + fsyncDirectory(filePath); +} + +export function readRecoverableJsonlLines(filePath: string): string[] { + const text = fs.readFileSync(filePath, "utf8"); + if (text.length === 0) return []; + const lines = text.endsWith("\n") + ? text.slice(0, -1).split("\n") + : text.split("\n"); + if (!text.endsWith("\n")) { + try { + JSON.parse(lines.at(-1)!); + } catch { + lines.pop(); + } + } + return lines; +} + +/** + * Repairs a torn final line and appends records for one owning writer. + * Concurrent calls for the same path are not supported. + */ +export function appendSyncedJsonlRecords( + filePath: string, + records: readonly string[], +): void { + if (records.length === 0) return; + const handle = fs.openSync(filePath, "r+"); + try { + const content = fs.readFileSync(handle); + let appendOffset = content.length; + let separator = ""; + if (appendOffset > 0 && content.at(-1) !== 0x0a) { + const lastNewline = content.lastIndexOf(0x0a); + if (lastNewline < 0) { + throw new Error( + `JSONL file '${filePath}' has no complete line`, + ); + } + const tail = content.subarray(lastNewline + 1).toString("utf8"); + try { + JSON.parse(tail); + separator = "\n"; + } catch { + appendOffset = lastNewline + 1; + fs.ftruncateSync(handle, appendOffset); + } + } + const payload = Buffer.from( + separator + records.map((record) => `${record}\n`).join(""), + ); + writeAll(handle, payload, appendOffset); + fs.fsyncSync(handle); + } finally { + fs.closeSync(handle); + } +} diff --git a/ts/packages/benchmarks/test/translationBench.checkpointPrimitives.spec.ts b/ts/packages/benchmarks/test/translationBench.checkpointPrimitives.spec.ts index 833cf7ed76..955d2a93ac 100644 --- a/ts/packages/benchmarks/test/translationBench.checkpointPrimitives.spec.ts +++ b/ts/packages/benchmarks/test/translationBench.checkpointPrimitives.spec.ts @@ -1,27 +1,49 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. - -import { describe, expect, it } from "@jest/globals"; - +import { afterAll, describe, expect, it } from "@jest/globals"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; import { + appendTranslationBenchCheckpointRows, createTranslationBenchRunFingerprint, - getTranslationBenchShardIndex, - splitTranslationBenchCheckpointLines, -} from "../src/translationBench/runner/scale.js"; + readTranslationBenchCheckpoint, + type TranslationBenchCheckpointHeader, + type TranslationBenchCheckpointRow, +} from "../src/translationBench/synthesizer/generationSupport.js"; -describe("translation bench checkpoint primitives", () => { - it("uses canonical fingerprints and stable shards", () => { - expect(createTranslationBenchRunFingerprint({ b: 2, a: 1 })).toBe( - createTranslationBenchRunFingerprint({ a: 1, b: 2 }), - ); - expect(getTranslationBenchShardIndex("case-1", 8)).toBe( - getTranslationBenchShardIndex("case-1", 8), - ); - }); +const directory = fs.mkdtempSync(path.join(os.tmpdir(), "translation-bench-")); +afterAll(() => fs.rmSync(directory, { recursive: true, force: true })); +const header: TranslationBenchCheckpointHeader = { + kind: "translation-bench-checkpoint", + version: 1, + runFingerprint: createTranslationBenchRunFingerprint({ run: 1 }), + settings: { model: "test" }, + shardIndex: 0, + shardCount: 1, +}; +const row = (caseId: string): TranslationBenchCheckpointRow => ({ + kind: "translation-bench-row", + version: 1, + phase: "generate", + model: "test", + scenario: "default", + caseId, + value: caseId, +}); - it("drops only an incomplete trailing JSONL row", () => { +describe("translation bench checkpoints", () => { + it("recovers a torn final row before appending", () => { + const checkpointPath = path.join(directory, "checkpoint.jsonl"); + appendTranslationBenchCheckpointRows(checkpointPath, header, [ + row("1"), + ]); + fs.appendFileSync(checkpointPath, '{"kind":"translation-bench-row"'); + appendTranslationBenchCheckpointRows(checkpointPath, header, [ + row("2"), + ]); expect( - splitTranslationBenchCheckpointLines('{"header":1}\n{"row":'), - ).toEqual(['{"header":1}']); + readTranslationBenchCheckpoint(checkpointPath).rows, + ).toEqual([row("1"), row("2")]); }); }); From fddbcca01e56e487957dab52a77f184e550b93ae Mon Sep 17 00:00:00 2001 From: typeagent-bot Date: Fri, 21 Aug 2026 00:48:49 +0000 Subject: [PATCH 7/9] docs: regenerate README.AUTOGEN.md, command reference, and action browser --- ts/packages/benchmarks/README.AUTOGEN.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ts/packages/benchmarks/README.AUTOGEN.md b/ts/packages/benchmarks/README.AUTOGEN.md index 825ed00a37..084c5705b5 100644 --- a/ts/packages/benchmarks/README.AUTOGEN.md +++ b/ts/packages/benchmarks/README.AUTOGEN.md @@ -3,7 +3,7 @@ - + # @typeagent/benchmarks — AI-generated documentation @@ -52,10 +52,10 @@ _None._ - [./src/core/prices.ts](./src/core/prices.ts) - [./src/core/rateLimiter.ts](./src/core/rateLimiter.ts) - [./src/core/tokenEstimate.ts](./src/core/tokenEstimate.ts) -- _…and 44 more under `./src/`._ +- _…and 45 more under `./src/`._ --- -_Auto-generated against commit `95c2a1d9ba80426f522f7ece727da39f8a577d9e` on `2026-08-19T17:31:21.317Z` by `docs-generate.yml`. Links validated at that commit; the working tree may have drifted by up to 24h. Re-run `pnpm --filter @typeagent/benchmarks docs:verify-links` to spot-check._ +_Auto-generated against commit `cdc601cd99a2f6dae1d83e6501d158c8cfcf1421` on `2026-08-21T00:46:31.017Z` by `docs-generate.yml`. Links validated at that commit; the working tree may have drifted by up to 24h. Re-run `pnpm --filter @typeagent/benchmarks docs:verify-links` to spot-check._ From 025b7dab33336ea3819cb1f927164043f18fb432 Mon Sep 17 00:00:00 2001 From: Dominic Nguyen Date: Fri, 21 Aug 2026 10:16:36 -0700 Subject: [PATCH 8/9] refactor: reuse checkpoint line parser --- .../synthesizer/jsonlCheckpoint.ts | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/ts/packages/benchmarks/src/translationBench/synthesizer/jsonlCheckpoint.ts b/ts/packages/benchmarks/src/translationBench/synthesizer/jsonlCheckpoint.ts index 3459547ce1..51f1b0bf6f 100644 --- a/ts/packages/benchmarks/src/translationBench/synthesizer/jsonlCheckpoint.ts +++ b/ts/packages/benchmarks/src/translationBench/synthesizer/jsonlCheckpoint.ts @@ -4,6 +4,8 @@ import fs from "node:fs"; import path from "node:path"; +import { splitTranslationBenchCheckpointLines } from "../runner/scale.js"; + function fsyncDirectory(filePath: string): void { if (process.platform === "win32") return; const directory = fs.openSync(path.dirname(filePath), "r"); @@ -48,19 +50,9 @@ export function initializeSyncedJsonlFile( } export function readRecoverableJsonlLines(filePath: string): string[] { - const text = fs.readFileSync(filePath, "utf8"); - if (text.length === 0) return []; - const lines = text.endsWith("\n") - ? text.slice(0, -1).split("\n") - : text.split("\n"); - if (!text.endsWith("\n")) { - try { - JSON.parse(lines.at(-1)!); - } catch { - lines.pop(); - } - } - return lines; + return splitTranslationBenchCheckpointLines( + fs.readFileSync(filePath, "utf8"), + ); } /** From 170503010f1f1bbfc9d95ca929ed12291c5d82e5 Mon Sep 17 00:00:00 2001 From: Dominic Nguyen Date: Fri, 21 Aug 2026 10:42:15 -0700 Subject: [PATCH 9/9] Remove unused checkpoint merge helper --- .../synthesizer/generationSupport.ts | 52 ------------------- 1 file changed, 52 deletions(-) diff --git a/ts/packages/benchmarks/src/translationBench/synthesizer/generationSupport.ts b/ts/packages/benchmarks/src/translationBench/synthesizer/generationSupport.ts index 23b91e6862..880fba5495 100644 --- a/ts/packages/benchmarks/src/translationBench/synthesizer/generationSupport.ts +++ b/ts/packages/benchmarks/src/translationBench/synthesizer/generationSupport.ts @@ -338,58 +338,6 @@ export function appendTranslationBenchCheckpointRows( }; } -export function mergeTranslationBenchCheckpoints( - checkpoints: readonly TranslationBenchCheckpoint[], -): TranslationBenchCheckpoint { - if (checkpoints.length === 0) { - throw new Error("No translation bench checkpoints to merge"); - } - const first = checkpoints[0]!.header; - const byShard = new Map>(); - for (const checkpoint of checkpoints) { - assertTranslationBenchCheckpointHeadersCompatible(checkpoint.header, { - ...first, - shardIndex: checkpoint.header.shardIndex, - }); - if (byShard.has(checkpoint.header.shardIndex)) { - throw new Error( - `Duplicate translation bench checkpoint shard ${checkpoint.header.shardIndex}`, - ); - } - byShard.set(checkpoint.header.shardIndex, checkpoint); - } - const rows: TranslationBenchCheckpointRow[] = []; - const resumeKeys = new Set(); - for (let index = 0; index < first.shardCount; index++) { - const checkpoint = byShard.get(index); - if (checkpoint === undefined) { - throw new Error(`Missing checkpoint shard: ${index}`); - } - for (const row of checkpoint.rows) { - const normalized = parseTranslationBenchCheckpointRow(row); - validateTranslationBenchCheckpointRowShard( - normalized, - checkpoint.header, - ); - const key = translationBenchResumeKey(normalized); - if (resumeKeys.has(key)) { - throw new Error( - `Duplicate translation bench resume key '${key}'`, - ); - } - resumeKeys.add(key); - rows.push(normalized); - } - } - rows.sort((left, right) => - compareText( - translationBenchResumeKey(left), - translationBenchResumeKey(right), - ), - ); - return { header: byShard.get(0)!.header, rows, resumeKeys }; -} - export function getTranslationBenchCatalogCensus( schemas: readonly TranslationBenchBenchmarkSchema[], ): TranslationBenchCatalogCensus {