Skip to content

fix: resolve embeddings package build errors #41

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
groupthinking merged 1 commit into from
Feb 28, 2026
Merged

fix: resolve embeddings package build errors #41

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions packages/embeddings/src/dataconnect-generated/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/**
* Stub types for Firebase Data Connect SDK
*
* The real SDK is generated by `firebase dataconnect:sdk:generate`.
* These stubs allow the package to compile independently.
*/

type UUID = `${string}-${string}-${string}-${string}-${string}`;

interface VideoEmbeddingRow {
id: string;
segmentType: string;
segmentIndex: number;
content: string;
createdAt: string;
job?: {
id: string;
title?: string;
videoUrl?: string;
};
}

interface ListEmbeddingsResponse {
data: { videoEmbeddings: VideoEmbeddingRow[] };
}

interface GetJobEmbeddingsResponse {
data: { videoEmbeddings: VideoEmbeddingRow[] };
}

interface DeleteJobEmbeddingsResponse {
data: { videoEmbedding_deleteMany: number };
}

export declare function listEmbeddings(vars: {
limit: number;
}): Promise<ListEmbeddingsResponse>;

export declare function getJobEmbeddings(vars: {
jobId: UUID;
}): Promise<GetJobEmbeddingsResponse>;

export declare function deleteJobEmbeddings(vars: {
jobId: UUID;
}): Promise<DeleteJobEmbeddingsResponse>;
Comment on lines +43 to +45
Copy link

@sentry sentry bot Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The new DataConnect SDK stubs have no runtime implementation. Calls to these functions will fail with a TypeError unless an external process generates the real SDK before deployment.
Severity: CRITICAL

Suggested Fix

Ensure the CI/CD or deployment pipeline explicitly runs the firebase dataconnect:sdk:generate command to replace the stub files with the actual SDK implementation before the application is deployed. Alternatively, add this command as a prebuild or prepare script in package.json to make the dependency explicit.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: packages/embeddings/src/dataconnect-generated/index.ts#L43-L45

Potential issue: The pull request introduces stub files for the DataConnect SDK using
`export declare function`. These declarations are type-only and are erased during
TypeScript compilation, resulting in no JavaScript implementation. Functions like
`listEmbeddings()`, `getJobEmbeddings()`, and `deleteJobEmbeddings()` are imported and
called in `packages/embeddings/src/embedding.ts`. At runtime, these functions will be
`undefined`, causing a `TypeError` when they are invoked. This relies on an external,
unverified build step to generate the actual SDK, which is not present in the package's
build scripts.

Did we get this right? 👍 / 👎 to inform future reviews.

Comment on lines +35 to +45
Copy link

Copilot AI Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

index.ts exports only declared functions. tsc will emit no runtime implementations for these exports, so the compiled dist/dataconnect-generated/index.js won't actually export listEmbeddings/getJobEmbeddings/deleteJobEmbeddings. Because embedding.ts imports these as runtime values, consumers will hit a runtime module export error (ESM) or ... is not a function (CJS) the first time any of these APIs are called. Provide real stub implementations (e.g., functions that throw a clear "SDK not generated" error) or make this a .d.ts type-only module and keep runtime imports pointing to a real generated implementation.

Suggested change
export declare function listEmbeddings(vars: {
limit: number;
}): Promise<ListEmbeddingsResponse>;
export declare function getJobEmbeddings(vars: {
jobId: UUID;
}): Promise<GetJobEmbeddingsResponse>;
export declare function deleteJobEmbeddings(vars: {
jobId: UUID;
}): Promise<DeleteJobEmbeddingsResponse>;
export async function listEmbeddings(vars: {
limit: number;
}): Promise<ListEmbeddingsResponse> {
throw new Error(
"Firebase Data Connect SDK not generated. " +
"Run `firebase dataconnect:sdk:generate` to generate the runtime implementation for listEmbeddings."
);
}
export async function getJobEmbeddings(vars: {
jobId: UUID;
}): Promise<GetJobEmbeddingsResponse> {
throw new Error(
"Firebase Data Connect SDK not generated. " +
"Run `firebase dataconnect:sdk:generate` to generate the runtime implementation for getJobEmbeddings."
);
}
export async function deleteJobEmbeddings(vars: {
jobId: UUID;
}): Promise<DeleteJobEmbeddingsResponse> {
throw new Error(
"Firebase Data Connect SDK not generated. " +
"Run `firebase dataconnect:sdk:generate` to generate the runtime implementation for deleteJobEmbeddings."
);
}

Copilot uses AI. Check for mistakes.
10 changes: 5 additions & 5 deletions packages/embeddings/src/embedding.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {
listEmbeddings,
getJobEmbeddings,
deleteJobEmbeddings,
} from "../dataconnect-generated";
} from "./dataconnect-generated/index.js";

// =============================================================================
// Types
Expand Down Expand Up @@ -108,7 +108,7 @@ export async function generateEmbedding(text: string): Promise<number[]> {
throw new Error(`Embedding API error: ${response.status} ${await response.text()}`);
}

const data = await response.json();
const data = (await response.json()) as { predictions: { embeddings: { values: number[] } }[] };
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While this type assertion works, the inline type { predictions: { embeddings: { values: number[] } }[] } is quite complex and is also used in the generateEmbeddings function. To avoid repetition and improve readability, it would be better to define a named type for this response structure in the Types section at the top of the file and reuse it in both places.

For example:

interface VertexEmbeddingResponse {
  predictions: {
    embeddings: {
      values: number[];
    };
  }[];
}

Then you could simply use (await response.json()) as VertexEmbeddingResponse.

return data.predictions[0].embeddings.values;
}

Expand All @@ -134,8 +134,8 @@ export async function generateEmbeddings(texts: string[]): Promise<number[][]> {
throw new Error(`Embedding API error: ${response.status} ${await response.text()}`);
}

const data = await response.json();
return data.predictions.map((p: { embeddings: { values: number[] } }) => p.embeddings.values);
const data = (await response.json()) as { predictions: { embeddings: { values: number[] } }[] };
return data.predictions.map((p) => p.embeddings.values);
}

/**
Expand All @@ -148,7 +148,7 @@ async function getAccessToken(): Promise<string> {
"http://metadata.google.internal/computeMetadata/v1/instance/service-accounts/default/token",
{ headers: { "Metadata-Flavor": "Google" } }
);
const data = await response.json();
const data = (await response.json()) as { access_token: string };
return data.access_token;
}

Expand Down
Loading