fix(alerts): enforce session limits for llm-only ingest

[mikemikimike]

Summary

Streams that emit only gen_ai.llm.call spans remain active and never reach the session-end alert hook. This change evaluates session cost and duration limits after post-ingest cost processing, so computed costs are included while daily budgets remain end-triggered.

Changes

• Add an explicit active-session progress evaluation path.
• Reuse the existing session-budget rule without duplicating its alert construction.
• Preserve compatibility with custom alert engines that do not implement the optional hook.
• Add synthetic rule tests and full pipeline tests for cost and duration thresholds.

Validation

• python -m pytest tests/synthetic/test_alert_rules.py tests/integration/test_full_pipeline.py -q — 49 passed.

uff check tokenjam/ — source and touched tests pass.

• The full suite currently hits a pre-existing Windows HOME/Path.home() failure in ests/unit/test_analyzer_scope.py::test_the_default_root_follows_a_repointed_home, reproduced unchanged from the base commit.

[greptile-apps]

Greptile Summary

The PR adds active-session budget and duration evaluation after cost processing while preserving asynchronous advisory-hook behavior and compatibility with custom alert engines.

• Adds lifetime session-limit deduplication and session-scoped cooldown keys.
• Restores session-limit state on startup and serializes concurrent first-firing checks.
• Adds integration, restart, concurrency, and asynchronous-hook coverage.

Confidence Score: 4/5

The PR is not yet safe to merge because bounded startup hydration can still resend a session-limit alert for the same active threshold crossing.

Lifetime session-limit deduplication is reconstructed from only the newest 10,000 alerts of each type, so an older active session is forgotten on restart and its next span can insert and dispatch the same alert again.

Files Needing Attention: tokenjam/core/alerts.py

Important Files Changed

Filename	Overview
tokenjam/core/alerts.py	Adds session-scoped lifetime deduplication, locking, and restart hydration, but bounded hydration still permits older active-session crossings to refire.
tokenjam/core/ingest.py	Evaluates active-session limits after cost processing in both synchronous and asynchronous hook modes.
tests/synthetic/test_alert_engine_restart.py	Covers ordinary restart and concurrent-evaluation deduplication, but does not exercise hydration-page eviction.
tests/integration/test_full_pipeline.py	Adds end-to-end coverage for cost and duration alerts on LLM-only active sessions.
tests/unit/test_async_hooks.py	Verifies deferred session-progress evaluation and fatal storage-error handling.
tests/synthetic/test_alert_rules.py	Adds direct rule coverage for active-session cost and duration threshold evaluation.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Ingest span] --> B[Persist span and session]
  B --> C[Compute and persist cost]
  C --> D{Async advisory hooks?}
  D -->|No| E[Evaluate active-session progress]
  D -->|Yes| F[Advisory worker]
  F --> E
  E --> G{Budget or duration exceeded?}
  G -->|No| H[Continue]
  G -->|Yes| I{Session/type key hydrated or fired?}
  I -->|Yes| H
  I -->|No| J[Persist and dispatch alert]

Loading

Prompt To Fix All With AI

### Issue 1
tokenjam/core/alerts.py:75
**Hydration cap permits refiring**

When a still-active session's prior cost or duration alert is older than 10,000 newer alerts of that type, restart hydration omits its lifetime deduplication key. The next span then treats the persistent threshold crossing as new, inserting and dispatching the same alert again.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

_{Reviews (6): Last reviewed commit: "Merge branch 'main' into fix/554-session..." | Re-trigger Greptile}

[mikemikimike]

Addressed the P1 review in commit 245df69: session-scoped budget and duration alerts now use session-aware cooldown keys and a per-session fired latch hydrated from persisted rows. Repeated spans in one session produce no duplicate rows, while separate sessions for the same agent alert independently; regression coverage includes both cases.

[mikemikimike]

Addressed the three P1 review findings in commit 4427837:

• Hydrate session-limit deduplication from persisted session alerts beyond the cooldown window, so restarts do not re-fire an already crossed limit.
• Make the session-limit check and first-write marker atomic with a lock, preventing concurrent evaluations from inserting duplicate alerts or dispatching duplicate notifications.
• Keep suppressed cooldown rows out of the lifetime session-limit marker, preserving the distinction between a cooldown suppression and a threshold crossing.

Validation:

• python -m pytest --basetemp .pytest-tmp2 tests/synthetic/test_alert_engine_restart.py tests/synthetic/test_alert_rules.py -q — 45 passed.
• ruff check --select I tokenjam/core/alerts.py tests/synthetic/test_alert_engine_restart.py — passed.
• mypy tokenjam/core/alerts.py — passed.
• git diff --check — passed.
• The repository-wide test run was started after installing .[dev], but the Windows environment exposed existing failures in unrelated analyzer/budget/MCP areas; no failures occurred in the changed alert tests before the run was stopped.

[mikemikimike]

The P1 finding is addressed in the current head 4427837:

• Session-limit deduplication is keyed by (session_id, alert_type) and hydrated from persisted alerts beyond the cooldown window.
• The first-write marker and alert insertion are serialized, so concurrent evaluations cannot create duplicate rows or notifications.
• Cooldown-suppressed rows do not mark the lifetime session-limit latch.

The focused restart, rule, and pipeline regression tests pass as documented in the earlier PR update.

[anilmurty]

@mikemikimike thanks for this. Your approach is the one I want to build on, so I'd like to get it landed.

Now that the workflow is approved, CI is red on two things:

1. mypy, in the lint job:

tokenjam/core/ingest.py:345: error: Argument 1 to "get_session" of "StorageBackend"
has incompatible type "str | None"; expected "str"  [arg-type]

2. tests/unit/test_async_hooks.py::test_async_hooks_execution fails on 3.10, 3.11 and 3.12:

AssertionError: Expected 'evaluate' to not have been called. Called 1 times.

That second one is worth more than a quick fix. evaluate_session_progress runs synchronously after _run_hooks(), and the extra db.get_session() round trip gives the TjHookWorker thread time to win the race the test asserts against. The architecture note in .claude/rules/core-architecture.md is that with [alerts] async_hooks = true the advisory hooks move off the ingest thread and only CostEngine stays synchronous. This puts session alert evaluation back on it unconditionally, plus one session read per span. On the 21,562-span replay from #554 that's 21.5k extra reads, and _check_session_budget builds the Alert object before _fire dedups it. Either respect async_hooks, or put a cheap threshold pre-check in front so the common path allocates nothing. PR #738 has a _session_end_gated_thresholds_exceeded() guard that does this well and is worth borrowing.

3. Critical Rule 45(b): the new except Exception at ingest.py:347 wraps a DB read and only logs. A DuckDB FatalException there gets absorbed and we keep ingesting against a dead instance. Route it through handle_if_fatal the way your _hydrate_from_db already does.

Smaller things, none blocking:

• _SESSION_LIMIT_HYDRATION_LIMIT = 100_000 adds two get_alerts queries to every AlertEngine construction, including every CLI invocation. The existing _HYDRATION_LIMIT = 10_000 was picked deliberately; a 10x jump plus two scans deserves a line in the PR body.
• The loop over recent in _hydrate_from_db is subsumed by the per-type queries below it.
• Widening the cooldown key to a 3-tuple means two sessions for the same agent can each fire SESSION_DURATION inside one cooldown window. I think that's right, but it changes fix(alerts): hydrate cooldown and failure-rate dedup state from the alerts table on startup #732's dedup semantics and should be stated.
• SESSION_DURATION now fires mid-session while the message reads "Session lasted {duration}s". Past tense for a session still running. "Session has run" is accurate.

I verified your tests are load-bearing: reverting alerts.py and ingest.py to main fails 7 of them, including both test_llm_only_session_* pipeline tests. That's the standard I want and a lot of PRs miss it.

One thing you should know: @Mr-Neutr0n independently fixed the same issue in #738. I'm going with yours because the latch placement inside _fire() is the correct choke point. Worth reading their pre-check and fatal handling.

[greptile-apps]

Hydration cap permits refiring

When a still-active session's prior crossing is older than 10,000 newer alerts of the same type, startup hydration omits its lifetime dedup key. The next span then inserts and dispatches the same session-limit alert again.

Knowledge Base Used:

• Optimization and governance
• Policies, alerts, and drift

Prompt To Fix With AI

This is a comment left during a code review.
Path: tokenjam/core/alerts.py
Line: 303-306

Comment:
**Hydration cap permits refiring**

When a still-active session's prior crossing is older than 10,000 newer alerts of the same type, startup hydration omits its lifetime dedup key. The next span then inserts and dispatches the same session-limit alert again.

**Knowledge Base Used:**
- [Optimization and governance](https://app.greptile.com/metabldr/-/custom-context/knowledge-base/metabuilder-labs/tokenjam/-/docs/optimization-and-governance.md)
- [Policies, alerts, and drift](https://app.greptile.com/metabldr/-/custom-context/knowledge-base/metabuilder-labs/tokenjam/-/docs/policies-alerts-and-drift.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[greptile-apps]

Hydration cap permits refiring

When a still-active session's prior cost or duration alert is older than 10,000 newer alerts of that type, restart hydration omits its lifetime deduplication key. The next span then treats the persistent threshold crossing as new, inserting and dispatching the same alert again.

Knowledge Base Used:

• Optimization and governance
• Policies, alerts, and drift

Prompt To Fix With AI

This is a comment left during a code review.
Path: tokenjam/core/alerts.py
Line: 75

Comment:
**Hydration cap permits refiring**

When a still-active session's prior cost or duration alert is older than 10,000 newer alerts of that type, restart hydration omits its lifetime deduplication key. The next span then treats the persistent threshold crossing as new, inserting and dispatching the same alert again.

**Knowledge Base Used:**
- [Optimization and governance](https://app.greptile.com/metabldr/-/custom-context/knowledge-base/metabuilder-labs/tokenjam/-/docs/optimization-and-governance.md)
- [Policies, alerts, and drift](https://app.greptile.com/metabldr/-/custom-context/knowledge-base/metabuilder-labs/tokenjam/-/docs/policies-alerts-and-drift.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[anilmurty]

@mikemikimike you've addressed all three. I read the diff rather than just the green checks:

• mypy — the narrowed session id clears lint.
• async_hooks is respected. if not self.config.alerts.async_hooks: keeps the sync path inline and hands the async path to the advisory worker, so session alert evaluation is off the ingest thread where the architecture says it should be. This was the one I cared most about.
• handle_if_fatal at ingest.py:387, so a DuckDB fatal in the progress read is recorded rather than swallowed.
• You also picked up the threshold pre-check before allocating alert objects, which was the good idea from @Mr-Neutr0n's fix: session-end-gated alerts unreachable for llm.call-only SDK i #738. That was the right thing to borrow.

On the test change — I looked at this carefully, because replacing an assertion that just started failing is exactly how a guard gets quietly removed, and we have a standing rule about it. Your diagnosis is correct and the change is sound: the old alert_mock.evaluate.assert_not_called() immediately after process() was passing only because the worker thread hadn't been scheduled yet. It was racy, not load-bearing, and your extra session read changed the timing enough to expose it. Blocking the worker inside the first hook makes it deterministic.

Worth knowing what the new version does and doesn't prove: schema_mock.validate.assert_not_called() while the worker is parked inside evaluate is now the assertion carrying the deferral property — if the advisory hooks moved back onto the ingest thread, validate would have run during process() and that line fails. The evaluate assertion no longer distinguishes inline from deferred on its own. That's fine, but if you ever touch this test again, that assert_not_called on validate is the load-bearing line — keep it.

The red CI you were fighting was never yours. claude-sonnet-5's introductory pricing expired on 2026-09-01 and five tests read the live rate, so main went red that day and every open PR inherited it. Fixed in #741, your branch is updated onto it, and I've re-approved the workflow.

Once the matrix comes back green this is ready to merge. Nothing further from me.

[anilmurty]

Thanks @mikemikimike — merging. You landed all three asks: async_hooks is respected so alert evaluation stays off the ingest thread, handle_if_fatal is wired into the progress read, and you picked up the threshold pre-check from @Mr-Neutr0n's #738.

Triaging Greptile's remaining open issue ("Hydration cap permits refiring", 4/5) rather than merging past it: it is real but bounded, and acceptable. get_alerts is ORDER BY fired_at DESC LIMIT 10000, so the cap drops the oldest rows — a still-active session whose alert fired more than 10,000 same-type alerts ago loses its dedup key on restart and refires once. That is the same bounded tradeoff _HYDRATION_LIMIT already carries on main for the other alert types, and it is documented in the constant's comment: a missed stale row degrades to treating an old alert as new, never to suppressing wrongly. One duplicate row in a corner that needs 10k+ same-type alerts is not worth another round trip. Filing it as a follow-up if it ever shows up in practice.

Also worth recording: the red CI on this PR was never yours — main was broken by an expired claude-sonnet-5 introductory rate, fixed in #741.