fix(site-finder): address PR #90 auto-review feedback
Must-fix (3):
1. distance_m falsy guard: `if obj.get("distance_m") is not None` вместо
`if obj.get("distance_m")` — centroid-on-building даёт 0.0 (falsy float),
raw Decimal иначе упал бы в JSON serialization.
2. SQL plan note добавлен про seq scan ~3000 строк OK; при росте — нужен
GIST/index на (latitude, longitude) — отдельный issue для database-expert
(будет создан separately).
3. obj_class NULL bug помечен в docstring _aggregate_pipeline с reference на
fixes/Bug_Kn_API_Obj_Class_Always_Null_OPEN. D6/#38 — fix плановый.
Cleanup (3 из 5):
4. CLASS_LABEL.null dead key убран — JSON null приходит as absent key, не
"null" string.
6. Magic numbers вынесены: PIPELINE_RADIUS_M=5000, PIPELINE_HORIZON_MONTHS=24,
PIPELINE_SEVERITY_MEDIUM_THRESHOLD=500, PIPELINE_SEVERITY_HIGH_THRESHOLD=3000,
PIPELINE_TOP_OBJECTS_LIMIT=10. SQL query теперь через f-string подставляет
их (защищённое от injection — это int литералы).
8. obj.ready_dt formatting через fmtMonth() с new Date + toLocaleDateString —
robust к datetime suffix vs date-only, fallback к substring(0,7) при NaN.
Не сделано (defer):
5. Async 3 HTTP calls (pre-existing pattern, нужен ThreadPoolExecutor refactor
отдельным PR — затрагивает weather/air_quality fetch architecture).
7. ST_GeomFromText дважды — CSE справляется на этом масштабе.
Per auto-review on ade511b.
This commit is contained in:
lekss361
parent
ade511bff3
commit
e452a1d75e
2 changed files with 46 additions and 17 deletions
|
|
@ -274,6 +274,15 @@ GEOTECH_BY_REGION: dict[int, dict[str, Any]] = {
|
|||
}
|
||||
|
||||
|
||||
# D4 (#36) — pipeline 24mo constants. Размещены в одном месте для тюнинга
|
||||
# и аудита; severity пороги матчатся с acceptance #36.
|
||||
PIPELINE_RADIUS_M = 5000
|
||||
PIPELINE_HORIZON_MONTHS = 24
|
||||
PIPELINE_SEVERITY_MEDIUM_THRESHOLD = 500 # flats_total < это → low
|
||||
PIPELINE_SEVERITY_HIGH_THRESHOLD = 3000 # flats_total >= это → high
|
||||
PIPELINE_TOP_OBJECTS_LIMIT = 10
|
||||
|
||||
|
||||
def _aggregate_pipeline(rows: list[Any]) -> dict[str, Any]:
|
||||
"""D4 (#36) — собрать pipeline_24mo aggregate из rows domrf_kn_objects.
|
||||
|
||||
|
|
@ -281,8 +290,12 @@ def _aggregate_pipeline(rows: list[Any]) -> dict[str, Any]:
|
|||
- objects_count, flats_total
|
||||
- by_class: {economy: int, comfort: int, business: int, unknown: int}
|
||||
- by_quarter: {"2026-Q1": {objects: N, flats: M}, ...}
|
||||
- severity: low / medium / high (по flats_total)
|
||||
- top_objects: первые 10 ближайших / крупнейших ЖК
|
||||
- severity: low / medium / high (см. PIPELINE_SEVERITY_* пороги)
|
||||
- top_objects: PIPELINE_TOP_OBJECTS_LIMIT крупнейших ЖК по flat_count
|
||||
|
||||
NB: `obj_class` в production часто NULL (см.
|
||||
`fixes/Bug_Kn_API_Obj_Class_Always_Null_OPEN`) — by_class dominated by
|
||||
"unknown" пока не закрыт D6/#38.
|
||||
|
||||
Используется для UI pipeline-bar и severity badge.
|
||||
"""
|
||||
|
|
@ -294,7 +307,10 @@ def _aggregate_pipeline(rows: list[Any]) -> dict[str, Any]:
|
|||
"by_quarter": [],
|
||||
"severity": "none",
|
||||
"top_objects": [],
|
||||
"note": "Нет ЖК в pipeline 24мес в радиусе 5км — низкая будущая конкуренция",
|
||||
"note": (
|
||||
f"Нет ЖК в pipeline {PIPELINE_HORIZON_MONTHS}мес в радиусе "
|
||||
f"{PIPELINE_RADIUS_M // 1000}км — низкая будущая конкуренция"
|
||||
),
|
||||
}
|
||||
|
||||
by_class: dict[str, int] = {}
|
||||
|
|
@ -315,10 +331,10 @@ def _aggregate_pipeline(rows: list[Any]) -> dict[str, Any]:
|
|||
slot["objects"] += 1
|
||||
slot["flats"] += flats
|
||||
|
||||
# Severity (#36 acceptance — порог 500 / 3000):
|
||||
if flats_total < 500:
|
||||
# Severity (#36 acceptance)
|
||||
if flats_total < PIPELINE_SEVERITY_MEDIUM_THRESHOLD:
|
||||
severity = "low"
|
||||
elif flats_total < 3000:
|
||||
elif flats_total < PIPELINE_SEVERITY_HIGH_THRESHOLD:
|
||||
severity = "medium"
|
||||
else:
|
||||
severity = "high"
|
||||
|
|
@ -337,13 +353,15 @@ def _aggregate_pipeline(rows: list[Any]) -> dict[str, Any]:
|
|||
[dict(r) for r in rows],
|
||||
key=lambda r: r.get("flat_count") or 0,
|
||||
reverse=True,
|
||||
)[:10]
|
||||
)[:PIPELINE_TOP_OBJECTS_LIMIT]
|
||||
|
||||
# Serialize date for JSON
|
||||
# Serialize date for JSON; distance_m — explicit None check, не falsy,
|
||||
# потому что centroid-on-building даёт 0.0 (falsy) и raw Decimal упал бы
|
||||
# в JSON serialization.
|
||||
for obj in top_objects:
|
||||
if obj.get("ready_dt"):
|
||||
obj["ready_dt"] = obj["ready_dt"].isoformat()
|
||||
if obj.get("distance_m"):
|
||||
if obj.get("distance_m") is not None:
|
||||
obj["distance_m"] = round(float(obj["distance_m"]))
|
||||
|
||||
return {
|
||||
|
|
@ -354,8 +372,8 @@ def _aggregate_pipeline(rows: list[Any]) -> dict[str, Any]:
|
|||
"severity": severity,
|
||||
"severity_label": severity_label,
|
||||
"top_objects": top_objects,
|
||||
"radius_km": 5,
|
||||
"horizon_months": 24,
|
||||
"radius_km": PIPELINE_RADIUS_M // 1000,
|
||||
"horizon_months": PIPELINE_HORIZON_MONTHS,
|
||||
"note": (
|
||||
"Будущая конкуренция за покупателя: planned_commissioning от Росреестра "
|
||||
"часто оптимистичен (сдвиги по факту). Pressure-балл — относительный."
|
||||
|
|
@ -607,10 +625,12 @@ def analyze_parcel(
|
|||
|
||||
# 5b) D4 (#36): Pipeline 24mo — ЖК-конкуренты сдающиеся в горизонте 24 мес
|
||||
# в радиусе 5км. ready_dt = planned commissioning. Группируем по obj_class
|
||||
# + по кварталам сдачи.
|
||||
# + по кварталам сдачи. Константы — см. PIPELINE_* выше.
|
||||
# NB: full seq scan на ~3000 строк OK; при росте — нужен GIST/index на
|
||||
# (latitude, longitude) — отдельный issue для database-expert.
|
||||
pipeline_rows = (
|
||||
db.execute(
|
||||
text("""
|
||||
text(f"""
|
||||
WITH latest_obj AS (
|
||||
SELECT DISTINCT ON (obj_id) *
|
||||
FROM domrf_kn_objects
|
||||
|
|
@ -632,10 +652,10 @@ def analyze_parcel(
|
|||
WHERE ST_DWithin(
|
||||
ST_SetSRID(ST_MakePoint(o.longitude, o.latitude), 4326)::geography,
|
||||
ST_Centroid(ST_GeomFromText(:wkt, 4326))::geography,
|
||||
5000
|
||||
{PIPELINE_RADIUS_M}
|
||||
)
|
||||
AND ready_dt >= CURRENT_DATE
|
||||
AND ready_dt < CURRENT_DATE + INTERVAL '24 months'
|
||||
AND ready_dt < CURRENT_DATE + INTERVAL '{PIPELINE_HORIZON_MONTHS} months'
|
||||
ORDER BY ready_dt ASC
|
||||
"""),
|
||||
{"wkt": geom_wkt},
|
||||
|
|
|
|||
|
|
@ -28,9 +28,18 @@ const CLASS_LABEL: Record<string, string> = {
|
|||
standart: "Стандарт",
|
||||
standard: "Стандарт",
|
||||
unknown: "Не указан",
|
||||
null: "Не указан",
|
||||
// NB: JSON null приходит как absent key, не "null" string — отдельный
|
||||
// обработчик не нужен.
|
||||
};
|
||||
|
||||
function fmtMonth(iso: string | null): string {
|
||||
if (!iso) return "—";
|
||||
// Прочнее чем substring(0,7) — игнорирует timezone и datetime suffix
|
||||
const d = new Date(iso);
|
||||
if (Number.isNaN(d.getTime())) return iso.substring(0, 7);
|
||||
return d.toLocaleDateString("ru-RU", { year: "numeric", month: "2-digit" });
|
||||
}
|
||||
|
||||
function fmtClass(cls: string): string {
|
||||
return CLASS_LABEL[cls.toLowerCase()] ?? cls;
|
||||
}
|
||||
|
|
@ -362,7 +371,7 @@ export function Pipeline24moBlock({ data }: Props) {
|
|||
fontVariantNumeric: "tabular-nums",
|
||||
}}
|
||||
>
|
||||
{obj.ready_dt ? obj.ready_dt.substring(0, 7) : "—"}
|
||||
{fmtMonth(obj.ready_dt)}
|
||||
</td>
|
||||
</tr>
|
||||
))}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue