2
5

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

バイブコーディングでOSSコントリビュートができた話(後編)

Posted at

導入

前回記事の続きです
ここまでで、Pull Requestを出すところまで完了しました。

この記事で伝えたいこと

  1. 生成AIの登場でOSSコントリビュートの敷居が下がって楽しくなってきたよ
  2. メンテナーからAIの知見を超えたレビューがもらえるよ(生成AI時代を生きるジュニアエンジニアにとってはとても貴重なもの)

コードレビューを受けて

リポジトリのメンテナーから以下のようなレビューをいただきました。

image.png

レビュー内容を把握するために、GPT4oに翻訳してもらったところ次のような内容でした

全てのエピソードを読み込む処理が非常に非効率です。なぜなら、何十万ものエピソードや何百ものポッドキャストがある場合、それぞれが追加のデータベースクエリとJavaオブジェクトへのパースを伴うからです。
統計情報を収集するためにすべてのエピソードをスキャンする必要がある場合、データベース側で直接処理した方がはるかに効率的です。そのため、統計データを生成しているSQLクエリを確認してもらえますか?

お恥ずかしながら、これは完全に初歩的なミスでした。
UI層にループを含むロジックを書くべきではないという基本を忘れていました。

僕がこのような実装をしてしまった理由は、対象のコードの周囲にも for 文が存在していたためです。
それを見て「このプロジェクトでは責務分離がそこまで厳格ではないのかも?」と、勝手に思い込んでしまったのです。

いわゆる“割れ窓理論”ってやつですね。今後は気をつけたいところです。

割れ窓理論とは
1枚の割れた窓ガラスを放置しておくと、さらに割られ、やがて街全体が荒れてしまうという理論。

実は、生成AIもこの“割れ窓”の影響を受けやすいと言われています。
周囲のコードに引っ張られる傾向があるため、指示を出す際にはその点にも気を配る必要がありそうです。

ちなみに、レビューに対する返信内容は以下のように日本語で書いた後、GPT4oに翻訳してもらっています。

レビューありがとうございます。
おっしゃる通り、こういった処理はUI層でやるべきではないですね。
SQLクエリ側で対応できるか確認してみます。改善できたらPRを更新します。
レビュー感謝です!

image.png

SQLクエリの修正へ

ということで、AIに指示を出しながらSQLの修正に取り掛かります。

image.png

image.png

DBReader.getStatistics() のSQLクエリを調査し、どこを修正すればよいかを洗い出してくれました。ナイス 👍

image.png

提案に乗ってそのまま調査を依頼します。

image.png

image.png

具体的な修正提案を受け、統計情報を計算するロジックをSQL側で処理することでパフォーマンス向上が見込めそうです。


ただし、気になるのは変更による影響範囲
DBReader.getStatistics() は7箇所から呼ばれており、安全性が気になったのでAIに依頼して調査してもらいました。

image.png

結果、SQLのカラムを追加するだけで既存処理への影響はないとわかり、安心して進めることができました😊

SQLの修正とJava側の対応

ここからSQLの具体的な修正案を考えていくフェーズになります。

image.png

image.png

追加するカラムの実装イメージを出してくれました。
次のステップを確認したところ良さそうだったのでこのまま進みます。

image.png

PodDBAdapter.getFeedStatisticsCursor のSQLクエリに
「6ヶ月以内に未再生エピソードが1つでもあるか」を判定する has_recent_unplayed カラムを追加してくれました。
新しい引数 sixMonthsAgo を追加し、Java側から値を渡せるようにしてくれました。

image.png

Java側で受け取る処理を書いてくれました!期待通りです!


ただ、生成されたコードに不明点が残るのは避けたいので、率直に聞きます。

image.png

image.png

なるほど、これは後方互換用のコンストラクタとのこと。
既存コードを壊さずに、新しいフィールドを導入するための配慮だとわかりました。勉強になります。

ということで修正をpushしてレビューも完了!
これで、アプリの端末DB参照時のパフォーマンスはかなり改善されたはずです。

ここまでの実装まとめ:
SQLクエリ内で EXISTS を用いたサブクエリを使い、「各番組に、6ヶ月以内の未再生エピソードが存在するか」を判定し、has_recent_unplayed カラムとして返す形に。

さらなる改善提案(AIを超えた知見!)

しばらくして、メンテナーからさらに有益なコメントが届きました。

image.png

翻訳
このサブクエリを使う場合、おそらくテーブルを自己結合する必要があるでしょう。SUM(CASE WHEN ( AND ) THEN 1 ELSE 0 END) のような式を使えば、追加のテーブルスキャンをせずに「最近のエピソードの数」を求められるはずです。

非常にありがたいご指摘です。
メンテナーのおっしゃる通りで、サブクエリ(EXISTS)を使うと、番組(Feed)ごとに毎回エピソード(FeedItems)のテーブルを参照しに行くことになってしまいます。

一方、番組とエピソードをJOINした上で、すべてまとめて「条件に合う行だけを数える(SUM(CASE WHEN ...))」ようにすれば、1回のスキャンで済むためパフォーマンスが大きく改善されます。

この指摘、実はCursorで自動生成されたコードには含まれていませんでした。
つまり、AIを超えた知見と言えるでしょう!


さっそく修正に取り掛かり、

image.png

image.png

image.png

image.png

image.png

実装完了!再度pushした結果、無事マージされました🎉

image.png

おわりに

AIと人間の知見が補完し合うことで、はじめてのコントリビュートとしては非常に学びの多い体験になったと感じています。
「バイブコーディング × OSS」の可能性を、改めて実感できた今日この頃です。

もしこの内容が面白かったら、前編もぜひどうぞ!

今回お世話になったメンテナーの ByteHamster さん

2
5
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
2
5

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?