Skip to content

Commit 06ec4d4

Browse files
GeorgeNgMsftclaude
andcommitted
Merge main into dev/georgeng/clientio_improvements_1
Resolved conflicts in favour of explicit cancelInteraction approach over the auto-cancel-on-disconnect (cancelByConnection) approach from #2178. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2 parents a0edc0a + 2f33b3b commit 06ec4d4

File tree

58 files changed

+2318
-500
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

58 files changed

+2318
-500
lines changed

TODO.md

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -64,34 +64,18 @@ This file collates all TODO comments found across the repository, organized by t
6464

6565
| File | Line | TODO | Effort | Feasibility | Scope | Recommendation | Needs Human? |
6666
|------|------|------|--------|-------------|-------|----------------|--------------|
67-
| `ts/examples/cacheRESTEndpoint/src/index.ts` | 51 | actionName should match interface name | Low | High | Local | Fix | No |
68-
| `ts/examples/cacheRESTEndpoint/src/index.ts` | 56 | actually parse the schema instead of using regex | Medium | High | Local | Fix | No |
69-
| `ts/examples/chat/src/codeChat/commandTransformer.ts` | 59 | Validate that result.data is a valid NamedArgs? | Low | High | Local | Fix | No |
7067
| `ts/examples/chat/src/codeChat/commandTransformer.ts` | 170 | the same for args (currently not used by code chat) | Low | Medium | Local | No Fix | Yes |
71-
| `ts/examples/commandHistogram/src/main.ts` | 65 | move DB name to .env/config? | Low | High | Local | Fix | No |
72-
| `ts/examples/commandHistogram/src/main.ts` | 67 | move collection name to .env/config? | Low | High | Local | Fix | No |
7368
| `ts/examples/docuProc/src/pdfImporter.ts` | 210 | Try pre-computing embeddings in parallel to fill the embeddings cache (is that cache safe?) | Medium | Medium | Local | Fix | Yes |
74-
| `ts/examples/docuProc/src/pdfImporter.ts` | 344 | Therefore remove. | Low | High | Local | Fix | No |
7569
| `ts/examples/docuProc/src/pdfQNAInteractiveApp.ts` | 1024 | Allow for multiple concurrent sessions. | High | Medium | Component | No Fix | Yes |
7670
| `ts/examples/docuProc/src/pdfQNAInteractiveApp.ts` | 1031 | Cut off by total size, not count. | Low | High | Local | Fix | No |
7771
| `ts/examples/docuProc/src/pdfQNAInteractiveApp.ts` | 1151 | limit how much we write per blob too (if there are multiple). | Low | High | Local | Fix | No |
7872
| `ts/examples/docuProc/src/pdfQNAInteractiveApp.ts` | 1178 | Colorize code blocks. | Low | High | Local | Fix | No |
7973
| `ts/examples/memoryProviders/src/sqlite/keyValueTable.ts` | 195 | support | Medium | Medium | Component | Fix | Yes |
80-
| `ts/examples/memoryProviders/src/sqlite/textTable.ts` | 333 | use IN clause | Low | High | Local | Fix | No |
81-
| `ts/examples/memoryProviders/src/sqlite/textTable.ts` | 381 | use a JOIN | Low | High | Local | Fix | No |
8274
| `ts/examples/memoryProviders/src/sqlite/textTable.ts` | 511 | optimize by lowering into DB if possible | Medium | Medium | Local | Fix | Yes |
8375
| `ts/examples/memoryProviders/src/sqlite/textTable.ts` | 613 | Optimize | Medium | Medium | Local | Fix | Yes |
84-
| `ts/examples/memoryProviders/src/sqlite/vectorTable.ts` | 70 | ID generation | Low | High | Local | Fix | No |
8576
| `ts/examples/schemaStudio/src/schemaCommands.ts` | 94 | Generating settings command schemas... | Medium | Medium | Component | Fix | Yes |
86-
| `ts/examples/schemaStudio/src/urlCommands.ts` | 88 | handle redirects + default parameters, etc. | Medium | Medium | Local | Fix | No |
87-
| `ts/examples/spelunker/src/main.ts` | 44 | switch to whatever interactive-app uses to parse the command line? | Low | High | Local | Fix | No |
88-
| `ts/examples/spelunker/src/main.ts` | 77 | Use a proper command-line parser? | Low | High | Local | Fix | No |
89-
| `ts/examples/spelunker/src/pythonChunker.ts` | 81 | validate that JSON matches our schema. | Low | High | Local | Fix | No |
9077
| `ts/examples/spelunker/src/pythonImporter.ts` | 4 | Most of this is not Python specific; generalize to other languages. | High | Medium | Component | Fix | Yes |
91-
| `ts/examples/spelunker/src/pythonImporter.ts` | 71 | Make generic over languages | High | Medium | Component | Fix | No |
92-
| `ts/examples/spelunker/src/pythonImporter.ts` | 128 | Make this a function argument | Low | High | Local | Fix | No |
9378
| `ts/examples/spelunker/src/pythonImporter.ts` | 181 | Try pre-computing embeddings in parallel to fill the embeddings cache (is that cache safe?) | Medium | Medium | Local | Fix | Yes |
94-
| `ts/examples/spelunker/src/pythonImporter.ts` | 302 | Therefore remove. | Low | High | Local | Fix | No |
9579
| `ts/examples/spelunker/src/queryInterface.ts` | 905 | Allow for multiple concurrent sessions. | High | Medium | Component | Fix | No |
9680
| `ts/examples/spelunker/src/queryInterface.ts` | 912 | Cut off by total size, not count. | Low | High | Local | Fix | No |
9781
| `ts/examples/spelunker/src/queryInterface.ts` | 1035 | limit how much we write per blob too (if there are multiple). | Low | High | Local | Fix | No |
@@ -123,15 +107,8 @@ This file collates all TODO comments found across the repository, organized by t
123107
| `ts/packages/agentSdk/src/agentInterface.ts` | 57 | enable non-stringify pas content. | Medium | Medium | Cross-cutting | Fix | Yes |
124108
| `ts/packages/agentSdk/src/agentInterface.ts` | 234 | only utf8 & base64 is supported for now. | Medium | Medium | Component | Fix | No |
125109
| `ts/packages/agentSdkWrapper/src/webtask/tracing/types.ts` | 138 | Phase 2: Extract key elements from HTML | High | Low | Component | No Fix | Yes |
126-
| `ts/packages/agents/browser/src/extension/serviceWorker/index.ts` | 347 | sniffing the RPC call arguments. Fix typing. | Low | High | Local | Fix | No |
127-
| `ts/packages/agents/browser/src/extension/views/entityGraphView.ts` | 1341 | limit this to "contains" relationships | Low | High | Local | Fix | No |
128110
| `ts/packages/agents/browser/src/extension/views/extensionServiceBase.ts` | 579 | remove "type" from this dictionary. That will remove the need to wrap these values in a "parameters" object | Medium | Medium | Component | Fix | No |
129-
| `ts/packages/agents/browser/src/extension/views/macrosLibrary.ts` | 650 | Implement macro editing | Medium | High | Component | Fix | No |
130111
| `ts/packages/agents/browser/src/extension/views/topicGraphView.ts` | 296 | Implement topic viewport neighborhood functionality | High | Medium | Component | Fix | Yes |
131-
| `ts/packages/agents/browser/src/views/client/pdf/core/pdfViewer.ts` | 245 | Implement text search functionality | Medium | High | Component | Fix | No |
132-
| `ts/packages/agents/browser/src/views/client/pdf/core/pdfViewer.ts` | 258 | Implement find next | Low | High | Local | Fix | No |
133-
| `ts/packages/agents/browser/src/views/client/pdf/core/pdfViewer.ts` | 266 | Implement find previous | Low | High | Local | Fix | No |
134-
| `ts/packages/agents/browser/src/views/client/pdf/core/pdfViewer.ts` | 282 | Implement download functionality | Medium | High | Component | Fix | No |
135112
| `ts/packages/agents/calendar/src/calendarActionHandlerV3.ts` | 1120 | Implement sophisticated date parsing | Medium | High | Local | Fix | No |
136113
| `ts/packages/agents/calendar/src/calendarActionHandlerV3.ts` | 1126 | Implement sophisticated time parsing | Medium | High | Local | Fix | No |
137114
| `ts/packages/agents/desktop/src/connector.ts` | 127 | add shared agent storage or known storage location (requires permissions, trusted agents, etc.) | High | Low | Cross-cutting | No Fix | Yes |
@@ -161,7 +138,6 @@ This file collates all TODO comments found across the repository, organized by t
161138
| `ts/packages/agents/spelunker/src/embeddings.ts` | 47 | tune | Medium | Medium | Local | No Fix | Yes |
162139
| `ts/packages/agents/spelunker/src/embeddings.ts` | 186 | Fix this | Medium | Medium | Local | Fix | Yes |
163140
| `ts/packages/agents/spelunker/src/eval.ts` | 27 | Read this from a file that can be edited before each run | Low | High | Local | Fix | No |
164-
| `ts/packages/agents/spelunker/src/pythonChunker.ts` | 51 | validate that JSON matches our schema. | Low | High | Local | Fix | No |
165141
| `ts/packages/agents/spelunker/src/searchCode.ts` | 237 | tune | Medium | Medium | Local | No Fix | Yes |
166142
| `ts/packages/agents/spelunker/src/searchCode.ts` | 238 | tune | Medium | Medium | Local | Fix | Yes |
167143
| `ts/packages/agents/spelunker/src/searchCode.ts` | 268 | tune | Medium | Medium | Local | Fix | Yes |
@@ -174,8 +150,6 @@ This file collates all TODO comments found across the repository, organized by t
174150
| `ts/packages/agents/spelunker/src/searchCode.ts` | 458 | Use appendDisplay (requires passing actionContext) | Low | Medium | Local | Fix | No |
175151
| `ts/packages/agents/spelunker/src/spelunkerActionHandler.ts` | 89 | What other standard functions could be handy here? | Medium | Low | Component | No Fix | Yes |
176152
| `ts/packages/agents/spelunker/src/summarizing.ts` | 42 | Prompt engineering | High | Medium | Local | Fix | Yes |
177-
| `ts/packages/agents/spelunker/src/summarizing.ts` | 160 | Remove export once we're using summaries again. | Low | High | Local | Fix | No |
178-
| `ts/packages/agents/spelunker/src/summarizing.ts` | 178 | Make the values two elements, comment start and comment end | Low | High | Local | Fix | No |
179153
| `ts/packages/agents/spelunker/src/typescriptChunker.ts` | 198 | Move to caller? | Low | High | Local | Fix | No |
180154
| `ts/packages/agents/video/src/videoActionHandler.ts` | 54 | dynamic duration | Medium | Medium | Local | Fix | Yes |
181155
| `ts/packages/agents/weather/src/weatherActionHandler.ts` | 227 | Add more sophisticated validation: | Medium | High | Local | Fix | No |

ts/examples/cacheRESTEndpoint/src/index.ts

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,13 @@ import {
1111
import { existsSync } from "fs";
1212
import path from "path";
1313
import { NodeType, SchemaParser } from "@typeagent/action-schema";
14-
import { NameValue } from "typeagent";
14+
15+
type SchemaEntry = {
16+
name: string;
17+
camelCaseName: string;
18+
value: string;
19+
id?: string;
20+
};
1521

1622
// Create an instance of an Express application
1723
const app = express();
@@ -26,7 +32,7 @@ const cacheFile = "data/v5_sample.json";
2632
let agentCache: AgentCache;
2733

2834
// load the schema
29-
let schemas: NameValue[] = [];
35+
let schemas: SchemaEntry[] = [];
3036
const schemaFile: string = `../../packages/agents/settings/src/settingsActionSchema.ts`;
3137

3238
// Set up a route for the root URL ('/')
@@ -48,17 +54,11 @@ app.get("/", async (req: Request, res: Response) => {
4854
matches.forEach((construction) => {
4955
// add the id of the schema (if it exists) to the action
5056
construction.match.actions.forEach((a) => {
51-
// TODO: actionName should match interface name
52-
const schema = schemas.find((s) =>
53-
s.name.startsWith(a.action.actionName),
57+
const schema = schemas.find(
58+
(s) => s.camelCaseName === a.action.actionName,
5459
);
5560
if (schema) {
56-
// TODO: actually parse the schema instead of using regex
57-
const idMatch = schema.value.match(/id:\s*([^;]+);/);
58-
const idValue = idMatch
59-
? idMatch[1].trim().replaceAll('"', "")
60-
: undefined;
61-
(a.action as any).id = idValue;
61+
(a.action as any).id = schema.id;
6262
}
6363
});
6464

@@ -100,11 +100,11 @@ app.listen(port, async () => {
100100
}
101101
});
102102

103-
function getActionSchema(filePath: string): NameValue[] {
103+
function getActionSchema(filePath: string): SchemaEntry[] {
104104
const schema = new SchemaParser();
105105
schema.loadSchema(filePath);
106106
const types = schema.actionTypeNames();
107-
const schemas: NameValue[] = [];
107+
const result: SchemaEntry[] = [];
108108
for (const type of types) {
109109
const node = schema.openActionNode(type);
110110
let schemaText = node?.symbol.value!;
@@ -113,7 +113,18 @@ function getActionSchema(filePath: string): NameValue[] {
113113
schemaText += "\n\n" + subType.symbol.value;
114114
}
115115
}
116-
schemas.push({ name: node?.symbol.name!, value: schemaText });
116+
const idChild = node!.children.find((c) => c.symbol.name === "id");
117+
const rawId = idChild?.symbol.value ?? "";
118+
const idValue = rawId ? JSON.parse(rawId.trim()) : undefined;
119+
const interfaceName = node?.symbol.name!;
120+
const camelCaseName =
121+
interfaceName.charAt(0).toLowerCase() + interfaceName.slice(1);
122+
result.push({
123+
name: interfaceName,
124+
camelCaseName,
125+
value: schemaText,
126+
id: idValue,
127+
});
117128
}
118-
return schemas;
129+
return result;
119130
}

ts/examples/chat/src/codeChat/commandTransformer.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,14 @@ export function createCommandTransformer(
5656
return undefined;
5757
} else {
5858
// console.log("[Success:]", JSON.stringify(result, null, 2));
59-
// TODO: Validate that result.data is a valid NamedArgs?
59+
if (
60+
typeof result.data !== "object" ||
61+
result.data === null ||
62+
typeof (result.data as NamedArgs).name !== "string"
63+
) {
64+
console.log("[Error:] result.data is not a valid NamedArgs");
65+
return undefined;
66+
}
6067
return result.data;
6168
}
6269
}

ts/examples/commandHistogram/src/main.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,10 @@ async function queryMongoDB(query: object) {
6262

6363
try {
6464
await client.connect();
65-
// TODO: move DB name to .env/config?
66-
const database = client.db("telemetrydb");
67-
// TODO: move collection name to .env/config?
68-
const collection = database.collection("dispatcherlogs");
65+
const database = client.db(process.env["DB_NAME"] ?? "telemetrydb");
66+
const collection = database.collection(
67+
process.env["COLLECTION_NAME"] ?? "dispatcherlogs",
68+
);
6969

7070
const documents = await collection.find(query).toArray();
7171
return documents;

ts/examples/docuProc/src/pdfImporter.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -340,8 +340,3 @@ export async function exponentialBackoff<T extends any[], R>(
340340
}
341341
}
342342
}
343-
344-
// Apply URL escaping to key. NOTE: Currently unused. TODO: Therefore remove.
345-
export function sanitizeKey(key: string): string {
346-
return encodeURIComponent(key).replace(/%20/g, "+"); // Encode spaces as plus, others as %xx.
347-
}

ts/examples/docuProc/src/pdfQNAInteractiveApp.ts

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1028,7 +1028,18 @@ async function findRecentAnswers(
10281028
}
10291029
// Assume the name field (the internal key) is a timestamp.
10301030
recentAnswers.sort((a, b) => b.name.localeCompare(a.name));
1031-
recentAnswers.splice(20); // TODO: Cut off by total size, not count.
1031+
// Cut off by total size (character count), not count.
1032+
let totalSize = 0;
1033+
const maxTotalSize = 100_000;
1034+
let cutoff = recentAnswers.length;
1035+
for (let i = 0; i < recentAnswers.length; i++) {
1036+
totalSize += JSON.stringify(recentAnswers[i]).length;
1037+
if (totalSize > maxTotalSize) {
1038+
cutoff = i;
1039+
break;
1040+
}
1041+
}
1042+
recentAnswers.splice(cutoff);
10321043
recentAnswers.reverse(); // Most recent last.
10331044
return recentAnswers;
10341045
}
@@ -1148,7 +1159,6 @@ export function writeChunkLines(
11481159
io: iapp.InteractiveIo,
11491160
lineBudget = 10,
11501161
): void {
1151-
// TODO: limit how much we write per blob too (if there are multiple).
11521162
writeMain(io, `\nCHUNK ID: ${chunk.id}`);
11531163
const formatContent = (content: string | string[]): string => {
11541164
if (Array.isArray(content)) {
@@ -1164,9 +1174,18 @@ export function writeChunkLines(
11641174
return content;
11651175
};
11661176

1167-
if (chunk.blobs[0].content) {
1168-
const content = formatContent(chunk.blobs[0].content);
1169-
writeMain(io, `Content: ${content}`);
1177+
const perBlobBudget = Math.ceil(lineBudget / chunk.blobs.length);
1178+
for (const blob of chunk.blobs) {
1179+
if (!blob.content) continue;
1180+
const content = formatContent(blob.content);
1181+
const blobLines = content.split(/\r?\n/);
1182+
const limit = Math.min(blobLines.length, perBlobBudget);
1183+
for (let i = 0; i < limit; i++) {
1184+
writeMain(io, blobLines[i]);
1185+
}
1186+
if (blobLines.length > perBlobBudget) {
1187+
writeNote(io, " ...");
1188+
}
11701189
}
11711190
}
11721191

@@ -1175,10 +1194,9 @@ export function wordWrap(text: string, wrapLength: number = 80): string {
11751194
const lines: string[] = [];
11761195
const prefixRegex = /^\s*((-|\*|\d+\.)\s+)?/;
11771196
for (let line of text.split(/[ ]*\r?\n/)) {
1178-
if (line.startsWith("```")) inCodeBlock = !inCodeBlock; // TODO: Colorize code blocks.
1179-
if (line.length <= wrapLength || inCodeBlock) {
1180-
// The whole line is deemed to fit.
1181-
lines.push(line);
1197+
if (line.startsWith("```")) inCodeBlock = !inCodeBlock;
1198+
if (inCodeBlock || line.startsWith("```")) {
1199+
lines.push(chalk.cyan(line));
11821200
continue;
11831201
}
11841202
// We must try to break.

ts/examples/memoryProviders/src/sqlite/textTable.ts

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -330,9 +330,21 @@ export async function createTextIndex<
330330
}
331331

332332
function getIds(texts: string[]): Promise<(TTextId | undefined)[]> {
333-
// TODO: use IN clause
333+
if (texts.length === 0) {
334+
return Promise.resolve([]);
335+
}
336+
const inClause = sql_makeInClause(texts);
337+
const rows = db
338+
.prepare(
339+
`SELECT stringId, value FROM ${textTable.tableName} WHERE value IN (${inClause})`,
340+
)
341+
.all() as StringTableRow[];
342+
const idByText = new Map<string, number>();
343+
for (const row of rows) {
344+
idByText.set(row.value, row.stringId);
345+
}
334346
return Promise.resolve(
335-
texts.map((t) => serializer.serialize(textTable.getId(t))),
347+
texts.map((t) => serializer.serialize(idByText.get(t))),
336348
);
337349
}
338350

@@ -378,13 +390,25 @@ export async function createTextIndex<
378390
values: string[],
379391
join?: string,
380392
): IterableIterator<ScoredItem<TSourceId>> {
381-
// TODO: use a JOIN
382-
const textIds = [...textTable.getIds(values)];
383-
const hits = postingsTable.getHits(textIds, join);
384-
if (hits) {
385-
for (const hit of hits) {
386-
yield hit;
387-
}
393+
if (values.length === 0) {
394+
return;
395+
}
396+
const inClause = sql_makeInClause(values);
397+
const innerJoin = `INNER JOIN ${textTable.tableName} ON keyId = stringId`;
398+
const sql = join
399+
? `SELECT valueId AS item, count(*) AS score FROM ${postingsTable.tableName}
400+
${innerJoin}
401+
${join} AND ${textTable.tableName}.value IN (${inClause})
402+
GROUP BY valueId
403+
ORDER BY score DESC`
404+
: `SELECT valueId AS item, count(*) AS score FROM ${postingsTable.tableName}
405+
${innerJoin}
406+
WHERE ${textTable.tableName}.value IN (${inClause})
407+
GROUP BY valueId
408+
ORDER BY score DESC`;
409+
const stmt = db.prepare(sql);
410+
for (const row of stmt.iterate()) {
411+
yield row as ScoredItem<TSourceId>;
388412
}
389413
}
390414

ts/examples/memoryProviders/src/sqlite/vectorTable.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the MIT License.
33

44
import * as sqlite from "better-sqlite3";
5+
import { randomUUID } from "node:crypto";
56
import {
67
Embedding,
78
ScoredItem,
@@ -40,6 +41,9 @@ export function createVectorTable<TKeyId extends ValueType = string>(
4041
const sql_add = db.prepare(
4142
`INSERT OR IGNORE INTO ${tableName} (keyId, embedding) VALUES (?, ?)`,
4243
);
44+
const sql_add_auto = db.prepare(
45+
`INSERT INTO ${tableName} (embedding) VALUES (?)`,
46+
);
4347
const sql_remove = db.prepare(`DELETE FROM ${tableName} WHERE keyId = ?`);
4448
const sql_all = db.prepare(`SELECT * from ${tableName}`);
4549

@@ -67,8 +71,16 @@ export function createVectorTable<TKeyId extends ValueType = string>(
6771

6872
function put(value: Embedding, id?: TKeyId | undefined): Promise<TKeyId> {
6973
if (id === undefined) {
70-
// TODO: ID generation
71-
throw Error("id required");
74+
const buffer = Buffer.from(value.buffer);
75+
if (keyType === "INTEGER") {
76+
const result = sql_add_auto.run(buffer);
77+
return Promise.resolve(
78+
Number(result.lastInsertRowid) as TKeyId,
79+
);
80+
}
81+
const newId = randomUUID() as TKeyId;
82+
sql_add.run(newId, buffer);
83+
return Promise.resolve(newId);
7284
}
7385
const buffer = Buffer.from(value.buffer);
7486
sql_add.run(id, buffer);

0 commit comments

Comments
 (0)