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

CB-554: weight: 0 does not exclude a profile from placement — it means 1.0, so architect profiles outweigh workers #28

New Issue
Closed
opened 2026-08-13 21:18:15 +02:00 by ltms · 1 comment
ltms commented 2026-08-13 21:18:15 +02:00
Owner
Copy Link
Copy Source

Symptom

Measured against the live bridged.yaml, under placement: weighted:

gx10    weight=0.5  -> 17%
terra   weight=0.5  -> 17%
opus    weight=1.0  -> 33%   *** BILLS THE CLAUDE SUBSCRIPTION ***
sol     weight=1.0  -> 33%

Both architect profiles are written weight: 0 in config. A third of every unqualified spawn
would land on the operator's Claude subscription.

Root cause

BridgedConfig.Worker's compact constructor:

weight = (weight == null || weight <= 0.0f) ? 1.0f : weight;

A non-positive weight is normalised to 1.0, not treated as exclusion. So weight: 0 — the
value an operator would naturally reach for to mean "never pick this" — produces the heaviest
candidate in a fleet whose real workers sit at 0.5.

The javadoc does say "Absent or non-positive ⇒ 1.0", so the code matches its own contract. The
defect is that there is no exclusion mechanism at all, and the most obvious-looking spelling
of one silently does the opposite of what it reads as.

The second door is shut the same way:

maxLoad = (maxLoad == null || maxLoad <= 0) ? null : maxLoad;

maxLoad: 0 normalises to null = unlimited, so it cannot express "never" either.

Why it matters

This is the mechanism the config relies on to keep unqualified spawns off two paid subscriptions
(opus on Claude, sol on the ChatGPT oauth). A long comment block in bridged.yaml asserted
confidently that weight 0 made an unqualified spawn "never" land there. It was false for as long
as it has existed — with the earlier single sonnet architect profile the share was 50%.

Nothing observable failed, which is precisely the problem: the failure mode is a silent, correct-
looking config that bills a subscription.

Fix — pick one, deliberately

  1. Honour weight: 0 as exclusion. Most intuitive, matches how every operator will read it.
    Breaks the documented "non-positive ⇒ 1.0" contract, so the javadoc and any test asserting it
    must change together.
  2. Add an explicit placementEligible: false (or role: architect) and leave weight
    normalisation alone. More verbose but unambiguous, and it separates "how much" from "whether".

Option 2 is probably right — "weight 0" and "not a candidate" are genuinely different statements,
and conflating them is what produced this. But whichever is chosen, weight: 0 must stop meaning
1.0: leaving that as a silent trap is worse than either fix.

Whichever lands, placement must fail loudly when every eligible candidate is exhausted rather
than falling back onto an ineligible profile.

Current mitigation (in place)

placement: fixed in bridged.yaml, which ignores weights entirely and routes every unqualified
spawn to defaultWorker (gx10). Cost: unqualified spawns no longer split across gx10/terra. Cheap,
because the charter has every lead name a profile explicitly, and explicit spawns bypass placement.

Restoring placement: weighted before this ticket lands silently re-opens the subscription.

Acceptance criteria

  • A profile marked ineligible is never returned by the weighted policy, at any live count.
  • An operator-visible way to express "never place here" that does not read as its opposite.
  • Exhausting all eligible candidates throws PlacementException; it never falls back to an
    ineligible profile.
  • bridged.yaml can return to placement: weighted with the architect profiles genuinely excluded,
    verified by a test that computes the actual selection distribution.
  • Related: maxLoad: 0 should either mean zero or be rejected at load, not silently mean unlimited.
## Symptom Measured against the live `bridged.yaml`, under `placement: weighted`: ``` gx10 weight=0.5 -> 17% terra weight=0.5 -> 17% opus weight=1.0 -> 33% *** BILLS THE CLAUDE SUBSCRIPTION *** sol weight=1.0 -> 33% ``` Both architect profiles are written `weight: 0` in config. A third of every unqualified spawn would land on the operator's Claude subscription. ## Root cause `BridgedConfig.Worker`'s compact constructor: ```java weight = (weight == null || weight <= 0.0f) ? 1.0f : weight; ``` A non-positive weight is normalised to **1.0**, not treated as exclusion. So `weight: 0` — the value an operator would naturally reach for to mean "never pick this" — produces the *heaviest* candidate in a fleet whose real workers sit at 0.5. The javadoc does say "Absent or non-positive ⇒ 1.0", so the code matches its own contract. The defect is that **there is no exclusion mechanism at all**, and the most obvious-looking spelling of one silently does the opposite of what it reads as. The second door is shut the same way: ```java maxLoad = (maxLoad == null || maxLoad <= 0) ? null : maxLoad; ``` `maxLoad: 0` normalises to null = *unlimited*, so it cannot express "never" either. ## Why it matters This is the mechanism the config relies on to keep unqualified spawns off two paid subscriptions (`opus` on Claude, `sol` on the ChatGPT oauth). A long comment block in `bridged.yaml` asserted confidently that weight 0 made an unqualified spawn "never" land there. It was false for as long as it has existed — with the earlier single `sonnet` architect profile the share was 50%. Nothing observable failed, which is precisely the problem: the failure mode is a silent, correct- looking config that bills a subscription. ## Fix — pick one, deliberately 1. **Honour `weight: 0` as exclusion.** Most intuitive, matches how every operator will read it. Breaks the documented "non-positive ⇒ 1.0" contract, so the javadoc and any test asserting it must change together. 2. **Add an explicit `placementEligible: false`** (or `role: architect`) and leave weight normalisation alone. More verbose but unambiguous, and it separates "how much" from "whether". Option 2 is probably right — "weight 0" and "not a candidate" are genuinely different statements, and conflating them is what produced this. But whichever is chosen, `weight: 0` must stop meaning 1.0: leaving that as a silent trap is worse than either fix. Whichever lands, placement must **fail loudly** when every eligible candidate is exhausted rather than falling back onto an ineligible profile. ## Current mitigation (in place) `placement: fixed` in `bridged.yaml`, which ignores weights entirely and routes every unqualified spawn to `defaultWorker` (gx10). Cost: unqualified spawns no longer split across gx10/terra. Cheap, because the charter has every lead name a profile explicitly, and explicit spawns bypass placement. **Restoring `placement: weighted` before this ticket lands silently re-opens the subscription.** ## Acceptance criteria - A profile marked ineligible is never returned by the weighted policy, at any live count. - An operator-visible way to express "never place here" that does not read as its opposite. - Exhausting all eligible candidates throws `PlacementException`; it never falls back to an ineligible profile. - `bridged.yaml` can return to `placement: weighted` with the architect profiles genuinely excluded, verified by a test that computes the actual selection distribution. - Related: `maxLoad: 0` should either mean zero or be rejected at load, not silently mean unlimited.
ltms commented 2026-08-15 13:10:08 +02:00
Author
Owner
Copy Link
Copy Source

Merged to main at c29c3f0. My own build on the merged tree: 764 tests, 0 failures, BUILD SUCCESS, exit 0.

One consequence the worker could not have seen

This fix changed the live fleet's behaviour, because bridged.yaml is gitignored and used weight: 0 as decoration — its own comment read weight 0 does NOT exclude — it normalizes to 1.0. That comment stopped being true the moment this merged.

The concrete breakage: defaultProfileFor(role) returns the first profile in a role pool, and my architect pool is exactly {opus, sol} — both at weight: 0. After this change every bridge_spawn{role: "architect"} with no explicit profile would have thrown all worker profiles have weight 0.

Fixed in local config alongside the merge:

  • opus stays weight: 0 — that is now genuinely right. It runs on the operator's subscription and is the lead's own profile, so no unqualified spawn should land on it. An explicit bridge_spawn{profile: "opus"} bypasses placement and still works.
  • sol → 1.0. It is the one architect a role-only spawn can now land on.
  • sonnet → 1.0. It was 0 back when that meant nothing; today it does real dev work (maxLoad raised to 3, carries a forge token), so excluding it would be the opposite of the intent.

This is worth recording as a general point: a config value that is a no-op today can be load-bearing tomorrow. The exception message this change added is what makes the failure legible rather than silent, so the design is right — the surprise was in my config, not in the code.

The worker's flagged open question (exception wording) is accepted as-is; it matches the existing quarantine messages.

Merged to `main` at `c29c3f0`. My own build on the merged tree: 764 tests, 0 failures, BUILD SUCCESS, exit 0. ### One consequence the worker could not have seen This fix changed the live fleet's behaviour, because `bridged.yaml` is gitignored and used `weight: 0` **as decoration** — its own comment read `weight 0 does NOT exclude — it normalizes to 1.0`. That comment stopped being true the moment this merged. The concrete breakage: `defaultProfileFor(role)` returns the first profile in a role pool, and my architect pool is exactly `{opus, sol}` — **both** at `weight: 0`. After this change every `bridge_spawn{role: "architect"}` with no explicit profile would have thrown `all worker profiles have weight 0`. Fixed in local config alongside the merge: - `opus` stays `weight: 0` — that is now genuinely right. It runs on the operator's subscription and is the lead's own profile, so no unqualified spawn should land on it. An explicit `bridge_spawn{profile: "opus"}` bypasses placement and still works. - `sol` → `1.0`. It is the one architect a role-only spawn can now land on. - `sonnet` → `1.0`. It was `0` back when that meant nothing; today it does real dev work (`maxLoad` raised to 3, carries a forge token), so excluding it would be the opposite of the intent. This is worth recording as a general point: a config value that is a no-op today can be load-bearing tomorrow. The exception message this change added is what makes the failure legible rather than silent, so the design is right — the surprise was in my config, not in the code. The worker's flagged open question (exception wording) is accepted as-is; it matches the existing quarantine messages.
ltms closed this issue 2026-08-15 13:10:15 +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#28
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?