Независимый review PR #28: есть два блокирующих замечания.
scripts/convert_vtb_broker_xlsx.py:181-185: конвертер не читает closing balance из отчёта и формирует его как opening + сумма извлечённых строк. Поэтому пропущенная строка/изменившийся формат XLSX тихо дают «сходящийся» JSON с неверным балансом; проверка на строке 189 тавтологична. Нужно извлечь оба баланса из раздела отчёта и сравнить рассчитанный closing с заявленным, иначе импорт может закрепить неверные данные.
scripts/convert_vtb_broker_xlsx.py:107-115: sourceId для одинаковых операций получает суффикс occurrence по порядку строк. При повторном отчёте, где появилась ещё одна идентичная операция раньше существующих (или изменился порядок строк), старые sourceId сдвигаются, и импорт создаёт дубликаты вместо идемпотентности. Нужен идентификатор, привязанный к исходной строке/стабильным полям отчёта (для денежных движений — source row или комбинация с ним), а не глобальный порядковый номер.
Проверки: backend/shared/frontend build и py_compile проходят. До исправления замечаний PR не рекомендую к merge.
Независимый review PR #28: есть два блокирующих замечания.
1. `scripts/convert_vtb_broker_xlsx.py:181-185`: конвертер не читает closing balance из отчёта и формирует его как `opening + сумма извлечённых строк`. Поэтому пропущенная строка/изменившийся формат XLSX тихо дают «сходящийся» JSON с неверным балансом; проверка на строке 189 тавтологична. Нужно извлечь оба баланса из раздела отчёта и сравнить рассчитанный closing с заявленным, иначе импорт может закрепить неверные данные.
2. `scripts/convert_vtb_broker_xlsx.py:107-115`: `sourceId` для одинаковых операций получает суффикс occurrence по порядку строк. При повторном отчёте, где появилась ещё одна идентичная операция раньше существующих (или изменился порядок строк), старые sourceId сдвигаются, и импорт создаёт дубликаты вместо идемпотентности. Нужен идентификатор, привязанный к исходной строке/стабильным полям отчёта (для денежных движений — source row или комбинация с ним), а не глобальный порядковый номер.
Проверки: backend/shared/frontend build и `py_compile` проходят. До исправления замечаний PR не рекомендую к merge.
Балансный блокер исправлен: теперь closing balance берётся из AF строки отчёта и сверяется с opening + суммой операций.
Блокирующее замечание остаётся по scripts/convert_vtb_broker_xlsx.py:107-115: sourceId одинаковых денежных операций строится как digest + occurrences[digest], то есть суффикс зависит от порядка и количества повторяющихся строк. Если в новом отчёте добавится ещё одна идентичная операция перед уже импортированными (либо изменится порядок), sourceId прежних операций сдвинутся, а backend-фингерпринты станут новыми — повторный импорт создаст дубликаты. Для заявленной идемпотентности нужен идентификатор, привязанный к исходной строке/стабильному полю отчёта, а не порядковый occurrence.
Проверки после commit: backend/shared/frontend build и py_compile проходят. PR всё ещё не рекомендую к merge до исправления sourceId.
Повторный review после commit 3d08c9e.
Балансный блокер исправлен: теперь closing balance берётся из AF строки отчёта и сверяется с opening + суммой операций.
Блокирующее замечание остаётся по `scripts/convert_vtb_broker_xlsx.py:107-115`: sourceId одинаковых денежных операций строится как `digest + occurrences[digest]`, то есть суффикс зависит от порядка и количества повторяющихся строк. Если в новом отчёте добавится ещё одна идентичная операция перед уже импортированными (либо изменится порядок), sourceId прежних операций сдвинутся, а backend-фингерпринты станут новыми — повторный импорт создаст дубликаты. Для заявленной идемпотентности нужен идентификатор, привязанный к исходной строке/стабильному полю отчёта, а не порядковый occurrence.
Проверки после commit: backend/shared/frontend build и `py_compile` проходят. PR всё ещё не рекомендую к merge до исправления sourceId.
Полный повторный review текущего PR после добавления UUIDv5/миграции.
Предыдущий sourceId-блокер устранён для конвертера: UUIDv5 детерминирован от sourceId, а occurrence остаётся единственным различителем неразличимых строк XLSX (ограничение источника). Балансная проверка также остаётся корректной.
Остаётся один блокирующий edge case на trust boundary: validateStructure() проверяет формат operationId, но не проверяет уникальность operationId внутри одного JSON. При двух транзакциях с одинаковым operationId и разными fingerprint ux_transactions_account_operation_id вызывает unique-violation; импорт откатывается с 500 вместо предсказуемой 4xx-валидации/дедупликации. Добавьте проверку duplicate operationId в validateSemantics (или явно обрабатывайте конфликт по operation_id).
Миграция 006 корректно добавляет UUID NOT NULL DEFAULT и уникальный индекс, включая существующие строки; порядок 006 перед исторической 005 функционально безопасен, хотя хронологически неидеален. Backend/shared/frontend build и py_compile проходят. До обработки duplicate operationId PR не рекомендую к merge.
Полный повторный review текущего PR после добавления UUIDv5/миграции.
Предыдущий sourceId-блокер устранён для конвертера: UUIDv5 детерминирован от sourceId, а occurrence остаётся единственным различителем неразличимых строк XLSX (ограничение источника). Балансная проверка также остаётся корректной.
Остаётся один блокирующий edge case на trust boundary: validateStructure() проверяет формат operationId, но не проверяет уникальность operationId внутри одного JSON. При двух транзакциях с одинаковым operationId и разными fingerprint ux_transactions_account_operation_id вызывает unique-violation; импорт откатывается с 500 вместо предсказуемой 4xx-валидации/дедупликации. Добавьте проверку duplicate operationId в validateSemantics (или явно обрабатывайте конфликт по operation_id).
Миграция 006 корректно добавляет UUID NOT NULL DEFAULT и уникальный индекс, включая существующие строки; порядок 006 перед исторической 005 функционально безопасен, хотя хронологически неидеален. Backend/shared/frontend build и py_compile проходят. До обработки duplicate operationId PR не рекомендую к merge.
Проверка duplicate operationId внутри одного JSON теперь корректно возвращает 422; предыдущий блокер устранён. UUIDv5 конвертера, балансная сверка, миграция и версии/changelog выглядят согласованно. Backend build, analytics test, shared/frontend build и py_compile проходят.
Остаётся блокирующий сценарий дедупликации между импортами: уникальный индекс (account_id, operation_id) добавлен, но INSERT использует ON CONFLICT (account_id, fingerprint). Если повторный источник прислал ту же стабильную operationId с исправленным amount/description (то есть новым fingerprint), импорт получает DB unique-violation и откатывается с 500. При введённом operationId он должен быть дедупликационным ключом: либо ON CONFLICT (account_id, operation_id) DO NOTHING, либо явная 4xx/согласованная обработка конфликта. До этого сценария PR не рекомендую к merge.
Финальный повторный review после b99555c.
Проверка duplicate operationId внутри одного JSON теперь корректно возвращает 422; предыдущий блокер устранён. UUIDv5 конвертера, балансная сверка, миграция и версии/changelog выглядят согласованно. Backend build, analytics test, shared/frontend build и py_compile проходят.
Остаётся блокирующий сценарий дедупликации между импортами: уникальный индекс `(account_id, operation_id)` добавлен, но INSERT использует `ON CONFLICT (account_id, fingerprint)`. Если повторный источник прислал ту же стабильную operationId с исправленным amount/description (то есть новым fingerprint), импорт получает DB unique-violation и откатывается с 500. При введённом operationId он должен быть дедупликационным ключом: либо `ON CONFLICT (account_id, operation_id) DO NOTHING`, либо явная 4xx/согласованная обработка конфликта. До этого сценария PR не рекомендую к merge.
Финальный review PR #28 после 3af9e9d: блокирующих замечаний нет.
Проверено: UUIDv5/sourceId конвертера, сверка opening/closing balance, JSON backward compatibility, duplicate operationId validation, дедупликация по fingerprint и operation_id через ON CONFLICT DO NOTHING, migration 006 и версии/changelog. Конфликт стабильного operationId между импортами теперь безопасно считается дублем.
Проверки прошли: backend build, analytics test, shared/frontend build, py_compile конвертера. Ограничение XLSX без broker operation ID (occurrence для полностью неразличимых строк) явно следует из источника и приемлемо.
Финальный review PR #28 после 3af9e9d: блокирующих замечаний нет.
Проверено: UUIDv5/sourceId конвертера, сверка opening/closing balance, JSON backward compatibility, duplicate operationId validation, дедупликация по fingerprint и operation_id через ON CONFLICT DO NOTHING, migration 006 и версии/changelog. Конфликт стабильного operationId между импортами теперь безопасно считается дублем.
Проверки прошли: backend build, analytics test, shared/frontend build, py_compile конвертера. Ограничение XLSX без broker operation ID (occurrence для полностью неразличимых строк) явно следует из источника и приемлемо.
admin
merged commit 2422ea88fd into main2026-08-19 15:01:36 +00:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Конвертер отчёта ВТБ XLSX: JSON движения денежных средств, sidecar портфеля/сделок и стабильные sourceId для идемпотентного повторного импорта. Расширен fingerprint импорта.
Независимый review PR #28: есть два блокирующих замечания.
scripts/convert_vtb_broker_xlsx.py:181-185: конвертер не читает closing balance из отчёта и формирует его какopening + сумма извлечённых строк. Поэтому пропущенная строка/изменившийся формат XLSX тихо дают «сходящийся» JSON с неверным балансом; проверка на строке 189 тавтологична. Нужно извлечь оба баланса из раздела отчёта и сравнить рассчитанный closing с заявленным, иначе импорт может закрепить неверные данные.scripts/convert_vtb_broker_xlsx.py:107-115:sourceIdдля одинаковых операций получает суффикс occurrence по порядку строк. При повторном отчёте, где появилась ещё одна идентичная операция раньше существующих (или изменился порядок строк), старые sourceId сдвигаются, и импорт создаёт дубликаты вместо идемпотентности. Нужен идентификатор, привязанный к исходной строке/стабильным полям отчёта (для денежных движений — source row или комбинация с ним), а не глобальный порядковый номер.Проверки: backend/shared/frontend build и
py_compileпроходят. До исправления замечаний PR не рекомендую к merge.Повторный review после commit
3d08c9e.Балансный блокер исправлен: теперь closing balance берётся из AF строки отчёта и сверяется с opening + суммой операций.
Блокирующее замечание остаётся по
scripts/convert_vtb_broker_xlsx.py:107-115: sourceId одинаковых денежных операций строится какdigest + occurrences[digest], то есть суффикс зависит от порядка и количества повторяющихся строк. Если в новом отчёте добавится ещё одна идентичная операция перед уже импортированными (либо изменится порядок), sourceId прежних операций сдвинутся, а backend-фингерпринты станут новыми — повторный импорт создаст дубликаты. Для заявленной идемпотентности нужен идентификатор, привязанный к исходной строке/стабильному полю отчёта, а не порядковый occurrence.Проверки после commit: backend/shared/frontend build и
py_compileпроходят. PR всё ещё не рекомендую к merge до исправления sourceId.Полный повторный review текущего PR после добавления UUIDv5/миграции.
Предыдущий sourceId-блокер устранён для конвертера: UUIDv5 детерминирован от sourceId, а occurrence остаётся единственным различителем неразличимых строк XLSX (ограничение источника). Балансная проверка также остаётся корректной.
Остаётся один блокирующий edge case на trust boundary: validateStructure() проверяет формат operationId, но не проверяет уникальность operationId внутри одного JSON. При двух транзакциях с одинаковым operationId и разными fingerprint ux_transactions_account_operation_id вызывает unique-violation; импорт откатывается с 500 вместо предсказуемой 4xx-валидации/дедупликации. Добавьте проверку duplicate operationId в validateSemantics (или явно обрабатывайте конфликт по operation_id).
Миграция 006 корректно добавляет UUID NOT NULL DEFAULT и уникальный индекс, включая существующие строки; порядок 006 перед исторической 005 функционально безопасен, хотя хронологически неидеален. Backend/shared/frontend build и py_compile проходят. До обработки duplicate operationId PR не рекомендую к merge.
Финальный повторный review после
b99555c.Проверка duplicate operationId внутри одного JSON теперь корректно возвращает 422; предыдущий блокер устранён. UUIDv5 конвертера, балансная сверка, миграция и версии/changelog выглядят согласованно. Backend build, analytics test, shared/frontend build и py_compile проходят.
Остаётся блокирующий сценарий дедупликации между импортами: уникальный индекс
(account_id, operation_id)добавлен, но INSERT используетON CONFLICT (account_id, fingerprint). Если повторный источник прислал ту же стабильную operationId с исправленным amount/description (то есть новым fingerprint), импорт получает DB unique-violation и откатывается с 500. При введённом operationId он должен быть дедупликационным ключом: либоON CONFLICT (account_id, operation_id) DO NOTHING, либо явная 4xx/согласованная обработка конфликта. До этого сценария PR не рекомендую к merge.Финальный review PR #28 после
3af9e9d: блокирующих замечаний нет.Проверено: UUIDv5/sourceId конвертера, сверка opening/closing balance, JSON backward compatibility, duplicate operationId validation, дедупликация по fingerprint и operation_id через ON CONFLICT DO NOTHING, migration 006 и версии/changelog. Конфликт стабильного operationId между импортами теперь безопасно считается дублем.
Проверки прошли: backend build, analytics test, shared/frontend build, py_compile конвертера. Ограничение XLSX без broker operation ID (occurrence для полностью неразличимых строк) явно следует из источника и приемлемо.