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

https://github.com/skorokithakis/stavrobot

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

נתחיל בהצגת קוד הפונקציה כמו שמצאתי אותה בפרויקט:

export function serializeMessagesForSummary(messages: AgentMessage[]): string {
  const lines: string[] = [];

  for (const message of messages) {
    if (message.role === "user") {
      let textContent: string;
      if (typeof message.content === "string") {
        textContent = message.content;
      } else {
        const content = Array.isArray(message.content) ? message.content : [];
        textContent = content
          .filter((block): block is TextContent => block.type === "text")
          .map((block) => block.text)
          .join("");
      }
      lines.push(`User: ${textContent}`);
    } else if (message.role === "assistant") {
      const content = Array.isArray(message.content) ? message.content : [];
      const textContent = content
        .filter((block): block is TextContent => block.type === "text")
        .map((block) => block.text)
        .join("");
      if (textContent) {
        lines.push(`Assistant: ${textContent}`);
      }
      for (const block of content) {
        if (block.type === "toolCall") {
          const toolCall = block as ToolCall;
          const args = Object.entries(toolCall.arguments)
            .map(([key, value]) => {
              if (typeof value === "string") {
                return `${key}=${JSON.stringify(value)}`;
              }
              if (typeof value === "object" && value !== null) {
                return `${key}=${JSON.stringify(value)}`;
              }
              return `${key}=${String(value)}`;
            })
            .join(", ");
          lines.push(`Assistant called ${toolCall.name}(${args})`);
        }
      }
    } else if (message.role === "toolResult") {
      const content = Array.isArray(message.content) ? message.content : [];
      const textContent = content
        .filter((block): block is TextContent => block.type === "text")
        .map((block) => block.text)
        .join("");
      lines.push(`Tool result (${message.toolName}): ${textContent}`);
    }
  }

  return lines.join("\n");
}

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

1. לולאה יוצרת במקום map

תבנית בעייתית ראשונה בקוד היא לולאת ה for. בדוגמה פשוטה שתי הלולאות האלה זהות:

const messages = [{text: 'one'}, {text: 'two'}, {text: 'three'}];

// 1. using for loop
const lines1 = [];
for (const message of messages) {
    lines1.push(message.text);
}

// 2. using map
const lines2 = messages.map(message => message.text);

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

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

const messages = [{text: 'one', times: 3}, {text: 'two'}, {text: 'three'}];

const lines1 = [];
for (const message of messages) {
  const times = message.times ?? 1;
  for (let i=0; i < times; i++) {
    lines1.push(message.text);
  }
}
console.log(lines1);

function serializeMessage(message) {
  const times = message.times ?? 1;
  return new Array(times).fill(message.text);
}

const lines2 = messages.flatMap(serializeMessage);
console.log(lines2);

שתי הלולאות זהות ואחריהן המשתנים lines1 ו lines2 מכילים את אותו מערך. ההבדלים הם באפשרות לשימוש חוזר ובקריאות. הלולאה השנייה מבטיחה לי שאני לוקח מערך ומחזיר מערך כנראה לא באותו גודל, זה לא קל כמו map אבל עדיין אני מבין מה קרה פה. יותר מזה, הגישה השניה השאירה אותי עם פונקציה serializeMessage בה אפשר להשתמש בנפרד מהלולאה.

נשווה את זה לקטע מתוך הפונקציה המקורית:

for (const message of messages) {
  if (message.role === "user") {
    let textContent: string;
    if (typeof message.content === "string") {
      textContent = message.content;
    } else {
      const content = Array.isArray(message.content) ? message.content : [];
      textContent = content
        .filter((block): block is TextContent => block.type === "text")
        .map((block) => block.text)
        .join("");
    }
    lines.push(`User: ${textContent}`);
  } else if (message.role === "assistant") {

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

תבנית טובה יותר לפונקציה היתה יכולה להיות:

function serializeAgentMessage(message: AgentMessage): string[] {
    switch(message.role) {
        case "user":
            return serializeUserMessage(message);
        case "assistant":
            return serializeAssistantMessage(message);
        case "toolResult":
            return serializeToolResult(message);
        default:
            throw new Error(`Unknown message role: ${message.role}`);

    }
}

export function serializeMessagesForSummary(messages: AgentMessage[]): string {
    return messages.flatMap(serializeAgentMessage).join("\n");
}

אנחנו רואים בפונקציה המקורית שחוסר הדיוק הוביל לכפל קוד והבלוק הזה מופיע מספר פעמים בפונקציה:

const textContent = content
  .filter((block): block is TextContent => block.type === "text")
  .map((block) => block.text)
  .join("");

2. טיפול לא מוסבר בכל המקרים האפשריים (ובמקרים לא אפשריים)

נקודה שניה שחוזרת המון בקוד AI היא תכנות דפנסיבי, כלומר קוד מהסוג הזה:

if (message.role === "user") {
  let textContent: string;
  if (typeof message.content === "string") {
    textContent = message.content;
  } else {
    const content = Array.isArray(message.content) ? message.content : [];
    textContent = content
      .filter((block): block is TextContent => block.type === "text")
      .map((block) => block.text)
      .join("");
  }

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

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

ספריית pi שממנה הגיע הטיפוס AgentMessage והיא שמגדירה את ההודעות השתמשה בשורה הבאה כדי למצוא תוכן טקסטואלי מהודעה:

const content =
    typeof this.message.content === "string"
        ? this.message.content
        : this.message.content.find((c) => c.type === "text")?.text || "";

חוץ מזה שהקוד קצר יותר הוא גם הרבה יותר מדויק. בזכות הדיוק הם לא צריכים את ה map וה join כדי לאחד את כל הבלוקים של הטקסט כי ממילא יש רק אחד.

3. חסר מעבר ניקיון

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

const args = Object.entries(toolCall.arguments)
  .map(([key, value]) => {
    if (typeof value === "string") {
      return `${key}=${JSON.stringify(value)}`;
    }
    if (typeof value === "object" && value !== null) {
      return `${key}=${JSON.stringify(value)}`;
    }
    return `${key}=${String(value)}`;
  })
  .join(", ");

על פניו יש פה שלושה מקרים כל אחד מייצר מחרוזת שונה, אבל בעצם שני המקרים הראשונים מכילים את אותו קוד לייצור מחרוזת ולא ברור באיזה מצבים המקרה השלישי אמור לטפל מאחר וארגומנטים של tool call תמיד ניתן להציג עם JSON.stringify. סך הכל ולמיטב הבנתי הקוד יהיה שקול ל:

const args = Object.entries(toolCall.arguments)
  .map(([key, value]) => `${key}=${JSON.stringify(value)}`)
  .join(", ");

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

4. שורה תחתונה

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

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