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

The startup coverage line reports "off" for errorPattern, but legacy classification is still running #415

New Issue
Closed
opened 2026-09-10 04:40:02 +02:00 by ltms · 0 comments
ltms commented 2026-09-10 04:40:02 +02:00
Owner
Copy Link
Copy Source

The defect

CompletionResolver.coverage() measures pattern coverage — how many profiles set a key. The log
line it produces reads as feature state. For errorPattern those are not the same thing, and the
line is false:

02:11:19.609 INFO  dev.ltms.fleet.Fleetd - backend-error classification (fleetd #201 Unit 5):
    off (no profile has an errorPattern configured; profiles: [gx, local, opus, xf])

Backend-error classification is not off. Every profile without an errorPattern falls back to a
built-in pattern, CompletionResolver.java:84:

private static final Pattern BACKEND_ERROR = Pattern.compile("(?i)\\bAPI Error\\s*:");

Fleetd.java:389-391 states this at the call site:

a profile with no configured errorPattern is simply absent here, so CompletionResolver falls
back to its built-in narrow (?i)\bAPI Error\s*: compatibility pattern for that profile's targets

For exhaustedPattern the same word is literally true — no fallback exists, so unset really is off.
One string, two meanings, and the false one is the reassuring direction: it tells an operator a
live classifier is disabled when it is running.

Why one method produces a wrong answer

coverage() is generic and takes only the key name (CompletionResolver.java):

public static String coverage(String patternKey, Set<String> allProfiles, Set<String> configuredProfiles) {
    if (configuredProfiles.isEmpty()) {
        return "off (no profile has an " + patternKey + " configured; profiles: " + sorted(allProfiles) + ")";
    }
    ...
}

Nothing in its inputs says whether the key has a fallback, so it cannot word the two cases
differently. The asymmetry is known and documented — in a different file.
FleetConfig.java:2029-2032:

Unset/null means, for errorPattern, "use CompletionResolver's built-in (?i)\bAPI Error\s*:
compatibility pattern", and for exhaustedPattern, "opt out of that classification"

So the fact needed to make this line correct is written down 1600 lines away from the method that
gets it wrong. The shared helper is what made the two keys look interchangeable — the same trap as
#404, where a lambda was wrong because it copied a correct neighbour of identical shape.

Consequences, in order of how much they cost

  1. An operator reading "off" concludes there is no classification and stops looking. The narrow
    built-in pattern is still live and still able to misclassify: a member's own prose quoting an
    API Error: line can trip it, which is why BackendOutagePolicy requires the line twice
    (BackendOutagePolicy.java:26). Someone debugging a false quarantine would be told the mechanism
    they are looking at is disabled.
  2. It contradicts the Features wiki, which already records the asymmetry correctly — omitting
    errorPattern still classifies, omitting exhaustedPattern yields nothing. The wiki is right and
    the log disagrees with it.
  3. It hides the narrower true statement, which is the useful one: no profile has customised the
    pattern, so all of them use the built-in.

The goal

The line must distinguish "this classification is not running" from "this classification is running
with its built-in default". Both are legitimate states; only one is currently sayable.

coverage() needs to know what unset means for the key it is describing. Pass that in rather than
branching on the key's name inside the method — a string comparison on "errorPattern" would put the
same fact in a third place, which is how it got wrong in the first place.

Suggested wording, not prescribed:

backend-error classification: built-in default for all profiles
    (no profile customises errorPattern; profiles: [...])
backend-exhausted classification: off
    (no profile has an exhaustedPattern configured; profiles: [...])

Acceptance

  • With no errorPattern anywhere, the line does not say the classification is off, and names the
    built-in pattern as what is in use.
  • With no exhaustedPattern anywhere, the line still says off, because that is true.
  • partial and full still work for both keys.
  • A test per key for the empty case, asserting the two produce different wording. A test that
    only checks a line was emitted would pass today.
  • Whatever carries "what does unset mean" is a single source that both keys read. If a future third
    pattern key is added without supplying it, that should not compile — see #391, where deleting a
    permissive default turned a missed mutation into a compile error.

Related

  • #395 — the ticket that added exhaustionDetectionArmed. Its premise needs the same correction:
    the fact is logged at startup, it is just at INFO and not on a queryable surface. Commented there.
  • #404 — a shared shape making a wrong copy look safe.
  • #407 — these coverage reporters are log-only and unpinned; deleting a body leaves the suite
    green. Fixing the wording without pinning it leaves the next regression invisible.

Found by the fleet01 lead on their startup log. Confirmed independently in this tree:
CompletionResolver.java:84 (the fallback), Fleetd.java:389-391 (the call-site comment),
FleetConfig.java:2029-2032 (the documented asymmetry), and coverage() taking no fallback
argument.

## The defect `CompletionResolver.coverage()` measures **pattern coverage** — how many profiles set a key. The log line it produces reads as **feature state**. For `errorPattern` those are not the same thing, and the line is false: ``` 02:11:19.609 INFO dev.ltms.fleet.Fleetd - backend-error classification (fleetd #201 Unit 5): off (no profile has an errorPattern configured; profiles: [gx, local, opus, xf]) ``` **Backend-error classification is not off.** Every profile without an `errorPattern` falls back to a built-in pattern, `CompletionResolver.java:84`: ```java private static final Pattern BACKEND_ERROR = Pattern.compile("(?i)\\bAPI Error\\s*:"); ``` `Fleetd.java:389-391` states this at the call site: > a profile with no configured `errorPattern` is simply absent here, so `CompletionResolver` falls > back to its built-in narrow `(?i)\bAPI Error\s*:` compatibility pattern for that profile's targets For `exhaustedPattern` the same word is literally true — no fallback exists, so unset really is off. **One string, two meanings, and the false one is the reassuring direction:** it tells an operator a live classifier is disabled when it is running. ## Why one method produces a wrong answer `coverage()` is generic and takes only the key name (`CompletionResolver.java`): ```java public static String coverage(String patternKey, Set<String> allProfiles, Set<String> configuredProfiles) { if (configuredProfiles.isEmpty()) { return "off (no profile has an " + patternKey + " configured; profiles: " + sorted(allProfiles) + ")"; } ... } ``` Nothing in its inputs says whether the key has a fallback, so it cannot word the two cases differently. **The asymmetry is known and documented — in a different file.** `FleetConfig.java:2029-2032`: > Unset/`null` means, for `errorPattern`, "use `CompletionResolver`'s built-in `(?i)\bAPI Error\s*:` > compatibility pattern", and for `exhaustedPattern`, "opt out of that classification" So the fact needed to make this line correct is written down 1600 lines away from the method that gets it wrong. The shared helper is what made the two keys look interchangeable — the same trap as #404, where a lambda was wrong because it copied a correct neighbour of identical shape. ## Consequences, in order of how much they cost 1. **An operator reading "off" concludes there is no classification and stops looking.** The narrow built-in pattern is still live and still able to misclassify: a member's own prose quoting an `API Error:` line can trip it, which is why `BackendOutagePolicy` requires the line twice (`BackendOutagePolicy.java:26`). Someone debugging a false quarantine would be told the mechanism they are looking at is disabled. 2. **It contradicts the Features wiki**, which already records the asymmetry correctly — omitting `errorPattern` still classifies, omitting `exhaustedPattern` yields nothing. The wiki is right and the log disagrees with it. 3. It hides the narrower true statement, which is the useful one: *no profile has customised the pattern, so all of them use the built-in.* ## The goal The line must distinguish "this classification is not running" from "this classification is running with its built-in default". Both are legitimate states; only one is currently sayable. `coverage()` needs to know what unset means for the key it is describing. Pass that in rather than branching on the key's name inside the method — a string comparison on `"errorPattern"` would put the same fact in a third place, which is how it got wrong in the first place. Suggested wording, not prescribed: ``` backend-error classification: built-in default for all profiles (no profile customises errorPattern; profiles: [...]) backend-exhausted classification: off (no profile has an exhaustedPattern configured; profiles: [...]) ``` ## Acceptance - With no `errorPattern` anywhere, the line does **not** say the classification is off, and names the built-in pattern as what is in use. - With no `exhaustedPattern` anywhere, the line still says off, because that is true. - `partial` and `full` still work for both keys. - A test per key for the empty case, asserting the two produce **different** wording. A test that only checks a line was emitted would pass today. - Whatever carries "what does unset mean" is a single source that both keys read. If a future third pattern key is added without supplying it, that should not compile — see #391, where deleting a permissive default turned a missed mutation into a compile error. ## Related - **#395** — the ticket that added `exhaustionDetectionArmed`. Its premise needs the same correction: the fact is logged at startup, it is just at INFO and not on a queryable surface. Commented there. - **#404** — a shared shape making a wrong copy look safe. - **#407** — these coverage reporters are log-only and unpinned; deleting a body leaves the suite green. Fixing the wording without pinning it leaves the next regression invisible. Found by the fleet01 lead on their startup log. Confirmed independently in this tree: `CompletionResolver.java:84` (the fallback), `Fleetd.java:389-391` (the call-site comment), `FleetConfig.java:2029-2032` (the documented asymmetry), and `coverage()` taking no fallback argument.
ltms closed this issue 2026-09-10 06:57:46 +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#415
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?