review according to guide

GUIDE.md

@@ -0,0 +1,176 @@
+## コンポーネントファイル構造
+
+| Meta | Content |
+| --------- | ----------------------------------------------- |
+| Level | 必須 |
+| Link | https://angular.io/guide/styleguide#style-04-06 |
+| AI Review | OFF |
+
+### 説明
+
+各コンポーネントは独自のディレクトリを持ち、そのディレクトリ内にはコンポーネントのクラスファイル、テンプレート、スタイル、テストが含まれるべきです。
+
+### 理由
+
+コンポーネントの関連ファイルを一箇所に集めることで、コンポーネントの再利用性を高め、管理を容易にします。また、新しい開発者がプロジェクトに参加した際の学習コストを削減します。
+
+### 例
+
+```
+// 良いコードの例
+/src
+ /app
+ /button
+ button.component.ts
+ button.component.html
+ button.component.css
+ button.component.spec.ts
+
+// 悪いコードの例
+/src
+ /components
+ button.ts
+ /styles
+ button.css
+ /views
+ button.html
+```
+
+### 例外ケース
+
+- ユーティリティ関数やサービスなど、特定のコンポーネントに紐付かない共有ファイルは共通のディレクトリに配置する。
+
+### リファクタリングガイドライン
+
+既存プロジェクトでは、大幅なディレクトリ構造の変更を避け、新規コンポーネントからこの規則に従うこと。時間が許せば、徐々に既存のコンポーネントも新しいファイル構造に移行させる。
+
+---
+
+## 非同期操作の取り扱い
+
+| Meta | Content |
+| --------- | ------------------------------------------------------------------------------------------- |
+| Level | 必須 |
+| Link | https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/async_function |
+| AI Review | ON |
+
+### 説明
+
+非同期操作は`async/await`パターンを使用して実装すること。コールバックや`.then()`、`.catch()`メソッドを用いた Promise チェーンは避ける。
+
+### 理由
+
+`async/await`は非同期コードをより読みやすく、同期的な流れで記述できるためです。コードの可読性とメンテナンス性が向上します。
+
+### 例
+
+```typescript
+// 良いコードの例
+async function getUser(id: string): Promise<User> {
+ try {
+ const response = await fetch(`/api/users/${id}`);
+ const user = await response.json();
+ return user;
+ } catch (error) {
+ throw new Error(error);
+ }
+}
+
+// 悪いコードの例
+function getUser(id: string): Promise<User> {
+ return fetch(`/api/users/${id}`)
+ .then((response) => response.json())
+ .then((user) => user)
+ .catch((error) => {
+ throw new Error(error);
+ });
+}
+```
+
+### 例外ケース
+
+- 既存のコードベースで`Promise`チェーンが広範に使用されている場合は、段階的に`async/await`にリファクタリングする。
+
+### リファクタリングガイドライン
+
+新規プロジェクトや新規機能では`async/await`を用いること。既存のコードは、重要なバグ修正や機能追加の際に、この新しいパターンに徐々に置き換えていく。
+
+## 型定義の適用
+
+| Meta | Content |
+| --------- | ------------------------------------------------------------------ |
+| Level | 必須 |
+| Link | https://www.typescriptlang.org/docs/handbook/2/everyday-types.html |
+| AI Review | ON |
+
+### 説明
+
+全ての変数、関数の引数、および戻り値には明示的な型またはインターフェースを指定すること。
+
+### 理由
+
+型を明示的に宣言することで、コンパイル時の型チェックの恩恵を受けられ、ランタイムエラーのリスクを減らします。また、コードの意図が明確になり、他の開発者がコードを理解しやすくなります。
+
+### 例
+
+```typescript
+// 良いコードの例
+function add(x: number, y: number): number {
+ return x + y;
+}
+
+// 悪いコードの例
+function add(x, y) {
+ return x + y;
+}
+```
+
+### 例外ケース
+
+- ライブラリが提供する関数や変数の型が any である場合は、適宜型アサーションを使用する。
+
+### リファクタリングガイドライン
+
+既存のコードでは、任意の型が使われていた場合、それを具体的な型に置き換えていく。新規コードでは初めから型を適用する。
+
+---
+
+## ユニットテストの実施
+
+| Meta | Content |
+| --------- | -------------------------------------- |
+| Level | 推奨 |
+| Link | https://jestjs.io/docs/getting-started |
+| AI Review | OFF |
+
+### 説明
+
+新しく追加する全ての関数に対してユニットテストを書き、既存の関数についても段階的にテストを追加する。
+
+### 理由
+
+ユニットテストにより、コードの品質を保ち、将来的な機能追加やリファクタリングが安全に行われるようにします。バグの発見を早期に行い、修正コストを削減する効果もあります。
+
+### 例
+
+```typescript
+// 良いコードの例（テストの例）
+describe("add function", () => {
+ it("adds two numbers correctly", () => {
+ expect(add(1, 2)).toBe(3);
+ });
+});
+
+// 悪いコードの例（テストがない）
+function add(x: number, y: number): number {
+ return x + y;
+}
+```
+
+### 例外ケース
+
+- 外部 API の結果に依存する関数など、テストが困難なケースでは、モックを使用して依存関係を切り離す。
+
+### リファクタリングガイドライン
+
+テストがない既存の関数に対しては、新しいバグ報告や機能追加があるたびにユニットテストを追加していく。新規関数では最初からテストを書くことをルール化する。

packages/ai-craftsman/src/ci.ts

@@ -1,15 +1,27 @@
+import { readFileSync } from "node:fs";
+import { parseMarkdown, type MarkdownSectionNode } from "./markdown.js";
 import { getDiff, postComment } from "./git.js";
 import { splitForEachDIff } from "./diff.js";
 import { chat } from "./openai.js";
 import { promiseAllWithConcurrencyLimit } from "./concurrent.js";
 
 const prompt = `\
 あなたは世界最高峰のプログラマーです。あなたの仕事はGitHubのdiffを見てコードレビューをすることです。
-以下のdiffをレビューして改善点を挙げてください。改善点が特にない場合は 'NO' と回答してください。
+以下のdiffに対して以下のコーディングガイドに従っているかをレビューしてください。
+コーディングガイドに従っている場合は "OK" とコメントしてください。
+コーディングガイドに従っていない場合は改善方法をコメントしてください。
 diffの各行の先頭には行番号がついていることに注意してください。
-回答形式は以下です。
 
 ---
+
+コーディングガイドは以下です。
+
+{{CODING_GUIDE}}
+
+---
+
+回答形式は以下です。
+
 ## 改善点
 lines: {{指摘開始行}},{{指摘終了行}}
 {{ここに改善点を書いてください}}
@@ -19,13 +31,43 @@ lines: {{指摘開始行}},{{指摘終了行}}
 {{ここに改善点を書いてください}}
 `;
 
+const buildPrompt = (rule: string) => {
+ return prompt.replace("{{CODING_GUIDE}}", rule);
+};
+
 const excludePatterns = [
  /.*node_modules/,
  /.*pnpm-lock.yaml$/,
  /.*yarn.lock$/,
  /.*package-lock.json$/,
+ /.*\.md$/,
 ];
 
+const readRules = () => {
+ const getRule = (node: MarkdownSectionNode) => {
+ let rule = `${node.title}\n\n${node.content}`;
+ if (node.children.length > 0) {
+ for (const child of node.children) {
+ rule += getRule(child);
+ }
+ }
+ return rule;
+ };
+
+ const markdown = readFileSync("GUIDE.md", "utf-8");
+ const parsed = parseMarkdown(markdown);
+ const rules: string[] = [];
+ for (const level1 of parsed.children) {
+ for (const level2 of level1.children) {
+ const rule = getRule(level2);
+ if (rule.match(/AI Review.*ON/g)) {
+ rules.push(getRule(level2));
+ }
+ }
+ }
+ return rules;
+};
+
 const splitComments = (
  response: string
 ): { start: number; end: number; comment: string }[] => {
@@ -52,9 +94,9 @@ const splitComments = (
  const comments: { start: number; end: number; comment: string }[] = [];
  let buf = "";
  const lines =
- response.match(/.*?(##\s*改善点[\s\S]*)/m)?.[0].split("\n") ?? [];
+ response.match(/.*?((#+)?\s*改善点[\s\S]*)/m)?.[0].split("\n") ?? [];
  for (const line of lines) {
- if (line?.match(/^##\s*改善点/)) {
+ if (line?.match(/^(#+)?\s*改善点/)) {
  const comment = build(buf);
  if (comment != null) {
  comments.push(comment);
@@ -75,6 +117,7 @@ const main = async () => {
  throw new Error("BASE_REF is not set");
  }
 
+ const rules = readRules();
  const promises: (() => Promise<void>)[] = [];
  for (const { diff, path } of getDiff(baseref)) {
  if (excludePatterns.some((pattern) => pattern.test(path))) {
@@ -86,28 +129,35 @@ const main = async () => {
  console.log(diff);
  const chunked = splitForEachDIff(diff);
  for (const source of chunked) {
- promises.push(async () => {
- const response = await chat([
- {
- content: prompt,
- role: "system",
- },
- {
- content: source.diff,
- role: "user",
- },
- ]);
-
- console.log({ response });
- for (const comment of splitComments(response)) {
- console.log({ comment });
- postComment(path, comment.start, comment.end, comment.comment);
- }
- });
+ for (const rule of rules) {
+ promises.push(async () => {
+ const response = await chat([
+ {
+ content: buildPrompt(rule),
+ role: "system",
+ },
+ {
+ content: source.diff,
+ role: "user",
+ },
+ ]);
+
+ console.log({ prompt: buildPrompt(rule), response });
+ for (const comment of splitComments(response)) {
+ console.log({ comment });
+ postComment(
+ path,
+ comment.start,
+ comment.end,
+ `${comment.comment}\n\nReference: ${rule.split("\n")[0]}`
+ );
+ }
+ });
+ }
  }
  }
 
- await promiseAllWithConcurrencyLimit(promises, 10);
+ await promiseAllWithConcurrencyLimit(promises, 5);
 };
 
 void main();

packages/ai-craftsman/src/openai.ts

@@ -18,12 +18,18 @@ export const chat = async (messages: OpenAI.ChatCompletionMessageParam[]) => {
 
  const model =
  tokenCount * 1.1 + 1024 < 4 * 1024 ? "gpt-3.5-turbo" : "gpt-3.5-turbo-16k";
- const response = await openai.chat.completions.create({
- model,
- messages,
- temperature: 0,
- max_tokens: Math.min(1024, (16 * 1024 - tokenCount) * 0.9),
- });
+ const response = await openai.chat.completions.create(
+ {
+ model,
+ messages,
+ temperature: 0,
+ max_tokens: Math.trunc(Math.min(1024, (16 * 1024 - tokenCount) * 0.9)),
+ },
+ {
+ maxRetries: 2,
+ timeout: Math.trunc(Math.max(90, Math.min(1000 * 20, tokenCount / 10))),
+ }
+ );
 
  return response.choices[0]?.message?.content ?? "";
 };