Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
550 changes: 466 additions & 84 deletions packages/ai-helper/README.md

Large diffs are not rendered by default.

4 changes: 3 additions & 1 deletion packages/ai-helper/i18n/aposAiHelper/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,7 @@
"rateLimitExceeded": "Rate limit exceeded, try again later",
"invalidRequest": "Invalid request, please review OpenAI's rules",
"utilityOperationLabel": "Generate an image with AI",
"errorMessage": "An error occurred."
"errorMessage": "An error occurred.",
"generateVariant": "Generate Variant",
"generationFailed": "Image generation failed. Please try again."
}
249 changes: 238 additions & 11 deletions packages/ai-helper/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,34 +2,261 @@ const fs = require('fs');
const path = require('path');

module.exports = {
init() {
if (!(process.env.APOS_OPENAI_KEY || process.env.APOS_AI_HELPER_MOCK)) {
// We do not document the mock because it is for internal testing
throw new Error('APOS_OPENAI_KEY must be set in your environment');
options: {
// Default providers for text and image generation
textProvider: 'openai',
imageProvider: 'openai',
// Legacy option support
textMaxTokens: 1000,
// Usage tracking - can be set via option or
// APOS_AI_HELPER_LOG_USAGE env var
// When true, usage data is logged to the console
// for cost tracking and auditing
logUsage: process.env.APOS_AI_HELPER_LOG_USAGE === 'true' || false,
Copy link
Member

Choose a reason for hiding this comment

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

Just check it for truthiness IMHO, but if not we must support 1 in particular for consistency with common practice including our own.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

logUsage eliminated.

// Usage storage - when true,
// usage data is permanently stored in MongoDB
// separate from logUsage to allow console logging
// without database bloat
storeUsage: process.env.APOS_AI_HELPER_STORE_USAGE === 'true' || false
},

async init(self) {
// Storage for registered providers
self.providers = new Map();

// Initialize usage storage collection if enabled
if (self.options.storeUsage) {
self.aposAiHelperUsage = self.apos.db.collection('aposAiHelperUsage');
await self.aposAiHelperUsage.createIndex(
{ createdAt: -1 },
{ name: 'createdAt_-1' }
Copy link
Member

Choose a reason for hiding this comment

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

I'd remove this as it's not better than the default convention (I think it might be the default convention).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed.

);

// Index for per-user usage timelines
await self.aposAiHelperUsage.createIndex(
{
userId: 1,
createdAt: -1
},
{ name: 'userId_1_createdAt_-1' }
Copy link
Member

Choose a reason for hiding this comment

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

Same re: not adding name if it's just the same as the default convention, which is fine

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we are removing the storage, this goes away.

);
self.apos.util.log('AI Helper usage storage enabled');
}

// Initialize usage tracking if enabled
if (self.options.logUsage) {
self.apos.util.log('AI Helper usage tracking enabled');
}
},

i18n: {
aposAiHelper: {
browser: true
}
},

bundle: {
directory: 'modules',
modules: getBundleModuleNames()
},
options: {
textModel: 'gpt-5.1',
Copy link
Member

Choose a reason for hiding this comment

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

We have bc provisions I assume?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Provisions added.

textMaxTokens: 1000,
imageModel: 'gpt-image-1-mini'
},

methods(self) {
return {
/**
* Register an AI provider module
* @param {Object} provider - The provider module (self)
* @param {Object} config - Provider configuration
* @param {string} config.name - Unique provider name (e.g., 'openai', 'anthropic')
* @param {string} config.label - Human-readable label
* @param {boolean} [config.text] - Supports text generation
* @param {boolean} [config.image] - Supports image generation
* @param {boolean} [config.imageVariation] - Supports image variations
*/
registerProvider(provider, config) {
Copy link
Member

Choose a reason for hiding this comment

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

I actually think a full apostrophecms module is overkill for a provider. Not worth the startup overhead. I would go with passing an ordinary object. It's fine if it happens to be an apostrophe module.

Copy link
Member

Choose a reason for hiding this comment

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

I recognize people will want to override options of the individual providers, but they only need to do that for the ones they are actually using. So if you add textProviderOptions and imageProviderOptions to the main module that should address that concern.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Switched to factories.

const {
name,
label,
text = false,
image = false,
imageVariation = false
} = config;

if (!name) {
throw new Error('Provider must have a name');
}

if (self.providers.has(name)) {
throw new Error(`Provider "${name}" is already registered`);
Copy link
Member

Choose a reason for hiding this comment

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

We should allow this as an override mechanism for our core providers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now allowed as last registered wins.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see it being allowed - not pushed yet?

}

// Validate that the provider implements required methods
if (text && typeof provider.generateText !== 'function') {
throw new Error(`Provider "${name}" claims text support but doesn't implement generateText()`);
Copy link
Member

Choose a reason for hiding this comment

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

Love the duck typing.

}

if (image && typeof provider.generateImage !== 'function') {
throw new Error(`Provider "${name}" claims image support but doesn't implement generateImage()`);
}

if (imageVariation && typeof provider.generateImageVariation !== 'function') {
throw new Error(`Provider "${name}" claims imageVariation support but doesn't implement generateImageVariation()`);
}

self.providers.set(name, {
module: provider,
label,
capabilities: {
text,
image,
imageVariation
}
});

self.apos.util.log(`AI provider registered: ${name} (${label})`);
Copy link
Member

Choose a reason for hiding this comment

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

Use the info log level.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed

},

/**
* Get a registered provider by name
* @param {string} name - Provider name
* @returns {Object} Provider info
*/
getProvider(name) {
const provider = self.providers.get(name);

if (!provider) {
const available = Array.from(self.providers.keys()).join(', ');
throw self.apos.error('notfound',
`AI provider "${name}" not found. Available providers: ${available || 'none'}`
Copy link
Member

Choose a reason for hiding this comment

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

Nice.

);
}

return provider;
},

/**
* Get the configured text provider
* @returns {Object} Provider module and capabilities
*/
getTextProvider() {
const providerName = self.options.textProvider;
const provider = self.getProvider(providerName);

if (!provider.capabilities.text) {
throw self.apos.error('invalid',
`Provider "${providerName}" does not support text generation`
);
}

return provider;
},

/**
* Get the configured image provider
* @returns {Object} Provider module and capabilities
*/
getImageProvider() {
const providerName = self.options.imageProvider;
const provider = self.getProvider(providerName);

if (!provider.capabilities.image) {
throw self.apos.error('invalid',
`Provider "${providerName}" does not support image generation`
);
}

return provider;
},

/**
* List all registered providers
* @returns {Array} Array of provider info
*/
listProviders() {
return Array.from(self.providers.entries()).map(([ name, info ]) => ({
name,
label: info.label,
capabilities: info.capabilities
}));
},

/**
* Check if user has permission to use AI features
*/
checkPermissions(req) {
// If the user cannot edit at least one content type, they have
// no business talking to the AI
if (!Object.keys(self.apos.modules).some(type => self.apos.permission.can(req, 'edit', type))) {
if (!Object.keys(self.apos.modules).some(type =>
self.apos.permission.can(req, 'edit', type)
)) {
throw self.apos.error('forbidden');
}
},

/**
* Log AI usage to console for monitoring and debugging
* @param {Object} req - Request object
* @param {Object} data - Usage data to log
* @param {string} data.type - 'text' or 'image'
* @param {string} data.provider - Provider name
* @param {string} data.model - Model used
* @param {string} data.prompt - User prompt
* @param {Object} [data.usage] - Token usage data
* @param {Object} [data.metadata] - Additional metadata
*/
logUsage(req, data) {
if (!self.options.logUsage) {
return;
}

const username = req.user?.username || 'unknown';
const {
type, provider, prompt, metadata
} = data;

console.log(`\n[AI Usage] ${type} generation by ${username}:`);
Copy link
Member

Choose a reason for hiding this comment

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

What's with all the console.log? This is structured data, use our structured logging features.

Copy link
Member

Choose a reason for hiding this comment

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

Please add our standard linter to the module so we can detect things like console.log calls that should not have been made (I recognize this is a little beyond the scope and will make the PR harder to read on a one-time basis, but it helps catch stuff).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the correct linter - eslint-config-apostrophe is already in the project.

console.log({
type,
provider,
...metadata,
prompt: prompt.length > 100 ? prompt.substring(0, 100) + '...' : prompt
});
},

/**
* Store AI usage to MongoDB for permanent audit trail
Copy link
Member

Choose a reason for hiding this comment

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

An audit trail of apostrophecms actions in general is a desirable feature but shouldn't appear out of the blue in just one optional module, that will lead to many audit collections in many modules etc. Also, if you use structured logging it should be possible for customers who really want it to implement auditing. So I think you should remove the db collection and the logging functionality.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Both removed.

* @param {Object} req - Request object
* @param {Object} data - Usage data to store
* @param {string} data.type - 'text' or 'image'
* @param {string} data.provider - Provider name
* @param {string} data.prompt - User prompt
* @param {Object} [data.metadata] - Additional metadata (usage, model, etc.)
*/
async storeUsage(req, data) {
if (!self.options.storeUsage) {
return;
}

const {
type, provider, prompt, metadata = {}
} = data;

const document = {
_id: self.apos.util.generateId(),
userId: req.user._id,
username: req.user.username || req.user._id,
createdAt: new Date(),
type,
provider,
prompt,
...metadata
};

try {
await self.aposAiHelperUsage.insertOne(document);
} catch (e) {
// Log error but don't fail the request
self.apos.util.error('Failed to store AI usage:', e);
}
}
};
}
Expand All @@ -41,4 +268,4 @@ function getBundleModuleNames() {
.readdirSync(source, { withFileTypes: true })
.filter(dirent => dirent.isDirectory())
.map(dirent => `@apostrophecms/${dirent.name}`);
}
};
Loading