feat(content-ai): Stability AI image generation integration [Story 484.1]#796
Open
netogarozi-dev wants to merge 2 commits into
Open
feat(content-ai): Stability AI image generation integration [Story 484.1]#796netogarozi-dev wants to merge 2 commits into
netogarozi-dev wants to merge 2 commits into
Conversation
…4.1] Implements @aiox/content-ai package with Stability AI v2beta API integration for programmatic image generation. Includes retry logic, error hierarchy, timeout handling, and full test coverage (8/8 tests passing). - packages/content-ai/stability.js: core implementation (generateImage + error classes) - packages/content-ai/index.js: public API entry point - packages/content-ai/bin/generate.js: CLI binary (--prompt flag) - packages/content-ai/tests/stability.test.js: 8 unit tests via node:test - packages/content-ai/eslint.config.cjs: ESM-compatible local ESLint config - jest.config.js: exclude content-ai from root Jest (uses node:test) - .gitignore: exclude output/ directory (generated images) Smoke test: 1.4MB PNG generated in 13s via sd3.5-large model. Architect review: CONCERNS verdict (GO) -- 0 CRITICAL/HIGH, 3 MEDIUM as tech debt. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ory 484.1] The package-local eslint.config.cjs is never loaded by ESLint v9 flat config (only the root config is read), so packages/content-ai/*.js failed lint as commonjs parsing. Added an override in eslint.config.js and removed the dead local config; ran --fix for trailing commas.
netogarozi-dev
requested review from
Pedrovaleriolopez and
oalanicolas
as code owners
|
@netogarozi-dev is attempting to deploy a commit to the SINKRA - AIOX Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
|
Welcome to aiox-core! Thanks for your first pull request. What happens next?
PR Checklist:
Thanks for contributing! |
github-actions
Bot
added
type: test
WalkthroughIntroduces the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
packages/content-ai/tests/stability.test.js (1)
81-81: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winUse English for test names for consistency.
All test names are written in Portuguese, which creates inconsistency with the rest of the codebase and most projects' naming conventions. Standardize on English across the test suite.
🔤 Proposed test name translations
- test('success: retorna metadados corretos e salva arquivo em disco', async () => { + test('success: returns correct metadata and saves file to disk', async () => { - test('ConfigurationError quando STABILITY_API_KEY está ausente', async () => { + test('ConfigurationError when STABILITY_API_KEY is missing', async () => { - test('ConfigurationError quando prompt é vazio', async () => { + test('ConfigurationError when prompt is empty', async () => { - test('429 → aguarda Retry-After → sucesso no segundo attempt', async () => { + test('429 → waits for Retry-After → succeeds on second attempt', async () => { - test('429 duas vezes → lança RateLimitError', async () => { + test('429 twice → throws RateLimitError', async () => { - test('500 → lança StabilityApiError sem retry', async () => { + test('500 → throws StabilityApiError without retry', async () => { - test('timeout → lança TimeoutError', async () => { + test('timeout → throws TimeoutError', async () => { - test('diretório de output criado automaticamente se não existir', async () => { + test('output directory created automatically if not exists', async () => {Also applies to: 99-99, 114-114, 127-127, 140-140, 155-155, 176-176, 198-198
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/content-ai/tests/stability.test.js` at line 81, The test names in the stability.test.js file are currently written in Portuguese, which creates inconsistency with the rest of the codebase. Translate all test description strings passed to the test function calls from Portuguese to English across the entire file. This includes the test at line 81 with the description "success: retorna metadados corretos e salva arquivo em disco" and all other Portuguese test descriptions referenced in the comments (at lines 99, 114, 127, 140, 155, 176, and 198). Replace each Portuguese description with its appropriate English equivalent while maintaining the semantic meaning of the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/content-ai/bin/generate.js`:
- Line 2: The import statement for stability.js uses a relative path which
violates the repository's absolute import rule. Replace the relative import path
`../stability.js` with the appropriate absolute import path as configured in
your project (typically a package alias or configured path alias). Keep all the
imported symbols (generateImage, ConfigurationError, StabilityApiError,
RateLimitError, TimeoutError) the same, only changing how the module is
referenced from a relative path to an absolute one.
- Around line 31-33: The error handling in the else block reads err.message
directly without verifying that err is an Error object; if a non-Error value
(such as a string or object) is thrown, err.message will be undefined, causing
the log to output "Unexpected error: undefined". Modify the process.stderr.write
call to safely handle both Error and non-Error thrown values by checking if err
is an instance of Error and using err.message when available, otherwise convert
err to a string using String(err) to preserve the actual error context in the
output.
In `@packages/content-ai/index.js`:
- Around line 1-2: The re-exports for generateImage, StabilityApiError,
RateLimitError, TimeoutError, and ConfigurationError are currently using
relative import paths (./stability.js) which violates the repository import
guidelines. Replace both relative paths with absolute module paths that follow
the project's module resolution configuration. Consult your project's module
path aliases (typically in jsconfig.json, tsconfig.json, or package.json exports
field) to determine the correct absolute path for the stability module, then
update both export statements to use that absolute path instead of the relative
./stability.js reference.
In `@packages/content-ai/stability.js`:
- Around line 162-168: The _log function is logging the raw prompt text by
including the prompt property with a slice of the user-provided input, which can
expose sensitive data or PII in logs. Remove the prompt field from the _log call
entirely to prevent logging any user-provided content, keeping only the
necessary metadata fields like event, model, aspect_ratio, and ts.
- Around line 98-104: The code assumes the Retry-After header is always numeric
seconds, but it can also be an HTTP-date format. When parseInt receives a
non-numeric string, it returns NaN, causing the setTimeout to resolve
immediately instead of backing off. Add a check after parsing the
retryAfterHeader to verify that the parsed value is a valid number using
Number.isNaN(), and if it is NaN, use RETRY_AFTER_DEFAULT_MS instead. This
ensures waitMs is always a valid number before being passed to setTimeout.
In `@packages/content-ai/tests/stability.test.js`:
- Line 3: Replace the relative import path with an absolute import path in the
stability.test.js file. Change the import statement that imports generateImage,
StabilityApiError, RateLimitError, TimeoutError, and ConfigurationError from
'../stability.js' to use the absolute import path
'`@aiox/content-ai/stability.js`' instead. This aligns with the coding guidelines
that require absolute imports over relative imports.
---
Nitpick comments:
In `@packages/content-ai/tests/stability.test.js`:
- Line 81: The test names in the stability.test.js file are currently written in
Portuguese, which creates inconsistency with the rest of the codebase. Translate
all test description strings passed to the test function calls from Portuguese
to English across the entire file. This includes the test at line 81 with the
description "success: retorna metadados corretos e salva arquivo em disco" and
all other Portuguese test descriptions referenced in the comments (at lines 99,
114, 127, 140, 155, 176, and 198). Replace each Portuguese description with its
appropriate English equivalent while maintaining the semantic meaning of the
test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ebcfc6eb-2744-402e-8095-59e7180ebf47
📒 Files selected for processing (8)
.gitignoreeslint.config.jsjest.config.jspackages/content-ai/bin/generate.jspackages/content-ai/index.jspackages/content-ai/package.jsonpackages/content-ai/stability.jspackages/content-ai/tests/stability.test.js
| @@ -0,0 +1,35 @@ | |||
| #!/usr/bin/env node | |||
| import { generateImage, ConfigurationError, StabilityApiError, RateLimitError, TimeoutError } from '../stability.js'; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use an absolute import in the CLI entrypoint.
Line 2 imports ../stability.js via a relative path; this breaks the repo-wide absolute-import rule.
As per coding guidelines, **/*.{js,jsx,ts,tsx}: Use absolute imports instead of relative imports in all code.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/content-ai/bin/generate.js` at line 2, The import statement for
stability.js uses a relative path which violates the repository's absolute
import rule. Replace the relative import path `../stability.js` with the
appropriate absolute import path as configured in your project (typically a
package alias or configured path alias). Keep all the imported symbols
(generateImage, ConfigurationError, StabilityApiError, RateLimitError,
TimeoutError) the same, only changing how the module is referenced from a
relative path to an absolute one.
Source: Coding guidelines
Comment on lines
+31
to
+33
| } else { | ||
| process.stderr.write(`Unexpected error: ${err.message}\n`); | ||
| } |
There was a problem hiding this comment.
Harden unexpected-error output for non-Error throws.
The fallback branch reads err.message directly; if a non-Error value is thrown, stderr logs undefined and loses context.
Suggested patch
} catch (err) {
+ const fallbackMessage = err instanceof Error ? err.message : String(err);
if (err instanceof ConfigurationError) {
process.stderr.write(`Configuration error: ${err.message}\n`);
} else if (err instanceof RateLimitError) {
process.stderr.write(`Rate limit exceeded: ${err.message}\n`);
} else if (err instanceof StabilityApiError) {
process.stderr.write(`API error (${err.statusCode}): ${err.message}\n`);
} else if (err instanceof TimeoutError) {
process.stderr.write(`Timeout: ${err.message}\n`);
} else {
- process.stderr.write(`Unexpected error: ${err.message}\n`);
+ process.stderr.write(`Unexpected error: ${fallbackMessage}\n`);
}
process.exit(1);
}As per coding guidelines, **/*.js: Verify error handling is comprehensive.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/content-ai/bin/generate.js` around lines 31 - 33, The error handling
in the else block reads err.message directly without verifying that err is an
Error object; if a non-Error value (such as a string or object) is thrown,
err.message will be undefined, causing the log to output "Unexpected error:
undefined". Modify the process.stderr.write call to safely handle both Error and
non-Error thrown values by checking if err is an instance of Error and using
err.message when available, otherwise convert err to a string using String(err)
to preserve the actual error context in the output.
Source: Coding guidelines
Comment on lines
+1
to
+2
| export { generateImage } from './stability.js'; | ||
| export { StabilityApiError, RateLimitError, TimeoutError, ConfigurationError } from './stability.js'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Switch re-exports to absolute module paths.
These re-exports currently use a relative path (./stability.js), which violates the repository import-path rule for JS/TS files.
As per coding guidelines, **/*.{js,jsx,ts,tsx}: Use absolute imports instead of relative imports in all code.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/content-ai/index.js` around lines 1 - 2, The re-exports for
generateImage, StabilityApiError, RateLimitError, TimeoutError, and
ConfigurationError are currently using relative import paths (./stability.js)
which violates the repository import guidelines. Replace both relative paths
with absolute module paths that follow the project's module resolution
configuration. Consult your project's module path aliases (typically in
jsconfig.json, tsconfig.json, or package.json exports field) to determine the
correct absolute path for the stability module, then update both export
statements to use that absolute path instead of the relative ./stability.js
reference.
Source: Coding guidelines
Comment on lines
+98
to
+104
| const retryAfterHeader = response.headers.get('Retry-After'); | ||
| const waitMs = retryAfterHeader | ||
| ? parseInt(retryAfterHeader, 10) * 1000 | ||
| : RETRY_AFTER_DEFAULT_MS; | ||
|
|
||
| _log({ event: 'stability.rate_limit.retry', waitMs, ts: new Date().toISOString() }); | ||
| await new Promise((resolve) => setTimeout(resolve, waitMs)); |
There was a problem hiding this comment.
Handle non-numeric Retry-After values before sleeping.
Line 99 assumes Retry-After is integer seconds. If the API returns an HTTP-date value, parseInt(...) yields NaN, and Line 104 retries immediately instead of backing off.
Proposed fix
+function _parseRetryAfterMs(retryAfterHeader) {
+ if (!retryAfterHeader) return RETRY_AFTER_DEFAULT_MS;
+
+ const seconds = Number(retryAfterHeader);
+ if (Number.isFinite(seconds) && seconds > 0) {
+ return seconds * 1000;
+ }
+
+ const retryAt = Date.parse(retryAfterHeader);
+ if (!Number.isNaN(retryAt)) {
+ return Math.max(retryAt - Date.now(), RETRY_AFTER_DEFAULT_MS);
+ }
+
+ return RETRY_AFTER_DEFAULT_MS;
+}
+
async function _callWithRetry(prompt, apiKey) {
const requestBody = _buildRequestBody(prompt);
const response = await _callApi(requestBody, apiKey);
if (response.status === 429) {
const retryAfterHeader = response.headers.get('Retry-After');
- const waitMs = retryAfterHeader
- ? parseInt(retryAfterHeader, 10) * 1000
- : RETRY_AFTER_DEFAULT_MS;
+ const waitMs = _parseRetryAfterMs(retryAfterHeader);As per coding guidelines, "Verify error handling is comprehensive."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const retryAfterHeader = response.headers.get('Retry-After'); | |
| const waitMs = retryAfterHeader | |
| ? parseInt(retryAfterHeader, 10) * 1000 | |
| : RETRY_AFTER_DEFAULT_MS; | |
| _log({ event: 'stability.rate_limit.retry', waitMs, ts: new Date().toISOString() }); | |
| await new Promise((resolve) => setTimeout(resolve, waitMs)); | |
| function _parseRetryAfterMs(retryAfterHeader) { | |
| if (!retryAfterHeader) return RETRY_AFTER_DEFAULT_MS; | |
| const seconds = Number(retryAfterHeader); | |
| if (Number.isFinite(seconds) && seconds > 0) { | |
| return seconds * 1000; | |
| } | |
| const retryAt = Date.parse(retryAfterHeader); | |
| if (!Number.isNaN(retryAt)) { | |
| return Math.max(retryAt - Date.now(), RETRY_AFTER_DEFAULT_MS); | |
| } | |
| return RETRY_AFTER_DEFAULT_MS; | |
| } | |
| async function _callWithRetry(prompt, apiKey) { | |
| const requestBody = _buildRequestBody(prompt); | |
| const response = await _callApi(requestBody, apiKey); | |
| if (response.status === 429) { | |
| const retryAfterHeader = response.headers.get('Retry-After'); | |
| const waitMs = _parseRetryAfterMs(retryAfterHeader); | |
| _log({ event: 'stability.rate_limit.retry', waitMs, ts: new Date().toISOString() }); | |
| await new Promise((resolve) => setTimeout(resolve, waitMs)); |
🧰 Tools
🪛 ast-grep (0.44.0)
[warning] 103-103: Avoid using the initial state variable in setState
Context: setTimeout(resolve, waitMs)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(setstate-same-var)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/content-ai/stability.js` around lines 98 - 104, The code assumes the
Retry-After header is always numeric seconds, but it can also be an HTTP-date
format. When parseInt receives a non-numeric string, it returns NaN, causing the
setTimeout to resolve immediately instead of backing off. Add a check after
parsing the retryAfterHeader to verify that the parsed value is a valid number
using Number.isNaN(), and if it is NaN, use RETRY_AFTER_DEFAULT_MS instead. This
ensures waitMs is always a valid number before being passed to setTimeout.
Source: Coding guidelines
Comment on lines
+162
to
+168
| _log({ | ||
| event: 'stability.generate.start', | ||
| prompt: prompt.slice(0, 200), | ||
| model: MODEL, | ||
| aspect_ratio: '1:1', | ||
| ts: new Date().toISOString(), | ||
| }); |
There was a problem hiding this comment.
Avoid logging raw prompt text.
Line 164 logs user-provided prompt content, which can leak sensitive data/PII into logs.
Proposed fix
_log({
event: 'stability.generate.start',
- prompt: prompt.slice(0, 200),
+ promptLength: prompt.length,
model: MODEL,
aspect_ratio: '1:1',
ts: new Date().toISOString(),
});As per coding guidelines, "Look for potential security vulnerabilities."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/content-ai/stability.js` around lines 162 - 168, The _log function
is logging the raw prompt text by including the prompt property with a slice of
the user-provided input, which can expose sensitive data or PII in logs. Remove
the prompt field from the _log call entirely to prevent logging any
user-provided content, keeping only the necessary metadata fields like event,
model, aspect_ratio, and ts.
Source: Coding guidelines
| @@ -0,0 +1,208 @@ | |||
| import { test, describe, beforeEach, afterEach } from 'node:test'; | |||
| import assert from 'node:assert/strict'; | |||
| import { generateImage, StabilityApiError, RateLimitError, TimeoutError, ConfigurationError } from '../stability.js'; | |||
There was a problem hiding this comment.
Use absolute import for the stability module.
Per coding guidelines, relative imports should be replaced with absolute imports. The package name is @aiox/content-ai, so this should use an absolute import path.
🔧 Proposed fix
-import { generateImage, StabilityApiError, RateLimitError, TimeoutError, ConfigurationError } from '../stability.js';
+import { generateImage, StabilityApiError, RateLimitError, TimeoutError, ConfigurationError } from '`@aiox/content-ai`';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import { generateImage, StabilityApiError, RateLimitError, TimeoutError, ConfigurationError } from '../stability.js'; | |
| import { generateImage, StabilityApiError, RateLimitError, TimeoutError, ConfigurationError } from '`@aiox/content-ai`'; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/content-ai/tests/stability.test.js` at line 3, Replace the relative
import path with an absolute import path in the stability.test.js file. Change
the import statement that imports generateImage, StabilityApiError,
RateLimitError, TimeoutError, and ConfigurationError from '../stability.js' to
use the absolute import path '`@aiox/content-ai/stability.js`' instead. This
aligns with the coding guidelines that require absolute imports over relative
imports.
Source: Coding guidelines
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
packages/content-ai/**override ineslint.config.jsand removed the dead local config.Test plan
npm run lint— 0 errorsnpm run typecheck— passesnode --test packages/content-ai/tests/stability.test.js— 8/8 passingnpm testhas 222 pre-existing failures (CodeRabbit CLI requiring WSL on this Windows host) — verified identical failures onmainbaseline, not a regression from this changeStory: docs/stories/epic-484-content-ai-integration/STORY-484.1-STABILITY-AI-API-INTEGRATION.md (status: InReview)
Summary by CodeRabbit
Release Notes
New Features
Tests
Chores