Начальная реализация 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.
Независимый review: есть замечания, требующие исправления до слияния:
numberValue() приводит количества/цены/оценки к JavaScript number перед записью в PostgreSQL NUMERIC — возможна потеря точности. Нужно валидировать decimal-строки и передавать их без binary float.
Валидация неполная: body as PortfolioFile не проверяет объект, positions.instrument, даты concludedAt/reportedAt, порядок периода и допустимые значения side. TypeError/ошибки PostgreSQL проходят как 500 вместо 4xx.
Issue #24 требует ключ сделки по номеру сделки, а при его отсутствии — ISIN + дата/время + сторона + количество + сумма. Сейчас sourceId обязателен, fallback не реализован.
Нет тестов нового 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.
deriveTradeSourceId() вызывает trade.sourceId?.trim() без проверки типа. Payload с sourceId: 123 приводит к TypeError и HTTP 500 вместо validation 4xx; то же касается потенциально некорректных типов isin/прочих строковых полей. Проверь тип sourceId до вызова derive или сделай функцию безопасной.
Добавленный тест покрывает только 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 начальной реализации.
Третий 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), это ограничение среды, а не ошибка теста.
Финальный независимый 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 main2026-08-20 20:15:14 +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.
agent
Начальная реализация 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.
Независимый review: есть замечания, требующие исправления до слияния:
numberValue()приводит количества/цены/оценки к JavaScriptnumberперед записью в PostgreSQL NUMERIC — возможна потеря точности. Нужно валидировать decimal-строки и передавать их без binary float.body as PortfolioFileне проверяет объект, positions.instrument, датыconcludedAt/reportedAt, порядок периода и допустимые значения side. TypeError/ошибки PostgreSQL проходят как 500 вместо 4xx.sourceIdобязателен, fallback не реализован.После исправлений нужен повторный review полного diff.
Повторный review PR #33: основные замечания исправлены — decimal-значения сохраняются строками, добавлены проверки структуры/дат/позиций/сделок, fallback ключа и runnable validation test; shared/backend build проходят.
Осталось исправить до финального approve:
deriveTradeSourceId()вызываетtrade.sourceId?.trim()без проверки типа. Payload сsourceId: 123приводит к TypeError и HTTP 500 вместо validation 4xx; то же касается потенциально некорректных типовisin/прочих строковых полей. Проверь типsourceIdдо вызова derive или сделай функцию безопасной.deriveTradeSourceId/две проверки validatePortfolio, но не проверяет сам импорт: повторный POST, уникальность fallback-ключа, rollback при ошибке и сохранение decimal. Это оставляет главную идемпотентность без runnable regression test.После этих двух пунктов нужен ещё один короткий review. Остальные критерии issue #24 (котировки/UI/расчёт позиций) явно остаются следующим этапом, что соответствует заявленному scope начальной реализации.
Третий 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: блокирующих замечаний не осталось. Проверены исправления типов
sourceId/operationIdи unit assertion; предыдущие замечания по decimal precision, строгой валидации, fallback ключу и DB idempotency test закрыты. Shared/backend build иgit diff --checkпроходят. Portfolio validation test в текущем sandbox не запускается из-за ограниченияtsxIPC pipe (EPERM), но это ограничение среды. Можно сливать PR.