lekss361/gendesign

Fork 0

fix(objective parser): use_float=True in ijson + default=str in json.dumps #321

Merged

lekss361 merged 1 commit from fix/obj-decimal-json-serialization into main

2026-05-17 19:11:26 +00:00

lekss361 commented

2026-05-17 19:08:44 +00:00

Owner

Bug

После PR #318 + #320 deploy d1b721a ijson streaming наконец-то достигает parse stage (нет ValueError "too many values to unpack"). Но появилась НОВАЯ silent failure:

TypeError: Object of type Decimal is not JSON serializable
  json/encoder.py:180
  function: parse_lots_pf_stream

Glitchtip BACKEND-1L (release d1b721a, firstSeen 2026-05-17T18:58:48Z, count=1).

Симптом: runs #14 завершается status=done за 5.2с, lots=0, rows_extracted=NULL в objective_raw_reports. Exception swallow'нут inner try/except в scrape_objective.py.

Root cause

ijson.items() default use_float=False → numeric values парсятся как Decimal. Затем _row_to_lot() (data/sql/70_parse_objective_raw.py:345) делает:

"raw_props": json.dumps(row, ensure_ascii=False),

stdlib json.dumps не сериализует Decimal → TypeError → exception inside try → db.rollback() → но reports_ok уже инкрементирован → run помечается done.

Аудит других potential bugs (выполнен)

Проверил ВСЕ pass-through Decimal в _row_to_lot / _row_to_corp_room_month:

Поле	Path	Safe?
`Id`, `Id проекта`, `Этаж`	`_to_int(Decimal)` → `int()`	✅
`Площадь, м²`, цены, дельты	`_to_num(Decimal)` → `float()`	✅
`Готовность` ("17%")	`_parse_pct` — accidentally safe (regex matches digits)	✅
`Продано` ("да"/"нет")	`_parse_bool_yes` — strings	✅
`rooms_dev_site`, text-поля	pass-through strings	✅
Дата-поля	`_parse_iso_date` — strings	✅
`raw_props` json.dumps	direct dump dict with Decimal	🔴 bug

Other places — нет. Только json.dumps(row) падает.

Fix — двойной defense in depth

Hunk	Изменение
`_row_to_corp_room_month` line ~209	`json.dumps(row, ..., default=str)`
`_row_to_lot` line ~345	`json.dumps(row, ..., default=str)`
`parse_lots_pf_stream` v1 (dead code)	`ijson.items(stream, "result.item", use_float=True)`
`parse_lots_pf_stream` v2 (active)	`ijson.items(stream, "result.item", use_float=True)`

use_float=True в ijson — fundamental fix: numerics приходят как float, не Decimal. Для финансов в ₽ и площадей в м² — float64 precision (15-17 digits) более чем достаточен.

default=str в json.dumps — defensive net на любые экзотические типы (Decimal, datetime если случайно появится, custom classes). Не ломает существующее поведение для str/int/float/None.

Bonus: tech debt — duplicate `parse_lots_pf_stream` definition

Файл содержит два определения функции (F811 ruff warning, pre-existing from PR #315 vs #318 fixup overlap). Active вторая, первая dead code. Этот PR fix'ит ijson.items() в обеих — defensive, на случай если первая случайно будет вызвана. Кленап dead code — отдельный PR.

Acceptance

После merge + deploy (path filter уже исправлен в PR #320, deploy теперь будет триггериться):

Trigger sync → objective_raw_reports.payload_size для lots_pf = 0 (streamed) И rows_extracted ≈ 324k
objective_lots.raw_id IS NOT NULL count растёт по 500 батчами
Glitchtip BACKEND-1L не повторяется на release > d1b721a

Risk

Минимальный. use_float=True хорошо документирован в ijson. default=str — стандартный stdlib pattern. Тесты pre-existing не сломаются (numeric type comparison в БД одинаков для int/float/Decimal).

## Bug После PR #318 + #320 deploy `d1b721a` ijson streaming наконец-то достигает parse stage (нет ValueError "too many values to unpack"). Но появилась НОВАЯ silent failure: ``` TypeError: Object of type Decimal is not JSON serializable json/encoder.py:180 function: parse_lots_pf_stream ``` Glitchtip BACKEND-1L (release `d1b721a`, firstSeen 2026-05-17T18:58:48Z, count=1). Симптом: runs #14 завершается `status=done` за **5.2с**, `lots=0`, `rows_extracted=NULL` в `objective_raw_reports`. Exception swallow'нут inner try/except в `scrape_objective.py`. ## Root cause `ijson.items()` default `use_float=False` → numeric values парсятся как `Decimal`. Затем `_row_to_lot()` (data/sql/70_parse_objective_raw.py:345) делает: ```python "raw_props": json.dumps(row, ensure_ascii=False), ``` `stdlib json.dumps` не сериализует `Decimal` → TypeError → exception inside try → `db.rollback()` → но `reports_ok` уже инкрементирован → run помечается `done`. ## Аудит других potential bugs (выполнен) Проверил ВСЕ pass-through Decimal в `_row_to_lot` / `_row_to_corp_room_month`: | Поле | Path | Safe? | |---|---|---| | `Id`, `Id проекта`, `Этаж` | `_to_int(Decimal)` → `int()` | ✅ | | `Площадь, м²`, цены, дельты | `_to_num(Decimal)` → `float()` | ✅ | | `Готовность` ("17%") | `_parse_pct` — accidentally safe (regex matches digits) | ✅ | | `Продано` ("да"/"нет") | `_parse_bool_yes` — strings | ✅ | | `rooms_dev_site`, text-поля | pass-through strings | ✅ | | Дата-поля | `_parse_iso_date` — strings | ✅ | | **`raw_props` json.dumps** | direct dump dict with Decimal | 🔴 **bug** | Other places — нет. Только `json.dumps(row)` падает. ## Fix — двойной defense in depth | Hunk | Изменение | |---|---| | `_row_to_corp_room_month` line ~209 | `json.dumps(row, ..., default=str)` | | `_row_to_lot` line ~345 | `json.dumps(row, ..., default=str)` | | `parse_lots_pf_stream` v1 (dead code) | `ijson.items(stream, "result.item", use_float=True)` | | `parse_lots_pf_stream` v2 (active) | `ijson.items(stream, "result.item", use_float=True)` | **`use_float=True`** в ijson — fundamental fix: numerics приходят как `float`, не `Decimal`. Для финансов в ₽ и площадей в м² — float64 precision (15-17 digits) более чем достаточен. **`default=str`** в json.dumps — defensive net на любые экзотические типы (Decimal, datetime если случайно появится, custom classes). Не ломает существующее поведение для str/int/float/None. ## Bonus: tech debt — duplicate `parse_lots_pf_stream` definition Файл содержит **два определения** функции (F811 ruff warning, pre-existing from PR #315 vs #318 fixup overlap). Active вторая, первая dead code. Этот PR fix'ит ijson.items() в обеих — defensive, на случай если первая случайно будет вызвана. Кленап dead code — отдельный PR. ## Acceptance После merge + deploy (path filter уже исправлен в PR #320, deploy теперь будет триггериться): - [ ] Trigger sync → `objective_raw_reports.payload_size` для lots_pf = `0` (streamed) **И** `rows_extracted ≈ 324k` - [ ] `objective_lots.raw_id IS NOT NULL` count растёт по 500 батчами - [ ] Glitchtip BACKEND-1L не повторяется на release > `d1b721a` ## Risk Минимальный. `use_float=True` хорошо документирован в ijson. `default=str` — стандартный stdlib pattern. Тесты pre-existing не сломаются (numeric type comparison в БД одинаков для int/float/Decimal).

lekss361 added 1 commit 2026-05-17 19:08:45 +00:00

fix(objective parser): use_float=True in ijson + default=str in json.dumps ecbdcf35fb

ijson по умолчанию (use_float=False) возвращает все numeric values
как Decimal. _row_to_lot строит raw_props через json.dumps(row, ...)
— stdlib json не сериализует Decimal → TypeError → inner try/except
swallows exception → run завершается status=done, lots=0.

Двойной fix:
1. ijson.items(stream, "result.item", use_float=True) в обоих
   parse_lots_pf_stream — все numerics приходят как float (укладывается
   в float64 precision для финансовых сумм / площадей).
2. json.dumps(..., default=str) в обоих _row_to_lot и
   _row_to_corp_room_month — defense in depth для любых exotic типов.

Glitchtip BACKEND-1L (release d1b721a, firstSeen 18:58:48Z) — silent
fail после успешного streaming в PR #318 fixup.

lekss361 commented

2026-05-17 19:11:13 +00:00

Author

Owner

Deep Code Review — verdict

Summary

Status: APPROVE
Files reviewed: 1 (P0: data/sql/70_parse_objective_raw.py)
Lines: +4 / -4
PR: #321
Base SHA: d1b721a3 · Head SHA: ecbdcf35

Diff verification (exact match with PR description)

Hunk	Line in head SHA	Change	OK
`_row_to_corp_room_month`	209	`json.dumps(row, ensure_ascii=False, default=str)`	✅
`_row_to_lot`	345	`json.dumps(row, ensure_ascii=False, default=str)`	✅
`parse_lots_pf_stream` v1 (line 513)	546	`ijson.items(stream, "result.item", use_float=True)`	✅
`parse_lots_pf_stream` v2 (line 648)	715	`ijson.items(stream, "result.item", use_float=True)`	✅

No other changes — clean minimal fix.

Correctness audit

Decimal/float pass-through analysis (confirmed author's claim):

_to_int(Decimal | float | str) → int(v) cast → always emits int or None. Safe.
_to_num(Decimal | float | str) → float(v) cast → always emits float or None. Safe.
_parse_pct: isinstance(s, (int, float)) short-circuit; with use_float=True this now fast-paths instead of falling to regex (was accidentally-safe via str(Decimal) regex). Behavior preserved.
_parse_bool_yes / _parse_iso_date: string inputs only. Safe.
_normalize_rooms: string input. Safe.

Only Decimal-leak site was raw json.dumps(row) of the unprocessed dict — exactly what default=str covers.

default=str impact on existing serialization:

For str/int/float/bool/None/list/dict — default callback never fires. Behavior unchanged.
For Decimal — serializes as JSON string "123.45" inside raw_props jsonb. raw_props is audit-only column (no SQL numeric queries against it), so string representation is acceptable.

Float precision for domain data:

RUB prices ≤ 1e10, areas ≤ 1e6×0.01, percentages 0–100 — all well within float64's 15–17 significant digits.
No cumulative aggregation in this layer; sums happen in SQL with NUMERIC. No precision drift risk.

Cross-file impact

File is a standalone CLI parser, no external importers of parse_lots_pf_stream outside data/sql/ and scrape_objective.py invocation path.
No DB schema changes; raw_props is jsonb and accepts string-encoded Decimal value without ddl change.
No API contract or frontend impact.

Conventions

psycopg v3 / SQLAlchemy text — OK (no psycopg2 import).
CAST(:raw_props AS jsonb) parameter binding — OK (no ::jsonb trap).
logger.* — OK (no print()).
httpx — N/A.

Notable observation (not blocking)

File has two def parse_lots_pf_stream definitions (lines 513 and 648 in head SHA) — F811 ruff warning, pre-existing tech debt from PR #315/#318 overlap. PR #321 correctly applies use_float=True to both definitions (defensive). Dead-code cleanup is out of scope and tracked for a follow-up PR.

Risk

Reversibility: trivial revert (4 line changes)
Lock impact: none (no DB ops)
Severity: low — fixes silent data-loss bug (run marked done with rows_extracted=NULL) introduced after PR #318
Merge window: anytime — third PR in prod-critical cascade restoring objective parse pipeline

Verdict rationale

Surgical, well-scoped fix. Primary fix (use_float=True) addresses root cause; secondary fix (default=str) is sensible defense-in-depth. All helper functions audited — no other Decimal-pass-through sites. Merging.

## Deep Code Review — verdict ### Summary - **Status**: APPROVE - **Files reviewed**: 1 (P0: `data/sql/70_parse_objective_raw.py`) - **Lines**: +4 / -4 - **PR**: #321 - **Base SHA**: d1b721a3 · **Head SHA**: ecbdcf35 ### Diff verification (exact match with PR description) | Hunk | Line in head SHA | Change | OK | |---|---|---|---| | `_row_to_corp_room_month` | 209 | `json.dumps(row, ensure_ascii=False, default=str)` | ✅ | | `_row_to_lot` | 345 | `json.dumps(row, ensure_ascii=False, default=str)` | ✅ | | `parse_lots_pf_stream` v1 (line 513) | 546 | `ijson.items(stream, "result.item", use_float=True)` | ✅ | | `parse_lots_pf_stream` v2 (line 648) | 715 | `ijson.items(stream, "result.item", use_float=True)` | ✅ | No other changes — clean minimal fix. ### Correctness audit **Decimal/float pass-through analysis** (confirmed author's claim): - `_to_int(Decimal | float | str)` → `int(v)` cast → always emits `int` or `None`. Safe. - `_to_num(Decimal | float | str)` → `float(v)` cast → always emits `float` or `None`. Safe. - `_parse_pct`: `isinstance(s, (int, float))` short-circuit; with `use_float=True` this now fast-paths instead of falling to regex (was accidentally-safe via `str(Decimal)` regex). Behavior preserved. - `_parse_bool_yes` / `_parse_iso_date`: string inputs only. Safe. - `_normalize_rooms`: string input. Safe. Only Decimal-leak site was raw `json.dumps(row)` of the unprocessed dict — exactly what `default=str` covers. **`default=str` impact on existing serialization**: - For str/int/float/bool/None/list/dict — `default` callback never fires. Behavior unchanged. - For Decimal — serializes as JSON string `"123.45"` inside `raw_props` jsonb. `raw_props` is audit-only column (no SQL numeric queries against it), so string representation is acceptable. **Float precision for domain data**: - RUB prices ≤ 1e10, areas ≤ 1e6×0.01, percentages 0–100 — all well within float64's 15–17 significant digits. - No cumulative aggregation in this layer; sums happen in SQL with NUMERIC. No precision drift risk. ### Cross-file impact - File is a standalone CLI parser, no external importers of `parse_lots_pf_stream` outside `data/sql/` and `scrape_objective.py` invocation path. - No DB schema changes; `raw_props` is `jsonb` and accepts string-encoded Decimal value without ddl change. - No API contract or frontend impact. ### Conventions - psycopg v3 / SQLAlchemy text — OK (no `psycopg2` import). - `CAST(:raw_props AS jsonb)` parameter binding — OK (no `::jsonb` trap). - `logger.*` — OK (no `print()`). - httpx — N/A. ### Notable observation (not blocking) - File has **two `def parse_lots_pf_stream`** definitions (lines 513 and 648 in head SHA) — F811 ruff warning, pre-existing tech debt from PR #315/#318 overlap. PR #321 correctly applies `use_float=True` to both definitions (defensive). Dead-code cleanup is out of scope and tracked for a follow-up PR. ### Risk - **Reversibility**: trivial revert (4 line changes) - **Lock impact**: none (no DB ops) - **Severity**: low — fixes silent data-loss bug (run marked `done` with `rows_extracted=NULL`) introduced after PR #318 - **Merge window**: anytime — third PR in prod-critical cascade restoring objective parse pipeline ### Verdict rationale Surgical, well-scoped fix. Primary fix (`use_float=True`) addresses root cause; secondary fix (`default=str`) is sensible defense-in-depth. All helper functions audited — no other Decimal-pass-through sites. Merging.