Skip to content

feat(backend): implement engine connection routing (issue #127) - #160

Merged
BerryUIKI merged 15 commits into
devfrom
bugfix/engine-connection-routing-127
Oct 6, 2026
Merged

BerryUIKI merged 15 commits into
devfrom
bugfix/engine-connection-routing-127

Conversation

@BerryUIKI

Copy link
Copy Markdown
Owner

Summary

Implements backend infrastructure for issue #127: resolving execution and lifecycle operations through the selected engine connection.

Problem

  • Creative runners used global comfy_client singleton instead of resolving from selected connection
  • No connection_id propagated from frontend to backend
  • cancel_task() checked engine type strings instead of actual connection
  • Cache didn't isolate results by connection
  • Frontend reconstructed URLs from port, ignoring configured endpoints

Backend Changes Implemented

Schema Updates

  • Added connection_id: Optional[str] to CreativeActionRequest
  • Added connection_id: str to GenerationProvenance
  • Maintained backward compatibility with legacy engine_id

ComfyUIClient Enhancement

  • Added base_url parameter to constructor
  • Uses configured URL instead of reconstructing from host:port
  • Supports custom protocols, paths, and non-localhost addresses

CreativeRunner Core Methods

  • _resolve_connection(): Resolves connection_id with engine_id fallback chain
  • _create_client(): Creates type-specific clients (ComfyUI/WebUI) from connections
  • compute_creative_cache_hash(): Includes connection_id for cache isolation (v0.3.0)
  • cancel_task(): Uses resolved connection for interrupt routing
  • execute(): Resolves connection before all operations

Test Coverage

  • Connection resolution with valid/invalid IDs
  • Legacy engine_id fallback mapping
  • Client creation for both engine types
  • Cache hash isolation verification
  • Execute flow with connection resolution
  • Cancellation routing to correct endpoint

What Works Now

✅ Backend resolves stable connection_id to engine endpoint
✅ Cache isolates results by connection (different connections = different cache entries)
✅ Cancellation targets the correct connection
✅ All operations (upload, queue, poll, download) use same resolved client
✅ Backward compatible with legacy engine_id
✅ External connections properly differentiated from managed

Frontend Changes Still Needed

The backend is complete and ready, but frontend integration is required for the full feature to work end-to-end.

Architecture Preservation

Cache Invalidation

Runner version bumped from 0.2.0 → 0.3.0 to invalidate old cache entries that don't include connection_id.

Related Issues

Verification Evidence

Backend Contract Tests: ✅ 8 connection routing tests added
Connection Resolution: ✅ Implemented with fallback chain
Cache Isolation: ✅ Verified with different connection_ids
Cancellation Routing: ✅ Targets correct connection

Live Engine Testing: ⏳ Requires GPU + frontend integration
Frontend Integration: ⏳ Awaiting separate PR for frontend changes

🤖 Generated with Claude Code

- Accept optional base_url parameter in ComfyUIClient constructor
- Use provided base_url if given, otherwise construct from host:port
- Update _create_client to pass base_url from connection.url

This allows ComfyUIClient to use the configured connection URL
directly instead of always reconstructing http://host:port.
Core implementation:
- Add connection_id to CreativeActionRequest and GenerationProvenance schemas
- Add base_url parameter to ComfyUIClient for configured endpoints
- Implement _resolve_connection() with fallback chain (connection_id > engine_id)
- Implement _create_client() for type-specific client creation
- Update compute_creative_cache_hash() to include connection_id (v0.3.0)
- Update cancel_task() to use resolved connection
- Update execute() to resolve connection before dispatch
- Pass resolved comfy_client to _run_comfy()
- Add comprehensive test suite for connection routing

Cache isolation: Results from different connections are properly isolated.
Backward compatibility: Legacy engine_id still supported with deprecation warning.
@BerryUIKI
BerryUIKI enabled auto-merge (squash) October 6, 2026 17:22
- Replace comfy_client global with ComfyUIClient class mocks
- Add engine_manager mocks for connection resolution
- Update test_img2img and test_inpaint to work with new architecture

All upload tests now properly mock the connection routing flow.
- Fix test_comfy_asset_transfer_contract to use ComfyUIClient mock
- Fix test_cancellation_and_actions to mock engine_manager
- All tests now work with connection resolution architecture

Tests properly mock the connection routing flow introduced in #127.
EngineConnection schema requires endpoint_url field. Updated all
mock connections in test_engine_connection_routing.py.
EngineConnection schema uses endpoint_url not url. Updated all
references in creative_runner and test files.
compute_creative_cache_hash now requires connection_id as second
positional parameter. Updated all test calls.
The actual EngineManager API uses get_engine() not get_connection().
Updated all references in creative_runner and test mocks.
…routing

- Replace global comfy_client patches with ComfyUIClient class mocks
- Add mock_engine_manager fixture to all tests
- Use shared connection fixtures from conftest.py
…_hash calls

Updated cache hash function signature requires connection_id as second
positional parameter for connection-aware cache isolation (v0.3.0).
All integration tests now use the shared mock_engine_manager fixture
to properly resolve engine connections during execution.
Removed orphaned duplicate test code that was causing IndentationError
during test collection.
- test_comfy_asset_transfer_contract: use proper EngineConnection objects instead of Mock
- test_cancellation_and_actions: add connection_id to active task and mock ComfyUIClient
- All tests now properly handle connection routing
The test was failing with 'No connection_id or engine_id provided' error
despite having engine_id in the request because engine_manager was not mocked.
@BerryUIKI
BerryUIKI merged commit 1e71cdf into dev Oct 6, 2026
3 checks passed
@BerryUIKI
BerryUIKI deleted the bugfix/engine-connection-routing-127 branch October 6, 2026 20:35
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