From ce05886831c8ed26119deebe054fd791163e97e6 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Thu, 10 Sep 2026 11:54:02 +0700 Subject: [PATCH] fleetd #415: pin which UnsetMeaning Fleetd pairs with which pattern key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found a gap: the earlier tests all called CompletionResolver.coverage() directly, supplying the UnsetMeaning themselves — proving the enum's wording, never that Fleetd's two call sites pair the right meaning with the right key. Swapping the two UnsetMeaning arguments at those call sites (recreating #415's defect with exhaustedPattern and errorPattern exchanged) compiled with 0 errors and left all 1506 tests green. Extract the two coverage-line call sites out of main() into package-private static factories (Fleetd.exhaustedPatternCoverageLine / errorPatternCoverageLine), the same pattern already used for capacitySource and worktreeBranchLookup. Add FleetdPatternCoverageLineTest, which calls both factories directly and asserts the actual wording each produces for the same empty-coverage input, including that the two differ. Also recorded the swap-mutation measurement (0 errors, 1506 green) in UnsetMeaning's javadoc so a future reader does not delete the new test as redundant with CompletionResolverTest. --- .../src/main/java/dev/ltms/fleet/Fleetd.java | 41 ++++++++++-- .../ltms/fleet/inject/CompletionResolver.java | 10 +++ .../fleet/FleetdPatternCoverageLineTest.java | 67 +++++++++++++++++++ 3 files changed, 114 insertions(+), 4 deletions(-) create mode 100644 fleetd/src/test/java/dev/ltms/fleet/FleetdPatternCoverageLineTest.java diff --git a/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java b/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java index 30a7081..7181b7a 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java +++ b/fleetd/src/main/java/dev/ltms/fleet/Fleetd.java @@ -380,8 +380,7 @@ public final class Fleetd { .map(session -> exhaustedPatternsByProfile.get(session.profile())) .orElse(null); log.info("backend-exhausted classification (CB-578 stage A): {}", - CompletionResolver.coverage("exhaustedPattern", CompletionResolver.UnsetMeaning.OFF, - cfg.profiles().keySet(), exhaustedPatternsByProfile.keySet())); + exhaustedPatternCoverageLine(cfg.profiles().keySet(), exhaustedPatternsByProfile.keySet())); // fleetd #201 Unit 5: classify a completion-fallback scrape that matches a profile's // configured backend-error refusal (a credential outage, a provider 5xx) as a backend error // rather than handing it back as a real answer. Compiled once at startup, keyed by profile @@ -401,8 +400,7 @@ public final class Fleetd { BackendErrorPatternLookup backendErrorPatterns = backendErrorPatternLookup(sessions::roster, errorPatternsByProfile); log.info("backend-error classification (fleetd #201 Unit 5): {}", - CompletionResolver.coverage("errorPattern", CompletionResolver.UnsetMeaning.BUILT_IN_DEFAULT, - cfg.profiles().keySet(), errorPatternsByProfile.keySet())); + errorPatternCoverageLine(cfg.profiles().keySet(), errorPatternsByProfile.keySet())); // CB-578 stage B: on a classification that actually wins, quarantine the exhausted profile's // CREDENTIAL — not the profile name — so a profile sharing that credential (e.g. two models // on one OpenAI account) is refused too, not just the one that happened to report it. Reads @@ -807,6 +805,41 @@ public final class Fleetd { }, quarantine, profile -> startupExhaustedPatterns.containsKey(profile)); } + /** + * fleetd #415 (review follow-up): package-private factory for the CB-578 stage A {@code + * exhaustedPattern} startup coverage line, paired explicitly with {@link + * CompletionResolver.UnsetMeaning#OFF} — {@code exhaustedPattern} has no fallback, so a + * profile with none configured really does have the classification off. + * + *

Extracted out of {@code main} for the same reason {@link #capacitySource} and {@link + * #worktreeBranchLookup} were: {@code coverage()}'s own tests ({@code CompletionResolverTest}) + * prove it words {@code OFF} and {@link CompletionResolver.UnsetMeaning#BUILT_IN_DEFAULT} + * correctly when a test supplies the meaning itself — they cannot prove {@code main} pairs the + * right meaning with the right key, which is the actual fleetd #415 defect. Measured: + * swapping the {@code UnsetMeaning} arguments between this method and {@link + * #errorPatternCoverageLine} — recreating #415's defect with the two keys exchanged — compiled + * with 0 errors and left all 1506 existing tests green before {@code + * FleetdPatternCoverageLineTest} was added to catch exactly that swap. + */ + static String exhaustedPatternCoverageLine(Set allProfiles, Set configuredProfiles) { + return CompletionResolver.coverage("exhaustedPattern", CompletionResolver.UnsetMeaning.OFF, + allProfiles, configuredProfiles); + } + + /** + * fleetd #415 (review follow-up): the {@code errorPattern} counterpart of {@link + * #exhaustedPatternCoverageLine}, paired explicitly with {@link + * CompletionResolver.UnsetMeaning#BUILT_IN_DEFAULT} — an unset {@code errorPattern} still runs + * backend-error classification against {@code CompletionResolver}'s built-in {@code + * BACKEND_ERROR} pattern, so the empty case is not "off". See {@link + * #exhaustedPatternCoverageLine}'s javadoc for the measured swap mutation this pairing guards + * against. + */ + static String errorPatternCoverageLine(Set allProfiles, Set configuredProfiles) { + return CompletionResolver.coverage("errorPattern", CompletionResolver.UnsetMeaning.BUILT_IN_DEFAULT, + allProfiles, configuredProfiles); + } + /** * fleetd #416: production source for {@code fleet_list}'s per-profile capacity facts. * diff --git a/fleetd/src/main/java/dev/ltms/fleet/inject/CompletionResolver.java b/fleetd/src/main/java/dev/ltms/fleet/inject/CompletionResolver.java index 697569d..d1faa05 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/inject/CompletionResolver.java +++ b/fleetd/src/main/java/dev/ltms/fleet/inject/CompletionResolver.java @@ -702,6 +702,16 @@ public final class CompletionResolver implements TurnListener { * {@code coverage()} cannot infer this from the key's name — the two keys it currently * describes disagree on it, and a string comparison on the name would just move the same bug * to a new spot — so every caller must state it explicitly. + * + *

This alone does not prove a caller passes the right one for its key. A + * test that calls {@code coverage()} directly and supplies the meaning itself only proves this + * enum is worded correctly, never that {@code Fleetd}'s two call sites pair each key with its + * true meaning — that pairing is #415's actual defect. Measured on review: swapping the two + * {@code UnsetMeaning} arguments at those call sites (giving {@code exhaustedPattern} the + * built-in-default wording and {@code errorPattern} the off wording — #415's exact defect with + * the keys exchanged) compiled with 0 errors and left all 1506 existing tests green. See + * {@code dev.ltms.fleet.Fleetd#exhaustedPatternCoverageLine}/{@code #errorPatternCoverageLine} + * and {@code FleetdPatternCoverageLineTest}, which exists specifically to catch that swap. */ public enum UnsetMeaning { /** No fallback exists: a profile with no configured pattern truly has this classification off. */ diff --git a/fleetd/src/test/java/dev/ltms/fleet/FleetdPatternCoverageLineTest.java b/fleetd/src/test/java/dev/ltms/fleet/FleetdPatternCoverageLineTest.java new file mode 100644 index 0000000..fdc7edb --- /dev/null +++ b/fleetd/src/test/java/dev/ltms/fleet/FleetdPatternCoverageLineTest.java @@ -0,0 +1,67 @@ +package dev.ltms.fleet; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.util.Set; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; + +/** + * fleetd #415 (review follow-up): {@code CompletionResolverTest} proves {@code coverage()} words + * {@code UnsetMeaning.OFF} and {@code UnsetMeaning.BUILT_IN_DEFAULT} correctly — but every one of + * those tests supplies the meaning itself. That proves the enum's wording, never that {@code + * Fleetd} pairs the right meaning with the right pattern key. That pairing is #415's actual + * defect: {@code coverage()} had no way to know what unset meant for its key, so the fix moved + * the fact to the caller — and nothing yet proved the caller states it correctly. + * + *

Measured or it didn't happen: swapping the two {@code UnsetMeaning} arguments at + * {@code Fleetd}'s two coverage call sites — giving {@code exhaustedPattern} the built-in-default + * wording and {@code errorPattern} the off wording, #415's exact defect with the keys exchanged — + * compiled with 0 errors and left all 1506 existing tests green. This class exists to turn that + * swap red. + * + *

It calls {@link Fleetd#exhaustedPatternCoverageLine} and {@link Fleetd#errorPatternCoverageLine} + * directly rather than reading {@code Fleetd.java} as source text (the shape {@code + * FleetdCompletionResolverWiringTest} uses for a different wiring gap): those two methods are the + * extracted call sites {@code main} actually invokes, following the same {@code static} factory + + * dedicated-test pattern as {@link Fleetd#capacitySource} and {@link Fleetd#worktreeBranchLookup}. + */ +class FleetdPatternCoverageLineTest { + + private static final Set PROFILES = Set.of("terra", "gx10"); + + @Test + @DisplayName("exhaustedPatternCoverageLine says off when no profile configures exhaustedPattern") + void exhaustedPatternCoverageLineSaysOffWhenNoProfileConfiguresIt() { + assertEquals("off (no profile has an exhaustedPattern configured; profiles: [gx10, terra])", + Fleetd.exhaustedPatternCoverageLine(PROFILES, Set.of())); + } + + @Test + @DisplayName("errorPatternCoverageLine says built-in default when no profile configures errorPattern") + void errorPatternCoverageLineSaysBuiltInDefaultWhenNoProfileConfiguresIt() { + assertEquals("built-in default for all profiles (no profile customises errorPattern; " + + "profiles: [gx10, terra])", + Fleetd.errorPatternCoverageLine(PROFILES, Set.of())); + } + + @Test + @DisplayName("the two keys produce different wording for the identical empty-coverage input") + void theTwoKeysProduceDifferentWordingForTheSameEmptyInput() { + String exhaustedLine = Fleetd.exhaustedPatternCoverageLine(PROFILES, Set.of()); + String errorLine = Fleetd.errorPatternCoverageLine(PROFILES, Set.of()); + + // Pinned individually above; restated here so this test alone still catches a swap even + // if one of the two tests above were ever deleted. + assertEquals("off (no profile has an exhaustedPattern configured; profiles: [gx10, terra])", + exhaustedLine); + assertEquals("built-in default for all profiles (no profile customises errorPattern; " + + "profiles: [gx10, terra])", errorLine); + assertNotEquals(exhaustedLine, errorLine, + "swapping which UnsetMeaning pairs with which pattern key at Fleetd's call sites " + + "must be caught here — that pairing, not coverage()'s own wording in isolation, " + + "is fleetd #415's actual defect"); + } +}