Skip to content

feat: Adobe Firefly Services as a drop-in image provider - #36

Merged
martinkrakowski merged 5 commits into
mainfrom
feat/firefly-adapter
Jun 9, 2026
Merged

feat: Adobe Firefly Services as a drop-in image provider#36
martinkrakowski merged 5 commits into
mainfrom
feat/firefly-adapter

Conversation

@martinkrakowski

@martinkrakowski martinkrakowski commented Jun 9, 2026

Copy link
Copy Markdown
Owner

PR Summary by Qodo

Add Adobe Firefly as a drop-in ImageGeneratorPort provider with graceful fallback
✨ Enhancement 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

Walkthroughs

User Description

Adds Adobe Firefly Services as a first-class image-generation provider — a live demonstration that the architecture is built for it.

The point

The domain depends only on ImageGeneratorPort. Adopting Firefly is one new adapter + one line at the composition root — the use case, compliance, export, and UI never change. That's the "Firefly is one adapter away" claim, made real.

What's here

  • FireflyImageGenerator (packages/CreativeGeneration): Adobe IMS client-credentials auth → Firefly v3 generate → fetch the presigned asset → cover-fit to the exact ratio. Degrades to the injected fallback on any failure, same pattern as the Imagen/OpenRouter adapters.
  • One-line wiring in apps/api/server/lib/pipeline.ts: selectable via the model picker or ?model=firefly; falls through to Imagen → OpenRouter → procedural when Firefly is unavailable or credentials are absent.
  • Provenance: a new firefly BackgroundSource flows to the run report and a FIREFLY badge in the review grid (so a Firefly run is visible, honest, and never silently a fallback).
  • Config: FIREFLY_CLIENT_ID / FIREFLY_CLIENT_SECRET documented in .env.example; README updated.

Quality

  • Full test coverage for the new adapter (mocked IMS/generate/image, success + every failure/fallback branch) and the new wiring branches.
  • The repo's 100% coverage gate stays green; typecheck clean.

Demo note: this PR is the "drop-in adapter" moment for the Campaign Foundry walkthrough. To make Firefly the default primary (rather than an explicit selection), it's a one-line reorder in imageGenerator().

AI Description
• Add Adobe Firefly Services adapter implementing ImageGeneratorPort with IMS auth and cover-fit
  output.
• Wire selectable firefly model into the API composition root and allowlist, preserving fallback
  chain.
• Surface Firefly provenance in UI/run reporting, with docs/env updates and full adapter tests.
Diagram
graph TD
  UI["Web UI (model picker + badges)"] --> API["API pipeline.ts (composition root)"] --> Port["ImageGeneratorPort"] --> FF["FireflyImageGenerator"] --> Adobe["Adobe APIs (IMS + Firefly v3)"]
  FF --> Imagen["GeminiImageGenerator (Imagen)"] --> OR["OpenRouterImageGenerator"] --> Proc["Procedural generator"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Cache IMS access tokens in FireflyImageGenerator
  • ➕ Reduces per-image latency and load (auth call amortized across requests).
  • ➕ Less risk of tripping IMS rate limits during large runs.
  • ➖ Requires token expiry tracking and concurrency-safe refresh logic.
  • ➖ More stateful adapter behavior; slightly higher implementation/test complexity.
2. Share a common cover-fit utility across adapters
  • ➕ Avoids duplicated cover-fit logic (OpenRouter, AssetReuse, Firefly).
  • ➕ Single place to tune image scaling/cropping behavior.
  • ➖ Refactor touches multiple adapters at once (higher blast radius).
  • ➖ Not strictly required to demonstrate the ports-and-adapters claim.
3. Use an official Adobe Firefly/IMS SDK (if available) instead of raw fetch
  • ➕ Potentially more robust auth handling, retries, and typings.
  • ➕ May reduce maintenance burden if API details change.
  • ➖ Introduces a new dependency and its lifecycle/security considerations.
  • ➖ SDK abstractions can be leaky; raw HTTP is straightforward here.

Recommendation: The PR’s approach (a dedicated Firefly adapter behind ImageGeneratorPort plus minimal composition-root wiring) is the right architectural fit for a ports-and-adapters system and keeps the domain/UI stable. The main worthwhile follow-up is token caching to reduce repeated IMS auth calls during large runs; the other alternatives are optional refinements rather than better core strategies.

Grey Divider

File Changes

Enhancement (7)
pipeline.ts Wire Firefly into imageGenerator selection and allowlist +15/-1

Wire Firefly into imageGenerator selection and allowlist

• Adds 'firefly' to the server allowlist and introduces Firefly as a selectable generator that falls back to Imagen → OpenRouter → procedural when unavailable or unconfigured.

apps/api/server/lib/pipeline.ts


page.tsx Render FIREFLY provenance badge styling +1/-0

Render FIREFLY provenance badge styling

• Extends the background-source badge mapping to include a Firefly label and styling for assets sourced from Firefly.

apps/web/src/app/(shell)/grid/page.tsx


models.ts Expose Adobe Firefly in the model picker catalog +1/-0

Expose Adobe Firefly in the model picker catalog

• Adds a 'firefly' model option so users can select Firefly as the primary generator from the UI.

apps/web/src/lib/models.ts


run-context.tsx Add 'firefly' to Asset backgroundSource union type +2/-2

Add 'firefly' to Asset backgroundSource union type

• Extends the UI run-report Asset type to include Firefly provenance for display and filtering.

apps/web/src/lib/run-context.tsx


BackgroundSource.vo.ts Add Firefly to BackgroundSource provenance type +4/-3

Add Firefly to BackgroundSource provenance type

• Extends the domain BackgroundSource union and documentation so Firefly provenance can flow through run reports.

packages/CampaignOrchestration/src/domain/value-objects/BackgroundSource.vo.ts


FireflyImageGenerator.ts Implement FireflyImageGenerator adapter (IMS auth → generate → fetch → cover-fit) +150/-0

Implement FireflyImageGenerator adapter (IMS auth → generate → fetch → cover-fit)

• Introduces a new ImageGeneratorPort adapter that authenticates via Adobe IMS client credentials, calls Firefly v3 generate, fetches the presigned asset, cover-fits to target dimensions, and degrades to an injected fallback on any failure.

packages/CreativeGeneration/src/infrastructure/adapters/FireflyImageGenerator.ts


index.ts Export Firefly adapter from CreativeGeneration infrastructure barrel +1/-0

Export Firefly adapter from CreativeGeneration infrastructure barrel

• Adds FireflyImageGenerator to the generated infrastructure exports so the API composition root can import it.

packages/CreativeGeneration/src/infrastructure/index.ts


Tests (3)
pipeline.test.ts Expand pipeline composition tests for Firefly branches +7/-1

Expand pipeline composition tests for Firefly branches

• Adds Firefly env keys to test setup, asserts 'firefly' is allowlisted, and exercises buildPipeline branches with/without Firefly credentials.

apps/api/server/lib/tests/pipeline.test.ts


grid.test.tsx Add FIREFLY badge coverage in grid rendering test +2/-0

Add FIREFLY badge coverage in grid rendering test

• Seeds a run containing a Firefly-sourced asset and asserts the FIREFLY provenance badge renders alongside existing badges.

apps/web/src/app/(shell)/grid/tests/grid.test.tsx


FireflyImageGenerator.test.ts Add comprehensive FireflyImageGenerator tests with mocked fetch +124/-0

Add comprehensive FireflyImageGenerator tests with mocked fetch

• Covers success path and multiple failure modes (auth, missing token, generate errors, missing URL, image fetch failure, rejected fetch), verifying fallback behavior and request shaping.

packages/CreativeGeneration/src/infrastructure/adapters/tests/FireflyImageGenerator.test.ts


Documentation (1)
README.md Add Firefly provider setup and update fallback-chain docs +11/-2

Add Firefly provider setup and update fallback-chain docs

• Documents Firefly credentials, selection via 'firefly' model, and updates the described fallback chain to include Firefly as an optional primary source.

README.md


Other (1)
.env.example Document Firefly IMS credentials in example env +7/-0

Document Firefly IMS credentials in example env

• Adds FIREFLY_CLIENT_ID and FIREFLY_CLIENT_SECRET entries with guidance and fallback behavior notes.

.env.example


Grey Divider

Qodo Logo

Demonstrates the ports-and-adapters payoff: FireflyImageGenerator implements the
existing ImageGeneratorPort (Adobe IMS client-credentials auth -> Firefly v3
generate -> fetch the presigned asset -> cover-fit), and adopting it is one new
adapter plus a single line at the composition root — the use case, compliance,
export, and UI are untouched.

- Selectable via the model picker / ?model=firefly; degrades to Imagen -> OpenRouter
  -> procedural when Firefly is unavailable or its credentials are absent.
- 'firefly' provenance flows through to the run report and a FIREFLY badge in the grid.
- Full test coverage for the adapter (mocked IMS/generate/image) and the new wiring;
  the repo's 100% gate stays green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@qodo-code-review

qodo-code-review Bot commented Jun 9, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1)

Context used
✅ Compliance rules (platform): 4 rules

Grey Divider


Action required

1. FireflyImageGenerator uses console.warn 📘 Rule violation ◔ Observability
Description
FireflyImageGenerator.resolveBackground() logs via console.warn with an interpolated string
instead of the shared structured logger. This breaks consistency/structured logging requirements and
makes filtering/alerting harder.
Code

packages/CreativeGeneration/src/infrastructure/adapters/FireflyImageGenerator.ts[R68-74]

+    } catch (error) {
+      const message = error instanceof Error ? error.message : String(error);
+      if (this.fallback) {
+        // Observable degradation — a bad credential or a Firefly outage drops to the
+        // next generator (which reports its own source), so a run never aborts.
+        console.warn(`[FireflyImageGenerator] failed for ${product.id} @ ${ratio.value}; using fallback. ${message}`);
+        return this.fallback.resolveBackground(product, ratio, context);
Evidence
PR Compliance ID 960794 requires that new logging uses the shared structured logger (not
console.*) and uses structured context fields rather than interpolated strings. The added line in
FireflyImageGenerator uses console.warn with an interpolated message, violating this
requirement.

Rule 960794: Use shared structured logger for all application logging
packages/CreativeGeneration/src/infrastructure/adapters/FireflyImageGenerator.ts[68-74]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A new logging statement uses `console.warn(...)` and string interpolation rather than the shared structured logger and structured (context, eventName) format.

## Issue Context
Compliance requires all new/modified logging to go through the shared logger at `src/infrastructure/logging/logger.ts`, using a structured call style (context object first, then a short machine-readable event name).

## Fix Focus Areas
- packages/CreativeGeneration/src/infrastructure/adapters/FireflyImageGenerator.ts[68-74]
- src/infrastructure/logging/logger.ts[1-120]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

2. No IMS token caching ✓ Resolved 🐞 Bug ☼ Reliability
Description
FireflyImageGenerator.resolveBackground() calls authenticate() on every background generation,
causing repeated IMS token requests during a campaign run (up to 8 in parallel). This adds avoidable
latency and increases the chance of transient auth failures that push runs into fallback generators.
Code

packages/CreativeGeneration/src/infrastructure/adapters/FireflyImageGenerator.ts[R58-66]

+  async resolveBackground(
+    product: Product,
+    ratio: AspectRatio,
+    context: BackgroundContext,
+  ): Promise<BackgroundResult> {
+    try {
+      const token = await this.authenticate();
+      const url = await this.generate(token, this.buildPrompt(product, context), ratio);
+      const cover = await this.coverFit(await this.fetchImage(url), ratio);
Evidence
The Firefly adapter obtains a new IMS token inside resolveBackground(), and the use case calls
imageGenerator.resolveBackground once per product×ratio cell with up to 8 workers; together this
means multiple token requests per run and bursts of concurrent IMS auth calls.

packages/CreativeGeneration/src/infrastructure/adapters/FireflyImageGenerator.ts[58-96]
packages/CampaignOrchestration/src/application/use-cases/GenerateCampaignUseCase.use-case.ts[20-49]
packages/CampaignOrchestration/src/application/use-cases/GenerateCampaignUseCase.use-case.ts[121-152]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`FireflyImageGenerator` performs an Adobe IMS client-credentials token request for every `resolveBackground()` call. Campaign runs resolve many backgrounds concurrently, so this multiplies auth traffic and adds latency.

## Issue Context
- The pipeline resolves one background per product×ratio cell, with bounded concurrency (`MAX_CONCURRENT_BACKGROUNDS = 8`).
- The Firefly adapter is new and can reuse a bearer token across multiple generations until it expires.

## Fix Focus Areas
- packages/CreativeGeneration/src/infrastructure/adapters/FireflyImageGenerator.ts[58-96]
- packages/CampaignOrchestration/src/application/use-cases/GenerateCampaignUseCase.use-case.ts[20-49]
- packages/CampaignOrchestration/src/application/use-cases/GenerateCampaignUseCase.use-case.ts[121-152]

## Suggested fix
1. Extend `ImsTokenResponse` to optionally include `expires_in` (if available) and cache `{ token, expiresAt }` on the instance.
2. Add an `inFlightAuth?: Promise<string>` (or similar) so concurrent `resolveBackground()` calls share a single token request.
3. In `authenticate()`:
  - If a cached token exists and `Date.now() < expiresAt`, return it.
  - Otherwise create/await `inFlightAuth`, store the token + computed expiry, then clear `inFlightAuth`.
4. (Optional but helpful) Add a unit test asserting that two parallel `resolveBackground()` calls result in only one IMS `fetch()` call when the cached token is still valid.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

3. Env log misses Firefly ✓ Resolved 🐞 Bug ◔ Observability
Description
apps/api/server/lib/env.ts announces configured image providers at startup but ignores
FIREFLY_CLIENT_ID/FIREFLY_CLIENT_SECRET. With only Firefly credentials set, it incorrectly warns
that generation is “procedural only,” reducing debuggability.
Code

apps/api/server/lib/pipeline.ts[R55-63]

function imageGenerator(selected?: string): ImageGeneratorPort {
  const procedural = new ProceduralBackgroundGenerator();
  const geminiKey = process.env.GEMINI_API_KEY ?? process.env.GOOGLE_API_KEY;
  const openRouterKey = process.env.OPENROUTER_API_KEY;
+  const fireflyId = process.env.FIREFLY_CLIENT_ID;
+  const fireflySecret = process.env.FIREFLY_CLIENT_SECRET;

  // An OpenRouter generator for a given model, falling back to procedural.
  const openRouter = (model?: string): ImageGeneratorPort =>
Evidence
The PR adds Firefly credential reads and generator selection in pipeline.ts, but the existing env.ts
provider summary only checks Imagen/OpenRouter keys and otherwise warns ‘procedural only’; that
warning becomes false when only Firefly creds are configured.

apps/api/server/lib/pipeline.ts[55-92]
apps/api/server/lib/env.ts[46-64]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The server startup log in `apps/api/server/lib/env.ts` determines whether GenAI providers are configured, but it only checks Imagen/OpenRouter env vars. This PR adds Firefly credentials and wiring, so the log output becomes misleading for Firefly-only setups.

## Issue Context
`pipeline.ts` now reads `FIREFLY_CLIENT_ID`/`FIREFLY_CLIENT_SECRET` and can construct `FireflyImageGenerator`, but `env.ts` does not reflect this in its provider summary.

## Fix Focus Areas
- apps/api/server/lib/env.ts[46-64]
- apps/api/server/lib/pipeline.ts[55-92]

## Suggested fix
1. In `env.ts`, detect Firefly creds (both `FIREFLY_CLIENT_ID` and `FIREFLY_CLIENT_SECRET`) and push a provider string like `firefly (FIREFLY_CLIENT_ID/FIREFLY_CLIENT_SECRET ✓; select with ?model=firefly)`.
2. Adjust the `procedural only — no GenAI keys detected` warning text to mention Firefly as well, so operators don’t chase the wrong configuration.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@martinkrakowski, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 17 minutes and 17 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 2831b639-c2b0-4fa1-8ed0-46f9bebd06f6

📥 Commits

Reviewing files that changed from the base of the PR and between 6862192 and a69be63.

📒 Files selected for processing (15)
  • .agents/session-log.md
  • .env.example
  • README.md
  • apps/api/server/lib/__tests__/env.test.ts
  • apps/api/server/lib/__tests__/pipeline.test.ts
  • apps/api/server/lib/env.ts
  • apps/api/server/lib/pipeline.ts
  • apps/web/src/app/(shell)/grid/__tests__/grid.test.tsx
  • apps/web/src/app/(shell)/grid/page.tsx
  • apps/web/src/lib/models.ts
  • apps/web/src/lib/run-context.tsx
  • packages/CampaignOrchestration/src/domain/value-objects/BackgroundSource.vo.ts
  • packages/CreativeGeneration/src/infrastructure/adapters/FireflyImageGenerator.ts
  • packages/CreativeGeneration/src/infrastructure/adapters/__tests__/FireflyImageGenerator.test.ts
  • packages/CreativeGeneration/src/infrastructure/index.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/firefly-adapter

Comment @coderabbitai help to get the list of available commands and usage tips.

martinkrakowski and others added 3 commits June 9, 2026 19:11
Every resolveBackground() ran a fresh client-credentials grant, so one
campaign run fired product × ratio token requests with up to 8 in
flight at once — avoidable latency plus a wider transient-auth failure
surface, for tokens IMS reports as ~24 h-lived. Cache {token,
expiresAt} with a 60 s refresh margin and share a single in-flight
grant between concurrent cells; a settled grant clears itself so a
failed one is retried rather than poisoning later generations.

Addresses qodo review finding #2 on PR #36.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
env.ts's provider log only knew the Imagen/OpenRouter keys, so a
Firefly-only setup warned "procedural only — no GenAI keys detected"
even though ?model=firefly would generate real imagery. List Firefly
when both credentials are present (noting it is opt-in via model
selection, not part of the default chain) and warn when only one of
the two credentials is set, since that silently does nothing.

Addresses qodo review finding #3 on PR #36.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@martinkrakowski

Copy link
Copy Markdown
Owner Author

qodo review triage — each finding verified against the codebase

# Finding Verdict Action
1 console.warn instead of shared structured logger ❌ Rejected none — see the threaded reply
2 No IMS token caching ✅ Confirmed fixed in 7eb89d2
3 Startup env log misses Firefly ✅ Confirmed fixed in ffd16db

1 — rejected. The rule mirrors AGENTS.md, but the logger it requires (src/infrastructure/logging/logger.ts) doesn't exist anywhere in the repo, no logging library is installed, the eslint-no-console enforcement isn't configured, and console.* with a [Component] prefix is the established convention — including the two sibling adapters this line is modelled on. Repo-wide structured-logger adoption (or amending AGENTS.md to match reality) is logged as a follow-up in .agents/session-log.md; until one happens, compliance rule 960794 will keep firing on every PR that logs.

2 — confirmed, fixed in 7eb89d2. resolveBackground() ran a fresh IMS client-credentials grant per call, and the use case resolves one background per product × ratio cell with up to 8 in flight (MAX_CONCURRENT_BACKGROUNDS) — e.g. a 4-product brief fired 12 token requests, the first 8 concurrently, for tokens IMS reports as ~24 h-lived. The adapter now caches {token, expiresAt} (60 s refresh margin; conservative 5 min TTL when IMS omits expires_in) and concurrent generations share a single in-flight grant; a settled grant clears itself so a failed one is retried per cell rather than poisoning later generations. Four new tests: sequential reuse, concurrent dedupe, expiry-margin refresh, failed-grant retry.

3 — confirmed, fixed in ffd16db. env.ts only checked the Imagen/OpenRouter keys, so a Firefly-only setup warned procedural only — no GenAI keys detected even though ?model=firefly would generate real imagery. The startup summary now lists firefly (FIREFLY_CLIENT_ID/FIREFLY_CLIENT_SECRET ✓; select the "firefly" model) — phrased that way because credentials alone don't change the default Imagen → OpenRouter chain — and additionally warns when only one of the two credentials is set, since a half-configured Firefly silently does nothing. Two new tests.

Verified locally: build, typecheck, lint, and the full suite (286/286 tests) all green.

🤖 Generated with Claude Code

CI's 100% branch gate caught the warning's untested ternary arm
(FIREFLY_CLIENT_SECRET set without FIREFLY_CLIENT_ID).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@martinkrakowski
martinkrakowski merged commit 31768bc into main Jun 9, 2026
3 checks passed
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.

1 participant