feat(advice): flag-gated auto-advice + always-on budget exceeded alerts#95
Conversation
- budget_threshold:exceeded → factual message to chat + advice_log (monthly dedup) - other triggers + AUTO_ADVICE_ENABLED=true → AI advice via sendSmartAdvice - other triggers + AUTO_ADVICE_ENABLED=false → log only, no Telegram message - recordAdviceSent moved from sendSmartAdvice to caller (maybeSmartAdvice) - formatAmount used for budget exceeded amounts (per project convention) - add test: suppressed logs consume daily quota (Codex review finding)
|
🔁 PR closed. Stage bot switched back to |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bed6c3cb9a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| await sendSmartAdvice(groupId, trigger, snapshot); | ||
| recordAdviceSent(groupId, trigger.tier); |
There was a problem hiding this comment.
recordAdviceSent now runs unconditionally after sendSmartAdvice, but sendSmartAdvice can return without sending anything (e.g., provider failure is swallowed in its catch, validator rejection, or short/empty output). In those cases this still consumes the in-memory tier cooldown, so subsequent triggers are suppressed for 1h/4h even though the user received no advice.
Useful? React with 👍 / 👎.
| await sendMessage(text); | ||
| database.adviceLogs.create({ | ||
| group_id: groupId, |
There was a problem hiding this comment.
This path writes an advice_log entry immediately after sendMessage without checking whether sending succeeded. In this codebase sendMessage is best-effort and can return null on delivery/parse errors, so a failed send still marks the topic as sent for monthly dedup and prevents retries, causing users to miss the budget-exceeded alert for the rest of the month.
Useful? React with 👍 / 👎.
…t duplicate alerts Race condition: two expenses added quickly both pass hasTopicThisMonth before either commits to DB (sendMessage is async and yields the event loop mid-flight). Fix: DB write + recordAdviceSent happen synchronously before the await. Also adds recordAdviceSent for :exceeded triggers (was missing, left cooldown stale). Regression test verifies advice_log is written before sendMessage is called. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The `:exceeded` branch in `maybeSmartAdvice` was added in bed6c3c as part of the three-way dispatch, creating a second code path that sent a `⚠️ бюджет превышен` message every time a budget was exceeded. The original `checkBudgetLimit` in `expense-saver.ts` already sent `🔴 ПРЕВЫШЕН БЮДЖЕТ!` — so two messages fired on every save. Fix: - Remove the `:exceeded` dispatch from `maybeSmartAdvice` and `checkSmartTriggers`; exceeded is a fact handled synchronously by `checkBudgetLimit`, not by the async advice flow. - Add monthly dedup to `checkBudgetLimit`: writes to `advice_log` before `sendMessage` so concurrent saves can't both fire. - Simplify the early-continue guard in `checkSmartTriggers` to remove the now-dead `!== 'exceeded'` exception. - Update tests accordingly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 participant
Summary
AUTO_ADVICE_ENABLEDenv flag (defaultfalse)maybeSmartAdvicethree-way dispatch:budget_threshold:exceeded→ factual message to chat, once per month per categoryAUTO_ADVICE_ENABLED=true→ AI advice to chatAUTO_ADVICE_ENABLED=false→ log context only, nothing sentcheckSmartTriggersand all trigger logic inadvice-triggers.tsunchangeddisable-auto-advice(PR Suppress auto-advice, log trigger + context #87) andclaude/financial-alert-env-flag-8BhYgOnce-per-month dedup for budget_exceeded
checkSmartTriggersalready callshasTopicThisMonthbefore returning exceeded triggers.maybeSmartAdvicewrites theadvice_logentry after sending — that entry is whathasTopicThisMonthfinds on the next call. No code changes to trigger logic needed.Test plan
bun run type-check— greenbun run lint— greenbun run test— all pass (3412/3412)pm2 logsas suppressed, nothing in chatAUTO_ADVICE_ENABLED=true, restart → velocity spike fires AI advice in chat🤖 Generated with Claude Code