Re: [PATCH 2/4] revision: expose check for paths maybe changed in Bloom filter

[Taylor Blau <ttaylorr@openai.com>]

On Fri, Jul 17, 2026 at 01:47:03PM -0700, Junio C Hamano wrote:
> >  	if (commit_graph_generation(commit) == GENERATION_NUMBER_INFINITY)
> >  		return -1;
> >
> >  	filter = get_bloom_filter(revs->repo, commit);
> > -

(This is an extreme nit-pick, but can we please try and avoid stray
changes like this? This one is not a huge deal, but it does make the
patch more difficult to read than necessary.)

> >  	if (!filter) {
> >  		count_bloom_filter_not_present++;
> >  		return -1;
> >  	}
> >
> > -	for (size_t nr = 0; !result && nr < revs->bloom_keyvecs_nr; nr++) {
> > -		result = bloom_filter_contains_vec(filter,
> > -						   revs->bloom_keyvecs[nr],
> > -						   revs->bloom_filter_settings);
> > -	}
> > +	result = revs_maybe_changed_in_bloom(revs, filter);
> > +	if (result < 0)
> > +		return result;
> >
> >  	if (result)
> >  		count_bloom_filter_maybe++;
>
> Doesn't this change skew the stats?

I believe so.

I had the same thinking, which is that without any key vectors, there is
no Bloom query to perform or account for, so that guard should stay
ahead of the generation and filter lookups.

> It could be that these two are intended "while at it we fix it too"
> improvements, but then they deserve to be mentioned in the proposed
> log message.  Personally, I think the first one that increments the
> _not_present statistics when keyvecs is empty a bug, though.

It seems separable. It may be worth fixing, but I would mention it
explicitly in the commit message.

Thanks,
Taylor