feat: Add Web Request Mocking System for Integration Tests #1334
Conversation
…egroup-integration-testss
Changed sim_getValidators to sim_getAllValidators to match the actual RPC method implementation. The method sim_getValidators does not exist and was causing CI test failures. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1. Added mock_web_responses property to JSON schema for all plugin types (openai-compatible, ollama, google, anthropic, custom) to allow web request mocking in integration tests 2. Fixed RPC method names in tests: sim_getValidators → sim_getAllValidators 3. Fixed mock URL key to match actual test URL: this-domain-definitely-does-not-exist-12345.com These fixes resolve CI test failures where: - Schema validation was rejecting mock_web_responses parameter - Tests were calling non-existent RPC method - Mock URL key didn't match the URL being tested Note: Further investigation needed into why web.lua mocking isn't intercepting requests even when host_data contains mock_web_responses. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/protocol_rpc/redis_subscriber.py (1)
145-153: Reconnection can leak Redis resources; close pubsub/client before restart.On exceptions in the listener loop you call start() without closing existing connections.
@@ except Exception as e: logger.error(f"Error in event listener: {e}") if self.is_running: - # Clean up before reconnection - self.is_running = False - self.subscription_task = None - # Attempt to reconnect after delay - await asyncio.sleep(5) - await self.start() + # Clean up before reconnection + self.is_running = False + self.subscription_task = None + try: + if self.pubsub: + await self.pubsub.unsubscribe() + await self.pubsub.close() + self.pubsub = None + if self.redis_client: + await self.redis_client.close() + self.redis_client = None + except Exception: + logger.warning("Suppressing cleanup error during reconnect") + # Attempt to reconnect after delay + await asyncio.sleep(5) + await self.start()
🧹 Nitpick comments (25)
.env.example (1)
167-167: Add documentation comment for TEST_WITH_MOCKS variable.The variable definition lacks an explanatory comment describing its purpose and impact. Per coding guidelines, environment variables should be documented in .env.example.
Apply this diff to add a descriptive comment:
# Tests parameters + # When true, enables mocking for both LLM providers and web requests during integration tests TEST_WITH_MOCKS="false"This clarifies that TEST_WITH_MOCKS unifies control over both LLM and web request mocking, helping developers understand the scope and behavior of the flag when they enable it.
backend/database_handler/migration/versions/96840ab9133a_add_blocked_at_and_worker_id_to_.py (1)
24-30: Consider formatting bothadd_columncalls consistently.The first
add_columncall (lines 24-27) places each argument on a separate line, while the second call (lines 28-30) places the table name and Column definition on the same line. For better readability and consistency, both calls should follow the same formatting pattern.Apply this diff to align the formatting:
op.add_column( - "transactions", sa.Column("worker_id", sa.String(length=255), nullable=True) + "transactions", + sa.Column("worker_id", sa.String(length=255), nullable=True), )tests/unit/consensus/test_base.py (1)
562-562: Document the rationale for custom interval value.This is the only test using a custom
interval=0.01parameter. While a faster polling interval (10ms vs 100ms default) is safe with mocks and can speed up tests, consider adding a brief comment explaining why this particular test benefits from more aggressive polling compared to others.appeal(transaction, transactions_processor) + # Use faster interval for rapid PENDING->ACTIVATED transition current_status = assert_transaction_status_match( transactions_processor, transaction, [TransactionStatus.PENDING.value, TransactionStatus.ACTIVATED.value], interval=0.01, )tests/unit/consensus/test_helpers.py (1)
35-36: Consider renaming USE_MOCK_LLMS for clarity.The variable
USE_MOCK_LLMSis now controlled byTEST_WITH_MOCKS, which covers both LLM and web request mocking. Consider renaming this variable toUSE_MOCKSorUSE_TEST_MOCKSto better reflect its expanded scope.# Configuration for LLM mocking -USE_MOCK_LLMS = os.getenv("TEST_WITH_MOCKS", "true").lower() == "true" +USE_MOCKS = os.getenv("TEST_WITH_MOCKS", "true").lower() == "true"Note: This would require updating all references throughout the file (lines 117, 124, 138, 252, 260, 617, 904).
backend/protocol_rpc/message_handler/worker_handler.py (1)
115-122: Use get_running_loop; fall back to asyncio.run when no loopget_event_loop can be misleading in 3.12. Prefer get_running_loop + fallback.
Apply:
- try: - loop = asyncio.get_event_loop() - if loop.is_running(): - # Schedule the async send operation - asyncio.create_task(self._send_event_to_server(log_event)) - else: - # If no loop is running, try to run it synchronously - asyncio.run(self._send_event_to_server(log_event)) + try: + loop = asyncio.get_running_loop() + loop.create_task(self._send_event_to_server(log_event)) except RuntimeError: # No event loop available, log the event locally only - logger.debug( - f"No event loop available for worker {self.worker_id}, " - f"event not sent to server: {log_event.name}" - ) + asyncio.run(self._send_event_to_server(log_event))tests/integration/fixtures/conftest.py (1)
20-53: Harmonize env parsing and add type hints; prefer warnings over printMake the toggle robust to common truthy values, annotate return types, and avoid print in tests.
Apply:
+from typing import Optional +import warnings @@ -def mock_llms(): +def mock_llms() -> bool: """Check if LLM mocking is enabled""" - env_var = os.getenv("TEST_WITH_MOCKS", "false") # default no mocking - if env_var == "true": - return True - return False + env = os.getenv("TEST_WITH_MOCKS", "false").strip().lower() + return env in {"1", "true", "yes", "on"} @@ -def mock_web_requests(): +def mock_web_requests() -> bool: @@ -def load_mock_web_responses(): +def load_mock_web_responses() -> dict | None: @@ - mock_file = os.path.join( - os.path.dirname(__file__), - "..", - "fixtures", - "mock_responses", - "web", - "example_responses.json", - ) + mock_file = Path(__file__).parent / "mock_responses" / "web" / "example_responses.json" @@ - with open(mock_file, "r") as f: - return json.loads(f.read()) + return json.loads(Path(mock_file).read_text()) except (FileNotFoundError, json.JSONDecodeError) as e: - print(f"Warning: Could not load mock web responses: {e}") + warnings.warn(f"Could not load mock web responses: {e}") return {}tests/integration/contracts/conftest.py (1)
14-14: Consider if wildcard import is necessary.Pytest automatically discovers fixtures from parent
conftest.pyfiles in the directory hierarchy. Thefrom conftest import *may be redundant unless you're importing pytest hooks, non-fixture symbols, or session-scoped configuration that needs explicit inheritance.If the parent conftest only contains fixtures, you can remove line 14 and rely on pytest's automatic fixture discovery. If there are specific symbols (like pytest hooks or configuration) that need to be imported, consider importing them explicitly:
from conftest import pytest_configure, mock_llms, mock_web_requestsThis would resolve the Ruff F403 warning and make dependencies explicit.
backend/database_handler/validators_registry.py (1)
47-48: Optional optimization: Consider targeted cache invalidation for read-only RPC endpoints.Your concern is valid. The RPC endpoints (
count_validators,get_all_validators,get_validator) are network-exposed andexpire_all()forces full session invalidation on every call. However, this appears intentional for consistency in a multi-service environment where validators are modified externally (evidenced by the Redis event system and reader/writer locks in ValidatorManager).The
expire_all()usage is justified because:
- ValidatorManager's
do_write()pattern explicitly re-reads validators after writes- Validators can be modified by other services via the Redis event system
- Fresh data is required for correctness, not just performance
Consider whether:
- The performance impact justifies optimization (depends on actual call frequency)
session.refresh()on specific validator objects is viable after internal write operations (create_validator at line 74)- Stateless/cacheless read endpoints could bypass invalidation for external RPC calls
backend/protocol_rpc/redis_subscriber.py (2)
23-28: Annotate CHANNELS as ClassVar and prefer immutable tuple.Prevents RUF012 and accidental per-instance mutation.
-from typing import Optional, Callable, Dict, Any +from typing import Optional, Callable, Dict, Any, Awaitable, ClassVar @@ - CHANNELS = [ - "consensus:events", - "transaction:events", - "general:events", - "validator:events", - ] + CHANNELS: ClassVar[tuple[str, ...]] = ( + "consensus:events", + "transaction:events", + "general:events", + "validator:events", + ) @@ - self.event_handlers: Dict[str, Callable] = {} + self.event_handlers: Dict[str, Callable[[Dict[str, Any]], Awaitable[None]]] = {}
225-233: Add broadcast routing for validator events (optional).Without a map entry, validator events go to "broadcast". Map explicitly if UI listens on a specific channel.
channel_map = { "consensus:events": "consensus", "transaction:events": "transactions", "general:events": "general", + "validator:events": "validators", }tests/integration/icontracts/conftest.py (1)
8-11: Avoid duplicating sys.path hacks here (keep centralized).If top-level conftest already adds
tests/integration/fixturestosys.path, this is redundant. Prefer a single place to manage it, or switch topytest_plugins = [...]to share fixture modules.-# Add fixtures to path -fixtures_path = Path(__file__).parent.parent / "fixtures" -if str(fixtures_path) not in sys.path: - sys.path.insert(0, str(fixtures_path)) +# Path augmentation handled in the top-level conftest. +# If specific fixture modules are needed here, use: +# pytest_plugins = ("fixtures.common", "fixtures.web_mocks",)Please confirm top-level conftest sets the path. If not, I can propose a
pytest_pluginslist with the actual fixture module names.backend/validators/web.lua (2)
27-37: Enable mocks before URL validation.
web.check_url(payload.url)runs before the mock lookup; this prevents mocking non-HTTP/placeholder URLs in tests. Check mocks first, then validate only when falling through to real requests.-function Render(ctx, payload) +function Render(ctx, payload) ---@cast payload WebRenderPayload - web.check_url(payload.url) - - -- Return mock response if web mocking is enabled - if should_mock_web_requests(ctx) then + -- Return mock response if web mocking is enabled + if should_mock_web_requests(ctx) then local mock_response = get_mock_web_response_from_table( ctx.host_data.mock_web_responses.render, payload.url ) if mock_response then @@ - end - end + end + end + + web.check_url(payload.url)And similarly in Request():
- web.check_url(payload.url) - -- Return mock response if web mocking is enabled + -- Return mock response if web mocking is enabled if should_mock_web_requests(ctx) then @@ - end + end + web.check_url(payload.url)
6-16: Pattern match order is nondeterministic (pairs).
pairs(table)iteration order is undefined; if multiple patterns match a URL, the chosen mock may vary between runs. Consider an ordered list or priority rules.Example approach:
- Accept an array
{ {pattern="...", response=...}, ... }and iterate in order, or- Sort keys by length descending before matching to prefer specific patterns.
backend/validators/__init__.py (1)
339-375: Improve error logging and narrow exception scope.Use
logger.exceptionto keep tracebacks; consider narrowing the catch where feasible.- except Exception as e: - logger.error(f"Failed to publish validator change event: {e}") + except Exception: + logger.exception("Failed to publish validator change event")Optional: keep a small, long-lived Redis publisher instead of creating a client per event if events are frequent.
backend/protocol_rpc/app_lifespan.py (1)
320-331: Minor: drop unused arg and remove f-prefix.Silences ARG001/F541.
- async def handle_validator_change(event_data): + async def handle_validator_change(_event_data): """Reload validators when they change.""" - logger.info(f"RPC worker reloading validators due to change event") + logger.info("RPC worker reloading validators due to change event") await validators_manager.restart()Optional: If multiple events fire in quick succession, add a short debounce (e.g., gather events for ~250–500ms) to avoid repeated restarts.
tests/integration/icontracts/tests/test_web_mocking_contract.py (2)
16-19: Remove unused helper to keep tests lean.
_deployment_error_to_tx_receiptis not used. Delete it to reduce noise.-def _deployment_error_to_tx_receipt(e: DeploymentError): - """Helper function to extract transaction receipt from deployment error""" - return e.transaction_result +
81-91: Tidy: drop unused var and redundant f-string; prefer logging over prints.
is_mocking_llmsis never used.print(f"...")without placeholders is an unnecessary f-string.- Consider logging instead of printing in tests to avoid noisy output.
- is_mocking = mock_web_requests() - is_mocking_llms = mock_llms() + is_mocking = mock_web_requests() - print(f"\nCurrent mocking configuration:") + print("\nCurrent mocking configuration:")Optional: switch to
logging.getLogger(__name__).info(...).backend/protocol_rpc/validators_init.py (2)
18-22: Add return type and replace print with structured logging.Annotate the coroutine’s return type and avoid
-import backend.validators as validators +import logging +import backend.validators as validators @@ -async def initialize_validators( +async def initialize_validators( validators_json: str, db_session: Session, validators_manager: validators.Manager, -) : +)-> None: @@ - if not validators_json: - print("No validators to initialize") + if not validators_json: + logging.getLogger(__name__).info("No validators to initialize") returnAlso applies to: 32-35
36-40: Preserve traceback by chaining exceptions.Use
from ewhen re-raising to keep context.- except json.JSONDecodeError as e: - raise ValueError(f"Invalid JSON in validators_json: {str(e)}") + except json.JSONDecodeError as e: + raise ValueError(f"Invalid JSON in validators_json: {e}") from e @@ - except Exception as e: - raise ValueError(f"Failed to create validator `{validator_data}`: {str(e)}") + except Exception as e: + raise ValueError( + f"Failed to create validator `{validator_data}`: {e}" + ) from eAlso applies to: 91-92
tests/integration/conftest.py (1)
17-34: Add type hints and fix unused arg; aligns with project guidelines.Annotate the fixture and rename unused
configto_(or type it) to satisfy Ruff and coding standards.+from typing import Callable, Optional, Dict, Any @@ -@pytest.fixture -def setup_validators(): +@pytest.fixture +def setup_validators() -> Callable[..., None]: @@ - def _setup(mock_response=None, mock_web_responses=None, n_validators=5): + def _setup( + mock_response: Optional[Dict[str, Any]] = None, + mock_web_responses: Optional[Dict[str, Any]] = None, + n_validators: int = 5, + ) -> None: @@ -def pytest_configure(config): +def pytest_configure(_: pytest.Config) -> None:If you prefer minimalism, just add
-> Nonereturns and renameconfigto_.Also applies to: 84-86
tests/integration/icontracts/tests/test_web_mocking.py (3)
1-7: Remove unused import.
get_contract_factoryisn’t used here.-import pytest -import json -from gltest import get_contract_factory +import pytest +import json
11-18: Avoid equality to True/False; add return annotations.Use truthiness and
notinstead of== True/False, and annotate tests as returningNone.-def test_web_mocking_enabled(): +def test_web_mocking_enabled() -> None: @@ - assert mock_web_requests() == True + assert mock_web_requests() @@ -def test_web_requests_with_real_urls(): +def test_web_requests_with_real_urls() -> None: @@ - assert mock_web_requests() == False + assert not mock_web_requests()Also applies to: 20-27
29-61: Minor polish: annotate remaining tests with -> None.Keeps tests consistent with project type-hint guidance.
-def test_web_mock_with_custom_responses(setup_validators): +def test_web_mock_with_custom_responses(setup_validators) -> None: @@ -def test_error_web_mock(setup_validators): +def test_error_web_mock(setup_validators) -> None: @@ -def test_default_mock_responses_loading(setup_validators): +def test_default_mock_responses_loading(setup_validators) -> None:Also applies to: 63-85, 87-99
backend/node/create_nodes/providers_schema.json (1)
156-162: Tighten schema for mock fields; add structure and descriptions.
mock_responseandmock_web_responsesare plain objects; define their shapes to improve validation and DX.Within each plugin_config block, replace generic object types with refs and descriptions:
- "mock_response": { - "type": "object" - }, - "mock_web_responses": { - "type": "object" - } + "mock_response": { + "$ref": "#/$defs/MockLLMResponse", + "description": "Synthetic LLM response payload injected when TEST_WITH_MOCKS=true." + }, + "mock_web_responses": { + "$ref": "#/$defs/MockWebResponses", + "description": "Synthetic web responses for render/request when TEST_WITH_MOCKS=true." + }Add the following $defs at the root (once):
{ "$defs": { "MockLLMResponse": { "type": "object", "additionalProperties": true }, "MockWebResponses": { "type": "object", "properties": { "render": { "type": "object", "additionalProperties": { "type": "object", "properties": { "text": { "type": "string" } }, "required": ["text"], "additionalProperties": false } }, "request": { "type": "object", "additionalProperties": { "type": "object", "properties": { "status": { "type": "integer", "minimum": 100, "maximum": 599 }, "body": { "type": ["string", "object", "array", "null"] }, "headers": { "type": "object", "additionalProperties": { "type": "string" } } }, "required": ["status", "body"], "additionalProperties": false } } }, "additionalProperties": false } } }This preserves flexibility while catching malformed fixtures early.
Also applies to: 311-316, 364-370, 418-424, 498-504
tests/unit/test_simulator_sessions.py (1)
344-358: Remove unusedmonkeypatchparameter.The test doesn't use the
monkeypatchfixture, so it can be removed from the function signature.Apply this diff:
-async def test_delete_all_validators_uses_request_session(monkeypatch): +async def test_delete_all_validators_uses_request_session(): registry_instance = SimpleNamespace( delete_all_validators=AsyncMock(), get_all_validators=MagicMock(return_value=[]), )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (40)
.env.example(1 hunks).github/workflows/backend_integration_tests_pr.yml(2 hunks)backend/consensus/worker_service.py(1 hunks)backend/database_handler/llm_providers.py(3 hunks)backend/database_handler/migration/versions/96840ab9133a_add_blocked_at_and_worker_id_to_.py(1 hunks)backend/database_handler/validators_registry.py(2 hunks)backend/node/create_nodes/providers_schema.json(5 hunks)backend/protocol_rpc/app_lifespan.py(3 hunks)backend/protocol_rpc/endpoints.py(5 hunks)backend/protocol_rpc/message_handler/redis_worker_handler.py(2 hunks)backend/protocol_rpc/message_handler/worker_handler.py(2 hunks)backend/protocol_rpc/redis_subscriber.py(1 hunks)backend/protocol_rpc/rpc_methods.py(3 hunks)backend/protocol_rpc/validators_init.py(3 hunks)backend/validators/__init__.py(4 hunks)backend/validators/greyboxing.lua(1 hunks)backend/validators/web.lua(4 hunks)docker-compose.yml(3 hunks)tests/integration/conftest.py(1 hunks)tests/integration/contracts/analytics/test_log_indexer.py(1 hunks)tests/integration/contracts/conftest.py(1 hunks)tests/integration/contracts/errors/test_error_llm.py(1 hunks)tests/integration/contracts/errors/test_error_web.py(1 hunks)tests/integration/contracts/storage/test_storage.py(1 hunks)tests/integration/contracts/storage/test_user_storage.py(1 hunks)tests/integration/features/concurrent_transactions.feature(1 hunks)tests/integration/features/load_testing.feature(1 hunks)tests/integration/features/parallel_deployment.feature(1 hunks)tests/integration/features/security_resilience.feature(1 hunks)tests/integration/features/transaction_state_machine.feature(1 hunks)tests/integration/features/websocket_notifications.feature(1 hunks)tests/integration/fixtures/conftest.py(1 hunks)tests/integration/fixtures/schemas/call_contract_function.py(1 hunks)tests/integration/icontracts/conftest.py(1 hunks)tests/integration/icontracts/tests/test_web_mocking.py(1 hunks)tests/integration/icontracts/tests/test_web_mocking_contract.py(1 hunks)tests/unit/consensus/test_base.py(1 hunks)tests/unit/consensus/test_helpers.py(2 hunks)tests/unit/test_simulator_sessions.py(7 hunks)tests/unit/test_validators_init.py(4 hunks)
✅ Files skipped from review due to trivial changes (6)
- tests/integration/features/load_testing.feature
- tests/integration/features/concurrent_transactions.feature
- tests/integration/features/parallel_deployment.feature
- tests/integration/features/websocket_notifications.feature
- tests/integration/features/transaction_state_machine.feature
- tests/integration/features/security_resilience.feature
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/backend_integration_tests_pr.yml
🧰 Additional context used
📓 Path-based instructions (10)
docker-compose*.yml
📄 CodeRabbit inference engine (AGENTS.md)
Use docker-compose*.yml files for composing services
Files:
docker-compose.yml
{backend,examples,tests}/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Python code must be formatted with Black and include type hints
Files:
tests/integration/contracts/storage/test_storage.pytests/integration/icontracts/conftest.pytests/unit/consensus/test_base.pytests/integration/contracts/analytics/test_log_indexer.pybackend/protocol_rpc/app_lifespan.pytests/integration/icontracts/tests/test_web_mocking_contract.pybackend/validators/__init__.pytests/unit/test_validators_init.pytests/integration/fixtures/conftest.pybackend/protocol_rpc/message_handler/redis_worker_handler.pytests/integration/conftest.pybackend/protocol_rpc/redis_subscriber.pytests/integration/contracts/storage/test_user_storage.pybackend/database_handler/validators_registry.pytests/unit/test_simulator_sessions.pybackend/protocol_rpc/validators_init.pytests/integration/fixtures/schemas/call_contract_function.pytests/integration/contracts/conftest.pybackend/protocol_rpc/rpc_methods.pytests/integration/contracts/errors/test_error_web.pybackend/consensus/worker_service.pybackend/protocol_rpc/message_handler/worker_handler.pybackend/database_handler/llm_providers.pytests/integration/icontracts/tests/test_web_mocking.pybackend/database_handler/migration/versions/96840ab9133a_add_blocked_at_and_worker_id_to_.pybackend/protocol_rpc/endpoints.pytests/integration/contracts/errors/test_error_llm.pytests/unit/consensus/test_helpers.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Backend tests should be written and run with pytest (unit, integration, e2e)
Files:
tests/integration/contracts/storage/test_storage.pytests/integration/icontracts/conftest.pytests/unit/consensus/test_base.pytests/integration/contracts/analytics/test_log_indexer.pytests/integration/icontracts/tests/test_web_mocking_contract.pytests/unit/test_validators_init.pytests/integration/fixtures/conftest.pytests/integration/conftest.pytests/integration/contracts/storage/test_user_storage.pytests/unit/test_simulator_sessions.pytests/integration/fixtures/schemas/call_contract_function.pytests/integration/contracts/conftest.pytests/integration/contracts/errors/test_error_web.pytests/integration/icontracts/tests/test_web_mocking.pytests/integration/contracts/errors/test_error_llm.pytests/unit/consensus/test_helpers.py
tests/{unit,integration,e2e,load}/**
📄 CodeRabbit inference engine (AGENTS.md)
Organize Pytest suites by scope under tests/{unit,integration,e2e,load}
Files:
tests/integration/contracts/storage/test_storage.pytests/integration/icontracts/conftest.pytests/unit/consensus/test_base.pytests/integration/contracts/analytics/test_log_indexer.pytests/integration/icontracts/tests/test_web_mocking_contract.pytests/unit/test_validators_init.pytests/integration/fixtures/conftest.pytests/integration/conftest.pytests/integration/contracts/storage/test_user_storage.pytests/unit/test_simulator_sessions.pytests/integration/fixtures/schemas/call_contract_function.pytests/integration/contracts/conftest.pytests/integration/contracts/errors/test_error_web.pytests/integration/icontracts/tests/test_web_mocking.pytests/integration/contracts/errors/test_error_llm.pytests/unit/consensus/test_helpers.py
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Target Python 3.12, use 4-space indentation, and rely on Black (via pre-commit) for formatting
Files:
tests/integration/contracts/storage/test_storage.pytests/integration/icontracts/conftest.pytests/unit/consensus/test_base.pytests/integration/contracts/analytics/test_log_indexer.pybackend/protocol_rpc/app_lifespan.pytests/integration/icontracts/tests/test_web_mocking_contract.pybackend/validators/__init__.pytests/unit/test_validators_init.pytests/integration/fixtures/conftest.pybackend/protocol_rpc/message_handler/redis_worker_handler.pytests/integration/conftest.pybackend/protocol_rpc/redis_subscriber.pytests/integration/contracts/storage/test_user_storage.pybackend/database_handler/validators_registry.pytests/unit/test_simulator_sessions.pybackend/protocol_rpc/validators_init.pytests/integration/fixtures/schemas/call_contract_function.pytests/integration/contracts/conftest.pybackend/protocol_rpc/rpc_methods.pytests/integration/contracts/errors/test_error_web.pybackend/consensus/worker_service.pybackend/protocol_rpc/message_handler/worker_handler.pybackend/database_handler/llm_providers.pytests/integration/icontracts/tests/test_web_mocking.pybackend/database_handler/migration/versions/96840ab9133a_add_blocked_at_and_worker_id_to_.pybackend/protocol_rpc/endpoints.pytests/integration/contracts/errors/test_error_llm.pytests/unit/consensus/test_helpers.py
backend/protocol_rpc/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Implement JSON-RPC endpoint handlers under backend/protocol_rpc/
Files:
backend/protocol_rpc/app_lifespan.pybackend/protocol_rpc/message_handler/redis_worker_handler.pybackend/protocol_rpc/redis_subscriber.pybackend/protocol_rpc/validators_init.pybackend/protocol_rpc/rpc_methods.pybackend/protocol_rpc/message_handler/worker_handler.pybackend/protocol_rpc/endpoints.py
backend/validators/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
LLM provider integrations should live under backend/validators/
Files:
backend/validators/__init__.py
tests/{unit,integration,e2e,load}/test_*.py
📄 CodeRabbit inference engine (AGENTS.md)
Name backend tests as test_.py in the nearest scope folder
Files:
tests/unit/test_validators_init.pytests/unit/test_simulator_sessions.py
backend/consensus/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Consensus logic must reside under backend/consensus/
Files:
backend/consensus/worker_service.py
.env.example
📄 CodeRabbit inference engine (CLAUDE.md)
Define and document required environment variables in .env.example (POSTGRES_, PROVIDERS_, ENABLE_DEBUG_ENDPOINTS, DEFAULT_VALIDATORS_COUNT)
Files:
.env.example
🧬 Code graph analysis (15)
backend/protocol_rpc/app_lifespan.py (4)
backend/validators/__init__.py (2)
Manager(142-375)restart(167-182)backend/protocol_rpc/validators_init.py (1)
initialize_validators(18-92)backend/consensus/worker_service.py (1)
handle_validator_change(105-108)backend/protocol_rpc/redis_subscriber.py (1)
register_handler(239-248)
tests/integration/icontracts/tests/test_web_mocking_contract.py (2)
tests/integration/fixtures/conftest.py (2)
mock_web_requests(28-31)mock_llms(20-25)tests/integration/conftest.py (1)
setup_validators(18-81)
backend/validators/__init__.py (5)
backend/database_handler/validators_registry.py (2)
delete_validator(92-96)delete_all_validators(98-100)backend/protocol_rpc/endpoints.py (2)
delete_validator(333-342)delete_all_validators(346-350)backend/protocol_rpc/rpc_methods.py (2)
delete_validator(183-190)delete_all_validators(194-197)backend/protocol_rpc/broadcast.py (1)
publish(79-86)backend/protocol_rpc/message_handler/redis_worker_handler.py (1)
close(234-239)
tests/unit/test_validators_init.py (6)
backend/protocol_rpc/validators_init.py (1)
initialize_validators(18-92)backend/validators/__init__.py (2)
delete_all_validators(121-126)create_validator(95-100)backend/database_handler/validators_registry.py (2)
delete_all_validators(98-100)create_validator(70-74)backend/protocol_rpc/endpoints.py (2)
delete_all_validators(346-350)create_validator(193-229)backend/database_handler/accounts_manager.py (1)
create_new_account(24-31)backend/domain/types.py (1)
LLMProvider(81-98)
backend/protocol_rpc/message_handler/redis_worker_handler.py (2)
backend/protocol_rpc/websocket.py (1)
unsubscribe(45-57)backend/protocol_rpc/message_handler/worker_handler.py (1)
close(147-151)
tests/integration/conftest.py (3)
tests/common/request.py (2)
payload(21-27)post_request_localhost(43-44)tests/common/response.py (1)
has_success_status(23-24)tests/integration/fixtures/conftest.py (3)
mock_llms(20-25)mock_web_requests(28-31)load_mock_web_responses(34-53)
backend/database_handler/validators_registry.py (4)
tests/db-sqlalchemy/conftest.py (1)
session(73-81)backend/database_handler/models.py (1)
Validators(148-166)backend/protocol_rpc/endpoints.py (1)
get_all_validators(353-354)backend/protocol_rpc/rpc_methods.py (1)
get_all_validators(201-204)
tests/unit/test_simulator_sessions.py (4)
backend/validators/__init__.py (2)
delete_validator(112-119)delete_all_validators(121-126)backend/database_handler/validators_registry.py (3)
delete_validator(92-96)delete_all_validators(98-100)get_all_validators(51-57)backend/protocol_rpc/endpoints.py (3)
delete_validator(333-342)delete_all_validators(346-350)get_all_validators(353-354)backend/protocol_rpc/rpc_methods.py (3)
delete_validator(183-190)delete_all_validators(194-197)get_all_validators(201-204)
backend/protocol_rpc/validators_init.py (6)
backend/validators/__init__.py (3)
Manager(142-375)delete_all_validators(121-126)create_validator(95-100)backend/database_handler/validators_registry.py (2)
delete_all_validators(98-100)create_validator(70-74)backend/protocol_rpc/endpoints.py (2)
delete_all_validators(346-350)create_validator(193-229)backend/database_handler/accounts_manager.py (2)
AccountsManager(12-82)create_new_account(24-31)backend/domain/types.py (2)
Validator(102-144)LLMProvider(81-98)backend/node/create_nodes/providers.py (1)
get_default_provider_for(99-110)
backend/protocol_rpc/rpc_methods.py (4)
backend/protocol_rpc/dependencies.py (1)
get_validators_manager(121-126)backend/validators/__init__.py (4)
create_validator(95-100)update_validator(102-110)delete_validator(112-119)delete_all_validators(121-126)backend/database_handler/validators_registry.py (4)
create_validator(70-74)update_validator(76-90)delete_validator(92-96)delete_all_validators(98-100)backend/protocol_rpc/endpoints.py (4)
create_validator(193-229)update_validator(292-329)delete_validator(333-342)delete_all_validators(346-350)
backend/consensus/worker_service.py (4)
backend/protocol_rpc/app_lifespan.py (1)
handle_validator_change(321-324)backend/validators/__init__.py (1)
restart(167-182)backend/validators/llm.py (1)
restart(167-169)backend/protocol_rpc/message_handler/redis_worker_handler.py (1)
subscribe_to_validator_events(74-117)
backend/protocol_rpc/message_handler/worker_handler.py (5)
backend/protocol_rpc/configuration.py (1)
GlobalConfiguration(5-8)backend/protocol_rpc/message_handler/types.py (2)
LogEvent(21-39)to_dict(30-39)backend/protocol_rpc/message_handler/fastapi_handler.py (3)
error(133-142)_socket_emit(87-95)send_message(166-172)backend/protocol_rpc/message_handler/redis_worker_handler.py (3)
_socket_emit(172-193)send_message(195-211)close(234-239)backend/protocol_rpc/message_handler/base.py (1)
_log_message(63-92)
backend/database_handler/llm_providers.py (1)
tests/db-sqlalchemy/conftest.py (1)
session(73-81)
tests/integration/icontracts/tests/test_web_mocking.py (4)
tests/integration/fixtures/conftest.py (1)
mock_web_requests(28-31)tests/common/request.py (2)
payload(21-27)post_request_localhost(43-44)tests/common/response.py (1)
has_success_status(23-24)tests/integration/conftest.py (1)
setup_validators(18-81)
backend/protocol_rpc/endpoints.py (3)
backend/validators/__init__.py (5)
Manager(142-375)create_validator(95-100)update_validator(102-110)delete_validator(112-119)delete_all_validators(121-126)backend/database_handler/validators_registry.py (5)
create_validator(70-74)update_validator(76-90)delete_validator(92-96)delete_all_validators(98-100)get_all_validators(51-57)backend/protocol_rpc/rpc_methods.py (5)
create_validator(103-122)update_validator(160-179)delete_validator(183-190)delete_all_validators(194-197)get_all_validators(201-204)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 167-167: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
🪛 Ruff (0.14.2)
tests/integration/icontracts/conftest.py
14-14: from conftest import * used; unable to detect undefined names
(F403)
backend/protocol_rpc/app_lifespan.py
321-321: Unused function argument: event_data
(ARG001)
323-323: f-string without any placeholders
Remove extraneous f prefix
(F541)
tests/integration/icontracts/tests/test_web_mocking_contract.py
82-82: Local variable is_mocking_llms is assigned to but never used
Remove assignment to unused variable is_mocking_llms
(F841)
84-84: f-string without any placeholders
Remove extraneous f prefix
(F541)
backend/validators/__init__.py
374-374: Do not catch blind exception: Exception
(BLE001)
375-375: Use logging.exception instead of logging.error
Replace with exception
(TRY400)
backend/protocol_rpc/message_handler/redis_worker_handler.py
107-107: Do not catch blind exception: Exception
(BLE001)
117-117: Store a reference to the return value of asyncio.create_task
(RUF006)
tests/integration/conftest.py
84-84: Unused function argument: config
(ARG001)
backend/protocol_rpc/redis_subscriber.py
23-28: Mutable class attributes should be annotated with typing.ClassVar
(RUF012)
tests/unit/test_simulator_sessions.py
269-269: Unused function argument: provider
(ARG001)
269-269: Unused function argument: model
(ARG001)
345-345: Unused function argument: monkeypatch
(ARG001)
tests/integration/contracts/conftest.py
14-14: from conftest import * used; unable to detect undefined names
(F403)
backend/protocol_rpc/rpc_methods.py
111-111: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
168-168: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
185-185: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
195-195: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
backend/protocol_rpc/message_handler/worker_handler.py
103-103: Do not catch blind exception: Exception
(BLE001)
tests/integration/icontracts/tests/test_web_mocking.py
17-17: Avoid equality comparisons to True; use mock_web_requests(): for truth checks
Replace with mock_web_requests()
(E712)
26-26: Avoid equality comparisons to False; use not mock_web_requests(): for false checks
Replace with not mock_web_requests()
(E712)
🔇 Additional comments (28)
docker-compose.yml (2)
62-62: Verify image sourcing and versioning strategy forjsonrpcservice.The service now specifies an explicit image with
:latesttag while retaining a build configuration. This creates ambiguity:
- Image availability: Is
genlayer-studio-jsonrpc:latestpre-built and available (in a registry, CI cache, or locally)? If images aren't available, compose will fall back to building, but this is implicit and unclear.- Non-deterministic tagging: Using
:latestviolates reproducibility best practices and can cause inconsistent deployments if the image is rebuilt.- Dual configuration: Having both
imageandbuildspecified is ambiguous—clarify whether this is transitional config or intentional for flexibility.To clarify the intent, please verify:
- Are these images built and pushed to a registry by the CI workflow as part of the mocking system PR changes?
- Should local development use the pre-built images or build locally?
- Is using
:latestintentional, or should specific version tags be used instead?If these images are expected to be pre-built for CI environments, consider:
- Using specific version tags or digests instead of
:latest.- Clarifying in comments whether this compose file is for local dev (use
build:) or CI (use pre-builtimage:).- Providing a separate compose override or
.envconfiguration to distinguish the two use cases.
203-203: Apply same image sourcing and versioning clarification todatabase-migrationandconsensus-workerservices.Both services at lines 203 and 219 have the same dual
image:/build:configuration with:latesttagging. The same concerns and recommendations from the jsonrpc review apply here.For
consensus-workerspecifically (a replicated service), ensure that all replicas pull the same image version to maintain consistency—using:latestcould result in different replicas running different image versions if the tag is updated during deployment.Also applies to: 219-219
backend/database_handler/migration/versions/96840ab9133a_add_blocked_at_and_worker_id_to_.py (3)
8-19: LGTM! Formatting changes aligned with Black.The revision metadata has been reformatted to use double quotes, which is consistent with Black's formatting preferences.
34-38: LGTM! Downgrade logic correctly mirrors upgrade.The downgrade function properly drops the columns added in the upgrade, and the single-line formatting is appropriate for these concise statements.
1-38: Verify whether this migration belongs in this PR.This database migration adds
blocked_atandworker_idcolumns to the transactions table, which appears unrelated to the PR's stated objective of introducing a web request mocking system for integration tests. While the formatting changes themselves are fine, please confirm whether this migration file should be included in this PR or moved to a separate one focused on transaction handling improvements.tests/unit/consensus/test_helpers.py (2)
29-31: LGTM: Consistent with unified mocking approach.The change from
TEST_WITH_MOCK_LLMStoTEST_WITH_MOCKScorrectly aligns with the PR's objective to unify LLM and web request mocking under a single environment variable.
874-880: LGTM: Explicit type annotation improves clarity.Making the
timeoutparameter explicitly typed asint | Noneis good practice and aligns with the coding guidelines requiring type hints. The implementation correctly handlesNoneby applying adaptive timeout values based on the mocking mode.As per coding guidelines.
tests/integration/contracts/errors/test_error_llm.py (1)
5-9: Import path update verified and correctThe new import path
tests.integration.contracts.errors.test_error_executionis correct—the file exists and all three functions are properly defined there. No stale references to the old path remain in the codebase.backend/protocol_rpc/message_handler/worker_handler.py (1)
26-31: Looking at the web search results, the Optional type from the typing module should be used for parameters that might accept None, and whenever you have a keyword argument with default value None, you should use Optional. Additionally, the old implicit optional behavior is no longer recommended; type checkers should move towards requiring the optional type to be made explicit.Now let me examine the specific file to verify the context and check imports:
Add Optional type hint for config parameter defaulting to None
Whenever you have a keyword argument with default value None, you should use Optional. The old implicit optional behavior is no longer recommended; type checkers should move towards requiring the optional type to be made explicit. Apply the suggested fix:
def __init__( self, - config: GlobalConfiguration = None, + config: Optional[GlobalConfiguration] = None, jsonrpc_url: Optional[str] = None, worker_id: Optional[str] = None, ):Ensure
Optionalis imported fromtypingat the top of the file.tests/integration/contracts/analytics/test_log_indexer.py (1)
5-7: LGTM! Fixture import path consolidation.The import path has been correctly updated to reference the centralized fixtures location, aligning with the PR's goal of unifying test infrastructure.
tests/integration/contracts/storage/test_user_storage.py (1)
5-7: LGTM! Consistent fixture import path update.The import path has been updated to match the centralized fixtures location.
tests/integration/contracts/storage/test_storage.py (1)
5-7: LGTM! Fixture import path aligned with new structure.The import path has been correctly updated to use the centralized fixtures location.
backend/consensus/worker_service.py (1)
104-111: Validator change handler looks good, but consider restart frequency.The handler correctly reloads validators when notified of changes. The error handling is delegated to the subscription layer (in
subscribe_to_validator_events, line 107), which catches exceptions during callback execution.However, consider whether frequent validator changes could cause restart thrashing, as
validators_manager.restart()acquires a writer lock and restarts both LLM and web modules. If this becomes an issue, you might want to implement debouncing or rate limiting.tests/integration/contracts/errors/test_error_web.py (1)
4-8: LGTM! Import path updated for error execution utilities.The import path has been correctly updated to reference the consolidated error execution module.
backend/protocol_rpc/message_handler/redis_worker_handler.py (1)
104-108: Acceptable exception handling in message processing loop.Catching
Exceptionhere is appropriate since this is a continuous message processing loop that should remain resilient to individual message processing failures. The error is logged at line 108, which provides visibility.backend/validators/web.lua (1)
97-111: LGTM on request/response mock shape and logging.Mocked status/body/headers handling and debug logs are clear and compatible with callers.
backend/validators/__init__.py (1)
234-241: Good: mock_web_responses plumbed into host_data.This cleanly mirrors existing
mock_responsehandling and enables web.lua to switch behavior under TEST_WITH_MOCKS.tests/integration/icontracts/tests/test_web_mocking_contract.py (1)
21-72: Scenario coverage looks good.Three URL cases (404, example.com, nonexistent domain) exercise both render and request paths against mocks; assertions are appropriate.
tests/unit/test_validators_init.py (2)
1-42: LGTM! Clean migration to validators_manager pattern.The tests correctly mock the new
validators_managerstructure usingSimpleNamespace, and all assertions properly verify the registry interactions.
44-174: Excellent test coverage with proper error handling.The tests thoroughly validate the new initialization flow, including:
- Correct delegation to
validators_manager.registrymethods- Proper account creation for each validator
- Exception propagation for invalid configurations
tests/unit/test_simulator_sessions.py (3)
170-214: LGTM! Consistent migration to validators_manager pattern.The test correctly validates that
create_validatoruses the providedvalidators_managerfor registry operations.
216-292: LGTM! Appropriate mocking for unit test isolation.The test correctly mocks
check_provider_is_availableand uses thevalidators_managerpattern. The unused parameters in the mock function (line 269-270) are intentional—the signature must match the real function, but the stub implementation doesn't need the values.
294-342: LGTM! Consistent validator management updates.Both tests correctly verify that the endpoints delegate to
validators_manager.registryfor update and delete operations.backend/protocol_rpc/rpc_methods.py (1)
102-198: LGTM! Correct FastAPI dependency injection usage.The RPC methods correctly accept
validators_managervia FastAPI'sDepends()and forward it to the implementation layer. The static analysis warnings aboutDepends()in argument defaults are false positives—this is the standard FastAPI pattern for dependency injection.backend/protocol_rpc/endpoints.py (4)
193-230: LGTM! Clean migration to centralized validator management.The function correctly delegates validator creation to
validators_manager.registrywhile maintaining session-scoped account creation. This aligns with the broader architectural shift to a centralizedvalidators.Manager.
232-289: LGTM! Consistent validator creation pattern.Both random validator creation functions correctly use
validators_manager.registryfor all validator lifecycle operations.
291-330: LGTM! Update operation migrated correctly.The function properly delegates validator updates to
validators_manager.registry.update_validatorwhile maintaining the existing provider validation logic.
332-351: LGTM! Simplified delete operations.Both delete functions correctly use
validators_manager.registryand no longer require a session parameter, resulting in a cleaner API surface. Thedelete_all_validatorscorrectly returns the (empty) list of remaining validators.
| try: | ||
| self.session.commit() | ||
| except IntegrityError: | ||
| # Another worker already seeded the providers, rollback and continue | ||
| self.session.rollback() |
There was a problem hiding this comment.
Add logging when IntegrityError occurs.
The silent handling of IntegrityError provides no observability when concurrent seeding conflicts occur. While the rollback strategy is sound for concurrent scenarios, the assumption that all IntegrityError exceptions stem from concurrent seeding may not hold—constraint violations or data integrity issues could also trigger this exception and would be silently masked.
Apply this diff to add logging:
try:
self.session.commit()
except IntegrityError:
# Another worker already seeded the providers, rollback and continue
+ import logging
+ logging.info("IntegrityError during reset_defaults, likely due to concurrent seeding. Rolling back.")
self.session.rollback()Alternatively, use a module-level logger if one exists.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In backend/database_handler/llm_providers.py around lines 22 to 26, the
IntegrityError is currently swallowed after rollback; modify the except block to
log the exception (including stack/exception details) with a clear message that
a concurrent seeding conflict may have occurred but other integrity issues are
possible, then rollback as before; prefer using the module-level logger if one
exists (logger.exception or logger.warning with exc_info=True) so the error and
context are recorded for debugging.
| try: | ||
| self.session.commit() | ||
| except IntegrityError: | ||
| # Another worker already seeded the providers, rollback and continue | ||
| self.session.rollback() |
There was a problem hiding this comment.
Add logging when IntegrityError occurs.
Similar to reset_defaults, this method silently handles IntegrityError without logging. Given that update_defaults performs more complex operations (deleting old defaults and adding/updating new ones), observability is especially important for diagnosing concurrent conflicts or unexpected constraint violations.
Apply this diff to add logging:
try:
self.session.commit()
except IntegrityError:
# Another worker already seeded the providers, rollback and continue
+ import logging
+ logging.info("IntegrityError during update_defaults, likely due to concurrent seeding. Rolling back.")
self.session.rollback()Alternatively, use a module-level logger if one exists.
📝 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.
| try: | |
| self.session.commit() | |
| except IntegrityError: | |
| # Another worker already seeded the providers, rollback and continue | |
| self.session.rollback() | |
| try: | |
| self.session.commit() | |
| except IntegrityError: | |
| # Another worker already seeded the providers, rollback and continue | |
| logger.info( | |
| "IntegrityError during update_defaults, likely due to concurrent seeding. " | |
| "Rolling back." | |
| ) | |
| self.session.rollback() |
🤖 Prompt for AI Agents
In backend/database_handler/llm_providers.py around lines 70 to 74, the except
IntegrityError block silently swallows failures; update it to log the exception
and context before rolling back: catch the exception as e, call the module-level
logger (or create one via logging.getLogger(__name__)) to emit an error-level
message that includes a clear description (e.g., "IntegrityError while
committing update_defaults") and the exception details (e), then perform
self.session.rollback(); keep behavior otherwise unchanged.
| async def subscribe_to_validator_events(self, callback): | ||
| """ | ||
| Subscribe to validator change events. | ||
| Used by consensus-worker to reload validators when they change. | ||
| Args: | ||
| callback: Async function to call when validator event received | ||
| """ | ||
| if not self.redis_client: | ||
| await self.initialize() | ||
|
|
||
| if not self.redis_client: | ||
| logger.error( | ||
| "Cannot subscribe to validator events: Redis client not initialized" | ||
| ) | ||
| return | ||
|
|
||
| redis_client = self.redis_client | ||
|
|
||
| async def listener(): | ||
| """Listen for validator events and call callback.""" | ||
| redis_pubsub = redis_client.pubsub() | ||
| await redis_pubsub.subscribe(self.VALIDATOR_CHANNEL) | ||
| logger.info( | ||
| f"Worker {self.worker_id} subscribed to {self.VALIDATOR_CHANNEL}" | ||
| ) | ||
|
|
||
| try: | ||
| async for message in redis_pubsub.listen(): | ||
| if message["type"] == "message": | ||
| try: | ||
| event_data = json.loads(message["data"]) | ||
| await callback(event_data) | ||
| except Exception as e: | ||
| logger.error(f"Error processing validator event: {e}") | ||
| except asyncio.CancelledError: | ||
| logger.info( | ||
| f"Worker {self.worker_id} validator event listener cancelled" | ||
| ) | ||
| await redis_pubsub.unsubscribe(self.VALIDATOR_CHANNEL) | ||
| await redis_pubsub.close() | ||
|
|
||
| # Start listener in background | ||
| asyncio.create_task(listener()) |
There was a problem hiding this comment.
Store and manage the background listener task reference.
The asyncio.create_task(listener()) at line 117 creates a background task without storing its reference. This prevents proper cleanup when the handler is closed, potentially leaving the listener task running after the Redis client is closed (line 237 in close()).
Apply this diff to track and clean up the listener task:
class RedisWorkerMessageHandler(MessageHandler):
# Redis channels for different event types
CONSENSUS_CHANNEL = "consensus:events"
TRANSACTION_CHANNEL = "transaction:events"
GENERAL_CHANNEL = "general:events"
VALIDATOR_CHANNEL = "validator:events"
def __init__(
self,
config: GlobalConfiguration = None,
worker_id: Optional[str] = None,
redis_url: Optional[str] = None,
):
# ... existing init code ...
+ self._listener_tasks: list[asyncio.Task] = []Then update the method to store the task:
- # Start listener in background
- asyncio.create_task(listener())
+ # Start listener in background and track it
+ task = asyncio.create_task(listener())
+ self._listener_tasks.append(task)And update the close() method to clean up tasks:
async def close(self):
"""Clean up resources."""
+ # Cancel all listener tasks
+ for task in self._listener_tasks:
+ if not task.done():
+ task.cancel()
+
+ # Wait for all tasks to complete
+ if self._listener_tasks:
+ await asyncio.gather(*self._listener_tasks, return_exceptions=True)
+ self._listener_tasks.clear()
+
if self.redis_client:
await self.redis_client.close()
self.redis_client = None
logger.info(f"Worker {self.worker_id} disconnected from Redis")Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.14.2)
107-107: Do not catch blind exception: Exception
(BLE001)
117-117: Store a reference to the return value of asyncio.create_task
(RUF006)
| except asyncio.TimeoutError: | ||
| logger.warning(f"Timeout sending event to JSON-RPC server: {log_event.name}") | ||
| logger.warning( | ||
| f"Timeout sending event to JSON-RPC server: {log_event.name}" | ||
| ) | ||
| except Exception as e: | ||
| logger.error(f"Error sending event to JSON-RPC server: {e}") | ||
|
|
There was a problem hiding this comment.
Avoid broad Exception catch; handle aiohttp errors explicitly
Catching Exception trips Ruff BLE001 and can hide real bugs. Narrow the excepts.
Apply:
- except asyncio.TimeoutError:
+ except asyncio.TimeoutError:
logger.warning(
f"Timeout sending event to JSON-RPC server: {log_event.name}"
)
- except Exception as e:
- logger.error(f"Error sending event to JSON-RPC server: {e}")
+ except aiohttp.ClientError as e:
+ logger.warning(f"HTTP client error sending event: {e}")Optionally let unexpected exceptions bubble (preferred), or log and re‑raise:
+ except Exception as e:
+ logger.exception("Unexpected error sending event")
+ raise🧰 Tools
🪛 Ruff (0.14.2)
103-103: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
In backend/protocol_rpc/message_handler/worker_handler.py around lines 99 to
105, the current except Exception: is too broad and triggers Ruff BLE001 and can
hide real bugs; replace it with explicit handlers for expected network errors
(e.g., catch aiohttp.ClientError and log the error) and optionally handle
asyncio.CancelledError separately if cancellation semantics are needed, then
either re-raise unexpected exceptions or remove the broad catch so they bubble
up; ensure the log includes the error details and keep the asyncio.TimeoutError
handler as-is.
| -- Return mock response if it exists | ||
| if ctx.host_data.mock_response then | ||
| local result | ||
| local mock_data | ||
|
|
||
| if args.template == "EqComparative" then | ||
| -- Return the matching response to gl.eq_principle_prompt_comparative request which contains a principle key in the payload | ||
| result = get_mock_response_from_table(ctx.host_data.mock_response.eq_principle_prompt_comparative, mapped_prompt.prompt.user_message) | ||
| mock_data = get_mock_response_from_table(ctx.host_data.mock_response.eq_principle_prompt_comparative, mapped_prompt.prompt.user_message) | ||
| elseif args.template == "EqNonComparativeValidator" then | ||
| -- Return the matching response to gl.eq_principle_prompt_non_comparative request which contains an output key in the payload | ||
| result = get_mock_response_from_table(ctx.host_data.mock_response.eq_principle_prompt_non_comparative, mapped_prompt.prompt.user_message) | ||
| mock_data = get_mock_response_from_table(ctx.host_data.mock_response.eq_principle_prompt_non_comparative, mapped_prompt.prompt.user_message) | ||
| else | ||
| -- Return the matching response to gl.exec_prompt request which does not contain any specific key in the payload | ||
| -- EqNonComparativeLeader is essentially just exec_prompt | ||
| result = get_mock_response_from_table(ctx.host_data.mock_response.response, mapped_prompt.prompt.user_message) | ||
| mock_data = get_mock_response_from_table(ctx.host_data.mock_response.response, mapped_prompt.prompt.user_message) | ||
| end | ||
|
|
||
| -- Wrap mock response in the same format as exec_prompt_in_provider returns | ||
| -- Convert to JSON string if it's a table, otherwise use as-is | ||
| local data_value | ||
| if type(mock_data) == "table" then | ||
| data_value = lib.rs.json_stringify(mock_data) | ||
| else | ||
| data_value = mock_data | ||
| end | ||
|
|
||
| local result = { | ||
| data = data_value, | ||
| consumed_gen = 0 | ||
| } | ||
| lib.log{level = "debug", message = "executed with", type = type(result), res = result} | ||
| return result | ||
| end |
There was a problem hiding this comment.
Nil-safety on mock tables and ensure result.data is always a string
Accessing ctx.host_data.mock_response.* can be nil; pairs(nil) in get_mock_response_from_table will error. Also, result.data may become boolean/number, while downstream expects a string like exec_prompt_in_provider. Guard subtables and coerce non-strings.
Apply:
@@
- if args.template == "EqComparative" then
+ if args.template == "EqComparative" then
-- Return the matching response to gl.eq_principle_prompt_comparative request which contains a principle key in the payload
- mock_data = get_mock_response_from_table(ctx.host_data.mock_response.eq_principle_prompt_comparative, mapped_prompt.prompt.user_message)
+ local src = (ctx.host_data.mock_response and ctx.host_data.mock_response.eq_principle_prompt_comparative) or {}
+ mock_data = get_mock_response_from_table(src, mapped_prompt.prompt.user_message)
elseif args.template == "EqNonComparativeValidator" then
-- Return the matching response to gl.eq_principle_prompt_non_comparative request which contains an output key in the payload
- mock_data = get_mock_response_from_table(ctx.host_data.mock_response.eq_principle_prompt_non_comparative, mapped_prompt.prompt.user_message)
+ local src = (ctx.host_data.mock_response and ctx.host_data.mock_response.eq_principle_prompt_non_comparative) or {}
+ mock_data = get_mock_response_from_table(src, mapped_prompt.prompt.user_message)
else
-- Return the matching response to gl.exec_prompt request which does not contain any specific key in the payload
-- EqNonComparativeLeader is essentially just exec_prompt
- mock_data = get_mock_response_from_table(ctx.host_data.mock_response.response, mapped_prompt.prompt.user_message)
+ local src = (ctx.host_data.mock_response and ctx.host_data.mock_response.response) or {}
+ mock_data = get_mock_response_from_table(src, mapped_prompt.prompt.user_message)
end
@@
- local data_value
- if type(mock_data) == "table" then
- data_value = lib.rs.json_stringify(mock_data)
- else
- data_value = mock_data
- end
+ local t = type(mock_data)
+ local data_value
+ if t == "table" then
+ data_value = lib.rs.json_stringify(mock_data)
+ elseif t ~= "string" then
+ data_value = tostring(mock_data)
+ else
+ data_value = mock_data
+ end| "private_key": Optional[str], | ||
| "stake": int, | ||
| "primary_model": { | ||
| "config": dict, | ||
| "model": str, | ||
| "provider": str, | ||
| "plugin": str, | ||
| "plugin_config": dict, | ||
| "fallback_validator": str, | ||
| "fallback_validator": Optional[str], | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
Optional[str] for schema fields looks consistent; add variable annotation and verify consumers handle typing.Union
The Optional updates align with existing Optional[...] usage in this schema file. To meet the type‑hints guideline and avoid mypy noise, annotate the top-level schema dict and import Any. Also please confirm any schema consumers don’t pass these typing objects into isinstance directly.
Apply:
-from typing import Optional
+from typing import Optional, Any
@@
-call_contract_function_response = {
+call_contract_function_response: dict[str, Any] = {Run to spot fragile isinstance-usage on this schema:
🏁 Script executed:
#!/bin/bash
rg -nP --type=py -C2 '\bcall_contract_function_response\b'
rg -nP --type=py -C2 'isinstance\s*\(.*call_contract_function_response'Length of output: 4181
🏁 Script executed:
#!/bin/bash
# Find the assert_dict_struct function definition
rg -nP --type=py -B2 -A10 'def\s+assert_dict_struct'Length of output: 703
🏁 Script executed:
#!/bin/bash
# Check the actual file to see the full schema structure
cat tests/integration/fixtures/schemas/call_contract_function.pyLength of output: 1656
🏁 Script executed:
#!/bin/bash
# Get complete assert_dict_struct and assert_is_instance implementations
cat tests/common/response.pyLength of output: 727
Remove Optional[...] type hints from schema dict values—they cause runtime TypeError in assert_dict_struct
The schema contains Optional[str] and Optional[dict] as values (lines 19, 26-27). When assert_dict_struct() validates responses, it calls isinstance(data, structure) on leaf nodes. Typing constructs like Optional[str] are not valid arguments to isinstance()—this will raise TypeError: isinstance() arg 2 must be a type or tuple of types at runtime.
Replace Optional constructs with actual types in the schema. Either:
- Use primitive types (
str,dict,int, etc.) and handle None cases separately in validator logic, or - Omit optional fields from the schema structure entirely if they're truly optional
Apply to lines 19, 26, 27 in tests/integration/fixtures/schemas/call_contract_function.py:
- Line 19:
"private_key": Optional[str],→"private_key": str, - Line 26:
"fallback_validator": Optional[str],→"fallback_validator": str, - Line 27:
"secondary_model": Optional[dict],→"secondary_model": dict,
🤖 Prompt for AI Agents
In tests/integration/fixtures/schemas/call_contract_function.py around lines 17
to 26, the schema uses typing.Optional[...] (lines 19, 26, 27) which causes
isinstance() TypeError at runtime; replace those Optional[...] entries with
regular types or remove optional fields: change "private_key": Optional[str] to
"private_key": str, change "fallback_validator": Optional[str] to
"fallback_validator": str, and change "secondary_model": Optional[dict] to
"secondary_model": dict (or remove any of these keys from the schema if the
field should be omitted when absent).
| def pytest_configure(config): | ||
| load_dotenv(override=True) | ||
| # Import main configuration | ||
| from conftest import * |
There was a problem hiding this comment.
Do not import from conftest; rely on pytest’s fixture discovery.
from conftest import * breaks pytest’s intended loading order and triggers Ruff F403; it can cause duplicate fixture registration/import cycles. Remove and let pytest auto-discover parent conftest fixtures.
-# Import main configuration
-from conftest import *
+# Rely on pytest hierarchical fixture discovery; no explicit import needed.Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.14.2)
14-14: from conftest import * used; unable to detect undefined names
(F403)
🤖 Prompt for AI Agents
In tests/integration/icontracts/conftest.py around line 14, remove the line
"from conftest import *" because importing conftest directly breaks pytest
fixture discovery and triggers Ruff F403; instead rely on pytest's automatic
fixture discovery by deleting that import and ensuring any needed fixtures are
defined or imported via proper test modules or a shared conftest in a parent
package so fixtures are discovered without explicit imports.
Fixes https://linear.app/genlayer-labs/issue/DXP-692/configure-github-actions-and-workflows-part2
Depends on https://linear.app/genlayer-labs/issue/DXP-684/separate-and-regroup-integration-tests / #1331
Summary
This PR introduces a comprehensive mocking system for web requests in integration tests, extending the existing LLM mocking infrastructure to include HTTP requests. Both mocking systems are now controlled by a single environment variable
TEST_WITH_MOCKS.Changes
🔧 Core Implementation
web.lua: Intercepts web requests when mocking is enabled, returning predefined responses from JSON filesbackend/validators/__init__.pyto passmock_web_responsesto validatorsconftest.pyto load and inject mock web responses alongside LLM mocks📁 Mock Response Infrastructure
/tests/integration/fixtures/mock_responses/web/directory structureexample_responses.jsonwith mock responses for common test scenariosrender(webpage content) andrequest(API calls) mock types🎯 Unified Configuration
TEST_WITH_MOCKSnow controls both LLM and web request mocking🧪 Test Coverage
test_web_mocking.py: Validates the mocking infrastructuretest_web_mocking_contract.py: Tests contracts that make web requests with mocksBehavior
Regular PRs (not approved)
Approved PRs
Benefits
Testing
Run integration tests with mocking:
Run integration tests without mocking:
Files Modified
.env.example- AddedTEST_WITH_MOCKSvariablebackend/validators/web.lua- Web request mocking logicbackend/validators/__init__.py- Mock response passingtests/integration/icontracts/conftest.py- Mock loading functions.github/workflows/backend_integration_tests_pr.yml- Workflow configurationtests/unit/consensus/test_helpers.py- Updated to use new variable nameRelated Issues
Addresses the need for comprehensive mocking in CI/CD pipelines to reduce costs and improve test reliability.
Summary by CodeRabbit
New Features
Tests
Chores