Skip to content

refactor: remove legacy provider wrappers#109

Open
caffeinum wants to merge 1 commit intomainfrom
cleanup/remove-legacy-providers
Open

refactor: remove legacy provider wrappers#109
caffeinum wants to merge 1 commit intomainfrom
cleanup/remove-legacy-providers

Conversation

@caffeinum
Copy link
Copy Markdown
Contributor

Summary

  • deleted 5 legacy providers (apify, replicate, fal, elevenlabs, higgsfield) — -1,462 net lines
  • action definitions now call @fal-ai/client, @elevenlabs/elevenlabs-js, @higgsfield/client directly instead of going through provider abstractions
  • removed replicate from all model definitions (fal is the only provider now)
  • kept ffmpeg, groq, fireworks, storage providers (infrastructure/unique capabilities)

Test plan

  • all 18 tests pass (bun run src/tests/all.test.ts)
  • zero typescript errors (tsc --noEmit)
  • biome lint passes
  • smoke test varg run image --prompt "test" with FAL_KEY
  • smoke test varg run voice --text "test" with ELEVENLABS_API_KEY

🤖 Generated with Claude Code

delete apify, replicate, fal, elevenlabs, higgsfield provider wrappers.
action definitions now call @fal-ai/client, @elevenlabs/elevenlabs-js,
@higgsfield/client directly instead of going through provider abstractions.

kept ffmpeg, groq, fireworks, storage providers (infrastructure/unique caps).
removed replicate from model definitions (fal is the only provider now).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Feb 6, 2026

📝 Walkthrough

Walkthrough

this pr consolidates the provider architecture by removing replicate support entirely and replacing legacy provider classes (fal, elevenlabs, higgsfield, apify, replicate) with direct client library calls and subscriptions, centralizing url handling and queue logging into shared utilities, and stripping large portions of the public api surface.

Changes

Cohort / File(s) Summary
Schema & Model Definitions
src/core/schema/shared.ts, src/definitions/models/flux.ts, src/definitions/models/kling.ts, src/definitions/models/nano-banana-pro.ts, src/definitions/models/wan.ts
removed replicate provider from enum and model definitions, leaving only fal as supported backend
New Action Utilities
src/definitions/actions/utils.ts
added ensureUrl helper for local-to-remote file uploading via fal.storage and logQueueUpdate for formatted queue status logging across action invocations
Action Refactoring - Direct Fal Subscriptions
src/definitions/actions/captions.ts, src/definitions/actions/grok-edit.ts, src/definitions/actions/music.ts, src/definitions/actions/qwen-angles.ts, src/definitions/actions/sync.ts, src/definitions/actions/video.ts
replaced falProvider with fal.subscribe calls, integrated ensureUrl for input normalization, added onQueueUpdate logging, removed upload-related storage logic
Action Refactoring - Direct Client Instantiation
src/definitions/actions/image.ts, src/definitions/actions/voice.ts
switched from HiggsfieldProvider/elevenlabsProvider to direct HiggsfieldClient and ElevenLabsClient instantiation; removed upload options from public signatures and storage dependencies
Provider File Deletions
src/providers/apify.ts, src/providers/elevenlabs.ts, src/providers/fal.ts, src/providers/higgsfield.ts, src/providers/replicate.ts
removed entire provider classes, utilities, and high-level wrappers totaling ~1400 lines of legacy provider infrastructure
Public API Consolidation
src/index.ts, src/providers/index.ts
removed ~35 re-exported identifiers from provider modules (MODELS, VOICES, provider singletons, high-level helpers); pruned provider registry to only groq, fireworks, ffmpeg, storage
Test Updates
src/tests/all.test.ts, src/tests/unit.test.ts
replaced provider-based live tests with direct client library calls; updated test expectations to reflect only 4 registered providers instead of 8

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🌊 from tangled providers we break free
direct clients flow like the open sea
replicate fades, fal remains so true
utils consolidate the old into new
meow—the architecture breathes fresh air ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed the title clearly summarizes the main change: removing legacy provider wrappers to simplify the codebase architecture.
Description check ✅ Passed the description is related to the changeset and provides context: it lists what was deleted, what's calling client libraries now, and includes a test plan.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch cleanup/remove-legacy-providers

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
src/definitions/actions/captions.ts (1)

155-160: ⚠️ Potential issue | 🟡 Minor

this ffmpeg command doesn't actually burn subtitles

line 160 just does -c copy — all the srt→ass conversion work above is unused. the comment on line 159 acknowledges this, but it's essentially dead code producing .ass files that nothing consumes. worth a todo/issue if you plan to wire the subtitle filter in later.

want me to open an issue to track wiring up the actual subtitle burn (e.g. -vf ass=file.ass)?

src/definitions/actions/music.ts (1)

173-179: ⚠️ Potential issue | 🟡 Minor

format fallback "wav" doesn't match schema default "mp3"

line 175 falls back to "wav" for the file extension, but the schema default on line 21 is "mp3". if format somehow arrives as falsy (it shouldn't after zod parsing, but the destructuring default on line 109 is also "mp3"), the extension would be wrong.

quick fix
-      const ext = format || "wav";
+      const ext = format || "mp3";
src/definitions/actions/sync.ts (1)

104-116: ⚠️ Potential issue | 🟡 Minor

originalVideo is destructured but never used — overlay does nothing

lipsyncOverlay accepts originalVideo but the ffmpeg command just copies lipsyncedVideo to outputPath. the jsdoc says "overlay lip-synced face onto original video" but there's no actual compositing happening. if this is intentionally a placeholder, the docstring is misleading.

src/tests/all.test.ts (1)

7-15: ⚠️ Potential issue | 🟡 Minor

stale doc comment references REPLICATE_API_TOKEN

replicate was removed in this pr but the file header still lists it as a required env var. quick cleanup to avoid confusion. meow~

suggested fix
  * Note: Most tests require API keys to be set in environment variables:
  * - FAL_API_KEY (or FAL_KEY)
- * - REPLICATE_API_TOKEN
  * - ELEVENLABS_API_KEY
  * - GROQ_API_KEY
🤖 Fix all issues with AI agents
In `@src/definitions/actions/video.ts`:
- Around line 56-57: FalResult is redundantly defined three times; create a
single module-scope type alias named FalResult (e.g., at top of
src/definitions/actions/video.ts) and remove the inline definitions from the
local scopes where they currently appear (the inline defs that shadow FalResult
inside the functions or blocks). Update any references to use the module-level
FalResult type so all occurrences (previously duplicated at the three locations)
share the same definition.
- Line 68: The duration property is using the falsy fallback operator
(String(duration || 5)) which will incorrectly replace 0; update each occurrence
of the duration field in src/definitions/actions/video.ts to use the nullish
coalescing operator instead (i.e., String(duration ?? 5)) so only null/undefined
trigger the default. Change all four occurrences referenced (the duration
property lines around the action definitions where duration is set) ensuring you
replace `||` with `??` consistently.

In `@src/definitions/actions/voice.ts`:
- Around line 91-93: Add an early guard to validate
process.env.ELEVENLABS_API_KEY before constructing the ElevenLabsClient: check
the env var and if missing throw or log a clear error and abort (so the
ElevenLabsClient constructor is never called with apiKey: undefined). Update the
code around the ElevenLabsClient instantiation in voice.ts to perform this check
and provide a descriptive message referencing ELEVENLABS_API_KEY.
🧹 Nitpick comments (13)
src/definitions/actions/captions.ts (1)

6-6: prefer Bun.write over writeFileSync from node:fs

line 134 and line 203 use writeFileSync while line 174 already uses Bun.file for reading. mixing both is inconsistent — and the coding guidelines say to prefer Bun.file/Bun.write over node:fs.

suggested diff
-import { writeFileSync } from "node:fs";

then at line 134:

-    writeFileSync(srtFile, result.srt);
+    await Bun.write(srtFile, result.srt);

and at line 203:

-  writeFileSync(assPath, assHeader + assDialogues);
+  await Bun.write(assPath, assHeader + assDialogues);

As per coding guidelines, "Prefer Bun.file over node:fs readFile/writeFile methods for file operations".

Also applies to: 134-134

src/definitions/actions/voice.ts (2)

60-70: hardcoded voice ids are brittle but fine for now

if elevenlabs changes or retires any of these ids, this will silently break. consider pulling from the api or at least adding a comment noting where these came from for future maintainers. meow.


75-81: upload is destructured but never used

leftover from the old storage upload flow — safe to drop.

tiny cleanup
  const {
    text,
    voice = "rachel",
    provider = "elevenlabs",
-   upload = false,
    outputPath,
  } = options;
src/definitions/actions/utils.ts (1)

22-28: uploaded blob has no content type

the new Blob([buffer]) on line 28 doesn't set a mime type. some fal endpoints or storage backends might care. could pass { type: file.type } from Bun.file to be safe.

small tweak
  const file = Bun.file(pathOrUrl);
  if (!(await file.exists())) {
    throw new Error(`Local file not found: ${pathOrUrl}`);
  }

  const buffer = await file.arrayBuffer();
- return fal.storage.upload(new Blob([buffer]));
+ return fal.storage.upload(new Blob([buffer], { type: file.type }));
src/definitions/actions/image.ts (4)

76-84: as string cast on a string literal bypasses fal client endpoint type safety

the as string cast on "fal-ai/flux-pro/v1.1" widens the type to suppress fal's endpoint validation. this is fine as a pragmatic workaround if the endpoint isn't in fal's type registry yet, but it hides typos. just flagging it — same pattern shows up in video.ts too.

also, FalResult is defined inline here and again in video.ts. could pull it into utils.ts to avoid the duplication.


26-29: dead uploaded field in output schema and interface

the uploaded field in imageOutputSchema (line 28) and ImageGenerationResult (line 67) is never populated now that upload logic is removed. leaving it around is harmless but a little confusing for consumers.

🧹 clean up the stale field
 const imageOutputSchema = z.object({
   imageUrl: z.string(),
-  uploaded: z.string().optional(),
 });
 export interface ImageGenerationResult {
   imageUrl: string;
-  uploaded?: string;
 }

Also applies to: 65-68


100-103: higgsfieldclient instantiated on every call

a new HiggsfieldClient is created each invocation of generateWithSoul. if the constructor does any setup (auth handshake, token fetch, etc.), this could add unnecessary latency. consider hoisting it to module scope with lazy init.

lazy singleton example
+let _higgsfieldClient: HiggsfieldClient | null = null;
+function getHiggsfieldClient(): HiggsfieldClient {
+  if (!_higgsfieldClient) {
+    _higgsfieldClient = new HiggsfieldClient({
+      apiKey: process.env.HIGGSFIELD_API_KEY || process.env.HF_API_KEY,
+      apiSecret: process.env.HIGGSFIELD_SECRET || process.env.HF_API_SECRET,
+    });
+  }
+  return _higgsfieldClient;
+}
+
 export async function generateWithSoul(
   prompt: string,
   options: { styleId?: string } = {},
 ): Promise<ImageGenerationResult> {
   console.log("[image] generating with higgsfield soul");
-  const client = new HiggsfieldClient({
-    apiKey: process.env.HIGGSFIELD_API_KEY || process.env.HF_API_KEY,
-    apiSecret: process.env.HIGGSFIELD_SECRET || process.env.HF_API_SECRET,
-  });
+  const client = getHiggsfieldClient();

54-62: styleId is accepted by generateWithSoul but never forwarded from the action

the execute handler calls generateWithSoul(prompt) without passing any styleId. since the input schema also doesn't expose a styleId field, this parameter is effectively unreachable from the action path. that's fine if generateWithSoul is also called directly as a convenience function, but worth noting.

src/definitions/actions/qwen-angles.ts (1)

109-169: significant duplication between execute and qwenAngles

the fal.subscribe call, result casting, and image extraction logic are copy-pasted between the action's execute handler and the qwenAngles convenience function (~60 lines). the execute handler could just delegate to qwenAngles:

♻️ deduplicate by delegating
   execute: async (inputs) => {
-    const {
-      image,
-      horizontalAngle,
-      verticalAngle,
-      zoom,
-      prompt,
-      loraScale,
-      guidanceScale,
-      numInferenceSteps,
-      negativePrompt,
-      seed,
-      outputFormat,
-      numImages,
-    } = inputs;
-
-    console.log("[action/qwen-angles] adjusting camera angle");
-
-    const imageUrl = await ensureUrl(image);
-    const result = await fal.subscribe(
-      "fal-ai/qwen-image-edit-2511-multiple-angles",
-      {
-        input: {
-          image_urls: [imageUrl],
-          horizontal_angle: horizontalAngle ?? 0,
-          vertical_angle: verticalAngle ?? 0,
-          zoom: zoom ?? 5,
-          additional_prompt: prompt,
-          lora_scale: loraScale ?? 1,
-          guidance_scale: guidanceScale ?? 4.5,
-          num_inference_steps: numInferenceSteps ?? 28,
-          acceleration: "regular",
-          negative_prompt: negativePrompt ?? "",
-          seed,
-          output_format: outputFormat ?? "png",
-          num_images: numImages ?? 1,
-        },
-        logs: true,
-        onQueueUpdate: logQueueUpdate("qwen-angles"),
-      },
-    );
-
-    const data = result.data as { ... };
-    ...
-    return { imageUrl: firstImage.url, images, seed: data?.seed, prompt: data?.prompt };
+    return qwenAngles(inputs.image, {
+      horizontalAngle: inputs.horizontalAngle,
+      verticalAngle: inputs.verticalAngle,
+      zoom: inputs.zoom,
+      prompt: inputs.prompt,
+      loraScale: inputs.loraScale,
+      guidanceScale: inputs.guidanceScale,
+      numInferenceSteps: inputs.numInferenceSteps,
+      negativePrompt: inputs.negativePrompt,
+      seed: inputs.seed,
+      outputFormat: inputs.outputFormat,
+      numImages: inputs.numImages,
+    });
   },

Also applies to: 177-237

src/definitions/actions/music.ts (2)

6-6: prefer Bun.file().write() over node:fs/promises writeFile

the coding guidelines say to prefer Bun.file over node:fs for file operations. line 183 uses writeFile from node:fs/promises.

🐰 bun-ify it
-import { writeFile } from "node:fs/promises";

and at the write site:

-      const response = await fetch(audio.url);
-      const buffer = await response.arrayBuffer();
-      await writeFile(filePath, Buffer.from(buffer));
+      const response = await fetch(audio.url);
+      const buffer = await response.arrayBuffer();
+      await Bun.write(filePath, buffer);

As per coding guidelines, **/*.{ts,tsx,js,jsx}: "Prefer Bun.file over node:fs readFile/writeFile methods for file operations".

Also applies to: 181-183


42-43: stale upload/uploadUrls leftovers from removed provider flow

upload is destructured on line 112 but never read. uploadUrls in the output schema (line 42) and MusicResult interface (line 95) are never populated. same story as image.ts — clean these up to avoid confusing consumers. meow 🐱

Also applies to: 81-81, 95-96, 112-112

src/definitions/actions/grok-edit.ts (1)

56-94: same duplication pattern — execute and grokEditVideo are near-identical

same story as qwen-angles.ts. the execute handler could just call grokEditVideo(prompt, video, { resolution }) and you'd cut ~35 lines.

♻️ delegate from execute
   execute: async (inputs) => {
     const { prompt, video, resolution } = inputs;
-
     console.log("[action/grok-edit] editing video with Grok Imagine");
-
-    const inputUrl = await ensureUrl(video);
-    const result = await fal.subscribe("xai/grok-imagine-video/edit-video", {
-      ...
-    });
-    const data = result.data as { ... };
-    const videoUrl = data?.video?.url;
-    if (!videoUrl) { throw ... }
-    return { videoUrl, width: ..., height: ..., duration: ..., fps: ... };
+    return grokEditVideo(prompt, video, { resolution: resolution ?? "auto" });
   },

Also applies to: 102-142

src/definitions/actions/video.ts (1)

100-104: stale uploaded and duration fields on VideoGenerationResult

same pattern as image.ts — uploaded is never set, and duration is no longer returned from the fal response in this code. if these are kept for backward compat, at least drop uploaded since it was an internal concern.

Comment on lines +56 to +57
type FalResult = { data: { video?: { url?: string } } };
let result: FalResult;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion | 🟠 Major

FalResult type defined three times in the same file

lines 56, 113, and 145 each define the same FalResult type. hoist it to module scope once.

one type to rule them all
+type FalResult = { data: { video?: { url?: string } } };
+
 export const definition: ActionDefinition<typeof schema> = {

then remove the three inline definitions.

Also applies to: 113-113, 145-145

🤖 Prompt for AI Agents
In `@src/definitions/actions/video.ts` around lines 56 - 57, FalResult is
redundantly defined three times; create a single module-scope type alias named
FalResult (e.g., at top of src/definitions/actions/video.ts) and remove the
inline definitions from the local scopes where they currently appear (the inline
defs that shadow FalResult inside the functions or blocks). Update any
references to use the module-level FalResult type so all occurrences (previously
duplicated at the three locations) share the same definition.

input: {
prompt,
image_url: imageUrl,
duration: String(duration || 5),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

use ?? instead of || for duration fallback

String(duration || 5) would coerce 0 to 5. the schema probably prevents 0, but ?? is semantically correct for "default when nullish" and avoids the subtle falsy trap.

quick swap
-        duration: String(duration || 5),
+        duration: String(duration ?? 5),

(same for all four occurrences)

Also applies to: 81-81, 121-121, 151-151

🤖 Prompt for AI Agents
In `@src/definitions/actions/video.ts` at line 68, The duration property is using
the falsy fallback operator (String(duration || 5)) which will incorrectly
replace 0; update each occurrence of the duration field in
src/definitions/actions/video.ts to use the nullish coalescing operator instead
(i.e., String(duration ?? 5)) so only null/undefined trigger the default. Change
all four occurrences referenced (the duration property lines around the action
definitions where duration is set) ensuring you replace `||` with `??`
consistently.

Comment on lines +91 to +93
const client = new ElevenLabsClient({
apiKey: process.env.ELEVENLABS_API_KEY,
});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

no guard if ELEVENLABS_API_KEY is missing

if the env var isn't set, the client gets apiKey: undefined and will fail later with a potentially confusing error from the http layer. a quick early check would make debugging easier.

suggested guard
+ const apiKey = process.env.ELEVENLABS_API_KEY;
+ if (!apiKey) {
+   throw new Error("ELEVENLABS_API_KEY environment variable is required");
+ }
+
  const client = new ElevenLabsClient({
-   apiKey: process.env.ELEVENLABS_API_KEY,
+   apiKey,
  });
📝 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 client = new ElevenLabsClient({
apiKey: process.env.ELEVENLABS_API_KEY,
});
const apiKey = process.env.ELEVENLABS_API_KEY;
if (!apiKey) {
throw new Error("ELEVENLABS_API_KEY environment variable is required");
}
const client = new ElevenLabsClient({
apiKey,
});
🤖 Prompt for AI Agents
In `@src/definitions/actions/voice.ts` around lines 91 - 93, Add an early guard to
validate process.env.ELEVENLABS_API_KEY before constructing the
ElevenLabsClient: check the env var and if missing throw or log a clear error
and abort (so the ElevenLabsClient constructor is never called with apiKey:
undefined). Update the code around the ElevenLabsClient instantiation in
voice.ts to perform this check and provide a descriptive message referencing
ELEVENLABS_API_KEY.

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