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

On Fri, 7 Aug 2026 at 05:03, Elijah Newren <[email protected]> wrote:
>
> >
> > 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.

> 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