add location extractor logic#28
Conversation
📝 WalkthroughWalkthroughAdds a location extraction pipeline that uses an LLM to identify Hebrew locations, geocodes them, converts polygons to bounded WKT, and exposes location context and instructions through extractor graph state. Shared extractor contracts, runtime configuration, dependencies, and comprehensive tests are included. ChangesLocation extraction pipeline
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ExtractorNode
participant LocationExtractorAgent
participant LangfuseLLM
participant Nominatim
participant Shapely
ExtractorNode->>LocationExtractorAgent: extract(query)
LocationExtractorAgent->>LangfuseLLM: invoke location prompt
LangfuseLLM-->>LocationExtractorAgent: location mapping JSON
LocationExtractorAgent->>Nominatim: request polygon geometry
Nominatim-->>LocationExtractorAgent: return GeoJSON geometry
LocationExtractorAgent->>Shapely: simplify geometry to bounded WKT
Shapely-->>LocationExtractorAgent: return WKT
LocationExtractorAgent-->>ExtractorNode: return context entries and state updates
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agent/src/agent/nodes/extractor.py`:
- Around line 129-157: Update extractor_node so construction failures from
optional extractors, especially LocationExtractor, are isolated and do not abort
the node. Instantiate each extractor within its own try block or worker, retain
successfully constructed extractors such as TimeExtractor and LLMExtractor, and
allow the existing concurrent execution and error handling to process only the
available extractors.
In `@agent/src/agent/services/geo_utils.py`:
- Around line 15-16: Update get_geojson_polygon so transient request or JSON
failures returning None are not stored by the lru_cache. Cache only successful
geometry results, or otherwise add a short-lived negative-cache policy while
preserving the existing successful lookup behavior.
- Around line 15-36: Update get_geojson_polygon to use a shared, thread-safe
monotonic rate limiter that spaces all Nominatim requests at least one second
apart, replacing the per-call time.sleep(1). Replace the browser User-Agent with
a configurable client-specific User-Agent/contact string, using the project’s
existing configuration mechanism where available.
- Around line 44-48: Update get_geojson_polygon() to return geometry data only
when its GeoJSON type is Polygon or MultiPolygon; reject Point, LineString, and
all other non-area types before returning. Preserve the existing None behavior
for missing geometry, and ensure geojson_to_simplified_wkt() cannot store
rejected geometries as wkt_polygon or location_wkt_instruction.
In `@agent/src/agent/services/location_extractor.py`:
- Around line 77-80: Update the identifier construction in the location
extraction flow to sanitize and validate LLM-produced English names into valid
instruction identifiers, including punctuation handling. Detect duplicate
identifiers such as normalized variants of the same name before populating
coords_dict or instruction_parts, and explicitly reject or resolve collisions
rather than overwriting state; keep names_dict behavior unchanged.
- Around line 142-152: Update the async run method to execute
_process_locations(locations_map) in a worker thread via the project’s
async-to-thread mechanism, awaiting its result instead of calling it directly on
the event loop. Preserve the existing parsing and returned
LocationExtractionResult behavior.
In `@agent/tests/test_location_extractor.py`:
- Around line 104-111: Update the fallback assertions in the
geojson_to_simplified_wkt tests to compare the returned value directly with
shape(geojson_dense).envelope.wkt for both fallback cases. Retain the existing
length and non-null checks as appropriate, but remove reliance on generic
POLYGON checks so the tests explicitly verify the envelope fallback branch.
- Around line 118-125: Update test_geocoding_api_failure to have requests.get
return a mocked response whose raise_for_status() raises HTTPError, rather than
making requests.get raise directly. Keep asserting get_geojson_polygon returns
None, verify raise_for_status() is called, and retain the requests.get call
assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9e334002-c8f3-4023-b525-6dde2cb65019
⛔ Files ignored due to path filters (1)
agent/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
agent/pyproject.tomlagent/src/agent/config.pyagent/src/agent/llm.pyagent/src/agent/nodes/extractor.pyagent/src/agent/nodes/satisfaction_check.pyagent/src/agent/nodes/schema_explorer.pyagent/src/agent/services/extractor_base.pyagent/src/agent/services/geo_utils.pyagent/src/agent/services/location_extractor.pyagent/src/agent/state.pyagent/tests/test_location_extractor.py
| # ── Node ────────────────────────────────────────────────────────────────────── | ||
|
|
||
|
|
||
| def extractor_node(state: AgentState, config: RunnableConfig | None = None) -> dict: | ||
| """Enrich the user query with additional context to help downstream phases. | ||
|
|
||
| Runs all extractors concurrently via a thread pool: | ||
| - TimeExtractor and LLMExtractor run for every request. | ||
| - LocationExtractor resolves Hebrew place names to WKT polygons. | ||
| - HTTPExtractor instances are added for each entry in active_extractors. | ||
|
|
||
| Each extractor may also contribute extra AgentState fields via state_update(). | ||
| """ | ||
| user_query = state["user_query"] | ||
| active_extractors = state.get("active_extractors") or [] | ||
| runtime_flags = state.get("runtime_flags") or {} | ||
|
|
||
| thread_id = config.get("configurable", {}).get("thread_id", "") if config else "" | ||
| publish_node_event_sync(thread_id, "extractor") | ||
|
|
||
| import concurrent.futures | ||
|
|
||
| extractors: List[BaseExtractor] = [ | ||
| TimeExtractor(runtime_flags=runtime_flags), | ||
| LLMExtractor(runtime_flags=runtime_flags), | ||
| LocationExtractor(runtime_flags=runtime_flags), | ||
| *[ | ||
| HTTPExtractor(ext["url"], ext["name"], runtime_flags=runtime_flags) | ||
| for ext in active_extractors | ||
| ], | ||
| ] |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Isolate extractor construction failures.
All extractors are instantiated before the futures error handling. If the new location prompt cannot be retrieved, LocationExtractor raises here and aborts the whole node, so even time and generic enrichments are lost. Construct each optional extractor inside an isolated try block or worker.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@agent/src/agent/nodes/extractor.py` around lines 129 - 157, Update
extractor_node so construction failures from optional extractors, especially
LocationExtractor, are isolated and do not abort the node. Instantiate each
extractor within its own try block or worker, retain successfully constructed
extractors such as TimeExtractor and LLMExtractor, and allow the existing
concurrent execution and error handling to process only the available
extractors.
| @lru_cache(maxsize=128) | ||
| def get_geojson_polygon(location_name: str) -> Optional[dict]: |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Do not cache transient geocoding failures.
Because the cached function converts request and JSON failures into None, one temporary outage suppresses retries for that location until cache eviction. Cache successful geometry only, or use a short TTL for negative results.
Also applies to: 49-51
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@agent/src/agent/services/geo_utils.py` around lines 15 - 16, Update
get_geojson_polygon so transient request or JSON failures returning None are not
stored by the lru_cache. Cache only successful geometry results, or otherwise
add a short-lived negative-cache policy while preserving the existing successful
lookup behavior.
| @lru_cache(maxsize=128) | ||
| def get_geojson_polygon(location_name: str) -> Optional[dict]: | ||
| """ | ||
| Fetches GeoJSON polygon for a Hebrew location name. | ||
| Returns the 'geometry' dict if found, else None. | ||
| """ | ||
| # Enforce Nominatim policy: max 1 request per second | ||
| time.sleep(1) | ||
| try: | ||
| url = "https://nominatim.openstreetmap.org/search" | ||
| params = { | ||
| "q": location_name, | ||
| "accept-language": "he", | ||
| "polygon_geojson": "1", | ||
| "limit": "1", | ||
| "format": "geojson" | ||
| } | ||
| headers = { | ||
| "User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36" | ||
| } | ||
|
|
||
| res = requests.get(url, params=params, headers=headers, timeout=10) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -e
printf '\n== geo_utils.py ==\n'
wc -l agent/src/agent/services/geo_utils.py
sed -n '1,220p' agent/src/agent/services/geo_utils.py
printf '\n== search for requests.get / User-Agent / limiter ==\n'
rg -n "User-Agent|requests\.get|lru_cache|sleep\(|rate limit|limiter|nominatim|geojson_polygon|get_geojson_polygon" agent/src -SRepository: StavPonte11/text2sql-onboarding
Length of output: 4619
🌐 Web query:
Nominatim usage policy User-Agent identify application contact email 1 request per second concurrent requests
💡 Result:
To use the public Nominatim API provided by OpenStreetMap, you must adhere to the official Usage Policy [1][2]. Key requirements include: User-Agent and Identification You must provide a valid HTTP Referer or User-Agent header that clearly identifies your application [1][2]. Generic or stock User-Agents assigned by HTTP libraries (e.g., "python-requests/2.x") are strictly prohibited and will not be accepted [1][2]. If you cannot control the User-Agent or Referer, or if you are making large numbers of requests, you should include a valid email address as a parameter in your API calls to help identify your traffic [3][4][5]. Request Rate and Concurrency The service enforces a strict absolute maximum limit of 1 request per second [1][2]. This limit applies to your application as a whole (the sum of traffic generated by all your users) [2][6]. Additionally, there is a technical limit enforced per IP address [6]. Because the service relies on donated servers with limited capacity, you are strongly encouraged to implement caching and use a proxy to manage requests efficiently [2][6]. Compliance Applications must be designed with the capability to switch to an alternative geocoding service at any time without requiring a software update [2]. The API must not be integrated into no-code or low-code platforms as a generic service, and developers must make a deliberate, informed decision to use it [2]. Failure to comply with these policies can result in your access being blocked [4][7].
Citations:
- 1: https://operations.osmfoundation.org/policies/nominatim/
- 2: https://operations.osmfoundation.org/policies/nominatim/?referrer=wordpress.com
- 3: https://nominatim.org/release-docs/develop/api/Search/
- 4: Access blocked per device osm-search/Nominatim#3790
- 5: https://nominatim.org/release-docs/latest/api/Search/
- 6: https://community.openstreetmap.org/t/clarification-on-nominatim-usage-policy/102661
- 7: https://community.openstreetmap.org/t/help-nominatim-returns-usage-limit-reached/103102
Enforce a shared Nominatim rate limit and app-specific User-Agent.
time.sleep(1) here only throttles each call individually, so concurrent requests can still exceed the 1 req/s application-wide limit. The current Chrome UA also doesn’t identify this service. Use a shared monotonic limiter and a configurable User-Agent/contact string for this client.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 35-35: Request-controlled URL passed to requests; validate against an allowlist to prevent SSRF.
Context: requests.get(url, params=params, headers=headers, timeout=10)
Note: [CWE-918] Server-Side Request Forgery (SSRF).
(ssrf-requests)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@agent/src/agent/services/geo_utils.py` around lines 15 - 36, Update
get_geojson_polygon to use a shared, thread-safe monotonic rate limiter that
spaces all Nominatim requests at least one second apart, replacing the per-call
time.sleep(1). Replace the browser User-Agent with a configurable
client-specific User-Agent/contact string, using the project’s existing
configuration mechanism where available.
| geom = features[0].get("geometry") | ||
| if not geom: | ||
| return None | ||
|
|
||
| return geom |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file list =="
git ls-files agent/src/agent/services/geo_utils.py
echo
echo "== outline =="
ast-grep outline agent/src/agent/services/geo_utils.py --view expanded
echo
echo "== file excerpt =="
cat -n agent/src/agent/services/geo_utils.py | sed -n '1,220p'
echo
echo "== call sites / mentions =="
rg -n "geo_utils|geometry|Polygon|MultiPolygon|LineString|Point|describe.*polygon|polygon" agent/src -SRepository: StavPonte11/text2sql-onboarding
Length of output: 7447
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== outline location_extractor =="
ast-grep outline agent/src/agent/services/location_extractor.py --view expanded
echo
echo "== relevant excerpt =="
cat -n agent/src/agent/services/location_extractor.py | sed -n '1,180p'Repository: StavPonte11/text2sql-onboarding
Length of output: 8393
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== search for wkt_polygon / polygon assumptions =="
rg -n "wkt_polygon|polygon\)|ST_Intersects|ST_Contains|ST_Within|ST_Area|ST_IsValid|geom_type|GeometryType|MULTIPOLYGON|POLYGON" agent/src -SRepository: StavPonte11/text2sql-onboarding
Length of output: 1042
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== outline location_extractor =="
ast-grep outline agent/src/agent/services/location_extractor.py --view expanded
echo
echo "== relevant excerpt =="
cat -n agent/src/agent/services/location_extractor.py | sed -n '1,180p'
echo
echo "== locations of called helper =="
rg -n "geojson_to_simplified_wkt|get_geojson_polygon|wkt_polygon|polygon" agent/src/agent/services/location_extractor.py -n -SRepository: StavPonte11/text2sql-onboarding
Length of output: 9239
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== location_extractor excerpt =="
cat -n agent/src/agent/services/location_extractor.py | sed -n '1,180p'Repository: StavPonte11/text2sql-onboarding
Length of output: 7262
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== geometry / polygon usage across agent/src =="
rg -n "wkt_polygon|coords_dict|polygon|ST_Intersects|ST_Contains|ST_Within|ST_Area|GeometryType|MultiPolygon|Polygon" agent/src -SRepository: StavPonte11/text2sql-onboarding
Length of output: 2412
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== extractor excerpt around location usage =="
cat -n agent/src/agent/nodes/extractor.py | sed -n '70,160p'Repository: StavPonte11/text2sql-onboarding
Length of output: 4364
Reject non-area geometries in geo_utils. get_geojson_polygon() can pass through Point/LineString, and geojson_to_simplified_wkt() then stores them as wkt_polygon/location_wkt_instruction as if they were areas. Accept only Polygon/MultiPolygon, or convert other geometry types with explicit, documented semantics.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@agent/src/agent/services/geo_utils.py` around lines 44 - 48, Update
get_geojson_polygon() to return geometry data only when its GeoJSON type is
Polygon or MultiPolygon; reject Point, LineString, and all other non-area types
before returning. Preserve the existing None behavior for missing geometry, and
ensure geojson_to_simplified_wkt() cannot store rejected geometries as
wkt_polygon or location_wkt_instruction.
| var_name = f"{loc.english_name.lower().replace(' ', '_')}_wkt" | ||
| instruction_parts.append(f"{var_name} = {loc.wkt_polygon}") | ||
| coords_dict[var_name] = loc.wkt_polygon | ||
| names_dict[loc.hebrew_name] = loc.english_name |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Validate identifiers and reject collisions before building state.
LLM-produced names are only lowercased and space-normalized. Punctuation can create invalid instruction identifiers, while values such as New York and new_york overwrite the same coords_dict entry. Generate validated identifiers and handle duplicates explicitly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@agent/src/agent/services/location_extractor.py` around lines 77 - 80, Update
the identifier construction in the location extraction flow to sanitize and
validate LLM-produced English names into valid instruction identifiers,
including punctuation handling. Detect duplicate identifiers such as normalized
variants of the same name before populating coords_dict or instruction_parts,
and explicitly reject or resolve collisions rather than overwriting state; keep
names_dict behavior unchanged.
| async def run(self, user_request: str) -> LocationExtractionResult: | ||
| # Step 1: LLM Call | ||
| messages = self.prompt_template.format_messages(user_query=user_request) | ||
| response = await self.llm.ainvoke(messages) | ||
| content = response.content | ||
|
|
||
| # Step 2: Robust JSON Parsing | ||
| locations_map = self._parse_llm_json(content) | ||
|
|
||
| # Step 3 & 4: Process locations | ||
| return self._process_locations(locations_map) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Move geocoding off the async event loop.
_process_locations() performs time.sleep() and synchronous requests.get() calls. Calling it directly from run() blocks every coroutine sharing this event loop, potentially for multiple seconds per location.
Proposed fix
+import asyncio
...
- return self._process_locations(locations_map)
+ return await asyncio.to_thread(self._process_locations, locations_map)📝 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.
| async def run(self, user_request: str) -> LocationExtractionResult: | |
| # Step 1: LLM Call | |
| messages = self.prompt_template.format_messages(user_query=user_request) | |
| response = await self.llm.ainvoke(messages) | |
| content = response.content | |
| # Step 2: Robust JSON Parsing | |
| locations_map = self._parse_llm_json(content) | |
| # Step 3 & 4: Process locations | |
| return self._process_locations(locations_map) | |
| async def run(self, user_request: str) -> LocationExtractionResult: | |
| # Step 1: LLM Call | |
| messages = self.prompt_template.format_messages(user_query=user_request) | |
| response = await self.llm.ainvoke(messages) | |
| content = response.content | |
| # Step 2: Robust JSON Parsing | |
| locations_map = self._parse_llm_json(content) | |
| # Step 3 & 4: Process locations | |
| return await asyncio.to_thread(self._process_locations, locations_map) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@agent/src/agent/services/location_extractor.py` around lines 142 - 152,
Update the async run method to execute _process_locations(locations_map) in a
worker thread via the project’s async-to-thread mechanism, awaiting its result
instead of calling it directly on the event loop. Preserve the existing parsing
and returned LocationExtractionResult behavior.
| # Set max_length extremely low (e.g. 120 characters) | ||
| # Binary search at tolerance=1.0 will still be too long or fail, | ||
| # and it will fall back to envelope (bounding box) which has only 5 vertices and fits. | ||
| wkt_result = geojson_to_simplified_wkt(geojson_dense, max_length=120) | ||
| assert wkt_result is not None | ||
| assert len(wkt_result) <= 120 | ||
| # Bounding box of unit circle region fits within limits | ||
| assert "POLYGON" in wkt_result |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
sed -n '1,220p' agent/tests/test_location_extractor.py
printf '\n---\n'
rg -n "def geojson_to_simplified_wkt|geojson_to_simplified_wkt\\(" -S .Repository: StavPonte11/text2sql-onboarding
Length of output: 8677
🏁 Script executed:
sed -n '1,220p' agent/src/agent/services/geo_utils.py
printf '\n---\n'
sed -n '220,320p' agent/tests/test_location_extractor.pyRepository: StavPonte11/text2sql-onboarding
Length of output: 8005
Assert the envelope fallback explicitly.
len(...) <= max_length and "POLYGON" still pass if plain simplification returns a shorter polygon, so these tests don’t prove the fallback branch ran. Compare the result against shape(geojson_dense).envelope.wkt in both fallback cases.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@agent/tests/test_location_extractor.py` around lines 104 - 111, Update the
fallback assertions in the geojson_to_simplified_wkt tests to compare the
returned value directly with shape(geojson_dense).envelope.wkt for both fallback
cases. Retain the existing length and non-null checks as appropriate, but remove
reliance on generic POLYGON checks so the tests explicitly verify the envelope
fallback branch.
| def test_geocoding_api_failure(mocker): | ||
| # Mock requests.get to throw HTTPError | ||
| mock_get = mocker.patch("requests.get", side_effect=requests.exceptions.HTTPError("Nominatim down")) | ||
|
|
||
| # Verify get_geojson_polygon catches it and returns None | ||
| result = get_geojson_polygon("failure_test_location") | ||
| assert result is None | ||
| mock_get.assert_called_once() |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate relevant definitions and surrounding context.
rg -n "def get_geojson_polygon|raise_for_status|requests.get|HTTPError|geocode" agent -S
# Show the test file around the referenced lines.
sed -n '1,220p' agent/tests/test_location_extractor.pyRepository: StavPonte11/text2sql-onboarding
Length of output: 9374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the implementation file and print the relevant section with line numbers.
impl_file=$(rg -l "def get_geojson_polygon" agent | head -n 1)
echo "Implementation file: $impl_file"
sed -n '1,260p' "$impl_file"Repository: StavPonte11/text2sql-onboarding
Length of output: 3748
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant test and implementation behavior precisely.
python3 - <<'PY'
from pathlib import Path
import re
test_path = Path("agent/tests/test_location_extractor.py")
print("TEST FILE EXISTS:", test_path.exists())
if test_path.exists():
text = test_path.read_text()
lines = text.splitlines()
for i in range(110, 132):
if i <= len(lines):
print(f"{i:4d}: {lines[i-1]}")
# Search for implementation file(s)
import subprocess, shlex, os, sys
res = subprocess.run(["bash", "-lc", "rg -n \"def get_geojson_polygon|raise_for_status|requests.get|HTTPError\" agent -S"], capture_output=True, text=True)
print("\nRG OUTPUT:\n", res.stdout)
print(res.stderr)
PYRepository: StavPonte11/text2sql-onboarding
Length of output: 1936
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the implementation around get_geojson_polygon and the test around the failure case.
impl=$(rg -l "def get_geojson_polygon" agent | head -n 1)
echo "IMPLEMENTATION=$impl"
echo "--- IMPLEMENTATION ---"
sed -n '1,220p' "$impl"
echo "--- TEST ---"
sed -n '110,140p' agent/tests/test_location_extractor.pyRepository: StavPonte11/text2sql-onboarding
Length of output: 4884
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path("agent/tests/test_location_extractor.py")
print(p.read_text()[:5000])
PYRepository: StavPonte11/text2sql-onboarding
Length of output: 5172
Exercise the HTTP-status failure path. Mock a response and make raise_for_status() raise HTTPError, then assert raise_for_status() is called; raising directly from requests.get only covers request-level failures.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@agent/tests/test_location_extractor.py` around lines 118 - 125, Update
test_geocoding_api_failure to have requests.get return a mocked response whose
raise_for_status() raises HTTPError, rather than making requests.get raise
directly. Keep asserting get_geojson_polygon returns None, verify
raise_for_status() is called, and retain the requests.get call assertion.
1 participant
Summary by CodeRabbit