導入
前回記事の続きです
ここまでで、Pull Requestを出すところまで完了しました。
この記事で伝えたいこと
- 生成AIの登場でOSSコントリビュートの敷居が下がって楽しくなってきたよ
- メンテナーからAIの知見を超えたレビューがもらえるよ(生成AI時代を生きるジュニアエンジニアにとってはとても貴重なもの)
コードレビューを受けて
リポジトリのメンテナーから以下のようなレビューをいただきました。
レビュー内容を把握するために、GPT4oに翻訳してもらったところ次のような内容でした
全てのエピソードを読み込む処理が非常に非効率です。なぜなら、何十万ものエピソードや何百ものポッドキャストがある場合、それぞれが追加のデータベースクエリとJavaオブジェクトへのパースを伴うからです。
統計情報を収集するためにすべてのエピソードをスキャンする必要がある場合、データベース側で直接処理した方がはるかに効率的です。そのため、統計データを生成しているSQLクエリを確認してもらえますか?
お恥ずかしながら、これは完全に初歩的なミスでした。
UI層にループを含むロジックを書くべきではないという基本を忘れていました。
僕がこのような実装をしてしまった理由は、対象のコードの周囲にも for 文が存在していたためです。
それを見て「このプロジェクトでは責務分離がそこまで厳格ではないのかも?」と、勝手に思い込んでしまったのです。
いわゆる“割れ窓理論”ってやつですね。今後は気をつけたいところです。
割れ窓理論とは
1枚の割れた窓ガラスを放置しておくと、さらに割られ、やがて街全体が荒れてしまうという理論。
実は、生成AIもこの“割れ窓”の影響を受けやすいと言われています。
周囲のコードに引っ張られる傾向があるため、指示を出す際にはその点にも気を配る必要がありそうです。
ちなみに、レビューに対する返信内容は以下のように日本語で書いた後、GPT4oに翻訳してもらっています。
レビューありがとうございます。
おっしゃる通り、こういった処理はUI層でやるべきではないですね。
SQLクエリ側で対応できるか確認してみます。改善できたらPRを更新します。
レビュー感謝です!
SQLクエリの修正へ
ということで、AIに指示を出しながらSQLの修正に取り掛かります。
DBReader.getStatistics() のSQLクエリを調査し、どこを修正すればよいかを洗い出してくれました。ナイス 👍
提案に乗ってそのまま調査を依頼します。
具体的な修正提案を受け、統計情報を計算するロジックをSQL側で処理することでパフォーマンス向上が見込めそうです。
ただし、気になるのは変更による影響範囲。
DBReader.getStatistics() は7箇所から呼ばれており、安全性が気になったのでAIに依頼して調査してもらいました。
結果、SQLのカラムを追加するだけで既存処理への影響はないとわかり、安心して進めることができました😊
SQLの修正とJava側の対応
ここからSQLの具体的な修正案を考えていくフェーズになります。
追加するカラムの実装イメージを出してくれました。
次のステップを確認したところ良さそうだったのでこのまま進みます。
PodDBAdapter.getFeedStatisticsCursor のSQLクエリに
「6ヶ月以内に未再生エピソードが1つでもあるか」を判定する has_recent_unplayed カラムを追加してくれました。
新しい引数 sixMonthsAgo を追加し、Java側から値を渡せるようにしてくれました。
Java側で受け取る処理を書いてくれました!期待通りです!
ただ、生成されたコードに不明点が残るのは避けたいので、率直に聞きます。
なるほど、これは後方互換用のコンストラクタとのこと。
既存コードを壊さずに、新しいフィールドを導入するための配慮だとわかりました。勉強になります。
ということで修正をpushしてレビューも完了!
これで、アプリの端末DB参照時のパフォーマンスはかなり改善されたはずです。
ここまでの実装まとめ:
SQLクエリ内で EXISTS を用いたサブクエリを使い、「各番組に、6ヶ月以内の未再生エピソードが存在するか」を判定し、has_recent_unplayed カラムとして返す形に。
さらなる改善提案(AIを超えた知見!)
しばらくして、メンテナーからさらに有益なコメントが届きました。
翻訳
このサブクエリを使う場合、おそらくテーブルを自己結合する必要があるでしょう。SUM(CASE WHEN ( AND ) THEN 1 ELSE 0 END) のような式を使えば、追加のテーブルスキャンをせずに「最近のエピソードの数」を求められるはずです。
非常にありがたいご指摘です。
メンテナーのおっしゃる通りで、サブクエリ(EXISTS)を使うと、番組(Feed)ごとに毎回エピソード(FeedItems)のテーブルを参照しに行くことになってしまいます。
一方、番組とエピソードをJOINした上で、すべてまとめて「条件に合う行だけを数える(SUM(CASE WHEN ...))」ようにすれば、1回のスキャンで済むためパフォーマンスが大きく改善されます。
この指摘、実はCursorで自動生成されたコードには含まれていませんでした。
つまり、AIを超えた知見と言えるでしょう!
さっそく修正に取り掛かり、
実装完了!再度pushした結果、無事マージされました🎉
おわりに
AIと人間の知見が補完し合うことで、はじめてのコントリビュートとしては非常に学びの多い体験になったと感じています。
「バイブコーディング × OSS」の可能性を、改めて実感できた今日この頃です。
もしこの内容が面白かったら、前編もぜひどうぞ!
今回お世話になったメンテナーの ByteHamster さん