Удалены contributions/load_for_kfo и фильтры общего свода ОСВ (load_osv_rows, build_with_members возвращены к main); OpenSpec change без требований вклада КФО. План доводки по ревью — docs/superpowers/plans/2026-11-06-goodwill-review-back.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
182 lines
20 KiB
Markdown
182 lines
20 KiB
Markdown
# Гудвилл: доводка бэка по ревью (arm-core #222) — план
|
||
|
||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||
|
||
**Goal:** довести PR arm-core #222 (ветка `feat/goodwill-back`, AURORA-1543) до мержа: убрать невызываемую заготовку КФО,
|
||
итоги по ГО и УК отдать с бэка, долю ограничить двумя знаками, сохранение сделать пакетным в одной транзакции, единицы ББ
|
||
читать через словарь `arm_statements`, привести контракт в docs (ветка `feat/204-goodwill-contract`, issue docs #204),
|
||
§8 правил и OpenSpec change к фактическому API.
|
||
|
||
**Architecture:** чистый расчёт — `api/src/domain/goodwill.py`; загрузка источников — `services/goodwill_source.py` +
|
||
`repositories/goodwill.py`; сервис/маршруты — `services/goodwill.py`, `api/endpoints/consolidations/goodwill.py`; экспорт —
|
||
`services/goodwill_export.py`. Общий свод ОСВ (`repositories/consolidations.load_osv_rows`,
|
||
`services/consolidated_osv.build_with_members`) возвращается к состоянию `main`.
|
||
|
||
**Tech Stack:** Python 3.11, FastAPI, SQLAlchemy 2 async, pydantic v2, openpyxl, pytest (маркеры `integration`, `fixtures`).
|
||
|
||
**Spec:** `openspec/changes/goodwill-table/` (этого репозитория) + страница аналитики DOC-259025 «Гудвилл» (версия 03.11.2026,
|
||
`mcp__sdlc-wiki__page code=DOC-259025`) + решения ревью ниже.
|
||
|
||
Рабочие каталоги: arm-core — `/Users/andrey.epifancev/projects/awp-ca/.wt/review-222/arm-core` (ветка `feat/goodwill-back`),
|
||
docs — `/Users/andrey.epifancev/projects/awp-ca/.wt/review-222/docs` (ветка `feat/204-goodwill-contract`). Git — только
|
||
`git -C <абсолютный путь worktree>`.
|
||
|
||
## Global Constraints
|
||
|
||
- Деньги — `Decimal`, `ROUND_HALF_UP` до 0.01, в API — строки с двумя знаками в рублях; `null` ≠ `0.00`.
|
||
- Доля владения: строка `0`…`100`, не больше **двух** знаков после запятой, запятая или точка; лишние знаки — 422
|
||
`INVALID_SHARE`, без округления и обрезки. В ответе — строка с двумя знаками (`"80.00"`).
|
||
- `is_head_entity` не используется; ГО — владелец строки 58.01 (и потомков) с контрагентом из периметра.
|
||
- Аудит — `GOODWILL_EDIT` (строки: `object_type=goodwill`; переключатель: `object_type=consolidation_version`,
|
||
`changed_field=goodwill_enabled`), выгрузка — `EXPORT`; новых enum и колонок нет, миграций нет.
|
||
- Коммиты — по-русски, в конце `Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>`.
|
||
- Решения ревью (дописать в `openspec/changes/goodwill-table/design.md`, раздел `## Decisions made without discussion`):
|
||
R-1 доля — 2 знака (DOC-259025 §4, §5.2, §9; DBML `numeric(7,4)` хранит без изменений); R-2 построчный `PUT …/goodwill/{head}/{sub}`
|
||
заменён пакетным `PUT …/goodwill` в одной транзакции (DOC-259025 §5.1 п. 4); R-3 заготовка вклада КФО (`contributions`,
|
||
`load_for_kfo`, фильтры общего свода) вынесена в AURORA-1546; R-4 итоги по ГО и итог УК считает бэк; R-5 единицы ББ —
|
||
через словарь единиц `arm_statements`; R-6 пометка результата КФО устаревшим — в AURORA-1546 (результата КФО ещё нет).
|
||
|
||
## Review Focus
|
||
|
||
- Пакет, где одна строка невалидна: ничего не сохраняется, ни одной записи `goodwill` и `audit_event`, в 422 есть ключ строки.
|
||
- Повторное сохранение без изменений: `updated_by/updated_at` не меняются, аудита нет (фронт шлёт весь набор отмеченных строк).
|
||
- Единицы ББ, исправленные аналитиком вручную («тыс.руб.», «в тысячах рублей»): УК всё равно переводится в рубли.
|
||
- Итог «Всего» блока: ФВ и УК — по всем действующим строкам блока, P и G — только отмеченным; «Нет вложений» не входит.
|
||
- Доля `"80,123"` — 422, а не молча `80.12`.
|
||
|
||
---
|
||
|
||
### Task 1: Убрать заготовку вклада КФО
|
||
|
||
**Files:**
|
||
- Modify: `api/src/domain/goodwill.py` (удалить `Contribution`, `contributions`)
|
||
- Modify: `api/src/services/goodwill.py` (удалить `contributions`, `BB_RULE_NO`, импорт `EliminationRule`, `StatementType`)
|
||
- Modify: `api/src/services/goodwill_source.py` (удалить `load_for_kfo`, параметр `pairs` у `_load`)
|
||
- Modify: `api/src/repositories/goodwill.py` (удалить `included_only` у `saved`, `company_ids` у `forms`)
|
||
- Restore from `origin/main`: `api/src/repositories/consolidations.py`, `api/src/services/consolidated_osv.py`
|
||
- Delete: `api/tests/test_goodwill_contribution.py`
|
||
- Modify: `openspec/changes/goodwill-table/{proposal,design,tasks}.md`, `specs/goodwill-table/spec.md`
|
||
|
||
**Interfaces:**
|
||
- Produces: `GoodwillSource.load(version_id: UUID) -> Sources` (единственный метод загрузки); `repo.saved(session, version_id) -> list[Goodwill]`;
|
||
`repo.forms(session, version_id)`.
|
||
|
||
- [ ] **Step 1:** `git -C <core> checkout origin/main -- api/src/repositories/consolidations.py api/src/services/consolidated_osv.py`;
|
||
убедиться `git diff origin/main -- <эти два файла>` пуст.
|
||
- [ ] **Step 2:** удалить перечисленный код и тест; `grep -rn "contributions\|load_for_kfo\|company_pairs\|included_only" api/src api/tests` — пусто.
|
||
- [ ] **Step 3:** в OpenSpec удалить требования «Подготовка вклада в КФО» и «Источники КФО только выбранных сохранённых пар»,
|
||
в design §6 и разделе «Отбор источников для КФО» оставить одну фразу «перенесено в AURORA-1546 (R-3)», в tasks.md пункты
|
||
4.2, 4.3, 7.x — пометить «перенесено в AURORA-1546».
|
||
- [ ] **Step 4:** `make lint` и `cd api && .venv/bin/pytest -m integration tests/test_api_goodwill.py tests/test_api_goodwill_mutations.py tests/test_goodwill_export.py tests/test_api_consolidated_osv.py -q` — зелёные.
|
||
- [ ] **Step 5:** коммит «Гудвилл: заготовка вклада КФО вынесена в AURORA-1546».
|
||
|
||
### Task 2: Доля — два знака, расчёт в домене, итоги по ГО и УК
|
||
|
||
**Files:**
|
||
- Modify: `api/src/domain/goodwill.py`, `api/src/schemas/goodwill.py`, `api/src/services/goodwill.py`, `api/src/services/goodwill_export.py`
|
||
- Test: `api/tests/test_goodwill_domain.py`, `api/tests/test_api_goodwill.py`, `api/tests/test_goodwill_export.py`, `api/tests/test_api_goodwill_mutations.py`
|
||
|
||
**Interfaces:**
|
||
- Produces (домен): `participation(capital: Decimal, share: Decimal) -> Decimal` (`capital*share/100`, HALF_UP до 0.01) — единственное
|
||
место формулы, `build` и сервис используют его; `totals(rows, enabled) -> dict[str, Decimal]` с ключами
|
||
`financial_investment_amount, charter_capital_amount, participation_amount, goodwill_amount, goodwill_in_kfo_amount`
|
||
(ФВ и УК — все действующие строки; P/G — действующие отмеченные; в КФО — `applicable` и `enabled`);
|
||
`group_totals(rows, enabled) -> dict[UUID, dict[str, Decimal]]` = `totals` по строкам каждой ГО (`head_id`).
|
||
- Produces (схемы): `GoodwillTotalsOut` + поле `charter_capital_amount: Money`; `GoodwillGroupOut {head_legal_entity: RefOut, totals: GoodwillTotalsOut}`;
|
||
`GoodwillTableOut.groups: list[GoodwillGroupOut]` — порядок как у строк (имя ГО, id); `Share` сериализуется `format(v, ".2f")`.
|
||
- Produces (сервис): `validate_decision(data) -> domain.Decision` принимает `^\d{1,3}(?:[.,]\d{1,2})?$` и `≤ 100`.
|
||
|
||
- [ ] **Step 1: тест домена на примере DOC-259025 §11.1** — `test_goodwill_domain.py::test_doc_example_totals`: ГО «Альфа»;
|
||
Бета ФВ 180000 УК 100000 доля 80 отмечена, комментарий → P 80000.00, G 100000.00; Гамма ФВ 50000 УК 100000 доля 80 отмечена → P 80000.00, G −30000.00;
|
||
Дельта ФВ 60000 УК 60000 без доли, не отмечена → P/G None. `totals(rows, True)` = ФВ 290000.00, УК 260000.00, P 160000.00,
|
||
G 70000.00, в КФО 100000.00; `totals(rows, False)["goodwill_in_kfo_amount"] == 0`; `group_totals` для «Альфа» совпадает с `totals`;
|
||
строка «Нет вложений» (сохранённое решение без ФВ) не меняет ни одной суммы.
|
||
- [ ] **Step 2: тест доли** — `test_api_goodwill_mutations.py`: параметры `test_invalid_share` дополнить `"80.123"`, `"80,123"`, `"100.01"`, `"-1"`, `"1e2"`;
|
||
`test_valid_share` — `"0"`, `"100"`, `"80,12"`, `"7.5"`; ответ строки отдаёт `"80.12"`.
|
||
- [ ] **Step 3:** прогнать — FAIL (нет `participation`, `group_totals`, `charter_capital_amount`, регулярка пропускает 4 знака).
|
||
- [ ] **Step 4:** реализовать по Interfaces; в `service.save` (до Task 3) и `build` убрать собственные формулы P в пользу `participation`;
|
||
`table()` заполняет `groups` из `group_totals`; сообщение `INVALID_SHARE` — «Доля — число от 0 до 100, не больше двух знаков после запятой».
|
||
- [ ] **Step 5: экспорт** — «Всего» блока брать из `table.groups` (удалить `_totals`), в «Итого по листу» добавить УК, формат доли `"0.00"`;
|
||
`test_goodwill_export.py`: значение УК в строке «Итого по листу» равно `totals.charter_capital_amount`.
|
||
- [ ] **Step 6:** `test_api_goodwill.py`: в ответе таблицы есть `groups[0].totals` и `totals.charter_capital_amount` с ожидаемыми строками.
|
||
- [ ] **Step 7:** `make lint`, интеграционные тесты гудвилла — PASS; коммит «Гудвилл: доля до двух знаков, итоги по ГО и УК с бэка».
|
||
|
||
### Task 3: Пакетное сохранение в одной транзакции
|
||
|
||
**Files:**
|
||
- Modify: `api/src/schemas/goodwill.py`, `api/src/services/goodwill.py`, `api/src/api/endpoints/consolidations/goodwill.py`
|
||
- Test: `api/tests/test_api_goodwill_mutations.py`, `api/tests/test_api_goodwill.py`, `api/tests/test_goodwill_export.py`, `api/tests/test_openapi_routes.py`
|
||
|
||
**Interfaces:**
|
||
- Consumes: `validate_decision`, `participation` (Task 2).
|
||
- Produces: маршрут `PUT /api/v1/consolidations/versions/{version_id}/goodwill`, право `CONSOLIDATION_EDIT`, тело
|
||
`GoodwillBatchIn {rows: list[GoodwillRowKeyIn]}` (`min_length=1`, `extra="forbid"`), где
|
||
`GoodwillRowKeyIn = {head_legal_entity_id: UUID, subsidiary_legal_entity_id: UUID, ownership_share: str | None, include_in_kfo: bool, comment: str | None (≤1000)}`;
|
||
ответ 200 `GoodwillTableOut`. Маршрут `PUT …/goodwill/{head}/{sub}` и `GoodwillService.save` удаляются; новый
|
||
`GoodwillService.save_rows(version_id: UUID, rows: list[GoodwillRowKeyIn], audit: RequestAudit) -> GoodwillTableOut`.
|
||
- Ошибки (все — откат всего пакета): 404 `NOT_FOUND` «Пара не найдена» и 422 `INVALID_SHARE` / `COMMENT_REQUIRED` /
|
||
`MONEY_OUT_OF_RANGE` с доп. полями `head_legal_entity_id`, `subsidiary_legal_entity_id` (для `NotFoundError` — создать
|
||
`DomainError(404, "NOT_FOUND", …, extra=…)`); пара дважды в пакете — 422 `DUPLICATE_ROW` с теми же полями; 409 правок версии — как раньше.
|
||
|
||
- [ ] **Step 1: тесты** (`test_api_goodwill_mutations.py`, переписать вызовы на пакет):
|
||
`test_batch_rolls_back_when_one_row_invalid` — две пары, вторая с `include_in_kfo=true` без комментария → 422 `COMMENT_REQUIRED`,
|
||
`detail.subsidiary_legal_entity_id` = id второй ДО, в БД 0 записей `goodwill` и 0 `audit_event(GOODWILL_EDIT)`;
|
||
`test_unchanged_rows_are_not_rewritten` — сохранить строку, повторить тот же пакет → `updated_at` прежний, событий `GOODWILL_EDIT` по-прежнему одно;
|
||
`test_new_row_with_defaults_is_not_created` — пакет `{share: null, include: false, comment: null}` по паре без записи → записей 0;
|
||
`test_duplicate_pair_in_batch` → 422 `DUPLICATE_ROW`; `test_unknown_pair` → 404 с ключом пары. Сценарии прежнего
|
||
`test_save_settings_and_disappearing_pair` и `test_money_overflow_keeps_database_unchanged` сохранить, переведя на пакет.
|
||
- [ ] **Step 2:** `test_openapi_routes.py` — путь `…/goodwill`: `{"get", "put"}`, путь `…/goodwill/{head_legal_entity_id}/{subsidiary_legal_entity_id}` удалён.
|
||
- [ ] **Step 3:** прогнать — FAIL.
|
||
- [ ] **Step 4:** реализовать `save_rows`: `lock_editable_version` → одна сборка таблицы → проверка дублей, пар и ввода всех
|
||
строк до любой записи → upsert только изменённых (сравнение `ownership_share`, `include_in_kfo`, `comment` с сохранённым;
|
||
для пары без записи «изменение» — отличие от `Decision()`) с пересчётом сумм как сейчас → `flush` → одна сборка таблицы →
|
||
`GOODWILL_EDIT` по каждой изменённой строке (`before` — строка ответа до, `null` для новой; `after` — строка ответа после) →
|
||
вернуть таблицу. Поведение «отсутствующий ввод не затирает сохранённое у неактивной пары» сохранить.
|
||
- [ ] **Step 5:** PASS; `make lint`; коммит «Гудвилл: пакетное сохранение таблицы одной транзакцией».
|
||
|
||
### Task 4: Единицы ББ через словарь arm_statements
|
||
|
||
**Files:**
|
||
- Modify: `api/src/services/statements_engine.py` (Protocol `StatementsEngine` и `ArmStatementsEngine`), `api/src/services/goodwill_source.py`
|
||
- Test: `api/tests/test_goodwill_source.py`
|
||
|
||
**Interfaces:**
|
||
- Produces: `StatementsEngine.canonical_units(text: str | None) -> str | None` — `normalize_caption(text)` (из `arm_statements.text`)
|
||
и первый совпавший шаблон `self._rules.units.spellings` → каноническая единица («руб.», «тыс. руб.», «млн руб.»), иначе `None`.
|
||
`capital_amounts` берёт множитель по `default_engine().canonical_units(units)`.
|
||
|
||
- [ ] **Step 1: тест** `test_capital_units_spellings` (параметры): `"тыс.руб."`, `"в тысячах рублей"`, `"тыс. руб."` → УК = значение × 1000;
|
||
`"руб"` → × 1; `"млн. руб."` → × 1000000; `"штук"` и `None` → `(None, None)`.
|
||
- [ ] **Step 2:** FAIL → реализовать → PASS; если пакетный словарь в ядре подменяется заглушкой в тестах (`StatementsEngine` фейки в `api/tests`),
|
||
добавить метод и туда (`grep -rn "class .*Engine" api/tests`).
|
||
- [ ] **Step 3:** `make lint`; коммит «Гудвилл: единицы ББ по словарю arm_statements».
|
||
|
||
### Task 5: Контракт, §8, OpenSpec, README
|
||
|
||
**Files:**
|
||
- docs: `dev/api-contract-goodwill.md`, `dev/api-contract-goodwill.openapi.yaml`, `context/05-domain-rules.md` (§8 «Гудвилл»)
|
||
- arm-core: `api/tests/test_goodwill_contract.py`, `README.md` (раздел «Таблица гудвилла», таблица «Ошибки API»),
|
||
`openspec/changes/goodwill-table/{proposal,design,tasks}.md`, `specs/goodwill-table/spec.md`
|
||
|
||
**Interfaces:**
|
||
- Consumes: фактический `app.openapi()` после Tasks 1–4.
|
||
|
||
- [ ] **Step 1: тест контракта** — читать `Path(__file__).resolve().parents[3] / "docs/dev/api-contract-goodwill.openapi.yaml"`,
|
||
при отсутствии файла — `pytest.skip("нет ../docs: контракт гудвилла не проверен")`; сравнение путей `/goodwill` и всех схем
|
||
контракта с `app.openapi()` — как сейчас; проверку `validate_decision` перевести на `"80,12"` → `Decimal("80.12")`.
|
||
- [ ] **Step 2: OpenAPI-копия** — выгрузить из `app.openapi()` пути с `/goodwill` и все схемы, на которые они ссылаются
|
||
(транзитивно, включая `RefOut`, `WarningOut`, `HTTPValidationError`), в `docs/dev/api-contract-goodwill.openapi.yaml` (`openapi`, `info`
|
||
с версией `1.0`, `paths`, `components.schemas`); тест Step 1 — PASS.
|
||
- [ ] **Step 3: MD-контракт** — переписать `docs/dev/api-contract-goodwill.md` под факт: источники (DOC-259025 03.11 первым), понятия
|
||
(ГО по вложениям, «Нет вложений», P/G только у отмеченных, предупреждения `CAPITAL_SOURCE_MISSING`/`CAPITAL_SOURCE_AMBIGUOUS`/
|
||
`OWNERSHIP_TOTAL_EXCEEDED`), эндпоинты (GET, PUT пакет, PUT settings, GET export с `sign=all|positive|negative|zero`), схемы, ошибки,
|
||
пример ответа с `groups`, открытые вопросы OQ-10/OQ-37; раздел «Что изменилось против черновика 30.10» — для фронта.
|
||
- [ ] **Step 4: §8** — заменить абзац «Гудвилл» в `05-domain-rules.md` на выжимку DOC-259025 §§1–8 с решениями R-1, R-5 и
|
||
пометкой «применение в КФО и исключение ББ-2 — AURORA-1546, не реализовано».
|
||
- [ ] **Step 5: arm-core** — README: ссылка на `../docs/dev/api-contract-goodwill.md`, пакетный PUT, коды `INVALID_SHARE`,
|
||
`COMMENT_REQUIRED`, `MONEY_OUT_OF_RANGE`, `DUPLICATE_ROW` в «Ошибки API»; OpenSpec — требования «Построчное сохранение» → «Пакетное сохранение»,
|
||
доля — два знака, итоги по ГО и УК, единицы; раздел `## Decisions made without discussion` с R-1…R-6 (дата 2026-11-06, решение,
|
||
обоснование, цена ошибки); `openspec validate goodwill-table --strict` — valid.
|
||
- [ ] **Step 6:** `make check` (весь unit + lint) и `make test-integration` в arm-core — зелёные; `make dbml-check` — без расхождений.
|
||
- [ ] **Step 7:** коммиты: docs — «Гудвилл: контракт API и §8 под DOC-259025 и ревью core #222 (#204)»; arm-core — «Гудвилл: контракт в docs, README, OpenSpec под ревью».
|