Found while building #533. This is shipped, user-facing, and it understates a safety rail by two orders of magnitude.
The bug
keel/web/render.py:150:
def pct(value: Decimal | float | None, *, places: int = 2) -> str:
if value is None:
return "--"
return f"{value:.{places}f}%"
It appends % without multiplying by 100. Every value it is given is a fraction, not a percentage:
keel/templates/config.live.yaml:73 — max_total_dd_pct: 0.20, meaning twenty percent.
keel/execution/guards.py:583 — if dd_total >= config.money_mgmt.max_total_dd_pct, a direct comparison, so dd_total is a fraction on the same scale.
So a 20% drawdown ceiling renders as 0.20%, and a live 5% drawdown renders as 0.05%.
Where it shows
Six call sites, all drawdown:
render.py:251 drawdown (total)
render.py:252 drawdown (weekly)
render.py:253 max total dd
render.py:254 max weekly dd
render.py:452 drawdown (total) (account view)
render.py:453 drawdown (weekly) (account view)
Why this one matters more than a formatting slip
This is rail 11, the drawdown breaker — the number an operator reads to judge how close the account is to halting itself. The page reports the account as having consumed 0.05% of a 0.20% budget when the truth is 5% of 20%.
Both the displayed position and the displayed ceiling are wrong by the same factor, so the ratio still looks right and nothing appears broken. That is what makes it survivable: it does not look like an error, it looks like a very conservative account. An operator sanity-checking the absolute number against their venue's own drawdown figure is the only person who would catch it.
The CLI and TUI are correct, and the contrast is the lesson
keel/commands/status.py:492-497 and keel/commands/tui.py:354-359 print the same fields:
drawdown: total=0.05 (ceiling 0.20) weekly=... (ceiling ...)
Raw, with no unit claimed. That is honest — ambiguous about the scale, but never wrong. The web renderer is the only surface that asserts a unit, and asserting it is what made it wrong.
The fix, and the part worth arguing about
Multiplying by 100 inside pct() is the one-line fix, but it leaves the trap: a helper named pct that takes a fraction will be handed a real percentage eventually, and nothing will catch it.
Better: make the scale explicit at the boundary — either a fraction_as_pct() that names what it converts, or rename the fields so _pct means percent everywhere and the fraction-valued ones say so. The config key max_total_dd_pct: 0.20 is itself the root of the confusion, though changing it is a breaking config change and needs its own decision.
Whatever is chosen, a test must pin it. A formatter whose correctness depends on remembering which scale its caller uses will drift back.
Relationship to the rewrite
#540 deletes render.py's HTML generation, and #533's serialiser deliberately does not inherit this bug — it passes the drawdown through unchanged with no % in the display string, and pins that with a test. So this will disappear from the browser when the rewrite lands.
That is not a reason to leave it. The rewrite is several issues away, and until then every operator using keel serve reads a drawdown breaker that is wrong by 100x.
Acceptance
Found while building #533. This is shipped, user-facing, and it understates a safety rail by two orders of magnitude.
The bug
keel/web/render.py:150:It appends
%without multiplying by 100. Every value it is given is a fraction, not a percentage:keel/templates/config.live.yaml:73—max_total_dd_pct: 0.20, meaning twenty percent.keel/execution/guards.py:583—if dd_total >= config.money_mgmt.max_total_dd_pct, a direct comparison, sodd_totalis a fraction on the same scale.So a 20% drawdown ceiling renders as
0.20%, and a live 5% drawdown renders as0.05%.Where it shows
Six call sites, all drawdown:
Why this one matters more than a formatting slip
This is rail 11, the drawdown breaker — the number an operator reads to judge how close the account is to halting itself. The page reports the account as having consumed 0.05% of a 0.20% budget when the truth is 5% of 20%.
Both the displayed position and the displayed ceiling are wrong by the same factor, so the ratio still looks right and nothing appears broken. That is what makes it survivable: it does not look like an error, it looks like a very conservative account. An operator sanity-checking the absolute number against their venue's own drawdown figure is the only person who would catch it.
The CLI and TUI are correct, and the contrast is the lesson
keel/commands/status.py:492-497andkeel/commands/tui.py:354-359print the same fields:Raw, with no unit claimed. That is honest — ambiguous about the scale, but never wrong. The web renderer is the only surface that asserts a unit, and asserting it is what made it wrong.
The fix, and the part worth arguing about
Multiplying by 100 inside
pct()is the one-line fix, but it leaves the trap: a helper namedpctthat takes a fraction will be handed a real percentage eventually, and nothing will catch it.Better: make the scale explicit at the boundary — either a
fraction_as_pct()that names what it converts, or rename the fields so_pctmeans percent everywhere and the fraction-valued ones say so. The config keymax_total_dd_pct: 0.20is itself the root of the confusion, though changing it is a breaking config change and needs its own decision.Whatever is chosen, a test must pin it. A formatter whose correctness depends on remembering which scale its caller uses will drift back.
Relationship to the rewrite
#540 deletes
render.py's HTML generation, and #533's serialiser deliberately does not inherit this bug — it passes the drawdown through unchanged with no%in the display string, and pins that with a test. So this will disappear from the browser when the rewrite lands.That is not a reason to leave it. The rewrite is several issues away, and until then every operator using
keel servereads a drawdown breaker that is wrong by 100x.Acceptance
Decimal("0.20")renders as20.00%, not0.20%.pct()caller added later cannot make the same mistake silently.