Skip to content

hilbert_cluster.sh's header says it is red, unregistered and its verbs absent; all three landed in 4b66555 and the paragraph instructing it was left behind #1043

Description

@OffgridwithJD

test/hilbert_cluster.sh's header tells a reader the suite is red on purpose, that the two
verbs it tests do not exist, and that it is deliberately absent from the matrix. All three were
true when written and none is true now. The paragraph's own instruction was carried out, in
the commit it named, and the paragraph was left in place.

The three claims, and what is actually the case

test/hilbert_cluster.sh:132

THIS SUITE IS RED ON PURPOSE UNTIL #889's SQL HALF LANDS. Neither cluster_hilbert nor
recluster_hilbert exists yet

Both exist:

pgcolumnar--1.0-alpha4.sql:949   CREATE FUNCTION pgcolumnar.cluster_hilbert(
pgcolumnar--1.0-alpha4.sql:996   CREATE FUNCTION pgcolumnar.recluster_hilbert(

test/hilbert_cluster.sh:143

IT IS DELIBERATELY NOT REGISTERED in test/run_all_versions.sh

It is registered, at test/run_all_versions.sh:95.

test/hilbert_cluster.sh:148

REGISTERING THIS SUITE IN run_all_versions.sh IS PART OF THE PR THAT LANDS #889's SQL HALF,
in the same commit that turns the suite green.

That is exactly what happened. 4b66555 "feat: the Hilbert clustering verbs, and open the
1.0-alpha4 cycle (#889)"
both adds the verbs and registers the suite — one commit, as
instructed. And the suite is green: hilbert_cluster=PASS on both suites (PG 17) and
suites (PG 18) in CI, most recently on #1041's run at 3a640b0.

Why this is worth a line rather than a shrug

This paragraph is the one that tells a reader not to run the suite, and it works. I hit it
an hour ago while starting a pytest port of hilbert_cluster under #432 and nearly stopped on
it: a suite documented as red-on-purpose and unregistered is not a suite you port. The
classification in #432's Phase 1 comment
says it is registered, portable, and the largest unpaired suite by check volume at 133 checks —
so the tree's two statements about this file contradict each other, and the wrong one is the
one sitting at the top of the file itself.

It also mis-sizes the work for anyone reading it: "98 passed + 74 failed + 9 unrunnable = 181"
describes a tree from 2026-09-09.

The shape, which is the transferable part

The header did not go stale by neglect. It contained an instruction, the instruction was
followed, and nothing removes an instruction once it has been carried out.
4b66555 did the
two things the paragraph asked for and had no reason to look back at the paragraph that asked;
8ce398b touched the file four days later, for #982's premise renames, and had no reason to
read the header at all.

A comment that says "do X in the PR that does Y" is a to-do item living in a file that nobody
re-reads after doing it. It is the same class as the census recipe fixed in #1041: correct when
written, invalidated by a later change to the thing it describes, and nothing gates prose.

Suggested fix

Replace the three paragraphs with what is now true: the verbs shipped in 4b66555, the suite is
registered and green, and the arms that were red before the SQL half landed are the arms that
now pin it. Keeping a short record of the pre-landing state is worth it — the "read the 42883
arms first"
guidance is good advice for the next suite written ahead of its feature — but it
should read as history rather than as the present tense.

Not fixed here because it is a comment-only change to a file I have a port in flight against,
and the CHANGELOG anchor makes even a one-line PR collide with whatever else is open (#996).

🤖 Generated with Claude Code

https://claude.ai/code/session_012RSw4qMHS7ByE7PY8Ns4cs

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions