Skip to content

Commit eed2a0e

Browse files
eaitbrahimclaude
andauthored
feat(web): cache the DCA plan's admission screen for 5 minutes (#856) (#870)
* feat(web): cache the DCA plan's admission screen for 5 minutes (#856) /rules?budget=... reloads every 15s, and each reload re-screened every allowlisted asset against the database through build_screen_report -- twenty times more often than the screen actually changes. Cache that one report (not the plan: budget/buffer still vary per request) on a new ScreenCache, keyed on (config_path, db_path), with an injectable clock (the reader's own now_ts, never a second time.time() read) and a thread-safe TTL of 300s (a HIT is exclusive of the boundary -- an entry exactly 300s old is stale, matching guards.py's own attest_due_ts semantics). The cache lives on ServeConfig, scoped to one deployment and shared by every ThreadingHTTPServer thread; the lock is held across a miss's rebuild, deliberately, to avoid a thundering herd on a single-operator dashboard. select_universe/build_dca_plan gain a screen_report_fn injection point (defaulting to build_screen_report, so every existing caller -- including the CLI -- is unaffected); read_dca_plan is the one caller that passes the cache lookup instead. The payload gains screen_age, a presentation-ready sentence ("Asset screen from 2m ago; refreshes every 5 min.") computed server-side from _human_age, and the /rules card shows it under the plan's state line. Read-only; no behaviour change beyond the screen the plan is built from being up to 5 minutes stale, which the card now says out loud. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(web): a cached screen's age is never negative (#856) A request stamped before a later-stamped one built the entry (it waited on the lock) read "-1s ago". Clamp the age to 0. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(web): the DCA card's screen line says when no screen ran, and states the cache's own TTL (#856) - awaiting card: screen_age is a worded 'not run yet' field, not a bare dash - the refresh interval is ScreenCache.TTL_SECONDS handed through, not prose - no screen age passed means not-screened, never a silent '0s ago' - an entry a whole TTL in the request's future (clock stepped back) is a miss Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(web): the screen cache key carries the allowlist and quote currency (#873) config.yaml is re-read per request, so an asset added at the same path was reported 'not screened' for up to 5 minutes. Also pins that a cache hit still reads the budget and the DCA rules fresh. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test(web): a same-query read after a rule insert shows the rule, so a per-query plan cache fails (#856) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent e8b89f6 commit eed2a0e

8 files changed

Lines changed: 616 additions & 9 deletions

File tree

‎keel/commands/dca_plan.py‎

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939

4040
from keel import agent
4141
from keel.commands._products import _history_product, parse_products_option
42-
from keel.commands.admission import ScreenFn, build_screen_report
42+
from keel.commands.admission import ScreenFn, ScreenReport, build_screen_report
4343
from keel.commands.rules import RulesOutcome, RulesRefused, RulesUsageError, add_rule_row
4444
from keel.config import Config
4545
from keel.data.repository import Repository
@@ -300,22 +300,28 @@ def select_universe(
300300
*,
301301
screen_fn: ScreenFn,
302302
weights_override: Mapping[str, Decimal] | None = None,
303+
screen_report_fn: Callable[[Repository, Config, ScreenFn], ScreenReport] = build_screen_report,
303304
) -> Universe:
304305
"""allowlist ∩ admitted ∩ positive weight, minus assets with a non-disabled DCA rule.
305306
306307
Admission comes from `build_screen_report` with the injected `screen_fn`
307308
(`keel.commands.assets.screen_product` in production) -- the one gate every candidate source
308309
routes through, never a laxer copy. READ-ONLY: this writes nothing.
310+
311+
`screen_report_fn` defaults to `build_screen_report` itself, so every caller before #856 is
312+
unaffected. `keel/web/api.py`'s `read_dca_plan` is the one caller that passes something else --
313+
a lookup into its `ServeConfig`'s `ScreenCache` -- so `/rules?budget=`'s 15s poll does not
314+
re-screen the whole allowlist against the database on every reload. `screen_fn` itself is left
315+
untouched by that: a cache HIT never even reaches `screen_fn`, and a MISS calls it exactly the
316+
way `build_screen_report` always has.
309317
"""
310318
quote = config.quote_currency
311319
# #849: the allowlist names assets, so a repeat (`[BTC, ETH, btc]` passes `load_config`) is
312320
# the SAME asset -- one allocation, one buy, one rule. `dict.fromkeys` keeps first-seen order;
313321
# nothing is dropped, because a repeat carries no weight of its own.
314322
allowlist = list(dict.fromkeys(asset.upper() for asset in config.allowlist))
315323
weights = _weights_by_asset(config.target_weights, "target_weights")
316-
admitted = {
317-
sp.asset.upper(): sp for sp in build_screen_report(repo, config, screen_fn).screened
318-
}
324+
admitted = {sp.asset.upper(): sp for sp in screen_report_fn(repo, config, screen_fn).screened}
319325
existing = existing_dca_rules(repo)
320326
existing_by_asset = {rule.asset: rule for rule in existing}
321327

@@ -801,9 +807,13 @@ def build_dca_plan(
801807
now_ts: int,
802808
screen_fn: ScreenFn,
803809
weights_override: Mapping[str, Decimal] | None = None,
810+
screen_report_fn: Callable[[Repository, Config, ScreenFn], ScreenReport] = build_screen_report,
804811
) -> DcaPlan:
805812
"""The whole proposal, every figure computed here and nowhere downstream. READ-ONLY.
806813
814+
`screen_report_fn` is threaded straight through to `select_universe`, unchanged -- see that
815+
function's docstring for why it exists (#856's screen cache).
816+
807817
Per buy: `monthly share x cadence_days / (365.25/12)`, rounded DOWN to cents; the monthly
808818
total is recomputed from that rounded buy and rounded down again; the fee estimate at the
809819
CONFIGURED `fees.taker_pct` is rounded UP (R5). Blockers (R6, R8) make the plan
@@ -840,7 +850,13 @@ def build_dca_plan(
840850
`None`) for a test to check directly, and `/api/dca-plan`'s card shows the identical sentence
841851
because it reads `plan.blockers` verbatim, as it always has.
842852
"""
843-
universe = select_universe(repo, config, screen_fn=screen_fn, weights_override=weights_override)
853+
universe = select_universe(
854+
repo,
855+
config,
856+
screen_fn=screen_fn,
857+
weights_override=weights_override,
858+
screen_report_fn=screen_report_fn,
859+
)
844860
cap = monthly_buy_cap(repo, config, venue=venue, now_ts=now_ts)
845861
cadence = inputs.cadence_days
846862
spend = _cents_down(inputs.budget_usd * (Decimal("1") - inputs.buffer_pct))

‎keel/web/api.py‎

Lines changed: 125 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,9 @@
3434

3535
from __future__ import annotations
3636

37+
import threading
3738
import time
38-
from collections.abc import Callable, Mapping, Sequence
39+
from collections.abc import Callable, Hashable, Mapping, Sequence
3940
from dataclasses import dataclass, field
4041
from datetime import UTC, datetime
4142
from pathlib import Path
@@ -783,6 +784,97 @@ def read_rules(cfg: ServeConfig, _query: Query, _state: Any, _now_ts: int) -> di
783784
return payload.rules_payload(rows)
784785

785786

787+
#: How long one deployment's admission screen stays cached (#856). 5 minutes: short enough that
788+
#: an operator who just fixed a candle gap or attested an asset sees it admitted within one
789+
#: coffee, long enough that `/rules?budget=`'s 15s poll (`server._REFRESH_SEC`) stops re-running
790+
#: the whole allowlist's `screen_product` against the database twenty times for every time an
791+
#: operator actually changes the budget or buffer in the address bar.
792+
_SCREEN_CACHE_TTL_SECONDS = 300
793+
794+
795+
@dataclass(frozen=True)
796+
class _ScreenCacheEntry:
797+
report: Any
798+
built_at_ts: int
799+
800+
801+
class ScreenCache:
802+
"""Caches one deployment's admission screen -- `build_screen_report`'s result -- across the
803+
repeated `/api/dca-plan` reads `/rules?budget=`'s 15s poll makes (#856).
804+
805+
**What is cached, and why not the whole plan.** `/api/dca-plan`'s budget and buffer are query
806+
parameters an operator can retype at will, and `build_dca_plan` recomputes every buy from
807+
them; caching the PLAN would serve one operator's first-typed budget back on their fifth
808+
request. The screen (`select_universe`'s admission gate) reads only the allowlist and the
809+
database, which the config and the database on disk govern -- not the query string -- so it
810+
is the one part of this read that is genuinely the same answer for `TTL_SECONDS`.
811+
812+
**Keyed on `(config_path, db_path)`, not relied on being one-per-`ServeConfig`.** In
813+
production a `ServeConfig` is frozen and built once per `keel serve` process (`server.py`), so
814+
a cache living on it is already scoped to one deployment by construction. Keying on the paths
815+
anyway is what keeps a `ScreenCache` correct on its own terms -- a test harness, or a future
816+
hot-reloaded `ServeConfig`, sharing one `ScreenCache` across two deployments still gets two
817+
entries rather than one deployment's screen answering for another's.
818+
819+
**And on what the screen reads from the config (#873).** `read_dca_plan` re-reads
820+
config.yaml on every request, so the same path can name a new allowlist a minute later. The
821+
caller's key therefore also carries the screened product ids and the quote currency --
822+
exactly the inputs `build_screen_report` takes from the config -- so an asset added at the
823+
same path is screened on the next read, never reported "not screened" for a whole TTL.
824+
825+
**Holds no connection and no repository.** A `ScreenReport` is a frozen dataclass of
826+
`MarketFacts`/`ScreenResult` rows -- plain data, read once and never re-touched by this cache.
827+
`build()` is a closure the CALLER makes from its OWN per-request repo and connection
828+
(`server.py`'s module docstring: each request opens its own SQLite connection); the entry
829+
stored here outlives that connection by design and never needs it again.
830+
831+
**Reads no clock of its own.** `now_ts` is the caller's -- `respond`'s one `time.time()`,
832+
already threaded down to every reader (`test_no_reader_reads_the_clock_a_second_time`'s AST
833+
scan is the rule this keeps: a second clock read here would let the entry's age and the
834+
envelope's `generated_at` describe two different instants).
835+
836+
**Thread-safe, and the lock is held across a MISS's rebuild -- deliberately.** `KeelServer` is
837+
a `ThreadingHTTPServer`: two polls of `/rules` landing at once, both past a stale entry, would
838+
otherwise both re-screen the whole allowlist concurrently (a thundering herd) for a page one
839+
operator reads alone on a loopback socket. Serialising the rebuild means the second thread's
840+
poll waits for the first's screen (a handful of indexed local DB reads, not a network call)
841+
and then reads the SAME answer, rather than building a redundant copy of its own. One thread
842+
blocked instead of two racing is the right trade for a single-operator dashboard.
843+
"""
844+
845+
TTL_SECONDS = _SCREEN_CACHE_TTL_SECONDS
846+
847+
def __init__(self) -> None:
848+
self._lock = threading.Lock()
849+
self._entries: dict[tuple[Hashable, ...], _ScreenCacheEntry] = {}
850+
851+
def get_or_build(
852+
self, key: tuple[Hashable, ...], now_ts: int, build: Callable[[], Any]
853+
) -> tuple[Any, int]:
854+
"""`(report, age_seconds)` for `key` as of `now_ts`.
855+
856+
A HIT is an entry younger than `TTL_SECONDS`; the boundary is EXCLUSIVE of freshness -- an
857+
entry exactly `TTL_SECONDS` old is a MISS, matching `keel/execution/guards.py`'s own
858+
`attest_due_ts <= now_ts` (equality already means due, not "an instant of grace left").
859+
A MISS calls `build()` while the lock is held (see the class docstring), so `build` must
860+
stay a plain, synchronous read -- exactly what `build_screen_report` is.
861+
862+
The window is symmetric: an entry built `TTL_SECONDS` or more AFTER `now_ts` is a MISS
863+
too. A lock-wait legitimately makes the age a second or two negative (read as 0 below),
864+
but an entry a whole TTL in this request's future means the wall clock stepped back, and
865+
serving it would keep a stale screen up for the step plus the TTL under "0s ago".
866+
"""
867+
with self._lock:
868+
entry = self._entries.get(key)
869+
if entry is not None and abs(now_ts - entry.built_at_ts) < self.TTL_SECONDS:
870+
# A request stamped before a later-stamped one built this entry (it waited on the
871+
# lock) reads age 0, never a negative age.
872+
return entry.report, max(0, now_ts - entry.built_at_ts)
873+
report = build()
874+
self._entries[key] = _ScreenCacheEntry(report, now_ts)
875+
return report, 0
876+
877+
786878
def read_dca_plan(cfg: ServeConfig, query: Query, _state: Any, now_ts: int) -> dict[str, Any]:
787879
"""The DCA proposal card on `/rules`, from THE service the CLI calls
788880
(`keel.commands.dca_plan.build_dca_plan`). READ-ONLY: it builds a plan and writes nothing;
@@ -797,7 +889,18 @@ def read_dca_plan(cfg: ServeConfig, query: Query, _state: Any, now_ts: int) -> d
797889
module's one way of saying "declined, and here is why" -- the CLI refuses the same error as a
798890
clean message rather than a traceback, and a 500 here would read "That report could not be
799891
built" over a message that already names the fix. The card shows the refusal's detail.
892+
893+
**#856: the admission screen is cached, nothing else is.** `cfg.screen_cache` (a `ScreenCache`
894+
scoped to this `ServeConfig`, i.e. this served deployment) answers `build_screen_report` for
895+
up to 5 minutes; on a HIT, `screen_fn` (`screen_product`) is never even called. The cached
896+
`ScreenReport` is handed to `build_dca_plan` through its `screen_report_fn` injection point --
897+
a fixed callable that ignores the arguments `select_universe` would otherwise pass
898+
`build_screen_report` and returns the cached report -- so budget/buffer keep varying per
899+
request exactly as before; only the screen itself is stale for up to 5 minutes, and the
900+
payload's `screen_age` says so.
800901
"""
902+
from keel.commands._products import _default_sim_products
903+
from keel.commands.admission import build_screen_report
801904
from keel.commands.assets import screen_product # resolved per call: one patch point
802905
from keel.commands.dca_plan import (
803906
DcaPlanError,
@@ -821,15 +924,34 @@ def read_dca_plan(cfg: ServeConfig, query: Query, _state: Any, now_ts: int) -> d
821924
config = load_config(cfg.config_path)
822925
repo = open_repo(cfg.db_path)
823926
try:
927+
screen_report, screen_age_seconds = cfg.screen_cache.get_or_build(
928+
(
929+
cfg.config_path,
930+
cfg.db_path,
931+
config.quote_currency,
932+
tuple(_default_sim_products(config)),
933+
),
934+
now_ts,
935+
lambda: build_screen_report(repo, config, screen_product),
936+
)
824937
plan = build_dca_plan(
825-
repo, config, inputs, venue=_bound_venue(cfg), now_ts=now_ts, screen_fn=screen_product
938+
repo,
939+
config,
940+
inputs,
941+
venue=_bound_venue(cfg),
942+
now_ts=now_ts,
943+
screen_fn=screen_product,
944+
screen_report_fn=lambda *_a, **_kw: screen_report,
826945
)
827946
except DcaPlanError as exc:
828947
raise ApiRefusal(400, "DCA plan refused", str(exc)) from exc
829948
finally:
830949
close_repo(repo)
831950
return payload.dca_plan_payload(
832-
plan, command=apply_command(inputs, config_path=cfg.config_path, db_path=cfg.db_path)
951+
plan,
952+
command=apply_command(inputs, config_path=cfg.config_path, db_path=cfg.db_path),
953+
screen_age_seconds=screen_age_seconds,
954+
screen_ttl_seconds=cfg.screen_cache.TTL_SECONDS,
833955
)
834956

835957

‎keel/web/payload.py‎

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3711,10 +3711,52 @@ def dca_plan_awaiting_payload(*, command: str) -> dict[str, Any]:
37113711
"existing": [],
37123712
"blockers": [],
37133713
"warnings": [],
3714+
# #856: nothing was screened for THIS response (no budget was asked for, so
3715+
# `read_dca_plan` never reached the cache) -- said in words, never a manufactured "0s
3716+
# ago" that would claim a screen that did not happen, and never `absent()`'s bare "—",
3717+
# which the card would place as an unlabeled line of its own.
3718+
"screen_age": _dca_not_screened(),
37143719
}
37153720

37163721

3717-
def dca_plan_payload(plan: DcaPlan, *, command: str) -> dict[str, Any]:
3722+
def _dca_not_screened() -> Field:
3723+
"""#856: `screen_age` when no admission screen stands behind the response -- the awaiting
3724+
card (no budget asked for) or a caller that measured no screen age. A judged field in words,
3725+
`unknown` like every other figure the awaiting card lacks."""
3726+
return label(
3727+
"not_screened",
3728+
display="Asset screen: not run yet -- it runs once a budget is entered.",
3729+
state=UNKNOWN,
3730+
)
3731+
3732+
3733+
def _dca_screen_age(age_seconds: int, ttl_seconds: int) -> Field:
3734+
"""#856's staleness note for the cached admission screen behind `/rules?budget=`.
3735+
3736+
`ScreenCache` (`keel/web/api.py`) holds one screen for `ttl_seconds` so the page's 15s poll
3737+
does not re-screen the whole allowlist against the database on every reload; this is the one
3738+
sentence that tells the operator the trade-off exists. `_human_age` / `_human_remaining`, not
3739+
fresh formatters, for the same reason `duration()` above uses them: the browser and the CLI
3740+
must never be able to disagree about how long the same span is. The interval is the TTL the
3741+
cache applies, handed in by the caller -- never a second copy of it written as prose here.
3742+
"""
3743+
return label(
3744+
str(age_seconds),
3745+
display=(
3746+
f"Asset screen from {_human_age(age_seconds)}; "
3747+
f"refreshes every {_human_remaining(ttl_seconds)}."
3748+
),
3749+
state=NEUTRAL,
3750+
)
3751+
3752+
3753+
def dca_plan_payload(
3754+
plan: DcaPlan,
3755+
*,
3756+
command: str,
3757+
screen_age_seconds: int | None = None,
3758+
screen_ttl_seconds: int | None = None,
3759+
) -> dict[str, Any]:
37183760
"""`keel.commands.dca_plan.DcaPlan`, as JSON. Every figure was computed by the service --
37193761
`weight_pct`, `taker_pct_display`, `buy_count` and the `worst_month_*` fields exist so nothing
37203762
here multiplies or counts (Rules 2 and 6). READ-ONLY: the card carries the CLI command, and
@@ -3786,6 +3828,11 @@ def dca_plan_payload(plan: DcaPlan, *, command: str) -> dict[str, Any]:
37863828
],
37873829
"blockers": list(plan.blockers),
37883830
"warnings": list(plan.warnings),
3831+
"screen_age": (
3832+
_dca_not_screened()
3833+
if screen_age_seconds is None or screen_ttl_seconds is None
3834+
else _dca_screen_age(screen_age_seconds, screen_ttl_seconds)
3835+
),
37893836
}
37903837

37913838

‎keel/web/server.py‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,16 @@ class ServeConfig:
114114
#: When this run began, for the remote session lifetime (#648). Defaults to the moment the
115115
#: config is built, which is the moment `keel serve` mints the token it bounds.
116116
started_at: float = field(default_factory=time.time)
117+
#: #856: `/api/dca-plan`'s cached admission screen, scoped to THIS deployment. `ServeConfig`
118+
#: is frozen and built once per `keel serve` process (`build_server`), then shared by every
119+
#: thread `ThreadingHTTPServer` hands a request to via `BoundKeelHandler.cfg` -- exactly the
120+
#: lifetime a 5-minute cache needs, and `default_factory` gives a fresh, empty `ScreenCache`
121+
#: to every `ServeConfig` built (so two servers, or two deployments in one test session,
122+
#: never share one). `ScreenCache` itself is a plain class in `keel/web/api.py`, not a
123+
#: dataclass field with a mutable default -- `field(default_factory=api.ScreenCache)` is what
124+
#: keeps a frozen `ServeConfig` correct here: the FIELD is never reassigned, only the cache
125+
#: object it names is ever mutated internally, through its own lock.
126+
screen_cache: api.ScreenCache = field(default_factory=api.ScreenCache)
117127

118128
@property
119129
def session_expired_at(self) -> float | None:

‎keel/web/static/js/render.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3307,6 +3307,12 @@ export function dcaPlanCard(plan, error) {
33073307
const stateLine = el("p");
33083308
stateLine.append(field(plan.state));
33093309
card.append(stateLine);
3310+
// #856: the admission screen behind this plan is cached for up to 5 minutes, so this figure
3311+
// can lag a fix (a re-fetched candle, a fresh attestation) by that long -- said here, in the
3312+
// server's own words, rather than left for the operator to notice on their own.
3313+
const screenAgeLine = el("p", "note");
3314+
screenAgeLine.append(field(plan.screen_age));
3315+
card.append(screenAgeLine);
33103316
card.append(
33113317
dcaPlanFigure("spend / month", plan.summary.spend),
33123318
dcaPlanFigure("budget", plan.summary.budget),

0 commit comments

Comments
 (0)