fix: NULL required_context_tokens handling in TUI ctx cell and metrics demand-ceiling path #96

alee · 2026-09-19T03:31:39Z

alee commented

2026-09-19 03:31:39 +00:00

Two independent bugs in required_context_tokens handling.

1. TUI `ctx` cell renders literal "None"

_render_decisions_table (src/tui.py:769) does str(r.get("required_context_tokens")),
so a decision row without a context figure shows the literal string "None" in the
ctx column — the same defect the profile cell was specifically written to avoid
(see .omo/notepads/tui-overhaul/issues.md, which recorded this as a one-line
follow-up but deliberately left it out of scope for that plan).

Fixed: str(r.get("required_context_tokens") or "") — an absent value renders blank.

No other cell in the same table uses the blind str(dict.get(...)) pattern on a
nullable column; id/kind/category/tier are non-nullable, profile is already guarded,
and the remaining cells pass through helpers.

Regression test: test_app_decision_table_ctx_column_blank_not_none — asserts a 50000
value renders "50000" and a None renders "", never the literal "None".
Proven non-vacuous: against old code the test fails with AssertionError: assert 'None' == ''.

2. NULL `required_context_tokens` raises TypeError in demand-ceiling logic

route_decisions.required_context_tokens is a nullable column. SQLite MAX() over an
all-NULL group returns NULL, not 0 — so a time window whose every row lacks a
token count produced None where 0 (or a valid max) was expected, and None > ceiling
raised TypeError. Three sites in src/metrics.py were affected:

demand_ceiling_warnings (the MAX/comparison step): observed_max = None → crash
demand_ceiling_warnings (the escalation p95 path): _percentile received a list
containing None mixed with ints → sorted() crashed
capability_demand_warnings (the MAX/comparison step): same shape as the first

Semantics: NULL is "no demand recorded for this row", not zero. All three are guarded
with presence-checks so a NULL value is excluded from comparisons — no crash, no
fabricated warning.

3 regression tests, all proven non-vacuous: each fails against old code with
TypeError: '>' not supported between instances of 'NoneType' and 'int'.

Verification

Full test suite: 2154 passed, 0 failed
TUI smoke test (started against production /metrics + SSE, 25s uptime): no errors
Non-vacuity confirmed by replaying both fixes against the original code
Only 4 files modified: src/tui.py (1 line), src/metrics.py (3 guard blocks),
tests/test_metrics.py (125 lines, 3 tests), tests/test_tui.py (23 lines, 1 test)

Two independent bugs in `required_context_tokens` handling. ### 1. TUI `ctx` cell renders literal "None" `_render_decisions_table` (src/tui.py:769) does `str(r.get("required_context_tokens"))`, so a decision row without a context figure shows the literal string `"None"` in the `ctx` column — the same defect the `profile` cell was specifically written to avoid (see `.omo/notepads/tui-overhaul/issues.md`, which recorded this as a one-line follow-up but deliberately left it out of scope for that plan). Fixed: `str(r.get("required_context_tokens") or "")` — an absent value renders blank. No other cell in the same table uses the blind `str(dict.get(...))` pattern on a nullable column; id/kind/category/tier are non-nullable, profile is already guarded, and the remaining cells pass through helpers. Regression test: `test_app_decision_table_ctx_column_blank_not_none` — asserts a 50000 value renders `"50000"` and a `None` renders `""`, never the literal `"None"`. Proven non-vacuous: against old code the test fails with `AssertionError: assert 'None' == ''`. ### 2. NULL `required_context_tokens` raises TypeError in demand-ceiling logic `route_decisions.required_context_tokens` is a nullable column. SQLite `MAX()` over an all-NULL group returns `NULL`, not `0` — so a time window whose every row lacks a token count produced `None` where `0` (or a valid max) was expected, and `None > ceiling` raised `TypeError`. Three sites in `src/metrics.py` were affected: - **`demand_ceiling_warnings`** (the MAX/comparison step): `observed_max = None` → crash - **`demand_ceiling_warnings`** (the escalation p95 path): `_percentile` received a list containing `None` mixed with ints → `sorted()` crashed - **`capability_demand_warnings`** (the MAX/comparison step): same shape as the first Semantics: NULL is "no demand recorded for this row", not zero. All three are guarded with presence-checks so a NULL value is excluded from comparisons — no crash, no fabricated warning. 3 regression tests, all proven non-vacuous: each fails against old code with `TypeError: '>' not supported between instances of 'NoneType' and 'int'`. ### Verification - Full test suite: 2154 passed, 0 failed - TUI smoke test (started against production `/metrics` + SSE, 25s uptime): no errors - Non-vacuity confirmed by replaying both fixes against the original code - Only 4 files modified: `src/tui.py` (1 line), `src/metrics.py` (3 guard blocks), `tests/test_metrics.py` (125 lines, 3 tests), `tests/test_tui.py` (23 lines, 1 test)

alee added 2 commits 2026-09-19 03:31:40 +00:00

fix(tui): render blank ctx cell instead of literal "None" 3ac15510a0

fix(metrics): guard NULL required_context_tokens in demand-ceiling comparisons f35caa7b3f

alee merged commit a566b4ede7 into main

2026-09-19 15:56:48 +00:00

alee referenced this issue from a commit

2026-09-19 15:56:49 +00:00

Merge pull request 'fix: NULL required_context_tokens handling in TUI ctx cell and metrics demand-ceiling path' (#96) from fix/required-context-null-guards into main

Sign in to join this conversation.