fleet/fleetd
3
0
Fork 0
Code Issues 80 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

FleetConfigValidateAllTest's class javadoc still narrates the change that created it #676

New Issue
Closed
opened 2026-10-03 20:55:32 +02:00 by ltms · 2 comments
ltms commented 2026-10-03 20:55:32 +02:00
Owner
Copy Link
Copy Source

Spotted while merging #668 (PR 674). Not caused by that change — pre-existing. I fixed the parts that #668 made false in 6f27522 and deliberately left this, because rewriting the class javadoc is a different change from the one being merged.

What is there

fleetd/src/test/java/dev/ltms/fleet/config/FleetConfigValidateAllTest.java opens by telling the story of the edit that produced the code, rather than describing the code:

  • :26 — "… six inside ConfigRef#reload(). This is a class of gap, not one line's mistake …"
  • :30 — "The fix replaces the six individual cfg.validateXxx() calls at each of the two real …"
  • :32 — "… rather than by a hand-maintained list of names. A hand-maintained list of six names …"
  • :71 — // ── Claim 1: the reflective sweep is a general mechanism, not six names in disguise ──

Under this project's comment rule that is history: the fix replaces, used to, before this. Git holds it, and the code is the current truth. The reasoning belongs in the commit message and the original MR.

The counts are the practical harm. "Six" was accurate when written. There are eight validators now, and the number will keep moving. Measured on main at 6f27522:

$ grep -oE 'public void validate[A-Za-z]+\(\)' fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java \
  | grep -v validateAll | sort -u | wc -l
8

Why it is worth a ticket rather than a shrug

This exact drift already cost something. The same file's claim-2 banner read "reaches seven of eight today", which #668's new case made plainly false; nobody noticed, because the change was twenty lines away and a diff review never shows an untouched line. Three more stale counts were sitting in the same file, one carrying (measured at review: 1491 tests, 0 failures) against a suite now at 1929.

That is the same failure this file was written to prevent, and the second time in one day it has bitten here.

What to do

Rewrite the class javadoc to describe what the tests protect, with no narrative of the change that introduced them:

  • Keep: what claim 1 proves, what claim 2 proves, and the division of labour between the canary and the reachability enumeration. That is a real constraint for a maintainer, and it is what stops the next person updating one and not the other.
  • Remove: the fix replaces, the account of what the code used to do, the measurement parentheticals, and every spelled-out count outside the one place the file deliberately keeps it.
  • Keep exactly one visible count, at :219, where the javadoc already explains that the number lives in the Set.of below so the two cannot drift apart. Everywhere else, say "every validator" and let the set be the denominator.

Acceptance criteria

  1. No comment in the file states a count of validators except the one at :219 that the surrounding sentence explains. Show the grep and a positive control, because a zero from a broken pattern reads exactly like a pass.
  2. No comment in the file contains a date, a test count, a ticket key, or a sentence in the past tense about what the code used to do.
  3. The division of labour between fleetConfigDeclaresExactlyTheseValidatorsToday() and validateAllReachesEveryOneOfTodaysRealValidators() is still stated somewhere a maintainer will read before editing either. Do not simply delete it — that constraint is the thing that stops #668 happening again.
  4. mvn clean install green in a throwaway worktree, never the main clone. Comment-only changes still need a build here: several tests in this module read source and doc files.

Not verified by me

Whether other test files in fleetd/src/test/java carry the same notebook-style javadoc. I looked at this one file because #668 took me into it. A sweep would probably find more, and that is a separate question from fixing this one.

Spotted while merging #668 (PR 674). Not caused by that change — pre-existing. I fixed the parts that #668 made *false* in `6f27522` and deliberately left this, because rewriting the class javadoc is a different change from the one being merged. ## What is there `fleetd/src/test/java/dev/ltms/fleet/config/FleetConfigValidateAllTest.java` opens by telling the story of the edit that produced the code, rather than describing the code: - `:26` — "… six inside `ConfigRef#reload()`. This is a class of gap, not one line's mistake …" - `:30` — "**The fix replaces** the six individual `cfg.validateXxx()` calls at each of the two real …" - `:32` — "… rather than by a hand-maintained list of names. A hand-maintained list of six names …" - `:71` — `// ── Claim 1: the reflective sweep is a general mechanism, not six names in disguise ──` Under this project's comment rule that is history: *the fix replaces*, *used to*, *before this*. Git holds it, and the code is the current truth. The reasoning belongs in the commit message and the original MR. The counts are the practical harm. "Six" was accurate when written. There are eight validators now, and the number will keep moving. Measured on `main` at `6f27522`: ``` $ grep -oE 'public void validate[A-Za-z]+\(\)' fleetd/src/main/java/dev/ltms/fleet/config/FleetConfig.java \ | grep -v validateAll | sort -u | wc -l 8 ``` ## Why it is worth a ticket rather than a shrug This exact drift already cost something. The same file's claim-2 banner read "reaches seven of eight today", which #668's new case made plainly false; nobody noticed, because the change was twenty lines away and a diff review never shows an untouched line. Three more stale counts were sitting in the same file, one carrying `(measured at review: 1491 tests, 0 failures)` against a suite now at 1929. That is the same failure this file was written to prevent, and the second time in one day it has bitten here. ## What to do Rewrite the class javadoc to describe what the tests protect, with no narrative of the change that introduced them: - Keep: what claim 1 proves, what claim 2 proves, and the division of labour between the canary and the reachability enumeration. That is a real constraint for a maintainer, and it is what stops the next person updating one and not the other. - Remove: *the fix replaces*, the account of what the code used to do, the measurement parentheticals, and every spelled-out count outside the one place the file deliberately keeps it. - Keep exactly one visible count, at `:219`, where the javadoc already explains that the number lives in the `Set.of` below so the two cannot drift apart. Everywhere else, say "every validator" and let the set be the denominator. ## Acceptance criteria 1. No comment in the file states a count of validators except the one at `:219` that the surrounding sentence explains. Show the grep and a positive control, because a zero from a broken pattern reads exactly like a pass. 2. No comment in the file contains a date, a test count, a ticket key, or a sentence in the past tense about what the code used to do. 3. The division of labour between `fleetConfigDeclaresExactlyTheseValidatorsToday()` and `validateAllReachesEveryOneOfTodaysRealValidators()` is still stated somewhere a maintainer will read before editing either. **Do not simply delete it** — that constraint is the thing that stops #668 happening again. 4. `mvn clean install` green in a throwaway worktree, never the main clone. Comment-only changes still need a build here: several tests in this module read source and doc files. ## Not verified by me Whether other test files in `fleetd/src/test/java` carry the same notebook-style javadoc. I looked at this one file because #668 took me into it. A sweep would probably find more, and that is a separate question from fixing this one.
ltms commented 2026-10-03 22:47:07 +02:00
Author
Owner
Copy Link
Copy Source

Mostly done in PR #691 (merged bfee23a). Staying open — one acceptance item is not met.

The javadoc rewrite rode along with #677's work. Three of the four criteria hold, and I checked each with the positive control this ticket asked for. One does not.

Criterion 1 — holds, with a control

Pattern (six|seven|eight|nine|[0-9]+) (of |individual |names|validator):

  • merged version: 0 matches
  • same pattern on the pre-merge version of the same file: 3 matches, at :30, :32 and :71 — the exact lines this ticket listed

The control fires, so the zero is a real pass and not a broken pattern.

The one deliberate count survived in the right place, and it is currently accurate:

 * above, on an unrelated class) — it is a visible denominator: today there are eight, named in

Live count on the merged tree: grep -oE 'public void validate[A-Za-z]+\(\)' … | grep -v validateAll | sort -u | wc -l = 8. Matches.

Criterion 2 — holds, with a control

Pattern (measured|used to|before this|no longer|previously|20[0-9]{2}-|[0-9]{3,} tests|CB-[0-9]+|#[0-9]{3}), case-insensitive: 0 matches on the merged file, 1 match on the pre-merge version. Control fires.

Criterion 3 — holds

The rewritten class javadoc keeps the division of labour: fleetConfigDeclaresExactlyTheseValidatorsToday() is named as "the canary for the validator set" and validateAllReachesEveryOneOfTodaysRealValidators() as "the reachability check for that set". Thinner than before, but the constraint a maintainer needs is stated and was not simply deleted.

Criterion 4 — holds

mvn -o clean install in a throwaway worktree after rm -rf target/surefire-reports: 1945 tests, 0 failures, BUILD SUCCESS, exit=0, 173 report files as the control.

What is left: a stale count survived inside a method name, and the class javadoc quotes it

:53   void theSweepMechanismIsGenericNotHardcodedToFleetConfigsSixNames() {

and the class javadoc now reads:

:22   * <p>{@link #theSweepMechanismIsGenericNotHardcodedToFleetConfigsSixNames()} and its neighbours

So "Six" is still a spelled-out count of FleetConfig's validators, it is wrong (there are eight), and the {@link} puts it back inside a comment — the one place criterion 1 says it must not be.

My own first grep missed this, because my pattern required a space before the noun and SixNames has none. I only caught it on a second, wider pass (grep -nEi '(six|seven|eight|nine)'). Worth recording next to this ticket's own warning about broken patterns: my pattern was not broken, it was too narrow, which reads identically.

This is the precise drift this ticket exists to stop. The count was right when written, the number keeps moving, and it now sits in an identifier where a diff review will never show it.

What finishes this

Rename the method so it states the property rather than a count — the property is that the sweep reads the target's method shape, not names it knows. Something like theSweepRunsEveryValidateMethodOnAnUnrelatedClass(). Then re-run both greps above with their controls, plus the wider (six|seven|eight|nine) pattern that caught this one.

Nothing else in the file needs touching, and the existing {@link} in the class javadoc updates with the rename.

## Mostly done in PR #691 (merged `bfee23a`). **Staying open — one acceptance item is not met.** The javadoc rewrite rode along with #677's work. Three of the four criteria hold, and I checked each with the positive control this ticket asked for. One does not. ### Criterion 1 — holds, with a control Pattern `(six|seven|eight|nine|[0-9]+) (of |individual |names|validator)`: - merged version: **0 matches** - same pattern on the pre-merge version of the same file: **3 matches**, at `:30`, `:32` and `:71` — the exact lines this ticket listed The control fires, so the zero is a real pass and not a broken pattern. The one deliberate count survived in the right place, and it is **currently accurate**: ``` * above, on an unrelated class) — it is a visible denominator: today there are eight, named in ``` Live count on the merged tree: `grep -oE 'public void validate[A-Za-z]+\(\)' … | grep -v validateAll | sort -u | wc -l` = **8**. Matches. ### Criterion 2 — holds, with a control Pattern `(measured|used to|before this|no longer|previously|20[0-9]{2}-|[0-9]{3,} tests|CB-[0-9]+|#[0-9]{3})`, case-insensitive: **0 matches** on the merged file, **1 match** on the pre-merge version. Control fires. ### Criterion 3 — holds The rewritten class javadoc keeps the division of labour: `fleetConfigDeclaresExactlyTheseValidatorsToday()` is named as "the canary for the validator set" and `validateAllReachesEveryOneOfTodaysRealValidators()` as "the reachability check for that set". Thinner than before, but the constraint a maintainer needs is stated and was not simply deleted. ### Criterion 4 — holds `mvn -o clean install` in a throwaway worktree after `rm -rf target/surefire-reports`: **1945 tests, 0 failures, BUILD SUCCESS**, `exit=0`, 173 report files as the control. --- ## What is left: a stale count survived inside a method name, and the class javadoc quotes it ```java :53 void theSweepMechanismIsGenericNotHardcodedToFleetConfigsSixNames() { ``` and the class javadoc now reads: ``` :22 * <p>{@link #theSweepMechanismIsGenericNotHardcodedToFleetConfigsSixNames()} and its neighbours ``` So "Six" is still a spelled-out count of `FleetConfig`'s validators, it is **wrong** (there are eight), and the `{@link}` puts it back inside a comment — the one place criterion 1 says it must not be. **My own first grep missed this**, because my pattern required a space before the noun and `SixNames` has none. I only caught it on a second, wider pass (`grep -nEi '(six|seven|eight|nine)'`). Worth recording next to this ticket's own warning about broken patterns: my pattern was not broken, it was too narrow, which reads identically. This is the precise drift this ticket exists to stop. The count was right when written, the number keeps moving, and it now sits in an identifier where a diff review will never show it. ### What finishes this Rename the method so it states the property rather than a count — the property is that the sweep reads the target's method shape, not names it knows. Something like `theSweepRunsEveryValidateMethodOnAnUnrelatedClass()`. Then re-run both greps above **with their controls**, plus the wider `(six|seven|eight|nine)` pattern that caught this one. Nothing else in the file needs touching, and the existing `{@link}` in the class javadoc updates with the rename.
ltms commented 2026-10-03 22:54:50 +02:00
Author
Owner
Copy Link
Copy Source

The last item is done. Closing.

PR #695 merged to main as d0688c8, pushed (bfee23a..d0688c8).

The method was renamed theSweepMechanismIsGenericNotHardcodedToFleetConfigsSixNames() → theSweepRunsEveryValidateMethodOnAnUnrelatedClass(), and the class javadoc's {@link} at :22 moved with it. Nothing else in the file was touched.

The new name states the property — the sweep reads the target's own method shape — instead of a count that has to be maintained. That removes the thing that drifted.

All four criteria now hold, each with its control, re-run by me on merged main

Criterion 1 — (six|seven|eight|nine|[0-9]+) (of |individual |names|validator): 0 on merged main, 3 on the pre-merge version (:30, :32, :71). Control fires.

Criterion 1, wider pattern — the one that caught this last item, (six|seven|eight|nine) case-insensitive:

173:     * above, on an unrelated class) — it is a visible denominator: today there are eight, named in

Exactly one match, and it is the deliberate denominator this ticket said to keep. Control: 11 matches on the pre-merge version. And the count is still correct — live validator count on merged main is 8.

Criterion 2 — (measured|used to|before this|no longer|previously|20[0-9]{2}-|[0-9]{3,} tests|CB-[0-9]+|#[0-9]{3}) case-insensitive: 0 on merged main, 1 on the pre-merge version. Control fires.

Criterion 3 — the division of labour survives: the class javadoc names fleetConfigDeclaresExactlyTheseValidatorsToday() as the canary and validateAllReachesEveryOneOfTodaysRealValidators() as the reachability check for that set. Not deleted.

Criterion 4 — mvn -o clean install in a throwaway worktree after rm -rf target/surefire-reports: 1946 tests, 0 failures, BUILD SUCCESS, exit=0, 173 *.xml report files. Merged tree hash 00a281044ae8bac3485e13e3c574be94933c7c0d equals the tree I built.

The lesson this ticket paid for twice

The first pass removed every stale count from the prose and left one inside a method name, where the class javadoc then quoted it through an {@link}. My first grep missed it because my pattern required a space before the noun and SixNames has none.

So: a pattern that is too narrow reads exactly like a clean file. This ticket already warned that a broken pattern reads like a pass; the narrower failure is the same shape and easier to walk into, because the pattern works — it just does not cover the case you care about. The fix both times was a second, wider pass.

Not done, and still open as a question

This ticket's own "not verified by me" stands: I did not sweep the rest of fleetd/src/test/java for the same notebook-style javadoc. The worker flagged it again from its side. If anyone wants that swept, it needs its own ticket — probably a hunter, since it is a multi-finding sweep rather than one diff.

## The last item is done. Closing. PR #695 merged to `main` as `d0688c8`, pushed (`bfee23a..d0688c8`). The method was renamed `theSweepMechanismIsGenericNotHardcodedToFleetConfigsSixNames()` → **`theSweepRunsEveryValidateMethodOnAnUnrelatedClass()`**, and the class javadoc's `{@link}` at `:22` moved with it. Nothing else in the file was touched. The new name states the property — the sweep reads the target's own method shape — instead of a count that has to be maintained. That removes the thing that drifted. ## All four criteria now hold, each with its control, re-run by me on merged `main` **Criterion 1** — `(six|seven|eight|nine|[0-9]+) (of |individual |names|validator)`: **0** on merged `main`, **3** on the pre-merge version (`:30`, `:32`, `:71`). Control fires. **Criterion 1, wider pattern** — the one that caught this last item, `(six|seven|eight|nine)` case-insensitive: ``` 173: * above, on an unrelated class) — it is a visible denominator: today there are eight, named in ``` **Exactly one match**, and it is the deliberate denominator this ticket said to keep. Control: **11** matches on the pre-merge version. And the count is still *correct* — live validator count on merged `main` is **8**. **Criterion 2** — `(measured|used to|before this|no longer|previously|20[0-9]{2}-|[0-9]{3,} tests|CB-[0-9]+|#[0-9]{3})` case-insensitive: **0** on merged `main`, **1** on the pre-merge version. Control fires. **Criterion 3** — the division of labour survives: the class javadoc names `fleetConfigDeclaresExactlyTheseValidatorsToday()` as the canary and `validateAllReachesEveryOneOfTodaysRealValidators()` as the reachability check for that set. Not deleted. **Criterion 4** — `mvn -o clean install` in a throwaway worktree after `rm -rf target/surefire-reports`: **1946 tests, 0 failures, BUILD SUCCESS**, `exit=0`, 173 `*.xml` report files. Merged tree hash `00a281044ae8bac3485e13e3c574be94933c7c0d` equals the tree I built. ## The lesson this ticket paid for twice The first pass removed every stale count from the prose and left one inside a method name, where the class javadoc then quoted it through an `{@link}`. My first grep missed it because my pattern required a space before the noun and `SixNames` has none. So: **a pattern that is too narrow reads exactly like a clean file.** This ticket already warned that a broken pattern reads like a pass; the narrower failure is the same shape and easier to walk into, because the pattern *works* — it just does not cover the case you care about. The fix both times was a second, wider pass. ## Not done, and still open as a question This ticket's own "not verified by me" stands: I did **not** sweep the rest of `fleetd/src/test/java` for the same notebook-style javadoc. The worker flagged it again from its side. If anyone wants that swept, it needs its own ticket — probably a `hunter`, since it is a multi-finding sweep rather than one diff.
ltms closed this issue 2026-10-03 22:54:53 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
blocked
Cannot start until something else lands. The body says what.
 needs-live-proof
Merged and green, but never shown working on the running daemon. Not the same as done.
 ready-to-delegate
Scope, files and acceptance criteria are written. A worker can be briefed from the body alone.
 silent-default
A feature that compiles, passes tests, and ships turned off. Nine recurrences and counting.
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agent kevin ltms
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#676
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?