Dynamic Review: an incremental review with an ancestor as previous commit becomes a full review
メンテナーはふだん 1 日以内に返信
まだ誰も着手していません。
評価
- 難易度
- 4/5
- 見積もり時間
- 3〜5日
- 初心者へのやさしさ
- 35/100
- issue の種類
- バグ
- 明瞭さ
- 明確に書かれている
- 活発さ
- 活発
- 技術スタック
- git, typescript
- 領域
- backend
調査の方向性
Start in the handler that catches ObjectUnavailable from tracked_tree(previous) and sets previous = None, then follow the previous_reviewed_sha not in store.objects check that skips the fetch after the filter tree:0 ancestry walk. Reproduce with the history base -> c1 -> c2 and an incremental claim, using the integration tests that run against real upload-pack. Done means the ancestor case reports comparison == "incremental" with the right previous_diff, while the refused-server, unknown-commit and sibling cases keep their current behaviour.
索引モデルが issue の本文から書いたものです。
説明
Found while reviewing PR #300 (review finding F4). Pre-existing: it is not caused by #296.
Problem
An incremental review whose previous reviewed commit is an ancestor of the head, the usual case when new commits are pushed on top, silently becomes a full review. The commits-only ancestry walk (filter tree:0) already stores the previous commit, so if previous_reviewed_sha not in store.objects skips the fetch. tracked_tree(previous) then finds no tree, raises ObjectUnavailable, and the handler turns that into previous = None: comparison is full and previous_diff is empty.
Reproduced on the code before and after #296 with the history base -> c1 -> c2, previous = c1, base branch = base, claim incremental. The existing incremental tests only use a previous commit that is not an ancestor, so none of them covers this.
Design
- Decide whether the previous commit's tree is present, not whether the commit is: fetch the previous commit when its tree is absent (a depth-1 fetch with the size filter), then read the tree.
- Keep the existing fallback (previous commit explicitly refused by the server → full mode with the prior state kept).
- Add a test with a linear history.
Test cases
| ID | kind | level | case | expected |
|---|---|---|---|---|
| T01 | regression | integration (real upload-pack) | previous = ancestor of head | comparison == "incremental", previous_diff holds the change since the previous commit; red before the fix |
| T02 | regression | integration | previous = sibling of head | unchanged: incremental |
| T03 | behavior | integration | previous commit refused by the server | full mode, prior state kept |
| T04 | behavior | integration | previous commit unknown to the repository | full mode |
| T05 | behavior | unit (checkout) | the tree of the previous commit is absent but the commit is present | the fetch happens once |
- 主要言語
- TypeScript
- スター
- 0
- フォーク
- 1
- 平均マージ
- 1時間 15分
- マージ済み PR（30日）
- 217
環境構築
- Dockerfile または Docker Compose ファイルあり
- プルリクエストのテンプレートあり
- コントリビューションガイドを読む
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
ExpidetGames/code-reviewer のほかの issue
-
area: app type: bug
難易度 2/5 1〜3時間 初心者へのやさしさ 70/100
ExpidetGames/code-reviewer#308 ·
メンテナーはふだん 1 日以内に返信
-
type: docs
難易度 2/5 1〜3時間 初心者へのやさしさ 78/100
ExpidetGames/code-reviewer#94 ·
メンテナーはふだん 1 日以内に返信
-
難易度 4/5 3〜5日 初心者へのやさしさ 25/100
ExpidetGames/code-reviewer#401 ·
メンテナーはふだん 1 日以内に返信
-
area: app type: chore
難易度 3/5 1〜2日 初心者へのやさしさ 45/100
ExpidetGames/code-reviewer#393 ·
メンテナーはふだん 1 日以内に返信
-
area: runner type: bug
難易度 4/5 3〜5日 初心者へのやさしさ 42/100
ExpidetGames/code-reviewer#390 ·
メンテナーはふだん 1 日以内に返信
ExpidetGames/code-reviewer の issue をすべて見る
似ている issue
-
難易度 1/5 1時間未満 初心者へのやさしさ 83/100
-
難易度 2/5 1〜3時間 初心者へのやさしさ 72/100
ranxianglei/billion-context#3583 ·
メンテナーはふだん 1 日以内に返信
-
bug:new
難易度 2/5 1〜3時間 初心者へのやさしさ 76/100
callstackincubator/simlock#673 ·
メンテナーはふだん 1 日以内に返信
-
documentation v3
難易度 2/5 1〜3時間 初心者へのやさしさ 73/100
メンテナーはふだん 1 日以内に返信
-
難易度 2/5 1〜3時間 初心者へのやさしさ 73/100
anomalyco/opencode#56380 · コメント 1 件 ·
メンテナーはふだん 1 日以内に返信