Przegląd dużych refaktoryzacji w AI: Dlaczego przejście testów pomija usunięty zamiar
Dowiedz się, dlaczego refaktoryzacje wspomagane przez sztuczną inteligencję w tajemnicy usuwają kod nośny, dlaczego testy nie wykrywają tego problemu oraz jakie pięć praktyk sprawdzania zapewnia bezpieczeństwo dużych refaktoryzacji przed ich wdrożeniem.
Źródła: I Let Claude Refactor 4,000 Lines. Here’s the Part I Had to Undo.
Assystenci do programowania oparte na AI są już na tyle skuteczni, że potrafią naprawić zaniedbany moduł w ciągu popołudnia, a rezultat zazwyczaj jest lepszy od tego, co zostało zastąpione. Niebezpieczeństwo nie leży w niedbale napisanym kodzie, lecz w kodzie wyglądającym poprawnie, który potajemnie usuwa funkcjonalności, których nikt nie zapisał. Ten przewodnik przedstawia rzeczywisty przykład takiej awarii, wyjaśnia, dlaczego zarówno zestaw testów, jak i osoba sprawdzająca kod jej nie zauważyły, oraz proponuje konkretny proces przeglądu kodu przy dużych przerobach wspomaganych przez AI.
Przerób, który poprawił kod, ale uszkodził system
Rozważmy moduł, w którym gromadzą się zmiany od dwóch lat: około 4000 linii rozproszonych w czterech plikach, część bazy kodu, której inżynierowie starają się unikać za wszelką cenę. Moduł ten ma niespójne radzenie sobie z błędami, trzy różne sposoby parsowania dat oraz kilka funkcji, czy to nadal są one wywoływane, nie jest pewne nawet dla samego programisty.
Rozwijający przekazuje kod asystentowi AI w dwóch sesjach i otrzymuje coś wyraźnie lepszego: jednolite wzory, poprawne wskazówki typowe, usunięte martwe gałęzie, o około jedną trzecią mniej wierszy. Testy przechodzą. Rozwijający sprawdza różnice, a przynajmniej tak sądzi, i wypuszcza kod.
Sześć dni później integracja z partnerem zaczyna tracić rekordy. Nie pojawiają się żadne wyraźne błędy; około cztery procent rekordów po prostu przestaje przychodzić. Przyczyną jest refaktoryzacja, a konkretnie zmiana, którą rozwijający przeczytał i zatwierdził.
Gdzie asystent naprawdę się wyróżnił
Warto być precyzyjnym co do tego, co poszło dobrze, ponieważ wniosek nie brzmi „AI słabo radzi sobie z refaktoryzacją”. Asystent doskonale poradził sobie ze wszystkimi transformacjami, które zależą wyłącznie od kodu przed nim:
- połączenie trzech pomocników do parsowania dat w jedną funkcję
Dla prostych, lokalnych zmian, które można zweryfikować poprzez ich przeczytanie, była szybsza i bardziej spójna niż człowiek, a nie traciła koncentracji po kilkuset liniach kodu, gdzie manualne refaktoryzacje zwykle tracą na jakości. Gdy zadaniem jest „sprawienie, by ten kod wyrażał to samo zachowanie jaśniej”, te narzędzia radzą sobie bardzo dobrze. Problemy pojawiają się w momencie, gdy określa się granice tego, co faktycznie obejmuje pojęcie „to samo zachowanie”.
Dlaczego usuwany jest kod nośny
Kod, który nie ma żadnego widocznego powodu istnienia, wygląda dokładnie jak zbędne elementy. Oto zatwierdzona zmiana:
# before
def normalise_timestamp(raw):
ts = raw.strip()
if ts.endswith("Z") and "." in ts:
ts = ts.split(".")[0] + "Z" # partner sends 7-digit microseconds
return datetime.fromisoformat(ts.replace("Z", "+00:00"))
# after
def normalise_timestamp(raw):
return datetime.fromisoformat(raw.strip().replace("Z", "+00:00"))
Usunięta gałąź wygląda jak typowy cel refaktoryzacji: przypadek specjalny, dziwnie szczegółowa kontrola sufiksu oraz rozdzielanie ciągu znaków. Wydaje się zbędna, ponieważ fromisoformat już parsuje daty w formacie ISO.
Problemem jest dane wejściowe. Jeden z partnerów wysyła daty z siedmioma cyframi dziesiętnymi. Przed wersją Python 3.11 funkcja fromisoformat rozumiała jedynie formaty generowane przez samą funkcję isoformat(), co oznaczało maksymalnie sześć cyfr dziesiętnych; dlatego wartość z siedmioma cyframi powodowała błąd ValueError. Wersje Python 3.11 i nowsze obsługują znacznie szerszy podzbiór standardu ISO 8601 i są bardziej wyrozumiałe w tym względzie, więc to, czy ta zmiana powoduje problemy, zależy od wersji języka – co jest kolejnym powodem, by sprawdzać, a nie zakładać. Ta gałąź powstała na skutek zgłoszenia od klienta sprzed osiemnastu miesięcy, a jej krótki komentarz nie wystarczył, by ktokolwiek, czy to człowiek, czy model, właściwie ocenił jej znaczenie w porównaniu z łatwością jej usunięcia. Należy również zauważyć, że stara gałąź robiła więcej, niż się wydawało: całkowicie odrzucała część dziesiętną, więc nawet wynik „nowszej” wersji różnił się dla danych, które faktycznie mogły zostać przetworzone.
Podstawowy problem jest prosty: model mógł zobaczyć kod, ale nie incydent, który go spowodował. Ticket, analiza postmortem, wątek rozmowy oraz godziny spędzone na debugowaniu znajdowały się poza plikiem. Uruchomienie polecenia git blame ujawniłoby komunikat z commitu typu „fix timestamp parsing for partner feed”, który stanowił całe uzasadnienie, ale nikt nie poprosił asystenta o sprawdzenie tego. Recenzent zatwierdził usunięcie z tego samego powodu, co zaproponował asystent: obaj analizowali różnice między wersjami, a nie historię zmian.
Dlaczego zestaw testów pozostał w stanie „zielonym”
Testy sprawdzały, co funkcja zwracała dla danych wejściowych, które ktoś wymyślił, a problematyczny dane wejściowy był, z definicji, takim, którego nikt nie wymyślił.
Pliki zawierały daty z trzema cyframi dziesiętnymi, ponieważ tak właśnie ludzie wpisują dane testowe ręcznie. Wartości siedmocyfrowe pojawiały się jedynie w transmisji na żywo jednego z partnerów.
To właśnie sprawia, że duże przeróbki są ryzykowne, niezależnie od udziału sztucznej inteligencji: przeszły zestaw testów dowodzi jedynie, że zachowanie opisane w testach przetrwało. Nie mówi nic o zachowaniach, których one nie opisują. Gdy przerabiasz trzydzieści linii ręcznie, myślisz o każdej z nich. Gdy jednocześnie zmienia się cztery tysiące linii, zestaw testów ostatecznie wykonuje niemal całą weryfikację, a nigdy nie został zaprojektowany tak, by udźwignąć takie obciążenie.
Metryki pokrycia nie były pomocne. Funkcja miała pełne pokrycie linii – każda linia została wykonywana podczas testów. Jedna z gałęzi po prostu nigdy nie została uruchomiona przy istotnych danych wejściowych. Pokrycie linii mierzy to, który kod został wykonywany, a nie które kształty danych wejściowych zostały wypróbowane.
Sortowanie zmian AI według poziomu ryzyka
Pракtycznym rozwiązaniem jest zaprzestanie jednolitej analizy różnic wygenerowanych przez AI oraz ich klasyfikacja według kategorii. Kategorie te charakteryzują się zupełnie różnymi profilami ryzyka:
- Niskie ryzyko: adnotacje typów, przemiany nazw, formatowanie, wczesne zwracanie wartości oraz podobne mechaniczne modyfikacje tekstu. Są one lokalne i łatwe do weryfikacji poprzez przeczytanie.
- Średnie ryzyko: łączenie duplikowanych funkcji pomocniczych. Zazwyczaj nie stanowi problemu, ale należy sprawdzić, czy połączona wersja obsługuje dane wejściowe wszystkich użytkowników.
- Wysokie ryzyko: usuwanie kodu, który wydaje się nie być używany, eliminacja przypadków specjalnych, które wyglądają na zbędne, oraz ujednolicenie zachowań, które wydają się niespójne.
Grupa o wysokim ryzyku ma wspólną cechę: w każdym przypadku model prawidłowo zauważa, że coś jest nie tak, a właśnie to, że coś wydaje się błędne, jest powodem jego istnienia.
Ostatni problem również uderza w cichszy sposób. Wyobraźmy sobie jeden moduł, który formatuje błędy inaczej niż pozostałe, oraz refaktoryzację, która ma sprawić, by był zgodny. Ta różnica była celowa, ponieważ dalszy odbiorca parsuje dokładnie taki kształt błędu. Dążenie do jednolitości było słusznym instynktem; zastosowanie go w tym przypadku okazało się błędem.
Pięć zasad dla następnej dużej refaktoryzacji
Są one uporządkowane mniej więcej według tego, w jakim stopniu każda z nich mogłaby zapobiec powyższemu incydentowi.
Zachowuj, aby każda różnica była na tyle mała, by można ją było rzeczywiście przejrzeć
Cztery tysiące linii to nie zmiana nadająca się do przejrzenia. Po kilkuset liniach większość ludzi tylko przegląda oczywiste problemy, zamiast analizować każdą część, bez względu na to, co sobie mówią. Ogranicz każdą sesję do jednego modułu, a każdą prośbę o pull request do zmian w jednym module. Wąskim gardłem nie jest przepustowość modelu, lecz uwaga osoby sprawdzającej.
Wymagaj uzasadnienia dla każdego usunięcia
Zanim zaakceptujesz wynik, poproś asystenta o listę wszystkiego, co usunął, oraz dla każdego elementu – dlaczego uznał, że usunięcie jest bezpieczne. Przedstawienie rozumowania w formie tekstu to jeden z najcenniejszych kroków, jakie możesz podjąć. Każde uzasadnienie w stylu „to wydawało się zbędne” powinno być traktowane jako sygnał do ręcznej weryfikacji.
Daj modelowi historię
Poproś asystenta, aby sprawdził narzędzie git blame dla każdej linii, której proponuje usunięcie, i podał komunikat z commitu. Powód istnienia nietypowej gałęzi jest często tam zapisany, a model dobrze wykorzystuje ten kontekst, gdy go ma; po prostu nie będzie go szukał bez wyraźnej prośby.
Napisz komentarze „dlaczego” przed refaktoryzacją
Najpierw dodawaj komentarze wyjaśniające, a nie później. Dokładny komentarz z podaniem nazwy partnera, ułamków siedmocyfrowych oraz limitu parsera sześciocyfrowego prawdopodobnie zapobiegłby tej usunięciu. Niejasna notatka o czasach zapisu danych od partnera tego nie zrobiła. Jeśli osoba lub narzędzie, które następnie przeczyta kod, nie będzie w stanie zrozumieć, dlaczego dana linia istnieje, ta linia jest o krok od zniknięcia w trakcie czyszczenia kodu.
Porównanie starej i nowej wersji z rzeczywistym ruchem
Przepuść dane z jednego dnia rzeczywistej pracy przez zarówno starą, jak i nową implementację, a następnie porównaj wyniki. W przypadku prostych transformacji takich jak analiza i normalizacja jest to tanie i pozwala wykryć dokładnie te problemy, których nie dostrzegają testy przygotowane specjalnie, ponieważ rzeczywisty ruch zawiera dane, o których nikt nie pomyślał, by je zapisać. Jeśli ponowne odtworzenie ruchu nie jest możliwe, najlepszym rozwiązaniem jest pobieranie próbek rzeczywistych danych do testów regresji.
Główne wnioski
Przeprojektowanie i tak było warte zachodu: moduł jest lepszy, praca została wykonana w ciągu popołudnia, a pojedynczy błąd regresji wśród czterech tysięcy zmienionych linii jest znacznie lepszy od wielu dużych, ręcznych przeprojektowań. Nazywanie tego wyniku „błędem AI” pomija sedno sprawy. System wyciągnął rozsądny wniosek na podstawie tego, co mógł zobaczyć – w przypadku szczególnym bez podanego powodu – a ludzki recenzent doszedł do tego samego wniosku na podstawie tego samego niekompletnego wyniku.
- Repozytorium to tylko część systemu; powody dziwnego kodu znajdują się w zgłoszeniach, komunikatach o zapisie zmian, analizach postmortem oraz w pamięci ludzi.
- Narzędzie, które widzi tylko kod, z pewnością usunie wszystko, co nie miało żadnego uzasadnienia zapisanego na piśmie.
- Zielone testy i pełne pokrycie linii kodu nie dowodzą, że zachowanie zostało zachowane dla danych, których nikt nie modelował.
Literatura pokrewna
- Lista kontrolna przeglądu kodu w React: Derived State, Reducers, Effects i Memo — Dowiedz się, jak rozpoznawać siedem najczęstszych problemów podczas przeglądu kodu w React, od duplikowanego stanu i ręcznego pobierania danych po niewłaściwe użycie funkcji Effects i nadmierne memoizowanie, oraz co napisać zamiast tego.
- Dzielenie zbyt dużych komponentów React bez regresji w produkcji — Krok po kroku: jak bezpiecznie dzielić duże komponenty React poprzez określenie zadań, wyodrębnienie czystej logiki, zarządzanie stanem oraz wdrażanie zmian w małych krokach.