feat(image_gen): port FAL backend to plugins/image_gen/fal#27966
feat(image_gen): port FAL backend to plugins/image_gen/fal#279660xDevNinja wants to merge 1 commit into
Conversation
Mirrors the architecture established by the web (NousResearch#25182), browser (NousResearch#25214), and video_gen (NousResearch#25126) plugin migrations: * `tools/fal_common.py` — stateless atoms shared by both FAL-backed plugins (image_gen + video_gen). Holds the lazy `fal_client` import helper, `_ManagedFalSyncClient`, `_normalize_fal_queue_url_format`, `_extract_http_status`. Stateful pieces (`fal_client` module global, `_managed_fal_client*` cache, `_submit_fal_request`, `_resolve_managed_fal_gateway`, `_get_managed_fal_client`) intentionally stay on `tools.image_generation_tool` so the existing `monkeypatch.setattr(image_tool, ...)` patch sites keep working unchanged. * `plugins/video_gen/fal/__init__.py` — drops its inline `_load_fal_client` duplicate; consumes `tools.fal_common.import_fal_client`. * `plugins/image_gen/fal/{plugin.yaml,__init__.py}` — new plugin. `FalImageGenProvider` is a thin registration adapter that resolves the legacy module via `import tools.image_generation_tool as _it` and calls `_it.image_generate_tool` + `_it._resolve_fal_model` at call time. The 18-model catalog, `_build_fal_payload`, managed- gateway selection, and Clarity Upscaler chaining all remain in `tools.image_generation_tool` as the single source of truth — the plugin is a registration adapter, not a parallel implementation. * `tools/image_generation_tool.py::_dispatch_to_plugin_provider` — drops the `configured == "fal"` skip. Setting `image_gen.provider: fal` now routes through the registry like any other provider; the plugin re-enters this module's pipeline so behavior is identical. Unset `image_gen.provider` still falls through to the in-tree pipeline (preserves no-config-with-FAL_KEY UX from NousResearch#15696). * `hermes_cli/tools_config.py` — drops the hardcoded "FAL.ai" row from `TOOL_CATEGORIES["image_gen"]["providers"]` (now injected by `_plugin_image_gen_providers` like every other backend) and the `getattr(provider, "name") == "fal"` skip that protected against duplication with the hardcoded row. The "Nous Subscription" row stays as a setup-flow entry — same shape browser kept "Nous Subscription (Browser Use cloud)" after NousResearch#25214. * `tests/plugins/image_gen/test_fal_provider.py` — 14 cases covering the ABC surface, call-time indirection (verifying `monkeypatch.setattr(image_tool, "image_generate_tool", ...)` takes effect through the plugin), response-shape stamping, exception handling, and registry wiring. * `tests/plugins/image_gen/check_parity_vs_main.py` — subprocess harness mirroring `tests/plugins/browser/check_parity_vs_main.py`. Pins one path to origin/main, one to the worktree; runs six scenarios (unset, explicit-fal-no-creds, explicit-fal-with-creds, explicit-fal-with-model, typo provider, managed-gateway-only) and diffs the reduced shape `{dispatch_kind, provider_name, model}` per scenario. The only acceptable diff is "legacy_fal → plugin (fal)" for explicit-FAL paths — every other delta is flagged as a regression. * `tests/hermes_cli/test_image_gen_picker.py::test_fal_surfaced_alongside_other_plugins` — flips the previous `test_fal_skipped_to_avoid_duplicate` to match the new shape (FAL is a plugin now, no dedup needed). Verified: 195/195 tests across `tests/{tools/test_image_generation*,tools/test_managed_media_gateways,plugins/image_gen,plugins/video_gen,hermes_cli/test_image_gen_picker}.py` pass on this branch with no test patches modified outside the picker test that asserted the old skip behaviour. Fixes NousResearch#26241
|
BoardJames triage: this looks shared/systemic rather than branch-local. The PR-specific checks (lint/nix/e2e/builds/attribution/history) are green where completed; the remaining blocker is the main |
|
Merged via PR #30380 — your commit was cherry-picked onto current main (your branch was 475 commits behind, cherry-pick was clean) and rebase-merged so your authorship is preserved in git log (commit 3ac2125). Thanks for the thorough work — the call-time indirection pattern + parity harness + zero-test-file-moves was exactly the right shape. |
What does this PR do?
Ports the FAL.ai image-generation backend out of
tools/image_generation_tool.pyand into a real plugin atplugins/image_gen/fal/, matching the architecture established by theweb (#25182), browser (#25214), and video_gen (#25126) plugin
migrations. After this lands,
TOOL_CATEGORIES["image_gen"]["providers"]carries only the "NousSubscription" setup-flow row — same shape browser kept after #25214.
Per the discussion at
#26241 (comment)
and the rules in
plugin-extraction-test-patch-compatibility.md,nothing in the existing
tests/tools/test_image_generation*.pyortests/tools/test_managed_media_gateways.pypatch surface needs tomove — the plugin reaches into the legacy module via
import tools.image_generation_tool as _itand resolves everypatchable name (
_submit_fal_request,_resolve_managed_fal_gateway,_get_managed_fal_client,fal_client,_managed_fal_client) atcall time.
Architecture
tools/fal_common.py(new) — stateless atoms shared by bothFAL-backed plugins:
import_fal_client()— the lazyfal_clientimport +tools.lazy_deps.ensureintegration that previously livedduplicated in
image_generation_toolandplugins/video_gen/fal/__init__.py._ManagedFalSyncClient— managed-queue wrapper. Now takesfal_clientas an injected argument instead of reading a moduleglobal, so the legacy module's
monkeypatch.setattr(image_tool, "fal_client", mock)keeps working when the wrapper is constructedvia
_get_managed_fal_client._normalize_fal_queue_url_format,_extract_http_status— purehelpers.
Stateful pieces stay on
tools.image_generation_tool(thefal_clientmodule global,_managed_fal_client*cache + lock,_submit_fal_request,_resolve_managed_fal_gateway,_get_managed_fal_client). Moving them intofal_commonwouldsilently defeat the existing patch sites that reach for
image_tool._managed_fal_clientetc., because the function'sfree-variable resolution would go against
fal_common's namespaceinstead. Rule 3 of the contributor doc, intact.
plugins/video_gen/fal/__init__.py— drops its inline_load_fal_clientduplicate; the cached_fal_clientmoduleglobal stays on the video plugin (per-module caches don't leak
across consumers). One small commit-scoped diff.
plugins/image_gen/fal/{plugin.yaml,__init__.py}— new plugin.FalImageGenProvideris a thin registration adapter that resolvesimport tools.image_generation_tool as _itinside every method(
is_available,list_models,default_model,generate). The18-model catalog,
_build_fal_payload, managed-gateway selection,and Clarity Upscaler chaining all remain in the legacy module —
the plugin is a registration adapter, not a parallel implementation.
generate()JSON-parses the legacyimage_generate_toolresponseand stamps
provider/prompt/aspect_ratio/modelfor the unifiedImageGenProviderresponse shape.tools/image_generation_tool.py::_dispatch_to_plugin_provider—drops the
configured == "fal"skip. Settingimage_gen.provider: falnow routes through the registry like anyother provider; the plugin re-enters this module's pipeline so the
behaviour is identical (the parity harness verifies this). Unset
image_gen.providerstill falls through to the in-tree pipeline,preserving the no-config-with-
FAL_KEYUX from refactor(memory): remove flush_memories entirely #15696.hermes_cli/tools_config.py— drops the hardcoded "FAL.ai" rowfrom
TOOL_CATEGORIES["image_gen"]["providers"]and theprovider.name == "fal"skip in_plugin_image_gen_providers.The "Nous Subscription" row stays — same shape browser kept "Nous
Subscription (Browser Use cloud)" after Mirror web-provider plugin migration for browser providers #25214.
Test coverage
tests/plugins/image_gen/test_fal_provider.py(new, 14 cases):name,display_name,list_modelsparity withFAL_MODELS,default_modelmatches legacyDEFAULT_MODEL,get_setup_schemaadvertisesFAL_KEY)._it.check_fal_api_key, swallowsexceptions so the picker never propagates them).
monkeypatch.setattr(image_tool, "image_generate_tool", fake)is picked up inside
FalImageGenProvider.generate(). This is theinvariant that lets every existing patch site keep working.
resolve_aspect_ratioclamps invalids tolandscape), passthrough kwarg filtering (drops
Nones beforeforwarding into the legacy payload builder), exception handling
(legacy
image_generate_toolraising →success=Falsewithtyped error), invalid JSON response handling, response-shape
stamping (
provider/prompt/aspect_ratio/model).register()callsctx.register_image_gen_provider).tests/plugins/image_gen/check_parity_vs_main.py(new) —subprocess harness mirroring
tests/plugins/browser/check_parity_vs_main.py. Pins one venv toorigin/main, one to the worktree, runs six scenarios:
legacy_fallegacy_fal; PR:plugin (fal)legacy_fal; PR:plugin (fal)modelfield matchesprovider_not_registerederrorlegacy_falThe only acceptable diff is "legacy_fal → plugin (fal)" for
explicit-FAL paths — flagged as
[DIFF]rather than[FAIL].Everything else is treated as a behavioural regression and exits
non-zero.
tests/hermes_cli/test_image_gen_picker.py— flippedtest_fal_skipped_to_avoid_duplicate→test_fal_surfaced_alongside_other_plugins.Asserts the new shape: FAL is a plugin,
_plugin_image_gen_providerssurfaces it like every other backend, no dedup needed.
Verification
tests/tools/test_image_generation*.pyandtests/tools/test_managed_media_gateways.pypatch sites areunchanged — the indirection pattern delivers on its promise.
Type of Change
Related
feat(video_gen): unified video_generate tool with pluggable provider backends #25126 (video_gen)
Out of scope (per issue body)
_upscale_imagebecoming pluggable — coupled to per-modelupscale: bool; can ship as its own plugin category later.plugins/video_gen/fal— file aseparate issue if useful.
ImageGenProvider.list_models()entries with stricter typing.