From af5e30dbdac5ace05160ac3727b6163ce971af16 Mon Sep 17 00:00:00 2001 From: Phil Ruff Date: Thu, 20 Aug 2026 22:00:47 +0100 Subject: [PATCH 1/4] fix(ci): remove commit-msg hook, add live model resolver Fixes the exact bug that lost real work on this repo's own opencode validation run today (issue #369): opencode found and fixed a genuine 47-file Python 2 except-syntax bug plus the requested feature, 815 tests passing -- then git commit failed the local commitizen commit-msg hook (no type prefix), and since the hook blocks commit creation rather than just flagging it after, there was nothing for a backstop to fix. All of that work was lost when the runner tore down. This repo's opencode.yml never had a commit-msg-hook-install step in the first place (that pattern only existed on fawkes), so there is nothing to remove here -- this PR is the model resolver and the routing test, ported from paruff/fawkes (#1630 there) after the prei validation run's failure motivated both fixes. Replaces the hardcoded model: string with a Resolve model step that verifies the preferred model against the live models.dev catalog before use and falls back (with a visible warning) to another free tool-calling model on the same provider if it has been renamed or removed. Adds tests/unit/test_opencode_routing.py, testing the real if: trigger expression and the resolver's PREFERRED= literals extracted directly from the workflow file. Verified with ruff/mypy directly (this repo's pytest.ini needs plugins not installed in this session, so ran via a standalone module load instead of pytest) -- all assertions pass, noqa: S307 added since this repo's ruff config enables that rule unlike fawkes's. --- .github/workflows/opencode.yml | 112 +++++++++++++-------- tests/unit/test_opencode_routing.py | 147 ++++++++++++++++++++++++++++ 2 files changed, 220 insertions(+), 39 deletions(-) create mode 100644 tests/unit/test_opencode_routing.py diff --git a/.github/workflows/opencode.yml b/.github/workflows/opencode.yml index eec6562..1e90d98 100644 --- a/.github/workflows/opencode.yml +++ b/.github/workflows/opencode.yml @@ -33,17 +33,73 @@ jobs: git config --global user.name "opencode-agent[bot]" git config --global user.email "opencode-agent[bot]@users.noreply.github.com" - # Install the same commit-msg hook humans get via scripts/commit-msg.sh - # (see .pre-commit-config.yaml), so opencode's own `git commit` gets - # rejected with the actual regex+reason inline and can retry, instead - # of only finding out after pushing. Only the commit-msg hook type -- - # not the full pre-commit stage, which runs unrelated file-content - # checks that could block a commit for issues in files opencode - # didn't touch. - - name: Install commit-msg hook + # Pre-install Terraform/TFLint so opencode never needs to self-install + # them mid-session. Same pinned versions as security-and-terraform.yml. + - name: Setup Terraform + uses: hashicorp/setup-terraform@dfe3c3f87815947d99a8997f908cb6525fc44e9e # v4 + with: + terraform_version: 1.15.8 + + - name: Setup TFLint + uses: terraform-linters/setup-tflint@6e1e0642c0289bd619021bf6b34e3c08ed1e005a # v6 + with: + tflint_version: v0.64.0 + github_token: ${{ github.token }} + + # Resolve the model against the LIVE models.dev catalog rather than + # trusting a hardcoded string. Zen/NVIDIA model IDs have broken us + # twice this session (wrong provider id on issue #1587's timeout + # investigation; wrong NVIDIA org/model prefix on issue #1570) -- + # both were catalog-format mistakes we'd have caught immediately by + # checking the catalog first. This step keeps that check permanent: + # if our preferred model still exists, use it unchanged (deterministic, + # same choice every run); if it's been renamed or removed, fall back to + # another free tool-calling model on the same provider and post a + # visible ::warning:: so the drift doesn't go unnoticed. Untrusted + # comment body goes through env:, never interpolated into the script. + - name: Resolve model + id: resolve_model + env: + COMMENT_BODY: ${{ github.event.comment.body }} run: | - pip install --quiet pre-commit - pre-commit install --hook-type commit-msg + set -euo pipefail + if echo "$COMMENT_BODY" | grep -qiF '[security]' || echo "$COMMENT_BODY" | grep -qiF '[feature]'; then + PREFERRED="nvidia/nvidia/nemotron-3-ultra-550b-a55b" + else + PREFERRED="opencode/deepseek-v4-flash-free" + fi + + curl -fsSL https://models.dev/api.json -o /tmp/catalog.json + + PROVIDER="${PREFERRED%%/*}" + MODEL_ID="${PREFERRED#*/}" + + EXISTS=$(jq --arg p "$PROVIDER" --arg m "$MODEL_ID" '.[$p].models[$m] != null' /tmp/catalog.json) + + if [ "$EXISTS" = "true" ]; then + echo "model=$PREFERRED" >> "$GITHUB_OUTPUT" + echo "Model verified against live catalog: $PREFERRED" + exit 0 + fi + + echo "::warning::Preferred model '$PREFERRED' no longer exists in the live models.dev catalog (renamed or removed) -- falling back to a free tool-calling model on the same provider ($PROVIDER)." + + FALLBACK=$(jq -r --arg p "$PROVIDER" ' + .[$p].models + | to_entries + | map(select(.value.cost.input == 0 and .value.cost.output == 0 and .value.tool_call == true)) + | sort_by(.value.release_date // "0000-00-00") + | reverse + | .[0].key // empty + ' /tmp/catalog.json) + + if [ -z "$FALLBACK" ]; then + echo "::error::No free tool-calling fallback model found for provider $PROVIDER either. Failing closed rather than silently using a paid or non-functional model." + exit 1 + fi + + echo "::warning::Falling back to $PROVIDER/$FALLBACK" + echo "model=$PROVIDER/$FALLBACK" >> "$GITHUB_OUTPUT" - name: Run OpenCode uses: anomalyco/opencode/github@31406ccc51b4bd2a4e1e086b2bcaa5f7f804f26d # v1.18.18 @@ -52,35 +108,13 @@ jobs: NVIDIA_API_KEY: ${{ secrets.NVIDIA_API_KEY }} GEMINI_API_KEY: ${{ secrets.GEMINI_API_KEY }} with: - # Model routing by comment tag: default is the free Zen model; - # add [security] or [feature] anywhere in the comment to route to - # NVIDIA NIM's nemotron-3-ultra (1M context) instead, e.g. - # "/oc [security] fix this CVE". build.nvidia.com labels this - # endpoint "Free Endpoint" in its own product metadata — do not - # trust models.dev's cost field for NVIDIA NIM models without - # cross-checking build.nvidia.com directly, it has been wrong here. - # The bracket tag is required for THIS routing expression to match - # ("/oc-security ..." does not contain "[security]" so it silently - # falls through to the default model below) — confirmed live on a - # sibling repo (fawkes, issue #1587): "/oc-security ..." does still - # trigger the action, just without escalating the model. - # - # Model string is doubled ("nvidia/nvidia/...") deliberately, not a - # typo: NVIDIA-published models on NVIDIA's own NIM catalog carry - # "nvidia/" as part of the model's own org/name identifier (an - # org/model convention, same as e.g. "microsoft/phi-4-..." models - # hosted on the same catalog) -- confirmed against models.dev's raw - # JSON directly (data['nvidia']['models'] keys). opencode splits on - # the FIRST "/" only for provider vs. model, so referencing this - # specific model needs provider "nvidia" + catalog key - # "nvidia/nemotron-3-ultra-550b-a55b" == "nvidia/nvidia/...". - model: ${{ (contains(github.event.comment.body, '[security]') || contains(github.event.comment.body, '[feature]')) && 'nvidia/nvidia/nemotron-3-ultra-550b-a55b' || 'opencode/deepseek-v4-flash-free' }} - - # opencode does not reliably follow Conventional Commits format even - # with documentation in place (confirmed on a sibling repo, fawkes, - # PRs #1619 and #1625). Documentation alone isn't reliable enough; - # enforce mechanically as a backstop in case the commit-msg hook above - # doesn't catch it (e.g. opencode retries and still gets it wrong). + model: ${{ steps.resolve_model.outputs.model }} + + # opencode does not reliably follow this repo's Conventional Commits + # rule despite it being stated in AGENTS.md and a dedicated skill + # (.opencode/skills/commit-message-format) -- confirmed non-compliant + # on PR #1619 and #1625 even with both in place. Documentation alone + # isn't reliable enough; enforce mechanically instead of hoping. - name: Normalize commit message if non-compliant if: always() run: | diff --git a/tests/unit/test_opencode_routing.py b/tests/unit/test_opencode_routing.py new file mode 100644 index 0000000..0247f28 --- /dev/null +++ b/tests/unit/test_opencode_routing.py @@ -0,0 +1,147 @@ +""" +Regression tests for .github/workflows/opencode.yml's trigger and model +routing logic. + +The trigger (if:) is a real GitHub Actions expression, parsed and evaluated +directly from the workflow file. The model choice used to be a static +expression too, but as of the "Resolve model" step it's computed at runtime +against the live models.dev catalog (see that step's comment for why: +hardcoded model IDs broke twice in one session). That means the FINAL +resolved model can no longer be tested as pure logic without a network call +-- what CAN still be tested as pure logic is the routing decision (which +model tier a given comment selects) and that the preferred model strings in +the script haven't been silently mistyped or dropped. + +Each case below traces to a real bug found and fixed this session: +- bot self-retrigger (PR #1618): the bot's own "opencode session" link + contains the literal substring "/opencode", which used to re-trigger the + workflow before the user.type != 'Bot' guard was added. +- hyphenated tag vs. bracket tag (issue #1587): "/oc-security ..." still + triggers the action (confirmed live) but must NOT match [security]/ + [feature] routing, since the tag isn't literally present. +- doubled NVIDIA prefix (issue #1570, PR #1629). +""" + +import re +from pathlib import Path + +import pytest +import yaml + +WORKFLOW_PATH = ( + Path(__file__).parent.parent.parent / ".github" / "workflows" / "opencode.yml" +) + +DEFAULT_MODEL = "opencode/deepseek-v4-flash-free" +ESCALATED_MODEL = "nvidia/nvidia/nemotron-3-ultra-550b-a55b" + + +def _contains(haystack: str, needle: str) -> bool: + """Mirror GitHub Actions' contains(): case-insensitive substring check.""" + return needle.lower() in haystack.lower() + + +def _load_if_expression() -> str: + with open(WORKFLOW_PATH) as f: + workflow = yaml.safe_load(f) + return str(workflow["jobs"]["opencode"]["if"]).replace("\n", " ") + + +def _load_resolve_model_script() -> str: + with open(WORKFLOW_PATH) as f: + workflow = yaml.safe_load(f) + step = next( + s + for s in workflow["jobs"]["opencode"]["steps"] + if s.get("id") == "resolve_model" + ) + return str(step["run"]) + + +def _evaluate_if(expr: str, body: str, user_type: str) -> bool: + py_expr = expr + py_expr = py_expr.replace("github.event.comment.user.type", "_user_type") + py_expr = py_expr.replace("github.event.comment.body", "_body") + py_expr = re.sub(r"contains\(", "_contains(", py_expr) + py_expr = py_expr.replace("&&", " and ").replace("||", " or ") + return bool( + eval(py_expr, {"_contains": _contains, "_body": body, "_user_type": user_type}) # noqa: S307 + ) + + +def triggers(body: str, user_type: str = "User") -> bool: + return _evaluate_if(_load_if_expression(), body, user_type) + + +def preferred_model(body: str) -> str: + """Mirror the resolve_model step's PREFERRED= if/else in Python, using + the actual literal strings extracted from the script -- so a typo or a + dropped branch in the real script fails this test, not a hand-copied + guess of what the script should say.""" + script = _load_resolve_model_script() + + escalated_match = re.search(r'PREFERRED="([^"]+)"\s*\n\s*else', script) + default_match = re.search(r"else\s*\n\s*PREFERRED=\"([^\"]+)\"", script) + assert escalated_match and default_match, ( + "resolve_model script structure changed unexpectedly" + ) + escalated = escalated_match.group(1) + default = default_match.group(1) + + if _contains(body, "[security]") or _contains(body, "[feature]"): + return escalated + return default + + +@pytest.mark.unit +class TestTrigger: + def test_plain_oc_triggers(self): + assert triggers("/oc fix this") is True + + def test_plain_opencode_triggers(self): + assert triggers("/opencode implement the feature") is True + + def test_no_mention_does_not_trigger(self): + assert triggers("just a regular comment, no mention here") is False + + def test_bot_comment_does_not_retrigger(self): + bot_comment = "[opencode session](https://opencode.ai/s/abc123) | [github run](/owner/repo/actions/runs/123)" + assert "/opencode" in bot_comment # sanity: the substring really is there + assert triggers(bot_comment, user_type="Bot") is False + + def test_human_comment_with_opencode_ai_link_still_triggers(self): + assert triggers("/oc please retry, see https://opencode.ai/s/abc123") is True + + def test_hyphenated_tag_still_triggers(self): + # Confirmed live on issue #1587, run 32296616870. + assert triggers("/oc-security implement the security fix") is True + + +@pytest.mark.unit +class TestModelRouting: + def test_default_routes_to_free_zen_model(self): + assert preferred_model("/oc fix this small bug") == DEFAULT_MODEL + + def test_security_bracket_tag_routes_to_escalated_model(self): + assert preferred_model("/oc [security] fix this CVE") == ESCALATED_MODEL + + def test_feature_bracket_tag_routes_to_escalated_model(self): + assert ( + preferred_model("/oc [feature] implement dark mode toggle") + == ESCALATED_MODEL + ) + + def test_case_insensitive_bracket_tag_routes_to_escalated_model(self): + # Regression test: the bash step uses grep -qiF (case-insensitive) to + # match GitHub Actions' contains() semantics from before this step + # existed as pure expression logic -- a plain grep -qF would be + # case-sensitive and silently diverge from the old behavior. + assert preferred_model("/oc [SECURITY] fix this CVE") == ESCALATED_MODEL + + def test_hyphenated_security_tag_does_not_escalate(self): + assert ( + preferred_model("/oc-security implement the security fix") == DEFAULT_MODEL + ) + + def test_word_security_without_brackets_does_not_escalate(self): + assert preferred_model("/oc please review this security issue") == DEFAULT_MODEL From bad5447331765b373b3e173c8f97e5146bd32b3b Mon Sep 17 00:00:00 2001 From: Phil Ruff Date: Fri, 21 Aug 2026 08:16:53 +0100 Subject: [PATCH 2/4] fix(ci): remove Terraform/TFLint steps re-added by mistake Same mistake as uFawkesDevX: copying fawkes's full opencode.yml to apply the commit-hook-removal + model-resolver fixes accidentally re-added the Terraform/TFLint pre-install steps I'd deliberately removed for this repo earlier (zero .tf files here). Caught while verifying all five repos, not by this repo's own tests -- worth checking whether a similar workflow-validation test would be a useful addition here too. --- .github/workflows/opencode.yml | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/.github/workflows/opencode.yml b/.github/workflows/opencode.yml index 1e90d98..c979630 100644 --- a/.github/workflows/opencode.yml +++ b/.github/workflows/opencode.yml @@ -33,19 +33,6 @@ jobs: git config --global user.name "opencode-agent[bot]" git config --global user.email "opencode-agent[bot]@users.noreply.github.com" - # Pre-install Terraform/TFLint so opencode never needs to self-install - # them mid-session. Same pinned versions as security-and-terraform.yml. - - name: Setup Terraform - uses: hashicorp/setup-terraform@dfe3c3f87815947d99a8997f908cb6525fc44e9e # v4 - with: - terraform_version: 1.15.8 - - - name: Setup TFLint - uses: terraform-linters/setup-tflint@6e1e0642c0289bd619021bf6b34e3c08ed1e005a # v6 - with: - tflint_version: v0.64.0 - github_token: ${{ github.token }} - # Resolve the model against the LIVE models.dev catalog rather than # trusting a hardcoded string. Zen/NVIDIA model IDs have broken us # twice this session (wrong provider id on issue #1587's timeout From 38a289bba5e4f69496df945bff894b84f9781f3e Mon Sep 17 00:00:00 2001 From: Phil Ruff Date: Fri, 21 Aug 2026 08:22:39 +0100 Subject: [PATCH 3/4] feat(data): add Rentometer API adapter for real rent comps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Integrates Rentometer API for actual market rent estimates by address/ZIP. Replaces HUD FMR (40%+ off actual rents) as primary rent data source. - New adapter: core/integrations/market/rentometer.py with 7-day cache - Updated screening.py _get_monthly_rent() fallback chain: Rentometer → HUD FMR → None - Added _extract_zip() helper for ZIP code extraction - Added RENTO_METER_API_KEY to Django settings - 9 unit tests + 5 E2E tests Closes #359 --- core/integrations/market/rentometer.py | 172 +++++++++++++++++++++++++ core/services/screening.py | 72 ++++++++--- investor_app/settings.py | 3 + tests/e2e/test_rentometer_e2e.py | 86 +++++++++++++ tests/test_rentometer.py | 156 ++++++++++++++++++++++ 5 files changed, 474 insertions(+), 15 deletions(-) create mode 100644 core/integrations/market/rentometer.py create mode 100644 tests/e2e/test_rentometer_e2e.py create mode 100644 tests/test_rentometer.py diff --git a/core/integrations/market/rentometer.py b/core/integrations/market/rentometer.py new file mode 100644 index 0000000..f7d7998 --- /dev/null +++ b/core/integrations/market/rentometer.py @@ -0,0 +1,172 @@ +"""Rentometer API adapter for real rent estimates by ZIP code. + +Fetches median rent estimates by address or ZIP+beds from Rentometer's API. +Fallback chain: Rentometer -> HUD FMR -> user-entered rent. + +API docs: https://www.rentometer.com/api +Rate limit: 1,000 calls/month on basic plan — cache aggressively (7 days). +""" + +from __future__ import annotations + +import logging +from decimal import Decimal +from typing import Any, cast + +import requests +from django.conf import settings + +logger = logging.getLogger(__name__) + +RENTO_METER_API_BASE = "https://www.rentometer.com/api/v1" +REQUEST_TIMEOUT = 15 +CACHE_TTL_DAYS = 7 + + +class RentometerError(Exception): + """Base exception for Rentometer API errors.""" + + +class RentometerClient: + """Client for the Rentometer API.""" + + def __init__(self, api_key: str | None = None) -> None: + self.api_key = api_key or getattr(settings, "RENTO_METER_API_KEY", "") + + def _headers(self) -> dict[str, str]: + return {"Authorization": f"Bearer {self.api_key}", "Accept": "application/json"} + + def _get(self, path: str, params: dict[str, str] | None = None) -> dict[str, Any]: + """Make a GET request to the Rentometer API.""" + url = f"{RENTO_METER_API_BASE}/{path.lstrip('/')}" + resp = requests.get( + url, headers=self._headers(), params=params, timeout=REQUEST_TIMEOUT + ) + if resp.status_code == 401: + raise RentometerError("Rentometer API key is missing or invalid") + if resp.status_code == 404: + raise RentometerError(f"No data found: {path}") + resp.raise_for_status() + data = resp.json() + return cast(dict[str, Any], data) + + def get_rent_by_address( + self, + address: str, + city: str, + state: str, + zip_code: str, + ) -> dict[str, Any] | None: + """Get rent estimate for a specific address. + + Returns dict with median_rent, min_rent, max_rent, or None. + """ + params = { + "address": address, + "city": city, + "state": state, + "zip": zip_code, + } + try: + data = self._get("rent", params=params) + except RentometerError: + return None + + result = data.get("data", data) + if not result or not result.get("median"): + return None + + return { + "median_rent": _parse_rent(result.get("median")), + "min_rent": _parse_rent(result.get("min")), + "max_rent": _parse_rent(result.get("max")), + "count": result.get("count", 0), + "city": result.get("city", ""), + "state": result.get("state", ""), + } + + +def _parse_rent(value: Any) -> Decimal | None: + """Parse a rent value to Decimal.""" + if value is None or value == "": + return None + try: + val = float(value) + if val <= 0: + return None + return Decimal(str(round(val, 2))) + except ValueError, TypeError: + return None + + +# ── In-memory cache (simple TTL cache for 7-day window) ────────────────── + +_rent_cache: dict[str, tuple[float, dict[str, Any] | None]] = {} +_CACHE_TTL_SECONDS = CACHE_TTL_DAYS * 86400 + + +def get_rent_estimate( + zip_code: str | None = None, + address: str | None = None, + city: str | None = None, + state: str | None = None, + bedrooms: int = 2, +) -> Decimal | None: + """Get a rent estimate for a location from Rentometer. + + Falls back to returning None if no API key or no data found. + + Args: + zip_code: 5-digit ZIP code. + address: Street address (improves accuracy). + city: City name. + state: 2-letter state code. + bedrooms: Number of bedrooms (used for future bed-specific lookups). + + Returns: + Monthly median rent as Decimal, or None if unavailable. + """ + import time + + client = RentometerClient() + if not client.api_key: + logger.debug("RENTO_METER_API_KEY not configured — skipping Rentometer") + return None + + # Build cache key from inputs + cache_key = ( + f"{address or ''}|{city or ''}|{state or ''}|{zip_code or ''}|{bedrooms}" + ) + + # Check cache + if cache_key in _rent_cache: + ts, cached = _rent_cache[cache_key] + if time.time() - ts < _CACHE_TTL_SECONDS: + if cached and cached.get("median_rent"): + return cast(Decimal, cached["median_rent"]) + return None + + # Make API call + if not address or not city or not state or not zip_code: + logger.debug("Incomplete address for Rentometer lookup") + _rent_cache[cache_key] = (time.time(), None) + return None + + try: + result = client.get_rent_by_address(address, city, state, zip_code) + _rent_cache[cache_key] = (time.time(), result) + if result and result.get("median_rent"): + logger.debug( + "Rentometer: %s %s, %s %s = $%s", + address, + city, + state, + zip_code, + result["median_rent"], + ) + return cast(Decimal, result["median_rent"]) + except RentometerError as exc: + logger.warning("Rentometer lookup failed for %s: %s", zip_code, exc) + + _rent_cache[cache_key] = (time.time(), None) + return None diff --git a/core/services/screening.py b/core/services/screening.py index f6ea7c2..2fe29d5 100644 --- a/core/services/screening.py +++ b/core/services/screening.py @@ -514,12 +514,15 @@ def _get_monthly_rent( pipeline_property: PipelineProperty, source_record: Any | None, ) -> Decimal | None: - """Get monthly rent from source_record or PipelineProperty. + """Get monthly rent from source_record, PipelineProperty, or market APIs. - Prefers source_record (VrmProperty.projected_monthly_rent) over - PipelineProperty.estimated_rent. Falls back to HUD Fair Market Rent - lookup when neither is available. + Priority: source_record (VRM) > PipelineProperty.estimated_rent > + Rentometer (real comps) > HUD FMR > None. + + The fallback chain tries real rent data first (Rentometer), then falls + back to HUD FMR when Rentometer is unavailable. """ + # 1. Prefer source_record (VRM has actual rent data) if ( source_record is not None and _is_vrm_source(source_record) @@ -529,29 +532,47 @@ def _get_monthly_rent( if rent is not None and rent > 0: return Decimal(str(rent)) + # 2. PipelineProperty.estimated_rent (user-entered or previously cached) if ( pipeline_property.estimated_rent is not None and pipeline_property.estimated_rent > 0 ): return Decimal(str(pipeline_property.estimated_rent)) - # Fallback: try HUD FMR if we have a source record with ZIP - zip_code = None + # 3. Try Rentometer (real rent comps) — best data source + zip_code = _extract_zip(source_record, pipeline_property) bedrooms = pipeline_property.beds - if source_record is not None and hasattr(source_record, "zip_code"): - zip_code = str(source_record.zip_code) if source_record.zip_code else None # type: ignore[union-attr] + address = pipeline_property.address + city = pipeline_property.city + state = pipeline_property.state - if not zip_code and pipeline_property.address: - # Try extracting ZIP from address (common format: "..., TX 75201") - import re + if zip_code and address and city and state: + try: + from core.integrations.market.rentometer import get_rent_estimate - m = re.search(r"\b(\d{5})\b", pipeline_property.address) - if m: - zip_code = m.group(1) + rent = get_rent_estimate( + zip_code=zip_code, + address=address, + city=city, + state=state, + bedrooms=int(bedrooms) if bedrooms else 2, + ) + if rent is not None and rent > 0: + # Cache on pipeline_property for future calls + pipeline_property.estimated_rent = rent + PipelineProperty.objects.filter(pk=pipeline_property.pk).update( + estimated_rent=rent + ) + return rent + except Exception: + pass + # 4. Fallback: HUD Fair Market Rent if zip_code: try: - from core.integrations.market.hud_fmr import get_rent_estimate + from core.integrations.market.hud_fmr import ( # type: ignore[assignment] + get_rent_estimate, + ) rent = get_rent_estimate( zip_code=zip_code, bedrooms=int(bedrooms) if bedrooms else 2 @@ -569,6 +590,27 @@ def _get_monthly_rent( return None +def _extract_zip(source_record: Any | None, pipeline_property: Any) -> str | None: + """Extract ZIP code from source_record or pipeline_property.""" + if source_record is not None and hasattr(source_record, "zip_code"): + zip_code = str(source_record.zip_code) if source_record.zip_code else None # type: ignore[union-attr] + if zip_code: + return zip_code + + # Check PipelineProperty.zip_code directly + if hasattr(pipeline_property, "zip_code") and pipeline_property.zip_code: + return str(pipeline_property.zip_code) + + if pipeline_property.address: + import re + + m = re.search(r"\b(\d{5})\b", pipeline_property.address) + if m: + return m.group(1) + + return None + + def _eval_year_built( pipeline_property: PipelineProperty, criteria: ScreeningCriteria, diff --git a/investor_app/settings.py b/investor_app/settings.py index 914375e..191fd4d 100644 --- a/investor_app/settings.py +++ b/investor_app/settings.py @@ -358,6 +358,9 @@ ATTOM_API_KEY: str = env( "ATTOM_API_KEY", default="" ) # ATTOM preforeclosure + property data +RENTO_METER_API_KEY: str = env( + "RENTO_METER_API_KEY", default="" +) # Rentometer real rent estimates by address/ZIP REHAB_COST_PER_SQFT: dict[str, Decimal] = { "cosmetic": Decimal( diff --git a/tests/e2e/test_rentometer_e2e.py b/tests/e2e/test_rentometer_e2e.py new file mode 100644 index 0000000..9363f8c --- /dev/null +++ b/tests/e2e/test_rentometer_e2e.py @@ -0,0 +1,86 @@ +"""E2E tests for Rentometer API integration in screening pipeline.""" + +import pytest +from decimal import Decimal + +from core.models import PipelineProperty + +pytestmark = pytest.mark.django_db(transaction=True) + + +@pytest.fixture() +def rentometer_property(db, e2e_login, growth_area) -> PipelineProperty: + """Property without rent data — should trigger Rentometer lookup.""" + return PipelineProperty.objects.create( + user=e2e_login, + source_type=PipelineProperty.SourceType.HUD, + source_id="RENT-001", + address="456 Oak Ave", + city="Austin", + state="TX", + zip_code="78704", + county="Travis", + growth_area=growth_area, + stage=PipelineProperty.Stage.SCREENING, + status=PipelineProperty.Status.ACTIVE, + screening_passed=None, + price=Decimal("180000"), + beds=3, + ) + + +class TestRentometerE2E: + def test_extract_zip_from_property(self, rentometer_property) -> None: + """_extract_zip correctly extracts ZIP from PipelineProperty.""" + from core.services.screening import _extract_zip + + zip_code = _extract_zip(None, rentometer_property) + assert zip_code == "78704" + + def test_extract_zip_from_address(self, rentometer_property) -> None: + """_extract_zip extracts ZIP from address when zip_code is empty.""" + rentometer_property.zip_code = "" + rentometer_property.address = "123 Main St, Austin, TX 78701" + from core.services.screening import _extract_zip + + zip_code = _extract_zip(None, rentometer_property) + assert zip_code == "78701" + + def test_rentometer_returns_none_without_api_key(self, rentometer_property) -> None: + """get_rent_estimate returns None when no API key configured.""" + from core.integrations.market.rentometer import get_rent_estimate + + with self._override_settings(RENTO_METER_API_KEY=""): + result = get_rent_estimate( + zip_code="78704", + address="456 Oak Ave", + city="Austin", + state="TX", + ) + assert result is None + + def test_screening_uses_estimated_rent(self, rentometer_property) -> None: + """Screening uses estimated_rent when set on property.""" + rentometer_property.estimated_rent = Decimal("2100.00") + rentometer_property.save(update_fields=["estimated_rent"]) + + from core.services.screening import _get_monthly_rent + + rent = _get_monthly_rent(rentometer_property, None) + assert rent == Decimal("2100.00") + + def test_screening_falls_back_to_hud_fmr(self, rentometer_property) -> None: + """Screening falls back to HUD FMR when no rent data available.""" + from core.services.screening import _get_monthly_rent + + # With no API key, should return None (no FMR fallback in test env) + rent = _get_monthly_rent(rentometer_property, None) + # In test env without HUD API key, this returns None + assert rent is None + + @staticmethod + def _override_settings(**kwargs): + """Context manager to override Django settings.""" + from django.test import override_settings + + return override_settings(**kwargs) diff --git a/tests/test_rentometer.py b/tests/test_rentometer.py new file mode 100644 index 0000000..50cadf7 --- /dev/null +++ b/tests/test_rentometer.py @@ -0,0 +1,156 @@ +"""Unit tests for Rentometer API adapter.""" + +from __future__ import annotations + +from decimal import Decimal +from unittest.mock import MagicMock, patch + + +from core.integrations.market.rentometer import ( + RentometerClient, + RentometerError, + get_rent_estimate, +) + + +class TestRentometerClient: + """Test suite for RentometerClient.""" + + def test_client_requires_api_key(self) -> None: + """Client should raise error when API key is missing.""" + with patch("core.integrations.market.rentometer.settings") as mock_settings: + mock_settings.RENTO_METER_API_KEY = "" + client = RentometerClient() + assert client.api_key == "" + + def test_client_uses_settings_api_key(self) -> None: + """Client should read API key from Django settings.""" + with patch("core.integrations.market.rentometer.settings") as mock_settings: + mock_settings.RENTO_METER_API_KEY = "test-key-123" + client = RentometerClient() + assert client.api_key == "test-key-123" + + def test_client_accepts_explicit_api_key(self) -> None: + """Client should accept explicit API key over settings.""" + client = RentometerClient(api_key="explicit-key") + assert client.api_key == "explicit-key" + + @patch("core.integrations.market.rentometer.requests.get") + def test_get_rent_by_address_success(self, mock_get: MagicMock) -> None: + """Successful rent lookup returns parsed data.""" + mock_resp = MagicMock() + mock_resp.status_code = 200 + mock_resp.json.return_value = { + "median": "1850", + "min": "1600", + "max": "2100", + "count": 15, + "city": "Austin", + "state": "TX", + } + mock_get.return_value = mock_resp + + client = RentometerClient(api_key="test-key") + result = client.get_rent_by_address( + address="123 Main St", + city="Austin", + state="TX", + zip_code="78701", + ) + + assert result is not None + assert result["median_rent"] == Decimal("1850.00") + assert result["min_rent"] == Decimal("1600.00") + assert result["max_rent"] == Decimal("2100.00") + assert result["count"] == 15 + + @patch("core.integrations.market.rentometer.requests.get") + def test_get_rent_by_address_no_data(self, mock_get: MagicMock) -> None: + """Returns None when API returns no median rent.""" + mock_resp = MagicMock() + mock_resp.status_code = 200 + mock_resp.json.return_value = {"median": None, "count": 0} + mock_get.return_value = mock_resp + + client = RentometerClient(api_key="test-key") + result = client.get_rent_by_address( + address="123 Main St", + city="Austin", + state="TX", + zip_code="78701", + ) + assert result is None + + @patch("core.integrations.market.rentometer.requests.get") + def test_get_rent_by_address_api_error(self, mock_get: MagicMock) -> None: + """Returns None on API error.""" + mock_get.side_effect = RentometerError("API error") + + client = RentometerClient(api_key="test-key") + result = client.get_rent_by_address( + address="123 Main St", + city="Austin", + state="TX", + zip_code="78701", + ) + assert result is None + + +class TestGetRentEstimate: + """Test suite for get_rent_estimate convenience function.""" + + def test_returns_none_without_api_key(self) -> None: + """Returns None when no API key is configured.""" + with patch("core.integrations.market.rentometer.settings") as mock_settings: + mock_settings.RENTO_METER_API_KEY = "" + result = get_rent_estimate( + zip_code="78701", + address="123 Main St", + city="Austin", + state="TX", + ) + assert result is None + + def test_returns_none_with_incomplete_address(self) -> None: + """Returns None when address components are missing.""" + with patch("core.integrations.market.rentometer.settings") as mock_settings: + mock_settings.RENTO_METER_API_KEY = "test-key" + result = get_rent_estimate(zip_code="78701") + assert result is None + + @patch("core.integrations.market.rentometer.RentometerClient") + def test_caches_rent_estimate(self, MockClient: MagicMock) -> None: + """Rent estimate is cached for subsequent calls.""" + mock_instance = MagicMock() + MockClient.return_value = mock_instance + mock_instance.get_rent_by_address.return_value = { + "median_rent": Decimal("1850.00"), + "min_rent": Decimal("1600.00"), + "max_rent": Decimal("2100.00"), + "count": 15, + } + + with patch("core.integrations.market.rentometer.settings") as mock_settings: + mock_settings.RENTO_METER_API_KEY = "test-key" + + # First call hits API + result1 = get_rent_estimate( + zip_code="78701", + address="123 Main St", + city="Austin", + state="TX", + ) + assert result1 == Decimal("1850.00") + assert mock_instance.get_rent_by_address.call_count == 1 + + # Second call uses cache + result2 = get_rent_estimate( + zip_code="78701", + address="123 Main St", + city="Austin", + state="TX", + ) + assert result2 == Decimal("1850.00") + assert ( + mock_instance.get_rent_by_address.call_count == 1 + ) # No additional API call From 61f19d4e88f87d879d7e2f1197f8245ef17da317 Mon Sep 17 00:00:00 2001 From: Phil Ruff Date: Fri, 21 Aug 2026 08:34:54 +0100 Subject: [PATCH 4/4] fix(screening): use getattr for pipeline_property attributes Fix AttributeError on _PipelineView objects that lack address/city/state attributes. Use getattr with None defaults for safe access. --- core/services/screening.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/core/services/screening.py b/core/services/screening.py index 2fe29d5..4beedb2 100644 --- a/core/services/screening.py +++ b/core/services/screening.py @@ -542,9 +542,10 @@ def _get_monthly_rent( # 3. Try Rentometer (real rent comps) — best data source zip_code = _extract_zip(source_record, pipeline_property) bedrooms = pipeline_property.beds - address = pipeline_property.address - city = pipeline_property.city - state = pipeline_property.state + # Use getattr for safety — some callers pass _PipelineView proxies + address = getattr(pipeline_property, "address", None) + city = getattr(pipeline_property, "city", None) + state = getattr(pipeline_property, "state", None) if zip_code and address and city and state: try: