Hacktoberfest 2026: the issues maintainers tagged for October, open and beginner-friendly. Browse Hacktoberfest issues

Dynamic Review: incremental reviews re-fetch and re-inflate small blobs per commit

Open
  • Is something wrong?
#303 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
4/5
Estimated time
3-5 days
Newbie friendliness
25/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Active
Tech stack
git, typescript
Domain
devtools, performance

Research direction

Start in the object store: ObjectStore._inflate charges inflated bytes before ObjectStore.unpack checks whether the object id is already stored, and the checkout fetch requests are built from the three depth-1 fetches. Read docs/dynamic-checkout.md for the accounting claim. Done when the unit case T02 (the same object in two packs is stored once, with preparation bytes growing only on the first) passes, and the incremental-review integration case T01 prepares within budget.

Written by the indexing model from the issue text.

Description

area: runner type: bug

Follows #296 / PR #300 (review finding F1).

Problem

Since #296 the checkout fetches the head, the merge base and the previous reviewed commit as three independent depth-1 fetches, each with filter blob:limit=<file limit + 1> and no have lines. Each fetch therefore sends every blob up to the file limit of its own tree again, and ObjectStore._inflate charges the inflated bytes before put() can see that the object is already stored. Every re-sent blob costs transfer bytes and preparation bytes again, although it is stored once.

Measured with a synthetic repository (real upload-pack, incompressible binary files of up to 500 KiB plus text files, main and the head as siblings), same script before and after #296:

small blobs per tree mode before #296 after #296
40 MiB binary + 40 MiB text full prepares (186 MiB prep) prepares (195 MiB prep, 137 MiB transfer)
same incremental, previous commit is a sibling prepares fails budget_preparation
70 MiB binary + 40 MiB text incremental, previous commit is a sibling prepares (244 MiB prep) fails budget_transfer

A full review has the same capacity as before. An incremental review with a separate previous commit (force push, rebase) has a ceiling of about 72–85 MiB of small blobs per tree, against about 108 MiB before. #296 fixed the opposite failure (one very large blob), so this is a capacity trade-off for repositories with tens of MiB of small files.

Design

Either or both:

  1. Count a stored object once. ObjectStore.unpack does not charge (or refunds) the inflated bytes when the object id is already in the store. Charge by what is newly stored.
  2. Do not resend what is held. Send have <head> (with the shallow lines a depth-1 fetch needs) on the merge-base and previous fetches, or fetch only the blobs of the changed paths. Verify against real upload-pack and live GitHub, and keep the rule that no blob is ever wanted by name (a name-wanted blob ignores the size filter).

The docs ("counted once") are corrected to say what is counted once. The new checkout: log line is used to check real repositories after the change.

Test cases

ID kind level case expected
T01 regression integration (real upload-pack) incremental review, previous commit a sibling, about 90 MiB of small blobs per tree prepares; fails before the fix with budget_preparation or budget_transfer
T02 behavior unit (objects) the same object arrives in two packs stored once; preparation bytes grow only for the first
T03 behavior unit (protocol) merge-base and previous fetch requests do not ask again for objects the store holds (have), and still send no blob by name
T04 regression integration full review of the same repository metrics unchanged or lower than before the change
T05 behavior integration server that ignores have still correct, only larger; bounded by the transfer budget
T06 docs review docs/dynamic-checkout.md the accounting statement is accurate, including the per-fetch resend if it remains
Dominant language
TypeScript
Stars
0
Forks
1
Avg merge
1h 15m
Merged PRs (30d)
217

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from ExpidetGames/code-reviewer

All issues in ExpidetGames/code-reviewer

Similar issues

More TypeScript issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.