Incremental refactoring legacy-модуля

1. Один шаг — одна проверяемая гипотеза

Когда впервые смотришь на старый модуль вроде mrr-engine, очень хочется выбрать один из двух неправильных режимов. «Не трогаю, оно страшное» консервирует проблему. «Claude, перепиши всё красиво» создаёт новую — свежую, дорогую и с ещё более злым diff.

Инкрементальный рефакторинг живёт по другой логике. Вы берёте маленький участок и заранее знаете, что меняете и чем оно защищено. Шаг удался — код чуть яснее. Не удался — теряете один commit, а не неделю жизни и веру в человечество.

Полезно держать в голове этот цикл:

RISK_MAP → маленькая цель → inspect → границы изменения → edit → checks → diff → review → commit → REFACTOR_LOG

Обратите внимание на важную деталь: здесь нет шага «надеемся, что Claude сам поймёт». Рамку задаёте вы — без неё модель мгновенно превращается из помощника в энергичного стажёра, который заодно решил улучшить ещё восемь вещей.

Поэтому сегодняшняя mental model урока такая: один шаг — одна проверяемая гипотеза. Не спасение системы и не повод «заодно» пофиксить древний баг, обновить dependency и перенести логику в новый пакет. Объясняете за минуту, что стало лучше и почему поведение не поменялось, — шаг выбран правильно.

2. Первый кусок берём из RISK_MAP.md

К этому моменту у вас уже есть RISK_MAP.md из legacy discovery и baseline, собранный поверх него в прошлой лекции. Значит, цель выбирается не «куда упал взгляд», а «что даёт пользу при минимальном радиусе взрыва». Хороший первый кандидат обычно выглядит скучно: вынести поиск тарифа в helper, изолировать логирование в маленький метод, извлечь одну чистую функцию из длинного метода, убрать дублирование внутри класса.

Небольшая таблица помогает быстро отделить полезные цели от опасных:

Главный вопрос перед выбором шага звучит так: «Если этот refactor окажется неудачным, смогу ли я откатить его одним commit и ничего не объяснять полдня?» Если ответ «нет», цель слишком крупная.

Ещё один полезный критерий — покрытие baseline. Если выбранный кусок уже защищён characterization tests, golden master и хотя бы одним smoke-check, это хороший знак. Если же вы не понимаете, какие проверки скажут вам «поведение сохранилось», менять рано. В таком случае проблема не в недостатке смелости. Проблема в том, что у шага нет safety net.

В CashFlow Dashboard разумный первый шаг почти никогда не лежит в самой нервной бизнес-логике. Не стоит начинать с refund-сценариев, сложных proration-правил или мест, где одновременно завязаны billing и payments. Начинайте с участка, который уже понятен по RISK_MAP.md, хорошо читается глазами и не тянет за собой три соседних модуля.

3. inspect-before-edit перед любой правкой

В legacy-контексте особенно опасно просить AI «сразу изменить код». Слишком велик шанс, что модель увидит локальную неаккуратность и решит сделать глобальную красоту. Поэтому здесь особенно полезен паттерн inspect-before-edit: сначала Claude читает код и описывает минимальную безопасную трансформацию, и только потом получает право на редактирование.

То есть ваш первый запрос к Claude в этой итерации должен быть не «рефакторни», а «объясни, что именно ты хочешь отрефакторить и почему». Например, так:

Пока ничего не меняй.

Посмотри только класс MrrCalculator и метод calculateMonthlyMrr.
Найди самый маленький безопасный refactor без изменения поведения.

Верни:
1. что именно предлагаешь изменить;
2. какие файлы придётся затронуть;
3. какие проверки из baseline должны остаться зелёными;
4. какие non-goals важно зафиксировать.

Такой запрос хорош сразу по нескольким причинам. Во-первых, он ограничивает область внимания модели. Во-вторых, заставляет её говорить языком инженерных ограничений, а не языком «можно сделать красивее». В-третьих, заранее привязывает будущий рефакторинг к существующему baseline, а не к абстрактной идее качества кода.

Хороший ответ на inspect-запрос от Claude обычно содержит четыре вещи. Он показывает точку изменения, объясняет пользу, перечисляет конкретные файлы и привязывает шаг к проверкам. Если вместо этого вы видите предложения вроде «заодно упростить несколько соседних сервисов», «вынести в новый пакет», «переименовать DTO для единообразия» — это не повод радоваться глубине мысли. Это сигнал, что scope уже пополз.

Здесь полезно быть немного скучным ревьюером. Если Claude предлагает больше двух файлов на самый первый шаг, просите сузить изменение. Если затрагивается публичный API, исключения, формат данных или SQL — просите вернуться на шаг назад. Если в ответе нет явного списка проверок, считайте, что inspect ещё не закончился.

Иногда новичкам кажется, что это замедляет работу. На практике всё наоборот. Пять минут на inspect экономят час на разбор странного diff, где вместе с одним helper-методом неожиданно приехали новые названия полей, другой порядок вызовов и «небольшое улучшение обработки null». Legacy очень любит наказывать за спешку — и Claude, честно говоря, тоже.

4. Фиксируем цель итерации и её границы

После inspect очень полезно буквально в нескольких строках зафиксировать границы текущего изменения. Это можно сделать в PR_DESCRIPTION.md, в REFACTOR_LOG.md или хотя бы в короткой заметке внутри сессии. Смысл не в бюрократии, а в том, чтобы самому себе не дать расширить задачу по дороге.

Минимальный шаблон может выглядеть так:

## Цель
Вынести поиск плана в helper `resolvePlan()` внутри `MrrCalculator`.

## Не делаем
Не меняем сигнатуры методов, исключения, SQL, схему БД и расчёт MRR.

## Проверки
- characterization: `monthly_mrr_basic`
- golden master: `may24_fixture`
- smoke: расчёт блока MRR на dashboard

Обратите внимание, как работает блок «Не делаем». Он кажется простым, но именно он чаще всего спасает рефакторинг от превращения в «рефактор плюс немножко bugfix плюс маленький redesign, пока код открыт». Как только вы явно пишете, что не меняете сигнатуры, SQL и поведение исключений, у вас появляется простая причина остановить Claude: «Это уже вне текущей итерации».

Ещё одна полезная привычка — ограничивать размер изменения не строками, а здравым смыслом. Например: один локальный смысловой шаг, один маленький diff, максимум один класс и связанные с ним тесты. Если во время правки становится видно, что нужно тронуть третий файл, лучше не героически продолжать, а остановиться и переоценить задачу. Иногда это всё ещё нормальный рефакторинг. Но иногда — уже другой шаг, который стоит выделить отдельно.

В legacy это особенно важно, потому что большой diff обманывает. Часто он выглядит «чистым»: код стал короче, названия красивее, методы аккуратнее. Но чем больше такой diff, тем сложнее доказать, что поведение правда не изменилось. А ведь именно это и есть главное условие сегодняшней работы.

5. Маленький refactor на примере mrr-engine

Теперь можно посмотреть, как выглядит нормальный первый шаг в коде. Допустим, в MrrCalculator поиск тарифа встроен прямо в метод расчёта. Это не катастрофа, но читать и тестировать такой код неудобно.

До изменения код может выглядеть так:

public BigDecimal calculateMonthlyMrr(Subscription sub) {
    Plan plan = planRepository.findByCode(sub.getPlanCode());
    if (plan == null) {
        throw new IllegalStateException("План не найден: " + sub.getPlanCode());
    }
    return plan.getMonthlyPrice(); // текущее поведение пока не трогаем
}

Это ещё не ужас-ужас, но видно, что внутри метода смешаны две мысли: найти план и посчитать MRR. Для первого рефактор-шажка достаточно вынести поиск плана в отдельный helper, не меняя ни сигнатуру, ни исключение, ни итоговый результат.

После изменения:

public BigDecimal calculateMonthlyMrr(Subscription sub) {
    return resolvePlan(sub).getMonthlyPrice(); // метод стал короче и понятнее
}

private Plan resolvePlan(Subscription sub) {
    Plan plan = planRepository.findByCode(sub.getPlanCode());
    if (plan == null) throw new IllegalStateException("План не найден: " + sub.getPlanCode());
    return plan;
}

Это хороший шаг по нескольким причинам. Во-первых, снаружи поведение осталось прежним. Во-вторых, diff маленький и легко читается. В-третьих, у вас появляется локальная точка, вокруг которой потом проще строить следующие изменения. И, в-четвёртых, если всё пошло не так, такой рефакторинг откатывается одним commit без философских споров.

На этом месте очень важно не поддаться соблазну: «раз уж я вынес resolvePlan, давайте ещё сразу введём кэш, заменим исключение, переименуем sub в subscription, а заодно нормализуем логирование». Именно в этот момент маленький полезный шаг превращается в длинный diff, который уже никто не хочет читать до конца.

Если хочется улучшить что-то ещё — отлично. Просто это уже следующий шаг. Legacy-модуль не убежит. А вот ясность текущей итерации убежит мгновенно.

6. После шага проверяем baseline, а не оптимизм

Самая частая ошибка после удачного маленького refactor — посмотреть на более красивый код и внутренне решить: «Ну тут явно ничего не сломалось». Legacy особенно любит такие моменты. Потому что именно после фразы «тут же только helper вынесли» обычно и выясняется, что где-то снаружи рассчитывали на текст исключения, порядок вызовов или побочный эффект, который никто не заметил.

Поэтому после каждого шага вы возвращаетесь не к эстетике, а к проверкам. Минимум — к тем, которые уже зафиксированы в CHARACTERIZATION_TESTS.md. Например, вот очень простой characterization test:

@Test
public void shouldKeepMonthlyPriceForBasicPlan() {
    BigDecimal actual = calculator.calculateMonthlyMrr(subscription("BASIC"));
    assertEquals(new BigDecimal("29.99"), actual); // поведение до и после refactor одинаковое
}

Смысл такого теста не в том, чтобы «доказать математическую истину». Его задача — подтвердить, что observable behavior для уже известного сценария не съехало из-за структурного изменения. Именно поэтому нельзя просто переписывать такие тесты под новую красоту кода. Они не мешают вам. Они страхуют вас.

Кроме автоматических проверок, после рефакторинга обязательно читают diff. Даже если tests зелёные. Даже если Claude уверенно пишет «изменения минимальны». Даже если вам уже хочется идти дальше. В legacy-контексте diff — это визуальное доказательство, что вы не утащили за собой лишнего.

Здесь очень уместен reviewer-agent из Workflow Kit. Команда вызова в вашей версии Claude Code может отличаться, но сам паттерн остаётся тем же: вы даёте независимому reviewer-контексту текущий diff и просите проверить именно claims о сохранении поведения.

Например, так:

Проверь текущий diff как refactor-reviewer.

Нужно ответить:
- сохранено ли поведение по заявленным сценариям;
- соблюдены ли non-goals;
- нет ли лишних файлов и скрытых изменений;
- какие проверки обязательно должны быть зелёными.

Код не меняй.

Если reviewer-agent пишет, что diff трогает больше, чем обещано, или видит скрытый behavior change, не спорьте с ним в стиле «но intention было хорошее». Здесь intention вторично. Важен только результат в коде и проверках. А если baseline внезапно покраснел, не надо просить Claude «чуть-чуть подправить тест». Сначала откатите последний шаг, перечитайте diff и вернитесь к inspect. Тесты — это safety net, а не помеха на пути к красоте.

Облачное review

Помимо локального reviewer-agent в этой работе уместен ещё один слой — облачное ревью (условно /ultrareview или аналог — точное имя и доступность могут меняться от версии к версии). Legacy-рефакторинг особенно подвержен скрытым регрессиям: поведение меняется незаметно, потому что характеризационные тесты, которые мы готовили в прошлой теме, могут не покрывать все критические пути — в legacy-коде покрытие исторически слабое.

Облачное ревью здесь работает как дополнительный детектор регрессий на каждый refactor PR: несколько агентов разбирают diff с фокусом на сохранение поведения, побочные эффекты, скрытые предположения и совместимость с зависимыми модулями. За счёт большего контекста облачная поверхность ловит связи между модулями, которые локальный reviewer subagent пропускает.

Применение простое: запускаете облачное ревью после того, как характеризационный suite прошёл локально и refactor PR готов к разбору. Размеченный diff с уровнями серьёзности дополняет вашу детерминированную страховочную сеть на семантическом уровне — но не заменяет её. Тесты доказывают, что поведение не уехало; облачное ревью лишь подсвечивает рискованные паттерны. Final approval всё равно остаётся за человеком.

И есть три случая, когда облачное ревью для legacy не стоит применять вовсе: код, который нельзя выгружать в облако из-за чувствительных данных или требований on-prem; совсем маленький рефакторинг одного метода, где затраты на запуск облачного ревью больше пользы; и слабый характеризационный suite — пока критические пути не покрыты хотя бы наполовину, сначала укрепляйте страховочную сеть, а не гоняйте облачное ревью поверх пустоты. Плагин помогает, но не утверждает — это правило одинаково работает и для локального reviewer-agent, и для облачной поверхности.

7. Commit и REFACTOR_LOG.md держат смысл цепочки

Legacy-модернизация редко ломается на одном большом решении. Куда чаще она ломается на десятом маленьком шаге, потому что уже никто не помнит, зачем делали третий и что именно защищал седьмой. Поэтому после каждого удачного шага рефакторинга полезно не только закоммитить код, но и оставить короткий след смысла.

Очень удобен для этого REFACTOR_LOG.md. В этой модульной линии он работает как единый рабочий журнал: сначала вы фиксируете в нём маленькие шаги рефакторинга, потом candidate slices и parity-заметки, а дальше — фазы roadmap, чтобы история модернизации не расползалась по разным файлам. Он может быть буквально на несколько строк:

## Шаг 1 — `resolvePlan`
Цель: вынести поиск плана из `calculateMonthlyMrr`
Изменённые файлы: `MrrCalculator.java`, `MrrCalculatorTest.java`
Проверки: `monthly_mrr_basic`, `may24_fixture`
Результат: зелёные
Следующий кандидат: отделить audit log от расчёта

Такой лог решает сразу несколько задач. Во-первых, сохраняет контекст между сессиями Claude. Во-вторых, помогает вам самому не забыть, почему текущая форма кода вообще появилась. В-третьих, если к модулю подключится другой человек или другой агент, ему не придётся гадать, где здесь уже был осознанный шаг, а где просто случайная форма legacy.

Commit при этом тоже должен быть узким и честным. Не cleanup, не fix stuff, не improve code. Лучше что-то вроде: refactor(mrr): вынести resolvePlan из calculateMonthlyMrr. Да, звучит менее героически. Зато через две недели вы сами себе скажете спасибо.

На этом месте полезно различать три уровня работы. Если изменение всё ещё живёт внутри одного класса или контракта и baseline его закрывает, остаёмся в local refactor. Если каждый следующий шаг упирается в SQL, audit, legacy-format или другой смешанный dependency-комок, пора искать seam и разрезать связи. А если старый и новый путь должны какое-то время жить параллельно под флагом и сравниваться на parity, это уже Strangler-сценарий, а не просто ещё один helper-method.

Именно из таких маленьких и хорошо зафиксированных шагов потом собирается настоящая модернизация. Не из вдохновляющей ночи, когда «мы наконец переписали это древнее чудовище», а из цепочки скучных, зелёных изменений, которые можно нормально проверить на review, после которых mrr-engine перестаёт быть чёрным ящиком и начинает выглядеть как серия понятных решений.