RSHB/arm-core
9
0
Fork 0
Code Issues 26 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

feat/corrections-back #208

Merged
aep merged 12 commits from feat/corrections-back into main 2026-11-02 19:35:33 +03:00
Conversation 2 Commits 12 Files Changed 50 +4568 -32
tsygankoviva commented 2026-10-29 19:17:55 +03:00
Owner
Copy Link

Нужен расчет соответствующих правил для ОСВ из задачи AURORA-1751 для построения ручки таблицы корректировок и проверок при заведении корректировки

Нужен расчет соответствующих правил для ОСВ из задачи AURORA-1751 для построения ручки таблицы корректировок и проверок при заведении корректировки
tsygankoviva added 2 commits 2026-10-29 19:17:58 +03:00
tsygankoviva added 3 commits 2026-10-30 19:44:44 +03:00 
# Conflicts:
#	api/src/api/dependencies/consolidations.py
tsygankoviva requested review from PotapovaA 2026-10-30 19:45:12 +03:00
tsygankoviva requested review from aep 2026-10-30 19:45:12 +03:00
tsygankoviva changed title from WIP: feat/corrections-back to feat/corrections-back 2026-10-30 19:45:16 +03:00
aep added 1 commit 2026-11-02 11:19:26 +03:00 
- README: вернуть потерянный абзац-предупреждение про e2e и queue group
  (был затёрт вставкой нового раздела); описание раздела про корректировки
  поправлено под архитектуру CorrectionAggregateSource (1540 не читает
  vgo_assignment напрямую, см. design.md Decision 1).
- design.md: Non-Goals ложно утверждал "DBML/ORM/миграция не меняются",
  хотя в ветке есть миграция 0011 (vgo_assignment, AURORA-1751); уточнена
  граница ответственности и добавлено пояснение в Migration Plan.
- tasks.md: зафиксировано, что make dbml-check не выполнялся — DBML v5.24
  ещё не существует в arm-ka-docs, вопрос поднимается отдельно.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
aep commented 2026-11-02 11:20:58 +03:00
Owner
Copy Link

Ревью, раунд 1.

README и OpenSpec (design.md/tasks.md) поправил сам прямо в ветке (7384471): вернул потерянный абзац про e2e/queue group, убрал из Non-Goals ложное «DBML/ORM/миграция не меняются» (миграция 0011 в ветке есть), отметил в tasks.md, что make dbml-check не выполнялся — DBML v5.24 ещё не существует в arm-ka-docs, этот вопрос подниму отдельно. Сюда не относится, можно не трогать.

По коду — не готово. Разношу находки по тому, чей это код, а не по тому, в чьём PR они лежат: часть — из ветки AURORA-1751 (Аня, коммиты 7194a55 + 9bae931, Илья их влил себе df892d1), часть — из AURORA-1540 (Илья, 086050c/0e678ac). Проверил по git log на оба файла каждой находки — ниже без пересечений.

Аня, AURORA-1751

C-1. Одна строка ОСВ попадает в агрегаты двух правил. services/vgo.py:144-162, _select_diff_aggregates. Второй цикл принимает наборы entry_ids по одному варианту, но не сверяется с уже принятыми в этом же цикле (used_ids.isdisjoint проверяется один раз, до всех update). Воспроизвёл: правило 6 → {A.76, B.60}, правило 7 → {A.76, B.76}, оба агрегата принимаются, строка A.76 учтена дважды. Прямое нарушение приёмочного критерия 1751 «ни одна строка не в двух правилах». Тест на пересечение кандидатов (не на совпадающие наборы, как сейчас test_pairs_are_grouped_and_entry_ids_are_used_once) нужен обязательно.

C-2. Разнесение считает не то, что утверждено 19–21.10. _find_aggregates/_build_groups, там же. Два расхождения с 1751 (проверил по справочнику правил и по domain-rules, без допущений):

  • Сумма агрегата — это сумма упакованных пар, а не всех строк компании под правилом, хотя правильная сумма уже посчитана рядом и выброшена: build_pairs (intercompany_matrix.py:599-600) возвращает amount_a/amount_b из _rule_totals (:458-463) — по всем строкам счетов правила, независимо от пакинга. Это прямое следствие решения владельца 19.10 («способ упаковки сумму элиминации не меняет», docs/context/05-domain-rules.md:688-689) и уже работает для шахматки. _find_aggregates (vgo.py:236-245) эту сумму получает и не использует, _build_groups (vgo.py:183-186) пересчитывает заново только по matched+with_discrepancy. Просто подставить готовую сумму тоже не выйдет: _find_aggregates гоняет build_pairs по free_rows (строки, не занятые предыдущими итерациями по rule×company_a×company_b), значит даже готовая сумма в каждом отдельном вызове может быть занижена порядком перебора — нужна сначала фаза «строка → ровно одно правило» (C-1/C-3), потом сумма по уже закреплённому набору.
  • Шаг «без пары, подходит под одно правило → в это правило» не реализован вовсе, ни для одного счёта. Для большинства счетов это и правда не баг — ты права, что, например, 60.02/60.22/60.32/62.01/62.03/62.21/62.31/76 общие для правил 6/7/8 (проверил в elimination_rules_v1.yaml), без встречной строки правило не определить. Но есть счета без такого пересечения: 01, 03, 58.01 (только правило 2, сторона А), 80/83 (только правило 2, сторона Б), сторона Б правила 6 (60.01, 60.03, 60.21, 60.31, 62.02, 62.22, 62.32) — они ни в одной другой маске не встречаются, значит однозначны без всякой пары. Для них шаг спецификации всё равно нужен, и его нет.
  • (Третье, менее спорное: при пересечении кандидатов правило с меньшим номером не выигрывает — результат зависит от порядка вставки участников в dict, а не от rule_no.)

Нужна отдельная чистая функция в domain/ по шагам 1–5 контракта (строка × сторона как позиция; для суммы можно переиспользовать _rule_totals/build_pairs.amount_a, если считать по полному своду, а не по free_rows), pack_rule_rows оставить только для детализации pairs внутри уже определённого агрегата.

C-3. Ручного переноса строки между правилами нет вообще. Миграция vgo_assignment есть, а писать в неё нечем: нет POST .../allocations, нет audit_action для решения по строке, нет проверки VERIFIED. Наложение уже существующих ручных решений поверх автоматики сделано хорошо (assignment_candidates исключает строки с решением из free_rows), но проверено только моками list_assignments.

Important:

  • I-4 0011_dbml_v5_24.py:31,38 (это твоя таблица, Илья только переименовал файл при слиянии) — FK osv_entry_id без ON DELETE, упадёт при удалении файла/перемаппинге, как только появится первая ручная запись.
  • I-9 vgo.py:96-101 — склеенная строка с частичным ручным решением валит весь GET 422-й на всю версию вместо деградации в нераспределённые с причиной.

Minor: двойной префикс имени CHECK-constraint в БД (ck_vgo_assignment_ck_vgo_assignment_* — имя в миграции и в naming convention ORM models/kfo.py складываются; тест это маскирует через .replace); POST /vgo-assignments/distribute — по сути чтение, должен быть GET с CONSOLIDATION_VIEW, наружу торчат внутренние поля хранилища; протечка строк вне правил в unassigned (_unassigned_rows); I-2 снимаю — vgo.py:63-73 (_manual_key_amount) для measure: turnover теряет одну сторону при одновременном Д/К, но сейчас все правила в elimination_rules_v1.yaml на measure: closing, а по closing на одном субсчёте дубля не бывает (спасибо Игорю за уточнение). Актуально станет только с ОФР-элиминацией по оборотам (AURORA-1374) — не чинить сейчас, просто держать в уме.

Илья, AURORA-1540

I-1 correction_source.py:27,165-172 + repositories/corrections.py:17-35 — сам разрешаешь side и держишь мёртвую копию схемы vgo_assignment, хотя по #188 это дело 1751 — жди уже разрешённую сторону из функции разнесения, когда Аня её отдаст.
I-5 .env.test:14 — порт 5431 вместо стандартных 5432, похоже на локальную настройку, у остальных интеграционные тесты пропустятся/упадут.
I-6 schemas/corrections.py:37-43,88-120 — нераспределённые строки и candidate_rule_ids/decision не доходят до контракта, фронт (#206) этого не получит.
I-7 corrections_view.py:31-34 — APPLIED-корректировка «осиротевшего» агрегата молча пропускается (continue), а не помечается.
I-8 correction_source.py:69,98-99 + corrections.py:143-206 — на PUT до 4 полных пересборок свода и 2 полных прогона разнесения Ани (она сама по себе не из дешёвых — knapsack + весь план счетов на каждый вызов build_pairs). Кешируй свод и результат разнесения на запрос.
I-11 — нет пометки КФО как устаревшего после сохранения корректировки (AURORA-1417), заведи follow-up в Jira если не входит в этот этап.
Minor: порог ровно 5% (domain/corrections.py:32, контракт говорит «от 5% включительно» — открытый вопрос, не твоё решение, просто имей в виду).

Интеграционные тесты и migration-cycle я не гонял — порт 5431 не даёт (I-5), посмотрите сами на стандартном compose перед следующим раундом.

Отдельный вопрос, не жду решения прямо сейчас: C-2 + C-3 — это фактически весь объём 1751, сейчас всё в одной ветке/PR на 1540+1751. Смотрите сами между собой: либо Аня дорабатывает 1751 здесь же, либо 1540 (почти готов после I-1/I-6/I-7) едет отдельно поверх текущего VgoService с пометкой «разнесение предварительное», а 1751 — отдельным PR со своим OpenSpec change.

Ревью, раунд 1. README и OpenSpec (design.md/tasks.md) поправил сам прямо в ветке (`7384471`): вернул потерянный абзац про e2e/queue group, убрал из Non-Goals ложное «DBML/ORM/миграция не меняются» (миграция 0011 в ветке есть), отметил в tasks.md, что `make dbml-check` не выполнялся — DBML v5.24 ещё не существует в arm-ka-docs, этот вопрос подниму отдельно. Сюда не относится, можно не трогать. По коду — не готово. Разношу находки по тому, чей это код, а не по тому, в чьём PR они лежат: часть — из ветки `AURORA-1751` (Аня, коммиты `7194a55` + `9bae931`, Илья их влил себе `df892d1`), часть — из `AURORA-1540` (Илья, `086050c`/`0e678ac`). Проверил по `git log` на оба файла каждой находки — ниже без пересечений. ### Аня, AURORA-1751 **C-1. Одна строка ОСВ попадает в агрегаты двух правил.** `services/vgo.py:144-162`, `_select_diff_aggregates`. Второй цикл принимает наборы `entry_ids` по одному варианту, но не сверяется с уже принятыми в этом же цикле (`used_ids.isdisjoint` проверяется один раз, до всех `update`). Воспроизвёл: правило 6 → {A.76, B.60}, правило 7 → {A.76, B.76}, оба агрегата принимаются, строка A.76 учтена дважды. Прямое нарушение приёмочного критерия 1751 «ни одна строка не в двух правилах». Тест на пересечение кандидатов (не на совпадающие наборы, как сейчас `test_pairs_are_grouped_and_entry_ids_are_used_once`) нужен обязательно. **C-2. Разнесение считает не то, что утверждено 19–21.10.** `_find_aggregates`/`_build_groups`, там же. Два расхождения с 1751 (проверил по справочнику правил и по domain-rules, без допущений): - **Сумма агрегата — это сумма упакованных пар, а не всех строк компании под правилом**, хотя правильная сумма уже посчитана рядом и выброшена: `build_pairs` (`intercompany_matrix.py:599-600`) возвращает `amount_a`/`amount_b` из `_rule_totals` (`:458-463`) — по всем строкам счетов правила, независимо от пакинга. Это прямое следствие решения владельца 19.10 («способ упаковки сумму элиминации не меняет», `docs/context/05-domain-rules.md:688-689`) и уже работает для шахматки. `_find_aggregates` (vgo.py:236-245) эту сумму получает и не использует, `_build_groups` (vgo.py:183-186) пересчитывает заново только по `matched`+`with_discrepancy`. Просто подставить готовую сумму тоже не выйдет: `_find_aggregates` гоняет `build_pairs` по `free_rows` (строки, не занятые предыдущими итерациями по rule×company_a×company_b), значит даже готовая сумма в каждом отдельном вызове может быть занижена порядком перебора — нужна сначала фаза «строка → ровно одно правило» (C-1/C-3), потом сумма по уже закреплённому набору. - **Шаг «без пары, подходит под одно правило → в это правило» не реализован вовсе, ни для одного счёта.** Для большинства счетов это и правда не баг — ты права, что, например, 60.02/60.22/60.32/62.01/62.03/62.21/62.31/76 общие для правил 6/7/8 (проверил в `elimination_rules_v1.yaml`), без встречной строки правило не определить. Но есть счета без такого пересечения: `01`, `03`, `58.01` (только правило 2, сторона А), `80`/`83` (только правило 2, сторона Б), сторона Б правила 6 (`60.01`, `60.03`, `60.21`, `60.31`, `62.02`, `62.22`, `62.32`) — они ни в одной другой маске не встречаются, значит однозначны без всякой пары. Для них шаг спецификации всё равно нужен, и его нет. - (Третье, менее спорное: при пересечении кандидатов правило с меньшим номером не выигрывает — результат зависит от порядка вставки участников в dict, а не от `rule_no`.) Нужна отдельная чистая функция в `domain/` по шагам 1–5 контракта (строка × сторона как позиция; для суммы можно переиспользовать `_rule_totals`/`build_pairs.amount_a`, если считать по полному своду, а не по `free_rows`), `pack_rule_rows` оставить только для детализации `pairs` внутри уже определённого агрегата. **C-3. Ручного переноса строки между правилами нет вообще.** Миграция `vgo_assignment` есть, а писать в неё нечем: нет `POST .../allocations`, нет `audit_action` для решения по строке, нет проверки VERIFIED. Наложение уже существующих ручных решений поверх автоматики сделано хорошо (`assignment_candidates` исключает строки с решением из `free_rows`), но проверено только моками `list_assignments`. Important: - I-4 `0011_dbml_v5_24.py:31,38` (это твоя таблица, Илья только переименовал файл при слиянии) — FK `osv_entry_id` без `ON DELETE`, упадёт при удалении файла/перемаппинге, как только появится первая ручная запись. - I-9 `vgo.py:96-101` — склеенная строка с частичным ручным решением валит весь GET 422-й на всю версию вместо деградации в нераспределённые с причиной. Minor: двойной префикс имени CHECK-constraint в БД (`ck_vgo_assignment_ck_vgo_assignment_*` — имя в миграции и в naming convention ORM `models/kfo.py` складываются; тест это маскирует через `.replace`); `POST /vgo-assignments/distribute` — по сути чтение, должен быть GET с `CONSOLIDATION_VIEW`, наружу торчат внутренние поля хранилища; протечка строк вне правил в `unassigned` (`_unassigned_rows`); **I-2 снимаю** — `vgo.py:63-73` (`_manual_key_amount`) для `measure: turnover` теряет одну сторону при одновременном Д/К, но сейчас все правила в `elimination_rules_v1.yaml` на `measure: closing`, а по closing на одном субсчёте дубля не бывает (спасибо Игорю за уточнение). Актуально станет только с ОФР-элиминацией по оборотам (AURORA-1374) — не чинить сейчас, просто держать в уме. ### Илья, AURORA-1540 **I-1** `correction_source.py:27,165-172` + `repositories/corrections.py:17-35` — сам разрешаешь `side` и держишь мёртвую копию схемы `vgo_assignment`, хотя по #188 это дело 1751 — жди уже разрешённую сторону из функции разнесения, когда Аня её отдаст. **I-5** `.env.test:14` — порт 5431 вместо стандартных 5432, похоже на локальную настройку, у остальных интеграционные тесты пропустятся/упадут. **I-6** `schemas/corrections.py:37-43,88-120` — нераспределённые строки и `candidate_rule_ids`/`decision` не доходят до контракта, фронт (#206) этого не получит. **I-7** `corrections_view.py:31-34` — APPLIED-корректировка «осиротевшего» агрегата молча пропускается (`continue`), а не помечается. **I-8** `correction_source.py:69,98-99` + `corrections.py:143-206` — на PUT до 4 полных пересборок свода и 2 полных прогона разнесения Ани (она сама по себе не из дешёвых — knapsack + весь план счетов на каждый вызов `build_pairs`). Кешируй свод и результат разнесения на запрос. **I-11** — нет пометки КФО как устаревшего после сохранения корректировки (AURORA-1417), заведи follow-up в Jira если не входит в этот этап. Minor: порог ровно 5% (`domain/corrections.py:32`, контракт говорит «от 5% включительно» — открытый вопрос, не твоё решение, просто имей в виду). Интеграционные тесты и `migration-cycle` я не гонял — порт 5431 не даёт (I-5), посмотрите сами на стандартном compose перед следующим раундом. Отдельный вопрос, не жду решения прямо сейчас: C-2 + C-3 — это фактически весь объём 1751, сейчас всё в одной ветке/PR на 1540+1751. Смотрите сами между собой: либо Аня дорабатывает 1751 здесь же, либо 1540 (почти готов после I-1/I-6/I-7) едет отдельно поверх текущего `VgoService` с пометкой «разнесение предварительное», а 1751 — отдельным PR со своим OpenSpec change.
tsygankoviva added 4 commits 2026-11-02 18:13:09 +03:00 
# Conflicts:
#	api/src/api/dependencies/consolidations.py
#	api/src/services/vgo.py
aep added 2 commits 2026-11-02 19:18:01 +03:00 
Был 5431 вместо 5432 из docs/dev/docker-compose.yml, как и утверждает
комментарий в шапке файла — интеграционные тесты не подключались к
стенду, поднятому make up.
attach_corrections молча пропускала (continue) корректировку, чей
агрегат (rule_id, пара ЮЛ) больше не существует после пересчёта
разнесения ВГО — вместо этого, как и в остальных ветках функции,
поднимаем ConflictError("CORRECTION_REVIEW_REQUIRED").
aep commented 2026-11-02 19:35:09 +03:00
Owner
Copy Link

Раунд 2.

C-1/C-2/C-3 закрыты: select_exact_aggregates обновляет used инкрементально внутри цикла, появилась чистая domain/vgo_distribution.py по шагам контракта, ручной перенос — PUT/DELETE vgo-assignments/rule + exclude с lock_editable_version и аудитом. I-1 (сторона из разнесения, не своя), I-6 (candidate_rule_ids/decision в контракте), I-8 (кеш таблицы на запрос, тест test_corrections_review_fixes.py) — тоже на месте.

I-5 и I-7 доделал сам прямо в ветке: порт в api/.env.test (948724c), осиротевшая APPLIED-корректировка теперь CORRECTION_REVIEW_REQUIRED вместо молчаливого continue, с тестом (d542a22).

I-11 — не трогаю, не входит в этап: AURORA-1417 уже заведена, kfo_result появится только в Срезе 6.

test-integration зелёный (1199/1199, не считая отдельного падения golden на s48/49/50/53 — оно не про вас, не на main тоже, фикс ушёл отдельно #212). lint/mypy чистые.

Вопрос про разделение 1540/1751 так и остался открытым — не вижу, чтобы вы между собой договорились, но код по обоим закрыт, так что мержу одним PR как есть.

Раунд 2 принимаю, мержу.

Раунд 2. C-1/C-2/C-3 закрыты: select_exact_aggregates обновляет used инкрементально внутри цикла, появилась чистая domain/vgo_distribution.py по шагам контракта, ручной перенос — PUT/DELETE vgo-assignments/rule + exclude с lock_editable_version и аудитом. I-1 (сторона из разнесения, не своя), I-6 (candidate_rule_ids/decision в контракте), I-8 (кеш таблицы на запрос, тест test_corrections_review_fixes.py) — тоже на месте. I-5 и I-7 доделал сам прямо в ветке: порт в api/.env.test (948724c), осиротевшая APPLIED-корректировка теперь CORRECTION_REVIEW_REQUIRED вместо молчаливого continue, с тестом (d542a22). I-11 — не трогаю, не входит в этап: AURORA-1417 уже заведена, kfo_result появится только в Срезе 6. test-integration зелёный (1199/1199, не считая отдельного падения golden на s48/49/50/53 — оно не про вас, не на main тоже, фикс ушёл отдельно #212). lint/mypy чистые. Вопрос про разделение 1540/1751 так и остался открытым — не вижу, чтобы вы между собой договорились, но код по обоим закрыт, так что мержу одним PR как есть. Раунд 2 принимаю, мержу.
aep merged commit afd924fc21 into main 2026-11-02 19:35:33 +03:00
Sign in to join this conversation.
Reviewers
No Reviewers
PotapovaA
aep
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
PotapovaA Raykov-MS SalminMP Veronika Yashin1994 aep grog maxim tsygankoviva
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: RSHB/arm-core#208
No description provided.
Delete Branch "feat/corrections-back"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?