Skip to content

Feat/ai samples cleanup#1072

Draft
sedanah-m wants to merge 4 commits into
feat/ai-samplesfrom
feat/ai-samples-cleanup
Draft

Feat/ai samples cleanup#1072
sedanah-m wants to merge 4 commits into
feat/ai-samplesfrom
feat/ai-samples-cleanup

Conversation

@sedanah-m

@sedanah-m sedanah-m commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Clean up some minor issues in the README file.

@sedanah-m
sedanah-m changed the base branch from master to feat/ai-samples July 20, 2026 21:12

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a new React and Vite-based sample application demonstrating various features of the Firebase AI SDK, including text generation and multi-turn chat. Key feedback highlights a security issue where the App Check debug token is unconditionally enabled in all environments, and a configuration mismatch where firebase-config.ts is entirely commented out. Additionally, there is redundant routing code in App.tsx, potential UI bugs in the chat feature when initialization fails, a generic component name in the structured output feature, an outdated @types/react-router-dom dependency, and a mismatch between the implemented App Check provider and the README instructions.

I am having trouble creating individual review comments. Click here to see my feedback.

ai/ai-samples/src/services/firebaseAIService.ts (20)

security-high high

The debug token is unconditionally set to true on window.FIREBASE_APPCHECK_DEBUG_TOKEN. This means App Check debug mode will be active in all environments, including production, which is a security risk. It also contradicts the README instructions, which state that debug mode should be enabled by setting VITE_APPCHECK_DEBUG_TOKEN=true in .env.local.

  if (import.meta.env.VITE_APPCHECK_DEBUG_TOKEN === 'true') {
    (window as any).FIREBASE_APPCHECK_DEBUG_TOKEN = true;
  }

ai/ai-samples/src/config/firebase-config.ts (1-7)

high

This configuration file is completely commented out and is never imported or used anywhere in the application. Instead, firebaseAIService.ts initializes its own Firebase instance using environment variables or a hardcoded placeholder. This directly contradicts the setup instructions in the README.md.

To fix this, we should uncomment this file, define the placeholder config here, and import the initialized app in firebaseAIService.ts.

import { initializeApp } from "firebase/app";

export const firebaseConfig = {
  apiKey: "YOUR_API_KEY",
  authDomain: "YOUR_AUTH_DOMAIN",
  projectId: "YOUR_PROJECT_ID",
  storageBucket: "YOUR_STORAGE_BUCKET",
  messagingSenderId: "YOUR_MESSAGING_SENDER_ID",
  appId: "YOUR_APP_ID"
};

export const app = initializeApp(firebaseConfig);

ai/ai-samples/src/App.tsx (20-31)

medium

This switch block checking isolatedFeature is redundant and dead code. When VITE_ISOLATED_FEATURE is set, index.tsx renders the corresponding feature component directly as the root of the application, bypassing the RouterProvider and <App /> entirely.

ai/ai-samples/src/features/chat/index.tsx (127)

medium

If the chat session fails to initialize on mount (e.g., due to missing or invalid Firebase configuration), chatSessionRef.current will be null. Currently, the input field remains enabled, and submitting the form silently returns without any feedback. The input field should be disabled when chatSessionRef.current is null to prevent users from attempting to send messages.

          disabled={loading || !chatSessionRef.current}

ai/ai-samples/src/features/chat/index.tsx (138)

medium

Disable the send button as well if the chat session is not initialized.

          disabled={loading || !input.trim() || !chatSessionRef.current}

ai/ai-samples/src/features/structured-output/index.tsx (3)

medium

The component is named Feature, which is very generic and inconsistent with other feature components in this project (e.g., FunctionCallingFeature, ImageGenerationFeature, MultimodalFeature). Renaming it to StructuredOutputFeature improves readability and makes debugging easier in React DevTools.

export default function StructuredOutputFeature() {

ai/ai-samples/package.json (28)

medium

@types/react-router-dom is included in devDependencies, but react-router-dom v6/v7 is written in TypeScript and provides its own built-in type definitions. Furthermore, version ^5.3.3 is outdated and incompatible with v7, so this dependency should be removed.

ai/ai-samples/src/services/firebaseAIService.ts (25)

medium

The code uses ReCaptchaEnterpriseProvider for App Check, but the README.md instructs users to register and use "reCAPTCHA v3" in production. These are distinct services with different configuration requirements. If reCAPTCHA v3 is intended, ReCaptchaV3Provider should be used instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant