diff --git a/examples/example_logs.ts b/examples/example_logs.ts index f2701af..a758f23 100644 --- a/examples/example_logs.ts +++ b/examples/example_logs.ts @@ -16,15 +16,18 @@ async function main() { console.log("Logger initialized") // mock retrieved documents - const retrieved_documents = [{"page_content": "Sample document"}] - const documents = retrieved_documents.map(doc => doc["page_content"]) + const retrieved_documents = [ + "Sample document 1", + {"page_content": "Sample document 2", "metadata": {"source": "website.com"}}, + {"page_content": "Sample document 3"} + ] console.log("Preparing to log with quotient_logger") try { const response = await quotient.logger.log({ user_query: "How do I cook a goose?", model_output: "The capital of France is Paris", - documents: documents, + documents: retrieved_documents, message_history: [ {"role": "system", "content": "You are an expert on geography."}, {"role": "user", "content": "What is the capital of France?"}, diff --git a/package.json b/package.json index f787c7e..e9eee61 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "quotientai", - "version": "0.0.3", + "version": "0.0.4", "description": "TypeScript client for QuotientAI API", "main": "dist/quotientai/index.js", "types": "dist/quotientai/index.d.ts", diff --git a/quotientai/exceptions.ts b/quotientai/exceptions.ts index df89e0f..b2e5c1e 100644 --- a/quotientai/exceptions.ts +++ b/quotientai/exceptions.ts @@ -1,4 +1,3 @@ - import axios, { AxiosError, AxiosRequestConfig, AxiosResponse, InternalAxiosRequestConfig } from 'axios'; export class QuotientAIError extends Error { @@ -8,6 +7,13 @@ export class QuotientAIError extends Error { } } +export class ValidationError extends QuotientAIError { + constructor(message: string) { + super(message); + this.name = 'ValidationError'; + } +} + export class APIError extends QuotientAIError { request: AxiosRequestConfig; body: any; diff --git a/quotientai/logger.ts b/quotientai/logger.ts index 2cd95c8..a214f0b 100644 --- a/quotientai/logger.ts +++ b/quotientai/logger.ts @@ -1,4 +1,5 @@ -import { LogEntry, LoggerConfig } from './types'; +import { LogEntry, LoggerConfig, LogDocument } from './types'; +import { ValidationError } from './exceptions'; interface LogsResource { create(params: LogEntry): Promise; @@ -41,6 +42,67 @@ export class QuotientLogger { return Math.random() < this.sampleRate; } + // Type guard function to check if an object is a valid LogDocument + private isValidLogDocument(obj: any): { valid: boolean; error?: string } { + try { + // Check if it has the required page_content property + if (!('page_content' in obj)) { + return { + valid: false, + error: "Missing required 'page_content' property" + }; + } + + // Check if page_content is a string + if (typeof obj.page_content !== 'string') { + return { + valid: false, + error: `The 'page_content' property must be a string, found ${typeof obj.page_content}` + }; + } + + // If metadata exists, check if it's an object + if ('metadata' in obj && obj.metadata !== null && typeof obj.metadata !== 'object') { + return { + valid: false, + error: `The 'metadata' property must be an object, found ${typeof obj.metadata}` + }; + } + + return { valid: true }; + } catch (error) { + return { valid: false, error: "Unexpected error validating document" }; + } + } + + // Validate document format + private validateDocuments(documents: (string | LogDocument)[]): void { + if (!documents || documents.length === 0) { + return; + } + + for (let i = 0; i < documents.length; i++) { + const doc = documents[i]; + if (typeof doc === 'string') { + continue; + } else if (typeof doc === 'object' && doc !== null) { + const validation = this.isValidLogDocument(doc); + if (!validation.valid) { + throw new ValidationError( + `Invalid document format at index ${i}: ${validation.error}. ` + + "Documents must be either strings or JSON objects with a 'page_content' string property and an optional 'metadata' object. " + + "To fix this, ensure each document follows the format: { page_content: 'your text content', metadata?: { key: 'value' } }" + ); + } + } else { + throw new ValidationError( + `Invalid document type at index ${i}. Found ${typeof doc}, but documents must be either strings or JSON objects with a 'page_content' property. ` + + "To fix this, provide documents as either simple strings or properly formatted objects: { page_content: 'your text content' }" + ); + } + } + } + // log a message // params: Omit async log(params: Omit): Promise { @@ -52,6 +114,11 @@ export class QuotientLogger { throw new Error('Logger is not properly configured. app_name and environment must be set.'); } + // Validate documents format + if (params.documents) { + this.validateDocuments(params.documents); + } + // Merge default tags with any tags provided at log time const mergedTags = { ...this.tags, ...(params.tags || {}) }; diff --git a/quotientai/resources/logs.ts b/quotientai/resources/logs.ts index eb3cc53..f772c82 100644 --- a/quotientai/resources/logs.ts +++ b/quotientai/resources/logs.ts @@ -1,4 +1,5 @@ import { BaseQuotientClient } from '../client'; +import { LogDocument } from '../types'; interface LogResponse { id: string; @@ -8,7 +9,7 @@ interface LogResponse { inconsistency_detection: boolean; user_query: string; model_output: string; - documents: string[]; + documents: (string | LogDocument)[]; message_history: any[] | null; instructions: string[] | null; tags: Record; @@ -26,7 +27,7 @@ interface CreateLogParams { inconsistency_detection: boolean; user_query: string; model_output: string; - documents: string[]; + documents: (string | LogDocument)[]; message_history?: any[] | null; instructions?: string[] | null; tags?: Record; @@ -50,7 +51,7 @@ export class Log { inconsistency_detection: boolean; user_query: string; model_output: string; - documents: string[]; + documents: (string | LogDocument)[]; message_history: any[] | null; instructions: string[] | null; tags: Record; @@ -109,7 +110,15 @@ export class LogsResource { if (params.offset !== undefined) queryParams.offset = params.offset; try { - const response = await this.client.get('/logs', { params: queryParams }) as LogsResponse; + const response = await this.client.get('/logs', queryParams) as LogsResponse; + + // Check if response has logs property and it's an array + if (!response || !response.logs || !Array.isArray(response.logs)) { + console.warn('No logs found. Please check your query parameters and try again.'); + return []; + } + + // Map the logs to Log objects return response.logs.map(logData => new Log(logData)); } catch (error) { console.error('Error listing logs:', error); diff --git a/quotientai/types.ts b/quotientai/types.ts index 2a422d8..c504aa1 100644 --- a/quotientai/types.ts +++ b/quotientai/types.ts @@ -75,12 +75,17 @@ export interface RunResult { expected: string; } +export interface LogDocument { + page_content: string; + metadata?: Record; +} + export interface LogEntry { app_name: string; environment: string; user_query: string; model_output: string; - documents: string[]; + documents: (string | LogDocument)[]; message_history?: Array> | null; instructions?: string[] | null; tags?: Record; diff --git a/tests/logger.test.ts b/tests/logger.test.ts index c970b68..a7fcf4c 100644 --- a/tests/logger.test.ts +++ b/tests/logger.test.ts @@ -1,6 +1,6 @@ - import { describe, it, expect, vi } from 'vitest'; import { QuotientLogger } from '../quotientai/logger'; +import { ValidationError } from '../quotientai/exceptions'; describe('QuotientLogger', () => { it('should initialize without being configured', () => { @@ -119,6 +119,205 @@ describe('QuotientLogger', () => { vi.spyOn(Math, 'random').mockRestore(); }); + it('should raise an error if required app_name or environment is missing after initialization', async () => { + const mockLogsResource = { create: vi.fn(), list: vi.fn() }; + const logger = new QuotientLogger(mockLogsResource); + const privateLogger = logger as any; + + // Initialize but with null values + privateLogger.init({}); + privateLogger.configured = true; + privateLogger.appName = null; + privateLogger.environment = 'test'; + + await expect(privateLogger.log({ message: 'test' })).rejects.toThrow('Logger is not properly configured. app_name and environment must be set.'); + + privateLogger.appName = 'test'; + privateLogger.environment = null; + + await expect(privateLogger.log({ message: 'test' })).rejects.toThrow('Logger is not properly configured. app_name and environment must be set.'); + }); + describe('Document Validation', () => { + it('should accept string documents', async () => { + const mockLogsResource = { create: vi.fn(), list: vi.fn() }; + const logger = new QuotientLogger(mockLogsResource); + const privateLogger = logger as any; + + privateLogger.init({ app_name: 'test_app', environment: 'test_environment' }); + + await privateLogger.log({ + user_query: 'test', + model_output: 'test', + documents: ['This is a string document'] + }); + + expect(mockLogsResource.create).toHaveBeenCalled(); + }); + + it('should accept valid LogDocument objects', async () => { + const mockLogsResource = { create: vi.fn(), list: vi.fn() }; + const logger = new QuotientLogger(mockLogsResource); + const privateLogger = logger as any; + + privateLogger.init({ app_name: 'test_app', environment: 'test_environment' }); + + await privateLogger.log({ + user_query: 'test', + model_output: 'test', + documents: [ + { page_content: 'This is valid content' }, + { page_content: 'Also valid', metadata: { source: 'test' } } + ] + }); + + expect(mockLogsResource.create).toHaveBeenCalled(); + }); + + it('should throw ValidationError when document is missing page_content', async () => { + const mockLogsResource = { create: vi.fn(), list: vi.fn() }; + const logger = new QuotientLogger(mockLogsResource); + const privateLogger = logger as any; + + privateLogger.init({ app_name: 'test_app', environment: 'test_environment' }); + + await expect(privateLogger.log({ + user_query: 'test', + model_output: 'test', + documents: [{ not_page_content: 'invalid' }] + })).rejects.toThrow(ValidationError); + + expect(mockLogsResource.create).not.toHaveBeenCalled(); + }); + + it('should throw ValidationError when page_content is not a string', async () => { + const mockLogsResource = { create: vi.fn(), list: vi.fn() }; + const logger = new QuotientLogger(mockLogsResource); + const privateLogger = logger as any; + + privateLogger.init({ app_name: 'test_app', environment: 'test_environment' }); + + await expect(privateLogger.log({ + user_query: 'test', + model_output: 'test', + documents: [{ page_content: 123 }] + })).rejects.toThrow(ValidationError); + + expect(mockLogsResource.create).not.toHaveBeenCalled(); + }); + + it('should throw ValidationError when metadata is not an object', async () => { + const mockLogsResource = { create: vi.fn(), list: vi.fn() }; + const logger = new QuotientLogger(mockLogsResource); + const privateLogger = logger as any; + + privateLogger.init({ app_name: 'test_app', environment: 'test_environment' }); + + await expect(privateLogger.log({ + user_query: 'test', + model_output: 'test', + documents: [{ page_content: 'Valid content', metadata: 'not an object' }] + })).rejects.toThrow(ValidationError); + + expect(mockLogsResource.create).not.toHaveBeenCalled(); + }); + + it('should throw ValidationError when document is not a string or object', async () => { + const mockLogsResource = { create: vi.fn(), list: vi.fn() }; + const logger = new QuotientLogger(mockLogsResource); + const privateLogger = logger as any; + + privateLogger.init({ app_name: 'test_app', environment: 'test_environment' }); + + await expect(privateLogger.log({ + user_query: 'test', + model_output: 'test', + documents: [123] + })).rejects.toThrow(ValidationError); + + expect(mockLogsResource.create).not.toHaveBeenCalled(); + }); + + it('should handle unexpected errors during validation', async () => { + const mockLogsResource = { create: vi.fn(), list: vi.fn() }; + const logger = new QuotientLogger(mockLogsResource); + const privateLogger = logger as any; + + privateLogger.init({ app_name: 'test_app', environment: 'test_environment' }); + + // Create a scenario where isValidLogDocument throws an error + const malformedObj = Object.create(null); + Object.defineProperty(malformedObj, 'page_content', { + get: () => { throw new Error('Unexpected error'); } + }); + + await expect(privateLogger.log({ + user_query: 'test', + model_output: 'test', + documents: [malformedObj] + })).rejects.toThrow(ValidationError); + + expect(mockLogsResource.create).not.toHaveBeenCalled(); + }); + }); + + describe('Direct Method Testing', () => { + it('should directly test isValidLogDocument with various inputs', () => { + const mockLogsResource = { create: vi.fn(), list: vi.fn() }; + const logger = new QuotientLogger(mockLogsResource); + const privateLogger = logger as any; + + // Valid document + expect(privateLogger.isValidLogDocument({ page_content: 'test' })).toEqual({ valid: true }); + + // Missing page_content + expect(privateLogger.isValidLogDocument({})).toEqual({ + valid: false, + error: "Missing required 'page_content' property" + }); + + // Non-string page_content + expect(privateLogger.isValidLogDocument({ page_content: 123 })).toEqual({ + valid: false, + error: "The 'page_content' property must be a string, found number" + }); + + // Invalid metadata + expect(privateLogger.isValidLogDocument({ page_content: 'test', metadata: 'not-object' })).toEqual({ + valid: false, + error: "The 'metadata' property must be an object, found string" + }); + + // null metadata should be valid + expect(privateLogger.isValidLogDocument({ page_content: 'test', metadata: null })).toEqual({ valid: true }); + }); + + it('should directly test validateDocuments with various inputs', () => { + const mockLogsResource = { create: vi.fn(), list: vi.fn() }; + const logger = new QuotientLogger(mockLogsResource); + const privateLogger = logger as any; + + // Empty array should not throw + expect(() => privateLogger.validateDocuments([])).not.toThrow(); + + // Null should not throw + expect(() => privateLogger.validateDocuments(null)).not.toThrow(); + + // Valid strings should not throw + expect(() => privateLogger.validateDocuments(['test', 'test2'])).not.toThrow(); + + // Valid objects should not throw + expect(() => privateLogger.validateDocuments([ + { page_content: 'test' }, + { page_content: 'test2', metadata: { source: 'test' } } + ])).not.toThrow(); + + // Mixed valid types should not throw + expect(() => privateLogger.validateDocuments([ + 'test', + { page_content: 'test' } + ])).not.toThrow(); + }); + }); }); diff --git a/tests/resources/logs.test.ts b/tests/resources/logs.test.ts index dd65205..2d4f68d 100644 --- a/tests/resources/logs.test.ts +++ b/tests/resources/logs.test.ts @@ -12,7 +12,7 @@ describe('LogsResource', () => { inconsistency_detection: false, user_query: 'What is the capital of France?', model_output: 'Paris is the capital of France.', - documents: ['doc1', 'doc2'], + documents: ['doc1', 'doc2', { page_content: 'doc3', metadata: { source: 'test' } }], message_history: [ { role: 'user', content: 'What is the capital of France?' }, { role: 'assistant', content: 'Paris is the capital of France.' } @@ -60,7 +60,7 @@ describe('LogsResource', () => { expect(logs[0].app_name).toBe('test-app'); expect(logs[0].environment).toBe('development'); expect(logs[0].created_at).toBeInstanceOf(Date); - expect(client.get).toHaveBeenCalledWith('/logs', { params: {} }); + expect(client.get).toHaveBeenCalledWith('/logs', {}); }); it('should list logs with query parameters', async () => { @@ -85,14 +85,12 @@ describe('LogsResource', () => { expect(logs).toHaveLength(1); expect(logs[0].environment).toBe('development'); expect(client.get).toHaveBeenCalledWith('/logs', { - params: { - app_name: 'test-app', - environment: 'development', - start_date: startDate.toISOString(), - end_date: endDate.toISOString(), - limit: 10, - offset: 0 - } + app_name: 'test-app', + environment: 'development', + start_date: startDate.toISOString(), + end_date: endDate.toISOString(), + limit: 10, + offset: 0 }); }); @@ -104,7 +102,47 @@ describe('LogsResource', () => { const logsResource = new LogsResource(client); await expect(logsResource.list()).rejects.toThrow('Test error'); expect(consoleSpy).toHaveBeenCalledWith('Error listing logs:', expect.any(Error)); - expect(client.get).toHaveBeenCalledWith('/logs', { params: {} }); + expect(client.get).toHaveBeenCalledWith('/logs', {}); + }); + + // Tests for the edge cases in list method (lines 117-119) + it('should handle null response from API', async () => { + const client = new BaseQuotientClient('test'); + vi.spyOn(client, 'get').mockResolvedValue(null); + const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + const logsResource = new LogsResource(client); + const logs = await logsResource.list(); + + expect(logs).toHaveLength(0); + expect(consoleSpy).toHaveBeenCalledWith('No logs found. Please check your query parameters and try again.'); + expect(client.get).toHaveBeenCalledWith('/logs', {}); + }); + + it('should handle response with missing logs property', async () => { + const client = new BaseQuotientClient('test'); + vi.spyOn(client, 'get').mockResolvedValue({}); + const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + const logsResource = new LogsResource(client); + const logs = await logsResource.list(); + + expect(logs).toHaveLength(0); + expect(consoleSpy).toHaveBeenCalledWith('No logs found. Please check your query parameters and try again.'); + expect(client.get).toHaveBeenCalledWith('/logs', {}); + }); + + it('should handle response with logs not being an array', async () => { + const client = new BaseQuotientClient('test'); + vi.spyOn(client, 'get').mockResolvedValue({ logs: 'not an array' }); + const consoleSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + const logsResource = new LogsResource(client); + const logs = await logsResource.list(); + + expect(logs).toHaveLength(0); + expect(consoleSpy).toHaveBeenCalledWith('No logs found. Please check your query parameters and try again.'); + expect(client.get).toHaveBeenCalledWith('/logs', {}); }); it('should create a log', async () => {