מה למדתי על Code Review מהסוכנים של Qt
החברים ב Qt שחררו מסמך סקיל שמלמד את קלוד קוד איך לעשות Code Review לקוד Qt. אפשר לקרוא כאן ואני ממליץ:
https://github.com/TheQtCompanyRnD/agent-skills/blob/main/skills/qt-cpp-review/SKILL.md
הנקודה הראשונה הקריטית שהמסמך מזכיר לנו בעבודה עם סוכני קידוד היא החשיבות של הפרומפט. מי שרוצה לקבל Code Review ברמה גבוהה לא יכול לבקש "תעשה לי Code Review" או אפילו "תעשה לי Code Review ותחפש בעיות ביצועים". הסוכן צריך פרומפט שישים אותו בנקודת התחלה מדויקת לגבי מה אתם מחפשים.
דף ההוראות של Qt מתחיל בזיהוי על מה בכלל אנחנו מחפשים לעשות Code Review, כאשר יש הוראות שונות אם רוצים לעשות Code Review על קומיט מסוים או Code Review על המערכת כולה. ממשיכים בבקשה לניתוח סטטי של הקוד ואז מגיע החלק המעניין - הגדרת 6 סוכני Review, כל אחד עם התפקיד שלו.
יש סוכנים שאחראיים על חיפוש בעיות נפוצות בקלאס מסוים או אוסף קלאסים, למשל הסוכן הראשון מחפש רק בעיות ב QAbstractItemModel. סוכן שני מחפש בכל המערכת בעיות זכרון, ושימו לב להגדרה שלו שכוללת ממש רשימה של בעיות זכרון נפוצות ב Qt:
Check for:
Structs/classes with raw pointers where new is visible and no corresponding delete/deleteLater/smart-pointer wrapping exists (Rule of Five violation)
Missing deleteLater() on QNetworkReply in finished handlers
Q_ASSERT wrapping side-effectful expressions (compiled out in release builds — the side effect disappears)
Q_ASSERT as the sole null guard (crashes in release)
Polymorphic QObject subclasses missing Q_DISABLE_COPY_MOVE
Polymorphic classes missing virtual destructor
QTimer/QObject created with new but no parent and no other lifecycle management (scope, smart pointer, explicit delete)
QObject::connect() called with potentially null sender/receiver outside a null guard (runtime warning)
m_recentlyAccessed-style tracking lists that maintain pointers to objects that may be deleted elsewhere (dangling)
Unbounded container growth (append without cap or trim)
Destructor not cleaning up owned children recursively
Abstract interfaces with no implementations beyond one class (YAGNI violation — codebase scope only)
References: references/qt-review-checklist.md § Ownership & Lifecycle, § Polymorphic Classes, § RAII Classes
גם אם במבט ראשון רשימה כזאת נראית כתופסת יותר מדי מקום ב Context Window, מודלים חזקים היום יודעים להתמודד עם זה ואפילו ירוויחו מהפרומפט המאוד ספציפי.
סוכן שלישי ממשיך עם בעיות רוחביות ודואג לחפש דברים שקשורים ל Thread Safety, סוכן רביעי מתמקד בשמות, סגנון ובעיות בשפה, לדוגמה:
Missing const on methods that don't modify state
סוכן חמישי מוקדש לטיפול בשגיאות והשישי לביצועים ואיכות כללית של הקוד עם הוראות כמו:
Expensive operation before cheap early-exit check (wasted allocation)
Missing re-entrancy guard on methods that emit signals which could trigger re-entry
הביקורת שיש לי על רשימה כזאת היא לא בהכרח על הרשימה אלא על האנשים שישתמשו בה. רשימה מדויקת של הערות מעודדת את המודל למצוא את הבעיות ברשימה, גם אם מדובר ב False Positives, ולכן מפתחים שישתמשו ב Code Review מבוסס סוכנים עם רשימה ספציפית צריכים לשים לב:
לקרוא בצורה ביקורתית את הנקודות שהמודל מחזיר, להבין מה אמיתי ומה False Positive.
לקרוא בצורה ביקורתית את הקוד עצמו כדי למצוא נקודות שלא נמצאות ברשימה או שהמודל פספס.
לעדכן את הרשימה מדי פעם ככל שמגלים נקודות שלא עלו ב Code Reviews או נקודות שמעודדות False Positives. מוחקים מה שיצר טעויות ומוסיפים את מה שלא עלה.
אני גם לא בטוח ש Skill זה המנגנון הכי טוב לבנות דפי הנחיות כאלה. מצד אחד טוב שאפשר להתקין Skill-ים בקלות וכל הסוכנים אימצו תמיכה במנגנון. מצד שני אולי יהיה יותר פשוט ונכון להשתמש בקובץ Prompt ייעודי שיושב בתוך הפרויקט ומפתחים יודעים לקרוא אותו ולעדכן אותו.