Prompty do code review: przegląd kodu z AI

Prompt do code review powinien mówić modelowi, czego szukać, jak poważne jest każde znalezisko i jaki dowód podać. "Przejrzyj ten kod" daje przyjazną listę sugestii, w której luka bezpieczeństwa siedzi między radą o docstringu a preferencją co do nazewnictwa. Prompt z listą kontrolną, poziomami wagi i prośbą o diffy daje raport, na podstawie którego da się działać, znalezisko po znalezisku.

Od "przejrzyj to" do listy kontrolnej z wagą

Kod w obu zakładkach ma dwa prawdziwe bugi. Porównaj, jak każda odpowiedź je przedstawia.

Przejrzyj toLista kontrolna z wagą

EditCopy

Możesz przejrzeć ten kod? def get_user(conn, username): cur = conn.cursor() cur.execute(f"SELECT id, email FROM users WHERE name = '{username}'") row = cur.fetchone() return {"id": row[0], "email": row[1]}

Try it

Example replyReplies vary between models and runs.

Ogólnie funkcja jest krótka i łatwa do zrozumienia. Kilka sugestii:

• Dodaj adnotacje typów, na przykład username: str, żeby sygnatura była czytelniejsza.
• Dodaj docstring wyjaśniający, co funkcja zwraca.
• Używaj zapytań parametryzowanych. Składanie SQL za pomocą f-stringów może prowadzić do SQL injection.
• Obsłuż brakujących użytkowników. fetchone() może zwrócić None.
• Nazewnictwo: fetch_user mogłoby lepiej opisywać dostęp do bazy danych.

Pierwsza odpowiedź znalazła oba bugi i właśnie na tym polega problem z ocenianiem przeglądu po tym, co wspomina. Injection jest na trzecim miejscu listy pięciu punktów, złagodzone do "może prowadzić", bez poprawki, obok rady o docstringu. Ktoś, kto tylko przebiegnie ją wzrokiem, doda adnotacje typów i pójdzie dalej.

Drugi prompt zmienił cztery rzeczy:

• Zakres. "Tylko bugi, bezpieczeństwo i obsługa błędów. Pomiń styl" usuwa szum. Styl to zadanie dla lintera i formatera, które są szybsze i nigdy nie przeczą same sobie.
• Waga. Zmusza model do uszeregowania, a uszeregowana lista mówi, od czego zacząć.
• Wejście wywołujące błąd. "x' OR '1'='1" zamienia mgliste ryzyko w demonstrację, którą można uruchomić. To także najlepszy filtr na fałszywe alarmy, co pokazuje ostatnia sekcja.
• Diffy. Mały diff na znalezisko można przyjąć albo odrzucić osobno. Przepisana funkcja miesza każdą poprawkę ze zmianami, o które nikt nie prosił.

"Jeśli na jakimś poziomie wagi nic nie ma, niczego nie wymyślaj" znaczy więcej, niż się wydaje. Poproszony o przegląd model ma skłonność do generowania znalezisk nawet wtedy, gdy niewiele jest do znalezienia, a najsłabsze z nich tylko wydłużają listę. Wyraźne pozwolenie, żeby na danym poziomie nic nie zgłaszać, ogranicza to wypełnianie.

Kontekst, którego potrzebuje recenzent

Ludzki recenzent wie, do czego służy kod i skąd pochodzą jego wejścia. Model wie tylko to, co wkleisz. "username pochodzi prosto z formularza logowania" to zdanie, które sprawia, że SQL injection jest znaleziskiem o wysokiej wadze, a nie teoretycznym. Przydatny kontekst do przeglądu:

• Skąd pochodzą wejścia: od użytkowników, z innej wewnętrznej usługi, z pliku konfiguracyjnego, który kontrolujesz.
• Co wywołuje kod i czego oczekuje w zamian.
• Środowisko uruchomieniowe: wersja języka, framework, sterownik bazy danych. Poprawka injection powyżej używa ?, bo tego placeholdera używa sqlite3; niektóre inne sterowniki używają %s.
• Na czym ci najbardziej zależy przy tej zmianie: poprawność, bezpieczeństwo, wydajność albo wszystkie trzy.

Prompt do przeglądu wielokrotnego użytku

Ten blok zamienia listę kontrolną w szablon. Zmień język, obszar i kod, a zasady zostaw. Wyłączenie części z ograniczeniami to szybki sposób, żeby zobaczyć, ile szumu wraca bez niej.

Szablon code review

EditCopy

Fill injęzykobszarkontekst

PartsRoleTaskContextInputFormatConstraints

Role

Jesteś seniorem recenzującym kod w JavaScript. Zgłaszasz prawdziwe problemy i nic więcej.

Task

Przejrzyj poniższy kod, skupiając się na bugach i obsłudze błędów.

Context

To działa w przeglądarce. `ids` to tablica numerycznych id z naszego własnego API.

Input

async function loadAll(ids) { const results = []; ids.forEach(async (id) => { const res = await fetch(`/api/items/${id}`); results.push(await res.json()); }); return results; }

Format

Dla każdego znaleziska: waga (wysoka, średnia albo niska), linia, wejście albo sytuacja, która je wywołuje, i minimalny diff. Najpoważniejsze najpierw.

Constraints

Żadnych uwag o stylu, nazewnictwie ani formatowaniu. Jeśli nie masz pewności, czy znalezisko jest prawdziwe, powiedz to i napisz, co by to potwierdziło. Jeśli nic nie znajdziesz, napisz "Brak znalezisk".

Try it

Example replyReplies vary between models and runs.

1. Wysoka: funkcja zwraca wynik, zanim zakończy się jakiekolwiek zapytanie (linia 3) forEach nie czeka na asynchroniczne callbacki. loadAll od razu zwraca pustą tablicę results, a dopisywanie następuje później. Każdy wywołujący dostaje [].

2. Średnia: nieudane zapytania są traktowane jak dane (linia 5) Odpowiedź 404 albo 500 nie jest wyjątkiem w fetch. Treść błędu w JSON zostaje sparsowana i dopisana, jakby była elementem, a treść niebędąca JSON-em sprawia, że res.json() rzuca wyjątek w callbacku, na który nikt nie czeka.

Poprawka dla obu problemów, która przy okazji zachowuje wyniki w tej samej kolejności co ids:

 async function loadAll(ids) {
-  const results = [];
-  ids.forEach(async (id) => {
-    const res = await fetch(`/api/items/${id}`);
-    results.push(await res.json());
-  });
-  return results;
+  return Promise.all(
+    ids.map(async (id) => {
+      const res = await fetch(`/api/items/${id}`);
+      if (!res.ok) throw new Error(`Item ${id}: HTTP ${res.status}`);
+      return res.json();
+    })
+  );
 }

Linia z rolą ustala poziom wymagań ("prawdziwe problemy i nic więcej") i właśnie tu role prompting pomaga najbardziej. Jeśli przegląd czyta program, na przykład skrypt publikujący znaleziska jako komentarze w pull requeście, poproś o JSON ze stałymi polami zamiast prozy; jak to zrobić, pokazuje structured output.

Radzenie sobie z fałszywymi alarmami

Część znalezisk będzie błędna. Model nie widzi wywołującego, który waliduje wejście, ograniczenia w bazie danych, które sprawia, że jakiś przypadek jest niemożliwy, ani powodu, dla którego dziwnie wyglądająca linia jest celowa. Może się też mylić co do tego, jak działa biblioteka; halucynacje AI wyjaśniają, dlaczego pewne siebie twierdzenie to nie to samo co sprawdzone.

Sprawdzaj każde znalezisko na jego wejściu wywołującym błąd. Jeśli nie da się podać wejścia, które w twoim systemie dociera do problemu, zaprotestuj, podając brakujący kontekst, zamiast przyjmować poprawkę:

Zakwestionuj znalezisko

EditCopy

Oznaczasz tę funkcję, bo `cents` może być undefined albo stringiem: function formatPrice(cents) { return `$${(cents / 100).toFixed(2)}`; } Jest wywoływana tylko z `renderCart`, która kończy działanie wcześniej, jeśli `Number.isInteger(cents)` nie jest prawdą. Czy to nadal jest problem? Jeśli nie, wycofaj znalezisko.

Try it

Example replyReplies vary between models and runs.

Przy takim sprawdzeniu nie: formatPrice zawsze dostaje liczbę całkowitą, więc znalezisko nie ma zastosowania i je wycofuję.

Jednej rzeczy to sprawdzenie nie obejmuje: ujemne liczby całkowite przechodzą przez Number.isInteger, a formatPrice(-500) zwraca "$-5.00". Jeśli do tej funkcji mogą trafiać zwroty albo rabaty, możesz chcieć zamiast tego "-$5.00". Jeśli nie mogą, nie ma nic do zmiany.

Model, któremu pokazano nowe dowody, powinien albo wycofać znalezisko, albo wyjaśnić, dlaczego nadal jest aktualne mimo tych dowodów. Obie reakcje są przydatne. Model, który po prostu zgadza się z każdym sprzeciwem, nie robi przeglądu, więc oceniaj odpowiedź po jej uzasadnieniu, a nie po tym, czy się zgodziła. Jeśli kod napisał sam model, przeprowadź przegląd w nowej rozmowie, żeby nie był odczytywany przez pryzmat tego samego rozumowania, które ten kod wytworzyło. Te same nawyki dotyczące zakresu, kontekstu i testów sprawiają, że kod jest lepszy już od początku; zobacz prompty do pisania kodu.