Skip to content

feat: Add DYNAMIC_AGE clamp stats and close-reason max_age field#142

Merged
keitaj merged 1 commit into
mainfrom
feat/dynamic-age-clamp-stats
May 2, 2026
Merged

feat: Add DYNAMIC_AGE clamp stats and close-reason max_age field#142
keitaj merged 1 commit into
mainfrom
feat/dynamic-age-clamp-stats

Conversation

@keitaj
Copy link
Copy Markdown
Owner

@keitaj keitaj commented May 2, 2026

Summary

  • Adds per-coin clamp counters to the DYNAMIC_AGE summary log so operators can see at a glance whether DYNAMIC_AGE_MIN is the binding constraint for a coin (min=85%) or the value moves freely.
  • Adds an optional effective_max_age argument to _record_close and surfaces it as a trailing max_age=Ns on each [close-reason] log line. This lets post-hoc analysis tell apart a long-aged force-close caused by a tight effective_max_age (DYNAMIC_AGE clamped low) from one that ran the full grace window.
  • Pure observability — no strategy behavior change. The new field is appended after the existing last_tier= so the close-event regex used downstream still matches.

Changes

  • strategies/market_making_strategy.py
    • New _dynamic_age_clamp_stats dict (per-coin: min_clamp, max_clamp, mid, raw_sum, raw_min, raw_max, samples).
    • _get_dynamic_position_age increments the right counter on each compute.
    • _log_dynamic_age emits one [mm] dyn-age <coin> samples=… min=…% mid=…% max=…% raw_avg=…s raw_range=[…-…] line per coin in addition to the existing snapshot. Sorted by min_clamp pct descending. Counters reset after each emit.
  • strategies/mm_position_closer.py
    • _record_close gets an optional effective_max_age parameter; emits max_age=Ns suffix when supplied.
    • All 5 call sites pass the value: manage() (unrealized loss path, force-close path) uses the effective_max_age already computed; cleanup_closed and on_position_closed pull it from a new _last_effective_max_age snapshot dict populated each manage() cycle. The dict is evicted on close.
  • Tests
    • test_dynamic_age_log.py: new TestDynamicAgeClampStats class covering counter increments for the three clamp regions, summary line format, sort order, and reset behavior. Existing fixture updated to initialize the new dict.
    • test_close_reason_logging.py: new TestRecordCloseEffectiveMaxAge and TestEffectiveMaxAgePropagation covering field presence/absence, regex backward compatibility, and the manage→cleanup_closed propagation path.
    • test_volatility_position_age.py: fixture updated for the new dict.

Test plan

  • flake8 (max-line-length 120) passes on the full repo.
  • pytest tests/ -q passes (930 tests, +12 new).
  • Existing [close-reason] regex last_tier=(\\S+) still captures the same group on the new log format (verified by test_existing_close_reason_regex_still_matches).
  • No production behavior change — only logging output is different.

🤖 Generated with Claude Code

Two small observability additions for diagnosing taker-fallback hot spots:

1. Per-coin clamp counters in `_get_dynamic_position_age` (min_clamp,
   max_clamp, mid, plus pre-clamp raw avg/min/max) emitted in the
   periodic `_log_dynamic_age` summary as
   "[mm] dyn-age <coin> samples=N min=X% mid=X% max=X% raw_avg=Ns
   raw_range=[Ms-Ks]" — one line per coin, sorted by min_clamp pct
   descending. A coin near 100% min tells operators DYNAMIC_AGE_MIN is
   the binding constraint vs. a structurally short grace window.

2. Optional `effective_max_age` argument on `_record_close`, surfaced as
   ` max_age=Ns` at the end of each [close-reason] log line. Plumbed
   through `manage()` (using the dynamic value already in scope) and
   propagated to externally-driven close paths (cleanup_closed,
   on_position_closed) via a new `_last_effective_max_age` snapshot
   dict. The trailing field is appended after the existing
   `last_tier=` so the existing close-event regex used by /review
   continues to match.

No behavior change; pure observability.
Copy link
Copy Markdown
Owner Author

@keitaj keitaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: DYNAMIC_AGE clamp stats and close-reason max_age field

Pure-observability change. No strategy behavior is touched; the new field on [close-reason] is appended after last_tier= so the existing last_tier=(\S+) regex still captures the same group (verified by test_existing_close_reason_regex_still_matches). Thread-safety of the new _last_effective_max_age dict mirrors the existing _open_positions pattern (atomic dict ops, no lock needed across main + WS threads). The _log_dynamic_age timer is now advanced after a successful emit instead of before — a minor improvement that means a logger exception leaves the next cycle to retry rather than silently skipping.


1. Boundary classification at the clamp endpoints (nit)

strategies/market_making_strategy.py (clamp classification block):

if raw_age <= self._dynamic_age_min:
    stats["min_clamp"] += 1
elif raw_age >= self._dynamic_age_max:
    stats["max_clamp"] += 1
else:
    stats["mid"] += 1

<= and >= mean a raw_age exactly equal to the floor/ceiling counts as a clamp even though no clamping happened. Operationally fine — "value is sitting at the floor" is exactly the signal operators want. Worth keeping in mind only if someone later expects strict-clamp counts (e.g. for a unit-test on a specifically-calibrated baseline). No change requested.


2. Defensive .get("samples", 0) (nit)

strategies/market_making_strategy.py (_log_dynamic_age):

samples = int(stats.get("samples", 0))

stats was created via setdefault with all keys including samples, so direct stats["samples"] would never KeyError. Defensive .get is harmless but slightly noisy. Not worth changing.


Verdict: LGTM

No blockers, two nits, both ignorable. Existing close-event regex backward-compat is explicitly tested. CI green per author note; will re-run before merge.

@keitaj keitaj merged commit 8729ed2 into main May 2, 2026
6 checks passed
@keitaj keitaj deleted the feat/dynamic-age-clamp-stats branch May 4, 2026 03:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant