Code Review עם AI: פרומפטים לבדיקת קוד

פרומפט ל-code review צריך לומר למודל מה לחפש, כמה חמור כל ממצא ואיזו הוכחה לתת. "תבדוק את הקוד הזה" מקבל רשימה ידידותית של הצעות שבה פרצת אבטחה יושבת בין טיפ על docstring להעדפה בשמות. פרומפט בדיקה עם רשימת בדיקה, רמות חומרה ובקשה ל-diffs מקבל דוח שאפשר לפעול לפיו, ממצא אחרי ממצא.

מ-"תבדוק את זה" לרשימת בדיקה עם חומרה

בקוד בשתי הלשוניות יש שני באגים אמיתיים. השוו איך כל תשובה מציגה אותם.

תבדוק את זהרשימת בדיקה עם חומרה

EditCopy

אתה יכול לבדוק את הקוד הזה? 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.

באופן כללי הפונקציה קצרה וקל לעקוב אחריה. כמה הצעות:

• הוסף type hints, למשל username: str, כדי שהחתימה תהיה ברורה יותר.
• הוסף docstring שמסביר מה הפונקציה מחזירה.
• השתמש בשאילתות עם פרמטרים. בניית SQL עם f-strings עלולה להוביל ל-SQL injection.
• טפל במשתמשים שלא קיימים. fetchone() יכול להחזיר None.
• שמות: fetch_user אולי יתאר טוב יותר את הגישה למסד הנתונים.

התשובה הראשונה מצאה את שני הבאגים, וזו בדיוק הבעיה בשיפוט בדיקה לפי מה שהיא מזכירה. ההזרקה נמצאת במקום השלישי ברשימה של חמישה, מרוככת ל-"עלולה להוביל", בלי תיקון, ליד טיפ על docstring. קורא שמרפרף עליה היה מוסיף type hints וממשיך הלאה.

הפרומפט השני שינה ארבעה דברים:

• היקף. "רק באגים, אבטחה וטיפול בשגיאות. דלג על סגנון" מסיר את הרעש. סגנון שייך ל-linter ול-formatter, שהם מהירים יותר ולעולם לא סותרים את עצמם.
• חומרה. זה מכריח את המודל לדרג, ורשימה מדורגת אומרת לכם מאיפה להתחיל.
• קלט שמפעיל את הבעיה. "x' OR '1'='1" הופך סיכון מעורפל להדגמה שאפשר להריץ. זה גם המסנן הכי טוב שלכם ל-false positives, כמו שהסעיף האחרון מראה.
• Diffs. diff קטן לכל ממצא אפשר להחיל או לדחות בנפרד. פונקציה משוכתבת מערבבת כל תיקון עם שינויים שאף אחד לא ביקש.

"אם אין שום דבר ברמת חומרה מסוימת, אל תמציא משהו" חשוב יותר ממה שזה נראה. כשמבקשים ממודל בדיקה, הוא נוטה להפיק ממצאים גם כשיש מעט למצוא, והחלשים שבהם מנפחים את הרשימה. הרשאה מפורשת לא לדווח כלום ברמה מסוימת מקצצת את הניפוח הזה.

ההקשר שהבודק צריך

בודק אנושי יודע למה הקוד נועד ומאיפה מגיעים הקלטים שלו. המודל יודע רק את מה שאתם מדביקים. "username מגיע ישירות מטופס התחברות" הוא המשפט שהופך את ה-SQL injection לממצא בחומרה גבוהה במקום לממצא תיאורטי. הקשר שימושי לבדיקה:

• מאיפה מגיעים הקלטים: משתמשים, שירות פנימי אחר, קובץ הגדרות שאתם שולטים בו.
• מה קורא לקוד ומה הוא מצפה לקבל בחזרה.
• סביבת הריצה: גרסת השפה, framework, דרייבר מסד הנתונים. התיקון להזרקה למעלה משתמש ב-? כי sqlite3 משתמש ב-placeholder הזה; דרייברים אחרים משתמשים ב-%s.
• מה הכי חשוב לכם בשינוי הזה: נכונות, אבטחה, ביצועים, או שלושתם.

פרומפט בדיקה לשימוש חוזר

הבלוק הזה הופך את רשימת הבדיקה לתבנית. שנו את השפה, את המיקוד ואת הקוד, והשאירו את הכללים. כיבוי חלק המגבלות הוא דרך מהירה לראות כמה רעש חוזר בלעדיו.

תבנית ל-code review

EditCopy

Fill inשפהמיקודהקשר

PartsRoleTaskContextInputFormatConstraints

Role

אתה בודק JavaScript בכיר. אתה מדווח על בעיות אמיתיות ועל שום דבר אחר.

Task

בדוק את הקוד למטה, עם מיקוד בבאגים וטיפול בשגיאות.

Context

זה רץ בדפדפן. `ids` הוא מערך של מזהים מספריים מה-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

לכל ממצא: חומרה (גבוהה, בינונית או נמוכה), השורה, קלט או מצב שמפעיל אותו, ו-diff מינימלי. החמורים ביותר קודם.

Constraints

בלי הערות על סגנון, שמות או עיצוב. אם אתה לא בטוח שממצא אמיתי, אמור זאת ואמור מה יאשר אותו. אם לא מצאת כלום, כתוב "אין ממצאים".

Try it

Example replyReplies vary between models and runs.

1. גבוהה: הפונקציה חוזרת לפני שבקשה כלשהי מסתיימת (שורה 3) forEach לא מחכה ל-callbacks אסינכרוניים. loadAll מחזירה מיד את המערך results הריק, וה-push קורים מאוחר יותר. כל מי שקורא לה מקבל [].

2. בינונית: בקשות שנכשלו מטופלות כנתונים (שורה 5) תגובת 404 או 500 היא לא חריגה ב-fetch. גוף שגיאה ב-JSON מפוענח ונדחף כאילו היה פריט, וגוף שאינו JSON גורם ל-res.json() לזרוק חריגה בתוך callback שאף אחד לא מחכה לו.

תיקון לשניהם, שגם שומר את התוצאות באותו סדר כמו 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();
+    })
+  );
 }

שורת התפקיד קובעת סטנדרט ("בעיות אמיתיות ושום דבר אחר"), ושם role prompting עוזר הכי הרבה. אם תוכנה קוראת את הבדיקה, למשל סקריפט שמפרסם ממצאים כהערות על pull request, בקשו JSON עם שדות קבועים במקום פרוזה; פלט מובנה מראה איך.

להתמודד עם false positives

חלק מהממצאים יהיו שגויים. המודל לא יכול לראות את הקוד הקורא שמאמת את הקלט, את האילוץ במסד הנתונים שהופך מקרה לבלתי אפשרי, או את הסיבה ששורה שנראית מוזרה נכתבה כך בכוונה. הוא יכול גם לטעות לגבי ההתנהגות של ספרייה; הזיות של AI מסביר למה טענה בטוחה בעצמה היא לא טענה שנבדקה.

בדקו כל ממצא מול הקלט שמפעיל אותו. אם אתם לא מצליחים לייצר קלט שמגיע לבעיה במערכת שלכם, ערערו עם ההקשר החסר במקום לקבל את התיקון:

לערער על ממצא

EditCopy

סימנת את הפונקציה הזו כי `cents` עלול להיות undefined או מחרוזת: function formatPrice(cents) { return `$${(cents / 100).toFixed(2)}`; } היא נקראת רק מ-`renderCart`, שיוצאת מוקדם אלא אם `Number.isInteger(cents)` הוא true. האם זו עדיין בעיה? אם לא, בטל את הממצא.

Try it

Example replyReplies vary between models and runs.

עם הבדיקה הזו, לא: formatPrice מקבלת תמיד רק מספר שלם, כך שהממצא לא רלוונטי ואני מבטל אותו.

דבר אחד שהבדיקה לא מכסה: מספרים שלמים שליליים עוברים את Number.isInteger, ו-formatPrice(-500) מחזירה "$-5.00". אם החזרים או הנחות יכולים להגיע לפונקציה הזו, אולי תעדיפו "-$5.00". אם לא, אין מה לשנות.

מודל שמראים לו ראיה חדשה צריך או לבטל את הממצא או להסביר למה הוא עדיין תקף לאור הראיה הזו. שתי האפשרויות שימושיות. מודל שפשוט מסכים עם כל ערעור לא בודק, אז שפטו את התשובה לפי הנימוקים שלה, לא לפי אם היא הסכימה. לקוד שהמודל כתב בעצמו, הריצו את הבדיקה בשיחה חדשה, כדי שהבדיקה לא תיקרא דרך אותו נימוק שהפיק את הקוד. אותם הרגלים של היקף, הקשר ובדיקות משפרים את הקוד כבר מההתחלה; ראו פרומפטים לכתיבת קוד.