Skip to content

Two more suites carry the stale not-registered instruction #1087 removed from hilbert_cluster.sh #1088

Description

@OffgridwithJD

#1087 removed three stale instruction paragraphs from test/hilbert_cluster.sh because they told the next reader to undo work that had already been done. Two sibling files carry the same defect, and one of them is the file #1087's new docs cite as the source of their measurement.

Found by sweeping the class rather than the instance, after #1087 merged at fcfd3e6. Everything below is measured against author/main at that commit.

test/hilbert_locality.sh

# DELIBERATELY NOT REGISTERED in test/run_all_versions.sh yet. Registering it is
# part of the PR that lands it in the matrix; harness_selftest.sh sweeps
# test/*.sh and asserts every suite is registered, so this file makes that arm
# red until then.

Both claims are false on main:

claim measured
not registered in run_all_versions.sh it is registered (present in the SUITES array, 253 entries)
"makes that arm red until then" harness_selftest.sh is green: 967 passed + 0 failed + 0 unrunnable

This is the file docs/features.md, docs/how-to.md and docs/best-practices.md now cite for the 1.24x–2.04x Hilbert-vs-Z-order range (ROWS=200000). So the suite backing alpha4's headline documentation tells a reader it is not in the matrix and is expected to redden the selftest.

test/hilbert_curve.sh

# THIS SUITE IS NOT REGISTERED in test/run_all_versions.sh, so no CI run
# dispatches it. That is deliberate while src/columnar_curve.c does not exist
# and every battery arm is RED by design; registering it belongs in the commit
# that adds the encoder, together with the CHANGELOG entry. Until then
# test/selftest/070 reports it as UNREGISTERED, which is the accurate state.

Three claims, all false:

claim measured
not registered it is registered
src/columnar_curve.c does not exist it exists, 127 lines, and links into pgcolumnar.so
selftest/070 reports it UNREGISTERED, "which is the accurate state" it is not the accurate state

Why this matters more than a stale comment usually would

Both paragraphs are instructions, in the present tense, and both instruct the reader toward the state the tree has already left. hilbert_curve.sh goes further and tells them registering the suite "belongs in the commit that adds the encoder" — a commit that has landed. #1087's own replacement text put it better than I can:

A stale instruction is worse than a stale fact, because it tells the next person to undo what was done.

Suggested fix

Same shape as #1087's: replace each paragraph with what is true now, and keep whatever part of the original was a real trap worth preserving. Neither file needs a logic change — hilbert_cluster.sh's fix in #1087 was comment-only and these would be too.

Note on how this was found, because the sweep has a trap in it

The forward sweep (files claiming "NOT REGISTERED", checked against the real SUITES list) found both. A reverse sweep I wrote to cross-check — registered suites whose header says the selftest arm is red — found nothing, and that was my instrument failing, not evidence of absence: hilbert_locality.sh wraps the phrase across two comment lines as makes that arm / # red until then, so a line-oriented grep cannot see it. It only worked after joining the comment continuations.

Worth knowing for anyone writing a guard for this class: the claim to match on is the registration status, which is structural, not the prose, which wraps.

test/run_all_versions.sh also matches not registered, but that is its own reconciliation diagnostic ("listed as debt but not registered: $_n") and is correct. Not a finding.

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