feat(chat): add persistent chat with history sidebar by SumitPatel-HQ · Pull Request #8 · SumitPatel-HQ/QueryCraft

SumitPatel-HQ · 2026-04-01T16:54:15Z

Summary

Commit 1: feat(auth): migrate from Clerk to Firebase authentication

Migrated authentication from Clerk to Firebase across backend and frontend
Replaced JWT verification with Firebase Admin SDK
Added user_id column to Database and QueryHistory for multi-tenant isolation
Updated all API routes to require authentication
Added session context helper for row-level security

Commit 2: feat(chat): add persistent chat with history sidebar

Added multi-turn conversation support with persisted history
New chat router with session management on backend
Chat history sidebar with search functionality
Added bookmarks and search pages for query management
Migrated all dashboard pages from server to client components

Commit 3: refactor(api): consolidate API layer and update dependencies

Removed Next.js proxy and server-side API client
Updated useApi hook with Firebase auth token handling
Updated Gemini client to use google-genai package
Added empty state UI when no database is selected

Added multi-turn conversation support with persisted history: - New chat router with session management on backend - Chat history sidebar with search functionality - Added bookmarks and search pages for query management - Migrated all dashboard pages from server to client components - Added UI components: Avatar, DropdownMenu, Dialog

vercel · 2026-04-01T16:54:23Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
querycraft	Error		Apr 1, 2026 5:13pm

Copilot

Pull request overview

This PR introduces persistent, multi-turn chat sessions with history, bookmarks, and search across chat messages, while also updating backend routing/auth for user isolation and migrating several dashboard pages to client-side data fetching.

Changes:

Added chat session/message/bookmark persistence with new backend router endpoints and SQLAlchemy models.
Added frontend chat hook and new pages for chat search and bookmarks.
Migrated multiple dashboard routes from server components to client components with useEffect-based fetching and auth gating (partial).

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
frontend/src/hooks/use-chat.ts	Adds a client hook for chat sessions/messages/search/bookmarks API calls.
frontend/src/components/ui/dropdown-menu.tsx	Introduces Radix-based dropdown menu UI primitives.
frontend/src/components/ui/avatar.tsx	Introduces Radix-based avatar UI primitives.
frontend/src/components/database/DatabaseUploadModal.tsx	Refactors upload modal to use shared `Dialog` components.
frontend/src/components/chat/ChatSearch.tsx	Adds a chat history search UI backed by `/api/v1/chat/search`.
frontend/src/components/chat/BookmarksPage.tsx	Adds a bookmarks UI backed by `/api/v1/bookmarks`.
frontend/src/app/dashboard/search/page.tsx	Wires the dashboard search route to the new ChatSearch component.
frontend/src/app/dashboard/page.tsx	Migrates dashboard home to a client component that fetches databases.
frontend/src/app/dashboard/databases/page.tsx	Migrates databases list to a client component that fetches databases.
frontend/src/app/dashboard/databases/[dbId]/layout.tsx	Migrates database detail layout to client-side fetching of database metadata.
frontend/src/app/dashboard/databases/[dbId]/chat/page.tsx	Migrates database chat page to client-side fetching of a single database.
frontend/src/app/dashboard/chat/page.tsx	Migrates main chat page to client-side fetching of databases (no auth gating yet).
frontend/src/app/dashboard/bookmarks/page.tsx	Wires the dashboard bookmarks route to the new BookmarksPage component.
backend/migrations/001_add_user_id.sql	Adds `user_id` columns + RLS policies (not currently wired into bootstrap flow).
backend/migrations/002_chat_history.sql	Adds chat tables + indexes + FTS + RLS policies (not currently wired into bootstrap flow).
backend/database/chat_models.py	Adds SQLAlchemy models for chat sessions/messages/bookmarks.
backend/api/routers/queries.py	Makes auth required and scopes DB/query history access by `user_id`.
backend/api/routers/databases.py	Makes auth required and scopes DB CRUD by `user_id` (and sets RLS context).
backend/api/routers/chat.py	Adds chat sessions/messages/bookmarks/search endpoints.
backend/api/routers/init.py	Exposes the new `chat_router`.

Comments suppressed due to low confidence (1)

backend/api/routers/databases.py:36

Filtering databases by DatabaseModel.user_id == user_id is correct for isolation, but it will make any existing rows with user_id IS NULL permanently invisible (previously possible when auth was optional). Consider adding a backfill/cleanup migration to populate user_id (or explicitly delete/mark legacy rows), and/or tighten the schema to make user_id non-null for new records to avoid silent 404s.

    with get_db() as db:
        set_current_user_context(db, user_id)
        databases = (
            db.query(DatabaseModel)
            .filter(DatabaseModel.user_id == user_id)
            .filter(DatabaseModel.is_active == True)
            .order_by(DatabaseModel.last_accessed.desc())
            .all()
        )

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-01T17:00:09Z

+  const response = await fetch(endpoint, {
+    ...options,
+    headers: {
+      "Content-Type": "application/json",
+      ...(token ? { Authorization: `Bearer ${token}` } : {}),
+      ...(options?.headers || {}),
+    },
+  });


apiFetch sets Content-Type: application/json for all requests (including GET). This makes the request non-simple and can trigger extra CORS preflight round-trips. Consider only setting Content-Type when sending a JSON body (e.g., POST/PUT/PATCH) or when options.body is present, and otherwise omit it so GET requests stay simple.

Copilot · 2026-04-01T17:00:09Z

+  async function runSearch() {
+    if (!q.trim()) {
+      setResults([]);
+      return;
+    }
+    const data = await search(q.trim());
+    setResults(data);
+  }


runSearch() awaits the API call but doesn’t handle errors. If /api/v1/chat/search fails (auth, network, 500), this will create an unhandled rejection and the UI won’t show any feedback. Wrap the await search(...) call in a try/catch and surface an error state (and/or clear results) to keep the page stable.

Copilot · 2026-04-01T17:00:09Z

+  useEffect(() => {
+    async function fetchDatabases() {
+      try {
+        const data = await api.getDatabases();
+        setDatabases(data);
+      } catch (e) {
+        const errorMessage = e instanceof Error ? e.message : "Failed to load databases";
+        setError(errorMessage);
+        console.error("Error fetching databases:", e);
+      } finally {
+        setLoading(false);
+      }
+    }
+
+    fetchDatabases();
+  }, [api]);


This page fetches databases immediately on mount. If auth is still loading, useApi().getDatabases() throws "Authentication is still loading" and you end up rendering an error instead of waiting. Consider gating the effect on auth readiness (like dashboard/page.tsx does) or treating the "auth loading" error as a retry condition instead of a terminal error.

Copilot · 2026-04-01T17:00:10Z

@@ -38,7 +54,9 @@ export default async function DatabaseDetailLayout({
          <Link href="/dashboard/databases" className="text-sm text-[#888888] hover:text-[#f0f0f0] transition-colors">
            Back to Databases
          </Link>
-          <div className="mt-3 font-semibold text-[#f0f0f0] truncate">{databaseName}</div>
+          <div className="mt-3 font-semibold text-[#f0f0f0] truncate">
+            {loading ? "Loading..." : databaseName}
+          </div>
          <div className="mt-2 inline-block text-xs px-2 py-1 rounded-md bg-[rgba(16,185,129,0.15)] text-[#10B981]">
            {status}
          </div>


While loading is true you render the database name as "Loading..." but still render the status badge computed as "Unknown" (with the green "Active" styling). This is misleading during load and also for failures. Consider showing a loading state for the badge (or hiding it) and styling/status text consistently for Active vs Inactive vs Unknown/error.

Copilot · 2026-04-01T17:00:10Z

+  const params = useParams();
+  const dbId = params.dbId as string;
  const id = parseInt(dbId, 10);
+
+  const [database, setDatabase] = useState<DatabaseResponse | null>(null);
+  const [loading, setLoading] = useState(true);
+  const api = useApi();


dbId is cast to string and parsed with parseInt, but there’s no validation for NaN/invalid route params. If dbId isn’t numeric, the code will call api.getDatabase(NaN) and produce confusing errors. Add a guard (e.g., Number.isFinite(id)) and render a clear 400-style message or redirect.

Copilot · 2026-04-01T17:00:11Z

+@router.get("/chat/search")
+async def search_chat(
+    q: str = Query(..., min_length=1), user: dict = Depends(get_current_user)
+):
+    user_id = user.get("uid")
+    with get_db() as db:
+        set_current_user_context(db, user_id)
+        messages = (
+            db.query(ChatMessage)
+            .join(ChatSession, ChatSession.id == ChatMessage.session_id)
+            .filter(ChatMessage.user_id == user_id)
+            .filter(ChatMessage.content.ilike(f"%{q}%"))
+            .order_by(ChatMessage.created_at.desc())
+            .limit(50)
+            .all()


The migration adds a generated content_tsv column and a GIN index for full-text search, but the search endpoint uses ILIKE '%q%', which can’t use that index and will degrade as message volume grows. Consider switching the search query to use Postgres FTS (content_tsv @@ plainto_tsquery(...)) so the intended index is utilized.

Copilot · 2026-04-01T17:00:11Z

+    session = relationship("ChatSession", back_populates="bookmarks", lazy="joined")
+    message = relationship("ChatMessage", back_populates="bookmarks", lazy="joined")


Bookmark.session and Bookmark.message are configured with lazy="joined", which forces joins to chat_sessions and chat_messages on every bookmark query. The /bookmarks endpoint only returns ids/note/timestamp, so these joins are wasted work. Consider using lazy="selectin" (or default lazy loading) and explicitly eager-load only in endpoints that actually need related data.

Suggested change

session = relationship("ChatSession", back_populates="bookmarks", lazy="joined")

message = relationship("ChatMessage", back_populates="bookmarks", lazy="joined")

session = relationship("ChatSession", back_populates="bookmarks", lazy="selectin")

message = relationship("ChatMessage", back_populates="bookmarks", lazy="selectin")

Copilot · 2026-04-01T17:00:11Z

    with get_db() as db:
+        set_current_user_context(db, user_id)
        # Get database
        database = (
-            db.query(DatabaseModel).filter(DatabaseModel.id == database_id).first()
+            db.query(DatabaseModel)
+            .filter(DatabaseModel.id == database_id)
+            .filter(DatabaseModel.user_id == user_id)
+            .first()
        )


DatabaseModel.user_id == user_id filtering will cause any pre-existing databases with user_id IS NULL to become inaccessible (404) even for their original owner (previously possible when auth was optional). If this is expected, consider adding an explicit migration/backfill plan and/or a clearer error message so users aren’t surprised by silent 404s.

Copilot · 2026-04-01T17:00:11Z

+CREATE TABLE IF NOT EXISTS chat_sessions (
+    id SERIAL PRIMARY KEY,
+    user_id VARCHAR(128) NOT NULL,
+    title VARCHAR(255),
+    created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
+    updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP
+);
+


These migration files aren’t referenced by the current bootstrap paths (docker-compose mounts backend/init.sql, and the app startup runs init_db()/create_all). As-is, RLS policies/indexes here won’t be applied unless someone runs them manually. Consider documenting how to apply migrations, adding a migration runner, or folding required schema changes into init.sql/init_db() so new environments are consistent.

Copilot · 2026-04-01T17:00:12Z

+ALTER TABLE databases ADD COLUMN IF NOT EXISTS user_id VARCHAR(128);
+CREATE INDEX IF NOT EXISTS idx_databases_user_id ON databases(user_id);
+
+ALTER TABLE query_history ADD COLUMN IF NOT EXISTS user_id VARCHAR(128);
+CREATE INDEX IF NOT EXISTS idx_query_history_user_id ON query_history(user_id);
+
+ALTER TABLE databases ENABLE ROW LEVEL SECURITY;
+ALTER TABLE query_history ENABLE ROW LEVEL SECURITY;
+
+DROP POLICY IF EXISTS databases_user_isolation ON databases;
+CREATE POLICY databases_user_isolation ON databases
+USING (user_id = current_setting('app.current_user_id', true));
+
+DROP POLICY IF EXISTS query_history_user_isolation ON query_history;
+CREATE POLICY query_history_user_isolation ON query_history
+USING (user_id = current_setting('app.current_user_id', true));


This migration introduces user_id columns and RLS policies, but the repo’s documented setup path uses backend/init.sql (and runtime init_db()), not a migration runner. Without an execution path, this file is easy to miss and environments can drift. Consider clarifying how/when migrations should be applied, or integrating the required DDL into the existing initialization flow.

SumitPatel-HQ · 2026-04-01T17:06:30Z

opencode

SumitPatel-HQ · 2026-04-01T17:06:44Z

opencode run

- Removed Next.js proxy and server-side API client - Updated useApi hook with Firebase auth token handling - Updated Gemini client to use google-genai package - Added empty state UI when no database is selected - Updated database detail pages with client-side loading

Copilot AI review requested due to automatic review settings April 1, 2026 16:54

Copilot started reviewing on behalf of SumitPatel-HQ April 1, 2026 16:54 View session

vercel Bot had a problem deploying to Preview April 1, 2026 16:56 Failure

Copilot AI reviewed Apr 1, 2026

View reviewed changes

SumitPatel-HQ added 2 commits April 1, 2026 22:40

opencode.yaml

aa3b0ab

SumitPatel-HQ merged commit 406a864 into main Apr 1, 2026
1 of 2 checks passed

vercel Bot had a problem deploying to Preview April 1, 2026 17:13 Failure

		session = relationship("ChatSession", back_populates="bookmarks", lazy="joined")
		message = relationship("ChatMessage", back_populates="bookmarks", lazy="joined")

Conversation

SumitPatel-HQ commented Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Commit 1: feat(auth): migrate from Clerk to Firebase authentication

Commit 2: feat(chat): add persistent chat with history sidebar

Commit 3: refactor(api): consolidate API layer and update dependencies

Uh oh!

vercel Bot commented Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

SumitPatel-HQ commented Apr 1, 2026

Uh oh!

SumitPatel-HQ commented Apr 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

SumitPatel-HQ commented Apr 1, 2026 •

edited

Loading

vercel Bot commented Apr 1, 2026 •

edited

Loading