Skip to content

Commit

Permalink
[chore]: Make TSConfig stricter and fix as many errors as possible (#…
Browse files Browse the repository at this point in the history
…1596)

* Tighten TSConfig; fix new errors

* Fix tests; required re-adding ts-node

* Checkpoint. Stuck on @types/json-schema

* Fix ollama and core

* cleanup

* Move ts-node back to root

* pnpm lock updated

* Remove skipLibCheck from firebase; google-cloud still uses outdated long

* Add back gablorkenTool

* Revert "Remove skipLibCheck from firebase; google-cloud still uses outdated long"

This reverts commit 0ba65df.
  • Loading branch information
inlined authored Jan 16, 2025
1 parent 10412f8 commit 6b541a3
Show file tree
Hide file tree
Showing 31 changed files with 402 additions and 142 deletions.
4 changes: 2 additions & 2 deletions js/ai/src/formats/types.d.ts → js/ai/src/formats/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@ import { GenerateResponseChunk } from '../generate.js';
import { Message } from '../message.js';
import { ModelRequest } from '../model.js';

type OutputContentTypes = 'application/json' | 'text/plain';
export type OutputContentTypes = 'application/json' | 'text/plain';

export interface Formatter<O = unknown, CO = unknown> {
name: string;
config: ModelRequest['output'];
handler: (schema?: JSONSchema) => {
parseMessage(message: Message): O;
parseChunk?: (chunk: GenerateResponseChunk, cursor?: CC) => CO;
parseChunk?: (chunk: GenerateResponseChunk) => CO;
instructions?: string;
};
}
20 changes: 9 additions & 11 deletions js/ai/src/generate/action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,17 +152,15 @@ async function generate(
streamingCallback
? (chunk: GenerateResponseChunkData) => {
// Store accumulated chunk data
if (streamingCallback) {
streamingCallback!(
new GenerateResponseChunk(chunk, {
index: 0,
role: 'model',
previousChunks: accumulatedChunks,
parser: resolvedFormat?.handler(request.output?.schema)
.parseChunk,
})
);
}
streamingCallback(
new GenerateResponseChunk(chunk, {
index: 0,
role: 'model',
previousChunks: accumulatedChunks,
parser: resolvedFormat?.handler(request.output?.schema)
.parseChunk,
})
);
accumulatedChunks.push(chunk);
}
: undefined,
Expand Down
8 changes: 0 additions & 8 deletions js/ai/src/generate/response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,6 @@ export class GenerateResponse<O = unknown> implements ModelResponseData {
this.request = options?.request;
}

private get assertMessage(): Message<O> {
if (!this.message)
throw new Error(
'Operation could not be completed because the response does not contain a generated message.'
);
return this.message;
}

/**
* Throws an error if the response does not contain valid output.
*/
Expand Down
5 changes: 4 additions & 1 deletion js/ai/src/model/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
*/

import { Document } from '../document.js';
import {
import type {
MediaPart,
MessageData,
ModelInfo,
Expand Down Expand Up @@ -129,12 +129,15 @@ export function validateSupport(options: {
};
}

// N.B. Figure out why array.findLast isn't available despite setting target
// to ES2022 (Node 16.14.0)
function lastUserMessage(messages: MessageData[]) {
for (let i = messages.length - 1; i >= 0; i--) {
if (messages[i].role === 'user') {
return messages[i];
}
}
return undefined;
}

/**
Expand Down
6 changes: 3 additions & 3 deletions js/ai/src/testing/model-tester.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,9 @@ const tests: Record<string, TestCase> = {
],
});

const want = '';
const want = /plus/i;
const got = response.text.trim();
assert.match(got, /plus/i);
assert.match(got, want);
},
history: async (registry: Registry, model: string) => {
const resolvedModel = (await registry.lookupAction(
Expand Down Expand Up @@ -155,7 +155,7 @@ export async function testModels(
registry: Registry,
models: string[]
): Promise<TestReport> {
const gablorkenTool = defineTool(
defineTool(
registry,
{
name: 'gablorkenTool',
Expand Down
1 change: 0 additions & 1 deletion js/ai/tests/formats/array_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,6 @@ describe('arrayFormat', () => {
it(st.desc, () => {
const parser = arrayFormatter.handler();
const chunks: GenerateResponseChunkData[] = [];
let lastCursor = 0;

for (const chunk of st.chunks) {
const newChunk: GenerateResponseChunkData = {
Expand Down
4 changes: 1 addition & 3 deletions js/ai/tests/formats/json_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,16 +64,14 @@ describe('jsonFormat', () => {
it(st.desc, () => {
const parser = jsonFormatter.handler();
const chunks: GenerateResponseChunkData[] = [];
let lastCursor = '';

for (const chunk of st.chunks) {
const newChunk: GenerateResponseChunkData = {
content: [{ text: chunk.text }],
};

const result = parser.parseChunk!(
new GenerateResponseChunk(newChunk, { previousChunks: [...chunks] }),
lastCursor
new GenerateResponseChunk(newChunk, { previousChunks: [...chunks] })
);
chunks.push(newChunk);

Expand Down
5 changes: 3 additions & 2 deletions js/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,11 +32,12 @@
"@opentelemetry/sdk-metrics": "^1.25.0",
"@opentelemetry/sdk-node": "^0.52.0",
"@opentelemetry/sdk-trace-base": "^1.25.0",
"@types/json-schema": "^7.0.15",
"ajv": "^8.12.0",
"body-parser": "^1.20.3",
"cors": "^2.8.5",
"ajv-formats": "^3.0.1",
"async-mutex": "^0.5.0",
"body-parser": "^1.20.3",
"cors": "^2.8.5",
"express": "^4.21.0",
"get-port": "^5.1.0",
"json-schema": "^0.4.0",
Expand Down
27 changes: 10 additions & 17 deletions js/core/src/tracing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,24 +79,17 @@ export async function enableTelemetry(
}

export async function cleanUpTracing(): Promise<void> {
return new Promise((resolve) => {
if (telemetrySDK) {
// Metrics are not flushed as part of the shutdown operation. If metrics
// are enabled, we need to manually flush them *before* the reader
// receives shutdown order.
const metricFlush = maybeFlushMetrics();
if (!telemetrySDK) {
return;
}

return metricFlush.then(() => {
return telemetrySDK!.shutdown().then(() => {
logger.debug('OpenTelemetry SDK shut down.');
telemetrySDK = null;
resolve();
});
});
} else {
resolve();
}
});
// Metrics are not flushed as part of the shutdown operation. If metrics
// are enabled, we need to manually flush them *before* the reader
// receives shutdown order.
await maybeFlushMetrics();
await telemetrySDK.shutdown();
logger.debug('OpenTelemetry SDK shut down.');
telemetrySDK = null;
}

/**
Expand Down
6 changes: 5 additions & 1 deletion js/core/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
{
"extends": "../tsconfig.json",
"include": ["src"]
"include": ["src"],
"compilerOptions": {
// DOM is needed for streaming APIs.
"lib": ["es2022", "dom"]
}
}
17 changes: 15 additions & 2 deletions js/genkit/src/client/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,19 @@ export async function runFlow<O = any>({
`Server returned: ${response.status}: ${await response.text()}`
);
}
const wrappedDesult = await response.json();
return wrappedDesult.result;
const wrappedResult = (await response.json()) as
| { result: O }
| { error: unknown };
if ('error' in wrappedResult) {
if (typeof wrappedResult.error === 'string') {
throw new Error(wrappedResult.error);
}
// TODO: The callable protocol defines an HttpError that has a JSON format of
// details?: string
// httpErrorCode: { canonicalName: string }
// message: string
// Should we create a new error class that parses this and exposes it as fields?
throw new Error(JSON.stringify(wrappedResult.error));
}
return wrappedResult.result;
}
7 changes: 6 additions & 1 deletion js/plugins/chroma/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
{
"extends": "../../tsconfig.json",
"include": ["src"]
"include": ["src"],
"compilerOptions": {
// chromadb does not compile cleany. It tries to import @xenova/transformers and
// chromadb-default-embed, which do not seem to be packaged with the library
"skipLibCheck": true
}
}
9 changes: 9 additions & 0 deletions js/plugins/firebase/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@
{
"extends": "../../tsconfig.json",
"compilerOptions": {
// Google protobuf libraries have a transitive dependency on the long
// library, which incorrectly implements the ESM/CSM dual mode. This
// started in 5.0.0 and protobufjs requires ^5.0.0.
// This can be removed if https://github.com/dcodeIO/long.js/pull/130 gets
// approved, though it's been sitting around for over a year without
// acceptance.
"skipLibCheck": true
},
"include": ["src"]
}
2 changes: 1 addition & 1 deletion js/plugins/google-cloud/src/gcpOpenTelemetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ export class GcpOpenTelemetry {
*/
class MetricExporterWrapper extends MetricExporter {
constructor(
private options?: ExporterOptions,
options?: ExporterOptions,
private errorHandler?: (error: Error) => void
) {
super(options);
Expand Down
16 changes: 8 additions & 8 deletions js/plugins/google-cloud/tests/metrics_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ describe('GoogleCloudMetrics', () => {
await ai.generate({ model: testModel, prompt: 'Hi' });
await ai.generate({ model: testModel, prompt: 'Yo' });

const spans = await getExportedSpans();
await getExportedSpans();

const requestCounter = await getCounterMetric('genkit/feature/requests');
const latencyHistogram = await getHistogramMetric('genkit/feature/latency');
Expand Down Expand Up @@ -544,7 +544,7 @@ describe('GoogleCloudMetrics', () => {

it('writes path metrics for a failing flow with exception in root', async () => {
const flow = createFlow(ai, 'testFlow', async () => {
const subPath = await ai.run('sub-action', async () => {
await ai.run('sub-action', async () => {
return 'done';
});
return Promise.reject(new Error('failed'));
Expand Down Expand Up @@ -582,8 +582,8 @@ describe('GoogleCloudMetrics', () => {

it('writes path metrics for a failing flow with exception in subaction', async () => {
const flow = createFlow(ai, 'testFlow', async () => {
const subPath1 = await ai.run('sub-action-1', async () => {
const subPath2 = await ai.run('sub-action-2', async () => {
await ai.run('sub-action-1', async () => {
await ai.run('sub-action-2', async () => {
return Promise.reject(new Error('failed'));
});
return 'done';
Expand Down Expand Up @@ -627,8 +627,8 @@ describe('GoogleCloudMetrics', () => {

it('writes path metrics for a flow with exception in action', async () => {
const flow = createFlow(ai, 'testFlow', async () => {
const subPath1 = await ai.run('sub-action-1', async () => {
const subPath2 = await ai.run('sub-action-2', async () => {
await ai.run('sub-action-1', async () => {
await ai.run('sub-action-2', async () => {
return 'done';
});
return Promise.reject(new Error('failed'));
Expand Down Expand Up @@ -674,10 +674,10 @@ describe('GoogleCloudMetrics', () => {

it('writes path metrics for a flow with an exception in a serial action', async () => {
const flow = createFlow(ai, 'testFlow', async () => {
const subPath1 = await ai.run('sub-action-1', async () => {
await ai.run('sub-action-1', async () => {
return 'done';
});
const subPath2 = await ai.run('sub-action-2', async () => {
await ai.run('sub-action-2', async () => {
return Promise.reject(new Error('failed'));
});
return 'done';
Expand Down
11 changes: 10 additions & 1 deletion js/plugins/google-cloud/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,13 @@
{
"extends": "../../tsconfig.json",
"include": ["src"]
"include": ["src"],
"compilerOptions": {
// Google protobuf libraries have a transitive dependency on the long
// library, which incorrectly implements the ESM/CSM dual mode. This
// started in 5.0.0 and protobufjs requires ^5.0.0.
// This can be removed if https://github.com/dcodeIO/long.js/pull/130 gets
// approved, though it's been sitting around for over a year without
// acceptance.
"skipLibCheck": true
}
}
13 changes: 12 additions & 1 deletion js/plugins/langchain/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,15 @@
{
"extends": "../../tsconfig.json",
"include": ["src"]
"include": ["src"],
"compilerOptions": {
// The langchain+core package has multiple ts errors:
// 1. In chat_models.d.ts and llms.d.ts, TypeScript cannot infer the type of `this`.
// this error might be resolved by upgrading typescript versions
// 2. outputs.d.cts and base.d.cts mix up ESM and CSM and are using the wrong loader.
//
// The langchain package also has errors:
// 1. callbacks.d.ts, evaluation.d.cts, and base.d.cts mix up ESM and CSM and are
// using the wrong loader
"skipLibCheck": true
}
}
2 changes: 1 addition & 1 deletion js/plugins/mcp/src/client/tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ function registerTool(
{
name: `${params.name}/${tool.name}`,
description: tool.description || '',
inputJsonSchema: tool.inputSchema,
inputJsonSchema: tool.inputSchema.properties,
outputSchema: z.any(),
},
async (args) => {
Expand Down
2 changes: 1 addition & 1 deletion js/plugins/mcp/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ async function transportFrom(params: McpClientOptions): Promise<Transport> {
const { SSEClientTransport } = await import(
'@modelcontextprotocol/sdk/client/sse.js'
);
return new SSEClientTransport(URL.parse(params.serverUrl)!);
return new SSEClientTransport(new URL(params.serverUrl));
}
if (params.serverProcess) {
const { StdioClientTransport } = await import(
Expand Down
5 changes: 4 additions & 1 deletion js/plugins/mcp/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,9 @@ export class GenkitMcpServer {
}
}

function toMcpPromptArguments(p: PromptAction): Prompt['arguments'] {
function toMcpPromptArguments(
p: PromptAction
): Prompt['arguments'] | undefined {
const jsonSchema = toJsonSchema({
schema: p.__action.inputSchema,
jsonSchema: p.__action.inputJsonSchema,
Expand Down Expand Up @@ -216,6 +218,7 @@ function toMcpPromptArguments(p: PromptAction): Prompt['arguments'] {
required: jsonSchema.required?.includes(k),
});
}
return args;
}

const ROLE_MAP = { model: 'assistant', user: 'user' } as const;
Expand Down
4 changes: 3 additions & 1 deletion js/plugins/ollama/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,12 @@
"peerDependencies": {
"genkit": "workspace:*"
},
"dependencies": {
"ollama": "^0.5.9"
},
"devDependencies": {
"@types/node": "^20.11.16",
"npm-run-all": "^4.1.5",
"ollama": "^0.5.9",
"rimraf": "^6.0.1",
"tsup": "^8.3.5",
"tsx": "^4.19.2",
Expand Down
2 changes: 1 addition & 1 deletion js/plugins/ollama/src/embeddings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ export function defineOllamaEmbedder(
);
}

const payload: EmbedResponse = await response.json();
const payload = (await response.json()) as EmbedResponse;

const embeddings: { embedding: number[] }[] = [];

Expand Down
Loading

0 comments on commit 6b541a3

Please sign in to comment.