[codex] Add LongBridge dry-run report summary#122
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bdb850af76
ℹ️ 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".
| send_message(message) | ||
| compact = str(message or "") | ||
| recorded_delivery_events.append( | ||
| { | ||
| "sink": "telegram", | ||
| "delivery_status": "sent", |
There was a problem hiding this comment.
Don't mark no-op Telegram sends as sent
When the default LongBridge sender is used with missing Telegram credentials (or when it swallows a post failure), send_message returns normally without delivering anything (notifications/telegram.py returns early for empty token/chat and catches exceptions). Because this wrapper unconditionally appends delivery_status: "sent" after any normal return, a dry-run report can claim validation_status: passed and include a sent delivery event even though no notification left the process; the delivery log should only record sent after the sender can confirm delivery, or should distinguish attempted/skipped/failed sends.
Useful? React with 👍 / 👎.
Summary
Validation
PYTHONPATH="/Users/lisiyi/Projects/LongBridgePlatform/.venv/lib/python3.13/site-packages:/Users/lisiyi/Projects/QuantPlatformKit/src" .venv/bin/python -m pytest -q tests/test_request_handling.py tests/test_runtime_reporting_adapters.py→16 passedKnown unrelated local verification issue
A broader local run including
tests/test_runtime_config_support.pyandtests/test_notifications.pyhad 6 failures because the local/Users/lisiyi/Projects/QuantPlatformKitcheckout is stale/diverged and shadows the installed package/profile registry in some test paths. The failures are profile-registry/translation mismatches unrelated to this report-summary change.