feat(adapters): log init for gchat + github, include registered adapters in not-found error#96
Conversation
…ers in not-found error - GoogleChatAdapter.initialize() now logs "Google Chat adapter initialized" matching the Slack/Teams pattern. - GitHubAdapter.initialize() now logs "GitHub adapter initialized" unconditionally (the existing "GitHub auth completed" was conditional on auth_token validation). - Chat.channel() / Chat.thread() not-found errors now append "(registered adapters: [...])" so operators can tell at a glance whether the lookup failed because the adapter was never constructed in this process vs. constructed but skipped. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughThis PR adds initialization logging to two chat adapters and enhances error diagnostics. GitHub and Google Chat adapters now emit info logs when initialization completes. Meanwhile, channel and thread lookup methods include registered adapter names in their error messages when an adapter is not found, aiding troubleshooting. ChangesLogging and Error Message Improvements
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/chat_sdk/chat.py`:
- Around line 1504-1508: Formatting drift in src/chat_sdk/chat.py causes CI to
fail; run the project formatter to fix whitespace/line-wrapping so `ruff format
--check` passes. Reformat the file (or run `ruff format` on the repository) and
ensure the blocks around the ChatError raise (references: ChatError,
adapter_name, channel_id, self._adapters) and the similar block later are
normalized to the project's style so the string concatenation/line breaks match
the formatter expectations. After formatting, re-run the linter/format check to
confirm the drift is resolved.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b8ca5f8f-9f9f-450b-8f7b-80e654fafc24
📒 Files selected for processing (3)
src/chat_sdk/adapters/github/adapter.pysrc/chat_sdk/adapters/google_chat/adapter.pysrc/chat_sdk/chat.py
| registered = sorted(self._adapters.keys()) | ||
| raise ChatError( | ||
| f'Adapter "{adapter_name}" not found for channel ID "{channel_id}" ' | ||
| f"(registered adapters: {registered})" | ||
| ) |
There was a problem hiding this comment.
Fix formatter drift in this file before merge.
ruff format --check is failing for src/chat_sdk/chat.py, so CI will stay red until this file is formatted.
Also applies to: 1566-1570
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/chat_sdk/chat.py` around lines 1504 - 1508, Formatting drift in
src/chat_sdk/chat.py causes CI to fail; run the project formatter to fix
whitespace/line-wrapping so `ruff format --check` passes. Reformat the file (or
run `ruff format` on the repository) and ensure the blocks around the ChatError
raise (references: ChatError, adapter_name, channel_id, self._adapters) and the
similar block later are normalized to the project's style so the string
concatenation/line breaks match the formatter expectations. After formatting,
re-run the linter/format check to confirm the drift is resolved.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds initialization logs to the GitHub and Google Chat adapters and enhances error messages in the channel and thread methods of chat.py by listing all registered adapters when a lookup fails. The reviewer suggested refactoring the duplicated error-raising logic into a private helper method to improve maintainability and reduce code duplication.
| if adapter is None: | ||
| raise ChatError(f'Adapter "{adapter_name}" not found for channel ID "{channel_id}"') | ||
| registered = sorted(self._adapters.keys()) | ||
| raise ChatError( | ||
| f'Adapter "{adapter_name}" not found for channel ID "{channel_id}" ' | ||
| f"(registered adapters: {registered})" | ||
| ) |
There was a problem hiding this comment.
This logic for raising an error when an adapter is not found is duplicated in the thread method. To improve maintainability and avoid code duplication, consider extracting this into a private helper method. You will also need to import NoReturn from typing.
| if adapter is None: | |
| raise ChatError(f'Adapter "{adapter_name}" not found for channel ID "{channel_id}"') | |
| registered = sorted(self._adapters.keys()) | |
| raise ChatError( | |
| f'Adapter "{adapter_name}" not found for channel ID "{channel_id}" ' | |
| f"(registered adapters: {registered})" | |
| ) | |
| if adapter is None: | |
| self._raise_adapter_not_found(adapter_name, "channel", channel_id) |
| if adapter is None: | ||
| raise ChatError(f'Adapter "{adapter_name}" not found for thread ID "{thread_id}"') | ||
| registered = sorted(self._adapters.keys()) | ||
| raise ChatError( | ||
| f'Adapter "{adapter_name}" not found for thread ID "{thread_id}" ' | ||
| f"(registered adapters: {registered})" | ||
| ) |
There was a problem hiding this comment.
As mentioned in my other comment, you can use the new _raise_adapter_not_found helper method here to reduce code duplication.
| if adapter is None: | |
| raise ChatError(f'Adapter "{adapter_name}" not found for thread ID "{thread_id}"') | |
| registered = sorted(self._adapters.keys()) | |
| raise ChatError( | |
| f'Adapter "{adapter_name}" not found for thread ID "{thread_id}" ' | |
| f"(registered adapters: {registered})" | |
| ) | |
| if adapter is None: | |
| self._raise_adapter_not_found(adapter_name, "thread", thread_id) |
|
Replaced by #104 (could not push merge-conflict resolution to the fork branch from an automated session). All original commits preserved in #104's branch history; @tony-chinchill-ai credited. Generated by Claude Code |
…ers in not-found error (#104) Operator-observability improvements driven by a 2026-05-19 chinchill-api incident. - GoogleChatAdapter.initialize() now logs "Google Chat adapter initialized" matching Slack/Teams - GitHubAdapter.initialize() now logs "GitHub adapter initialized" unconditionally — the existing "GitHub auth completed" log only fires when _auth_token is set, so App-credential deployments had no init signal - Chat.channel() and Chat.thread() not-found errors now append "(registered adapters: [...])" so operators can disambiguate "adapter never constructed in this process" from "constructed but lookup name wrong" Original authorship: @tony-chinchill-ai (commit a298c2f preserved in merge history). Replaces #96 after conflict resolution with #90's chat.get_user method.
2 participants
Summary
GoogleChatAdapter.initialize()now logsGoogle Chat adapter initializedmatching the existing Slack/Teams pattern. Today only Slack and Teams emit per-adapter init lines; gchat and GitHub are silent, so operators can't see at a glance whether each adapter actually came up.GitHubAdapter.initialize()now logsGitHub adapter initializedunconditionally. The existingGitHub auth completedlog only fires when_auth_tokenis set and the user-info fetch succeeds, so multi-tenant deployments (which use App credentials, not_auth_token) had no init signal.Chat.channel()andChat.thread()not-found errors now append(registered adapters: [...])so operators can disambiguate "the adapter was never constructed in this process" from "constructed but the lookup name is wrong".Why now
Driven by a chinchill-api incident on 2026-05-19: a DBOS worker hit
Adapter "github" not found for thread ID "github:tony-chinchill-ai/test:issue:1"with no other diagnostic context. With these changes the same error becomesAdapter "github" not found for thread ID "github:..." (registered adapters: ['slack', 'teams']), immediately pointing at the GitHub-adapter construction path rather than the lookup path.Test plan
pytest(no behavioral changes; only added log calls + error-message string).[gchat] Google Chat adapter initializedand[github] GitHub adapter initializedlines.Adapter not founderror and confirm the registered-adapter list appears in the message.🤖 Generated with Claude Code
Summary by CodeRabbit