-
Notifications
You must be signed in to change notification settings - Fork 9
Insufficient test coverage and CI coverage for production validator #81
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| name: Tests | ||
|
|
||
| on: | ||
| pull_request: | ||
| push: | ||
| branches: [main] | ||
|
|
||
| jobs: | ||
| test: | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: "3.12" | ||
|
|
||
| - name: Install uv | ||
| uses: astral-sh/setup-uv@v4 | ||
|
|
||
| - name: Install dependencies | ||
| run: uv sync --extra dev | ||
|
|
||
| - name: Lint | ||
| run: uv run ruff check src/crusades/core/ src/crusades/storage/ src/crusades/api/ tests/ | ||
|
|
||
| - name: Test with coverage | ||
| run: uv run pytest tests/ --cov=crusades.core --cov=crusades.storage --cov=crusades.api --cov-report=xml --cov-report=term -v | ||
|
|
||
| - name: Upload coverage | ||
| if: github.event_name == 'pull_request' | ||
| uses: orgoro/coverage@v3.2 | ||
| with: | ||
| coverageFile: coverage.xml | ||
| token: ${{ secrets.GITHUB_TOKEN }} | ||
| thresholdAll: 60 | ||
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,159 @@ | ||||||||||||
| #!/usr/bin/env python3 | ||||||||||||
| """Health monitor for crusades validator with Discord alerting. | ||||||||||||
|
|
||||||||||||
| Probes the validator's /health endpoint on a configurable interval. | ||||||||||||
| Posts to a Discord webhook when the validator is unreachable, returning | ||||||||||||
| non-200, or stale (zero evaluations in the staleness window). | ||||||||||||
|
|
||||||||||||
| Sends recovery notifications when the validator returns to healthy after | ||||||||||||
| a failure period. | ||||||||||||
|
|
||||||||||||
| Configuration via environment variables: | ||||||||||||
| VALIDATOR_URL - Base URL of the validator API (default: http://localhost:8080) | ||||||||||||
| DISCORD_WEBHOOK_URL - Discord webhook for alerts (optional; omit for log-only mode) | ||||||||||||
| CHECK_INTERVAL_SECONDS - Seconds between health checks (default: 60) | ||||||||||||
| STALE_THRESHOLD_SECONDS - Seconds of zero evaluations before alerting (default: 3600) | ||||||||||||
| ALERT_COOLDOWN_SECONDS - Minimum seconds between repeat alerts (default: 300) | ||||||||||||
| """ | ||||||||||||
|
|
||||||||||||
| import json | ||||||||||||
| import logging | ||||||||||||
| import os | ||||||||||||
| import sys | ||||||||||||
| import time | ||||||||||||
| from datetime import UTC, datetime | ||||||||||||
| from urllib.error import URLError | ||||||||||||
| from urllib.request import Request, urlopen | ||||||||||||
|
|
||||||||||||
| logging.basicConfig( | ||||||||||||
| level=logging.INFO, | ||||||||||||
| format="%(asctime)s | %(levelname)s | %(message)s", | ||||||||||||
| ) | ||||||||||||
| logger = logging.getLogger("health_monitor") | ||||||||||||
|
|
||||||||||||
| VALIDATOR_URL = os.getenv("VALIDATOR_URL", "http://localhost:8080").rstrip("/") | ||||||||||||
| DISCORD_WEBHOOK_URL = os.getenv("DISCORD_WEBHOOK_URL", "") | ||||||||||||
| CHECK_INTERVAL = int(os.getenv("CHECK_INTERVAL_SECONDS", "60")) | ||||||||||||
| STALE_THRESHOLD = int(os.getenv("STALE_THRESHOLD_SECONDS", "3600")) | ||||||||||||
| ALERT_COOLDOWN = int(os.getenv("ALERT_COOLDOWN_SECONDS", "300")) | ||||||||||||
|
Comment on lines
+37
to
+38
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The environment variable 🗑️ Suggested fix: Remove unused variable DISCORD_WEBHOOK_URL = os.getenv("DISCORD_WEBHOOK_URL", "")
CHECK_INTERVAL = int(os.getenv("CHECK_INTERVAL_SECONDS", "60"))
-STALE_THRESHOLD = int(os.getenv("STALE_THRESHOLD_SECONDS", "3600"))
ALERT_COOLDOWN = int(os.getenv("ALERT_COOLDOWN_SECONDS", "300"))Also remove from the docstring (line 15). 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| def probe_health() -> dict: | ||||||||||||
| """Probe the validator /health endpoint. | ||||||||||||
|
|
||||||||||||
| Returns: | ||||||||||||
| Parsed JSON response, or a dict with 'error' key on failure. | ||||||||||||
| """ | ||||||||||||
| url = f"{VALIDATOR_URL}/health" | ||||||||||||
| try: | ||||||||||||
| req = Request(url, method="GET") | ||||||||||||
| with urlopen(req, timeout=10) as resp: | ||||||||||||
| if resp.status != 200: | ||||||||||||
| return {"error": f"HTTP {resp.status}"} | ||||||||||||
| return json.loads(resp.read().decode()) | ||||||||||||
| except URLError as e: | ||||||||||||
| return {"error": f"Unreachable: {e.reason}"} | ||||||||||||
| except Exception as e: | ||||||||||||
| return {"error": str(e)} | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| def post_discord(content: str, color: int = 0xFF0000) -> None: | ||||||||||||
| """Post an embed to Discord webhook. Fails silently if no webhook configured.""" | ||||||||||||
| if not DISCORD_WEBHOOK_URL: | ||||||||||||
| return | ||||||||||||
|
|
||||||||||||
| payload = json.dumps({ | ||||||||||||
| "embeds": [{ | ||||||||||||
| "title": "Crusades Validator Health Alert", | ||||||||||||
| "description": content, | ||||||||||||
| "color": color, | ||||||||||||
| "timestamp": datetime.now(UTC).isoformat(), | ||||||||||||
| }] | ||||||||||||
| }).encode() | ||||||||||||
|
|
||||||||||||
| try: | ||||||||||||
| req = Request( | ||||||||||||
| DISCORD_WEBHOOK_URL, | ||||||||||||
| data=payload, | ||||||||||||
| headers={"Content-Type": "application/json"}, | ||||||||||||
| method="POST", | ||||||||||||
| ) | ||||||||||||
| with urlopen(req, timeout=10): | ||||||||||||
| pass | ||||||||||||
| except Exception as e: | ||||||||||||
| logger.error(f"Discord webhook failed: {e}") | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| def evaluate_health(data: dict) -> tuple[bool, str]: | ||||||||||||
| """Evaluate health response. | ||||||||||||
|
|
||||||||||||
| Returns: | ||||||||||||
| (is_healthy, reason) | ||||||||||||
| """ | ||||||||||||
| if "error" in data: | ||||||||||||
| return False, data["error"] | ||||||||||||
|
|
||||||||||||
| status = data.get("status", "unknown") | ||||||||||||
| if status == "unhealthy": | ||||||||||||
| return False, "Validator reports unhealthy (DB disconnected)" | ||||||||||||
|
|
||||||||||||
| if status == "degraded": | ||||||||||||
| return False, ( | ||||||||||||
| f"Validator is degraded: 0 evaluations in last hour, " | ||||||||||||
| f"queue_depth={data.get('queue_depth', '?')}" | ||||||||||||
| ) | ||||||||||||
|
|
||||||||||||
| return True, "healthy" | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| def run() -> None: | ||||||||||||
| """Main monitoring loop.""" | ||||||||||||
| logger.info(f"Monitoring {VALIDATOR_URL}/health every {CHECK_INTERVAL}s") | ||||||||||||
| if not DISCORD_WEBHOOK_URL: | ||||||||||||
| logger.warning("DISCORD_WEBHOOK_URL not set - running in log-only mode") | ||||||||||||
|
|
||||||||||||
| is_alerting = False | ||||||||||||
| last_alert_time = 0.0 | ||||||||||||
|
|
||||||||||||
| while True: | ||||||||||||
| data = probe_health() | ||||||||||||
| healthy, reason = evaluate_health(data) | ||||||||||||
|
|
||||||||||||
| if healthy: | ||||||||||||
| if is_alerting: | ||||||||||||
| # Recovery | ||||||||||||
| logger.info("Validator recovered") | ||||||||||||
| post_discord( | ||||||||||||
| f"Validator at `{VALIDATOR_URL}` has **recovered**.\n" | ||||||||||||
| f"Status: `{data.get('status', 'healthy')}`\n" | ||||||||||||
| f"Evaluations/1h: `{data.get('evaluations_1h', 'N/A')}`", | ||||||||||||
| color=0x00FF00, | ||||||||||||
| ) | ||||||||||||
| is_alerting = False | ||||||||||||
| else: | ||||||||||||
| logger.info(f"OK | evals_1h={data.get('evaluations_1h', '?')} " | ||||||||||||
| f"queue={data.get('queue_depth', '?')}") | ||||||||||||
| else: | ||||||||||||
| now = time.time() | ||||||||||||
| if not is_alerting or (now - last_alert_time) >= ALERT_COOLDOWN: | ||||||||||||
| logger.error(f"ALERT | {reason}") | ||||||||||||
| post_discord( | ||||||||||||
| f"Validator at `{VALIDATOR_URL}` is **down or degraded**.\n" | ||||||||||||
| f"Reason: `{reason}`\n" | ||||||||||||
| f"Checked at: `{datetime.now(UTC).isoformat()}`", | ||||||||||||
| color=0xFF0000, | ||||||||||||
| ) | ||||||||||||
| is_alerting = True | ||||||||||||
| last_alert_time = now | ||||||||||||
| else: | ||||||||||||
| logger.warning(f"Still failing: {reason} (cooldown active)") | ||||||||||||
|
|
||||||||||||
| time.sleep(CHECK_INTERVAL) | ||||||||||||
|
|
||||||||||||
|
|
||||||||||||
| if __name__ == "__main__": | ||||||||||||
| try: | ||||||||||||
| run() | ||||||||||||
| except KeyboardInterrupt: | ||||||||||||
| logger.info("Monitor stopped") | ||||||||||||
| sys.exit(0) | ||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -123,8 +123,33 @@ def create_app(api_key: str | None = None) -> FastAPI: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @app.get("/health") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| async def health_check(): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Health check endpoint.""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return {"status": "healthy"} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Health check endpoint with staleness detection. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Returns DB connectivity, last evaluation time, and queue depth | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| so external probes can detect a running-but-stale validator. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| client = get_db_client() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if client is None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return {"status": "healthy", "db_connected": False} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| validator_status = client.get_validator_status() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| queue = client.get_queue_stats() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| evaluations_1h = validator_status.get("evaluations_completed_1h", 0) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| status = "healthy" if evaluations_1h > 0 else "degraded" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "status": status, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "db_connected": True, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "evaluations_1h": evaluations_1h, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+132
to
+145
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
At Line 132, Suggested fix try:
client = get_db_client()
if client is None:
return {"status": "healthy", "db_connected": False}
+ db_connected = not isinstance(client, MockClient)
validator_status = client.get_validator_status()
queue = client.get_queue_stats()
evaluations_1h = validator_status.get("evaluations_completed_1h", 0)
status = "healthy" if evaluations_1h > 0 else "degraded"
return {
"status": status,
- "db_connected": True,
+ "db_connected": db_connected,
"evaluations_1h": evaluations_1h,
"queue_depth": queue.get("queued_count", 0),
"current_evaluation": validator_status.get("current_evaluation"),
"validator_status": validator_status.get("status", "unknown"),
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "queue_depth": queue.get("queued_count", 0), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "current_evaluation": validator_status.get("current_evaluation"), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "validator_status": validator_status.get("status", "unknown"), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| except Exception: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.exception("Health check failed") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return {"status": "unhealthy", "db_connected": False} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # ============================================================ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,124 @@ | ||
| """Shared test fixtures for crusades test suite.""" | ||
|
|
||
| import uuid | ||
| from datetime import UTC, datetime, timedelta | ||
|
|
||
| import pytest | ||
| import pytest_asyncio | ||
|
|
||
| from crusades.core.protocols import SubmissionStatus | ||
| from crusades.storage.database import Database | ||
| from crusades.storage.models import ( | ||
| EvaluationModel, | ||
| SubmissionModel, | ||
| ) | ||
|
|
||
|
|
||
| @pytest_asyncio.fixture | ||
| async def db(): | ||
| """Create an in-memory async Database instance with tables initialized.""" | ||
| database = Database(url="sqlite+aiosqlite:///:memory:") | ||
| await database.initialize() | ||
| yield database | ||
| await database.close() | ||
|
|
||
|
|
||
| @pytest_asyncio.fixture | ||
| async def seeded_db(db: Database): | ||
| """Database seeded with submissions and evaluations in various states.""" | ||
| async with db.session_factory() as session: | ||
| now = datetime.now(UTC) | ||
|
|
||
| # Finished submission with evaluations (high score) | ||
| sub1 = SubmissionModel( | ||
| submission_id="sub-finished-1", | ||
| miner_hotkey="hotkey_alice", | ||
| miner_uid=1, | ||
| code_hash="hash_abc", | ||
| bucket_path="https://example.com/train1.py", | ||
| spec_version=19, | ||
| status=SubmissionStatus.FINISHED, | ||
| final_score=65.5, | ||
| code_content="def inner_steps(): pass", | ||
| created_at=now - timedelta(hours=2), | ||
| ) | ||
| session.add(sub1) | ||
|
|
||
| eval1 = EvaluationModel( | ||
| evaluation_id=str(uuid.uuid4()), | ||
| submission_id="sub-finished-1", | ||
| evaluator_hotkey="validator_1", | ||
| mfu=65.5, | ||
| tokens_per_second=1200.0, | ||
| total_tokens=24000, | ||
| wall_time_seconds=20.0, | ||
| success=True, | ||
| created_at=now - timedelta(hours=2), | ||
| ) | ||
| session.add(eval1) | ||
|
|
||
| # Finished submission (lower score, older) | ||
| sub2 = SubmissionModel( | ||
| submission_id="sub-finished-2", | ||
| miner_hotkey="hotkey_bob", | ||
| miner_uid=2, | ||
| code_hash="hash_def", | ||
| bucket_path="https://example.com/train2.py", | ||
| spec_version=19, | ||
| status=SubmissionStatus.FINISHED, | ||
| final_score=55.0, | ||
| created_at=now - timedelta(hours=5), | ||
| ) | ||
| session.add(sub2) | ||
|
|
||
| eval2 = EvaluationModel( | ||
| evaluation_id=str(uuid.uuid4()), | ||
| submission_id="sub-finished-2", | ||
| evaluator_hotkey="validator_1", | ||
| mfu=55.0, | ||
| tokens_per_second=1000.0, | ||
| total_tokens=20000, | ||
| wall_time_seconds=20.0, | ||
| success=True, | ||
| created_at=now - timedelta(hours=5), | ||
| ) | ||
| session.add(eval2) | ||
|
|
||
| # Pending submission | ||
| sub3 = SubmissionModel( | ||
| submission_id="sub-pending-1", | ||
| miner_hotkey="hotkey_carol", | ||
| miner_uid=3, | ||
| code_hash="hash_ghi", | ||
| bucket_path="https://example.com/train3.py", | ||
| spec_version=19, | ||
| status=SubmissionStatus.PENDING, | ||
| created_at=now - timedelta(minutes=10), | ||
| ) | ||
| session.add(sub3) | ||
|
|
||
| # Failed submission | ||
| sub4 = SubmissionModel( | ||
| submission_id="sub-failed-1", | ||
| miner_hotkey="hotkey_dave", | ||
| miner_uid=4, | ||
| code_hash="hash_jkl", | ||
| bucket_path="https://example.com/train4.py", | ||
| spec_version=19, | ||
| status=SubmissionStatus.FAILED_VALIDATION, | ||
| error_message="SYNTAX_ERROR: invalid syntax", | ||
| created_at=now - timedelta(hours=1), | ||
| ) | ||
| session.add(sub4) | ||
|
|
||
| await session.commit() | ||
|
|
||
| return db | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def mock_db_client(): | ||
| """Return a MockClient for API tests that don't need real DB.""" | ||
| from crusades.tui.client import MockClient | ||
|
|
||
| return MockClient() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Coverage gate does not meet the linked objective.
Line 36 sets
thresholdAll: 60, but the linked issue objective specifies coverage target >80%. This leaves the requirement only partially implemented.Minimal fix
- name: Upload coverage if: github.event_name == 'pull_request' uses: orgoro/coverage@v3.2 with: coverageFile: coverage.xml token: ${{ secrets.GITHUB_TOKEN }} - thresholdAll: 60 + thresholdAll: 80📝 Committable suggestion
🤖 Prompt for AI Agents