Skip to content

Conversation

@tikazyq
Copy link
Collaborator

@tikazyq tikazyq commented Sep 14, 2025

No description provided.

Copilot AI and others added 23 commits August 27, 2025 03:22
…-820c-2ebb78495a45

Implement document upload and management system for AI agents
…-aa10-dc3033ec42f1

Complete TypeORM to Prisma migration with functional services and smart fallback system
- Introduced `PrismaServiceBase` to encapsulate shared logic for Prisma services.
- Updated `PrismaDevlogService` and `PrismaProjectService` to extend from `PrismaServiceBase`.
- Implemented singleton pattern with TTL-based cleanup for service instances.
- Simplified Prisma client initialization and fallback handling.
- Removed redundant initialization logic from individual services.
- Enhanced lifecycle hooks for better extensibility and maintainability.
Copilot AI review requested due to automatic review settings September 14, 2025 06:39
@vercel
Copy link

vercel bot commented Sep 14, 2025

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

Project Deployment Preview Comments Updated (UTC)
devlog-web Error Error Sep 14, 2025 6:39am

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements a comprehensive migration from TypeORM to Prisma for database management, replacing multiple environment variables with a single DATABASE_URL configuration. The migration includes new Prisma services, schema definitions, and document management capabilities while maintaining API compatibility.

Key Changes:

  • Complete migration from TypeORM entities to Prisma schema
  • Simplified database configuration using only DATABASE_URL
  • New document management system with file upload and type detection
  • Prisma-based service implementations with fallback mode support

Reviewed Changes

Copilot reviewed 81 out of 83 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
prisma/schema.prisma New Prisma schema defining all database models with PostgreSQL optimizations
prisma/migrations/ Database migration files for schema setup and enum refactoring
packages/core/src/services/prisma-*.ts New Prisma-based service implementations replacing TypeORM services
packages/core/src/utils/prisma-config.ts New Prisma client configuration replacing TypeORM config
packages/mcp/src/tools/document-tools.ts New MCP tools for document management operations
turbo.json Simplified environment variables using only DATABASE_URL
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

model ChatSession {
id String @id
agent String // AgentType as string
timestamp String // ISO string
Copy link

Copilot AI Sep 14, 2025

Choose a reason for hiding this comment

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

Using String for timestamps instead of DateTime reduces type safety and requires manual conversion. Consider using DateTime type with @db.Timestamptz for better type safety and automatic timestamp handling.

Suggested change
timestamp String // ISO string
timestamp DateTime @db.Timestamptz // Timestamp with timezone

Copilot uses AI. Check for mistakes.
Comment on lines +48 to +49
const prismaModule = await import('@prisma/client');
const configModule = await import('../utils/prisma-config.js');
Copy link

Copilot AI Sep 14, 2025

Choose a reason for hiding this comment

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

Dynamic imports in initialization could impact performance. Consider using static imports or lazy loading patterns since Prisma client is a core dependency.

Copilot uses AI. Check for mistakes.
if (this.isFallbackMode) {
console.warn('[DocumentService] uploadDocument() called in fallback mode - returning mock document');

const documentId = `doc-${Date.now()}-${Math.random().toString(36).substr(2, 9)}`;
Copy link

Copilot AI Sep 14, 2025

Choose a reason for hiding this comment

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

ID generation is duplicated across multiple places. Consider using the existing generateDocumentId utility from id-generator.ts for consistency.

Copilot uses AI. Check for mistakes.
Comment on lines +441 to +469
// For getDocument, we need to find which devlog contains the document
// This is a limitation of the current API design - we'll try a simple approach
// by searching through recent devlogs
const devlogs = await this.apiClient.listDevlogs({
page: 1,
limit: 20,
sortBy: 'updatedAt',
sortOrder: 'desc'
});

let document = null;
for (const devlog of devlogs.items || []) {
try {
document = await this.apiClient.getDocument(devlog.id!, args.documentId);
break;
} catch (err) {
// Document not found in this devlog, continue searching
continue;
}
}

if (!document) {
return this.toStandardResponse(false, null, `Document ${args.documentId} not found`);
}

return this.toStandardResponse(
true,
document,
`Retrieved document: ${document.originalName || args.documentId}`,
Copy link

Copilot AI Sep 14, 2025

Choose a reason for hiding this comment

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

Searching through multiple devlogs sequentially to find a document could be inefficient. Consider adding a search endpoint that can locate documents directly by ID across all devlogs.

Suggested change
// For getDocument, we need to find which devlog contains the document
// This is a limitation of the current API design - we'll try a simple approach
// by searching through recent devlogs
const devlogs = await this.apiClient.listDevlogs({
page: 1,
limit: 20,
sortBy: 'updatedAt',
sortOrder: 'desc'
});
let document = null;
for (const devlog of devlogs.items || []) {
try {
document = await this.apiClient.getDocument(devlog.id!, args.documentId);
break;
} catch (err) {
// Document not found in this devlog, continue searching
continue;
}
}
if (!document) {
return this.toStandardResponse(false, null, `Document ${args.documentId} not found`);
}
return this.toStandardResponse(
true,
document,
`Retrieved document: ${document.originalName || args.documentId}`,
// Use the searchDocuments endpoint to locate the document by ID across all devlogs
const searchResult = await this.apiClient.searchDocuments({ documentId: args.documentId });
const foundDocument = searchResult.items && searchResult.items.length > 0 ? searchResult.items[0] : null;
if (!foundDocument) {
return this.toStandardResponse(false, null, `Document ${args.documentId} not found`);
}
return this.toStandardResponse(
true,
foundDocument,
`Retrieved document: ${foundDocument.originalName || args.documentId}`,

Copilot uses AI. Check for mistakes.
Comment on lines +453 to +456
const maxTextSize = 64 * 1024; // 64KB limit
return textContent.length > maxTextSize
? textContent.substring(0, maxTextSize) + '...[truncated]'
: textContent;
Copy link

Copilot AI Sep 14, 2025

Choose a reason for hiding this comment

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

Magic number 64KB should be extracted as a named constant. Also consider making this configurable since different environments may have different text size limits.

Copilot uses AI. Check for mistakes.
Comment on lines +52 to +56
this.JWT_SECRET = process.env.JWT_SECRET || 'fallback-secret-for-development';

if (!process.env.JWT_SECRET && process.env.NODE_ENV === 'production') {
throw new Error('JWT_SECRET environment variable is required in production');
}
Copy link

Copilot AI Sep 14, 2025

Choose a reason for hiding this comment

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

The fallback JWT secret could be a security risk if accidentally used in production. Consider throwing an error immediately if JWT_SECRET is not set, regardless of environment.

Suggested change
this.JWT_SECRET = process.env.JWT_SECRET || 'fallback-secret-for-development';
if (!process.env.JWT_SECRET && process.env.NODE_ENV === 'production') {
throw new Error('JWT_SECRET environment variable is required in production');
}
if (!process.env.JWT_SECRET) {
throw new Error('JWT_SECRET environment variable is required');
}
this.JWT_SECRET = process.env.JWT_SECRET;

Copilot uses AI. Check for mistakes.
@tikazyq
Copy link
Collaborator Author

tikazyq commented Sep 14, 2025

@copilot can you work on the suggestion?

@tikazyq tikazyq closed this Sep 14, 2025
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.

2 participants