Skip to content

fix: prefer rent price for mixed buy-rent search results#13

Open
ADikmarov wants to merge 1 commit into
0xMH:mainfrom
ADikmarov:fix/rent-search-koophuur-price
Open

fix: prefer rent price for mixed buy-rent search results#13
ADikmarov wants to merge 1 commit into
0xMH:mainfrom
ADikmarov:fix/rent-search-koophuur-price

Conversation

@ADikmarov

Copy link
Copy Markdown

Summary

  • Pass the requested search offering type into search-result parsing.
  • Prefer rent_price fields for category="rent" results when Funda returns mixed koophuur listings with both sale and rent prices.
  • Add regression coverage for mixed buy/rent search hits and parser invocation.

Why

Funda can return listings whose offering_type is ['buy', 'rent'] for rental searches. Previously the parser selected the first offering type (buy) and therefore exposed the sale price (for example 400000) even though the user searched category="rent" and the raw payload also contained rent_price (for example 2250).

Verification

  • uv run python -m py_compile funda/*.py examples/*.py tests/*.py
  • uv run python -m unittest discover -s tests -v
  • PYFUNDA_LIVE=1 uv run python -m unittest tests.test_live -v
  • uv build
  • Manual smoke: client.search('amsterdam', category='rent', max_price=2500)[0] now returns offering_type == 'rent' and price.amount == raw['_source']['price']['rent_price'][0].

@0xMH 0xMH left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few concerns before merging.

Inconsistent offering_type application: _hit now resolves the offering type via the preferred hint, but _property_details still uses first(source.get("offering_type")). For a mixed ["buy", "rent"] listing this can yield offering_type="rent" on the Listing while raw_offering_type/status are derived from "buy". The preferred type should either flow through that path too, or the parser should intentionally keep those concepts separate with test coverage.

Comment thread funda/_search_parser.py
rent_amount = first(price.get("rent_price"))
sale_range = mapping(price.get("selling_price_range"))
rent_range = mapping(price.get("rent_price_range"))
range_data = sale_range or rent_range

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The initial range_data = sale_range or rent_range is now dead code since both branches of the if/else immediately reassign it. Please drop that line. The else branch also just re-derives the original behavior, so this could collapse into a single prefer_rent flag instead of duplicating three lines per branch.

Comment thread funda/_search_parser.py
)


def _offering_type(value: Any, preferred: str | None = None) -> str | None:

@0xMH 0xMH Jul 8, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_offering_type() adds a second raw-offering normalization path next to the existing first() + normalize_offering_type() pattern. Since preferred selection is now part of parser behavior, consider centralizing that handling and using it consistently rather than only in _hit.

Comment thread funda/_search_parser.py
path = source.get("object_detail_page_relative_url")
tiny_id = tiny_id_from_url(path)
offering_type = normalize_offering_type(first(source.get("offering_type")))
offering_type = _offering_type(source.get("offering_type"), preferred_offering_type)

@0xMH 0xMH Jul 8, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Threading preferred_offering_type from the client into the pure parser couples parsing to caller intent. If that context is required to choose between sale and rent fields, can we make it an explicit parser-level decision and apply it consistently through the parsed model? Right now it enters through _hit, but related metadata still follows the old first-offering path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants