Re: [PATCH v7 10/10] commit-reach: remove commit-date ordering fallback

From: Kristofer Karlsson <krka@spotify.com>
Date: Aug 7, 2026, 12:05 UTC
Message-ID: <CAL71e4OTYDrbY-kjeJSa-iWtUN=96Q+2_vEB-_tScu_afPS6EA@mail.gmail.com>
In-Reply-To: <CABPp-BFJDc5t3cCj9mZBkXs+Ee+EoWUKw2p9UXq7tSGoosqMmw@mail.gmail.com>

On Fri, 7 Aug 2026 at 05:03, Elijah Newren <newren@gmail.com> wrote:

Show 11 quoted lines

>
> >
> > For v1 commit-graphs where generation numbers saturate at
> > GENERATION_NUMBER_V1_MAX, introduce a topological ceiling that
> > the early exit gates compare against instead of
> > GENERATION_NUMBER_INFINITY. This ensures saturated commits are
> > treated as unordered, preventing premature termination when
> > generation values are unreliable.
>
> Should the work associated with this paragraph come earlier so 8/10
> doesn't have its weird split?

I am thinking that I am mostly happy with the sequencing of the code changes, but aligning it with the documentation seems to be the messy part.

I am going to spend some time on reworking the documentation overall because I think that's the main remaining work and feedback on the code changes seem to have slowed down to effectively zero (which I'm happy about, though I've always found it easier to write correct code than useful documentation, so now the real work starts).

We could of course introduce the topo_ceiling already in patch 8, but there would be no behavior change since gen_ordered is disabled for v1 commit-graphs.

But let me know if you still want me to move that code change, I don't feel too strongly about it.

> Nice seeing all the date-ordering stuff get ripped out.

Agreed, since I am new to this I have ran into multiple gotchas when dealing with both v1 and v2, so the more we can unify the code paths, the better. And I suppose the performance improvement is a nice bonus (though people who still have v1 graphs and are using the latest git version should probably just switch to v2 instead).

> Good, together with the setting of state->topo_ceiling, this fixes the
> GENERATION_NUMBER_V1_MAX issue.

I did consider making this a utility function but I realized that this was the only part of the code where topological ordering mattered for correctness so it was nice that I could avoid that.

Show 5 quoted lines

> The code and tests look good, my main issue is that the documentation
> and code are not consistent at patch 08/10, so we need some way of
> correcting that.  I don't know whether that means splitting the code
> differently in patches 8 & 10, or splitting the documentation
> differently or something else.  Thoughts?

Yes, I will take a whole new stab at the documentation changes as a whole since a lot of things have changed in the code since v1. This potentially means a larger rewrite so it will likely take a few days days until I am happy enough with it.

> Anyway, nicely done overall, this is nearly ready to merge; it just
> needs a few small touch-ups.

Thanks, happy to hear that! Kristofer