fix: limit plugin telegram alerts to manual review#171
Conversation
Co-Authored-By: Codex <noreply@openai.com>
Pigbibi
force-pushed
the
codex/fix-plugin-telegram-manual-review-20260703
branch
from
7746e43 to
636f2b1
Compare
![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: 7746e4383c
ℹ️ 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".
| return ( | ||
| _coerce_bool(controls.get("position_control_allowed"), default=False) | ||
| or _coerce_bool(policy.get("position_control_allowed"), default=False) | ||
| ) |
There was a problem hiding this comment.
For defend/delever signals where consumption_evidence_status='automation_approved' and position_control_allowed=True but strategy_runtime_metadata_allowed=False, should_alert_strategy_plugin_signal() still emits an alert because _is_strategy_position_control_notice() only hands signals to the platform cycle when runtime metadata is allowed. This new predicate suppresses Telegram based only on position-control permission, so those defensive/delever signals are neither carried by the platform cycle notification nor sent to the plugin Telegram channel.
Useful? React with 👍 / 👎.
| if target_type == "notification_target": | ||
| return True | ||
| action = str(getattr(signal, "suggested_action", None) or "").strip().lower() | ||
| if action in _MANUAL_REVIEW_ACTIONS: |
There was a problem hiding this comment.
When a guard/data-quality signal arrives with suggested_action='blocked' and no notification_only controls, the shared alert builder still treats it as an alert (blocked is in STRATEGY_PLUGIN_ALERT_ACTIONS), but this new prefilter only lets notify_manual_review or notification-only signals reach build_strategy_plugin_alert_messages(). That makes market_regime_control/blocked or crisis guard-blocked alerts produce zero Telegram deliveries, so operators miss the alert that tells them to fix the blocked data/source condition.
Useful? React with 👍 / 👎.
| manual_review_signals = tuple( | ||
| signal for signal in signals if _is_manual_review_telegram_signal(signal) | ||
| ) | ||
| messages = build_strategy_plugin_alert_messages( | ||
| signals, | ||
| manual_review_signals, |
There was a problem hiding this comment.
When publish_strategy_plugin_alerts() is configured with channels=("telegram",) for a regular alerting plugin signal such as the existing crisis_response_shadow true_crisis/defend signal with no notification-only controls, this prefilter drops the signal before the shared alert-builder logic can send it. The other channels still deliver it and the dispatcher tests now lose the Telegram send/count, so Telegram-only alert deployments silently miss these ordinary plugin alerts.
Useful? React with 👍 / 👎.
Co-Authored-By: Codex <noreply@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ccc2abb18
ℹ️ 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".
| _DEFAULT_TELEGRAM_BODY_MAX_CHARS = 3900 | ||
| _MARKET_REGIME_CONTROL_PLUGIN = "market_regime_control" | ||
| _AUTOMATED_POSITION_ACTIONS = frozenset({"defend", "delever"}) | ||
| _MANUAL_REVIEW_ACTIONS = frozenset({"notify_manual_review"}) |
There was a problem hiding this comment.
The runtime contract in docs/strategy_plugin_runtime_contract.md:203-205 lists watch_only as remaining in the plugin-alert stream, but this new action set only allows notify_manual_review. For a strategy-mounted watch signal with canonical_route='watch', suggested_action='watch_only', and the default controls (no notification_profile/capital_impact), build_strategy_plugin_alert_messages() would create an alert because the route is not no_action, yet the Telegram prefilter rejects it before the builder runs, so documented watch-only Telegram alerts disappear.
Useful? React with 👍 / 👎.
1 participant
Summary\n- limit strategy plugin Telegram alerts to manual-review/notification-only signals\n- skip auto-consumable plugin signals so platform cycle notifications carry them\n- remove platform context from plugin Telegram messages and add a unified manual-review header\n\n## Tests\n- python3 -m pytest tests/test_strategy_plugin_telegram_notifications.py tests/test_strategy_plugins.py::StrategyPluginsTests::test_strategy_plugin_auto_position_control_signal_stays_with_strategy_notification tests/test_strategy_plugins.py::StrategyPluginsTests::test_strategy_plugin_manual_review_strategy_signal_still_alerts_plugin_bot tests/test_strategy_plugins.py::StrategyPluginsTests::test_strategy_plugin_notification_target_still_alerts_plugin_bot -q\n- python3 -m ruff check src/quant_platform_kit/notifications/strategy_plugin_telegram.py tests/test_strategy_plugin_telegram_notifications.py\n- git diff --check