Добавить импорт портфеля брокерского отчёта #33

Merged
admin merged 4 commits from feature/portfolio-holdings into main 2026-08-20 20:15:14 +00:00
Collaborator

Начальная реализация issue #24: миграции portfolio_reports/positions/trades, импорт broker-portfolio-1.0 через POST /api/import/portfolio, идемпотентность по account+sourceId и снимки позиций. Денежные операции остаются в отдельном statement import и не смешиваются с портфельной аналитикой.

Версии: backend 0.8.0, shared 0.4.0.

Начальная реализация issue #24: миграции portfolio_reports/positions/trades, импорт broker-portfolio-1.0 через POST /api/import/portfolio, идемпотентность по account+sourceId и снимки позиций. Денежные операции остаются в отдельном statement import и не смешиваются с портфельной аналитикой. Версии: backend 0.8.0, shared 0.4.0.
agent added 1 commit 2026-08-20 11:56:08 +00:00
agent reviewed 2026-08-20 11:57:40 +00:00
agent left a comment
Author
Collaborator

Независимый review: есть замечания, требующие исправления до слияния:

  1. numberValue() приводит количества/цены/оценки к JavaScript number перед записью в PostgreSQL NUMERIC — возможна потеря точности. Нужно валидировать decimal-строки и передавать их без binary float.
  2. Валидация неполная: body as PortfolioFile не проверяет объект, positions.instrument, даты concludedAt/reportedAt, порядок периода и допустимые значения side. TypeError/ошибки PostgreSQL проходят как 500 вместо 4xx.
  3. Issue #24 требует ключ сделки по номеру сделки, а при его отсутствии — ISIN + дата/время + сторона + количество + сумма. Сейчас sourceId обязателен, fallback не реализован.
  4. Нет тестов нового endpoint/service: нужны проверки повторного импорта, повторного отчёта, частичных/нескольких сделок, invalid payload и rollback.

После исправлений нужен повторный review полного diff.

Независимый review: есть замечания, требующие исправления до слияния: 1. `numberValue()` приводит количества/цены/оценки к JavaScript `number` перед записью в PostgreSQL NUMERIC — возможна потеря точности. Нужно валидировать decimal-строки и передавать их без binary float. 2. Валидация неполная: `body as PortfolioFile` не проверяет объект, positions.instrument, даты `concludedAt`/`reportedAt`, порядок периода и допустимые значения side. TypeError/ошибки PostgreSQL проходят как 500 вместо 4xx. 3. Issue #24 требует ключ сделки по номеру сделки, а при его отсутствии — ISIN + дата/время + сторона + количество + сумма. Сейчас `sourceId` обязателен, fallback не реализован. 4. Нет тестов нового endpoint/service: нужны проверки повторного импорта, повторного отчёта, частичных/нескольких сделок, invalid payload и rollback. После исправлений нужен повторный review полного diff.
agent added 1 commit 2026-08-20 12:02:00 +00:00
agent added 1 commit 2026-08-20 12:02:55 +00:00
agent reviewed 2026-08-20 12:02:56 +00:00
agent left a comment
Author
Collaborator

Повторный review PR #33: основные замечания исправлены — decimal-значения сохраняются строками, добавлены проверки структуры/дат/позиций/сделок, fallback ключа и runnable validation test; shared/backend build проходят.

Осталось исправить до финального approve:

  1. deriveTradeSourceId() вызывает trade.sourceId?.trim() без проверки типа. Payload с sourceId: 123 приводит к TypeError и HTTP 500 вместо validation 4xx; то же касается потенциально некорректных типов isin/прочих строковых полей. Проверь тип sourceId до вызова derive или сделай функцию безопасной.
  2. Добавленный тест покрывает только deriveTradeSourceId/две проверки validatePortfolio, но не проверяет сам импорт: повторный POST, уникальность fallback-ключа, rollback при ошибке и сохранение decimal. Это оставляет главную идемпотентность без runnable regression test.

После этих двух пунктов нужен ещё один короткий review. Остальные критерии issue #24 (котировки/UI/расчёт позиций) явно остаются следующим этапом, что соответствует заявленному scope начальной реализации.

Повторный review PR #33: основные замечания исправлены — decimal-значения сохраняются строками, добавлены проверки структуры/дат/позиций/сделок, fallback ключа и runnable validation test; shared/backend build проходят. Осталось исправить до финального approve: 1. `deriveTradeSourceId()` вызывает `trade.sourceId?.trim()` без проверки типа. Payload с `sourceId: 123` приводит к TypeError и HTTP 500 вместо validation 4xx; то же касается потенциально некорректных типов `isin`/прочих строковых полей. Проверь тип `sourceId` до вызова derive или сделай функцию безопасной. 2. Добавленный тест покрывает только `deriveTradeSourceId`/две проверки validatePortfolio, но не проверяет сам импорт: повторный POST, уникальность fallback-ключа, rollback при ошибке и сохранение decimal. Это оставляет главную идемпотентность без runnable regression test. После этих двух пунктов нужен ещё один короткий review. Остальные критерии issue #24 (котировки/UI/расчёт позиций) явно остаются следующим этапом, что соответствует заявленному scope начальной реализации.
agent reviewed 2026-08-20 12:03:28 +00:00
agent left a comment
Author
Collaborator

Третий review PR #33: DB integration test повторного импорта/дедупликации добавлен, импорт получил injectable DB connector, версия и changelog обновлены; сборки shared/backend проходят. Основные замечания предыдущих review закрыты.

Остаётся один блокирующий edge case на trust boundary: deriveTradeSourceId() всё ещё делает trade.sourceId?.trim() без проверки типа. Payload с sourceId: 123 вызывает TypeError и попадает в 500 вместо 4xx validation response. Добавь проверку sourceId в validatePortfolio до вызова derive (или безопасное приведение/ошибку в derive). После этого можно считать review без блокирующих замечаний.

DB integration test локально не запускал: sandbox блокирует IPC pipe tsx (EPERM), это ограничение среды, а не ошибка теста.

Третий review PR #33: DB integration test повторного импорта/дедупликации добавлен, импорт получил injectable DB connector, версия и changelog обновлены; сборки shared/backend проходят. Основные замечания предыдущих review закрыты. Остаётся один блокирующий edge case на trust boundary: `deriveTradeSourceId()` всё ещё делает `trade.sourceId?.trim()` без проверки типа. Payload с `sourceId: 123` вызывает TypeError и попадает в 500 вместо 4xx validation response. Добавь проверку `sourceId` в `validatePortfolio` до вызова derive (или безопасное приведение/ошибку в derive). После этого можно считать review без блокирующих замечаний. DB integration test локально не запускал: sandbox блокирует IPC pipe `tsx` (EPERM), это ограничение среды, а не ошибка теста.
agent added 1 commit 2026-08-20 12:04:10 +00:00
agent reviewed 2026-08-20 12:04:41 +00:00
agent left a comment
Author
Collaborator

Финальный независимый review PR #33: блокирующих замечаний не осталось. Проверены исправления типов sourceId/operationId и unit assertion; предыдущие замечания по decimal precision, строгой валидации, fallback ключу и DB idempotency test закрыты. Shared/backend build и git diff --check проходят. Portfolio validation test в текущем sandbox не запускается из-за ограничения tsx IPC pipe (EPERM), но это ограничение среды. Можно сливать PR.

Финальный независимый review PR #33: блокирующих замечаний не осталось. Проверены исправления типов `sourceId`/`operationId` и unit assertion; предыдущие замечания по decimal precision, строгой валидации, fallback ключу и DB idempotency test закрыты. Shared/backend build и `git diff --check` проходят. Portfolio validation test в текущем sandbox не запускается из-за ограничения `tsx` IPC pipe (EPERM), но это ограничение среды. Можно сливать PR.
admin merged commit 4172b0c8e4 into main 2026-08-20 20:15:14 +00:00
Sign in to join this conversation.
No Reviewers
No Label
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: admin/family_budget#33