Skip to content

The browser shows the drawdown breaker 100x too small: pct() appends % to a fraction #542

Description

@eaitbrahim

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:73max_total_dd_pct: 0.20, meaning twenty percent.
  • keel/execution/guards.py:583if 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

  • Drawdown values and ceilings render at the correct scale in both the status and account views.
  • A test asserts the rendered output for a known fraction — e.g. Decimal("0.20") renders as 20.00%, not 0.20%.
  • The fraction-versus-percent boundary is explicit in a name or a type, not left to the caller's memory.
  • Any other pct() caller added later cannot make the same mistake silently.

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingrailsUn-overridable safety rail / guard (Compliance & rails)

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions